Potential fix for rtx/red issue where red is removed only from the remote description.

webrtc/media/engine/webrtcvideoengine2.cc

Index: webrtc/media/engine/webrtcvideoengine2.cc
diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc
index 383f29a9228250f4ab9fa171802e1da8f586ca19..c679b4e581bc873fb4c5cd4f99def624b8bc262c 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -870,6 +870,21 @@ bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) {
                                      : webrtc::RtcpMode::kCompound);
       }
     }
+    if (changed_params.codec) {
+      if (changed_params.codec->fec.red_payload_type == -1) {
+        red_enabled_remote_peer_ = rtc::Optional<bool>(false);
+        LOG(LS_ERROR) << "Codec changed to not have RED. Recv streams: "
+                      << receive_streams_.size() << " " << this;
+        for (auto& kv : receive_streams_) {
+          // In practice VideoChannel::SetRemoteContent appears to most of the
+          // time also call UpdateRemoteStreams, which recreates the receive
+          // streams. If that's always true this call isn't needed.
+          kv.second->DisableFec();
+        }
+      } else {
+        red_enabled_remote_peer_ = rtc::Optional<bool>(true);
+      }
+    }
   }
   send_params_ = params;
   return true;
@@ -1148,8 +1163,9 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
                                         bool default_stream) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
 
-  LOG(LS_INFO) << "AddRecvStream" << (default_stream ? " (default stream)" : "")
-               << ": " << sp.ToString();
+  LOG(LS_WARNING) << "AddRecvStream"

[pbos-webrtc, 2016/05/09 19:57:01]

INFO

[stefan-webrtc, 2016/05/16 08:49:55]

Done.

+                  << (default_stream ? " (default stream)" : "") << ": "
+                  << sp.ToString();
   if (!ValidateStreamParams(sp))
     return false;
 
@@ -1196,7 +1212,7 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
 
 void WebRtcVideoChannel2::ConfigureReceiverRtp(
     webrtc::VideoReceiveStream::Config* config,
-    const StreamParams& sp) const {
+    const StreamParams& sp) {
   uint32_t ssrc = sp.first_ssrc();
 
   config->rtp.remote_ssrc = ssrc;
@@ -1225,7 +1241,21 @@ void WebRtcVideoChannel2::ConfigureReceiverRtp(
   }
 
   for (size_t i = 0; i < recv_codecs_.size(); ++i) {
-    MergeFecConfig(recv_codecs_[i].fec, &config->rtp.fec);
+    if (red_enabled_remote_peer_.value_or(true)) {
+      LOG(LS_WARNING) << "Remote peer has red " << this;

[Taylor Brandstetter, 2016/05/09 18:40:36]

May want to make this log message look nicer (and the one below).

[pbos-webrtc, 2016/05/09 19:57:01]

INFO

[stefan-webrtc, 2016/05/16 08:49:55]

Done.

+
+      // TODO(holmer): Why is this merged if WebRtcVideoReceiveStream anyway
+      // uses recv_codecs_ to set up FEC, and then overwrites its config_?
+      MergeFecConfig(recv_codecs_[i].fec, &config->rtp.fec);

[Taylor Brandstetter, 2016/05/09 18:40:36]

If this is actually completely redundant, you might as well just remove it and
see if any tests break.

[stefan-webrtc, 2016/05/16 08:49:55]

Done.

+    } else {
+      LOG(LS_WARNING) << "Remote peer doesn't have red " << this;

[pbos-webrtc, 2016/05/09 19:57:00]

INFO

[stefan-webrtc, 2016/05/16 08:49:55]

Done.

+      config->rtp.fec.red_payload_type = -1;
+      config->rtp.fec.ulpfec_payload_type = -1;

[Taylor Brandstetter, 2016/05/09 18:40:36]

Dumb question, but it possible to use ulpfec without red?

[stefan-webrtc, 2016/05/16 08:49:55]

In theory, maybe, but we don't support it.

+      config->rtp.fec.red_rtx_payload_type = -1;
+      recv_codecs_[i].fec.red_payload_type = -1;
+      recv_codecs_[i].fec.ulpfec_payload_type = -1;
+      recv_codecs_[i].fec.red_rtx_payload_type = -1;
+    }
   }
 
   for (size_t i = 0; i < recv_codecs_.size(); ++i) {
@@ -1465,6 +1495,9 @@ void WebRtcVideoChannel2::SetInterface(NetworkInterface* iface) {
 bool WebRtcVideoChannel2::SendRtp(const uint8_t* data,
                                   size_t len,
                                   const webrtc::PacketOptions& options) {
+  static int i = 0;

[pbos-webrtc, 2016/05/09 19:57:00]

don't commit this ;p

[stefan-webrtc, 2016/05/16 08:49:55]

Acknowledged.

+  if (i++ % 100 == 5)
+    return true;

[Taylor Brandstetter, 2016/05/09 18:40:36]

Remember to remove this before submitting

[stefan-webrtc, 2016/05/16 08:49:55]

Acknowledged.

   rtc::CopyOnWriteBuffer packet(data, len, kMaxRtpPacketLen);
   rtc::PacketOptions rtc_options;
   rtc_options.packet_id = options.packet_id;
@@ -2480,6 +2513,10 @@ WebRtcVideoChannel2::WebRtcVideoReceiveStream::GetVideoReceiverInfo() {
   return info;
 }
 
+void WebRtcVideoChannel2::WebRtcVideoReceiveStream::DisableFec() {

[pbos-webrtc, 2016/05/09 19:57:01]

Can you re-enable FEC after this change by negotiation, or how does that work?

+  stream_->DisableFec();

[pbos-webrtc, 2016/05/09 19:57:01]

I think this method should instead recreate the stream without FEC enabled
instead of adding APIs below here for something temporary.

+}

[stefan-webrtc, 2016/05/09 13:32:44]

Will be removed

+
 WebRtcVideoChannel2::VideoCodecSettings::VideoCodecSettings()
     : rtx_payload_type(-1) {}
 
@@ -2566,7 +2603,7 @@ WebRtcVideoChannel2::MapCodecs(const std::vector<VideoCodec>& codecs) {
        it != rtx_mapping.end();
        ++it) {
     if (!payload_used[it->first]) {
-      LOG(LS_ERROR) << "RTX mapped to payload not in codec list.";
+      LOG(LS_ERROR) << "RTX mapped to payload not in codec list " << it->first;
       return std::vector<VideoCodecSettings>();
     }
     if (payload_codec_type[it->first] != VideoCodec::CODEC_VIDEO &&