Browse Source

audio: Mix multiple streams in float32 to prevent clipping.

This only does this work if actually mixing; if the physical device only
has a single stream bound to it, it'll just write the data to the hardware
without the extra drama.

Fixes #8123.
Ryan C. Gordon 1 year ago
parent
commit
fd7cd91dc9
3 changed files with 80 additions and 27 deletions
  1. 68 24
      src/audio/SDL_audio.c
  2. 2 2
      src/audio/SDL_audiocvt.c
  3. 10 1
      src/audio/SDL_sysaudio.h

+ 68 - 24
src/audio/SDL_audio.c

@@ -730,8 +730,8 @@ SDL_bool SDL_OutputAudioThreadIterate(SDL_AudioDevice *device)
 
 
     SDL_bool retval = SDL_TRUE;
     SDL_bool retval = SDL_TRUE;
     int buffer_size = device->buffer_size;
     int buffer_size = device->buffer_size;
-    Uint8 *mix_buffer = current_audio.impl.GetDeviceBuf(device, &buffer_size);
-    if (!mix_buffer) {
+    Uint8 *device_buffer = current_audio.impl.GetDeviceBuf(device, &buffer_size);
+    if (!device_buffer) {
         retval = SDL_FALSE;
         retval = SDL_FALSE;
     } else {
     } else {
         SDL_assert(buffer_size <= device->buffer_size);  // you can ask for less, but not more.
         SDL_assert(buffer_size <= device->buffer_size);  // you can ask for less, but not more.
@@ -739,59 +739,83 @@ SDL_bool SDL_OutputAudioThreadIterate(SDL_AudioDevice *device)
         switch (ChooseMixStrategy(device)) {
         switch (ChooseMixStrategy(device)) {
             case MIXSTRATEGY_SILENCE: {
             case MIXSTRATEGY_SILENCE: {
                 //SDL_Log("MIX STRATEGY: SILENCE");
                 //SDL_Log("MIX STRATEGY: SILENCE");
-                SDL_memset(mix_buffer, device->silence_value, buffer_size);
+                SDL_memset(device_buffer, device->silence_value, buffer_size);
                 break;
                 break;
             }
             }
 
 
             case MIXSTRATEGY_COPYONE: {
             case MIXSTRATEGY_COPYONE: {
                 //SDL_Log("MIX STRATEGY: COPYONE");
                 //SDL_Log("MIX STRATEGY: COPYONE");
-                SDL_assert(device->logical_devices != NULL);
-                SDL_assert(device->logical_devices->next == NULL);
-                SDL_assert(device->logical_devices->bound_streams != NULL);
-                SDL_assert(device->logical_devices->bound_streams->next_binding == NULL);
-                const int br = SDL_GetAudioStreamData(device->logical_devices->bound_streams, mix_buffer, buffer_size);
+                SDL_LogicalAudioDevice *logdev = device->logical_devices;
+                SDL_assert(logdev != NULL);
+                SDL_assert(logdev->next == NULL);
+                SDL_assert(logdev->bound_streams != NULL);
+                SDL_assert(logdev->bound_streams->next_binding == NULL);
+
+                SDL_AudioStream *stream = logdev->bound_streams;
+                SDL_SetAudioStreamFormat(stream, NULL, &device->spec);
+                const int br = SDL_GetAudioStreamData(stream, device_buffer, buffer_size);
                 if (br < 0) {  // Probably OOM. Kill the audio device; the whole thing is likely dying soon anyhow.
                 if (br < 0) {  // Probably OOM. Kill the audio device; the whole thing is likely dying soon anyhow.
                     retval = SDL_FALSE;
                     retval = SDL_FALSE;
-                    SDL_memset(mix_buffer, device->silence_value, buffer_size);  // just supply silence to the device before we die.
+                    SDL_memset(device_buffer, device->silence_value, buffer_size);  // just supply silence to the device before we die.
                 } else if (br < buffer_size) {
                 } else if (br < buffer_size) {
-                    SDL_memset(mix_buffer + br, device->silence_value, buffer_size - br);  // silence whatever we didn't write to.
+                    SDL_memset(device_buffer + br, device->silence_value, buffer_size - br);  // silence whatever we didn't write to.
                 }
                 }
                 break;
                 break;
             }
             }
 
 
             case MIXSTRATEGY_MIX: {
             case MIXSTRATEGY_MIX: {
                 //SDL_Log("MIX STRATEGY: MIX");
                 //SDL_Log("MIX STRATEGY: MIX");
-                SDL_memset(mix_buffer, device->silence_value, buffer_size);  // start with silence.
+                float *mix_buffer = (float *) ((device->spec.format == SDL_AUDIO_F32) ? device_buffer : device->mix_buffer);
+                const int needed_samples = buffer_size / (SDL_AUDIO_BITSIZE(device->spec.format) / 8);
+                const int work_buffer_size = needed_samples * sizeof (float);
+                SDL_AudioSpec outspec;
+
+                SDL_assert(work_buffer_size <= device->work_buffer_size);
+
+                outspec.format = SDL_AUDIO_F32SYS;
+                outspec.channels = device->spec.channels;
+                outspec.freq = device->spec.freq;
+                outspec.format = SDL_AUDIO_F32SYS;
+
+                SDL_memset(mix_buffer, '\0', buffer_size);  // start with silence.
+
                 for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev != NULL; logdev = logdev->next) {
                 for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev != NULL; logdev = logdev->next) {
                     if (SDL_AtomicGet(&logdev->paused)) {
                     if (SDL_AtomicGet(&logdev->paused)) {
                         continue;  // paused? Skip this logical device.
                         continue;  // paused? Skip this logical device.
                     }
                     }
 
 
                     for (SDL_AudioStream *stream = logdev->bound_streams; stream != NULL; stream = stream->next_binding) {
                     for (SDL_AudioStream *stream = logdev->bound_streams; stream != NULL; stream = stream->next_binding) {
+                        SDL_SetAudioStreamFormat(stream, NULL, &outspec);
                         /* this will hold a lock on `stream` while getting. We don't explicitly lock the streams
                         /* 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.
                            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
                            (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.) */
                            the same stream to different devices at the same time, though.) */
-                        const int br = SDL_GetAudioStreamData(stream, device->work_buffer, buffer_size);
+                        const int br = SDL_GetAudioStreamData(stream, device->work_buffer, work_buffer_size);
                         if (br < 0) {  // Probably OOM. Kill the audio device; the whole thing is likely dying soon anyhow.
                         if (br < 0) {  // Probably OOM. Kill the audio device; the whole thing is likely dying soon anyhow.
                             retval = SDL_FALSE;
                             retval = SDL_FALSE;
                             break;
                             break;
                         } else if (br > 0) {  // it's okay if we get less than requested, we mix what we have.
                         } else if (br > 0) {  // it's okay if we get less than requested, we mix what we have.
-                            // !!! FIXME: this needs to mix to float32 or int32, so we don't clip.
-                            if (SDL_MixAudioFormat(mix_buffer, device->work_buffer, device->spec.format, br, SDL_MIX_MAXVOLUME) < 0) {  // !!! FIXME: allow streams to specify gain?
-                                SDL_assert(!"We probably ended up with some totally unexpected audio format here");
+                            if (SDL_MixAudioFormat((Uint8 *) mix_buffer, device->work_buffer, SDL_AUDIO_F32SYS, br, SDL_MIX_MAXVOLUME) < 0) {  // !!! FIXME: allow streams to specify gain?
+                                SDL_assert(!"This shouldn't happen.");
                                 retval = SDL_FALSE;  // uh...?
                                 retval = SDL_FALSE;  // uh...?
                                 break;
                                 break;
                             }
                             }
                         }
                         }
                     }
                     }
                 }
                 }
+
+                if (((Uint8 *) mix_buffer) != device_buffer) {
+                    // !!! FIXME: we can't promise the device buf is aligned/padded for SIMD.
+                    //ConvertAudio(needed_samples * device->spec.channels, mix_buffer, SDL_AUDIO_F32SYS, device->spec.channels, device_buffer, device->spec.format, device->spec.channels, device->work_buffer);
+                    ConvertAudio(needed_samples / device->spec.channels, mix_buffer, SDL_AUDIO_F32SYS, device->spec.channels, device->work_buffer, device->spec.format, device->spec.channels, NULL);
+                    SDL_memcpy(device_buffer, device->work_buffer, buffer_size);
+                }
                 break;
                 break;
             }
             }
         }
         }
 
 
         // PlayDevice SHOULD NOT BLOCK, as we are holding a lock right now. Block in WaitDevice instead!
         // PlayDevice SHOULD NOT BLOCK, as we are holding a lock right now. Block in WaitDevice instead!
