From 9e60a8994fbfd63a541848da2e30109c4f99da75 Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Tue, 14 Jan 2025 16:35:04 -0500 Subject: [PATCH] audio: Allow streams to change the device-side channels maps. Fixes #11881. --- include/SDL3/SDL_audio.h | 20 +++++++++++++++++++- src/audio/SDL_audio.c | 38 +++++++++++++++++++++++++++++++++++--- src/audio/SDL_audiocvt.c | 13 ++++++++----- src/audio/SDL_sysaudio.h | 6 +++++- 4 files changed, 67 insertions(+), 10 deletions(-) diff --git a/include/SDL3/SDL_audio.h b/include/SDL3/SDL_audio.h index fdfc376ae8..544f4b7a19 100644 --- a/include/SDL3/SDL_audio.h +++ b/include/SDL3/SDL_audio.h @@ -942,7 +942,7 @@ extern SDL_DECLSPEC void SDLCALL SDL_CloseAudioDevice(SDL_AudioDeviceID devid); * Binding a stream to a device will set its output format for playback * devices, and its input format for recording devices, so they match the * device's settings. The caller is welcome to change the other end of the - * stream's format at any time. + * stream's format at any time with SDL_SetAudioStreamFormat(). * * \param devid an audio device to bind a stream to. * \param streams an array of audio streams to bind. @@ -1104,6 +1104,12 @@ extern SDL_DECLSPEC bool SDLCALL SDL_GetAudioStreamFormat(SDL_AudioStream *strea * next sound file, and start putting that new data while the previous sound * file is still queued, and everything will still play back correctly. * + * If a stream is bound to a device, then the format of the side of the stream + * bound to a device cannot be changed (src_spec for recording devices, + * dst_spec for playback devices). Attempts to make a change to this side + * will be ignored, but this will not report an error. The other side's format + * can be changed. + * * \param stream the stream the format is being changed. * \param src_spec the new format of the audio input; if NULL, it is not * changed. @@ -1298,6 +1304,11 @@ extern SDL_DECLSPEC int * SDLCALL SDL_GetAudioStreamOutputChannelMap(SDL_AudioSt * race condition hasn't changed the format while this call is setting the * channel map. * + * Unlike attempting to change the stream's format, the input channel map on a + * stream bound to a recording device is permitted to change at any time; any + * data added to the stream from the device after this call will have the new + * mapping, but previously-added data will still have the prior mapping. + * * \param stream the SDL_AudioStream to change. * \param chmap the new channel map, NULL to reset to default. * \param count The number of channels in the map. @@ -1349,6 +1360,13 @@ extern SDL_DECLSPEC bool SDLCALL SDL_SetAudioStreamInputChannelMap(SDL_AudioStre * race condition hasn't changed the format while this call is setting the * channel map. * + * Unlike attempting to change the stream's format, the output channel map on + * a stream bound to a recording device is permitted to change at any time; + * any data added to the stream after this call will have the new mapping, but + * previously-added data will still have the prior mapping. When the channel + * map doesn't match the hardware's channel layout, SDL will convert the data + * before feeding it to the device for playback. + * * \param stream the SDL_AudioStream to change. * \param chmap the new channel map, NULL to reset to default. * \param count The number of channels in the map. diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c index bc1704b60e..db4c55f3fd 100644 --- a/src/audio/SDL_audio.c +++ b/src/audio/SDL_audio.c @@ -281,6 +281,18 @@ bool SDL_AudioSpecsEqual(const SDL_AudioSpec *a, const SDL_AudioSpec *b, const i return true; } +bool SDL_AudioChannelMapsEqual(int channels, const int *channel_map_a, const int *channel_map_b) +{ + if (channel_map_a == channel_map_b) { + return true; + } else if ((channel_map_a != NULL) != (channel_map_b != NULL)) { + return false; + } else if (channel_map_a && (SDL_memcmp(channel_map_a, channel_map_b, sizeof (*channel_map_a) * channels) != 0)) { + return false; + } + return true; +} + // Zombie device implementation... @@ -1134,7 +1146,7 @@ bool SDL_PlaybackAudioThreadIterate(SDL_AudioDevice *device) SDL_AudioStream *stream = logdev->bound_streams; // We should have updated this elsewhere if the format changed! - SDL_assert(SDL_AudioSpecsEqual(&stream->dst_spec, &device->spec, stream->dst_chmap, device->chmap)); + SDL_assert(SDL_AudioSpecsEqual(&stream->dst_spec, &device->spec, NULL, NULL)); const int br = SDL_GetAtomicInt(&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. @@ -1143,6 +1155,12 @@ bool SDL_PlaybackAudioThreadIterate(SDL_AudioDevice *device) } else if (br < buffer_size) { SDL_memset(device_buffer + br, device->silence_value, buffer_size - br); // silence whatever we didn't write to. } + + // generally channel maps will line up, but if the audio stream's chmap has been explicitly changed, do a final swizzle to device layout. + if ((br > 0) && (!SDL_AudioChannelMapsEqual(device->spec.channels, stream->dst_chmap, device->chmap))) { + ConvertAudio(br / SDL_AUDIO_FRAMESIZE(device->spec), device_buffer, device->spec.format, device->spec.channels, NULL, + device_buffer, device->spec.format, device->spec.channels, device->chmap, NULL, 1.0f); + } } else { // need to actually mix (or silence the buffer) float *final_mix_buffer = (float *) ((device->spec.format == SDL_AUDIO_F32) ? device_buffer : device->mix_buffer); const int needed_samples = buffer_size / SDL_AUDIO_BYTESIZE(device->spec.format); @@ -1170,7 +1188,7 @@ bool SDL_PlaybackAudioThreadIterate(SDL_AudioDevice *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)); + SDL_assert(SDL_AudioSpecsEqual(&stream->dst_spec, &outspec, NULL, NULL)); /* 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. @@ -1181,6 +1199,11 @@ bool SDL_PlaybackAudioThreadIterate(SDL_AudioDevice *device) failed = true; break; } else if (br > 0) { // it's okay if we get less than requested, we mix what we have. + // generally channel maps will line up, but if the audio stream's chmap has been explicitly changed, do a final swizzle to device layout. + if (!SDL_AudioChannelMapsEqual(device->spec.channels, stream->dst_chmap, device->chmap)) { + ConvertAudio(br / SDL_AUDIO_FRAMESIZE(device->spec), device->work_buffer, device->spec.format, device->spec.channels, NULL, + device->work_buffer, device->spec.format, device->spec.channels, device->chmap, NULL, 1.0f); + } MixFloat32Audio(mix_buffer, (float *) device->work_buffer, br); } } @@ -1303,11 +1326,20 @@ bool SDL_RecordingAudioThreadIterate(SDL_AudioDevice *device) SDL_assert(stream->src_spec.channels == device->spec.channels); SDL_assert(stream->src_spec.freq == device->spec.freq); + void *final_buf = output_buffer; + + // generally channel maps will line up, but if the audio stream's chmap has been explicitly changed, do a final swizzle to stream layout. + if (!SDL_AudioChannelMapsEqual(device->spec.channels, stream->src_chmap, device->chmap)) { + final_buf = device->mix_buffer; // this is otherwise unused on recording devices, so it makes convenient scratch space here. + ConvertAudio(br / SDL_AUDIO_FRAMESIZE(device->spec), output_buffer, device->spec.format, device->spec.channels, NULL, + final_buf, device->spec.format, device->spec.channels, stream->src_chmap, NULL, 1.0f); + } + /* this will hold a lock on `stream` while putting. 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.) */ - if (!SDL_PutAudioStreamData(stream, output_buffer, br)) { + if (!SDL_PutAudioStreamData(stream, final_buf, br)) { // oh crud, we probably ran out of memory. This is possibly an overreaction to kill the audio device, but it's likely the whole thing is going down in a moment anyhow. failed = true; break; diff --git a/src/audio/SDL_audiocvt.c b/src/audio/SDL_audiocvt.c index fcfe0afaed..daf44da02b 100644 --- a/src/audio/SDL_audiocvt.c +++ b/src/audio/SDL_audiocvt.c @@ -154,7 +154,7 @@ static void SwizzleAudio(const int num_frames, void *dst, const void *src, int c { const int bitsize = (int) SDL_AUDIO_BITSIZE(fmt); - bool has_null_mappings = false; + bool has_null_mappings = false; // !!! FIXME: calculate this when setting the channel map instead. for (int i = 0; i < channels; i++) { if (map[i] == -1) { has_null_mappings = true; @@ -257,6 +257,11 @@ void ConvertAudio(int num_frames, const int dst_bitsize = (int) SDL_AUDIO_BITSIZE(dst_format); const int dst_sample_frame_size = (dst_bitsize / 8) * dst_channels; + const bool chmaps_match = (src_channels == dst_channels) && SDL_AudioChannelMapsEqual(src_channels, src_map, dst_map); + if (chmaps_match) { + src_map = dst_map = NULL; // NULL both these out so we don't do any unnecessary swizzling. + } + /* Type conversion goes like this now: - swizzle through source channel map to "standard" layout. - byteswap to CPU native format first if necessary. @@ -635,8 +640,6 @@ bool SetAudioStreamChannelMap(SDL_AudioStream *stream, const SDL_AudioSpec *spec // already have this map, don't allocate/copy it again. } else if (SDL_ChannelMapIsBogus(chmap, channels)) { result = SDL_SetError("Invalid channel mapping"); - } else if ((isinput != -1) && stream->bound_device && (!!isinput == !!stream->bound_device->physical_device->recording)) { - // quietly refuse to change the format of the end currently bound to a device. } else { if (SDL_ChannelMapIsDefault(chmap, channels)) { chmap = NULL; // just apply a default mapping. @@ -661,12 +664,12 @@ bool SetAudioStreamChannelMap(SDL_AudioStream *stream, const SDL_AudioSpec *spec bool SDL_SetAudioStreamInputChannelMap(SDL_AudioStream *stream, const int *chmap, int channels) { - return SetAudioStreamChannelMap(stream, &stream->src_spec, &stream->src_chmap, chmap, channels, true); + return SetAudioStreamChannelMap(stream, &stream->src_spec, &stream->src_chmap, chmap, channels, 1); } bool SDL_SetAudioStreamOutputChannelMap(SDL_AudioStream *stream, const int *chmap, int channels) { - return SetAudioStreamChannelMap(stream, &stream->dst_spec, &stream->dst_chmap, chmap, channels, false); + return SetAudioStreamChannelMap(stream, &stream->dst_spec, &stream->dst_chmap, chmap, channels, 0); } int *SDL_GetAudioStreamInputChannelMap(SDL_AudioStream *stream, int *count) diff --git a/src/audio/SDL_sysaudio.h b/src/audio/SDL_sysaudio.h index a733052d3a..d36c63ad87 100644 --- a/src/audio/SDL_sysaudio.h +++ b/src/audio/SDL_sysaudio.h @@ -125,9 +125,13 @@ extern void ConvertAudio(int num_frames, // Compare two SDL_AudioSpecs, return true if they match exactly. // Using SDL_memcmp directly isn't safe, since potential padding might not be initialized. -// either channel maps can be NULL for the default (and both should be if you don't care about them). +// either channel map can be NULL for the default (and both should be if you don't care about them). extern bool SDL_AudioSpecsEqual(const SDL_AudioSpec *a, const SDL_AudioSpec *b, const int *channel_map_a, const int *channel_map_b); +// See if two channel maps match +// either channel map can be NULL for the default (and both should be if you don't care about them). +extern bool SDL_AudioChannelMapsEqual(int channels, const int *channel_map_a, const int *channel_map_b); + // allocate+copy a channel map. extern int *SDL_ChannelMapDup(const int *origchmap, int channels);