Hooking up audio network adaptor to VoE.

webrtc/modules/audio_coding/codecs/audio_encoder.cc

Index: webrtc/modules/audio_coding/codecs/audio_encoder.cc
diff --git a/webrtc/modules/audio_coding/codecs/audio_encoder.cc b/webrtc/modules/audio_coding/codecs/audio_encoder.cc
index 1216484b4d189d3dd3b93247019851db3a3f17f5..956c4e086f0dbf244cb348ad93083d19ad252411 100644
--- a/webrtc/modules/audio_coding/codecs/audio_encoder.cc
+++ b/webrtc/modules/audio_coding/codecs/audio_encoder.cc
@@ -77,9 +77,13 @@ void AudioEncoder::DisableAudioNetworkAdaptor() {}
 void AudioEncoder::OnReceivedUplinkBandwidth(int uplink_bandwidth_bps) {}
 
 void AudioEncoder::OnReceivedUplinkPacketLossFraction(
-    float uplink_packet_loss_fraction) {}
+    float uplink_packet_loss_fraction) {
+  SetProjectedPacketLossRate(uplink_packet_loss_fraction);

[minyue-webrtc, 2016/10/04 11:03:55]

Change the default behavior to call SetProjectedPacketLossRate() so that
channel.cc needs to call OnReceivedUplinkPacketLossFraction() only.

[kwiberg-webrtc, 2016/10/06 09:46:13]

Hmmm... would anyone ever have any reason to override this method to make it do
something other than this default? If we can't think of any such reason, that
may be a sign that there should be just one method...

[minyue-webrtc, 2016/10/06 10:32:38]

See OnReceivedTargetAudioBitrate, which is easier to explain.

+}
 
-void AudioEncoder::OnReceivedTargetAudioBitrate(int target_audio_bitrate_bps) {}
+void AudioEncoder::OnReceivedTargetAudioBitrate(int target_audio_bitrate_bps) {
+  SetTargetBitrate(target_audio_bitrate_bps);

[minyue-webrtc, 2016/10/04 11:03:55]

Change the default behavior to call SetTargetBitrate() so that
channel.cc needs to call OnReceivedTargetAudioBitrate() only.

[kwiberg-webrtc, 2016/10/06 09:46:13]

Same question to ponder as for OnReceivedUplinkPacketLossFraction.

[minyue-webrtc, 2016/10/06 10:32:38]

SetTargetBitrate has a defined behavior, which is to change encoder's bit rate,
while OnReceivedTargetAudioBitrate can invoke AudioNetworkAdaptor.

Of course, we may consider putting such logic in this default behavior. But then
AudioNetworkAdaptor should be defined in AudioEncoder, while currently it only
belongs to AudioEncoderOpus.

[kwiberg-webrtc, 2016/10/06 10:57:11]

No, SetTargetBitrate is defined like this:

  // Tells the encoder what average bitrate we'd like it to produce. The
  // encoder is free to adjust or disregard the given bitrate (the default
  // implementation does the latter).
  virtual void SetTargetBitrate(int target_bps);

How is this different from OnReceivedTargetAudioBitrate:

  // Provides target audio bitrate to this encoder to allow it to adapt.
  virtual void OnReceivedTargetAudioBitrate(int target_audio_bitrate_bps);

The way I read it, both of them say, "Here's a target bitrate that I'd like you
to meet if you feel like it." Whether the encoder does so by making use of an
internal ANA object or not is not something the interface should be concerned
with.

The packet loss methods are also similar in exactly this fashion.

Sorry for not catching this in the review of the CL that adds these methods.

[minyue-webrtc, 2016/10/11 09:00:22]

Per offline discussion, we may replace SetTargetBitrate and
SetProjectedPacketLossRate with OnReceivedTargetAudioBitrate and
OnReceivedUplinkPacketLossFraction. This will invoke a PSA and it will be
carried out in a follow-up CL: https://codereview.webrtc.org/2411613002/

+}
 
 void AudioEncoder::OnReceivedRtt(int rtt_ms) {}