From 05760b59d024c7dbc4bc28bc47a9d579bc6c483e Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Thu, 11 Jun 2026 13:42:14 -0400 Subject: [PATCH 1/2] audio: Don't force minimum device spec. Now we allow physical devices to reopen at a higher spec if necessary. Added test/testdynaudioreopen.c to stress test this change. Fixes #14558. --- src/audio/SDL_audio.c | 64 ++++++++++------ test/CMakeLists.txt | 1 + test/testdynaudioreopen.c | 151 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 21 deletions(-) create mode 100644 test/testdynaudioreopen.c diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c index eb5f6456e23fb..5ca607d855878 100644 --- a/src/audio/SDL_audio.c +++ b/src/audio/SDL_audio.c @@ -1810,15 +1810,54 @@ static bool OpenPhysicalAudioDevice(SDL_AudioDevice *device, const SDL_AudioSpec { SerializePhysicalDeviceClose(device); // make sure another thread that's closing didn't release the lock to let the device thread join... - if (device->currently_opened) { - return true; // we're already good. - } - // Just pretend to open a zombie device. It can still collect logical devices on a default device under the assumption they will all migrate when the default device is officially changed. if (SDL_GetAtomicInt(&device->zombie)) { return true; // Braaaaaaaaains. } + SDL_AudioSpec spec; + SDL_copyp(&spec, inspec ? inspec : &device->default_spec); + PrepareAudioFormat(device->recording, &spec); + + if (device->currently_opened) { + SDL_AudioSpec current; + SDL_copyp(¤t, &device->spec); + + // if something has already opened the device at a lower quality, attempt to reopen it with the new request. + // This prevents something intentionally low quality, like VoIP playback, from making the system sound bad + // because it opened the hardware before the CD-quality background music arrived. In theory this could cause + // an audio hitch, but it would be a one-time thing, and as we've learned from default device migration, not + // actually that painful in practice. + if ((SDL_AUDIO_BITSIZE(spec.format) <= SDL_AUDIO_BITSIZE(current.format)) && (spec.channels <= current.channels) && (spec.freq <= current.freq)) { + return true; // we're already good. + } + + // uhoh, have to reopen the device...choose the "better" values from each spec. + spec.format = (SDL_AUDIO_BITSIZE(spec.format) > SDL_AUDIO_BITSIZE(current.format)) ? spec.format : current.format; + spec.channels = SDL_max(spec.channels, current.channels); + spec.freq = SDL_max(spec.freq, current.freq); + + SDL_Log("AUDIO: attempt to reopen device '%s' at higher spec! (%s,%d,%d => %s,%d,%d)", device->name, SDL_GetAudioFormatName(current.format), current.channels, current.freq, SDL_GetAudioFormatName(spec.format), spec.channels, spec.freq); + ClosePhysicalAudioDevice(device); + if (!OpenPhysicalAudioDevice(device, &spec)) { + // no good, try to go back to our original spec... + if (!OpenPhysicalAudioDevice(device, ¤t)) { + // okay, _now_ we're in trouble. Report the device as disconnected, since we've just broken all the existing logical devices. :( + // !!! FIXME: the logical devices need a thread to run the zombie implementation. + SDL_AudioDeviceDisconnected(device); + return false; + } + } + + // adjust all the attached audio streams to the new format, send format change events, etc. + SDL_copyp(&spec, &device->spec); // save off whatever we ended up with. + SDL_copyp(&device->spec, ¤t); // put it back to what it was so the next function call doesn't return immediately. The next call will reset it properly. + SDL_AudioDeviceFormatChangedAlreadyLocked(device, &spec, device->sample_frames); // if this fails, it's probably because we're out of memory and didn't send the events, but the device is _probably_ functional! + + return true; // carry on with the reconfigured device! + } + + // These start with the backend's implementation, but we might swap them out with zombie versions later. device->WaitDevice = current_audio.impl.WaitDevice; device->PlayDevice = current_audio.impl.PlayDevice; @@ -1826,23 +1865,6 @@ static bool OpenPhysicalAudioDevice(SDL_AudioDevice *device, const SDL_AudioSpec device->WaitRecordingDevice = current_audio.impl.WaitRecordingDevice; device->RecordDevice = current_audio.impl.RecordDevice; device->FlushRecording = current_audio.impl.FlushRecording; - - SDL_AudioSpec spec; - SDL_copyp(&spec, inspec ? inspec : &device->default_spec); - PrepareAudioFormat(device->recording, &spec); - - /* We impose a simple minimum on device formats. This prevents something low quality, like an old game using S8/8000Hz audio, - from ruining a music thing playing at CD quality that tries to open later, or some VoIP library that opens for mono output - ruining your surround-sound game because it got there first. - These are just requests! The backend may change any of these values during OpenDevice method! */ - - const SDL_AudioFormat minimum_format = device->recording ? DEFAULT_AUDIO_RECORDING_FORMAT : DEFAULT_AUDIO_PLAYBACK_FORMAT; - const int minimum_channels = device->recording ? DEFAULT_AUDIO_RECORDING_CHANNELS : DEFAULT_AUDIO_PLAYBACK_CHANNELS; - const int minimum_freq = device->recording ? DEFAULT_AUDIO_RECORDING_FREQUENCY : DEFAULT_AUDIO_PLAYBACK_FREQUENCY; - - device->spec.format = (SDL_AUDIO_BITSIZE(minimum_format) >= SDL_AUDIO_BITSIZE(spec.format)) ? minimum_format : spec.format; - device->spec.channels = SDL_max(minimum_channels, spec.channels); - device->spec.freq = SDL_max(minimum_freq, spec.freq); device->sample_frames = SDL_GetDefaultSampleFramesFromFreq(device->spec.freq); SDL_UpdatedAudioDeviceFormat(device); // start this off sane. diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 074c21035da96..d458148d137a9 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -354,6 +354,7 @@ endif() add_sdl_test_executable(checkkeys SOURCES checkkeys.c NAME83 chkkeys) add_sdl_test_executable(loopwave NEEDS_RESOURCES TESTUTILS MAIN_CALLBACKS SOURCES loopwave.c) +add_sdl_test_executable(testdynaudioreopen NEEDS_RESOURCES TESTUTILS MAIN_CALLBACKS SOURCES testdynaudioreopen.c) add_sdl_test_executable(testsurround SOURCES testsurround.c NAME83 surround) add_sdl_test_executable(testresample NEEDS_RESOURCES SOURCES testresample.c NAME83 resample) add_sdl_test_executable(testaudioinfo SOURCES testaudioinfo.c NAME83 audioinf) diff --git a/test/testdynaudioreopen.c b/test/testdynaudioreopen.c new file mode 100644 index 0000000000000..de48042161ee6 --- /dev/null +++ b/test/testdynaudioreopen.c @@ -0,0 +1,151 @@ +#define SDL_MAIN_USE_CALLBACKS 1 +#include +#include + +static SDL_AudioStream *stream = NULL; +static Uint8 *wav_data = NULL; +static Uint32 wav_data_len = 0; +static SDL_AudioSpec dev_spec; +static Uint64 next_reopen = 0; +static int next_spec = 0; + +static const SDL_AudioSpec specs[] = { + /* mono */ + { SDL_AUDIO_U8, 1, 8000 }, { SDL_AUDIO_S8, 1, 8000 }, { SDL_AUDIO_S16, 1, 8000 }, { SDL_AUDIO_S32, 1, 8000 }, { SDL_AUDIO_F32, 1, 8000 }, { SDL_AUDIO_F32, 1, 8000 }, + { SDL_AUDIO_U8, 1, 11025 }, { SDL_AUDIO_S8, 1, 11025 }, { SDL_AUDIO_S16, 1, 11025 }, { SDL_AUDIO_S32, 1, 11025 }, { SDL_AUDIO_F32, 1, 11025 }, { SDL_AUDIO_F32, 1, 11025 }, + { SDL_AUDIO_U8, 1, 22050 }, { SDL_AUDIO_S8, 1, 22050 }, { SDL_AUDIO_S16, 1, 22050 }, { SDL_AUDIO_S32, 1, 22050 }, { SDL_AUDIO_F32, 1, 22050 }, { SDL_AUDIO_F32, 1, 22050 }, + { SDL_AUDIO_U8, 1, 44100 }, { SDL_AUDIO_S8, 1, 44100 }, { SDL_AUDIO_S16, 1, 44100 }, { SDL_AUDIO_S32, 1, 44100 }, { SDL_AUDIO_F32, 1, 44100 }, { SDL_AUDIO_F32, 1, 44100 }, + { SDL_AUDIO_U8, 1, 48000 }, { SDL_AUDIO_S8, 1, 48000 }, { SDL_AUDIO_S16, 1, 48000 }, { SDL_AUDIO_S32, 1, 48000 }, { SDL_AUDIO_F32, 1, 48000 }, { SDL_AUDIO_F32, 1, 48000 }, + { SDL_AUDIO_U8, 1, 96000 }, { SDL_AUDIO_S8, 1, 96000 }, { SDL_AUDIO_S16, 1, 96000 }, { SDL_AUDIO_S32, 1, 96000 }, { SDL_AUDIO_F32, 1, 96000 }, { SDL_AUDIO_F32, 1, 96000 }, + /* stereo */ + { SDL_AUDIO_U8, 2, 8000 }, { SDL_AUDIO_S8, 2, 8000 }, { SDL_AUDIO_S16, 2, 8000 }, { SDL_AUDIO_S32, 2, 8000 }, { SDL_AUDIO_F32, 2, 8000 }, { SDL_AUDIO_F32, 2, 8000 }, + { SDL_AUDIO_U8, 2, 11025 }, { SDL_AUDIO_S8, 2, 11025 }, { SDL_AUDIO_S16, 2, 11025 }, { SDL_AUDIO_S32, 2, 11025 }, { SDL_AUDIO_F32, 2, 11025 }, { SDL_AUDIO_F32, 2, 11025 }, + { SDL_AUDIO_U8, 2, 22050 }, { SDL_AUDIO_S8, 2, 22050 }, { SDL_AUDIO_S16, 2, 22050 }, { SDL_AUDIO_S32, 2, 22050 }, { SDL_AUDIO_F32, 2, 22050 }, { SDL_AUDIO_F32, 2, 22050 }, + { SDL_AUDIO_U8, 2, 44100 }, { SDL_AUDIO_S8, 2, 44100 }, { SDL_AUDIO_S16, 2, 44100 }, { SDL_AUDIO_S32, 2, 44100 }, { SDL_AUDIO_F32, 2, 44100 }, { SDL_AUDIO_F32, 2, 44100 }, + { SDL_AUDIO_U8, 2, 48000 }, { SDL_AUDIO_S8, 2, 48000 }, { SDL_AUDIO_S16, 2, 48000 }, { SDL_AUDIO_S32, 2, 48000 }, { SDL_AUDIO_F32, 2, 48000 }, { SDL_AUDIO_F32, 2, 48000 }, + { SDL_AUDIO_U8, 2, 96000 }, { SDL_AUDIO_S8, 2, 96000 }, { SDL_AUDIO_S16, 2, 96000 }, { SDL_AUDIO_S32, 2, 96000 }, { SDL_AUDIO_F32, 2, 96000 }, { SDL_AUDIO_F32, 2, 96000 }, +}; + +static const char *SpecToString(const SDL_AudioSpec *spec) +{ + static char buf[256]; + SDL_snprintf(buf, sizeof (buf), "%s, %d channel%s, %dHz", SDL_GetAudioFormatName(spec->format), spec->channels, (spec->channels == 1) ? "" : "s", spec->freq); + return buf; +} + +static SDL_AudioDeviceID open_device(void) +{ + SDL_AudioDeviceID devid = 0; + + while (next_spec < SDL_arraysize(specs)) { + const SDL_AudioSpec *spec = &specs[next_spec++]; + + if ((SDL_AUDIO_BITSIZE(spec->format) <= SDL_AUDIO_BITSIZE(dev_spec.format)) && + (spec->channels <= dev_spec.channels) && + (spec->freq <= dev_spec.freq)) { + continue; /* don't bother, it won't trigger a reopen. */ + } + + SDL_Log("Requesting device open at %s", SpecToString(spec)); + devid = SDL_OpenAudioDevice(SDL_AUDIO_DEVICE_DEFAULT_PLAYBACK, spec); + if (!devid) { + SDL_Log("Couldn't open audio device: %s", SDL_GetError()); + } else { + SDL_GetAudioDeviceFormat(devid, &dev_spec, NULL); + SDL_Log("Device actually opened at %s", SpecToString(&dev_spec)); + } + + next_reopen = SDL_GetTicks() + 2000; + break; + } + + return devid; +} + +SDL_AppResult SDL_AppInit(void **appstate, int argc, char *argv[]) +{ + SDL_AudioSpec wav_spec; + char *wav_path = NULL; + SDL_AudioDeviceID devid = 0; + + /* this doesn't have to run very much, so give up tons of CPU time between iterations. */ + SDL_SetHint(SDL_HINT_MAIN_CALLBACK_RATE, "5"); + + if (!SDL_Init(SDL_INIT_AUDIO)) { + SDL_Log("Couldn't initialize SDL: %s", SDL_GetError()); + return SDL_APP_FAILURE; + } + + SDL_Log("Using audio driver: %s", SDL_GetCurrentAudioDriver()); + + SDL_asprintf(&wav_path, "%ssample.wav", SDL_GetBasePath()); /* allocate a string of the full file path */ + if (!SDL_LoadWAV(wav_path, &wav_spec, &wav_data, &wav_data_len)) { + SDL_Log("Couldn't load .wav file: %s", SDL_GetError()); + return SDL_APP_FAILURE; + } + + SDL_free(wav_path); /* done with this string. */ + + devid = open_device(); + if (!devid) { + return SDL_APP_FAILURE; + } + + /* Create our audio stream in the same format as the .wav file. It'll convert to what the audio hardware wants. */ + stream = SDL_CreateAudioStream(&wav_spec, NULL); + if (!stream) { + SDL_Log("Couldn't create audio stream: %s", SDL_GetError()); + return SDL_APP_FAILURE; + } + + if (!SDL_BindAudioStream(devid, stream)) { + SDL_Log("Couldn't bind audio stream: %s", SDL_GetError()); + return SDL_APP_FAILURE; + } + + return SDL_APP_CONTINUE; /* carry on with the program! */ +} + +/* This function runs when a new event (mouse input, keypresses, etc) occurs. */ +SDL_AppResult SDL_AppEvent(void *appstate, SDL_Event *event) +{ + if (event->type == SDL_EVENT_QUIT) { + return SDL_APP_SUCCESS; /* end the program, reporting success to the OS. */ + } + return SDL_APP_CONTINUE; /* carry on with the program! */ +} + +/* This function runs once per frame, and is the heart of the program. */ +SDL_AppResult SDL_AppIterate(void *appstate) +{ + /* see if we need to feed the audio stream more data yet. + We're being lazy here, but if there's less than the entire wav file left to play, + just shove a whole copy of it into the queue, so we always have _tons_ of + data queued for playback. */ + if (SDL_GetAudioStreamQueued(stream) < (int)wav_data_len) { + /* feed more data to the stream. It will queue at the end, and trickle out as the hardware needs more data. */ + SDL_PutAudioStreamData(stream, wav_data, wav_data_len); + } + + /* we just open a new logical device and don't bind a stream to it, or even keep + a reference to it. We just want to force SDL to reopen the physical device at + a higher spec. */ + if (SDL_GetTicks() >= next_reopen) { + if (next_spec < SDL_arraysize(specs)) { + open_device(); /* if this fails, that's good data, but keep going. */ + } else { + SDL_Log("No more reopen attempts, we're done!"); + return SDL_APP_SUCCESS; + } + } + + return SDL_APP_CONTINUE; +} + +/* This function runs once at shutdown. */ +void SDL_AppQuit(void *appstate, SDL_AppResult result) +{ + SDL_free(wav_data); /* strictly speaking, this isn't necessary because the process is ending, but it's good policy. */ + /* SDL will clean up the stream and devices for us. */ +} + From 6b19d5558a725fd6ff61cf65126830c227bfc611 Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Thu, 11 Jun 2026 15:43:39 -0400 Subject: [PATCH 2/2] audio: Add some formal debug logging to device open/close attempts. --- src/audio/SDL_audio.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c index 5ca607d855878..63abe3ebc3366 100644 --- a/src/audio/SDL_audio.c +++ b/src/audio/SDL_audio.c @@ -109,6 +109,15 @@ static SDL_AudioDriver current_audio; // Deduplicated list of audio bootstrap drivers. static const AudioBootStrap *deduped_bootstrap[SDL_arraysize(bootstrap) - 1]; +static const char *GetShortAudioFormatName(SDL_AudioFormat fmt) +{ + const char *fmtstr = SDL_GetAudioFormatName(fmt); + if (fmtstr) { + fmtstr += 10; // skip "SDL_AUDIO_" + } + return fmtstr; +} + int SDL_GetNumAudioDrivers(void) { static int num_drivers = -1; @@ -1683,8 +1692,15 @@ static void SerializePhysicalDeviceClose(SDL_AudioDevice *device) // this expects the device lock to be held. static void ClosePhysicalAudioDevice(SDL_AudioDevice *device) { + SDL_assert(device != NULL); + SerializePhysicalDeviceClose(device); + if (device->currently_opened) { // there might be other cleanup even when closed, but only log this if we were fully up and running. + const char *devtypestr = device->recording ? "recording" : "playback"; + SDL_LogDebug(SDL_LOG_CATEGORY_AUDIO, "AUDIO: closing %s device '%s'", devtypestr, device->name); + } + SDL_SetAtomicInt(&device->shutdown, 1); // YOU MUST PROTECT KEY POINTS WITH SerializePhysicalDeviceClose() WHILE THE THREAD JOINS @@ -1815,6 +1831,8 @@ static bool OpenPhysicalAudioDevice(SDL_AudioDevice *device, const SDL_AudioSpec return true; // Braaaaaaaaains. } + const char *devtypestr = device->recording ? "recording" : "playback"; + SDL_AudioSpec spec; SDL_copyp(&spec, inspec ? inspec : &device->default_spec); PrepareAudioFormat(device->recording, &spec); @@ -1837,7 +1855,7 @@ static bool OpenPhysicalAudioDevice(SDL_AudioDevice *device, const SDL_AudioSpec spec.channels = SDL_max(spec.channels, current.channels); spec.freq = SDL_max(spec.freq, current.freq); - SDL_Log("AUDIO: attempt to reopen device '%s' at higher spec! (%s,%d,%d => %s,%d,%d)", device->name, SDL_GetAudioFormatName(current.format), current.channels, current.freq, SDL_GetAudioFormatName(spec.format), spec.channels, spec.freq); + SDL_LogDebug(SDL_LOG_CATEGORY_AUDIO, "AUDIO: attempt to reopen %s device '%s' at higher spec! (%s,%d,%d => %s,%d,%d)", devtypestr, device->name, GetShortAudioFormatName(current.format), current.channels, current.freq, GetShortAudioFormatName(spec.format), spec.channels, spec.freq); ClosePhysicalAudioDevice(device); if (!OpenPhysicalAudioDevice(device, &spec)) { // no good, try to go back to our original spec... @@ -1854,6 +1872,8 @@ static bool OpenPhysicalAudioDevice(SDL_AudioDevice *device, const SDL_AudioSpec SDL_copyp(&device->spec, ¤t); // put it back to what it was so the next function call doesn't return immediately. The next call will reset it properly. SDL_AudioDeviceFormatChangedAlreadyLocked(device, &spec, device->sample_frames); // if this fails, it's probably because we're out of memory and didn't send the events, but the device is _probably_ functional! + SDL_LogDebug(SDL_LOG_CATEGORY_AUDIO, "AUDIO: %s device '%s' is now at spec (%s,%d,%d)", devtypestr, device->name, GetShortAudioFormatName(device->spec.format), device->spec.channels, device->spec.freq); + return true; // carry on with the reconfigured device! } @@ -1868,8 +1888,11 @@ static bool OpenPhysicalAudioDevice(SDL_AudioDevice *device, const SDL_AudioSpec device->sample_frames = SDL_GetDefaultSampleFramesFromFreq(device->spec.freq); SDL_UpdatedAudioDeviceFormat(device); // start this off sane. + SDL_LogDebug(SDL_LOG_CATEGORY_AUDIO, "AUDIO: attempt to open %s device '%s' at spec (%s,%d,%d)", devtypestr, device->name, GetShortAudioFormatName(spec.format), spec.channels, spec.freq); + device->currently_opened = true; // mark this true even if impl.OpenDevice fails, so we know to clean up. if (!current_audio.impl.OpenDevice(device)) { + SDL_LogDebug(SDL_LOG_CATEGORY_AUDIO, "AUDIO: open of %s device '%s' failed: %s", devtypestr, device->name, SDL_GetError()); ClosePhysicalAudioDevice(device); // clean up anything the backend left half-initialized. return false; } @@ -1879,6 +1902,7 @@ static bool OpenPhysicalAudioDevice(SDL_AudioDevice *device, const SDL_AudioSpec // Allocate a scratch audio buffer device->work_buffer = (Uint8 *)SDL_aligned_alloc(SDL_GetSIMDAlignment(), device->work_buffer_size); if (!device->work_buffer) { + SDL_LogDebug(SDL_LOG_CATEGORY_AUDIO, "AUDIO: open of %s device '%s' failed: %s", devtypestr, device->name, SDL_GetError()); ClosePhysicalAudioDevice(device); return false; } @@ -1886,6 +1910,7 @@ static bool OpenPhysicalAudioDevice(SDL_AudioDevice *device, const SDL_AudioSpec if (device->spec.format != SDL_AUDIO_F32) { device->mix_buffer = (Uint8 *)SDL_aligned_alloc(SDL_GetSIMDAlignment(), device->work_buffer_size); if (!device->mix_buffer) { + SDL_LogDebug(SDL_LOG_CATEGORY_AUDIO, "AUDIO: open of %s device '%s' failed: %s", devtypestr, device->name, SDL_GetError()); ClosePhysicalAudioDevice(device); return false; } @@ -1898,11 +1923,14 @@ static bool OpenPhysicalAudioDevice(SDL_AudioDevice *device, const SDL_AudioSpec device->thread = SDL_CreateThread(device->recording ? RecordingAudioThread : PlaybackAudioThread, threadname, device); if (!device->thread) { + SDL_LogDebug(SDL_LOG_CATEGORY_AUDIO, "AUDIO: open of %s device '%s' failed: %s", devtypestr, device->name, SDL_GetError()); ClosePhysicalAudioDevice(device); return SDL_SetError("Couldn't create audio thread"); } } + SDL_LogDebug(SDL_LOG_CATEGORY_AUDIO, "AUDIO: attempt to open %s device '%s' succeeded! Opened at spec (%s,%d,%d)", devtypestr, device->name, GetShortAudioFormatName(device->spec.format), device->spec.channels, device->spec.freq); + return true; }