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

Prevent ALSA audio corruption #43928

Merged
merged 1 commit into from
Nov 28, 2020
Merged

Conversation

charasyn
Copy link
Contributor

When using the ALSA driver, corruption would occur if snd_pcm_writei
was unable to consume the entire sound buffer. This would occur
frequently on the Raspberry Pi 3 which uses the snd_bcm2835 audio
driver.

This bug resulted from incorrect pointer math on line 187, resulting in
the sample source pointer being advanced by total * ad->channels bytes
instead of total * ad->channels samples. In my opinion, the best fix
is to change *src to type int16_t, since that is the sample type in
use.

Fixes #43927.

When using the ALSA driver, corruption would occur if `snd_pcm_writei`
was unable to consume the entire sound buffer. This would occur
frequently on the Raspberry Pi 3 which uses the `snd_bcm2835` audio
driver.

This bug resulted from incorrect pointer math on line 187, resulting in
the sample source pointer being advanced by `total * ad->channels` bytes
instead of `total * ad->channels` samples. In my opinion, the best fix
is to change `*src` to type `int16_t`, since that is the sample type in
use.

Fixes godotengine#43927.
@Calinou Calinou added bug cherrypick:3.x Considered for cherry-picking into a future 3.x release platform:linuxbsd topic:audio labels Nov 27, 2020
@Calinou Calinou added this to the 4.0 milestone Nov 27, 2020
@akien-mga akien-mga merged commit 4f486bc into godotengine:master Nov 28, 2020
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@hiulit
Copy link

hiulit commented Nov 28, 2020

Do you think this could be back ported to 3.x.x?

@akien-mga
Copy link
Member

That's what cherrypick:3.2 means.

@hiulit
Copy link

hiulit commented Nov 28, 2020

Oh, sorry! I didn't see that. Thanks!

@hiulit
Copy link

hiulit commented Nov 28, 2020

FYI, I've compiled Godot 3.2.3 on a Raspberry Pi 4 with that change and it fixed the issue! :)

@akien-mga
Copy link
Member

Cherry-picked for 3.2.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Nov 29, 2020
@cesarizu
Copy link
Contributor

cesarizu commented Nov 30, 2020

I was having this problem with ALSA. Installing pulseaudio and running with --audio-driver ALSA made it less of a problem (sound would last longer before crashing).

I've applied this patch to the 3.2.3-stable branch and tested on a raspberry pi (64 bits OS) but I'm still getting these kind of messages:

pulseaudio E: [alsa-sink-bcm2835 HDMI 1] alsa-sink.c: ALSA woke us up to write new data to the device, but there was actually nothing to write.
pulseaudio E: [alsa-sink-bcm2835 HDMI 1] alsa-sink.c: Most likely this is a bug in the ALSA driver 'snd_bcm2835'. Please report this issue to the ALSA developers.
pulseaudio E: [alsa-sink-bcm2835 HDMI 1] alsa-sink.c: We were woken up with POLLOUT set -- however a subsequent snd_pcm_avail() returned 0 or another value < min_avail.

Sometimes it kills pulseaudio completely:

systemd pulseaudio.service: Main process exited, code=killed, status=6/ABRT
systemd pulseaudio.service: Failed with result 'signal'.
systemd pulseaudio.service: Service RestartSec=100ms expired, scheduling restart.
systemd pulseaudio.service: Scheduled restart job, restart counter is at 1.
pulseaudio W: [pulseaudio] pid.c: Stale PID file, overwriting.

And also:

Game.x86_64 ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
Game.x86_64 ALSA lib pcm.c:8427:(snd_pcm_recover) cannot recovery from underrun, prepare failed: Input/output error
Game.x86_64 ERROR: thread_func: ALSA: Failed and can't recover: Input/output error
Game.x86_64    At: drivers/alsa/audio_driver_alsa.cpp:207.

@hiulit
Copy link

hiulit commented Nov 30, 2020

I'm not an expert @cesarizu , but try uninstalling pulseaudio (it should'nt be needed) and recompile Godot with this patch. I've successfully compiled 2.1.6-stable, 3.1.2-stable and 3.2.3-stable on a Raspberry Pi 4 (running Raspberry Pi Os 32 bits) with the audio fix and all the games I've tried seem to work well. For some reason I couldn't compile 3.0.6-stable.

@efornara
Copy link
Contributor

@cesarizu The problem you are noticing was likely already there before this patch. If you follow the original discussion (https://github.com/efornara/frt/issues/14), it looks like that, at least on a Raspberry Pi 3, changing the project audio mix rate to 44000 and increasing the audio latency to 20ms was needed to fix the buffer underrun problem. Maybe this helps.

@cesarizu
Copy link
Contributor

cesarizu commented Nov 30, 2020

@hiulit and @efornara yes indeed I been having this problem for some time. I'm running a few games in multiple raspberry pi 4 (64bits) in some sort of a kiosk (14h a day) for months.

Initially, without pulseaudio installed, I would get very choppy audio and the errors would show up very quickly (in the first hour). Then audio would stop. After installing pulseaudio I would get the error once a day and the audio wouldn't stop most of the days. Without --audio-driver ALSA I would get the same errors but the audio would be still a bit choppy.

I've been running with the latency to 45ms for weeks but I haven't tried changing the audio mix rate to 48000. I will do that right away.

I just applied this patch a few days ago (as soon as I saw it). Since then, I've seen around the same number of errors but also pulseaudio started to crash completely and I'm seeing some new errors like:

pulseaudio W: [pulseaudio] pstream.c: Received invalid frame size: 0
pulseaudio E: [pulseaudio] pstream.c: Assertion 're->data || re->memblock' failed at pulsecore/pstream.c:862, function do_read(). Aborting.

I'll try uninstalling pulseaudio and making the other changes and I'll report back any news on that.

@hiulit
Copy link

hiulit commented Dec 1, 2020

@akien-mga will this only be cherry picked for 3.2? Or will it be ported to previous versions? Because this issue seems to affect every version.
I've successfully compiled 2.1.6-stable, 3.1.2-stable and 3.2.3-stable on a Raspberry Pi 4 (but for some reason I couldn't compile 3.0.6-stable) and this change solves the issue in all the versions.
If you don't intend to port it to previous versions, will you be interested in me creating pull requests with the fix for those versions?

@akien-mga
Copy link
Member

I can cherry-pick it to earlier branches but aside from 2.1, I don't plan to make new 3.0 or 3.1 releases unless there's a strong demand for it (which there hasn't been so far).

@hiulit
Copy link

hiulit commented Dec 1, 2020

Ok, I understand. Thanks!

@cesarizu
Copy link
Contributor

cesarizu commented Dec 3, 2020

Just an update: I've been running the PIs almost continuously for a couple days with some different configurations and this is what works and doesn't work for me:

  1. I'm using Mix Rate = 48000 and Output Latency = 20m in the settings for all tests
  2. I'm running on Raspbian 64bits
  3. Godot 3.2.3-stable with this PR compiled with LLVM

These are the results:

A. Without pulseaudio installed (with or without --audio-driver ALSA) works perfectly, no ALSA errors
B. With pulseaudio installed and using --audio-driver ALSA works as before, ALSA errors once a day but audio continues.
C. With pulseaudio installed and not using --audio-driver ALSA (so using pulseaudio driver) breaks pulseaudio completely after a few hours of continued use. Restarting the game restores sound (as pulseaudio restarts automatically).

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

Successfully merging this pull request may close these issues.

Glitched audio on Raspberry Pi 3 when using ALSA driver
6 participants