In 2.30, AUDIO_S8 produces clicking noises on linux

[randomouscrap98]

I've been having trouble with programs built using SDL 2.30, where the authors are still on 2.0.20 or some other 2.0 version. Building with their version of SDL (whatever it may be), I am able to build their programs and run them without any sound issues. However, regardless of whether I build SDL from source or get SDL from a package manager (like pacman), their programs produce an incessant clicking sound. After some trial and error, I found that I can get the absolute most basic program to produce this clicking sound when using SDL 2.30, while using SDL 2.0.20 or other versions that the authors are using does not produce this issue for this example program:

#include <SDL.h>

SDL_AudioSpec audio_spec;

void audio_callback(void *userdata, Uint8 *stream, int len) {
    // Fill the audio stream with silence
    memset(stream, audio_spec.silence, len);
}

int main() {
    // Initialize SDL
    if (SDL_Init(SDL_INIT_AUDIO) < 0) {
        // Handle initialization error
        return -1;
    }

    // Set the audio format (8khz 8-bit mono)
    audio_spec.freq = 8000;
    audio_spec.format = AUDIO_S8;
    audio_spec.channels = 1;
    audio_spec.samples = 1024;
    audio_spec.callback = audio_callback;

    // Open the audio device
    if (SDL_OpenAudio(&audio_spec, NULL) < 0) {
        // Handle audio device opening error
        return -1;
    }

    // Start playing audio
    SDL_PauseAudio(0);

    // Wait for some time (or perform other tasks)
    SDL_Delay(5000); // Wait for 5 seconds

    // Pause audio playback
    SDL_PauseAudio(1);

    // Close the audio device and quit SDL
    SDL_CloseAudio();
    SDL_Quit();

    return 0;
}

Here are some notes on testing:

• This bug occurs on 3 different computers (though they are all running some form of arch linux)
• The bug occurs whether I use the package or build from source
• Using AUDIO_S16SYS or other formats does not produce a clicking sound
• Changing the samples changes how frequently the clicks occur (greater samples = less frequent)
• Changing the freq changes how frequently the clicks occur and the pitch of the click (higher freq = more frequent + higher pitch, 44100 makes the clicks inaudible on my sound hardware (or just to my decayed ears))
• Both overfilling or underfilling the buffer produce the same issue
• Filing the buffer with actual sound will indeed play the sounds, but the clicking still persists over it all.

I can provide more information as needed. I'm aware this may be an arch-specific issue with some dependent library, but I get the feeling it has something to do with the timing of the callback (perhaps it's not firing fast enough? IDK, I'm not familiar with this stuff, sorry)

[icculus]

Hmm, I thought we fixed this. I'll check it here.

[icculus]

Oh yeah, this 100% reproduces here.

[icculus]

Likely culprit:

==242476== Memcheck, a memory error detector
==242476== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==242476== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==242476== Command: ./tests8
==242476== 
==242476== Thread 4 SDLAudioP1:
==242476== Invalid read of size 16
==242476==    at 0x48907E7: _mm_loadu_si128 (emmintrin.h:706)
==242476==    by 0x48907E7: SDL_Convert_S8_to_F32_SSE2 (SDL_audiotypecvt.c:353)
==242476==    by 0x488D690: SDL_ConvertAudio_REAL (SDL_audiocvt.c:281)
==242476==    by 0x488F8CF: SDL_AudioStreamPutInternal (SDL_audiocvt.c:1225)
==242476==    by 0x488FA86: SDL_AudioStreamPut_REAL (SDL_audiocvt.c:1294)
==242476==    by 0x4883559: SDL_RunAudio (SDL_audio.c:743)
==242476==    by 0x4935ED4: SDL_RunThread (SDL_thread.c:292)
==242476==    by 0x4AA9C9E: RunThread (SDL_systhread.c:76)
==242476==    by 0x4C2B6B9: start_thread (pthread_create.c:444)
==242476==    by 0x4CBA043: clone (clone.S:100)
==242476==  Address 0x4ee9870 is 16 bytes before a block of size 4,128 alloc'd
==242476==    at 0x4843738: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==242476==    by 0x49339BE: real_realloc (SDL_malloc.c:5198)
==242476==    by 0x4933CC7: SDL_realloc_REAL (SDL_malloc.c:5326)
==242476==    by 0x488EB13: EnsureStreamBufferSize (SDL_audiocvt.c:882)
==242476==    by 0x488F6F8: SDL_AudioStreamPutInternal (SDL_audiocvt.c:1173)
==242476==    by 0x488FA86: SDL_AudioStreamPut_REAL (SDL_audiocvt.c:1294)
==242476==    by 0x4883559: SDL_RunAudio (SDL_audio.c:743)
==242476==    by 0x4935ED4: SDL_RunThread (SDL_thread.c:292)
==242476==    by 0x4AA9C9E: RunThread (SDL_systhread.c:76)
==242476==    by 0x4C2B6B9: start_thread (pthread_create.c:444)
==242476==    by 0x4CBA043: clone (clone.S:100)
==242476== 

