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

webrtc/media/engine/webrtcvideoengine2.cc

 /*
  *  Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 852 matching lines @@
              "codec or RTCP mode has changed.";
       for (auto& kv : receive_streams_) {
         RTC_DCHECK(kv.second != nullptr);
         kv.second->SetFeedbackParameters(
             HasNack(send_codec_->codec), HasRemb(send_codec_->codec),
             HasTransportCc(send_codec_->codec),
             params.rtcp.reduced_size ? webrtc::RtcpMode::kReducedSize
                                      : 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;
 }
 
 webrtc::RtpParameters WebRtcVideoChannel2::GetRtpParameters(
     uint32_t ssrc) const {
   rtc::CritScope stream_lock(&stream_crit_);
   auto it = send_streams_.find(ssrc);
   if (it == send_streams_.end()) {
@@ skipping 258 matching lines @@
 }
 
 bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp) {
   return AddRecvStream(sp, false);
 }
 
 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;
 
   uint32_t ssrc = sp.first_ssrc();
   RTC_DCHECK(ssrc != 0);  // TODO(pbos): Is this ever valid?
 
   rtc::CritScope stream_lock(&stream_crit_);
   // Remove running stream if this was a default stream.
   const auto& prev_stream = receive_streams_.find(ssrc);
   if (prev_stream != receive_streams_.end()) {
@@ skipping 26 matching lines @@
 
   receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
       call_, sp, config, external_decoder_factory_, default_stream,
       recv_codecs_);
 
   return true;
 }
 
 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;
   config->rtp.local_ssrc = rtcp_receiver_report_ssrc_;
 
   config->rtp.extensions = recv_rtp_extensions_;
   // Whether or not the receive stream sends reduced size RTCP is determined
   // by the send params.
   // TODO(deadbeef): Once we change "send_params" to "sender_params" and
   // "recv_params" to "receiver_params", we should get this out of
   // receiver_params_.
   config->rtp.rtcp_mode = send_params_.rtcp.reduced_size
                               ? webrtc::RtcpMode::kReducedSize
                               : webrtc::RtcpMode::kCompound;
 
   // TODO(pbos): This protection is against setting the same local ssrc as
   // remote which is not permitted by the lower-level API. RTCP requires a
   // corresponding sender SSRC. Figure out what to do when we don't have
   // (receive-only) or know a good local SSRC.
   if (config->rtp.remote_ssrc == config->rtp.local_ssrc) {
     if (config->rtp.local_ssrc != kDefaultRtcpReceiverReportSsrc) {
       config->rtp.local_ssrc = kDefaultRtcpReceiverReportSsrc;
     } else {
       config->rtp.local_ssrc = kDefaultRtcpReceiverReportSsrc + 1;
     }
   }
 
   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) {
     uint32_t rtx_ssrc;
     if (recv_codecs_[i].rtx_payload_type != -1 &&
         sp.GetFidSsrc(ssrc, &rtx_ssrc)) {
       webrtc::VideoReceiveStream::Config::Rtp::Rtx& rtx =
           config->rtp.rtx[recv_codecs_[i].codec.id];
       rtx.ssrc = rtx_ssrc;
       rtx.payload_type = recv_codecs_[i].rtx_payload_type;
@@ skipping 219 matching lines @@
   // due to lack of socket buffer space, although it's not yet clear what the
   // ideal value should be.
   MediaChannel::SetOption(NetworkInterface::ST_RTP,
                           rtc::Socket::OPT_SNDBUF,
                           kVideoRtpBufferSize);
 }
 
 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;
   return MediaChannel::SendPacket(&packet, rtc_options);
 }
 
 bool WebRtcVideoChannel2::SendRtcp(const uint8_t* data, size_t len) {
   rtc::CopyOnWriteBuffer packet(data, len, kMaxRtpPacketLen);
   return MediaChannel::SendRtcp(&packet, rtc::PacketOptions());
 }
@@ skipping 995 matching lines @@
 
   info.codec_name = GetCodecNameFromPayloadType(stats.current_payload_type);
 
   info.firs_sent = stats.rtcp_packet_type_counts.fir_packets;
   info.plis_sent = stats.rtcp_packet_type_counts.pli_packets;
   info.nacks_sent = stats.rtcp_packet_type_counts.nack_packets;
 
   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) {}
 
 bool WebRtcVideoChannel2::VideoCodecSettings::operator==(
     const WebRtcVideoChannel2::VideoCodecSettings& other) const {
   return codec == other.codec &&
          fec.ulpfec_payload_type == other.fec.ulpfec_payload_type &&
          fec.red_payload_type == other.fec.red_payload_type &&
          fec.red_rtx_payload_type == other.fec.red_rtx_payload_type &&
          rtx_payload_type == other.rtx_payload_type;
@@ skipping 66 matching lines @@
   }
 
   // One of these codecs should have been a video codec. Only having FEC
   // parameters into this code is a logic error.
   RTC_DCHECK(!video_codecs.empty());
 
   for (std::map<int, int>::const_iterator it = rtx_mapping.begin();
        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 &&
         payload_codec_type[it->first] != VideoCodec::CODEC_RED) {
       LOG(LS_ERROR) << "RTX not mapped to regular video codec or RED codec.";
       return std::vector<VideoCodecSettings>();
     }
 
     if (it->first == fec_settings.red_payload_type) {
       fec_settings.red_rtx_payload_type = it->second;
     }
   }
 
   for (size_t i = 0; i < video_codecs.size(); ++i) {
     video_codecs[i].fec = fec_settings;
     if (rtx_mapping[video_codecs[i].codec.id] != 0 &&
         rtx_mapping[video_codecs[i].codec.id] !=
             fec_settings.red_payload_type) {
       video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
     }
   }
 
   return video_codecs;
 }
 
 }  // namespace cricket