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.

Created attachment 2751 [details]
Test program for the uninitialized audio subsystem error messages.

Attaching a program that tries to use the audio conversion without initializing the SDL audio subsystem and then displays the error messages.

Maybe it's better to report an error immediately if the audio subsystem isn't initialized.

--ryan.

Created attachment 2752 [details]
Patch that adds a check for an initialized audio subsystem to SDL_BuildAudioCVT. Also changes some comments and error messages to reflect the implementation better.

Something like this? There's the small detail that the conversion functions don't get set to NULL on SDL_AudioQuit and the check now blocks it from working, but I'm guessing no one will really expect them to in this scenario.

I also noticed that SDL_BuildAudioCVT seems to accept negative sample rates. Is this intentional?

Created attachment 2753 [details]
Revised test program for the uninitialized audio subsystem error messages.

I noticed that the previous program had bugs. It now also retries all steps after calling SDL_Quit once.

(Sorry if you get a lot of copies of this email, we're touching dozens of bug reports right now.)

Tagging a bunch of bugs as target-2.0.6.

This means we're in the final stretch for an official SDL 2.0.6 release! These are the bugs we really want to fix before shipping if humanly possible.

That being said, we don't promise to fix them because of this tag, we just want to make sure we don't forget to deal with them before we bless a final 2.0.6 release, and generally be organized about what we're aiming to ship. After some debate, we might just remove this tag again and deal with it for a later release.

Hopefully you'll hear more about this bug soon. If you have more information (including "this got fixed at some point, nevermind"), we would love to have you come add more information to the bug report when you have a moment.

Thanks!
--ryan.

This patch is now https://hg.libsdl.org/SDL/rev/86b1fde471c6, thanks!

--ryan.

This change causes Emulationstation to segmentation fault (some times terminates due to std:alloc). Does that mean something needs to be fixed in Emulationstation? Works fine with this reversed.

After a very quick look at the code: Yes, there is a bug in the EmulationStation code.

There's no check for errors after a call to SDL_BuildAudioCVT. If an error occurs, I think it's possible that it allocates memory of an undefined size.

https://github.com/Aloshi/EmulationStation/blob/646bede3d9ec0acf0ae378415edac136774a66c5/es-core/src/Sound.cpp#L69

I'm not sure why that would happen after this specific change. There's an SDL_InitSubSystem call in the AudioManager::init function. Does it not get properly called?

I can't really tell, as I'm not a developer. Emulationstation isn't developed anymore either, except for RetroPie's fork. Perhaps they will fix it down the road, until then I can just revert this change as it works fine for me without it. Sorry to bother everyone :)

No bother at all. It's important to know how existing projects using SDL2 behave.

I created a bug report on their GitHub issue tracker.

https://github.com/RetroPie/EmulationStation/issues/230

Would you be able to test it should anyone ever have the time to make some changes? No commitment required, of course. ;)

This issue actually raises the question if this API change (requirement of initialized audio subsystem) is breaking backwards compatibility. I don't see the documentation saying it is needed in 2.0.5.

We really shouldn't require audio device initialization for converting audio in place.

I'll take a look at that.

Fixed!
https://hg.libsdl.org/SDL/rev/f40c2dedaded

(In reply to Sam Lantinga from comment #11)
> We really shouldn't require audio device initialization for converting audio
> in place.

I'm okay with this working either way, but I don't think it's unreasonable to think of SDL_Init() as initializing subsystems as a whole and require it even if you don't plan to playback or record audio.

I mention this because calling SDL_ChooseAudioConverters could be a race condition if you have multiple threads converting audio, so it was nice to require explicit init so this would definitely be set up by the time converters need it.

--ryan.

The race condition should be pointers getting overwritten with the same values, so I'm okay with this. I'd hate to break applications that aren't expecting this change.

(In reply to Sam Lantinga from comment #12)
> Fixed!
> https://hg.libsdl.org/SDL/rev/f40c2dedaded

This solves it for me :)

Created attachment 2899 [details]
Patch that corrects documentation.

The documentation should also be adjusted again. Attached patch does that.

Got it, thanks!
https://hg.libsdl.org/SDL/rev/41bc8b8e9758