diff options
author | laforge@chromium.org <laforge@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-28 18:33:38 +0000 |
---|---|---|
committer | laforge@chromium.org <laforge@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-28 18:33:38 +0000 |
commit | b51a11f2f289a8707f4604d72e7b8c430a9632cc (patch) | |
tree | 4fb4490641f00b0b336e39da9286f301bb187180 | |
parent | 7eccc58d9f16fc4bcfbf2fcebb76a03469a55061 (diff) | |
download | chromium_src-b51a11f2f289a8707f4604d72e7b8c430a9632cc.zip chromium_src-b51a11f2f289a8707f4604d72e7b8c430a9632cc.tar.gz chromium_src-b51a11f2f289a8707f4604d72e7b8c430a9632cc.tar.bz2 |
Revert 128895 - Fix a couple of regressions that made it in before the weekend.
1) Since the 'closed' closure now runs on the calling thread and not the audio thread assertions in the callback failed.
2) output_buffer_size() was returning the input buffer size. now fixed.
Also updating the output side to follow the same rule for the 'onclosed' callback.
BUG=120110,120111
TEST=Run content_unittests.
Review URL: http://codereview.chromium.org/9858007
TBR=tommi@chromium.org
Review URL: http://codereview.chromium.org/9837118
git-svn-id: svn://svn.chromium.org/chrome/branches/1084/src@129450 0039d316-1c4b-4281-b951-d872f2087c98
9 files changed, 43 insertions, 30 deletions
diff --git a/content/browser/renderer_host/media/audio_input_renderer_host.cc b/content/browser/renderer_host/media/audio_input_renderer_host.cc index 85d269f..031f520 100644 --- a/content/browser/renderer_host/media/audio_input_renderer_host.cc +++ b/content/browser/renderer_host/media/audio_input_renderer_host.cc @@ -374,12 +374,22 @@ void AudioInputRendererHost::CloseAndDeleteStream(AudioEntry* entry) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); if (!entry->pending_close) { - entry->controller->Close(base::Bind(&AudioInputRendererHost::DeleteEntry, + entry->controller->Close(base::Bind(&AudioInputRendererHost::OnStreamClosed, this, entry)); entry->pending_close = true; } } +void AudioInputRendererHost::OnStreamClosed(AudioEntry* entry) { + // We should be on the the audio-manager thread now. + DCHECK(entry->controller->message_loop()->BelongsToCurrentThread()); + + // Delete the entry after we've closed the stream. + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&AudioInputRendererHost::DeleteEntry, this, entry)); +} + void AudioInputRendererHost::DeleteEntry(AudioEntry* entry) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); diff --git a/content/browser/renderer_host/media/audio_input_renderer_host.h b/content/browser/renderer_host/media/audio_input_renderer_host.h index 6d2a8b8..b89e960 100644 --- a/content/browser/renderer_host/media/audio_input_renderer_host.h +++ b/content/browser/renderer_host/media/audio_input_renderer_host.h @@ -175,6 +175,9 @@ class CONTENT_EXPORT AudioInputRendererHost // is closed. void CloseAndDeleteStream(AudioEntry* entry); + // Called on the audio thread after the audio input stream is closed. + void OnStreamClosed(AudioEntry* entry); + // Delete an audio entry and close the related audio stream. void DeleteEntry(AudioEntry* entry); diff --git a/content/browser/renderer_host/media/audio_renderer_host.cc b/content/browser/renderer_host/media/audio_renderer_host.cc index 290b94a..76691e1 100644 --- a/content/browser/renderer_host/media/audio_renderer_host.cc +++ b/content/browser/renderer_host/media/audio_renderer_host.cc @@ -329,11 +329,19 @@ void AudioRendererHost::CloseAndDeleteStream(AudioEntry* entry) { if (!entry->pending_close) { entry->controller->Close( - base::Bind(&AudioRendererHost::DeleteEntry, this, entry)); + base::Bind(&AudioRendererHost::OnStreamClosed, this, entry)); entry->pending_close = true; } } +void AudioRendererHost::OnStreamClosed(AudioEntry* entry) { + // Delete the entry on the IO thread after we've closed the stream. + // (We're currently on the audio thread). + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&AudioRendererHost::DeleteEntry, this, entry)); +} + void AudioRendererHost::DeleteEntry(AudioEntry* entry) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); diff --git a/content/browser/renderer_host/media/audio_renderer_host.h b/content/browser/renderer_host/media/audio_renderer_host.h index a506531..a9ca80f 100644 --- a/content/browser/renderer_host/media/audio_renderer_host.h +++ b/content/browser/renderer_host/media/audio_renderer_host.h @@ -159,6 +159,9 @@ class CONTENT_EXPORT AudioRendererHost // is closed. void CloseAndDeleteStream(AudioEntry* entry); + // Called on the audio thread after the audio stream is closed. + void OnStreamClosed(AudioEntry* entry); + // Delete an audio entry and close the related audio stream. void DeleteEntry(AudioEntry* entry); diff --git a/content/renderer/media/webrtc_audio_device_impl.h b/content/renderer/media/webrtc_audio_device_impl.h index b3f3534..4c43498 100644 --- a/content/renderer/media/webrtc_audio_device_impl.h +++ b/content/renderer/media/webrtc_audio_device_impl.h @@ -258,7 +258,7 @@ class CONTENT_EXPORT WebRtcAudioDeviceImpl return input_audio_parameters_.frames_per_buffer(); } size_t output_buffer_size() const { - return output_audio_parameters_.frames_per_buffer(); + return input_audio_parameters_.frames_per_buffer(); } int input_channels() const { return input_audio_parameters_.channels(); diff --git a/media/audio/audio_input_controller.h b/media/audio/audio_input_controller.h index feeb895..3e9bd4b 100644 --- a/media/audio/audio_input_controller.h +++ b/media/audio/audio_input_controller.h @@ -55,12 +55,12 @@ // kRecording // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // Close() ==> DoClose() -// state_ = kClosed // AudioInputStream::Stop() // AudioInputStream::Close() // SyncWriter::Close() -// Closure::Run() <--------------. -// (closure-task) +// Closure::Run() +// (closure-task) <----------------. +// kClosed // // The audio thread itself is owned by the AudioManager that the // AudioInputController holds a reference to. When performing tasks on the diff --git a/media/audio/audio_output_controller.cc b/media/audio/audio_output_controller.cc index f5646f2..0291c38 100644 --- a/media/audio/audio_output_controller.cc +++ b/media/audio/audio_output_controller.cc @@ -96,8 +96,8 @@ void AudioOutputController::Flush() { void AudioOutputController::Close(const base::Closure& closed_task) { DCHECK(!closed_task.is_null()); DCHECK(message_loop_); - message_loop_->PostTaskAndReply(FROM_HERE, base::Bind( - &AudioOutputController::DoClose, base::Unretained(this)), closed_task); + message_loop_->PostTask(FROM_HERE, base::Bind( + &AudioOutputController::DoClose, base::Unretained(this), closed_task)); } void AudioOutputController::SetVolume(double volume) { @@ -240,7 +240,7 @@ void AudioOutputController::DoFlush() { // TODO(hclam): Actually flush the audio device. } -void AudioOutputController::DoClose() { +void AudioOutputController::DoClose(const base::Closure& closed_task) { DCHECK(message_loop_->BelongsToCurrentThread()); if (state_ != kClosed) { @@ -248,6 +248,8 @@ void AudioOutputController::DoClose() { sync_reader_->Close(); state_ = kClosed; } + + closed_task.Run(); } void AudioOutputController::DoSetVolume(double volume) { diff --git a/media/audio/audio_output_controller.h b/media/audio/audio_output_controller.h index 09878c6..42e8d5a 100644 --- a/media/audio/audio_output_controller.h +++ b/media/audio/audio_output_controller.h @@ -184,7 +184,7 @@ class MEDIA_EXPORT AudioOutputController void PollAndStartIfDataReady(); void DoPause(); void DoFlush(); - void DoClose(); + void DoClose(const base::Closure& closed_task); void DoSetVolume(double volume); void DoReportError(int code); diff --git a/media/audio/audio_output_controller_unittest.cc b/media/audio/audio_output_controller_unittest.cc index cd65f5b..3736a90 100644 --- a/media/audio/audio_output_controller_unittest.cc +++ b/media/audio/audio_output_controller_unittest.cc @@ -6,7 +6,6 @@ #include "base/environment.h" #include "base/basictypes.h" #include "base/logging.h" -#include "base/message_loop.h" #include "base/synchronization/waitable_event.h" #include "media/audio/audio_output_controller.h" #include "testing/gmock/include/gmock/gmock.h" @@ -66,23 +65,13 @@ ACTION_P(SignalEvent, event) { // Closes AudioOutputController synchronously. static void CloseAudioController(AudioOutputController* controller) { - controller->Close(MessageLoop::QuitClosure()); - MessageLoop::current()->Run(); + base::WaitableEvent closed_event(true, false); + controller->Close(base::Bind(&base::WaitableEvent::Signal, + base::Unretained(&closed_event))); + closed_event.Wait(); } -class AudioOutputControllerTest : public testing::Test { - public: - AudioOutputControllerTest() {} - virtual ~AudioOutputControllerTest() {} - - protected: - MessageLoopForIO message_loop_; - - private: - DISALLOW_COPY_AND_ASSIGN(AudioOutputControllerTest); -}; - -TEST_F(AudioOutputControllerTest, CreateAndClose) { +TEST(AudioOutputControllerTest, CreateAndClose) { scoped_ptr<AudioManager> audio_manager(AudioManager::Create()); if (!audio_manager->HasAudioOutputDevices()) return; @@ -106,7 +95,7 @@ TEST_F(AudioOutputControllerTest, CreateAndClose) { CloseAudioController(controller); } -TEST_F(AudioOutputControllerTest, PlayPauseClose) { +TEST(AudioOutputControllerTest, PlayPauseClose) { scoped_ptr<AudioManager> audio_manager(AudioManager::Create()); if (!audio_manager->HasAudioOutputDevices()) return; @@ -128,8 +117,6 @@ TEST_F(AudioOutputControllerTest, PlayPauseClose) { EXPECT_CALL(sync_reader, Read(_, kHardwareBufferSize)) .WillRepeatedly(DoAll(SignalEvent(&event), Return(4))); - EXPECT_CALL(sync_reader, DataReady()) - .WillRepeatedly(Return(true)); EXPECT_CALL(event_handler, OnPaused(NotNull())) .WillOnce(InvokeWithoutArgs(&pause_event, &base::WaitableEvent::Signal)); EXPECT_CALL(sync_reader, Close()); @@ -154,7 +141,7 @@ TEST_F(AudioOutputControllerTest, PlayPauseClose) { } -TEST_F(AudioOutputControllerTest, HardwareBufferTooLarge) { +TEST(AudioOutputControllerTest, HardwareBufferTooLarge) { scoped_ptr<AudioManager> audio_manager(AudioManager::Create()); if (!audio_manager->HasAudioOutputDevices()) return; |