Update FFmpegAudioDecoder to support multi-frame packets.

media/filters/ffmpeg_audio_decoder.cc

 // Copyright (c) 2012 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 "media/filters/ffmpeg_audio_decoder.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/message_loop_proxy.h"
 #include "media/base/audio_decoder_config.h"
 #include "media/base/data_buffer.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/demuxer.h"
 #include "media/base/pipeline.h"
 #include "media/ffmpeg/ffmpeg_common.h"
 #include "media/filters/ffmpeg_glue.h"
 
 namespace media {
 
+// Helper structure for managing multiple decoded audio frames per packet.
+struct QueuedAudioBuffer {
+  AudioDecoder::Status status;
+  scoped_refptr<Buffer> buffer;
+};
+
 // Returns true if the decode result was end of stream.
 static inline bool IsEndOfStream(int result, int decoded_size, Buffer* input) {
   // Three conditions to meet to declare end of stream for this decoder:
   // 1. FFmpeg didn't read anything.
   // 2. FFmpeg didn't output anything.
   // 3. An end of stream buffer is received.
   return result == 0 && decoded_size == 0 && input->IsEndOfStream();
 }
 
 FFmpegAudioDecoder::FFmpegAudioDecoder(
@@ skipping 113 matching lines @@
   bytes_per_frame_ = codec_context_->channels * bits_per_channel_ / 8;
   status_cb.Run(PIPELINE_OK);
 }
 
 void FFmpegAudioDecoder::DoReset(const base::Closure& closure) {
   avcodec_flush_buffers(codec_context_);
   output_timestamp_base_ = kNoTimestamp();
   total_frames_decoded_ = 0;
   last_input_timestamp_ = kNoTimestamp();
   output_bytes_to_drop_ = 0;
+  queued_audio_.clear();
   closure.Run();
 }
 
 void FFmpegAudioDecoder::DoRead(const ReadCB& read_cb) {
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK(!read_cb.is_null());
   CHECK(read_cb_.is_null()) << "Overlapping decodes are not supported.";
 
   read_cb_ = read_cb;
-  ReadFromDemuxerStream();
+
+  // If we've got any queued audio from the last packet we decoded, hand it out.
+  if (!queued_audio_.empty()) {

[acolwell GONE FROM CHROMIUM, 2012/08/28 14:40:49]

flip condition, early return, & drop the else

[DaleCurtis, 2012/08/28 20:19:16]

Done.

+    base::ResetAndReturn(&read_cb_).Run(
+        queued_audio_.front().status, queued_audio_.front().buffer);
+    queued_audio_.erase(queued_audio_.begin());

[acolwell GONE FROM CHROMIUM, 2012/08/28 14:40:49]

nit: use pop_front()

[DaleCurtis, 2012/08/28 20:19:16]

Done.

+  } else {
+    ReadFromDemuxerStream();
+  }
 }
 
 void FFmpegAudioDecoder::DoDecodeBuffer(
     DemuxerStream::Status status,
     const scoped_refptr<DecoderBuffer>& input) {
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK(!read_cb_.is_null());
+  DCHECK(queued_audio_.empty());
 
   if (status != DemuxerStream::kOk) {
     DCHECK(!input);
     // TODO(acolwell): Add support for reinitializing the decoder when
     // |status| == kConfigChanged. For now we just trigger a decode error.
     AudioDecoder::Status decoder_status =
         (status == DemuxerStream::kAborted) ? kAborted : kDecodeError;
     base::ResetAndReturn(&read_cb_).Run(decoder_status, NULL);
     return;
   }
@@ skipping 34 matching lines @@
   }
 
   AVPacket packet;
   av_init_packet(&packet);
   packet.data = const_cast<uint8*>(input->GetData());
   packet.size = input->GetDataSize();
 
   PipelineStatistics statistics;
   statistics.audio_bytes_decoded = input->GetDataSize();
 
-  // Reset frame to default values.
-  avcodec_get_frame_defaults(av_frame_);
+  // Each audio packet may contain several frames, so we must call the decoder
+  // until we've exhausted the packet.  Regardless of the packet size we always
+  // want to hand it to the decoder at least once, otherwise we would end up
+  // skipping end of stream packets since they have a size of zero.
+  do {
+    // Reset frame to default values.
+    avcodec_get_frame_defaults(av_frame_);
 
-  int frame_decoded = 0;
-  int result = avcodec_decode_audio4(
-      codec_context_, av_frame_, &frame_decoded, &packet);
+    int frame_decoded = 0;
+    int result = avcodec_decode_audio4(
+        codec_context_, av_frame_, &frame_decoded, &packet);
 
-  if (result < 0) {
-    DCHECK(!input->IsEndOfStream())
-        << "End of stream buffer produced an error! "
-        << "This is quite possibly a bug in the audio decoder not handling "
-        << "end of stream AVPackets correctly.";
+    if (result < 0) {
+      DCHECK(!input->IsEndOfStream())
+          << "End of stream buffer produced an error! "
+          << "This is quite possibly a bug in the audio decoder not handling "
+          << "end of stream AVPackets correctly.";
 
-    DLOG(ERROR) << "Error decoding an audio frame with timestamp: "
-                << input->GetTimestamp().InMicroseconds() << " us, duration: "
-                << input->GetDuration().InMicroseconds() << " us, packet size: "
-                << input->GetDataSize() << " bytes";
+      DLOG(ERROR)
+          << "Error decoding an audio frame with timestamp: "
+          << input->GetTimestamp().InMicroseconds() << " us, duration: "
+          << input->GetDuration().InMicroseconds() << " us, packet size: "
+          << input->GetDataSize() << " bytes";
 
+      // We've hit an error, if we don't already have any decoded audio queued
+      // we need to ask for more data from the demuxer to fulfill this read.
+      if (queued_audio_.empty()) {
+        ReadFromDemuxerStream();
+        return;
+      } else {
+        break;

[acolwell GONE FROM CHROMIUM, 2012/08/28 14:40:49]

It looks like you should be able to unconditionally break here and
ReadFromDemuxerStream() will still get called.

[DaleCurtis, 2012/08/28 20:19:16]

Done.

+      }
+    }
+
+    // Update packet size and data pointer in case we need to call the decoder
+    // with the remaining bytes from this packet.
+    packet.size -= result;
+    packet.data += result;
+
+    if (output_timestamp_base_ == kNoTimestamp() && !input->IsEndOfStream()) {
+      DCHECK(input->GetTimestamp() != kNoTimestamp());
+      if (output_bytes_to_drop_ > 0) {
+        // Currently Vorbis is the only codec that causes us to drop samples.
+        // If we have to drop samples it always means the timeline starts at 0.
+        DCHECK(is_vorbis);
+        output_timestamp_base_ = base::TimeDelta();
+      } else {
+        output_timestamp_base_ = input->GetTimestamp();
+      }
+    }
+
+    const uint8* decoded_audio_data = NULL;
+    int decoded_audio_size = 0;
+    if (frame_decoded) {
+      int output_sample_rate = av_frame_->sample_rate;
+      if (output_sample_rate != samples_per_second_) {
+        DLOG(ERROR) << "Output sample rate (" << output_sample_rate
+                    << ") doesn't match expected rate " << samples_per_second_;
+
+        QueuedAudioBuffer queue_entry = { kDecodeError, NULL };
+        queued_audio_.push_back(queue_entry);
+        continue;

[acolwell GONE FROM CHROMIUM, 2012/08/28 14:40:49]

This should break & not continue decoding.

[DaleCurtis, 2012/08/28 20:19:16]

Done.

+      }
+
+      decoded_audio_data = av_frame_->data[0];
+      decoded_audio_size = av_samples_get_buffer_size(
+          NULL, codec_context_->channels, av_frame_->nb_samples,
+          codec_context_->sample_fmt, 1);
+    }
+
+    scoped_refptr<DataBuffer> output;
+
+    if (decoded_audio_size > 0 && output_bytes_to_drop_ > 0) {
+      int dropped_size = std::min(decoded_audio_size, output_bytes_to_drop_);
+      decoded_audio_data += dropped_size;
+      decoded_audio_size -= dropped_size;
+      output_bytes_to_drop_ -= dropped_size;
+    }
+
+    if (decoded_audio_size > 0) {
+      DCHECK_EQ(decoded_audio_size % bytes_per_frame_, 0)
+          << "Decoder didn't output full frames";
+
+      // Copy the audio samples into an output buffer.
+      output = new DataBuffer(decoded_audio_size);
+      output->SetDataSize(decoded_audio_size);
+      uint8* data = output->GetWritableData();
+      memcpy(data, decoded_audio_data, decoded_audio_size);
+
+      base::TimeDelta timestamp = GetNextOutputTimestamp();
+      total_frames_decoded_ += decoded_audio_size / bytes_per_frame_;
+
+      output->SetTimestamp(timestamp);
+      output->SetDuration(GetNextOutputTimestamp() - timestamp);
+    } else if (IsEndOfStream(result, decoded_audio_size, input)) {
+      // Create an end of stream output buffer.
+      output = new DataBuffer(0);

[acolwell GONE FROM CHROMIUM, 2012/08/28 14:40:49]

You should set packet.size = 0 here since we don't want to generate multiple end
of stream buffers.

[DaleCurtis, 2012/08/28 20:19:16]

Should already be zero here, changed to DCHECK.

+    }
+
+    if (output) {
+      QueuedAudioBuffer queue_entry = { kOk, output };
+      queued_audio_.push_back(queue_entry);
+    }
+  } while (packet.size > 0);
+
+  // Decoding finished successfully, update statistics.
+  statistics_cb_.Run(statistics);

[acolwell GONE FROM CHROMIUM, 2012/08/28 14:40:49]

I think this needs to be moved into the bottom of the loop.

[DaleCurtis, 2012/08/28 20:19:16]

Done.

+
+  // We exhausted the provided packet, but it wasn't enough for a frame.  Ask
+  // for more data in order to fulfill this read.
+  if (queued_audio_.empty()) {
     ReadFromDemuxerStream();
     return;
   }
 
-  if (result > 0)
-    DCHECK_EQ(result, input->GetDataSize());
-
-  if (output_timestamp_base_ == kNoTimestamp() && !input->IsEndOfStream()) {
-    DCHECK(input->GetTimestamp() != kNoTimestamp());
-    if (output_bytes_to_drop_ > 0) {
-      // Currently Vorbis is the only codec that causes us to drop samples.
-      // If we have to drop samples it always means the timeline starts at 0.
-      DCHECK(is_vorbis);
-      output_timestamp_base_ = base::TimeDelta();
-    } else {
-      output_timestamp_base_ = input->GetTimestamp();
-    }
-  }
-
-  const uint8* decoded_audio_data = NULL;
-  int decoded_audio_size = 0;
-  if (frame_decoded) {
-    int output_sample_rate = av_frame_->sample_rate;
-    if (output_sample_rate != samples_per_second_) {
-      DLOG(ERROR) << "Output sample rate (" << output_sample_rate
-                  << ") doesn't match expected rate " << samples_per_second_;
-      base::ResetAndReturn(&read_cb_).Run(kDecodeError, NULL);
-      return;
-    }
-
-    decoded_audio_data = av_frame_->data[0];
-    decoded_audio_size = av_samples_get_buffer_size(
-        NULL, codec_context_->channels, av_frame_->nb_samples,
-        codec_context_->sample_fmt, 1);
-  }
-
-  scoped_refptr<DataBuffer> output;
-
-  if (decoded_audio_size > 0 && output_bytes_to_drop_ > 0) {
-    int dropped_size = std::min(decoded_audio_size, output_bytes_to_drop_);
-    decoded_audio_data += dropped_size;
-    decoded_audio_size -= dropped_size;
-    output_bytes_to_drop_ -= dropped_size;
-  }
-
-  if (decoded_audio_size > 0) {
-    DCHECK_EQ(decoded_audio_size % bytes_per_frame_, 0)
-        << "Decoder didn't output full frames";
-
-    // Copy the audio samples into an output buffer.
-    output = new DataBuffer(decoded_audio_size);
-    output->SetDataSize(decoded_audio_size);
-    uint8* data = output->GetWritableData();
-    memcpy(data, decoded_audio_data, decoded_audio_size);
-
-    base::TimeDelta timestamp = GetNextOutputTimestamp();
-    total_frames_decoded_ += decoded_audio_size / bytes_per_frame_;
-
-    output->SetTimestamp(timestamp);
-    output->SetDuration(GetNextOutputTimestamp() - timestamp);
-  } else if (IsEndOfStream(result, decoded_audio_size, input)) {
-    // Create an end of stream output buffer.
-    output = new DataBuffer(0);
-  }
-
-  // Decoding finished successfully, update stats and execute callback.
-  statistics_cb_.Run(statistics);
-
-  if (!output) {
-    ReadFromDemuxerStream();
-    return;
-  }
-  base::ResetAndReturn(&read_cb_).Run(kOk, output);
+  // Execute callback to return the first frame we decoded.
+  base::ResetAndReturn(&read_cb_).Run(
+      queued_audio_.front().status, queued_audio_.front().buffer);
+  queued_audio_.erase(queued_audio_.begin());

[acolwell GONE FROM CHROMIUM, 2012/08/28 14:40:49]

use pop_front()

[DaleCurtis, 2012/08/28 20:19:16]

Done.

 }
 
 void FFmpegAudioDecoder::ReadFromDemuxerStream() {
   DCHECK(!read_cb_.is_null());
 
   demuxer_stream_->Read(base::Bind(&FFmpegAudioDecoder::DecodeBuffer, this));
 }
 
 void FFmpegAudioDecoder::DecodeBuffer(
     DemuxerStream::Status status,
     const scoped_refptr<DecoderBuffer>& buffer) {
   DCHECK_EQ(status != DemuxerStream::kOk, !buffer) << status;
 
   // TODO(scherkus): fix FFmpegDemuxerStream::Read() to not execute our read
   // callback on the same execution stack so we can get rid of forced task post.
   message_loop_->PostTask(FROM_HERE, base::Bind(
       &FFmpegAudioDecoder::DoDecodeBuffer, this, status, buffer));
 }
 
 base::TimeDelta FFmpegAudioDecoder::GetNextOutputTimestamp() const {
   DCHECK(output_timestamp_base_ != kNoTimestamp());
   double decoded_us = (total_frames_decoded_ / samples_per_second_) *
       base::Time::kMicrosecondsPerSecond;
   return output_timestamp_base_ + base::TimeDelta::FromMicroseconds(decoded_us);
 }
 }  // namespace media