Replace AudioDecoder::ProduceAudioSamples/ConsumeAudioSamples with read callbacks.

Review URL: http://codereview.chromium.org/8763010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@115148 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 288 insertions, 410 deletions

diff --git a/content/renderer/media/audio_renderer_impl_unittest.cc b/content/renderer/media/audio_renderer_impl_unittest.cc
index 21c2c99..7a2895b 100644
--- a/content/renderer/media/audio_renderer_impl_unittest.cc
+++ b/content/renderer/media/audio_renderer_impl_unittest.cc
@@ -168,18 +168,6 @@ TEST_F(AudioRendererImplTest, SetVolume) {
   WaitForIOThreadCompletion();
 }
 
-TEST_F(AudioRendererImplTest, Stop) {
-  // Execute Stop() codepath.
-  // Tasks will be posted internally on the IO thread.
-  renderer_->Stop(media::NewExpectedClosure());
-
-  WaitForIOThreadCompletion();
-
-  // It's possible that the upstream decoder replies right after being stopped.
-  scoped_refptr<media::Buffer> buffer(new media::DataBuffer(kSize));
-  renderer_->ConsumeAudioSamples(buffer);
-}
-
 TEST_F(AudioRendererImplTest, UpdateEarliestEndTime) {
   renderer_->SetPlaybackRate(1.0f);
   WaitForIOThreadCompletion();
diff --git a/media/base/filters.cc b/media/base/filters.cc
index c137216..7132661 100644
--- a/media/base/filters.cc
+++ b/media/base/filters.cc
@@ -79,8 +79,4 @@ AudioDecoder::AudioDecoder() {}
 
 AudioDecoder::~AudioDecoder() {}
 
-void AudioDecoder::ConsumeAudioSamples(scoped_refptr<Buffer> buffer) {
-  consume_audio_samples_callback_.Run(buffer);
-}
-
 }  // namespace media
