From 2abb970da799c080d386e8d2967bd9a29547a16f Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Sun, 26 Jul 2020 17:27:26 -0700 Subject: [PATCH 01/17] Add support for OpenBSD platform via sndio host --- .gitignore | 1 + Cargo.toml | 4 + README.md | 1 + asio-sys/Cargo.toml | 2 +- src/error.rs | 2 +- src/host/mod.rs | 2 + src/host/sndio/adapters.rs | 100 ++++ src/host/sndio/endian.rs | 14 + src/host/sndio/mod.rs | 901 +++++++++++++++++++++++++++++++++++++ src/host/sndio/runner.rs | 318 +++++++++++++ src/platform/mod.rs | 19 + 11 files changed, 1362 insertions(+), 2 deletions(-) create mode 100644 src/host/sndio/adapters.rs create mode 100644 src/host/sndio/endian.rs create mode 100644 src/host/sndio/mod.rs create mode 100644 src/host/sndio/runner.rs diff --git a/.gitignore b/.gitignore index 0289afe13..a2acc0f72 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ .DS_Store recorded.wav rls*.log +rusty-tags.* diff --git a/Cargo.toml b/Cargo.toml index 8b9f163fd..20909647b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,10 @@ nix = "0.15.0" libc = "0.2.65" jack = { version = "0.6.5", optional = true } +[target.'cfg(target_os = "openbsd")'.dependencies] +sndio-sys = "0.0.*" +libc = "0.2.65" + [target.'cfg(any(target_os = "macos", target_os = "ios"))'.dependencies] coreaudio-rs = { version = "0.9.1", default-features = false, features = ["audio_unit", "core_audio"] } core-foundation-sys = "0.6.2" # For linking to CoreFoundation.framework and handling device name `CFString`s. diff --git a/README.md b/README.md index 72d24eb66..3e9ac8b71 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ Currently supported hosts include: - macOS (via CoreAudio) - iOS (via CoreAudio) - Android (via Oboe) +- OpenBSD (via sndio) - Emscripten Note that on Linux, the ALSA development files are required. These are provided diff --git a/asio-sys/Cargo.toml b/asio-sys/Cargo.toml index 47b9964d6..253cb87ff 100644 --- a/asio-sys/Cargo.toml +++ b/asio-sys/Cargo.toml @@ -9,7 +9,7 @@ license = "Apache-2.0" keywords = ["audio", "sound", "asio", "steinberg"] build = "build.rs" -[target.'cfg(any(target_os = "windows"))'.build-dependencies] +[build-dependencies] bindgen = "0.54.0" walkdir = "2" cc = "1.0.25" diff --git a/src/error.rs b/src/error.rs index f404f297b..952232eec 100644 --- a/src/error.rs +++ b/src/error.rs @@ -147,7 +147,7 @@ pub enum PauseStreamError { } /// Errors that might occur while a stream is running. -#[derive(Debug, Error)] +#[derive(Debug, Error, Clone)] pub enum StreamError { /// The device no longer exists. This can happen if the device is disconnected while the /// program is running. diff --git a/src/host/mod.rs b/src/host/mod.rs index 555b01022..cd33107c4 100644 --- a/src/host/mod.rs +++ b/src/host/mod.rs @@ -14,6 +14,8 @@ pub(crate) mod jack; pub(crate) mod null; #[cfg(target_os = "android")] pub(crate) mod oboe; +#[cfg(target_os = "openbsd")] +pub(crate) mod sndio; #[cfg(windows)] pub(crate) mod wasapi; #[cfg(all(target_arch = "wasm32", feature = "wasm-bindgen"))] diff --git a/src/host/sndio/adapters.rs b/src/host/sndio/adapters.rs new file mode 100644 index 000000000..d26f31681 --- /dev/null +++ b/src/host/sndio/adapters.rs @@ -0,0 +1,100 @@ +use crate::{Data, InputCallbackInfo, OutputCallbackInfo, Sample, SampleFormat}; + +/// When given an input data callback that expects samples in the specified sample format, return +/// an input data callback that expects samples in the I16 sample format. The `buffer_size` is in +/// samples. +pub(super) fn input_adapter_callback( + mut original_data_callback: D, + buffer_size: usize, + sample_format: SampleFormat, +) -> Box +where + D: FnMut(&Data, &InputCallbackInfo) + Send + 'static, +{ + if sample_format == SampleFormat::I16 { + // no-op + return Box::new(original_data_callback); + } + + // Make the backing buffer for the Data used in the closure. + let mut buf: Vec = vec![0].repeat(buffer_size * sample_format.sample_size()); + + Box::new(move |data: &Data, info: &InputCallbackInfo| { + // Note: we construct adapted_data here instead of in the parent function because buf needs + // to be owned by the closure. + let mut adapted_data = + unsafe { Data::from_parts(buf.as_mut_ptr() as *mut _, buffer_size, sample_format) }; + let data_slice: &[i16] = data.as_slice().unwrap(); // unwrap OK because data is always i16 + match sample_format { + SampleFormat::F32 => { + let adapted_slice: &mut [f32] = adapted_data.as_slice_mut().unwrap(); // unwrap OK because of the match + assert_eq!(data_slice.len(), adapted_slice.len()); + for (i, adapted_ref) in adapted_slice.iter_mut().enumerate() { + *adapted_ref = data_slice[i].to_f32(); + } + } + SampleFormat::U16 => { + let adapted_slice: &mut [u16] = adapted_data.as_slice_mut().unwrap(); // unwrap OK because of the match + assert_eq!(data_slice.len(), adapted_slice.len()); + for (i, adapted_ref) in adapted_slice.iter_mut().enumerate() { + *adapted_ref = data_slice[i].to_u16(); + } + } + SampleFormat::I16 => { + unreachable!("i16 should've already been handled above"); + } + } + original_data_callback(&adapted_data, info); + }) +} + +/// When given an output data callback that expects a place to write samples in the specified +/// sample format, return an output data callback that expects a place to write samples in the I16 +/// sample format. The `buffer_size` is in samples. +pub(super) fn output_adapter_callback( + mut original_data_callback: D, + buffer_size: usize, + sample_format: SampleFormat, +) -> Box +where + D: FnMut(&mut Data, &OutputCallbackInfo) + Send + 'static, +{ + if sample_format == SampleFormat::I16 { + // no-op + return Box::new(original_data_callback); + } + + // Make the backing buffer for the Data used in the closure. + let mut buf: Vec = vec![0].repeat(buffer_size * sample_format.sample_size()); + + Box::new(move |data: &mut Data, info: &OutputCallbackInfo| { + // Note: we construct adapted_data here instead of in the parent function because buf needs + // to be owned by the closure. + let mut adapted_data = + unsafe { Data::from_parts(buf.as_mut_ptr() as *mut _, buffer_size, sample_format) }; + + // Populate buf / adapted_data. + original_data_callback(&mut adapted_data, info); + + let data_slice: &mut [i16] = data.as_slice_mut().unwrap(); // unwrap OK because data is always i16 + match sample_format { + SampleFormat::F32 => { + let adapted_slice: &[f32] = adapted_data.as_slice().unwrap(); // unwrap OK because of the match + assert_eq!(data_slice.len(), adapted_slice.len()); + for (i, data_ref) in data_slice.iter_mut().enumerate() { + *data_ref = adapted_slice[i].to_i16(); + } + } + SampleFormat::U16 => { + let adapted_slice: &[u16] = adapted_data.as_slice().unwrap(); // unwrap OK because of the match + assert_eq!(data_slice.len(), adapted_slice.len()); + for (i, data_ref) in data_slice.iter_mut().enumerate() { + *data_ref = adapted_slice[i].to_i16(); + } + } + SampleFormat::I16 => { + unreachable!("i16 should've already been handled above"); + } + } + }) +} diff --git a/src/host/sndio/endian.rs b/src/host/sndio/endian.rs new file mode 100644 index 000000000..73b9153d2 --- /dev/null +++ b/src/host/sndio/endian.rs @@ -0,0 +1,14 @@ +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(super) enum Endian { + BE, + LE, +} + +pub(super) fn get_endianness() -> Endian { + let n: i16 = 1; + match n.to_ne_bytes()[0] { + 1 => Endian::LE, + 0 => Endian::BE, + _ => unreachable!("unexpected value in byte"), + } +} diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs new file mode 100644 index 000000000..d99f4ce12 --- /dev/null +++ b/src/host/sndio/mod.rs @@ -0,0 +1,901 @@ +extern crate libc; +extern crate sndio_sys; + +mod adapters; +mod endian; +mod runner; +use self::adapters::{input_adapter_callback, output_adapter_callback}; +use self::runner::runner; + +use std::collections::HashMap; +use std::convert::From; +use std::mem::{self, MaybeUninit}; +use std::sync::{mpsc, Arc, Mutex}; +use std::thread; + +use thiserror::Error; + +use crate::{ + BackendSpecificError, BufferSize, BuildStreamError, Data, DefaultStreamConfigError, + DeviceNameError, DevicesError, HostUnavailable, InputCallbackInfo, OutputCallbackInfo, + PauseStreamError, PlayStreamError, SampleFormat, SampleRate, StreamConfig, StreamError, + SupportedBufferSize, SupportedStreamConfig, SupportedStreamConfigRange, + SupportedStreamConfigsError, +}; + +use traits::{DeviceTrait, HostTrait, StreamTrait}; + +pub type SupportedInputConfigs = ::std::vec::IntoIter; +pub type SupportedOutputConfigs = ::std::vec::IntoIter; + +/// Default multiple of the round field of a sio_par struct to use for the buffer size. +const DEFAULT_ROUND_MULTIPLE: usize = 2; + +const DEFAULT_SAMPLE_RATE: SampleRate = SampleRate(48000); +const SUPPORTED_SAMPLE_RATES: &[SampleRate] = + &[SampleRate(8000), SampleRate(44100), SampleRate(48000)]; + +#[derive(Clone, Debug, Error)] +pub enum SndioError { + #[error("The requested device is no longer available. For example, it has been unplugged.")] + DeviceNotAvailable, + + #[error("{0}")] + BackendSpecific(BackendSpecificError), +} + +impl From for BuildStreamError { + fn from(e: SndioError) -> BuildStreamError { + match e { + SndioError::DeviceNotAvailable => BuildStreamError::DeviceNotAvailable, + SndioError::BackendSpecific(bse) => BuildStreamError::BackendSpecific { err: bse }, + } + } +} + +impl From for DefaultStreamConfigError { + fn from(e: SndioError) -> DefaultStreamConfigError { + match e { + SndioError::DeviceNotAvailable => DefaultStreamConfigError::DeviceNotAvailable, + SndioError::BackendSpecific(bse) => { + DefaultStreamConfigError::BackendSpecific { err: bse } + } + } + } +} + +impl From for PauseStreamError { + fn from(e: SndioError) -> PauseStreamError { + match e { + SndioError::DeviceNotAvailable => PauseStreamError::DeviceNotAvailable, + SndioError::BackendSpecific(bse) => PauseStreamError::BackendSpecific { err: bse }, + } + } +} + +impl From for StreamError { + fn from(e: SndioError) -> StreamError { + match e { + SndioError::DeviceNotAvailable => StreamError::DeviceNotAvailable, + SndioError::BackendSpecific(bse) => StreamError::BackendSpecific { err: bse }, + } + } +} + +impl From for SupportedStreamConfigsError { + fn from(e: SndioError) -> SupportedStreamConfigsError { + match e { + SndioError::DeviceNotAvailable => SupportedStreamConfigsError::DeviceNotAvailable, + SndioError::BackendSpecific(bse) => { + SupportedStreamConfigsError::BackendSpecific { err: bse } + } + } + } +} + +pub struct Devices { + returned: bool, +} + +impl Iterator for Devices { + type Item = Device; + fn next(&mut self) -> Option { + if self.returned { + None + } else { + self.returned = true; + Some(Device::new()) + } + } +} + +impl Devices { + fn new() -> Devices { + Devices { returned: false } + } +} + +/// The shared state between Device and Stream. Responsible for closing handle when dropped. +struct InnerState { + /// If device has been open with sio_open, contains a handle. Note that even though this is a + /// pointer type and so doesn't follow Rust's borrowing rules, we should be careful not to copy + /// it out because that may render Mutex ineffective in enforcing exclusive access. + hdl: Option<*mut sndio_sys::sio_hdl>, + + /// Buffer overrun/underrun behavior -- ignore/sync/error? + behavior: BufferXrunBehavior, + + /// If a buffer size was chosen, contains that value. + buffer_size: Option, + + /// If the device was configured, stores the sndio-configured parameters. + par: Option, + + /// Map of sample rate to parameters. + /// Guaranteed to not be None if hdl is not None. + sample_rate_to_par: Option>, + + /// Indicates if the read/write thread is started, shutting down, or stopped. + status: Status, + + /// Each input Stream that has not been dropped has its callbacks in an element of this Vec. + /// The last element is guaranteed to not be None. + input_callbacks: Vec>, + + /// Each output Stream that has not been dropped has its callbacks in an element of this Vec. + /// The last element is guaranteed to not be None. + output_callbacks: Vec>, + + /// Channel used for signalling that the runner thread should wakeup because there is now a + /// Stream. This will only be None if there is no runner thread. + wakeup_sender: Option>, +} + +struct InputCallbacks { + data_callback: Box, + error_callback: Box, +} + +struct OutputCallbacks { + data_callback: Box, + error_callback: Box, +} + +unsafe impl Send for InnerState {} + +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +enum Status { + /// Initial state. No thread running. Device/Stream methods will start thread and change this + /// to Running. + Stopped, + + /// Thread is running (unless it encountered an error). + Running, +} + +impl InnerState { + fn new() -> Self { + InnerState { + hdl: None, + behavior: BufferXrunBehavior::Sync, + par: None, + sample_rate_to_par: None, + buffer_size: None, + status: Status::Stopped, + input_callbacks: vec![], + output_callbacks: vec![], + wakeup_sender: None, + } + } + + fn open(&mut self) -> Result<(), SndioError> { + if self.hdl.is_some() { + // Already open + return Ok(()); + } + + let hdl = unsafe { + // The transmute is needed because this C string is *const u8 in one place but *const i8 in another place. + let devany_ptr = mem::transmute::<_, *const i8>(sndio_sys::SIO_DEVANY as *const _); + let nonblocking = true as i32; + sndio_sys::sio_open( + devany_ptr, + sndio_sys::SIO_PLAY | sndio_sys::SIO_REC, + nonblocking, + ) + }; + if hdl.is_null() { + return Err(SndioError::DeviceNotAvailable); + } + self.hdl = Some(hdl); + + let mut sample_rate_to_par = HashMap::new(); + for rate in SUPPORTED_SAMPLE_RATES { + let mut par = new_sio_par(); + + // Use I16 at 48KHz; mono playback & record + par.bits = 16; + par.sig = 1; + par.le = match endian::get_endianness() { + endian::Endian::BE => 0, + endian::Endian::LE => 1, + }; // Native byte order + par.rchan = 1; // mono record + par.pchan = 1; // mono playback + par.rate = rate.0; + par.xrun = match self.behavior { + BufferXrunBehavior::Ignore => 0, + BufferXrunBehavior::Sync => 1, + BufferXrunBehavior::Error => 2, + }; + + // Set it on device and get it back to see what is valid. + self.negotiate_params(&mut par)?; + + if par.rchan != 1 { + return Err(backend_specific_error(format!( + "unexpected number of record channels: {}", + par.rchan + ))); + } + + if par.pchan != 1 { + return Err(backend_specific_error(format!( + "unexpected number of playback channels: {}", + par.pchan + ))); + } + + if par.rate != rate.0 { + return Err(backend_specific_error(format!( + "unexpected sample rate (frames per second): expected {}, got {}", + rate.0, par.rate + ))); + } + + // TODO: more checks -- bits, bps, sig, le, msb + + sample_rate_to_par.insert(rate.0, par); + } + self.sample_rate_to_par = Some(sample_rate_to_par); + Ok(()) + } + + fn start(&mut self) -> Result<(), SndioError> { + if self.hdl.is_none() { + return Err(backend_specific_error( + "cannot start a device that hasn't been opened yet", + )); + } + let status = unsafe { + // "The sio_start() function puts the device in a waiting state: the device + // will wait for playback data to be provided (using the sio_write() + // function). Once enough data is queued to ensure that play buffers will + // not underrun, actual playback is started automatically." + sndio_sys::sio_start(self.hdl.unwrap()) // Unwrap OK because of check above + }; + if status != 1 { + return Err(backend_specific_error("failed to start stream")); + } + Ok(()) + } + + fn stop(&mut self) -> Result<(), SndioError> { + if self.hdl.is_none() { + // Nothing to do -- device is not open. + return Ok(()); + } + let status = unsafe { + // The sio_stop() function puts the audio subsystem in the same state as before + // sio_start() is called. It stops recording, drains the play buffer and then stops + // playback. If samples to play are queued but playback hasn't started yet then + // playback is forced immediately; playback will actually stop once the buffer is + // drained. In no case are samples in the play buffer discarded. + sndio_sys::sio_stop(self.hdl.unwrap()) // Unwrap OK because of check above + }; + if status != 1 { + return Err(backend_specific_error("error calling sio_stop")); + } + Ok(()) + } + + // TODO: make these 4 methods generic (new CallbackSet where T is either InputCallbacks or OutputCallbacks) + /// Puts the supplied callbacks into the vector in the first free position, or at the end. The + /// index of insertion is returned. + fn add_output_callbacks(&mut self, callbacks: OutputCallbacks) -> usize { + for (i, cbs) in self.output_callbacks.iter_mut().enumerate() { + if cbs.is_none() { + *cbs = Some(callbacks); + return i; + } + } + // If there were previously no callbacks, wakeup the runner thread. + if self.input_callbacks.len() == 0 && self.output_callbacks.len() == 0 { + if let Some(ref sender) = self.wakeup_sender { + let _ = sender.send(()); + } + } + self.output_callbacks.push(Some(callbacks)); + self.output_callbacks.len() - 1 + } + + /// Removes the callbacks at specified index, returning them. Panics if the index is invalid + /// (out of range or there is a None element at that position). + fn remove_output_callbacks(&mut self, index: usize) -> OutputCallbacks { + let cbs = self.output_callbacks[index].take().unwrap(); + while self.output_callbacks.len() > 0 + && self.output_callbacks[self.output_callbacks.len() - 1].is_none() + { + self.output_callbacks.pop(); + } + cbs + } + + /// Puts the supplied callbacks into the vector in the first free position, or at the end. The + /// index of insertion is returned. + fn add_input_callbacks(&mut self, callbacks: InputCallbacks) -> usize { + for (i, cbs) in self.input_callbacks.iter_mut().enumerate() { + if cbs.is_none() { + *cbs = Some(callbacks); + return i; + } + } + // If there were previously no callbacks, wakeup the runner thread. + if self.input_callbacks.len() == 0 && self.output_callbacks.len() == 0 { + if let Some(ref sender) = self.wakeup_sender { + let _ = sender.send(()); + } + } + self.input_callbacks.push(Some(callbacks)); + self.input_callbacks.len() - 1 + } + + /// Removes the callbacks at specified index, returning them. Panics if the index is invalid + /// (out of range or there is a None element at that position). + fn remove_input_callbacks(&mut self, index: usize) -> InputCallbacks { + let cbs = self.input_callbacks[index].take().unwrap(); + while self.input_callbacks.len() > 0 + && self.input_callbacks[self.input_callbacks.len() - 1].is_none() + { + self.input_callbacks.pop(); + } + cbs + } + + /// Send an error to all input and output error callbacks. + fn error(&mut self, e: impl Into) { + let e = e.into(); + for cbs in &mut self.input_callbacks { + if let Some(cbs) = cbs { + (cbs.error_callback)(e.clone()); + } + } + for cbs in &mut self.output_callbacks { + if let Some(cbs) = cbs { + (cbs.error_callback)(e.clone()); + } + } + } + + /// Calls sio_setpar and sio_getpar on the passed in sio_par struct. Before calling this, the + /// caller should have initialized `par` with `new_sio_par` and then set the desired parameters + /// on it. After calling (assuming an error is not returned), the caller should check the + /// parameters to see if they are OK. + /// + /// This should not be called if the device is running! However, it will panic if the device is + /// not opened yet. + fn negotiate_params(&mut self, par: &mut sndio_sys::sio_par) -> Result<(), SndioError> { + // What follows is the suggested parameter negotiation from the man pages. + self.set_params(par)?; + + let status = unsafe { + // Retrieve the actual parameters of the device. + sndio_sys::sio_getpar(self.hdl.unwrap(), par as *mut _) + }; + if status != 1 { + return Err(backend_specific_error( + "failed to get device-supported parameters with sio_getpar", + ) + .into()); + } + + if par.bits != 16 || par.bps != 2 { + // We have to check both because of the possibility of padding (usually an issue with + // 24 bits not 16 though). + return Err(backend_specific_error(format!( + "unexpected sample size (not 16bit): bits/sample: {}, bytes/sample: {})", + par.bits, par.bps + )) + .into()); + } + + if par.sig != 1 { + return Err(backend_specific_error( + "sndio device does not support I16 but we need it to", + ) + .into()); + } + Ok(()) + } + + /// Calls sio_setpar on the passed in sio_par struct. This sets the device parameters. + fn set_params(&mut self, par: &sndio_sys::sio_par) -> Result<(), SndioError> { + if self.hdl.is_none() { + return Err(backend_specific_error( + "cannot set params if device is not open", + )); + } + let mut newpar = new_sio_par(); + // This is a little hacky -- testing indicates the __magic from sio_initpar needs to be + // preserved when calling sio_setpar. Unfortunately __magic is the wrong value after + // retrieval from sio_getpar. + newpar.bits = par.bits; + newpar.bps = par.bps; + newpar.sig = par.sig; + newpar.le = par.le; + newpar.msb = par.msb; + newpar.rchan = par.rchan; + newpar.pchan = par.pchan; + newpar.rate = par.rate; + newpar.appbufsz = par.appbufsz; + newpar.bufsz = par.bufsz; + newpar.round = par.round; + newpar.xrun = par.xrun; + let status = unsafe { + // Request the device using our parameters + // unwrap OK because of the check at the top of this function. + sndio_sys::sio_setpar(self.hdl.unwrap(), &mut newpar as *mut _) + }; + if status != 1 { + return Err(backend_specific_error("failed to set parameters with sio_setpar").into()); + } + Ok(()) + } +} + +impl Drop for InnerState { + fn drop(&mut self) { + if let Some(hdl) = self.hdl.take() { + unsafe { + sndio_sys::sio_close(hdl); + } + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum BufferXrunBehavior { + Ignore, // SIO_IGNORE + Sync, // SIO_SYNC + Error, // SIO_ERROR +} + +#[derive(Clone)] +pub struct Device { + inner_state: Arc>, +} + +impl Device { + pub fn new() -> Self { + Device { + inner_state: Arc::new(Mutex::new(InnerState::new())), + } + } + + pub fn set_xrun_behavior(&mut self, behavior: BufferXrunBehavior) { + let mut inner_state = self.inner_state.lock().unwrap(); + inner_state.behavior = behavior; + } +} + +impl DeviceTrait for Device { + type SupportedInputConfigs = SupportedInputConfigs; + type SupportedOutputConfigs = SupportedOutputConfigs; + type Stream = Stream; + + #[inline] + fn name(&self) -> Result { + Ok("sndio default device".to_owned()) + } + + #[inline] + fn supported_input_configs( + &self, + ) -> Result { + let mut inner_state = self.inner_state.lock().unwrap(); + + if inner_state.sample_rate_to_par.is_none() { + inner_state.open()?; + } + + if inner_state.sample_rate_to_par.is_none() { + return Err(backend_specific_error("no sample rate map!").into()); + } + + let mut config_ranges = vec![]; + // unwrap OK because of the check at the top of this function. + for (_, par) in inner_state.sample_rate_to_par.as_ref().unwrap() { + let config = supported_config_from_par(par, par.rchan); + config_ranges.push(SupportedStreamConfigRange { + channels: config.channels, + min_sample_rate: config.sample_rate, + max_sample_rate: config.sample_rate, + buffer_size: config.buffer_size, + sample_format: config.sample_format, + }); + } + + Ok(config_ranges.into_iter()) + } + + #[inline] + fn supported_output_configs( + &self, + ) -> Result { + let mut inner_state = self.inner_state.lock().unwrap(); + + if inner_state.sample_rate_to_par.is_none() { + inner_state.open()?; + } + + if inner_state.sample_rate_to_par.is_none() { + return Err(backend_specific_error("no sample rate map!").into()); + } + + let mut config_ranges = vec![]; + // unwrap OK because of the check at the top of this function. + for (_, par) in inner_state.sample_rate_to_par.as_ref().unwrap() { + let config = supported_config_from_par(par, par.pchan); + config_ranges.push(SupportedStreamConfigRange { + channels: config.channels, + min_sample_rate: config.sample_rate, + max_sample_rate: config.sample_rate, + buffer_size: config.buffer_size, + sample_format: config.sample_format, + }); + } + + Ok(config_ranges.into_iter()) + } + + #[inline] + fn default_input_config(&self) -> Result { + let mut inner_state = self.inner_state.lock().unwrap(); + + if inner_state.sample_rate_to_par.is_none() { + inner_state.open()?; + } + + // unwrap OK because the open call above will ensure this is not None. + let config = if let Some(par) = inner_state + .sample_rate_to_par + .as_ref() + .unwrap() + .get(&DEFAULT_SAMPLE_RATE.0) + { + supported_config_from_par(par, par.rchan) + } else { + return Err( + backend_specific_error("missing map of sample rates to sio_par structs!").into(), + ); + }; + + Ok(config) + } + + #[inline] + fn default_output_config(&self) -> Result { + let mut inner_state = self.inner_state.lock().unwrap(); + + if inner_state.sample_rate_to_par.is_none() { + inner_state.open()?; + } + + // unwrap OK because the open call above will ensure this is not None. + let config = if let Some(par) = inner_state + .sample_rate_to_par + .as_ref() + .unwrap() + .get(&DEFAULT_SAMPLE_RATE.0) + { + supported_config_from_par(par, par.pchan) + } else { + return Err( + backend_specific_error("missing map of sample rates to sio_par structs!").into(), + ); + }; + + Ok(config) + } + + fn build_input_stream_raw( + &self, + config: &StreamConfig, + sample_format: SampleFormat, + data_callback: D, + error_callback: E, + ) -> Result + where + D: FnMut(&Data, &InputCallbackInfo) + Send + 'static, + E: FnMut(StreamError) + Send + 'static, + { + let inner_state_arc = self.inner_state.clone(); + + let mut inner_state = self.inner_state.lock().unwrap(); + + setup_stream(&mut inner_state, config)?; + + let boxed_data_cb = if sample_format != SampleFormat::I16 { + input_adapter_callback( + data_callback, + inner_state.buffer_size.unwrap(), // unwrap OK because configured in setup_stream, above + sample_format, + ) + } else { + Box::new(data_callback) + }; + + let idx = inner_state.add_input_callbacks(InputCallbacks { + data_callback: boxed_data_cb, + error_callback: Box::new(error_callback), + }); + + if inner_state.status != Status::Running { + thread::spawn(move || runner(inner_state_arc)); + inner_state.status = Status::Running; + } + + drop(inner_state); // Unlock + Ok(Stream { + inner_state: self.inner_state.clone(), + is_output: false, + index: idx, + }) + } + + /// Create an output stream. + fn build_output_stream_raw( + &self, + config: &StreamConfig, + sample_format: SampleFormat, + data_callback: D, + error_callback: E, + ) -> Result + where + D: FnMut(&mut Data, &OutputCallbackInfo) + Send + 'static, + E: FnMut(StreamError) + Send + 'static, + { + let inner_state_arc = self.inner_state.clone(); + + let mut inner_state = self.inner_state.lock().unwrap(); + + setup_stream(&mut inner_state, config)?; + + let boxed_data_cb = if sample_format != SampleFormat::I16 { + output_adapter_callback( + data_callback, + inner_state.buffer_size.unwrap(), // unwrap OK because configured in setup_stream, above + sample_format, + ) + } else { + Box::new(data_callback) + }; + + let idx = inner_state.add_output_callbacks(OutputCallbacks { + data_callback: boxed_data_cb, + error_callback: Box::new(error_callback), + }); + + if inner_state.status != Status::Running { + thread::spawn(move || runner(inner_state_arc)); + inner_state.status = Status::Running; + } + + drop(inner_state); // Unlock + Ok(Stream { + inner_state: self.inner_state.clone(), + is_output: true, + index: idx, + }) + } +} + +/// Common code shared between build_input_stream_raw and build_output_stream_raw +fn setup_stream( + inner_state: &mut InnerState, + config: &StreamConfig, +) -> Result<(), BuildStreamError> { + if inner_state.sample_rate_to_par.is_none() { + inner_state.open()?; + } + + // TODO: one day we should be able to remove this + assert_eq!( + inner_state.input_callbacks.len() + inner_state.output_callbacks.len() > 0, + inner_state.par.is_some(), + "par can be None if and only if there are no input or output callbacks" + ); + + let par; // Either the currently configured par for existing streams or the one we will set + if let Some(configured_par) = inner_state.par { + par = configured_par; + + // Perform some checks + if par.rate != config.sample_rate.0 as u32 { + return Err(backend_specific_error("sample rates don't match").into()); + } + } else { + // No running streams yet; we get to set the par. + // unwrap OK because this is setup on inner_state.open() call above + if let Some(par_) = inner_state + .sample_rate_to_par + .as_ref() + .unwrap() + .get(&config.sample_rate.0) + { + par = par_.clone(); + } else { + return Err(backend_specific_error(format!( + "no configuration for sample rate {}", + config.sample_rate.0 + )) + .into()); + } + } + + // Round up the buffer size the user selected to the next multiple of par.round. If there + // was already a stream created with a different buffer size, return an error (sorry). + // Note: if we want stereo support, this will need to change. + let round = par.round as usize; + let desired_buffer_size = match config.buffer_size { + BufferSize::Fixed(requested) => { + if requested > 0 { + requested as usize + round - ((requested - 1) as usize % round) - 1 + } else { + round + } + } + BufferSize::Default => { + if let Some(bufsize) = inner_state.buffer_size { + bufsize + } else { + DEFAULT_ROUND_MULTIPLE * round + } + } + }; + + if inner_state.buffer_size.is_some() && inner_state.buffer_size != Some(desired_buffer_size) { + return Err(backend_specific_error("buffer sizes don't match").into()); + } + + if inner_state.par.is_none() { + let mut par = par; + par.appbufsz = desired_buffer_size as u32; + inner_state.buffer_size = Some(desired_buffer_size); + inner_state.set_params(&par)?; + inner_state.par = Some(par.clone()); + } + Ok(()) +} + +fn supported_config_from_par(par: &sndio_sys::sio_par, num_channels: u32) -> SupportedStreamConfig { + SupportedStreamConfig { + channels: num_channels as u16, + sample_rate: SampleRate(par.rate), // TODO: actually frames per second, not samples per second. Important for adding multi-channel support + buffer_size: SupportedBufferSize::Range { + min: par.round, + max: 10 * par.round, // There isn't really a max. + // Also note that min and max hold frame counts not + // sample counts. This would matter if stereo was + // supported. + }, + sample_format: SampleFormat::I16, + } +} + +fn new_sio_par() -> sndio_sys::sio_par { + let mut par = MaybeUninit::::uninit(); + unsafe { + sndio_sys::sio_initpar(par.as_mut_ptr()); + par.assume_init() + } +} + +fn backend_specific_error(desc: impl Into) -> SndioError { + SndioError::BackendSpecific(BackendSpecificError { + description: desc.into(), + }) +} + +pub struct Host; + +impl Host { + pub fn new() -> Result { + Ok(Host) + } + + pub fn default_output_device() -> Option { + Some(Device::new()) + } +} + +impl HostTrait for Host { + type Devices = Devices; + type Device = Device; + + fn is_available() -> bool { + // Assume this host is always available on sndio. + true + } + + fn devices(&self) -> Result { + Ok(Devices::new()) + } + + fn default_input_device(&self) -> Option { + Some(Device::new()) + } + + fn default_output_device(&self) -> Option { + Some(Device::new()) + } +} + +pub struct Stream { + inner_state: Arc>, + + /// True if this is output; false if this is input. + is_output: bool, + + /// Index into input_callbacks or output_callbacks + index: usize, +} + +impl StreamTrait for Stream { + fn play(&self) -> Result<(), PlayStreamError> { + // No-op since the stream was already started by build_output_stream_raw + Ok(()) + } + + // sndio doesn't support pausing. + fn pause(&self) -> Result<(), PauseStreamError> { + Err(backend_specific_error("pausing is not implemented").into()) + } +} + +impl Drop for Stream { + /// Requests a shutdown from the callback (runner) thread and waits for it to finish shutting down. + /// If the thread is already stopped, nothing happens. + fn drop(&mut self) { + let mut inner_state = self.inner_state.lock().unwrap(); + if self.is_output { + inner_state.remove_output_callbacks(self.index); + } else { + inner_state.remove_input_callbacks(self.index); + } + + if inner_state.input_callbacks.len() == 0 + && inner_state.output_callbacks.len() == 0 + && inner_state.status == Status::Running + { + if let Some(ref sender) = inner_state.wakeup_sender { + let _ = sender.send(()); + } + } + } +} + +impl Drop for Device { + fn drop(&mut self) { + let inner_state = self.inner_state.lock().unwrap(); + if inner_state.input_callbacks.len() == 0 + && inner_state.output_callbacks.len() == 0 + && inner_state.status == Status::Running + { + // Attempt to wakeup runner thread + if let Some(ref sender) = inner_state.wakeup_sender { + let _ = sender.send(()); + } + } + } +} diff --git a/src/host/sndio/runner.rs b/src/host/sndio/runner.rs new file mode 100644 index 000000000..f7db8bf4a --- /dev/null +++ b/src/host/sndio/runner.rs @@ -0,0 +1,318 @@ +use std::sync::{mpsc, Arc, Mutex}; +use std::time::{Duration, Instant}; + +use super::{backend_specific_error, InnerState, Status}; + +use crate::{ + Data, InputCallbackInfo, InputStreamTimestamp, OutputCallbackInfo, OutputStreamTimestamp, + SampleFormat, StreamInstant, +}; + +/// The runner thread handles playing and/or recording +pub(super) fn runner(inner_state_arc: Arc>) { + let buffer_size: usize; + let start_time: Instant; + let latency: Duration; + let mut clear_output_buf_needed = false; + let (wakeup_sender, wakeup_receiver) = mpsc::channel(); + { + let mut inner_state = inner_state_arc.lock().unwrap(); + inner_state.wakeup_sender = Some(wakeup_sender); + + buffer_size = inner_state.buffer_size.unwrap(); // Unwrap OK because it's always picked before Stream is created + if buffer_size == 0 { + // Probably unreachable + inner_state.error(backend_specific_error("could not determine buffer size")); + return; + } + + if let Err(err) = inner_state.open() { + inner_state.error(err); + return; + } + if let Err(err) = inner_state.start() { + inner_state.error(err); + return; + } + + // unwrap OK because par will not be None once a Stream is created, and we can't get here + // before then. + latency = Duration::from_secs(1) * buffer_size as u32 / inner_state.par.unwrap().rate; + start_time = Instant::now(); + } + + let mut output_buf = [0i16].repeat(buffer_size); // Allocate buffer of correct size + let mut input_buf = [0i16].repeat(buffer_size); // Allocate buffer of correct size + let mut output_data = unsafe { + Data::from_parts( + output_buf.as_mut_ptr() as *mut (), + output_buf.len(), + SampleFormat::I16, + ) + }; + let input_data = unsafe { + Data::from_parts( + input_buf.as_mut_ptr() as *mut (), + input_buf.len(), + SampleFormat::I16, + ) + }; + let data_byte_size = output_data.len * output_data.sample_format.sample_size(); + + let mut output_offset_bytes_into_buf: u64 = 0; // Byte offset in output buf to sio_write + let mut input_offset_bytes_into_buf: u64 = 0; // Byte offset in input buf to sio_read + let mut paused = false; + loop { + // See if shutdown requested in inner_state.status; if so, break + let mut nfds; + let mut pollfds: Vec; + { + let mut inner_state = inner_state_arc.lock().unwrap(); + // If there's nothing to do, wait until that's no longer the case. + if inner_state.input_callbacks.len() == 0 && inner_state.output_callbacks.len() == 0 { + if !paused { + if let Err(_) = inner_state.stop() { + // No callbacks to error with + break; + } + } + paused = true; + inner_state.par = None; // Allow a stream with different parameters to come along + while let Ok(_) = wakeup_receiver.try_recv() {} // While the lock is still held, drain the channel. + + // Unlock to prevent deadlock + drop(inner_state); + + // Block until a callback has been added; unwrap OK because the sender is in the + // Arc so it won't get dropped while this thread is running. + wakeup_receiver.recv().unwrap(); + } + } + + // If there no Streams and no Device then there is nothing to do -- exit. Note: this is + // only correct if there are no Weak references to this InnerState anywhere. + if Arc::strong_count(&inner_state_arc) == 1 { + break; + } + + { + let mut inner_state = inner_state_arc.lock().unwrap(); + if paused { + if inner_state.input_callbacks.len() == 0 && inner_state.output_callbacks.len() == 0 + { + // Spurious wakeup + continue; + } + + if let Err(err) = inner_state.start() { + inner_state.error(backend_specific_error(format!( + "failed to unpause after new Stream created: {:?}", + err + ))); + break; + } + paused = false; + } + nfds = unsafe { + sndio_sys::sio_nfds(inner_state.hdl.unwrap()) // Unwrap OK because of open call above + }; + if nfds <= 0 { + inner_state.error(backend_specific_error(format!( + "cannot allocate {} pollfd structs", + nfds + ))); + break; + } + pollfds = [libc::pollfd { + fd: 0, + events: 0, + revents: 0, + }] + .repeat(nfds as usize); + + // Populate pollfd structs with sndio_sys::sio_pollfd + nfds = unsafe { + sndio_sys::sio_pollfd( + inner_state.hdl.unwrap(), // Unwrap OK because of open call above + pollfds.as_mut_ptr(), + (libc::POLLOUT | libc::POLLIN) as i32, + ) + }; + if nfds <= 0 || nfds > pollfds.len() as i32 { + inner_state.error(backend_specific_error(format!( + "invalid pollfd count from sio_pollfd: {}", + nfds + ))); + break; + } + } + + // Poll (block until ready to write) + let status = unsafe { libc::poll(pollfds.as_mut_ptr(), nfds as u32, -1) }; + if status < 0 { + let mut inner_state = inner_state_arc.lock().unwrap(); + inner_state.error(backend_specific_error(format!( + "poll failed: returned {}", + status + ))); + break; + } + + let revents; + { + let mut inner_state = inner_state_arc.lock().unwrap(); + // Unwrap OK because of open call above + revents = + unsafe { sndio_sys::sio_revents(inner_state.hdl.unwrap(), pollfds.as_mut_ptr()) } + as i16; + if revents & libc::POLLHUP != 0 { + inner_state.error(backend_specific_error("device disappeared")); + break; + } + if revents & (libc::POLLOUT | libc::POLLIN) == 0 { + continue; + } + } + + let elapsed = Instant::now().duration_since(start_time); + if revents & libc::POLLOUT != 0 { + // At this point we know data can be written + let mut output_callback_info = OutputCallbackInfo { + timestamp: OutputStreamTimestamp { + callback: StreamInstant::new( + elapsed.as_secs() as i64, + elapsed.as_nanos() as u32, + ), + playback: StreamInstant::new(0, 0), // Set below + }, + }; + output_callback_info.timestamp.playback = output_callback_info + .timestamp + .callback + .add(latency) + .unwrap(); // TODO: figure out if overflow can happen + + { + let mut inner_state = inner_state_arc.lock().unwrap(); + + if output_offset_bytes_into_buf == 0 { + // The whole output buffer has been written (or this is the first time). Fill it. + if inner_state.output_callbacks.len() == 0 { + if clear_output_buf_needed { + // There is probably nonzero data in the buffer from previous output + // Streams. Zero it out. + for sample in output_buf.iter_mut() { + *sample = 0; + } + clear_output_buf_needed = false; + } + } else { + for opt_cbs in &mut inner_state.output_callbacks { + if let Some(cbs) = opt_cbs { + // Really we shouldn't have more than one output callback as they are + // stepping on each others' data. + // TODO: perhaps we should not call these callbacks while holding the lock + (cbs.data_callback)(&mut output_data, &output_callback_info); + } + } + clear_output_buf_needed = true; + } + } + + // unwrap OK because .open was called + let bytes_written = unsafe { + sndio_sys::sio_write( + inner_state.hdl.unwrap(), + (output_data.data as *const u8).add(output_offset_bytes_into_buf as usize) + as *const _, + data_byte_size as u64 - output_offset_bytes_into_buf, + ) + }; + + if bytes_written <= 0 { + inner_state.error(backend_specific_error("no bytes written; EOF?")); + break; + } + + output_offset_bytes_into_buf += bytes_written; + if output_offset_bytes_into_buf as usize > data_byte_size { + inner_state.error(backend_specific_error("too many bytes written!")); + break; + } + + if output_offset_bytes_into_buf as usize == data_byte_size { + // Everything written; need to call data callback again. + output_offset_bytes_into_buf = 0; + }; + } + } + + if revents & libc::POLLIN != 0 { + // At this point, we know data can be read + let mut input_callback_info = InputCallbackInfo { + timestamp: InputStreamTimestamp { + callback: StreamInstant::new( + elapsed.as_secs() as i64, + elapsed.as_nanos() as u32, + ), + capture: StreamInstant::new(0, 0), + }, + }; + if let Some(capture_instant) = input_callback_info.timestamp.callback.sub(latency) { + input_callback_info.timestamp.capture = capture_instant; + } else { + println!("cpal(sndio): Underflow while calculating capture timestamp"); // TODO: is this possible? Handle differently? + input_callback_info.timestamp.capture = input_callback_info.timestamp.callback; + } + + { + let mut inner_state = inner_state_arc.lock().unwrap(); + + // unwrap OK because .open was called + let bytes_read = unsafe { + sndio_sys::sio_read( + inner_state.hdl.unwrap(), + (input_data.data as *const u8).add(input_offset_bytes_into_buf as usize) + as *mut _, + data_byte_size as u64 - input_offset_bytes_into_buf, + ) + }; + + if bytes_read <= 0 { + inner_state.error(backend_specific_error("no bytes read; EOF?")); + break; + } + + input_offset_bytes_into_buf += bytes_read; + if input_offset_bytes_into_buf as usize > data_byte_size { + inner_state.error(backend_specific_error("too many bytes read!")); + break; + } + + if input_offset_bytes_into_buf as usize == data_byte_size { + // Input buffer is full; need to call data callback again. + input_offset_bytes_into_buf = 0; + }; + + if input_offset_bytes_into_buf == 0 { + for opt_cbs in &mut inner_state.input_callbacks { + if let Some(cbs) = opt_cbs { + // TODO: perhaps we should not call these callbacks while holding the lock + (cbs.data_callback)(&input_data, &input_callback_info); + } + } + } + } + } + } + + { + let mut inner_state = inner_state_arc.lock().unwrap(); + inner_state.wakeup_sender = None; + if !paused { + let _ = inner_state.stop(); // Can't do anything with error since no error callbacks left + inner_state.par = None; + } + inner_state.status = Status::Stopped; + } +} diff --git a/src/platform/mod.rs b/src/platform/mod.rs index 554c784f3..8375f95d2 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -474,6 +474,24 @@ mod platform_impl { } } +#[cfg(target_os = "openbsd")] +mod platform_impl { + pub use crate::host::sndio::{ + Device as SndioDevice, Devices as SndioDevices, Host as SndioHost, Stream as SndioStream, + SupportedInputConfigs as SndioSupportedInputConfigs, + SupportedOutputConfigs as SndioSupportedOutputConfigs, + }; + + impl_platform_host!(Sndio sndio "sndio"); + + /// The default host for the current compilation target platform. + pub fn default_host() -> Host { + SndioHost::new() + .expect("the default host should always be available") + .into() + } +} + #[cfg(any(target_os = "macos", target_os = "ios"))] mod platform_impl { pub use crate::host::coreaudio::{ @@ -579,6 +597,7 @@ mod platform_impl { target_os = "linux", target_os = "dragonfly", target_os = "freebsd", + target_os = "openbsd", target_os = "macos", target_os = "ios", target_os = "emscripten", From 37600ddf7414431525099cb98b2203519e0d0ebd Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Thu, 29 Oct 2020 22:05:06 -0700 Subject: [PATCH 02/17] Restore incorrectly modified line in asio-sys build-deps --- asio-sys/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/asio-sys/Cargo.toml b/asio-sys/Cargo.toml index 253cb87ff..47b9964d6 100644 --- a/asio-sys/Cargo.toml +++ b/asio-sys/Cargo.toml @@ -9,7 +9,7 @@ license = "Apache-2.0" keywords = ["audio", "sound", "asio", "steinberg"] build = "build.rs" -[build-dependencies] +[target.'cfg(any(target_os = "windows"))'.build-dependencies] bindgen = "0.54.0" walkdir = "2" cc = "1.0.25" From 07ceda79565811679612235d3f4bcd9aaa26b227 Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Thu, 5 Nov 2020 20:58:40 -0800 Subject: [PATCH 03/17] sndio: refactor adapters --- src/host/sndio/adapters.rs | 140 +++++++++++++++++++++++-------------- 1 file changed, 88 insertions(+), 52 deletions(-) diff --git a/src/host/sndio/adapters.rs b/src/host/sndio/adapters.rs index d26f31681..f16d68261 100644 --- a/src/host/sndio/adapters.rs +++ b/src/host/sndio/adapters.rs @@ -11,41 +11,57 @@ pub(super) fn input_adapter_callback( where D: FnMut(&Data, &InputCallbackInfo) + Send + 'static, { - if sample_format == SampleFormat::I16 { - // no-op - return Box::new(original_data_callback); - } - - // Make the backing buffer for the Data used in the closure. - let mut buf: Vec = vec![0].repeat(buffer_size * sample_format.sample_size()); - - Box::new(move |data: &Data, info: &InputCallbackInfo| { - // Note: we construct adapted_data here instead of in the parent function because buf needs - // to be owned by the closure. - let mut adapted_data = - unsafe { Data::from_parts(buf.as_mut_ptr() as *mut _, buffer_size, sample_format) }; - let data_slice: &[i16] = data.as_slice().unwrap(); // unwrap OK because data is always i16 - match sample_format { - SampleFormat::F32 => { - let adapted_slice: &mut [f32] = adapted_data.as_slice_mut().unwrap(); // unwrap OK because of the match + match sample_format { + SampleFormat::I16 => { + // no-op + return Box::new(original_data_callback); + } + SampleFormat::F32 => { + // Make the backing buffer for the Data used in the closure. + let mut adapted_buf = vec![0f32].repeat(buffer_size); + Box::new(move |data: &Data, info: &InputCallbackInfo| { + let data_slice: &[i16] = data.as_slice().unwrap(); // unwrap OK because data is always i16 + let adapted_slice = &mut adapted_buf; assert_eq!(data_slice.len(), adapted_slice.len()); for (i, adapted_ref) in adapted_slice.iter_mut().enumerate() { *adapted_ref = data_slice[i].to_f32(); } - } - SampleFormat::U16 => { - let adapted_slice: &mut [u16] = adapted_data.as_slice_mut().unwrap(); // unwrap OK because of the match + + // Note: we construct adapted_data here instead of in the parent function because adapted_buf needs + // to be owned by the closure. + let adapted_data = unsafe { + Data::from_parts( + adapted_buf.as_mut_ptr() as *mut _, + buffer_size, + sample_format, + ) + }; + original_data_callback(&adapted_data, info); + }) + } + SampleFormat::U16 => { + let mut adapted_buf = vec![0u16].repeat(buffer_size); + Box::new(move |data: &Data, info: &InputCallbackInfo| { + let data_slice: &[i16] = data.as_slice().unwrap(); // unwrap OK because data is always i16 + let adapted_slice = &mut adapted_buf; assert_eq!(data_slice.len(), adapted_slice.len()); for (i, adapted_ref) in adapted_slice.iter_mut().enumerate() { *adapted_ref = data_slice[i].to_u16(); } - } - SampleFormat::I16 => { - unreachable!("i16 should've already been handled above"); - } + + // Note: we construct adapted_data here instead of in the parent function because adapted_buf needs + // to be owned by the closure. + let adapted_data = unsafe { + Data::from_parts( + adapted_buf.as_mut_ptr() as *mut _, + buffer_size, + sample_format, + ) + }; + original_data_callback(&adapted_data, info); + }) } - original_data_callback(&adapted_data, info); - }) + } } /// When given an output data callback that expects a place to write samples in the specified @@ -59,42 +75,62 @@ pub(super) fn output_adapter_callback( where D: FnMut(&mut Data, &OutputCallbackInfo) + Send + 'static, { - if sample_format == SampleFormat::I16 { - // no-op - return Box::new(original_data_callback); - } - - // Make the backing buffer for the Data used in the closure. - let mut buf: Vec = vec![0].repeat(buffer_size * sample_format.sample_size()); + match sample_format { + SampleFormat::I16 => { + // no-op + return Box::new(original_data_callback); + } + SampleFormat::F32 => { + // Make the backing buffer for the Data used in the closure. + let mut adapted_buf = vec![0f32].repeat(buffer_size); - Box::new(move |data: &mut Data, info: &OutputCallbackInfo| { - // Note: we construct adapted_data here instead of in the parent function because buf needs - // to be owned by the closure. - let mut adapted_data = - unsafe { Data::from_parts(buf.as_mut_ptr() as *mut _, buffer_size, sample_format) }; + Box::new(move |data: &mut Data, info: &OutputCallbackInfo| { + // Note: we construct adapted_data here instead of in the parent function because + // adapted_buf needs to be owned by the closure. + let mut adapted_data = unsafe { + Data::from_parts( + adapted_buf.as_mut_ptr() as *mut _, + buffer_size, + sample_format, + ) + }; - // Populate buf / adapted_data. - original_data_callback(&mut adapted_data, info); + // Populate adapted_buf / adapted_data. + original_data_callback(&mut adapted_data, info); - let data_slice: &mut [i16] = data.as_slice_mut().unwrap(); // unwrap OK because data is always i16 - match sample_format { - SampleFormat::F32 => { - let adapted_slice: &[f32] = adapted_data.as_slice().unwrap(); // unwrap OK because of the match + let data_slice: &mut [i16] = data.as_slice_mut().unwrap(); // unwrap OK because data is always i16 + let adapted_slice = &adapted_buf; assert_eq!(data_slice.len(), adapted_slice.len()); for (i, data_ref) in data_slice.iter_mut().enumerate() { *data_ref = adapted_slice[i].to_i16(); } - } - SampleFormat::U16 => { - let adapted_slice: &[u16] = adapted_data.as_slice().unwrap(); // unwrap OK because of the match + }) + } + SampleFormat::U16 => { + // Make the backing buffer for the Data used in the closure. + let mut adapted_buf = vec![0u16].repeat(buffer_size); + + Box::new(move |data: &mut Data, info: &OutputCallbackInfo| { + // Note: we construct adapted_data here instead of in the parent function because + // adapted_buf needs to be owned by the closure. + let mut adapted_data = unsafe { + Data::from_parts( + adapted_buf.as_mut_ptr() as *mut _, + buffer_size, + sample_format, + ) + }; + + // Populate adapted_buf / adapted_data. + original_data_callback(&mut adapted_data, info); + + let data_slice: &mut [i16] = data.as_slice_mut().unwrap(); // unwrap OK because data is always i16 + let adapted_slice = &adapted_buf; assert_eq!(data_slice.len(), adapted_slice.len()); for (i, data_ref) in data_slice.iter_mut().enumerate() { *data_ref = adapted_slice[i].to_i16(); } - } - SampleFormat::I16 => { - unreachable!("i16 should've already been handled above"); - } + }) } - }) + } } From 8e7227b4693a3a51cf3f73719dbaa3e51dedfeaf Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Thu, 5 Nov 2020 21:23:15 -0800 Subject: [PATCH 04/17] sndio: simpler endianness check --- src/host/sndio/endian.rs | 14 -------------- src/host/sndio/mod.rs | 12 +++++++----- 2 files changed, 7 insertions(+), 19 deletions(-) delete mode 100644 src/host/sndio/endian.rs diff --git a/src/host/sndio/endian.rs b/src/host/sndio/endian.rs deleted file mode 100644 index 73b9153d2..000000000 --- a/src/host/sndio/endian.rs +++ /dev/null @@ -1,14 +0,0 @@ -#[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub(super) enum Endian { - BE, - LE, -} - -pub(super) fn get_endianness() -> Endian { - let n: i16 = 1; - match n.to_ne_bytes()[0] { - 1 => Endian::LE, - 0 => Endian::BE, - _ => unreachable!("unexpected value in byte"), - } -} diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index d99f4ce12..3ef74a5f4 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -2,7 +2,6 @@ extern crate libc; extern crate sndio_sys; mod adapters; -mod endian; mod runner; use self::adapters::{input_adapter_callback, output_adapter_callback}; use self::runner::runner; @@ -44,6 +43,12 @@ pub enum SndioError { BackendSpecific(BackendSpecificError), } +#[cfg(target_endian = "big")] +const IS_LITTLE_ENDIAN: u32 = 0; + +#[cfg(target_endian = "little")] +const IS_LITTLE_ENDIAN: u32 = 1; + impl From for BuildStreamError { fn from(e: SndioError) -> BuildStreamError { match e { @@ -216,10 +221,7 @@ impl InnerState { // Use I16 at 48KHz; mono playback & record par.bits = 16; par.sig = 1; - par.le = match endian::get_endianness() { - endian::Endian::BE => 0, - endian::Endian::LE => 1, - }; // Native byte order + par.le = IS_LITTLE_ENDIAN; // Native byte order par.rchan = 1; // mono record par.pchan = 1; // mono playback par.rate = rate.0; From 7f11fd17e20398d10a27c8441917e9636d69431b Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Sun, 8 Nov 2020 18:15:02 -0800 Subject: [PATCH 05/17] sndio: simplify Devices iterator --- src/host/sndio/mod.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index 3ef74a5f4..e81b5ee15 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -98,27 +98,21 @@ impl From for SupportedStreamConfigsError { } } -pub struct Devices { - returned: bool, +pub struct Devices(Option); + +impl Devices { + fn new() -> Self { + Devices(Some(Device::new())) + } } impl Iterator for Devices { type Item = Device; - fn next(&mut self) -> Option { - if self.returned { - None - } else { - self.returned = true; - Some(Device::new()) - } + fn next(&mut self) -> Option { + self.0.take() } } -impl Devices { - fn new() -> Devices { - Devices { returned: false } - } -} /// The shared state between Device and Stream. Responsible for closing handle when dropped. struct InnerState { From 2e0c63bc61927cf1bf0086774eefe3ef8ee27f58 Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Tue, 10 Nov 2020 20:44:44 -0800 Subject: [PATCH 06/17] rustfmt fix --- src/host/sndio/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index e81b5ee15..aa8f4e9f9 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -113,7 +113,6 @@ impl Iterator for Devices { } } - /// The shared state between Device and Stream. Responsible for closing handle when dropped. struct InnerState { /// If device has been open with sio_open, contains a handle. Note that even though this is a From eeafe6f031919fbf3fa0a6700b13f3017565e852 Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Wed, 11 Nov 2020 21:51:08 -0800 Subject: [PATCH 07/17] sndio: avoid unsafe impl Send on InnerState --- src/host/sndio/mod.rs | 20 +++++++++++--------- src/host/sndio/runner.rs | 14 +++++++------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index aa8f4e9f9..c63001744 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -113,12 +113,16 @@ impl Iterator for Devices { } } +struct SioHdl(*mut sndio_sys::sio_hdl); + +unsafe impl Send for SioHdl {} + /// The shared state between Device and Stream. Responsible for closing handle when dropped. struct InnerState { /// If device has been open with sio_open, contains a handle. Note that even though this is a /// pointer type and so doesn't follow Rust's borrowing rules, we should be careful not to copy /// it out because that may render Mutex ineffective in enforcing exclusive access. - hdl: Option<*mut sndio_sys::sio_hdl>, + hdl: Option, /// Buffer overrun/underrun behavior -- ignore/sync/error? behavior: BufferXrunBehavior, @@ -159,8 +163,6 @@ struct OutputCallbacks { error_callback: Box, } -unsafe impl Send for InnerState {} - #[derive(Debug, Copy, Clone, Eq, PartialEq)] enum Status { /// Initial state. No thread running. Device/Stream methods will start thread and change this @@ -205,7 +207,7 @@ impl InnerState { if hdl.is_null() { return Err(SndioError::DeviceNotAvailable); } - self.hdl = Some(hdl); + self.hdl = Some(SioHdl(hdl)); let mut sample_rate_to_par = HashMap::new(); for rate in SUPPORTED_SAMPLE_RATES { @@ -267,7 +269,7 @@ impl InnerState { // will wait for playback data to be provided (using the sio_write() // function). Once enough data is queued to ensure that play buffers will // not underrun, actual playback is started automatically." - sndio_sys::sio_start(self.hdl.unwrap()) // Unwrap OK because of check above + sndio_sys::sio_start(self.hdl.as_ref().unwrap().0) // Unwrap OK because of check above }; if status != 1 { return Err(backend_specific_error("failed to start stream")); @@ -286,7 +288,7 @@ impl InnerState { // playback. If samples to play are queued but playback hasn't started yet then // playback is forced immediately; playback will actually stop once the buffer is // drained. In no case are samples in the play buffer discarded. - sndio_sys::sio_stop(self.hdl.unwrap()) // Unwrap OK because of check above + sndio_sys::sio_stop(self.hdl.as_ref().unwrap().0) // Unwrap OK because of check above }; if status != 1 { return Err(backend_specific_error("error calling sio_stop")); @@ -385,7 +387,7 @@ impl InnerState { let status = unsafe { // Retrieve the actual parameters of the device. - sndio_sys::sio_getpar(self.hdl.unwrap(), par as *mut _) + sndio_sys::sio_getpar(self.hdl.as_ref().unwrap().0, par as *mut _) }; if status != 1 { return Err(backend_specific_error( @@ -439,7 +441,7 @@ impl InnerState { let status = unsafe { // Request the device using our parameters // unwrap OK because of the check at the top of this function. - sndio_sys::sio_setpar(self.hdl.unwrap(), &mut newpar as *mut _) + sndio_sys::sio_setpar(self.hdl.as_ref().unwrap().0, &mut newpar as *mut _) }; if status != 1 { return Err(backend_specific_error("failed to set parameters with sio_setpar").into()); @@ -452,7 +454,7 @@ impl Drop for InnerState { fn drop(&mut self) { if let Some(hdl) = self.hdl.take() { unsafe { - sndio_sys::sio_close(hdl); + sndio_sys::sio_close(hdl.0); } } } diff --git a/src/host/sndio/runner.rs b/src/host/sndio/runner.rs index f7db8bf4a..03b0c9553 100644 --- a/src/host/sndio/runner.rs +++ b/src/host/sndio/runner.rs @@ -114,7 +114,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { paused = false; } nfds = unsafe { - sndio_sys::sio_nfds(inner_state.hdl.unwrap()) // Unwrap OK because of open call above + sndio_sys::sio_nfds(inner_state.hdl.as_ref().unwrap().0) // Unwrap OK because of open call above }; if nfds <= 0 { inner_state.error(backend_specific_error(format!( @@ -133,7 +133,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { // Populate pollfd structs with sndio_sys::sio_pollfd nfds = unsafe { sndio_sys::sio_pollfd( - inner_state.hdl.unwrap(), // Unwrap OK because of open call above + inner_state.hdl.as_ref().unwrap().0, // Unwrap OK because of open call above pollfds.as_mut_ptr(), (libc::POLLOUT | libc::POLLIN) as i32, ) @@ -162,9 +162,9 @@ pub(super) fn runner(inner_state_arc: Arc>) { { let mut inner_state = inner_state_arc.lock().unwrap(); // Unwrap OK because of open call above - revents = - unsafe { sndio_sys::sio_revents(inner_state.hdl.unwrap(), pollfds.as_mut_ptr()) } - as i16; + revents = unsafe { + sndio_sys::sio_revents(inner_state.hdl.as_ref().unwrap().0, pollfds.as_mut_ptr()) + } as i16; if revents & libc::POLLHUP != 0 { inner_state.error(backend_specific_error("device disappeared")); break; @@ -222,7 +222,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { // unwrap OK because .open was called let bytes_written = unsafe { sndio_sys::sio_write( - inner_state.hdl.unwrap(), + inner_state.hdl.as_ref().unwrap().0, (output_data.data as *const u8).add(output_offset_bytes_into_buf as usize) as *const _, data_byte_size as u64 - output_offset_bytes_into_buf, @@ -271,7 +271,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { // unwrap OK because .open was called let bytes_read = unsafe { sndio_sys::sio_read( - inner_state.hdl.unwrap(), + inner_state.hdl.as_ref().unwrap().0, (input_data.data as *const u8).add(input_offset_bytes_into_buf as usize) as *mut _, data_byte_size as u64 - input_offset_bytes_into_buf, From 47877cf899e9c13fe02726d21fb747a759a5c2c6 Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Sun, 22 Nov 2020 18:09:12 -0800 Subject: [PATCH 08/17] sndio: refactor InnerState to be enum (reduce the Options and unwraps) --- src/host/sndio/mod.rs | 1025 ++++++++++++++++++++++---------------- src/host/sndio/runner.rs | 324 ++++++++---- 2 files changed, 824 insertions(+), 525 deletions(-) diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index c63001744..615cfc728 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -115,100 +115,13 @@ impl Iterator for Devices { struct SioHdl(*mut sndio_sys::sio_hdl); -unsafe impl Send for SioHdl {} - -/// The shared state between Device and Stream. Responsible for closing handle when dropped. -struct InnerState { - /// If device has been open with sio_open, contains a handle. Note that even though this is a - /// pointer type and so doesn't follow Rust's borrowing rules, we should be careful not to copy - /// it out because that may render Mutex ineffective in enforcing exclusive access. - hdl: Option, - - /// Buffer overrun/underrun behavior -- ignore/sync/error? - behavior: BufferXrunBehavior, - - /// If a buffer size was chosen, contains that value. - buffer_size: Option, - - /// If the device was configured, stores the sndio-configured parameters. - par: Option, - - /// Map of sample rate to parameters. - /// Guaranteed to not be None if hdl is not None. - sample_rate_to_par: Option>, - - /// Indicates if the read/write thread is started, shutting down, or stopped. - status: Status, - - /// Each input Stream that has not been dropped has its callbacks in an element of this Vec. - /// The last element is guaranteed to not be None. - input_callbacks: Vec>, - - /// Each output Stream that has not been dropped has its callbacks in an element of this Vec. - /// The last element is guaranteed to not be None. - output_callbacks: Vec>, - - /// Channel used for signalling that the runner thread should wakeup because there is now a - /// Stream. This will only be None if there is no runner thread. - wakeup_sender: Option>, -} - -struct InputCallbacks { - data_callback: Box, - error_callback: Box, -} - -struct OutputCallbacks { - data_callback: Box, - error_callback: Box, -} - -#[derive(Debug, Copy, Clone, Eq, PartialEq)] -enum Status { - /// Initial state. No thread running. Device/Stream methods will start thread and change this - /// to Running. - Stopped, - - /// Thread is running (unless it encountered an error). - Running, -} - -impl InnerState { - fn new() -> Self { - InnerState { - hdl: None, - behavior: BufferXrunBehavior::Sync, - par: None, - sample_rate_to_par: None, - buffer_size: None, - status: Status::Stopped, - input_callbacks: vec![], - output_callbacks: vec![], - wakeup_sender: None, - } - } - - fn open(&mut self) -> Result<(), SndioError> { - if self.hdl.is_some() { - // Already open - return Ok(()); - } - - let hdl = unsafe { - // The transmute is needed because this C string is *const u8 in one place but *const i8 in another place. - let devany_ptr = mem::transmute::<_, *const i8>(sndio_sys::SIO_DEVANY as *const _); - let nonblocking = true as i32; - sndio_sys::sio_open( - devany_ptr, - sndio_sys::SIO_PLAY | sndio_sys::SIO_REC, - nonblocking, - ) - }; - if hdl.is_null() { - return Err(SndioError::DeviceNotAvailable); - } - self.hdl = Some(SioHdl(hdl)); - +impl SioHdl { + /// Returns a map of sample rates to sio_par by re-configuring the device. This should not be + /// performed while recording or playing, or even after configuring the device for this state! + fn get_par_map( + &mut self, + behavior: BufferXrunBehavior, + ) -> Result, SndioError> { let mut sample_rate_to_par = HashMap::new(); for rate in SUPPORTED_SAMPLE_RATES { let mut par = new_sio_par(); @@ -220,7 +133,7 @@ impl InnerState { par.rchan = 1; // mono record par.pchan = 1; // mono playback par.rate = rate.0; - par.xrun = match self.behavior { + par.xrun = match behavior { BufferXrunBehavior::Ignore => 0, BufferXrunBehavior::Sync => 1, BufferXrunBehavior::Error => 2, @@ -254,124 +167,7 @@ impl InnerState { sample_rate_to_par.insert(rate.0, par); } - self.sample_rate_to_par = Some(sample_rate_to_par); - Ok(()) - } - - fn start(&mut self) -> Result<(), SndioError> { - if self.hdl.is_none() { - return Err(backend_specific_error( - "cannot start a device that hasn't been opened yet", - )); - } - let status = unsafe { - // "The sio_start() function puts the device in a waiting state: the device - // will wait for playback data to be provided (using the sio_write() - // function). Once enough data is queued to ensure that play buffers will - // not underrun, actual playback is started automatically." - sndio_sys::sio_start(self.hdl.as_ref().unwrap().0) // Unwrap OK because of check above - }; - if status != 1 { - return Err(backend_specific_error("failed to start stream")); - } - Ok(()) - } - - fn stop(&mut self) -> Result<(), SndioError> { - if self.hdl.is_none() { - // Nothing to do -- device is not open. - return Ok(()); - } - let status = unsafe { - // The sio_stop() function puts the audio subsystem in the same state as before - // sio_start() is called. It stops recording, drains the play buffer and then stops - // playback. If samples to play are queued but playback hasn't started yet then - // playback is forced immediately; playback will actually stop once the buffer is - // drained. In no case are samples in the play buffer discarded. - sndio_sys::sio_stop(self.hdl.as_ref().unwrap().0) // Unwrap OK because of check above - }; - if status != 1 { - return Err(backend_specific_error("error calling sio_stop")); - } - Ok(()) - } - - // TODO: make these 4 methods generic (new CallbackSet where T is either InputCallbacks or OutputCallbacks) - /// Puts the supplied callbacks into the vector in the first free position, or at the end. The - /// index of insertion is returned. - fn add_output_callbacks(&mut self, callbacks: OutputCallbacks) -> usize { - for (i, cbs) in self.output_callbacks.iter_mut().enumerate() { - if cbs.is_none() { - *cbs = Some(callbacks); - return i; - } - } - // If there were previously no callbacks, wakeup the runner thread. - if self.input_callbacks.len() == 0 && self.output_callbacks.len() == 0 { - if let Some(ref sender) = self.wakeup_sender { - let _ = sender.send(()); - } - } - self.output_callbacks.push(Some(callbacks)); - self.output_callbacks.len() - 1 - } - - /// Removes the callbacks at specified index, returning them. Panics if the index is invalid - /// (out of range or there is a None element at that position). - fn remove_output_callbacks(&mut self, index: usize) -> OutputCallbacks { - let cbs = self.output_callbacks[index].take().unwrap(); - while self.output_callbacks.len() > 0 - && self.output_callbacks[self.output_callbacks.len() - 1].is_none() - { - self.output_callbacks.pop(); - } - cbs - } - - /// Puts the supplied callbacks into the vector in the first free position, or at the end. The - /// index of insertion is returned. - fn add_input_callbacks(&mut self, callbacks: InputCallbacks) -> usize { - for (i, cbs) in self.input_callbacks.iter_mut().enumerate() { - if cbs.is_none() { - *cbs = Some(callbacks); - return i; - } - } - // If there were previously no callbacks, wakeup the runner thread. - if self.input_callbacks.len() == 0 && self.output_callbacks.len() == 0 { - if let Some(ref sender) = self.wakeup_sender { - let _ = sender.send(()); - } - } - self.input_callbacks.push(Some(callbacks)); - self.input_callbacks.len() - 1 - } - - /// Removes the callbacks at specified index, returning them. Panics if the index is invalid - /// (out of range or there is a None element at that position). - fn remove_input_callbacks(&mut self, index: usize) -> InputCallbacks { - let cbs = self.input_callbacks[index].take().unwrap(); - while self.input_callbacks.len() > 0 - && self.input_callbacks[self.input_callbacks.len() - 1].is_none() - { - self.input_callbacks.pop(); - } - cbs - } - - /// Send an error to all input and output error callbacks. - fn error(&mut self, e: impl Into) { - let e = e.into(); - for cbs in &mut self.input_callbacks { - if let Some(cbs) = cbs { - (cbs.error_callback)(e.clone()); - } - } - for cbs in &mut self.output_callbacks { - if let Some(cbs) = cbs { - (cbs.error_callback)(e.clone()); - } - } + Ok(sample_rate_to_par) } /// Calls sio_setpar and sio_getpar on the passed in sio_par struct. Before calling this, the @@ -387,7 +183,7 @@ impl InnerState { let status = unsafe { // Retrieve the actual parameters of the device. - sndio_sys::sio_getpar(self.hdl.as_ref().unwrap().0, par as *mut _) + sndio_sys::sio_getpar(self.0, par as *mut _) }; if status != 1 { return Err(backend_specific_error( @@ -417,11 +213,6 @@ impl InnerState { /// Calls sio_setpar on the passed in sio_par struct. This sets the device parameters. fn set_params(&mut self, par: &sndio_sys::sio_par) -> Result<(), SndioError> { - if self.hdl.is_none() { - return Err(backend_specific_error( - "cannot set params if device is not open", - )); - } let mut newpar = new_sio_par(); // This is a little hacky -- testing indicates the __magic from sio_initpar needs to be // preserved when calling sio_setpar. Unfortunately __magic is the wrong value after @@ -441,7 +232,7 @@ impl InnerState { let status = unsafe { // Request the device using our parameters // unwrap OK because of the check at the top of this function. - sndio_sys::sio_setpar(self.hdl.as_ref().unwrap().0, &mut newpar as *mut _) + sndio_sys::sio_setpar(self.0, &mut newpar as *mut _) }; if status != 1 { return Err(backend_specific_error("failed to set parameters with sio_setpar").into()); @@ -450,17 +241,353 @@ impl InnerState { } } +unsafe impl Send for SioHdl {} + +/// The shared state between Device and Stream. Responsible for closing handle when dropped. +enum InnerState { + Init { + /// Buffer overrun/underrun behavior -- ignore/sync/error? + behavior: BufferXrunBehavior, + }, + Opened { + /// Contains a handle returned from sio_open. Note that even though this is a pointer type + /// and so doesn't follow Rust's borrowing rules, we should be careful not to copy it out + /// because that may render Mutex ineffective in enforcing exclusive access. + hdl: SioHdl, + + /// Map of sample rate to parameters. + sample_rate_to_par: HashMap, + }, + Running { + /// Contains a handle returned from sio_open. Note that even though this is a pointer type + /// and so doesn't follow Rust's borrowing rules, we should be careful not to copy it out + /// because that may render Mutex ineffective in enforcing exclusive access. + hdl: SioHdl, + + /// Contains the chosen buffer size, in elements not bytes. + buffer_size: usize, + + /// Stores the sndio-configured parameters. + par: sndio_sys::sio_par, + + /// Map of sample rate to parameters. + sample_rate_to_par: HashMap, + + /// Each input Stream that has not been dropped has its callbacks in an element of this Vec. + /// The last element is guaranteed to not be None. + input_callbacks: Vec>, + + /// Each output Stream that has not been dropped has its callbacks in an element of this Vec. + /// The last element is guaranteed to not be None. + output_callbacks: Vec>, + + /// Whether the runner thread was spawned yet. + thread_spawned: bool, + + /// Channel used for signalling that the runner thread should wakeup because there is now a + /// Stream. This will only be None if either 1) the runner thread has not yet started and + /// set this value, or 2) the runner thread has exited. + wakeup_sender: Option>, + }, +} + +struct InputCallbacks { + data_callback: Box, + error_callback: Box, +} + +struct OutputCallbacks { + data_callback: Box, + error_callback: Box, +} + +impl InnerState { + fn new() -> Self { + InnerState::Init { + behavior: BufferXrunBehavior::Sync, + } + } + + fn open(&mut self) -> Result<(), SndioError> { + match self { + InnerState::Opened { .. } | InnerState::Running { .. } => Ok(()), // Already opened + InnerState::Init { ref behavior } => { + let hdl = unsafe { + // The transmute is needed because this C string is *const u8 in one place but *const i8 in another place. + let devany_ptr = + mem::transmute::<_, *const i8>(sndio_sys::SIO_DEVANY as *const _); + let nonblocking = true as i32; + sndio_sys::sio_open( + devany_ptr, + sndio_sys::SIO_PLAY | sndio_sys::SIO_REC, + nonblocking, + ) + }; + if hdl.is_null() { + return Err(SndioError::DeviceNotAvailable); + } + + let mut hdl = SioHdl(hdl); + let sample_rate_to_par = hdl.get_par_map(*behavior)?; + *self = InnerState::Opened { + hdl, + sample_rate_to_par, + }; + Ok(()) + } + } + } + + fn start(&mut self) -> Result<(), SndioError> { + match self { + InnerState::Running { hdl, .. } => { + let status = unsafe { + // "The sio_start() function puts the device in a waiting state: the device + // will wait for playback data to be provided (using the sio_write() + // function). Once enough data is queued to ensure that play buffers will + // not underrun, actual playback is started automatically." + sndio_sys::sio_start(hdl.0) // Unwrap OK because of check above + }; + if status != 1 { + Err(backend_specific_error("failed to start stream")) + } else { + Ok(()) + } + } + _ => Err(backend_specific_error( + "cannot start a device that hasn't been opened yet", + )), + } + } + + fn stop(&mut self) -> Result<(), SndioError> { + match self { + InnerState::Running { hdl, .. } => { + let status = unsafe { + // The sio_stop() function puts the audio subsystem in the same state as before + // sio_start() is called. It stops recording, drains the play buffer and then stops + // playback. If samples to play are queued but playback hasn't started yet then + // playback is forced immediately; playback will actually stop once the buffer is + // drained. In no case are samples in the play buffer discarded. + sndio_sys::sio_stop(hdl.0) // Unwrap OK because of check above + }; + if status != 1 { + Err(backend_specific_error("error calling sio_stop")) + } else { + Ok(()) + } + } + _ => { + // Nothing to do -- device is not open. + Ok(()) + } + } + } + + // TODO: make these 4 methods generic (new CallbackSet where T is either InputCallbacks or OutputCallbacks) + /// Puts the supplied callbacks into the vector in the first free position, or at the end. The + /// index of insertion is returned. + fn add_output_callbacks(&mut self, callbacks: OutputCallbacks) -> Result { + match self { + InnerState::Running { + ref input_callbacks, + ref mut output_callbacks, + ref mut wakeup_sender, + .. + } => { + for (i, cbs) in output_callbacks.iter_mut().enumerate() { + if cbs.is_none() { + *cbs = Some(callbacks); + return Ok(i); + } + } + // If there were previously no callbacks, wakeup the runner thread. + if input_callbacks.len() == 0 && output_callbacks.len() == 0 { + if let Some(ref sender) = wakeup_sender { + let _ = sender.send(()); + } + } + output_callbacks.push(Some(callbacks)); + Ok(output_callbacks.len() - 1) + } + _ => Err(backend_specific_error("device is not in a running state")), + } + } + + /// Removes the callbacks at specified index, returning them. Panics if the index is invalid + /// (out of range or there is a None element at that position). + fn remove_output_callbacks(&mut self, index: usize) -> Result { + match *self { + InnerState::Running { + ref mut output_callbacks, + .. + } => { + let cbs = output_callbacks[index].take().unwrap(); + while output_callbacks.len() > 0 + && output_callbacks[output_callbacks.len() - 1].is_none() + { + output_callbacks.pop(); + } + Ok(cbs) + } + _ => Err(backend_specific_error("device is not in a running state")), + } + } + + /// Puts the supplied callbacks into the vector in the first free position, or at the end. The + /// index of insertion is returned. + fn add_input_callbacks(&mut self, callbacks: InputCallbacks) -> Result { + match self { + InnerState::Running { + ref mut input_callbacks, + ref output_callbacks, + ref mut wakeup_sender, + .. + } => { + for (i, cbs) in input_callbacks.iter_mut().enumerate() { + if cbs.is_none() { + *cbs = Some(callbacks); + return Ok(i); + } + } + // If there were previously no callbacks, wakeup the runner thread. + if input_callbacks.len() == 0 && output_callbacks.len() == 0 { + if let Some(ref sender) = wakeup_sender { + let _ = sender.send(()); + } + } + input_callbacks.push(Some(callbacks)); + Ok(input_callbacks.len() - 1) + } + _ => Err(backend_specific_error("device is not in a running state")), + } + } + + /// Removes the callbacks at specified index, returning them. Panics if the index is invalid + /// (out of range or there is a None element at that position). + fn remove_input_callbacks(&mut self, index: usize) -> Result { + match *self { + InnerState::Running { + ref mut input_callbacks, + .. + } => { + let cbs = input_callbacks[index].take().unwrap(); + while input_callbacks.len() > 0 + && input_callbacks[input_callbacks.len() - 1].is_none() + { + input_callbacks.pop(); + } + Ok(cbs) + } + _ => Err(backend_specific_error("device is not in a running state")), + } + } + + /// Send an error to all input and output error callbacks. + fn error(&mut self, e: impl Into) { + match *self { + InnerState::Running { + ref mut input_callbacks, + ref mut output_callbacks, + .. + } => { + let e = e.into(); + for cbs in input_callbacks { + if let Some(cbs) = cbs { + (cbs.error_callback)(e.clone()); + } + } + for cbs in output_callbacks { + if let Some(cbs) = cbs { + (cbs.error_callback)(e.clone()); + } + } + } + _ => {} // Drop the error + } + } + + /// Common code shared between build_input_stream_raw and build_output_stream_raw + fn setup_stream(&mut self, config: &StreamConfig) -> Result<(), BuildStreamError> { + // If not already open, make sure it's open + match self { + InnerState::Init { .. } => { + self.open()?; + } + _ => {} + } + + match self { + InnerState::Init { .. } => { + // Probably unreachable + Err(backend_specific_error("device was expected to be opened").into()) + } + InnerState::Opened { + hdl, + sample_rate_to_par, + } => { + // No running streams yet; we get to set the par. + // unwrap OK because this is setup on self.open() call above + let mut par; + if let Some(par_) = sample_rate_to_par.get(&config.sample_rate.0) { + par = par_.clone(); + } else { + return Err(backend_specific_error(format!( + "no configuration for sample rate {}", + config.sample_rate.0 + )) + .into()); + } + + let buffer_size = determine_buffer_size(&config.buffer_size, par.round, None)?; + + // Transition to running + par.appbufsz = buffer_size as u32; + hdl.set_params(&par)?; + let mut tmp: HashMap = HashMap::new(); + mem::swap(&mut tmp, sample_rate_to_par); + + *self = InnerState::Running { + hdl: SioHdl(hdl.0), // Just this once, it's ok to copy this + buffer_size, + par, + sample_rate_to_par: tmp, + input_callbacks: vec![], + output_callbacks: vec![], + thread_spawned: false, + wakeup_sender: None, + }; + Ok(()) + } + InnerState::Running { + par, buffer_size, .. + } => { + // TODO: allow setting new par like above flow if input_callbacks and + // output_callbacks are both zero. + + // Perform some checks + if par.rate != config.sample_rate.0 as u32 { + return Err(backend_specific_error("sample rates don't match").into()); + } + determine_buffer_size(&config.buffer_size, par.round, Some(*buffer_size))?; + Ok(()) + } + } + } +} + impl Drop for InnerState { fn drop(&mut self) { - if let Some(hdl) = self.hdl.take() { - unsafe { + match self { + InnerState::Running { hdl, .. } => unsafe { sndio_sys::sio_close(hdl.0); - } + }, + _ => {} } } } -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, Copy)] pub enum BufferXrunBehavior { Ignore, // SIO_IGNORE Sync, // SIO_SYNC @@ -479,9 +606,19 @@ impl Device { } } - pub fn set_xrun_behavior(&mut self, behavior: BufferXrunBehavior) { + pub fn set_xrun_behavior(&mut self, b: BufferXrunBehavior) -> Result<(), SndioError> { let mut inner_state = self.inner_state.lock().unwrap(); - inner_state.behavior = behavior; + match *inner_state { + InnerState::Init { + ref mut behavior, .. + } => { + *behavior = b; + Ok(()) + } + _ => Err(backend_specific_error( + "the xrun behavior can only be specified at initialization time", + )), + } } } @@ -501,28 +638,38 @@ impl DeviceTrait for Device { ) -> Result { let mut inner_state = self.inner_state.lock().unwrap(); - if inner_state.sample_rate_to_par.is_none() { - inner_state.open()?; - } - - if inner_state.sample_rate_to_par.is_none() { - return Err(backend_specific_error("no sample rate map!").into()); + match *inner_state { + InnerState::Init { .. } => { + inner_state.open()?; + } + _ => {} } - let mut config_ranges = vec![]; - // unwrap OK because of the check at the top of this function. - for (_, par) in inner_state.sample_rate_to_par.as_ref().unwrap() { - let config = supported_config_from_par(par, par.rchan); - config_ranges.push(SupportedStreamConfigRange { - channels: config.channels, - min_sample_rate: config.sample_rate, - max_sample_rate: config.sample_rate, - buffer_size: config.buffer_size, - sample_format: config.sample_format, - }); + match *inner_state { + InnerState::Running { + ref sample_rate_to_par, + .. + } + | InnerState::Opened { + ref sample_rate_to_par, + .. + } => { + let mut config_ranges = vec![]; + for (_, par) in sample_rate_to_par { + let config = supported_config_from_par(par, par.rchan); + config_ranges.push(SupportedStreamConfigRange { + channels: config.channels, + min_sample_rate: config.sample_rate, + max_sample_rate: config.sample_rate, + buffer_size: config.buffer_size, + sample_format: config.sample_format, + }); + } + + Ok(config_ranges.into_iter()) + } + _ => Err(backend_specific_error("device has not yet been opened").into()), } - - Ok(config_ranges.into_iter()) } #[inline] @@ -531,78 +678,108 @@ impl DeviceTrait for Device { ) -> Result { let mut inner_state = self.inner_state.lock().unwrap(); - if inner_state.sample_rate_to_par.is_none() { - inner_state.open()?; - } - - if inner_state.sample_rate_to_par.is_none() { - return Err(backend_specific_error("no sample rate map!").into()); + match *inner_state { + InnerState::Init { .. } => { + inner_state.open()?; + } + _ => {} } - let mut config_ranges = vec![]; - // unwrap OK because of the check at the top of this function. - for (_, par) in inner_state.sample_rate_to_par.as_ref().unwrap() { - let config = supported_config_from_par(par, par.pchan); - config_ranges.push(SupportedStreamConfigRange { - channels: config.channels, - min_sample_rate: config.sample_rate, - max_sample_rate: config.sample_rate, - buffer_size: config.buffer_size, - sample_format: config.sample_format, - }); + match *inner_state { + InnerState::Running { + ref sample_rate_to_par, + .. + } + | InnerState::Opened { + ref sample_rate_to_par, + .. + } => { + let mut config_ranges = vec![]; + for (_, par) in sample_rate_to_par { + let config = supported_config_from_par(par, par.pchan); + config_ranges.push(SupportedStreamConfigRange { + channels: config.channels, + min_sample_rate: config.sample_rate, + max_sample_rate: config.sample_rate, + buffer_size: config.buffer_size, + sample_format: config.sample_format, + }); + } + + Ok(config_ranges.into_iter()) + } + _ => Err(backend_specific_error("device has not yet been opened").into()), } - - Ok(config_ranges.into_iter()) } #[inline] fn default_input_config(&self) -> Result { let mut inner_state = self.inner_state.lock().unwrap(); - if inner_state.sample_rate_to_par.is_none() { - inner_state.open()?; + match *inner_state { + InnerState::Init { .. } => { + inner_state.open()?; + } + _ => {} } - // unwrap OK because the open call above will ensure this is not None. - let config = if let Some(par) = inner_state - .sample_rate_to_par - .as_ref() - .unwrap() - .get(&DEFAULT_SAMPLE_RATE.0) - { - supported_config_from_par(par, par.rchan) - } else { - return Err( - backend_specific_error("missing map of sample rates to sio_par structs!").into(), - ); - }; - - Ok(config) + match *inner_state { + InnerState::Running { + ref sample_rate_to_par, + .. + } + | InnerState::Opened { + ref sample_rate_to_par, + .. + } => { + let config = if let Some(par) = sample_rate_to_par.get(&DEFAULT_SAMPLE_RATE.0) { + supported_config_from_par(par, par.rchan) + } else { + return Err(backend_specific_error( + "missing map of sample rates to sio_par structs!", + ) + .into()); + }; + + Ok(config) + } + _ => Err(backend_specific_error("device has not yet been opened").into()), + } } #[inline] fn default_output_config(&self) -> Result { let mut inner_state = self.inner_state.lock().unwrap(); - if inner_state.sample_rate_to_par.is_none() { - inner_state.open()?; + match *inner_state { + InnerState::Init { .. } => { + inner_state.open()?; + } + _ => {} } - // unwrap OK because the open call above will ensure this is not None. - let config = if let Some(par) = inner_state - .sample_rate_to_par - .as_ref() - .unwrap() - .get(&DEFAULT_SAMPLE_RATE.0) - { - supported_config_from_par(par, par.pchan) - } else { - return Err( - backend_specific_error("missing map of sample rates to sio_par structs!").into(), - ); - }; - - Ok(config) + match *inner_state { + InnerState::Running { + ref sample_rate_to_par, + .. + } + | InnerState::Opened { + ref sample_rate_to_par, + .. + } => { + let config = if let Some(par) = sample_rate_to_par.get(&DEFAULT_SAMPLE_RATE.0) { + supported_config_from_par(par, par.pchan) + } else { + return Err(backend_specific_error( + "missing map of sample rates to sio_par structs!", + ) + .into()); + }; + + Ok(config) + } + _ => Err(backend_specific_error("device has not yet been opened").into()), + } } fn build_input_stream_raw( @@ -620,26 +797,41 @@ impl DeviceTrait for Device { let mut inner_state = self.inner_state.lock().unwrap(); - setup_stream(&mut inner_state, config)?; + inner_state.setup_stream(config)?; - let boxed_data_cb = if sample_format != SampleFormat::I16 { - input_adapter_callback( - data_callback, - inner_state.buffer_size.unwrap(), // unwrap OK because configured in setup_stream, above - sample_format, - ) - } else { - Box::new(data_callback) - }; + let idx; + let boxed_data_cb; + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => { + return Err(backend_specific_error("stream was not properly setup").into()); + } + InnerState::Running { + ref buffer_size, .. + } => { + boxed_data_cb = if sample_format != SampleFormat::I16 { + input_adapter_callback(data_callback, *buffer_size, sample_format) + } else { + Box::new(data_callback) + }; + } + } - let idx = inner_state.add_input_callbacks(InputCallbacks { + idx = inner_state.add_input_callbacks(InputCallbacks { data_callback: boxed_data_cb, error_callback: Box::new(error_callback), - }); - - if inner_state.status != Status::Running { - thread::spawn(move || runner(inner_state_arc)); - inner_state.status = Status::Running; + })?; + + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => {} + InnerState::Running { + ref mut thread_spawned, + .. + } => { + if !*thread_spawned { + thread::spawn(move || runner(inner_state_arc)); + *thread_spawned = true; + } + } } drop(inner_state); // Unlock @@ -666,26 +858,41 @@ impl DeviceTrait for Device { let mut inner_state = self.inner_state.lock().unwrap(); - setup_stream(&mut inner_state, config)?; + inner_state.setup_stream(config)?; - let boxed_data_cb = if sample_format != SampleFormat::I16 { - output_adapter_callback( - data_callback, - inner_state.buffer_size.unwrap(), // unwrap OK because configured in setup_stream, above - sample_format, - ) - } else { - Box::new(data_callback) - }; + let idx; + let boxed_data_cb; + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => { + return Err(backend_specific_error("stream was not properly setup").into()); + } + InnerState::Running { + ref buffer_size, .. + } => { + boxed_data_cb = if sample_format != SampleFormat::I16 { + output_adapter_callback(data_callback, *buffer_size, sample_format) + } else { + Box::new(data_callback) + }; + } + } - let idx = inner_state.add_output_callbacks(OutputCallbacks { + idx = inner_state.add_output_callbacks(OutputCallbacks { data_callback: boxed_data_cb, error_callback: Box::new(error_callback), - }); - - if inner_state.status != Status::Running { - thread::spawn(move || runner(inner_state_arc)); - inner_state.status = Status::Running; + })?; + + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => {} + InnerState::Running { + ref mut thread_spawned, + .. + } => { + if !*thread_spawned { + thread::spawn(move || runner(inner_state_arc)); + *thread_spawned = true; + } + } } drop(inner_state); // Unlock @@ -697,84 +904,6 @@ impl DeviceTrait for Device { } } -/// Common code shared between build_input_stream_raw and build_output_stream_raw -fn setup_stream( - inner_state: &mut InnerState, - config: &StreamConfig, -) -> Result<(), BuildStreamError> { - if inner_state.sample_rate_to_par.is_none() { - inner_state.open()?; - } - - // TODO: one day we should be able to remove this - assert_eq!( - inner_state.input_callbacks.len() + inner_state.output_callbacks.len() > 0, - inner_state.par.is_some(), - "par can be None if and only if there are no input or output callbacks" - ); - - let par; // Either the currently configured par for existing streams or the one we will set - if let Some(configured_par) = inner_state.par { - par = configured_par; - - // Perform some checks - if par.rate != config.sample_rate.0 as u32 { - return Err(backend_specific_error("sample rates don't match").into()); - } - } else { - // No running streams yet; we get to set the par. - // unwrap OK because this is setup on inner_state.open() call above - if let Some(par_) = inner_state - .sample_rate_to_par - .as_ref() - .unwrap() - .get(&config.sample_rate.0) - { - par = par_.clone(); - } else { - return Err(backend_specific_error(format!( - "no configuration for sample rate {}", - config.sample_rate.0 - )) - .into()); - } - } - - // Round up the buffer size the user selected to the next multiple of par.round. If there - // was already a stream created with a different buffer size, return an error (sorry). - // Note: if we want stereo support, this will need to change. - let round = par.round as usize; - let desired_buffer_size = match config.buffer_size { - BufferSize::Fixed(requested) => { - if requested > 0 { - requested as usize + round - ((requested - 1) as usize % round) - 1 - } else { - round - } - } - BufferSize::Default => { - if let Some(bufsize) = inner_state.buffer_size { - bufsize - } else { - DEFAULT_ROUND_MULTIPLE * round - } - } - }; - - if inner_state.buffer_size.is_some() && inner_state.buffer_size != Some(desired_buffer_size) { - return Err(backend_specific_error("buffer sizes don't match").into()); - } - - if inner_state.par.is_none() { - let mut par = par; - par.appbufsz = desired_buffer_size as u32; - inner_state.buffer_size = Some(desired_buffer_size); - inner_state.set_params(&par)?; - inner_state.par = Some(par.clone()); - } - Ok(()) -} - fn supported_config_from_par(par: &sndio_sys::sio_par, num_channels: u32) -> SupportedStreamConfig { SupportedStreamConfig { channels: num_channels as u16, @@ -866,18 +995,27 @@ impl Drop for Stream { fn drop(&mut self) { let mut inner_state = self.inner_state.lock().unwrap(); if self.is_output { - inner_state.remove_output_callbacks(self.index); + let _ = inner_state.remove_output_callbacks(self.index); } else { - inner_state.remove_input_callbacks(self.index); + let _ = inner_state.remove_input_callbacks(self.index); } - if inner_state.input_callbacks.len() == 0 - && inner_state.output_callbacks.len() == 0 - && inner_state.status == Status::Running - { - if let Some(ref sender) = inner_state.wakeup_sender { - let _ = sender.send(()); + match *inner_state { + InnerState::Running { + ref input_callbacks, + ref output_callbacks, + ref thread_spawned, + ref wakeup_sender, + .. + } => { + if input_callbacks.len() == 0 && output_callbacks.len() == 0 && *thread_spawned { + // Wake up runner thread so it can shut down + if let Some(ref sender) = wakeup_sender { + let _ = sender.send(()); + } + } } + _ => {} } } } @@ -885,14 +1023,55 @@ impl Drop for Stream { impl Drop for Device { fn drop(&mut self) { let inner_state = self.inner_state.lock().unwrap(); - if inner_state.input_callbacks.len() == 0 - && inner_state.output_callbacks.len() == 0 - && inner_state.status == Status::Running - { - // Attempt to wakeup runner thread - if let Some(ref sender) = inner_state.wakeup_sender { - let _ = sender.send(()); + match *inner_state { + InnerState::Running { + ref input_callbacks, + ref output_callbacks, + ref thread_spawned, + ref wakeup_sender, + .. + } => { + if input_callbacks.len() == 0 && output_callbacks.len() == 0 && *thread_spawned { + // Wake up runner thread so it can shut down + if let Some(ref sender) = wakeup_sender { + let _ = sender.send(()); + } + } } + _ => {} } } } + +fn determine_buffer_size( + requested: &BufferSize, + round: u32, + configured_size: Option, +) -> Result { + let round = round as usize; + // Round up the buffer size the user selected to the next multiple of par.round. If there + // was already a stream created with a different buffer size, return an error (sorry). + // Note: if we want stereo support, this will need to change. + let desired_buffer_size = match requested { + BufferSize::Fixed(requested) => { + let requested = *requested as usize; + if requested > 0 { + requested + round - ((requested - 1) % round) - 1 + } else { + round + } + } + BufferSize::Default => { + if let Some(bufsize) = configured_size { + bufsize + } else { + DEFAULT_ROUND_MULTIPLE * round + } + } + }; + + if configured_size.is_some() && configured_size != Some(desired_buffer_size) { + return Err(backend_specific_error("buffer sizes don't match").into()); + } + Ok(desired_buffer_size) +} diff --git a/src/host/sndio/runner.rs b/src/host/sndio/runner.rs index 03b0c9553..7321be797 100644 --- a/src/host/sndio/runner.rs +++ b/src/host/sndio/runner.rs @@ -1,7 +1,7 @@ use std::sync::{mpsc, Arc, Mutex}; use std::time::{Duration, Instant}; -use super::{backend_specific_error, InnerState, Status}; +use super::{backend_specific_error, InnerState}; use crate::{ Data, InputCallbackInfo, InputStreamTimestamp, OutputCallbackInfo, OutputStreamTimestamp, @@ -17,27 +17,38 @@ pub(super) fn runner(inner_state_arc: Arc>) { let (wakeup_sender, wakeup_receiver) = mpsc::channel(); { let mut inner_state = inner_state_arc.lock().unwrap(); - inner_state.wakeup_sender = Some(wakeup_sender); + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => { + // Unlikely error state + inner_state.error(backend_specific_error( + "inner state should be InnerState::Running", + )); + return; + } + InnerState::Running { + wakeup_sender: ref mut wakeup_sender_, + buffer_size: ref buffer_size_, + ref par, + .. + } => { + *wakeup_sender_ = Some(wakeup_sender); - buffer_size = inner_state.buffer_size.unwrap(); // Unwrap OK because it's always picked before Stream is created - if buffer_size == 0 { - // Probably unreachable - inner_state.error(backend_specific_error("could not determine buffer size")); - return; - } + buffer_size = *buffer_size_; + if buffer_size == 0 { + // Probably unreachable + inner_state.error(backend_specific_error("could not determine buffer size")); + return; + } - if let Err(err) = inner_state.open() { - inner_state.error(err); - return; + latency = Duration::from_secs(1) * buffer_size as u32 / par.rate; + } } + if let Err(err) = inner_state.start() { inner_state.error(err); return; } - // unwrap OK because par will not be None once a Stream is created, and we can't get here - // before then. - latency = Duration::from_secs(1) * buffer_size as u32 / inner_state.par.unwrap().rate; start_time = Instant::now(); } @@ -69,7 +80,27 @@ pub(super) fn runner(inner_state_arc: Arc>) { { let mut inner_state = inner_state_arc.lock().unwrap(); // If there's nothing to do, wait until that's no longer the case. - if inner_state.input_callbacks.len() == 0 && inner_state.output_callbacks.len() == 0 { + let input_cbs; + let output_cbs; + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => { + // Unlikely error state + inner_state.error(backend_specific_error( + "inner state should be InnerState::Running", + )); + break; + } + InnerState::Running { + ref input_callbacks, + ref output_callbacks, + .. + } => { + input_cbs = input_callbacks; + output_cbs = output_callbacks; + } + } + + if input_cbs.len() == 0 && output_cbs.len() == 0 { if !paused { if let Err(_) = inner_state.stop() { // No callbacks to error with @@ -77,7 +108,6 @@ pub(super) fn runner(inner_state_arc: Arc>) { } } paused = true; - inner_state.par = None; // Allow a stream with different parameters to come along while let Ok(_) = wakeup_receiver.try_recv() {} // While the lock is still held, drain the channel. // Unlock to prevent deadlock @@ -98,10 +128,24 @@ pub(super) fn runner(inner_state_arc: Arc>) { { let mut inner_state = inner_state_arc.lock().unwrap(); if paused { - if inner_state.input_callbacks.len() == 0 && inner_state.output_callbacks.len() == 0 - { - // Spurious wakeup - continue; + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => { + // Unlikely error state + inner_state.error(backend_specific_error( + "inner state should be InnerState::Running", + )); + break; + } + InnerState::Running { + ref input_callbacks, + ref output_callbacks, + .. + } => { + if input_callbacks.len() == 0 && output_callbacks.len() == 0 { + // Spurious wakeup + continue; + } + } } if let Err(err) = inner_state.start() { @@ -113,37 +157,46 @@ pub(super) fn runner(inner_state_arc: Arc>) { } paused = false; } - nfds = unsafe { - sndio_sys::sio_nfds(inner_state.hdl.as_ref().unwrap().0) // Unwrap OK because of open call above - }; - if nfds <= 0 { - inner_state.error(backend_specific_error(format!( - "cannot allocate {} pollfd structs", - nfds - ))); - break; - } - pollfds = [libc::pollfd { - fd: 0, - events: 0, - revents: 0, - }] - .repeat(nfds as usize); - - // Populate pollfd structs with sndio_sys::sio_pollfd - nfds = unsafe { - sndio_sys::sio_pollfd( - inner_state.hdl.as_ref().unwrap().0, // Unwrap OK because of open call above - pollfds.as_mut_ptr(), - (libc::POLLOUT | libc::POLLIN) as i32, - ) - }; - if nfds <= 0 || nfds > pollfds.len() as i32 { - inner_state.error(backend_specific_error(format!( - "invalid pollfd count from sio_pollfd: {}", - nfds - ))); - break; + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => { + // Unlikely error state + inner_state.error(backend_specific_error( + "inner state should be InnerState::Running", + )); + break; + } + InnerState::Running { ref mut hdl, .. } => { + nfds = unsafe { sndio_sys::sio_nfds(hdl.0) }; + if nfds <= 0 { + inner_state.error(backend_specific_error(format!( + "cannot allocate {} pollfd structs", + nfds + ))); + break; + } + pollfds = [libc::pollfd { + fd: 0, + events: 0, + revents: 0, + }] + .repeat(nfds as usize); + + // Populate pollfd structs with sndio_sys::sio_pollfd + nfds = unsafe { + sndio_sys::sio_pollfd( + hdl.0, + pollfds.as_mut_ptr(), + (libc::POLLOUT | libc::POLLIN) as i32, + ) + }; + if nfds <= 0 || nfds > pollfds.len() as i32 { + inner_state.error(backend_specific_error(format!( + "invalid pollfd count from sio_pollfd: {}", + nfds + ))); + break; + } + } } } @@ -161,17 +214,26 @@ pub(super) fn runner(inner_state_arc: Arc>) { let revents; { let mut inner_state = inner_state_arc.lock().unwrap(); - // Unwrap OK because of open call above - revents = unsafe { - sndio_sys::sio_revents(inner_state.hdl.as_ref().unwrap().0, pollfds.as_mut_ptr()) - } as i16; + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => { + // Unlikely error state + inner_state.error(backend_specific_error( + "inner state should be InnerState::Running", + )); + break; + } + InnerState::Running { ref mut hdl, .. } => { + revents = unsafe { sndio_sys::sio_revents(hdl.0, pollfds.as_mut_ptr()) } as i16; + } + } if revents & libc::POLLHUP != 0 { inner_state.error(backend_specific_error("device disappeared")); break; } - if revents & (libc::POLLOUT | libc::POLLIN) == 0 { - continue; - } + } + + if revents & (libc::POLLOUT | libc::POLLIN) == 0 { + continue; } let elapsed = Instant::now().duration_since(start_time); @@ -195,40 +257,60 @@ pub(super) fn runner(inner_state_arc: Arc>) { { let mut inner_state = inner_state_arc.lock().unwrap(); - if output_offset_bytes_into_buf == 0 { - // The whole output buffer has been written (or this is the first time). Fill it. - if inner_state.output_callbacks.len() == 0 { - if clear_output_buf_needed { - // There is probably nonzero data in the buffer from previous output - // Streams. Zero it out. - for sample in output_buf.iter_mut() { - *sample = 0; - } - clear_output_buf_needed = false; - } - } else { - for opt_cbs in &mut inner_state.output_callbacks { - if let Some(cbs) = opt_cbs { - // Really we shouldn't have more than one output callback as they are - // stepping on each others' data. - // TODO: perhaps we should not call these callbacks while holding the lock - (cbs.data_callback)(&mut output_data, &output_callback_info); + let bytes_written; + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => { + // Unlikely error state + inner_state.error(backend_specific_error( + "inner state should be InnerState::Running", + )); + break; + } + InnerState::Running { + ref mut hdl, + ref mut output_callbacks, + .. + } => { + if output_offset_bytes_into_buf == 0 { + // The whole output buffer has been written (or this is the first time). Fill it. + if output_callbacks.len() == 0 { + if clear_output_buf_needed { + // There is probably nonzero data in the buffer from previous output + // Streams. Zero it out. + for sample in output_buf.iter_mut() { + *sample = 0; + } + clear_output_buf_needed = false; + } + } else { + for opt_cbs in output_callbacks { + if let Some(cbs) = opt_cbs { + // Really we shouldn't have more than one output callback as they are + // stepping on each others' data. + // TODO: perhaps we should not call these callbacks while holding the lock + (cbs.data_callback)( + &mut output_data, + &output_callback_info, + ); + } + } + clear_output_buf_needed = true; } } - clear_output_buf_needed = true; + + // unwrap OK because .open was called + bytes_written = unsafe { + sndio_sys::sio_write( + hdl.0, + (output_data.data as *const u8) + .add(output_offset_bytes_into_buf as usize) + as *const _, + data_byte_size as u64 - output_offset_bytes_into_buf, + ) + }; } } - // unwrap OK because .open was called - let bytes_written = unsafe { - sndio_sys::sio_write( - inner_state.hdl.as_ref().unwrap().0, - (output_data.data as *const u8).add(output_offset_bytes_into_buf as usize) - as *const _, - data_byte_size as u64 - output_offset_bytes_into_buf, - ) - }; - if bytes_written <= 0 { inner_state.error(backend_specific_error("no bytes written; EOF?")); break; @@ -269,14 +351,27 @@ pub(super) fn runner(inner_state_arc: Arc>) { let mut inner_state = inner_state_arc.lock().unwrap(); // unwrap OK because .open was called - let bytes_read = unsafe { - sndio_sys::sio_read( - inner_state.hdl.as_ref().unwrap().0, - (input_data.data as *const u8).add(input_offset_bytes_into_buf as usize) - as *mut _, - data_byte_size as u64 - input_offset_bytes_into_buf, - ) - }; + let bytes_read; + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => { + // Unlikely error state + inner_state.error(backend_specific_error( + "inner state should be InnerState::Running", + )); + break; + } + InnerState::Running { ref mut hdl, .. } => { + bytes_read = unsafe { + sndio_sys::sio_read( + hdl.0, + (input_data.data as *const u8) + .add(input_offset_bytes_into_buf as usize) + as *mut _, + data_byte_size as u64 - input_offset_bytes_into_buf, + ) + } + } + } if bytes_read <= 0 { inner_state.error(backend_specific_error("no bytes read; EOF?")); @@ -295,10 +390,24 @@ pub(super) fn runner(inner_state_arc: Arc>) { }; if input_offset_bytes_into_buf == 0 { - for opt_cbs in &mut inner_state.input_callbacks { - if let Some(cbs) = opt_cbs { - // TODO: perhaps we should not call these callbacks while holding the lock - (cbs.data_callback)(&input_data, &input_callback_info); + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => { + // Unlikely error state + inner_state.error(backend_specific_error( + "inner state should be InnerState::Running", + )); + break; + } + InnerState::Running { + ref mut input_callbacks, + .. + } => { + for opt_cbs in input_callbacks { + if let Some(cbs) = opt_cbs { + // TODO: perhaps we should not call these callbacks while holding the lock + (cbs.data_callback)(&input_data, &input_callback_info); + } + } } } } @@ -308,11 +417,22 @@ pub(super) fn runner(inner_state_arc: Arc>) { { let mut inner_state = inner_state_arc.lock().unwrap(); - inner_state.wakeup_sender = None; if !paused { let _ = inner_state.stop(); // Can't do anything with error since no error callbacks left - inner_state.par = None; } - inner_state.status = Status::Stopped; + match *inner_state { + InnerState::Init { .. } | InnerState::Opened { .. } => { + // anlikely error state but nothing to do with error + return; + } + InnerState::Running { + ref mut wakeup_sender, + ref mut thread_spawned, + .. + } => { + *wakeup_sender = None; + *thread_spawned = false; + } + } } } From 51d34869042f8e242fa14543dbda1b31ab02d984 Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Tue, 22 Dec 2020 21:17:27 -0800 Subject: [PATCH 09/17] sndio: address feedback from code review * Add SampleRateMap with SampleRate keys rather than raw HashMap with u32 keys. * Use FrameCount type alias rather than usize for buffer size. * More robust error handling. * Document InnerState state transitions. --- src/host/sndio/adapters.rs | 22 ++--- src/host/sndio/mod.rs | 163 ++++++++++++++++++++++++++----------- src/host/sndio/runner.rs | 13 +-- 3 files changed, 133 insertions(+), 65 deletions(-) diff --git a/src/host/sndio/adapters.rs b/src/host/sndio/adapters.rs index f16d68261..6089e46aa 100644 --- a/src/host/sndio/adapters.rs +++ b/src/host/sndio/adapters.rs @@ -1,11 +1,11 @@ -use crate::{Data, InputCallbackInfo, OutputCallbackInfo, Sample, SampleFormat}; +use crate::{Data, FrameCount, InputCallbackInfo, OutputCallbackInfo, Sample, SampleFormat}; /// When given an input data callback that expects samples in the specified sample format, return /// an input data callback that expects samples in the I16 sample format. The `buffer_size` is in /// samples. pub(super) fn input_adapter_callback( mut original_data_callback: D, - buffer_size: usize, + buffer_size: FrameCount, sample_format: SampleFormat, ) -> Box where @@ -18,7 +18,7 @@ where } SampleFormat::F32 => { // Make the backing buffer for the Data used in the closure. - let mut adapted_buf = vec![0f32].repeat(buffer_size); + let mut adapted_buf = vec![0f32].repeat(buffer_size as usize); Box::new(move |data: &Data, info: &InputCallbackInfo| { let data_slice: &[i16] = data.as_slice().unwrap(); // unwrap OK because data is always i16 let adapted_slice = &mut adapted_buf; @@ -32,7 +32,7 @@ where let adapted_data = unsafe { Data::from_parts( adapted_buf.as_mut_ptr() as *mut _, - buffer_size, + buffer_size as usize, // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! sample_format, ) }; @@ -40,7 +40,7 @@ where }) } SampleFormat::U16 => { - let mut adapted_buf = vec![0u16].repeat(buffer_size); + let mut adapted_buf = vec![0u16].repeat(buffer_size as usize); Box::new(move |data: &Data, info: &InputCallbackInfo| { let data_slice: &[i16] = data.as_slice().unwrap(); // unwrap OK because data is always i16 let adapted_slice = &mut adapted_buf; @@ -54,7 +54,7 @@ where let adapted_data = unsafe { Data::from_parts( adapted_buf.as_mut_ptr() as *mut _, - buffer_size, + buffer_size as usize, // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! sample_format, ) }; @@ -69,7 +69,7 @@ where /// sample format. The `buffer_size` is in samples. pub(super) fn output_adapter_callback( mut original_data_callback: D, - buffer_size: usize, + buffer_size: FrameCount, sample_format: SampleFormat, ) -> Box where @@ -82,7 +82,7 @@ where } SampleFormat::F32 => { // Make the backing buffer for the Data used in the closure. - let mut adapted_buf = vec![0f32].repeat(buffer_size); + let mut adapted_buf = vec![0f32].repeat(buffer_size as usize); Box::new(move |data: &mut Data, info: &OutputCallbackInfo| { // Note: we construct adapted_data here instead of in the parent function because @@ -90,7 +90,7 @@ where let mut adapted_data = unsafe { Data::from_parts( adapted_buf.as_mut_ptr() as *mut _, - buffer_size, + buffer_size as usize, // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! sample_format, ) }; @@ -108,7 +108,7 @@ where } SampleFormat::U16 => { // Make the backing buffer for the Data used in the closure. - let mut adapted_buf = vec![0u16].repeat(buffer_size); + let mut adapted_buf = vec![0u16].repeat(buffer_size as usize); Box::new(move |data: &mut Data, info: &OutputCallbackInfo| { // Note: we construct adapted_data here instead of in the parent function because @@ -116,7 +116,7 @@ where let mut adapted_data = unsafe { Data::from_parts( adapted_buf.as_mut_ptr() as *mut _, - buffer_size, + buffer_size as usize, // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! sample_format, ) }; diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index 615cfc728..5212562fd 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -6,6 +6,7 @@ mod runner; use self::adapters::{input_adapter_callback, output_adapter_callback}; use self::runner::runner; +use std::collections::hash_map; use std::collections::HashMap; use std::convert::From; use std::mem::{self, MaybeUninit}; @@ -16,9 +17,9 @@ use thiserror::Error; use crate::{ BackendSpecificError, BufferSize, BuildStreamError, Data, DefaultStreamConfigError, - DeviceNameError, DevicesError, HostUnavailable, InputCallbackInfo, OutputCallbackInfo, - PauseStreamError, PlayStreamError, SampleFormat, SampleRate, StreamConfig, StreamError, - SupportedBufferSize, SupportedStreamConfig, SupportedStreamConfigRange, + DeviceNameError, DevicesError, FrameCount, HostUnavailable, InputCallbackInfo, + OutputCallbackInfo, PauseStreamError, PlayStreamError, SampleFormat, SampleRate, StreamConfig, + StreamError, SupportedBufferSize, SupportedStreamConfig, SupportedStreamConfigRange, SupportedStreamConfigsError, }; @@ -27,8 +28,8 @@ use traits::{DeviceTrait, HostTrait, StreamTrait}; pub type SupportedInputConfigs = ::std::vec::IntoIter; pub type SupportedOutputConfigs = ::std::vec::IntoIter; -/// Default multiple of the round field of a sio_par struct to use for the buffer size. -const DEFAULT_ROUND_MULTIPLE: usize = 2; +/// Default multiple of the round field of a sio_par struct to use for the buffer size (in frames). +const DEFAULT_ROUND_MULTIPLE: u32 = 2; const DEFAULT_SAMPLE_RATE: SampleRate = SampleRate(48000); const SUPPORTED_SAMPLE_RATES: &[SampleRate] = @@ -118,11 +119,11 @@ struct SioHdl(*mut sndio_sys::sio_hdl); impl SioHdl { /// Returns a map of sample rates to sio_par by re-configuring the device. This should not be /// performed while recording or playing, or even after configuring the device for this state! - fn get_par_map( + fn get_sample_rate_map( &mut self, behavior: BufferXrunBehavior, - ) -> Result, SndioError> { - let mut sample_rate_to_par = HashMap::new(); + ) -> Result { + let mut sample_rate_map = SampleRateMap::new(); for rate in SUPPORTED_SAMPLE_RATES { let mut par = new_sio_par(); @@ -165,9 +166,9 @@ impl SioHdl { // TODO: more checks -- bits, bps, sig, le, msb - sample_rate_to_par.insert(rate.0, par); + sample_rate_map.insert(*rate, par); } - Ok(sample_rate_to_par) + Ok(sample_rate_map) } /// Calls sio_setpar and sio_getpar on the passed in sio_par struct. Before calling this, the @@ -231,7 +232,6 @@ impl SioHdl { newpar.xrun = par.xrun; let status = unsafe { // Request the device using our parameters - // unwrap OK because of the check at the top of this function. sndio_sys::sio_setpar(self.0, &mut newpar as *mut _) }; if status != 1 { @@ -241,9 +241,52 @@ impl SioHdl { } } +// It is necessary to add the Send marker trait to this struct because the Arc> +// which contains it needs to be passed to the runner thread. This is sound as long as the sio_hdl +// pointer is not copied out of its SioHdl and used while the mutex is unlocked. unsafe impl Send for SioHdl {} -/// The shared state between Device and Stream. Responsible for closing handle when dropped. +struct SampleRateMap(pub HashMap); + +impl SampleRateMap { + fn new() -> Self { + SampleRateMap(HashMap::new()) + } + + fn get(&self, rate: SampleRate) -> Option<&sndio_sys::sio_par> { + self.0.get(&rate.0) + } + + fn insert(&mut self, rate: SampleRate, par: sndio_sys::sio_par) -> Option { + self.0.insert(rate.0, par) + } + + fn iter(&self) -> SampleRateMapIter<'_> { + SampleRateMapIter { + iter: self.0.iter(), + } + } +} + +struct SampleRateMapIter<'a> { + iter: hash_map::Iter<'a, u32, sndio_sys::sio_par>, +} + +impl<'a> Iterator for SampleRateMapIter<'a> { + type Item = (SampleRate, &'a sndio_sys::sio_par); + + fn next(&mut self) -> Option { + self.iter + .next() + .map(|(sample_rate, par)| (SampleRate(*sample_rate), par)) + } +} + +/// The shared state between `Device` and `Stream`. Responsible for closing handle when dropped. +/// Upon `Device` creation, this is in the `Init` state. Calling `.open` transitions +/// this to the `Opened` state (this generally happens when getting input or output configs). +/// From there, the state can transition to `Running` once at least one `Stream` has been created +/// with the `build_input_stream_raw` or `build_output_stream_raw` functions. enum InnerState { Init { /// Buffer overrun/underrun behavior -- ignore/sync/error? @@ -256,7 +299,7 @@ enum InnerState { hdl: SioHdl, /// Map of sample rate to parameters. - sample_rate_to_par: HashMap, + sample_rate_map: SampleRateMap, }, Running { /// Contains a handle returned from sio_open. Note that even though this is a pointer type @@ -265,13 +308,13 @@ enum InnerState { hdl: SioHdl, /// Contains the chosen buffer size, in elements not bytes. - buffer_size: usize, + buffer_size: FrameCount, /// Stores the sndio-configured parameters. par: sndio_sys::sio_par, /// Map of sample rate to parameters. - sample_rate_to_par: HashMap, + sample_rate_map: SampleRateMap, /// Each input Stream that has not been dropped has its callbacks in an element of this Vec. /// The last element is guaranteed to not be None. @@ -310,7 +353,9 @@ impl InnerState { fn open(&mut self) -> Result<(), SndioError> { match self { - InnerState::Opened { .. } | InnerState::Running { .. } => Ok(()), // Already opened + InnerState::Opened { .. } | InnerState::Running { .. } => { + Err(backend_specific_error("device is already open")) + } InnerState::Init { ref behavior } => { let hdl = unsafe { // The transmute is needed because this C string is *const u8 in one place but *const i8 in another place. @@ -328,10 +373,10 @@ impl InnerState { } let mut hdl = SioHdl(hdl); - let sample_rate_to_par = hdl.get_par_map(*behavior)?; + let sample_rate_map = hdl.get_sample_rate_map(*behavior)?; *self = InnerState::Opened { hdl, - sample_rate_to_par, + sample_rate_map, }; Ok(()) } @@ -524,12 +569,11 @@ impl InnerState { } InnerState::Opened { hdl, - sample_rate_to_par, + sample_rate_map, } => { // No running streams yet; we get to set the par. - // unwrap OK because this is setup on self.open() call above let mut par; - if let Some(par_) = sample_rate_to_par.get(&config.sample_rate.0) { + if let Some(par_) = sample_rate_map.get(config.sample_rate) { par = par_.clone(); } else { return Err(backend_specific_error(format!( @@ -544,14 +588,14 @@ impl InnerState { // Transition to running par.appbufsz = buffer_size as u32; hdl.set_params(&par)?; - let mut tmp: HashMap = HashMap::new(); - mem::swap(&mut tmp, sample_rate_to_par); + let mut tmp = SampleRateMap::new(); + mem::swap(&mut tmp, sample_rate_map); *self = InnerState::Running { hdl: SioHdl(hdl.0), // Just this once, it's ok to copy this buffer_size, par, - sample_rate_to_par: tmp, + sample_rate_map: tmp, input_callbacks: vec![], output_callbacks: vec![], thread_spawned: false, @@ -607,7 +651,9 @@ impl Device { } pub fn set_xrun_behavior(&mut self, b: BufferXrunBehavior) -> Result<(), SndioError> { - let mut inner_state = self.inner_state.lock().unwrap(); + let mut inner_state = self.inner_state.lock().map_err(|e| { + backend_specific_error(format!("InnerState unlock error: {:?}", e)).into() + })?; match *inner_state { InnerState::Init { ref mut behavior, .. @@ -636,7 +682,12 @@ impl DeviceTrait for Device { fn supported_input_configs( &self, ) -> Result { - let mut inner_state = self.inner_state.lock().unwrap(); + let mut inner_state = + self.inner_state + .lock() + .map_err(|e| -> SupportedStreamConfigsError { + backend_specific_error(format!("InnerState unlock error: {:?}", e)).into() + })?; match *inner_state { InnerState::Init { .. } => { @@ -647,15 +698,15 @@ impl DeviceTrait for Device { match *inner_state { InnerState::Running { - ref sample_rate_to_par, + ref sample_rate_map, .. } | InnerState::Opened { - ref sample_rate_to_par, + ref sample_rate_map, .. } => { let mut config_ranges = vec![]; - for (_, par) in sample_rate_to_par { + for (_, par) in sample_rate_map.iter() { let config = supported_config_from_par(par, par.rchan); config_ranges.push(SupportedStreamConfigRange { channels: config.channels, @@ -676,7 +727,12 @@ impl DeviceTrait for Device { fn supported_output_configs( &self, ) -> Result { - let mut inner_state = self.inner_state.lock().unwrap(); + let mut inner_state = + self.inner_state + .lock() + .map_err(|e| -> SupportedStreamConfigsError { + backend_specific_error(format!("InnerState unlock error: {:?}", e)).into() + })?; match *inner_state { InnerState::Init { .. } => { @@ -687,15 +743,15 @@ impl DeviceTrait for Device { match *inner_state { InnerState::Running { - ref sample_rate_to_par, + ref sample_rate_map, .. } | InnerState::Opened { - ref sample_rate_to_par, + ref sample_rate_map, .. } => { let mut config_ranges = vec![]; - for (_, par) in sample_rate_to_par { + for (_, par) in sample_rate_map.iter() { let config = supported_config_from_par(par, par.pchan); config_ranges.push(SupportedStreamConfigRange { channels: config.channels, @@ -714,7 +770,12 @@ impl DeviceTrait for Device { #[inline] fn default_input_config(&self) -> Result { - let mut inner_state = self.inner_state.lock().unwrap(); + let mut inner_state = self + .inner_state + .lock() + .map_err(|e| -> DefaultStreamConfigError { + backend_specific_error(format!("InnerState unlock error: {:?}", e)).into() + })?; match *inner_state { InnerState::Init { .. } => { @@ -725,14 +786,14 @@ impl DeviceTrait for Device { match *inner_state { InnerState::Running { - ref sample_rate_to_par, + ref sample_rate_map, .. } | InnerState::Opened { - ref sample_rate_to_par, + ref sample_rate_map, .. } => { - let config = if let Some(par) = sample_rate_to_par.get(&DEFAULT_SAMPLE_RATE.0) { + let config = if let Some(par) = sample_rate_map.get(DEFAULT_SAMPLE_RATE) { supported_config_from_par(par, par.rchan) } else { return Err(backend_specific_error( @@ -749,7 +810,12 @@ impl DeviceTrait for Device { #[inline] fn default_output_config(&self) -> Result { - let mut inner_state = self.inner_state.lock().unwrap(); + let mut inner_state = self + .inner_state + .lock() + .map_err(|e| -> DefaultStreamConfigError { + backend_specific_error(format!("InnerState unlock error: {:?}", e)).into() + })?; match *inner_state { InnerState::Init { .. } => { @@ -760,14 +826,14 @@ impl DeviceTrait for Device { match *inner_state { InnerState::Running { - ref sample_rate_to_par, + ref sample_rate_map, .. } | InnerState::Opened { - ref sample_rate_to_par, + ref sample_rate_map, .. } => { - let config = if let Some(par) = sample_rate_to_par.get(&DEFAULT_SAMPLE_RATE.0) { + let config = if let Some(par) = sample_rate_map.get(DEFAULT_SAMPLE_RATE) { supported_config_from_par(par, par.pchan) } else { return Err(backend_specific_error( @@ -856,7 +922,9 @@ impl DeviceTrait for Device { { let inner_state_arc = self.inner_state.clone(); - let mut inner_state = self.inner_state.lock().unwrap(); + let mut inner_state = self.inner_state.lock().map_err(|e| -> BuildStreamError { + backend_specific_error(format!("InnerState unlock error: {:?}", e)).into() + })?; inner_state.setup_stream(config)?; @@ -906,7 +974,7 @@ impl DeviceTrait for Device { fn supported_config_from_par(par: &sndio_sys::sio_par, num_channels: u32) -> SupportedStreamConfig { SupportedStreamConfig { - channels: num_channels as u16, + channels: num_channels as u16, // Conversion is courtesy of type mismatch between sndio and RustAudio. sample_rate: SampleRate(par.rate), // TODO: actually frames per second, not samples per second. Important for adding multi-channel support buffer_size: SupportedBufferSize::Range { min: par.round, @@ -1045,16 +1113,15 @@ impl Drop for Device { fn determine_buffer_size( requested: &BufferSize, - round: u32, - configured_size: Option, -) -> Result { - let round = round as usize; + round: FrameCount, + configured_size: Option, +) -> Result { // Round up the buffer size the user selected to the next multiple of par.round. If there // was already a stream created with a different buffer size, return an error (sorry). // Note: if we want stereo support, this will need to change. let desired_buffer_size = match requested { BufferSize::Fixed(requested) => { - let requested = *requested as usize; + let requested = *requested; if requested > 0 { requested + round - ((requested - 1) % round) - 1 } else { diff --git a/src/host/sndio/runner.rs b/src/host/sndio/runner.rs index 7321be797..466f8b9e8 100644 --- a/src/host/sndio/runner.rs +++ b/src/host/sndio/runner.rs @@ -4,13 +4,13 @@ use std::time::{Duration, Instant}; use super::{backend_specific_error, InnerState}; use crate::{ - Data, InputCallbackInfo, InputStreamTimestamp, OutputCallbackInfo, OutputStreamTimestamp, - SampleFormat, StreamInstant, + Data, FrameCount, InputCallbackInfo, InputStreamTimestamp, OutputCallbackInfo, + OutputStreamTimestamp, SampleFormat, StreamInstant, }; /// The runner thread handles playing and/or recording pub(super) fn runner(inner_state_arc: Arc>) { - let buffer_size: usize; + let buffer_size: FrameCount; let start_time: Instant; let latency: Duration; let mut clear_output_buf_needed = false; @@ -40,7 +40,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { return; } - latency = Duration::from_secs(1) * buffer_size as u32 / par.rate; + latency = Duration::from_secs(1) * buffer_size / par.rate; } } @@ -52,8 +52,9 @@ pub(super) fn runner(inner_state_arc: Arc>) { start_time = Instant::now(); } - let mut output_buf = [0i16].repeat(buffer_size); // Allocate buffer of correct size - let mut input_buf = [0i16].repeat(buffer_size); // Allocate buffer of correct size + // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! + let mut output_buf = [0i16].repeat(buffer_size as usize); // Allocate buffer of correct size + let mut input_buf = [0i16].repeat(buffer_size as usize); // Allocate buffer of correct size let mut output_data = unsafe { Data::from_parts( output_buf.as_mut_ptr() as *mut (), From d419f0fa5cdbaf47ec2e127d9fbe86a471627bf5 Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Tue, 26 Jan 2021 20:21:35 -0800 Subject: [PATCH 10/17] sndio: change input/output_callbacks storage to a HashMap indexed by usize --- src/host/sndio/mod.rs | 87 +++++++++++++++------------------------- src/host/sndio/runner.rs | 46 ++++++--------------- 2 files changed, 46 insertions(+), 87 deletions(-) diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index 5212562fd..671a0374a 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -318,11 +318,11 @@ enum InnerState { /// Each input Stream that has not been dropped has its callbacks in an element of this Vec. /// The last element is guaranteed to not be None. - input_callbacks: Vec>, + input_callbacks: (usize, HashMap), /// Each output Stream that has not been dropped has its callbacks in an element of this Vec. /// The last element is guaranteed to not be None. - output_callbacks: Vec>, + output_callbacks: (usize, HashMap), /// Whether the runner thread was spawned yet. thread_spawned: bool, @@ -440,20 +440,16 @@ impl InnerState { ref mut wakeup_sender, .. } => { - for (i, cbs) in output_callbacks.iter_mut().enumerate() { - if cbs.is_none() { - *cbs = Some(callbacks); - return Ok(i); - } - } // If there were previously no callbacks, wakeup the runner thread. - if input_callbacks.len() == 0 && output_callbacks.len() == 0 { + if input_callbacks.1.len() == 0 && output_callbacks.1.len() == 0 { if let Some(ref sender) = wakeup_sender { let _ = sender.send(()); } } - output_callbacks.push(Some(callbacks)); - Ok(output_callbacks.len() - 1) + let index = output_callbacks.0; + output_callbacks.1.insert(index, callbacks); + output_callbacks.0 = index + 1; + Ok(index) } _ => Err(backend_specific_error("device is not in a running state")), } @@ -466,15 +462,7 @@ impl InnerState { InnerState::Running { ref mut output_callbacks, .. - } => { - let cbs = output_callbacks[index].take().unwrap(); - while output_callbacks.len() > 0 - && output_callbacks[output_callbacks.len() - 1].is_none() - { - output_callbacks.pop(); - } - Ok(cbs) - } + } => Ok(output_callbacks.1.remove(&index).unwrap()), _ => Err(backend_specific_error("device is not in a running state")), } } @@ -489,20 +477,16 @@ impl InnerState { ref mut wakeup_sender, .. } => { - for (i, cbs) in input_callbacks.iter_mut().enumerate() { - if cbs.is_none() { - *cbs = Some(callbacks); - return Ok(i); - } - } // If there were previously no callbacks, wakeup the runner thread. - if input_callbacks.len() == 0 && output_callbacks.len() == 0 { + if input_callbacks.1.len() == 0 && output_callbacks.1.len() == 0 { if let Some(ref sender) = wakeup_sender { let _ = sender.send(()); } } - input_callbacks.push(Some(callbacks)); - Ok(input_callbacks.len() - 1) + let index = input_callbacks.0; + input_callbacks.1.insert(index, callbacks); + input_callbacks.0 = index + 1; + Ok(index) } _ => Err(backend_specific_error("device is not in a running state")), } @@ -515,15 +499,7 @@ impl InnerState { InnerState::Running { ref mut input_callbacks, .. - } => { - let cbs = input_callbacks[index].take().unwrap(); - while input_callbacks.len() > 0 - && input_callbacks[input_callbacks.len() - 1].is_none() - { - input_callbacks.pop(); - } - Ok(cbs) - } + } => Ok(input_callbacks.1.remove(&index).unwrap()), _ => Err(backend_specific_error("device is not in a running state")), } } @@ -537,15 +513,11 @@ impl InnerState { .. } => { let e = e.into(); - for cbs in input_callbacks { - if let Some(cbs) = cbs { - (cbs.error_callback)(e.clone()); - } + for cbs in input_callbacks.1.values_mut() { + (cbs.error_callback)(e.clone()); } - for cbs in output_callbacks { - if let Some(cbs) = cbs { - (cbs.error_callback)(e.clone()); - } + for cbs in output_callbacks.1.values_mut() { + (cbs.error_callback)(e.clone()); } } _ => {} // Drop the error @@ -596,8 +568,8 @@ impl InnerState { buffer_size, par, sample_rate_map: tmp, - input_callbacks: vec![], - output_callbacks: vec![], + input_callbacks: (0, HashMap::new()), + output_callbacks: (0, HashMap::new()), thread_spawned: false, wakeup_sender: None, }; @@ -618,6 +590,17 @@ impl InnerState { } } } + + fn has_streams(&self) -> bool { + match self { + InnerState::Running { + ref input_callbacks, + ref output_callbacks, + .. + } => input_callbacks.1.len() > 0 || output_callbacks.1.len() > 0, + _ => false, + } + } } impl Drop for InnerState { @@ -1070,13 +1053,11 @@ impl Drop for Stream { match *inner_state { InnerState::Running { - ref input_callbacks, - ref output_callbacks, ref thread_spawned, ref wakeup_sender, .. } => { - if input_callbacks.len() == 0 && output_callbacks.len() == 0 && *thread_spawned { + if !inner_state.has_streams() && *thread_spawned { // Wake up runner thread so it can shut down if let Some(ref sender) = wakeup_sender { let _ = sender.send(()); @@ -1093,13 +1074,11 @@ impl Drop for Device { let inner_state = self.inner_state.lock().unwrap(); match *inner_state { InnerState::Running { - ref input_callbacks, - ref output_callbacks, ref thread_spawned, ref wakeup_sender, .. } => { - if input_callbacks.len() == 0 && output_callbacks.len() == 0 && *thread_spawned { + if !inner_state.has_streams() && *thread_spawned { // Wake up runner thread so it can shut down if let Some(ref sender) = wakeup_sender { let _ = sender.send(()); diff --git a/src/host/sndio/runner.rs b/src/host/sndio/runner.rs index 466f8b9e8..233c71fa8 100644 --- a/src/host/sndio/runner.rs +++ b/src/host/sndio/runner.rs @@ -81,8 +81,6 @@ pub(super) fn runner(inner_state_arc: Arc>) { { let mut inner_state = inner_state_arc.lock().unwrap(); // If there's nothing to do, wait until that's no longer the case. - let input_cbs; - let output_cbs; match *inner_state { InnerState::Init { .. } | InnerState::Opened { .. } => { // Unlikely error state @@ -91,17 +89,10 @@ pub(super) fn runner(inner_state_arc: Arc>) { )); break; } - InnerState::Running { - ref input_callbacks, - ref output_callbacks, - .. - } => { - input_cbs = input_callbacks; - output_cbs = output_callbacks; - } + _ => {} } - if input_cbs.len() == 0 && output_cbs.len() == 0 { + if !inner_state.has_streams() { if !paused { if let Err(_) = inner_state.stop() { // No callbacks to error with @@ -137,12 +128,8 @@ pub(super) fn runner(inner_state_arc: Arc>) { )); break; } - InnerState::Running { - ref input_callbacks, - ref output_callbacks, - .. - } => { - if input_callbacks.len() == 0 && output_callbacks.len() == 0 { + InnerState::Running { .. } => { + if !inner_state.has_streams() { // Spurious wakeup continue; } @@ -274,7 +261,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { } => { if output_offset_bytes_into_buf == 0 { // The whole output buffer has been written (or this is the first time). Fill it. - if output_callbacks.len() == 0 { + if output_callbacks.1.len() == 0 { if clear_output_buf_needed { // There is probably nonzero data in the buffer from previous output // Streams. Zero it out. @@ -284,16 +271,11 @@ pub(super) fn runner(inner_state_arc: Arc>) { clear_output_buf_needed = false; } } else { - for opt_cbs in output_callbacks { - if let Some(cbs) = opt_cbs { - // Really we shouldn't have more than one output callback as they are - // stepping on each others' data. - // TODO: perhaps we should not call these callbacks while holding the lock - (cbs.data_callback)( - &mut output_data, - &output_callback_info, - ); - } + for cbs in output_callbacks.1.values_mut() { + // Really we shouldn't have more than one output callback as they are + // stepping on each others' data. + // TODO: perhaps we should not call these callbacks while holding the lock + (cbs.data_callback)(&mut output_data, &output_callback_info); } clear_output_buf_needed = true; } @@ -403,11 +385,9 @@ pub(super) fn runner(inner_state_arc: Arc>) { ref mut input_callbacks, .. } => { - for opt_cbs in input_callbacks { - if let Some(cbs) = opt_cbs { - // TODO: perhaps we should not call these callbacks while holding the lock - (cbs.data_callback)(&input_data, &input_callback_info); - } + for cbs in input_callbacks.1.values_mut() { + // TODO: perhaps we should not call these callbacks while holding the lock + (cbs.data_callback)(&input_data, &input_callback_info); } } } From 2e00be6b8ac3ede17a043f7f305708bfd1136c17 Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Tue, 26 Jan 2021 21:55:11 -0800 Subject: [PATCH 11/17] sndio: test CI with linux-sndio feature --- .github/workflows/cpal.yml | 16 ++++++++++++++++ Cargo.toml | 5 +++++ src/host/mod.rs | 5 ++++- src/platform/mod.rs | 26 +++++++++++++++++++++++--- 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/.github/workflows/cpal.yml b/.github/workflows/cpal.yml index 8c637b632..434bd20af 100644 --- a/.github/workflows/cpal.yml +++ b/.github/workflows/cpal.yml @@ -82,6 +82,17 @@ jobs: run: sudo apt-get install libasound2-dev - name: Install libjack run: sudo apt-get install libjack-jackd2-dev libjack-jackd2-0 + - name: Install sndio + run: | + sudo apt-get install build-essential && + git clone https://caoua.org/git/sndio && + cd sndio && + ./configure && + make && + sudo mkdir /var/run/sndiod && + sudo useradd -r -g audio -s /sbin/nologin -d /var/run/sndiod sndiod && + sudo make install && + sudo ldconfig - name: Install stable uses: actions-rs/toolchain@v1 with: @@ -98,6 +109,11 @@ jobs: with: command: test args: --all --all-features --verbose + - name: Run with jack feature + uses: actions-rs/cargo@v1 + with: + command: test + args: --all --features jack --verbose linux-check-and-test-armv7: runs-on: ubuntu-latest diff --git a/Cargo.toml b/Cargo.toml index e95550339..6cd83a989 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,7 @@ keywords = ["audio", "sound"] [features] asio = ["asio-sys", "num-traits"] # Only available on Windows. See README for setup instructions. +linux-sndio = ["sndio-sys", "libc"] # sndio on Linux (normally this is only used on OpenBSD) [dependencies] thiserror = "1.0.2" @@ -34,6 +35,10 @@ libc = "0.2.65" parking_lot = "0.11" jack = { version = "0.6.5", optional = true } +[target.'cfg(target_os = "linux")'.dependencies] +sndio-sys = { version = "0.0.*", optional = true } +libc = { version = "0.2.65", optional = true } + [target.'cfg(target_os = "openbsd")'.dependencies] sndio-sys = "0.0.*" libc = "0.2.65" diff --git a/src/host/mod.rs b/src/host/mod.rs index cd33107c4..b8f22b6ef 100644 --- a/src/host/mod.rs +++ b/src/host/mod.rs @@ -14,7 +14,10 @@ pub(crate) mod jack; pub(crate) mod null; #[cfg(target_os = "android")] pub(crate) mod oboe; -#[cfg(target_os = "openbsd")] +#[cfg(any( + target_os = "openbsd", + all(target_os = "linux", feature = "linux-sndio") +))] pub(crate) mod sndio; #[cfg(windows)] pub(crate) mod wasapi; diff --git a/src/platform/mod.rs b/src/platform/mod.rs index 8375f95d2..acc9554e5 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -448,30 +448,50 @@ macro_rules! impl_platform_host { // TODO: Add pulseaudio and jack here eventually. #[cfg(any(target_os = "linux", target_os = "dragonfly", target_os = "freebsd"))] mod platform_impl { + #[cfg(not(feature = "linux-sndio"))] pub use crate::host::alsa::{ Device as AlsaDevice, Devices as AlsaDevices, Host as AlsaHost, Stream as AlsaStream, SupportedInputConfigs as AlsaSupportedInputConfigs, SupportedOutputConfigs as AlsaSupportedOutputConfigs, }; - #[cfg(feature = "jack")] + #[cfg(all(feature = "jack", not(feature = "linux-sndio")))] pub use crate::host::jack::{ Device as JackDevice, Devices as JackDevices, Host as JackHost, Stream as JackStream, SupportedInputConfigs as JackSupportedInputConfigs, SupportedOutputConfigs as JackSupportedOutputConfigs, }; - #[cfg(feature = "jack")] + #[cfg(all(feature = "jack", not(feature = "linux-sndio")))] impl_platform_host!(Jack jack "JACK", Alsa alsa "ALSA"); - #[cfg(not(feature = "jack"))] + #[cfg(all(not(feature = "jack"), not(feature = "linux-sndio")))] impl_platform_host!(Alsa alsa "ALSA"); /// The default host for the current compilation target platform. + #[cfg(not(feature = "linux-sndio"))] pub fn default_host() -> Host { AlsaHost::new() .expect("the default host should always be available") .into() } + + #[cfg(feature = "linux-sndio")] + pub use crate::host::sndio::{ + Device as SndioDevice, Devices as SndioDevices, Host as SndioHost, Stream as SndioStream, + SupportedInputConfigs as SndioSupportedInputConfigs, + SupportedOutputConfigs as SndioSupportedOutputConfigs, + }; + + #[cfg(feature = "linux-sndio")] + impl_platform_host!(Sndio sndio "sndio"); + + /// The default host for the current compilation target platform. + #[cfg(feature = "linux-sndio")] + pub fn default_host() -> Host { + SndioHost::new() + .expect("the default host should always be available") + .into() + } } #[cfg(target_os = "openbsd")] From 842069b4f92701f182e447dba8564195c07d46ec Mon Sep 17 00:00:00 2001 From: Greg Steuck Date: Thu, 28 Jan 2021 21:37:34 -0800 Subject: [PATCH 12/17] Wrap (usize, HashMap) into a struct --- src/host/sndio/mod.rs | 64 +++++++++++++++++++++++++--------------- src/host/sndio/runner.rs | 6 ++-- 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index 671a0374a..6cc8ea8ff 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -282,6 +282,32 @@ impl<'a> Iterator for SampleRateMapIter<'a> { } } +struct CallbackPile { + index : usize, + store : HashMap, +} + +impl CallbackPile { + fn new() -> Self { + Self { index: 0, store: HashMap::new() } + } + + fn remove_at(&mut self, index : usize) -> T { + self.store.remove(&index).unwrap() + } + + fn empty(&self) -> bool { + self.store.len() == 0 + } + + fn append(&mut self, t : T) -> usize { + let index = self.index; + self.store.insert(index, t); + self.index = index + 1; + index + } +} + /// The shared state between `Device` and `Stream`. Responsible for closing handle when dropped. /// Upon `Device` creation, this is in the `Init` state. Calling `.open` transitions /// this to the `Opened` state (this generally happens when getting input or output configs). @@ -318,11 +344,11 @@ enum InnerState { /// Each input Stream that has not been dropped has its callbacks in an element of this Vec. /// The last element is guaranteed to not be None. - input_callbacks: (usize, HashMap), + input_callbacks: CallbackPile, /// Each output Stream that has not been dropped has its callbacks in an element of this Vec. /// The last element is guaranteed to not be None. - output_callbacks: (usize, HashMap), + output_callbacks: CallbackPile, /// Whether the runner thread was spawned yet. thread_spawned: bool, @@ -441,14 +467,10 @@ impl InnerState { .. } => { // If there were previously no callbacks, wakeup the runner thread. - if input_callbacks.1.len() == 0 && output_callbacks.1.len() == 0 { - if let Some(ref sender) = wakeup_sender { - let _ = sender.send(()); - } + if input_callbacks.empty() && output_callbacks.empty() { + wakeup_sender.as_ref().map(|sender| sender.send(())); } - let index = output_callbacks.0; - output_callbacks.1.insert(index, callbacks); - output_callbacks.0 = index + 1; + let index = output_callbacks.append(callbacks); Ok(index) } _ => Err(backend_specific_error("device is not in a running state")), @@ -462,7 +484,7 @@ impl InnerState { InnerState::Running { ref mut output_callbacks, .. - } => Ok(output_callbacks.1.remove(&index).unwrap()), + } => Ok(output_callbacks.remove_at(index)), _ => Err(backend_specific_error("device is not in a running state")), } } @@ -478,14 +500,10 @@ impl InnerState { .. } => { // If there were previously no callbacks, wakeup the runner thread. - if input_callbacks.1.len() == 0 && output_callbacks.1.len() == 0 { - if let Some(ref sender) = wakeup_sender { - let _ = sender.send(()); - } + if input_callbacks.empty() && output_callbacks.empty() { + wakeup_sender.as_ref().map(|sender| sender.send(())); } - let index = input_callbacks.0; - input_callbacks.1.insert(index, callbacks); - input_callbacks.0 = index + 1; + let index = input_callbacks.append(callbacks); Ok(index) } _ => Err(backend_specific_error("device is not in a running state")), @@ -499,7 +517,7 @@ impl InnerState { InnerState::Running { ref mut input_callbacks, .. - } => Ok(input_callbacks.1.remove(&index).unwrap()), + } => Ok(input_callbacks.remove_at(index)), _ => Err(backend_specific_error("device is not in a running state")), } } @@ -513,10 +531,10 @@ impl InnerState { .. } => { let e = e.into(); - for cbs in input_callbacks.1.values_mut() { + for cbs in input_callbacks.store.values_mut() { (cbs.error_callback)(e.clone()); } - for cbs in output_callbacks.1.values_mut() { + for cbs in output_callbacks.store.values_mut() { (cbs.error_callback)(e.clone()); } } @@ -568,8 +586,8 @@ impl InnerState { buffer_size, par, sample_rate_map: tmp, - input_callbacks: (0, HashMap::new()), - output_callbacks: (0, HashMap::new()), + input_callbacks: CallbackPile::new(), + output_callbacks: CallbackPile::new(), thread_spawned: false, wakeup_sender: None, }; @@ -597,7 +615,7 @@ impl InnerState { ref input_callbacks, ref output_callbacks, .. - } => input_callbacks.1.len() > 0 || output_callbacks.1.len() > 0, + } => !input_callbacks.empty() || !output_callbacks.empty(), _ => false, } } diff --git a/src/host/sndio/runner.rs b/src/host/sndio/runner.rs index 233c71fa8..73b6202c2 100644 --- a/src/host/sndio/runner.rs +++ b/src/host/sndio/runner.rs @@ -261,7 +261,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { } => { if output_offset_bytes_into_buf == 0 { // The whole output buffer has been written (or this is the first time). Fill it. - if output_callbacks.1.len() == 0 { + if output_callbacks.empty() { if clear_output_buf_needed { // There is probably nonzero data in the buffer from previous output // Streams. Zero it out. @@ -271,7 +271,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { clear_output_buf_needed = false; } } else { - for cbs in output_callbacks.1.values_mut() { + for cbs in output_callbacks.store.values_mut() { // Really we shouldn't have more than one output callback as they are // stepping on each others' data. // TODO: perhaps we should not call these callbacks while holding the lock @@ -385,7 +385,7 @@ pub(super) fn runner(inner_state_arc: Arc>) { ref mut input_callbacks, .. } => { - for cbs in input_callbacks.1.values_mut() { + for cbs in input_callbacks.store.values_mut() { // TODO: perhaps we should not call these callbacks while holding the lock (cbs.data_callback)(&input_data, &input_callback_info); } From 96a744759766e7eadafcbb0542975f0bcad72768 Mon Sep 17 00:00:00 2001 From: Greg Steuck Date: Thu, 28 Jan 2021 22:44:13 -0800 Subject: [PATCH 13/17] Shrink input_adapter_callback by parametrizing by type --- src/host/sndio/adapters.rs | 70 ++++++++++++-------------------------- src/host/sndio/mod.rs | 37 ++++++++++++-------- 2 files changed, 43 insertions(+), 64 deletions(-) diff --git a/src/host/sndio/adapters.rs b/src/host/sndio/adapters.rs index 6089e46aa..3d33dcd23 100644 --- a/src/host/sndio/adapters.rs +++ b/src/host/sndio/adapters.rs @@ -1,67 +1,39 @@ use crate::{Data, FrameCount, InputCallbackInfo, OutputCallbackInfo, Sample, SampleFormat}; +use std::convert::TryFrom; /// When given an input data callback that expects samples in the specified sample format, return /// an input data callback that expects samples in the I16 sample format. The `buffer_size` is in /// samples. -pub(super) fn input_adapter_callback( +pub(super) fn input_adapter_callback( mut original_data_callback: D, buffer_size: FrameCount, sample_format: SampleFormat, ) -> Box where D: FnMut(&Data, &InputCallbackInfo) + Send + 'static, + T: TryFrom + Copy + Send + Default + 'static, { - match sample_format { - SampleFormat::I16 => { - // no-op - return Box::new(original_data_callback); - } - SampleFormat::F32 => { - // Make the backing buffer for the Data used in the closure. - let mut adapted_buf = vec![0f32].repeat(buffer_size as usize); - Box::new(move |data: &Data, info: &InputCallbackInfo| { - let data_slice: &[i16] = data.as_slice().unwrap(); // unwrap OK because data is always i16 - let adapted_slice = &mut adapted_buf; - assert_eq!(data_slice.len(), adapted_slice.len()); - for (i, adapted_ref) in adapted_slice.iter_mut().enumerate() { - *adapted_ref = data_slice[i].to_f32(); - } + let mut adapted_buf = vec![T::default()].repeat(buffer_size as usize); - // Note: we construct adapted_data here instead of in the parent function because adapted_buf needs - // to be owned by the closure. - let adapted_data = unsafe { - Data::from_parts( - adapted_buf.as_mut_ptr() as *mut _, - buffer_size as usize, // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! - sample_format, - ) - }; - original_data_callback(&adapted_data, info); - }) + Box::new(move |data: &Data, info: &InputCallbackInfo| { + let data_slice: &[i16] = data.as_slice().unwrap(); // unwrap OK because data is always i16 + let adapted_slice = &mut adapted_buf; + assert_eq!(data_slice.len(), adapted_slice.len()); + for (i, adapted_ref) in adapted_slice.iter_mut().enumerate() { + *adapted_ref = T::try_from(data_slice[i]).unwrap_or_default(); } - SampleFormat::U16 => { - let mut adapted_buf = vec![0u16].repeat(buffer_size as usize); - Box::new(move |data: &Data, info: &InputCallbackInfo| { - let data_slice: &[i16] = data.as_slice().unwrap(); // unwrap OK because data is always i16 - let adapted_slice = &mut adapted_buf; - assert_eq!(data_slice.len(), adapted_slice.len()); - for (i, adapted_ref) in adapted_slice.iter_mut().enumerate() { - *adapted_ref = data_slice[i].to_u16(); - } - // Note: we construct adapted_data here instead of in the parent function because adapted_buf needs - // to be owned by the closure. - let adapted_data = unsafe { - Data::from_parts( - adapted_buf.as_mut_ptr() as *mut _, - buffer_size as usize, // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! - sample_format, - ) - }; - original_data_callback(&adapted_data, info); - }) - } - } + // Note: we construct adapted_data here instead of in the parent function because adapted_buf needs + // to be owned by the closure. + let adapted_data = unsafe { + Data::from_parts( + adapted_buf.as_mut_ptr() as *mut _, + buffer_size as usize, // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! + sample_format, + ) + }; + original_data_callback(&adapted_data, info); + }) } /// When given an output data callback that expects a place to write samples in the specified diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index 6cc8ea8ff..192b2dcbe 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -283,28 +283,31 @@ impl<'a> Iterator for SampleRateMapIter<'a> { } struct CallbackPile { - index : usize, - store : HashMap, + index: usize, + store: HashMap, } impl CallbackPile { fn new() -> Self { - Self { index: 0, store: HashMap::new() } + Self { + index: 0, + store: HashMap::new(), + } } - fn remove_at(&mut self, index : usize) -> T { - self.store.remove(&index).unwrap() + fn remove_at(&mut self, index: usize) -> T { + self.store.remove(&index).unwrap() } fn empty(&self) -> bool { - self.store.len() == 0 + self.store.len() == 0 } - fn append(&mut self, t : T) -> usize { - let index = self.index; + fn append(&mut self, t: T) -> usize { + let index = self.index; self.store.insert(index, t); self.index = index + 1; - index + index } } @@ -470,7 +473,7 @@ impl InnerState { if input_callbacks.empty() && output_callbacks.empty() { wakeup_sender.as_ref().map(|sender| sender.send(())); } - let index = output_callbacks.append(callbacks); + let index = output_callbacks.append(callbacks); Ok(index) } _ => Err(backend_specific_error("device is not in a running state")), @@ -503,7 +506,7 @@ impl InnerState { if input_callbacks.empty() && output_callbacks.empty() { wakeup_sender.as_ref().map(|sender| sender.send(())); } - let index = input_callbacks.append(callbacks); + let index = input_callbacks.append(callbacks); Ok(index) } _ => Err(backend_specific_error("device is not in a running state")), @@ -875,10 +878,14 @@ impl DeviceTrait for Device { InnerState::Running { ref buffer_size, .. } => { - boxed_data_cb = if sample_format != SampleFormat::I16 { - input_adapter_callback(data_callback, *buffer_size, sample_format) - } else { - Box::new(data_callback) + boxed_data_cb = match sample_format { + SampleFormat::F32 => { + input_adapter_callback::(data_callback, *buffer_size, sample_format) + } + SampleFormat::U16 => { + input_adapter_callback::(data_callback, *buffer_size, sample_format) + } + SampleFormat::I16 => Box::new(data_callback), }; } } From aefb585a61c1d06d2ff2bf76b34119574dea8b4f Mon Sep 17 00:00:00 2001 From: Greg Steuck Date: Fri, 29 Jan 2021 21:07:29 -0800 Subject: [PATCH 14/17] Add TypeSampleFormat - type-level counterpart to SampleFormat --- src/samples_formats.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/samples_formats.rs b/src/samples_formats.rs index faa02319e..03466b75b 100644 --- a/src/samples_formats.rs +++ b/src/samples_formats.rs @@ -23,6 +23,23 @@ impl SampleFormat { } } +/// Returns the SampleFormat for the given standard type. +pub trait TypeSampleFormat { + fn sample_format() -> SampleFormat; +} + +impl TypeSampleFormat for f32 { + fn sample_format() -> SampleFormat { SampleFormat::F32 } +} + +impl TypeSampleFormat for u16 { + fn sample_format() -> SampleFormat { SampleFormat::U16 } +} + +impl TypeSampleFormat for i16 { + fn sample_format() -> SampleFormat { SampleFormat::I16 } +} + /// Trait for containers that contain PCM data. pub unsafe trait Sample: Copy + Clone { /// The `SampleFormat` corresponding to this data type. From f64997d9ef75505ce1f31dd4a1f0596848f7a9f8 Mon Sep 17 00:00:00 2001 From: Greg Steuck Date: Fri, 29 Jan 2021 21:08:50 -0800 Subject: [PATCH 15/17] Deduplicate output_adapter_callback and avoid indexing Avoids duplicating information with: * TypeSampleFormat to avoid redundant values * Sample trait for value conversion * Common data_from_vec fn Indexing typically means a runtime check whereas iterators don't. --- src/host/sndio/adapters.rs | 102 ++++++++++++------------------------- src/host/sndio/mod.rs | 16 +++--- 2 files changed, 42 insertions(+), 76 deletions(-) diff --git a/src/host/sndio/adapters.rs b/src/host/sndio/adapters.rs index 3d33dcd23..e999e1035 100644 --- a/src/host/sndio/adapters.rs +++ b/src/host/sndio/adapters.rs @@ -1,5 +1,5 @@ -use crate::{Data, FrameCount, InputCallbackInfo, OutputCallbackInfo, Sample, SampleFormat}; -use std::convert::TryFrom; +use crate::{Data, FrameCount, InputCallbackInfo, OutputCallbackInfo, Sample}; +use samples_formats::TypeSampleFormat; /// When given an input data callback that expects samples in the specified sample format, return /// an input data callback that expects samples in the I16 sample format. The `buffer_size` is in @@ -7,31 +7,24 @@ use std::convert::TryFrom; pub(super) fn input_adapter_callback( mut original_data_callback: D, buffer_size: FrameCount, - sample_format: SampleFormat, ) -> Box where D: FnMut(&Data, &InputCallbackInfo) + Send + 'static, - T: TryFrom + Copy + Send + Default + 'static, + T: Sample + TypeSampleFormat + Copy + Send + Default + 'static, { - let mut adapted_buf = vec![T::default()].repeat(buffer_size as usize); + let mut adapted_buf = vec![T::default(); buffer_size as usize]; Box::new(move |data: &Data, info: &InputCallbackInfo| { let data_slice: &[i16] = data.as_slice().unwrap(); // unwrap OK because data is always i16 let adapted_slice = &mut adapted_buf; assert_eq!(data_slice.len(), adapted_slice.len()); - for (i, adapted_ref) in adapted_slice.iter_mut().enumerate() { - *adapted_ref = T::try_from(data_slice[i]).unwrap_or_default(); + for (adapted_ref, data_element) in adapted_slice.iter_mut().zip(data_slice.iter()) { + *adapted_ref = T::from(data_element); } // Note: we construct adapted_data here instead of in the parent function because adapted_buf needs // to be owned by the closure. - let adapted_data = unsafe { - Data::from_parts( - adapted_buf.as_mut_ptr() as *mut _, - buffer_size as usize, // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! - sample_format, - ) - }; + let adapted_data = unsafe { data_from_vec(&mut adapted_buf) }; original_data_callback(&adapted_data, info); }) } @@ -39,70 +32,39 @@ where /// When given an output data callback that expects a place to write samples in the specified /// sample format, return an output data callback that expects a place to write samples in the I16 /// sample format. The `buffer_size` is in samples. -pub(super) fn output_adapter_callback( +pub(super) fn output_adapter_callback( mut original_data_callback: D, buffer_size: FrameCount, - sample_format: SampleFormat, ) -> Box where D: FnMut(&mut Data, &OutputCallbackInfo) + Send + 'static, + T: Sample + TypeSampleFormat + Copy + Send + Default + 'static, { - match sample_format { - SampleFormat::I16 => { - // no-op - return Box::new(original_data_callback); - } - SampleFormat::F32 => { - // Make the backing buffer for the Data used in the closure. - let mut adapted_buf = vec![0f32].repeat(buffer_size as usize); - - Box::new(move |data: &mut Data, info: &OutputCallbackInfo| { - // Note: we construct adapted_data here instead of in the parent function because - // adapted_buf needs to be owned by the closure. - let mut adapted_data = unsafe { - Data::from_parts( - adapted_buf.as_mut_ptr() as *mut _, - buffer_size as usize, // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! - sample_format, - ) - }; + let mut adapted_buf = vec![T::default(); buffer_size as usize]; + Box::new(move |data: &mut Data, info: &OutputCallbackInfo| { + // Note: we construct adapted_data here instead of in the parent function because + // adapted_buf needs to be owned by the closure. + let mut adapted_data = unsafe { data_from_vec(&mut adapted_buf) }; - // Populate adapted_buf / adapted_data. - original_data_callback(&mut adapted_data, info); + // Populate adapted_buf / adapted_data. + original_data_callback(&mut adapted_data, info); - let data_slice: &mut [i16] = data.as_slice_mut().unwrap(); // unwrap OK because data is always i16 - let adapted_slice = &adapted_buf; - assert_eq!(data_slice.len(), adapted_slice.len()); - for (i, data_ref) in data_slice.iter_mut().enumerate() { - *data_ref = adapted_slice[i].to_i16(); - } - }) + let data_slice: &mut [i16] = data.as_slice_mut().unwrap(); // unwrap OK because data is always i16 + let adapted_slice = &adapted_buf; + assert_eq!(data_slice.len(), adapted_slice.len()); + for (data_ref, adapted_element) in data_slice.iter_mut().zip(adapted_slice.iter()) { + *data_ref = adapted_element.to_i16(); } - SampleFormat::U16 => { - // Make the backing buffer for the Data used in the closure. - let mut adapted_buf = vec![0u16].repeat(buffer_size as usize); - - Box::new(move |data: &mut Data, info: &OutputCallbackInfo| { - // Note: we construct adapted_data here instead of in the parent function because - // adapted_buf needs to be owned by the closure. - let mut adapted_data = unsafe { - Data::from_parts( - adapted_buf.as_mut_ptr() as *mut _, - buffer_size as usize, // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! - sample_format, - ) - }; - - // Populate adapted_buf / adapted_data. - original_data_callback(&mut adapted_data, info); + }) +} - let data_slice: &mut [i16] = data.as_slice_mut().unwrap(); // unwrap OK because data is always i16 - let adapted_slice = &adapted_buf; - assert_eq!(data_slice.len(), adapted_slice.len()); - for (i, data_ref) in data_slice.iter_mut().enumerate() { - *data_ref = adapted_slice[i].to_i16(); - } - }) - } - } +unsafe fn data_from_vec(adapted_buf: &mut Vec) -> Data +where + T: TypeSampleFormat, +{ + Data::from_parts( + adapted_buf.as_mut_ptr() as *mut _, + adapted_buf.len(), // TODO: this is converting a FrameCount to a number of samples; invalid for stereo! + T::sample_format(), + ) } diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index 192b2dcbe..006a89328 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -880,10 +880,10 @@ impl DeviceTrait for Device { } => { boxed_data_cb = match sample_format { SampleFormat::F32 => { - input_adapter_callback::(data_callback, *buffer_size, sample_format) + input_adapter_callback::(data_callback, *buffer_size) } SampleFormat::U16 => { - input_adapter_callback::(data_callback, *buffer_size, sample_format) + input_adapter_callback::(data_callback, *buffer_size) } SampleFormat::I16 => Box::new(data_callback), }; @@ -945,10 +945,14 @@ impl DeviceTrait for Device { InnerState::Running { ref buffer_size, .. } => { - boxed_data_cb = if sample_format != SampleFormat::I16 { - output_adapter_callback(data_callback, *buffer_size, sample_format) - } else { - Box::new(data_callback) + boxed_data_cb = match sample_format { + SampleFormat::F32 => { + output_adapter_callback::(data_callback, *buffer_size) + } + SampleFormat::U16 => { + output_adapter_callback::(data_callback, *buffer_size) + } + SampleFormat::I16 => Box::new(data_callback), }; } } From 2a068cc4742886ec3bdcb2176ec9e4129a991665 Mon Sep 17 00:00:00 2001 From: Greg Steuck Date: Sat, 30 Jan 2021 10:22:06 -0800 Subject: [PATCH 16/17] Deduplicate config_ranges --- src/host/sndio/mod.rs | 62 +++++++++++++------------------------------ 1 file changed, 19 insertions(+), 43 deletions(-) diff --git a/src/host/sndio/mod.rs b/src/host/sndio/mod.rs index 006a89328..8ad32c054 100644 --- a/src/host/sndio/mod.rs +++ b/src/host/sndio/mod.rs @@ -6,7 +6,6 @@ mod runner; use self::adapters::{input_adapter_callback, output_adapter_callback}; use self::runner::runner; -use std::collections::hash_map; use std::collections::HashMap; use std::convert::From; use std::mem::{self, MaybeUninit}; @@ -261,24 +260,23 @@ impl SampleRateMap { self.0.insert(rate.0, par) } - fn iter(&self) -> SampleRateMapIter<'_> { - SampleRateMapIter { - iter: self.0.iter(), - } - } -} - -struct SampleRateMapIter<'a> { - iter: hash_map::Iter<'a, u32, sndio_sys::sio_par>, -} - -impl<'a> Iterator for SampleRateMapIter<'a> { - type Item = (SampleRate, &'a sndio_sys::sio_par); - - fn next(&mut self) -> Option { - self.iter - .next() - .map(|(sample_rate, par)| (SampleRate(*sample_rate), par)) + fn config_ranges(&self, num_channels: F) -> Vec + where + F: Fn(&sndio_sys::sio_par) -> u32, + { + self.0 + .values() + .map(|par| { + let config = supported_config_from_par(par, num_channels(par)); + SupportedStreamConfigRange { + channels: config.channels, + min_sample_rate: config.sample_rate, + max_sample_rate: config.sample_rate, + buffer_size: config.buffer_size, + sample_format: config.sample_format, + } + }) + .collect() } } @@ -709,18 +707,7 @@ impl DeviceTrait for Device { ref sample_rate_map, .. } => { - let mut config_ranges = vec![]; - for (_, par) in sample_rate_map.iter() { - let config = supported_config_from_par(par, par.rchan); - config_ranges.push(SupportedStreamConfigRange { - channels: config.channels, - min_sample_rate: config.sample_rate, - max_sample_rate: config.sample_rate, - buffer_size: config.buffer_size, - sample_format: config.sample_format, - }); - } - + let config_ranges = sample_rate_map.config_ranges(|par| par.rchan); Ok(config_ranges.into_iter()) } _ => Err(backend_specific_error("device has not yet been opened").into()), @@ -754,18 +741,7 @@ impl DeviceTrait for Device { ref sample_rate_map, .. } => { - let mut config_ranges = vec![]; - for (_, par) in sample_rate_map.iter() { - let config = supported_config_from_par(par, par.pchan); - config_ranges.push(SupportedStreamConfigRange { - channels: config.channels, - min_sample_rate: config.sample_rate, - max_sample_rate: config.sample_rate, - buffer_size: config.buffer_size, - sample_format: config.sample_format, - }); - } - + let config_ranges = sample_rate_map.config_ranges(|par| par.pchan); Ok(config_ranges.into_iter()) } _ => Err(backend_specific_error("device has not yet been opened").into()), From ddd08e3bec2c5523522cfe597b8c955f2ffe0dce Mon Sep 17 00:00:00 2001 From: Aaron Miller Date: Mon, 1 Feb 2021 12:34:46 -0800 Subject: [PATCH 17/17] cargo fmt --- src/samples_formats.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/samples_formats.rs b/src/samples_formats.rs index 03466b75b..ebfbd7633 100644 --- a/src/samples_formats.rs +++ b/src/samples_formats.rs @@ -29,15 +29,21 @@ pub trait TypeSampleFormat { } impl TypeSampleFormat for f32 { - fn sample_format() -> SampleFormat { SampleFormat::F32 } + fn sample_format() -> SampleFormat { + SampleFormat::F32 + } } impl TypeSampleFormat for u16 { - fn sample_format() -> SampleFormat { SampleFormat::U16 } + fn sample_format() -> SampleFormat { + SampleFormat::U16 + } } impl TypeSampleFormat for i16 { - fn sample_format() -> SampleFormat { SampleFormat::I16 } + fn sample_format() -> SampleFormat { + SampleFormat::I16 + } } /// Trait for containers that contain PCM data.