From 720fa5ccf7f70bb768701c8f76d2079a38c897d9 Mon Sep 17 00:00:00 2001 From: "dalecurtis@chromium.org" Date: Wed, 9 May 2012 22:58:48 +0000 Subject: Fix dropped Play event when there's a pending pause. If the message queue is built in such a way that DoPlay()->DoPause()->DoPlay() ends up executing before the the first DoPlay()'s PollAndStartIfDataReady() runs, we'll end up dropping the play and keeping the pause. Fixed by ensuring we drop the pause if we receive a play before the pause is consumed by PollAndStartIfDataReady(). BUG=111272 TEST=unittests, test.html test case in bug. Manual seeking in buffered areas. Review URL: https://chromiumcodereview.appspot.com/10386037 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@136162 0039d316-1c4b-4281-b951-d872f2087c98 --- media/audio/audio_output_controller.cc | 7 +++- media/audio/audio_output_controller_unittest.cc | 50 ++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-) (limited to 'media') diff --git a/media/audio/audio_output_controller.cc b/media/audio/audio_output_controller.cc index cdaca79..da90f10 100644 --- a/media/audio/audio_output_controller.cc +++ b/media/audio/audio_output_controller.cc @@ -148,8 +148,13 @@ void AudioOutputController::DoPlay() { DCHECK(message_loop_->BelongsToCurrentThread()); // We can start from created or paused state. - if (state_ != kCreated && state_ != kPaused) + if (state_ != kCreated && state_ != kPaused) { + // If a pause is pending drop it. Otherwise the controller might hang since + // the corresponding play event has already occurred. + if (state_ == kPausedWhenStarting) + state_ = kStarting; return; + } state_ = kStarting; diff --git a/media/audio/audio_output_controller_unittest.cc b/media/audio/audio_output_controller_unittest.cc index 05f3d7f..f40a9ae 100644 --- a/media/audio/audio_output_controller_unittest.cc +++ b/media/audio/audio_output_controller_unittest.cc @@ -153,7 +153,6 @@ TEST_F(AudioOutputControllerTest, PlayPauseClose) { CloseAudioController(controller); } - TEST_F(AudioOutputControllerTest, HardwareBufferTooLarge) { scoped_ptr audio_manager(AudioManager::Create()); if (!audio_manager->HasAudioOutputDevices()) @@ -175,4 +174,53 @@ TEST_F(AudioOutputControllerTest, HardwareBufferTooLarge) { ASSERT_FALSE(controller); } +TEST_F(AudioOutputControllerTest, PlayPausePlayClose) { + scoped_ptr audio_manager(AudioManager::Create()); + if (!audio_manager->HasAudioOutputDevices()) + return; + + MockAudioOutputControllerEventHandler event_handler; + base::WaitableEvent event(false, false); + EXPECT_CALL(event_handler, OnCreated(NotNull())) + .WillOnce(InvokeWithoutArgs(&event, &base::WaitableEvent::Signal)); + + // OnPlaying() will be called only once. + base::WaitableEvent play_event(false, false); + EXPECT_CALL(event_handler, OnPlaying(NotNull())) + .WillOnce(InvokeWithoutArgs(&play_event, &base::WaitableEvent::Signal)); + + // OnPaused() should never be called since the pause during kStarting is + // dropped when the second play comes in. + EXPECT_CALL(event_handler, OnPaused(NotNull())) + .Times(0); + + MockAudioOutputControllerSyncReader sync_reader; + EXPECT_CALL(sync_reader, UpdatePendingBytes(_)) + .Times(AtLeast(2)); + EXPECT_CALL(sync_reader, Read(_, kHardwareBufferSize)) + .WillRepeatedly(DoAll(SignalEvent(&event), Return(4))); + EXPECT_CALL(sync_reader, DataReady()) + .WillRepeatedly(Return(true)); + EXPECT_CALL(sync_reader, Close()); + + AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, + kSampleRate, kBitsPerSample, kSamplesPerPacket); + scoped_refptr controller = + AudioOutputController::Create( + audio_manager.get(), &event_handler, params, &sync_reader); + ASSERT_TRUE(controller.get()); + + // Wait for OnCreated() to be called. + event.Wait(); + + ASSERT_FALSE(play_event.IsSignaled()); + controller->Play(); + controller->Pause(); + controller->Play(); + play_event.Wait(); + + // Now stop the controller. + CloseAudioController(controller); +} + } // namespace media -- cgit v1.1