Index: media/audio/mac/audio_output_mac.cc |
diff --git a/media/audio/mac/audio_output_mac.cc b/media/audio/mac/audio_output_mac.cc |
index a81dae01107c0dd8ae315fae14ea4a56d1ace726..02e8c6354dd7479197d5974400bc4568fa679da6 100644 |
--- a/media/audio/mac/audio_output_mac.cc |
+++ b/media/audio/mac/audio_output_mac.cc |
@@ -91,7 +91,7 @@ void PCMQueueOutAudioOutputStream::HandleError(OSStatus err) { |
// source_ can be set to NULL from another thread. We need to cache its |
// pointer while we operate here. Note that does not mean that the source |
// has been destroyed. |
- AudioSourceCallback* source = source_; |
+ AudioSourceCallback* source = GetSource(); |
if (source) |
source->OnError(this, static_cast<int>(err)); |
NOTREACHED() << "error code " << err; |
@@ -157,7 +157,7 @@ bool PCMQueueOutAudioOutputStream::Open() { |
// Create the actual queue object and let the OS use its own thread to |
// run its CFRunLoop. |
err = AudioQueueNewOutput(&format_, RenderCallback, this, NULL, |
- kCFRunLoopCommonModes, 0, &audio_queue_); |
+ kCFRunLoopCommonModes, 0, &audio_queue_); |
if (err != noErr) { |
HandleError(err); |
return false; |
@@ -314,7 +314,8 @@ void PCMQueueOutAudioOutputStream::Close() { |
void PCMQueueOutAudioOutputStream::Stop() { |
// We request a synchronous stop, so the next call can take some time. In |
// the windows implementation we block here as well. |
- source_ = NULL; |
+ SetSource(NULL); |
+ |
// We set the source to null to signal to the data queueing thread it can stop |
// queueing data, however at most one callback might still be in flight which |
// could attempt to enqueue right after the next call. Rather that trying to |
@@ -378,10 +379,10 @@ bool PCMQueueOutAudioOutputStream::CheckForAdjustedLayout( |
return false; |
} |
-// Note to future hackers of this function: Do not add locks here because we |
-// call out to third party source that might do crazy things including adquire |
-// external locks or somehow re-enter here because its legal for them to call |
-// some audio functions. |
+// Note to future hackers of this function: Do not add locks to this function |
+// that are held through any calls made back into AudioQueue APIs, or other |
+// OS audio functions. This is because the OS dispatch may grab external |
+// locks, or possibly re-enter this function which can lead to a deadlock. |
void PCMQueueOutAudioOutputStream::RenderCallback(void* p_this, |
AudioQueueRef queue, |
AudioQueueBufferRef buffer) { |
@@ -391,7 +392,7 @@ void PCMQueueOutAudioOutputStream::RenderCallback(void* p_this, |
static_cast<PCMQueueOutAudioOutputStream*>(p_this); |
// Call the audio source to fill the free buffer with data. Not having a |
// source means that the queue has been closed. This is not an error. |
- AudioSourceCallback* source = audio_stream->source_; |
+ AudioSourceCallback* source = audio_stream->GetSource(); |
if (!source) |
return; |
@@ -467,8 +468,9 @@ void PCMQueueOutAudioOutputStream::RenderCallback(void* p_this, |
if (err == kAudioQueueErr_EnqueueDuringReset) { |
// This is the error you get if you try to enqueue a buffer and the |
// queue has been closed. Not really a problem if indeed the queue |
- // has been closed. |
- if (!audio_stream->source_) |
+ // has been closed. We recheck the value of source now to see if it has |
+ // indeed been closed. |
+ if (!audio_stream->GetSource()) |
return; |
} |
audio_stream->HandleError(err); |
@@ -478,7 +480,7 @@ void PCMQueueOutAudioOutputStream::RenderCallback(void* p_this, |
void PCMQueueOutAudioOutputStream::Start(AudioSourceCallback* callback) { |
DCHECK(callback); |
OSStatus err = noErr; |
- source_ = callback; |
+ SetSource(callback); |
pending_bytes_ = 0; |
// Ask the source to pre-fill all our buffers before playing. |
for (uint32 ix = 0; ix != kNumBuffers; ++ix) { |
@@ -500,3 +502,14 @@ void PCMQueueOutAudioOutputStream::Start(AudioSourceCallback* callback) { |
return; |
} |
} |
+ |
+void PCMQueueOutAudioOutputStream::SetSource(AudioSourceCallback* source) { |
+ base::AutoLock lock(source_lock_); |
+ source_ = source; |
+} |
+ |
+AudioOutputStream::AudioSourceCallback* |
+PCMQueueOutAudioOutputStream::GetSource() { |
+ base::AutoLock lock(source_lock_); |
+ return source_; |
+} |