This needs to be looked at, as it's a regression. I'll ping James as well.

(In reply to Sam Lantinga from comment #1)
> This needs to be looked at, as it's a regression. I'll ping James as well.

Fwiw, this seems to be okay on a Raspberry Pi 3 running RetroPie. I can reflash the unit with Raspbian and predict this will reproduce because it's going through PulseAudio's ALSA bridge -> PulseAudio -> actual ALSA...RetroPie doesn't have PulseAudio running by default, and there's no stutter running our loopwave test there, but Raspbian uses Pulse, if I recall correctly.

Obviously, Raspbian is the standard installation in the RPi universe, though, just throwing a data point onto the pile here. It might be worth trying coolcv with an SDL_AUDIODRIVER=pulse environment variable and see if that helps, since it'll talk directly to PulseAudio and not the ALSA bridge (and thus never run this problem code in SDL at all).

--ryan.

Followup: Raspbian doesn't use PulseAudio by default (at least in the latest version?), and ALSA works fine with loopwave there.

This was on a Raspberry Pi 3 playing through the HDMI connection. It's possible older RPis don't have the processing power, Raspbian used to use Pulse, Raspbian since fixed something, there was a USB power cord that was barely giving enough current, this doesn't work through the headphone jack, loopwave doesn't open the device in a way that causes problems, or a dozen other possible culprits.

I couldn't get coolcv to run (it fails with an EGL initialization problem or something). Might have been my fault or another SDL bug?

We probably need more information to narrow this down at this point. It isn't necessarily a wide-reaching regression, though.

--ryan.

It does happen on RetroPie - just that RetroPie ships with a custom patched SDL which has this patched.

(I am a RetroPie developer)

(In reply to Jools Wills from comment #5)
> (I am a RetroPie developer)

Doh, I didn't realize.  :)

I thought I was using an SDL that I built from scratch, but maybe I didn't? While we're here:

- What model Raspberry Pi are you using (or is it just any model)? I've got RPi 1, 2 and 3 here.
- Is there any magic to reproduce this, or is it literally "just run coolcv on a fresh install of RetroPie 4.2 with a unpatched SDL"?
- Is there a specific coolcv version, and any particular ROM that does it?

The code that's causing the problem is there because ALSA freezes up if we unplug a USB audio device and this is the only way we currently know to catch that without hanging, so we'd like to not remove that code if we can find a suitable workaround that makes everyone happy.

--ryan.

>What model Raspberry Pi are you using (or is it just any model)? I've got RPi 1, 2 and 3 here.

Affects all models.

- Is there any magic to reproduce this, or is it literally "just run coolcv on a fresh install of RetroPie 4.2 with a unpatched SDL"?

Yep - or on Raspbian.

- Is there a specific coolcv version, and any particular ROM that does it?

All versions and all games.

(In reply to Jools Wills from comment #7)

Thanks!

Oh, one more: HDMI or headphone jack port?

--ryan.

I believe it affects both HDMI and jack audio output, but when I tested, I was using the Jack.

Can reproduce this now.

--ryan.

Created attachment 2909 [details]
Possible fix via snd_pcm_state()


I'm attaching a patch that fixes the Raspberry Pi issue, but someone will have to check that this doesn't regress the unplugging-a-USB-device-hangs-the-audio-thread issue before committing.

If this doesn't work, here's snd_pcm_wait, as it looks in the current version of RetroPie:

http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/pcm/pcm.c;hb=0f4f48d37716be0e6ddccb2124c5e09d5bd1cab3#l2401

My guess is that deep in the snd_bcm2835 driver, that snd_pcm_may_wait_for_avail_min() call waits _way_ too long for whatever bug or legitimate hardware-based reason, so I'm hoping that the snd_pcm_state() call made in there correctly reports the disconnect without blocking without the may_wait thing.

Failing that, ALSA offers an API to let you get at what you need to poll() the device's file handle, which could be worth a try to prevent a hang; as you can see, snd_pcm_wait() uses this internally, and I presume this is not where the delay is happening.

Alternately: we could call snd_pcm_nonblock() and, I presume, never lock up in the snd_pcm_writei() call if the device is unplugged, but we'd have to make sure a few other places aren't relying on blocking behaviour (ALSA_WaitDevice() would need something written, presumably not calling snd_pcm_wait()).

--ryan.

(In reply to Ryan C. Gordon from comment #11)
> Created attachment 2909 [details]
> Possible fix via snd_pcm_state()

This fix didn't work on the hardware that freezes up on unplugged USB audio devices, but fwiw, both the Raspberry Pi and a generic Linux box correctly report an error without freezing up in this case.

Looks like the stutter on the Raspberry Pi comes from the poll() inside of libasound that happens in snd_pcm_wait; I get the same results if I do the poll() directly in SDL.

That snd_pcm_wait() is there to work around a different buggy driver (most drivers don't lock up when you unplug a USB device), so I think the easiest and best solution is to just #ifdef out the snd_pcm_wait workaround on the Pi, because the Pi doesn't need it anyhow, so after all this research to make sure we did the right thing, we ended up back at your original solution anyhow.  :)

Eventually we might try setting the hardware to be non-blocking with snd_pcm_nonblock(), which might work everywhere, but that would take more changes, work, testing and risk than we want at this point in the release cycle.

I have removed the snd_pcm_wait() call, which as you already knew, fixes the issue on RetroPie. I have built and tested this with the latest RetroPie release and coolcv just to be absolutely sure.

That patch is now https://hg.libsdl.org/SDL/rev/cdf55fac5366 ...thanks for your patience as we researched this issue quite deeply.

(Unrelated to this bug: if you have other patches against SDL that are being shipped with RetroPie, let us know; we will upstream them if at all possible!)

--ryan.

We have some other changes which have been on this bugtracker for some time. 

eg

https://bugzilla.libsdl.org/show_bug.cgi?id=3277
and
https://bugzilla.libsdl.org/show_bug.cgi?id=3353

neither of which have had much feedback unfortunately.