Consolidate SetSendParameters into one setter.

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 225 matching lines @@
   if (!rtx_ssrcs.empty() && primary_ssrcs.size() != rtx_ssrcs.size()) {
     LOG(LS_ERROR)
         << "RTX SSRCs exist, but don't cover all SSRCs (unsupported): "
         << sp.ToString();
     return false;
   }
 
   return true;
 }
 
-inline const webrtc::RtpExtension* FindHeaderExtension(
+inline bool ContainsHeaderExtension(
     const std::vector<webrtc::RtpExtension>& extensions,
     const std::string& name) {
   for (const auto& kv : extensions) {
     if (kv.name == name) {
-      return &kv;
+      return true;
     }
   }
-  return NULL;
+  return false;
 }
 
 // Merges two fec configs and logs an error if a conflict arises
 // such that merging in different order would trigger a different output.
 static void MergeFecConfig(const webrtc::FecConfig& other,
                            webrtc::FecConfig* output) {
   if (other.ulpfec_payload_type != -1) {
     if (output->ulpfec_payload_type != -1 &&
         output->ulpfec_payload_type != other.ulpfec_payload_type) {
       LOG(LS_WARNING) << "Conflict merging ulpfec_payload_type configs: "
@@ skipping 275 matching lines @@
           encoder_factory->codecs())) {
     simulcast_encoder_factory_.reset(
         new WebRtcSimulcastEncoderFactory(encoder_factory));
     encoder_factory = simulcast_encoder_factory_.get();
   }
   external_encoder_factory_ = encoder_factory;
 
   video_codecs_ = GetSupportedCodecs();
 }
 
-bool WebRtcVideoEngine2::EnableTimedRender() {
-  // TODO(pbos): Figure out whether this can be removed.
-  return true;
-}
-
 // Checks to see whether we comprehend and could receive a particular codec
 bool WebRtcVideoEngine2::FindCodec(const VideoCodec& in) {
   // TODO(pbos): Probe encoder factory to figure out that the codec is supported
   // if supported by the encoder factory. Add a corresponding test that fails
   // with this code (that doesn't ask the factory).
   for (size_t j = 0; j < video_codecs_.size(); ++j) {
     VideoCodec codec(video_codecs_[j].id, video_codecs_[j].name, 0, 0, 0, 0);
     if (codec.Matches(in)) {
       return true;
     }
@@ skipping 138 matching lines @@
     // 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::GetChangedSendParameters(
+    const VideoSendParameters& params,
+    ChangedSendParameters* changed_params) const {
+  if (!ValidateCodecFormats(params.codecs) ||
+      !ValidateRtpExtensions(params.extensions)) {
+    return false;
+  }
+
+  // ==== SEND CODEC ====
+  const std::vector<VideoCodecSettings> supported_codecs =
+      FilterSupportedCodecs(MapCodecs(params.codecs));
+
+  if (supported_codecs.empty()) {
+    LOG(LS_ERROR) << "No video codecs supported.";
+    return false;
+  }
+
+  if (!send_codec_ || supported_codecs.front() != *send_codec_) {
+    // Send codec has changed.
+    changed_params->codec =
+        rtc::Optional<VideoCodecSettings>(supported_codecs.front());
+  }
+
+  // ==== RTP HEADER EXTENSIONS ====
+  std::vector<webrtc::RtpExtension> filtered_extensions = FilterRtpExtensions(
+      params.extensions, webrtc::RtpExtension::IsSupportedForVideo, true);
+  if (send_rtp_extensions_ != filtered_extensions) {
+    changed_params->rtp_header_extensions =
+        rtc::Optional<std::vector<webrtc::RtpExtension>>(filtered_extensions);
+  }
+
+  // ==== MAX BITRATE ====
+  if (params.max_bandwidth_bps != bitrate_config_.max_bitrate_bps &&
+      params.max_bandwidth_bps >= 0) {
+    // 0 uncaps max bitrate (-1).
+    changed_params->max_bandwidth_bps = rtc::Optional<int>(
+        params.max_bandwidth_bps == 0 ? -1 : params.max_bandwidth_bps);
+  }
+
+  // ==== OPTIONS ====
+  // TODO(pbos): Require VideoSendParameters to contain a full set of options
+  // and check if params.options != options_ instead of applying a delta.
+  VideoOptions old_options = options_;

[Taylor Brandstetter, 2016/01/21 23:47:16]

I don't think you need "old_options", we can just compare "new_options" to
"options_".

[pbos-webrtc, 2016/01/26 16:57:49]

Done.

+  VideoOptions new_options = options_;
+  new_options.SetAll(params.options);
+  if (!(new_options == old_options)) {
+    changed_params->options = rtc::Optional<VideoOptions>(new_options);
+  }
+
+  changed_params->rtcp_mode = params.rtcp.reduced_size
+                                  ? webrtc::RtcpMode::kReducedSize
+                                  : webrtc::RtcpMode::kCompound;
+
+  return true;
+}
+
 bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) {
   TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetSendParameters");
   LOG(LS_INFO) << "SetSendParameters: " << params.ToString();
-  // TODO(pbos): Refactor this to only recreate the send streams once
-  // instead of 4 times.
-  if (!SetSendCodecs(params.codecs) ||
-      !SetSendRtpHeaderExtensions(params.extensions) ||
-      !SetMaxSendBandwidth(params.max_bandwidth_bps) ||
-      !SetOptions(params.options)) {
+  ChangedSendParameters changed_params;
+  if (!GetChangedSendParameters(params, &changed_params)) {
     return false;
   }
-  if (send_params_.rtcp.reduced_size != params.rtcp.reduced_size) {
+
+  bool bitrate_config_changed = false;
+
+  bool codec_set = false;
+  if (changed_params.codec) {
+    const VideoCodecSettings& codec_settings = *changed_params.codec;
+    send_codec_ = rtc::Optional<VideoCodecSettings>(codec_settings);
+    VideoCodec codec = codec_settings.codec;
+    codec_set = true;
+
+    LOG(LS_INFO) << "Using codec: " << codec.ToString();
+    // TODO(holmer): Changing the codec parameters shouldn't necessarily mean
+    // that we change the min/max of bandwidth estimation. Reevaluate this.
+    int bitrate_kbps;

[Taylor Brandstetter, 2016/01/21 23:47:16]

Since this method is already pretty huge, I would define a
"GetBitrateConfigForCodec" helper function and call it here.

[pbos-webrtc, 2016/01/26 16:57:49]

Done.

+    if (codec.GetParam(kCodecParamMinBitrate, &bitrate_kbps) &&
+        bitrate_kbps > 0) {
+      bitrate_config_.min_bitrate_bps = bitrate_kbps * 1000;
+    } else {
+      bitrate_config_.min_bitrate_bps = 0;
+    }
+    if (codec.GetParam(kCodecParamStartBitrate, &bitrate_kbps) &&
+        bitrate_kbps > 0) {
+      bitrate_config_.start_bitrate_bps = bitrate_kbps * 1000;
+    } else {
+      // Do not reconfigure start bitrate unless it's specified and positive.
+      bitrate_config_.start_bitrate_bps = -1;
+    }
+    if (codec.GetParam(kCodecParamMaxBitrate, &bitrate_kbps) &&
+        bitrate_kbps > 0) {
+      bitrate_config_.max_bitrate_bps = bitrate_kbps * 1000;
+    } else {
+      bitrate_config_.max_bitrate_bps = -1;
+    }
+    bitrate_config_changed = true;
+  }
+
+  if (changed_params.rtp_header_extensions) {
+    send_rtp_extensions_ = *changed_params.rtp_header_extensions;
+  }
+
+  if (changed_params.max_bandwidth_bps) {
+    // TODO(pbos): Figure out whether b=AS means max bitrate for this
+    // WebRtcVideoChannel2 (in which case we're good), or per sender (SSRC), in
+    // which case this should not set a Call::BitrateConfig but rather
+    // reconfigure

[Taylor Brandstetter, 2016/01/21 23:47:16]

nit: move "// all senders." up a line.

[pbos-webrtc, 2016/01/26 16:57:49]

Done.

+    // all senders.
+    int max_bitrate_bps = *changed_params.max_bandwidth_bps;
+    bitrate_config_.start_bitrate_bps = -1;
+    bitrate_config_.max_bitrate_bps = max_bitrate_bps;
+    if (max_bitrate_bps > 0 &&
+        bitrate_config_.min_bitrate_bps > max_bitrate_bps) {
+      bitrate_config_.min_bitrate_bps = max_bitrate_bps;
+    }
+    bitrate_config_changed = true;
+  }
+
+  if (bitrate_config_changed) {
+    call_->SetBitrateConfig(bitrate_config_);
+  }
+
+  if (changed_params.options) {
+    options_.SetAll(*changed_params.options);
+    {
+      rtc::CritScope lock(&capturer_crit_);
+      if (options_.cpu_overuse_detection)

[Taylor Brandstetter, 2016/01/21 23:47:16]

nit: add {}

[pbos-webrtc, 2016/01/26 16:57:49]

Done.

+        signal_cpu_adaptation_ = *options_.cpu_overuse_detection;
+    }
+    rtc::DiffServCodePoint dscp =
+        options_.dscp.value_or(false) ? rtc::DSCP_AF41 : rtc::DSCP_DEFAULT;
+    MediaChannel::SetDscp(dscp);
+  }
+
+  {
     rtc::CritScope stream_lock(&stream_crit_);
     for (auto& kv : send_streams_) {
-      kv.second->SetSendParameters(params);
+      kv.second->SetSendParameters(changed_params);
+    }
+    if (codec_set) {

[Taylor Brandstetter, 2016/01/21 23:47:16]

Instead of "codec_set", can you just use "if (changed_params.codec)"?

[pbos-webrtc, 2016/01/26 16:57:49]

Done.

+      // Update receive feedback parameters from new codec.
+      LOG(LS_INFO)
+          << "SetFeedbackOptions on all the receive streams because the send "
+             "codec 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));
+      }
     }
   }
   send_params_ = params;
   return true;
 }
 
 bool WebRtcVideoChannel2::SetRecvParameters(const VideoRecvParameters& params) {
   TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetRecvParameters");
   LOG(LS_INFO) << "SetRecvParameters: " << params.ToString();
   // TODO(pbos): Refactor this to only recreate the recv streams once
@@ skipping 62 matching lines @@
   rtc::CritScope stream_lock(&stream_crit_);
   for (std::map<uint32_t, WebRtcVideoReceiveStream*>::iterator it =
            receive_streams_.begin();
        it != receive_streams_.end(); ++it) {
     it->second->SetRecvCodecs(recv_codecs_);
   }
 
   return true;
 }
 
-bool WebRtcVideoChannel2::SetSendCodecs(const std::vector<VideoCodec>& codecs) {
-  TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetSendCodecs");
-  LOG(LS_INFO) << "SetSendCodecs: " << CodecVectorToString(codecs);
-  if (!ValidateCodecFormats(codecs)) {
-    return false;
-  }
-
-  const std::vector<VideoCodecSettings> supported_codecs =
-      FilterSupportedCodecs(MapCodecs(codecs));
-
-  if (supported_codecs.empty()) {
-    LOG(LS_ERROR) << "No video codecs supported.";
-    return false;
-  }
-
-  LOG(LS_INFO) << "Using codec: " << supported_codecs.front().codec.ToString();
-
-  if (send_codec_ && supported_codecs.front() == *send_codec_) {
-    LOG(LS_INFO) << "Ignore call to SetSendCodecs because first supported "
-                    "codec hasn't changed.";
-    // Using same codec, avoid reconfiguring.
-    return true;
-  }
-
-  send_codec_ = rtc::Optional<WebRtcVideoChannel2::VideoCodecSettings>(
-      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_) {
-    RTC_DCHECK(kv.second != nullptr);
-    kv.second->SetCodec(supported_codecs.front());
-  }
-  LOG(LS_INFO)
-      << "SetFeedbackOptions on all the receive streams because the send "
-         "codec has changed.";
-  for (auto& kv : receive_streams_) {
-    RTC_DCHECK(kv.second != nullptr);
-    kv.second->SetFeedbackParameters(
-        HasNack(supported_codecs.front().codec),
-        HasRemb(supported_codecs.front().codec),
-        HasTransportCc(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;
-  if (codec.GetParam(kCodecParamMinBitrate, &bitrate_kbps) &&
-      bitrate_kbps > 0) {
-    bitrate_config_.min_bitrate_bps = bitrate_kbps * 1000;
-  } else {
-    bitrate_config_.min_bitrate_bps = 0;
-  }
-  if (codec.GetParam(kCodecParamStartBitrate, &bitrate_kbps) &&
-      bitrate_kbps > 0) {
-    bitrate_config_.start_bitrate_bps = bitrate_kbps * 1000;
-  } else {
-    // Do not reconfigure start bitrate unless it's specified and positive.
-    bitrate_config_.start_bitrate_bps = -1;
-  }
-  if (codec.GetParam(kCodecParamMaxBitrate, &bitrate_kbps) &&
-      bitrate_kbps > 0) {
-    bitrate_config_.max_bitrate_bps = bitrate_kbps * 1000;
-  } else {
-    bitrate_config_.max_bitrate_bps = -1;
-  }
-  call_->SetBitrateConfig(bitrate_config_);
-
-  return true;
-}
+void WebRtcVideoChannel2::SetSendCodec(

[Taylor Brandstetter, 2016/01/21 23:47:16]

What's the purpose of this method?

[pbos-webrtc, 2016/01/26 16:57:49]

Moved over in this patchset, removed. Thanks.

+    const VideoCodecSettings& codec_settings) {}
 
 bool WebRtcVideoChannel2::GetSendCodec(VideoCodec* codec) {
   if (!send_codec_) {
     LOG(LS_VERBOSE) << "GetSendCodec: No send codec set.";
     return false;
   }
   *codec = send_codec_->codec;
   return true;
 }
 
@@ skipping 18 matching lines @@
     StartAllSendStreams();
   } else {
     StopAllSendStreams();
   }
   sending_ = send;
   return true;
 }
 
 bool WebRtcVideoChannel2::SetVideoSend(uint32_t ssrc, bool enable,
                                        const VideoOptions* options) {
+  TRACE_EVENT0("webrtc", "SetVideoSend");
+  LOG(LS_INFO) << "SetVideoSend (ssrc= " << ssrc << ", enable = " << enable
+               << "options: " << (options ? options->ToString() : "nullptr")
+               << ").";
+
   // TODO(solenberg): The state change should be fully rolled back if any one of
   //                  these calls fail.
   if (!MuteStream(ssrc, !enable)) {
     return false;
   }
   if (enable && options) {
-    return SetOptions(*options);
-  } else {
-    return true;
+    VideoSendParameters new_params = send_params_;
+    new_params.options.SetAll(*options);
+    SetSendParameters(send_params_);
   }
+  return true;
 }
 
 bool WebRtcVideoChannel2::ValidateSendSsrcAvailability(
     const StreamParams& sp) const {
   for (uint32_t ssrc: sp.ssrcs) {
     if (send_ssrcs_.find(ssrc) != send_ssrcs_.end()) {
       LOG(LS_ERROR) << "Send stream with SSRC '" << ssrc << "' already exists.";
       return false;
     }
   }
@@ skipping 319 matching lines @@
     if (send_streams_.find(ssrc) == send_streams_.end()) {
       LOG(LS_ERROR) << "No sending stream on ssrc " << ssrc;
       return false;
     }
     if (!send_streams_[ssrc]->SetCapturer(capturer)) {
       return false;
     }
   }
 
   if (capturer) {
-    capturer->SetApplyRotation(
-        !FindHeaderExtension(send_rtp_extensions_,
-                             kRtpVideoRotationHeaderExtension));
+    capturer->SetApplyRotation(!ContainsHeaderExtension(
+        send_rtp_extensions_, kRtpVideoRotationHeaderExtension));
   }
   {
     rtc::CritScope lock(&capturer_crit_);
     capturers_[ssrc] = capturer;
   }
   return true;
 }
 
 bool WebRtcVideoChannel2::SendIntraFrame() {
   // TODO(pbos): Implement.
@@ skipping 116 matching lines @@
 
   rtc::CritScope stream_lock(&stream_crit_);
   for (std::map<uint32_t, 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");
-  if (!ValidateRtpExtensions(extensions)) {
-    return false;
-  }
-  std::vector<webrtc::RtpExtension> filtered_extensions = FilterRtpExtensions(
-      extensions, webrtc::RtpExtension::IsSupportedForVideo, true);
-  if (send_rtp_extensions_ == filtered_extensions) {
-    LOG(LS_INFO) << "Ignoring call to SetRecvRtpHeaderExtensions because "
-                    "header extensions haven't changed.";
-    return true;
-  }
-  send_rtp_extensions_.swap(filtered_extensions);
-
-  const webrtc::RtpExtension* cvo_extension = FindHeaderExtension(
-      send_rtp_extensions_, kRtpVideoRotationHeaderExtension);
-
-  rtc::CritScope stream_lock(&stream_crit_);
-  for (std::map<uint32_t, WebRtcVideoSendStream*>::iterator it =
-           send_streams_.begin();
-       it != send_streams_.end(); ++it) {
-    it->second->SetRtpExtensions(send_rtp_extensions_);
-    it->second->SetApplyRotation(!cvo_extension);
-  }
-  return true;
-}
-
-// Counter-intuitively this method doesn't only set global bitrate caps but also
-// per-stream codec max bitrates. This is to permit SetMaxSendBitrate (b=AS) to
-// raise bitrates above the 2000k default bitrate cap.
-bool WebRtcVideoChannel2::SetMaxSendBandwidth(int max_bitrate_bps) {
-  // TODO(pbos): Figure out whether b=AS means max bitrate for this
-  // WebRtcVideoChannel2 (in which case we're good), or per sender (SSRC), in
-  // which case this should not set a Call::BitrateConfig but rather reconfigure
-  // all senders.
-  LOG(LS_INFO) << "SetMaxSendBandwidth: " << max_bitrate_bps << "bps.";
-  if (max_bitrate_bps == bitrate_config_.max_bitrate_bps)
-    return true;
-
-  if (max_bitrate_bps < 0) {
-    // Option not set.
-    return true;
-  }
-  if (max_bitrate_bps == 0) {
-    // Unsetting max bitrate.
-    max_bitrate_bps = -1;
-  }
-  bitrate_config_.start_bitrate_bps = -1;
-  bitrate_config_.max_bitrate_bps = max_bitrate_bps;
-  if (max_bitrate_bps > 0 &&
-      bitrate_config_.min_bitrate_bps > max_bitrate_bps) {
-    bitrate_config_.min_bitrate_bps = max_bitrate_bps;
-  }
-  call_->SetBitrateConfig(bitrate_config_);
-  rtc::CritScope stream_lock(&stream_crit_);
-  for (auto& kv : send_streams_)
-    kv.second->SetMaxBitrateBps(max_bitrate_bps);
-  return true;
-}
-
-bool WebRtcVideoChannel2::SetOptions(const VideoOptions& options) {
-  TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetOptions");
-  LOG(LS_INFO) << "SetOptions: " << options.ToString();
-  VideoOptions old_options = options_;
-  options_.SetAll(options);
-  if (options_ == old_options) {
-    // No new options to set.
-    return true;
-  }
-  {
-    rtc::CritScope lock(&capturer_crit_);
-    if (options_.cpu_overuse_detection)
-      signal_cpu_adaptation_ = *options_.cpu_overuse_detection;
-  }
-  rtc::DiffServCodePoint dscp =
-      options_.dscp.value_or(false) ? rtc::DSCP_AF41 : rtc::DSCP_DEFAULT;
-  MediaChannel::SetDscp(dscp);
-  rtc::CritScope stream_lock(&stream_crit_);
-  for (std::map<uint32_t, WebRtcVideoSendStream*>::iterator it =
-           send_streams_.begin();
-       it != send_streams_.end(); ++it) {
-    it->second->SetOptions(options_);
-  }
-  return true;
+// TODO(pbos): Remove SetOptions in favor of SetSendParameters.
+void WebRtcVideoChannel2::SetOptions(const VideoOptions& options) {
+  VideoSendParameters new_params = send_params_;
+  new_params.options.SetAll(options);
+  SetSendParameters(send_params_);
 }
 
 void WebRtcVideoChannel2::SetInterface(NetworkInterface* iface) {
   MediaChannel::SetInterface(iface);
   // Set the RTP recv/send buffer to a bigger size
   MediaChannel::SetOption(NetworkInterface::ST_RTP,
                           rtc::Socket::OPT_RCVBUF,
                           kVideoRtpBufferSize);
 
   // Speculative change to increase the outbound socket buffer size.
@@ skipping 102 matching lines @@
     const std::vector<webrtc::RtpExtension>& rtp_extensions,
     // TODO(deadbeef): Don't duplicate information between send_params,
     // rtp_extensions, options, etc.
     const VideoSendParameters& send_params)
     : ssrcs_(sp.ssrcs),
       ssrc_groups_(sp.ssrc_groups),
       call_(call),
       external_encoder_factory_(external_encoder_factory),
       stream_(NULL),
       parameters_(config, options, max_bitrate_bps, codec_settings),
+      reconfigure_encoder_(false),

[Taylor Brandstetter, 2016/01/21 23:47:16]

nit: Can initialize this to false in the header file.

[pbos-webrtc, 2016/01/26 16:57:49]

If so I'd like to do that for all members, maybe in a separate CL.

       allocated_encoder_(NULL, webrtc::kVideoCodecUnknown, false),
       capturer_(NULL),
       sending_(false),
       muted_(false),
       old_adapt_changes_(0),
       first_frame_timestamp_ms_(0),
       last_frame_timestamp_ms_(0) {
   parameters_.config.rtp.max_packet_size = kVideoMtu;
 
   sp.GetPrimarySsrcs(&parameters_.config.rtp.ssrcs);
   sp.GetFidSsrcs(parameters_.config.rtp.ssrcs,
                  &parameters_.config.rtp.rtx.ssrcs);
   parameters_.config.rtp.c_name = sp.cname;
   parameters_.config.rtp.extensions = rtp_extensions;
   parameters_.config.rtp.rtcp_mode = send_params.rtcp.reduced_size
                                          ? webrtc::RtcpMode::kReducedSize
                                          : webrtc::RtcpMode::kCompound;
 
   if (codec_settings) {
-    SetCodec(*codec_settings);
+    SetCodecAndOptions(*codec_settings, parameters_.options);
   }
 }
 
 WebRtcVideoChannel2::WebRtcVideoSendStream::~WebRtcVideoSendStream() {
   DisconnectCapturer();
   if (stream_ != NULL) {
     call_->DestroyVideoSendStream(stream_);
   }
   DestroyVideoEncoder(&allocated_encoder_);
 }
@@ skipping 93 matching lines @@
     }
 
     capturer_ = capturer;
   }
   // Lock cannot be held while connecting the capturer to prevent lock-order
   // violations.
   capturer->SignalVideoFrame.connect(this, &WebRtcVideoSendStream::InputFrame);
   return true;
 }
 
+// TODO(pbos): Apply this on the VideoAdapter instead!
 bool WebRtcVideoChannel2::WebRtcVideoSendStream::SetVideoFormat(
     const VideoFormat& format) {
   if ((format.width == 0 || format.height == 0) &&
       format.width != format.height) {
     LOG(LS_ERROR) << "Can't set VideoFormat, width or height is zero (but not "
                      "both, 0x0 drops frames).";
     return false;
   }
 
   rtc::CritScope cs(&lock_);
@@ skipping 32 matching lines @@
   }
   capturer->SignalVideoFrame.disconnect(this);
   return true;
 }
 
 const std::vector<uint32_t>&
 WebRtcVideoChannel2::WebRtcVideoSendStream::GetSsrcs() const {
   return ssrcs_;
 }
 
