Fix audio sink not resuming when an underflow is resolved while paused.

The AudioRendererSink was not having its Play() method called if an underflow
occurred, then the presentation was paused, the underflow was resolved, and
then the presentation was unpaused. This patch fixes that scenario.

BUG=291735
TEST=AudioRendererImplTest.Underflow_SetPlaybackRate

Review URL: https://codereview.chromium.org/27553002

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

5 files changed, 218 insertions, 24 deletions

diff --git a/media/base/fake_audio_renderer_sink.cc b/media/base/fake_audio_renderer_sink.cc
new file mode 100644
index 0000000..d42db6d
--- /dev/null
+++ b/media/base/fake_audio_renderer_sink.cc
@@ -0,0 +1,86 @@
+// Copyright 2013 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/base/fake_audio_renderer_sink.h"
+
+#include "base/logging.h"
+
+namespace media {
+
+FakeAudioRendererSink::FakeAudioRendererSink()
+    : state_(kUninitialized),
+      callback_(NULL) {
+}
+
+FakeAudioRendererSink::~FakeAudioRendererSink() {
+  DCHECK(!callback_);
+}
+
+void FakeAudioRendererSink::Initialize(const AudioParameters& params,
+                                       RenderCallback* callback) {
+  DCHECK_EQ(state_, kUninitialized);
+  DCHECK(!callback_);
+  DCHECK(callback);
+
+  callback_ = callback;
+  ChangeState(kInitialized);
+}
+
+void FakeAudioRendererSink::Start() {
+  DCHECK_EQ(state_, kInitialized);
+  ChangeState(kStarted);
+}
+
+void FakeAudioRendererSink::Stop() {
+  callback_ = NULL;
+  ChangeState(kStopped);
+}
+
+void FakeAudioRendererSink::Pause() {
+  DCHECK(state_ == kStarted || state_ == kPlaying) << "state_ " << state_;
+  ChangeState(kPaused);
+}
+
+void FakeAudioRendererSink::Play() {
+  DCHECK(state_ == kStarted || state_ == kPaused) << "state_ " << state_;
+  DCHECK_EQ(state_, kPaused);
+  ChangeState(kPlaying);
+}
+
+bool FakeAudioRendererSink::SetVolume(double volume) {
+  return true;
+}
+
+bool FakeAudioRendererSink::Render(AudioBus* dest, int audio_delay_milliseconds,
+                                   int* frames_written) {
+  if (state_ != kPlaying)
+    return false;
+
+  *frames_written = callback_->Render(dest, audio_delay_milliseconds);
+  return true;
+}
+
+void FakeAudioRendererSink::OnRenderError() {
+  DCHECK_NE(state_, kUninitialized);
+  DCHECK_NE(state_, kStopped);
+
+  callback_->OnRenderError();
+}
+
+void FakeAudioRendererSink::ChangeState(State new_state) {
+  static const char* kStateNames[] = {
+    "kUninitialized",
+    "kInitialized",
+    "kStarted",
+    "kPaused",
+    "kPlaying",
+    "kStopped"
+  };
+
+  DVLOG(1) << __FUNCTION__ << " : "
+           << kStateNames[state_] << " -> " << kStateNames[new_state];
+  state_ = new_state;
+}
+
+}  // namespace media
diff --git a/media/base/fake_audio_renderer_sink.h b/media/base/fake_audio_renderer_sink.h
new file mode 100644
index 0000000..b548224
--- /dev/null
+++ b/media/base/fake_audio_renderer_sink.h
@@ -0,0 +1,61 @@
+// Copyright 2013 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.
+
+#ifndef MEDIA_BASE_FAKE_AUDIO_RENDERER_SINK_H_
+#define MEDIA_BASE_FAKE_AUDIO_RENDERER_SINK_H_
+
+#include "media/audio/audio_parameters.h"
+#include "media/base/audio_renderer_sink.h"
+
+namespace media {
+
+class FakeAudioRendererSink : public AudioRendererSink {
+ public:
+  enum State {
+    kUninitialized,
+    kInitialized,
+    kStarted,
+    kPaused,
+    kPlaying,
+    kStopped
+  };
+
+  FakeAudioRendererSink();
+
+  virtual void Initialize(const AudioParameters& params,
+                          RenderCallback* callback) OVERRIDE;
+  virtual void Start() OVERRIDE;
+  virtual void Stop() OVERRIDE;
+  virtual void Pause() OVERRIDE;
+  virtual void Play() OVERRIDE;
+  virtual bool SetVolume(double volume) OVERRIDE;
+
+  // Attempts to call Render() on the callback provided to
+  // Initialize() with |dest| and |audio_delay_milliseconds|.
+  // Returns true and sets |frames_written| to the return value of the
+  // Render() call.
+  // Returns false if this object is in a state where calling Render()
+  // should not occur. (i.e., in the kPaused or kStopped state.) The
+  // value of |frames_written| is undefined if false is returned.
+  bool Render(AudioBus* dest, int audio_delay_milliseconds,
+              int* frames_written);
+  void OnRenderError();
+
+  State state() const { return state_; }
+
+ protected:
+  virtual ~FakeAudioRendererSink();
+
+ private:
+  void ChangeState(State new_state);
+
+  State state_;
+  RenderCallback* callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeAudioRendererSink);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_FAKE_AUDIO_RENDERER_SINK_H_
diff --git a/media/filters/audio_renderer_impl.cc b/media/filters/audio_renderer_impl.cc
index 82047ef..a4b0370 100644
--- a/media/filters/audio_renderer_impl.cc
+++ b/media/filters/audio_renderer_impl.cc
@@ -317,6 +317,7 @@ void AudioRendererImpl::SetVolume(float volume) {
 void AudioRendererImpl::DecodedAudioReady(
     AudioDecoder::Status status,
     const scoped_refptr<AudioBuffer>& buffer) {
+  DVLOG(1) << __FUNCTION__ << "(" << status << ")";
   DCHECK(message_loop_->BelongsToCurrentThread());
 
   base::AutoLock auto_lock(lock_);
@@ -368,36 +369,41 @@ bool AudioRendererImpl::HandleSplicerBuffer(
     // no more data will be arriving.
     if (state_ == kUnderflow || state_ == kRebuffering)
       ChangeState_Locked(kPlaying);
+  } else {
+    if (state_ == kPrerolling && IsBeforePrerollTime(buffer))
+      return true;
+
+    if (state_ != kUninitialized && state_ != kStopped)
+      algorithm_->EnqueueBuffer(buffer);
   }
 
   switch (state_) {
     case kUninitialized:
       NOTREACHED();
       return false;
+
     case kPaused:
-      if (!buffer->end_of_stream())
-        algorithm_->EnqueueBuffer(buffer);
       DCHECK(!pending_read_);
       base::ResetAndReturn(&pause_cb_).Run();
       return false;
+
     case kPrerolling:
-      if (IsBeforePrerollTime(buffer))
+      if (!buffer->end_of_stream() && !algorithm_->IsQueueFull())
         return true;
-
-      if (!buffer->end_of_stream()) {
-        algorithm_->EnqueueBuffer(buffer);
-        if (!algorithm_->IsQueueFull())
-          return false;
-      }
       ChangeState_Locked(kPaused);
       base::ResetAndReturn(&preroll_cb_).Run(PIPELINE_OK);
       return false;
+
     case kPlaying:
     case kUnderflow:
+      return false;
+
     case kRebuffering:
-      if (!buffer->end_of_stream())
-        algorithm_->EnqueueBuffer(buffer);
+      if (!algorithm_->IsQueueFull())
+        return true;
+      ChangeState_Locked(kPlaying);
       return false;
+
     case kStopped:
       return false;
   }
@@ -441,6 +447,7 @@ bool AudioRendererImpl::CanRead_Locked() {
 }
 
 void AudioRendererImpl::SetPlaybackRate(float playback_rate) {
+  DVLOG(1) << __FUNCTION__ << "(" << playback_rate << ")";
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_GE(playback_rate, 0);
   DCHECK(sink_);
@@ -461,7 +468,8 @@ void AudioRendererImpl::SetPlaybackRate(float playback_rate) {
 
 bool AudioRendererImpl::IsBeforePrerollTime(
     const scoped_refptr<AudioBuffer>& buffer) {
-  return (state_ == kPrerolling) && buffer.get() && !buffer->end_of_stream() &&
+  DCHECK_EQ(state_, kPrerolling);
+  return buffer && !buffer->end_of_stream() &&
          (buffer->timestamp() + buffer->duration()) < preroll_timestamp_;
 }
 
@@ -486,9 +494,6 @@ int AudioRendererImpl::Render(AudioBus* audio_bus,
     if (playback_rate == 0)
       return 0;
 
-    if (state_ == kRebuffering && algorithm_->IsQueueFull())
-      ChangeState_Locked(kPlaying);
-
     // Mute audio by returning 0 when not playing.
     if (state_ != kPlaying)
       return 0;
diff --git a/media/filters/audio_renderer_impl_unittest.cc b/media/filters/audio_renderer_impl_unittest.cc
index e52210a..b9cea30 100644
--- a/media/filters/audio_renderer_impl_unittest.cc
+++ b/media/filters/audio_renderer_impl_unittest.cc
@@ -11,8 +11,8 @@
 #include "base/strings/stringprintf.h"
 #include "media/base/audio_buffer.h"
 #include "media/base/audio_timestamp_helper.h"
+#include "media/base/fake_audio_renderer_sink.h"
 #include "media/base/gmock_callback_support.h"
-#include "media/base/mock_audio_renderer_sink.h"
 #include "media/base/mock_filters.h"
 #include "media/base/test_helpers.h"
 #include "media/filters/audio_renderer_impl.h"
@@ -25,8 +25,6 @@ using ::testing::_;
 using ::testing::AnyNumber;
 using ::testing::Invoke;
 using ::testing::Return;
-using ::testing::NiceMock;
-using ::testing::StrictMock;
 
 namespace media {
 
@@ -71,10 +69,10 @@ class AudioRendererImplTest : public ::testing::Test {
 
     ScopedVector<AudioDecoder> decoders;
     decoders.push_back(decoder_);
-
+    sink_ = new FakeAudioRendererSink();
     renderer_.reset(new AudioRendererImpl(
         message_loop_.message_loop_proxy(),
-        new NiceMock<MockAudioRendererSink>(),
+        sink_,
         decoders.Pass(),
         SetDecryptorReadyCB(),
         false));
@@ -119,7 +117,6 @@ class AudioRendererImplTest : public ::testing::Test {
   void Initialize() {
     EXPECT_CALL(*decoder_, Initialize(_, _, _))
         .WillOnce(RunCallback<1>(PIPELINE_OK));
-
     InitializeWithStatus(PIPELINE_OK);
 
     next_timestamp_.reset(
@@ -236,10 +233,15 @@ class AudioRendererImplTest : public ::testing::Test {
   //
   // |muted| is optional and if passed will get set if the value of
   // the consumed data is muted audio.
-  bool ConsumeBufferedData(uint32 requested_frames, bool* muted) {
+  bool ConsumeBufferedData(int requested_frames, bool* muted) {
     scoped_ptr<AudioBus> bus =
-        AudioBus::Create(kChannels, std::max(requested_frames, 1u));
-    uint32 frames_read = renderer_->Render(bus.get(), 0);
+        AudioBus::Create(kChannels, std::max(requested_frames, 1));
+    int frames_read;
+    if (!sink_->Render(bus.get(), 0, &frames_read)) {
+      if (muted)
+        *muted = true;
+      return false;
+    }
 
     if (muted)
       *muted = frames_read < 1 || bus->channel(0)[0] == kMutedAudio;
@@ -341,6 +343,7 @@ class AudioRendererImplTest : public ::testing::Test {
   // Fixture members.
   base::MessageLoop message_loop_;
   scoped_ptr<AudioRendererImpl> renderer_;
+  scoped_refptr<FakeAudioRendererSink> sink_;
 
  private:
   TimeTicks GetTime() {
@@ -532,6 +535,43 @@ TEST_F(AudioRendererImplTest, Underflow_ResumeFromCallback) {
   EXPECT_FALSE(muted);
 }
 
+TEST_F(AudioRendererImplTest, Underflow_SetPlaybackRate) {
+  Initialize();
+  Preroll();
+  Play();
+
+  // Drain internal buffer, we should have a pending read.
+  EXPECT_TRUE(ConsumeBufferedData(frames_buffered(), NULL));
+  WaitForPendingRead();
+
+  EXPECT_EQ(FakeAudioRendererSink::kPlaying, sink_->state());
+
+  // 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())
+      .WillOnce(Invoke(this, &AudioRendererImplTest::CallResumeAfterUnderflow));
+  EXPECT_FALSE(ConsumeBufferedData(kDataSize, NULL));
+  EXPECT_EQ(0u, frames_buffered());
+
+  EXPECT_EQ(FakeAudioRendererSink::kPlaying, sink_->state());
+
+  // Simulate playback being paused.
+  renderer_->SetPlaybackRate(0);
+
+  EXPECT_EQ(FakeAudioRendererSink::kPaused, sink_->state());
+
+  // Deliver data to resolve the underflow.
+  DeliverRemainingAudio();
+
+  EXPECT_EQ(FakeAudioRendererSink::kPaused, sink_->state());
+
+  // Simulate playback being resumed.
+  renderer_->SetPlaybackRate(1);
+
+  EXPECT_EQ(FakeAudioRendererSink::kPlaying, sink_->state());
+}
+
 TEST_F(AudioRendererImplTest, AbortPendingRead_Preroll) {
   Initialize();
 
diff --git a/media/media.gyp b/media/media.gyp
index 686163c..dfcc858 100644
--- a/media/media.gyp
+++ b/media/media.gyp
@@ -1081,6 +1081,8 @@
         'audio/test_audio_input_controller_factory.h',
         'base/fake_audio_render_callback.cc',
         'base/fake_audio_render_callback.h',
+        'base/fake_audio_renderer_sink.cc',
+        'base/fake_audio_renderer_sink.h',
         'base/gmock_callback_support.h',
         'base/mock_audio_renderer_sink.cc',
         'base/mock_audio_renderer_sink.h',