From ba8d354e93115b220370f716f2b6263b21c6cfc4 Mon Sep 17 00:00:00 2001 From: mitchmindtree Date: Fri, 21 Jun 2019 02:38:59 +0200 Subject: [PATCH] Improve handling of `BuildStreamError` throughout crate. --- src/alsa/mod.rs | 126 ++++++++++++++++++++++++++++--------------- src/coreaudio/mod.rs | 15 ++++-- src/lib.rs | 18 +++++-- src/wasapi/stream.rs | 74 +++++++++++++++++-------- 4 files changed, 160 insertions(+), 73 deletions(-) diff --git a/src/alsa/mod.rs b/src/alsa/mod.rs index 1c8006e..de5e691 100644 --- a/src/alsa/mod.rs +++ b/src/alsa/mod.rs @@ -690,29 +690,41 @@ impl EventLoop { ) { -16 /* determined empirically */ => return Err(BuildStreamError::DeviceNotAvailable), -22 => return Err(BuildStreamError::InvalidArgument), - e => if check_errors(e).is_err() { - return Err(BuildStreamError::Unknown); + e => if let Err(description) = check_errors(e) { + let err = BackendSpecificError { description }; + return Err(err.into()); } } let hw_params = HwParams::alloc(); - set_hw_params_from_format(capture_handle, &hw_params, format); + set_hw_params_from_format(capture_handle, &hw_params, format) + .map_err(|description| BackendSpecificError { description })?; let can_pause = alsa::snd_pcm_hw_params_can_pause(hw_params.0) == 1; - let (buffer_len, period_len) = set_sw_params_from_format(capture_handle, format); + let (buffer_len, period_len) = set_sw_params_from_format(capture_handle, format) + .map_err(|description| BackendSpecificError { description })?; - check_errors(alsa::snd_pcm_prepare(capture_handle)) - .expect("could not get playback handle"); + if let Err(desc) = check_errors(alsa::snd_pcm_prepare(capture_handle)) { + let description = format!("could not get capture handle: {}", desc); + let err = BackendSpecificError { description }; + return Err(err.into()); + } let num_descriptors = { let num_descriptors = alsa::snd_pcm_poll_descriptors_count(capture_handle); - debug_assert!(num_descriptors >= 1); + if num_descriptors == 0 { + let description = "poll descriptor count for capture stream was 0".to_string(); + let err = BackendSpecificError { description }; + return Err(err.into()); + } num_descriptors as usize }; let new_stream_id = StreamId(self.next_stream_id.fetch_add(1, Ordering::Relaxed)); - assert_ne!(new_stream_id.0, usize::max_value()); // check for overflows + if new_stream_id.0 == usize::max_value() { + return Err(BuildStreamError::StreamIdOverflow); + } let stream_inner = StreamInner { id: new_stream_id.clone(), @@ -728,8 +740,11 @@ impl EventLoop { buffer: None, }; - check_errors(alsa::snd_pcm_start(capture_handle)) - .expect("could not start capture stream"); + if let Err(desc) = check_errors(alsa::snd_pcm_start(capture_handle)) { + let description = format!("could not start capture stream: {}", desc); + let err = BackendSpecificError { description }; + return Err(err.into()); + } self.push_command(Command::NewStream(stream_inner)); Ok(new_stream_id) @@ -754,29 +769,41 @@ impl EventLoop { ) { -16 /* determined empirically */ => return Err(BuildStreamError::DeviceNotAvailable), -22 => return Err(BuildStreamError::InvalidArgument), - e => if check_errors(e).is_err() { - return Err(BuildStreamError::Unknown); + e => if let Err(description) = check_errors(e) { + let err = BackendSpecificError { description }; + return Err(err.into()) } } let hw_params = HwParams::alloc(); - set_hw_params_from_format(playback_handle, &hw_params, format); + set_hw_params_from_format(playback_handle, &hw_params, format) + .map_err(|description| BackendSpecificError { description })?; let can_pause = alsa::snd_pcm_hw_params_can_pause(hw_params.0) == 1; - let (buffer_len, period_len) = set_sw_params_from_format(playback_handle, format); + let (buffer_len, period_len) = set_sw_params_from_format(playback_handle, format) + .map_err(|description| BackendSpecificError { description })?; - check_errors(alsa::snd_pcm_prepare(playback_handle)) - .expect("could not get playback handle"); + if let Err(desc) = check_errors(alsa::snd_pcm_prepare(playback_handle)) { + let description = format!("could not get playback handle: {}", desc); + let err = BackendSpecificError { description }; + return Err(err.into()); + } let num_descriptors = { let num_descriptors = alsa::snd_pcm_poll_descriptors_count(playback_handle); - debug_assert!(num_descriptors >= 1); + if num_descriptors == 0 { + let description = "poll descriptor count for playback stream was 0".to_string(); + let err = BackendSpecificError { description }; + return Err(err.into()); + } num_descriptors as usize }; let new_stream_id = StreamId(self.next_stream_id.fetch_add(1, Ordering::Relaxed)); - assert_ne!(new_stream_id.0, usize::max_value()); // check for overflows + if new_stream_id.0 == usize::max_value() { + return Err(BuildStreamError::StreamIdOverflow); + } let stream_inner = StreamInner { id: new_stream_id.clone(), @@ -826,12 +853,12 @@ unsafe fn set_hw_params_from_format( format: &Format, ) -> Result<(), String> { if let Err(e) = check_errors(alsa::snd_pcm_hw_params_any(pcm_handle, hw_params.0)) { - return Err("Errors on pcm handle".to_string()); + return Err(format!("errors on pcm handle: {}", e)); } if let Err(e) = check_errors(alsa::snd_pcm_hw_params_set_access(pcm_handle, hw_params.0, alsa::SND_PCM_ACCESS_RW_INTERLEAVED)) { - return Err("Handle not acessible".to_string()); + return Err(format!("handle not acessible: {}", e)); } let data_type = if cfg!(target_endian = "big") { @@ -851,29 +878,34 @@ unsafe fn set_hw_params_from_format( if let Err(e) = check_errors(alsa::snd_pcm_hw_params_set_format(pcm_handle, hw_params.0, data_type)) { - return Err("Format could not be set".to_string()); + return Err(format!("format could not be set: {}", e)); } if let Err(e) = check_errors(alsa::snd_pcm_hw_params_set_rate(pcm_handle, hw_params.0, format.sample_rate.0 as libc::c_uint, 0)) { - return Err("Sample rate could not be set".to_string()); + return Err(format!("sample rate could not be set: {}", e)); } if let Err(e) = check_errors(alsa::snd_pcm_hw_params_set_channels(pcm_handle, hw_params.0, format.channels as libc::c_uint)) { - return Err("Channel count could not be set".to_string()); + return Err(format!("channel count could not be set: {}", e)); } + + // TODO: Review this. 200ms seems arbitrary... let mut max_buffer_size = format.sample_rate.0 as alsa::snd_pcm_uframes_t / format.channels as alsa::snd_pcm_uframes_t / 5; // 200ms of buffer - check_errors(alsa::snd_pcm_hw_params_set_buffer_size_max(pcm_handle, + if let Err(e) = check_errors(alsa::snd_pcm_hw_params_set_buffer_size_max(pcm_handle, hw_params.0, &mut max_buffer_size)) - .unwrap(); + { + return Err(format!("max buffer size could not be set: {}", e)); + } + if let Err(e) = check_errors(alsa::snd_pcm_hw_params(pcm_handle, hw_params.0)) { - return Err("Hardware params could not be set.".to_string()); + return Err(format!("hardware params could not be set: {}", e)); } Ok(()) @@ -882,34 +914,42 @@ unsafe fn set_hw_params_from_format( unsafe fn set_sw_params_from_format( pcm_handle: *mut alsa::snd_pcm_t, format: &Format, -) -> (usize, usize) +) -> Result<(usize, usize), String> { let mut sw_params = mem::uninitialized(); // TODO: RAII - check_errors(alsa::snd_pcm_sw_params_malloc(&mut sw_params)).unwrap(); - check_errors(alsa::snd_pcm_sw_params_current(pcm_handle, sw_params)).unwrap(); - check_errors(alsa::snd_pcm_sw_params_set_start_threshold(pcm_handle, - sw_params, - 0)) - .unwrap(); + if let Err(e) = check_errors(alsa::snd_pcm_sw_params_malloc(&mut sw_params)) { + return Err(format!("snd_pcm_sw_params_malloc failed: {}", e)); + } + if let Err(e) = check_errors(alsa::snd_pcm_sw_params_current(pcm_handle, sw_params)) { + return Err(format!("snd_pcm_sw_params_current failed: {}", e)); + } + if let Err(e) = check_errors(alsa::snd_pcm_sw_params_set_start_threshold(pcm_handle, sw_params, 0)) { + return Err(format!("snd_pcm_sw_params_set_start_threshold failed: {}", e)); + } let (buffer_len, period_len) = { let mut buffer = mem::uninitialized(); let mut period = mem::uninitialized(); - check_errors(alsa::snd_pcm_get_params(pcm_handle, &mut buffer, &mut period)) - .expect("could not initialize buffer"); - assert!(buffer != 0); - check_errors(alsa::snd_pcm_sw_params_set_avail_min(pcm_handle, - sw_params, - period)) - .unwrap(); + if let Err(e) = check_errors(alsa::snd_pcm_get_params(pcm_handle, &mut buffer, &mut period)) { + return Err(format!("failed to initialize buffer: {}", e)); + } + if buffer == 0 { + return Err(format!("initialization resulted in a null buffer")); + } + if let Err(e) = check_errors(alsa::snd_pcm_sw_params_set_avail_min(pcm_handle, sw_params, period)) { + return Err(format!("snd_pcm_sw_params_set_avail_min failed: {}", e)); + } let buffer = buffer as usize * format.channels as usize; let period = period as usize * format.channels as usize; (buffer, period) }; - check_errors(alsa::snd_pcm_sw_params(pcm_handle, sw_params)).unwrap(); + if let Err(e) = check_errors(alsa::snd_pcm_sw_params(pcm_handle, sw_params)) { + return Err(format!("snd_pcm_sw_params failed: {}", e)); + } + alsa::snd_pcm_sw_params_free(sw_params); - (buffer_len, period_len) + Ok((buffer_len, period_len)) } /// Wrapper around `hw_params`. @@ -945,8 +985,6 @@ impl Drop for StreamInner { #[inline] fn check_errors(err: libc::c_int) -> Result<(), String> { - use std::ffi; - if err < 0 { unsafe { let s = ffi::CStr::from_ptr(alsa::snd_strerror(err)) diff --git a/src/coreaudio/mod.rs b/src/coreaudio/mod.rs index feff675..a80d001 100644 --- a/src/coreaudio/mod.rs +++ b/src/coreaudio/mod.rs @@ -513,13 +513,16 @@ impl EventLoop { // If the requested sample rate is different to the device sample rate, update the device. if sample_rate as u32 != format.sample_rate.0 { - // In order to avoid breaking existing input streams we `panic!` if there is already an - // active input stream for this device with the actual sample rate. + // In order to avoid breaking existing input streams we return an error if there is + // already an active input stream for this device with the actual sample rate. for stream in &*self.streams.lock().unwrap() { if let Some(stream) = stream.as_ref() { if stream.device_id == device.audio_device_id { - panic!("cannot change device sample rate for stream as an existing stream \ - is already running at the current sample rate."); + let description = "cannot change device sample rate for stream as an \ + existing stream is already running at the current sample rate" + .into(); + let err = BackendSpecificError { description }; + return Err(err.into()); } } } @@ -618,7 +621,9 @@ impl EventLoop { let timer = ::std::time::Instant::now(); while sample_rate != reported_rate { if timer.elapsed() > ::std::time::Duration::from_secs(1) { - panic!("timeout waiting for sample rate update for device"); + let description = "timeout waiting for sample rate update for device".into(); + let err = BackendSpecificError { description }; + return Err(err.into()); } ::std::thread::sleep(::std::time::Duration::from_millis(5)); } diff --git a/src/lib.rs b/src/lib.rs index adeda83..6b1f6cd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -371,9 +371,15 @@ pub enum BuildStreamError { /// Trying to use capture capabilities on an output only format yields this. #[fail(display = "The requested device does not support this capability (invalid argument)")] InvalidArgument, - /// The C-Layer returned an error we don't know about - #[fail(display = "An unknown error in the Backend occured")] - Unknown, + /// Occurs if adding a new Stream ID would cause an integer overflow. + #[fail(display = "Adding a new stream ID would cause an overflow")] + StreamIdOverflow, + /// See the `BackendSpecificError` docs for more information about this error variant. + #[fail(display = "{}", err)] + BackendSpecific { + #[fail(cause)] + err: BackendSpecificError, + } } /// An iterator yielding all `Device`s currently available to the system. @@ -777,6 +783,12 @@ impl From for DefaultFormatError { } } +impl From for BuildStreamError { + fn from(err: BackendSpecificError) -> Self { + BuildStreamError::BackendSpecific { err } + } +} + // If a backend does not provide an API for retrieving supported formats, we query it with a bunch // of commonly used rates. This is always the case for wasapi and is sometimes the case for alsa. // diff --git a/src/wasapi/stream.rs b/src/wasapi/stream.rs index 3a140b5..ae13c26 100644 --- a/src/wasapi/stream.rs +++ b/src/wasapi/stream.rs @@ -20,6 +20,7 @@ use std::sync::mpsc::{channel, Sender, Receiver}; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; +use BackendSpecificError; use BuildStreamError; use Format; use SampleFormat; @@ -123,9 +124,14 @@ impl EventLoop { // Obtaining a `IAudioClient`. let audio_client = match device.build_audioclient() { + Ok(client) => client, Err(ref e) if e.raw_os_error() == Some(AUDCLNT_E_DEVICE_INVALIDATED) => return Err(BuildStreamError::DeviceNotAvailable), - e => e.unwrap(), + Err(e) => { + let description = format!("{}", e); + let err = BackendSpecificError { description }; + return Err(err.into()); + } }; // Computing the format and initializing the device. @@ -158,7 +164,9 @@ impl EventLoop { }, Err(e) => { (*audio_client).Release(); - panic!("{:?}", e); + let description = format!("{}", e); + let err = BackendSpecificError { description }; + return Err(err.into()); }, Ok(()) => (), }; @@ -179,7 +187,9 @@ impl EventLoop { }, Err(e) => { (*audio_client).Release(); - panic!("{:?}", e); + let description = format!("{}", e); + let err = BackendSpecificError { description }; + return Err(err.into()); }, Ok(()) => (), }; @@ -192,16 +202,17 @@ impl EventLoop { let event = synchapi::CreateEventA(ptr::null_mut(), 0, 0, ptr::null()); if event == ptr::null_mut() { (*audio_client).Release(); - panic!("Failed to create event"); + let description = format!("failed to create event"); + let err = BackendSpecificError { description }; + return Err(err.into()); } - match check_result((*audio_client).SetEventHandle(event)) { - Err(_) => { - (*audio_client).Release(); - panic!("Failed to call SetEventHandle") - }, - Ok(_) => (), - }; + if let Err(e) = check_result((*audio_client).SetEventHandle(event)) { + (*audio_client).Release(); + let description = format!("failed to call SetEventHandle: {}", e); + let err = BackendSpecificError { description }; + return Err(err.into()); + } event }; @@ -222,7 +233,9 @@ impl EventLoop { }, Err(e) => { (*audio_client).Release(); - panic!("{:?}", e); + let description = format!("failed to build capture client: {}", e); + let err = BackendSpecificError { description }; + return Err(err.into()); }, Ok(()) => (), }; @@ -231,7 +244,9 @@ impl EventLoop { }; let new_stream_id = StreamId(self.next_stream_id.fetch_add(1, Ordering::Relaxed)); - assert_ne!(new_stream_id.0, usize::max_value()); // check for overflows + if new_stream_id.0 == usize::max_value() { + return Err(BuildStreamError::StreamIdOverflow); + } // Once we built the `StreamInner`, we add a command that will be picked up by the // `run()` method and added to the `RunContext`. @@ -270,9 +285,14 @@ impl EventLoop { // Obtaining a `IAudioClient`. let audio_client = match device.build_audioclient() { + Ok(client) => client, Err(ref e) if e.raw_os_error() == Some(AUDCLNT_E_DEVICE_INVALIDATED) => return Err(BuildStreamError::DeviceNotAvailable), - e => e.unwrap(), + Err(e) => { + let description = format!("{}", e); + let err = BackendSpecificError { description }; + return Err(err.into()); + } }; // Computing the format and initializing the device. @@ -303,7 +323,9 @@ impl EventLoop { }, Err(e) => { (*audio_client).Release(); - panic!("{:?}", e); + let description = format!("{}", e); + let err = BackendSpecificError { description }; + return Err(err.into()); }, Ok(()) => (), }; @@ -316,13 +338,17 @@ impl EventLoop { let event = synchapi::CreateEventA(ptr::null_mut(), 0, 0, ptr::null()); if event == ptr::null_mut() { (*audio_client).Release(); - panic!("Failed to create event"); + let description = format!("failed to create event"); + let err = BackendSpecificError { description }; + return Err(err.into()); } match check_result((*audio_client).SetEventHandle(event)) { - Err(_) => { + Err(e) => { (*audio_client).Release(); - panic!("Failed to call SetEventHandle") + let description = format!("failed to call SetEventHandle: {}", e); + let err = BackendSpecificError { description }; + return Err(err.into()); }, Ok(_) => (), }; @@ -343,7 +369,9 @@ impl EventLoop { }, Err(e) => { (*audio_client).Release(); - panic!("{:?}", e); + let description = format!("failed to obtain buffer size: {}", e); + let err = BackendSpecificError { description }; + return Err(err.into()); }, Ok(()) => (), }; @@ -367,7 +395,9 @@ impl EventLoop { }, Err(e) => { (*audio_client).Release(); - panic!("{:?}", e); + let description = format!("failed to build render client: {}", e); + let err = BackendSpecificError { description }; + return Err(err.into()); }, Ok(()) => (), }; @@ -376,7 +406,9 @@ impl EventLoop { }; let new_stream_id = StreamId(self.next_stream_id.fetch_add(1, Ordering::Relaxed)); - assert_ne!(new_stream_id.0, usize::max_value()); // check for overflows + if new_stream_id.0 == usize::max_value() { + return Err(BuildStreamError::StreamIdOverflow); + } // Once we built the `StreamInner`, we add a command that will be picked up by the // `run()` method and added to the `RunContext`.