From 5adaf735dc8caba2cd082b61bc0760ce9f0450f9 Mon Sep 17 00:00:00 2001 From: kwiberg Date: Tue, 4 Oct 2016 09:33:27 -0700 Subject: [PATCH] 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} --- .../acm2/acm_receive_test_oldapi.cc | 27 ++- .../acm2/acm_receive_test_oldapi.h | 7 +- .../modules/audio_coding/acm2/acm_receiver.cc | 25 +++ .../modules/audio_coding/acm2/acm_receiver.h | 4 + .../audio_coding/acm2/audio_coding_module.cc | 18 ++ .../audio_coding_module_unittest_oldapi.cc | 184 ++++++++++-------- .../include/audio_coding_module.h | 5 + .../audio_coding/neteq/decoder_database.cc | 21 +- .../audio_coding/neteq/decoder_database.h | 5 + .../audio_coding/neteq/include/neteq.h | 5 + .../neteq/mock/mock_decoder_database.h | 2 + .../neteq/neteq_external_decoder_unittest.cc | 5 +- .../modules/audio_coding/neteq/neteq_impl.cc | 29 +++ .../modules/audio_coding/neteq/neteq_impl.h | 3 + .../audio_coding/neteq/neteq_impl_unittest.cc | 11 +- .../audio_coding/neteq/neteq_unittest.cc | 58 +++--- 16 files changed, 286 insertions(+), 123 deletions(-) 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 71a8744f81..9f6eb5c9c0 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 +#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 @@ bool RemapPltypeAndUseThisCodec(const char* plname, } return true; } + +AudioCodingModule::Config MakeAcmConfig( + Clock* clock, + rtc::scoped_refptr 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 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 @@ AcmReceiveTestToggleOutputFreqOldApi::AcmReceiveTestToggleOutputFreqOldApi( : 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 5d6fbb3cef..aab68aef7f 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 #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 @@ class AcmReceiveTestOldApi { AcmReceiveTestOldApi(PacketSource* packet_source, AudioSink* audio_sink, int output_freq_hz, - NumOutputChannels exptected_output_channels); + NumOutputChannels exptected_output_channels, + rtc::scoped_refptr decoder_factory); virtual ~AcmReceiveTestOldApi(); // Registers the codecs with default parameters from ACM. @@ -56,6 +59,8 @@ class AcmReceiveTestOldApi { // 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 89eee00fc3..0b19310b84 100644 --- a/webrtc/modules/audio_coding/acm2/acm_receiver.cc +++ b/webrtc/modules/audio_coding/acm2/acm_receiver.cc @@ -230,6 +230,31 @@ int32_t AcmReceiver::AddCodec(int acm_codec_id, 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 06946f41d8..ea85456458 100644 --- a/webrtc/modules/audio_coding/acm2/acm_receiver.h +++ b/webrtc/modules/audio_coding/acm2/acm_receiver.h @@ -113,6 +113,10 @@ class AcmReceiver { 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 99b539ab64..a845c017d1 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 @@ class AudioCodingModuleImpl final : public AudioCodingModule { // 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 @@ int AudioCodingModuleImpl::PlayoutFrequency() const { 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 60e90f7d3b..b4020610b7 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 @@ class AudioCodingModuleTestOldApi : public ::testing::Test { // Set up L16 codec. virtual void SetUpL16Codec() { + audio_format_ = + rtc::Optional(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 @@ class AudioCodingModuleTestOldApi : public ::testing::Test { 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 audio_format_; CodecInst codec_; + Clock* clock_; }; @@ -391,11 +399,14 @@ class AudioCodingModuleTestWithComfortNoiseOldApi : 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 @@ class AcmIsacMtTestOldApi : public AudioCodingModuleMtTestOldApi { void RegisterCodec() override { static_assert(kSampleRateHz == 16000, "test designed for iSAC 16 kHz"); + audio_format_ = + rtc::Optional(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 @@ class AcmReceiverBitExactnessOldApi : public ::testing::Test { 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& external_decoders) { + rtc::scoped_refptr decoder_factory, + rtc::FunctionView decoder_reg) { const std::string input_file_name = webrtc::test::ResourcePath("audio_coding/neteq_universal_new", "rtp"); std::unique_ptr packet_source( @@ -939,16 +958,11 @@ class AcmReceiverBitExactnessOldApi : public ::testing::Test { 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 @@ TEST_F(AcmReceiverBitExactnessOldApi, 8kHzOutput) { Run(8000, PlatformChecksum("dce4890259e9ded50f472455aa470a6f", "1c4ada78b12612147f3026920f8dcc14", "d804791edf2d00be2bc31c81a47368d4", - "b2611f7323ab1209d5056399d0babbf5"), - std::vector()); + "b2611f7323ab1209d5056399d0babbf5")); } TEST_F(AcmReceiverBitExactnessOldApi, 16kHzOutput) { Run(16000, PlatformChecksum("27356bddffaa42b5c841b49aa3a070c5", "5667d1872fc351244092ae995e5a5b32", "53f5dc8088148479ca112c4c6d0e91cb", - "4061a876d64d6cec5a38450acf4f245d"), - std::vector()); + "4061a876d64d6cec5a38450acf4f245d")); } TEST_F(AcmReceiverBitExactnessOldApi, 32kHzOutput) { Run(32000, PlatformChecksum("eb326547e83292305423b0a7ea57fea1", "be7fc3140e6b5188c2e5fae0a394543b", "eab9a0bff17320d6457d04f4c56563c6", - "b60241ef0bac4a75f66eead04e71bb12"), - std::vector()); + "b60241ef0bac4a75f66eead04e71bb12")); } TEST_F(AcmReceiverBitExactnessOldApi, 48kHzOutput) { Run(48000, PlatformChecksum("7eb79ea39b68472a5b04cf9a56e49cda", "f8cdd6e018688b2fff25c9b865bebdbb", "2d18f0f06e7e2fc63b74d06e3c58067f", - "81c3e4d24ebec23ca48f42fbaec4aba0"), - std::vector()); + "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 GetSupportedDecoders() override { + return fact_->GetSupportedDecoders(); + } + std::unique_ptr 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 mock_decoder_; + AudioDecoderPcmU pcmu_decoder_; + DecodeForwarder decode_forwarder_; + rtc::scoped_refptr 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 external_decoders; - external_decoders.push_back(ed); - + rtc::scoped_refptr> factory( + new rtc::RefCountedObject); 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 @@ class AcmSenderBitExactnessOldApi : public ::testing::Test, // 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. diff --git a/webrtc/modules/audio_coding/include/audio_coding_module.h b/webrtc/modules/audio_coding/include/audio_coding_module.h index fc8ae1ed51..946ad1d82a 100644 --- a/webrtc/modules/audio_coding/include/audio_coding_module.h +++ b/webrtc/modules/audio_coding/include/audio_coding_module.h @@ -479,6 +479,11 @@ class AudioCodingModule { // virtual int32_t PlayoutFrequency() const = 0; + // Registers a decoder for the given payload type. Returns true iff + // successful. + virtual bool RegisterReceiveCodec(int rtp_payload_type, + const SdpAudioFormat& audio_format) = 0; + /////////////////////////////////////////////////////////////////////////// // int32_t RegisterReceiveCodec() // Register possible decoders, can be called multiple times for diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.cc b/webrtc/modules/audio_coding/neteq/decoder_database.cc index ebea7e89a8..07c3471a86 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database.cc +++ b/webrtc/modules/audio_coding/neteq/decoder_database.cc @@ -85,7 +85,12 @@ bool DecoderDatabase::DecoderInfo::IsType(const std::string& name) const { rtc::Optional DecoderDatabase::DecoderInfo::CngDecoder::Create(const SdpAudioFormat& format) { if (STR_CASE_CMP(format.name.c_str(), "CN") == 0) { - return rtc::Optional({format.clockrate_hz}); + // CN has a 1:1 RTP clock rate to sample rate ratio. + const int sample_rate_hz = format.clockrate_hz; + RTC_DCHECK(sample_rate_hz == 8000 || sample_rate_hz == 16000 || + sample_rate_hz == 32000 || sample_rate_hz == 48000); + return rtc::Optional( + {sample_rate_hz}); } else { return rtc::Optional(); } @@ -141,6 +146,20 @@ int DecoderDatabase::RegisterPayload(uint8_t rtp_payload_type, return kOK; } +int DecoderDatabase::RegisterPayload(int rtp_payload_type, + const SdpAudioFormat& audio_format) { + if (rtp_payload_type < 0 || rtp_payload_type > 0x7f) { + return kInvalidRtpPayloadType; + } + const auto ret = decoders_.insert(std::make_pair( + rtp_payload_type, DecoderInfo(audio_format, decoder_factory_.get()))); + if (ret.second == false) { + // Database already contains a decoder with type |rtp_payload_type|. + return kDecoderExists; + } + return kOK; +} + int DecoderDatabase::InsertExternal(uint8_t rtp_payload_type, NetEqDecoder codec_type, const std::string& codec_name, diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.h b/webrtc/modules/audio_coding/neteq/decoder_database.h index 6fd7a396c5..157e0225d2 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database.h +++ b/webrtc/modules/audio_coding/neteq/decoder_database.h @@ -145,6 +145,11 @@ class DecoderDatabase { NetEqDecoder codec_type, const std::string& name); + // Registers a decoder for the given payload type. Returns kOK on success; + // otherwise an error code. + virtual int RegisterPayload(int rtp_payload_type, + const SdpAudioFormat& audio_format); + // Registers an externally created AudioDecoder object, and associates it // as a decoder of type |codec_type| with |rtp_payload_type|. virtual int InsertExternal(uint8_t rtp_payload_type, diff --git a/webrtc/modules/audio_coding/neteq/include/neteq.h b/webrtc/modules/audio_coding/neteq/include/neteq.h index 8520905b10..5b17e128d8 100644 --- a/webrtc/modules/audio_coding/neteq/include/neteq.h +++ b/webrtc/modules/audio_coding/neteq/include/neteq.h @@ -178,6 +178,11 @@ class NetEq { const std::string& codec_name, uint8_t rtp_payload_type) = 0; + // Associates |rtp_payload_type| with the given codec, which NetEq will + // instantiate when it needs it. Returns true iff successful. + virtual bool RegisterPayloadType(int rtp_payload_type, + const SdpAudioFormat& audio_format) = 0; + // Removes |rtp_payload_type| from the codec database. Returns 0 on success, // -1 on failure. virtual int RemovePayloadType(uint8_t rtp_payload_type) = 0; diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h b/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h index 03ea2452fc..4018879164 100644 --- a/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h +++ b/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h @@ -35,6 +35,8 @@ class MockDecoderDatabase : public DecoderDatabase { MOCK_METHOD3(RegisterPayload, int(uint8_t rtp_payload_type, NetEqDecoder codec_type, const std::string& name)); + MOCK_METHOD2(RegisterPayload, + int(int rtp_payload_type, const SdpAudioFormat& audio_format)); MOCK_METHOD4(InsertExternal, int(uint8_t rtp_payload_type, NetEqDecoder codec_type, diff --git a/webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc index 3f6a80b5da..f1ccfa9fde 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc @@ -184,9 +184,8 @@ class NetEqExternalVsInternalDecoderTest : public NetEqExternalDecoderUnitTest, } void SetUp() override { - ASSERT_EQ(NetEq::kOK, neteq_internal_->RegisterPayloadType( - NetEqDecoder::kDecoderPCM16Bswb32kHz, - "pcm16-swb32", kPayloadType)); + ASSERT_EQ(true, neteq_internal_->RegisterPayloadType( + kPayloadType, SdpAudioFormat("L16", 32000, 1))); } void GetAndVerifyOutput() override { diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc index e21294aa38..b60f1b86e5 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc @@ -279,6 +279,35 @@ int NetEqImpl::RegisterExternalDecoder(AudioDecoder* decoder, return kOK; } +bool NetEqImpl::RegisterPayloadType(int rtp_payload_type, + const SdpAudioFormat& audio_format) { + LOG(LS_VERBOSE) << "NetEqImpl::RegisterPayloadType: payload type " + << rtp_payload_type << ", codec " << audio_format; + rtc::CritScope lock(&crit_sect_); + switch (decoder_database_->RegisterPayload(rtp_payload_type, audio_format)) { + case DecoderDatabase::kOK: + return true; + case DecoderDatabase::kInvalidRtpPayloadType: + error_code_ = kInvalidRtpPayloadType; + return false; + case DecoderDatabase::kCodecNotSupported: + error_code_ = kCodecNotSupported; + return false; + case DecoderDatabase::kDecoderExists: + error_code_ = kDecoderExists; + return false; + case DecoderDatabase::kInvalidSampleRate: + error_code_ = kInvalidSampleRate; + return false; + case DecoderDatabase::kInvalidPointer: + error_code_ = kInvalidPointer; + return false; + default: + error_code_ = kOtherError; + return false; + } +} + int NetEqImpl::RemovePayloadType(uint8_t rtp_payload_type) { rtc::CritScope lock(&crit_sect_); int ret = decoder_database_->Remove(rtp_payload_type); diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.h b/webrtc/modules/audio_coding/neteq/neteq_impl.h index e143a2a43f..505f0f2a66 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.h +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.h @@ -119,6 +119,9 @@ class NetEqImpl : public webrtc::NetEq { const std::string& codec_name, uint8_t rtp_payload_type) override; + bool RegisterPayloadType(int rtp_payload_type, + const SdpAudioFormat& audio_format) override; + // Removes |rtp_payload_type| from the codec database. Returns 0 on success, // -1 on failure. int RemovePayloadType(uint8_t rtp_payload_type) override; diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc index 8e5a21d876..1859e566e7 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -211,7 +211,7 @@ TEST(NetEq, CreateAndDestroy) { delete neteq; } -TEST_F(NetEqImplTest, RegisterPayloadType) { +TEST_F(NetEqImplTest, RegisterPayloadTypeNetEqDecoder) { CreateInstance(); uint8_t rtp_payload_type = 0; NetEqDecoder codec_type = NetEqDecoder::kDecoderPCMu; @@ -221,6 +221,15 @@ TEST_F(NetEqImplTest, RegisterPayloadType) { neteq_->RegisterPayloadType(codec_type, kCodecName, rtp_payload_type); } +TEST_F(NetEqImplTest, RegisterPayloadType) { + CreateInstance(); + constexpr int rtp_payload_type = 0; + const SdpAudioFormat format("pcmu", 8000, 1); + EXPECT_CALL(*mock_decoder_database_, + RegisterPayload(rtp_payload_type, format)); + neteq_->RegisterPayloadType(rtp_payload_type, format); +} + TEST_F(NetEqImplTest, RemovePayloadType) { CreateInstance(); uint8_t rtp_payload_type = 0; diff --git a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc index c227b5bb39..11fd9b440f 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc @@ -45,6 +45,8 @@ RTC_POP_IGNORING_WUNDEF() DEFINE_bool(gen_ref, false, "Generate reference files."); +namespace webrtc { + namespace { const std::string& PlatformChecksum(const std::string& checksum_general, @@ -110,52 +112,42 @@ void AddMessage(FILE* file, rtc::MessageDigest* digest, #endif // WEBRTC_NETEQ_UNITTEST_BITEXACT void LoadDecoders(webrtc::NetEq* neteq) { - // Load PCMu. - ASSERT_EQ(0, neteq->RegisterPayloadType(webrtc::NetEqDecoder::kDecoderPCMu, - "pcmu", 0)); - // Load PCMa. + ASSERT_EQ(true, + neteq->RegisterPayloadType(0, SdpAudioFormat("pcmu", 8000, 1))); + // Use non-SdpAudioFormat argument when registering PCMa, so that we get test + // coverage for that as well. ASSERT_EQ(0, neteq->RegisterPayloadType(webrtc::NetEqDecoder::kDecoderPCMa, "pcma", 8)); #ifdef WEBRTC_CODEC_ILBC - // Load iLBC. - ASSERT_EQ(0, neteq->RegisterPayloadType(webrtc::NetEqDecoder::kDecoderILBC, - "ilbc", 102)); + ASSERT_EQ(true, + neteq->RegisterPayloadType(102, SdpAudioFormat("ilbc", 8000, 1))); #endif #if defined(WEBRTC_CODEC_ISAC) || defined(WEBRTC_CODEC_ISACFX) - // Load iSAC. - ASSERT_EQ(0, neteq->RegisterPayloadType(webrtc::NetEqDecoder::kDecoderISAC, - "isac", 103)); + ASSERT_EQ(true, + neteq->RegisterPayloadType(103, SdpAudioFormat("isac", 16000, 1))); #endif #ifdef WEBRTC_CODEC_ISAC - // Load iSAC SWB. - ASSERT_EQ(0, neteq->RegisterPayloadType(webrtc::NetEqDecoder::kDecoderISACswb, - "isac-swb", 104)); + ASSERT_EQ(true, + neteq->RegisterPayloadType(104, SdpAudioFormat("isac", 32000, 1))); #endif #ifdef WEBRTC_CODEC_OPUS - ASSERT_EQ(0, neteq->RegisterPayloadType(webrtc::NetEqDecoder::kDecoderOpus, - "opus", 111)); + ASSERT_EQ(true, + neteq->RegisterPayloadType( + 111, SdpAudioFormat("opus", 48000, 2, {{"stereo", "0"}}))); #endif - // Load PCM16B nb. - ASSERT_EQ(0, neteq->RegisterPayloadType(webrtc::NetEqDecoder::kDecoderPCM16B, - "pcm16-nb", 93)); - // Load PCM16B wb. - ASSERT_EQ(0, neteq->RegisterPayloadType( - webrtc::NetEqDecoder::kDecoderPCM16Bwb, "pcm16-wb", 94)); - // Load PCM16B swb32. - ASSERT_EQ( - 0, neteq->RegisterPayloadType( - webrtc::NetEqDecoder::kDecoderPCM16Bswb32kHz, "pcm16-swb32", 95)); - // Load CNG 8 kHz. - ASSERT_EQ(0, neteq->RegisterPayloadType(webrtc::NetEqDecoder::kDecoderCNGnb, - "cng-nb", 13)); - // Load CNG 16 kHz. - ASSERT_EQ(0, neteq->RegisterPayloadType(webrtc::NetEqDecoder::kDecoderCNGwb, - "cng-wb", 98)); + ASSERT_EQ(true, + neteq->RegisterPayloadType(93, SdpAudioFormat("L16", 8000, 1))); + ASSERT_EQ(true, + neteq->RegisterPayloadType(94, SdpAudioFormat("L16", 16000, 1))); + ASSERT_EQ(true, + neteq->RegisterPayloadType(95, SdpAudioFormat("L16", 32000, 1))); + ASSERT_EQ(true, + neteq->RegisterPayloadType(13, SdpAudioFormat("cn", 8000, 1))); + ASSERT_EQ(true, + neteq->RegisterPayloadType(98, SdpAudioFormat("cn", 16000, 1))); } } // namespace -namespace webrtc { - class ResultSink { public: explicit ResultSink(const std::string& output_file);