From fb30d291ce4ce87bbe80c0a997ff12e364a4e532 Mon Sep 17 00:00:00 2001 From: Alex Moon Date: Wed, 18 Nov 2020 10:22:41 -0700 Subject: [PATCH 1/3] Avoid calling alsa::PCM::new() multiple times. Ensures that alsa::PCM::new() is called only once for a given Device so that we are not rapidly opening and closing ALSA devices. --- Cargo.toml | 1 + src/host/alsa/enumerate.rs | 36 ++++++++++++++++---------- src/host/alsa/mod.rs | 53 ++++++++++++++++++++++++++++++++------ 3 files changed, 68 insertions(+), 22 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fd831e9a8..b5daa78f8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ lazy_static = "1.3" alsa = "0.4.3" nix = "0.15.0" libc = "0.2.65" +parking_lot = "0.11" jack = { version = "0.6.5", optional = true } [target.'cfg(any(target_os = "macos", target_os = "ios"))'.dependencies] diff --git a/src/host/alsa/enumerate.rs b/src/host/alsa/enumerate.rs index a269d7b8c..00e2bb979 100644 --- a/src/host/alsa/enumerate.rs +++ b/src/host/alsa/enumerate.rs @@ -1,4 +1,5 @@ use super::alsa; +use super::parking_lot::Mutex; use super::Device; use {BackendSpecificError, DevicesError}; @@ -48,21 +49,16 @@ impl Iterator for Devices { }; // See if the device has an available output stream. - let has_available_output = { - let playback_handle = - alsa::pcm::PCM::new(&name, alsa::Direction::Playback, true); - playback_handle.is_ok() - }; + let playback = alsa::pcm::PCM::new(&name, alsa::Direction::Playback, true).ok(); // See if the device has an available input stream. - let has_available_input = { - let capture_handle = - alsa::pcm::PCM::new(&name, alsa::Direction::Capture, true); - capture_handle.is_ok() - }; + let capture = alsa::pcm::PCM::new(&name, alsa::Direction::Capture, true).ok(); - if has_available_output || has_available_input { - return Some(Device(name)); + if playback.is_some() || capture.is_some() { + return Some(Device { + name, + handles: Mutex::new(super::DeviceHandles { playback, capture }), + }); } } } @@ -72,12 +68,24 @@ impl Iterator for Devices { #[inline] pub fn default_input_device() -> Option { - Some(Device("default".to_owned())) + Some(Device { + name: "default".to_owned(), + handles: Mutex::new(super::DeviceHandles { + playback: None, + capture: None, + }), + }) } #[inline] pub fn default_output_device() -> Option { - Some(Device("default".to_owned())) + Some(Device { + name: "default".to_owned(), + handles: Mutex::new(super::DeviceHandles { + playback: None, + capture: None, + }), + }) } impl From for DevicesError { diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index b11e3282a..809709a9f 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -1,7 +1,9 @@ extern crate alsa; extern crate libc; +extern crate parking_lot; use self::alsa::poll::Descriptors; +use self::parking_lot::Mutex; use crate::{ BackendSpecificError, BufferSize, BuildStreamError, ChannelCount, Data, DefaultStreamConfigError, DeviceNameError, DevicesError, InputCallbackInfo, OutputCallbackInfo, @@ -163,8 +165,15 @@ impl Drop for TriggerReceiver { } } -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct Device(String); +struct DeviceHandles { + playback: Option, + capture: Option, +} + +pub struct Device { + name: String, + handles: Mutex, +} impl Device { fn build_stream_inner( @@ -173,10 +182,21 @@ impl Device { sample_format: SampleFormat, stream_type: alsa::Direction, ) -> Result { - let name = &self.0; + let name = &self.name; - let handle = match alsa::pcm::PCM::new(name, stream_type, true).map_err(|e| (e, e.errno())) - { + let device_handle = { + let mut guard = self.handles.lock(); + match stream_type { + alsa::Direction::Playback => guard.playback.take(), + alsa::Direction::Capture => guard.capture.take(), + } + }; + + let handle_result = Ok(device_handle).transpose().unwrap_or_else(|| { + alsa::pcm::PCM::new(name, stream_type, true).map_err(|e| (e, e.errno())) + }); + + let handle = match handle_result { Err((_, Some(nix::errno::Errno::EBUSY))) => { return Err(BuildStreamError::DeviceNotAvailable) } @@ -229,16 +249,33 @@ impl Device { #[inline] fn name(&self) -> Result { - Ok(self.0.clone()) + Ok(self.name.clone()) } fn supported_configs( &self, stream_t: alsa::Direction, ) -> Result, SupportedStreamConfigsError> { - let name = &self.0; + let name = &self.name; + + let mut guard = self.handles.lock(); + + let device_handle = match stream_t { + alsa::Direction::Playback => &mut guard.playback, + alsa::Direction::Capture => &mut guard.capture, + }; + + let handle_result = match device_handle { + Some(handle) => Ok(handle), + None => alsa::pcm::PCM::new(name, stream_t, true) + .map(|handle| { + *device_handle = Some(handle); + device_handle.as_mut().unwrap() + }) + .map_err(|e| (e, e.errno())), + }; - let handle = match alsa::pcm::PCM::new(name, stream_t, true).map_err(|e| (e, e.errno())) { + let handle = match handle_result { Err((_, Some(nix::errno::Errno::ENOENT))) | Err((_, Some(nix::errno::Errno::EBUSY))) => { return Err(SupportedStreamConfigsError::DeviceNotAvailable) From b89df31797d4d98a84458defabb67474064d7913 Mon Sep 17 00:00:00 2001 From: Alex Moon Date: Mon, 7 Dec 2020 16:14:21 -0700 Subject: [PATCH 2/3] Refactor ALSA handle manipulation into methods on DeviceHandles. --- src/host/alsa/mod.rs | 73 ++++++++++++++++++++++++++------------------ 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index 809709a9f..cee47232c 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -170,6 +170,41 @@ struct DeviceHandles { capture: Option, } +impl DeviceHandles { + fn handle_mut(&mut self, stream_type: alsa::Direction) -> &mut Option { + match stream_type { + alsa::Direction::Playback => &mut self.playback, + alsa::Direction::Capture => &mut self.capture, + } + } + + /// Get a mutable reference to the `alsa::PCM` handle for a specific `stream_type`. + /// If the handle is not yet opened, it will be opened and stored in `self`. + fn get_mut( + &mut self, + name: &str, + stream_type: alsa::Direction, + ) -> Result<&mut alsa::PCM, alsa::Error> { + let pcm = self.handle_mut(stream_type); + match pcm { + Some(pcm) => Ok(pcm), + None => { + *pcm = Some(alsa::pcm::PCM::new(name, stream_type, true)?); + Ok(pcm.as_mut().unwrap()) + } + } + } + + /// Take ownership of the `alsa::PCM` handle for a specific `stream_type`. + /// If the handle is not yet opened, it will be opened and returned. + fn take(&mut self, name: &str, stream_type: alsa::Direction) -> Result { + match self.handle_mut(stream_type).take() { + Some(pcm) => Ok(pcm), + None => Ok(alsa::pcm::PCM::new(name, stream_type, true)?), + } + } +} + pub struct Device { name: String, handles: Mutex, @@ -182,19 +217,11 @@ impl Device { sample_format: SampleFormat, stream_type: alsa::Direction, ) -> Result { - let name = &self.name; - - let device_handle = { - let mut guard = self.handles.lock(); - match stream_type { - alsa::Direction::Playback => guard.playback.take(), - alsa::Direction::Capture => guard.capture.take(), - } - }; - - let handle_result = Ok(device_handle).transpose().unwrap_or_else(|| { - alsa::pcm::PCM::new(name, stream_type, true).map_err(|e| (e, e.errno())) - }); + let handle_result = self + .handles + .lock() + .take(&self.name, stream_type) + .map_err(|e| (e, e.errno())); let handle = match handle_result { Err((_, Some(nix::errno::Errno::EBUSY))) => { @@ -256,24 +283,10 @@ impl Device { &self, stream_t: alsa::Direction, ) -> Result, SupportedStreamConfigsError> { - let name = &self.name; - let mut guard = self.handles.lock(); - - let device_handle = match stream_t { - alsa::Direction::Playback => &mut guard.playback, - alsa::Direction::Capture => &mut guard.capture, - }; - - let handle_result = match device_handle { - Some(handle) => Ok(handle), - None => alsa::pcm::PCM::new(name, stream_t, true) - .map(|handle| { - *device_handle = Some(handle); - device_handle.as_mut().unwrap() - }) - .map_err(|e| (e, e.errno())), - }; + let handle_result = guard + .get_mut(&self.name, stream_t) + .map_err(|e| (e, e.errno())); let handle = match handle_result { Err((_, Some(nix::errno::Errno::ENOENT))) From 9c4e240a7c4598575cb52d08dc1a7f8678873490 Mon Sep 17 00:00:00 2001 From: Alex Moon Date: Wed, 9 Dec 2020 08:33:18 -0700 Subject: [PATCH 3/3] Further refactoring of DeviceHandles. Also fixes rustaudio/cpal#515. --- src/host/alsa/enumerate.rs | 46 +++++++++----------------------------- src/host/alsa/mod.rs | 46 ++++++++++++++++++++++++++------------ 2 files changed, 42 insertions(+), 50 deletions(-) diff --git a/src/host/alsa/enumerate.rs b/src/host/alsa/enumerate.rs index 00e2bb979..32f1894e8 100644 --- a/src/host/alsa/enumerate.rs +++ b/src/host/alsa/enumerate.rs @@ -1,6 +1,6 @@ use super::alsa; use super::parking_lot::Mutex; -use super::Device; +use super::{Device, DeviceHandles}; use {BackendSpecificError, DevicesError}; /// ALSA implementation for `Devices`. @@ -27,37 +27,17 @@ impl Iterator for Devices { match self.hint_iter.next() { None => return None, Some(hint) => { - let name = hint.name; - - let io = hint.direction; - - if let Some(io) = io { - if io != alsa::Direction::Playback { - continue; - } - } - - let name = match name { - Some(name) => { - // Ignoring the `null` device. - if name == "null" { - continue; - } - name - } - _ => continue, + let name = match hint.name { + None => continue, + // Ignoring the `null` device. + Some(name) if name == "null" => continue, + Some(name) => name, }; - // See if the device has an available output stream. - let playback = alsa::pcm::PCM::new(&name, alsa::Direction::Playback, true).ok(); - - // See if the device has an available input stream. - let capture = alsa::pcm::PCM::new(&name, alsa::Direction::Capture, true).ok(); - - if playback.is_some() || capture.is_some() { + if let Ok(handles) = DeviceHandles::open(&name) { return Some(Device { name, - handles: Mutex::new(super::DeviceHandles { playback, capture }), + handles: Mutex::new(handles), }); } } @@ -70,10 +50,7 @@ impl Iterator for Devices { pub fn default_input_device() -> Option { Some(Device { name: "default".to_owned(), - handles: Mutex::new(super::DeviceHandles { - playback: None, - capture: None, - }), + handles: Mutex::new(Default::default()), }) } @@ -81,10 +58,7 @@ pub fn default_input_device() -> Option { pub fn default_output_device() -> Option { Some(Device { name: "default".to_owned(), - handles: Mutex::new(super::DeviceHandles { - playback: None, - capture: None, - }), + handles: Mutex::new(Default::default()), }) } diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index cee47232c..d306519dd 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -165,17 +165,45 @@ impl Drop for TriggerReceiver { } } +#[derive(Default)] struct DeviceHandles { playback: Option, capture: Option, } impl DeviceHandles { - fn handle_mut(&mut self, stream_type: alsa::Direction) -> &mut Option { - match stream_type { + /// Create `DeviceHandles` for `name` and try to open a handle for both + /// directions. Returns `Ok` if either direction is opened successfully. + fn open(name: &str) -> Result { + let mut handles = Self::default(); + let playback_err = handles.try_open(name, alsa::Direction::Playback).err(); + let capture_err = handles.try_open(name, alsa::Direction::Capture).err(); + if let Some(err) = capture_err.and(playback_err) { + Err(err) + } else { + Ok(handles) + } + } + + /// 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 + /// `Option` is guaranteed to be `Some(..)`. + fn try_open( + &mut self, + name: &str, + stream_type: alsa::Direction, + ) -> Result<&mut Option, alsa::Error> { + let handle = match stream_type { alsa::Direction::Playback => &mut self.playback, alsa::Direction::Capture => &mut self.capture, + }; + + if handle.is_none() { + *handle = Some(alsa::pcm::PCM::new(name, stream_type, true)?); } + + Ok(handle) } /// Get a mutable reference to the `alsa::PCM` handle for a specific `stream_type`. @@ -185,23 +213,13 @@ impl DeviceHandles { name: &str, stream_type: alsa::Direction, ) -> Result<&mut alsa::PCM, alsa::Error> { - let pcm = self.handle_mut(stream_type); - match pcm { - Some(pcm) => Ok(pcm), - None => { - *pcm = Some(alsa::pcm::PCM::new(name, stream_type, true)?); - Ok(pcm.as_mut().unwrap()) - } - } + Ok(self.try_open(name, stream_type)?.as_mut().unwrap()) } /// Take ownership of the `alsa::PCM` handle for a specific `stream_type`. /// If the handle is not yet opened, it will be opened and returned. fn take(&mut self, name: &str, stream_type: alsa::Direction) -> Result { - match self.handle_mut(stream_type).take() { - Some(pcm) => Ok(pcm), - None => Ok(alsa::pcm::PCM::new(name, stream_type, true)?), - } + Ok(self.try_open(name, stream_type)?.take().unwrap()) } }