Simplify CodecManager::RegisterEncoder()

webrtc/modules/audio_coding/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 131 matching lines @@
       return 0;
     case RentACodec::RegistrationResult::kBadFreq:
       WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, dummy_id,
                    "RegisterSendCodec() failed, invalid frequency for CNG"
                    " registration");
       return -1;
     case RentACodec::RegistrationResult::kSkip:
       break;
   }
 
-  // Check if the codec is already registered as send codec.
-  bool new_codec = true;
-  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;
+  encoder_is_opus_ = IsOpus(send_codec);
+  if (encoder_is_opus_) {
+    // VAD/DTX not supported.
+    codec_stack_params_.use_cng = 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.
-      codec_stack_params_.use_cng = false;
-    }
-    AudioEncoder* enc = rent_a_codec_.RentEncoder(send_codec);
-    if (!enc)
-      return -1;
-    rent_a_codec_.RentEncoderStack(enc, &codec_stack_params_);
-    RTC_DCHECK(CurrentEncoder());
-
-    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 ||
+  // Recreate the encoder if anything except the send bitrate has changed.
+  if (!CurrentEncoder() || send_codec_inst_.pltype != send_codec.pltype ||
+      STR_CASE_CMP(send_codec_inst_.plname, send_codec.plname) != 0 ||
+      send_codec_inst_.plfreq != send_codec.plfreq ||
       send_codec_inst_.pacsize != send_codec.pacsize ||
       send_codec_inst_.channels != send_codec.channels) {
+    RTC_DCHECK(CodecSupported(send_codec));
     AudioEncoder* enc = rent_a_codec_.RentEncoder(send_codec);
     if (!enc)
       return -1;
     rent_a_codec_.RentEncoderStack(enc, &codec_stack_params_);
     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) {
-    CurrentEncoder()->SetTargetBitrate(send_codec.rate);
-    send_codec_inst_.rate = send_codec.rate;
-  }
-
+  send_codec_inst_ = send_codec;

[hlundin-webrtc, 2015/12/01 12:11:45]

I'd like a DCHECK at this point to verify that all parameters but the bitrate
are unchanged. Maybe through an auxiliary function:

bool AllButRateEqual(const CodecInst& a, CodecInst b) {
  b.rate = a.rate;
  return a == b;
}

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

But they're not---we pass through here whether or not the bitrate was the only
thing that changed.

[hlundin-webrtc, 2015/12/01 13:07:04]

Oh, well, then I guess not...

+  CurrentEncoder()->SetTargetBitrate(send_codec_inst_.rate);
   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),
@@ skipping 93 matching lines @@
       CurrentEncoder()->SetFec(enable_codec_fec) && enable_codec_fec;
   return codec_stack_params_.use_codec_fec == enable_codec_fec ? 0 : -1;
 }
 
 AudioDecoder* CodecManager::GetAudioDecoder(const CodecInst& codec) {
   return IsIsac(codec) ? rent_a_codec_.RentIsacDecoder() : nullptr;
 }
 
 }  // namespace acm2
 }  // namespace webrtc