Consolidate SetSendParameters into one setter.

talk/media/webrtc/webrtcvideoengine2.cc

Index: talk/media/webrtc/webrtcvideoengine2.cc
diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc
index 55c07426d090598877f1c4a6656d5936f20d5c99..e2f19968bc24791fdc885e22be57d9958ee128b8 100644
--- a/talk/media/webrtc/webrtcvideoengine2.cc
+++ b/talk/media/webrtc/webrtcvideoengine2.cc
@@ -243,15 +243,15 @@ static bool ValidateStreamParams(const StreamParams& sp) {
   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
@@ -547,11 +547,6 @@ void WebRtcVideoEngine2::SetExternalEncoderFactory(
   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
@@ -710,21 +705,156 @@ bool WebRtcVideoChannel2::ReceiveCodecsHaveChanged(
   return false;
 }
 
+bool WebRtcVideoChannel2::FilterSendParameters(

[pthatcher1, 2016/01/07 21:03:43]

I think a better name here would be GetChangedSendParameters.

[pbos-webrtc, 2016/01/20 18:09:03]

Done.

+    const VideoSendParameters& params,
+    FilteredSendParameters* out_params) const {

[pthatcher1, 2016/01/07 21:03:43]

And this ChangedSendParameters* changed_params

Filtering is just a side effect.  The primary effect is finding what changed and
what didn't.

[pbos-webrtc, 2016/01/20 18:09:03]

Done.

+  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.
+    out_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) {
+    out_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).
+    out_params->max_bandwidth_bps = rtc::Optional<int>(
+        params.max_bandwidth_bps == 0 ? -1 : params.max_bandwidth_bps);
+  }
+
+  // ==== OPTIONS ====
+
+  VideoOptions old_options = options_;
+  VideoOptions new_options = options_;
+  new_options.SetAll(params.options);
+  if (!(new_options == old_options)) {

[pthatcher1, 2016/01/07 21:03:43]

Can you put a TODO to see if we can just do:

if (params.options != options_) {
  out_params->options = rtc::Optional<VideoOptions>(params.options);
}

? 

In other words, require that VideoSendParameters always have a full set of
options not a partial set.  

[pbos-webrtc, 2016/01/20 18:09:03]

Done.

+    out_params->options = rtc::Optional<VideoOptions>(new_options);
+  }
+
+  out_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)) {
+  FilteredSendParameters filtered_params;
+  if (!FilterSendParameters(params, &filtered_params)) {
     return false;
   }
-  if (send_params_.rtcp.reduced_size != params.rtcp.reduced_size) {
+
+  bool bitrate_config_changed = false;
+
+  bool codec_set = false;
+  if (filtered_params.codec) {
+    const VideoCodecSettings& codec_settings = *filtered_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;
+    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 (filtered_params.rtp_header_extensions) {
+    send_rtp_extensions_ = *filtered_params.rtp_header_extensions;
+  }
+
+  if (filtered_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
+    // all senders.
+    int max_bitrate_bps = *filtered_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 (filtered_params.options) {
+    options_.SetAll(*filtered_params.options);
+    {
+      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 (auto& kv : send_streams_) {
-      kv.second->SetSendParameters(params);
+      kv.second->SetSendParameters(filtered_params);
+    }
+    if (codec_set) {
+      // 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;
@@ -807,77 +937,8 @@ bool WebRtcVideoChannel2::SetRecvCodecs(const std::vector<VideoCodec>& 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(
+    const VideoCodecSettings& codec_settings) {
 }
 
 bool WebRtcVideoChannel2::GetSendCodec(VideoCodec* codec) {
@@ -917,13 +978,21 @@ bool WebRtcVideoChannel2::SetSend(bool send) {
 
 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);
+    // TODO(pbos): Provide VideoSendParameters here instead if possible, and
+    // remove SetOptions.
+    SetOptions(*options);

[pthatcher1, 2016/01/07 21:03:43]

If you pulled out most of the logic in SetSendParameters(VideoSendParameters)
into a separate ChangeSendParameters(ChangedVideoSendParameters), then you could
do this:

if (*options != options_) {
  ChangedSendParameters changed_params;
  VideoOptions new_options = options_;
  options_.SetAll(options);
  changed_params.options = rtc::Optional<VideoOptions>(new_options);
  ChangeSendParameters(changed_params);
}

Or if you store the last set of VideoSendParameters, you could do this:
 
VideoSendParameters new_params = send_params_;
new_params.options.SetAll(options);
SetSendParameters(new_params);


I think I like the second better.

[pbos-webrtc, 2016/01/20 18:09:04]

Done.

+    return true;
   } else {
     return true;
   }

[pthatcher1, 2016/01/07 21:03:43]

These both return true, so you could probably just have it be:

if (enable && options) {
  ...
}
return true;

[pbos-webrtc, 2016/01/20 18:09:04]

Yeah that looks dumb. Assuming it has evolved over time. Done.

@@ -1267,7 +1336,7 @@ bool WebRtcVideoChannel2::SetCapturer(uint32_t ssrc, VideoCapturer* capturer) {
 
   if (capturer) {
     capturer->SetApplyRotation(
-        !FindHeaderExtension(send_rtp_extensions_,
+        !ContainsHeaderExtension(send_rtp_extensions_,
                              kRtpVideoRotationHeaderExtension));
   }
   {
@@ -1405,75 +1474,12 @@ bool WebRtcVideoChannel2::SetRecvRtpHeaderExtensions(
   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();
+// TODO(pbos): Remove SetOptions in favor of SetSendParameters.
+void WebRtcVideoChannel2::SetOptions(const VideoOptions& options) {
   VideoOptions old_options = options_;
   options_.SetAll(options);
-  if (options_ == old_options) {
-    // No new options to set.
-    return true;
+  if (options == old_options) {
+    return;
   }
   {
     rtc::CritScope lock(&capturer_crit_);
@@ -1489,7 +1495,6 @@ bool WebRtcVideoChannel2::SetOptions(const VideoOptions& options) {
        it != send_streams_.end(); ++it) {
     it->second->SetOptions(options_);
   }
-  return true;
 }
 
 void WebRtcVideoChannel2::SetInterface(NetworkInterface* iface) {
@@ -1631,7 +1636,7 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
                                          : webrtc::RtcpMode::kCompound;
 
   if (codec_settings) {
-    SetCodec(*codec_settings);
+    SetCodecAndOptions(*codec_settings, parameters_.options);
   }
 }
 
@@ -1745,6 +1750,7 @@ bool WebRtcVideoChannel2::WebRtcVideoSendStream::SetCapturer(
   return true;
 }
 
+// TODO(pbos): Apply this on the VideoAdapter instead!
 bool WebRtcVideoChannel2::WebRtcVideoSendStream::SetVideoFormat(
     const VideoFormat& format) {
   if ((format.width == 0 || format.height == 0) &&
@@ -1797,15 +1803,6 @@ 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(

[pthatcher1, 2016/01/08 00:34:57]

By the way, as far as I can tell, this is only used by 
VideoRtpSender::SetVideoSend, which gets its options from
VideoSource::Initialize, which is only used to turn on video_noise_reduction,
which shoudn't be a track constraint at all but is currently used by AppRTC with
the constraint "googNoiseReduction".  If we can come up with a better way to
turn it on (or just always turn it on), We could remove this method (I think).

[pbos-webrtc, 2016/01/20 18:09:04]

Acknowledged.

     const VideoOptions& options) {
   rtc::CritScope cs(&lock_);
@@ -1818,13 +1815,6 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetOptions(
   }
 }
 
-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;
@@ -1884,8 +1874,7 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodecAndOptions(
     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,
@@ -1935,23 +1924,53 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodecAndOptions(
   }
 }
 
-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 FilteredSendParameters& params) {
   rtc::CritScope cs(&lock_);
-  parameters_.config.rtp.rtcp_mode = send_params.rtcp.reduced_size
-                                         ? webrtc::RtcpMode::kReducedSize
-                                         : webrtc::RtcpMode::kCompound;
-  if (stream_ != nullptr) {
+  bool needs_restart = false;
+  bool force_reconfigure = false;

[pthatcher1, 2016/01/07 21:03:43]

needs_restart means "RecreateWebRtcStream();"
force_reconfigure means "last_dimensions_.width = 0;"

That's confusing.  It might be less confusing as:

bool recreate_stream = false;
bool reconfigure_stream = false;

With a comment describing the difference.

[pbos-webrtc, 2016/01/20 18:09:04]

Done.

+  if (parameters_.config.rtp.rtcp_mode != params.rtcp_mode) {
+    parameters_.config.rtp.rtcp_mode = params.rtcp_mode;
+    needs_restart = true;
+  }
+  if (params.rtp_header_extensions) {
+    parameters_.config.rtp.extensions = *params.rtp_header_extensions;
+    if (capturer_) {
+      capturer_->SetApplyRotation(!ContainsHeaderExtension(
+          *params.rtp_header_extensions, kRtpVideoRotationHeaderExtension));
+    }
+    needs_restart = true;
+  }
+  if (params.max_bandwidth_bps) {
+    parameters_.max_bitrate_bps = *params.max_bandwidth_bps;
+    if (!parameters_.encoder_config.streams.empty() && stream_ != nullptr) {
+      // Already running, force a stream reconfigure to set the new max bitrate.
+      // TODO(pbos): Just have it reconfigure on the next frame instead. Do this
+      // before submitting.
+      force_reconfigure = true;
+    }
+  }
+  if (force_reconfigure) {
+    int width = last_dimensions_.width;
+    last_dimensions_.width = 0;
+    SetDimensions(width, last_dimensions_.height,
+                  last_dimensions_.is_screencast);
+  }

[pthatcher1, 2016/01/07 21:03:43]

If you're going to recreate the stream later, do you still need to do this?

If so, do you need to do this before calling SetCodecAndOptions?  If so, could
you please carefully leave a comment explaining that?

[pbos-webrtc, 2016/01/20 18:09:04]

Nope, I don't. Made this lazy and reinitialize on next frame instead (if it
hasn't been recreated already).

+  // Set codecs and options.
+  if (params.codec) {
+    SetCodecAndOptions(*params.codec,
+                       params.options ? *params.options : parameters_.options);
+    needs_restart = false;
+  } else if (params.options) {
+    // Reconfigure if codecs are already set.
+    if (parameters_.codec_settings) {
+      SetCodecAndOptions(*parameters_.codec_settings, *params.options);
+      needs_restart = false;
+    } else {
+      parameters_.options = *params.options;
+    }
+  }
+  if (needs_restart) {
     LOG(LS_INFO) << "RecreateWebRtcStream (send) because of SetSendParameters";
     RecreateWebRtcStream();
   }
@@ -2184,21 +2203,6 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::FillBandwidthEstimationInfo(
   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_);