Don't do reconfiguration if recv codec order/preference changes

talk/media/webrtc/webrtcvideoengine2.cc

 /*
  * libjingle
  * Copyright 2014 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 838 matching lines @@
   for (size_t i = 0; i < mapped_codecs.size(); ++i) {
     const VideoCodecSettings& codec = mapped_codecs[i];
     if (CodecIsInternallySupported(codec.codec.name) ||
         CodecIsExternallySupported(codec.codec.name)) {
       supported_codecs.push_back(codec);
     }
   }
   return supported_codecs;
 }
 
+bool WebRtcVideoChannel2::CodecsHaveChanged(
+    std::vector<VideoCodecSettings> before,
+    std::vector<VideoCodecSettings> after) {
+  if (before.size() != after.size())
+    return true;

[pthatcher1, 2015/08/18 20:24:32]

{}s please

[Taylor Brandstetter, 2015/08/20 23:07:47]

Done.

+  // The receive codec order doesn't matter, so we sort the codecs before
+  // comparing. This is necessary because currently the
+  // only way to change the send codec is to munge SDP, which causes
+  // the receive codec list to change order, which causes the streams
+  // to be recreates which causes a "blink" of black video.  In order
+  // to support munging the SDP in this way without recreating receive
+  // streams, we ignore the order of the received codecs so that
+  // changing the order doesn't cause this "blink".
+  auto comparison =
+      [](const VideoCodecSettings& codec1, const VideoCodecSettings& codec2) {
+        return codec1.codec.id > codec2.codec.id;
+      };
+  std::sort(before.begin(), before.end(), comparison);
+  std::sort(after.begin(), after.end(), comparison);
+  for (size_t i = 0; i < before.size(); ++i) {
+    // For the same reason that we sort the codecs, we also ignore the
+    // preference.  We don't want a preference change on the receive
+    // side to cause recreation of the stream.
+    before[i].codec.preference = 0;
+    after[i].codec.preference = 0;
+    if (before[i] != after[i]) {
+      return true;
+    }
+  }
+  return false;
+}
+
 bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) {
   // TODO(pbos): Refactor this to only recreate the send streams once
   // instead of 4 times.
   return (SetSendCodecs(params.codecs) &&
           SetSendRtpHeaderExtensions(params.extensions) &&
           SetMaxSendBandwidth(params.max_bandwidth_bps) &&
           SetOptions(params.options));
 }
 
 bool WebRtcVideoChannel2::SetRecvParameters(const VideoRecvParameters& params) {
   // TODO(pbos): Refactor this to only recreate the recv streams once
   // instead of twice.
   return (SetRecvCodecs(params.codecs) &&
           SetRecvRtpHeaderExtensions(params.extensions));
 }
 
+std::string WebRtcVideoChannel2::CodecSettingsVectorToString(
+    const std::vector<VideoCodecSettings>& codecs) {
+  std::stringstream out;
+  out << '{';
+  for (size_t i = 0; i < codecs.size(); ++i) {
+    out << codecs[i].codec.ToString();
+    if (i != codecs.size() - 1) {
+      out << ", ";
+    }
+  }
+  out << '}';
+  return out.str();
+}
+
 bool WebRtcVideoChannel2::SetRecvCodecs(const std::vector<VideoCodec>& codecs) {
   TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetRecvCodecs");
   LOG(LS_INFO) << "SetRecvCodecs: " << CodecVectorToString(codecs);
   if (!ValidateCodecFormats(codecs)) {
     return false;
   }
 
   const std::vector<VideoCodecSettings> mapped_codecs = MapCodecs(codecs);
   if (mapped_codecs.empty()) {
     LOG(LS_ERROR) << "SetRecvCodecs called without any video codecs.";
     return false;
   }
 
-  const std::vector<VideoCodecSettings> supported_codecs =
+  std::vector<VideoCodecSettings> supported_codecs =
       FilterSupportedCodecs(mapped_codecs);
 
   if (mapped_codecs.size() != supported_codecs.size()) {
     LOG(LS_ERROR) << "SetRecvCodecs called with unsupported video codecs.";
     return false;
   }
 
   // Prevent reconfiguration when setting identical receive codecs.
-  if (recv_codecs_.size() == supported_codecs.size()) {
-    bool reconfigured = false;
-    for (size_t i = 0; i < supported_codecs.size(); ++i) {
-      if (recv_codecs_[i] != supported_codecs[i]) {
-        reconfigured = true;
-        break;
-      }
-    }
-    if (!reconfigured)
-      return true;
+  if (!CodecsHaveChanged(recv_codecs_, supported_codecs)) {
+    LOG(LS_INFO)
+        << "Ignoring call to SetRecvCodecs because codecs haven't changed.";
+    return true;
   }
 
+  LOG(LS_INFO) << "Changing recv codecs from "
+               << CodecSettingsVectorToString(recv_codecs_) << " to "
+               << CodecSettingsVectorToString(supported_codecs);
   recv_codecs_ = supported_codecs;
 
   rtc::CritScope stream_lock(&stream_crit_);
   for (std::map<uint32, WebRtcVideoReceiveStream*>::iterator it =
            receive_streams_.begin();
        it != receive_streams_.end();
        ++it) {
     it->second->SetRecvCodecs(recv_codecs_);
   }
 
@@ skipping 12 matching lines @@
 
   if (supported_codecs.empty()) {
     LOG(LS_ERROR) << "No video codecs supported.";
     return false;
   }
 
   LOG(LS_INFO) << "Using codec: " << supported_codecs.front().codec.ToString();
 
   VideoCodecSettings old_codec;
   if (send_codec_.Get(&old_codec) && supported_codecs.front() == old_codec) {
+    LOG(LS_INFO) << "Ignore call to SetSendCodecs because first supported "
+                    "codec hasn't changed.";
     // Using same codec, avoid reconfiguring.
     return true;
   }
 
   send_codec_.Set(supported_codecs.front());
 
   rtc::CritScope stream_lock(&stream_crit_);
+  LOG(LS_INFO) << "Change the send codec because SetSendCodecs has a different "
+                  "first supported codec.";
   for (auto& kv : send_streams_) {
     DCHECK(kv.second != nullptr);
     kv.second->SetCodec(supported_codecs.front());
   }
+  LOG(LS_INFO) << "SetNackAndRemb on all the receive streams because the send "
+                  "codec has changed.";
   for (auto& kv : receive_streams_) {
     DCHECK(kv.second != nullptr);
     kv.second->SetNackAndRemb(HasNack(supported_codecs.front().codec),
                               HasRemb(supported_codecs.front().codec));
   }
 
   // TODO(holmer): Changing the codec parameters shouldn't necessarily mean that
   // we change the min/max of bandwidth estimation. Reevaluate this.
   VideoCodec codec = supported_codecs.front().codec;
   int bitrate_kbps;
@@ skipping 107 matching lines @@
                                 send_codec_,
                                 sp,
                                 send_rtp_extensions_);
 
   uint32 ssrc = sp.first_ssrc();
   DCHECK(ssrc != 0);
   send_streams_[ssrc] = stream;
 
   if (rtcp_receiver_report_ssrc_ == kDefaultRtcpReceiverReportSsrc) {
     rtcp_receiver_report_ssrc_ = ssrc;
+    LOG(LS_INFO) << "SetLocalSsrc on all the receive streams because we added "
+                    "a send stream.";
     for (auto& kv : receive_streams_)
       kv.second->SetLocalSsrc(ssrc);
   }
   if (default_send_ssrc_ == 0) {
     default_send_ssrc_ = ssrc;
   }
   if (sending_) {
     stream->Start();
   }
 
@@ skipping 369 matching lines @@
 bool WebRtcVideoChannel2::SetRecvRtpHeaderExtensions(
     const std::vector<RtpHeaderExtension>& extensions) {
   TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetRecvRtpHeaderExtensions");
   LOG(LS_INFO) << "SetRecvRtpHeaderExtensions: "
                << RtpExtensionsToString(extensions);
   if (!ValidateRtpHeaderExtensionIds(extensions))
     return false;
 
   std::vector<webrtc::RtpExtension> filtered_extensions =
       FilterRtpExtensions(extensions);
-  if (!RtpExtensionsHaveChanged(recv_rtp_extensions_, filtered_extensions))
+  if (!RtpExtensionsHaveChanged(recv_rtp_extensions_, filtered_extensions)) {
+    LOG(LS_INFO) << "Ignoring call to SetRecvRtpHeaderExtensions because "
+                    "header extensions haven't changed.";
     return true;
+  }
 
   recv_rtp_extensions_ = filtered_extensions;
 
   rtc::CritScope stream_lock(&stream_crit_);
   for (std::map<uint32, WebRtcVideoReceiveStream*>::iterator it =
            receive_streams_.begin();
        it != receive_streams_.end();
        ++it) {
     it->second->SetRtpExtensions(recv_rtp_extensions_);
   }
   return true;
 }
 
 bool WebRtcVideoChannel2::SetSendRtpHeaderExtensions(
     const std::vector<RtpHeaderExtension>& extensions) {
   TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetSendRtpHeaderExtensions");
   LOG(LS_INFO) << "SetSendRtpHeaderExtensions: "
                << RtpExtensionsToString(extensions);
   if (!ValidateRtpHeaderExtensionIds(extensions))
     return false;
 
   std::vector<webrtc::RtpExtension> filtered_extensions =
       FilterRtpExtensions(extensions);
-  if (!RtpExtensionsHaveChanged(send_rtp_extensions_, filtered_extensions))
+  if (!RtpExtensionsHaveChanged(send_rtp_extensions_, filtered_extensions)) {
+    LOG(LS_INFO) << "Ignoring call to SetSendRtpHeaderExtensions because "
+                    "header extensions haven't changed.";
     return true;
+  }
 
   send_rtp_extensions_ = filtered_extensions;
 
   const webrtc::RtpExtension* cvo_extension = FindHeaderExtension(
       send_rtp_extensions_, kRtpVideoRotationHeaderExtension);
 
   rtc::CritScope stream_lock(&stream_crit_);
   for (std::map<uint32, WebRtcVideoSendStream*>::iterator it =
            send_streams_.begin();
        it != send_streams_.end();
@@ skipping 368 matching lines @@
     return;
 
   capturer_->SetApplyRotation(apply_rotation);
 }
 
 void WebRtcVideoChannel2::WebRtcVideoSendStream::SetOptions(
     const VideoOptions& options) {
   rtc::CritScope cs(&lock_);
   VideoCodecSettings codec_settings;
   if (parameters_.codec_settings.Get(&codec_settings)) {
+    LOG(LS_INFO) << "SetCodecAndOptions because of SetOptions; options="
+                 << options.ToString();
     SetCodecAndOptions(codec_settings, options);
   } else {
     parameters_.options = options;
   }
 }
 
 void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodec(
     const VideoCodecSettings& codec_settings) {
   rtc::CritScope cs(&lock_);
+  LOG(LS_INFO) << "SetCodecAndOptions because of SetCodec.";
   SetCodecAndOptions(codec_settings, parameters_.options);
 }
 
 webrtc::VideoCodecType CodecTypeFromName(const std::string& name) {
   if (CodecNamesEq(name, kVp8CodecName)) {
     return webrtc::kVideoCodecVP8;
   } else if (CodecNamesEq(name, kVp9CodecName)) {
     return webrtc::kVideoCodecVP9;
   } else if (CodecNamesEq(name, kH264CodecName)) {
     return webrtc::kVideoCodecH264;
@@ skipping 76 matching lines @@
 
   parameters_.config.rtp.nack.rtp_history_ms =
       HasNack(codec_settings.codec) ? kNackHistoryMs : 0;
 
   options.suspend_below_min_bitrate.Get(
       &parameters_.config.suspend_below_min_bitrate);
 
   parameters_.codec_settings.Set(codec_settings);
   parameters_.options = options;
 
+  LOG(LS_INFO)
+      << "RecreateWebRtcStream (send) because of SetCodecAndOptions; options="
+      << options.ToString();
   RecreateWebRtcStream();
   if (allocated_encoder_.encoder != new_encoder.encoder) {
     DestroyVideoEncoder(&allocated_encoder_);
     allocated_encoder_ = new_encoder;
   }
 }
 
 void WebRtcVideoChannel2::WebRtcVideoSendStream::SetRtpExtensions(
     const std::vector<webrtc::RtpExtension>& rtp_extensions) {
   rtc::CritScope cs(&lock_);
   parameters_.config.rtp.extensions = rtp_extensions;
-  if (stream_ != nullptr)
+  if (stream_ != nullptr) {
+    LOG(LS_INFO) << "RecreateWebRtcStream (send) because of SetRtpExtensions";
     RecreateWebRtcStream();
+  }
 }
 
 webrtc::VideoEncoderConfig
 WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig(
     const Dimensions& dimensions,
     const VideoCodec& codec) const {
   webrtc::VideoEncoderConfig encoder_config;
   if (dimensions.is_screencast) {
     int screencast_min_bitrate_kbps;
     parameters_.options.screencast_min_bitrate.Get(
@@ skipping 259 matching lines @@
       default_stream_(default_stream),
       config_(config),
       external_decoder_factory_(external_decoder_factory),
       renderer_(NULL),
       last_width_(-1),
       last_height_(-1),
       first_frame_timestamp_(-1),
       estimated_remote_start_ntp_time_ms_(0) {
   config_.renderer = this;
   // SetRecvCodecs will also reset (start) the VideoReceiveStream.
+  LOG(LS_INFO) << "SetRecvCodecs (recv) because we are creating the receive "
+                  "stream for the first time: "
+               << CodecSettingsVectorToString(recv_codecs);
   SetRecvCodecs(recv_codecs);
 }
 
 WebRtcVideoChannel2::WebRtcVideoReceiveStream::AllocatedDecoder::
     AllocatedDecoder(webrtc::VideoDecoder* decoder,
                      webrtc::VideoCodecType type,
                      bool external)
     : decoder(decoder),
       external_decoder(nullptr),
       type(type),
@@ skipping 75 matching lines @@
     decoder.payload_name = recv_codecs[i].codec.name;
     config_.decoders.push_back(decoder);
   }
 
   // TODO(pbos): Reconfigure RTX based on incoming recv_codecs.
   config_.rtp.fec = recv_codecs.front().fec;
   config_.rtp.nack.rtp_history_ms =
       HasNack(recv_codecs.begin()->codec) ? kNackHistoryMs : 0;
 
   ClearDecoders(&old_decoders);
+  LOG(LS_INFO) << "RecreateWebRtcStream (recv) because of SetRecvCodecs: "
+               << CodecSettingsVectorToString(recv_codecs);
   RecreateWebRtcStream();
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetLocalSsrc(
     uint32_t local_ssrc) {
   // TODO(pbos): Consider turning this sanity check into a DCHECK. You should
   // not be able to create a sender with the same SSRC as a receiver, but right
   // now this can't be done due to unittests depending on receiving what they
   // are sending from the same MediaChannel.
-  if (local_ssrc == config_.rtp.remote_ssrc)
+  if (local_ssrc == config_.rtp.remote_ssrc) {
+    LOG(LS_INFO) << "Ignoring call to SetLocalSsrc because parameters are "
+                    "unchanged; local_ssrc=" << local_ssrc;
     return;
+  }
 
   config_.rtp.local_ssrc = local_ssrc;
+  LOG(LS_INFO)
+      << "RecreateWebRtcStream (recv) because of SetLocalSsrc; local_ssrc="
+      << local_ssrc;
   RecreateWebRtcStream();
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetNackAndRemb(
     bool nack_enabled, bool remb_enabled) {
   int nack_history_ms = nack_enabled ? kNackHistoryMs : 0;
   if (config_.rtp.nack.rtp_history_ms == nack_history_ms &&
       config_.rtp.remb == remb_enabled) {
+    LOG(LS_INFO) << "Ignoring call to SetNackAndRemb because parameters are "
+                    "unchanged; nack=" << nack_enabled
+                 << ", remb=" << remb_enabled;
     return;
   }
   config_.rtp.remb = remb_enabled;
   config_.rtp.nack.rtp_history_ms = nack_history_ms;
+  LOG(LS_INFO) << "RecreateWebRtcStream (recv) because of SetNackAndRemb; nack="
+               << nack_enabled << ", remb=" << remb_enabled;
   RecreateWebRtcStream();
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRtpExtensions(
     const std::vector<webrtc::RtpExtension>& extensions) {
   config_.rtp.extensions = extensions;
+  LOG(LS_INFO) << "RecreateWebRtcStream (recv) because of SetRtpExtensions";
   RecreateWebRtcStream();
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() {
   if (stream_ != NULL) {
     call_->DestroyVideoReceiveStream(stream_);
   }
   stream_ = call_->CreateVideoReceiveStream(config_);
   stream_->Start();
 }
@@ skipping 223 matching lines @@
       video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
     }
   }
 
   return video_codecs;
 }
 
 }  // namespace cricket
 
 #endif  // HAVE_WEBRTC_VIDEO