Change the way we are stopping audio stream on Mac once again.

media/audio/mac/audio_output_mac.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/audio/mac/audio_output_mac.h"
 
 #include <CoreServices/CoreServices.h>
 
 #include "base/basictypes.h"
 #include "base/debug/trace_event.h"
@@ skipping 42 matching lines @@
       packet_size_(params.GetBytesPerBuffer()),
       silence_bytes_(0),
       volume_(1),
       pending_bytes_(0),
       num_source_channels_(params.channels()),
       source_layout_(params.channel_layout()),
       num_core_channels_(0),
       should_swizzle_(false),
       should_down_mix_(false),
       stopped_event_(true /* manual reset */, false /* initial state */),
-      num_buffers_left_(kNumBuffers) {
+      stop_requested_(false) {
   // We must have a manager.
   DCHECK(manager_);
   // A frame is one sample across all channels. In interleaved audio the per
   // frame fields identify the set of n |channels|. In uncompressed audio, a
   // packet is always one frame.
   format_.mSampleRate = params.sample_rate();
   format_.mFormatID = kAudioFormatLinearPCM;
   format_.mFormatFlags = kLinearPCMFormatFlagIsPacked;
   format_.mBitsPerChannel = params.bits_per_sample();
   format_.mChannelsPerFrame = params.channels();
@@ skipping 298 matching lines @@
       core_channel_orderings_[input_channel] == kEmptyChannel &&
       kChannelOrderings[source_layout_][input_channel] > kEmptyChannel &&
       kChannelOrderings[source_layout_][output_channel] == kEmptyChannel) {
     channel_remap_[core_channel_orderings_[output_channel]] =
         kChannelOrderings[source_layout_][input_channel];
     return true;
   }
   return false;
 }
 
+void PCMQueueOutAudioOutputStream::IsRunningCallback(
+    void* p_this,
+    AudioQueueRef queue,
+    AudioQueuePropertyID inID) {
+  PCMQueueOutAudioOutputStream* audio_stream =
+      static_cast<PCMQueueOutAudioOutputStream*>(p_this);
+  UInt32 running = 0;
+  UInt32 size = sizeof(running);
+  OSStatus err = AudioQueueGetProperty(queue,
+                                       kAudioQueueProperty_IsRunning,
+                                       &running,
+                                       &size);
+  if (err) {
+    audio_stream->HandleError(err);
+    return;
+  }
+  if (!running) {
+    // Remove property listener, we don't need it anymore.
+    OSStatus err = AudioQueueRemovePropertyListener(
+        queue,
+        kAudioQueueProperty_IsRunning,
+        PCMQueueOutAudioOutputStream::IsRunningCallback,
+        audio_stream);
+    if (err != noErr)
+      audio_stream->HandleError(err);
+
+    // Ok, finally signal that stream is not running.
+    audio_stream->stopped_event_.Signal();
+  }
+}
+
 // 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) {
   TRACE_EVENT0("audio", "PCMQueueOutAudioOutputStream::RenderCallback");
 
   PCMQueueOutAudioOutputStream* audio_stream =
       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 stopped.
   AudioSourceCallback* source = audio_stream->GetSource();
   if (!source) {
     // PCMQueueOutAudioOutputStream::Stop() is waiting for callback to
-    // stop the stream and signal when all callbacks are done.
-    // (we probably can stop the stream there, but it is better to have
-    // all the complex logic in one place; stopping latency is not very
-    // important if you reuse audio stream in the mixer and not close it
-    // immediately).
-    --audio_stream->num_buffers_left_;
-    if (audio_stream->num_buffers_left_ == kNumBuffers - 1) {
-      // First buffer after stop requested, stop the queue.
-      OSStatus err = AudioQueueStop(audio_stream->audio_queue_, true);
+    // stop the stream and signal when all activity ceased (we probably
+    // can stop the stream there, but it is better to have all the
+    // complex logic in one place; stopping latency is not very important
+    // if you reuse audio stream in the mixer and not close it immediately).
+    if (!audio_stream->stop_requested_) {
+      audio_stream->stop_requested_ = true;

[scherkus (not reviewing), 2012/06/18 19:40:53]

do we need to reset this to false at some point in time?

[enal1, 2012/06/18 19:58:47]

When starting the stream. in PCMQueueOutAudioOutputStream::Start().

+      // Before actually stopping the stream, register callback that
+      // OS would call when status of "is running" property changes.
+      // This way we (hopefully) would know when stream is actually
+      // stopped and it is safe to dispose it.
+      OSStatus err = AudioQueueAddPropertyListener(
+          queue,

[scherkus (not reviewing), 2012/06/18 19:40:53]

minor nit: in line 448 you use audio_stream->audio_queue -- double checking but
are these the same objects?

if so can you be consistent with which variable you use?

[enal1, 2012/06/18 19:58:47]

Done.

+          kAudioQueueProperty_IsRunning,
+          PCMQueueOutAudioOutputStream::IsRunningCallback,
+          audio_stream);
       if (err != noErr)
         audio_stream->HandleError(err);
-    }
-    if (audio_stream->num_buffers_left_ == 0) {
-      // Now we finally saw all the buffers.
-      // Signal that stopping is complete.
-      // Should never touch audio_stream after signaling as it
-      // can be deleted at any moment.
-      audio_stream->stopped_event_.Signal();
+      // Now stop the queue.
+      err = AudioQueueStop(audio_stream->audio_queue_, true);
+      if (err != noErr)
+        audio_stream->HandleError(err);
     }
     return;
   }
 
   // Adjust the number of pending bytes by subtracting the amount played.
   if (!static_cast<AudioQueueUserData*>(buffer->mUserData)->empty_buffer)
     audio_stream->pending_bytes_ -= buffer->mAudioDataByteSize;
   uint32 capacity = buffer->mAudioDataBytesCapacity;
   // TODO(sergeyu): Specify correct hardware delay for AudioBuffersState.
   uint32 filled = source->OnMoreData(
@@ skipping 75 matching lines @@
 void PCMQueueOutAudioOutputStream::Start(AudioSourceCallback* callback) {
   DCHECK(callback);
   DLOG_IF(ERROR, !audio_queue_) << "Open() has not been called successfully";
   if (!audio_queue_)
     return;
 
   OSStatus err = noErr;
   SetSource(callback);
   pending_bytes_ = 0;
   stopped_event_.Reset();
-  num_buffers_left_ = kNumBuffers;
+  stop_requested_ = false;
   // Ask the source to pre-fill all our buffers before playing.
   for (uint32 ix = 0; ix != kNumBuffers; ++ix) {
     buffer_[ix]->mAudioDataByteSize = 0;
     // Caller waits for 1st packet to become available, but not for others,
     // so we wait for them here.
     if (ix != 0) {
       AudioSourceCallback* source = GetSource();
       if (source)
         source->WaitTillDataReady();
     }
@@ skipping 20 matching lines @@
   source_ = source;
 }
 
 AudioOutputStream::AudioSourceCallback*
 PCMQueueOutAudioOutputStream::GetSource() {
   base::AutoLock lock(source_lock_);
   return source_;
 }
 
 }  // namespace media