-void WebRtcVideoChannel2::WebRtcVideoSendStream::SetApplyRotation(
-    bool apply_rotation) {
-  rtc::CritScope cs(&lock_);
-  if (capturer_ == NULL)
-    return;
-
-  capturer_->SetApplyRotation(apply_rotation);
-}
-
 void WebRtcVideoChannel2::WebRtcVideoSendStream::SetOptions(
     const VideoOptions& options) {
   rtc::CritScope cs(&lock_);
   if (parameters_.codec_settings) {
     LOG(LS_INFO) << "SetCodecAndOptions because of SetOptions; options="
                  << options.ToString();
     SetCodecAndOptions(*parameters_.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;
   }
   return webrtc::kVideoCodecUnknown;
 }
@@ skipping 39 matching lines @@
     external_encoder_factory_->DestroyVideoEncoder(encoder->external_encoder);
   }
   delete encoder->encoder;
 }
 
 void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodecAndOptions(
     const VideoCodecSettings& codec_settings,
     const VideoOptions& options) {
   parameters_.encoder_config =
       CreateVideoEncoderConfig(last_dimensions_, codec_settings.codec);
-  if (parameters_.encoder_config.streams.empty())
-    return;
+  RTC_DCHECK(!parameters_.encoder_config.streams.empty());
 
   format_ = VideoFormat(codec_settings.codec.width,
                         codec_settings.codec.height,
                         VideoFormat::FpsToInterval(30),
                         FOURCC_I420);
 
   AllocatedEncoder new_encoder = CreateVideoEncoder(codec_settings.codec);
   parameters_.config.encoder_settings.encoder = new_encoder.encoder;
   parameters_.config.encoder_settings.payload_name = codec_settings.codec.name;
   parameters_.config.encoder_settings.payload_type = codec_settings.codec.id;
@@ skipping 29 matching lines @@
   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) {
-    LOG(LS_INFO) << "RecreateWebRtcStream (send) because of SetRtpExtensions";
-    RecreateWebRtcStream();
-  }
-}
-
 void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSendParameters(
-    const VideoSendParameters& send_params) {
+    const ChangedSendParameters& params) {
   rtc::CritScope cs(&lock_);
-  parameters_.config.rtp.rtcp_mode = send_params.rtcp.reduced_size
-                                         ? webrtc::RtcpMode::kReducedSize
-                                         : webrtc::RtcpMode::kCompound;
-  if (stream_ != nullptr) {
+  // |recreate_stream| means construction-time parameters have changed and the
+  // sending stream needs to be reset with the new config.
+  bool recreate_stream = false;
+  if (parameters_.config.rtp.rtcp_mode != params.rtcp_mode) {
+    parameters_.config.rtp.rtcp_mode = params.rtcp_mode;
+    recreate_stream = true;
+  }
+  if (params.rtp_header_extensions) {
+    parameters_.config.rtp.extensions = *params.rtp_header_extensions;
+    if (capturer_) {
+      capturer_->SetApplyRotation(!ContainsHeaderExtension(
+          *params.rtp_header_extensions, kRtpVideoRotationHeaderExtension));
+    }
+    recreate_stream = true;
+  }
+  if (params.max_bandwidth_bps) {
+    // Max bitrate has changed, reconfigure encoder settings on the next frame
+    // or stream recreation.
+    parameters_.max_bitrate_bps = *params.max_bandwidth_bps;
+    reconfigure_encoder_ = true;

[Taylor Brandstetter, 2016/01/21 23:47:16]

Maybe rename this to something like "pending_reconfigure_encoder_", or
"reconfigure_encoder_on_next_frame_". Or just describe it in the header file. It
was unclear to me at first why this was a member variable, until I realized that
it didn't take effect until the next InputFrame.

[pbos-webrtc, 2016/01/26 16:57:49]

pending_encoder_reconfiguration_ done.

+  }
+  // Set codecs and options.
+  if (params.codec) {
+    SetCodecAndOptions(*params.codec,
+                       params.options ? *params.options : parameters_.options);
+    recreate_stream = false;

[Taylor Brandstetter, 2016/01/21 23:47:16]

Instead of resetting recreate_stream to false, you could just return early here.
That may be more clear to someone reading the code.

[pbos-webrtc, 2016/01/26 16:57:49]

Done.

+  } else if (params.options) {
+    // Reconfigure if codecs are already set.
+    if (parameters_.codec_settings) {
+      SetCodecAndOptions(*parameters_.codec_settings, *params.options);
+      recreate_stream = false;

[Taylor Brandstetter, 2016/01/21 23:47:16]

Same here.

[pbos-webrtc, 2016/01/26 16:57:49]

Done.

+    } else {
+      parameters_.options = *params.options;
+    }
+  }
+  if (recreate_stream) {
     LOG(LS_INFO) << "RecreateWebRtcStream (send) because of SetSendParameters";
     RecreateWebRtcStream();
   }
 }
 
 webrtc::VideoEncoderConfig
 WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig(
     const Dimensions& dimensions,
     const VideoCodec& codec) const {
   webrtc::VideoEncoderConfig encoder_config;
@@ skipping 51 matching lines @@
         config.tl0_bitrate_kbps * 1000);
   }
   return encoder_config;
 }
 
 void WebRtcVideoChannel2::WebRtcVideoSendStream::SetDimensions(
     int width,
     int height,
     bool is_screencast) {
   if (last_dimensions_.width == width && last_dimensions_.height == height &&
-      last_dimensions_.is_screencast == is_screencast) {
+      last_dimensions_.is_screencast == is_screencast &&
+      !reconfigure_encoder_) {
     // Configured using the same parameters, do not reconfigure.
     return;
   }
   LOG(LS_INFO) << "SetDimensions: " << width << "x" << height
                << (is_screencast ? " (screencast)" : " (not screencast)");
 
   last_dimensions_.width = width;
   last_dimensions_.height = height;
   last_dimensions_.is_screencast = is_screencast;
 
   RTC_DCHECK(!parameters_.encoder_config.streams.empty());
 
   RTC_CHECK(parameters_.codec_settings);
   VideoCodecSettings codec_settings = *parameters_.codec_settings;
 
   webrtc::VideoEncoderConfig encoder_config =
       CreateVideoEncoderConfig(last_dimensions_, codec_settings.codec);
 
   encoder_config.encoder_specific_settings = ConfigureVideoEncoderSettings(
       codec_settings.codec, parameters_.options, is_screencast);
 
   bool stream_reconfigured = stream_->ReconfigureVideoEncoder(encoder_config);
 
   encoder_config.encoder_specific_settings = NULL;
+  reconfigure_encoder_ = false;
 
   if (!stream_reconfigured) {
     LOG(LS_WARNING) << "Failed to reconfigure video encoder for dimensions: "
                     << width << "x" << height;
     return;
   }
 
   parameters_.encoder_config = encoder_config;
 }
 
