Allow applications to control audio send bitrate through RtpParameters.

webrtc/media/engine/webrtcvoiceengine.cc

Index: webrtc/media/engine/webrtcvoiceengine.cc
diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc
index 21094fde4d45af8baed7c5228237a5caba363e04..66681b9e1d8e6771a22a5a5940dcf41a5aa10d8d 100644
--- a/webrtc/media/engine/webrtcvoiceengine.cc
+++ b/webrtc/media/engine/webrtcvoiceengine.cc
@@ -1081,13 +1081,16 @@ int WebRtcVoiceEngine::CreateVoEChannel() {
 class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
     : public AudioSource::Sink {
  public:
-  WebRtcAudioSendStream(int ch, webrtc::AudioTransport* voe_audio_transport,
-                        uint32_t ssrc, const std::string& c_name,
+  WebRtcAudioSendStream(int ch,
+                        webrtc::AudioTransport* voe_audio_transport,
+                        uint32_t ssrc,
+                        const std::string& c_name,
                         const std::vector<webrtc::RtpExtension>& extensions,
                         webrtc::Call* call)
       : voe_audio_transport_(voe_audio_transport),
         call_(call),
-        config_(nullptr) {
+        config_(nullptr),
+        rtp_parameters_(CreateRtpParametersWithOneEncoding()) {
     RTC_DCHECK_GE(ch, 0);
     // TODO(solenberg): Once we're not using FakeWebRtcVoiceEngine anymore:
     // RTC_DCHECK(voe_audio_transport);
@@ -1198,6 +1201,12 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
     return config_.voe_channel_id;
   }
 
+  webrtc::RtpParameters rtp_parameters() const { return rtp_parameters_; }

[the sun, 2016/04/06 09:27:41]

Return a const &

[skvlad, 2016/04/07 00:50:49]

Done. It's a bit dangerous though - if the caller stores the reference or gets a
pointer to it, it may change unexpectedly.

[the sun, 2016/04/07 08:22:21]

True, which is why I didn't comment on WVoMC::GetRtpParameters() returning by
value. This class is internal to WVoMC though so we can be a bit more picky with
what to expect of the user.

+
+  void set_rtp_parameters(const webrtc::RtpParameters& parameters) {

[the sun, 2016/04/06 09:27:41]

RTC_DCHECK_EQ(1, parameters.encodings.size());
or is it LE?

[skvlad, 2016/04/07 00:50:50]

It's supposed to be exactly 1 at the moment. Added the check.

+    rtp_parameters_ = parameters;
+  }
+
  private:
   void UpdateSendState() {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
@@ -1223,6 +1232,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
   // goes away.
   AudioSource* source_ = nullptr;
   bool send_ = false;
+  webrtc::RtpParameters rtp_parameters_;
 
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(WebRtcAudioSendStream);
 };
@@ -1359,7 +1369,7 @@ bool WebRtcVoiceMediaChannel::SetSendParameters(
     }
   }
 
-  if (!SetMaxSendBandwidth(params.max_bandwidth_bps)) {
+  if (!SetSendBitrate(params.max_bandwidth_bps)) {
     return false;
   }
   return SetOptions(params.options);
@@ -1393,6 +1403,53 @@ bool WebRtcVoiceMediaChannel::SetRecvParameters(
   return true;
 }
 
+webrtc::RtpParameters WebRtcVoiceMediaChannel::GetRtpParameters(
+    uint32_t ssrc) const {
+  RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+  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 WebRtcVoiceMediaChannel::SetRtpParameters(
+    uint32_t ssrc,
+    const webrtc::RtpParameters& parameters) {
+  RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+  if (!ValidateRtpParameters(parameters)) {
+    return false;
+  }
+  auto it = send_streams_.find(ssrc);
+  if (it == send_streams_.end()) {
+    LOG(LS_WARNING) << "Attempting to set RTP parameters for stream with ssrc "
+                    << ssrc << " which doesn't exist.";
+    return false;
+  }
+
+  int combined_bitrate_limit =
+      MinPositive(parameters.encodings[0].max_bitrate_bps, send_bitrate_bps_);
+  if (!SetSendBitrate(it->second->channel(), combined_bitrate_limit)) {
+    LOG(LS_WARNING) << "Failed to apply the bitrate limit.";
+    return false;
+  }
+  it->second->set_rtp_parameters(parameters);
+  return true;
+}
+
+bool WebRtcVoiceMediaChannel::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;
+}
+
 bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) {
   RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
   LOG(LS_INFO) << "Setting voice channel options: "
@@ -1587,7 +1644,7 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
 
   // Cache the codecs in order to configure the channel created later.
   for (const auto& ch : send_streams_) {
-    if (!SetSendCodecs(ch.second->channel())) {
+    if (!SetSendCodecs(ch.second->channel(), ch.second->rtp_parameters())) {
       return false;
     }
   }
@@ -1614,7 +1671,9 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
 }
 
 // Apply current codec settings to a single voe::Channel used for sending.
-bool WebRtcVoiceMediaChannel::SetSendCodecs(int channel) {
+bool WebRtcVoiceMediaChannel::SetSendCodecs(
+    int channel,
+    const webrtc::RtpParameters& rtp_parameters) {
   // Disable VAD, FEC, and RED unless we know the other side wants them.
   engine()->voe()->codec()->SetVADStatus(channel, false);
   engine()->voe()->rtp()->SetNACKStatus(channel, false, 0);
@@ -1683,9 +1742,9 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(int channel) {
     }
   }
 
-  if (send_bitrate_setting_) {
-    SetSendBitrateInternal(send_bitrate_bps_);
-  }
+  SetSendBitrate(channel,

[the sun, 2016/04/06 09:27:41]

Add:
TODO(solenberg): SetSendBitrate() yields another call to SetSendCodec(). Check
if it is possible to fuse with the previous call in this function.

[skvlad, 2016/04/07 00:50:50]

Done.

+                 MinPositive(send_bitrate_bps_,
+                             rtp_parameters.encodings[0].max_bitrate_bps));
 
   // Set the CN payloadtype and the VAD status.
   if (send_codec_spec_.cng_payload_type != -1) {
@@ -1880,13 +1939,14 @@ bool WebRtcVoiceMediaChannel::AddSendStream(const StreamParams& sp) {
   // delete the channel in case failure happens below.
   webrtc::AudioTransport* audio_transport =
       engine()->voe()->base()->audio_transport();
-  send_streams_.insert(std::make_pair(ssrc, new WebRtcAudioSendStream(
-      channel, audio_transport, ssrc, sp.cname, send_rtp_extensions_, call_)));
+  WebRtcAudioSendStream* stream = new WebRtcAudioSendStream(
+      channel, audio_transport, ssrc, sp.cname, send_rtp_extensions_, call_);
+  send_streams_.insert(std::make_pair(ssrc, stream));
 
   // Set the current codecs to be used for the new channel. We need to do this
   // after adding the channel to send_channels_, because of how max bitrate is
   // currently being configured by SetSendCodec().
-  if (HasSendCodec() && !SetSendCodecs(channel)) {
+  if (HasSendCodec() && !SetSendCodecs(channel, stream->rtp_parameters())) {
     RemoveSendStream(ssrc);
     return false;
   }
@@ -2310,43 +2370,45 @@ bool WebRtcVoiceMediaChannel::MuteStream(uint32_t ssrc, bool muted) {
   return true;
 }
 
-// TODO(minyue): SetMaxSendBandwidth() is subject to be renamed to
-// SetMaxSendBitrate() in future.
-bool WebRtcVoiceMediaChannel::SetMaxSendBandwidth(int bps) {
-  LOG(LS_INFO) << "WebRtcVoiceMediaChannel::SetMaxSendBandwidth.";
-  return SetSendBitrateInternal(bps);
-}
-
-bool WebRtcVoiceMediaChannel::SetSendBitrateInternal(int bps) {
-  LOG(LS_INFO) << "WebRtcVoiceMediaChannel::SetSendBitrateInternal.";
-
-  send_bitrate_setting_ = true;
+bool WebRtcVoiceMediaChannel::SetSendBitrate(int bps) {
+  LOG(LS_INFO) << "WebRtcVoiceMediaChannel::SetSendBitrate.";
   send_bitrate_bps_ = bps;
 
-  if (!HasSendCodec()) {
-    LOG(LS_INFO) << "The send codec has not been set up yet. "
-                 << "The send bitrate setting will be applied later.";
-    return true;
+  for (const auto& ch : send_streams_) {

[the sun, 2016/04/06 09:27:41]

super nit: use 'kv' for the variable instead (sorry, the code isn't all updated
to reflect we're iterating over ssrc+stream key-value pairs, instead of
channels, as it use to be).

[skvlad, 2016/04/07 00:50:50]

Done.

+    int channel_bitrate =
+        MinPositive(send_bitrate_bps_,
+                    ch.second->rtp_parameters().encodings[0].max_bitrate_bps);
+
+    if (!SetSendBitrate(ch.second->channel(), channel_bitrate)) {
+      return false;
+    }
   }
+  return true;
+}
 
+bool WebRtcVoiceMediaChannel::SetSendBitrate(int channel, int bps) {

[the sun, 2016/04/06 09:27:41]

How about you send in the const RtpParameters& here so you don't have to repeat
the channel_bitrate computation?

[skvlad, 2016/04/07 00:50:49]

Done.

   // Bitrate is auto by default.
   // TODO(bemasc): Fix this so that if SetMaxSendBandwidth(50) is followed by
   // SetMaxSendBandwith(0), the second call removes the previous limit.
   if (bps <= 0)
     return true;
 
+  if (!HasSendCodec()) {
+    LOG(LS_INFO) << "The send codec has not been set up yet. "
+                 << "The send bitrate setting will be applied later.";
+    return true;
+  }
+
   webrtc::CodecInst codec = send_codec_spec_.codec_inst;
   bool is_multi_rate = WebRtcVoiceCodecs::IsCodecMultiRate(codec);
 
   if (is_multi_rate) {
     // If codec is multi-rate then just set the bitrate.
     codec.rate = bps;
-    for (const auto& ch : send_streams_) {

[the sun, 2016/04/06 09:27:41]

Great to remove this!

-      if (!SetSendCodec(ch.second->channel(), codec)) {
-        LOG(LS_INFO) << "Failed to set codec " << codec.plname
-                     << " to bitrate " << bps << " bps.";
-        return false;
-      }
+    if (!SetSendCodec(channel, codec)) {
+      LOG(LS_INFO) << "Failed to set codec " << codec.plname << " to bitrate "
+                   << bps << " bps.";
+      return false;
     }
     return true;
   } else {