From 342f74005fc47dc32555907a3eb07718fcb028a5 Mon Sep 17 00:00:00 2001 From: kwiberg Date: Thu, 16 Jun 2016 03:18:00 -0700 Subject: [PATCH] NetEq: Ask AudioDecoder for sample rate instead of passing it as an argument BUG=webrtc:5801 NOTRY=true Review-Url: https://codereview.webrtc.org/2027993002 Cr-Commit-Position: refs/heads/master@{#13162} --- .../modules/audio_coding/acm2/acm_receiver.cc | 2 +- .../audio_coding_module_unittest_oldapi.cc | 3 +++ .../audio_coding/neteq/decoder_database.cc | 16 +++++------- .../audio_coding/neteq/decoder_database.h | 16 +++--------- .../neteq/decoder_database_unittest.cc | 2 +- .../audio_coding/neteq/include/neteq.h | 12 ++++----- .../neteq/mock/mock_decoder_database.h | 9 ++++--- .../modules/audio_coding/neteq/neteq_impl.cc | 7 +++-- .../modules/audio_coding/neteq/neteq_impl.h | 3 +-- .../audio_coding/neteq/neteq_impl_unittest.cc | 26 +++++++++++++------ .../tools/neteq_external_decoder_test.cc | 2 +- 11 files changed, 48 insertions(+), 50 deletions(-) diff --git a/webrtc/modules/audio_coding/acm2/acm_receiver.cc b/webrtc/modules/audio_coding/acm2/acm_receiver.cc index c15239f0f3..bad484cda4 100644 --- a/webrtc/modules/audio_coding/acm2/acm_receiver.cc +++ b/webrtc/modules/audio_coding/acm2/acm_receiver.cc @@ -242,7 +242,7 @@ int32_t AcmReceiver::AddCodec(int acm_codec_id, ret_val = neteq_->RegisterPayloadType(neteq_decoder, name, payload_type); } else { ret_val = neteq_->RegisterExternalDecoder( - audio_decoder, neteq_decoder, name, payload_type, sample_rate_hz); + audio_decoder, neteq_decoder, name, payload_type); } if (ret_val != NetEq::kOK) { LOG(LERROR) << "AcmReceiver::AddCodec " << acm_codec_id 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 55cb5726c1..ce8d86d36a 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 @@ -1019,6 +1019,9 @@ TEST_F(AcmReceiverBitExactnessOldApi, 48kHzOutputExternalDecoder) { 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)); diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.cc b/webrtc/modules/audio_coding/neteq/decoder_database.cc index ce402a76ac..165522f0df 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database.cc +++ b/webrtc/modules/audio_coding/neteq/decoder_database.cc @@ -32,16 +32,16 @@ DecoderDatabase::DecoderInfo::DecoderInfo(NetEqDecoder ct, : codec_type(ct), name(nm), audio_format_(acm2::RentACodec::NetEqDecoderToSdpAudioFormat(ct)), + external_decoder_(nullptr), cng_decoder_(CngDecoder::Create(ct)) {} DecoderDatabase::DecoderInfo::DecoderInfo(NetEqDecoder ct, const std::string& nm, - int sample_rate_hz, AudioDecoder* ext_dec) : codec_type(ct), name(nm), audio_format_(acm2::RentACodec::NetEqDecoderToSdpAudioFormat(ct)), - external_decoder({sample_rate_hz, ext_dec}) { + external_decoder_(ext_dec) { RTC_CHECK(ext_dec); } @@ -50,10 +50,10 @@ DecoderDatabase::DecoderInfo::~DecoderInfo() = default; AudioDecoder* DecoderDatabase::DecoderInfo::GetDecoder( AudioDecoderFactory* factory) { - if (external_decoder) { + if (external_decoder_) { RTC_DCHECK(!decoder_); - RTC_DCHECK(external_decoder->decoder); - return external_decoder->decoder; + RTC_DCHECK(!cng_decoder_); + return external_decoder_; } RTC_DCHECK(audio_format_); if (!decoder_) { @@ -115,7 +115,6 @@ int DecoderDatabase::RegisterPayload(uint8_t rtp_payload_type, int DecoderDatabase::InsertExternal(uint8_t rtp_payload_type, NetEqDecoder codec_type, const std::string& codec_name, - int fs_hz, AudioDecoder* decoder) { if (rtp_payload_type > 0x7F) { return kInvalidRtpPayloadType; @@ -123,14 +122,11 @@ int DecoderDatabase::InsertExternal(uint8_t rtp_payload_type, if (!CodecSupported(codec_type)) { return kCodecNotSupported; } - if (fs_hz != 8000 && fs_hz != 16000 && fs_hz != 32000 && fs_hz != 48000) { - return kInvalidSampleRate; - } if (!decoder) { return kInvalidPointer; } std::pair ret; - DecoderInfo info(codec_type, codec_name, fs_hz, decoder); + DecoderInfo info(codec_type, codec_name, decoder); ret = decoders_.insert(std::make_pair(rtp_payload_type, std::move(info))); if (ret.second == false) { // Database already contains a decoder with type |rtp_payload_type|. diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.h b/webrtc/modules/audio_coding/neteq/decoder_database.h index 4169dc2b1b..404f2575b2 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database.h +++ b/webrtc/modules/audio_coding/neteq/decoder_database.h @@ -44,7 +44,6 @@ class DecoderDatabase { DecoderInfo(NetEqDecoder ct, const std::string& nm); DecoderInfo(NetEqDecoder ct, const std::string& nm, - int sample_rate_hz, AudioDecoder* ext_dec); DecoderInfo(DecoderInfo&&); ~DecoderInfo(); @@ -57,10 +56,10 @@ class DecoderDatabase { void DropDecoder() { decoder_.reset(); } int SampleRateHz() const { - RTC_DCHECK_EQ(1, !!decoder_ + !!external_decoder + !!cng_decoder_); + RTC_DCHECK_EQ(1, !!decoder_ + !!external_decoder_ + !!cng_decoder_); return decoder_ ? decoder_->SampleRateHz() - : external_decoder ? external_decoder->sample_rate_hz - : cng_decoder_->sample_rate_hz; + : external_decoder_ ? external_decoder_->SampleRateHz() + : cng_decoder_->sample_rate_hz; } const NetEqDecoder codec_type; @@ -71,13 +70,7 @@ class DecoderDatabase { std::unique_ptr decoder_; // Set iff this is an external decoder. - struct ExternalDecoder { - // TODO(kwiberg): Remove sample_rate_hz once we can trust all decoders to - // implement SampleRateHz(). - int sample_rate_hz; - AudioDecoder* decoder; - }; - const rtc::Optional external_decoder; + AudioDecoder* const external_decoder_; // Set iff this is a comfort noise decoder. struct CngDecoder { @@ -120,7 +113,6 @@ class DecoderDatabase { virtual int InsertExternal(uint8_t rtp_payload_type, NetEqDecoder codec_type, const std::string& codec_name, - int fs_hz, AudioDecoder* decoder); // Removes the entry for |rtp_payload_type| from the database. diff --git a/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc b/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc index 1aade2c7c0..380e719d1d 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc @@ -141,7 +141,7 @@ TEST(DecoderDatabase, ExternalDecoder) { // Load into database. EXPECT_EQ(DecoderDatabase::kOK, db.InsertExternal(kPayloadType, NetEqDecoder::kDecoderPCMu, - kCodecName, 8000, &decoder)); + kCodecName, &decoder)); EXPECT_EQ(1, db.Size()); // Get decoder and make sure we get the external one. EXPECT_EQ(&decoder, db.GetDecoder(kPayloadType)); diff --git a/webrtc/modules/audio_coding/neteq/include/neteq.h b/webrtc/modules/audio_coding/neteq/include/neteq.h index fd1041a1ea..b03cff7765 100644 --- a/webrtc/modules/audio_coding/neteq/include/neteq.h +++ b/webrtc/modules/audio_coding/neteq/include/neteq.h @@ -183,16 +183,14 @@ class NetEq { // Provides an externally created decoder object |decoder| to insert in the // decoder database. The decoder implements a decoder of type |codec| and - // associates it with |rtp_payload_type| and |codec_name|. The decoder will - // produce samples at the rate |sample_rate_hz|. Returns kOK on success, kFail - // on failure. - // The name is only used to provide information back to the caller about the - // decoders. Hence, the name is arbitrary, and may be empty. + // associates it with |rtp_payload_type| and |codec_name|. Returns kOK on + // success, kFail on failure. The name is only used to provide information + // back to the caller about the decoders. Hence, the name is arbitrary, and + // may be empty. virtual int RegisterExternalDecoder(AudioDecoder* decoder, NetEqDecoder codec, const std::string& codec_name, - uint8_t rtp_payload_type, - int sample_rate_hz) = 0; + uint8_t rtp_payload_type) = 0; // Removes |rtp_payload_type| from the codec database. Returns 0 on success, // -1 on failure. 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 60ae0f6501..3865db2cee 100644 --- a/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h +++ b/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h @@ -33,10 +33,11 @@ class MockDecoderDatabase : public DecoderDatabase { MOCK_METHOD3(RegisterPayload, int(uint8_t rtp_payload_type, NetEqDecoder codec_type, const std::string& name)); - MOCK_METHOD5(InsertExternal, - int(uint8_t rtp_payload_type, NetEqDecoder codec_type, - const std::string& codec_name, int fs_hz, - AudioDecoder* decoder)); + MOCK_METHOD4(InsertExternal, + int(uint8_t rtp_payload_type, + NetEqDecoder codec_type, + const std::string& codec_name, + AudioDecoder* decoder)); MOCK_METHOD1(Remove, int(uint8_t rtp_payload_type)); MOCK_CONST_METHOD1(GetDecoderInfo, diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc index 15eb51b72c..422252c5ac 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc @@ -260,8 +260,7 @@ int NetEqImpl::RegisterPayloadType(NetEqDecoder codec, int NetEqImpl::RegisterExternalDecoder(AudioDecoder* decoder, NetEqDecoder codec, const std::string& codec_name, - uint8_t rtp_payload_type, - int sample_rate_hz) { + uint8_t rtp_payload_type) { rtc::CritScope lock(&crit_sect_); LOG(LS_VERBOSE) << "RegisterExternalDecoder " << static_cast(rtp_payload_type) << " " @@ -271,8 +270,8 @@ int NetEqImpl::RegisterExternalDecoder(AudioDecoder* decoder, assert(false); return kFail; } - int ret = decoder_database_->InsertExternal( - rtp_payload_type, codec, codec_name, sample_rate_hz, decoder); + int ret = decoder_database_->InsertExternal(rtp_payload_type, codec, + codec_name, decoder); if (ret != DecoderDatabase::kOK) { switch (ret) { case DecoderDatabase::kInvalidRtpPayloadType: diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.h b/webrtc/modules/audio_coding/neteq/neteq_impl.h index 2b10f97e47..e24dfb9e9b 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.h +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.h @@ -128,8 +128,7 @@ class NetEqImpl : public webrtc::NetEq { int RegisterExternalDecoder(AudioDecoder* decoder, NetEqDecoder codec, const std::string& codec_name, - uint8_t rtp_payload_type, - int sample_rate_hz) override; + uint8_t rtp_payload_type) override; // Removes |rtp_payload_type| from the codec database. Returns 0 on success, // -1 on failure. diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc index 2bf1f604bb..f9b9e7bc60 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -446,7 +446,7 @@ TEST_F(NetEqImplTest, VerifyTimestampPropagation) { EXPECT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( &decoder_, NetEqDecoder::kDecoderPCM16B, - "dummy name", kPayloadType, kSampleRateHz)); + "dummy name", kPayloadType)); // Insert one packet. EXPECT_EQ(NetEq::kOK, @@ -509,6 +509,8 @@ TEST_F(NetEqImplTest, ReorderedPacket) { // Create a mock decoder object. MockAudioDecoder mock_decoder; EXPECT_CALL(mock_decoder, Reset()).WillRepeatedly(Return()); + EXPECT_CALL(mock_decoder, SampleRateHz()) + .WillRepeatedly(Return(kSampleRateHz)); EXPECT_CALL(mock_decoder, Channels()).WillRepeatedly(Return(1)); EXPECT_CALL(mock_decoder, IncomingPacket(_, kPayloadLengthBytes, _, _, _)) .WillRepeatedly(Return(0)); @@ -525,7 +527,7 @@ TEST_F(NetEqImplTest, ReorderedPacket) { Return(kPayloadLengthSamples))); EXPECT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( &mock_decoder, NetEqDecoder::kDecoderPCM16B, - "dummy name", kPayloadType, kSampleRateHz)); + "dummy name", kPayloadType)); // Insert one packet. EXPECT_EQ(NetEq::kOK, @@ -658,6 +660,8 @@ TEST_F(NetEqImplTest, CodecInternalCng) { // Create a mock decoder object. MockAudioDecoder mock_decoder; EXPECT_CALL(mock_decoder, Reset()).WillRepeatedly(Return()); + EXPECT_CALL(mock_decoder, SampleRateHz()) + .WillRepeatedly(Return(kSampleRateKhz * 1000)); EXPECT_CALL(mock_decoder, Channels()).WillRepeatedly(Return(1)); EXPECT_CALL(mock_decoder, IncomingPacket(_, kPayloadLengthBytes, _, _, _)) .WillRepeatedly(Return(0)); @@ -700,7 +704,7 @@ TEST_F(NetEqImplTest, CodecInternalCng) { EXPECT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( &mock_decoder, NetEqDecoder::kDecoderOpus, - "dummy name", kPayloadType, kSampleRateKhz * 1000)); + "dummy name", kPayloadType)); // Insert one packet (decoder will return speech). EXPECT_EQ(NetEq::kOK, @@ -844,7 +848,7 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) { EXPECT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( &decoder_, NetEqDecoder::kDecoderPCM16B, - "dummy name", kPayloadType, kSampleRateHz)); + "dummy name", kPayloadType)); // Insert one packet. payload[0] = kFirstPayloadValue; // This will make Decode() fail. @@ -938,6 +942,8 @@ TEST_F(NetEqImplTest, DecodedPayloadTooShort) { // Create a mock decoder object. MockAudioDecoder mock_decoder; EXPECT_CALL(mock_decoder, Reset()).WillRepeatedly(Return()); + EXPECT_CALL(mock_decoder, SampleRateHz()) + .WillRepeatedly(Return(kSampleRateHz)); EXPECT_CALL(mock_decoder, Channels()).WillRepeatedly(Return(1)); EXPECT_CALL(mock_decoder, IncomingPacket(_, kPayloadLengthBytes, _, _, _)) .WillRepeatedly(Return(0)); @@ -956,7 +962,7 @@ TEST_F(NetEqImplTest, DecodedPayloadTooShort) { Return(kPayloadLengthSamples - 5))); EXPECT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( &mock_decoder, NetEqDecoder::kDecoderPCM16B, - "dummy name", kPayloadType, kSampleRateHz)); + "dummy name", kPayloadType)); // Insert one packet. EXPECT_EQ(NetEq::kOK, @@ -1003,6 +1009,8 @@ TEST_F(NetEqImplTest, DecodingError) { // Create a mock decoder object. MockAudioDecoder mock_decoder; EXPECT_CALL(mock_decoder, Reset()).WillRepeatedly(Return()); + EXPECT_CALL(mock_decoder, SampleRateHz()) + .WillRepeatedly(Return(kSampleRateHz)); EXPECT_CALL(mock_decoder, Channels()).WillRepeatedly(Return(1)); EXPECT_CALL(mock_decoder, IncomingPacket(_, kPayloadLengthBytes, _, _, _)) .WillRepeatedly(Return(0)); @@ -1047,7 +1055,7 @@ TEST_F(NetEqImplTest, DecodingError) { EXPECT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( &mock_decoder, NetEqDecoder::kDecoderPCM16B, - "dummy name", kPayloadType, kSampleRateHz)); + "dummy name", kPayloadType)); // Insert packets. for (int i = 0; i < 6; ++i) { @@ -1117,6 +1125,8 @@ TEST_F(NetEqImplTest, DecodingErrorDuringInternalCng) { // Create a mock decoder object. MockAudioDecoder mock_decoder; EXPECT_CALL(mock_decoder, Reset()).WillRepeatedly(Return()); + EXPECT_CALL(mock_decoder, SampleRateHz()) + .WillRepeatedly(Return(kSampleRateHz)); EXPECT_CALL(mock_decoder, Channels()).WillRepeatedly(Return(1)); EXPECT_CALL(mock_decoder, IncomingPacket(_, kPayloadLengthBytes, _, _, _)) .WillRepeatedly(Return(0)); @@ -1157,7 +1167,7 @@ TEST_F(NetEqImplTest, DecodingErrorDuringInternalCng) { EXPECT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( &mock_decoder, NetEqDecoder::kDecoderPCM16B, - "dummy name", kPayloadType, kSampleRateHz)); + "dummy name", kPayloadType)); // Insert 2 packets. This will make netEq into codec internal CNG mode. for (int i = 0; i < 2; ++i) { @@ -1293,7 +1303,7 @@ class NetEqImplTest120ms : public NetEqImplTest { ASSERT_EQ(2u, decoder_->Channels()); EXPECT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( decoder_.get(), NetEqDecoder::kDecoderOpus_2ch, - "120ms codec", kPayloadType, kSamplingFreq_)); + "120ms codec", kPayloadType)); } std::unique_ptr decoder_; diff --git a/webrtc/modules/audio_coding/neteq/tools/neteq_external_decoder_test.cc b/webrtc/modules/audio_coding/neteq/tools/neteq_external_decoder_test.cc index cbbfd27d5c..f1ec7b1c91 100644 --- a/webrtc/modules/audio_coding/neteq/tools/neteq_external_decoder_test.cc +++ b/webrtc/modules/audio_coding/neteq/tools/neteq_external_decoder_test.cc @@ -34,7 +34,7 @@ NetEqExternalDecoderTest::NetEqExternalDecoderTest(NetEqDecoder codec, void NetEqExternalDecoderTest::Init() { ASSERT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder(decoder_, codec_, name_, - kPayloadType, sample_rate_hz_)); + kPayloadType)); } void NetEqExternalDecoderTest::InsertPacket(