@@ skipping 116 matching lines @@
   for (std::map<uint32_t, webrtc::VideoSendStream::StreamStats>::iterator it =
            stats.substreams.begin();
        it != stats.substreams.end(); ++it) {
     bwe_info->transmit_bitrate += it->second.total_bitrate_bps;
     bwe_info->retransmit_bitrate += it->second.retransmit_bitrate_bps;
   }
   bwe_info->target_enc_bitrate += stats.target_media_bitrate_bps;
   bwe_info->actual_enc_bitrate += stats.media_bitrate_bps;
 }
 
-void WebRtcVideoChannel2::WebRtcVideoSendStream::SetMaxBitrateBps(
-    int max_bitrate_bps) {
-  rtc::CritScope cs(&lock_);
-  parameters_.max_bitrate_bps = max_bitrate_bps;
-
-  // No need to reconfigure if the stream hasn't been configured yet.
-  if (parameters_.encoder_config.streams.empty())
-    return;
-
-  // Force a stream reconfigure to set the new max bitrate.
-  int width = last_dimensions_.width;
-  last_dimensions_.width = 0;
-  SetDimensions(width, last_dimensions_.height, last_dimensions_.is_screencast);
-}
-
 void WebRtcVideoChannel2::WebRtcVideoSendStream::RecreateWebRtcStream() {
   if (stream_ != NULL) {
     call_->DestroyVideoSendStream(stream_);
   }
 
   RTC_CHECK(parameters_.codec_settings);
   parameters_.encoder_config.encoder_specific_settings =
       ConfigureVideoEncoderSettings(
           parameters_.codec_settings->codec, parameters_.options,
           parameters_.encoder_config.content_type ==
               webrtc::VideoEncoderConfig::ContentType::kScreen);
 
   webrtc::VideoSendStream::Config config = parameters_.config;
   if (!config.rtp.rtx.ssrcs.empty() && config.rtp.rtx.payload_type == -1) {
     LOG(LS_WARNING) << "RTX SSRCs configured but there's no configured RTX "
                        "payload type the set codec. Ignoring RTX.";
     config.rtp.rtx.ssrcs.clear();
   }
   stream_ = call_->CreateVideoSendStream(config, parameters_.encoder_config);
 
   parameters_.encoder_config.encoder_specific_settings = NULL;
+  reconfigure_encoder_ = false;
 
   if (sending_) {
     stream_->Start();
   }
 }
 
 WebRtcVideoChannel2::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream(
     webrtc::Call* call,
     const StreamParams& sp,
     const webrtc::VideoReceiveStream::Config& config,
@@ skipping 416 matching lines @@
       video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
     }
   }
 
   return video_codecs;
 }
 
 }  // namespace cricket
 
 #endif  // HAVE_WEBRTC_VIDEO