From ba112e77a81376d4f794f54d10115f597a9ab1ee Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Tue, 23 Dec 2025 21:48:16 +0100 Subject: [PATCH 1/4] fix(alsa): hybrid enumeration and direction detection Combines ALSA hints with physical card probing to ensure all hardware devices are enumerated, even when hints don't list them. - Add hybrid enumeration: hints (virtual/configured) + physical probing - Add direction detection from hint metadata and physical hardware - Add Default implementation for Device - Remove unused StreamType enum Fixes #1079 --- CHANGELOG.md | 12 +++ src/host/alsa/enumerate.rs | 173 ++++++++++++++++++++++++++++--------- src/host/alsa/mod.rs | 109 +++++++++-------------- 3 files changed, 183 insertions(+), 111 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 975abbdf7..3b3f14238 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added +- **ALSA**: `Default` implementation for `Device` (returns the ALSA "default" device). + +### Fixed + +- **ALSA**: Device enumeration now includes both hints and physical cards. + +### Changed +- **ALSA**: Devices now report direction from hint metadata and physical hardware probing. + ## [0.17.0] - 2025-12-20 ### Added diff --git a/src/host/alsa/enumerate.rs b/src/host/alsa/enumerate.rs index 3ae312047..be754e58d 100644 --- a/src/host/alsa/enumerate.rs +++ b/src/host/alsa/enumerate.rs @@ -4,59 +4,137 @@ use std::{ }; use super::{alsa, Device}; -use crate::{BackendSpecificError, DevicesError}; +use crate::{BackendSpecificError, DeviceDirection, DevicesError}; -/// ALSA's implementation for `Devices`. -pub struct Devices { - hint_iter: alsa::device_name::HintIter, - enumerated_pcm_ids: HashSet, +const HW_PREFIX: &str = "hw"; +const PLUGHW_PREFIX: &str = "plughw"; + +/// Information about a physical device +struct PhysicalDevice { + card_index: u32, + card_name: Option, + device_index: u32, + device_name: Option, + direction: DeviceDirection, } -impl Devices { - pub fn new() -> Result { - // Enumerate ALL devices from ALSA hints (same as aplay -L) - alsa::device_name::HintIter::new_str(None, "pcm") - .map(|hint_iter| Self { - hint_iter, - enumerated_pcm_ids: HashSet::new(), - }) - .map_err(DevicesError::from) +/// Iterator over available ALSA PCM devices (physical hardware and virtual/plugin devices). +pub type Devices = std::vec::IntoIter; + +/// Enumerates all available ALSA PCM devices (physical hardware and virtual/plugin devices). +/// +/// We enumerate both ALSA hints and physical devices because: +/// - Hints provide virtual devices, user configurations, and card-specific devices with metadata +/// - Physical probing provides traditional numeric naming (hw:CARD=0,DEV=0) for compatibility +pub fn devices() -> Result { + let mut devices = Vec::new(); + let mut seen_pcm_ids = HashSet::new(); + + let physical_devices = physical_devices(); + + // Add all hint devices, including virtual devices + if let Ok(hints) = alsa::device_name::HintIter::new_str(None, "pcm") { + for hint in hints { + if let Ok(device) = Device::try_from(hint) { + seen_pcm_ids.insert(device.pcm_id.clone()); + devices.push(device); + } + } } -} -impl Iterator for Devices { - type Item = Device; - - fn next(&mut self) -> Option { - loop { - let hint = self.hint_iter.next()?; - if let Ok(device) = Self::Item::try_from(hint) { - if self.enumerated_pcm_ids.insert(device.pcm_id.clone()) { - return Some(device); - } else { - // Skip duplicate PCM IDs - continue; - } + // Add hw:/plughw: for all physical devices with numeric index (traditional naming) + for phys_dev in physical_devices { + for prefix in [HW_PREFIX, PLUGHW_PREFIX] { + let pcm_id = format!( + "{}:CARD={},DEV={}", + prefix, phys_dev.card_index, phys_dev.device_index + ); + + if seen_pcm_ids.insert(pcm_id.clone()) { + devices.push(Device { + pcm_id, + desc: Some(format_device_description(&phys_dev, prefix)), + direction: phys_dev.direction, + handles: Arc::new(Mutex::new(Default::default())), + }); } } } -} -pub fn default_input_device() -> Option { - Some(default_device()) + Ok(devices.into_iter()) } -pub fn default_output_device() -> Option { - Some(default_device()) +/// Formats device description in ALSA style: "Card Name, Device Name\nPurpose" +fn format_device_description(phys_dev: &PhysicalDevice, prefix: &str) -> String { + // "Card Name, Device Name" or variations + let first_line = match (&phys_dev.card_name, &phys_dev.device_name) { + (Some(card), Some(device)) => format!("{}, {}", card, device), + (Some(card), None) => card.clone(), + (None, Some(device)) => device.clone(), + (None, None) => format!("Card {}", phys_dev.card_index), + }; + + // ALSA standard description + let second_line = match prefix { + HW_PREFIX => "Direct hardware device without any conversions", + PLUGHW_PREFIX => "Hardware device with all software conversions", + _ => "", + }; + + format!("{}\n{}", first_line, second_line) } -pub fn default_device() -> Device { - Device { - pcm_id: "default".to_string(), - desc: Some("Default Audio Device".to_string()), - direction: None, - handles: Arc::new(Mutex::new(Default::default())), +fn physical_devices() -> Vec { + let mut devices = Vec::new(); + for card in alsa::card::Iter::new().filter_map(Result::ok) { + let card_index = card.get_index() as u32; + let ctl = match alsa::Ctl::new(&format!("{}:{}", HW_PREFIX, card_index), false) { + Ok(ctl) => ctl, + Err(_) => continue, + }; + let card_name = ctl + .card_info() + .ok() + .and_then(|info| info.get_name().ok().map(|s| s.to_string())); + + for device_index in alsa::ctl::DeviceIter::new(&ctl) { + let device_index = device_index as u32; + let playback_info = ctl + .pcm_info(device_index, 0, alsa::Direction::Playback) + .ok(); + let capture_info = ctl.pcm_info(device_index, 0, alsa::Direction::Capture).ok(); + + let (direction, device_name) = match (&playback_info, &capture_info) { + (Some(p_info), Some(_c_info)) => ( + DeviceDirection::Duplex, + p_info.get_name().ok().map(|s| s.to_string()), + ), + (Some(p_info), None) => ( + DeviceDirection::Output, + p_info.get_name().ok().map(|s| s.to_string()), + ), + (None, Some(c_info)) => ( + DeviceDirection::Input, + c_info.get_name().ok().map(|s| s.to_string()), + ), + (None, None) => { + // Device doesn't exist - skip + continue; + } + }; + + let device_name = device_name.unwrap_or_else(|| format!("Device {}", device_index)); + devices.push(PhysicalDevice { + card_index, + card_name: card_name.clone(), + device_index, + device_name: Some(device_name), + direction, + }); + } } + + devices } impl From for DevicesError { @@ -74,12 +152,25 @@ impl TryFrom for Device { description: "ALSA hint missing PCM ID".to_string(), })?; - // Include all devices from ALSA hints (matches `aplay -L` behavior) + // ALSA docs say that NULL indicates Duplex, but we deviate because that only indicates that + // both directions are defined in the ALSA config, not that both actually work. Virtual + // devices typically define both while only one direction actually works. + let direction = hint.direction.map_or(DeviceDirection::Unknown, Into::into); + Ok(Self { pcm_id: pcm_id.to_owned(), desc: hint.desc, - direction: None, + direction, handles: Arc::new(Mutex::new(Default::default())), }) } } + +impl From for DeviceDirection { + fn from(direction: alsa::Direction) -> Self { + match direction { + alsa::Direction::Playback => DeviceDirection::Output, + alsa::Direction::Capture => DeviceDirection::Input, + } + } +} diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index ac5f48677..c71c6c067 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -17,9 +17,10 @@ use std::{ }; use self::alsa::poll::Descriptors; -pub use self::enumerate::{default_input_device, default_output_device, Devices}; +pub use self::enumerate::Devices; use crate::{ + iter::{SupportedInputConfigs, SupportedOutputConfigs}, traits::{DeviceTrait, HostTrait, StreamTrait}, BackendSpecificError, BufferSize, BuildStreamError, ChannelCount, Data, DefaultStreamConfigError, DeviceDescription, DeviceDescriptionBuilder, DeviceDirection, @@ -29,23 +30,7 @@ use crate::{ SupportedStreamConfigRange, SupportedStreamConfigsError, I24, U24, }; -impl From for DeviceDirection { - fn from(direction: alsa::Direction) -> Self { - match direction { - alsa::Direction::Capture => DeviceDirection::Input, - alsa::Direction::Playback => DeviceDirection::Output, - } - } -} - -/// Parses ALSA multi-line description into separate lines. -fn parse_alsa_description(description: &str) -> Vec { - description - .lines() - .map(|line| line.trim().to_string()) - .filter(|line| !line.is_empty()) - .collect() -} +mod enumerate; // ALSA Buffer Size Behavior // ========================= @@ -98,11 +83,9 @@ fn parse_alsa_description(description: &str) -> Vec { // (start_threshold = 2 periods), ensuring low latency even with large multi-period ring // buffers. -pub use crate::iter::{SupportedInputConfigs, SupportedOutputConfigs}; +const DEFAULT_DEVICE: &str = "default"; -mod enumerate; - -/// The default linux, dragonfly, freebsd and netbsd host type. +/// The default Linux and BSD host type. #[derive(Debug)] pub struct Host; @@ -117,20 +100,20 @@ impl HostTrait for Host { type Device = Device; fn is_available() -> bool { - // Assume ALSA is always available on linux/dragonfly/freebsd/netbsd. + // Assume ALSA is always available on Linux and BSD. true } fn devices(&self) -> Result { - Devices::new() + enumerate::devices() } fn default_input_device(&self) -> Option { - default_input_device() + Some(Device::default()) } fn default_output_device(&self) -> Option { - default_output_device() + Some(Device::default()) } } @@ -312,15 +295,15 @@ impl DeviceHandles { pub struct Device { pcm_id: String, desc: Option, - direction: Option, + direction: DeviceDirection, handles: Arc>, } impl PartialEq for Device { fn eq(&self, other: &Self) -> bool { - // Devices are equal if they have the same PCM ID and direction. + // Devices are equal if they have the same PCM ID. // The handles field is not part of device identity. - self.pcm_id == other.pcm_id && self.direction == other.direction + self.pcm_id == other.pcm_id } } @@ -328,14 +311,7 @@ impl Eq for Device {} impl std::hash::Hash for Device { fn hash(&self, state: &mut H) { - // Hash based on PCM ID and direction for consistency with PartialEq self.pcm_id.hash(state); - // Manually hash direction since alsa::Direction doesn't implement Hash - match self.direction { - Some(alsa::Direction::Capture) => 0u8.hash(state), - Some(alsa::Direction::Playback) => 1u8.hash(state), - None => 2u8.hash(state), - } } } @@ -443,17 +419,19 @@ impl Device { .unwrap_or(&self.pcm_id) .to_string(); - let mut builder = DeviceDescriptionBuilder::new(name).driver(self.pcm_id.clone()); + let mut builder = DeviceDescriptionBuilder::new(name) + .driver(self.pcm_id.clone()) + .direction(self.direction); if let Some(ref desc) = self.desc { - let lines = parse_alsa_description(desc); + let lines = desc + .lines() + .map(|line| line.trim().to_string()) + .filter(|line| !line.is_empty()) + .collect(); builder = builder.extended(lines); } - if let Some(dir) = self.direction { - builder = builder.direction(dir.into()); - } - Ok(builder.build()) } @@ -657,6 +635,19 @@ impl Device { } } +impl Default for Device { + fn default() -> Self { + // "default" is a virtual ALSA device that redirects to the configured default. We cannot + // determine its actual capabilities without opening it, so we return Unknown direction. + Self { + pcm_id: DEFAULT_DEVICE.to_owned(), + desc: Some("Default Audio Device".to_string()), + direction: DeviceDirection::Unknown, + handles: Arc::new(Mutex::new(Default::default())), + } + } +} + struct StreamInner { // Flag used to check when to stop polling, regardless of the state of the stream // (e.g. broken due to a disconnected device). @@ -699,12 +690,6 @@ struct StreamInner { // Assume that the ALSA library is built with thread safe option. unsafe impl Sync for StreamInner {} -#[derive(Debug, Eq, PartialEq)] -enum StreamType { - Input, - Output, -} - pub struct Stream { /// The high-priority audio processing thread calling callbacks. /// Option used for moving out in destructor. @@ -806,13 +791,7 @@ fn input_stream_worker( PollDescriptorsFlow::Ready { status, delay_frames, - stream_type, } => { - debug_assert_eq!( - stream_type, - StreamType::Input, - "expected input stream, but polling descriptors indicated output", - ); if let Err(err) = process_input( stream, &mut ctxt.transfer_buffer, @@ -858,13 +837,7 @@ fn output_stream_worker( PollDescriptorsFlow::Ready { status, delay_frames, - stream_type, } => { - debug_assert_eq!( - stream_type, - StreamType::Output, - "expected output stream, but polling descriptors indicated input", - ); if let Err(err) = process_output( stream, &mut ctxt.transfer_buffer, @@ -903,7 +876,6 @@ enum PollDescriptorsFlow { Continue, Return, Ready { - stream_type: StreamType, status: alsa::pcm::Status, delay_frames: usize, }, @@ -945,14 +917,12 @@ fn poll_descriptors_and_prepare_buffer( let description = String::from("`alsa::poll()` returned POLLERR"); return Err(BackendSpecificError { description }); } - let stream_type = match revents { - alsa::poll::Flags::OUT => StreamType::Output, - alsa::poll::Flags::IN => StreamType::Input, - _ => { - // Nothing to process, poll again - return Ok(PollDescriptorsFlow::Continue); - } - }; + + // Check if data is ready for processing (either input or output) + if !revents.contains(alsa::poll::Flags::IN) && !revents.contains(alsa::poll::Flags::OUT) { + // Nothing to process, poll again + return Ok(PollDescriptorsFlow::Continue); + } let status = stream.channel.status()?; let avail_frames = match stream.channel.avail() { @@ -975,7 +945,6 @@ fn poll_descriptors_and_prepare_buffer( } Ok(PollDescriptorsFlow::Ready { - stream_type, status, delay_frames, }) From edf13faa9984f015428a1f164fc26b8d75c89b35 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Tue, 30 Dec 2025 16:10:39 +0100 Subject: [PATCH 2/4] fix(alsa): treat NULL ALSA IOID as Duplex Per ALSA docs, NULL IOID indicates both input and output; mark hints with NULL as DeviceDirection::Duplex. Whether a stream actually works in a direction must be determined by attempting to open it. --- src/host/alsa/enumerate.rs | 8 ++++---- src/traits.rs | 6 ++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/host/alsa/enumerate.rs b/src/host/alsa/enumerate.rs index be754e58d..194d37d2d 100644 --- a/src/host/alsa/enumerate.rs +++ b/src/host/alsa/enumerate.rs @@ -152,10 +152,10 @@ impl TryFrom for Device { description: "ALSA hint missing PCM ID".to_string(), })?; - // ALSA docs say that NULL indicates Duplex, but we deviate because that only indicates that - // both directions are defined in the ALSA config, not that both actually work. Virtual - // devices typically define both while only one direction actually works. - let direction = hint.direction.map_or(DeviceDirection::Unknown, Into::into); + // Per ALSA docs (https://alsa-project.org/alsa-doc/alsa-lib/group___hint.html), + // NULL IOID means both Input/Output. Whether a stream can actually open in a given + // direction can only be determined by attempting to open it. + let direction = hint.direction.map_or(DeviceDirection::Duplex, Into::into); Ok(Self { pcm_id: pcm_id.to_owned(), diff --git a/src/traits.rs b/src/traits.rs index 6e682d59f..1b0e717b4 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -130,15 +130,13 @@ pub trait DeviceTrait { /// True if the device supports audio input, otherwise false fn supports_input(&self) -> bool { self.supported_input_configs() - .map(|mut iter| iter.next().is_some()) - .unwrap_or(false) + .map_or(false, |mut iter| iter.next().is_some()) } /// True if the device supports audio output, otherwise false fn supports_output(&self) -> bool { self.supported_output_configs() - .map(|mut iter| iter.next().is_some()) - .unwrap_or(false) + .map_or(false, |mut iter| iter.next().is_some()) } /// An iterator yielding formats that are supported by the backend. From 1386415079c4465def0bf565696fefa3f7793275 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Tue, 30 Dec 2025 16:13:46 +0100 Subject: [PATCH 3/4] fix(alsa): avoid poisoning during enumeration * Don't open devices during enumeration to avoid leaking file descriptors from failed snd_pcm_open (e.g. alsaequal returning EPERM). * Treat EPERM and EAGAIN like ENOENT/EBUSY and return DeviceNotAvailable. --- src/host/alsa/mod.rs | 24 +++++++++++++++++++++++- src/traits.rs | 4 ++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index c71c6c067..b01ae4753 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -135,6 +135,25 @@ impl DeviceTrait for Device { Device::id(self) } + // Override trait defaults to avoid opening devices during enumeration. + // + // ALSA does not guarantee transactional cleanup on failed snd_pcm_open(). Opening plugins like + // alsaequal that fail with EPERM can leak FDs, poisoning the ALSA backend for the process + // lifetime (subsequent device opens fail with EBUSY until process exit). + fn supports_input(&self) -> bool { + matches!( + self.direction, + DeviceDirection::Input | DeviceDirection::Duplex + ) + } + + fn supports_output(&self) -> bool { + matches!( + self.direction, + DeviceDirection::Output | DeviceDirection::Duplex + ) + } + fn supported_input_configs( &self, ) -> Result { @@ -449,7 +468,10 @@ impl Device { .map_err(|e| (e, e.errno())); let handle = match handle_result { - Err((_, libc::ENOENT)) | Err((_, libc::EBUSY)) => { + Err((_, libc::ENOENT)) + | Err((_, libc::EBUSY)) + | Err((_, libc::EPERM)) + | Err((_, libc::EAGAIN)) => { return Err(SupportedStreamConfigsError::DeviceNotAvailable) } Err((_, libc::EINVAL)) => return Err(SupportedStreamConfigsError::InvalidArgument), diff --git a/src/traits.rs b/src/traits.rs index 1b0e717b4..2c3bccc28 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -130,13 +130,13 @@ pub trait DeviceTrait { /// True if the device supports audio input, otherwise false fn supports_input(&self) -> bool { self.supported_input_configs() - .map_or(false, |mut iter| iter.next().is_some()) + .is_ok_and(|mut iter| iter.next().is_some()) } /// True if the device supports audio output, otherwise false fn supports_output(&self) -> bool { self.supported_output_configs() - .map_or(false, |mut iter| iter.next().is_some()) + .is_ok_and(|mut iter| iter.next().is_some()) } /// An iterator yielding formats that are supported by the backend. From dfec6acfda658127aebe4ae7345252229e84813b Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Tue, 30 Dec 2025 21:56:18 +0100 Subject: [PATCH 4/4] refactor(alsa): handle more open errors * Map ENOENT, EBUSY, EPERM and EAGAIN returned when taking a PCM handle to DeviceNotAvailable. * Add a comment to clarify reuse of cached device handles to avoid opening the device twice. * Minor documentation improvements. --- src/host/alsa/mod.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index b01ae4753..884da59c9 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -274,7 +274,7 @@ struct DeviceHandles { impl DeviceHandles { /// Get a mutable reference to the `Option` for a specific `stream_type`. /// If the `Option` is `None`, the `alsa::PCM` will be opened and placed in - /// the `Option` before returning. If `handle_mut()` returns `Ok` the contained + /// the `Option` before returning. If `try_open()` returns `Ok` the contained /// `Option` is guaranteed to be `Some(..)`. fn try_open( &mut self, @@ -320,8 +320,6 @@ pub struct Device { impl PartialEq for Device { fn eq(&self, other: &Self) -> bool { - // Devices are equal if they have the same PCM ID. - // The handles field is not part of device identity. self.pcm_id == other.pcm_id } } @@ -363,19 +361,22 @@ impl Device { } } - let handle_result = self + let handle = match self .handles .lock() .unwrap() .take(&self.pcm_id, stream_type) - .map_err(|e| (e, e.errno())); - - let handle = match handle_result { - Err((_, libc::EBUSY)) => return Err(BuildStreamError::DeviceNotAvailable), + .map_err(|e| (e, e.errno())) + { + Err((_, libc::ENOENT)) + | Err((_, libc::EBUSY)) + | Err((_, libc::EPERM)) + | Err((_, libc::EAGAIN)) => return Err(BuildStreamError::DeviceNotAvailable), Err((_, libc::EINVAL)) => return Err(BuildStreamError::InvalidArgument), Err((e, _)) => return Err(e.into()), Ok(handle) => handle, }; + let can_pause = set_hw_params_from_format(&handle, conf, sample_format)?; let period_samples = set_sw_params_from_format(&handle, conf, stream_type)?; @@ -462,12 +463,15 @@ impl Device { &self, stream_t: alsa::Direction, ) -> Result, SupportedStreamConfigsError> { + // Open device handle and cache it for reuse in build_stream_inner(). + // This avoids opening the device twice in the common workflow: + // 1. Query supported configs (opens and caches handle) + // 2. Build stream (takes cached handle, or opens if not cached) let mut guard = self.handles.lock().unwrap(); - let handle_result = guard + let pcm = match guard .get_mut(&self.pcm_id, stream_t) - .map_err(|e| (e, e.errno())); - - let handle = match handle_result { + .map_err(|e| (e, e.errno())) + { Err((_, libc::ENOENT)) | Err((_, libc::EBUSY)) | Err((_, libc::EPERM)) @@ -476,10 +480,10 @@ impl Device { } Err((_, libc::EINVAL)) => return Err(SupportedStreamConfigsError::InvalidArgument), Err((e, _)) => return Err(e.into()), - Ok(handle) => handle, + Ok(pcm) => pcm, }; - let hw_params = alsa::pcm::HwParams::any(handle)?; + let hw_params = alsa::pcm::HwParams::any(pcm)?; // Test both LE and BE formats to detect what the hardware actually supports. // LE is listed first as it's the common case for most audio hardware.