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

media/ffmpeg/ffmpeg_common.cc

@@ -270,7 +270,7 @@ static AVSampleFormat SampleFormatToAVSampleFormat(SampleFormat sample_format) {
   return AV_SAMPLE_FMT_NONE;
 }
 
-static void AVCodecContextToAudioDecoderConfig(
+void AVCodecContextToAudioDecoderConfig(
     const AVCodecContext* codec_context,
     bool is_encrypted,
     AudioDecoderConfig* config,

media/ffmpeg/ffmpeg_common.h

@@ -94,6 +94,12 @@ void VideoDecoderConfigToAVCodecContext(
     const VideoDecoderConfig& config,
     AVCodecContext* codec_context);
 
+MEDIA_EXPORT void AVCodecContextToAudioDecoderConfig(
+    const AVCodecContext* codec_context,
+    bool is_encrypted,
+    AudioDecoderConfig* config,
+    bool record_stats);
+
 // Converts FFmpeg's channel layout to chrome's ChannelLayout.  |channels| can
 // be used when FFmpeg's channel layout is not informative in order to make a
 // good guess about the plausible channel layout based on number of channels.

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

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_;

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.

media/filters/opus_audio_decoder_unittest.cc

+// 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

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));

media/media.gyp

@@ -1037,6 +1037,7 @@
         'filters/h264_bit_reader_unittest.cc',
         'filters/h264_parser_unittest.cc',
         'filters/h264_to_annex_b_bitstream_converter_unittest.cc',
+        'filters/opus_audio_decoder_unittest.cc',
         'filters/pipeline_integration_test.cc',
         'filters/pipeline_integration_test_base.cc',
         'filters/skcanvas_video_renderer_unittest.cc',
@@ -1116,6 +1117,7 @@
             'filters/ffmpeg_glue_unittest.cc',
             'filters/ffmpeg_h264_to_annex_b_bitstream_converter_unittest.cc',
             'filters/ffmpeg_video_decoder_unittest.cc',
+            'filters/opus_audio_decoder_unittest.cc',
             'filters/pipeline_integration_test.cc',
             'filters/pipeline_integration_test_base.cc',
           ],