Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stuttering audio in coolcv in SDL2.0.5 #2407

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments
Closed

Stuttering audio in coolcv in SDL2.0.5 #2407

SDLBugzilla opened this issue Feb 11, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: 2.0.5
Reported for operating system, platform: Linux, ARM

Comments on the original bug report:

On 2017-03-03 16:34:06 +0000, Jools Wills wrote:

coolcv (colecovision emulator) has stuttering audio when running against sdl 2.0.5 on the raspberry pi.

I have bisected the issue to this commit

https://hg.libsdl.org/SDL/rev/a45d3b96cf85

reverting this commit fixes the issue.

On 2017-08-11 19:06:58 +0000, Sam Lantinga wrote:

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

On 2017-09-01 14:28:50 +0000, Ryan C. Gordon wrote:

(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.

On 2017-09-01 17:04:58 +0000, Ryan C. Gordon wrote:

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.

On 2017-09-01 18:14:18 +0000, Jools Wills wrote:

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

On 2017-09-01 18:15:00 +0000, Jools Wills wrote:

(I am a RetroPie developer)

On 2017-09-01 19:45:14 +0000, Ryan C. Gordon wrote:

(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.

On 2017-09-01 20:04:00 +0000, Jools Wills wrote:

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.

On 2017-09-01 20:50:49 +0000, Ryan C. Gordon wrote:

(In reply to Jools Wills from comment # 7)

Thanks!

Oh, one more: HDMI or headphone jack port?

--ryan.

On 2017-09-01 20:52:59 +0000, Jools Wills wrote:

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

On 2017-09-02 21:19:35 +0000, Ryan C. Gordon wrote:

Can reproduce this now.

--ryan.

On 2017-09-03 06:21:58 +0000, Ryan C. Gordon wrote:

Created attachment 2909
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.

On 2017-09-07 05:30:10 +0000, Ryan C. Gordon wrote:

(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.

On 2017-09-10 01:20:58 +0000, Ryan C. Gordon wrote:

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.

On 2017-09-13 04:37:53 +0000, Jools Wills wrote:

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant