Bug461936 - Update to libsydneyaudio r3760 to fix deadlock when shutting down a stream - rs=roc
authorChris Double <chris.double@double.co.nz>
Tue, 04 Nov 2008 21:23:55 +1300
changeset 21269 05b6f4f2101e9aab2732bf9d1575d1a8a2260d4f
parent 21268 9dcff436bd1bb7348bc0c1d1b255a0f9c3a7b642
child 21270 18403769ec19f8d64ddf8e30d6a76468ae5a4fbc
push idunknown
push userunknown
push dateunknown
reviewersroc
bugs461936
milestone1.9.1b2pre
Bug461936 - Update to libsydneyaudio r3760 to fix deadlock when shutting down a stream - rs=roc
media/libsydneyaudio/README_MOZILLA
media/libsydneyaudio/src/sydney_audio_mac.c
media/libsydneyaudio/src/sydney_audio_sunaudio.c
media/libsydneyaudio/src/sydney_audio_waveapi.c
--- a/media/libsydneyaudio/README_MOZILLA
+++ b/media/libsydneyaudio/README_MOZILLA
@@ -1,8 +1,8 @@
 The source from this directory was copied from the libsydneyaudio svn
 source using the update.sh script. The only changes made were those
 applied by update.sh and the addition/upate of Makefile.in files for
 the Mozilla build system.
 
 http://svn.annodex.net/libsydneyaudio/trunk
 