-        if (current_audio.impl.PlayDevice(device, mix_buffer, buffer_size) < 0) {
+        if (current_audio.impl.PlayDevice(device, device_buffer, buffer_size) < 0) {
             retval = SDL_FALSE;
             retval = SDL_FALSE;
         }
         }
     }
     }
@@ -1148,10 +1172,11 @@ static void ClosePhysicalAudioDevice(SDL_AudioDevice *device)
         device->hidden = NULL;  // just in case.
         device->hidden = NULL;  // just in case.
     }
     }
 
 
-    if (device->work_buffer) {
-        SDL_aligned_free(device->work_buffer);
-        device->work_buffer = NULL;
-    }
+    SDL_aligned_free(device->work_buffer);
+    device->work_buffer = NULL;
+
+    SDL_aligned_free(device->mix_buffer);
+    device->mix_buffer = NULL;
 
 
     SDL_memcpy(&device->spec, &device->default_spec, sizeof (SDL_AudioSpec));
     SDL_memcpy(&device->spec, &device->default_spec, sizeof (SDL_AudioSpec));
     device->sample_frames = 0;
     device->sample_frames = 0;
@@ -1239,6 +1264,8 @@ void SDL_UpdatedAudioDeviceFormat(SDL_AudioDevice *device)
 {
 {
     device->silence_value = SDL_GetSilenceValueForFormat(device->spec.format);
     device->silence_value = SDL_GetSilenceValueForFormat(device->spec.format);
     device->buffer_size = device->sample_frames * (SDL_AUDIO_BITSIZE(device->spec.format) / 8) * device->spec.channels;
     device->buffer_size = device->sample_frames * (SDL_AUDIO_BITSIZE(device->spec.format) / 8) * device->spec.channels;
+    device->work_buffer_size = device->sample_frames * sizeof (float) * device->spec.channels;
+    device->work_buffer_size = SDL_max(device->buffer_size, device->work_buffer_size);  // just in case we end up with a 64-bit audio format at some point.
 }
 }
 
 
 char *SDL_GetAudioThreadName(SDL_AudioDevice *device, char *buf, size_t buflen)
 char *SDL_GetAudioThreadName(SDL_AudioDevice *device, char *buf, size_t buflen)
@@ -1282,12 +1309,20 @@ static int OpenPhysicalAudioDevice(SDL_AudioDevice *device, const SDL_AudioSpec
     SDL_UpdatedAudioDeviceFormat(device);  // in case the backend changed things and forgot to call this.
     SDL_UpdatedAudioDeviceFormat(device);  // in case the backend changed things and forgot to call this.
 
 
     // Allocate a scratch audio buffer
     // Allocate a scratch audio buffer
-    device->work_buffer = (Uint8 *)SDL_aligned_alloc(SDL_SIMDGetAlignment(), device->buffer_size);
+    device->work_buffer = (Uint8 *)SDL_aligned_alloc(SDL_SIMDGetAlignment(), device->work_buffer_size);
     if (device->work_buffer == NULL) {
     if (device->work_buffer == NULL) {
         ClosePhysicalAudioDevice(device);
         ClosePhysicalAudioDevice(device);
         return SDL_OutOfMemory();
         return SDL_OutOfMemory();
     }
     }
 
 
+    if (device->spec.format != SDL_AUDIO_F32SYS) {
+        device->mix_buffer = (Uint8 *)SDL_aligned_alloc(SDL_SIMDGetAlignment(), device->work_buffer_size);
+        if (device->mix_buffer == NULL) {
+            ClosePhysicalAudioDevice(device);
+            return SDL_OutOfMemory();
+        }
+    }
+
     // Start the audio thread if necessary
     // Start the audio thread if necessary
     SDL_AtomicSet(&device->thread_alive, 1);
     SDL_AtomicSet(&device->thread_alive, 1);
     if (!current_audio.impl.ProvidesOwnCallbackThread) {
     if (!current_audio.impl.ProvidesOwnCallbackThread) {
@@ -1765,7 +1800,7 @@ int SDL_AudioDeviceFormatChangedAlreadyLocked(SDL_AudioDevice *device, const SDL
 {
 {
     SDL_bool kill_device = SDL_FALSE;
     SDL_bool kill_device = SDL_FALSE;
 
 
-    const int orig_buffer_size = device->buffer_size;
+    const int orig_work_buffer_size = device->work_buffer_size;
     const SDL_bool iscapture = device->iscapture;
     const SDL_bool iscapture = device->iscapture;
 
 
     if ((device->spec.format != newspec->format) || (device->spec.channels != newspec->channels) || (device->spec.freq != newspec->freq)) {
     if ((device->spec.format != newspec->format) || (device->spec.channels != newspec->channels) || (device->spec.freq != newspec->freq)) {
@@ -1782,12 +1817,21 @@ int SDL_AudioDeviceFormatChangedAlreadyLocked(SDL_AudioDevice *device, const SDL
     if (!kill_device) {
     if (!kill_device) {
         device->sample_frames = new_sample_frames;
         device->sample_frames = new_sample_frames;
         SDL_UpdatedAudioDeviceFormat(device);
         SDL_UpdatedAudioDeviceFormat(device);
-        if (device->work_buffer && (device->buffer_size > orig_buffer_size)) {
+        if (device->work_buffer && (device->work_buffer_size > orig_work_buffer_size)) {
             SDL_aligned_free(device->work_buffer);
             SDL_aligned_free(device->work_buffer);
-            device->work_buffer = (Uint8 *)SDL_aligned_alloc(SDL_SIMDGetAlignment(), device->buffer_size);
+            device->work_buffer = (Uint8 *)SDL_aligned_alloc(SDL_SIMDGetAlignment(), device->work_buffer_size);
             if (!device->work_buffer) {
             if (!device->work_buffer) {
                 kill_device = SDL_TRUE;
                 kill_device = SDL_TRUE;
             }
             }
+
+            SDL_aligned_free(device->mix_buffer);
+            device->mix_buffer = NULL;
+            if (device->spec.format != SDL_AUDIO_F32SYS) {
+                device->mix_buffer = (Uint8 *)SDL_aligned_alloc(SDL_SIMDGetAlignment(), device->work_buffer_size);
+                if (!device->mix_buffer) {
+                    kill_device = SDL_TRUE;
+                }
+            }
         }
         }
     }
     }
 
 

+ 2 - 2
src/audio/SDL_audiocvt.c

@@ -488,8 +488,8 @@ static SDL_bool SDL_IsSupportedChannelCount(const int channels)
 //
 //
 // The scratch buffer must be able to store `num_frames * CalculateMaxSampleFrameSize(src_format, src_channels, dst_format, dst_channels)` bytes.
 // The scratch buffer must be able to store `num_frames * CalculateMaxSampleFrameSize(src_format, src_channels, dst_format, dst_channels)` bytes.
 // If the scratch buffer is NULL, this restriction applies to the output buffer instead.
 // If the scratch buffer is NULL, this restriction applies to the output buffer instead.
-static void ConvertAudio(int num_frames, const void *src, SDL_AudioFormat src_format, int src_channels,
-                         void *dst, SDL_AudioFormat dst_format, int dst_channels, void* scratch)
+void ConvertAudio(int num_frames, const void *src, SDL_AudioFormat src_format, int src_channels,
+                  void *dst, SDL_AudioFormat dst_format, int dst_channels, void* scratch)
 {
 {
     SDL_assert(src != NULL);
     SDL_assert(src != NULL);
     SDL_assert(dst != NULL);
     SDL_assert(dst != NULL);

+ 10 - 1
src/audio/SDL_sysaudio.h

@@ -114,6 +114,11 @@ extern SDL_bool SDL_CaptureAudioThreadIterate(SDL_AudioDevice *device);
 extern void SDL_CaptureAudioThreadShutdown(SDL_AudioDevice *device);
 extern void SDL_CaptureAudioThreadShutdown(SDL_AudioDevice *device);
 extern void SDL_AudioThreadFinalize(SDL_AudioDevice *device);
 extern void SDL_AudioThreadFinalize(SDL_AudioDevice *device);
 
 
+// this gets used from the audio device threads. It has rules, don't use this if you don't know how to use it!
+void ConvertAudio(int num_frames, const void *src, SDL_AudioFormat src_format, int src_channels,
+                  void *dst, SDL_AudioFormat dst_format, int dst_channels, void* scratch);
+
+
 typedef struct SDL_AudioDriverImpl
 typedef struct SDL_AudioDriverImpl
 {
 {
     void (*DetectDevices)(SDL_AudioDevice **default_output, SDL_AudioDevice **default_capture);
     void (*DetectDevices)(SDL_AudioDevice **default_output, SDL_AudioDevice **default_capture);
@@ -265,8 +270,12 @@ struct SDL_AudioDevice
     // SDL_TRUE if this is a capture device instead of an output device
     // SDL_TRUE if this is a capture device instead of an output device
     SDL_bool iscapture;
     SDL_bool iscapture;
 
 
-    // Scratch buffer used for mixing.
+    // Scratch buffers used for mixing.
     Uint8 *work_buffer;
     Uint8 *work_buffer;
+    Uint8 *mix_buffer;
+
+    // Size of work_buffer (and mix_buffer) in bytes.
+    int work_buffer_size;
 
 
     // A thread to feed the audio device
     // A thread to feed the audio device
     SDL_Thread *thread;
     SDL_Thread *thread;