Replace AudioDecoder::ProduceAudioSamples/ConsumeAudioSamples with read callbacks.

media/filters/ffmpeg_audio_decoder.cc

 // Copyright (c) 2011 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 "media/base/audio_decoder_config.h"
 #include "media/base/data_buffer.h"
 #include "media/base/demuxer.h"
@@ skipping 34 matching lines @@
 }
 
 
 FFmpegAudioDecoder::FFmpegAudioDecoder(MessageLoop* message_loop)
     : message_loop_(message_loop),
       codec_context_(NULL),
       bits_per_channel_(0),
       channel_layout_(CHANNEL_LAYOUT_NONE),
       samples_per_second_(0),
       decoded_audio_size_(AVCODEC_MAX_AUDIO_FRAME_SIZE),
-      decoded_audio_(static_cast<uint8*>(av_malloc(decoded_audio_size_))),
-      pending_reads_(0) {
+      decoded_audio_(static_cast<uint8*>(av_malloc(decoded_audio_size_))) {
 }
 
 FFmpegAudioDecoder::~FFmpegAudioDecoder() {
   av_free(decoded_audio_);
 
   // XXX: should we require Stop() to be called? this might end up getting
   // called on a random thread due to refcounting.
   if (codec_context_) {
     av_free(codec_context_->extradata);
     avcodec_close(codec_context_);
@@ skipping 13 matching lines @@
     const StatisticsCallback& stats_callback) {
   // TODO(scherkus): change Initialize() signature to pass |stream| as a
   // scoped_refptr<>.
   scoped_refptr<DemuxerStream> ref_stream(stream);
   message_loop_->PostTask(
       FROM_HERE,
       base::Bind(&FFmpegAudioDecoder::DoInitialize, this,
                  ref_stream, callback, stats_callback));
 }
 
-void FFmpegAudioDecoder::ProduceAudioSamples(scoped_refptr<Buffer> buffer) {
-  message_loop_->PostTask(
-      FROM_HERE,
-      base::Bind(&FFmpegAudioDecoder::DoProduceAudioSamples, this,
-                 buffer));
+void FFmpegAudioDecoder::Read(const ReadCB& callback) {
+  // TODO(scherkus): forced task post since AudioRendererBase::SamplesReady()

[vrk (LEFT CHROMIUM), 2011/12/03 03:10:17]

nit: What's the TODO? Should this just be a comment?

[scherkus (not reviewing), 2011/12/07 05:25:12]

done and updated FFmpegVideoDecoder as well

+  // will call Read() on FFmpegAudioDecoder's thread as we executed |read_cb_|.
+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &FFmpegAudioDecoder::DoRead, this, callback));
 }
 
 int FFmpegAudioDecoder::bits_per_channel() {
   return bits_per_channel_;
 }
 
 ChannelLayout FFmpegAudioDecoder::channel_layout() {
   return channel_layout_;
 }
 
@@ skipping 44 matching lines @@
 
   callback.Run();
 }
 
 void FFmpegAudioDecoder::DoFlush(const base::Closure& callback) {
   avcodec_flush_buffers(codec_context_);
   estimated_next_timestamp_ = kNoTimestamp;
   callback.Run();
 }
 
-void FFmpegAudioDecoder::DoProduceAudioSamples(
-    const scoped_refptr<Buffer>& output) {
-  output_buffers_.push_back(output);
+void FFmpegAudioDecoder::DoRead(const ReadCB& callback) {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+  CHECK(!callback.is_null());

[vrk (LEFT CHROMIUM), 2011/12/03 03:10:17]

CHECK? Do you mean DCHECK?

[scherkus (not reviewing), 2011/12/07 05:25:12]

Done.

+  CHECK(read_cb_.is_null()) << "Overlapping decodes are not supported.";
+
+  read_cb_ = callback;
   ReadFromDemuxerStream();
 }
 
 void FFmpegAudioDecoder::DoDecodeBuffer(const scoped_refptr<Buffer>& input) {
-  DCHECK(!output_buffers_.empty());
-  DCHECK_GT(pending_reads_, 0);
-  pending_reads_--;
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+  DCHECK(!read_cb_.is_null());
 
   // FFmpeg tends to seek Ogg audio streams in the middle of nowhere, giving us
   // a whole bunch of AV_NOPTS_VALUE packets.  Discard them until we find
   // something valid.  Refer to http://crbug.com/49709
   if (input->GetTimestamp() == kNoTimestamp &&
       estimated_next_timestamp_ == kNoTimestamp &&
       !input->IsEndOfStream()) {
     ReadFromDemuxerStream();
     return;
   }
@@ skipping 47 matching lines @@
   } else if (IsEndOfStream(result, decoded_audio_size, input)) {
     // Create an end of stream output buffer.
     output = new DataBuffer(0);
     output->SetTimestamp(input->GetTimestamp());
     output->SetDuration(input->GetDuration());
   }
 
   // Decoding finished successfully, update stats and execute callback.
   stats_callback_.Run(statistics);
   if (output) {
-    DCHECK_GT(output_buffers_.size(), 0u);
-    output_buffers_.pop_front();
-
-    ConsumeAudioSamples(output);
+    DeliverSamples(output);

[vrk (LEFT CHROMIUM), 2011/12/03 03:10:17]

nit: remove {}

[scherkus (not reviewing), 2011/12/07 05:25:12]

other one-line statements in this file + ffmpegvideodecoder use {} -- going with
consistency here

as a side, I try to use use {} for non-trivial ifs (i.e., there's an else clause
or the statement requires 3+ lines) but that's purely preferential!

   } else {
     ReadFromDemuxerStream();
   }
 }
 
 void FFmpegAudioDecoder::ReadFromDemuxerStream() {
-  DCHECK(!output_buffers_.empty())
-      << "Reads should only occur if there are output buffers.";
+  DCHECK(!read_cb_.is_null());
 
-  pending_reads_++;
   demuxer_stream_->Read(base::Bind(&FFmpegAudioDecoder::DecodeBuffer, this));
 }
 
 void FFmpegAudioDecoder::DecodeBuffer(const scoped_refptr<Buffer>& buffer) {
-  message_loop_->PostTask(
-      FROM_HERE,
-      base::Bind(&FFmpegAudioDecoder::DoDecodeBuffer, this, buffer));
+  // TODO(scherkus): forced task post since FFmpegDemuxerStream::Read() can

[vrk (LEFT CHROMIUM), 2011/12/03 03:10:17]

nit: ditto above comment re: "TODO"

[scherkus (not reviewing), 2011/12/07 05:25:12]

Done + FFmpegVideoDecoder

+  // immediately execute our callback on FFmpegAudioDecoder's thread.
+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &FFmpegAudioDecoder::DoDecodeBuffer, this, buffer));
 }
 
 void FFmpegAudioDecoder::UpdateDurationAndTimestamp(
     const Buffer* input,
     DataBuffer* output) {
   // Always calculate duration based on the actual number of samples decoded.
   base::TimeDelta duration = CalculateDuration(output->GetDataSize());
   output->SetDuration(duration);
 
   // Use the incoming timestamp if it's valid.
@@ skipping 12 matching lines @@
 }
 
 base::TimeDelta FFmpegAudioDecoder::CalculateDuration(int size) {
   int64 denominator = ChannelLayoutToChannelCount(channel_layout_) *
       bits_per_channel_ / 8 * samples_per_second_;
   double microseconds = size /
       (denominator / static_cast<double>(base::Time::kMicrosecondsPerSecond));
   return base::TimeDelta::FromMicroseconds(static_cast<int64>(microseconds));
 }
 
+void FFmpegAudioDecoder::DeliverSamples(const scoped_refptr<Buffer>& samples) {
+  // Reset the callback before running to protect against reentrancy.
+  ReadCB read_cb = read_cb_;
+  read_cb_.Reset();
+  read_cb.Run(samples);
+}
+
 }  // namespace media