From f6a280ab7f24cb40f41f78b71d1968a26bdcb2ed Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Fri, 7 Oct 2016 15:13:46 -0400 Subject: [PATCH] audio: Don't trust audio drivers to drain pending audio. This tends to be a frequent spot where drivers hang, and the waits were often unreliable in any case. Instead, our audio thread now alerts the driver that we're done streaming audio (which currently XAudio2 uses to alert the system not to warn about the impending underflow) and then SDL_Delay()'s for a duration that's reasonable to drain the DMA buffers before closing the device. --- src/audio/SDL_audio.c | 22 +++++++++++++--------- src/audio/SDL_sysaudio.h | 2 +- src/audio/arts/SDL_artsaudio.c | 8 -------- src/audio/directsound/SDL_directsound.c | 17 ----------------- src/audio/fusionsound/SDL_fsaudio.c | 8 -------- src/audio/psp/SDL_pspaudio.c | 1 - src/audio/pulseaudio/SDL_pulseaudio.c | 23 ----------------------- src/audio/qsa/SDL_qsa_audio.c | 19 ------------------- src/audio/sndio/SDL_sndioaudio.c | 8 +------- src/audio/winmm/SDL_winmm.c | 19 ------------------- src/audio/xaudio2/SDL_xaudio2.c | 22 ++++------------------ 11 files changed, 19 insertions(+), 130 deletions(-) diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c index d085cb90a8fc9..08b801ed875e8 100644 --- a/src/audio/SDL_audio.c +++ b/src/audio/SDL_audio.c @@ -191,11 +191,6 @@ SDL_AudioGetDeviceBuf_Default(_THIS) return NULL; } -static void -SDL_AudioWaitDone_Default(_THIS) -{ /* no-op. */ -} - static int SDL_AudioCaptureFromDevice_Default(_THIS, void *buffer, int buflen) { @@ -207,6 +202,11 @@ SDL_AudioFlushCapture_Default(_THIS) { /* no-op. */ } +static void +SDL_AudioPrepareToClose_Default(_THIS) +{ /* no-op. */ +} + static void SDL_AudioCloseDevice_Default(_THIS) { /* no-op. */ @@ -292,9 +292,9 @@ finish_audio_entry_points_init(void) FILL_STUB(PlayDevice); FILL_STUB(GetPendingBytes); FILL_STUB(GetDeviceBuf); - FILL_STUB(WaitDone); FILL_STUB(CaptureFromDevice); FILL_STUB(FlushCapture); + FILL_STUB(PrepareToClose); FILL_STUB(CloseDevice); FILL_STUB(LockDevice); FILL_STUB(UnlockDevice); @@ -715,6 +715,9 @@ SDL_FinalizeAudioDevice(SDL_AudioDevice *device) return; } + SDL_AtomicSet(&device->shutdown, 1); /* just in case. */ + SDL_AtomicSet(&device->enabled, 0); + /* lock/unlock here so we don't race if the audio thread saw the shutdown var without locking, and the thread that requested shutdown is now trying to unlock the mutex while we destroy it. Threading is hard. */ @@ -811,9 +814,10 @@ SDL_RunAudio(void *devicep) } } + current_audio.impl.PrepareToClose(device); + /* Wait for the audio to drain. */ - /* !!! FIXME: can we rename this WaitDrain? */ - current_audio.impl.WaitDone(device); + SDL_Delay(((device->spec.samples * 1000) / device->spec.freq) * 2); SDL_FinalizeAudioDevice(device); @@ -1153,7 +1157,7 @@ close_audio_device(SDL_AudioDevice * device) if (!device->iscapture) { const SDL_AudioSpec *spec = &device->spec; - delay = ((spec.samples * 1000) / spec.freq) * 2; + delay = ((spec->samples * 1000) / spec->freq) * 2; } /* Lock to make sure an audio callback doesn't fire after we return. diff --git a/src/audio/SDL_sysaudio.h b/src/audio/SDL_sysaudio.h index e6d797fcf8c7e..943169bf7c19e 100644 --- a/src/audio/SDL_sysaudio.h +++ b/src/audio/SDL_sysaudio.h @@ -79,9 +79,9 @@ typedef struct SDL_AudioDriverImpl void (*PlayDevice) (_THIS); int (*GetPendingBytes) (_THIS); Uint8 *(*GetDeviceBuf) (_THIS); - void (*WaitDone) (_THIS); int (*CaptureFromDevice) (_THIS, void *buffer, int buflen); void (*FlushCapture) (_THIS); + void (*PrepareToClose) (_THIS); /**< Called between run and draining wait for playback devices */ void (*CloseDevice) (_THIS); void (*LockDevice) (_THIS); void (*UnlockDevice) (_THIS); diff --git a/src/audio/arts/SDL_artsaudio.c b/src/audio/arts/SDL_artsaudio.c index a7e4aa601a7d0..6054e36b68f5b 100644 --- a/src/audio/arts/SDL_artsaudio.c +++ b/src/audio/arts/SDL_artsaudio.c @@ -185,13 +185,6 @@ ARTS_PlayDevice(_THIS) #endif } -static void -ARTS_WaitDone(_THIS) -{ - /* !!! FIXME: camp here until buffer drains... SDL_Delay(???); */ -} - - static Uint8 * ARTS_GetDeviceBuf(_THIS) { @@ -356,7 +349,6 @@ ARTS_Init(SDL_AudioDriverImpl * impl) impl->WaitDevice = ARTS_WaitDevice; impl->GetDeviceBuf = ARTS_GetDeviceBuf; impl->CloseDevice = ARTS_CloseDevice; - impl->WaitDone = ARTS_WaitDone; impl->Deinitialize = ARTS_Deinitialize; impl->OnlyHasDefaultOutputDevice = 1; diff --git a/src/audio/directsound/SDL_directsound.c b/src/audio/directsound/SDL_directsound.c index 55259e7e6b2cf..5d261c92eba20 100644 --- a/src/audio/directsound/SDL_directsound.c +++ b/src/audio/directsound/SDL_directsound.c @@ -307,22 +307,6 @@ DSOUND_GetDeviceBuf(_THIS) return (this->hidden->locked_buf); } -static void -DSOUND_WaitDone(_THIS) -{ - Uint8 *stream = DSOUND_GetDeviceBuf(this); - - /* Wait for the playing chunk to finish */ - if (stream != NULL) { - SDL_memset(stream, this->spec.silence, this->spec.size); - DSOUND_PlayDevice(this); - } - DSOUND_WaitDevice(this); - - /* Stop the looping sound buffer */ - IDirectSoundBuffer_Stop(this->hidden->mixbuf); -} - static int DSOUND_CaptureFromDevice(_THIS, void *buffer, int buflen) { @@ -600,7 +584,6 @@ DSOUND_Init(SDL_AudioDriverImpl * impl) impl->OpenDevice = DSOUND_OpenDevice; impl->PlayDevice = DSOUND_PlayDevice; impl->WaitDevice = DSOUND_WaitDevice; - impl->WaitDone = DSOUND_WaitDone; impl->GetDeviceBuf = DSOUND_GetDeviceBuf; impl->CaptureFromDevice = DSOUND_CaptureFromDevice; impl->FlushCapture = DSOUND_FlushCapture; diff --git a/src/audio/fusionsound/SDL_fsaudio.c b/src/audio/fusionsound/SDL_fsaudio.c index 00f015bba3e94..daa0f9dd66c40 100644 --- a/src/audio/fusionsound/SDL_fsaudio.c +++ b/src/audio/fusionsound/SDL_fsaudio.c @@ -151,13 +151,6 @@ SDL_FS_PlayDevice(_THIS) #endif } -static void -SDL_FS_WaitDone(_THIS) -{ - this->hidden->stream->Wait(this->hidden->stream, - this->hidden->mixsamples * FUSION_BUFFERS); -} - static Uint8 * SDL_FS_GetDeviceBuf(_THIS) @@ -319,7 +312,6 @@ SDL_FS_Init(SDL_AudioDriverImpl * impl) impl->WaitDevice = SDL_FS_WaitDevice; impl->GetDeviceBuf = SDL_FS_GetDeviceBuf; impl->CloseDevice = SDL_FS_CloseDevice; - impl->WaitDone = SDL_FS_WaitDone; impl->Deinitialize = SDL_FS_Deinitialize; impl->OnlyHasDefaultOutputDevice = 1; diff --git a/src/audio/psp/SDL_pspaudio.c b/src/audio/psp/SDL_pspaudio.c index d36190edcfdd0..bd3456d3ff7de 100644 --- a/src/audio/psp/SDL_pspaudio.c +++ b/src/audio/psp/SDL_pspaudio.c @@ -152,7 +152,6 @@ PSPAUDIO_Init(SDL_AudioDriverImpl * impl) impl->PlayDevice = PSPAUDIO_PlayDevice; impl->WaitDevice = PSPAUDIO_WaitDevice; impl->GetDeviceBuf = PSPAUDIO_GetDeviceBuf; - impl->WaitDone = PSPAUDIO_WaitDevice; impl->CloseDevice = PSPAUDIO_CloseDevice; impl->ThreadInit = PSPAUDIO_ThreadInit; diff --git a/src/audio/pulseaudio/SDL_pulseaudio.c b/src/audio/pulseaudio/SDL_pulseaudio.c index 64fc5d32632e1..9ced49d210fec 100644 --- a/src/audio/pulseaudio/SDL_pulseaudio.c +++ b/src/audio/pulseaudio/SDL_pulseaudio.c @@ -368,28 +368,6 @@ PULSEAUDIO_PlayDevice(_THIS) } } -static void -PULSEAUDIO_WaitDone(_THIS) -{ - if (SDL_AtomicGet(&this->enabled)) { - struct SDL_PrivateAudioData *h = this->hidden; - pa_operation *o = PULSEAUDIO_pa_stream_drain(h->stream, stream_operation_complete_no_op, NULL); - if (o) { - while (PULSEAUDIO_pa_operation_get_state(o) != PA_OPERATION_DONE) { - if (PULSEAUDIO_pa_context_get_state(h->context) != PA_CONTEXT_READY || - PULSEAUDIO_pa_stream_get_state(h->stream) != PA_STREAM_READY || - PULSEAUDIO_pa_mainloop_iterate(h->mainloop, 1, NULL) < 0) { - PULSEAUDIO_pa_operation_cancel(o); - break; - } - } - PULSEAUDIO_pa_operation_unref(o); - } - } -} - - - static Uint8 * PULSEAUDIO_GetDeviceBuf(_THIS) { @@ -776,7 +754,6 @@ PULSEAUDIO_Init(SDL_AudioDriverImpl * impl) impl->WaitDevice = PULSEAUDIO_WaitDevice; impl->GetDeviceBuf = PULSEAUDIO_GetDeviceBuf; impl->CloseDevice = PULSEAUDIO_CloseDevice; - impl->WaitDone = PULSEAUDIO_WaitDone; impl->Deinitialize = PULSEAUDIO_Deinitialize; impl->CaptureFromDevice = PULSEAUDIO_CaptureFromDevice; impl->FlushCapture = PULSEAUDIO_FlushCapture; diff --git a/src/audio/qsa/SDL_qsa_audio.c b/src/audio/qsa/SDL_qsa_audio.c index c8f9fa91addc5..149cad90adcd6 100644 --- a/src/audio/qsa/SDL_qsa_audio.c +++ b/src/audio/qsa/SDL_qsa_audio.c @@ -708,24 +708,6 @@ QSA_DetectDevices(void) } } -static void -QSA_WaitDone(_THIS) -{ - if (!this->hidden->iscapture) { - if (this->hidden->audio_handle != NULL) { - /* Wait till last fragment is played and stop channel */ - snd_pcm_plugin_flush(this->hidden->audio_handle, - SND_PCM_CHANNEL_PLAYBACK); - } - } else { - if (this->hidden->audio_handle != NULL) { - /* Discard all unread data and stop channel */ - snd_pcm_plugin_flush(this->hidden->audio_handle, - SND_PCM_CHANNEL_CAPTURE); - } - } -} - static void QSA_Deinitialize(void) { @@ -759,7 +741,6 @@ QSA_Init(SDL_AudioDriverImpl * impl) impl->PlayDevice = QSA_PlayDevice; impl->GetDeviceBuf = QSA_GetDeviceBuf; impl->CloseDevice = QSA_CloseDevice; - impl->WaitDone = QSA_WaitDone; impl->Deinitialize = QSA_Deinitialize; impl->LockDevice = NULL; impl->UnlockDevice = NULL; diff --git a/src/audio/sndio/SDL_sndioaudio.c b/src/audio/sndio/SDL_sndioaudio.c index 797204fce6ebd..fb7d78ff17f6b 100644 --- a/src/audio/sndio/SDL_sndioaudio.c +++ b/src/audio/sndio/SDL_sndioaudio.c @@ -170,16 +170,11 @@ SNDIO_GetDeviceBuf(_THIS) return this->hidden->mixbuf; } -static void -SNDIO_WaitDone(_THIS) -{ - SNDIO_sio_stop(this->hidden->dev); -} - static void SNDIO_CloseDevice(_THIS) { if ( this->hidden->dev != NULL ) { + SNDIO_sio_stop(this->hidden->dev); SNDIO_sio_close(this->hidden->dev); } SDL_free(this->hidden->mixbuf); @@ -304,7 +299,6 @@ SNDIO_Init(SDL_AudioDriverImpl * impl) impl->WaitDevice = SNDIO_WaitDevice; impl->PlayDevice = SNDIO_PlayDevice; impl->GetDeviceBuf = SNDIO_GetDeviceBuf; - impl->WaitDone = SNDIO_WaitDone; impl->CloseDevice = SNDIO_CloseDevice; impl->Deinitialize = SNDIO_Deinitialize; impl->OnlyHasDefaultOutputDevice = 1; /* !!! FIXME: sndio can handle multiple devices. */ diff --git a/src/audio/winmm/SDL_winmm.c b/src/audio/winmm/SDL_winmm.c index 1cf8020990da1..34f543ddb33f6 100644 --- a/src/audio/winmm/SDL_winmm.c +++ b/src/audio/winmm/SDL_winmm.c @@ -135,24 +135,6 @@ WINMM_PlayDevice(_THIS) this->hidden->next_buffer = (this->hidden->next_buffer + 1) % NUM_BUFFERS; } -static void -WINMM_WaitDone(_THIS) -{ - int i, left; - - do { - left = NUM_BUFFERS; - for (i = 0; i < NUM_BUFFERS; ++i) { - if (this->hidden->wavebuf[i].dwFlags & WHDR_DONE) { - --left; - } - } - if (left > 0) { - SDL_Delay(100); - } - } while (left > 0); -} - static int WINMM_CaptureFromDevice(_THIS, void *buffer, int buflen) { @@ -422,7 +404,6 @@ WINMM_Init(SDL_AudioDriverImpl * impl) impl->OpenDevice = WINMM_OpenDevice; impl->PlayDevice = WINMM_PlayDevice; impl->WaitDevice = WINMM_WaitDevice; - impl->WaitDone = WINMM_WaitDone; impl->GetDeviceBuf = WINMM_GetDeviceBuf; impl->CaptureFromDevice = WINMM_CaptureFromDevice; impl->FlushCapture = WINMM_FlushCapture; diff --git a/src/audio/xaudio2/SDL_xaudio2.c b/src/audio/xaudio2/SDL_xaudio2.c index 1724e31b39d4c..a18e5d24ec856 100644 --- a/src/audio/xaudio2/SDL_xaudio2.c +++ b/src/audio/xaudio2/SDL_xaudio2.c @@ -232,28 +232,14 @@ XAUDIO2_WaitDevice(_THIS) } static void -XAUDIO2_WaitDone(_THIS) +XAUDIO2_PrepareToClose(_THIS) { IXAudio2SourceVoice *source = this->hidden->source; - XAUDIO2_VOICE_STATE state; - SDL_assert(!SDL_AtomicGet(&this->enabled)); /* flag that stops playing. */ - IXAudio2SourceVoice_Discontinuity(source); -#if SDL_XAUDIO2_WIN8 - IXAudio2SourceVoice_GetState(source, &state, XAUDIO2_VOICE_NOSAMPLESPLAYED); -#else - IXAudio2SourceVoice_GetState(source, &state); -#endif - while (state.BuffersQueued > 0) { - SDL_SemWait(this->hidden->semaphore); -#if SDL_XAUDIO2_WIN8 - IXAudio2SourceVoice_GetState(source, &state, XAUDIO2_VOICE_NOSAMPLESPLAYED); -#else - IXAudio2SourceVoice_GetState(source, &state); -#endif + if (source) { + IXAudio2SourceVoice_Discontinuity(source); } } - static void XAUDIO2_CloseDevice(_THIS) { @@ -489,7 +475,7 @@ XAUDIO2_Init(SDL_AudioDriverImpl * impl) impl->OpenDevice = XAUDIO2_OpenDevice; impl->PlayDevice = XAUDIO2_PlayDevice; impl->WaitDevice = XAUDIO2_WaitDevice; - impl->WaitDone = XAUDIO2_WaitDone; + impl->PrepareToClose = XAUDIO2_PrepareToClose; impl->GetDeviceBuf = XAUDIO2_GetDeviceBuf; impl->CloseDevice = XAUDIO2_CloseDevice; impl->Deinitialize = XAUDIO2_Deinitialize;