fix: enumerate ALSA devices like aplay -L #992
Conversation
|
cc: @abique |
|
For reference, this is how it enumerated on v0.15 on the same system: So my PR restores what v0.15 was able to do and adds access to:
I'd like to have a bit more community feedback on this than #916 did. cc'ing some users that put in ALSA contributions relatively recently. What do you think @attackgoat, @cdellacqua, @lautarodragan, @jwagner? (hope that's OK - trying to build the community and can use your help - but let me know if you don't want me to do that again) |
There was a problem hiding this comment.
Pull Request Overview
This PR changes the ALSA device enumeration approach to match aplay -L behavior, providing access to all available ALSA devices including virtual devices and specific device variants.
- Replaces card-based enumeration with ALSA hints enumeration using
HintIter - Removes hardcoded builtin device list in favor of comprehensive device discovery
- Adds deduplication logic to prevent duplicate devices in enumeration
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/host/alsa/enumerate.rs | Replaces card iteration with ALSA hints enumeration to match aplay -L output |
| CHANGELOG.md | Documents the enumeration behavior change |
|
LGTM and doesn't break my usage. As a brand new user I was confused about the devices CPAL reported, and now that I run this branch I see recognizable devices that make sense. |
|
Looks good. The only thing that I would change is the error handling, although it might be like that for compatibility reasons, so feel free to discard the following suggestion. I would eagerly fail if ALSA can't provide hints. This should also simplify the Iterator impl. Something like: impl Devices {
pub fn new() -> Result<Self, DevicesError> {
// Enumerate ALL devices from ALSA hints (same as aplay -L)
alsa::device_name::HintIter::new_str(None, "pcm")
.map(|hint_iter| Devices {
hint_iter,
enumerated_pcm_ids: HashSet::new(),
})
.map_err(DevicesError::from)
}
}And in the Iterator the next function could then be simplified as: fn next(&mut self) -> Option<Device> {
loop {
match self.hint_iter.next().map(|hint| hint.name) {
// Skip if we've already enumerated this device by PCM ID
Some(Some(name)) if !self.enumerated_pcm_ids.contains(&name) => {
let device = open_device(&name, name.clone());
self.enumerated_pcm_ids.insert(name.clone());
return Some(device);
}
Some(_) => (),
None => return None, // All devices enumerated
}
}
}note: |
|
Thank you for responding to the ping @attackgoat and @cdellacqua and taking the time to review.
As this change will need to push the minor semver anyway to 0.17 or beyond, we can entertain improvements to the error handling. What are you thinking?
Great idea. Coming weeks I don't have an ALSA device to test on, so I'll get on it later. |
Thank you for maintaining this library! Glad to help
Oh, sorry for the confusion. The first and second paragraphs were about the same thing: "eagerly fail if ALSA can't provide hints" |
|
Thanks a lot for picking this up Roderick! I'm wondering how user friendly the device names are going to be. The DeviceTrait define name as A name like I think it would be best to expose to different methods in the DeviceTrait, one delivering a human readable label, another one delivering a (hopefully) stable platform specific 'id'. This could happen by adding an As a bonus this would then could also address the lack of persistable unique ids for cpal devices. As for the sheer amount of (virtual) devices being overwhelming exposing a device type (Hardware, Virtual, Monitor, ..) might help. Lastly, I'm not arguing that this has to be in scope for this PR but it might be good to get it sorted before the next release since in a way not having "human readable" (again arguable) labels is a bit of a regression. |
|
For my personal use-case, it doesn't matter to me whether enumeration returns verbose ALSA device names, or just friendlier names (as the original PR to cpal does), so long as cpal can open any ALSA device using a verbose ASLA device name. I don't see why there is a need to unnecessarily restrict which device cpal can open, so long as it exists. Device enumeration is then free to return whatever it likes (although that may still break some consumers of cpal if they're doing some kind of comparison or lookup in the enumerated device list). |
roderickvd
force-pushed
the
fix/enumerate-virtual-devices
branch
2 times, most recently
from
489f5e2 to
03a517c
Compare
|
@cdellacqua I implemented your proposal. Much cleaner indeed. @jwagner love the ideas. While it'd be a bigger change indeed, it wasn't difficult to provide a middle ground in 73f3dd6 already. For devices that have a description, it's appended in parentheses: I am with @jduncanator that I expect an average Linux user to be able to use
That private Would appreciate a short thumbs up if this is working for y'all 👍 |
- Return single config when all sample rate ranges have identical min/max values - Improve iterator usage for better code clarity and consistency - Follow RtAudio approach for handling device sample rate inconsistencies Fixes issues with redundant sample rate configs that most CoreAudio devices report.
- Enumerate all ALSA devices directly from hints - Store device description and display it in Device::name - Simplify default device creation and handling
- Use Rust format string syntax for error messages - Remove redundant reference in DeviceHandles::open call - Simplify timespec_to_nanos calculation
roderickvd
force-pushed
the
fix/enumerate-virtual-devices
branch
from
03a517c to
8dcee4a
Compare
Move device construction logic into a TryFrom implementation for alsa::device_name::Hint. Simplifies iterator and improves error handling.
|
Hey @roderickvd, Gave the branch a quick test in my application, seems to work. At least in my context the descriptions definitely help. I like it! 👍 
|
On one of my systems, cpal v0.16 enumerates these and only these devices:
As reported in #991 this is problematic because it:
hw,plughw, something elseDEV=0when there could be moreNew behavior
This PR changes this behavior to enumerate all devices like
aplay -Lputs out:This solves all problems mentioned.
System configuration
For reference the configuration as
aplayreports:References
Fixes #991