Move Rent-A-Codec out of CodecManager

webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc

 /*
  *  Copyright (c) 2012 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 115 matching lines @@
 AudioCodingModuleImpl::~AudioCodingModuleImpl() = default;
 
 int32_t AudioCodingModuleImpl::Encode(const InputData& input_data) {
   AudioEncoder::EncodedInfo encoded_info;
   uint8_t previous_pltype;
 
   // Check if there is an encoder before.
   if (!HaveValidEncoder("Process"))
     return -1;
 
-  AudioEncoder* audio_encoder = codec_manager_.CurrentEncoder();
+  AudioEncoder* audio_encoder = rent_a_codec_.GetEncoderStack();
   // Scale the timestamp to the codec's RTP timestamp rate.
   uint32_t rtp_timestamp =
       first_frame_ ? input_data.input_timestamp
                    : last_rtp_timestamp_ +
                          rtc::CheckedDivExact(
                              input_data.input_timestamp - last_timestamp_,
                              static_cast<uint32_t>(rtc::CheckedDivExact(
                                  audio_encoder->SampleRateHz(),
                                  audio_encoder->RtpTimestampRateHz())));
   last_timestamp_ = input_data.input_timestamp;
@@ skipping 44 matching lines @@
   return static_cast<int32_t>(encode_buffer_.size());
 }
 
 /////////////////////////////////////////
 //   Sender
 //
 
 // Can be called multiple times for Codec, CNG, RED.
 int AudioCodingModuleImpl::RegisterSendCodec(const CodecInst& send_codec) {
   CriticalSectionScoped lock(acm_crit_sect_.get());
-  return codec_manager_.RegisterEncoder(send_codec);
+  if (!codec_manager_.RegisterEncoder(send_codec)) {
+    return -1;

[hlundin-webrtc, 2015/12/15 13:42:03]

I'd like to see some logging, so that it is possible to understand what happened
and why.

[kwiberg-webrtc, 2015/12/16 10:57:48]

codec_manager_.RegisterEncoder already logs in all cases where it returns false,
providing more detail than what we could produce at this point.

It'd be nice if logging was visible in the function signature... that'd be
another advantage of passing a logger object to functions that want to log.

[hlundin-webrtc, 2015/12/16 11:58:35]

Acknowledged.

+  }
+  auto* sp = codec_manager_.GetStackParams();
+  if (codec_manager_.GetCodecInst() && !sp->speech_encoder) {

[hlundin-webrtc, 2015/12/15 13:42:03]

Please, add a comment to explain this condition. In particular, what does the
value of codec_manager_.GetCodecInst() mean, and what if it does not evaluate to
true?

[kwiberg-webrtc, 2015/12/16 10:57:48]

Done.

+    AudioEncoder* enc =
+        rent_a_codec_.RentEncoder(*codec_manager_.GetCodecInst());
+    if (!enc)
+      return -1;

[hlundin-webrtc, 2015/12/15 13:42:03]

Log here too, please.

[kwiberg-webrtc, 2015/12/16 10:57:48]

Same answer: rent_a_codec_.RentEncoder already logs.

+    sp->speech_encoder = enc;
+  }
+  if (sp->speech_encoder)
+    rent_a_codec_.RentEncoderStack(sp);
+  return 0;
 }
 
 void AudioCodingModuleImpl::RegisterExternalSendCodec(
     AudioEncoder* external_speech_encoder) {
   CriticalSectionScoped lock(acm_crit_sect_.get());
-  codec_manager_.RegisterEncoder(external_speech_encoder);
+  auto* sp = codec_manager_.GetStackParams();
+  sp->speech_encoder = external_speech_encoder;
+  rent_a_codec_.RentEncoderStack(sp);
 }
 
 // Get current send codec.
 rtc::Optional<CodecInst> AudioCodingModuleImpl::SendCodec() const {
   CriticalSectionScoped lock(acm_crit_sect_.get());
-  return codec_manager_.GetCodecInst();
+  auto* ci = codec_manager_.GetCodecInst();
+  if (ci) {
+    return rtc::Optional<CodecInst>(*ci);
+  }
+  auto* enc = codec_manager_.GetStackParams()->speech_encoder;
+  if (enc) {
+    return rtc::Optional<CodecInst>(CodecManager::ForgeCodecInst(enc));
+  }
+  return rtc::Optional<CodecInst>();
 }
 
 // Get current send frequency.
 int AudioCodingModuleImpl::SendFrequency() const {
   WEBRTC_TRACE(webrtc::kTraceStream, webrtc::kTraceAudioCoding, id_,
                "SendFrequency()");
   CriticalSectionScoped lock(acm_crit_sect_.get());
 
-  if (!codec_manager_.CurrentEncoder()) {
+  auto* enc = rent_a_codec_.GetEncoderStack();

[hlundin-webrtc, 2015/12/15 13:42:03]

const auto*
(Because GetEncoderStack() violates the style guide and returns a non-const
pointer to a member through a const-qualified method.)

[kwiberg-webrtc, 2015/12/16 10:57:48]

Done.

+  if (!enc) {
     WEBRTC_TRACE(webrtc::kTraceStream, webrtc::kTraceAudioCoding, id_,
                  "SendFrequency Failed, no codec is registered");
     return -1;
   }
 
-  return codec_manager_.CurrentEncoder()->SampleRateHz();
+  return enc->SampleRateHz();
 }
 
 void AudioCodingModuleImpl::SetBitRate(int bitrate_bps) {
   CriticalSectionScoped lock(acm_crit_sect_.get());
-  if (codec_manager_.CurrentEncoder()) {
-    codec_manager_.CurrentEncoder()->SetTargetBitrate(bitrate_bps);
+  auto* enc = rent_a_codec_.GetEncoderStack();
+  if (enc) {
+    enc->SetTargetBitrate(bitrate_bps);
   }
 }
 
 // Register a transport callback which will be called to deliver
 // the encoded buffers.
 int AudioCodingModuleImpl::RegisterTransportCallback(
     AudioPacketizationCallback* transport) {
   CriticalSectionScoped lock(callback_crit_sect_.get());
   packetization_callback_ = transport;
   return 0;
@@ skipping 46 matching lines @@
   const AudioFrame* ptr_frame;
   // Perform a resampling, also down-mix if it is required and can be
   // performed before resampling (a down mix prior to resampling will take
   // place if both primary and secondary encoders are mono and input is in
   // stereo).
   if (PreprocessToAddData(audio_frame, &ptr_frame) < 0) {
     return -1;
   }
 
   // Check whether we need an up-mix or down-mix?
-  bool remix = ptr_frame->num_channels_ !=
-               codec_manager_.CurrentEncoder()->NumChannels();
+  const int current_num_channels =
+      rent_a_codec_.GetEncoderStack()->NumChannels();
+  bool remix = ptr_frame->num_channels_ != current_num_channels;
 
   if (remix) {
     if (ptr_frame->num_channels_ == 1) {
       if (UpMix(*ptr_frame, WEBRTC_10MS_PCM_AUDIO, input_data->buffer) < 0)
         return -1;
     } else {
       if (DownMix(*ptr_frame, WEBRTC_10MS_PCM_AUDIO, input_data->buffer) < 0)
         return -1;
     }
   }
 
   // When adding data to encoders this pointer is pointing to an audio buffer
   // with correct number of channels.
   const int16_t* ptr_audio = ptr_frame->data_;
 
   // For pushing data to primary, point the |ptr_audio| to correct buffer.
-  if (codec_manager_.CurrentEncoder()->NumChannels() !=
-      ptr_frame->num_channels_)
+  if (current_num_channels != ptr_frame->num_channels_)

[hlundin-webrtc, 2015/12/15 13:42:02]

Did ptr_frame->num_channels_ change since remix was set? Otherwise, you can just
condition on remix here. (Maybe change the name and make it const, too.)

[kwiberg-webrtc, 2015/12/16 10:57:48]

No, it really shouldn't. Done.

     ptr_audio = input_data->buffer;
 
   input_data->input_timestamp = ptr_frame->timestamp_;
   input_data->audio = ptr_audio;
   input_data->length_per_channel = ptr_frame->samples_per_channel_;
-  input_data->audio_channel = codec_manager_.CurrentEncoder()->NumChannels();
+  input_data->audio_channel = current_num_channels;
 
   return 0;
 }
 
 // Perform a resampling and down-mix if required. We down-mix only if
 // encoder is mono and input is stereo. In case of dual-streaming, both
 // encoders has to be mono for down-mix to take place.
 // |*ptr_out| will point to the pre-processed audio-frame. If no pre-processing
 // is required, |*ptr_out| points to |in_frame|.
 int AudioCodingModuleImpl::PreprocessToAddData(const AudioFrame& in_frame,
                                                const AudioFrame** ptr_out) {
-  bool resample = (in_frame.sample_rate_hz_ !=
-                   codec_manager_.CurrentEncoder()->SampleRateHz());
+  const auto* enc = rent_a_codec_.GetEncoderStack();
+  bool resample = in_frame.sample_rate_hz_ != enc->SampleRateHz();

[hlundin-webrtc, 2015/12/15 13:42:02]

const

[kwiberg-webrtc, 2015/12/16 10:57:48]

Done.

 
   // This variable is true if primary codec and secondary codec (if exists)
   // are both mono and input is stereo.
-  bool down_mix = (in_frame.num_channels_ == 2) &&
-                  (codec_manager_.CurrentEncoder()->NumChannels() == 1);
+  bool down_mix = in_frame.num_channels_ == 2 && enc->NumChannels() == 1;

[hlundin-webrtc, 2015/12/15 13:42:03]

const
Add a TODO to generalize the condition; it should probably be
"in_frame.num_channels_ > enc->NumChannels()"

[kwiberg-webrtc, 2015/12/16 10:57:48]

Done.

 
   if (!first_10ms_data_) {
     expected_in_ts_ = in_frame.timestamp_;
     expected_codec_ts_ = in_frame.timestamp_;
     first_10ms_data_ = true;
   } else if (in_frame.timestamp_ != expected_in_ts_) {
     // TODO(turajs): Do we need a warning here.
     expected_codec_ts_ +=
         (in_frame.timestamp_ - expected_in_ts_) *
-        static_cast<uint32_t>(
-            (static_cast<double>(
-                 codec_manager_.CurrentEncoder()->SampleRateHz()) /
-             static_cast<double>(in_frame.sample_rate_hz_)));
+        static_cast<uint32_t>(static_cast<double>(enc->SampleRateHz()) /
+                              static_cast<double>(in_frame.sample_rate_hz_));
     expected_in_ts_ = in_frame.timestamp_;
   }
 
 
   if (!down_mix && !resample) {
     // No pre-processing is required.
     expected_in_ts_ += static_cast<uint32_t>(in_frame.samples_per_channel_);
     expected_codec_ts_ += static_cast<uint32_t>(in_frame.samples_per_channel_);
     *ptr_out = &in_frame;
     return 0;
@@ skipping 18 matching lines @@
 
   preprocess_frame_.timestamp_ = expected_codec_ts_;
   preprocess_frame_.samples_per_channel_ = in_frame.samples_per_channel_;
   preprocess_frame_.sample_rate_hz_ = in_frame.sample_rate_hz_;
   // If it is required, we have to do a resampling.
   if (resample) {
     // The result of the resampler is written to output frame.
     dest_ptr_audio = preprocess_frame_.data_;
 
     int samples_per_channel = resampler_.Resample10Msec(
-        src_ptr_audio, in_frame.sample_rate_hz_,
-        codec_manager_.CurrentEncoder()->SampleRateHz(),
+        src_ptr_audio, in_frame.sample_rate_hz_, enc->SampleRateHz(),
         preprocess_frame_.num_channels_, AudioFrame::kMaxDataSizeSamples,
         dest_ptr_audio);
 
     if (samples_per_channel < 0) {
       WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, id_,
                    "Cannot add 10 ms audio, resampling failed");
       return -1;
     }
     preprocess_frame_.samples_per_channel_ =
         static_cast<size_t>(samples_per_channel);
-    preprocess_frame_.sample_rate_hz_ =
-        codec_manager_.CurrentEncoder()->SampleRateHz();
+    preprocess_frame_.sample_rate_hz_ = enc->SampleRateHz();
   }
 
   expected_codec_ts_ +=
       static_cast<uint32_t>(preprocess_frame_.samples_per_channel_);
   expected_in_ts_ += static_cast<uint32_t>(in_frame.samples_per_channel_);
 
   return 0;
 }
 
 /////////////////////////////////////////
 //   (RED) Redundant Coding
 //
 
 bool AudioCodingModuleImpl::REDStatus() const {
   CriticalSectionScoped lock(acm_crit_sect_.get());
-  return codec_manager_.red_enabled();
+  return codec_manager_.GetStackParams()->use_red;
 }
 
 // Configure RED status i.e on/off.
-int AudioCodingModuleImpl::SetREDStatus(
+int AudioCodingModuleImpl::SetREDStatus(bool enable_red) {

[hlundin-webrtc, 2015/12/15 13:42:03]

Does this compile when WEBRTC_CODEC_RED is not defined?

[kwiberg-webrtc, 2015/12/16 10:57:48]

It really should. It's not an error for a function to ignore one of its
arguments. Commenting out the name ensures that it's an error to try to use it,
but doing that conditionally as was done here is just a mess to read.

 #ifdef WEBRTC_CODEC_RED
-    bool enable_red) {
   CriticalSectionScoped lock(acm_crit_sect_.get());
-  return codec_manager_.SetCopyRed(enable_red) ? 0 : -1;
+  if (!codec_manager_.SetCopyRed(enable_red)) {
+    return -1;
+  }
+  auto* sp = codec_manager_.GetStackParams();
+  if (sp->speech_encoder)
+    rent_a_codec_.RentEncoderStack(sp);
+  return 0;
 #else
-    bool /* enable_red */) {
   WEBRTC_TRACE(webrtc::kTraceWarning, webrtc::kTraceAudioCoding, id_,
                "  WEBRTC_CODEC_RED is undefined");
   return -1;
 #endif
 }
 
 /////////////////////////////////////////
 //   (FEC) Forward Error Correction (codec internal)
 //
 
 bool AudioCodingModuleImpl::CodecFEC() const {
   CriticalSectionScoped lock(acm_crit_sect_.get());
-  return codec_manager_.codec_fec_enabled();
+  return codec_manager_.GetStackParams()->use_codec_fec;
 }
 
 int AudioCodingModuleImpl::SetCodecFEC(bool enable_codec_fec) {
   CriticalSectionScoped lock(acm_crit_sect_.get());
-  return codec_manager_.SetCodecFEC(enable_codec_fec);
+  if (!codec_manager_.SetCodecFEC(enable_codec_fec)) {
+    return -1;
+  }
+  auto* sp = codec_manager_.GetStackParams();
+  if (sp->speech_encoder)
+    rent_a_codec_.RentEncoderStack(sp);
+  if (enable_codec_fec) {
+    return sp->use_codec_fec ? 0 : -1;
+  } else {
+    RTC_DCHECK(!sp->use_codec_fec);
+    return 0;
+  }
 }
 
 int AudioCodingModuleImpl::SetPacketLossRate(int loss_rate) {
   CriticalSectionScoped lock(acm_crit_sect_.get());
   if (HaveValidEncoder("SetPacketLossRate")) {
-    codec_manager_.CurrentEncoder()->SetProjectedPacketLossRate(loss_rate /
+    rent_a_codec_.GetEncoderStack()->SetProjectedPacketLossRate(loss_rate /
                                                                 100.0);
   }
   return 0;
 }
 
 /////////////////////////////////////////
 //   (VAD) Voice Activity Detection
 //
 int AudioCodingModuleImpl::SetVAD(bool enable_dtx,
                                   bool enable_vad,
                                   ACMVADMode mode) {
   // Note: |enable_vad| is not used; VAD is enabled based on the DTX setting.
   RTC_DCHECK_EQ(enable_dtx, enable_vad);
   CriticalSectionScoped lock(acm_crit_sect_.get());
-  return codec_manager_.SetVAD(enable_dtx, mode);
+  if (!codec_manager_.SetVAD(enable_dtx, mode)) {
+    return -1;
+  }
+  auto* sp = codec_manager_.GetStackParams();
+  if (sp->speech_encoder)
+    rent_a_codec_.RentEncoderStack(sp);
+  return 0;
 }
 
 // Get VAD/DTX settings.
 int AudioCodingModuleImpl::VAD(bool* dtx_enabled, bool* vad_enabled,
                                ACMVADMode* mode) const {
   CriticalSectionScoped lock(acm_crit_sect_.get());
-  codec_manager_.VAD(dtx_enabled, vad_enabled, mode);
+  const auto* sp = codec_manager_.GetStackParams();
+  *dtx_enabled = *vad_enabled = sp->use_cng;
+  *mode = sp->vad_mode;
   return 0;
 }
 
 /////////////////////////////////////////
 //   Receiver
 //
 
 int AudioCodingModuleImpl::InitializeReceiver() {
   CriticalSectionScoped lock(acm_crit_sect_.get());
   return InitializeReceiverSafe();
@@ skipping 66 matching lines @@
 
   // Check if the payload-type is valid.
   if (!RentACodec::IsPayloadTypeValid(codec.pltype)) {
     LOG_F(LS_ERROR) << "Invalid payload type " << codec.pltype << " for "
                     << codec.plname;
     return -1;
   }
 
   // Get |decoder| associated with |codec|. |decoder| is NULL if |codec| does
   // not own its decoder.
-  return receiver_.AddCodec(*codec_index, codec.pltype, codec.channels,
-                            codec.plfreq, codec_manager_.GetAudioDecoder(codec),
-                            codec.plname);
+  return receiver_.AddCodec(
+      *codec_index, codec.pltype, codec.channels, codec.plfreq,
+      STR_CASE_CMP(codec.plname, "isac") == 0 ? rent_a_codec_.RentIsacDecoder()
+                                              : nullptr,
+      codec.plname);
 }
 
 int AudioCodingModuleImpl::RegisterExternalReceiveCodec(
     int rtp_payload_type,
     AudioDecoder* external_decoder,
     int sample_rate_hz,
     int num_channels,
     const std::string& name) {
   CriticalSectionScoped lock(acm_crit_sect_.get());
   RTC_DCHECK(receiver_initialized_);
@@ skipping 121 matching lines @@
     case kVoip:
       app = AudioEncoder::Application::kSpeech;
       break;
     case kAudio:
       app = AudioEncoder::Application::kAudio;
       break;
     default:
       FATAL();
       return 0;
   }
-  return codec_manager_.CurrentEncoder()->SetApplication(app) ? 0 : -1;
+  return rent_a_codec_.GetEncoderStack()->SetApplication(app) ? 0 : -1;
 }
 
 // Informs Opus encoder of the maximum playback rate the receiver will render.
 int AudioCodingModuleImpl::SetOpusMaxPlaybackRate(int frequency_hz) {
   CriticalSectionScoped lock(acm_crit_sect_.get());
   if (!HaveValidEncoder("SetOpusMaxPlaybackRate")) {
     return -1;
   }
   if (!codec_manager_.CurrentEncoderIsOpus())
     return -1;
-  codec_manager_.CurrentEncoder()->SetMaxPlaybackRate(frequency_hz);
+  rent_a_codec_.GetEncoderStack()->SetMaxPlaybackRate(frequency_hz);
   return 0;
 }
 
 int AudioCodingModuleImpl::EnableOpusDtx() {
   CriticalSectionScoped lock(acm_crit_sect_.get());
   if (!HaveValidEncoder("EnableOpusDtx")) {
     return -1;
   }
   if (!codec_manager_.CurrentEncoderIsOpus())
     return -1;
-  return codec_manager_.CurrentEncoder()->SetDtx(true) ? 0 : -1;
+  return rent_a_codec_.GetEncoderStack()->SetDtx(true) ? 0 : -1;
 }
 
 int AudioCodingModuleImpl::DisableOpusDtx() {
   CriticalSectionScoped lock(acm_crit_sect_.get());
   if (!HaveValidEncoder("DisableOpusDtx")) {
     return -1;
   }
   if (!codec_manager_.CurrentEncoderIsOpus())
     return -1;
-  return codec_manager_.CurrentEncoder()->SetDtx(false) ? 0 : -1;
+  return rent_a_codec_.GetEncoderStack()->SetDtx(false) ? 0 : -1;
 }
 
 int AudioCodingModuleImpl::PlayoutTimestamp(uint32_t* timestamp) {
   return receiver_.GetPlayoutTimestamp(timestamp) ? 0 : -1;
 }
 
 bool AudioCodingModuleImpl::HaveValidEncoder(const char* caller_name) const {
-  if (!codec_manager_.CurrentEncoder()) {
+  if (!rent_a_codec_.GetEncoderStack()) {
     WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, id_,
                  "%s failed: No send codec is registered.", caller_name);
     return false;
   }
   return true;
 }
 
 int AudioCodingModuleImpl::UnregisterReceiveCodec(uint8_t payload_type) {
   return receiver_.RemoveCodec(payload_type);
 }
@@ skipping 15 matching lines @@
   return receiver_.LeastRequiredDelayMs();
 }
 
 void AudioCodingModuleImpl::GetDecodingCallStatistics(
       AudioDecodingCallStats* call_stats) const {
   receiver_.GetDecodingCallStatistics(call_stats);
 }
 
 }  // namespace acm2
 }  // namespace webrtc