Injectable audio encoders: WebRtcVoiceEngine and company

webrtc/audio/audio_send_stream.cc

 /*
  *  Copyright (c) 2015 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 12 matching lines @@
 #include "webrtc/modules/congestion_controller/include/congestion_controller.h"
 #include "webrtc/modules/pacing/paced_sender.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "webrtc/voice_engine/channel_proxy.h"
 #include "webrtc/voice_engine/include/voe_base.h"
 #include "webrtc/voice_engine/include/voe_volume_control.h"
 #include "webrtc/voice_engine/voice_engine_impl.h"
 
 namespace webrtc {
 
-namespace {
-
-constexpr char kOpusCodecName[] = "opus";
-
-bool IsCodec(const webrtc::CodecInst& codec, const char* ref_name) {
-  return (STR_CASE_CMP(codec.plname, ref_name) == 0);
-}
-}  // namespace
-
 namespace internal {
 AudioSendStream::AudioSendStream(
     const webrtc::AudioSendStream::Config& config,
     const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
     rtc::TaskQueue* worker_queue,
     PacketRouter* packet_router,
     CongestionController* congestion_controller,
     BitrateAllocator* bitrate_allocator,
     RtcEventLog* event_log,
     RtcpRttStats* rtcp_rtt_stats)
@@ skipping 226 matching lines @@
   channel_proxy_->SetVADStatus(false);
   channel_proxy_->SetCodecFECStatus(false);
 
   // We disable audio network adaptor here. This will on one hand make sure that
   // audio network adaptor is disabled by default, and on the other allow audio
   // network adaptor to be reconfigured, since SetReceiverFrameLengthRange can
   // be only called when audio network adaptor is disabled.
   channel_proxy_->DisableAudioNetworkAdaptor();
 
   const auto& send_codec_spec = config_.send_codec_spec;
-
-  // We set the codec first, since the below extra configuration is only applied
-  // to the "current" codec.
-
-  // If codec is already configured, we do not it again.
-  // TODO(minyue): check if this check is really needed, or can we move it into
-  // |codec->SetSendCodec|.
-  webrtc::CodecInst current_codec = {0};
-  if (!channel_proxy_->GetSendCodec(&current_codec) ||
-      (send_codec_spec.codec_inst != current_codec)) {
-    if (!channel_proxy_->SetSendCodec(send_codec_spec.codec_inst)) {
-      LOG(LS_WARNING) << "SetSendCodec() failed.";
-      return false;
-    }
+  channel_proxy_->SetSendFormat(send_codec_spec.payload_type,

[the sun, 2017/02/22 14:35:01]

I'd like to get to a point when AudioSendStream doesn't create the encoder, but
that is taken care of in WVoMC. Perhaps I'm naive but I thought some of the
things below (SetBitrate, or anything that calls ModifyEncoder) and which is not
dynamic, would be handled by the encoder factory or the encoder's factory
method.

[ossu, 2017/02/22 15:03:35]

I've not be able to satisfactorily solve the bitrate thing, unfortunately. I was
hoping to combine it with other bandwidth related things into a struct that
could be set during construction and updated dynamically. Now it looks like
we're trying to have the encoder listen directly to the bandwidth estimator,
which I believe is good. Then we won't have to do plumbing through several
layers each time something in that interface changes.

So I've left the bitrate thing for now. I'm not sure how widely it's used.
Nothing in our code seems to set AudioCodec.bitrate to anything but 0 or what's
taken from AcmCodecDB (which is static). It might be that we can remove this
call.

The only other ModifyEncoder call is EnableAudioNetworkAdaptor. Since it's still
a WIP, I've left it as-is for now. At least it doesn't rely on all the other
Setters being called first, in the correct order.

+                                send_codec_spec.format.format,
+                                config_.encoder_factory.get());
+  // TODO(ossu): Formalize bandwidth parameters and send along to encoder
+  // constructor.
+  if (send_codec_spec.target_bitrate_bps) {
+    channel_proxy_->SetBitrate(*send_codec_spec.target_bitrate_bps, 0);
   }
-
-  // Codec internal FEC. Treat any failure as fatal internal error.
-  if (send_codec_spec.enable_codec_fec) {
-    if (!channel_proxy_->SetCodecFECStatus(true)) {
-      LOG(LS_WARNING) << "SetCodecFECStatus() failed.";
-      return false;
-    }
-  }
-
-  // DTX and maxplaybackrate are only set if current codec is Opus.
-  if (IsCodec(send_codec_spec.codec_inst, kOpusCodecName)) {
-    if (!channel_proxy_->SetOpusDtx(send_codec_spec.enable_opus_dtx)) {
-      LOG(LS_WARNING) << "SetOpusDtx() failed.";
-      return false;
-    }
-
-    // If opus_max_playback_rate <= 0, the default maximum playback rate
-    // (48 kHz) will be used.
-    if (send_codec_spec.opus_max_playback_rate > 0) {
-      if (!channel_proxy_->SetOpusMaxPlaybackRate(
-              send_codec_spec.opus_max_playback_rate)) {
-        LOG(LS_WARNING) << "SetOpusMaxPlaybackRate() failed.";
-        return false;
-      }
-    }
-
-    if (config_.audio_network_adaptor_config) {
-      // Audio network adaptor is only allowed for Opus currently.
-      // |SetReceiverFrameLengthRange| needs to be called before
-      // |EnableAudioNetworkAdaptor|.
-      channel_proxy_->SetReceiverFrameLengthRange(send_codec_spec.min_ptime_ms,
-                                                  send_codec_spec.max_ptime_ms);
-      channel_proxy_->EnableAudioNetworkAdaptor(
-          *config_.audio_network_adaptor_config);
-      LOG(LS_INFO) << "Audio network adaptor enabled on SSRC "
-                   << config_.rtp.ssrc;
-    }
+  if (config_.audio_network_adaptor_config) {
+    // Audio network adaptor is only allowed for Opus currently.
+    channel_proxy_->EnableAudioNetworkAdaptor(
+        *config_.audio_network_adaptor_config);
+    LOG(LS_INFO) << "Audio network adaptor enabled on SSRC "
+                 << config_.rtp.ssrc;
   }
 
   // Set the CN payloadtype and the VAD status.
   if (send_codec_spec.cng_payload_type != -1) {
     // The CN payload type for 8000 Hz clockrate is fixed at 13.
     if (send_codec_spec.cng_plfreq != 8000) {
       webrtc::PayloadFrequencies cn_freq;
       switch (send_codec_spec.cng_plfreq) {
         case 16000:
           cn_freq = webrtc::kFreq16000Hz;
@@ skipping 14 matching lines @@
         // Not returning false because the SetSendCNPayloadType will fail if
         // the channel is already sending.
         // This can happen if the remote description is applied twice, for
         // example in the case of ROAP on top of JSEP, where both side will
         // send the offer.
       }
     }
 
     // Only turn on VAD if we have a CN payload type that matches the
     // clockrate for the codec we are going to use.
-    if (send_codec_spec.cng_plfreq == send_codec_spec.codec_inst.plfreq &&
-        send_codec_spec.codec_inst.channels == 1) {
+    if (send_codec_spec.cng_plfreq ==
+            send_codec_spec.format.format.clockrate_hz &&
+        send_codec_spec.format.info.num_channels == 1) {
       // TODO(minyue): If CN frequency == 48000 Hz is allowed, consider the
       // interaction between VAD and Opus FEC.
       if (!channel_proxy_->SetVADStatus(true)) {
         LOG(LS_WARNING) << "SetVADStatus() failed.";
         return false;
       }
     }
   }
   return true;
 }
 
 }  // namespace internal
 }  // namespace webrtc