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}diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn
index b222ad7..3009012 100644
--- a/modules/audio_coding/BUILD.gn
+++ b/modules/audio_coding/BUILD.gn
@@ -938,8 +938,6 @@
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 @@
deps = [
":audio_coding_module_typedefs",
":cng",
- ":g711",
- ":isac_fix",
":neteq_decoder_enum",
- ":pcm16b",
"..:module_api",
"../..:webrtc_common",
"../../api:optional",
@@ -1025,28 +1020,6 @@
"../../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
diff --git a/modules/audio_coding/acm2/audio_coding_module_unittest.cc b/modules/audio_coding/acm2/audio_coding_module_unittest.cc
index 6d0c37e..f95dca2 100644
--- a/modules/audio_coding/acm2/audio_coding_module_unittest.cc
+++ b/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"
diff --git a/modules/audio_coding/neteq/audio_decoder_impl.cc b/modules/audio_coding/neteq/audio_decoder_impl.cc
deleted file mode 100644
index 01e934b..0000000
--- a/modules/audio_coding/neteq/audio_decoder_impl.cc
+++ /dev/null
@@ -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
diff --git a/modules/audio_coding/neteq/audio_decoder_impl.h b/modules/audio_coding/neteq/audio_decoder_impl.h
deleted file mode 100644
index 87757ba..0000000
--- a/modules/audio_coding/neteq/audio_decoder_impl.h
+++ /dev/null
@@ -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_
diff --git a/modules/audio_coding/neteq/audio_decoder_unittest.cc b/modules/audio_coding/neteq/audio_decoder_unittest.cc
index 203b856..ae164fd 100644
--- a/modules/audio_coding/neteq/audio_decoder_unittest.cc
+++ b/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 @@
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
diff --git a/modules/audio_coding/neteq/decoder_database.cc b/modules/audio_coding/neteq/decoder_database.cc
index feb0203..bd0053b 100644
--- a/modules/audio_coding/neteq/decoder_database.cc
+++ b/modules/audio_coding/neteq/decoder_database.cc
@@ -58,6 +58,17 @@
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 @@
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) {
diff --git a/modules/audio_coding/neteq/decoder_database.h b/modules/audio_coding/neteq/decoder_database.h
index b41ff37..5f0d173 100644
--- a/modules/audio_coding/neteq/decoder_database.h
+++ b/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 @@
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;
diff --git a/modules/audio_coding/neteq/decoder_database_unittest.cc b/modules/audio_coding/neteq/decoder_database_unittest.cc
index 626420a..53fbadf 100644
--- a/modules/audio_coding/neteq/decoder_database_unittest.cc
+++ b/modules/audio_coding/neteq/decoder_database_unittest.cc
@@ -34,7 +34,14 @@
}
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, 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, 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, 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, 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, ""));
diff --git a/modules/audio_coding/neteq/delay_manager.h b/modules/audio_coding/neteq/delay_manager.h
index 0de03fc..0d082c8 100644
--- a/modules/audio_coding/neteq/delay_manager.h
+++ b/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)
diff --git a/modules/audio_coding/neteq/include/neteq.h b/modules/audio_coding/neteq/include/neteq.h
index e6cafa8..d66a3ea 100644
--- a/modules/audio_coding/neteq/include/neteq.h
+++ b/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)
diff --git a/modules/audio_coding/neteq/test/RTPencode.cc b/modules/audio_coding/neteq/test/RTPencode.cc
index 289993e..9e3bc6c 100644
--- a/modules/audio_coding/neteq/test/RTPencode.cc
+++ b/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"
/************************/
diff --git a/modules/audio_coding/neteq/timestamp_scaler.cc b/modules/audio_coding/neteq/timestamp_scaler.cc
index 3b67a38..d7aa9fe 100644
--- a/modules/audio_coding/neteq/timestamp_scaler.cc
+++ b/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 @@
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 @@
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_;