Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1062)

Unified Diff: media/filters/audio_decoder_unittest.cc

Issue 311373004: Consolidate and improve audio decoding test for all decoders. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Megapatch! Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | media/filters/audio_file_reader.h » ('j') | media/filters/audio_file_reader.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/filters/audio_decoder_unittest.cc
diff --git a/media/filters/audio_decoder_unittest.cc b/media/filters/audio_decoder_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b903cb1eb39bf2027ffd99348a3aa9bbe4d234ae
--- /dev/null
+++ b/media/filters/audio_decoder_unittest.cc
@@ -0,0 +1,409 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <deque>
+
+#include "base/bind.h"
+#include "base/md5.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "build/build_config.h"
+#include "media/base/audio_buffer.h"
+#include "media/base/audio_bus.h"
+#include "media/base/audio_hash.h"
+#include "media/base/decoder_buffer.h"
+#include "media/base/test_data_util.h"
+#include "media/base/test_helpers.h"
+#include "media/ffmpeg/ffmpeg_common.h"
+#include "media/filters/audio_file_reader.h"
+#include "media/filters/ffmpeg_audio_decoder.h"
+#include "media/filters/in_memory_url_protocol.h"
+#include "media/filters/opus_audio_decoder.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
wolenetz 2014/06/06 21:00:03 nit: add using testing::ValuesIn; ?
DaleCurtis 2014/06/06 21:16:59 No, I dislike those.
+namespace media {
+
+// The number of packets to read and then decode from each file.
+static const int kDecodeRuns = 3;
+
+enum AudioDecoderType {
+ FFMPEG_DECODER,
+ OPUS_DECODER,
+};
+
+struct AudioSample {
wolenetz 2014/06/06 21:00:03 nit: "Sample" seems strange term to me for what ap
DaleCurtis 2014/06/12 22:00:47 Done.
+ const int64 timestamp;
+ const int64 duration;
+ const char* hash;
+};
+
+struct AudioDecoderTestData {
+ const AudioDecoderType decoder_type;
+ const char* filename;
+ const AudioSample* samples;
+};
+
+// Tells gtest how to print our AudioDecoderTestData structure.
+std::ostream& operator<<(std::ostream& os, const AudioDecoderTestData& data) {
+ return os << data.filename;
wolenetz 2014/06/06 21:00:04 nit: include decoder type or any summary of the sa
DaleCurtis 2014/06/06 21:17:00 No, not relevant for figuring out what's wrong. Th
+}
+
+class AudioDecoderTest : public testing::TestWithParam<AudioDecoderTestData> {
+ public:
+ AudioDecoderTest() : pending_decode_(false), pending_reset_(false) {
+ switch (GetParam().decoder_type) {
+ case FFMPEG_DECODER:
+ decoder_.reset(new FFmpegAudioDecoder(
+ message_loop_.message_loop_proxy(), LogCB()));
+ break;
+ case OPUS_DECODER:
+ decoder_.reset(
+ new OpusAudioDecoder(message_loop_.message_loop_proxy()));
wolenetz 2014/06/06 21:00:04 nit: wrap previous line like Decoder(\n ?
DaleCurtis 2014/06/06 21:17:00 Are you arguing with our deity, clang-format?! :)
wolenetz 2014/06/06 21:52:31 I figured as much. No worries.
+ break;
+ }
+ }
+
+ virtual ~AudioDecoderTest() {
+ EXPECT_FALSE(pending_decode_);
+ EXPECT_FALSE(pending_reset_);
+ }
+
+ protected:
+ void SatisfyPendingDecode() { base::RunLoop().RunUntilIdle(); }
+
+ void SendEndOfStream() {
+ pending_decode_ = true;
wolenetz 2014/06/06 21:00:04 First, ASSERT/EXPECT_FALSE(pending_decode_); and A
DaleCurtis 2014/06/12 22:00:47 Done.
+ decoder_->Decode(
+ DecoderBuffer::CreateEOSBuffer(),
+ base::Bind(&AudioDecoderTest::DecodeFinished, base::Unretained(this)));
+ base::RunLoop().RunUntilIdle();
+ }
+
+ void Initialize() {
+ // Load the test data file.
+ data_ = ReadTestDataFile(GetParam().filename);
+ protocol_.reset(
+ new InMemoryUrlProtocol(data_->data(), data_->data_size(), false));
+ reader_.reset(new AudioFileReader(protocol_.get()));
+ reader_->Open();
wolenetz 2014/06/06 21:00:04 nit: We only decode/verify the first enumerated au
DaleCurtis 2014/06/06 21:17:00 No, we don't support multiple streams of the same
+
+ // Load the first packet and save its timestamp.
+ AVPacket packet;
+ ASSERT_TRUE(reader_->ReadPacketForTesting(&packet));
+ start_timestamp_ = ConvertFromTimeBase(
wolenetz 2014/06/06 21:00:04 nit: Verify expected start timestamp?
DaleCurtis 2014/06/06 21:16:59 Maybe. I'll see about adding it.
DaleCurtis 2014/06/12 22:00:48 Done.
+ reader_->codec_context_for_testing()->time_base, packet.pts);
+ av_free_packet(&packet);
+
+ // Seek back to the unconverted timestamp.
+ ASSERT_TRUE(
+ reader_->SeekForTesting(base::TimeDelta::FromMicroseconds(packet.pts)));
+
+ AudioDecoderConfig config;
+ AVCodecContextToAudioDecoderConfig(
wolenetz 2014/06/06 21:00:04 nit: Verify expected config?
DaleCurtis 2014/06/06 21:17:00 Maybe. I could verify codec, channels, and sample
DaleCurtis 2014/06/12 22:00:47 Done.
+ reader_->codec_context_for_testing(), false, &config, false);
wolenetz 2014/06/06 21:00:05 Can the config change during any test? If not, ver
DaleCurtis 2014/06/06 21:17:00 This is already checked within the decoder. Only
+ InitializeDecoder(config);
+ }
+
+ void InitializeDecoder(const AudioDecoderConfig& config) {
+ decoder_->Initialize(config, NewExpectedStatusCB(PIPELINE_OK));
+ base::RunLoop().RunUntilIdle();
+ }
+
+ void Decode() {
+ pending_decode_ = true;
wolenetz 2014/06/06 21:00:04 ditto: (expect nothing pending, or add tests if th
DaleCurtis 2014/06/12 22:00:47 Done.
+
+ AVPacket packet;
+ ASSERT_TRUE(reader_->ReadPacketForTesting(&packet));
+
+ scoped_refptr<DecoderBuffer> buffer =
+ DecoderBuffer::CopyFrom(packet.data, packet.size);
+ buffer->set_timestamp(ConvertFromTimeBase(
+ reader_->codec_context_for_testing()->time_base, packet.pts));
wolenetz 2014/06/06 21:00:03 Do decoders care at all about decode timestamp? Do
DaleCurtis 2014/06/06 21:17:00 No. DecodeTimestamp is a StreamParserBuffer only c
+ buffer->set_duration(ConvertFromTimeBase(
+ reader_->codec_context_for_testing()->time_base, packet.duration));
+ decoder_->Decode(
+ buffer,
+ base::Bind(&AudioDecoderTest::DecodeFinished, base::Unretained(this)));
+ base::RunLoop().RunUntilIdle();
+ av_free_packet(&packet);
wolenetz 2014/06/06 21:00:03 nit: Why free after RunUntilIdle() and not before?
DaleCurtis 2014/06/06 21:17:00 To ensure all operations which might be using the
+ }
+
+ void Reset() {
+ pending_reset_ = true;
wolenetz 2014/06/06 21:00:04 ditto: (expect nothing pending, or add tests if th
DaleCurtis 2014/06/12 22:00:48 Done.
+ decoder_->Reset(
+ base::Bind(&AudioDecoderTest::ResetFinished, base::Unretained(this)));
+ base::RunLoop().RunUntilIdle();
+ }
+
+ void Stop() {
+ decoder_->Stop();
wolenetz 2014/06/06 21:00:05 nit: does setting some flag like |stopped_| true m
DaleCurtis 2014/06/12 22:00:48 Seems unnecessary, that'll blow up in all sorts of
+ base::RunLoop().RunUntilIdle();
+ }
+
+ void Seek(base::TimeDelta seek_time) {
+ Reset();
+ decoded_audio_.clear();
+ const base::TimeDelta converted_time =
wolenetz 2014/06/06 21:00:04 ditto of audio_file_reader.cc comment: this looks
DaleCurtis 2014/06/06 21:16:59 Reading the docs, it looks like this is wrong. Th
wolenetz 2014/06/06 21:52:31 I'm not sure about the automatic conversion, unles
DaleCurtis 2014/06/12 22:00:47 Ah yeah, you're correct. I need to convert to the
+ base::TimeDelta::FromMicroseconds(ConvertToTimeBase(
+ reader_->codec_context_for_testing()->time_base, seek_time));
+ ASSERT_TRUE(reader_->SeekForTesting(converted_time));
+ }
+
+ void DecodeFinished(AudioDecoder::Status status,
+ const scoped_refptr<AudioBuffer>& buffer) {
+ EXPECT_TRUE(pending_decode_);
+ pending_decode_ = false;
+
+ if (status == AudioDecoder::kNotEnoughData) {
wolenetz 2014/06/06 21:00:04 Verify versus expectation of not enough data?
DaleCurtis 2014/06/06 21:16:59 I don't understand what you're saying.
wolenetz 2014/06/06 21:52:31 Sorry. Can we deterministically expect and verify
DaleCurtis 2014/06/12 22:00:47 Not easily and I don't think it adds any value tha
+ EXPECT_TRUE(buffer.get() == NULL);
+ Decode();
+ return;
+ }
+
+ decoded_audio_.push_back(buffer);
+
+ // If we hit a NULL buffer or have a pending reset, we expect an abort.
+ if (buffer.get() == NULL || pending_reset_) {
+ EXPECT_TRUE(buffer.get() == NULL);
+ EXPECT_EQ(status, AudioDecoder::kAborted);
wolenetz 2014/06/06 21:00:04 nit: here and multiple other places in this file:
DaleCurtis 2014/06/12 22:00:47 Done.
+ return;
+ }
+
+ EXPECT_EQ(status, AudioDecoder::kOk);
+ }
+
+ void ResetFinished() {
+ EXPECT_TRUE(pending_reset_);
+ // Reset should always finish after Decode.
+ EXPECT_FALSE(pending_decode_);
+
+ pending_reset_ = false;
+ }
+
+ // Generates an exact hash of the audio signal. Should not be used for checks
+ // across platforms as audio varies slightly across platforms.
wolenetz 2014/06/06 21:00:04 Is cross-platform audio variance a bug?
DaleCurtis 2014/06/06 21:17:00 No, it's expected due to differing implementations
wolenetz 2014/06/06 21:52:31 Much learning for me today. Many wow :)
+ std::string GetDecodedAudioMD5(size_t i) {
+ CHECK_LT(i, decoded_audio_.size());
+ const scoped_refptr<AudioBuffer>& buffer = decoded_audio_[i];
+
+ scoped_ptr<AudioBus> output =
+ AudioBus::Create(buffer->channel_count(), buffer->frame_count());
+ buffer->ReadFrames(buffer->frame_count(), 0, 0, output.get());
+
+ // Generate an exact MD5 hash for comparison of multiple runs on the same
wolenetz 2014/06/06 21:00:05 nit: truncated comment
DaleCurtis 2014/06/12 22:00:47 Removed. Function comment says more.
+ base::MD5Context context;
+ base::MD5Init(&context);
+ for (int ch = 0; ch < output->channels(); ++ch) {
+ base::MD5Update(
+ &context,
+ base::StringPiece(reinterpret_cast<char*>(output->channel(ch)),
+ output->frames() * sizeof(*output->channel(ch))));
+ }
+ base::MD5Digest digest;
+ base::MD5Final(&digest, &context);
+ return base::MD5DigestToBase16(digest);
+ }
+
+ void ExpectDecodedAudio(size_t i, const std::string& exact_hash) {
+ EXPECT_LT(i, decoded_audio_.size());
wolenetz 2014/06/06 21:00:03 nit: s/EXPECT/ASSERT or CHECK/
DaleCurtis 2014/06/12 22:00:47 Done.
+ const scoped_refptr<AudioBuffer>& buffer = decoded_audio_[i];
+
+ const AudioSample& sample_info = GetParam().samples[i];
+ EXPECT_EQ(sample_info.timestamp, buffer->timestamp().InMicroseconds());
+ EXPECT_EQ(sample_info.duration, buffer->duration().InMicroseconds());
+ EXPECT_FALSE(buffer->end_of_stream());
+
+ scoped_ptr<AudioBus> output =
+ AudioBus::Create(buffer->channel_count(), buffer->frame_count());
+ buffer->ReadFrames(buffer->frame_count(), 0, 0, output.get());
+
+ // Generate a lossy hash of the audio used for comparison across platforms.
+ AudioHash audio_hash;
+ audio_hash.Update(output.get(), output->frames());
wolenetz 2014/06/06 21:00:05 nit: I'm not sure the claim of "Value chosen by di
DaleCurtis 2014/06/06 21:17:00 Again, I don't understand what you're asking. The
wolenetz 2014/06/06 21:52:31 I was commenting that the apparent 'randomness' of
DaleCurtis 2014/06/12 22:00:48 Still don't understand :) That value is used for t
wolenetz 2014/06/16 23:36:08 We're missing each other :) I think AudioHash's cl
+ EXPECT_EQ(sample_info.hash, audio_hash.ToString());
+
+ if (!exact_hash.empty())
+ EXPECT_EQ(exact_hash, GetDecodedAudioMD5(i));
wolenetz 2014/06/06 21:00:04 nit: have a test show that this portion of the met
DaleCurtis 2014/06/12 22:00:47 Done.
+ }
+
+ void ExpectEndOfStream(size_t i) {
+ EXPECT_LT(i, decoded_audio_.size());
wolenetz 2014/06/06 21:00:03 nit: s/EXPECT/ASSERT or CHECK/
DaleCurtis 2014/06/12 22:00:47 Done.
+ EXPECT_TRUE(decoded_audio_[i]->end_of_stream());
+ }
+
+ size_t decoded_audio_size() const { return decoded_audio_.size(); }
+ base::TimeDelta start_timestamp() const { return start_timestamp_; }
+
+ private:
+ base::MessageLoop message_loop_;
+ scoped_refptr<DecoderBuffer> data_;
+ scoped_ptr<InMemoryUrlProtocol> protocol_;
+ scoped_ptr<AudioFileReader> reader_;
+
+ scoped_ptr<AudioDecoder> decoder_;
+ bool pending_decode_;
+ bool pending_reset_;
+
+ std::deque<scoped_refptr<AudioBuffer> > decoded_audio_;
+ base::TimeDelta start_timestamp_;
+
+ DISALLOW_COPY_AND_ASSIGN(AudioDecoderTest);
+};
+
+TEST_P(AudioDecoderTest, Initialize) {
+ Initialize();
+ Stop();
+}
+
+TEST_P(AudioDecoderTest, InitializeWithNoCodecDelay) {
+ if (GetParam().decoder_type != OPUS_DECODER)
wolenetz 2014/06/06 21:00:03 nit: Can this test can be done explicitly (and sim
DaleCurtis 2014/06/06 21:17:00 Possibly, but it's dirty since I'll need to make A
wolenetz 2014/06/06 21:52:31 That's icky. Please forget I asked :).
+ return;
+
+ const uint8_t kOpusExtraData[] = {
+ 0x4f, 0x70, 0x75, 0x73, 0x48, 0x65, 0x61, 0x64, 0x01, 0x02,
+ // The next two bytes represent the codec delay.
+ 0x00, 0x00, 0x80, 0xbb, 0x00, 0x00, 0x00, 0x00, 0x00};
+ AudioDecoderConfig decoder_config;
+ decoder_config.Initialize(kCodecOpus,
+ kSampleFormatF32,
+ CHANNEL_LAYOUT_STEREO,
+ 48000,
+ kOpusExtraData,
+ ARRAYSIZE_UNSAFE(kOpusExtraData),
+ false,
+ false,
+ base::TimeDelta::FromMilliseconds(80),
+ 0);
+ InitializeDecoder(decoder_config);
+ Stop();
+}
+
+TEST_P(AudioDecoderTest, ProduceAudioSamples) {
+ Initialize();
+
+ // Run the test multiple times with a seek back to the beginning in between.
+ std::vector<std::string> decoded_audio_md5_hashes;
+ for (int i = 0; i < 2; ++i) {
+ for (int j = 0; j < kDecodeRuns; ++j) {
+ Decode();
+
+ // On the first pass record the exact MD5 hash for each decoded buffer.
+ if (i == 0)
+ decoded_audio_md5_hashes.push_back(GetDecodedAudioMD5(j));
+ }
+
+ ASSERT_EQ(static_cast<size_t>(kDecodeRuns), decoded_audio_size());
+
+ // On the first pass verify the basic audio hash and sample info. On the
+ // second, verify the exact MD5 sum for each packet. It shouldn't change.
+ for (int j = 0; j < kDecodeRuns; ++j)
+ ExpectDecodedAudio(j, i == 0 ? "" : decoded_audio_md5_hashes[j]);
+
+ // Call one more time to trigger EOS.
+ SendEndOfStream();
+
+ ASSERT_EQ(static_cast<size_t>(kDecodeRuns + 1), decoded_audio_size());
+ ExpectEndOfStream(kDecodeRuns);
+
+ // Seek back to the beginning.
+ Seek(start_timestamp());
+ }
+
+ Stop();
+}
+
+TEST_P(AudioDecoderTest, DecodeAbort) {
+ Initialize();
+ Decode();
+ Stop();
wolenetz 2014/06/06 21:00:04 We had some further verification in previous FFmpe
DaleCurtis 2014/06/12 22:00:48 Fixed. Exposed a bug in the opus decoder!
+}
+
+TEST_P(AudioDecoderTest, PendingDecode_Stop) {
+ Initialize();
+ Decode();
+ Stop();
+ SatisfyPendingDecode();
wolenetz 2014/06/06 21:00:04 Is there some deterministic [partial/empty] decode
DaleCurtis 2014/06/06 21:17:00 I don't understand what you're asking. ProduceAudi
wolenetz 2014/06/06 21:52:31 Sorry. I'm confused about the difference between t
DaleCurtis 2014/06/12 22:00:47 Reworked since this covers the Decode() -> Stop()
+}
+
+TEST_P(AudioDecoderTest, PendingDecode_Reset) {
+ Initialize();
+ Decode();
+ Reset();
+ SatisfyPendingDecode();
+ Stop();
wolenetz 2014/06/06 21:00:03 ditto
DaleCurtis 2014/06/12 22:00:47 Nuked. ProduceAudioSamples covers this now. The
+}
+
+TEST_P(AudioDecoderTest, PendingDecode_ResetStop) {
+ Initialize();
+ Decode();
+ Reset();
+ Stop();
+ SatisfyPendingDecode();
wolenetz 2014/06/06 21:00:05 ditto
DaleCurtis 2014/06/12 22:00:47 Nuked, ProduceAudioSamples covers the Reset() -> S
+}
+
+const AudioSample kBearOpusSamples[] = {
+ {0, 3500, "-0.26,0.87,1.36,0.84,-0.30,-1.22,"},
+ {3500, 10000, "0.09,0.23,0.21,0.03,-0.17,-0.24,"},
+ {13500, 10000, "0.10,0.24,0.23,0.04,-0.14,-0.23,"},
+};
+
+const AudioDecoderTestData kOpusTests[] = {
+ {OPUS_DECODER, "bear-opus.ogg", kBearOpusSamples},
+};
+
+INSTANTIATE_TEST_CASE_P(OpusAudioDecoderTest,
+ AudioDecoderTest,
+ testing::ValuesIn(kOpusTests));
+
+#if defined(USE_PROPRIETARY_CODECS)
+const AudioSample kSfxMp3Samples[] = {
+ {0, 1065, "2.81,3.99,4.53,4.10,3.08,2.46,"},
+ {1065, 26122, "-3.81,-4.14,-3.90,-3.36,-3.03,-3.23,"},
+ {27188, 26122, "4.24,3.95,4.22,4.78,5.13,4.93,"},
+};
+
+const AudioSample kSfxAdtsSamples[] = {
+ {0, 23219, "-1.90,-1.53,-0.15,1.28,1.23,-0.33,"},
+ {23219, 23219, "0.54,0.88,2.19,3.54,3.24,1.63,"},
+ {46439, 23219, "1.42,1.69,2.95,4.23,4.02,2.36,"},
+};
+#endif
+
+#if defined(OS_CHROMEOS)
+const AudioSample kBearFlacSamples[] = {
+ {0, 104489, "-2.24,-0.86,0.82,2.06,1.41,-0.16,"},
+ {104489, 104489, "-1.58,-0.52,1.70,2.34,2.06,-0.05,"},
+ {208979, 104489, "-2.17,-0.75,1.03,2.14,1.41,-0.25,"},
+};
+#endif
+
+const AudioSample kBearOgvSamples[] = {
+ {0, 13061, "-2.09,-0.21,1.34,2.09,0.76,-0.95,"},
+ {13061, 23219, "-1.44,-1.27,0.18,1.37,1.95,0.13,"},
+ {36281, 23219, "-1.80,-1.41,-0.13,1.30,1.65,0.01,"},
+};
+
+const AudioSample kSfxWaveSamples[] = {
+ {0, 23219, "-1.23,-0.87,0.47,1.85,1.88,0.29,"},
+ {23219, 23219, "0.75,1.10,2.43,3.78,3.53,1.93,"},
+ {46439, 23219, "1.27,1.56,2.83,4.13,3.87,2.23,"},
+};
+
+const AudioDecoderTestData kFFmpegTests[] = {
+#if defined(USE_PROPRIETARY_CODECS)
+ {FFMPEG_DECODER, "sfx.mp3", kSfxMp3Samples},
+ {FFMPEG_DECODER, "sfx.adts", kSfxAdtsSamples},
+#endif
+#if defined(OS_CHROMEOS)
+ {FFMPEG_DECODER, "bear.flac", kBearFlacSamples},
+#endif
+ {FFMPEG_DECODER, "sfx_f32le.wav", kSfxWaveSamples},
+ {FFMPEG_DECODER, "bear.ogv", kBearOgvSamples},
+};
+
+INSTANTIATE_TEST_CASE_P(FFmpegAudioDecoderTest,
+ AudioDecoderTest,
+ testing::ValuesIn(kFFmpegTests));
+
+} // namespace media
« no previous file with comments | « no previous file | media/filters/audio_file_reader.h » ('j') | media/filters/audio_file_reader.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698