Switch to using new ACM methods for encoder management

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 5bbbce3c406b1c8ebfbbd3ef2e7533e335e836de..ed1a64b337da0057a27e2a963a2fe3fea1f701fa 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -391,7 +391,9 @@ int32_t Channel::OnInitializeDecoder(
   receiveCodec.pacsize = dummyCodec.pacsize;
 
   // Register the new codec to the ACM
-  if (audio_coding_->RegisterReceiveCodec(receiveCodec) == -1) {
+  if (audio_coding_->RegisterReceiveCodec(receiveCodec, [&] {
+        return rent_a_codec_.RentIsacDecoder();
+      }) == -1) {
     WEBRTC_TRACE(kTraceWarning, kTraceVoice, VoEId(_instanceId, _channelId),
                  "Channel::OnInitializeDecoder() invalid codec ("
                  "pt=%d, name=%s) received - 1",
@@ -966,8 +968,9 @@ int32_t Channel::Init() {
 
     // Register default PT for outband 'telephone-event'
     if (!STR_CASE_CMP(codec.plname, "telephone-event")) {
-      if ((_rtpRtcpModule->RegisterSendPayload(codec) == -1) ||
-          (audio_coding_->RegisterReceiveCodec(codec) == -1)) {
+      if (_rtpRtcpModule->RegisterSendPayload(codec) == -1 ||
+          audio_coding_->RegisterReceiveCodec(
+              codec, [&] { return rent_a_codec_.RentIsacDecoder(); }) == -1) {
         WEBRTC_TRACE(kTraceWarning, kTraceVoice, VoEId(_instanceId, _channelId),
                      "Channel::Init() failed to register outband "
                      "'telephone-event' (%d/%d) correctly",
@@ -976,9 +979,15 @@ int32_t Channel::Init() {
     }
 
     if (!STR_CASE_CMP(codec.plname, "CN")) {
-      if ((audio_coding_->RegisterSendCodec(codec) == -1) ||
-          (audio_coding_->RegisterReceiveCodec(codec) == -1) ||
-          (_rtpRtcpModule->RegisterSendPayload(codec) == -1)) {
+      if (!codec_manager_.RegisterEncoder(codec) ||
+          !codec_manager_.MakeEncoder(&rent_a_codec_,
+                                      [&](std::unique_ptr<AudioEncoder> enc) {
+                                        audio_coding_->SetEncoder(
+                                            std::move(enc));
+                                      }) ||
+          audio_coding_->RegisterReceiveCodec(
+              codec, [&] { return rent_a_codec_.RentIsacDecoder(); }) == -1 ||
+          _rtpRtcpModule->RegisterSendPayload(codec) == -1) {
         WEBRTC_TRACE(kTraceWarning, kTraceVoice, VoEId(_instanceId, _channelId),
                      "Channel::Init() failed to register CN (%d/%d) "
                      "correctly - 1",
@@ -1195,7 +1204,7 @@ int32_t Channel::DeRegisterVoiceEngineObserver() {
 }
 
 int32_t Channel::GetSendCodec(CodecInst& codec) {
-  auto send_codec = audio_coding_->SendCodec();
+  auto send_codec = codec_manager_.GetCodecInst();
   if (send_codec) {
     codec = *send_codec;
     return 0;
@@ -1211,7 +1220,11 @@ int32_t Channel::SetSendCodec(const CodecInst& codec) {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
                "Channel::SetSendCodec()");
 
-  if (audio_coding_->RegisterSendCodec(codec) != 0) {
+  if (!codec_manager_.RegisterEncoder(codec) ||
+      !codec_manager_.MakeEncoder(
+          &rent_a_codec_, [&](std::unique_ptr<AudioEncoder> new_enc) {
+            audio_coding_->SetEncoder(std::move(new_enc));
+          })) {
     WEBRTC_TRACE(kTraceError, kTraceVoice, VoEId(_instanceId, _channelId),
                  "SetSendCodec() failed to register codec to ACM");
     return -1;
@@ -1258,10 +1271,11 @@ int32_t Channel::SetVADStatus(bool enableVAD,
                               bool disableDTX) {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
                "Channel::SetVADStatus(mode=%d)", mode);
-  assert(!(disableDTX && enableVAD));  // disableDTX mode is deprecated.

[hlundin-webrtc, 2016/03/29 20:23:32]

Why did the assert have to go? The last parameter is still deprecated, and now
also unused. Maybe document it in the interface file (if you cannot remove the
parameter now).

[kwiberg-webrtc, 2016/03/29 23:25:32]

Good question. I remember removing it for a reason, but I can’t recall what
reason... I’ll restore it (as a DCHECK).

[hlundin-webrtc, 2016/03/30 08:36:10]

Good.

[kwiberg-webrtc, 2016/03/30 10:53:50]

Done.

-  // To disable VAD, DTX must be disabled too
-  disableDTX = ((enableVAD == false) ? true : disableDTX);
-  if (audio_coding_->SetVAD(!disableDTX, enableVAD, mode) != 0) {
+  if (!codec_manager_.SetVAD(enableVAD, mode) ||
+      !codec_manager_.MakeEncoder(
+          &rent_a_codec_, [&](std::unique_ptr<AudioEncoder> new_enc) {
+            audio_coding_->SetEncoder(std::move(new_enc));
+          })) {
     _engineStatisticsPtr->SetLastError(VE_AUDIO_CODING_MODULE_ERROR,
                                        kTraceError,
                                        "SetVADStatus() failed to set VAD");
@@ -1273,13 +1287,10 @@ int32_t Channel::SetVADStatus(bool enableVAD,
 int32_t Channel::GetVADStatus(bool& enabledVAD,
                               ACMVADMode& mode,
                               bool& disabledDTX) {
-  if (audio_coding_->VAD(&disabledDTX, &enabledVAD, &mode) != 0) {
-    _engineStatisticsPtr->SetLastError(
-        VE_AUDIO_CODING_MODULE_ERROR, kTraceError,
-        "GetVADStatus() failed to get VAD status");
-    return -1;
-  }
-  disabledDTX = !disabledDTX;
+  const auto* params = codec_manager_.GetStackParams();
+  enabledVAD = params->use_cng;
+  mode = params->vad_mode;
+  disabledDTX = !params->use_cng;
   return 0;
 }
 
@@ -1342,9 +1353,11 @@ int32_t Channel::SetRecPayloadType(const CodecInst& codec) {
       return -1;
     }
   }
-  if (audio_coding_->RegisterReceiveCodec(codec) != 0) {
+  if (audio_coding_->RegisterReceiveCodec(

[hlundin-webrtc, 2016/03/29 20:23:32]

One could wonder if there is any valid case for registering a decoder with the
same RTP payload type as an already registered one that warrants this try-retry
construct. I could only come up with an historical reason, where we used to
allow registering CNG at different sample rates to one and the same payload
type. But that case should be no more.

[kwiberg-webrtc, 2016/03/29 23:25:32]

I don’t know of a valid case either.

[hlundin-webrtc, 2016/03/30 08:36:10]

Right. You may want to add a TODO that this is essentially the same that is done
in AcmReceiver::AddCodec. It's up to you.

[kwiberg-webrtc, 2016/03/30 10:53:50]

Done.

+          codec, [&] { return rent_a_codec_.RentIsacDecoder(); }) != 0) {
     audio_coding_->UnregisterReceiveCodec(codec.pltype);
-    if (audio_coding_->RegisterReceiveCodec(codec) != 0) {
+    if (audio_coding_->RegisterReceiveCodec(
+            codec, [&] { return rent_a_codec_.RentIsacDecoder(); }) != 0) {
       _engineStatisticsPtr->SetLastError(
           VE_AUDIO_CODING_MODULE_ERROR, kTraceError,
           "SetRecPayloadType() ACM registration failed - 1");
@@ -1391,7 +1404,11 @@ int32_t Channel::SetSendCNPayloadType(int type, PayloadFrequencies frequency) {
   // Modify the payload type (must be set to dynamic range)
   codec.pltype = type;
 
-  if (audio_coding_->RegisterSendCodec(codec) != 0) {
+  if (!codec_manager_.RegisterEncoder(codec) ||
+      !codec_manager_.MakeEncoder(&rent_a_codec_,
+                                  [&](std::unique_ptr<AudioEncoder> enc) {
+                                    audio_coding_->SetEncoder(std::move(enc));
+                                  })) {
     _engineStatisticsPtr->SetLastError(
         VE_AUDIO_CODING_MODULE_ERROR, kTraceError,
         "SetSendCNPayloadType() failed to register CN to ACM");
@@ -2854,7 +2871,11 @@ int Channel::SetREDStatus(bool enable, int redPayloadtype) {
     }
   }
 
-  if (audio_coding_->SetREDStatus(enable) != 0) {
+  if (!codec_manager_.SetCopyRed(enable) ||
+      !codec_manager_.MakeEncoder(
+          &rent_a_codec_, [&](std::unique_ptr<AudioEncoder> new_enc) {
+            audio_coding_->SetEncoder(std::move(new_enc));
+          })) {
     _engineStatisticsPtr->SetLastError(
         VE_AUDIO_CODING_MODULE_ERROR, kTraceError,
         "SetREDStatus() failed to set RED state in the ACM");
@@ -2864,7 +2885,7 @@ int Channel::SetREDStatus(bool enable, int redPayloadtype) {
 }
 
 int Channel::GetREDStatus(bool& enabled, int& redPayloadtype) {
-  enabled = audio_coding_->REDStatus();
+  enabled = codec_manager_.GetStackParams()->use_red;
   if (enabled) {
     int8_t payloadType = 0;
     if (_rtpRtcpModule->SendREDPayloadType(&payloadType) != 0) {
@@ -2884,7 +2905,11 @@ int Channel::SetCodecFECStatus(bool enable) {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
                "Channel::SetCodecFECStatus()");
 
-  if (audio_coding_->SetCodecFEC(enable) != 0) {
+  if (!codec_manager_.SetCodecFEC(enable) ||
+      !codec_manager_.MakeEncoder(
+          &rent_a_codec_, [&](std::unique_ptr<AudioEncoder> new_enc) {
+            audio_coding_->SetEncoder(std::move(new_enc));
+          })) {
     _engineStatisticsPtr->SetLastError(
         VE_AUDIO_CODING_MODULE_ERROR, kTraceError,
         "SetCodecFECStatus() failed to set FEC state");
@@ -2894,8 +2919,7 @@ int Channel::SetCodecFECStatus(bool enable) {
 }
 
 bool Channel::GetCodecFECStatus() {
-  bool enabled = audio_coding_->CodecFEC();
-  return enabled;
+  return codec_manager_.GetStackParams()->use_codec_fec;
 }
 
 void Channel::SetNACKStatus(bool enable, int maxNumberOfPackets) {
@@ -3437,7 +3461,11 @@ int Channel::SetRedPayloadType(int red_payload_type) {
   }
 
   codec.pltype = red_payload_type;
-  if (audio_coding_->RegisterSendCodec(codec) < 0) {
+  if (!codec_manager_.RegisterEncoder(codec) ||
+      !codec_manager_.MakeEncoder(
+          &rent_a_codec_, [&](std::unique_ptr<AudioEncoder> new_enc) {
+            audio_coding_->SetEncoder(std::move(new_enc));
+          })) {
     _engineStatisticsPtr->SetLastError(
         VE_AUDIO_CODING_MODULE_ERROR, kTraceError,
         "SetRedPayloadType() RED registration in ACM module failed");