-The svn revision number used was r3733.
+The svn revision number used was r3760.
--- a/media/libsydneyaudio/src/sydney_audio_mac.c
+++ b/media/libsydneyaudio/src/sydney_audio_mac.c
@@ -255,36 +255,39 @@ sa_stream_open(sa_stream_t *s) {
 
 int
 sa_stream_destroy(sa_stream_t *s) {
 
   if (s == NULL) {
     return SA_SUCCESS;
   }
 
-  pthread_mutex_lock(&s->mutex);
-
   /*
-   * Shut down the audio output device.
+   * Shut down the audio output device.  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.
+   * This does not need to be protected by the mutex anyway because
+   * AudioOutputUnitStop, when called from the non-callback thread, blocks
+   * until in-flight callbacks complete and the HAL shuts down.  See:
+   * http://lists.apple.com/archives/coreaudio-api/2005/Dec/msg00055.html
    */
   int result = SA_SUCCESS;
   if (s->output_unit != NULL) {
     if (s->playing && AudioOutputUnitStop(s->output_unit) != 0) {
       result = SA_ERROR_SYSTEM;
     }
     if (AudioUnitUninitialize(s->output_unit) != 0) {
       result = SA_ERROR_SYSTEM;
     }
     if (CloseComponent(s->output_unit) != noErr) {
       result = SA_ERROR_SYSTEM;
     }
   }
 
-  pthread_mutex_unlock(&s->mutex);
-
   /*
    * Release resources.
    */
   if (pthread_mutex_destroy(&s->mutex) != 0) {
     result = SA_ERROR_SYSTEM;
   }
   while (s->bl_head != NULL) {
     sa_buf  * next = s->bl_head->next;
@@ -541,40 +544,51 @@ sa_stream_get_position(sa_stream_t *s, s
 
 int
 sa_stream_pause(sa_stream_t *s) {
 
   if (s == NULL || s->output_unit == NULL) {
     return SA_ERROR_NO_INIT;
   }
 
-  pthread_mutex_lock(&s->mutex);
+  /*
+   * 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);
-  pthread_mutex_unlock(&s->mutex);
+
   return SA_SUCCESS;
 }
 
 
 int
 sa_stream_resume(sa_stream_t *s) {
 
   if (s == NULL || s->output_unit == NULL) {
     return SA_ERROR_NO_INIT;
   }
 
   pthread_mutex_lock(&s->mutex);
-
   /*
    * The audio device resets its mSampleTime counter after pausing,
    * so we need to clear our tracking value to keep that in sync.
    */
   s->bytes_played = 0;
+  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);
 
-  pthread_mutex_unlock(&s->mutex);
   return SA_SUCCESS;
 }
 
 
 static sa_buf *
 new_buffer(void) {
   sa_buf  * b = malloc(sizeof(sa_buf) + BUF_SIZE);
   if (b != NULL) {
--- a/media/libsydneyaudio/src/sydney_audio_sunaudio.c
+++ b/media/libsydneyaudio/src/sydney_audio_sunaudio.c
@@ -46,34 +46,56 @@
 #include <pthread.h>
 #include "sydney_audio.h"
 
 #define DEFAULT_AUDIO_DEVICE "/dev/audio" 
 
 #define LOOP_WHILE_EINTR(v,func) do { (v) = (func); } \
                 while ((v) == -1 && errno == EINTR);
 
+typedef struct sa_buf sa_buf;
+struct sa_buf {
+  unsigned int      size; /* the size of data */
+  sa_buf            *next;
+  unsigned char     data[]; /* sound data */
+};
+
 struct sa_stream 
 {
-  int        audio_fd;
+  int               audio_fd;
+  pthread_mutex_t   mutex;
+  pthread_t         thread_id;
+  int               playing;
+  int64_t           bytes_played;
 
   /* audio format info */
   /* default setting */
   unsigned int      default_n_channels;
   unsigned int      default_rate;
   unsigned int      default_precision;
 
   /* used settings */
   unsigned int      rate;
   unsigned int      n_channels;
   unsigned int      precision;
-  int64_t           bytes_played;
 
+  /* buffer list */
+  sa_buf            *bl_head;
+  sa_buf            *bl_tail;
 };
 
+/* Use a default buffer size with enough room for one second of audio,
+ * assuming stereo data at 44.1kHz with 32 bits per channel, and impose
+ * a generous limit on the number of buffers.
+ */
+#define BUF_SIZE    (2 * 44100 * 4)
+
+static void* audio_callback(void* s);
+
+static sa_buf *new_buffer(int size);
 
 /*
  * -----------------------------------------------------------------------------
  * Startup and shutdown functions
  * -----------------------------------------------------------------------------
  */
 
 int
@@ -101,22 +123,29 @@ sa_stream_create_pcm(
     return SA_ERROR_NOT_SUPPORTED;
 
   /*
    * Allocate the instance and required resources.
    */
   if ((s = malloc(sizeof(sa_stream_t))) == NULL) 
     return SA_ERROR_OOM;
 
+  if (pthread_mutex_init(&s->mutex, NULL) != 0) {
+    free(s);
+    return SA_ERROR_SYSTEM;
+  }
+
   s->audio_fd = NULL;
-
   s->rate = rate;
   s->n_channels = n_channels;
   s->precision = 16;
+
+  s->playing = 0;
   s->bytes_played = 0;
+  s->bl_tail = s->bl_head = NULL;
 
   *_s = s;
 
   return SA_SUCCESS;
 }
 
 
 int
@@ -192,114 +221,226 @@ sa_stream_open(sa_stream_t *s)
   s->audio_fd = fd;
 
   return SA_SUCCESS;
 }
 
 int
 sa_stream_destroy(sa_stream_t *s) 
 {
-  int result = SA_SUCCESS;
+  int result; 
 
   if (s == NULL) 
     return SA_SUCCESS;
 
+
+  pthread_mutex_lock(&s->mutex);
+
+  result = SA_SUCCESS;
+
   /*
    * Shut down the audio output device.
+   * and release resources
    */
   if (s->audio_fd != NULL) 
   {
     if (close(s->audio_fd) < 0) 
     {
       perror("Close sun audio fd failed");
       result = SA_ERROR_SYSTEM;
     }
   }
 
+  s->thread_id = 0;
+
+  while (s->bl_head != NULL) {
+    sa_buf  * next = s->bl_head->next;
+    free(s->bl_head);
+    s->bl_head = next;
+  }
+
+  pthread_mutex_unlock(&s->mutex);
+
+  if (pthread_mutex_destroy(&s->mutex) != 0) {
+    result = SA_ERROR_SYSTEM;
+  }
+
   free(s);
 
   return result;
 }
 
 /*
  * -----------------------------------------------------------------------------
  * Data read and write functions
  * -----------------------------------------------------------------------------
  */
 
 int
 sa_stream_write(sa_stream_t *s, const void *data, size_t nbytes) 
 {
 
-  int result = SA_SUCCESS;
-  int total = 0;
-  int bytes = 0;
-  int fd;
-  int i;
-  audio_info_t ainfo;
+  int result;
+  sa_buf *buf;
 
   if (s == NULL || s->audio_fd == NULL) 
     return SA_ERROR_NO_INIT;
 
   if (nbytes == 0) 
     return SA_SUCCESS;
 
-  fd = s->audio_fd;
+
+ /*
+  * Append the new data to the end of our buffer list.
+  */
+  result = SA_SUCCESS;
+  buf = new_buffer(nbytes);
+  memcpy(buf->data,data, nbytes);
 
-  while (total < nbytes ) 
-  {
-    LOOP_WHILE_EINTR(bytes,(write(fd, (void *)(((unsigned char *)data)  total), nbytes-total)));
+  if ( buf == NULL)
+    return SA_ERROR_OOM;
+
+  pthread_mutex_lock(&s->mutex);
+  if (!s->bl_head)
+    s->bl_head = buf;
+  else
+    s->bl_tail->next = buf;
 
-    total = bytes;
-    if (total != nbytes)
-      printf("SunAudio\tWrite completed short - %d vs %d. Write more data\n",total,nbytes);
-  }
+  s->bl_tail = buf;
+
+  pthread_mutex_unlock(&s->mutex);
 
-  s->bytes_played += nbytes;
+ /*
+  * 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) {
+    s->playing = 1;
+    if (pthread_create(&s->thread_id, NULL, audio_callback, s) != 0) {
+      result = SA_ERROR_SYSTEM;
+    }
+  } 
 
   return result;
 }
 
+static void* 
+audio_callback(void* data)
+{
+  sa_stream_t* s = (sa_stream_t*)data;
+  sa_buf *buf;
+  int fd,nbytes_written,bytes,nbytes;
 
+  fd = s->audio_fd;
+
+  while (1)
+  { 
+    if (s->thread_id == 0)
+      break;
+
+    pthread_mutex_lock(&s->mutex);
+    while (s->bl_head) 
+    {
+      buf = s->bl_head;
+      s->bl_head = s->bl_head->next;
+
+      nbytes_written = 0; 
+      nbytes = buf->size;
+
+      while (nbytes_written < nbytes)
+      {
+        LOOP_WHILE_EINTR(bytes,(write(fd, (void *)((buf->data)+nbytes_written), nbytes-nbytes_written)));
+
+        nbytes_written += bytes;
+        if (nbytes_written != nbytes)
+          printf("SunAudio\tWrite completed short - %d vs %d. Write more data\n",nbytes_written,nbytes);
+      }
+
+      free(buf);
+      s->bytes_played += nbytes;
+     }
+     pthread_mutex_unlock(&s->mutex);
+   }
+
+  return NULL;
+}
 
 /*
  * -----------------------------------------------------------------------------
  * General query and support functions
  * -----------------------------------------------------------------------------
  */
 
 int
-sa_stream_get_position(sa_stream_t *s, sa_position_t position, int64_t *pos) 
+sa_stream_get_write_size(sa_stream_t *s, size_t *size) 
 {
+  sa_buf  * b;
+  size_t    used = 0;
 
-  if (s == NULL || s->audio_fd == NULL) 
+  if (s == NULL ) 
     return SA_ERROR_NO_INIT;
 
-  if (position != SA_POSITION_WRITE_SOFTWARE) 
-    return SA_ERROR_NOT_SUPPORTED;
+  /* there is no interface to get the avaiable writing buffer size
+   * in sun audio, we return max size here to force sa_stream_write() to
+   * be called when there is data to be played
+   */
+  *size = BUF_SIZE; 
+
+  return SA_SUCCESS;
+}
+
+/* ---------------------------------------------------------------------------
+ * General query and support functions
+ * -----------------------------------------------------------------------------
+ */
 
+int
+sa_stream_get_position(sa_stream_t *s, sa_position_t position, int64_t *pos) 
+{
+  if (s == NULL) {
+    return SA_ERROR_NO_INIT;
+  }
+  if (position != SA_POSITION_WRITE_SOFTWARE) {
+    return SA_ERROR_NOT_SUPPORTED;
+  }
+
+  pthread_mutex_lock(&s->mutex);
   *pos = s->bytes_played;
+  pthread_mutex_unlock(&s->mutex);
   return SA_SUCCESS;
 }
 
+static sa_buf *
+new_buffer(int size) 
+{
+  sa_buf  * b = malloc(sizeof(sa_buf) + size);
+  if (b != NULL) {
+    b->size  = size;
+    b->next  = NULL;
+  }
+  return b;
+}
+
 /*
  * -----------------------------------------------------------------------------
  * Extension functions
  * -----------------------------------------------------------------------------
  */
 
 int
 sa_stream_set_volume_abs(sa_stream_t *s, float vol) 
 {
   unsigned int newVolume = 0;
   int err;
   audio_info_t audio_info;
 
 
-  newVolume = (AUDIO_MAX_GAIN-AUDIO_MIN_GAIN)*volAUDIO_MIN_GAIN;
+  newVolume = (AUDIO_MAX_GAIN-AUDIO_MIN_GAIN)*vol+AUDIO_MIN_GAIN;
 
   /* Check if the new volume is valid or not */
   if ( newVolume < AUDIO_MIN_GAIN || newVolume > AUDIO_MAX_GAIN )
     return SA_ERROR_INVALID;
 
   AUDIO_INITINFO(&audio_info);
   audio_info.play.gain = newVolume;
   err=ioctl(s->audio_fd,AUDIO_SETINFO,&audio_info);    // The actual setting of the parameters
--- a/media/libsydneyaudio/src/sydney_audio_waveapi.c
+++ b/media/libsydneyaudio/src/sydney_audio_waveapi.c
@@ -218,16 +218,18 @@ int sa_stream_get_write_size(sa_stream_t
 /** Close/destroy everything */
 int sa_stream_destroy(sa_stream_t *s) {
   int status;
 
   ERROR_IF_NO_INIT(s);
   /* close and release all allocated resources */
   status = closeAudio(s);
 
+  free(s);
+
   return status;
 }
 
 #define LEFT_CHANNEL_MASK 0x0000FFFF
 #define RIGHT_CHANNEL_MASK 0xFFFF0000
 
 /** 
  * retrieved volume as an int in a scale from 0x0000 to 0xFFFF