Protected streams report RTP messages directly to the FlexFec streams

webrtc/media/engine/webrtcvideoengine.cc

Index: webrtc/media/engine/webrtcvideoengine.cc
diff --git a/webrtc/media/engine/webrtcvideoengine.cc b/webrtc/media/engine/webrtcvideoengine.cc
index b6447329e265519da1f19d29f1227006feeff8c0..5b61ede8fd1bce6cbb60c58edb987aedbaa97709 100644
--- a/webrtc/media/engine/webrtcvideoengine.cc
+++ b/webrtc/media/engine/webrtcvideoengine.cc
@@ -2154,6 +2154,7 @@ WebRtcVideoChannel::WebRtcVideoReceiveStream::AllocatedDecoder::
 
 WebRtcVideoChannel::WebRtcVideoReceiveStream::~WebRtcVideoReceiveStream() {
   if (flexfec_stream_) {
+    MaybeDissociateFlexfecFromVideo();
     call_->DestroyFlexfecReceiveStream(flexfec_stream_);
   }
   call_->DestroyVideoReceiveStream(stream_);
@@ -2334,24 +2335,42 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetRecvParameters(
 void WebRtcVideoChannel::WebRtcVideoReceiveStream::
     RecreateWebRtcVideoStream() {
   if (stream_) {
+    MaybeDissociateFlexfecFromVideo();
     call_->DestroyVideoReceiveStream(stream_);
     stream_ = nullptr;
   }
   webrtc::VideoReceiveStream::Config config = config_.Copy();
   config.rtp.protected_by_flexfec = (flexfec_stream_ != nullptr);
   stream_ = call_->CreateVideoReceiveStream(std::move(config));
+  MaybeAssociateFlexfecFromVideo();
   stream_->Start();
 }
 
 void WebRtcVideoChannel::WebRtcVideoReceiveStream::
     MaybeRecreateWebRtcFlexfecStream() {
   if (flexfec_stream_) {
+    MaybeDissociateFlexfecFromVideo();
     call_->DestroyFlexfecReceiveStream(flexfec_stream_);
     flexfec_stream_ = nullptr;
   }
   if (flexfec_config_.IsCompleteAndEnabled()) {
     flexfec_stream_ = call_->CreateFlexfecReceiveStream(flexfec_config_);
     flexfec_stream_->Start();
+    MaybeAssociateFlexfecFromVideo();
+  }
+}
+
+void WebRtcVideoChannel::WebRtcVideoReceiveStream::
+    MaybeAssociateFlexfecFromVideo() {
+  if (stream_ && flexfec_stream_) {
+    stream_->AddSecondarySink(flexfec_stream_);
+  }
+}
+
+void WebRtcVideoChannel::WebRtcVideoReceiveStream::
+    MaybeDissociateFlexfecFromVideo() {
+  if (stream_ && flexfec_stream_) {
+    stream_->RemoveSecondarySink(flexfec_stream_);
   }
 }

[Taylor Brandstetter, 2017/07/31 15:59:25]

Would it be feasible to cover this with a basic test or couple of tests in
webrtcvideoengine_unittest.cc?

[eladalon, 2017/07/31 16:46:03]

I will look into this tomorrow; pushing this batch of comments before I have an
answer to this because we're in different timezones, and I have a question
attached to one of the other comments.

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

This would be great to have, as the current VideoEngine unit tests only check
for the existence of the FlexfecReceiveStream object and the validity of the
corresponding FlexfecReceiveStream::Config.

Our perf and end-to-end tests in video/ do not involve the
WebRtcVideoEngine/WebRtcVideoChannel layer, so this is a place for potential
configuration bugs to appear in the future.