Enable setting the maximum bitrate limit in RtpSender.

webrtc/media/engine/webrtcvideoengine2.cc

 /*
  *  Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 775 matching lines @@
 
   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 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;
   }
 
   if (bitrate_config_changed) {
     call_->SetBitrateConfig(bitrate_config_);
   }
 
   if (changed_params.options)
     send_params_.options.SetAll(*changed_params.options);
@@ skipping 12 matching lines @@
         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;
 }
+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,
     ChangedRecvParameters* changed_params) const {
   if (!ValidateCodecFormats(params.codecs) ||
       !ValidateRtpExtensions(params.extensions)) {
     return false;
   }
 
   // Handle receive codecs.
@@ skipping 642 matching lines @@
       number_of_cpu_adapt_changes_(0),
       capturer_(nullptr),
       external_encoder_factory_(external_encoder_factory),
       stream_(nullptr),
       parameters_(config, send_params.options, max_bitrate_bps, codec_settings),
       pending_encoder_reconfiguration_(false),
       allocated_encoder_(nullptr, webrtc::kVideoCodecUnknown, false),
       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;
 
   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
@@ skipping 261 matching lines @@
     bool recreate_stream = false;
     if (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;
       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;
     }
     if (params.options)
       SetOptions(*params.options);
 
     // Set codecs and options.
     if (params.codec) {
       SetCodec(*params.codec);
@@ skipping 13 matching lines @@
   // that might cause a lock order inversion.
   if (params.rtp_header_extensions) {
     sink_wants_.rotation_applied = !ContainsHeaderExtension(
         *params.rtp_header_extensions, kRtpVideoRotationHeaderExtension);
     if (capturer_) {
       capturer_->AddOrUpdateSink(this, sink_wants_);
     }
   }
 }
 
+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,
     const VideoCodec& codec) const {
   webrtc::VideoEncoderConfig encoder_config;
   bool is_screencast = parameters_.options.is_screencast.value_or(false);
   if (is_screencast) {
     encoder_config.min_transmit_bitrate_bps =
         1000 * parameters_.options.screencast_min_bitrate_kbps.value_or(0);
     encoder_config.content_type =
@@ skipping 18 matching lines @@
   clamped_codec.width = width;
   clamped_codec.height = height;
 
   // By default, the stream count for the codec configuration should match the
   // number of negotiated ssrcs. But if the codec is blacklisted for simulcast
   // or a screencast, only configure a single stream.
   size_t stream_count = parameters_.config.rtp.ssrcs.size();
   if (IsCodecBlacklistedForSimulcast(codec.name) || is_screencast) {
     stream_count = 1;
   }
 

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

I think we could simplify by just doing the following here:

// We use the minimum of the cross-stream max bitrate 
// and per-stream max bitrate.
int max_bitrate_bps = MinPositive(
  new_parameters.encodings[0].max_bitrate_bps,
  parameters_.max_bitrate_bps)
encoder_config.streams =
    CreateVideoStreams(clamped_codec, parameters_.options,
                       max_bitrate_bps, stream_count);

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

Done.

   encoder_config.streams =
       CreateVideoStreams(clamped_codec, parameters_.options,
                          parameters_.max_bitrate_bps, stream_count);
 
   // Conference mode screencast uses 2 temporal layers split at 100kbit.
   if (parameters_.conference_mode && is_screencast &&
       encoder_config.streams.size() == 1) {
     ScreenshareLayerConfig config = ScreenshareLayerConfig::GetDefault();
 
     // For screenshare in conference mode, tl0 and tl1 bitrates are piggybacked
@@ skipping 667 matching lines @@
         rtx_mapping[video_codecs[i].codec.id] !=
             fec_settings.red_payload_type) {
       video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
     }
   }
 
   return video_codecs;
 }
 
 }  // namespace cricket