[icculus]

Bisecting confirmed what I suspected, it's this commit: 1e2f3118117a3cd6d483ea090db8d15a50f3bfdc

[icculus]

I think this is the fix, but I need to verify further:

diff --git a/src/audio/SDL_audiotypecvt.c b/src/audio/SDL_audiotypecvt.c
index d99d2def5..ddd1c7ac0 100644
--- a/src/audio/SDL_audiotypecvt.c
+++ b/src/audio/SDL_audiotypecvt.c
@@ -348,7 +348,7 @@ static void SDLCALL SDL_Convert_S8_to_F32_SSE2(SDL_AudioCVT *cvt, SDL_AudioForma
         i -= 16;

         {
-        const __m128i bytes = _mm_xor_si128(_mm_loadu_si128((const __m128i *)&src[i-16]), flipper);
+        const __m128i bytes = _mm_xor_si128(_mm_loadu_si128((const __m128i *)&src[i]), flipper);

         const __m128i shorts1 = _mm_unpacklo_epi8(bytes, zero);
         const __m128i shorts2 = _mm_unpackhi_epi8(bytes, zero);

This matches what the Sint16 code does (since we do i -= 16;, we don't need to slide back more with src[i-16] too).

[icculus]

Okay, .wav files apparently can't be in Sint8 format, so I converted test/sample.wav to Uint8 and had test/loopwave.c convert it at startup:

diff --git a/test/loopwave.c b/test/loopwave.c
index 15f13658c..9bec3a7a1 100644
--- a/test/loopwave.c
+++ b/test/loopwave.c
@@ -140,6 +140,15 @@ int main(int argc, char *argv[])

     wave.spec.callback = fillerup;

+if (wave.spec.format == AUDIO_U8) {
+    SDL_Log("Converted wavefile data from U8 to S8");
+    wave.spec.format = AUDIO_S8;
+    const Uint8 *src = (const Uint8 *) wave.sound;
+    Sint8 *dst = (Sint8 *) wave.sound;
+    for (int i = 0; i < wave.soundlen; i++) {
+        dst[i] = (Sint8) (((Sint16) src[i]) - 128);
+    }
+}
     /* Show the list of available drivers */
     SDL_Log("Available audio drivers:");
     for (i = 0; i < SDL_GetNumAudioDrivers(); ++i) {

...with the previous patch, the repeating click goes from 100% reproducible to completely fixed, so this is good to go.

[icculus]

Fixed in 4316c5ec02d793acf43a5c16f9a335a16d120f65 for the SDL2 branch, and cherry-picked to ad342dfca98b397e83ef48fa59cab60d035e02d3 for the release-2.30.x branch.

[slouken]

I assume this wasn't an issue for SDL3?

[icculus]

It was not.

[ericoporto]

I don't know if it's the same issue but SDL_ConvertAudio is failing for me in release 2.30.0 for a specific use we have with a specific video. It's working in 2.28.5 though. Just before I try to come up with some minimal reproducible (which I need to untangle from sdl_sound, our code and mojoAL) , is there anything else that changed in 2.28.5 to 2.30.0 regarding to audio?

[ivan-mogilko]

I don't know if it's the same issue but SDL_ConvertAudio is failing for me in release 2.30.0 for a specific use we have with a specific video. It's working in 2.28.5 though. Just before I try to come up with some minimal reproducible (which I need to untangle from sdl_sound, our code and mojoAL) , is there anything else that changed in 2.28.5 to 2.30.0 regarding to audio?

Our video decoder library loads audio as AUDIO_S16, so this has to be something else.

EDIT: maybe this? 384fcea585573db4e5707c249746008f25a0fbac

[slouken]

Please enter a new issue for tracking, and include a minimal example if possible.

[ericoporto]

Curiously if I download the source files and rebuild myself from the release 2.30.0 the generated SDL2.dll works fine. @slouken could you rebuild the Windows artifacts that are attached with the release? Both the VC and the Win x86/64 ones. I think the issue is just in the binaries.

include a minimal example if possible

I tried but they work because I am linking statically. I then noticed that rebuilding the things from source myself and using my dll files makes it work. I use MSVC instead of MinGW, but hopefully it doesn't matter.

[ivan-mogilko]

The commit which I mentioned above fixes a memory corruption in function SDL_Convert_S16_to_F32_Scalar. When, for example, I build SDL2 myself on my machine, SSE2 is enabled, and so converter has SSE2 functions enabled, and these are used during conversion. So, speculating, if the released binaries cause Scalar functions to be selected by audio conv instead, then that would be one theoretical possibility why there may be a difference in behavior. Is there any way to tell which conversion function exactly does SDL2 select when constructing a SDL_AudioCVT?

[ivan-mogilko]

Please enter a new issue for tracking, and include a minimal example if possible.

Right, I'm sorry, I opened a proper ticket for this problem: #9127