 Chromium Code Reviews
 Chromium Code Reviews Issue 2362703002:
  Adding audio network adaptor to AudioEncoderOpus.  (Closed)
    
  
    Issue 2362703002:
  Adding audio network adaptor to AudioEncoderOpus.  (Closed) 
  | Index: webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc | 
| diff --git a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc | 
| index d03f2d3cc2440c3d5fa029e91c4dd037fcf54c96..46256cc98696aef325ba31f6d801ae8bb740b7fd 100644 | 
| --- a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc | 
| +++ b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc | 
| @@ -33,6 +33,7 @@ AudioEncoderOpus::Config CreateConfig(const CodecInst& codec_inst) { | 
| config.payload_type = codec_inst.pltype; | 
| config.application = config.num_channels == 1 ? AudioEncoderOpus::kVoip | 
| : AudioEncoderOpus::kAudio; | 
| + config.audio_network_adaptor_enabled = false; | 
| return config; | 
| } | 
| @@ -104,13 +105,17 @@ int AudioEncoderOpus::Config::GetBitrateBps() const { | 
| return num_channels == 1 ? 32000 : 64000; // Default value. | 
| } | 
| -AudioEncoderOpus::AudioEncoderOpus(const Config& config) | 
| - : packet_loss_rate_(0.0), inst_(nullptr) { | 
| +AudioEncoderOpus::AudioEncoderOpus( | 
| + const Config& config, | 
| + std::unique_ptr<AudioNetworkAdaptor> audio_network_adaptor) | 
| + : packet_loss_rate_(0.0), | 
| + inst_(nullptr), | 
| + audio_network_adaptor_(std::move(audio_network_adaptor)) { | 
| RTC_CHECK(RecreateEncoderInstance(config)); | 
| } | 
| AudioEncoderOpus::AudioEncoderOpus(const CodecInst& codec_inst) | 
| - : AudioEncoderOpus(CreateConfig(codec_inst)) {} | 
| + : AudioEncoderOpus(CreateConfig(codec_inst), nullptr) {} | 
| AudioEncoderOpus::~AudioEncoderOpus() { | 
| RTC_CHECK_EQ(0, WebRtcOpus_EncoderFree(inst_)); | 
| @@ -141,15 +146,23 @@ void AudioEncoderOpus::Reset() { | 
| } | 
| bool AudioEncoderOpus::SetFec(bool enable) { | 
| - auto conf = config_; | 
| - conf.fec_enabled = enable; | 
| - return RecreateEncoderInstance(conf); | 
| + if (enable) { | 
| + RTC_CHECK_EQ(0, WebRtcOpus_EnableFec(inst_)); | 
| + } else { | 
| + RTC_CHECK_EQ(0, WebRtcOpus_DisableFec(inst_)); | 
| + } | 
| + config_.fec_enabled = enable; | 
| + return true; | 
| } | 
| bool AudioEncoderOpus::SetDtx(bool enable) { | 
| - auto conf = config_; | 
| - conf.dtx_enabled = enable; | 
| - return RecreateEncoderInstance(conf); | 
| + if (enable) { | 
| + RTC_CHECK_EQ(0, WebRtcOpus_EnableDtx(inst_)); | 
| + } else { | 
| + RTC_CHECK_EQ(0, WebRtcOpus_DisableDtx(inst_)); | 
| + } | 
| + config_.dtx_enabled = enable; | 
| + return true; | 
| } | 
| bool AudioEncoderOpus::GetDtx() const { | 
| @@ -192,6 +205,63 @@ void AudioEncoderOpus::SetTargetBitrate(int bits_per_second) { | 
| RTC_CHECK_EQ(0, WebRtcOpus_SetBitRate(inst_, config_.GetBitrateBps())); | 
| } | 
| +bool AudioEncoderOpus::SetAudioNetworkAdaptor(bool enable) { | 
| + if (config_.audio_network_adaptor_enabled == enable) | 
| + return true; | 
| + config_.audio_network_adaptor_enabled = enable; | 
| + if (config_.audio_network_adaptor_enabled) { | 
| + // TODO(minyue): Create AudioNetworkAdaptorImpl. | 
| + } else { | 
| + audio_network_adaptor_.reset(nullptr); | 
| + } | 
| + return true; | 
| +} | 
| 
kwiberg-webrtc
2016/09/27 09:35:54
Hmm. I don't quite understand in what circumstance
 
minyue-webrtc
2016/09/27 16:02:33
My intention is to make it hook up in similar mann
 | 
| + | 
| +void AudioEncoderOpus::OnReceivedUplinkBandwidth(int uplink_bandwidth_bps) { | 
| + if (!config_.audio_network_adaptor_enabled) | 
| + return; | 
| + RTC_DCHECK(audio_network_adaptor_); | 
| + audio_network_adaptor_->SetUplinkBandwidth(uplink_bandwidth_bps); | 
| + ApplyAudioNetworkAdaptor(); | 
| +} | 
| + | 
| +void AudioEncoderOpus::OnReceivedUplinkPacketLossFraction( | 
| + float uplink_packet_loss_fraction) { | 
| + if (!config_.audio_network_adaptor_enabled) | 
| + return; | 
| + RTC_DCHECK(audio_network_adaptor_); | 
| + audio_network_adaptor_->SetUplinkPacketLossFraction( | 
| + uplink_packet_loss_fraction); | 
| + ApplyAudioNetworkAdaptor(); | 
| +} | 
| + | 
| +void AudioEncoderOpus::OnReceivedTargetAudioBitrate( | 
| + int target_audio_bitrate_bps) { | 
| + if (!config_.audio_network_adaptor_enabled) | 
| + return; | 
| + RTC_DCHECK(audio_network_adaptor_); | 
| + audio_network_adaptor_->SetTargetAudioBitrate(target_audio_bitrate_bps); | 
| + ApplyAudioNetworkAdaptor(); | 
| +} | 
| + | 
| +void AudioEncoderOpus::OnReceivedRtt(int rtt_ms) { | 
| + if (!config_.audio_network_adaptor_enabled) | 
| + return; | 
| + RTC_DCHECK(audio_network_adaptor_); | 
| + audio_network_adaptor_->SetRtt(rtt_ms); | 
| + ApplyAudioNetworkAdaptor(); | 
| +} | 
| + | 
| +void AudioEncoderOpus::SetReceiverFrameLengthRange(int min_frame_length_ms, | 
| + int max_frame_length_ms) { | 
| + if (!config_.audio_network_adaptor_enabled) | 
| + return; | 
| + RTC_DCHECK(audio_network_adaptor_); | 
| + audio_network_adaptor_->SetReceiverFrameLengthRange(min_frame_length_ms, | 
| + max_frame_length_ms); | 
| + ApplyAudioNetworkAdaptor(); | 
| +} | 
| + | 
| AudioEncoder::EncodedInfo AudioEncoderOpus::EncodeImpl( | 
| uint32_t rtp_timestamp, | 
| rtc::ArrayView<const int16_t> audio, | 
| @@ -226,6 +296,9 @@ AudioEncoder::EncodedInfo AudioEncoderOpus::EncodeImpl( | 
| }); | 
| input_buffer_.clear(); | 
| + // Will use new packet size for next encoding. | 
| + config_.frame_size_ms = next_frame_size_ms_; | 
| + | 
| info.encoded_timestamp = first_timestamp_in_buffer_; | 
| info.payload_type = config_.payload_type; | 
| info.send_even_if_empty = true; // Allows Opus to send empty packets. | 
| @@ -282,7 +355,42 @@ bool AudioEncoderOpus::RecreateEncoderInstance(const Config& config) { | 
| WebRtcOpus_SetPacketLossRate( | 
| inst_, static_cast<int32_t>(packet_loss_rate_ * 100 + .5))); | 
| config_ = config; | 
| + | 
| + num_channels_to_encode_ = 0; // Opus automatic mode. | 
| + next_frame_size_ms_ = config_.frame_size_ms; | 
| return true; | 
| } | 
| +void AudioEncoderOpus::SetFrameLength(int frame_length_ms) { | 
| + next_frame_size_ms_ = frame_length_ms; | 
| +} | 
| + | 
| +void AudioEncoderOpus::SetNumChannelsToEncode(size_t num_channels_to_encode) { | 
| + RTC_DCHECK_GT(num_channels_to_encode, 0u); | 
| + RTC_DCHECK_LE(num_channels_to_encode, config_.num_channels); | 
| + | 
| + if (num_channels_to_encode_ == num_channels_to_encode) | 
| + return; | 
| + | 
| + // RTC_CHECK_EQ(0, WebRtcOpus_SetForceChannels(inst_, | 
| + // num_channels_to_encode)); | 
| 
kwiberg-webrtc
2016/09/27 09:35:54
Remove commented-out code.
 
minyue-webrtc
2016/09/27 16:02:33
Yes, we can do it now. Since WebRtcOpus_SetForceCh
 | 
| + num_channels_to_encode_ = num_channels_to_encode; | 
| +} | 
| + | 
| +void AudioEncoderOpus::ApplyAudioNetworkAdaptor() { | 
| 
kwiberg-webrtc
2016/09/27 09:35:55
"ApplyAudioNetworkAdaptorConfig"?
 
minyue-webrtc
2016/09/27 16:02:33
I think ApplyAudioNetworkAdaptor may be better sin
 | 
| + auto config = audio_network_adaptor_->GetEncoderRuntimeConfig(); | 
| + | 
| + RTC_DCHECK(config.bitrate_bps && config.frame_length_ms && | 
| + config.uplink_packet_loss_fraction && config.enable_fec && | 
| + config.enable_dtx && config.num_channels); | 
| 
kwiberg-webrtc
2016/09/27 09:35:54
It's often better to split things like this up int
 
minyue-webrtc
2016/09/27 16:02:33
ok. will change in next patch set.
 | 
| + RTC_DCHECK(*config.frame_length_ms == 20 || *config.frame_length_ms == 60); | 
| + | 
| + SetTargetBitrate(*config.bitrate_bps); | 
| + SetFrameLength(*config.frame_length_ms); | 
| + SetFec(*config.enable_fec); | 
| + SetProjectedPacketLossRate(*config.uplink_packet_loss_fraction); | 
| + SetDtx(*config.enable_dtx); | 
| + SetNumChannelsToEncode(*config.num_channels); | 
| +} | 
| + | 
| } // namespace webrtc |