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

webrtc/modules/audio_coding/main/acm2/codec_manager.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 222 matching lines @@
   if (send_codec.channels != 1) {
     if (dtx_enabled_) {
       WEBRTC_TRACE(webrtc::kTraceWarning, webrtc::kTraceAudioCoding, dummy_id,
                    "VAD/DTX is turned off, not supported when sending stereo.");
     }
     dtx_enabled_ = false;
   }
 
   // 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);
     new_codec = !old_codec_id || *new_codec_id != *old_codec_id;
   }
 
   if (RedPayloadType(send_codec.plfreq) == -1) {
     red_enabled_ = false;
   }
 
   encoder_is_opus_ = IsOpus(send_codec);
 
   if (new_codec) {
     // This is a new codec. Register it and return.
     RTC_DCHECK(CodecSupported(send_codec));
     if (IsOpus(send_codec)) {
       // VAD/DTX not supported.
       dtx_enabled_ = false;
     }
     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_);
 
     send_codec_inst_ = send_codec;
     return 0;
   }
 
   // This is an existing codec; re-create it if any parameters have changed.
   if (send_codec_inst_.plfreq != send_codec.plfreq ||
       send_codec_inst_.pacsize != send_codec.pacsize ||
       send_codec_inst_.channels != send_codec.channels) {
     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;
   send_codec_inst_.channels = send_codec.channels;
   send_codec_inst_.pltype = send_codec.pltype;
 
   // 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;
 }
 
 void CodecManager::RegisterEncoder(AudioEncoder* external_speech_encoder) {
   // Make up a CodecInst.
   send_codec_inst_.channels = external_speech_encoder->NumChannels();
   send_codec_inst_.plfreq = external_speech_encoder->SampleRateHz();
   send_codec_inst_.pacsize = rtc::CheckedDivExact(
       static_cast<int>(external_speech_encoder->Max10MsFramesInAPacket() *
                        send_codec_inst_.plfreq),
       100);
   send_codec_inst_.pltype = -1;  // Not valid.
   send_codec_inst_.rate = -1;    // Not valid.
   static const char kName[] = "external";
   memcpy(send_codec_inst_.plname, kName, sizeof(kName));
 
   if (send_codec_inst_.channels != 1)
     dtx_enabled_ = false;
   if (codec_fec_enabled_) {
     // Switch FEC on. On failure, remember that FEC is off.
     if (!external_speech_encoder->SetFec(true))
       codec_fec_enabled_ = false;
   } else {
     // Switch FEC off. This shouldn't fail.
     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 {
   int dummy_id = 0;
   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>();
   }
   return rtc::Optional<CodecInst>(send_codec_inst_);
 }
 
 bool CodecManager::SetCopyRed(bool enable) {
   if (enable && codec_fec_enabled_) {
     WEBRTC_TRACE(webrtc::kTraceWarning, webrtc::kTraceAudioCoding, 0,
                  "Codec internal FEC and RED cannot be co-enabled.");
     return false;
   }
   if (enable && RedPayloadType(send_codec_inst_.plfreq) == -1) {
     WEBRTC_TRACE(webrtc::kTraceWarning, webrtc::kTraceAudioCoding, 0,
                  "Cannot enable RED at %i Hz.", send_codec_inst_.plfreq);
     return false;
   }
   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;
 }
 
 int CodecManager::SetVAD(bool enable, ACMVADMode mode) {
   // Sanity check of the mode.
   RTC_DCHECK(mode == VADNormal || mode == VADLowBitrate || mode == VADAggr ||
              mode == VADVeryAggr);
 
   // 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,
                  "VAD/DTX not supported for stereo sending");
     dtx_enabled_ = false;
     return -1;
   }
 
   // If a send codec is registered, set VAD/DTX for the codec.
   if (IsOpus(send_codec_inst_)) {
     // VAD/DTX not supported.
     dtx_enabled_ = false;
     return 0;
   }
 
   if (dtx_enabled_ != enable || vad_mode_ != 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;
 }
 
 void CodecManager::VAD(bool* dtx_enabled,
                        bool* vad_enabled,
                        ACMVADMode* mode) const {
   *dtx_enabled = dtx_enabled_;
   *vad_enabled = dtx_enabled_;
   *mode = vad_mode_;
 }
 
 int CodecManager::SetCodecFEC(bool enable_codec_fec) {
   if (enable_codec_fec == true && red_enabled_ == true) {
     WEBRTC_TRACE(webrtc::kTraceWarning, webrtc::kTraceAudioCoding, 0,
                  "Codec internal FEC and RED cannot be co-enabled.");
     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;
 }
 
 AudioDecoder* CodecManager::GetAudioDecoder(const CodecInst& codec) {
   return IsIsac(codec) ? rent_a_codec_.RentIsacDecoder() : nullptr;
 }
 
 int CodecManager::CngPayloadType(int sample_rate_hz) const {
   switch (sample_rate_hz) {
     case 8000:
@@ skipping 19 matching lines @@
     case 48000:
       return -1;
     default:
       FATAL() << sample_rate_hz << " Hz is not supported";
       return -1;
   }
 }
 
 }  // namespace acm2
 }  // namespace webrtc