Replace AudioDecoder::ProduceAudioSamples/ConsumeAudioSamples with read callbacks.

media/filters/audio_renderer_base.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/audio_renderer_base.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/logging.h"
 #include "media/base/filter_host.h"
 #include "media/filters/audio_renderer_algorithm_ola.h"
 
 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() {
   // Stop() should have been called and |algorithm_| should have been destroyed.
   DCHECK(state_ == kUninitialized || state_ == kStopped);
   DCHECK(!algorithm_.get());
 }
 
 void AudioRendererBase::Play(const base::Closure& callback) {
   base::AutoLock auto_lock(lock_);
   DCHECK_EQ(kPaused, state_);
   state_ = kPlaying;
   callback.Run();
 }
 
 void AudioRendererBase::Pause(const base::Closure& callback) {
   base::AutoLock auto_lock(lock_);
   DCHECK(state_ == kPlaying || state_ == kUnderflow || state_ == kRebuffering);
   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 {
     state_ = kPaused;
   }
 }
 
 void AudioRendererBase::Stop(const base::Closure& callback) {
   OnStop();
   {
     base::AutoLock auto_lock(lock_);
     state_ = kStopped;
     algorithm_.reset(NULL);
   }
   if (!callback.is_null()) {
     callback.Run();
   }
 }
 
 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;
   seek_timestamp_ = time;
 
   // Throw away everything and schedule our reads.
   last_fill_buffer_time_ = base::TimeDelta();
   recieved_end_of_stream_ = false;
   rendered_end_of_stream_ = false;
 
   // |algorithm_| will request more reads.
   algorithm_->FlushBuffers();
 }
 
 void AudioRendererBase::Initialize(AudioDecoder* decoder,
                                    const base::Closure& init_callback,
                                    const base::Closure& underflow_callback) {
   DCHECK(decoder);
   DCHECK(!init_callback.is_null());
   DCHECK(!underflow_callback.is_null());
   DCHECK_EQ(kUninitialized, state_);
   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);
 
   // Construct the algorithm.
   algorithm_.reset(new AudioRendererAlgorithmOLA());
 
   // Initialize our algorithm with media properties, initial playback rate,
   // and a callback to request more reads from the data source.
   ChannelLayout channel_layout = decoder_->channel_layout();
   int channels = ChannelLayoutToChannelCount(channel_layout);
@@ skipping 25 matching lines @@
 void AudioRendererBase::ResumeAfterUnderflow(bool buffer_more_audio) {
   base::AutoLock auto_lock(lock_);
   if (state_ == kUnderflow) {
     if (buffer_more_audio)
       algorithm_->IncreaseQueueCapacity();
 
     state_ = kRebuffering;
   }
 }
 
-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.
   // Refer to http://crbug.com/16059
   if (state_ == kStopped) {
     return;
   }
 
   // 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.
   if (state_ == kSeeking) {
     DCHECK(!seek_cb_.is_null());
     if (algorithm_->IsQueueFull() || recieved_end_of_stream_) {
       // Transition into paused whether we have data in |algorithm_| or not.
       // FillBuffer() will play silence if there's nothing to fill.
       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();
       pause_callback_.Reset();
     }
   }
 }
 
 uint32 AudioRendererBase::FillBuffer(uint8* dest,
                                      uint32 dest_len,
@@ skipping 66 matching lines @@
     // finer time update events.
     last_fill_buffer_time -= playback_delay;
     host()->SetTime(last_fill_buffer_time);
   }
 
   return dest_written;
 }
 
 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) {
   algorithm_->set_playback_rate(playback_rate);
 }
 
 float AudioRendererBase::GetPlaybackRate() {
   return algorithm_->playback_rate();
 }
 
 }  // namespace media