AcmReceiver: Look up last decoder in NetEq's table of decoders

webrtc/modules/audio_coding/acm2/acm_receiver.cc

Index: webrtc/modules/audio_coding/acm2/acm_receiver.cc
diff --git a/webrtc/modules/audio_coding/acm2/acm_receiver.cc b/webrtc/modules/audio_coding/acm2/acm_receiver.cc
index 9a882aac374a93c05bff8f7fbd866984e2c3c58f..2ae75485e0ff928e2c7029ff4b7d0e1baef41883 100644
--- a/webrtc/modules/audio_coding/acm2/acm_receiver.cc
+++ b/webrtc/modules/audio_coding/acm2/acm_receiver.cc
@@ -32,23 +32,8 @@ namespace webrtc {
 
 namespace acm2 {
 
-namespace {
-
-// Is the given codec a CNG codec?
-// TODO(kwiberg): Move to RentACodec.
-bool IsCng(int codec_id) {
-  auto i = RentACodec::CodecIdFromIndex(codec_id);
-  return (i && (*i == RentACodec::CodecId::kCNNB ||
-                *i == RentACodec::CodecId::kCNWB ||
-                *i == RentACodec::CodecId::kCNSWB ||
-                *i == RentACodec::CodecId::kCNFB));
-}
-
-}  // namespace
-
 AcmReceiver::AcmReceiver(const AudioCodingModule::Config& config)
-    : last_audio_decoder_(nullptr),
-      last_audio_buffer_(new int16_t[AudioFrame::kMaxDataSizeSamples]),
+    : last_audio_buffer_(new int16_t[AudioFrame::kMaxDataSizeSamples]),
       neteq_(NetEq::Create(config.neteq_config, config.decoder_factory)),
       clock_(config.clock),
       resampled_last_output_frame_(true) {
@@ -95,29 +80,25 @@ int AcmReceiver::InsertPacket(const WebRtcRTPHeader& rtp_header,
   {
     rtc::CritScope lock(&crit_sect_);
 
-    const Decoder* decoder = RtpHeaderToDecoder(*header, incoming_payload[0]);
-    if (!decoder) {
+    const rtc::Optional<CodecInst> ci =
+        RtpHeaderToDecoder(*header, incoming_payload[0]);
+    if (!ci) {
       LOG_F(LS_ERROR) << "Payload-type "
                       << static_cast<int>(header->payloadType)
                       << " is not registered.";
       return -1;
     }
-    const int sample_rate_hz = [&decoder] {
-      const auto ci = RentACodec::CodecIdFromIndex(decoder->acm_codec_id);
-      return ci ? RentACodec::CodecInstById(*ci)->plfreq : -1;
-    }();
-    receive_timestamp = NowInTimestamp(sample_rate_hz);
-
-    // If this is a CNG while the audio codec is not mono, skip pushing in
-    // packets into NetEq.
-    if (IsCng(decoder->acm_codec_id) && last_audio_decoder_ &&
-        last_audio_decoder_->channels > 1)
+    receive_timestamp = NowInTimestamp(ci->plfreq);
+
+    if (STR_CASE_CMP(ci->plname, "cn") == 0) {
+      if (last_audio_decoder_ && last_audio_decoder_->channels > 1) {

[hlundin-webrtc, 2016/09/19 11:37:36]

This is functionality best kept inside NetEq. ACM should not have to bother.
Feel free to do that in a follow-up. :)

[kwiberg-webrtc, 2016/09/19 12:18:34]

Moving this into NetEq would mean that AcmReceiver would stop keeping track of
last decoder and last sample rate, right? Then yes, that would certainly be a
separate CL (one which would take AcmReceiver even closer to having no job left
to do).

[hlundin-webrtc, 2016/09/19 12:22:03]

Acknowledged.

+        // This is a CNG and the audio codec is not mono, so skip pushing in
+        // packets into NetEq.
         return 0;
-    if (!IsCng(decoder->acm_codec_id) &&
-        decoder->acm_codec_id !=
-            *RentACodec::CodecIndexFromId(RentACodec::CodecId::kAVT)) {
-      last_audio_decoder_ = decoder;
-      last_packet_sample_rate_hz_ = rtc::Optional<int>(decoder->sample_rate_hz);
+      }
+    } else {
+      last_audio_decoder_ = ci;
+      last_packet_sample_rate_hz_ = rtc::Optional<int>(ci->plfreq);
     }
 
   }  // |crit_sect_| is released.
@@ -282,7 +263,7 @@ int AcmReceiver::RemoveAllCodecs() {
   }
 
   // No codec is registered, invalidate last audio decoder.
-  last_audio_decoder_ = nullptr;
+  last_audio_decoder_ = rtc::Optional<CodecInst>();
   last_packet_sample_rate_hz_ = rtc::Optional<int>();
   return ret_val;
 }
@@ -297,8 +278,8 @@ int AcmReceiver::RemoveCodec(uint8_t payload_type) {
     LOG(LERROR) << "AcmReceiver::RemoveCodec" << static_cast<int>(payload_type);
     return -1;
   }
-  if (last_audio_decoder_ == &it->second) {
-    last_audio_decoder_ = nullptr;
+  if (last_audio_decoder_ && payload_type == last_audio_decoder_->pltype) {
+    last_audio_decoder_ = rtc::Optional<CodecInst>();
     last_packet_sample_rate_hz_ = rtc::Optional<int>();
   }
   decoders_.erase(it);
@@ -318,11 +299,7 @@ int AcmReceiver::LastAudioCodec(CodecInst* codec) const {
   if (!last_audio_decoder_) {
     return -1;
   }
-  *codec = *RentACodec::CodecInstById(
-      *RentACodec::CodecIdFromIndex(last_audio_decoder_->acm_codec_id));
-  codec->pltype = last_audio_decoder_->payload_type;
-  codec->channels = last_audio_decoder_->channels;
-  codec->plfreq = last_audio_decoder_->sample_rate_hz;
+  *codec = *last_audio_decoder_;
   return 0;
 }
 
@@ -386,20 +363,17 @@ void AcmReceiver::ResetInitialDelay() {
   // TODO(turajs): Should NetEq Buffer be flushed?
 }
 
-const AcmReceiver::Decoder* AcmReceiver::RtpHeaderToDecoder(
+const rtc::Optional<CodecInst> AcmReceiver::RtpHeaderToDecoder(
     const RTPHeader& rtp_header,
     uint8_t payload_type) const {

[hlundin-webrtc, 2016/09/19 11:37:36]

Rename to "first_payload_byte". This code is very confusing otherwise.

[kwiberg-webrtc, 2016/09/19 12:18:34]

Good idea.

-  auto it = decoders_.find(rtp_header.payloadType);
-  const auto red_index =
-      RentACodec::CodecIndexFromId(RentACodec::CodecId::kRED);
-  if (red_index &&  // This ensures that RED is defined in WebRTC.
-      it != decoders_.end() && it->second.acm_codec_id == *red_index) {
-    // This is a RED packet, get the payload of the audio codec.
-    it = decoders_.find(payload_type & 0x7F);
+  const rtc::Optional<CodecInst> ci =
+      neteq_->GetDecoder(rtp_header.payloadType);
+  if (ci && STR_CASE_CMP(ci->plname, "red") == 0) {
+    // This is a RED packet. Get the payload of the audio codec.
+    return neteq_->GetDecoder(payload_type & 0x7f);
+  } else {
+    return ci;
   }
-
-  // Check if the payload is registered.
-  return it != decoders_.end() ? &it->second : nullptr;
 }
 
 uint32_t AcmReceiver::NowInTimestamp(int decoder_sampling_rate) const {