-
Notifications
You must be signed in to change notification settings - Fork 537
Channel names: CoreAudio and ASIO #1254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
a18f2ba
c1db839
bb3a90e
4fac942
112515f
e7a2442
2fb7494
3a23e25
f79cf46
6bd1356
73567ec
c454a83
786b3ef
178d099
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ pub mod errors; | |
| #[cfg(target_os = "windows")] | ||
| use std::os::raw::c_long; | ||
| use std::{ | ||
| ffi::{CStr, CString}, | ||
| ffi::CString, | ||
| os::raw::{c_char, c_double, c_void}, | ||
| ptr::null_mut, | ||
| sync::{ | ||
|
|
@@ -981,6 +981,18 @@ impl Driver { | |
| drop(dcb); | ||
| drop(removed); | ||
| } | ||
|
|
||
| /// Returns the name of the channel at the given index. | ||
| /// | ||
| /// `channel` is a 0-based channel index. `is_input` selects the input (`true`) or output | ||
| /// (`false`) direction. | ||
| /// | ||
| /// The driver must already be loaded (i.e. this `Driver` instance must be alive). | ||
| pub fn channel_name(&self, channel: i32, is_input: bool) -> Result<String, AsioError> { | ||
| let _guard = self.inner.lock_state(); | ||
| let info = asio_channel_info(channel, is_input)?; | ||
| Ok(driver_name_to_utf8(&info.name).into_owned()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was already the case elsewhere with other names, but it's occurring to me that this will UB if there's ever a driver that doesn't NUL-terminate.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch |
||
| } | ||
| } | ||
|
|
||
| impl DriverState { | ||
|
|
@@ -1107,6 +1119,10 @@ fn asio_get_buffer_sizes() -> Result<BufferSizes, AsioError> { | |
| /// Retrieve the `ASIOChannelInfo` associated with the channel at the given index on either the | ||
| /// input or output stream (`true` for input). | ||
| fn asio_channel_info(channel: c_long, is_input: bool) -> Result<ai::ASIOChannelInfo, AsioError> { | ||
| if channel < 0 { | ||
| return Err(AsioError::InvalidInput); | ||
| } | ||
|
|
||
| let mut channel_info = ai::ASIOChannelInfo { | ||
| // Which channel we are querying | ||
| channel, | ||
|
|
@@ -1137,7 +1153,15 @@ fn stream_data_type(is_input: bool) -> Result<AsioSampleType, AsioError> { | |
| /// | ||
| /// This converts to utf8. | ||
| fn driver_name_to_utf8(bytes: &[c_char]) -> std::borrow::Cow<'_, str> { | ||
| unsafe { CStr::from_ptr(bytes.as_ptr()).to_string_lossy() } | ||
| let length = bytes | ||
| .iter() | ||
| .position(|&byte| byte == 0) | ||
| .unwrap_or(bytes.len()); | ||
| let bytes = bytes[..length] | ||
| .iter() | ||
| .map(|&byte| byte as u8) | ||
| .collect::<Vec<_>>(); | ||
| String::from_utf8_lossy(&bytes).into_owned().into() | ||
| } | ||
|
|
||
| /// Convert an `ASIOTimeStamp` (high and low 32-bit halves) to a `u64` nanosecond value. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,8 @@ pub struct Device { | |
| input_sample_format: Option<SampleFormat>, | ||
| output_sample_format: Option<SampleFormat>, | ||
| supported_sample_rates: Box<[SampleRate]>, | ||
| input_channel_names: Box<[String]>, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CoreAudio doesn't seem to cache this. What's the preferred approach? Lazily like CoreAudio or caching it during enumeration here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I honestly prefer to do it lazily, my rationale is the following: the operation for CoreAudio is low-cost and can be done on demand. Since most users won't need it, I prefer for a lower-level crate to avoid such implicit operations. The operation for ASIO on the other hand is not low-cost because it requires loading the driver and potentially unloading the previous one. It's also risky if there's a callback running in another thread. That's why I find it reasonable to cache it there, but here, I would argue it's unnecessary. |
||
| output_channel_names: Box<[String]>, | ||
|
|
||
| pub(super) current_callback_flag: Arc<AtomicU32>, | ||
| } | ||
|
|
@@ -123,6 +125,26 @@ impl Device { | |
| } | ||
| configs | ||
| } | ||
|
|
||
| pub fn get_channel_name(&self, channel_index: u16, input: bool) -> Result<String, Error> { | ||
| let names = if input { | ||
| &self.input_channel_names | ||
| } else { | ||
| &self.output_channel_names | ||
| }; | ||
|
|
||
| names.get(channel_index as usize).cloned().ok_or_else(|| { | ||
| Error::with_message( | ||
| ErrorKind::InvalidInput, | ||
| format!( | ||
| "channel index {} is out of range (device has {} {} channels)", | ||
| channel_index, | ||
| names.len(), | ||
| if input { "input" } else { "output" }, | ||
| ), | ||
| ) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| impl PartialEq for Device { | ||
|
|
@@ -185,6 +207,12 @@ impl Iterator for Devices { | |
| if channels.ins == 0 && channels.outs == 0 { | ||
| continue; | ||
| } | ||
| let Ok(channels_in) = ChannelCount::try_from(channels.ins) else { | ||
| continue; | ||
| }; | ||
| let Ok(channels_out) = ChannelCount::try_from(channels.outs) else { | ||
| continue; | ||
| }; | ||
|
|
||
| // Some drivers (e.g. Realtek ASIO) return 0 for sample_rate() until a | ||
| // stream is active. Treat 0 as "not yet known" rather than skipping. | ||
|
|
@@ -209,18 +237,39 @@ impl Iterator for Devices { | |
| .filter(|&r| driver.can_sample_rate(r.into()).unwrap_or(false)) | ||
| .collect(); | ||
|
|
||
| let input_channel_names: Box<[String]> = (0..channels_in) | ||
| .map(|ch| { | ||
| driver | ||
| .channel_name(ch.into(), true) | ||
| .ok() | ||
| .filter(|name| !name.is_empty()) | ||
| .unwrap_or_else(|| format!("Input {ch}")) | ||
| }) | ||
| .collect(); | ||
| let output_channel_names: Box<[String]> = (0..channels_out) | ||
| .map(|ch| { | ||
| driver | ||
| .channel_name(ch.into(), false) | ||
| .ok() | ||
| .filter(|name| !name.is_empty()) | ||
| .unwrap_or_else(|| format!("Output {ch}")) | ||
| }) | ||
| .collect(); | ||
|
|
||
| self.current_driver = Some(driver); | ||
|
|
||
| return Some(Device { | ||
| name, | ||
| channels_in: channels.ins as ChannelCount, | ||
| channels_out: channels.outs as ChannelCount, | ||
| channels_in, | ||
| channels_out, | ||
| sample_rate: sample_rate as SampleRate, | ||
| buffer_size_min: buffer_size_range.min as FrameCount, | ||
| buffer_size_max: buffer_size_range.max as FrameCount, | ||
| input_sample_format, | ||
| output_sample_format, | ||
| supported_sample_rates, | ||
| input_channel_names, | ||
| output_channel_names, | ||
| // Initialize with sentinel value so it never matches global flag state (0 or 1). | ||
| current_callback_flag: Arc::new(AtomicU32::new(u32::MAX)), | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about the
input: booloris_input: boolarguments in public functions. In the rest of cpal, that's split betweensupports_input/output,default_input/output_config, etc. That's more readable thanchannel_name(1, true)- what argument isn't self-explanatory.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
before rewriting this completely, what do you think could be a better API? Maybe
input_channel_name(1)andoutput_channel_name(1)?