We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4805

Summary: Unsigned audio formats output wrong silence value
Product: SDL Reporter: Florian Behr <mail>
Component: audioAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: develop, sezeroz
Version: 2.0.10Keywords: target-2.0.12
Hardware: All   
OS: Windows 10   
Attachments: repro case

Description Florian Behr 2019-09-17 12:18:28 UTC 
Created attachment 3971 [details]
repro case

When creating an audio device on Windows 10 with an unsigned sample format sdl produces an audio "pop" and whining noise instead of silence.

The simple reproduction case is attached, and an audacity screencap showing the problem can be seen here: https://imgur.com/a/08VEVNK

the problem appears to be that in the process of initializing the audio device the silence value does not get set properly on the SDL_AudioSpec, but there are actually two different bugs for U8 and U16. 

The U16 case is the simpler one, the silence value is set in SDL_audio.c:SDL_Calculate_AudioSpec(). U16 formats are just not even considered here.

The U8 case is a bit more complex. The U8 silence value is initially set correctly in SDL_audio.c:SDL_Calculate_AudioSpec() called in SDL_audio.c:prepare_audiospec():1210, and passed out of the function to the user correctly, but is then internally mangled during WASAPI initialization. The SDL_AudioSpec that is used internally from this point on is a copy of the one passed back out to the user. 

At SDL_wasapi.c:WASAPI_PrepDevice():564 the SDL format on the SDL_AudioSpec is replaced for what appears to be a WASAPI native format value, although I'm not entirely sure of that. But whatever the new value signifies it is different from the SDL defined AUDIO_U8. 

Then later at SDL_wasapi.c:WASAPI_PrepDevice():615 there's another call of SDL_audio.c:SDL_Calculate_AudioSpec() to recalculate potential changes, but since at this point the format value of the SDL_AudioSpec is not recognized by the switch statement, the silence value is set to the default 0 for signed formats, instead of the correct 128. This faulty silence value is then used for filling samples sent to the audio device, and presumably produces the whining noise as it forces the speaker into holding and end-position.
Comment 1 Ryan C. Gordon 2019-09-20 20:47:33 UTC 
We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.
Comment 2 Ryan C. Gordon 2019-09-20 20:48:44 UTC 
We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.
Comment 3 Ryan C. Gordon 2019-09-25 19:24:49 UTC 
(In reply to Florian Behr from comment #0)
> The U16 case is the simpler one, the silence value is set in
> SDL_audio.c:SDL_Calculate_AudioSpec(). U16 formats are just not even
> considered here.

We have a deeper problem, though: SDL_AudioSpec::silence is a Uint8. It can't fit the silence value for Uint16 (0x8000), and this value is generally used with memset() out in the wild anyhow, so even if we could change SDL_AudioSpec--we can't without breaking binary compatibility--it wouldn't be useful to do so.

I propose that we just report the silence value as 0x80 for both U8 and U16. For U8, it'll be perfect silence, and for U16, it's...close enough (samples will clear to 0x8080, which is 32896, which is 128 above silence (or 0.1953 percent above silence). It's not perfect, but it'll do, I suppose.

For 2.1, we can change how the silence field works, or maybe just remove U16 support.

--ryan.
Comment 4 Ryan C. Gordon 2019-09-25 19:54:37 UTC 
> I propose that we just report the silence value as 0x80 for both U8 and U16.
> For U8, it'll be perfect silence, and for U16, it's...close enough (samples
> will clear to 0x8080, which is 32896, which is 128 above silence (or 0.1953
> percent above silence). It's not perfect, but it'll do, I suppose.

This is how it works in https://hg.libsdl.org/SDL/rev/599ed6e984d2, now. I'm still looking at the WASAPI issue.

--ryan.
Comment 5 Ryan C. Gordon 2020-01-21 21:37:20 UTC 
(In reply to Ryan C. Gordon from comment #4)
> This is how it works in https://hg.libsdl.org/SDL/rev/599ed6e984d2, now. I'm
> still looking at the WASAPI issue.

This appears to work on WASAPI now, either through the previous fix, or something else, or a misunderstanding (among other things, internally, SDL keeps an AudioSpec for what it wants to feed the hardware and what it expects the callback to feed to SDL: if you were looking at device->spec and saw silence is 0, that was correct, because WASAPI wants float32 data, but device->callbackspec should have a silence of 0x80, since the callback is feeding us Uint8 data).

While tracking this down, though, I noticed that SDL_LoadWAV doesn't set the silence fields correctly for the loaded file, always initializing it to zero, which is now fixed in https://hg.libsdl.org/SDL/rev/f0750959bb29 .

If I've misunderstood, please reopen this bug, but I think we're good to go otherwise.

--ryan.