Enable setting the maximum bitrate limit in RtpSender.

webrtc/media/engine/webrtcvideoengine2.cc

Index: webrtc/media/engine/webrtcvideoengine2.cc
diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc
index 5f8488705184488def8fbaca74d04dadb29fa6c8..03b6c88f45ec63d5b651c20da3da82981844cdc4 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -793,12 +793,18 @@ bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) {
     // 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 = *changed_params.max_bandwidth_bps;
+    int max_global_bitrate = *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_.max_bitrate_bps = max_global_bitrate;
+    if (max_global_bitrate > 0 &&
+        bitrate_config_.min_bitrate_bps > max_global_bitrate) {
+      bitrate_config_.min_bitrate_bps = max_global_bitrate;

[pthatcher1, 2016/03/16 00:48:42]

I think you can leave the code as it was with the code I describe below.

[skvlad, 2016/03/16 02:29:22]

Done.

+      // TODO(skvlad): Remove the global limits once we transition to
+      // unified plan SDP.

[pthatcher1, 2016/03/16 00:48:42]

I think you can remove this comment and replace it with the one mentioned in the
header file.

[skvlad, 2016/03/16 02:29:22]

Done.

+
+      // TODO(skvlad): Ensure that the encoder does not get reconfigured when
+      // one of the two maximums change, but the effective maximum for the
+      // stream does not.

[pthatcher1, 2016/03/16 00:48:42]

That's probably an optimization that isn't worth doing, since we'll be getting
rid of having two maximums and only having one once we move to unified plan.

[skvlad, 2016/03/16 02:29:22]

This optimization was in fact working in the previous CL - I forgot to remove
the TODO. I've replaced it with the simplified version now, which no longer has
the optimization.
Setting each limit to the same value it used to be still does not trigger
reconfiguration, though.

     }
     bitrate_config_changed = true;
   }
@@ -831,6 +837,32 @@ bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) {
   send_params_ = params;
   return true;
 }
+webrtc::RtpParameters WebRtcVideoChannel2::GetRtpParameters(
+    uint32_t ssrc) const {
+  rtc::CritScope stream_lock(&stream_crit_);
+  auto it = send_streams_.find(ssrc);
+  if (it == send_streams_.end()) {
+    LOG(LS_WARNING) << "Attempting to get RTP parameters for stream with ssrc "
+                    << ssrc << " which doesn't exist.";
+    return webrtc::RtpParameters();
+  }
+
+  return it->second->rtp_parameters();
+}
+
+bool WebRtcVideoChannel2::SetRtpParameters(
+    uint32_t ssrc,
+    const webrtc::RtpParameters& parameters) {
+  rtc::CritScope stream_lock(&stream_crit_);
+  auto it = send_streams_.find(ssrc);
+  if (it == send_streams_.end()) {
+    LOG(LS_ERROR) << "Attempting to set RTP parameters for stream with ssrc "
+                  << ssrc << " which doesn't exist.";
+    return false;
+  }
+
+  return it->second->SetRtpParameters(parameters);
+}
 
 bool WebRtcVideoChannel2::GetChangedRecvParameters(
     const VideoRecvParameters& params,
@@ -1493,7 +1525,9 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
       sending_(false),
       muted_(false),
       first_frame_timestamp_ms_(0),
-      last_frame_timestamp_ms_(0) {
+      last_frame_timestamp_ms_(0),
+      rtp_parameters_(CreateRtpParametersWithOneEncoding()),
+      global_max_bitrate_(max_bitrate_bps) {
   parameters_.config.rtp.max_packet_size = kVideoMtu;
   parameters_.conference_mode = send_params.conference_mode;
 
@@ -1775,10 +1809,9 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSendParameters(
       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;
-      pending_encoder_reconfiguration_ = true;
+      global_max_bitrate_ = *params.max_bandwidth_bps;
+      ApplyBitrateLimit(MinPositive(
+          rtp_parameters_.encodings[0].max_bitrate_bps, global_max_bitrate_));

[pthatcher1, 2016/03/16 00:48:42]

I think we can just leave this code as it was with the change I mention below.

[skvlad, 2016/03/16 02:29:22]

Done.

     }
     if (params.conference_mode) {
       parameters_.conference_mode = *params.conference_mode;
@@ -1812,6 +1845,39 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSendParameters(
   }
 }
 
+bool WebRtcVideoChannel2::WebRtcVideoSendStream::SetRtpParameters(
+    const webrtc::RtpParameters& new_parameters) {
+  if (!ValidateRtpParameters(new_parameters)) {
+    return false;
+  }
+
+  rtc::CritScope cs(&lock_);
+  int per_stream_bitrate_limit = new_parameters.encodings[0].max_bitrate_bps;
+  int computed_limit =
+      MinPositive(per_stream_bitrate_limit, global_max_bitrate_);
+  ApplyBitrateLimit(computed_limit);
+  rtp_parameters_ = new_parameters;
+  return true;

[pthatcher1, 2016/03/16 00:48:42]

I think this whole method could be more simple:


if (!ValidateRtpParameters(new_parameters)) {
  return false;
}

if (new_parameters.max_bitrate_bps != rtp_paramters_.max_bitrate_bps) {
  // The encoder will be reconfigured with the new bitrate on the next video
frame.
  pending_encoder_configuration_ = true;
}

[skvlad, 2016/03/16 02:29:22]

Done.

+}
+
+bool WebRtcVideoChannel2::WebRtcVideoSendStream::ValidateRtpParameters(
+    const webrtc::RtpParameters& rtp_parameters) {
+  if (rtp_parameters.encodings.size() != 1) {
+    LOG(LS_ERROR)
+        << "Attempted to set RtpParameters without exactly one encoding";
+    return false;
+  }
+  return true;
+}
+
+void WebRtcVideoChannel2::WebRtcVideoSendStream::ApplyBitrateLimit(
+    int max_bitrate_bps) {
+  if (parameters_.max_bitrate_bps != max_bitrate_bps) {
+    parameters_.max_bitrate_bps = max_bitrate_bps;
+    pending_encoder_reconfiguration_ = true;
+  }
+}
+
 webrtc::VideoEncoderConfig
 WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig(
     const Dimensions& dimensions,