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

ALSA crash on init with EINVAL in snd_pcm_sw_params #66

Open
jtsiomb opened this issue Nov 1, 2023 · 22 comments
Open

ALSA crash on init with EINVAL in snd_pcm_sw_params #66

jtsiomb opened this issue Nov 1, 2023 · 22 comments
Assignees
Labels
bug Something isn't working

Comments

@jtsiomb
Copy link

jtsiomb commented Nov 1, 2023

martypc crashes on init on my GNU/Linux system with: thread 'main' panicked at 'Failed to build an output audio stream: BackendSpecific { err: BackendSpecificError { description: "ALSA function 'snd_pcm_sw_params' failed with error 'EINVAL: Invalid argument'" } }', /usr/local/src/martypc/core/src/sound.rs:146:14

I'm running debian GNU/Linux without any audio daemons, just ALSA. The version of martypc I've built is the current git HEAD (f18b73e).

Here's the relevant part of a full backtrace, with all the rust unwind and backtrace display noise removed:

16:     0x5555557b5a2d - core::result::Result<T,E>::expect::h5577ddc278659af8
                           at /usr/src/rustc-1.70.0/library/core/src/result.rs:1046:23
17:     0x5555557a86dc - marty_core::sound::SoundPlayer::new::h52d87d0165aed411
                           at /usr/local/src/martypc/core/src/sound.rs:139:29
18:     0x5555557940df - martypc_pixels_desktop::run::h7f3e9de3742bc3cb
                           at /usr/local/src/martypc/frontends/martypc_pixels_desktop/src/lib.rs:523:36
19:     0x5555557917c7 - martypc::main::h2ad35e0e7d3e7dd8
                           at /usr/local/src/martypc/frontends/martypc_pixels_desktop/src/main.rs:38:5
@jtsiomb jtsiomb added the bug Something isn't working label Nov 1, 2023
@dbalsom
Copy link
Owner

dbalsom commented Nov 1, 2023

your hardware configuration and RUST_LOG=trace output, please.

@dbalsom
Copy link
Owner

dbalsom commented Nov 1, 2023

also, do the cpal examples run?
https://github.com/RustAudio/cpal

@jtsiomb
Copy link
Author

jtsiomb commented Nov 1, 2023

Here's the stderr output when running with RUST_LOG=trace: https://pastebin.com/3tUFTdYS

The cpal examples seem to run fine. The beep example beeps, and the enumerate example produces this output (split into three parts because pastebin refused to accept more than 512kb per paste):

@jtsiomb
Copy link
Author

jtsiomb commented Nov 2, 2023

What information do you need about my hardware configuration? Sound hardware is some on-board HDA-compatible on the motherboard: "Audio device: Intel Corporation 100 Series/C230 Series Chipset Family HD Audio Controller (rev 31)"

@dbalsom
Copy link
Owner

dbalsom commented Nov 4, 2023

Can you provide the output of the 'enumerate' example from cpal examples?

@jtsiomb
Copy link
Author

jtsiomb commented Nov 4, 2023

I did, it's the three-part pastebin above.

@dbalsom
Copy link
Owner

dbalsom commented Nov 9, 2023

This seems to be similar to bevyengine/bevy#2699

From the linked article:
https://wiki.archlinux.org/title/Advanced_Linux_Sound_Architecture#Dmix

Integrated motherboard sound cards (such as Intel HD Audio), usually do not support hardware mixing. On such cards, software mixing is done by an ALSA plugin called dmix. This feature is enabled automatically if hardware mixing is unavailable.

can you verify if dmix is enabled?

@jtsiomb
Copy link
Author

jtsiomb commented Nov 9, 2023

It is, but it's also irrelevant, I'm not trying to play back multiple audio streams, nor is the error about failing to open the dmix device.

@dbalsom
Copy link
Owner

dbalsom commented Nov 9, 2023

are you using any particular distribution I could attempt to repo with? Nevermind, I missed the 'debian' in your initial message.

Is there a reason you're not running any audio daemon?

@jtsiomb
Copy link
Author

jtsiomb commented Nov 9, 2023

Debian yes, but it shouldn't make any difference, as the default debian setup is very different to what I'm running.

I'm not running audio daemons because they are unnecessary bloat. The native ALSA kernel interface is more than sufficient for what I need for audio output, and I prefer to err towards simplicity.

@dbalsom
Copy link
Owner

dbalsom commented Nov 9, 2023

Fair enough.

MartyPC is opening the audio device twice in rapid succession, and I think that is the problem. The long story short is it was a result of trying to encapsulate cpal's callback generics in a generic SoundPlayer struct. But to create the SoundPlayer struct I have to know the type of the sample format, which I need to ask cpal for, and it opens the device to retrieve that. The sample rate is returned only to be passed to the SoundPlayer's new() method, which opens the device again with the specified sample rate. This is not a great design, but didn't cause problems if the audio device can be opened multiple times.

Theoretically the device should be closed when the device struct is dropped at the end of the first function that reports the sample rate, but I suppose that it takes a non-zero amount of time to actually close the device in a manner such that it can be opened again.

