From ac2edad8090a4bf79d0bd11a5860c7c10ad73fb2 Mon Sep 17 00:00:00 2001
From: "Ryan C. Gordon" <[EMAIL REDACTED]>
Date: Fri, 13 Dec 2024 14:59:16 -0500
Subject: [PATCH] audio: Simplify and unify audio stream format updating when
devices change.
This used to do different things for recording streams in different parts of
the codebase, not to mention two separate codepaths through
UpdateAudioStreamFormatsPhysical in any case. My hope is this should kill off
a few corner case bugs...for example, this one:
Fixes #8402.
---
src/audio/SDL_audio.c | 77 +++++++++++++------------------------------
1 file changed, 23 insertions(+), 54 deletions(-)
diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c
index ada657be40472..e477a749cf15d 100644
--- a/src/audio/SDL_audio.c
+++ b/src/audio/SDL_audio.c
@@ -238,34 +238,35 @@ static void UpdateAudioStreamFormatsPhysical(SDL_AudioDevice *device)
return;
}
- if (device->recording) { // for recording devices, we only want to move to float32 for postmix and gain, which we'll handle elsewhere.
- // we _do_ need to make sure the channel map is correct, though...
- for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev; logdev = logdev->next) {
- for (SDL_AudioStream *stream = logdev->bound_streams; stream; stream = stream->next_binding) {
- // set the proper end of the stream to the device's channel map. This will obtain stream->lock itself.
- SetAudioStreamChannelMap(stream, &stream->src_spec, &stream->src_chmap, device->chmap, device->spec.channels, -1);
- }
- }
- } else {
- const bool simple_copy = AudioDeviceCanUseSimpleCopy(device);
- SDL_AudioSpec spec;
+ const bool recording = device->recording;
+ SDL_AudioSpec spec;
+ SDL_copyp(&spec, &device->spec);
- device->simple_copy = simple_copy;
- SDL_copyp(&spec, &device->spec);
+ const SDL_AudioFormat devformat = spec.format;
+ if (!recording) {
+ const bool simple_copy = AudioDeviceCanUseSimpleCopy(device);
+ device->simple_copy = simple_copy;
if (!simple_copy) {
spec.format = SDL_AUDIO_F32; // mixing and postbuf operates in float32 format.
}
+ }
- for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev; logdev = logdev->next) {
- for (SDL_AudioStream *stream = logdev->bound_streams; stream; stream = stream->next_binding) {
- // set the proper end of the stream to the device's format.
- // SDL_SetAudioStreamFormat does a ton of validation just to memcpy an audiospec.
- SDL_LockMutex(stream->lock);
- SDL_copyp(&stream->dst_spec, &spec);
- SetAudioStreamChannelMap(stream, &stream->dst_spec, &stream->dst_chmap, device->chmap, spec.channels, -1); // this should be fast for normal cases, though!
- SDL_UnlockMutex(stream->lock);
- }
+ for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev; logdev = logdev->next) {
+ if (recording) {
+ const bool need_float32 = (logdev->postmix || logdev->gain != 1.0f);
+ spec.format = need_float32 ? SDL_AUDIO_F32 : devformat;
+ }
+
+ for (SDL_AudioStream *stream = logdev->bound_streams; stream; stream = stream->next_binding) {
+ // set the proper end of the stream to the device's format.
+ // SDL_SetAudioStreamFormat does a ton of validation just to memcpy an audiospec.
+ SDL_AudioSpec *streamspec = recording ? &stream->src_spec : &stream->dst_spec;
+ int **streamchmap = recording ? &stream->src_chmap : &stream->dst_chmap;
+ SDL_LockMutex(stream->lock);
+ SDL_copyp(streamspec, &spec);
+ SetAudioStreamChannelMap(stream, streamspec, streamchmap, device->chmap, device->spec.channels, -1); // this should be fast for normal cases, though!
+ SDL_UnlockMutex(stream->lock);
}
}
}
@@ -1826,17 +1827,6 @@ bool SDL_SetAudioDeviceGain(SDL_AudioDeviceID devid, float gain)
bool result = false;
if (logdev) {
logdev->gain = gain;
- if (device->recording) {
- const bool need_float32 = (logdev->postmix || logdev->gain != 1.0f);
- for (SDL_AudioStream *stream = logdev->bound_streams; stream; stream = stream->next_binding) {
- // set the proper end of the stream to the device's format.
- // SDL_SetAudioStreamFormat does a ton of validation just to memcpy an audiospec.
- SDL_LockMutex(stream->lock);
- stream->src_spec.format = need_float32 ? SDL_AUDIO_F32 : device->spec.format;
- SDL_UnlockMutex(stream->lock);
- }
- }
-
UpdateAudioStreamFormatsPhysical(device);
result = true;
}
@@ -1860,17 +1850,6 @@ bool SDL_SetAudioPostmixCallback(SDL_AudioDeviceID devid, SDL_AudioPostmixCallba
if (result) {
logdev->postmix = callback;
logdev->postmix_userdata = userdata;
-
- if (device->recording) {
- const bool need_float32 = (callback || logdev->gain != 1.0f);
- for (SDL_AudioStream *stream = logdev->bound_streams; stream; stream = stream->next_binding) {
- // set the proper end of the stream to the device's format.
- // SDL_SetAudioStreamFormat does a ton of validation just to memcpy an audiospec.
- SDL_LockMutex(stream->lock);
- stream->src_spec.format = need_float32 ? SDL_AUDIO_F32 : device->spec.format;
- SDL_UnlockMutex(stream->lock);
- }
- }
}
UpdateAudioStreamFormatsPhysical(device);
@@ -1940,7 +1919,6 @@ bool SDL_BindAudioStreams(SDL_AudioDeviceID devid, SDL_AudioStream **streams, in
if (result) {
// Now that everything is verified, chain everything together.
- const bool recording = device->recording;
for (int i = 0; i < num_streams; i++) {
SDL_AudioStream *stream = streams[i];
if (stream) { // shouldn't be NULL, but just in case...
@@ -1951,15 +1929,6 @@ bool SDL_BindAudioStreams(SDL_AudioDeviceID devid, SDL_AudioStream **streams, in
logdev->bound_streams->prev_binding = stream;
}
logdev->bound_streams = stream;
-
- if (recording) {
- SDL_copyp(&stream->src_spec, &device->spec);
- if (logdev->postmix) {
- stream->src_spec.format = SDL_AUDIO_F32;
- }
- SDL_SetAudioStreamInputChannelMap(stream, device->chmap, device->spec.channels); // this should be fast for normal cases, though!
- }
-
SDL_UnlockMutex(stream->lock);
}
}