Protected streams report RTP messages directly to the FlexFec streams

webrtc/video/rtp_video_stream_receiver_unittest.cc

Index: webrtc/video/rtp_video_stream_receiver_unittest.cc
diff --git a/webrtc/video/rtp_video_stream_receiver_unittest.cc b/webrtc/video/rtp_video_stream_receiver_unittest.cc
index 3e94e53294a5f105c612154fd460d27c5d318d04..43492f06117b11eabf358612d893756d0bcbd5d1 100644
--- a/webrtc/video/rtp_video_stream_receiver_unittest.cc
+++ b/webrtc/video/rtp_video_stream_receiver_unittest.cc
@@ -14,6 +14,7 @@
 #include "webrtc/common_video/h264/h264_common.h"
 #include "webrtc/media/base/mediaconstants.h"
 #include "webrtc/modules/pacing/packet_router.h"
+#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h"
 #include "webrtc/modules/utility/include/process_thread.h"
 #include "webrtc/modules/video_coding/frame_object.h"
 #include "webrtc/modules/video_coding/include/video_coding_defines.h"
@@ -22,6 +23,7 @@
 #include "webrtc/modules/video_coding/timing.h"
 #include "webrtc/rtc_base/bytebuffer.h"
 #include "webrtc/rtc_base/logging.h"
+#include "webrtc/rtc_base/ptr_util.h"
 #include "webrtc/system_wrappers/include/clock.h"
 #include "webrtc/system_wrappers/include/field_trial_default.h"
 #include "webrtc/test/field_trial.h"
@@ -93,6 +95,25 @@ class MockOnCompleteFrameCallback
   rtc::ByteBufferWriter buffer_;
 };
 
+class MockRtpPacketSink : public RtpPacketSinkInterface {
+ public:
+  MOCK_METHOD1(OnRtpPacket, void(const RtpPacketReceived&));
+};
+
+std::unique_ptr<RtpPacketReceived> CreateRtpPacketReceived(
+    uint32_t ssrc,
+    uint16_t sequence_number) {
+  auto packet = rtc::MakeUnique<RtpPacketReceived>();
+  packet->SetSsrc(ssrc);
+  packet->SetSequenceNumber(sequence_number);
+  return packet;
+}
+
+MATCHER_P(SamePacketAs, other, "") {
+  return arg.Ssrc() == other.Ssrc() &&
+         arg.SequenceNumber() == other.SequenceNumber();
+}
+
 }  // namespace
 
 class RtpVideoStreamReceiverTest : public testing::Test {
@@ -344,4 +365,87 @@ TEST_F(RtpVideoStreamReceiverTest, RequestKeyframeIfFirstFrameIsDelta) {
                                                     &rtp_header);
 }
 
+TEST_F(RtpVideoStreamReceiverTest, SecondarySinksGetRtpNotifications) {
+  rtp_video_stream_receiver_->StartReceive();
+
+  MockRtpPacketSink secondary_sink_1;
+  MockRtpPacketSink secondary_sink_2;
+
+  rtp_video_stream_receiver_->AddSecondarySink(&secondary_sink_1);
+  rtp_video_stream_receiver_->AddSecondarySink(&secondary_sink_2);
+
+  constexpr uint32_t ssrc = 111;
+  constexpr size_t sequence_number = 222;
+  auto rtp_packet = CreateRtpPacketReceived(ssrc, sequence_number);
+  EXPECT_CALL(secondary_sink_1, OnRtpPacket(SamePacketAs(*rtp_packet)));
+  EXPECT_CALL(secondary_sink_2, OnRtpPacket(SamePacketAs(*rtp_packet)));
+
+  rtp_video_stream_receiver_->OnRtpPacket(*rtp_packet);
+
+  // Test tear-down.
+  rtp_video_stream_receiver_->StopReceive();
+  rtp_video_stream_receiver_->RemoveSecondarySink(&secondary_sink_1);
+  rtp_video_stream_receiver_->RemoveSecondarySink(&secondary_sink_2);
+}
+
+TEST_F(RtpVideoStreamReceiverTest, RemovedSecondarySinksGetNoRtpNotifications) {
+  rtp_video_stream_receiver_->StartReceive();
+
+  MockRtpPacketSink secondary_sink;
+
+  rtp_video_stream_receiver_->AddSecondarySink(&secondary_sink);
+  rtp_video_stream_receiver_->RemoveSecondarySink(&secondary_sink);
+
+  constexpr uint32_t ssrc = 111;
+  constexpr size_t sequence_number = 222;
+  auto rtp_packet = CreateRtpPacketReceived(ssrc, sequence_number);
+
+  EXPECT_CALL(secondary_sink, OnRtpPacket(SamePacketAs(*rtp_packet))).Times(0);
+
+  rtp_video_stream_receiver_->OnRtpPacket(*rtp_packet);
+
+  // Test tear-down.
+  rtp_video_stream_receiver_->StopReceive();
+}
+
+TEST_F(RtpVideoStreamReceiverTest,
+       OnlyRemovedSecondarySinksExcludedFromNotifications) {
+  rtp_video_stream_receiver_->StartReceive();
+
+  MockRtpPacketSink kept_secondary_sink;
+  MockRtpPacketSink removed_secondary_sink;
+
+  rtp_video_stream_receiver_->AddSecondarySink(&kept_secondary_sink);
+  rtp_video_stream_receiver_->AddSecondarySink(&removed_secondary_sink);
+  rtp_video_stream_receiver_->RemoveSecondarySink(&removed_secondary_sink);
+
+  constexpr uint32_t ssrc = 111;
+  constexpr size_t sequence_number = 222;
+  auto rtp_packet = CreateRtpPacketReceived(ssrc, sequence_number);
+  EXPECT_CALL(kept_secondary_sink, OnRtpPacket(SamePacketAs(*rtp_packet)));
+
+  rtp_video_stream_receiver_->OnRtpPacket(*rtp_packet);
+
+  // Test tear-down.
+  rtp_video_stream_receiver_->StopReceive();
+  rtp_video_stream_receiver_->RemoveSecondarySink(&kept_secondary_sink);
+}
+
+// TODO(eladalon): !!! Discuss - do we want to allow secondaries to be

[eladalon, 2017/07/24 14:19:00]

Danil, your thoughts?

[danilchap, 2017/07/24 15:02:07]

I do not have strong opinion here.
Probably not: seems that flag is explicitly designed for better
construction/destruction of the stream, so should be fine to ignore packets
then.
But may be secondary sinks should have own way to deal with that periods.

[eladalon, 2017/07/24 15:53:55]

Acknowledged.

+// notified of packets before the primary got StartReceive() called?
+// Add an appropriate test.
+
+#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
+TEST_F(RtpVideoStreamReceiverTest, RepeatedSecondarySinkDisallowed) {
+  MockRtpPacketSink secondary_sink;
+
+  rtp_video_stream_receiver_->AddSecondarySink(&secondary_sink);
+  EXPECT_DEATH(rtp_video_stream_receiver_->AddSecondarySink(&secondary_sink),
+               "");
+
+  // Test tear-down.
+  rtp_video_stream_receiver_->RemoveSecondarySink(&secondary_sink);
+}
+#endif
+
 }  // namespace webrtc