Bug 525401 - sa_stream_drain should return an error if called when stream not playing. r=chris.double

media/libsydneyaudio/bug525401_drain_deadlock.patch

@@ -0,0 +1,235 @@
+diff --git a/media/libsydneyaudio/src/sydney_audio_alsa.c b/media/libsydneyaudio/src/sydney_audio_alsa.c
+--- a/media/libsydneyaudio/src/sydney_audio_alsa.c
++++ b/media/libsydneyaudio/src/sydney_audio_alsa.c
+@@ -315,16 +315,19 @@ sa_stream_resume(sa_stream_t *s) {
+ 
+ 
+ int
+ sa_stream_drain(sa_stream_t *s)
+ {
+   if (s == NULL || s->output_unit == NULL) {
+     return SA_ERROR_NO_INIT;
+   }
++  if (snd_pcm_state(s->output_unit) != SND_PCM_STATE_RUNNING) {
++    return SA_ERROR_INVALID;
++  }
+   snd_pcm_drain(s->output_unit);
+   return SA_SUCCESS;
+ }
+ 
+ 
+ 
+ /*
+  * -----------------------------------------------------------------------------
+diff --git a/media/libsydneyaudio/src/sydney_audio_mac.c b/media/libsydneyaudio/src/sydney_audio_mac.c
+--- a/media/libsydneyaudio/src/sydney_audio_mac.c
++++ b/media/libsydneyaudio/src/sydney_audio_mac.c
+@@ -396,20 +396,20 @@ sa_stream_write(sa_stream_t *s, const vo
+ 
+   /*
+    * Once we have our first block of audio data, enable the audio callback
+    * function. This doesn't need to be protected by the mutex, because
+    * s->playing is not used in the audio callback thread, and it's probably
+    * better not to be inside the lock when we enable the audio callback.
+    */
+   if (!s->playing) {
++    if (AudioOutputUnitStart(s->output_unit) != 0) {
++      return SA_ERROR_SYSTEM;
++    }
+     s->playing = TRUE;
+-    if (AudioOutputUnitStart(s->output_unit) != 0) {
+-      result = SA_ERROR_SYSTEM;
+-    }
+   }
+ 
+   return result;
+ }
+ 
+ 
+ static OSStatus
+ audio_callback(
+@@ -553,17 +553,20 @@ sa_stream_pause(sa_stream_t *s) {
+   }
+ 
+   /*
+    * Don't hold the mutex when stopping the audio device, because it is
+    * possible to deadlock with this thread holding mutex then waiting on an
+    * internal Core Audio lock, and with the callback thread holding the Core
+    * Audio lock and waiting on the mutex.
+   */
+-  AudioOutputUnitStop(s->output_unit);
++  if (AudioOutputUnitStop(s->output_unit) != 0) {
++    return SA_ERROR_SYSTEM;
++  }
++  s->playing = FALSE;
+ 
+   return SA_SUCCESS;
+ }
+ 
+ 
+ int
+ sa_stream_resume(sa_stream_t *s) {
+ 
+@@ -581,17 +584,20 @@ sa_stream_resume(sa_stream_t *s) {
+   pthread_mutex_unlock(&s->mutex);
+ 
+   /*
+    * Don't hold the mutex when starting the audio device, because it is
+    * possible to deadlock with this thread holding mutex then waiting on an
+    * internal Core Audio lock, and with the callback thread holding the Core
+    * Audio lock and waiting on the mutex.
+   */
+-  AudioOutputUnitStart(s->output_unit);
++  if (AudioOutputUnitStart(s->output_unit) != 0) {
++    return SA_ERROR_SYSTEM;
++  }
++  s->playing = TRUE;
+ 
+   return SA_SUCCESS;
+ }
+ 
+ 
+ static sa_buf *
+ new_buffer(void) {
+   sa_buf  * b = malloc(sizeof(sa_buf) + BUF_SIZE);
+@@ -607,16 +613,20 @@ new_buffer(void) {
+ 
+ int
+ sa_stream_drain(sa_stream_t *s)
+ {
+   if (s == NULL || s->output_unit == NULL) {
+     return SA_ERROR_NO_INIT;
+   }
+ 
++  if (!s->playing) {
++    return SA_ERROR_INVALID;
++  }
++
+   while (1) {
+     pthread_mutex_lock(&s->mutex);
+     sa_buf  * b;
+     size_t    used = 0;
+     for (b = s->bl_head; b != NULL; b = b->next) {
+       used += b->end - b->start;
+     }
+     pthread_mutex_unlock(&s->mutex);
+diff --git a/media/libsydneyaudio/src/sydney_audio_waveapi.c b/media/libsydneyaudio/src/sydney_audio_waveapi.c
+--- a/media/libsydneyaudio/src/sydney_audio_waveapi.c
++++ b/media/libsydneyaudio/src/sydney_audio_waveapi.c
+@@ -111,16 +111,18 @@ struct sa_stream {
+   sa_pcm_format_t	format;   
+  
+   HWAVEOUT			  hWaveOut;
+   HANDLE			    callbackEvent;
+   CRITICAL_SECTION  waveCriticalSection;  
+   WAVEHDR*			  waveBlocks;  
+   volatile int		waveFreeBlockCount;
+   int				      waveCurrentBlock;
++
++  int playing;
+ };
+ 
+ 
+ /** Forward definitions of audio api specific functions */
+ int allocateBlocks(int size, int count, WAVEHDR** blocks);
+ int freeBlocks(WAVEHDR* blocks);
+ int openAudio(sa_stream_t *s);
+ int closeAudio(sa_stream_t * s);
+@@ -157,16 +159,17 @@ int sa_stream_create_pcm(sa_stream_t **s
+   }
+    
+   _s->rwMode = mode;
+   _s->format = format;
+   _s->rate = rate;
+   _s->channels = nchannels;
+   _s->deviceName = DEFAULT_DEVICE_NAME;
+   _s->device = DEFAULT_DEVICE;
++  _s->playing = 0;
+ 
+   *s = _s; 
+   return SA_SUCCESS;
+ }
+ 
+ /** Initialise the device */
+ int sa_stream_open(sa_stream_t *s) {  
+   int status = SA_SUCCESS;
+@@ -300,33 +303,41 @@ int sa_stream_get_position(sa_stream_t *
+ int sa_stream_resume(sa_stream_t *s) {
+   int status;  
+   
+   ERROR_IF_NO_INIT(s);
+ 
+   status = waveOutRestart(s->hWaveOut);
+   HANDLE_WAVE_ERROR(status, "resuming audio playback");
+ 
++  s->playing = 1;
++
+   return SA_SUCCESS;
+ }
+ /** Pause audio playback (do not empty the buffer) */
+ int sa_stream_pause(sa_stream_t *s) {
+   int status;
+ 
+   ERROR_IF_NO_INIT(s);
+   
+   status = waveOutPause(s->hWaveOut);
+   HANDLE_WAVE_ERROR(status, "resuming audio playback");
+ 
++  s->playing = 0;
++
+   return SA_SUCCESS;
+ }
+ /** Block until all audio has been played */
+ int sa_stream_drain(sa_stream_t *s) {
+   ERROR_IF_NO_INIT(s);
+   
++  if (!s->playing) {
++    return SA_ERROR_INVALID;
++  }
++
+   /* wait for all blocks to complete */
+   EnterCriticalSection(&(s->waveCriticalSection));
+   while(s->waveFreeBlockCount < BLOCK_COUNT) {
+     LeaveCriticalSection(&(s->waveCriticalSection));
+     Sleep(10);
+     EnterCriticalSection(&(s->waveCriticalSection));
+   }
+   LeaveCriticalSection(&(s->waveCriticalSection));
+@@ -484,16 +495,18 @@ int closeAudio(sa_stream_t * s) {
+     s->waveBlocks = NULL;
+   }
+ 
+   status = waveOutClose(s->hWaveOut);    
+   if (status != MMSYSERR_NOERROR) {
+     result = getSAErrorCode(status);
+   }
+ 
++  s->playing = 0;
++
+   DeleteCriticalSection(&(s->waveCriticalSection));
+   CloseHandle(s->callbackEvent);
+   
+   return result;
+ }
+ /**
+  * \brief - writes PCM audio samples to audio device
+  * \param s - valid handle to opened sydney stream
+@@ -545,16 +558,18 @@ int writeAudio(sa_stream_t *s, LPSTR dat
+     /*
+      * point to the next block
+      */
+     (s->waveCurrentBlock)++;
+     (s->waveCurrentBlock) %= BLOCK_COUNT;		
+ 
+     current = &(s->waveBlocks[s->waveCurrentBlock]);
+     current->dwUser = 0;
++
++    s->playing = 1;
+   }
+   return SA_SUCCESS;
+ }
+ 
+ /**
+  * \brief - audio callback function called when next WAVE header is played by audio device
+  */
+ void CALLBACK waveOutProc(

media/libsydneyaudio/src/sydney_audio_alsa.c

@@ -320,6 +320,9 @@ sa_stream_drain(sa_stream_t *s)
   if (s == NULL || s->output_unit == NULL) {
     return SA_ERROR_NO_INIT;
   }
+  if (snd_pcm_state(s->output_unit) != SND_PCM_STATE_RUNNING) {
+    return SA_ERROR_INVALID;
+  }
   snd_pcm_drain(s->output_unit);
   return SA_SUCCESS;
 }

media/libsydneyaudio/src/sydney_audio_mac.c

@@ -401,10 +401,10 @@ sa_stream_write(sa_stream_t *s, const void *data, size_t nbytes) {
    * better not to be inside the lock when we enable the audio callback.
    */
   if (!s->playing) {
-    s->playing = TRUE;
     if (AudioOutputUnitStart(s->output_unit) != 0) {
-      result = SA_ERROR_SYSTEM;
+      return SA_ERROR_SYSTEM;
     }
+    s->playing = TRUE;
   }
 
   return result;
@@ -558,7 +558,10 @@ sa_stream_pause(sa_stream_t *s) {
    * internal Core Audio lock, and with the callback thread holding the Core
    * Audio lock and waiting on the mutex.
   */
-  AudioOutputUnitStop(s->output_unit);
+  if (AudioOutputUnitStop(s->output_unit) != 0) {
+    return SA_ERROR_SYSTEM;
+  }
+  s->playing = FALSE;
 
   return SA_SUCCESS;
 }
@@ -586,7 +589,10 @@ sa_stream_resume(sa_stream_t *s) {
    * internal Core Audio lock, and with the callback thread holding the Core
    * Audio lock and waiting on the mutex.
   */
-  AudioOutputUnitStart(s->output_unit);
+  if (AudioOutputUnitStart(s->output_unit) != 0) {
+    return SA_ERROR_SYSTEM;
+  }
+  s->playing = TRUE;
 
   return SA_SUCCESS;
 }
@@ -612,6 +618,10 @@ sa_stream_drain(sa_stream_t *s)
     return SA_ERROR_NO_INIT;
   }
 
+  if (!s->playing) {
+    return SA_ERROR_INVALID;
+  }
+
   while (1) {
     pthread_mutex_lock(&s->mutex);
     sa_buf  * b;

media/libsydneyaudio/src/sydney_audio_waveapi.c

@@ -116,6 +116,8 @@ struct sa_stream {
   WAVEHDR*			  waveBlocks;  
   volatile int		waveFreeBlockCount;
   int				      waveCurrentBlock;
+
+  int playing;
 };
 
 
@@ -162,6 +164,7 @@ int sa_stream_create_pcm(sa_stream_t **s,
   _s->channels = nchannels;
   _s->deviceName = DEFAULT_DEVICE_NAME;
   _s->device = DEFAULT_DEVICE;
+  _s->playing = 0;
 
   *s = _s; 
   return SA_SUCCESS;
@@ -305,6 +308,8 @@ int sa_stream_resume(sa_stream_t *s) {
   status = waveOutRestart(s->hWaveOut);
   HANDLE_WAVE_ERROR(status, "resuming audio playback");
 
+  s->playing = 1;
+
   return SA_SUCCESS;
 }
 /** Pause audio playback (do not empty the buffer) */
@@ -316,12 +321,18 @@ int sa_stream_pause(sa_stream_t *s) {
   status = waveOutPause(s->hWaveOut);
   HANDLE_WAVE_ERROR(status, "resuming audio playback");
 
+  s->playing = 0;
+
   return SA_SUCCESS;
 }
 /** Block until all audio has been played */
 int sa_stream_drain(sa_stream_t *s) {
   ERROR_IF_NO_INIT(s);
   
+  if (!s->playing) {
+    return SA_ERROR_INVALID;
+  }
+
   /* wait for all blocks to complete */
   EnterCriticalSection(&(s->waveCriticalSection));
   while(s->waveFreeBlockCount < BLOCK_COUNT) {
@@ -489,6 +500,8 @@ int closeAudio(sa_stream_t * s) {
     result = getSAErrorCode(status);
   }
 
+  s->playing = 0;
+
   DeleteCriticalSection(&(s->waveCriticalSection));
   CloseHandle(s->callbackEvent);
   
@@ -550,6 +563,8 @@ int writeAudio(sa_stream_t *s, LPSTR data, int bytes) {
 
     current = &(s->waveBlocks[s->waveCurrentBlock]);
     current->dwUser = 0;
+
+    s->playing = 1;
   }
   return SA_SUCCESS;
 }

media/libsydneyaudio/update.sh

@@ -10,5 +10,6 @@ patch -p4 <include-CoreServices.patch
 patch -p4 <sydney_os2_base.patch
 patch -p4 <sydney_os2_moz.patch
 patch -p3 <bug495794_closeAudio.patch
-patch -p3 < bug495558_alsa_endian.patch
+patch -p3 <bug495558_alsa_endian.patch
+patch -p3 <bug525401_drain_deadlock.patch
 patch -p3 <bug526411_latency.patch