Don't double correct for discarded codec delay frames.

The existing code was correcting twice for discarded codec delay
frames.  Once by adjusting the initial timestamp and twice by
discarding those frames before generating the output timestamp.

This change also adds a test helper to AudioFileReader so we can
demux audio files in tests without duplicating code.  As a bonus
it provides some additional coverage to AudioFileReader.

I've also reused several FFmpegCommon methods where possible.

BUG=360961
TEST=Adds opus unit tests. Plus a basic playback test for ogg.
NOTRY=true

Review URL: https://codereview.chromium.org/257563007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266639 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 240 insertions, 14 deletions

diff --git a/media/filters/audio_file_reader.cc b/media/filters/audio_file_reader.cc
index 58dee72..80f8ce6 100644
--- a/media/filters/audio_file_reader.cc
+++ b/media/filters/audio_file_reader.cc
@@ -126,14 +126,7 @@ int AudioFileReader::Read(AudioBus* audio_bus) {
   bool continue_decoding = true;
 
   while (current_frame < audio_bus->frames() && continue_decoding &&
-         av_read_frame(glue_->format_context(), &packet) >= 0 &&
-         av_dup_packet(&packet) >= 0) {
-    // Skip packets from other streams.
-    if (packet.stream_index != stream_index_) {
-      av_free_packet(&packet);
-      continue;
-    }
-
+         ReadPacket(&packet)) {
     // Make a shallow copy of packet so we can slide packet.data as frames are
     // decoded from the packet; otherwise av_free_packet() will corrupt memory.
     AVPacket packet_temp = packet;
@@ -245,4 +238,21 @@ int AudioFileReader::GetNumberOfFrames() const {
   return static_cast<int>(ceil(GetDuration().InSecondsF() * sample_rate()));
 }
 
+bool AudioFileReader::ReadPacketForTesting(AVPacket* output_packet) {
+  return ReadPacket(output_packet);
+}
+
+bool AudioFileReader::ReadPacket(AVPacket* output_packet) {
+  while (av_read_frame(glue_->format_context(), output_packet) >= 0 &&
+         av_dup_packet(output_packet) >= 0) {
+    // Skip packets from other streams.
+    if (output_packet->stream_index != stream_index_) {
+      av_free_packet(output_packet);
+      continue;
+    }
+    return true;
+  }
+  return false;
+}
+
 }  // namespace media
diff --git a/media/filters/audio_file_reader.h b/media/filters/audio_file_reader.h
index 8a4ed27..e7b7b4b 100644
--- a/media/filters/audio_file_reader.h
+++ b/media/filters/audio_file_reader.h
@@ -10,6 +10,7 @@
 #include "media/base/media_export.h"
 
 struct AVCodecContext;
+struct AVPacket;
 
 namespace base { class TimeDelta; }
 
@@ -54,7 +55,19 @@ class MEDIA_EXPORT AudioFileReader {
   base::TimeDelta GetDuration() const;
   int GetNumberOfFrames() const;
 
+  // Helper methods which allows AudioFileReader to double as a test utility for
+  // demuxing audio files.  Returns true if a packet could be demuxed from the
+  // first audio stream in the file, |output_packet| will contain the demuxed
+  // packet then.
+  bool ReadPacketForTesting(AVPacket* output_packet);
+
+  const AVCodecContext* codec_context_for_testing() const {
+    return codec_context_;
+  }
+
  private:
+  bool ReadPacket(AVPacket* output_packet);
+
   scoped_ptr<FFmpegGlue> glue_;
   AVCodecContext* codec_context_;
   int stream_index_;
diff --git a/media/filters/opus_audio_decoder.cc b/media/filters/opus_audio_decoder.cc
index 4e5e71f..5ddc244 100644
--- a/media/filters/opus_audio_decoder.cc
+++ b/media/filters/opus_audio_decoder.cc
@@ -289,6 +289,9 @@ void OpusAudioDecoder::Reset(const base::Closure& closure) {
 void OpusAudioDecoder::Stop() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
+  if (!opus_decoder_)
+    return;
+
   opus_multistream_decoder_ctl(opus_decoder_, OPUS_RESET_STATE);
   ResetTimestampState();
   CloseDecoder();
@@ -398,7 +401,8 @@ bool OpusAudioDecoder::ConfigureDecoder() {
 
   if (config_.codec_delay() != opus_extra_data.skip_samples) {
     DLOG(ERROR) << "Invalid file. Codec Delay in container does not match the "
-                << "value in Opus Extra Data.";
+                << "value in Opus Extra Data. " << config_.codec_delay()
+                << " vs " << opus_extra_data.skip_samples;
     return false;
   }
 
@@ -491,12 +495,7 @@ bool OpusAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& input,
   if (output_timestamp_helper_->base_timestamp() == kNoTimestamp() &&
       !input->end_of_stream()) {
     DCHECK(input->timestamp() != kNoTimestamp());
-    // Adjust the timestamp helper so the base timestamp is corrected for frames
-    // dropped due to codec delay.
     output_timestamp_helper_->SetBaseTimestamp(input->timestamp());
-    output_timestamp_helper_->SetBaseTimestamp(
-        input->timestamp() -
-        output_timestamp_helper_->GetFrameDuration(config_.codec_delay()));
   }
 
   // Trim off any extraneous allocation.
diff --git a/media/filters/opus_audio_decoder_unittest.cc b/media/filters/opus_audio_decoder_unittest.cc
new file mode 100644
index 0000000..199dd7c
--- /dev/null
+++ b/media/filters/opus_audio_decoder_unittest.cc
@@ -0,0 +1,196 @@
+// 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/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "media/base/audio_buffer.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/in_memory_url_protocol.h"
+#include "media/filters/opus_audio_decoder.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace media {
+
+class OpusAudioDecoderTest : public testing::Test {
+ public:
+  OpusAudioDecoderTest()
+      : decoder_(new OpusAudioDecoder(message_loop_.message_loop_proxy())),
+        pending_decode_(false),
+        pending_reset_(false) {
+    // Load the test data file.
+    data_ = ReadTestDataFile("bear-opus.ogg");
+    protocol_.reset(
+        new InMemoryUrlProtocol(data_->data(), data_->data_size(), false));
+    reader_.reset(new AudioFileReader(protocol_.get()));
+    reader_->Open();
+
+    AudioDecoderConfig config;
+    AVCodecContextToAudioDecoderConfig(
+        reader_->codec_context_for_testing(), false, &config, false);
+    decoder_->Initialize(config, NewExpectedStatusCB(PIPELINE_OK));
+    base::RunLoop().RunUntilIdle();
+  }
+
+  virtual ~OpusAudioDecoderTest() {
+    EXPECT_FALSE(pending_decode_);
+    EXPECT_FALSE(pending_reset_);
+  }
+
+ protected:
+  void SatisfyPendingDecode() { base::RunLoop().RunUntilIdle(); }
+
+  void SendEndOfStream() {
+    pending_decode_ = true;
+    decoder_->Decode(DecoderBuffer::CreateEOSBuffer(),
+                     base::Bind(&OpusAudioDecoderTest::DecodeFinished,
+                                base::Unretained(this)));
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void Decode() {
+    pending_decode_ = true;
+
+    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));
+    buffer->set_duration(ConvertFromTimeBase(
+        reader_->codec_context_for_testing()->time_base, packet.duration));
+    decoder_->Decode(buffer,
+                     base::Bind(&OpusAudioDecoderTest::DecodeFinished,
+                                base::Unretained(this)));
+    av_free_packet(&packet);
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void Reset() {
+    pending_reset_ = true;
+    decoder_->Reset(base::Bind(&OpusAudioDecoderTest::ResetFinished,
+                               base::Unretained(this)));
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void Stop() {
+    decoder_->Stop();
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void DecodeFinished(AudioDecoder::Status status,
+                      const scoped_refptr<AudioBuffer>& buffer) {
+    EXPECT_TRUE(pending_decode_);
+    pending_decode_ = false;
+
+    if (status == AudioDecoder::kNotEnoughData) {
+      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);
+      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;
+  }
+
+  void ExpectDecodedAudio(size_t i, int64 timestamp, int64 duration) {
+    EXPECT_LT(i, decoded_audio_.size());
+    EXPECT_EQ(timestamp, decoded_audio_[i]->timestamp().InMicroseconds());
+    EXPECT_EQ(duration, decoded_audio_[i]->duration().InMicroseconds());
+    EXPECT_FALSE(decoded_audio_[i]->end_of_stream());
+  }
+
+  void ExpectEndOfStream(size_t i) {
+    EXPECT_LT(i, decoded_audio_.size());
+    EXPECT_TRUE(decoded_audio_[i]->end_of_stream());
+  }
+
+  size_t decoded_audio_size() const {
+    return decoded_audio_.size();
+  }
+
+ private:
+  base::MessageLoop message_loop_;
+  scoped_refptr<DecoderBuffer> data_;
+  scoped_ptr<InMemoryUrlProtocol> protocol_;
+  scoped_ptr<AudioFileReader> reader_;
+
+  scoped_ptr<OpusAudioDecoder> decoder_;
+  bool pending_decode_;
+  bool pending_reset_;
+
+  std::deque<scoped_refptr<AudioBuffer> > decoded_audio_;
+
+  DISALLOW_COPY_AND_ASSIGN(OpusAudioDecoderTest);
+};
+
+TEST_F(OpusAudioDecoderTest, Initialize) {
+  Stop();
+}
+
+TEST_F(OpusAudioDecoderTest, ProduceAudioSamples) {
+  Decode();
+  Decode();
+  Decode();
+
+  ASSERT_EQ(3u, decoded_audio_size());
+  ExpectDecodedAudio(0, 0, 3500);
+  ExpectDecodedAudio(1, 3500, 10000);
+  ExpectDecodedAudio(2, 13500, 10000);
+
+  // Call one more time to trigger EOS.
+  SendEndOfStream();
+  ASSERT_EQ(4u, decoded_audio_size());
+  ExpectEndOfStream(3);
+  Stop();
+}
+
+TEST_F(OpusAudioDecoderTest, DecodeAbort) {
+  Decode();
+  Stop();
+}
+
+TEST_F(OpusAudioDecoderTest, PendingDecode_Stop) {
+  Decode();
+  Stop();
+  SatisfyPendingDecode();
+}
+
+TEST_F(OpusAudioDecoderTest, PendingDecode_Reset) {
+  Decode();
+  Reset();
+  SatisfyPendingDecode();
+  Stop();
+}
+
+TEST_F(OpusAudioDecoderTest, PendingDecode_ResetStop) {
+  Decode();
+  Reset();
+  Stop();
+  SatisfyPendingDecode();
+}
+
+}  // namespace media
diff --git a/media/filters/pipeline_integration_test.cc b/media/filters/pipeline_integration_test.cc
index a4e47c4..96a4e34 100644
--- a/media/filters/pipeline_integration_test.cc
+++ b/media/filters/pipeline_integration_test.cc
@@ -528,6 +528,14 @@ TEST_F(PipelineIntegrationTest, BasicPlayback) {
   ASSERT_TRUE(WaitUntilOnEnded());
 }
 
+TEST_F(PipelineIntegrationTest, BasicPlaybackOpusOgg) {
+  ASSERT_TRUE(Start(GetTestDataFilePath("bear-opus.ogg"), PIPELINE_OK));
+
+  Play();
+
+  ASSERT_TRUE(WaitUntilOnEnded());
+}
+
 TEST_F(PipelineIntegrationTest, BasicPlaybackHashed) {
   ASSERT_TRUE(Start(
       GetTestDataFilePath("bear-320x240.webm"), PIPELINE_OK, kHashed));