diff options
author | tommi@chromium.org <tommi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-14 12:52:06 +0000 |
---|---|---|
committer | tommi@chromium.org <tommi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-14 12:52:06 +0000 |
commit | 7af0889d30c9a27b916aa49d4bfea7ea92465072 (patch) | |
tree | 6c3c727872115ac05f28c324dab0a8ece1c67155 /media | |
parent | 2068a212d343e9b901d3480cfe63041220a37c61 (diff) | |
download | chromium_src-7af0889d30c9a27b916aa49d4bfea7ea92465072.zip chromium_src-7af0889d30c9a27b916aa49d4bfea7ea92465072.tar.gz chromium_src-7af0889d30c9a27b916aa49d4bfea7ea92465072.tar.bz2 |
Make linux output streams consistent with other output stream implementations.
This include the following changes:
* The implementation of Start() for ALSA was implemented "asynchronously" on the audio thread.
This isn't necessary since Start() is already called on the audio thread and when it eventually runs, it would block the audio thread anyway.
Also, the OnPlaying() notification could be fired before the stream is actually at the 'playing' state, which is not consistent with other implementations.
* The pulse and alsa output stream classes use the message loop that the AudioManager owns - not a separate pointer.
In practice this only removes a member variable but the message loop being used previously belonged to the AudioManager anyaway.
* Close is now synchronous like it is in other implementations.
* Updated documentation.
This also simplifies the threading model for these classes and they are now (like the other stream classes) for all intents and purposes, single threaded.
Background:
We're working on making more of our tests work across all supported platforms.
We identified this problem with these particular implementations when running tests that previously only ran for other platforms.
Review URL: http://codereview.chromium.org/8930006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114415 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'media')
-rw-r--r-- | media/audio/linux/alsa_output.cc | 289 | ||||
-rw-r--r-- | media/audio/linux/alsa_output.h | 29 | ||||
-rw-r--r-- | media/audio/linux/alsa_output_unittest.cc | 32 | ||||
-rw-r--r-- | media/audio/linux/audio_manager_linux.cc | 10 | ||||
-rw-r--r-- | media/audio/openbsd/audio_manager_openbsd.cc | 2 | ||||
-rw-r--r-- | media/audio/pulse/pulse_output.cc | 40 | ||||
-rw-r--r-- | media/audio/pulse/pulse_output.h | 8 |
7 files changed, 161 insertions, 249 deletions
diff --git a/media/audio/linux/alsa_output.cc b/media/audio/linux/alsa_output.cc index 3086853..d924cd6 100644 --- a/media/audio/linux/alsa_output.cc +++ b/media/audio/linux/alsa_output.cc @@ -4,19 +4,14 @@ // // THREAD SAFETY // -// AlsaPcmOutputStream object is *not* thread-safe -- we assume that -// client thread - creates the object and calls the public APIs. -// message loop thread - executes all the internal tasks including querying -// the data source for more data, writing to the alsa device, and closing -// the alsa device. -// is actually the same thread. +// AlsaPcmOutputStream object is *not* thread-safe and should only be used +// from the audio thread. We DCHECK on this assumption whenever we can. // +// SEMANTICS OF Close() // -// SEMANTICS OF CloseTask() -// -// The CloseTask() is responsible for cleaning up any resources that were -// acquired after a successful Open(). CloseTask() would revoke any -// scheduled outstanding runnable methods. +// Close() is responsible for cleaning up any resources that were acquired after +// a successful Open(). Close() will nullify any scheduled outstanding runnable +// methods. // // // SEMANTICS OF ERROR STATES @@ -26,11 +21,11 @@ // that the playback_handle should no longer be used either because of a // hardware/low-level event. // -// When |state_| == kInError, all public API functions will fail -// with an error (Start() will call the OnError() function on the callback -// immediately), or no-op themselves with the exception of Close(). Even if an -// error state has been entered, if Open() has previously returned successfully, -// Close() must be called to cleanup the ALSA devices and release resources. +// When |state_| == kInError, all public API functions will fail with an error +// (Start() will call the OnError() function on the callback immediately), or +// no-op themselves with the exception of Close(). Even if an error state has +// been entered, if Open() has previously returned successfully, Close() must be +// called to cleanup the ALSA devices and release resources. // // When |stop_stream_| is set, no more commands will be made against the // ALSA device, and playback will effectively stop. From the client's point of @@ -54,6 +49,7 @@ #include "media/base/data_buffer.h" #include "media/base/seekable_buffer.h" +namespace { // Amount of time to wait if we've exhausted the data source. This is to avoid // busy looping. static const uint32 kNoDataSleepMilliseconds = 10; @@ -74,7 +70,7 @@ static const int kPcmRecoverIsSilent = 1; static const int kPcmRecoverIsSilent = 0; #endif -// ALSA is currently limited to 48Khz. +// ALSA is currently limited to 48kHz. // TODO(fbarchard): Resample audio from higher frequency to 48000. static const int kAlsaMaxSampleRate = 48000; @@ -147,6 +143,7 @@ static void Swizzle51Layout(Format* b, uint32 filled) { b[5] = aac[5]; // LFE } } +} // end namespace std::ostream& operator<<(std::ostream& os, AlsaPcmOutputStream::InternalState state) { @@ -185,8 +182,7 @@ const uint32 AlsaPcmOutputStream::kMinLatencyMicros = AlsaPcmOutputStream::AlsaPcmOutputStream(const std::string& device_name, const AudioParameters& params, AlsaWrapper* wrapper, - AudioManagerLinux* manager, - MessageLoop* message_loop) + AudioManagerLinux* manager) : requested_device_name_(device_name), pcm_format_(alsa_util::BitsToFormat(params.bits_per_sample)), channels_(params.channels), @@ -206,12 +202,11 @@ AlsaPcmOutputStream::AlsaPcmOutputStream(const std::string& device_name, manager_(manager), playback_handle_(NULL), frames_per_packet_(packet_size_ / bytes_per_frame_), - message_loop_(message_loop), ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)), state_(kCreated), volume_(1.0f), source_callback_(NULL) { - DCHECK_EQ(MessageLoop::current(), message_loop_); + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); // Sanity check input values. if ((params.sample_rate > kAlsaMaxSampleRate) || (params.sample_rate <= 0)) { @@ -236,19 +231,14 @@ AlsaPcmOutputStream::~AlsaPcmOutputStream() { DCHECK(current_state == kCreated || current_state == kIsClosed || current_state == kInError); - - // TODO(ajwong): Ensure that CloseTask has been called and the - // playback handle released by DCHECKing that playback_handle_ is NULL. - // Currently, because of Bug 18217, there is a race condition on destruction - // where the stream is not always stopped and closed, causing this to fail. + DCHECK(!playback_handle_); } bool AlsaPcmOutputStream::Open() { - DCHECK_EQ(MessageLoop::current(), message_loop_); + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); - if (state() == kInError) { + if (state() == kInError) return false; - } if (!CanTransitionTo(kIsOpened)) { NOTREACHED() << "Invalid state: " << state(); @@ -260,70 +250,12 @@ bool AlsaPcmOutputStream::Open() { // object's public API is only called on one thread so the state cannot // transition out from under us. TransitionTo(kIsOpened); - message_loop_->PostTask( - FROM_HERE, - base::Bind(&AlsaPcmOutputStream::OpenTask, weak_factory_.GetWeakPtr())); - - return true; -} - -void AlsaPcmOutputStream::Close() { - DCHECK_EQ(MessageLoop::current(), message_loop_); - - // Sanity check that the transition occurs correctly. It is safe to - // continue anyways because all operations for closing are idempotent. - if (TransitionTo(kIsClosed) != kIsClosed) { - NOTREACHED() << "Unable to transition Closed."; - } - - message_loop_->PostTask( - FROM_HERE, - base::Bind(&AlsaPcmOutputStream::CloseTask, weak_factory_.GetWeakPtr())); -} - -void AlsaPcmOutputStream::Start(AudioSourceCallback* callback) { - DCHECK_EQ(MessageLoop::current(), message_loop_); - - CHECK(callback); - - set_source_callback(callback); - - // Only post the task if we can enter the playing state. - if (TransitionTo(kIsPlaying) == kIsPlaying) { - message_loop_->PostTask(FROM_HERE, base::Bind( - &AlsaPcmOutputStream::StartTask, weak_factory_.GetWeakPtr())); - } -} - -void AlsaPcmOutputStream::Stop() { - DCHECK_EQ(MessageLoop::current(), message_loop_); - - // Reset the callback, so that it is not called anymore. - set_source_callback(NULL); - - TransitionTo(kIsStopped); -} - -void AlsaPcmOutputStream::SetVolume(double volume) { - DCHECK_EQ(MessageLoop::current(), message_loop_); - - volume_ = static_cast<float>(volume); -} - -void AlsaPcmOutputStream::GetVolume(double* volume) { - DCHECK_EQ(MessageLoop::current(), message_loop_); - - *volume = volume_; -} - -void AlsaPcmOutputStream::OpenTask() { - DCHECK_EQ(message_loop_, MessageLoop::current()); // Try to open the device. if (requested_device_name_ == kAutoSelectDevice) { playback_handle_ = AutoSelectDevice(latency_micros_); if (playback_handle_) - VLOG(1) << "Auto-selected device: " << device_name_; + DVLOG(1) << "Auto-selected device: " << device_name_; } else { device_name_ = requested_device_name_; playback_handle_ = alsa_util::OpenPlaybackDevice(wrapper_, @@ -356,73 +288,103 @@ void AlsaPcmOutputStream::OpenTask() { alsa_buffer_frames_ = buffer_size; } } + + return true; } -void AlsaPcmOutputStream::StartTask() { - DCHECK_EQ(message_loop_, MessageLoop::current()); +void AlsaPcmOutputStream::Close() { + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); - if (stop_stream_) { - return; - } + // Sanity check that the transition occurs correctly. It is safe to + // continue anyways because all operations for closing are idempotent. + if (TransitionTo(kIsClosed) != kIsClosed) { + NOTREACHED() << "Unable to transition Closed."; + } else { + // Shutdown the audio device. + if (playback_handle_ && + alsa_util::CloseDevice(wrapper_, playback_handle_) < 0) { + LOG(WARNING) << "Unable to close audio device. Leaking handle."; + } + playback_handle_ = NULL; - if (state() != kIsPlaying) { - return; - } + // Release the buffer. + buffer_.reset(); - // Before starting, the buffer might have audio from previous user of this - // device. - buffer_->Clear(); + // Signal anything that might already be scheduled to stop. + stop_stream_ = true; // Not necessary in production, but unit tests + // uses the flag to verify that stream was closed. + weak_factory_.InvalidateWeakPtrs(); - // When starting again, drop all packets in the device and prepare it again - // incase we are restarting from a pause state and need to flush old data. - int error = wrapper_->PcmDrop(playback_handle_); - if (error < 0 && error != -EAGAIN) { - LOG(ERROR) << "Failure clearing playback device (" - << wrapper_->PcmName(playback_handle_) << "): " - << wrapper_->StrError(error); - stop_stream_ = true; - return; + // Signal to the manager that we're closed and can be removed. + // Should be last call in the method as it deletes "this". + manager_->ReleaseOutputStream(this); } +} - error = wrapper_->PcmPrepare(playback_handle_); - if (error < 0 && error != -EAGAIN) { - LOG(ERROR) << "Failure preparing stream (" - << wrapper_->PcmName(playback_handle_) << "): " - << wrapper_->StrError(error); - stop_stream_ = true; +void AlsaPcmOutputStream::Start(AudioSourceCallback* callback) { + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); + + CHECK(callback); + + if (stop_stream_) return; - } - ScheduleNextWrite(false); -} + set_source_callback(callback); + + // Only post the task if we can enter the playing state. + if (TransitionTo(kIsPlaying) == kIsPlaying) { + // Before starting, the buffer might have audio from previous user of this + // device. + buffer_->Clear(); -void AlsaPcmOutputStream::CloseTask() { - DCHECK_EQ(message_loop_, MessageLoop::current()); + // When starting again, drop all packets in the device and prepare it again + // in case we are restarting from a pause state and need to flush old data. + int error = wrapper_->PcmDrop(playback_handle_); + if (error < 0 && error != -EAGAIN) { + LOG(ERROR) << "Failure clearing playback device (" + << wrapper_->PcmName(playback_handle_) << "): " + << wrapper_->StrError(error); + stop_stream_ = true; + } else { + error = wrapper_->PcmPrepare(playback_handle_); + if (error < 0 && error != -EAGAIN) { + LOG(ERROR) << "Failure preparing stream (" + << wrapper_->PcmName(playback_handle_) << "): " + << wrapper_->StrError(error); + stop_stream_ = true; + } + } - // Shutdown the audio device. - if (playback_handle_ && - alsa_util::CloseDevice(wrapper_, playback_handle_) < 0) { - LOG(WARNING) << "Unable to close audio device. Leaking handle."; + if (!stop_stream_) + ScheduleNextWrite(false); } - playback_handle_ = NULL; +} + +void AlsaPcmOutputStream::Stop() { + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); - // Release the buffer. - buffer_.reset(); + // Reset the callback, so that it is not called anymore. + set_source_callback(NULL); + + TransitionTo(kIsStopped); +} - // Signal anything that might already be scheduled to stop. - stop_stream_ = true; // Not necessary in production, but unit tests - // uses the flag to verify that stream was closed. - weak_factory_.InvalidateWeakPtrs(); +void AlsaPcmOutputStream::SetVolume(double volume) { + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); - // Signal to the manager that we're closed and can be removed. - // Should be last call in the method as it deletes "this". - manager_->ReleaseOutputStream(this); + volume_ = static_cast<float>(volume); +} + +void AlsaPcmOutputStream::GetVolume(double* volume) { + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); + + *volume = volume_; } void AlsaPcmOutputStream::BufferPacket(bool* source_exhausted) { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); - // If stopped, simulate a 0-lengthed packet. + // If stopped, simulate a 0-length packet. if (stop_stream_) { buffer_->Clear(); *source_exhausted = true; @@ -448,8 +410,7 @@ void AlsaPcmOutputStream::BufferPacket(bool* source_exhausted) { packet->GetBufferSize(), AudioBuffersState(buffer_delay, hardware_delay)); - CHECK(packet_size <= packet->GetBufferSize()) << - "Data source overran buffer."; + CHECK(packet_size <= packet->GetBufferSize()); // This should not happen, but in case it does, drop any trailing bytes // that aren't large enough to make a frame. Without this, packet writing @@ -465,8 +426,7 @@ void AlsaPcmOutputStream::BufferPacket(bool* source_exhausted) { bytes_per_sample_, volume_)) { // Adjust packet size for downmix. - packet_size = - packet_size / bytes_per_frame_ * bytes_per_output_frame_; + packet_size = packet_size / bytes_per_frame_ * bytes_per_output_frame_; } else { LOG(ERROR) << "Folding failed"; } @@ -513,7 +473,7 @@ void AlsaPcmOutputStream::BufferPacket(bool* source_exhausted) { } void AlsaPcmOutputStream::WritePacket() { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); // If the device is in error, just eat the bytes. if (stop_stream_) { @@ -521,9 +481,8 @@ void AlsaPcmOutputStream::WritePacket() { return; } - if (state() == kIsStopped) { + if (state() == kIsStopped) return; - } CHECK_EQ(buffer_->forward_bytes() % bytes_per_output_frame_, 0u); @@ -578,15 +537,13 @@ void AlsaPcmOutputStream::WritePacket() { } void AlsaPcmOutputStream::WriteTask() { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); - if (stop_stream_) { + if (stop_stream_) return; - } - if (state() == kIsStopped) { + if (state() == kIsStopped) return; - } bool source_exhausted; BufferPacket(&source_exhausted); @@ -596,11 +553,10 @@ void AlsaPcmOutputStream::WriteTask() { } void AlsaPcmOutputStream::ScheduleNextWrite(bool source_exhausted) { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); - if (stop_stream_) { + if (stop_stream_) return; - } uint32 frames_avail_wanted = alsa_buffer_frames_ / 2; uint32 available_frames = GetAvailableFrames(); @@ -637,21 +593,20 @@ void AlsaPcmOutputStream::ScheduleNextWrite(bool source_exhausted) { } // Avoid busy looping if the data source is exhausted. - if (source_exhausted) { + if (source_exhausted) next_fill_time_ms = std::max(next_fill_time_ms, kNoDataSleepMilliseconds); - } // Only schedule more reads/writes if we are still in the playing state. if (state() == kIsPlaying) { if (next_fill_time_ms == 0) { - message_loop_->PostTask(FROM_HERE, base::Bind( + manager_->GetMessageLoop()->PostTask(FROM_HERE, base::Bind( &AlsaPcmOutputStream::WriteTask, weak_factory_.GetWeakPtr())); } else { // TODO(ajwong): Measure the reliability of the delay interval. Use // base/metrics/histogram.h. - message_loop_->PostDelayedTask( - FROM_HERE, base::Bind( - &AlsaPcmOutputStream::WriteTask, weak_factory_.GetWeakPtr()), + manager_->GetMessageLoop()->PostDelayedTask(FROM_HERE, + base::Bind(&AlsaPcmOutputStream::WriteTask, + weak_factory_.GetWeakPtr()), next_fill_time_ms); } } @@ -673,9 +628,8 @@ std::string AlsaPcmOutputStream::FindDeviceForChannels(uint32 channels) { static const char kNameHintName[] = "NAME"; const char* wanted_device = GuessSpecificDeviceName(channels); - if (!wanted_device) { + if (!wanted_device) return ""; - } std::string guessed_device; void** hints = NULL; @@ -702,7 +656,7 @@ std::string AlsaPcmOutputStream::FindDeviceForChannels(uint32 channels) { } } - // Destory the hint now that we're done with it. + // Destroy the hint now that we're done with it. wrapper_->DeviceNameFreeHint(hints); hints = NULL; } else { @@ -741,11 +695,10 @@ snd_pcm_sframes_t AlsaPcmOutputStream::GetCurrentDelay() { } snd_pcm_sframes_t AlsaPcmOutputStream::GetAvailableFrames() { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); - if (stop_stream_) { + if (stop_stream_) return 0; - } // Find the number of frames queued in the sound device. snd_pcm_sframes_t available_frames = @@ -768,7 +721,7 @@ snd_pcm_t* AlsaPcmOutputStream::AutoSelectDevice(unsigned int latency) { // For auto-selection: // 1) Attempt to open a device that best matches the number of channels // requested. - // 2) If that fails, attempt the "plug:" version of it incase ALSA can + // 2) If that fails, attempt the "plug:" version of it in case ALSA can // remap do some software conversion to make it work. // 3) Fallback to kDefaultDevice. // 4) If that fails too, try the "plug:" version of kDefaultDevice. @@ -856,7 +809,7 @@ bool AlsaPcmOutputStream::CanTransitionTo(InternalState to) { AlsaPcmOutputStream::InternalState AlsaPcmOutputStream::TransitionTo(InternalState to) { - DCHECK_EQ(MessageLoop::current(), message_loop_); + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); if (!CanTransitionTo(to)) { NOTREACHED() << "Cannot transition from: " << state_ << " to: " << to; @@ -876,22 +829,20 @@ uint32 AlsaPcmOutputStream::RunDataCallback(uint8* dest, AudioBuffersState buffers_state) { TRACE_EVENT0("audio", "AlsaPcmOutputStream::RunDataCallback"); - if (source_callback_) { + if (source_callback_) return source_callback_->OnMoreData(this, dest, max_size, buffers_state); - } return 0; } void AlsaPcmOutputStream::RunErrorCallback(int code) { - if (source_callback_) { + if (source_callback_) source_callback_->OnError(this, code); - } } // Changes the AudioSourceCallback to proxy calls to. Pass in NULL to // release ownership of the currently registered callback. void AlsaPcmOutputStream::set_source_callback(AudioSourceCallback* callback) { - DCHECK_EQ(MessageLoop::current(), message_loop_); + DCHECK_EQ(MessageLoop::current(), manager_->GetMessageLoop()); source_callback_ = callback; } diff --git a/media/audio/linux/alsa_output.h b/media/audio/linux/alsa_output.h index 37edf6f..54d38a0 100644 --- a/media/audio/linux/alsa_output.h +++ b/media/audio/linux/alsa_output.h @@ -11,19 +11,12 @@ // will return false, and Start() will call OnError() immediately on the // provided callback. // -// TODO(ajwong): The OnClose() and OnError() calling needing fixing. +// If the stream is successfully opened, Close() must be called. After Close +// has been called, the object should be regarded as deleted and not touched. // -// If the stream is successfully opened, Close() must be called before the -// stream is deleted as Close() is responsible for ensuring resource cleanup -// occurs. -// -// This object's thread-safety is a little tricky. This object's public API -// can only be called from the thread that created the object. Calling the -// public APIs in any method that may cause concurrent execution will result in -// a race condition. When modifying the code in this class, please read the -// threading assumptions at the top of the implementation file to avoid -// introducing race conditions between tasks posted to the internal -// message_loop, and the thread calling the public APIs. +// AlsaPcmOutputStream is a single threaded class that should only be used from +// the audio thread. When modifying the code in this class, please read the +// threading assumptions at the top of the implementation. #ifndef MEDIA_AUDIO_LINUX_ALSA_OUTPUT_H_ #define MEDIA_AUDIO_LINUX_ALSA_OUTPUT_H_ @@ -73,8 +66,7 @@ class MEDIA_EXPORT AlsaPcmOutputStream : public AudioOutputStream { AlsaPcmOutputStream(const std::string& device_name, const AudioParameters& params, AlsaWrapper* wrapper, - AudioManagerLinux* manager, - MessageLoop* message_loop); + AudioManagerLinux* manager); virtual ~AlsaPcmOutputStream(); @@ -123,11 +115,6 @@ class MEDIA_EXPORT AlsaPcmOutputStream : public AudioOutputStream { }; friend std::ostream& operator<<(std::ostream& os, InternalState); - // Various tasks that complete actions started in the public API. - void OpenTask(); - void StartTask(); - void CloseTask(); - // Functions to get another packet from the data source and write it into the // ALSA device. void BufferPacket(bool* source_exhausted); @@ -204,10 +191,6 @@ class MEDIA_EXPORT AlsaPcmOutputStream : public AudioOutputStream { scoped_ptr<media::SeekableBuffer> buffer_; uint32 frames_per_packet_; - // The message loop responsible for querying the data source, and writing to - // the output device. - MessageLoop* message_loop_; - // Allows us to run tasks on the AlsaPcmOutputStream instance which are // bound by its lifetime. base::WeakPtrFactory<AlsaPcmOutputStream> weak_factory_; diff --git a/media/audio/linux/alsa_output_unittest.cc b/media/audio/linux/alsa_output_unittest.cc index 50db8c4..5b8405a 100644 --- a/media/audio/linux/alsa_output_unittest.cc +++ b/media/audio/linux/alsa_output_unittest.cc @@ -84,8 +84,13 @@ class MockAudioManagerLinux : public AudioManagerLinux { const AudioParameters& params, const std::string& device_id)); MOCK_METHOD0(MuteAll, void()); MOCK_METHOD0(UnMuteAll, void()); - MOCK_METHOD1(ReleaseOutputStream, void(AudioOutputStream* stream)); + + // We don't mock this method since all tests will do the same thing + // and use the current message loop. + virtual MessageLoop* GetMessageLoop() { + return MessageLoop::current(); + } }; class AlsaPcmOutputStreamTest : public testing::Test { @@ -110,8 +115,7 @@ class AlsaPcmOutputStreamTest : public testing::Test { return new AlsaPcmOutputStream(kTestDeviceName, params, &mock_alsa_wrapper_, - mock_manager_, - &message_loop_); + mock_manager_); } // Helper function to malloc the string returned by DeviceNameHint for NAME. @@ -216,8 +220,7 @@ TEST_F(AlsaPcmOutputStreamTest, ConstructedState) { test_stream_.reset(new AlsaPcmOutputStream(kTestDeviceName, bad_bps_params, &mock_alsa_wrapper_, - mock_manager_, - &message_loop_)); + mock_manager_)); EXPECT_EQ(AlsaPcmOutputStream::kInError, test_stream_->state()); // Bad format. @@ -227,8 +230,7 @@ TEST_F(AlsaPcmOutputStreamTest, ConstructedState) { test_stream_.reset(new AlsaPcmOutputStream(kTestDeviceName, bad_format_params, &mock_alsa_wrapper_, - mock_manager_, - &message_loop_)); + mock_manager_)); EXPECT_EQ(AlsaPcmOutputStream::kInError, test_stream_->state()); } @@ -256,7 +258,6 @@ TEST_F(AlsaPcmOutputStreamTest, LatencyFloor) { test_stream_.reset(CreateStream(kTestChannelLayout, kPacketFramesInMinLatency)); ASSERT_TRUE(test_stream_->Open()); - message_loop_.RunAllPending(); // Now close it and test that everything was released. EXPECT_CALL(mock_alsa_wrapper_, PcmClose(kFakeHandle)).WillOnce(Return(0)); @@ -264,7 +265,6 @@ TEST_F(AlsaPcmOutputStreamTest, LatencyFloor) { .WillOnce(Return(kTestDeviceName)); EXPECT_CALL(mock_manager(), ReleaseOutputStream(test_stream_.get())); test_stream_->Close(); - message_loop_.RunAllPending(); Mock::VerifyAndClear(&mock_alsa_wrapper_); Mock::VerifyAndClear(mock_manager_); @@ -287,7 +287,6 @@ TEST_F(AlsaPcmOutputStreamTest, LatencyFloor) { test_stream_.reset(CreateStream(kTestChannelLayout, kOverMinLatencyPacketSize)); ASSERT_TRUE(test_stream_->Open()); - message_loop_.RunAllPending(); // Now close it and test that everything was released. EXPECT_CALL(mock_alsa_wrapper_, PcmClose(kFakeHandle)) @@ -296,7 +295,6 @@ TEST_F(AlsaPcmOutputStreamTest, LatencyFloor) { .WillOnce(Return(kTestDeviceName)); EXPECT_CALL(mock_manager(), ReleaseOutputStream(test_stream_.get())); test_stream_->Close(); - message_loop_.RunAllPending(); Mock::VerifyAndClear(&mock_alsa_wrapper_); Mock::VerifyAndClear(mock_manager_); @@ -331,7 +329,6 @@ TEST_F(AlsaPcmOutputStreamTest, OpenClose) { // Open the stream. ASSERT_TRUE(test_stream_->Open()); - message_loop_.RunAllPending(); EXPECT_EQ(AlsaPcmOutputStream::kIsOpened, test_stream_->state()); EXPECT_EQ(kFakeHandle, test_stream_->playback_handle_); @@ -346,7 +343,6 @@ TEST_F(AlsaPcmOutputStreamTest, OpenClose) { .WillOnce(Return(kTestDeviceName)); EXPECT_CALL(mock_manager(), ReleaseOutputStream(test_stream_.get())); test_stream_->Close(); - message_loop_.RunAllPending(); EXPECT_TRUE(test_stream_->playback_handle_ == NULL); EXPECT_FALSE(test_stream_->buffer_.get()); @@ -359,11 +355,8 @@ TEST_F(AlsaPcmOutputStreamTest, PcmOpenFailed) { EXPECT_CALL(mock_alsa_wrapper_, StrError(kTestFailedErrno)) .WillOnce(Return(kDummyMessage)); - // Open still succeeds since PcmOpen is delegated to another thread. ASSERT_TRUE(test_stream_->Open()); ASSERT_EQ(AlsaPcmOutputStream::kIsOpened, test_stream_->state()); - ASSERT_FALSE(test_stream_->stop_stream_); - message_loop_.RunAllPending(); // Ensure internal state is set for a no-op stream if PcmOpen() failes. EXPECT_EQ(AlsaPcmOutputStream::kIsOpened, test_stream_->state()); @@ -374,7 +367,6 @@ TEST_F(AlsaPcmOutputStreamTest, PcmOpenFailed) { // Close the stream since we opened it to make destruction happy. EXPECT_CALL(mock_manager(), ReleaseOutputStream(test_stream_.get())); test_stream_->Close(); - message_loop_.RunAllPending(); } TEST_F(AlsaPcmOutputStreamTest, PcmSetParamsFailed) { @@ -394,8 +386,6 @@ TEST_F(AlsaPcmOutputStreamTest, PcmSetParamsFailed) { // no changes. ASSERT_TRUE(test_stream_->Open()); EXPECT_EQ(AlsaPcmOutputStream::kIsOpened, test_stream_->state()); - ASSERT_FALSE(test_stream_->stop_stream_); - message_loop_.RunAllPending(); // Ensure internal state is set for a no-op stream if PcmSetParams() failes. EXPECT_EQ(AlsaPcmOutputStream::kIsOpened, test_stream_->state()); @@ -406,7 +396,6 @@ TEST_F(AlsaPcmOutputStreamTest, PcmSetParamsFailed) { // Close the stream since we opened it to make destruction happy. EXPECT_CALL(mock_manager(), ReleaseOutputStream(test_stream_.get())); test_stream_->Close(); - message_loop_.RunAllPending(); } TEST_F(AlsaPcmOutputStreamTest, StartStop) { @@ -425,7 +414,6 @@ TEST_F(AlsaPcmOutputStreamTest, StartStop) { // Open the stream. ASSERT_TRUE(test_stream_->Open()); - message_loop_.RunAllPending(); // Expect Device setup. EXPECT_CALL(mock_alsa_wrapper_, PcmDrop(kFakeHandle)) @@ -467,7 +455,6 @@ TEST_F(AlsaPcmOutputStreamTest, StartStop) { EXPECT_CALL(mock_alsa_wrapper_, PcmName(kFakeHandle)) .WillOnce(Return(kTestDeviceName)); test_stream_->Close(); - message_loop_.RunAllPending(); } TEST_F(AlsaPcmOutputStreamTest, WritePacket_FinishedPacket) { @@ -784,7 +771,6 @@ TEST_F(AlsaPcmOutputStreamTest, ScheduleNextWrite) { // tasks unless running on valgrind. The code below is needed to keep // heapcheck happy. test_stream_->stop_stream_ = true; - message_loop_.RunAllPending(); test_stream_->TransitionTo(AlsaPcmOutputStream::kIsClosed); } diff --git a/media/audio/linux/audio_manager_linux.cc b/media/audio/linux/audio_manager_linux.cc index a85806f..6aaf591 100644 --- a/media/audio/linux/audio_manager_linux.cc +++ b/media/audio/linux/audio_manager_linux.cc @@ -65,10 +65,10 @@ AudioOutputStream* AudioManagerLinux::MakeAudioOutputStream( if (active_output_stream_count_ >= kMaxOutputStreams) return NULL; - AudioOutputStream* stream; + AudioOutputStream* stream = NULL; #if defined(USE_PULSEAUDIO) if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kUsePulseAudio)) { - stream = new PulseAudioOutputStream(params, this, GetMessageLoop()); + stream = new PulseAudioOutputStream(params, this); } else { #endif std::string device_name = AlsaPcmOutputStream::kAutoSelectDevice; @@ -77,12 +77,12 @@ AudioOutputStream* AudioManagerLinux::MakeAudioOutputStream( device_name = CommandLine::ForCurrentProcess()->GetSwitchValueASCII( switches::kAlsaOutputDevice); } - stream = new AlsaPcmOutputStream(device_name, params, wrapper_.get(), this, - GetMessageLoop()); + stream = new AlsaPcmOutputStream(device_name, params, wrapper_.get(), this); #if defined(USE_PULSEAUDIO) } #endif ++active_output_stream_count_; + DCHECK(stream); return stream; } @@ -95,8 +95,6 @@ AudioInputStream* AudioManagerLinux::MakeAudioInputStream( if (params.format == AudioParameters::AUDIO_MOCK) { return FakeAudioInputStream::MakeFakeStream(params); - } else if (params.format != AudioParameters::AUDIO_PCM_LINEAR) { - return NULL; } if (!initialized()) diff --git a/media/audio/openbsd/audio_manager_openbsd.cc b/media/audio/openbsd/audio_manager_openbsd.cc index 40b8a62..7222748 100644 --- a/media/audio/openbsd/audio_manager_openbsd.cc +++ b/media/audio/openbsd/audio_manager_openbsd.cc @@ -63,7 +63,7 @@ AudioOutputStream* AudioManagerOpenBSD::MakeAudioOutputStream( AudioOutputStream* stream; #if defined(USE_PULSEAUDIO) if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kUsePulseAudio)) { - stream = new PulseAudioOutputStream(params, this, GetMessageLoop()); + stream = new PulseAudioOutputStream(params, this); active_streams_.insert(stream); return stream; } diff --git a/media/audio/pulse/pulse_output.cc b/media/audio/pulse/pulse_output.cc index 5b55509..c58d5bf 100644 --- a/media/audio/pulse/pulse_output.cc +++ b/media/audio/pulse/pulse_output.cc @@ -112,18 +112,21 @@ static size_t MicrosecondsToBytes( base::Time::kMicrosecondsPerSecond; } +// static void PulseAudioOutputStream::ContextStateCallback(pa_context* context, void* state_addr) { pa_context_state_t* state = static_cast<pa_context_state_t*>(state_addr); *state = pa_context_get_state(context); } -void PulseAudioOutputStream::WriteRequestCallback( - pa_stream* playback_handle, size_t length, void* stream_addr) { +// static +void PulseAudioOutputStream::WriteRequestCallback(pa_stream* playback_handle, + size_t length, + void* stream_addr) { PulseAudioOutputStream* stream = - static_cast<PulseAudioOutputStream*>(stream_addr); + reinterpret_cast<PulseAudioOutputStream*>(stream_addr); - DCHECK_EQ(stream->message_loop_, MessageLoop::current()); + DCHECK_EQ(stream->manager_->GetMessageLoop(), MessageLoop::current()); stream->write_callback_handled_ = true; @@ -132,8 +135,7 @@ void PulseAudioOutputStream::WriteRequestCallback( } PulseAudioOutputStream::PulseAudioOutputStream(const AudioParameters& params, - AudioManagerPulse* manager, - MessageLoop* message_loop) + AudioManagerPulse* manager) : channel_layout_(params.channel_layout), channel_count_(ChannelLayoutToChannelCount(channel_layout_)), sample_format_(BitsToPASampleFormat(params.bits_per_sample)), @@ -149,11 +151,9 @@ PulseAudioOutputStream::PulseAudioOutputStream(const AudioParameters& params, volume_(1.0f), stream_stopped_(true), write_callback_handled_(false), - message_loop_(message_loop), ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)), source_callback_(NULL) { - DCHECK_EQ(message_loop_, MessageLoop::current()); - DCHECK(manager_); + DCHECK_EQ(manager_->GetMessageLoop(), MessageLoop::current()); // TODO(slock): Sanity check input values. } @@ -167,7 +167,7 @@ PulseAudioOutputStream::~PulseAudioOutputStream() { } bool PulseAudioOutputStream::Open() { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(manager_->GetMessageLoop(), MessageLoop::current()); // TODO(slock): Possibly move most of this to an OpenPlaybackDevice function // in a new class 'pulse_util', like alsa_util. @@ -270,7 +270,7 @@ void PulseAudioOutputStream::Reset() { } void PulseAudioOutputStream::Close() { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(manager_->GetMessageLoop(), MessageLoop::current()); Reset(); @@ -280,7 +280,7 @@ void PulseAudioOutputStream::Close() { } void PulseAudioOutputStream::WaitForWriteRequest() { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(manager_->GetMessageLoop(), MessageLoop::current()); if (stream_stopped_) return; @@ -290,7 +290,7 @@ void PulseAudioOutputStream::WaitForWriteRequest() { write_callback_handled_ = false; pa_mainloop_iterate(pa_mainloop_, 1, NULL); if (!write_callback_handled_) { - message_loop_->PostTask(FROM_HERE, base::Bind( + manager_->GetMessageLoop()->PostTask(FROM_HERE, base::Bind( &PulseAudioOutputStream::WaitForWriteRequest, weak_factory_.GetWeakPtr())); } @@ -350,13 +350,13 @@ void PulseAudioOutputStream::FulfillWriteRequest(size_t requested_bytes) { if (bytes_written < requested_bytes) { // We weren't able to buffer enough data to fulfill the request. Try to // fulfill the rest of the request later. - message_loop_->PostTask(FROM_HERE, base::Bind( + manager_->GetMessageLoop()->PostTask(FROM_HERE, base::Bind( &PulseAudioOutputStream::FulfillWriteRequest, weak_factory_.GetWeakPtr(), requested_bytes - bytes_written)); } else { // Continue playback. - message_loop_->PostTask(FROM_HERE, base::Bind( + manager_->GetMessageLoop()->PostTask(FROM_HERE, base::Bind( &PulseAudioOutputStream::WaitForWriteRequest, weak_factory_.GetWeakPtr())); } @@ -382,7 +382,7 @@ void PulseAudioOutputStream::WriteToStream(size_t bytes_to_write, } void PulseAudioOutputStream::Start(AudioSourceCallback* callback) { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(manager_->GetMessageLoop(), MessageLoop::current()); CHECK(callback); DLOG_IF(ERROR, !playback_handle_) << "Open() has not been called successfully"; @@ -396,25 +396,25 @@ void PulseAudioOutputStream::Start(AudioSourceCallback* callback) { stream_stopped_ = false; // Start playback. - message_loop_->PostTask(FROM_HERE, base::Bind( + manager_->GetMessageLoop()->PostTask(FROM_HERE, base::Bind( &PulseAudioOutputStream::WaitForWriteRequest, weak_factory_.GetWeakPtr())); } void PulseAudioOutputStream::Stop() { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(manager_->GetMessageLoop(), MessageLoop::current()); stream_stopped_ = true; } void PulseAudioOutputStream::SetVolume(double volume) { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(manager_->GetMessageLoop(), MessageLoop::current()); volume_ = static_cast<float>(volume); } void PulseAudioOutputStream::GetVolume(double* volume) { - DCHECK_EQ(message_loop_, MessageLoop::current()); + DCHECK_EQ(manager_->GetMessageLoop(), MessageLoop::current()); *volume = volume_; } diff --git a/media/audio/pulse/pulse_output.h b/media/audio/pulse/pulse_output.h index d795caf..5275934 100644 --- a/media/audio/pulse/pulse_output.h +++ b/media/audio/pulse/pulse_output.h @@ -42,13 +42,11 @@ typedef AudioManagerOpenBSD AudioManagerPulse; #endif struct AudioParameters; -class MessageLoop; class PulseAudioOutputStream : public AudioOutputStream { public: PulseAudioOutputStream(const AudioParameters& params, - AudioManagerPulse* manager, - MessageLoop* message_loop); + AudioManagerPulse* manager); virtual ~PulseAudioOutputStream(); @@ -123,10 +121,6 @@ class PulseAudioOutputStream : public AudioOutputStream { // set of pa_mainloop iterations. bool write_callback_handled_; - // Message loop used to post WaitForWriteTasks. Used to prevent blocking on - // the audio thread while waiting for PulseAudio write callbacks. - MessageLoop* message_loop_; - // Allows us to run tasks on the PulseAudioOutputStream instance which are // bound by its lifetime. base::WeakPtrFactory<PulseAudioOutputStream> weak_factory_; |