Move CNG and RED management into the Rent-A-Codec

webrtc/modules/audio_coding/main/acm2/codec_manager.cc

Index: webrtc/modules/audio_coding/main/acm2/codec_manager.cc
diff --git a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc
index ac21020dc2e3847b4e79b2dde50fad31f405092b..e4c2a5e402ce1376d725b71e7305b92a51bb60d2 100644
--- a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc
+++ b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc
@@ -240,7 +240,7 @@ int CodecManager::RegisterEncoder(const CodecInst& send_codec) {
 
   // Check if the codec is already registered as send codec.
   bool new_codec = true;
-  if (codec_owner_.Encoder()) {
+  if (CurrentEncoder()) {
     auto new_codec_id = RentACodec::CodecIdByInst(send_codec_inst_);
     RTC_DCHECK(new_codec_id);
     auto old_codec_id = RentACodec::CodecIdFromIndex(codec_id);
@@ -263,10 +263,14 @@ int CodecManager::RegisterEncoder(const CodecInst& send_codec) {
     AudioEncoder* enc = rent_a_codec_.RentEncoder(send_codec);
     if (!enc)
       return -1;
-    codec_owner_.SetEncoders(
-        enc, dtx_enabled_ ? CngPayloadType(send_codec.plfreq) : -1,
-        vad_mode_, red_enabled_ ? RedPayloadType(send_codec.plfreq) : -1);
-    RTC_DCHECK(codec_owner_.Encoder());
+    rent_a_codec_.RentEncoderStack(
+        enc, dtx_enabled_

[hlundin-webrtc, 2015/11/16 08:59:17]

The two conditionals in this expression are slightly horrible. And they return a
few lines further down. I think you should break them out into two private
member functions:

rtc::Optional<RentACodec::CngConfig> OptionalCngConfig(int
rtp_timestamp_rate_hz) const;

rtc::Optional<int> OptionalRedPayloadType(int rtp_timestamp_rate_hz) const;

Or, break out the whole expression into a private member function:
void RentEncoderStack(int rtp_timestamp_rate_hz) const {
  rent_a_codec_.RentEncoderStack(...);
  RTC_DCHECK(CurrentEncoder());
}

[kwiberg-webrtc, 2015/11/16 12:01:37]

Done.

+                 ? rtc::Optional<RentACodec::CngConfig>(RentACodec::CngConfig{
+                       CngPayloadType(send_codec.plfreq), vad_mode_})
+                 : rtc::Optional<RentACodec::CngConfig>(),
+        red_enabled_ ? rtc::Optional<int>(RedPayloadType(send_codec.plfreq))
+                     : rtc::Optional<int>());
+    RTC_DCHECK(CurrentEncoder());
 
     codec_fec_enabled_ = codec_fec_enabled_ &&
                          enc->SetFec(codec_fec_enabled_);
@@ -282,10 +286,14 @@ int CodecManager::RegisterEncoder(const CodecInst& send_codec) {
     AudioEncoder* enc = rent_a_codec_.RentEncoder(send_codec);
     if (!enc)
       return -1;
-    codec_owner_.SetEncoders(
-        enc, dtx_enabled_ ? CngPayloadType(send_codec.plfreq) : -1,
-        vad_mode_, red_enabled_ ? RedPayloadType(send_codec.plfreq) : -1);
-    RTC_DCHECK(codec_owner_.Encoder());
+    rent_a_codec_.RentEncoderStack(
+        enc, dtx_enabled_
+                 ? rtc::Optional<RentACodec::CngConfig>(RentACodec::CngConfig{
+                       CngPayloadType(send_codec.plfreq), vad_mode_})
+                 : rtc::Optional<RentACodec::CngConfig>(),
+        red_enabled_ ? rtc::Optional<int>(RedPayloadType(send_codec.plfreq))
+                     : rtc::Optional<int>());
+    RTC_DCHECK(CurrentEncoder());
   }
   send_codec_inst_.plfreq = send_codec.plfreq;
   send_codec_inst_.pacsize = send_codec.pacsize;
@@ -294,12 +302,12 @@ int CodecManager::RegisterEncoder(const CodecInst& send_codec) {
 
   // Check if a change in Rate is required.
   if (send_codec.rate != send_codec_inst_.rate) {
-    codec_owner_.Encoder()->SetTargetBitrate(send_codec.rate);
+    CurrentEncoder()->SetTargetBitrate(send_codec.rate);
     send_codec_inst_.rate = send_codec.rate;
   }
 
   codec_fec_enabled_ =
-      codec_fec_enabled_ && codec_owner_.Encoder()->SetFec(codec_fec_enabled_);
+      codec_fec_enabled_ && CurrentEncoder()->SetFec(codec_fec_enabled_);
 
   return 0;
 }
@@ -328,11 +336,17 @@ void CodecManager::RegisterEncoder(AudioEncoder* external_speech_encoder) {
     const bool success = external_speech_encoder->SetFec(false);
     RTC_DCHECK(success);
   }
-  int cng_pt = dtx_enabled_
-                   ? CngPayloadType(external_speech_encoder->SampleRateHz())
-                   : -1;
-  int red_pt = red_enabled_ ? RedPayloadType(send_codec_inst_.plfreq) : -1;
-  codec_owner_.SetEncoders(external_speech_encoder, cng_pt, vad_mode_, red_pt);
+
+  rent_a_codec_.RentEncoderStack(
+      external_speech_encoder,
+      dtx_enabled_
+          ? rtc::Optional<RentACodec::CngConfig>(RentACodec::CngConfig{
+                CngPayloadType(external_speech_encoder->SampleRateHz()),
+                vad_mode_})
+          : rtc::Optional<RentACodec::CngConfig>(),
+      red_enabled_ ? rtc::Optional<int>(RedPayloadType(
+                         external_speech_encoder->SampleRateHz()))
+                   : rtc::Optional<int>());
 }
 
 rtc::Optional<CodecInst> CodecManager::GetCodecInst() const {
@@ -340,7 +354,7 @@ rtc::Optional<CodecInst> CodecManager::GetCodecInst() const {
   WEBRTC_TRACE(webrtc::kTraceStream, webrtc::kTraceAudioCoding, dummy_id,
                "SendCodec()");
 
-  if (!codec_owner_.Encoder()) {
+  if (!CurrentEncoder()) {
     WEBRTC_TRACE(webrtc::kTraceStream, webrtc::kTraceAudioCoding, dummy_id,
                  "SendCodec Failed, no codec is registered");
     return rtc::Optional<CodecInst>();
@@ -361,10 +375,16 @@ bool CodecManager::SetCopyRed(bool enable) {
   }
   if (red_enabled_ != enable) {
     red_enabled_ = enable;
-    if (codec_owner_.Encoder()) {
-      int cng_pt = dtx_enabled_ ? CngPayloadType(send_codec_inst_.plfreq) : -1;
-      int red_pt = red_enabled_ ? RedPayloadType(send_codec_inst_.plfreq) : -1;
-      codec_owner_.ChangeCngAndRed(cng_pt, vad_mode_, red_pt);
+    if (CurrentEncoder()) {
+      rent_a_codec_.RentEncoderStack(
+          rent_a_codec_.GetEncoder(),
+          dtx_enabled_
+              ? rtc::Optional<RentACodec::CngConfig>(RentACodec::CngConfig{
+                    CngPayloadType(send_codec_inst_.plfreq), vad_mode_})
+              : rtc::Optional<RentACodec::CngConfig>(),
+          red_enabled_
+              ? rtc::Optional<int>(RedPayloadType(send_codec_inst_.plfreq))
+              : rtc::Optional<int>());
     }
   }
   return true;
@@ -377,7 +397,7 @@ int CodecManager::SetVAD(bool enable, ACMVADMode mode) {
 
   // Check that the send codec is mono. We don't support VAD/DTX for stereo
   // sending.
-  const auto* enc = codec_owner_.Encoder();
+  auto* enc = rent_a_codec_.GetEncoder();
   const bool stereo_send = enc ? (enc->NumChannels() != 1) : false;
   if (enable && stereo_send) {
     WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, 0,
@@ -397,9 +417,14 @@ int CodecManager::SetVAD(bool enable, ACMVADMode mode) {
     dtx_enabled_ = enable;
     vad_mode_ = mode;
     if (enc) {
-      int cng_pt = dtx_enabled_ ? CngPayloadType(send_codec_inst_.plfreq) : -1;
-      int red_pt = red_enabled_ ? RedPayloadType(send_codec_inst_.plfreq) : -1;
-      codec_owner_.ChangeCngAndRed(cng_pt, vad_mode_, red_pt);
+      rent_a_codec_.RentEncoderStack(
+          enc, dtx_enabled_
+                   ? rtc::Optional<RentACodec::CngConfig>(RentACodec::CngConfig{
+                         CngPayloadType(send_codec_inst_.plfreq), vad_mode_})
+                   : rtc::Optional<RentACodec::CngConfig>(),
+          red_enabled_
+              ? rtc::Optional<int>(RedPayloadType(send_codec_inst_.plfreq))
+              : rtc::Optional<int>());
     }
   }
   return 0;
@@ -420,9 +445,9 @@ int CodecManager::SetCodecFEC(bool enable_codec_fec) {
     return -1;
   }
 
-  RTC_CHECK(codec_owner_.Encoder());
+  RTC_CHECK(CurrentEncoder());
   codec_fec_enabled_ =
-      codec_owner_.Encoder()->SetFec(enable_codec_fec) && enable_codec_fec;
+      CurrentEncoder()->SetFec(enable_codec_fec) && enable_codec_fec;
   return codec_fec_enabled_ == enable_codec_fec ? 0 : -1;
 }