From 94f1ba7c43cba91788bf889b8a6fa4bf6fe8cdc5 Mon Sep 17 00:00:00 2001 From: Chris Double Date: Tue, 4 Nov 2008 21:23:55 +1300 Subject: [PATCH] Bug461936 - Update to libsydneyaudio r3760 to fix deadlock when shutting down a stream - rs=roc --- media/libsydneyaudio/README_MOZILLA | 2 +- media/libsydneyaudio/src/sydney_audio_mac.c | 32 ++- .../src/sydney_audio_sunaudio.c | 191 +++++++++++++++--- .../libsydneyaudio/src/sydney_audio_waveapi.c | 2 + 4 files changed, 192 insertions(+), 35 deletions(-) diff --git a/media/libsydneyaudio/README_MOZILLA b/media/libsydneyaudio/README_MOZILLA index 9200cf9d042..802a6c325d4 100644 --- a/media/libsydneyaudio/README_MOZILLA +++ b/media/libsydneyaudio/README_MOZILLA @@ -5,4 +5,4 @@ the Mozilla build system. http://svn.annodex.net/libsydneyaudio/trunk -The svn revision number used was r3733. +The svn revision number used was r3760. diff --git a/media/libsydneyaudio/src/sydney_audio_mac.c b/media/libsydneyaudio/src/sydney_audio_mac.c index af8f434fbdb..e9f444e9720 100644 --- a/media/libsydneyaudio/src/sydney_audio_mac.c +++ b/media/libsydneyaudio/src/sydney_audio_mac.c @@ -260,10 +260,15 @@ sa_stream_destroy(sa_stream_t *s) { 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) { @@ -278,8 +283,6 @@ sa_stream_destroy(sa_stream_t *s) { } } - pthread_mutex_unlock(&s->mutex); - /* * Release resources. */ @@ -546,9 +549,14 @@ sa_stream_pause(sa_stream_t *s) { 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; } @@ -561,15 +569,21 @@ sa_stream_resume(sa_stream_t *s) { } 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; } diff --git a/media/libsydneyaudio/src/sydney_audio_sunaudio.c b/media/libsydneyaudio/src/sydney_audio_sunaudio.c index 7367231d379..162dc8a5413 100644 --- a/media/libsydneyaudio/src/sydney_audio_sunaudio.c +++ b/media/libsydneyaudio/src/sydney_audio_sunaudio.c @@ -51,9 +51,20 @@ #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 */ @@ -65,10 +76,21 @@ struct sa_stream 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); /* * ----------------------------------------------------------------------------- @@ -106,12 +128,19 @@ sa_stream_create_pcm( if ((s = malloc(sizeof(sa_stream_t))) == NULL) return SA_ERROR_OOM; - s->audio_fd = NULL; + 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; @@ -197,13 +226,19 @@ sa_stream_open(sa_stream_t *s) 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) { @@ -214,6 +249,20 @@ sa_stream_destroy(sa_stream_t *s) } } + 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; @@ -229,12 +278,8 @@ 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; @@ -242,23 +287,83 @@ sa_stream_write(sa_stream_t *s, const void *data, size_t nbytes) if (nbytes == 0) return SA_SUCCESS; - fd = s->audio_fd; - while (total < nbytes ) - { - LOOP_WHILE_EINTR(bytes,(write(fd, (void *)(((unsigned char *)data) total), nbytes-total))); + /* + * Append the new data to the end of our buffer list. + */ + result = SA_SUCCESS; + buf = new_buffer(nbytes); + memcpy(buf->data,data, nbytes); - total = bytes; - if (total != nbytes) - printf("SunAudio\tWrite completed short - %d vs %d. Write more data\n",total,nbytes); - } + if ( buf == NULL) + return SA_ERROR_OOM; - s->bytes_played += nbytes; + pthread_mutex_lock(&s->mutex); + if (!s->bl_head) + s->bl_head = buf; + else + s->bl_tail->next = buf; + + s->bl_tail = buf; + + pthread_mutex_unlock(&s->mutex); + + /* + * 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; +} /* * ----------------------------------------------------------------------------- @@ -267,19 +372,55 @@ sa_stream_write(sa_stream_t *s, const void *data, size_t nbytes) */ 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; - *pos = s->bytes_played; 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 @@ -294,7 +435,7 @@ sa_stream_set_volume_abs(sa_stream_t *s, float vol) 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 ) diff --git a/media/libsydneyaudio/src/sydney_audio_waveapi.c b/media/libsydneyaudio/src/sydney_audio_waveapi.c index 53df0fee4c8..edcfb6d8663 100644 --- a/media/libsydneyaudio/src/sydney_audio_waveapi.c +++ b/media/libsydneyaudio/src/sydney_audio_waveapi.c @@ -223,6 +223,8 @@ int sa_stream_destroy(sa_stream_t *s) { /* close and release all allocated resources */ status = closeAudio(s); + free(s); + return status; }