diff --git a/media/base/filters.h b/media/base/filters.h
index 654dae1..ed3af89 100644
--- a/media/base/filters.h
+++ b/media/base/filters.h
@@ -164,18 +164,15 @@ class MEDIA_EXPORT AudioDecoder : public Filter {
   virtual void Initialize(DemuxerStream* stream, const base::Closure& callback,
                           const StatisticsCallback& stats_callback) = 0;
 
-  // Renderer provides an output buffer for Decoder to write to. These buffers
-  // will be recycled to renderer via the permanent callback.
+  // Request samples to be decoded and returned via the provided callback.
+  // Only one read may be in flight at any given time.
   //
-  // We could also pass empty pointer here to let decoder provide buffers pool.
-  virtual void ProduceAudioSamples(scoped_refptr<Buffer> buffer) = 0;
-
-  // Installs a permanent callback for passing decoded audio output.
-  typedef base::Callback<void(scoped_refptr<Buffer>)> ConsumeAudioSamplesCB;
-  void set_consume_audio_samples_callback(
-      const ConsumeAudioSamplesCB& callback) {
-    consume_audio_samples_callback_ = callback;
-  }
+  // Implementations guarantee that the callback will not be called from within
+  // this method.
+  //
+  // Sample buffers will be non-NULL yet may be end of stream buffers.
+  typedef base::Callback<void(scoped_refptr<Buffer>)> ReadCB;
+  virtual void Read(const ReadCB& callback) = 0;
 
   // Returns various information about the decoded audio format.
   virtual int bits_per_channel() = 0;
@@ -185,12 +182,6 @@ class MEDIA_EXPORT AudioDecoder : public Filter {
  protected:
   AudioDecoder();
   virtual ~AudioDecoder();
-
-  // Executes the permanent callback to pass off decoded audio.
-  void ConsumeAudioSamples(scoped_refptr<Buffer> buffer);
-
- private:
-  ConsumeAudioSamplesCB consume_audio_samples_callback_;
 };
 
 
diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
index b3758f2..7961e44 100644
--- a/media/base/mock_filters.h
+++ b/media/base/mock_filters.h
@@ -213,15 +213,12 @@ class MockAudioDecoder : public AudioDecoder {
   MOCK_METHOD3(Initialize, void(DemuxerStream* stream,
                                 const base::Closure& callback,
                                 const StatisticsCallback& stats_callback));
+  MOCK_METHOD1(Read, void(const ReadCB& callback));
   MOCK_METHOD1(ProduceAudioSamples, void(scoped_refptr<Buffer>));
   MOCK_METHOD0(bits_per_channel, int(void));
   MOCK_METHOD0(channel_layout, ChannelLayout(void));
   MOCK_METHOD0(samples_per_second, int(void));
 
-  void ConsumeAudioSamplesForTest(scoped_refptr<Buffer> buffer) {
-    AudioDecoder::ConsumeAudioSamples(buffer);
-  }
-
  protected:
   virtual ~MockAudioDecoder();
 
diff --git a/media/filters/audio_renderer_algorithm_base.cc b/media/filters/audio_renderer_algorithm_base.cc
index 2a05a24..25b6a29 100644
--- a/media/filters/audio_renderer_algorithm_base.cc
+++ b/media/filters/audio_renderer_algorithm_base.cc
@@ -283,6 +283,10 @@ uint32 AudioRendererAlgorithmBase::QueueSize() {
   return queue_.forward_bytes();
 }
 
+uint32 AudioRendererAlgorithmBase::QueueCapacity() {
+  return queue_.forward_capacity();
+}
+
 void AudioRendererAlgorithmBase::IncreaseQueueCapacity() {
   queue_.set_forward_capacity(
       std::min(2 * queue_.forward_capacity(), max_queue_capacity_));
diff --git a/media/filters/audio_renderer_algorithm_base.h b/media/filters/audio_renderer_algorithm_base.h
index c6888c2..6ea8db5 100644
--- a/media/filters/audio_renderer_algorithm_base.h
+++ b/media/filters/audio_renderer_algorithm_base.h
@@ -71,9 +71,14 @@ class MEDIA_EXPORT AudioRendererAlgorithmBase {
   // Returns true if we have enough data
   bool IsQueueFull();
 
-  // Returns the number of bytes left in |queue_|.
+  // Returns the number of bytes left in |queue_|, which may be larger than
+  // QueueCapacity() in the event that a read callback delivered more data than
+  // |queue_| was intending to hold.
   uint32 QueueSize();
 
+  // Returns the capacity of |queue_|.
+  uint32 QueueCapacity();
+
   // Increase the capacity of |queue_| if possible.
   void IncreaseQueueCapacity();
 
diff --git a/media/filters/audio_renderer_base.cc b/media/filters/audio_renderer_base.cc
index 03c9ed0..e3f2164 100644
--- a/media/filters/audio_renderer_base.cc
+++ b/media/filters/audio_renderer_base.cc
@@ -14,15 +14,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::DecodedAudioReady,
+                          base::Unretained(this))) {
 }
 
 AudioRendererBase::~AudioRendererBase() {
@@ -44,8 +42,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 {
@@ -68,7 +66,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 read must complete before seeking";
   DCHECK(seek_cb_.is_null());
   state_ = kSeeking;
   seek_cb_ = cb;
@@ -93,11 +91,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);
 
@@ -143,15 +136,13 @@ void AudioRendererBase::ResumeAfterUnderflow(bool buffer_more_audio) {
   }
 }
 
-void AudioRendererBase::ConsumeAudioSamples(scoped_refptr<Buffer> buffer_in) {
+void AudioRendererBase::DecodedAudioReady(scoped_refptr<Buffer> buffer) {
   base::AutoLock auto_lock(lock_);
   DCHECK(state_ == kPaused || state_ == kSeeking || state_ == kPlaying ||
-         state_ == kUnderflow || state_ == kRebuffering ||
-         state_ == kStopped);
-  if (!pending_reads_)
-    return;
+         state_ == kUnderflow || state_ == kRebuffering || state_ == kStopped);
 
-  --pending_reads_;
+  CHECK(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.
@@ -162,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 (buffer->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 && !buffer->IsEndOfStream() &&
+             (buffer->GetTimestamp() + buffer->GetDuration()) <
                  seek_timestamp_) {
     ScheduleRead_Locked();
   } else {
     // Note: Calling this may schedule more reads.
-    algorithm_->EnqueueBuffer(buffer_in);
+    algorithm_->EnqueueBuffer(buffer);
   }
 
   // Check for our preroll complete condition.
@@ -187,8 +178,8 @@ void AudioRendererBase::ConsumeAudioSamples(scoped_refptr<Buffer> buffer_in) {
       state_ = kPaused;
       ResetAndRunCB(&seek_cb_, PIPELINE_OK);
     }
-  } else if (state_ == kPaused && pending_reads_ == 0) {
-    // No more pending reads!  We're now officially "paused".
+  } else if (state_ == kPaused && !pending_read_) {
+    // No more pending read!  We're now officially "paused".
     if (!pause_callback_.is_null()) {
       pause_callback_.Run();
       pause_callback_.Reset();
@@ -216,6 +207,8 @@ uint32 AudioRendererBase::FillBuffer(uint8* dest,
       // zeros.  This gets around the tricky situation of pausing and resuming
       // the audio IPC layer in Chrome.  Ideally, we should return zero and then
       // the subclass can restart the conversation.
+      //
+      // This should get handled by the subclass http://crbug.com/106600
       const uint32 kZeroLength = 8192;
       dest_written = std::min(kZeroLength, dest_len);
       memset(dest, 0, dest_written);
@@ -274,14 +267,10 @@ 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);
-  }
+  if (pending_read_)
+    return;
+  pending_read_ = true;
+  decoder_->Read(read_cb_);
 }
 
 void AudioRendererBase::SetPlaybackRate(float playback_rate) {
diff --git a/media/filters/audio_renderer_base.h b/media/filters/audio_renderer_base.h
index 26aa76b..a578204 100644
--- a/media/filters/audio_renderer_base.h
+++ b/media/filters/audio_renderer_base.h
@@ -57,9 +57,8 @@ class MEDIA_EXPORT AudioRendererBase : public AudioRenderer {
   // this time, such as stopping any running threads.
   virtual void OnStop() = 0;
 
-  // Called when a AudioDecoder completes decoding and decrements
-  // |pending_reads_|.
-  virtual void ConsumeAudioSamples(scoped_refptr<Buffer> buffer_in);
+  // Callback from the audio decoder delivering decoded audio samples.
+  void DecodedAudioReady(scoped_refptr<Buffer> buffer);
 
   // Fills the given buffer with audio data by delegating to its |algorithm_|.
   // FillBuffer() also takes care of updating the clock. Returns the number of
@@ -92,6 +91,8 @@ class MEDIA_EXPORT AudioRendererBase : public AudioRenderer {
   virtual float GetPlaybackRate();
 
  private:
+  friend class AudioRendererBaseTest;
+
   // Helper method that schedules an asynchronous read from the decoder and
   // increments |pending_reads_|.
   //
@@ -119,17 +120,13 @@ class MEDIA_EXPORT AudioRendererBase : public AudioRenderer {
   };
   State state_;
 
+  // Keep track of our outstanding read to |decoder_|.
+  bool pending_read_;
+
   // Keeps track of whether we received and rendered the end of stream buffer.
   bool recieved_end_of_stream_;
   bool rendered_end_of_stream_;
 
-  // Keeps track of our pending reads.  We *must* have no pending reads before
-  // executing the pause callback, otherwise we breach the contract that all
-  // filters are idling.
-  //
-  // We use size_t since we compare against std::deque::size().
-  size_t pending_reads_;
-
   // Audio time at end of last call to FillBuffer().
   // TODO(ralphl): Update this value after seeking.
   base::TimeDelta last_fill_buffer_time_;
@@ -142,6 +139,8 @@ class MEDIA_EXPORT AudioRendererBase : public AudioRenderer {
 
   base::TimeDelta seek_timestamp_;
 
+  AudioDecoder::ReadCB read_cb_;
+
   DISALLOW_COPY_AND_ASSIGN(AudioRendererBase);
 };
 
diff --git a/media/filters/audio_renderer_base_unittest.cc b/media/filters/audio_renderer_base_unittest.cc
index 6f53c0c..964686e 100644
--- a/media/filters/audio_renderer_base_unittest.cc
+++ b/media/filters/audio_renderer_base_unittest.cc
@@ -14,14 +14,17 @@
 
 using ::testing::_;
 using ::testing::AnyNumber;
-using ::testing::InSequence;
 using ::testing::Invoke;
-using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::StrictMock;
 
 namespace media {
 
+// Constants for distinguishing between muted audio and playing audio when using
+// ConsumeBufferedData().
+static uint8 kMutedAudio = 0x00;
+static uint8 kPlayingAudio = 0x99;
+
 // Mocked subclass of AudioRendererBase for testing purposes.
 class MockAudioRendererBase : public AudioRendererBase {
  public:
@@ -36,15 +39,7 @@ class MockAudioRendererBase : public AudioRendererBase {
   MOCK_METHOD3(OnInitialize, bool(int, ChannelLayout, int));
   MOCK_METHOD0(OnStop, void());
 
-  // Used for verifying check points during tests.
-  MOCK_METHOD1(CheckPoint, void(int id));
-
  private:
-  FRIEND_TEST_ALL_PREFIXES(AudioRendererBaseTest, OneCompleteReadCycle);
-  FRIEND_TEST_ALL_PREFIXES(AudioRendererBaseTest, Underflow);
-  FRIEND_TEST_ALL_PREFIXES(AudioRendererBaseTest, Underflow_EndOfStream);
-  friend class AudioRendererBaseTest;
-
   DISALLOW_COPY_AND_ASSIGN(MockAudioRendererBase);
 };
 
@@ -53,13 +48,12 @@ class AudioRendererBaseTest : public ::testing::Test {
   // Give the decoder some non-garbage media properties.
   AudioRendererBaseTest()
       : renderer_(new MockAudioRendererBase()),
-        decoder_(new MockAudioDecoder()),
-        pending_reads_(0) {
+        decoder_(new MockAudioDecoder()) {
     renderer_->set_host(&host_);
 
-    // Queue all reads from the decoder.
-    EXPECT_CALL(*decoder_, ProduceAudioSamples(_))
-        .WillRepeatedly(Invoke(this, &AudioRendererBaseTest::EnqueueCallback));
+    // Queue all reads from the decoder by default.
+    ON_CALL(*decoder_, Read(_))
+        .WillByDefault(Invoke(this, &AudioRendererBaseTest::SaveReadCallback));
 
     // Set up audio properties.
     ON_CALL(*decoder_, bits_per_channel())
@@ -78,337 +72,253 @@ class AudioRendererBaseTest : public ::testing::Test {
   }
 
   virtual ~AudioRendererBaseTest() {
-    // Expect a call into the subclass.
     EXPECT_CALL(*renderer_, OnStop());
     renderer_->Stop(NewExpectedClosure());
   }
 
-  MOCK_METHOD0(OnUnderflow, void());
+  MOCK_METHOD1(OnSeekComplete, void(PipelineStatus));
+  FilterStatusCB NewSeekCB() {
+    return base::Bind(&AudioRendererBaseTest::OnSeekComplete,
+                      base::Unretained(this));
+  }
 
+  MOCK_METHOD0(OnUnderflow, void());
   base::Closure NewUnderflowClosure() {
     return base::Bind(&AudioRendererBaseTest::OnUnderflow,
                       base::Unretained(this));
   }
 
-  scoped_refptr<DataBuffer> CreateBuffer(int data_size, uint8 value) {
-    scoped_refptr<DataBuffer> buffer(new DataBuffer(data_size));
-    buffer->SetDataSize(data_size);
-    memset(buffer->GetWritableData(), value, buffer->GetDataSize());
-    return buffer;
+  void Initialize() {
+    EXPECT_CALL(*renderer_, OnInitialize(_, _, _))
+        .WillOnce(Return(true));
+    renderer_->Initialize(
+        decoder_, NewExpectedClosure(), NewUnderflowClosure());
   }
 
-  void WriteUntilNoPendingReads(int data_size, uint8 value,
-                                uint32* bytes_buffered) {
-    while (pending_reads_ > 0) {
-      --pending_reads_;
-      *bytes_buffered += data_size;
-      decoder_->ConsumeAudioSamplesForTest(CreateBuffer(data_size, value));
-    }
+  void Preroll() {
+    // Seek to trigger prerolling.
+    EXPECT_CALL(*decoder_, Read(_));
+    renderer_->Seek(base::TimeDelta(), NewSeekCB());
+
+    // Fill entire buffer to complete prerolling.
+    EXPECT_CALL(*this, OnSeekComplete(PIPELINE_OK));
+    DeliverRemainingAudio();
   }
 
-  void ConsumeBufferedData(uint32 data_size, uint8 expected_value,
-                           uint32* bytes_buffered) {
-    DCHECK(bytes_buffered);
-    DCHECK_GT(*bytes_buffered, 0u);
-
-    base::TimeDelta playback_delay(base::TimeDelta::FromSeconds(1));
-    scoped_array<uint8> buffer(new uint8[data_size]);
-    while (*bytes_buffered > 0) {
-      EXPECT_EQ(data_size, renderer_->FillBuffer(buffer.get(), data_size,
-                                                 playback_delay, false));
-      EXPECT_EQ(expected_value, buffer[0]);
-      *bytes_buffered -= data_size;
-    }
+  void Play() {
+    renderer_->Play(NewExpectedClosure());
+    renderer_->SetPlaybackRate(1.0f);
+  }
+
+  // Delivers |size| bytes with value kPlayingAudio to |renderer_|.
+  //
+  // There must be a pending read callback.
+  void FulfillPendingRead(size_t size) {
+    CHECK(!read_cb_.is_null());
+    scoped_refptr<DataBuffer> buffer(new DataBuffer(size));
+    buffer->SetDataSize(size);
+    memset(buffer->GetWritableData(), kPlayingAudio, buffer->GetDataSize());
+
+    AudioDecoder::ReadCB read_cb;
+    std::swap(read_cb, read_cb_);
+    read_cb.Run(buffer);
+  }
+
+  // Delivers an end of stream buffer to |renderer_|.
+  //
+  // There must be a pending read callback.
+  void DeliverEndOfStream() {
+    FulfillPendingRead(0);
+  }
+
+  // Delivers bytes until |renderer_|'s internal buffer is full and no longer
+  // has pending reads.
+  void DeliverRemainingAudio() {
+    CHECK(!read_cb_.is_null());
+    FulfillPendingRead(bytes_remaining_in_buffer());
+    CHECK(read_cb_.is_null());
   }
 
-  void ExpectUnderflow(uint32 data_size, int checkpoint_value) {
-    scoped_array<uint8> buffer(new uint8[data_size]);
-    base::TimeDelta playback_delay(base::TimeDelta::FromSeconds(1));
+  // Attempts to consume |size| bytes from |renderer_|'s internal buffer,
+  // returning true if all |size| bytes were consumed, false if less than
+  // |size| bytes were consumed.
+  //
+  // |muted| is optional and if passed will get set if the byte value of
+  // the consumed data is muted audio.
+  bool ConsumeBufferedData(uint32 size, bool* muted) {
+    scoped_array<uint8> buffer(new uint8[size]);
+    uint32 bytes_read = renderer_->FillBuffer(buffer.get(), size,
+                                              base::TimeDelta(), true);
+    if (bytes_read > 0 && muted) {
+      *muted = (buffer[0] == kMutedAudio);
+    }
+    return (bytes_read == size);
+  }
 
-    EXPECT_CALL(*this, OnUnderflow());
-    EXPECT_CALL(*renderer_, CheckPoint(checkpoint_value));
-    EXPECT_EQ(0u, renderer_->FillBuffer(buffer.get(), data_size, playback_delay,
-                                        false));
-    renderer_->CheckPoint(checkpoint_value);
+  uint32 bytes_buffered() {
+    return renderer_->algorithm_->QueueSize();
   }
 
+  uint32 buffer_capacity() {
+    return renderer_->algorithm_->QueueCapacity();
+  }
 
- protected:
-  static const size_t kMaxQueueSize;
+  uint32 bytes_remaining_in_buffer() {
+    // This can happen if too much data was delivered, in which case the buffer
+    // will accept the data but not increase capacity.
+    if (bytes_buffered() > buffer_capacity()) {
+      return 0;
+    }
+    return buffer_capacity() - bytes_buffered();
+  }
 
   // Fixture members.
   scoped_refptr<MockAudioRendererBase> renderer_;
   scoped_refptr<MockAudioDecoder> decoder_;
   StrictMock<MockFilterHost> host_;
-
-  // Number of asynchronous read requests sent to |decoder_|.
-  size_t pending_reads_;
+  AudioDecoder::ReadCB read_cb_;
 
  private:
-  void EnqueueCallback(scoped_refptr<Buffer> buffer) {
-    ++pending_reads_;
+  void SaveReadCallback(const AudioDecoder::ReadCB& callback) {
+    CHECK(read_cb_.is_null()) << "Overlapping reads are not permitted";
+    read_cb_ = callback;
   }
 
   DISALLOW_COPY_AND_ASSIGN(AudioRendererBaseTest);
 };
 
-const size_t AudioRendererBaseTest::kMaxQueueSize = 1u;
-
 TEST_F(AudioRendererBaseTest, Initialize_Failed) {
-  InSequence s;
-
-  // Our subclass will fail when asked to initialize.
   EXPECT_CALL(*renderer_, OnInitialize(_, _, _))
       .WillOnce(Return(false));
-
-  // We expect to receive an error.
   EXPECT_CALL(host_, SetError(PIPELINE_ERROR_INITIALIZATION_FAILED));
-
-  // Initialize, we expect to have no reads.
   renderer_->Initialize(decoder_, NewExpectedClosure(), NewUnderflowClosure());
-  EXPECT_EQ(0u, pending_reads_);
+
+  // We should have no reads.
+  EXPECT_TRUE(read_cb_.is_null());
 }
 
 TEST_F(AudioRendererBaseTest, Initialize_Successful) {
-  InSequence s;
-
-  // Then our subclass will be asked to initialize.
   EXPECT_CALL(*renderer_, OnInitialize(_, _, _))
       .WillOnce(Return(true));
-
-  // Initialize, we shouldn't have any reads.
   renderer_->Initialize(decoder_, NewExpectedClosure(), NewUnderflowClosure());
 
-  EXPECT_EQ(0u, pending_reads_);
-
-  // Now seek to trigger prerolling, verifying the callback hasn't been
-  // executed yet.
-  EXPECT_CALL(*renderer_, CheckPoint(0));
-  renderer_->Seek(base::TimeDelta(), NewExpectedStatusCB(PIPELINE_OK));
-  EXPECT_EQ(kMaxQueueSize, pending_reads_);
-  renderer_->CheckPoint(0);
-
-  // Now satisfy the read requests.  Our callback should be executed after
-  // exiting this loop.
-  while (pending_reads_) {
-    scoped_refptr<DataBuffer> buffer(new DataBuffer(1024));
-    buffer->SetDataSize(1024);
-    --pending_reads_;
-    decoder_->ConsumeAudioSamplesForTest(buffer);
-  }
+  // We should have no reads.
+  EXPECT_TRUE(read_cb_.is_null());
 }
 
-TEST_F(AudioRendererBaseTest, OneCompleteReadCycle) {
-  InSequence s;
-
-  // Then our subclass will be asked to initialize.
-  EXPECT_CALL(*renderer_, OnInitialize(_, _, _))
-      .WillOnce(Return(true));
-
-  // Initialize, we shouldn't have any reads.
-  renderer_->Initialize(decoder_, NewExpectedClosure(), NewUnderflowClosure());
-  EXPECT_EQ(0u, pending_reads_);
-
-  // Now seek to trigger prerolling, verifying the callback hasn't been
-  // executed yet.
-  EXPECT_CALL(*renderer_, CheckPoint(0));
-  renderer_->Seek(base::TimeDelta(), NewExpectedStatusCB(PIPELINE_OK));
-  EXPECT_EQ(kMaxQueueSize, pending_reads_);
-  renderer_->CheckPoint(0);
-
-  // Now satisfy the read requests.  Our callback should be executed after
-  // exiting this loop.
-  const uint32 kDataSize = 1024;
-  uint32 bytes_buffered = 0;
-
-  WriteUntilNoPendingReads(kDataSize, 1, &bytes_buffered);
-
-  // Then set the renderer to play state.
-  renderer_->Play(NewExpectedClosure());
-  renderer_->SetPlaybackRate(1.0f);
-  EXPECT_EQ(1.0f, renderer_->GetPlaybackRate());
-
-  // Then flush the data in the renderer by reading from it.
-  uint8 buffer[kDataSize];
-  for (size_t i = 0; i < kMaxQueueSize; ++i) {
-    EXPECT_EQ(kDataSize,
-              renderer_->FillBuffer(buffer, kDataSize,
-                                    base::TimeDelta(), true));
-    bytes_buffered -= kDataSize;
-  }
+TEST_F(AudioRendererBaseTest, Preroll) {
+  Initialize();
+  Preroll();
+}
 
-  // Make sure the read request queue is full.
-  EXPECT_EQ(kMaxQueueSize, pending_reads_);
+TEST_F(AudioRendererBaseTest, Play) {
+  Initialize();
+  Preroll();
+  Play();
 
-  // Fulfill the read with an end-of-stream packet.
-  scoped_refptr<DataBuffer> last_buffer(new DataBuffer(0));
-  decoder_->ConsumeAudioSamplesForTest(last_buffer);
-  --pending_reads_;
+  // Drain internal buffer, we should have a pending read.
+  EXPECT_CALL(*decoder_, Read(_));
+  EXPECT_TRUE(ConsumeBufferedData(bytes_buffered(), NULL));
+}
 
-  // We shouldn't report ended until all data has been flushed out.
-  EXPECT_FALSE(renderer_->HasEnded());
+TEST_F(AudioRendererBaseTest, EndOfStream) {
+  Initialize();
+  Preroll();
+  Play();
 
-  // We should have one less read request in the queue.
-  EXPECT_EQ(kMaxQueueSize - 1, pending_reads_);
-
-  // Flush the entire internal buffer and verify NotifyEnded() isn't called
-  // right away.
-  EXPECT_CALL(*renderer_, CheckPoint(1));
-  EXPECT_EQ(0u, bytes_buffered % kDataSize);
-  while (bytes_buffered > 0) {
-    EXPECT_EQ(kDataSize,
-              renderer_->FillBuffer(buffer, kDataSize,
-                                    base::TimeDelta(), true));
-    bytes_buffered -= kDataSize;
-  }
+  // Drain internal buffer, we should have a pending read.
+  EXPECT_CALL(*decoder_, Read(_));
+  EXPECT_TRUE(ConsumeBufferedData(bytes_buffered(), NULL));
 
-  // Although we've emptied the buffer, we don't consider ourselves ended until
-  // we request another buffer.  This way we know the last of the audio has
-  // played.
+  // Fulfill the read with an end-of-stream packet, we shouldn't report ended
+  // nor have a read until we drain the internal buffer.
+  DeliverEndOfStream();
   EXPECT_FALSE(renderer_->HasEnded());
-  renderer_->CheckPoint(1);
 
-  // Do an additional read to trigger NotifyEnded().
+  // Drain internal buffer, now we should report ended.
   EXPECT_CALL(host_, NotifyEnded());
-  EXPECT_EQ(0u, renderer_->FillBuffer(buffer, kDataSize,
-                                      base::TimeDelta(), true));
-
-  // We should now report ended.
+  EXPECT_TRUE(ConsumeBufferedData(bytes_buffered(), NULL));
   EXPECT_TRUE(renderer_->HasEnded());
-
-  // Further reads should return muted audio and not notify any more.
-  EXPECT_EQ(0u, renderer_->FillBuffer(buffer, kDataSize,
-                                      base::TimeDelta(), true));
 }
 
 TEST_F(AudioRendererBaseTest, Underflow) {
-  InSequence s;
-
-  base::TimeDelta playback_delay(base::TimeDelta::FromSeconds(1));
-
-  // Then our subclass will be asked to initialize.
-  EXPECT_CALL(*renderer_, OnInitialize(_, _, _))
-      .WillOnce(Return(true));
-
-  // Initialize, we shouldn't have any reads.
-  renderer_->Initialize(decoder_, NewExpectedClosure(), NewUnderflowClosure());
-  EXPECT_EQ(0u, pending_reads_);
-
-  // Now seek to trigger prerolling, verifying the callback hasn't been
-  // executed yet.
-  EXPECT_CALL(*renderer_, CheckPoint(0));
-  renderer_->Seek(base::TimeDelta(), NewExpectedStatusCB(PIPELINE_OK));
-  EXPECT_EQ(kMaxQueueSize, pending_reads_);
-  renderer_->CheckPoint(0);
-
-  // Now satisfy the read requests.  Our callback should be executed after
-  // exiting this loop.
-  const uint32 kDataSize = 1024;
-  uint32 bytes_buffered = 0;
-
-  WriteUntilNoPendingReads(kDataSize, 1, &bytes_buffered);
+  Initialize();
+  Preroll();
+  Play();
 
-  uint32 bytes_for_preroll = bytes_buffered;
+  // Drain internal buffer, we should have a pending read.
+  EXPECT_CALL(*decoder_, Read(_));
+  EXPECT_TRUE(ConsumeBufferedData(bytes_buffered(), NULL));
 
-  // Then set the renderer to play state.
-  renderer_->Play(NewExpectedClosure());
-  renderer_->SetPlaybackRate(1.0f);
-  EXPECT_EQ(1.0f, renderer_->GetPlaybackRate());
-
-  // Consume all of the data passed into the renderer.
-  ConsumeBufferedData(kDataSize, 1, &bytes_buffered);
-
-  // Make sure there are read requests pending.
-  EXPECT_GT(pending_reads_, 0u);
-
-  // Verify the next FillBuffer() call triggers calls the underflow callback
-  // since the queue is empty.
-  ExpectUnderflow(kDataSize, 1);
-
-  // Verify that zeroed out buffers are being returned during the underflow.
-  uint32 zeros_to_read = 5 * kDataSize;
-  ConsumeBufferedData(kDataSize, 0, &zeros_to_read);
+  // Verify the next FillBuffer() call triggers the underflow callback
+  // since the decoder hasn't delivered any data after it was drained.
+  const size_t kDataSize = 1024;
+  EXPECT_CALL(*this, OnUnderflow());
+  EXPECT_FALSE(ConsumeBufferedData(kDataSize, NULL));
 
   renderer_->ResumeAfterUnderflow(false);
 
-  // Verify we are still getting zeroed out buffers since no new data has been
-  // pushed to the renderer.
-  zeros_to_read = 5 * kDataSize;
-  ConsumeBufferedData(kDataSize, 0, &zeros_to_read);
-
-  // Satisfy all pending read requests.
-  WriteUntilNoPendingReads(kDataSize, 2, &bytes_buffered);
-
-  EXPECT_GE(bytes_buffered, bytes_for_preroll);
-
-  // Verify that we are now getting the new data.
-  ConsumeBufferedData(kDataSize, 2, &bytes_buffered);
+  // Verify after resuming that we're still not getting data.
+  //
+  // NOTE: FillBuffer() satisfies the read but returns muted audio, which
+  // is crazy http://crbug.com/106600
+  bool muted = false;
+  EXPECT_EQ(0u, bytes_buffered());
+  EXPECT_TRUE(ConsumeBufferedData(kDataSize, &muted));
+  EXPECT_TRUE(muted);
+
+  // Deliver data, we should get non-muted audio.
+  DeliverRemainingAudio();
+  EXPECT_CALL(*decoder_, Read(_));
+  EXPECT_TRUE(ConsumeBufferedData(kDataSize, &muted));
+  EXPECT_FALSE(muted);
 }
 
-
 TEST_F(AudioRendererBaseTest, Underflow_EndOfStream) {
-  InSequence s;
-
-  base::TimeDelta playback_delay(base::TimeDelta::FromSeconds(1));
-
-  // Then our subclass will be asked to initialize.
-  EXPECT_CALL(*renderer_, OnInitialize(_, _, _))
-      .WillOnce(Return(true));
-
-  // Initialize, we shouldn't have any reads.
-  renderer_->Initialize(decoder_, NewExpectedClosure(), NewUnderflowClosure());
-  EXPECT_EQ(0u, pending_reads_);
-
-  // Now seek to trigger prerolling, verifying the callback hasn't been
-  // executed yet.
-  EXPECT_CALL(*renderer_, CheckPoint(0));
-  renderer_->Seek(base::TimeDelta(), NewExpectedStatusCB(PIPELINE_OK));
-  EXPECT_EQ(kMaxQueueSize, pending_reads_);
-  renderer_->CheckPoint(0);
-
-  // Now satisfy the read requests.  Our callback should be executed after
-  // exiting this loop.
-  const uint32 kDataSize = 1024;
-  uint32 bytes_buffered = 0;
-
-  WriteUntilNoPendingReads(kDataSize, 1, &bytes_buffered);
-
-  // Then set the renderer to play state.
-  renderer_->Play(NewExpectedClosure());
-  renderer_->SetPlaybackRate(1.0f);
-  EXPECT_EQ(1.0f, renderer_->GetPlaybackRate());
-
-  // Consume all of the data passed into the renderer.
-  ConsumeBufferedData(kDataSize, 1, &bytes_buffered);
-
-  // Make sure there are read requests pending.
-  EXPECT_GT(pending_reads_, 0u);
-
-  // Verify the next FillBuffer() call triggers calls the underflow callback
-  // since the queue is empty.
-  ExpectUnderflow(kDataSize, 1);
-
-  DCHECK_GE(pending_reads_, 2u);
-
-  // Write a buffer.
-  bytes_buffered += kDataSize;
-  --pending_reads_;
-  decoder_->ConsumeAudioSamplesForTest(CreateBuffer(kDataSize, 3));
-
-  // Verify we are getting zeroed out buffers since we haven't pushed enough
-  // data to satisfy preroll.
-  uint32 zeros_to_read = 5 * kDataSize;
-  ConsumeBufferedData(kDataSize, 0, &zeros_to_read);
-
-  // Send end of stream buffers for all pending reads.
-  while (pending_reads_ > 0) {
-    scoped_refptr<DataBuffer> buffer(new DataBuffer(0));
-    --pending_reads_;
-    decoder_->ConsumeAudioSamplesForTest(buffer);
-  }
-
-  // Verify that we are now getting the new data.
-  ConsumeBufferedData(kDataSize, 3, &bytes_buffered);
+  Initialize();
+  Preroll();
+  Play();
+
+  // Drain internal buffer, we should have a pending read.
+  EXPECT_CALL(*decoder_, Read(_));
+  EXPECT_TRUE(ConsumeBufferedData(bytes_buffered(), NULL));
+
+  // Verify the next FillBuffer() call triggers the underflow callback
+  // since the decoder hasn't delivered any data after it was drained.
+  const size_t kDataSize = 1024;
+  EXPECT_CALL(*this, OnUnderflow());
+  EXPECT_FALSE(ConsumeBufferedData(kDataSize, NULL));
+
+  // Deliver a little bit of data.
+  EXPECT_CALL(*decoder_, Read(_));
+  FulfillPendingRead(kDataSize);
+
+  // Verify we're getting muted audio during underflow.
+  //
+  // NOTE: FillBuffer() satisfies the read but returns muted audio, which
+  // is crazy http://crbug.com/106600
+  bool muted = false;
+  EXPECT_EQ(kDataSize, bytes_buffered());
+  EXPECT_TRUE(ConsumeBufferedData(kDataSize, &muted));
+  EXPECT_TRUE(muted);
+
+  // Now deliver end of stream, we should get our little bit of data back.
+  DeliverEndOfStream();
+  EXPECT_CALL(*decoder_, Read(_));
+  EXPECT_EQ(kDataSize, bytes_buffered());
+  EXPECT_TRUE(ConsumeBufferedData(kDataSize, &muted));
+  EXPECT_FALSE(muted);
+
+  // Deliver another end of stream buffer and attempt to read to make sure
+  // we're truly at the end of stream.
+  //
+  // TODO(scherkus): fix AudioRendererBase and AudioRendererAlgorithmBase to
+  // stop reading after receiving an end of stream buffer. It should have also
+  // called NotifyEnded() http://crbug.com/106641
+  DeliverEndOfStream();
+  EXPECT_CALL(host_, NotifyEnded());
+  EXPECT_FALSE(ConsumeBufferedData(kDataSize, &muted));
+  EXPECT_FALSE(muted);
 }
 
 }  // namespace media
diff --git a/media/filters/ffmpeg_audio_decoder.cc b/media/filters/ffmpeg_audio_decoder.cc
index 3a0ab34..61aa02b 100644
--- a/media/filters/ffmpeg_audio_decoder.cc
+++ b/media/filters/ffmpeg_audio_decoder.cc
@@ -52,8 +52,7 @@ FFmpegAudioDecoder::FFmpegAudioDecoder(MessageLoop* message_loop)
       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() {
@@ -87,11 +86,11 @@ void FFmpegAudioDecoder::Initialize(
                  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) {
+  // Complete operation asynchronously on different stack of execution as per
+  // the API contract of AudioDecoder::Read()
+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &FFmpegAudioDecoder::DoRead, this, callback));
 }
 
 int FFmpegAudioDecoder::bits_per_channel() {
@@ -156,16 +155,18 @@ void FFmpegAudioDecoder::DoFlush(const base::Closure& callback) {
   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_);
+  DCHECK(!callback.is_null());
+  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
@@ -233,27 +234,23 @@ void FFmpegAudioDecoder::DoDecodeBuffer(const scoped_refptr<Buffer>& input) {
   // 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);
   } 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): 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, buffer));
 }
 
 void FFmpegAudioDecoder::UpdateDurationAndTimestamp(
@@ -286,4 +283,11 @@ base::TimeDelta FFmpegAudioDecoder::CalculateDuration(int size) {
   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
diff --git a/media/filters/ffmpeg_audio_decoder.h b/media/filters/ffmpeg_audio_decoder.h
index 3f33abf..2ee96ff 100644
--- a/media/filters/ffmpeg_audio_decoder.h
+++ b/media/filters/ffmpeg_audio_decoder.h
@@ -27,7 +27,7 @@ class MEDIA_EXPORT FFmpegAudioDecoder : public AudioDecoder {
   // AudioDecoder implementation.
   virtual void Initialize(DemuxerStream* stream, const base::Closure& callback,
                           const StatisticsCallback& stats_callback) OVERRIDE;
-  virtual void ProduceAudioSamples(scoped_refptr<Buffer> output) OVERRIDE;
+  virtual void Read(const ReadCB& callback) OVERRIDE;
   virtual int bits_per_channel() OVERRIDE;
   virtual ChannelLayout channel_layout() OVERRIDE;
   virtual int samples_per_second() OVERRIDE;
@@ -38,7 +38,7 @@ class MEDIA_EXPORT FFmpegAudioDecoder : public AudioDecoder {
                     const base::Closure& callback,
                     const StatisticsCallback& stats_callback);
   void DoFlush(const base::Closure& callback);
-  void DoProduceAudioSamples(const scoped_refptr<Buffer>& output);
+  void DoRead(const ReadCB& callback);
   void DoDecodeBuffer(const scoped_refptr<Buffer>& input);
 
   // Reads from the demuxer stream with corresponding callback method.
@@ -53,6 +53,9 @@ class MEDIA_EXPORT FFmpegAudioDecoder : public AudioDecoder {
   // Calculates duration based on size of decoded audio bytes.
   base::TimeDelta CalculateDuration(int size);
 
+  // Delivers decoded samples to |read_cb_| and resets the callback.
+  void DeliverSamples(const scoped_refptr<Buffer>& samples);
+
   MessageLoop* message_loop_;
 
   scoped_refptr<DemuxerStream> demuxer_stream_;
@@ -72,11 +75,7 @@ class MEDIA_EXPORT FFmpegAudioDecoder : public AudioDecoder {
   const int decoded_audio_size_;
   uint8* decoded_audio_;  // Allocated via av_malloc().
 
-  // Holds downstream-provided buffers.
-  std::list<scoped_refptr<Buffer> > output_buffers_;
-
-  // Tracks reads issued for compressed data.
-  int pending_reads_;
+  ReadCB read_cb_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(FFmpegAudioDecoder);
 };
diff --git a/media/filters/ffmpeg_audio_decoder_unittest.cc b/media/filters/ffmpeg_audio_decoder_unittest.cc
index 0e67c63..f8ba102 100644
--- a/media/filters/ffmpeg_audio_decoder_unittest.cc
+++ b/media/filters/ffmpeg_audio_decoder_unittest.cc
@@ -54,10 +54,6 @@ class FFmpegAudioDecoderTest : public testing::Test {
     // Push in an EOS buffer.
     encoded_audio_.push_back(new DataBuffer(0));
 
-    decoder_->set_consume_audio_samples_callback(
-        base::Bind(&FFmpegAudioDecoderTest::DecodeFinished,
-                   base::Unretained(this)));
-
     config_.Initialize(kCodecVorbis,
                        16,
                        CHANNEL_LAYOUT_STEREO,
@@ -93,6 +89,12 @@ class FFmpegAudioDecoderTest : public testing::Test {
     read_callback.Run(buffer);
   }
 
+  void Read() {
+    decoder_->Read(base::Bind(
+        &FFmpegAudioDecoderTest::DecodeFinished, base::Unretained(this)));
+    message_loop_.RunAllPending();
+  }
+
   void DecodeFinished(scoped_refptr<Buffer> buffer) {
     decoded_audio_.push_back(buffer);
   }
@@ -157,12 +159,9 @@ TEST_F(FFmpegAudioDecoderTest, ProduceAudioSamples) {
   EXPECT_CALL(statistics_callback_, OnStatistics(_))
       .Times(5);
 
-  // We have to use a buffer to trigger a read. Silly.
-  scoped_refptr<DataBuffer> buffer(0);
-  decoder_->ProduceAudioSamples(buffer);
-  decoder_->ProduceAudioSamples(buffer);
-  decoder_->ProduceAudioSamples(buffer);
-  message_loop_.RunAllPending();
+  Read();
+  Read();
+  Read();
 
   // We should have three decoded audio buffers.
   //
@@ -173,10 +172,7 @@ TEST_F(FFmpegAudioDecoderTest, ProduceAudioSamples) {
   ExpectDecodedAudio(2, 2902, 23219);
 
   // Call one more time to trigger EOS.
-  //
-  // TODO(scherkus): EOS should flush data, not overwrite timestamps with zero.
-  decoder_->ProduceAudioSamples(buffer);
-  message_loop_.RunAllPending();
+  Read();
   ASSERT_EQ(4u, decoded_audio_.size());
   ExpectEndOfStream(3);
 
diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc
index 66b25b6..c1fa54d 100644
--- a/media/filters/ffmpeg_video_decoder.cc
+++ b/media/filters/ffmpeg_video_decoder.cc
@@ -171,8 +171,8 @@ void FFmpegVideoDecoder::Flush(const base::Closure& callback) {
 }
 
 void FFmpegVideoDecoder::Read(const ReadCB& callback) {
-  // TODO(scherkus): forced task post since VideoRendererBase::FrameReady() will
-  // call Read() on FFmpegVideoDecoder's thread as we executed |read_cb_|.
+  // Complete operation asynchronously on different stack of execution as per
+  // the API contract of VideoDecoder::Read()
   message_loop_->PostTask(FROM_HERE, base::Bind(
       &FFmpegVideoDecoder::DoRead, this, callback));
 }
@@ -183,7 +183,7 @@ const gfx::Size& FFmpegVideoDecoder::natural_size() {
 
 void FFmpegVideoDecoder::DoRead(const ReadCB& callback) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-  CHECK(!callback.is_null());
+  DCHECK(!callback.is_null());
   CHECK(read_cb_.is_null()) << "Overlapping decodes are not supported.";
 
   // This can happen during shutdown after Stop() has been called.
@@ -211,8 +211,8 @@ void FFmpegVideoDecoder::ReadFromDemuxerStream() {
 }
 
 void FFmpegVideoDecoder::DecodeBuffer(const scoped_refptr<Buffer>& buffer) {
-  // TODO(scherkus): forced task post since FFmpegDemuxerStream::Read() can
-  // immediately execute our callback on FFmpegVideoDecoder's thread.
+  // 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(
       &FFmpegVideoDecoder::DoDecodeBuffer, this, buffer));
 }