From d8399e630f3f4886d455e2c4d2307794b60261c0 Mon Sep 17 00:00:00 2001 From: Karl Wiberg Date: Mon, 25 May 2015 14:39:56 +0200 Subject: [PATCH] Also provide sample rate when registering decoders This replaces the old practice of looking up the sample rate in a table, which won't work when we add support for external decoders. COAUTHOR=henrik.lundin@webrtc.org BUG=4474 R=jmarusic@webrtc.org, minyue@webrtc.org Review URL: https://webrtc-codereview.appspot.com/54469004 Cr-Commit-Position: refs/heads/master@{#9276} --- .../audio_coding/main/acm2/acm_receiver.cc | 11 ++++-- .../audio_coding/main/acm2/acm_receiver.h | 3 ++ .../main/acm2/acm_receiver_unittest.cc | 34 +++++++++++-------- .../main/acm2/acm_receiver_unittest_oldapi.cc | 34 +++++++++++-------- .../main/acm2/audio_coding_module_impl.cc | 4 ++- .../audio_coding/neteq/interface/neteq.h | 7 ++-- .../modules/audio_coding/neteq/neteq_impl.cc | 4 +-- .../modules/audio_coding/neteq/neteq_impl.h | 7 ++-- .../audio_coding/neteq/neteq_impl_unittest.cc | 18 +++++----- .../tools/neteq_external_decoder_test.cc | 4 +-- 10 files changed, 75 insertions(+), 51 deletions(-) diff --git a/webrtc/modules/audio_coding/main/acm2/acm_receiver.cc b/webrtc/modules/audio_coding/main/acm2/acm_receiver.cc index b6333ec1b0..2b562d2f53 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_receiver.cc +++ b/webrtc/modules/audio_coding/main/acm2/acm_receiver.cc @@ -475,6 +475,7 @@ int AcmReceiver::GetAudio(int desired_freq_hz, AudioFrame* audio_frame) { int32_t AcmReceiver::AddCodec(int acm_codec_id, uint8_t payload_type, int channels, + int sample_rate_hz, AudioDecoder* audio_decoder) { assert(acm_codec_id >= 0); NetEqDecoder neteq_decoder = ACMCodecDB::neteq_decoders_[acm_codec_id]; @@ -491,7 +492,8 @@ int32_t AcmReceiver::AddCodec(int acm_codec_id, auto it = decoders_.find(payload_type); if (it != decoders_.end()) { const Decoder& decoder = it->second; - if (decoder.acm_codec_id == acm_codec_id && decoder.channels == channels) { + if (decoder.acm_codec_id == acm_codec_id && decoder.channels == channels && + decoder.sample_rate_hz == sample_rate_hz) { // Re-registering the same codec. Do nothing and return. return 0; } @@ -511,8 +513,8 @@ int32_t AcmReceiver::AddCodec(int acm_codec_id, if (!audio_decoder) { ret_val = neteq_->RegisterPayloadType(neteq_decoder, payload_type); } else { - ret_val = neteq_->RegisterExternalDecoder( - audio_decoder, neteq_decoder, payload_type); + ret_val = neteq_->RegisterExternalDecoder(audio_decoder, neteq_decoder, + payload_type, sample_rate_hz); } if (ret_val != NetEq::kOK) { LOG_FERR3(LS_ERROR, "AcmReceiver::AddCodec", acm_codec_id, @@ -524,6 +526,7 @@ int32_t AcmReceiver::AddCodec(int acm_codec_id, decoder.acm_codec_id = acm_codec_id; decoder.payload_type = payload_type; decoder.channels = channels; + decoder.sample_rate_hz = sample_rate_hz; decoders_[payload_type] = decoder; return 0; } @@ -625,6 +628,7 @@ int AcmReceiver::LastAudioCodec(CodecInst* codec) const { sizeof(CodecInst)); codec->pltype = last_audio_decoder_->payload_type; codec->channels = last_audio_decoder_->channels; + codec->plfreq = last_audio_decoder_->sample_rate_hz; return 0; } @@ -686,6 +690,7 @@ int AcmReceiver::DecoderByPayloadType(uint8_t payload_type, sizeof(CodecInst)); codec->pltype = decoder.payload_type; codec->channels = decoder.channels; + codec->plfreq = decoder.sample_rate_hz; return 0; } diff --git a/webrtc/modules/audio_coding/main/acm2/acm_receiver.h b/webrtc/modules/audio_coding/main/acm2/acm_receiver.h index bdc4b844e2..46207fd449 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_receiver.h +++ b/webrtc/modules/audio_coding/main/acm2/acm_receiver.h @@ -45,6 +45,7 @@ class AcmReceiver { // This field is meaningful for codecs where both mono and // stereo versions are registered under the same ID. int channels; + int sample_rate_hz; }; // Constructor of the class @@ -94,6 +95,7 @@ class AcmReceiver { // Input: // - acm_codec_id : ACM codec ID. // - payload_type : payload type. + // - sample_rate_hz : sample rate. // - audio_decoder : pointer to a decoder object. If it is NULL // then NetEq will internally create the decoder // object. Otherwise, NetEq will store this pointer @@ -113,6 +115,7 @@ class AcmReceiver { int AddCodec(int acm_codec_id, uint8_t payload_type, int channels, + int sample_rate_hz, AudioDecoder* audio_decoder); // diff --git a/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest.cc b/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest.cc index 5ec39c64c9..6234d4f501 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest.cc +++ b/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest.cc @@ -109,7 +109,8 @@ class AcmReceiverTest : public AudioPacketizationCallback, int n = 0; while (id[n] >= 0) { ASSERT_EQ(0, receiver_->AddCodec(id[n], codecs_[id[n]].pltype, - codecs_[id[n]].channels, NULL)); + codecs_[id[n]].channels, + codecs_[id[n]].plfreq, NULL)); ++n; } } @@ -157,8 +158,9 @@ TEST_F(AcmReceiverTest, DISABLED_ON_ANDROID(AddCodecGetCodec)) { // Add codec. for (int n = 0; n < ACMCodecDB::kNumCodecs; ++n) { if (n & 0x1) // Just add codecs with odd index. - EXPECT_EQ(0, receiver_->AddCodec(n, codecs_[n].pltype, - codecs_[n].channels, NULL)); + EXPECT_EQ(0, + receiver_->AddCodec(n, codecs_[n].pltype, codecs_[n].channels, + codecs_[n].plfreq, NULL)); } // Get codec and compare. for (int n = 0; n < ACMCodecDB::kNumCodecs; ++n) { @@ -185,10 +187,12 @@ TEST_F(AcmReceiverTest, DISABLED_ON_ANDROID(AddCodecChangePayloadType)) { CodecInst test_codec; // Register the same codec with different payload types. - EXPECT_EQ(0, receiver_->AddCodec(codec_id, ref_codec1.pltype, - ref_codec1.channels, NULL)); - EXPECT_EQ(0, receiver_->AddCodec(codec_id, ref_codec2.pltype, - ref_codec2.channels, NULL)); + EXPECT_EQ( + 0, receiver_->AddCodec(codec_id, ref_codec1.pltype, ref_codec1.channels, + ref_codec1.plfreq, NULL)); + EXPECT_EQ( + 0, receiver_->AddCodec(codec_id, ref_codec2.pltype, ref_codec2.channels, + ref_codec2.plfreq, NULL)); // Both payload types should exist. EXPECT_EQ(0, receiver_->DecoderByPayloadType(ref_codec1.pltype, &test_codec)); @@ -208,10 +212,12 @@ TEST_F(AcmReceiverTest, DISABLED_ON_ANDROID(AddCodecChangeCodecId)) { CodecInst test_codec; // Register the same payload type with different codec ID. - EXPECT_EQ(0, receiver_->AddCodec(codec_id1, ref_codec1.pltype, - ref_codec1.channels, NULL)); - EXPECT_EQ(0, receiver_->AddCodec(codec_id2, ref_codec2.pltype, - ref_codec2.channels, NULL)); + EXPECT_EQ( + 0, receiver_->AddCodec(codec_id1, ref_codec1.pltype, ref_codec1.channels, + ref_codec1.plfreq, NULL)); + EXPECT_EQ( + 0, receiver_->AddCodec(codec_id2, ref_codec2.pltype, ref_codec2.channels, + ref_codec2.plfreq, NULL)); // Make sure that the last codec is used. EXPECT_EQ(0, receiver_->DecoderByPayloadType(ref_codec2.pltype, &test_codec)); @@ -223,8 +229,8 @@ TEST_F(AcmReceiverTest, DISABLED_ON_ANDROID(AddCodecRemoveCodec)) { const int codec_id = ACMCodecDB::kPCMA; EXPECT_EQ(0, ACMCodecDB::Codec(codec_id, &codec)); const int payload_type = codec.pltype; - EXPECT_EQ(0, receiver_->AddCodec(codec_id, codec.pltype, - codec.channels, NULL)); + EXPECT_EQ(0, receiver_->AddCodec(codec_id, codec.pltype, codec.channels, + codec.plfreq, NULL)); // Remove non-existing codec should not fail. ACM1 legacy. EXPECT_EQ(0, receiver_->RemoveCodec(payload_type + 1)); @@ -280,7 +286,7 @@ TEST_F(AcmReceiverTest, DISABLED_ON_ANDROID(PostdecodingVad)) { const int id = ACMCodecDB::kPCM16Bwb; ASSERT_EQ(0, receiver_->AddCodec(id, codecs_[id].pltype, codecs_[id].channels, - NULL)); + codecs_[id].plfreq, NULL)); const int kNumPackets = 5; const int num_10ms_frames = codecs_[id].pacsize / (codecs_[id].plfreq / 100); AudioFrame frame; diff --git a/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest_oldapi.cc b/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest_oldapi.cc index 269d19c81d..5800fb7367 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest_oldapi.cc +++ b/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest_oldapi.cc @@ -108,7 +108,8 @@ class AcmReceiverTestOldApi : public AudioPacketizationCallback, int n = 0; while (id[n] >= 0) { ASSERT_EQ(0, receiver_->AddCodec(id[n], codecs_[id[n]].pltype, - codecs_[id[n]].channels, NULL)); + codecs_[id[n]].channels, + codecs_[id[n]].plfreq, NULL)); ++n; } } @@ -156,8 +157,9 @@ TEST_F(AcmReceiverTestOldApi, DISABLED_ON_ANDROID(AddCodecGetCodec)) { // Add codec. for (int n = 0; n < ACMCodecDB::kNumCodecs; ++n) { if (n & 0x1) // Just add codecs with odd index. - EXPECT_EQ(0, receiver_->AddCodec(n, codecs_[n].pltype, - codecs_[n].channels, NULL)); + EXPECT_EQ(0, + receiver_->AddCodec(n, codecs_[n].pltype, codecs_[n].channels, + codecs_[n].plfreq, NULL)); } // Get codec and compare. for (int n = 0; n < ACMCodecDB::kNumCodecs; ++n) { @@ -184,10 +186,12 @@ TEST_F(AcmReceiverTestOldApi, DISABLED_ON_ANDROID(AddCodecChangePayloadType)) { CodecInst test_codec; // Register the same codec with different payloads. - EXPECT_EQ(0, receiver_->AddCodec(codec_id, ref_codec1.pltype, - ref_codec1.channels, NULL)); - EXPECT_EQ(0, receiver_->AddCodec(codec_id, ref_codec2.pltype, - ref_codec2.channels, NULL)); + EXPECT_EQ( + 0, receiver_->AddCodec(codec_id, ref_codec1.pltype, ref_codec1.channels, + ref_codec1.plfreq, NULL)); + EXPECT_EQ( + 0, receiver_->AddCodec(codec_id, ref_codec2.pltype, ref_codec2.channels, + ref_codec2.plfreq, NULL)); // Both payload types should exist. EXPECT_EQ(0, receiver_->DecoderByPayloadType(ref_codec1.pltype, &test_codec)); @@ -207,10 +211,12 @@ TEST_F(AcmReceiverTestOldApi, DISABLED_ON_ANDROID(AddCodecChangeCodecId)) { CodecInst test_codec; // Register the same payload type with different codec ID. - EXPECT_EQ(0, receiver_->AddCodec(codec_id1, ref_codec1.pltype, - ref_codec1.channels, NULL)); - EXPECT_EQ(0, receiver_->AddCodec(codec_id2, ref_codec2.pltype, - ref_codec2.channels, NULL)); + EXPECT_EQ( + 0, receiver_->AddCodec(codec_id1, ref_codec1.pltype, ref_codec1.channels, + ref_codec1.plfreq, NULL)); + EXPECT_EQ( + 0, receiver_->AddCodec(codec_id2, ref_codec2.pltype, ref_codec2.channels, + ref_codec2.plfreq, NULL)); // Make sure that the last codec is used. EXPECT_EQ(0, receiver_->DecoderByPayloadType(ref_codec2.pltype, &test_codec)); @@ -222,8 +228,8 @@ TEST_F(AcmReceiverTestOldApi, DISABLED_ON_ANDROID(AddCodecRemoveCodec)) { const int codec_id = ACMCodecDB::kPCMA; EXPECT_EQ(0, ACMCodecDB::Codec(codec_id, &codec)); const int payload_type = codec.pltype; - EXPECT_EQ(0, receiver_->AddCodec(codec_id, codec.pltype, - codec.channels, NULL)); + EXPECT_EQ(0, receiver_->AddCodec(codec_id, codec.pltype, codec.channels, + codec.plfreq, NULL)); // Remove non-existing codec should not fail. ACM1 legacy. EXPECT_EQ(0, receiver_->RemoveCodec(payload_type + 1)); @@ -279,7 +285,7 @@ TEST_F(AcmReceiverTestOldApi, DISABLED_ON_ANDROID(PostdecodingVad)) { const int id = ACMCodecDB::kPCM16Bwb; ASSERT_EQ(0, receiver_->AddCodec(id, codecs_[id].pltype, codecs_[id].channels, - NULL)); + codecs_[id].plfreq, NULL)); const int kNumPackets = 5; const int num_10ms_frames = codecs_[id].pacsize / (codecs_[id].plfreq / 100); AudioFrame frame; diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc index b6470dcfdd..0da2eb5c06 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc +++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc @@ -575,7 +575,8 @@ int AudioCodingModuleImpl::InitializeReceiverSafe() { for (int i = 0; i < ACMCodecDB::kNumCodecs; i++) { if (IsCodecRED(i) || IsCodecCN(i)) { uint8_t pl_type = static_cast(ACMCodecDB::database_[i].pltype); - if (receiver_.AddCodec(i, pl_type, 1, NULL) < 0) { + int fs = ACMCodecDB::database_[i].plfreq; + if (receiver_.AddCodec(i, pl_type, 1, fs, NULL) < 0) { WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, id_, "Cannot register master codec."); return -1; @@ -644,6 +645,7 @@ int AudioCodingModuleImpl::RegisterReceiveCodec(const CodecInst& codec) { // Get |decoder| associated with |codec|. |decoder| is NULL if |codec| does // not own its decoder. return receiver_.AddCodec(codec_id, codec.pltype, codec.channels, + codec.plfreq, codec_manager_.GetAudioDecoder(codec)); } diff --git a/webrtc/modules/audio_coding/neteq/interface/neteq.h b/webrtc/modules/audio_coding/neteq/interface/neteq.h index 587a1a377c..a641c9e8df 100644 --- a/webrtc/modules/audio_coding/neteq/interface/neteq.h +++ b/webrtc/modules/audio_coding/neteq/interface/neteq.h @@ -170,11 +170,12 @@ 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|. Returns kOK on success, - // kFail on failure. + // associates it with |rtp_payload_type|. The decoder will produce samples + // at the rate |sample_rate_hz|. Returns kOK on success, kFail on failure. virtual int RegisterExternalDecoder(AudioDecoder* decoder, enum NetEqDecoder codec, - uint8_t rtp_payload_type) = 0; + uint8_t rtp_payload_type, + int sample_rate_hz) = 0; // 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.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc index 784174f33a..b299e88f7a 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc @@ -204,7 +204,8 @@ int NetEqImpl::RegisterPayloadType(enum NetEqDecoder codec, int NetEqImpl::RegisterExternalDecoder(AudioDecoder* decoder, enum NetEqDecoder codec, - uint8_t rtp_payload_type) { + uint8_t rtp_payload_type, + int sample_rate_hz) { CriticalSectionScoped lock(crit_sect_.get()); LOG_API2(static_cast(rtp_payload_type), codec); if (!decoder) { @@ -212,7 +213,6 @@ int NetEqImpl::RegisterExternalDecoder(AudioDecoder* decoder, assert(false); return kFail; } - const int sample_rate_hz = CodecSampleRateHz(codec); int ret = decoder_database_->InsertExternal(rtp_payload_type, codec, sample_rate_hz, decoder); if (ret != DecoderDatabase::kOK) { diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.h b/webrtc/modules/audio_coding/neteq/neteq_impl.h index 76b23e30cf..248071f825 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.h +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.h @@ -117,11 +117,12 @@ class NetEqImpl : public webrtc::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|. Returns kOK on success, kFail on - // failure. + // associates it with |rtp_payload_type|. The decoder will produce samples + // at the rate |sample_rate_hz|. Returns kOK on success, kFail on failure. int RegisterExternalDecoder(AudioDecoder* decoder, enum NetEqDecoder codec, - uint8_t rtp_payload_type) override; + uint8_t rtp_payload_type, + int sample_rate_hz) 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 b03d1a290e..05a8de25cb 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -457,8 +457,8 @@ TEST_F(NetEqImplTest, VerifyTimestampPropagation) { } decoder_; EXPECT_EQ(NetEq::kOK, - neteq_->RegisterExternalDecoder( - &decoder_, kDecoderPCM16B, kPayloadType)); + neteq_->RegisterExternalDecoder(&decoder_, kDecoderPCM16B, + kPayloadType, kSampleRateHz)); // Insert one packet. EXPECT_EQ(NetEq::kOK, @@ -535,8 +535,8 @@ TEST_F(NetEqImplTest, ReorderedPacket) { SetArgPointee<5>(AudioDecoder::kSpeech), Return(kPayloadLengthSamples))); EXPECT_EQ(NetEq::kOK, - neteq_->RegisterExternalDecoder( - &mock_decoder, kDecoderPCM16B, kPayloadType)); + neteq_->RegisterExternalDecoder(&mock_decoder, kDecoderPCM16B, + kPayloadType, kSampleRateHz)); // Insert one packet. EXPECT_EQ(NetEq::kOK, @@ -719,9 +719,9 @@ TEST_F(NetEqImplTest, CodecInternalCng) { SetArgPointee<5>(AudioDecoder::kSpeech), Return(kPayloadLengthSamples))); - EXPECT_EQ(NetEq::kOK, - neteq_->RegisterExternalDecoder( - &mock_decoder, kDecoderOpus, kPayloadType)); + EXPECT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( + &mock_decoder, kDecoderOpus, kPayloadType, + kSampleRateKhz * 1000)); // Insert one packet (decoder will return speech). EXPECT_EQ(NetEq::kOK, @@ -860,8 +860,8 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) { .WillRepeatedly(Return(kNetEqMaxFrameSize)); EXPECT_EQ(NetEq::kOK, - neteq_->RegisterExternalDecoder( - &decoder_, kDecoderPCM16B, kPayloadType)); + neteq_->RegisterExternalDecoder(&decoder_, kDecoderPCM16B, + kPayloadType, kSampleRateHz)); // Insert one packet. payload[0] = kFirstPayloadValue; // This will make Decode() fail. 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 3eb4a29805..52c34bb79c 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 @@ -29,8 +29,8 @@ NetEqExternalDecoderTest::NetEqExternalDecoderTest(NetEqDecoder codec, } void NetEqExternalDecoderTest::Init() { - ASSERT_EQ(NetEq::kOK, - neteq_->RegisterExternalDecoder(decoder_, codec_, kPayloadType)); + ASSERT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( + decoder_, codec_, kPayloadType, sample_rate_hz_)); } void NetEqExternalDecoderTest::InsertPacket(WebRtcRTPHeader rtp_header,