Injectable audio encoders: WebRtcVoiceEngine and company

webrtc/voice_engine/channel.cc

Index: webrtc/voice_engine/channel.cc
diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc
index 8c9b00179f37c705a4356475d3baaa9aef52da7b..f56d1d822c84f24d1534791c38f02cc3fa74f4fd 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -1303,9 +1303,15 @@ bool Channel::SetEncoder(int payload_type,
   }
 
   audio_coding_->SetEncoder(std::move(encoder));
+  codec_manager_.UnsetCodecInst();
   return true;
 }
 
+void Channel::ModifyEncoder(
+    rtc::FunctionView<void(std::unique_ptr<AudioEncoder>*)> modifier) {
+  audio_coding_->ModifyEncoder(modifier);
+}
+
 int32_t Channel::RegisterVoiceEngineObserver(VoiceEngineObserver& observer) {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
                "Channel::RegisterVoiceEngineObserver()");
@@ -1337,9 +1343,16 @@ int32_t Channel::DeRegisterVoiceEngineObserver() {
 }
 
 int32_t Channel::GetSendCodec(CodecInst& codec) {
-  auto send_codec = codec_manager_.GetCodecInst();
-  if (send_codec) {
-    codec = *send_codec;
+  {

[ossu, 2017/04/26 15:45:36]

At first, I had this code just ask the ACM directly, however it turns out that
when Channel does SetSendCodec, all the codec info within the ACM is discarded,
and it's treated as just another external codec. The ACM has its own
codec_manager_ instance it (I guess) sometimes uses to create encoders.

Now, we'll use the CodecInst passed to SetSendCodec if it's available (it's set
up with PCMU as the default codec in Channel::Init), or ask the ACM for info if
it's not - since it has the ability to make something reasonable up for external
encoders.

When calling Channel::SetEncoder, it will ensure we discard any previously
configured CodecInst by calling CodecManager::UnsetCodecInst() above.

[kwiberg-webrtc, 2017/04/26 20:03:36]

Yes. Hopefully we can remove that soon-ish.
Ack.

We really need to make it a priority to remove all uses of CodecInst sooner
rather than later; this sort of code feels like it could cause much weeping and
gnashing of teeth the next time we need to change it and have to figure out how
it works...

+    const CodecInst* send_codec = codec_manager_.GetCodecInst();
+    if (send_codec) {
+      codec = *send_codec;
+      return 0;
+    }
+  }
+  rtc::Optional<CodecInst> acm_send_codec = audio_coding_->SendCodec();
+  if (acm_send_codec) {
+    codec = *acm_send_codec;
     return 0;
   }
   return -1;