audio: Fix locking in backends that manage their own callback threads.

Otherwise you might get a race where an app pauses the device, but
the audio callback still manages to run after the pause is in place.
This commit is contained in:
Ryan C. Gordon 2022-05-20 21:07:25 -04:00
parent a95f5a792c
commit dc62fec5e9
No known key found for this signature in database
GPG Key ID: FA148B892AB48044
6 changed files with 59 additions and 60 deletions

View File

@ -522,8 +522,12 @@ static void
outputCallback(void *inUserData, AudioQueueRef inAQ, AudioQueueBufferRef inBuffer) outputCallback(void *inUserData, AudioQueueRef inAQ, AudioQueueBufferRef inBuffer)
{ {
SDL_AudioDevice *this = (SDL_AudioDevice *) inUserData; SDL_AudioDevice *this = (SDL_AudioDevice *) inUserData;
SDL_LockMutex(this->mixer_lock);
if (SDL_AtomicGet(&this->hidden->shutdown)) { if (SDL_AtomicGet(&this->hidden->shutdown)) {
return; /* don't do anything. */ SDL_UnlockMutex(this->mixer_lock);
return; /* don't do anything, since we don't even want to enqueue this buffer again. */
} }
if (!SDL_AtomicGet(&this->enabled) || SDL_AtomicGet(&this->paused)) { if (!SDL_AtomicGet(&this->enabled) || SDL_AtomicGet(&this->paused)) {
@ -536,10 +540,8 @@ outputCallback(void *inUserData, AudioQueueRef inAQ, AudioQueueBufferRef inBuffe
while (remaining > 0) { while (remaining > 0) {
if (SDL_AudioStreamAvailable(this->stream) == 0) { if (SDL_AudioStreamAvailable(this->stream) == 0) {
/* Generate the data */ /* Generate the data */
SDL_LockMutex(this->mixer_lock);
(*this->callbackspec.callback)(this->callbackspec.userdata, (*this->callbackspec.callback)(this->callbackspec.userdata,
this->hidden->buffer, this->hidden->bufferSize); this->hidden->buffer, this->hidden->bufferSize);
SDL_UnlockMutex(this->mixer_lock);
this->hidden->bufferOffset = 0; this->hidden->bufferOffset = 0;
SDL_AudioStreamPut(this->stream, this->hidden->buffer, this->hidden->bufferSize); SDL_AudioStreamPut(this->stream, this->hidden->buffer, this->hidden->bufferSize);
} }
@ -565,10 +567,8 @@ outputCallback(void *inUserData, AudioQueueRef inAQ, AudioQueueBufferRef inBuffe
UInt32 len; UInt32 len;
if (this->hidden->bufferOffset >= this->hidden->bufferSize) { if (this->hidden->bufferOffset >= this->hidden->bufferSize) {
/* Generate the data */ /* Generate the data */
SDL_LockMutex(this->mixer_lock);
(*this->callbackspec.callback)(this->callbackspec.userdata, (*this->callbackspec.callback)(this->callbackspec.userdata,
this->hidden->buffer, this->hidden->bufferSize); this->hidden->buffer, this->hidden->bufferSize);
SDL_UnlockMutex(this->mixer_lock);
this->hidden->bufferOffset = 0; this->hidden->bufferOffset = 0;
} }
@ -587,6 +587,8 @@ outputCallback(void *inUserData, AudioQueueRef inAQ, AudioQueueBufferRef inBuffe
AudioQueueEnqueueBuffer(this->hidden->audioQueue, inBuffer, 0, NULL); AudioQueueEnqueueBuffer(this->hidden->audioQueue, inBuffer, 0, NULL);
inBuffer->mAudioDataByteSize = inBuffer->mAudioDataBytesCapacity; inBuffer->mAudioDataByteSize = inBuffer->mAudioDataBytesCapacity;
SDL_UnlockMutex(this->mixer_lock);
} }
static void static void

View File

@ -28,6 +28,11 @@
#include <emscripten/emscripten.h> #include <emscripten/emscripten.h>
/* !!! FIXME: this currently expects that the audio callback runs in the main thread,
!!! FIXME: in intervals when the application isn't running, but that may not be
!!! FIXME: true always once pthread support becomes widespread. Revisit this code
!!! FIXME: at some point and see what needs to be done for that! */
static void static void
FeedAudioDevice(_THIS, const void *buf, const int buflen) FeedAudioDevice(_THIS, const void *buf, const int buflen)
{ {

View File

@ -49,21 +49,19 @@ FillSound(void *device, void *stream, size_t len,
SDL_AudioDevice *audio = (SDL_AudioDevice *) device; SDL_AudioDevice *audio = (SDL_AudioDevice *) device;
SDL_AudioCallback callback = audio->callbackspec.callback; SDL_AudioCallback callback = audio->callbackspec.callback;
SDL_LockMutex(audio->mixer_lock);
/* Only do something if audio is enabled */ /* Only do something if audio is enabled */
if (!SDL_AtomicGet(&audio->enabled) || SDL_AtomicGet(&audio->paused)) { if (!SDL_AtomicGet(&audio->enabled) || SDL_AtomicGet(&audio->paused)) {
if (audio->stream) { if (audio->stream) {
SDL_AudioStreamClear(audio->stream); SDL_AudioStreamClear(audio->stream);
} }
SDL_memset(stream, audio->spec.silence, len); SDL_memset(stream, audio->spec.silence, len);
return; } else {
}
SDL_assert(audio->spec.size == len); SDL_assert(audio->spec.size == len);
if (audio->stream == NULL) { /* no conversion necessary. */ if (audio->stream == NULL) { /* no conversion necessary. */
SDL_LockMutex(audio->mixer_lock);
callback(audio->callbackspec.userdata, (Uint8 *) stream, len); callback(audio->callbackspec.userdata, (Uint8 *) stream, len);
SDL_UnlockMutex(audio->mixer_lock);
} else { /* streaming/converting */ } else { /* streaming/converting */
const int stream_len = audio->callbackspec.size; const int stream_len = audio->callbackspec.size;
const int ilen = (int) len; const int ilen = (int) len;
@ -84,6 +82,9 @@ FillSound(void *device, void *stream, size_t len,
} }
} }
SDL_UnlockMutex(audio->mixer_lock);
}
static void static void
HAIKUAUDIO_CloseDevice(_THIS) HAIKUAUDIO_CloseDevice(_THIS)
{ {

View File

@ -50,7 +50,7 @@ static void nacl_audio_callback(void* stream, uint32_t buffer_size, PP_TimeDelta
SDL_AudioDevice* _this = (SDL_AudioDevice*) data; SDL_AudioDevice* _this = (SDL_AudioDevice*) data;
SDL_AudioCallback callback = _this->callbackspec.callback; SDL_AudioCallback callback = _this->callbackspec.callback;
SDL_LockMutex(private->mutex); /* !!! FIXME: is this mutex necessary? */ SDL_LockMutex(_this->mixer_lock);
/* Only do something if audio is enabled */ /* Only do something if audio is enabled */
if (!SDL_AtomicGet(&_this->enabled) || SDL_AtomicGet(&_this->paused)) { if (!SDL_AtomicGet(&_this->enabled) || SDL_AtomicGet(&_this->paused)) {
@ -58,15 +58,11 @@ static void nacl_audio_callback(void* stream, uint32_t buffer_size, PP_TimeDelta
SDL_AudioStreamClear(_this->stream); SDL_AudioStreamClear(_this->stream);
} }
SDL_memset(stream, _this->spec.silence, len); SDL_memset(stream, _this->spec.silence, len);
return; } else {
}
SDL_assert(_this->spec.size == len); SDL_assert(_this->spec.size == len);
if (_this->stream == NULL) { /* no conversion necessary. */ if (_this->stream == NULL) { /* no conversion necessary. */
SDL_LockMutex(_this->mixer_lock);
callback(_this->callbackspec.userdata, stream, len); callback(_this->callbackspec.userdata, stream, len);
SDL_UnlockMutex(_this->mixer_lock);
} else { /* streaming/converting */ } else { /* streaming/converting */
const int stream_len = _this->callbackspec.size; const int stream_len = _this->callbackspec.size;
while (SDL_AudioStreamAvailable(_this->stream) < len) { while (SDL_AudioStreamAvailable(_this->stream) < len) {
@ -84,8 +80,9 @@ static void nacl_audio_callback(void* stream, uint32_t buffer_size, PP_TimeDelta
SDL_memset(stream, _this->spec.silence, len); SDL_memset(stream, _this->spec.silence, len);
} }
} }
}
SDL_UnlockMutex(private->mutex); SDL_UnlockMutex(_this->mixer_lock);
} }
static void NACLAUDIO_CloseDevice(SDL_AudioDevice *device) { static void NACLAUDIO_CloseDevice(SDL_AudioDevice *device) {
@ -94,7 +91,6 @@ static void NACLAUDIO_CloseDevice(SDL_AudioDevice *device) {
SDL_PrivateAudioData *hidden = (SDL_PrivateAudioData *) device->hidden; SDL_PrivateAudioData *hidden = (SDL_PrivateAudioData *) device->hidden;
ppb_audio->StopPlayback(hidden->audio); ppb_audio->StopPlayback(hidden->audio);
SDL_DestroyMutex(hidden->mutex);
core->ReleaseResource(hidden->audio); core->ReleaseResource(hidden->audio);
} }
@ -109,7 +105,6 @@ NACLAUDIO_OpenDevice(_THIS, const char *devname) {
return SDL_OutOfMemory(); return SDL_OutOfMemory();
} }
private->mutex = SDL_CreateMutex();
_this->spec.freq = 44100; _this->spec.freq = 44100;
_this->spec.format = AUDIO_S16LSB; _this->spec.format = AUDIO_S16LSB;
_this->spec.channels = 2; _this->spec.channels = 2;

View File

@ -34,7 +34,6 @@
#define private _this->hidden #define private _this->hidden
typedef struct SDL_PrivateAudioData { typedef struct SDL_PrivateAudioData {
SDL_mutex* mutex;
PP_Resource audio; PP_Resource audio;
} SDL_PrivateAudioData; } SDL_PrivateAudioData;

View File

@ -910,6 +910,7 @@ output_callback(void *data)
* and run the callback with the work buffer to keep the callback * and run the callback with the work buffer to keep the callback
* firing regularly in case the audio is being used as a timer. * firing regularly in case the audio is being used as a timer.
*/ */
SDL_LockMutex(this->mixer_lock);
if (!SDL_AtomicGet(&this->paused)) { if (!SDL_AtomicGet(&this->paused)) {
if (SDL_AtomicGet(&this->enabled)) { if (SDL_AtomicGet(&this->enabled)) {
dst = spa_buf->datas[0].data; dst = spa_buf->datas[0].data;
@ -919,18 +920,13 @@ output_callback(void *data)
} }
if (!this->stream) { if (!this->stream) {
SDL_LockMutex(this->mixer_lock);
this->callbackspec.callback(this->callbackspec.userdata, dst, this->callbackspec.size); this->callbackspec.callback(this->callbackspec.userdata, dst, this->callbackspec.size);
SDL_UnlockMutex(this->mixer_lock);
} else { } else {
int got; int got;
/* Fire the callback until we have enough to fill a buffer */ /* Fire the callback until we have enough to fill a buffer */
while (SDL_AudioStreamAvailable(this->stream) < this->spec.size) { while (SDL_AudioStreamAvailable(this->stream) < this->spec.size) {
SDL_LockMutex(this->mixer_lock);
this->callbackspec.callback(this->callbackspec.userdata, this->work_buffer, this->callbackspec.size); this->callbackspec.callback(this->callbackspec.userdata, this->work_buffer, this->callbackspec.size);
SDL_UnlockMutex(this->mixer_lock);
SDL_AudioStreamPut(this->stream, this->work_buffer, this->callbackspec.size); SDL_AudioStreamPut(this->stream, this->work_buffer, this->callbackspec.size);
} }
@ -940,6 +936,7 @@ output_callback(void *data)
} else { } else {
SDL_memset(spa_buf->datas[0].data, this->spec.silence, this->spec.size); SDL_memset(spa_buf->datas[0].data, this->spec.silence, this->spec.size);
} }
SDL_UnlockMutex(this->mixer_lock);
spa_buf->datas[0].chunk->offset = 0; spa_buf->datas[0].chunk->offset = 0;
spa_buf->datas[0].chunk->stride = this->hidden->stride; spa_buf->datas[0].chunk->stride = this->hidden->stride;