fix crash in AUAudioOutputStream::Render()

media/audio/mac/audio_low_latency_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_low_latency_output_mac.h"
 
 #include <CoreServices/CoreServices.h>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
@@ skipping 32 matching lines @@
 // 6) The same thread that called stop will call Close() where we cleanup
 // and notify the audio manager, which likely will destroy this object.
 
 AUAudioOutputStream::AUAudioOutputStream(
     AudioManagerMac* manager, const AudioParameters& params)
     : manager_(manager),
       source_(NULL),
       output_unit_(0),
       output_device_id_(kAudioObjectUnknown),
       volume_(1),
-      hardware_latency_frames_(0) {
+      hardware_latency_frames_(0),
+      stopped_(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 |
                          kLinearPCMFormatFlagIsSignedInteger;
   format_.mBitsPerChannel = params.bits_per_sample();
@@ skipping 107 matching lines @@
   // destruction.
   manager_->ReleaseOutputStream(this);
 }
 
 void AUAudioOutputStream::Start(AudioSourceCallback* callback) {
   DCHECK(callback);
   DLOG_IF(ERROR, !output_unit_) << "Open() has not been called successfully";
   if (!output_unit_)
     return;
 
+  stopped_ = false;
   source_ = callback;
 
   AudioOutputUnitStart(output_unit_);
 }
 
 void AUAudioOutputStream::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;
+  if (stopped_)
+    return;
+
+  stopped_ = true;

[Chris Rogers, 2012/03/28 18:55:10]

Should this maybe be set to "true" only after AudioOutputUnitStop() has
returned?

and you can also remove the check down below for stopped_ in the Render() method

[no longer working on chromium, 2012/03/28 19:25:32]

Done.

 
   AudioOutputUnitStop(output_unit_);
+
+  source_ = NULL;
 }
 
 void AUAudioOutputStream::SetVolume(double volume) {
   if (!output_unit_)
     return;
   volume_ = static_cast<float>(volume);
 
   // TODO(crogers): set volume property
 }
 
 void AUAudioOutputStream::GetVolume(double* volume) {
   if (!output_unit_)
     return;
   *volume = volume_;
 }
 
 // Pulls on our provider to get rendered audio stream.
 // Note to future hackers of this function: Do not add locks here because this
 // is running on a real-time thread (for low-latency).
 OSStatus AUAudioOutputStream::Render(UInt32 number_of_frames,
                                      AudioBufferList* io_data,
                                      const AudioTimeStamp* output_time_stamp) {
+  if (stopped_)

[no longer working on chromium, 2012/03/27 11:01:32]

I don't use atomic operation here since there is comment requiring not to add
locks in this real-time thread.

[Chris Rogers, 2012/03/28 18:55:10]

Strictly speaking, I don't think it's even necessary to do this check here,
since you're now calling AudioOutputStop() (which is synchronous) before setting
source_ = NULL 

On 2012/03/27 11:01:32, xians1 wrote:

[no longer working on chromium, 2012/03/28 19:25:32]

True, thanks for pointing out. It just feels safer to use a check to make sure
we quit the callback as quickly as possible after Stop() is called. But I agree
this is not necessary.

+    return -1;
+
   // Update the playout latency.
   double playout_latency_frames = GetPlayoutLatency(output_time_stamp);
 
   AudioBuffer& buffer = io_data->mBuffers[0];
   uint8* audio_data = reinterpret_cast<uint8*>(buffer.mData);
   uint32 hardware_pending_bytes = static_cast<uint32>
       ((playout_latency_frames + 0.5) * format_.mBytesPerFrame);
   uint32 filled = source_->OnMoreData(
       this, audio_data, buffer.mDataByteSize,
       AudioBuffersState(0, hardware_pending_bytes));
@@ skipping 113 matching lines @@
   // Get the delay between the moment getting the callback and the scheduled
   // time stamp that tells when the data is going to be played out.
   UInt64 output_time_ns = AudioConvertHostTimeToNanos(
       output_time_stamp->mHostTime);
   UInt64 now_ns = AudioConvertHostTimeToNanos(AudioGetCurrentHostTime());
   double delay_frames = static_cast<double>
       (1e-9 * (output_time_ns - now_ns) * format_.mSampleRate);
 
   return (delay_frames + hardware_latency_frames_);
 }