AudioCodingModule: Specify decoders using SdpAudioFormat
NetEq already uses SdpAudioFormat internally; this CL adds an
AudioCodingModule::RegisterReceiveCodec overload that accepts
SdpAudioFormat, and propagates it through AcmReceiver into NetEq.
The intention is to get rid of the other ways to specify decoders and
always use SdpAudioFormat. (And eventually to do the same for encoders
too.)
NOTRY=true
BUG=5801
Review-Url: https://codereview.webrtc.org/2365653004
Cr-Commit-Position: refs/heads/master@{#14506}
diff --git a/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.cc b/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.cc
index 71a8744..9f6eb5c 100644
--- a/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.cc
+++ b/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.cc
@@ -15,6 +15,7 @@
#include <memory>
+#include "webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.h"
#include "webrtc/modules/audio_coding/include/audio_coding_module.h"
#include "webrtc/modules/audio_coding/neteq/tools/audio_sink.h"
#include "webrtc/modules/audio_coding/neteq/tools/packet.h"
@@ -97,20 +98,32 @@
}
return true;
}
+
+AudioCodingModule::Config MakeAcmConfig(
+ Clock* clock,
+ rtc::scoped_refptr<AudioDecoderFactory> decoder_factory) {
+ AudioCodingModule::Config config;
+ config.id = 0;
+ config.clock = clock;
+ config.decoder_factory = std::move(decoder_factory);
+ return config;
+}
+
} // namespace
AcmReceiveTestOldApi::AcmReceiveTestOldApi(
PacketSource* packet_source,
AudioSink* audio_sink,
int output_freq_hz,
- NumOutputChannels exptected_output_channels)
+ NumOutputChannels exptected_output_channels,
+ rtc::scoped_refptr<AudioDecoderFactory> decoder_factory)
: clock_(0),
- acm_(webrtc::AudioCodingModule::Create(0, &clock_)),
+ acm_(webrtc::AudioCodingModule::Create(
+ MakeAcmConfig(&clock_, std::move(decoder_factory)))),
packet_source_(packet_source),
audio_sink_(audio_sink),
output_freq_hz_(output_freq_hz),
- exptected_output_channels_(exptected_output_channels) {
-}
+ exptected_output_channels_(exptected_output_channels) {}
AcmReceiveTestOldApi::~AcmReceiveTestOldApi() = default;
@@ -209,12 +222,12 @@
: AcmReceiveTestOldApi(packet_source,
audio_sink,
output_freq_hz_1,
- exptected_output_channels),
+ exptected_output_channels,
+ CreateBuiltinAudioDecoderFactory()),
output_freq_hz_1_(output_freq_hz_1),
output_freq_hz_2_(output_freq_hz_2),
toggle_period_ms_(toggle_period_ms),
- last_toggle_time_ms_(clock_.TimeInMilliseconds()) {
-}
+ last_toggle_time_ms_(clock_.TimeInMilliseconds()) {}
void AcmReceiveTestToggleOutputFreqOldApi::AfterGetAudio() {
if (clock_.TimeInMilliseconds() >= last_toggle_time_ms_ + toggle_period_ms_) {
diff --git a/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.h b/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.h
index 5d6fbb3..aab68ae 100644
--- a/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.h
+++ b/webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.h
@@ -15,6 +15,8 @@
#include <string>
#include "webrtc/base/constructormagic.h"
+#include "webrtc/base/scoped_ref_ptr.h"
+#include "webrtc/modules/audio_coding/codecs/audio_decoder_factory.h"
#include "webrtc/system_wrappers/include/clock.h"
namespace webrtc {
@@ -37,7 +39,8 @@
AcmReceiveTestOldApi(PacketSource* packet_source,
AudioSink* audio_sink,
int output_freq_hz,
- NumOutputChannels exptected_output_channels);
+ NumOutputChannels exptected_output_channels,
+ rtc::scoped_refptr<AudioDecoderFactory> decoder_factory);
virtual ~AcmReceiveTestOldApi();
// Registers the codecs with default parameters from ACM.
@@ -56,6 +59,8 @@
// Runs the test and returns true if successful.
void Run();
+ AudioCodingModule* get_acm() { return acm_.get(); }
+
protected:
// Method is called after each block of output audio is received from ACM.
virtual void AfterGetAudio() {}
diff --git a/webrtc/modules/audio_coding/acm2/acm_receiver.cc b/webrtc/modules/audio_coding/acm2/acm_receiver.cc
index 89eee00..0b19310 100644
--- a/webrtc/modules/audio_coding/acm2/acm_receiver.cc
+++ b/webrtc/modules/audio_coding/acm2/acm_receiver.cc
@@ -230,6 +230,31 @@
return 0;
}
+bool AcmReceiver::AddCodec(int rtp_payload_type,
+ const SdpAudioFormat& audio_format) {
+ const auto old_format = neteq_->GetDecoderFormat(rtp_payload_type);
+ if (old_format && *old_format == audio_format) {
+ // Re-registering the same codec. Do nothing and return.
+ return true;
+ }
+
+ if (neteq_->RemovePayloadType(rtp_payload_type) != NetEq::kOK &&
+ neteq_->LastError() != NetEq::kDecoderNotFound) {
+ LOG(LERROR) << "AcmReceiver::AddCodec: Could not remove existing decoder"
+ " for payload type "
+ << rtp_payload_type;
+ return false;
+ }
+
+ const bool success =
+ neteq_->RegisterPayloadType(rtp_payload_type, audio_format);
+ if (!success) {
+ LOG(LERROR) << "AcmReceiver::AddCodec failed for payload type "
+ << rtp_payload_type << ", decoder format " << audio_format;
+ }
+ return success;
+}
+
void AcmReceiver::FlushBuffers() {
neteq_->FlushBuffers();
}
diff --git a/webrtc/modules/audio_coding/acm2/acm_receiver.h b/webrtc/modules/audio_coding/acm2/acm_receiver.h
index 06946f4..ea85456 100644
--- a/webrtc/modules/audio_coding/acm2/acm_receiver.h
+++ b/webrtc/modules/audio_coding/acm2/acm_receiver.h
@@ -113,6 +113,10 @@
AudioDecoder* audio_decoder,
const std::string& name);
+ // Adds a new decoder to the NetEq codec database. Returns true iff
+ // successful.
+ bool AddCodec(int rtp_payload_type, const SdpAudioFormat& audio_format);
+
//
// Sets a minimum delay for packet buffer. The given delay is maintained,
// unless channel condition dictates a higher delay.
diff --git a/webrtc/modules/audio_coding/acm2/audio_coding_module.cc b/webrtc/modules/audio_coding/acm2/audio_coding_module.cc
index 99b539a..a845c01 100644
--- a/webrtc/modules/audio_coding/acm2/audio_coding_module.cc
+++ b/webrtc/modules/audio_coding/acm2/audio_coding_module.cc
@@ -121,6 +121,9 @@
// Get current playout frequency.
int PlayoutFrequency() const override;
+ bool RegisterReceiveCodec(int rtp_payload_type,
+ const SdpAudioFormat& audio_format) override;
+
int RegisterReceiveCodec(const CodecInst& receive_codec) override;
int RegisterReceiveCodec(
const CodecInst& receive_codec,
@@ -987,6 +990,21 @@
return receiver_.last_output_sample_rate_hz();
}
+bool AudioCodingModuleImpl::RegisterReceiveCodec(
+ int rtp_payload_type,
+ const SdpAudioFormat& audio_format) {
+ rtc::CritScope lock(&acm_crit_sect_);
+ RTC_DCHECK(receiver_initialized_);
+
+ if (!acm2::RentACodec::IsPayloadTypeValid(rtp_payload_type)) {
+ LOG_F(LS_ERROR) << "Invalid payload-type " << rtp_payload_type
+ << " for decoder.";
+ return false;
+ }
+
+ return receiver_.AddCodec(rtp_payload_type, audio_format);
+}
+
int AudioCodingModuleImpl::RegisterReceiveCodec(const CodecInst& codec) {
rtc::CritScope lock(&acm_crit_sect_);
auto* ef = encoder_factory_.get();
diff --git a/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc b/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
index 60e90f7..b402061 100644
--- a/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
+++ b/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
@@ -19,6 +19,7 @@
#include "webrtc/base/thread_annotations.h"
#include "webrtc/modules/audio_coding/acm2/acm_receive_test_oldapi.h"
#include "webrtc/modules/audio_coding/acm2/acm_send_test_oldapi.h"
+#include "webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.h"
#include "webrtc/modules/audio_coding/codecs/audio_encoder.h"
#include "webrtc/modules/audio_coding/codecs/g711/audio_decoder_pcm.h"
#include "webrtc/modules/audio_coding/codecs/g711/audio_encoder_pcm.h"
@@ -182,13 +183,15 @@
// Set up L16 codec.
virtual void SetUpL16Codec() {
+ audio_format_ =
+ rtc::Optional<SdpAudioFormat>(SdpAudioFormat("L16", kSampleRateHz, 1));
ASSERT_EQ(0, AudioCodingModule::Codec("L16", &codec_, kSampleRateHz, 1));
codec_.pltype = kPayloadType;
}
virtual void RegisterCodec() {
- ASSERT_EQ(0, acm_->RegisterReceiveCodec(codec_));
- ASSERT_EQ(0, acm_->RegisterSendCodec(codec_));
+ EXPECT_EQ(true, acm_->RegisterReceiveCodec(kPayloadType, *audio_format_));
+ EXPECT_EQ(0, acm_->RegisterSendCodec(codec_));
}
virtual void InsertPacketAndPullAudio() {
@@ -232,7 +235,12 @@
PacketizationCallbackStubOldApi packet_cb_;
WebRtcRTPHeader rtp_header_;
AudioFrame input_frame_;
+
+ // These two have to be kept in sync for now. In the future, we'll be able to
+ // eliminate the CodecInst and keep only the SdpAudioFormat.
+ rtc::Optional<SdpAudioFormat> audio_format_;
CodecInst codec_;
+
Clock* clock_;
};
@@ -391,11 +399,14 @@
: public AudioCodingModuleTestOldApi {
protected:
void RegisterCngCodec(int rtp_payload_type) {
+ EXPECT_EQ(true,
+ acm_->RegisterReceiveCodec(
+ rtp_payload_type, SdpAudioFormat("cn", kSampleRateHz, 1)));
+
CodecInst codec;
- AudioCodingModule::Codec("CN", &codec, kSampleRateHz, 1);
+ EXPECT_EQ(0, AudioCodingModule::Codec("CN", &codec, kSampleRateHz, 1));
codec.pltype = rtp_payload_type;
- ASSERT_EQ(0, acm_->RegisterReceiveCodec(codec));
- ASSERT_EQ(0, acm_->RegisterSendCodec(codec));
+ EXPECT_EQ(0, acm_->RegisterSendCodec(codec));
}
void VerifyEncoding() override {
@@ -651,13 +662,15 @@
void RegisterCodec() override {
static_assert(kSampleRateHz == 16000, "test designed for iSAC 16 kHz");
+ audio_format_ =
+ rtc::Optional<SdpAudioFormat>(SdpAudioFormat("isac", kSampleRateHz, 1));
AudioCodingModule::Codec("ISAC", &codec_, kSampleRateHz, 1);
codec_.pltype = kPayloadType;
// Register iSAC codec in ACM, effectively unregistering the PCM16B codec
// registered in AudioCodingModuleTestOldApi::SetUp();
- ASSERT_EQ(0, acm_->RegisterReceiveCodec(codec_));
- ASSERT_EQ(0, acm_->RegisterSendCodec(codec_));
+ EXPECT_EQ(true, acm_->RegisterReceiveCodec(kPayloadType, *audio_format_));
+ EXPECT_EQ(0, acm_->RegisterSendCodec(codec_));
}
void InsertPacket() override {
@@ -914,9 +927,15 @@
std::string name;
};
+ void Run(int output_freq_hz, const std::string& checksum_ref) {
+ Run(output_freq_hz, checksum_ref, CreateBuiltinAudioDecoderFactory(),
+ [](AudioCodingModule*) {});
+ }
+
void Run(int output_freq_hz,
const std::string& checksum_ref,
- const std::vector<ExternalDecoder>& external_decoders) {
+ rtc::scoped_refptr<AudioDecoderFactory> decoder_factory,
+ rtc::FunctionView<void(AudioCodingModule*)> decoder_reg) {
const std::string input_file_name =
webrtc::test::ResourcePath("audio_coding/neteq_universal_new", "rtp");
std::unique_ptr<test::RtpFileSource> packet_source(
@@ -939,16 +958,11 @@
test::AudioSinkFork output(&checksum, &output_file);
test::AcmReceiveTestOldApi test(
- packet_source.get(),
- &output,
- output_freq_hz,
- test::AcmReceiveTestOldApi::kArbitraryChannels);
+ packet_source.get(), &output, output_freq_hz,
+ test::AcmReceiveTestOldApi::kArbitraryChannels,
+ std::move(decoder_factory));
ASSERT_NO_FATAL_FAILURE(test.RegisterNetEqTestCodecs());
- for (const auto& ed : external_decoders) {
- ASSERT_EQ(0, test.RegisterExternalReceiveCodec(
- ed.rtp_payload_type, ed.external_decoder,
- ed.sample_rate_hz, ed.num_channels, ed.name));
- }
+ decoder_reg(test.get_acm());
test.Run();
std::string checksum_string = checksum.Finish();
@@ -965,95 +979,110 @@
Run(8000, PlatformChecksum("dce4890259e9ded50f472455aa470a6f",
"1c4ada78b12612147f3026920f8dcc14",
"d804791edf2d00be2bc31c81a47368d4",
- "b2611f7323ab1209d5056399d0babbf5"),
- std::vector<ExternalDecoder>());
+ "b2611f7323ab1209d5056399d0babbf5"));
}
TEST_F(AcmReceiverBitExactnessOldApi, 16kHzOutput) {
Run(16000, PlatformChecksum("27356bddffaa42b5c841b49aa3a070c5",
"5667d1872fc351244092ae995e5a5b32",
"53f5dc8088148479ca112c4c6d0e91cb",
- "4061a876d64d6cec5a38450acf4f245d"),
- std::vector<ExternalDecoder>());
+ "4061a876d64d6cec5a38450acf4f245d"));
}
TEST_F(AcmReceiverBitExactnessOldApi, 32kHzOutput) {
Run(32000, PlatformChecksum("eb326547e83292305423b0a7ea57fea1",
"be7fc3140e6b5188c2e5fae0a394543b",
"eab9a0bff17320d6457d04f4c56563c6",
- "b60241ef0bac4a75f66eead04e71bb12"),
- std::vector<ExternalDecoder>());
+ "b60241ef0bac4a75f66eead04e71bb12"));
}
TEST_F(AcmReceiverBitExactnessOldApi, 48kHzOutput) {
Run(48000, PlatformChecksum("7eb79ea39b68472a5b04cf9a56e49cda",
"f8cdd6e018688b2fff25c9b865bebdbb",
"2d18f0f06e7e2fc63b74d06e3c58067f",
- "81c3e4d24ebec23ca48f42fbaec4aba0"),
- std::vector<ExternalDecoder>());
+ "81c3e4d24ebec23ca48f42fbaec4aba0"));
}
TEST_F(AcmReceiverBitExactnessOldApi, 48kHzOutputExternalDecoder) {
- // Class intended to forward a call from a mock DecodeInternal to Decode on
- // the real decoder's Decode. DecodeInternal for the real decoder isn't
- // public.
- class DecodeForwarder {
+ class ADFactory : public AudioDecoderFactory {
public:
- DecodeForwarder(AudioDecoder* decoder) : decoder_(decoder) {}
- int Decode(const uint8_t* encoded,
- size_t encoded_len,
- int sample_rate_hz,
- int16_t* decoded,
- AudioDecoder::SpeechType* speech_type) {
- return decoder_->Decode(encoded, encoded_len, sample_rate_hz,
- decoder_->PacketDuration(encoded, encoded_len) *
- decoder_->Channels() * sizeof(int16_t),
- decoded, speech_type);
+ ADFactory()
+ : mock_decoder_(new MockAudioDecoder()),
+ pcmu_decoder_(1),
+ decode_forwarder_(&pcmu_decoder_),
+ fact_(CreateBuiltinAudioDecoderFactory()) {
+ // Set expectations on the mock decoder and also delegate the calls to
+ // the real decoder.
+ EXPECT_CALL(*mock_decoder_, IncomingPacket(_, _, _, _, _))
+ .Times(AtLeast(1))
+ .WillRepeatedly(
+ Invoke(&pcmu_decoder_, &AudioDecoderPcmU::IncomingPacket));
+ EXPECT_CALL(*mock_decoder_, SampleRateHz())
+ .Times(AtLeast(1))
+ .WillRepeatedly(
+ Invoke(&pcmu_decoder_, &AudioDecoderPcmU::SampleRateHz));
+ EXPECT_CALL(*mock_decoder_, Channels())
+ .Times(AtLeast(1))
+ .WillRepeatedly(Invoke(&pcmu_decoder_, &AudioDecoderPcmU::Channels));
+ EXPECT_CALL(*mock_decoder_, DecodeInternal(_, _, _, _, _))
+ .Times(AtLeast(1))
+ .WillRepeatedly(Invoke(&decode_forwarder_, &DecodeForwarder::Decode));
+ EXPECT_CALL(*mock_decoder_, HasDecodePlc())
+ .Times(AtLeast(1))
+ .WillRepeatedly(
+ Invoke(&pcmu_decoder_, &AudioDecoderPcmU::HasDecodePlc));
+ EXPECT_CALL(*mock_decoder_, PacketDuration(_, _))
+ .Times(AtLeast(1))
+ .WillRepeatedly(
+ Invoke(&pcmu_decoder_, &AudioDecoderPcmU::PacketDuration));
+ EXPECT_CALL(*mock_decoder_, Die());
+ }
+ std::vector<AudioCodecSpec> GetSupportedDecoders() override {
+ return fact_->GetSupportedDecoders();
+ }
+ std::unique_ptr<AudioDecoder> MakeAudioDecoder(
+ const SdpAudioFormat& format) override {
+ return format.name == "MockPCMu" ? std::move(mock_decoder_)
+ : fact_->MakeAudioDecoder(format);
}
private:
- AudioDecoder* const decoder_;
+ // Class intended to forward a call from a mock DecodeInternal to Decode on
+ // the real decoder's Decode. DecodeInternal for the real decoder isn't
+ // public.
+ class DecodeForwarder {
+ public:
+ DecodeForwarder(AudioDecoder* decoder) : decoder_(decoder) {}
+ int Decode(const uint8_t* encoded,
+ size_t encoded_len,
+ int sample_rate_hz,
+ int16_t* decoded,
+ AudioDecoder::SpeechType* speech_type) {
+ return decoder_->Decode(encoded, encoded_len, sample_rate_hz,
+ decoder_->PacketDuration(encoded, encoded_len) *
+ decoder_->Channels() * sizeof(int16_t),
+ decoded, speech_type);
+ }
+
+ private:
+ AudioDecoder* const decoder_;
+ };
+
+ std::unique_ptr<MockAudioDecoder> mock_decoder_;
+ AudioDecoderPcmU pcmu_decoder_;
+ DecodeForwarder decode_forwarder_;
+ rtc::scoped_refptr<AudioDecoderFactory> fact_; // Fallback factory.
};
- AudioDecoderPcmU decoder(1);
- DecodeForwarder decode_forwarder(&decoder);
- MockAudioDecoder mock_decoder;
- // Set expectations on the mock decoder and also delegate the calls to the
- // real decoder.
- EXPECT_CALL(mock_decoder, IncomingPacket(_, _, _, _, _))
- .Times(AtLeast(1))
- .WillRepeatedly(Invoke(&decoder, &AudioDecoderPcmU::IncomingPacket));
- EXPECT_CALL(mock_decoder, SampleRateHz())
- .Times(AtLeast(1))
- .WillRepeatedly(Invoke(&decoder, &AudioDecoderPcmU::SampleRateHz));
- EXPECT_CALL(mock_decoder, Channels())
- .Times(AtLeast(1))
- .WillRepeatedly(Invoke(&decoder, &AudioDecoderPcmU::Channels));
- EXPECT_CALL(mock_decoder, DecodeInternal(_, _, _, _, _))
- .Times(AtLeast(1))
- .WillRepeatedly(Invoke(&decode_forwarder, &DecodeForwarder::Decode));
- EXPECT_CALL(mock_decoder, HasDecodePlc())
- .Times(AtLeast(1))
- .WillRepeatedly(Invoke(&decoder, &AudioDecoderPcmU::HasDecodePlc));
- EXPECT_CALL(mock_decoder, PacketDuration(_, _))
- .Times(AtLeast(1))
- .WillRepeatedly(Invoke(&decoder, &AudioDecoderPcmU::PacketDuration));
- ExternalDecoder ed;
- ed.rtp_payload_type = 0;
- ed.external_decoder = &mock_decoder;
- ed.sample_rate_hz = 8000;
- ed.num_channels = 1;
- ed.name = "MockPCMU";
- std::vector<ExternalDecoder> external_decoders;
- external_decoders.push_back(ed);
-
+ rtc::scoped_refptr<rtc::RefCountedObject<ADFactory>> factory(
+ new rtc::RefCountedObject<ADFactory>);
Run(48000, PlatformChecksum("7eb79ea39b68472a5b04cf9a56e49cda",
"f8cdd6e018688b2fff25c9b865bebdbb",
"2d18f0f06e7e2fc63b74d06e3c58067f",
"81c3e4d24ebec23ca48f42fbaec4aba0"),
- external_decoders);
-
- EXPECT_CALL(mock_decoder, Die());
+ factory, [](AudioCodingModule* acm) {
+ acm->RegisterReceiveCodec(0, {"MockPCMu", 8000, 1});
+ });
}
#endif
@@ -1141,8 +1170,9 @@
// Have the output audio sent both to file and to the checksum calculator.
test::AudioSinkFork output(&audio_checksum, &output_file);
const int kOutputFreqHz = 8000;
- test::AcmReceiveTestOldApi receive_test(
- this, &output, kOutputFreqHz, expected_channels);
+ test::AcmReceiveTestOldApi receive_test(this, &output, kOutputFreqHz,
+ expected_channels,
+ CreateBuiltinAudioDecoderFactory());
ASSERT_NO_FATAL_FAILURE(receive_test.RegisterDefaultCodecs());
// This is where the actual test is executed.