Protected streams report RTP messages directly to the FlexFec streams

webrtc/video/rtp_video_stream_receiver.cc

Index: webrtc/video/rtp_video_stream_receiver.cc
diff --git a/webrtc/video/rtp_video_stream_receiver.cc b/webrtc/video/rtp_video_stream_receiver.cc
index 4204670c7eb5094278a093c3b3867d92c26d26ca..3c40d3c95600b8eebaf08c937d841fe566a85253 100644
--- a/webrtc/video/rtp_video_stream_receiver.cc
+++ b/webrtc/video/rtp_video_stream_receiver.cc
@@ -10,6 +10,7 @@
 
 #include "webrtc/video/rtp_video_stream_receiver.h"
 
+#include <algorithm>
 #include <utility>
 #include <vector>
 
@@ -191,6 +192,8 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
 }
 
 RtpVideoStreamReceiver::~RtpVideoStreamReceiver() {
+  RTC_DCHECK(secondary_sinks_.empty());
+
   if (nack_module_) {
     process_thread_->DeRegisterModule(nack_module_.get());
   }
@@ -308,6 +311,8 @@ int32_t RtpVideoStreamReceiver::OnInitializeDecoder(
 // This method handles both regular RTP packets and packets recovered
 // via FlexFEC.
 void RtpVideoStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) {
+  RTC_DCHECK_RUN_ON(&worker_thread_checker_);

[eladalon, 2017/08/01 08:19:38]

This will not work for video_loopback, because DirectTransport creates its own
thread, "NetworkProcess". I don't want to put a lock in actual code just to get
rid of a problem in tests, and I don't want to remove this check, either.
Refactoring DirectTransport would likely not be an easy task. Any suggestions?
The simplest I can think of is checking for (ON-WORKER-THREAD ||
IN-SPECIFIC-BUILDS-LIST), where the latter would whitelist the loopback,
unit-tests, etc. Wdyt?
+danilchap@, +stefan@, +solenberg@

[brandtr, 2017/08/01 08:34:15]

Nice! This addition makes the code faster to read.

+
   {
     rtc::CritScope lock(&receive_cs_);
     if (!receiving_) {
@@ -361,6 +366,10 @@ void RtpVideoStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) {
     rtp_receive_statistics_->IncomingPacket(
         header, packet.size(), IsPacketRetransmitted(header, in_order));
   }
+
+  for (auto* secondary_sink : secondary_sinks_) {

[brandtr, 2017/08/01 08:34:15]

RtpPacketSinkInterface* ?

[eladalon, 2017/08/01 09:30:28]

Done.

+    secondary_sink->OnRtpPacket(packet);
+  }
 }
 
 int32_t RtpVideoStreamReceiver::RequestKeyFrame() {
@@ -428,6 +437,27 @@ rtc::Optional<int64_t> RtpVideoStreamReceiver::LastReceivedKeyframePacketMs()
   return packet_buffer_->LastReceivedKeyframePacketMs();
 }
 
+void RtpVideoStreamReceiver::AddSecondarySink(RtpPacketSinkInterface* sink) {
+  RTC_DCHECK_RUN_ON(&worker_thread_checker_);
+  RTC_DCHECK(std::find(secondary_sinks_.cbegin(), secondary_sinks_.cend(),
+                       sink) == secondary_sinks_.cend());
+  secondary_sinks_.push_back(sink);
+}
+
+void RtpVideoStreamReceiver::RemoveSecondarySink(
+    const RtpPacketSinkInterface* sink) {
+  RTC_DCHECK_RUN_ON(&worker_thread_checker_);
+  auto it = std::find(secondary_sinks_.begin(), secondary_sinks_.end(), sink);
+  if (it == secondary_sinks_.end()) {
+    // We might be rolling-back a call whose setup failed mid-way. In such a
+    // case, it's simpler to remove "everything" rather than remember what
+    // has already been added.
+    LOG(LS_WARNING) << "Removal of unknown sink.";
+    return;
+  }
+  secondary_sinks_.erase(it);
+}
+
 void RtpVideoStreamReceiver::ReceivePacket(const uint8_t* packet,
                                            size_t packet_length,
                                            const RTPHeader& header,