Audio pause and seek better

BUG=39885

Pause for audio is now in effect in less than 200ms.

Seek for audio is also much better as we play also very short amount
after seek.

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

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

4 files changed, 94 insertions, 74 deletions

diff --git a/chrome/renderer/media/audio_renderer_impl.cc b/chrome/renderer/media/audio_renderer_impl.cc
index a1cae17..15a98a5 100644
--- a/chrome/renderer/media/audio_renderer_impl.cc
+++ b/chrome/renderer/media/audio_renderer_impl.cc
@@ -137,16 +137,36 @@ void AudioRendererImpl::SetPlaybackRate(float rate) {
   }
 }
 
+void AudioRendererImpl::Pause(media::FilterCallback* callback) {
+  AudioRendererBase::Pause(callback);
+  AutoLock auto_lock(lock_);
+  if (stopped_)
+    return;
+
+  io_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(this, &AudioRendererImpl::PauseTask));
+}
+
 void AudioRendererImpl::Seek(base::TimeDelta time,
                              media::FilterCallback* callback) {
   AudioRendererBase::Seek(time, callback);
+  AutoLock auto_lock(lock_);
+  if (stopped_)
+    return;
 
+  io_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(this, &AudioRendererImpl::SeekTask));
+}
+
+
+void AudioRendererImpl::Play(media::FilterCallback* callback) {
+  AudioRendererBase::Play(callback);
   AutoLock auto_lock(lock_);
   if (stopped_)
     return;
 
   io_loop_->PostTask(FROM_HERE,
-    NewRunnableMethod(this, &AudioRendererImpl::SeekTask));
+      NewRunnableMethod(this, &AudioRendererImpl::PlayTask));
 }
 
 void AudioRendererImpl::SetVolume(float volume) {
@@ -268,6 +288,8 @@ void AudioRendererImpl::PauseTask() {
 void AudioRendererImpl::SeekTask() {
   DCHECK(MessageLoop::current() == io_loop_);
 
+  // We have to pause the audio stream before we can flush.
+  filter_->Send(new ViewHostMsg_PauseAudioStream(0, stream_id_));
   filter_->Send(new ViewHostMsg_FlushAudioStream(0, stream_id_));
 }
 
diff --git a/chrome/renderer/media/audio_renderer_impl.h b/chrome/renderer/media/audio_renderer_impl.h
index a0e99d8..7c3aa58 100644
--- a/chrome/renderer/media/audio_renderer_impl.h
+++ b/chrome/renderer/media/audio_renderer_impl.h
@@ -33,60 +33,6 @@
 //    Properties of this filter is also set in this thread.
 // 3. Audio decoder thread (If there's one.)
 //    Responsible for decoding audio data and gives raw PCM data to this object.
-//
-// Methods categorized according to the thread(s) they are running on.
-//
-// Render thread
-// +-- CreateFactory()
-// |     Helper method for construction this class.
-// \-- IsMediaFormatSupported()
-//       Helper method to identify media formats accepted by this class for
-//       construction.
-//
-// IO thread (Main thread in render process)
-// +-- OnCreateStream()
-// |     Sends an IPC message to browser to create audio output stream and
-// |     register this object with AudioMessageFilter.
-// |-- OnSetVolume()
-// |     Sends an IPC message to browser to set volume.
-// |-- OnNotifyPacketReady
-// |     Try to fill the shared memory with decoded audio packet and sends IPC
-// |     messages to browser if packet is ready.
-// |-- OnRequestPacket()
-// |     Called from AudioMessageFilter when an audio packet requested is
-// |     received from browser process.
-// |-- OnStateChanged()
-// |     Called from AudioMessageFilter upon state change of the audio output
-// |     stream in the browser process. Error of the stream is reported here.
-// |-- OnCreated()
-// |     Called from AudioMessageFilter upon successful creation of audio output
-// |     stream in the browser process, called along with a SharedMemoryHandle.
-// |-- OnVolume()
-// |     Called from AudioMessageFilter about the volume of the audio output
-// |     stream.
-// \-- OnDestroy()
-//       Release resources that live inside io thread.
-//
-// Pipeline thread
-// +-- AudioRendererImpl()
-// |     Constructor method.
-// |-- ~AudioRendererImpl()
-// |     Destructor method.
-// |-- SetPlaybackRate()
-// |     Given the playback rate information.
-// |-- GetMediaFormat()
-// |     Obtain the current media format of this unit.
-// |-- SetVolume()
-// |     Given the volume information.
-// |-- OnInitialize()
-// |     Called from AudioRendererBase for initialization event.
-// \-- OnStop()
-//       Called from AudioRendererBase for stop event.
-//
-// Audio decoder thread (If there's one.)
-// \-- OnReadComplete()
-//       A raw PCM audio packet buffer is received here, this method is called
-//       from pipeline thread if audio decoder thread does not exist.
 
 #ifndef CHROME_RENDERER_MEDIA_AUDIO_RENDERER_IMPL_H_
 #define CHROME_RENDERER_MEDIA_AUDIO_RENDERER_IMPL_H_
@@ -130,7 +76,9 @@ class AudioRendererImpl : public media::AudioRendererBase,
   // Methods called on pipeline thread ----------------------------------------
   // media::MediaFilter implementation.
   virtual void SetPlaybackRate(float rate);
+  virtual void Pause(media::FilterCallback* callback);
   virtual void Seek(base::TimeDelta time, media::FilterCallback* callback);
+  virtual void Play(media::FilterCallback* callback);
 
   // media::AudioRenderer implementation.
   virtual void SetVolume(float volume);
diff --git a/media/audio/audio_output_controller.cc b/media/audio/audio_output_controller.cc
index 11ee96c..74e2261 100644
--- a/media/audio/audio_output_controller.cc
+++ b/media/audio/audio_output_controller.cc
@@ -212,9 +212,7 @@ void AudioOutputController::DoPlay() {
   }
 
   // We start the AudioOutputStream lazily.
-  if (old_state == kCreated) {
-    stream_->Start(this);
-  }
+  stream_->Start(this);
 
   // Tell the event handler that we are now playing.
   handler_->OnPlaying(this);
@@ -228,12 +226,16 @@ void AudioOutputController::DoPause() {
     return;
 
   // Sets the |state_| to kPaused so we don't draw more audio data.
-  // TODO(hclam): Actually pause the audio device.
   {
     AutoLock auto_lock(lock_);
     state_ = kPaused;
   }
 
+  // Then we stop the audio device. This is not the perfect solution because
+  // it discards all the internal buffer in the audio device.
+  // TODO(hclam): Actually pause the audio device.
+  stream_->Stop();
+
   handler_->OnPaused(this);
 }
 
diff --git a/media/audio/audio_output_controller_unittest.cc b/media/audio/audio_output_controller_unittest.cc
index 0783b09..7839ec1 100644
--- a/media/audio/audio_output_controller_unittest.cc
+++ b/media/audio/audio_output_controller_unittest.cc
@@ -88,10 +88,6 @@ TEST(AudioOutputControllerTest, CreateAndClose) {
 
   // Close the controller immediately.
   controller->Close();
-
-  // TODO(hclam): Make sure releasing the reference to this
-  // object actually destruct it.
-  controller = NULL;
 }
 
 TEST(AudioOutputControllerTest, PlayAndClose) {
@@ -133,10 +129,6 @@ TEST(AudioOutputControllerTest, PlayAndClose) {
   // Now stop the controller. This should shutdown the internal
   // thread and we hold the only reference to it.
   controller->Close();
-
-  // TODO(hclam): Make sure releasing the reference to this
-  // object actually destruct it.
-  controller = NULL;
 }
 
 TEST(AudioOutputControllerTest, PlayPauseClose) {
@@ -189,10 +181,70 @@ TEST(AudioOutputControllerTest, PlayPauseClose) {
   // Now stop the controller. This should shutdown the internal
   // thread and we hold the only reference to it.
   controller->Close();
+}
+
+TEST(AudioOutputControllerTest, PlayPausePlay) {
+  if (!HasAudioOutputDevices() || IsRunningHeadless())
+    return;
+
+  MockAudioOutputControllerEventHandler event_handler;
+  base::WaitableEvent event(false, false);
+  int count = 0;
+
+  // If OnCreated is called then signal the event.
+  EXPECT_CALL(event_handler, OnCreated(NotNull()))
+      .Times(Exactly(1))
+      .WillOnce(InvokeWithoutArgs(&event, &base::WaitableEvent::Signal));
+
+  // OnPlaying() will be called only once.
+  EXPECT_CALL(event_handler, OnPlaying(NotNull()))
+      .Times(Exactly(1))
+      .RetiresOnSaturation();
+
+  // If OnMoreData() is called enough then signal the event.
+  EXPECT_CALL(event_handler, OnMoreData(NotNull(), _, 0))
+      .Times(AtLeast(10))
+      .WillRepeatedly(SignalEvent(&event, &count, 10));
+
+  // And then OnPaused() will be called.
+  EXPECT_CALL(event_handler, OnPaused(NotNull()))
+      .Times(Exactly(1))
+      .WillOnce(InvokeWithoutArgs(&event, &base::WaitableEvent::Signal));
+
+  // OnPlaying() will be called only once.
+  EXPECT_CALL(event_handler, OnPlaying(NotNull()))
+    .Times(Exactly(1))
+    .RetiresOnSaturation();
+
+  scoped_refptr<AudioOutputController> controller =
+      AudioOutputController::Create(&event_handler,
+                                    AudioManager::AUDIO_PCM_LINEAR, kChannels,
+                                    kSampleRate, kBitsPerSample,
+                                    kHardwareBufferSize, kBufferCapacity);
+  ASSERT_TRUE(controller.get());
+
+  // Wait for OnCreated() to be called.
+  event.Wait();
+  event.Reset();
+
+  // Play and then wait for the event to be signaled.
+  controller->Play();
+  event.Wait();
+  event.Reset();
+
+  // And then wait for pause to complete.
+  controller->Pause();
+  event.Wait();
+  event.Reset();
 
-  // TODO(hclam): Make sure releasing the reference to this
-  // object actually destruct it.
-  controller = NULL;
+  // Then we play again.
+  // Play and then wait for the event to be signaled.
+  controller->Play();
+  event.Wait();
+
+  // Now stop the controller. This should shutdown the internal
+  // thread and we hold the only reference to it.
+  controller->Close();
 }
 
 TEST(AudioOutputControllerTest, HardwareBufferTooLarge) {
@@ -228,10 +280,6 @@ TEST(AudioOutputControllerTest, CloseTwice) {
   // Close the controller immediately.
   controller->Close();
   controller->Close();
-
-  // TODO(hclam): Make sure releasing the reference to this
-  // object actually destruct it.
-  controller = NULL;
 }
 
 }  // namespace media