AcmReceiver: Eliminate AcmReceiver::decoders_

BUG=webrtc:5801

Review-Url: https://codereview.webrtc.org/2351183002
Cr-Commit-Position: refs/heads/master@{#14335}

webrtc/modules/audio_coding/acm2/acm_receiver.cc

@@ -180,9 +180,12 @@ int AcmReceiver::GetAudio(int desired_freq_hz,
 int32_t AcmReceiver::AddCodec(int acm_codec_id,
                               uint8_t payload_type,
                               size_t channels,
-                              int sample_rate_hz,
+                              int /*sample_rate_hz*/,
                               AudioDecoder* audio_decoder,
                               const std::string& name) {
+  // TODO(kwiberg): This function has been ignoring the |sample_rate_hz|
+  // argument for a long time. Arguably, it should simply be removed.
+
   const auto neteq_decoder = [acm_codec_id, channels]() -> NetEqDecoder {
     if (acm_codec_id == -1)
       return NetEqDecoder::kDecoderArbitrary;  // External decoder.
@@ -194,29 +197,22 @@ int32_t AcmReceiver::AddCodec(int acm_codec_id,
     RTC_DCHECK(ned) << "Invalid codec ID: " << static_cast<int>(*cid);
     return *ned;
   }();
+  const rtc::Optional<SdpAudioFormat> new_format =
+      RentACodec::NetEqDecoderToSdpAudioFormat(neteq_decoder);
 
   rtc::CritScope lock(&crit_sect_);
 
-  // The corresponding NetEq decoder ID.
+  const SdpAudioFormat* const old_format =
-  // If this codec has been registered before.
+      neteq_->GetDecoderFormat(payload_type);
-  auto it = decoders_.find(payload_type);
+  if (old_format && new_format && *old_format == *new_format) {
-  if (it != decoders_.end()) {
+    // Re-registering the same codec. Do nothing and return.
-    const Decoder& decoder = it->second;
+    return 0;
-    if (acm_codec_id != -1 && decoder.acm_codec_id == acm_codec_id &&
+  }
-        decoder.channels == channels &&
-        decoder.sample_rate_hz == sample_rate_hz) {
-      // Re-registering the same codec. Do nothing and return.
-      return 0;
-    }
 
-    // Changing codec. First unregister the old codec, then register the new
+  if (neteq_->RemovePayloadType(payload_type) != NetEq::kOK &&
-    // one.
+      neteq_->LastError() != NetEq::kDecoderNotFound) {
-    if (neteq_->RemovePayloadType(payload_type) != NetEq::kOK) {
+    LOG(LERROR) << "Cannot remove payload " << static_cast<int>(payload_type);
-      LOG(LERROR) << "Cannot remove payload " << static_cast<int>(payload_type);
+    return -1;
-      return -1;
-    }
-
-    decoders_.erase(it);
   }
 
   int ret_val;
@@ -232,13 +228,6 @@ int32_t AcmReceiver::AddCodec(int acm_codec_id,
                 << " channels: " << channels;
     return -1;
   }
-
-  Decoder decoder;
-  decoder.acm_codec_id = acm_codec_id;
-  decoder.payload_type = payload_type;
-  decoder.channels = channels;
-  decoder.sample_rate_hz = sample_rate_hz;
-  decoders_[payload_type] = decoder;
   return 0;
 }
 
@@ -249,18 +238,14 @@ void AcmReceiver::FlushBuffers() {
 void AcmReceiver::RemoveAllCodecs() {
   rtc::CritScope lock(&crit_sect_);
   neteq_->RemoveAllPayloadTypes();
-  decoders_.clear();
   last_audio_decoder_ = rtc::Optional<CodecInst>();
   last_packet_sample_rate_hz_ = rtc::Optional<int>();
 }
 
 int AcmReceiver::RemoveCodec(uint8_t payload_type) {
   rtc::CritScope lock(&crit_sect_);
-  auto it = decoders_.find(payload_type);
+  if (neteq_->RemovePayloadType(payload_type) != NetEq::kOK &&
-  if (it == decoders_.end()) {  // Such a payload-type is not registered.
+      neteq_->LastError() != NetEq::kDecoderNotFound) {
-    return 0;
-  }
-  if (neteq_->RemovePayloadType(payload_type) != NetEq::kOK) {
     LOG(LERROR) << "AcmReceiver::RemoveCodec" << static_cast<int>(payload_type);
     return -1;
   }
@@ -268,7 +253,6 @@ int AcmReceiver::RemoveCodec(uint8_t payload_type) {
     last_audio_decoder_ = rtc::Optional<CodecInst>();
     last_packet_sample_rate_hz_ = rtc::Optional<int>();
   }
-  decoders_.erase(it);
   return 0;
 }
 

webrtc/modules/audio_coding/acm2/acm_receiver.h

@@ -274,8 +274,6 @@ class AcmReceiver {
   std::unique_ptr<int16_t[]> last_audio_buffer_ GUARDED_BY(crit_sect_);
   CallStatistics call_stats_ GUARDED_BY(crit_sect_);
   NetEq* neteq_;
-  // Decoders map is keyed by payload type
-  std::map<uint8_t, Decoder> decoders_ GUARDED_BY(crit_sect_);
   Clock* clock_;  // TODO(henrik.lundin) Make const if possible.
   bool resampled_last_output_frame_ GUARDED_BY(crit_sect_);
   rtc::Optional<int> last_packet_sample_rate_hz_ GUARDED_BY(crit_sect_);

webrtc/modules/audio_coding/codecs/audio_format.cc

@@ -10,6 +10,8 @@
 
 #include "webrtc/modules/audio_coding/codecs/audio_format.h"
 
+#include "webrtc/common_types.h"
+
 namespace webrtc {
 
 SdpAudioFormat::SdpAudioFormat(const SdpAudioFormat&) = default;
@@ -33,6 +35,12 @@ SdpAudioFormat::~SdpAudioFormat() = default;
 SdpAudioFormat& SdpAudioFormat::operator=(const SdpAudioFormat&) = default;
 SdpAudioFormat& SdpAudioFormat::operator=(SdpAudioFormat&&) = default;
 
+bool operator==(const SdpAudioFormat& a, const SdpAudioFormat& b) {
+  return STR_CASE_CMP(a.name.c_str(), b.name.c_str()) == 0 &&
+         a.clockrate_hz == b.clockrate_hz && a.num_channels == b.num_channels &&
+         a.parameters == b.parameters;
+}
+
 void swap(SdpAudioFormat& a, SdpAudioFormat& b) {
   using std::swap;
   swap(a.name, b.name);

webrtc/modules/audio_coding/codecs/audio_format.h

@@ -35,6 +35,11 @@ struct SdpAudioFormat {
   SdpAudioFormat& operator=(const SdpAudioFormat&);
   SdpAudioFormat& operator=(SdpAudioFormat&&);
 
+  friend bool operator==(const SdpAudioFormat& a, const SdpAudioFormat& b);
+  friend bool operator!=(const SdpAudioFormat& a, const SdpAudioFormat& b) {
+    return !(a == b);
+  }
+
   std::string name;
   int clockrate_hz;
   int num_channels;

webrtc/modules/audio_coding/neteq/decoder_database.h

@@ -64,9 +64,8 @@ class DecoderDatabase {
       return decoder ? decoder->SampleRateHz() : cng_decoder_->sample_rate_hz;
     }
 
-    const SdpAudioFormat& GetFormat() const {
+    const SdpAudioFormat* GetFormat() const {
-      RTC_DCHECK(audio_format_);
+      return audio_format_ ? &*audio_format_ : nullptr;
-      return *audio_format_;
     }
 
     // Returns true if |codec_type| is comfort noise.

webrtc/modules/audio_coding/neteq/include/neteq.h

@@ -259,6 +259,11 @@ class NetEq {
   // value if we have no decoder for that payload type.
   virtual rtc::Optional<CodecInst> GetDecoder(int payload_type) const = 0;
 
+  // Returns the decoder format for the given payload type. Returns null if no
+  // such payload type was registered, or if it was registered without
+  // providing an SdpAudioFormat.
+  virtual const SdpAudioFormat* GetDecoderFormat(int payload_type) const = 0;
+
   // Not implemented.
   virtual int SetTargetNumberOfChannels() = 0;
 

webrtc/modules/audio_coding/neteq/neteq_impl.cc

@@ -457,6 +457,18 @@ rtc::Optional<CodecInst> NetEqImpl::GetDecoder(int payload_type) const {
   return rtc::Optional<CodecInst>(ci);
 }
 
+const SdpAudioFormat* NetEqImpl::GetDecoderFormat(int payload_type) const {
+  rtc::CritScope lock(&crit_sect_);
+  const DecoderDatabase::DecoderInfo* const di =
+      decoder_database_->GetDecoderInfo(payload_type);
+  if (!di) {
+    return nullptr;  // Payload type not registered.
+  }
+  // This will return null if the payload type was registered without an
+  // SdpAudioFormat.
+  return di->GetFormat();
+}
+
 int NetEqImpl::SetTargetNumberOfChannels() {
   return kNotImplemented;
 }

webrtc/modules/audio_coding/neteq/neteq_impl.h

@@ -173,6 +173,8 @@ class NetEqImpl : public webrtc::NetEq {
 
   rtc::Optional<CodecInst> GetDecoder(int payload_type) const override;
 
+  const SdpAudioFormat* GetDecoderFormat(int payload_type) const override;
+
   int SetTargetNumberOfChannels() override;
 
   int SetTargetSampleRate() override;

webrtc/modules/audio_coding/neteq/timestamp_scaler.cc

@@ -50,7 +50,7 @@ uint32_t TimestampScaler::ToInternal(uint32_t external_timestamp,
       // support timestamp scaling of them.
       denominator_ = numerator_;
     } else {
-      denominator_ = info->GetFormat().clockrate_hz;
+      denominator_ = info->GetFormat()->clockrate_hz;
     }
   }
   if (numerator_ != denominator_) {