Wire up x-google-{min,start,max}-bitrate to WebRtcVoiceMediaChannel.

webrtc/media/engine/webrtcvoiceengine.cc

 /*
  *  Copyright (c) 2004 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 259 matching lines @@
 
   // If OPUS, change what we send according to the "stereo" codec
   // parameter, and not the "channels" parameter.  We set
   // voe_codec.channels to 2 if "stereo=1" and 1 otherwise.  If
   // the bitrate is not specified, i.e. is <= zero, we set it to the
   // appropriate default value for mono or stereo Opus.
   voe_codec->channels = IsCodecFeatureEnabled(codec, kCodecParamStereo) ? 2 : 1;
   voe_codec->rate = GetOpusBitrate(codec, *max_playback_rate);
 }
 
+webrtc::Call::Config::BitrateConfig GetBitrateConfigForCodec(
+    const AudioCodec& codec) {
+  webrtc::Call::Config::BitrateConfig config;
+  int bitrate_kbps;

[the sun, 2016/11/30 11:22:27]

nit: always default init

[stefan-webrtc, 2016/11/30 11:44:42]

Done.

+  if (codec.GetParam(kCodecParamMinBitrate, &bitrate_kbps) &&
+      bitrate_kbps > 0) {
+    config.min_bitrate_bps = bitrate_kbps * 1000;
+  } else {
+    config.min_bitrate_bps = 0;
+  }
+  if (codec.GetParam(kCodecParamStartBitrate, &bitrate_kbps) &&
+      bitrate_kbps > 0) {
+    config.start_bitrate_bps = bitrate_kbps * 1000;
+  } else {
+    // Do not reconfigure start bitrate unless it's specified and positive.
+    config.start_bitrate_bps = -1;

[the sun, 2016/11/30 11:22:27]

Change the optional fields in the config to use rtc::Optional

[stefan-webrtc, 2016/11/30 11:44:42]

I'd prefer making interface changes later.

+  }
+  if (codec.GetParam(kCodecParamMaxBitrate, &bitrate_kbps) &&
+      bitrate_kbps > 0) {
+    config.max_bitrate_bps = bitrate_kbps * 1000;
+  } else {
+    config.max_bitrate_bps = -1;
+  }
+  return config;
+}
+
 webrtc::AudioState::Config MakeAudioStateConfig(VoEWrapper* voe_wrapper) {
   webrtc::AudioState::Config config;
   config.voice_engine = voe_wrapper->engine();
   config.audio_mixer = webrtc::AudioMixerImpl::Create();
   return config;
 }
 
 class WebRtcVoiceCodecs final {
  public:
   // TODO(solenberg): Do this filtering once off-line, add a simple AudioCodec
@@ skipping 1319 matching lines @@
   RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
   LOG(LS_INFO) << "WebRtcVoiceMediaChannel::SetSendParameters: "
                << params.ToString();
   // TODO(pthatcher): Refactor this to be more clean now that we have
   // all the information at once.
 
   if (!SetSendCodecs(params.codecs)) {
     return false;
   }
 
+  if (params.max_bandwidth_bps >= 0) {

[the sun, 2016/11/30 11:22:27]

Is this logic the same for video? Does it make sense to make a util?

[stefan-webrtc, 2016/11/30 11:44:43]

It is the same, not sure about making it a util though... There are many small
things in these files that are similar for video and audio.

I made the GetBitrateConfigForCodec() a util though, that seems more natural.

+    // Note that max_bandwidth_bps intentionally takes priority over the
+    // bitrate config for the codec. This allows FEC to be applied above the

[minyue-webrtc, 2016/11/29 22:10:55]

Not sure about the reasoning on FEC in the comment.

[stefan-webrtc, 2016/11/30 11:44:43]

I made it a bit more generic.

+    // codec target bitrate.
+    bitrate_config_.max_bitrate_bps =
+        params.max_bandwidth_bps == 0 ? -1 : params.max_bandwidth_bps;
+  }
+  call_->SetBitrateConfig(bitrate_config_);
+
   if (!ValidateRtpExtensions(params.extensions)) {
     return false;
   }
   std::vector<webrtc::RtpExtension> filtered_extensions =
       FilterRtpExtensions(params.extensions,
                           webrtc::RtpExtension::IsSupportedForAudio, true);
   if (send_rtp_extensions_ != filtered_extensions) {
     send_rtp_extensions_.swap(filtered_extensions);
     for (auto& it : send_streams_) {
       it.second->RecreateAudioSendStream(send_rtp_extensions_);
@@ skipping 297 matching lines @@
     // playback rate, and opus internal dtx.
     if (IsCodec(*codec, kOpusCodecName)) {
       GetOpusConfig(*codec, &send_codec_spec.codec_inst,
                     &send_codec_spec.enable_codec_fec,
                     &send_codec_spec.opus_max_playback_rate,
                     &send_codec_spec.enable_opus_dtx,
                     &send_codec_spec.min_ptime_ms,
                     &send_codec_spec.max_ptime_ms);
     }
 
+    bitrate_config_ = GetBitrateConfigForCodec(*codec);
+    if (send_codec_spec_set_ && send_codec_spec == send_codec_spec_) {

[minyue-webrtc, 2016/11/29 22:10:55]

I don't think this is the place we reset codec. please consider line 1255

[the sun, 2016/11/30 11:22:27]

I don't get the logic involving send_codec_spec_set_. IIUC it is there so that
we never reset bw when the same codec is set multiple times. Can it be
simplified by always clearing bitrate_config_.start_bitrate_bps after pushing
the config to call_ (as well as clearing it here - or preferably at line
2039-ish - when codec specs are identical)?

[stefan-webrtc, 2016/11/30 11:44:42]

I tried to the changes into WebRtcAudioSendStream instead, but it prevents us
from using cricket::AudioCodec to determine the BitrateConfig, which makes it
difficult to reuse the same function for doing that for both audio and video.

Since there's no clear benefit of having it in WebRtcAudioSendStream I would
prefer this way.

[stefan-webrtc, 2016/11/30 11:44:43]

I don't think the send_codec_spec_set_ check is needed, removed it. That I think
makes it more like you suggested. Also moved it down.

+      // If the codec isn't changing, set the start bitrate to -1 which means
+      // "unchanged" so that BWE isn't affected.
+      bitrate_config_.start_bitrate_bps = -1;
+    }
+
     // Set packet size if the AudioCodec param kCodecParamPTime is set.
     int ptime_ms = 0;
     if (codec->GetParam(kCodecParamPTime, &ptime_ms)) {
       if (!WebRtcVoiceCodecs::SetPTimeAsPacketSize(
           &send_codec_spec.codec_inst, ptime_ms)) {
         LOG(LS_WARNING) << "Failed to set packet size for codec "
                         << send_codec_spec.codec_inst.plname;
         return false;
       }
     }
@@ skipping 37 matching lines @@
         dtmf_payload_type_ = rtc::Optional<int>(dtmf_codec.id);
         dtmf_payload_freq_ = dtmf_codec.clockrate;
         break;
       }
     }
   }
 
   // Apply new settings to all streams.
   if (send_codec_spec_ != send_codec_spec) {
     send_codec_spec_ = std::move(send_codec_spec);
+    send_codec_spec_set_ = true;
     for (const auto& kv : send_streams_) {
       kv.second->RecreateAudioSendStream(send_codec_spec_);
     }
   }
 
   // Check if the transport cc feedback or NACK status has changed on the
   // preferred send codec, and in that case reconfigure all receive streams.
   if (recv_transport_cc_enabled_ != send_codec_spec_.transport_cc_enabled ||
       recv_nack_enabled_ != send_codec_spec_.nack_enabled) {
     LOG(LS_INFO) << "Recreate all the receive streams because the send "
@@ skipping 635 matching lines @@
   RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
   const auto it = send_streams_.find(ssrc);
   if (it != send_streams_.end()) {
     return it->second->channel();
   }
   return -1;
 }
 }  // namespace cricket
 
 #endif  // HAVE_WEBRTC_VOICE