NetEq: Drop unnecessary dependency on the audio decoder implementations

BUG=webrtc:8396

Change-Id: I7524dae93b43b656a13fdd535e48373bc29b405e
Reviewed-on: https://webrtc-review.googlesource.com/10804
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20310}

modules/audio_coding/BUILD.gn

@@ -938,8 +938,6 @@ rtc_static_library("neteq") {
   sources = [
     "neteq/accelerate.cc",
     "neteq/accelerate.h",
-    "neteq/audio_decoder_impl.cc",
-    "neteq/audio_decoder_impl.h",
     "neteq/audio_multi_vector.cc",
     "neteq/audio_multi_vector.h",
     "neteq/audio_vector.cc",
@@ -1012,10 +1010,7 @@ rtc_static_library("neteq") {
   deps = [
     ":audio_coding_module_typedefs",
     ":cng",
-    ":g711",
-    ":isac_fix",
     ":neteq_decoder_enum",
-    ":pcm16b",
     "..:module_api",
     "../..:webrtc_common",
     "../../api:optional",
@@ -1025,28 +1020,6 @@ rtc_static_library("neteq") {
     "../../rtc_base:rtc_base_approved",
     "../../system_wrappers",
   ]
-
-  defines = []
-
-  if (rtc_include_ilbc) {
-    defines += [ "WEBRTC_CODEC_ILBC" ]
-    deps += [ ":ilbc" ]
-  }
-  if (rtc_include_opus) {
-    defines += [ "WEBRTC_CODEC_OPUS" ]
-    deps += [ ":webrtc_opus" ]
-  }
-  if (!build_with_mozilla) {
-    if (current_cpu == "arm") {
-      defines += [ "WEBRTC_CODEC_ISACFX" ]
-      deps += [ ":isac_fix" ]
-    } else {
-      defines += [ "WEBRTC_CODEC_ISAC" ]
-      deps += [ ":isac" ]
-    }
-    defines += [ "WEBRTC_CODEC_G722" ]
-    deps += [ ":g722" ]
-  }
 }
 
 # Although providing only test support, this target must be outside of the

modules/audio_coding/acm2/audio_coding_module_unittest.cc

@@ -24,7 +24,6 @@
 #include "modules/audio_coding/codecs/opus/audio_encoder_opus.h"
 #include "modules/audio_coding/include/audio_coding_module.h"
 #include "modules/audio_coding/include/audio_coding_module_typedefs.h"
-#include "modules/audio_coding/neteq/audio_decoder_impl.h"
 #include "modules/audio_coding/neteq/tools/audio_checksum.h"
 #include "modules/audio_coding/neteq/tools/audio_loop.h"
 #include "modules/audio_coding/neteq/tools/constant_pcm_packet_source.h"

modules/audio_coding/neteq/audio_decoder_impl.cc

@@ -1,88 +0,0 @@
-/*
- *  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.
- */
-
-#include "modules/audio_coding/neteq/audio_decoder_impl.h"
-
-#include <assert.h>
-
-#include "modules/audio_coding/codecs/g711/audio_decoder_pcm.h"
-#include "rtc_base/checks.h"
-#ifdef WEBRTC_CODEC_G722
-#include "modules/audio_coding/codecs/g722/audio_decoder_g722.h"
-#endif
-#ifdef WEBRTC_CODEC_ILBC
-#include "modules/audio_coding/codecs/ilbc/audio_decoder_ilbc.h"
-#endif
-#ifdef WEBRTC_CODEC_ISACFX
-#include "modules/audio_coding/codecs/isac/fix/include/audio_decoder_isacfix.h"  // nogncheck
-#include "modules/audio_coding/codecs/isac/fix/include/audio_encoder_isacfix.h"  // nogncheck
-#endif
-#ifdef WEBRTC_CODEC_ISAC
-#include "modules/audio_coding/codecs/isac/main/include/audio_decoder_isac.h"  // nogncheck
-#include "modules/audio_coding/codecs/isac/main/include/audio_encoder_isac.h"  // nogncheck
-#endif
-#ifdef WEBRTC_CODEC_OPUS
-#include "modules/audio_coding/codecs/opus/audio_decoder_opus.h"
-#endif
-#include "modules/audio_coding/codecs/pcm16b/audio_decoder_pcm16b.h"
-
-namespace webrtc {
-
-bool CodecSupported(NetEqDecoder codec_type) {
-  switch (codec_type) {
-    case NetEqDecoder::kDecoderPCMu:
-    case NetEqDecoder::kDecoderPCMa:
-    case NetEqDecoder::kDecoderPCMu_2ch:
-    case NetEqDecoder::kDecoderPCMa_2ch:
-#ifdef WEBRTC_CODEC_ILBC
-    case NetEqDecoder::kDecoderILBC:
-#endif
-#if defined(WEBRTC_CODEC_ISACFX) || defined(WEBRTC_CODEC_ISAC)
-    case NetEqDecoder::kDecoderISAC:
-#endif
-#ifdef WEBRTC_CODEC_ISAC
-    case NetEqDecoder::kDecoderISACswb:
-#endif
-    case NetEqDecoder::kDecoderPCM16B:
-    case NetEqDecoder::kDecoderPCM16Bwb:
-    case NetEqDecoder::kDecoderPCM16Bswb32kHz:
-    case NetEqDecoder::kDecoderPCM16Bswb48kHz:
-    case NetEqDecoder::kDecoderPCM16B_2ch:
-    case NetEqDecoder::kDecoderPCM16Bwb_2ch:
-    case NetEqDecoder::kDecoderPCM16Bswb32kHz_2ch:
-    case NetEqDecoder::kDecoderPCM16Bswb48kHz_2ch:
-    case NetEqDecoder::kDecoderPCM16B_5ch:
-#ifdef WEBRTC_CODEC_G722
-    case NetEqDecoder::kDecoderG722:
-    case NetEqDecoder::kDecoderG722_2ch:
-#endif
-#ifdef WEBRTC_CODEC_OPUS
-    case NetEqDecoder::kDecoderOpus:
-    case NetEqDecoder::kDecoderOpus_2ch:
-#endif
-    case NetEqDecoder::kDecoderRED:
-    case NetEqDecoder::kDecoderAVT:
-    case NetEqDecoder::kDecoderAVT16kHz:
-    case NetEqDecoder::kDecoderAVT32kHz:
-    case NetEqDecoder::kDecoderAVT48kHz:
-    case NetEqDecoder::kDecoderCNGnb:
-    case NetEqDecoder::kDecoderCNGwb:
-    case NetEqDecoder::kDecoderCNGswb32kHz:
-    case NetEqDecoder::kDecoderCNGswb48kHz:
-    case NetEqDecoder::kDecoderArbitrary: {
-      return true;
-    }
-    default: {
-      return false;
-    }
-  }
-}
-
-}  // namespace webrtc

modules/audio_coding/neteq/audio_decoder_impl.h

@@ -1,31 +0,0 @@
-/*
- *  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.
- */
-
-#ifndef MODULES_AUDIO_CODING_NETEQ_AUDIO_DECODER_IMPL_H_
-#define MODULES_AUDIO_CODING_NETEQ_AUDIO_DECODER_IMPL_H_
-
-#include <assert.h>
-
-#include "api/audio_codecs/audio_decoder.h"
-#include "modules/audio_coding/neteq/neteq_decoder_enum.h"
-#include "rtc_base/constructormagic.h"
-#include "typedefs.h"  // NOLINT(build/include)
-
-#ifdef WEBRTC_CODEC_G722
-#include "modules/audio_coding/codecs/g722/g722_interface.h"
-#endif
-
-namespace webrtc {
-
-// Returns true if |codec_type| is supported.
-bool CodecSupported(NetEqDecoder codec_type);
-
-}  // namespace webrtc
-#endif  // MODULES_AUDIO_CODING_NETEQ_AUDIO_DECODER_IMPL_H_

modules/audio_coding/neteq/audio_decoder_unittest.cc

@@ -8,8 +8,6 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#include "modules/audio_coding/neteq/audio_decoder_impl.h"
-
 #include <assert.h>
 #include <stdlib.h>
 
@@ -629,65 +627,4 @@ TEST_F(AudioDecoderOpusStereoTest, SetTargetBitrate) {
   TestOpusSetTargetBitrates(audio_encoder_.get());
 }
 
-namespace {
-#ifdef WEBRTC_CODEC_ILBC
-const bool has_ilbc = true;
-#else
-const bool has_ilbc = false;
-#endif
-#if defined(WEBRTC_CODEC_ISAC) || defined(WEBRTC_CODEC_ISACFX)
-const bool has_isac = true;
-#else
-const bool has_isac = false;
-#endif
-#ifdef WEBRTC_CODEC_ISAC
-const bool has_isac_swb = true;
-#else
-const bool has_isac_swb = false;
-#endif
-#ifdef WEBRTC_CODEC_G722
-const bool has_g722 = true;
-#else
-const bool has_g722 = false;
-#endif
-#ifdef WEBRTC_CODEC_OPUS
-const bool has_opus = true;
-#else
-const bool has_opus = false;
-#endif
-}  // namespace
-
-TEST(AudioDecoder, CodecSupported) {
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCMu));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCMa));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCMu_2ch));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCMa_2ch));
-  EXPECT_EQ(has_ilbc, CodecSupported(NetEqDecoder::kDecoderILBC));
-  EXPECT_EQ(has_isac, CodecSupported(NetEqDecoder::kDecoderISAC));
-  EXPECT_EQ(has_isac_swb, CodecSupported(NetEqDecoder::kDecoderISACswb));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCM16B));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCM16Bwb));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCM16Bswb32kHz));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCM16Bswb48kHz));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCM16B_2ch));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCM16Bwb_2ch));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCM16Bswb32kHz_2ch));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCM16Bswb48kHz_2ch));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderPCM16B_5ch));
-  EXPECT_EQ(has_g722, CodecSupported(NetEqDecoder::kDecoderG722));
-  EXPECT_EQ(has_g722, CodecSupported(NetEqDecoder::kDecoderG722_2ch));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderRED));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderAVT));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderAVT16kHz));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderAVT32kHz));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderAVT48kHz));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderCNGnb));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderCNGwb));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderCNGswb32kHz));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderCNGswb48kHz));
-  EXPECT_TRUE(CodecSupported(NetEqDecoder::kDecoderArbitrary));
-  EXPECT_EQ(has_opus, CodecSupported(NetEqDecoder::kDecoderOpus));
-  EXPECT_EQ(has_opus, CodecSupported(NetEqDecoder::kDecoderOpus_2ch));
-}
-
 }  // namespace webrtc

modules/audio_coding/neteq/decoder_database.cc

@@ -58,6 +58,17 @@ DecoderDatabase::DecoderInfo::DecoderInfo(const SdpAudioFormat& audio_format,
 DecoderDatabase::DecoderInfo::DecoderInfo(DecoderInfo&&) = default;
 DecoderDatabase::DecoderInfo::~DecoderInfo() = default;
 
+bool DecoderDatabase::DecoderInfo::CanGetDecoder() const {
+  if (subtype_ == Subtype::kNormal && !external_decoder_ && !decoder_) {
+    // TODO(ossu): Keep a check here for now, since a number of tests create
+    // DecoderInfos without factories.
+    RTC_DCHECK(factory_);
+    return factory_->IsSupportedDecoder(audio_format_);
+  } else {
+    return true;
+  }
+}
+
 AudioDecoder* DecoderDatabase::DecoderInfo::GetDecoder() const {
   if (subtype_ != Subtype::kNormal) {
     // These are handled internally, so they have no AudioDecoder objects.
@@ -161,16 +172,17 @@ int DecoderDatabase::RegisterPayload(uint8_t rtp_payload_type,
   if (rtp_payload_type > 0x7F) {
     return kInvalidRtpPayloadType;
   }
-  // kCodecArbitrary is only supported through InsertExternal.
-  if (codec_type == NetEqDecoder::kDecoderArbitrary ||
-      !CodecSupported(codec_type)) {
-    return kCodecNotSupported;
+  if (codec_type == NetEqDecoder::kDecoderArbitrary) {
+    return kCodecNotSupported;  // Only supported through InsertExternal.
   }
   const auto opt_format = NetEqDecoderToSdpAudioFormat(codec_type);
   if (!opt_format) {
     return kCodecNotSupported;
   }
   DecoderInfo info(*opt_format, decoder_factory_, name);
+  if (!info.CanGetDecoder()) {
+    return kCodecNotSupported;
+  }
   auto ret =
       decoders_.insert(std::make_pair(rtp_payload_type, std::move(info)));
   if (ret.second == false) {

modules/audio_coding/neteq/decoder_database.h

@@ -19,7 +19,7 @@
 #include "api/audio_codecs/audio_format.h"
 #include "common_types.h"  // NOLINT(build/include)  // NULL
 #include "modules/audio_coding/codecs/cng/webrtc_cng.h"
-#include "modules/audio_coding/neteq/audio_decoder_impl.h"
+#include "modules/audio_coding/neteq/neteq_decoder_enum.h"
 #include "modules/audio_coding/neteq/packet.h"
 #include "rtc_base/constructormagic.h"
 #include "rtc_base/scoped_ref_ptr.h"
@@ -55,6 +55,10 @@ class DecoderDatabase {
     DecoderInfo(DecoderInfo&&);
     ~DecoderInfo();
 
+    // Was this info object created with a specification that allows us to
+    // actually produce a decoder?
+    bool CanGetDecoder() const;
+
     // Get the AudioDecoder object, creating it first if necessary.
     AudioDecoder* GetDecoder() const;
 

modules/audio_coding/neteq/decoder_database_unittest.cc

@@ -34,7 +34,14 @@ TEST(DecoderDatabase, CreateAndDestroy) {
 }
 
 TEST(DecoderDatabase, InsertAndRemove) {
-  DecoderDatabase db(new rtc::RefCountedObject<MockAudioDecoderFactory>);
+  rtc::scoped_refptr<MockAudioDecoderFactory> factory(
+      new rtc::RefCountedObject<MockAudioDecoderFactory>);
+  EXPECT_CALL(*factory, IsSupportedDecoder(_))
+      .WillOnce(Invoke([](const SdpAudioFormat& format) {
+        EXPECT_EQ("pcmu", format.name);
+        return true;
+      }));
+  DecoderDatabase db(factory);
   const uint8_t kPayloadType = 0;
   const std::string kCodecName = "Robert\'); DROP TABLE Students;";
   EXPECT_EQ(
@@ -48,7 +55,18 @@ TEST(DecoderDatabase, InsertAndRemove) {
 }
 
 TEST(DecoderDatabase, InsertAndRemoveAll) {
-  DecoderDatabase db(new rtc::RefCountedObject<MockAudioDecoderFactory>);
+  rtc::scoped_refptr<MockAudioDecoderFactory> factory(
+      new rtc::RefCountedObject<MockAudioDecoderFactory>);
+  EXPECT_CALL(*factory, IsSupportedDecoder(_))
+      .WillOnce(Invoke([](const SdpAudioFormat& format) {
+        EXPECT_EQ("pcmu", format.name);
+        return true;
+      }))
+      .WillOnce(Invoke([](const SdpAudioFormat& format) {
+        EXPECT_EQ("pcma", format.name);
+        return true;
+      }));
+  DecoderDatabase db(factory);
   const std::string kCodecName1 = "Robert\'); DROP TABLE Students;";
   const std::string kCodecName2 = "https://xkcd.com/327/";
   EXPECT_EQ(DecoderDatabase::kOK,
@@ -65,6 +83,11 @@ TEST(DecoderDatabase, InsertAndRemoveAll) {
 TEST(DecoderDatabase, GetDecoderInfo) {
   rtc::scoped_refptr<MockAudioDecoderFactory> factory(
       new rtc::RefCountedObject<MockAudioDecoderFactory>);
+  EXPECT_CALL(*factory, IsSupportedDecoder(_))
+      .WillOnce(Invoke([](const SdpAudioFormat& format) {
+        EXPECT_EQ("pcmu", format.name);
+        return true;
+      }));
   auto* decoder = new MockAudioDecoder;
   EXPECT_CALL(*factory, MakeAudioDecoderMock(_, _))
       .WillOnce(Invoke([decoder](const SdpAudioFormat& format,
@@ -100,7 +123,14 @@ TEST(DecoderDatabase, GetDecoder) {
 }
 
 TEST(DecoderDatabase, TypeTests) {
-  DecoderDatabase db(new rtc::RefCountedObject<MockAudioDecoderFactory>);
+  rtc::scoped_refptr<MockAudioDecoderFactory> factory(
+      new rtc::RefCountedObject<MockAudioDecoderFactory>);
+  EXPECT_CALL(*factory, IsSupportedDecoder(_))
+      .WillOnce(Invoke([](const SdpAudioFormat& format) {
+        EXPECT_EQ("pcmu", format.name);
+        return true;
+      }));
+  DecoderDatabase db(factory);
   const uint8_t kPayloadTypePcmU = 0;
   const uint8_t kPayloadTypeCng = 13;
   const uint8_t kPayloadTypeDtmf = 100;
@@ -163,11 +193,19 @@ TEST(DecoderDatabase, ExternalDecoder) {
 }
 
 TEST(DecoderDatabase, CheckPayloadTypes) {
-  DecoderDatabase db(new rtc::RefCountedObject<MockAudioDecoderFactory>);
+  constexpr int kNumPayloads = 10;
+  rtc::scoped_refptr<MockAudioDecoderFactory> factory(
+      new rtc::RefCountedObject<MockAudioDecoderFactory>);
+  EXPECT_CALL(*factory, IsSupportedDecoder(_))
+      .Times(kNumPayloads)
+      .WillRepeatedly(Invoke([](const SdpAudioFormat& format) {
+        EXPECT_EQ("pcmu", format.name);
+        return true;
+      }));
+  DecoderDatabase db(factory);
   // Load a number of payloads into the database. Payload types are 0, 1, ...,
   // while the decoder type is the same for all payload types (this does not
   // matter for the test).
-  const int kNumPayloads = 10;
   for (uint8_t payload_type = 0; payload_type < kNumPayloads; ++payload_type) {
     EXPECT_EQ(DecoderDatabase::kOK,
               db.RegisterPayload(payload_type, NetEqDecoder::kDecoderPCMu, ""));

modules/audio_coding/neteq/delay_manager.h

@@ -16,7 +16,6 @@
 #include <memory>
 #include <vector>
 
-#include "modules/audio_coding/neteq/audio_decoder_impl.h"
 #include "modules/audio_coding/neteq/tick_timer.h"
 #include "rtc_base/constructormagic.h"
 #include "typedefs.h"  // NOLINT(build/include)

modules/audio_coding/neteq/include/neteq.h

@@ -16,9 +16,10 @@
 #include <string>
 #include <vector>
 
+#include "api/audio_codecs/audio_decoder.h"
 #include "api/optional.h"
 #include "common_types.h"  // NOLINT(build/include)
-#include "modules/audio_coding/neteq/audio_decoder_impl.h"
+#include "modules/audio_coding/neteq/neteq_decoder_enum.h"
 #include "rtc_base/constructormagic.h"
 #include "rtc_base/scoped_ref_ptr.h"
 #include "typedefs.h"  // NOLINT(build/include)

modules/audio_coding/neteq/test/RTPencode.cc

@@ -29,7 +29,6 @@
 #include "typedefs.h"  // NOLINT(build/include)
 
 // needed for NetEqDecoder
-#include "modules/audio_coding/neteq/audio_decoder_impl.h"
 #include "modules/audio_coding/neteq/include/neteq.h"
 
 /************************/

modules/audio_coding/neteq/timestamp_scaler.cc

@@ -11,6 +11,7 @@
 #include "modules/audio_coding/neteq/timestamp_scaler.h"
 
 #include "modules/audio_coding/neteq/decoder_database.h"
+#include "rtc_base/checks.h"
 
 namespace webrtc {
 
@@ -59,7 +60,7 @@ uint32_t TimestampScaler::ToInternal(uint32_t external_timestamp,
       first_packet_received_ = true;
     }
     const int64_t external_diff = int64_t{external_timestamp} - external_ref_;
-    assert(denominator_ > 0);  // Should not be possible.
+    RTC_DCHECK_GT(denominator_, 0);
     external_ref_ = external_timestamp;
     internal_ref_ += (external_diff * numerator_) / denominator_;
     return internal_ref_;
@@ -76,7 +77,7 @@ uint32_t TimestampScaler::ToExternal(uint32_t internal_timestamp) const {
     return internal_timestamp;
   } else {
     const int64_t internal_diff = int64_t{internal_timestamp} - internal_ref_;
-    assert(numerator_ > 0);  // Should not be possible.
+    RTC_DCHECK_GT(numerator_, 0);
     // Do not update references in this method.
     // Switch |denominator_| and |numerator_| to convert the other way.
     return external_ref_ + (internal_diff * denominator_) / numerator_;