I think the solution is to return the device handle along with the sample rate from ::get_sample_rate, and then I can pass both to new(), so we don't have to reopen the device when creating a SoundPlayer.

@jtsiomb
Copy link
Author

jtsiomb commented Nov 9, 2023

Interesting. I tried adding thread::sleep_ms(10000) after the get_sample_format call in frontends/martypc_pixels_desktop/src/libs.rs but aside from the 10 second pause, it didn't make a difference, so I'm not sure it's a matter of time or inability to re-open the device.

I commented out the whole block, including get_sample_format and the match block, and replaced it with let sp = SoundPlayer::new::<i16>(); and it still failed in exactly the same way. I'm sure 16bit signed samples are supported, but just in case I also tried both other variants, and no difference. Btw I then also printed the result of get_sample_format and it is F32.

I hope this helps. If you want me to test some experiment let me know, but I'm not at all familiar with rust, so spell out any changes, or give me a patch to apply.

@dbalsom
Copy link
Owner

dbalsom commented Nov 9, 2023

let's just see what this prints out
sound.rs.patch

@jtsiomb
Copy link
Author

jtsiomb commented Nov 9, 2023

Default audio device: default
Default audio config: SupportedStreamConfig { channels: 2, sample_rate: SampleRate(44100), buffer_size: Range { min: 170, max: 1466015503 }, sample_format: F32 }

@fuel-pcbox
Copy link

TBH I don't think this should count as a bug, most Linux software requires an audio daemon to run properly, including emulators. The fact that you're not running one because you think it's "unnecessary bloat" is a you problem, not a problem of the software for not being able to handle it.

@jtsiomb
Copy link
Author

jtsiomb commented Nov 15, 2023

most Linux software requires an audio daemon to run properly

Citation needed.

Obviously how important compatibility with a variety of setups is to a project, is up to the author to decide. But Linux has an audio interface, it's called ALSA, it doesn't require any intermediate daemons to play audio, and despite your assertion, most programs work with it just fine.

Since most GNU/Linux distributions install pulse audio by default, and most users don't bother to change that, often the ALSA code path of many projects gets very little testing. And while most projects intend to support ALSA, sometimes unnoticed regressions crop up in the non-pulse audio code path. That's why I considered this to be a bug, and thought I should report it.

@dbalsom
Copy link
Owner

dbalsom commented Nov 15, 2023

I would have probably said this was an unsupported configuration had not the cpal examples worked. But they do, so that implies that I am doing something that can be corrected, and if it is minor I am willing to do it. But if it isn't opening the device twice, I am not sure.

I have been considering an alternative SDL frontend and I have been slowly factoring pieces of the current wgpu frontend into common front end libraries to enable sharing code between the two frontends. I will note based on the logs provided that I anticipate even if the sound problem was resolved, MartyPC may run poorly on this system. wgpu did not initialize vulkan, and we're falling back to the opengl backend, which based on other issues opened likely will have poor performance.

So this may be another case where an SDL front end will work better and hopefully address the sound issue as well.

@dbalsom
Copy link
Owner

dbalsom commented Dec 6, 2023

I plan to switch from using cpal directly to using rodio, which uses cpal under the hood but I will not have to do the awkward generic-instantiation initialization dance myself, which is almost worth it just by itself.

Whether that will fix things or not...?

rodio provides a mixing facility, which I will eventually need to implement multiple audio sources (pc speaker + dac + opl emulation, as well as any sounds the front end might want to play (floppy noises? who knows))

rodio has some examples, if you could check if they run?

In any case 0.2.0 will make sound optional, so you can at least run the emulator regardless.

@jtsiomb
Copy link
Author

jtsiomb commented Dec 7, 2023

Yeap, I just compiled rodio and its examples work fine.

And I agree, being able to run the emulator without audio at all, would be a good thing to add.

@dbalsom
Copy link
Owner

dbalsom commented Dec 22, 2023

In the meantime until I can implement rodio, I made the change to re-use the device handle, whether that helps or not. If not, at least now you can disable audio with --noaudio

@jtsiomb
Copy link
Author

jtsiomb commented Dec 23, 2023

Thank you, having that option is always good.

Unfortunately I can't test this. It complains that epaint requires rustc 1.72, while I have 1.70 which is what debian unstable packages at the moment. I'll get back to you as soon as 1.72 becomes available in debian.

@peterfirefly
Copy link

Unfortunately I can't test this. It complains that epaint requires rustc 1.72, while I have 1.70 which is what debian unstable packages at the moment. I'll get back to you as soon as 1.72 becomes available in debian.

You can easily install 1.72 (or any other relevant version) yourself -- and easily remove it or upgrade it if you like -- with the rustup tool. Rustup itself is also easy to install, either from the rust people:

https://rustup.rs/

or via 'apt install rustup' on trixie and sid:

https://packages.debian.org/search?suite=default&section=all&arch=any&searchon=names&keywords=rustup

It is normal for people to use rustup to always use the newest stable version of rust (or sometimes a recent nightly version or sometimes a specific older version). Rustup is the official tool for doing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants