Replace AudioDecoder::ProduceAudioSamples/ConsumeAudioSamples with read callbacks.

media/filters/audio_renderer_base.cc

Index: media/filters/audio_renderer_base.cc
diff --git a/media/filters/audio_renderer_base.cc b/media/filters/audio_renderer_base.cc
index 2528a70f5a6608d71a4a4e6eca51d66b6ef269ca..b128726c16bd1b0b3fbb746a2fefd3d43a731d8b 100644
--- a/media/filters/audio_renderer_base.cc
+++ b/media/filters/audio_renderer_base.cc
@@ -15,15 +15,13 @@
 
 namespace media {
 
-// Upper bound on the number of pending AudioDecoder reads.
-// TODO(acolwell): Experiment with reducing this to 1.
-const size_t kMaxPendingReads = 4;
-
 AudioRendererBase::AudioRendererBase()
     : state_(kUninitialized),
+      pending_read_(false),
       recieved_end_of_stream_(false),
       rendered_end_of_stream_(false),
-      pending_reads_(0) {
+      read_cb_(base::Bind(&AudioRendererBase::SamplesReady,
+                          base::Unretained(this))) {
 }
 
 AudioRendererBase::~AudioRendererBase() {
@@ -45,8 +43,8 @@ void AudioRendererBase::Pause(const base::Closure& callback) {
   pause_callback_ = callback;
   state_ = kPaused;
 
-  // We'll only pause when we've finished all pending reads.
-  if (pending_reads_ == 0) {
+  // Pause only when we've completed our pending read.
+  if (!pending_read_) {
     pause_callback_.Run();
     pause_callback_.Reset();
   } else {
@@ -69,7 +67,7 @@ void AudioRendererBase::Stop(const base::Closure& callback) {
 void AudioRendererBase::Seek(base::TimeDelta time, const FilterStatusCB& cb) {
   base::AutoLock auto_lock(lock_);
   DCHECK_EQ(kPaused, state_);
-  DCHECK_EQ(0u, pending_reads_) << "Pending reads should have completed";
+  DCHECK(!pending_read_) << "Pending reads must complete before seeking";
   DCHECK(seek_cb_.is_null());
   state_ = kSeeking;
   seek_cb_ = cb;
@@ -94,11 +92,6 @@ void AudioRendererBase::Initialize(AudioDecoder* decoder,
   decoder_ = decoder;
   underflow_callback_ = underflow_callback;
 
-  // Use base::Unretained() as the decoder doesn't need to ref us.
-  decoder_->set_consume_audio_samples_callback(
-      base::Bind(&AudioRendererBase::ConsumeAudioSamples,
-                 base::Unretained(this)));
-
   // Create a callback so our algorithm can request more reads.
   base::Closure cb = base::Bind(&AudioRendererBase::ScheduleRead_Locked, this);
 
@@ -144,12 +137,12 @@ void AudioRendererBase::ResumeAfterUnderflow(bool buffer_more_audio) {
   }
 }
 
-void AudioRendererBase::ConsumeAudioSamples(scoped_refptr<Buffer> buffer_in) {
+void AudioRendererBase::SamplesReady(scoped_refptr<Buffer> samples) {

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

Not sure why SamplesReady/samples is plural? Looks like it's just one sample.

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

Changed to DecodedAudioReady() which I hope will be less ambiguous

   base::AutoLock auto_lock(lock_);
   DCHECK(state_ == kPaused || state_ == kSeeking || state_ == kPlaying ||
          state_ == kUnderflow || state_ == kRebuffering);
-  DCHECK_GT(pending_reads_, 0u);
-  --pending_reads_;
+  DCHECK(pending_read_);
+  pending_read_ = false;
 
   // TODO(scherkus): this happens due to a race, primarily because Stop() is a
   // synchronous call when it should be asynchronous and accept a callback.
@@ -160,20 +153,20 @@ void AudioRendererBase::ConsumeAudioSamples(scoped_refptr<Buffer> buffer_in) {
 
   // Don't enqueue an end-of-stream buffer because it has no data, otherwise
   // discard decoded audio data until we reach our desired seek timestamp.
-  if (buffer_in->IsEndOfStream()) {
+  if (samples->IsEndOfStream()) {
     recieved_end_of_stream_ = true;
 
     // Transition to kPlaying if we are currently handling an underflow since no
     // more data will be arriving.
     if (state_ == kUnderflow || state_ == kRebuffering)
       state_ = kPlaying;
-  } else if (state_ == kSeeking && !buffer_in->IsEndOfStream() &&
-             (buffer_in->GetTimestamp() + buffer_in->GetDuration()) <
+  } else if (state_ == kSeeking && !samples->IsEndOfStream() &&
+             (samples->GetTimestamp() + samples->GetDuration()) <
                  seek_timestamp_) {
     ScheduleRead_Locked();
   } else {
     // Note: Calling this may schedule more reads.
-    algorithm_->EnqueueBuffer(buffer_in);
+    algorithm_->EnqueueBuffer(samples);
   }
 
   // Check for our preroll complete condition.
@@ -185,7 +178,7 @@ void AudioRendererBase::ConsumeAudioSamples(scoped_refptr<Buffer> buffer_in) {
       state_ = kPaused;
       ResetAndRunCB(&seek_cb_, PIPELINE_OK);
     }
-  } else if (state_ == kPaused && pending_reads_ == 0) {
+  } else if (state_ == kPaused && !pending_read_) {
     // No more pending reads!  We're now officially "paused".
     if (!pause_callback_.is_null()) {
       pause_callback_.Run();
@@ -272,14 +265,9 @@ uint32 AudioRendererBase::FillBuffer(uint8* dest,
 
 void AudioRendererBase::ScheduleRead_Locked() {
   lock_.AssertAcquired();
-  if (pending_reads_ < kMaxPendingReads) {
-    ++pending_reads_;
-    // TODO(jiesun): We use dummy buffer to feed decoder to let decoder to
-    // provide buffer pools. In the future, we may want to implement real
-    // buffer pool to recycle buffers.
-    scoped_refptr<Buffer> buffer;
-    decoder_->ProduceAudioSamples(buffer);
-  }
+  DCHECK(!pending_read_);
+  pending_read_ = true;
+  decoder_->Read(read_cb_);

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

nit: Looks like it's unnecessary to make |read_cb_| a member. Just call
base::Bind(AudioRendererBase::SamplesReady, ...) here.

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

somewhat of an optimization to avoid needlessly re-mallocing the base::Bind()
result given we callback remains the same throughout the lifetime of playback

 }
 
 void AudioRendererBase::SetPlaybackRate(float playback_rate) {