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::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;
   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(
+    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 19 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);
+    // 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.

 }
 
 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 320 matching lines @@
       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_,
+        !ContainsHeaderExtension(send_rtp_extensions_,
                              kRtpVideoRotationHeaderExtension));
   }
   {
     rtc::CritScope lock(&capturer_crit_);
     capturers_[ssrc] = capturer;
   }
   return true;
 }
 
 bool WebRtcVideoChannel2::SendIntraFrame() {
@@ skipping 117 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();
+// 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_);
     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;
 }
 
 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 121 matching lines @@
   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(

[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_);
   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 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();
   }
 }
 
 webrtc::VideoEncoderConfig
 WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig(
     const Dimensions& dimensions,
     const VideoCodec& codec) const {
   webrtc::VideoEncoderConfig encoder_config;
@@ skipping 212 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 ==
@@ skipping 451 matching lines @@
       video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
     }
   }
 
   return video_codecs;
 }
 
 }  // namespace cricket
 
 #endif  // HAVE_WEBRTC_VIDEO