Skip to content

Commit

Permalink
Added a safer way to pause system audio
Browse files Browse the repository at this point in the history 
The AAudio driver implemented pause/resume by dangerously locking the audio devices. If there was an audio hotplug event or a background thread tried to interact with the audio system, this could cause deadlocks.

This implements a safer way to pause and resume all system audio, without taking audio device locks.
  • Loading branch information
@slouken
slouken committed Jul 26, 2024
1 parent a1a8278 commit a2d2d26
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 97 deletions.
92 changes: 61 additions & 31 deletions src/audio/SDL_audio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
#include "SDL_audio_c.h"
#include "SDL_sysaudio.h"
#include "../thread/SDL_systhread.h"
#include "aaudio/SDL_aaudio.h"
#include "openslES/SDL_openslES.h"

// Available audio drivers
static const AudioBootStrap *const bootstrap[] = {
Expand Down Expand Up @@ -99,6 +101,7 @@ static const AudioBootStrap *const bootstrap[] = {
};

static SDL_AudioDriver current_audio;
static SDL_bool SDL_system_audio_paused;

// Deduplicated list of audio bootstrap drivers.
static const AudioBootStrap *deduped_bootstrap[SDL_arraysize(bootstrap) - 1];
Expand Down Expand Up @@ -1120,7 +1123,7 @@ SDL_bool SDL_PlaybackAudioThreadIterate(SDL_AudioDevice *device)
// We should have updated this elsewhere if the format changed!
SDL_assert(SDL_AudioSpecsEqual(&stream->dst_spec, &device->spec, stream->dst_chmap, device->chmap));

const int br = SDL_AtomicGet(&logdev->paused) ? 0 : SDL_GetAudioStreamDataAdjustGain(stream, device_buffer, buffer_size, logdev->gain);
const int br = (SDL_system_audio_paused || SDL_AtomicGet(&logdev->paused)) ? 0 : SDL_GetAudioStreamDataAdjustGain(stream, device_buffer, buffer_size, logdev->gain);
if (br < 0) { // Probably OOM. Kill the audio device; the whole thing is likely dying soon anyhow.
failed = SDL_TRUE;
SDL_memset(device_buffer, device->silence_value, buffer_size); // just supply silence to the device before we die.
Expand All @@ -1140,39 +1143,41 @@ SDL_bool SDL_PlaybackAudioThreadIterate(SDL_AudioDevice *device)

SDL_memset(final_mix_buffer, '\0', work_buffer_size); // start with silence.

for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev; logdev = logdev->next) {
if (SDL_AtomicGet(&logdev->paused)) {
continue; // paused? Skip this logical device.
}

const SDL_AudioPostmixCallback postmix = logdev->postmix;
float *mix_buffer = final_mix_buffer;
if (postmix) {
mix_buffer = device->postmix_buffer;
SDL_memset(mix_buffer, '\0', work_buffer_size); // start with silence.
}
if (!SDL_system_audio_paused) {
for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev; logdev = logdev->next) {
if (SDL_AtomicGet(&logdev->paused)) {
continue; // paused? Skip this logical device.
}

for (SDL_AudioStream *stream = logdev->bound_streams; stream; stream = stream->next_binding) {
// We should have updated this elsewhere if the format changed!
SDL_assert(SDL_AudioSpecsEqual(&stream->dst_spec, &outspec, stream->dst_chmap, device->chmap));
const SDL_AudioPostmixCallback postmix = logdev->postmix;
float *mix_buffer = final_mix_buffer;
if (postmix) {
mix_buffer = device->postmix_buffer;
SDL_memset(mix_buffer, '\0', work_buffer_size); // start with silence.
}

/* this will hold a lock on `stream` while getting. We don't explicitly lock the streams
for iterating here because the binding linked list can only change while the device lock is held.
(we _do_ lock the stream during binding/unbinding to make sure that two threads can't try to bind
the same stream to different devices at the same time, though.) */
const int br = SDL_GetAudioStreamDataAdjustGain(stream, device->work_buffer, work_buffer_size, logdev->gain);
if (br < 0) { // Probably OOM. Kill the audio device; the whole thing is likely dying soon anyhow.
failed = SDL_TRUE;
break;
} else if (br > 0) { // it's okay if we get less than requested, we mix what we have.
MixFloat32Audio(mix_buffer, (float *) device->work_buffer, br);
for (SDL_AudioStream *stream = logdev->bound_streams; stream; stream = stream->next_binding) {
// We should have updated this elsewhere if the format changed!
SDL_assert(SDL_AudioSpecsEqual(&stream->dst_spec, &outspec, stream->dst_chmap, device->chmap));

/* this will hold a lock on `stream` while getting. We don't explicitly lock the streams
for iterating here because the binding linked list can only change while the device lock is held.
(we _do_ lock the stream during binding/unbinding to make sure that two threads can't try to bind
the same stream to different devices at the same time, though.) */
const int br = SDL_GetAudioStreamDataAdjustGain(stream, device->work_buffer, work_buffer_size, logdev->gain);
if (br < 0) { // Probably OOM. Kill the audio device; the whole thing is likely dying soon anyhow.
failed = SDL_TRUE;
break;
} else if (br > 0) { // it's okay if we get less than requested, we mix what we have.
MixFloat32Audio(mix_buffer, (float *) device->work_buffer, br);
}
}
}

if (postmix) {
SDL_assert(mix_buffer == device->postmix_buffer);
postmix(logdev->postmix_userdata, &outspec, mix_buffer, work_buffer_size);
MixFloat32Audio(final_mix_buffer, mix_buffer, work_buffer_size);
if (postmix) {
SDL_assert(mix_buffer == device->postmix_buffer);
postmix(logdev->postmix_userdata, &outspec, mix_buffer, work_buffer_size);
MixFloat32Audio(final_mix_buffer, mix_buffer, work_buffer_size);
}
}
}

Expand Down Expand Up @@ -1258,7 +1263,7 @@ SDL_bool SDL_RecordingAudioThreadIterate(SDL_AudioDevice *device)
int br = device->RecordDevice(device, device->work_buffer, device->buffer_size);
if (br < 0) { // uhoh, device failed for some reason!
failed = SDL_TRUE;
} else if (br > 0) { // queue the new data to each bound stream.
} else if (br > 0 && !SDL_system_audio_paused) { // queue the new data to each bound stream.
for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev; logdev = logdev->next) {
if (SDL_AtomicGet(&logdev->paused)) {
continue; // paused? Skip this logical device.
Expand Down Expand Up @@ -2414,6 +2419,31 @@ int SDL_AudioDeviceFormatChanged(SDL_AudioDevice *device, const SDL_AudioSpec *n
return retval;
}

// This is currently only used on Android.
// If we want to open this API up, we'll have to account for audio drivers that set ProvidesOwnCallbackThread.
void SDL_PauseSystemAudio(void)
{
if (!SDL_system_audio_paused) {
AAUDIO_PauseDevices();
OPENSLES_PauseDevices();
SDL_system_audio_paused = SDL_TRUE;
}
}

void SDL_ResumeSystemAudio(void)
{
if (SDL_system_audio_paused) {
AAUDIO_ResumeDevices();
OPENSLES_ResumeDevices();
SDL_system_audio_paused = SDL_FALSE;
}
}

SDL_bool SDL_SystemAudioPaused(void)
{
return SDL_system_audio_paused;
}

// This is an internal function, so SDL_PumpEvents() can check for pending audio device events.
// ("UpdateSubsystem" is the same naming that the other things that hook into PumpEvents use.)
void SDL_UpdateAudio(void)
Expand Down
4 changes: 4 additions & 0 deletions src/audio/SDL_audio_c.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@
#ifndef SDL_audio_c_h_
#define SDL_audio_c_h_

extern void SDL_PauseSystemAudio(void);
extern void SDL_ResumeSystemAudio(void);
extern SDL_bool SDL_SystemAudioPaused(void);

extern void SDL_UpdateAudio(void);

#endif /* SDL_audio_c_h_ */
4 changes: 0 additions & 4 deletions src/audio/aaudio/SDL_aaudio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,29 +440,25 @@ static SDL_bool PauseOneDevice(SDL_AudioDevice *device, void *userdata)
SDL_SetError("%s : %s", __func__, ctx.AAudio_convertResultToText(res));
}

SDL_LockMutex(device->lock);
hidden->resume = SDL_TRUE;
}
}
return SDL_FALSE; // keep enumerating.
}

// Pause (block) all non already paused audio devices by taking their mixer lock
void AAUDIO_PauseDevices(void)
{
if (ctx.handle) { // AAUDIO driver is used?
(void) SDL_FindPhysicalAudioDeviceByCallback(PauseOneDevice, NULL);
}
}

// Resume (unblock) all non already paused audio devices by releasing their mixer lock
static SDL_bool ResumeOneDevice(SDL_AudioDevice *device, void *userdata)
{
struct SDL_PrivateAudioData *hidden = device->hidden;
if (hidden) {
if (hidden->resume) {
hidden->resume = SDL_FALSE;
SDL_UnlockMutex(device->lock);
}

if (hidden->stream) {
Expand Down
41 changes: 0 additions & 41 deletions src/audio/android/SDL_androidaudio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@

struct SDL_PrivateAudioData
{
int resume; // Resume device if it was paused automatically
};

static SDL_AudioDevice *playbackDevice = NULL;
Expand Down Expand Up @@ -126,46 +125,6 @@ static void ANDROIDAUDIO_CloseDevice(SDL_AudioDevice *device)
}
}

// Pause (block) all non already paused audio devices by taking their mixer lock
void ANDROIDAUDIO_PauseDevices(void)
{
// TODO: Handle multiple devices?
struct SDL_PrivateAudioData *hidden;
if (playbackDevice && playbackDevice->hidden) {
hidden = (struct SDL_PrivateAudioData *)playbackDevice->hidden;
SDL_LockMutex(playbackDevice->lock);
hidden->resume = SDL_TRUE;
}

if (recordingDevice && recordingDevice->hidden) {
hidden = (struct SDL_PrivateAudioData *)recordingDevice->hidden;
SDL_LockMutex(recordingDevice->lock);
hidden->resume = SDL_TRUE;
}
}

// Resume (unblock) all non already paused audio devices by releasing their mixer lock
void ANDROIDAUDIO_ResumeDevices(void)
{
// TODO: Handle multiple devices?
struct SDL_PrivateAudioData *hidden;
if (playbackDevice && playbackDevice->hidden) {
hidden = (struct SDL_PrivateAudioData *)playbackDevice->hidden;
if (hidden->resume) {
hidden->resume = SDL_FALSE;
SDL_UnlockMutex(playbackDevice->lock);
}
}

if (recordingDevice && recordingDevice->hidden) {
hidden = (struct SDL_PrivateAudioData *)recordingDevice->hidden;
if (hidden->resume) {
hidden->resume = SDL_FALSE;
SDL_UnlockMutex(recordingDevice->lock);
}
}
}

static SDL_bool ANDROIDAUDIO_Init(SDL_AudioDriverImpl *impl)
{
// !!! FIXME: if on Android API < 24, DetectDevices and Deinitialize should be NULL and OnlyHasDefaultPlaybackDevice and OnlyHasDefaultRecordingDevice should be SDL_TRUE, since audio device enum and hotplug appears to require Android 7.0+.
Expand Down
12 changes: 0 additions & 12 deletions src/audio/android/SDL_androidaudio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,4 @@
#ifndef SDL_androidaudio_h_
#define SDL_androidaudio_h_

#ifdef SDL_AUDIO_DRIVER_ANDROID

void ANDROIDAUDIO_ResumeDevices(void);
void ANDROIDAUDIO_PauseDevices(void);

#else

static void ANDROIDAUDIO_ResumeDevices(void) {}
static void ANDROIDAUDIO_PauseDevices(void) {}

#endif

#endif // SDL_androidaudio_h_
12 changes: 3 additions & 9 deletions src/video/android/SDL_androidevents.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@
#include "../SDL_sysvideo.h"
#include "../../events/SDL_events_c.h"

#include "../../audio/android/SDL_androidaudio.h"
#include "../../audio/aaudio/SDL_aaudio.h"
#include "../../audio/openslES/SDL_openslES.h"
#include "../../audio/SDL_audio_c.h"


#ifdef SDL_VIDEO_OPENGL_EGL
Expand Down Expand Up @@ -99,18 +97,14 @@ void Android_InitEvents(void)

static void Android_PauseAudio(void)
{
ANDROIDAUDIO_PauseDevices();
OPENSLES_PauseDevices();
AAUDIO_PauseDevices();
SDL_PauseSystemAudio();
Android_PausedAudio = SDL_TRUE;
}

static void Android_ResumeAudio(void)
{
if (Android_PausedAudio) {
ANDROIDAUDIO_ResumeDevices();
OPENSLES_ResumeDevices();
AAUDIO_ResumeDevices();
SDL_ResumeSystemAudio();
Android_PausedAudio = SDL_FALSE;
}
}
Expand Down

0 comments on commit a2d2d26

Please sign in to comment.