From 347d35129ee61a4bce2fd732fdd2852e164257b2 Mon Sep 17 00:00:00 2001 From: kwiberg Date: Thu, 16 Jun 2016 01:59:09 -0700 Subject: [PATCH] AudioDecoder: Remove the default implementation of SampleRateHz And implement SampleRateHz in a bunch of mocks. BUG=webrtc:5801 NOTRY=true Review-Url: https://codereview.webrtc.org/2029543002 Cr-Commit-Position: refs/heads/master@{#13161} --- .../modules/audio_coding/codecs/audio_decoder.cc | 4 ---- .../modules/audio_coding/codecs/audio_decoder.h | 7 +------ .../neteq/mock/mock_external_decoder_pcm16b.h | 16 +++++++++++----- .../neteq/neteq_external_decoder_unittest.cc | 4 ++-- .../audio_coding/neteq/neteq_impl_unittest.cc | 13 ++++++++++--- .../neteq/neteq_network_stats_unittest.cc | 16 ++++++++++------ 6 files changed, 34 insertions(+), 26 deletions(-) diff --git a/webrtc/modules/audio_coding/codecs/audio_decoder.cc b/webrtc/modules/audio_coding/codecs/audio_decoder.cc index e91161e33c..442ddc1e4b 100644 --- a/webrtc/modules/audio_coding/codecs/audio_decoder.cc +++ b/webrtc/modules/audio_coding/codecs/audio_decoder.cc @@ -82,10 +82,6 @@ bool AudioDecoder::PacketHasFec(const uint8_t* encoded, return false; } -int AudioDecoder::SampleRateHz() const { - return -1; -} - AudioDecoder::SpeechType AudioDecoder::ConvertSpeechType(int16_t type) { switch (type) { case 0: // TODO(hlundin): Both iSAC and Opus return 0 for speech. diff --git a/webrtc/modules/audio_coding/codecs/audio_decoder.h b/webrtc/modules/audio_coding/codecs/audio_decoder.h index 7b7047f7de..13581bc247 100644 --- a/webrtc/modules/audio_coding/codecs/audio_decoder.h +++ b/webrtc/modules/audio_coding/codecs/audio_decoder.h @@ -95,12 +95,7 @@ class AudioDecoder { // Returns the actual sample rate of the decoder's output. This value may not // change during the lifetime of the decoder. - // NOTE: For now, this has a default implementation that returns an unusable - // value (-1). That default implementation will go away soon, and at the same - // time callers will start relying on the return value, so make sure you - // override it with something that returns a correct value! - // TODO(kwiberg): Remove the default implementation. - virtual int SampleRateHz() const; + virtual int SampleRateHz() const = 0; // The number of channels in the decoder's output. This value may not change // during the lifetime of the decoder. diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_external_decoder_pcm16b.h b/webrtc/modules/audio_coding/neteq/mock/mock_external_decoder_pcm16b.h index 42c17ae054..23a3ec4c99 100644 --- a/webrtc/modules/audio_coding/neteq/mock/mock_external_decoder_pcm16b.h +++ b/webrtc/modules/audio_coding/neteq/mock/mock_external_decoder_pcm16b.h @@ -23,11 +23,11 @@ namespace webrtc { using ::testing::_; using ::testing::Invoke; -// Implement an external version of the PCM16b decoder. This is a copy from -// audio_decoder_impl.{cc, h}. +// Implement an external version of the PCM16b decoder. class ExternalPcm16B : public AudioDecoder { public: - ExternalPcm16B() {} + explicit ExternalPcm16B(int sample_rate_hz) + : sample_rate_hz_(sample_rate_hz) {} void Reset() override {} int DecodeInternal(const uint8_t* encoded, @@ -35,21 +35,24 @@ class ExternalPcm16B : public AudioDecoder { int sample_rate_hz, int16_t* decoded, SpeechType* speech_type) override { + EXPECT_EQ(sample_rate_hz_, sample_rate_hz); size_t ret = WebRtcPcm16b_Decode(encoded, encoded_len, decoded); *speech_type = ConvertSpeechType(1); return static_cast(ret); } + int SampleRateHz() const override { return sample_rate_hz_; } size_t Channels() const override { return 1; } private: + const int sample_rate_hz_; RTC_DISALLOW_COPY_AND_ASSIGN(ExternalPcm16B); }; // Create a mock of ExternalPcm16B which delegates all calls to the real object. // The reason is that we can then track that the correct calls are being made. -class MockExternalPcm16B : public ExternalPcm16B { +class MockExternalPcm16B : public AudioDecoder { public: - MockExternalPcm16B() { + explicit MockExternalPcm16B(int sample_rate_hz) : real_(sample_rate_hz) { // By default, all calls are delegated to the real object. ON_CALL(*this, DecodeInternal(_, _, _, _, _)) .WillByDefault(Invoke(&real_, &ExternalPcm16B::DecodeInternal)); @@ -85,6 +88,9 @@ class MockExternalPcm16B : public ExternalPcm16B { MOCK_METHOD0(ErrorCode, int()); + int SampleRateHz() const /* override */ { return real_.SampleRateHz(); } + size_t Channels() const /* override */ { return real_.Channels(); } + private: ExternalPcm16B real_; }; 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 f4f65aed2d..a039013712 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc @@ -175,7 +175,7 @@ class NetEqExternalVsInternalDecoderTest : public NetEqExternalDecoderUnitTest, NetEqExternalVsInternalDecoderTest() : NetEqExternalDecoderUnitTest(NetEqDecoder::kDecoderPCM16Bswb32kHz, 32000, - new MockExternalPcm16B), + new MockExternalPcm16B(32000)), sample_rate_hz_(32000) { NetEq::Config config; config.sample_rate_hz = sample_rate_hz_; @@ -248,7 +248,7 @@ class LargeTimestampJumpTest : public NetEqExternalDecoderUnitTest, LargeTimestampJumpTest() : NetEqExternalDecoderUnitTest(NetEqDecoder::kDecoderPCM16B, 8000, - new MockExternalPcm16B), + new MockExternalPcm16B(8000)), test_state_(kInitialPhase) { EXPECT_CALL(*external_decoder(), HasDecodePlc()) .WillRepeatedly(Return(false)); diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc index 34e36dc60c..2bf1f604bb 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -434,6 +434,8 @@ TEST_F(NetEqImplTest, VerifyTimestampPropagation) { void Reset() override { next_value_ = 1; } + int SampleRateHz() const override { return kSampleRateHz; } + size_t Channels() const override { return 1; } uint16_t next_value() const { return next_value_; } @@ -808,6 +810,7 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) { MOCK_CONST_METHOD2(PacketDuration, int(const uint8_t*, size_t)); MOCK_METHOD5(DecodeInternal, int(const uint8_t*, size_t, int, int16_t*, SpeechType*)); + int SampleRateHz() const /* override */ { return kSampleRateHz; } size_t Channels() const /* override */ { return kChannels; } } decoder_; @@ -1213,8 +1216,9 @@ TEST_F(NetEqImplTest, TickTimerIncrement) { class Decoder120ms : public AudioDecoder { public: - Decoder120ms(SpeechType speech_type) - : next_value_(1), + Decoder120ms(int sample_rate_hz, SpeechType speech_type) + : sample_rate_hz_(sample_rate_hz), + next_value_(1), speech_type_(speech_type) {} int DecodeInternal(const uint8_t* encoded, @@ -1222,6 +1226,7 @@ class Decoder120ms : public AudioDecoder { int sample_rate_hz, int16_t* decoded, SpeechType* speech_type) override { + EXPECT_EQ(sample_rate_hz_, sample_rate_hz); size_t decoded_len = rtc::CheckedDivExact(sample_rate_hz, 1000) * 120 * Channels(); for (size_t i = 0; i < decoded_len; ++i) { @@ -1232,9 +1237,11 @@ class Decoder120ms : public AudioDecoder { } void Reset() override { next_value_ = 1; } + int SampleRateHz() const override { return sample_rate_hz_; } size_t Channels() const override { return 2; } private: + int sample_rate_hz_; int16_t next_value_; SpeechType speech_type_; }; @@ -1282,7 +1289,7 @@ class NetEqImplTest120ms : public NetEqImplTest { } void Register120msCodec(AudioDecoder::SpeechType speech_type) { - decoder_.reset(new Decoder120ms(speech_type)); + decoder_.reset(new Decoder120ms(kSamplingFreq_, speech_type)); ASSERT_EQ(2u, decoder_->Channels()); EXPECT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( decoder_.get(), NetEqDecoder::kDecoderOpus_2ch, diff --git a/webrtc/modules/audio_coding/neteq/neteq_network_stats_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_network_stats_unittest.cc index cf413a4ca8..ca7c459676 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_network_stats_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_network_stats_unittest.cc @@ -30,9 +30,10 @@ class MockAudioDecoder final : public AudioDecoder { // http://crbug.com/428099. static const int kPacketDuration = 960; // 48 kHz * 20 ms - explicit MockAudioDecoder(size_t num_channels) - : num_channels_(num_channels), fec_enabled_(false) { - } + MockAudioDecoder(int sample_rate_hz, size_t num_channels) + : sample_rate_hz_(sample_rate_hz), + num_channels_(num_channels), + fec_enabled_(false) {} ~MockAudioDecoder() /* override */ { Die(); } MOCK_METHOD0(Die, void()); @@ -53,6 +54,8 @@ class MockAudioDecoder final : public AudioDecoder { return fec_enabled_; } + int SampleRateHz() const /* override */ { return sample_rate_hz_; } + size_t Channels() const /* override */ { return num_channels_; } void set_fec_enabled(bool enable_fec) { fec_enabled_ = enable_fec; } @@ -81,6 +84,7 @@ class MockAudioDecoder final : public AudioDecoder { } private: + const int sample_rate_hz_; const size_t num_channels_; bool fec_enabled_; }; @@ -278,21 +282,21 @@ NetEqNetworkStatsTest(NetEqDecoder codec, }; TEST(NetEqNetworkStatsTest, DecodeFec) { - MockAudioDecoder decoder(1); + MockAudioDecoder decoder(48000, 1); NetEqNetworkStatsTest test(NetEqDecoder::kDecoderOpus, 48000, &decoder); test.DecodeFecTest(); EXPECT_CALL(decoder, Die()).Times(1); } TEST(NetEqNetworkStatsTest, StereoDecodeFec) { - MockAudioDecoder decoder(2); + MockAudioDecoder decoder(48000, 2); NetEqNetworkStatsTest test(NetEqDecoder::kDecoderOpus, 48000, &decoder); test.DecodeFecTest(); EXPECT_CALL(decoder, Die()).Times(1); } TEST(NetEqNetworkStatsTest, NoiseExpansionTest) { - MockAudioDecoder decoder(1); + MockAudioDecoder decoder(48000, 1); NetEqNetworkStatsTest test(NetEqDecoder::kDecoderOpus, 48000, &decoder); test.NoiseExpansionTest(); EXPECT_CALL(decoder, Die()).Times(1);