From 9e60a8994fbfd63a541848da2e30109c4f99da75 Mon Sep 17 00:00:00 2001
From: "Ryan C. Gordon" <[EMAIL REDACTED]>
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 fdfc376ae86a6..544f4b7a19285 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 bc1704b60eb42..db4c55f3fde3a 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 fcfe0afaed4a6..daf44da02b80e 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 a733052d3a0a2..d36c63ad87f46 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);