-
Notifications
You must be signed in to change notification settings - Fork 442
feat: stable Device::id() #1014
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
8cc6f25
9161bbb
845baac
bd9b071
60c3c26
2a93a70
fb1efd0
626b823
c9d6b05
6f8587f
24ef5ac
8b5b8d0
068e171
f9301b8
2972b52
99fc8b7
43bc3e0
6e93a1b
59f4a9f
b811266
eefc6ef
f40bb9d
492a98e
30645a5
44ec96b
01a240d
6eb9e7a
d516476
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 |
---|---|---|
|
@@ -6,16 +6,18 @@ use crate::host::coreaudio::macos::StreamInner; | |
use crate::traits::DeviceTrait; | ||
use crate::{ | ||
BackendSpecificError, BufferSize, BuildStreamError, ChannelCount, Data, | ||
DefaultStreamConfigError, DeviceNameError, InputCallbackInfo, OutputCallbackInfo, SampleFormat, | ||
SampleRate, StreamConfig, StreamError, SupportedBufferSize, SupportedStreamConfig, | ||
SupportedStreamConfigRange, SupportedStreamConfigsError, | ||
DefaultStreamConfigError, DeviceId, DeviceIdError, DeviceNameError, InputCallbackInfo, | ||
OutputCallbackInfo, SampleFormat, SampleRate, StreamConfig, StreamError, SupportedBufferSize, | ||
SupportedStreamConfig, SupportedStreamConfigRange, SupportedStreamConfigsError, | ||
}; | ||
use coreaudio::audio_unit::render_callback::{self, data}; | ||
use coreaudio::audio_unit::{AudioUnit, Element, Scope}; | ||
use objc2_audio_toolbox::{ | ||
kAudioOutputUnitProperty_CurrentDevice, kAudioOutputUnitProperty_EnableIO, | ||
kAudioUnitProperty_StreamFormat, | ||
}; | ||
use objc2_core_audio::kAudioDevicePropertyDeviceUID; | ||
use objc2_core_audio::kAudioObjectPropertyElementMain; | ||
use objc2_core_audio::{ | ||
kAudioDevicePropertyAvailableNominalSampleRates, kAudioDevicePropertyBufferFrameSize, | ||
kAudioDevicePropertyBufferFrameSizeRange, kAudioDevicePropertyDeviceIsAlive, | ||
|
@@ -29,6 +31,8 @@ use objc2_core_audio::{ | |
use objc2_core_audio_types::{ | ||
AudioBuffer, AudioBufferList, AudioStreamBasicDescription, AudioValueRange, | ||
}; | ||
use objc2_core_foundation::CFString; | ||
use objc2_core_foundation::Type; | ||
|
||
pub use super::enumerate::{ | ||
default_input_device, default_output_device, SupportedInputConfigs, SupportedOutputConfigs, | ||
|
@@ -44,6 +48,7 @@ use std::time::{Duration, Instant}; | |
|
||
use super::property_listener::AudioObjectPropertyListener; | ||
use coreaudio::audio_unit::macos_helpers::get_device_name; | ||
|
||
/// Attempt to set the device sample rate to the provided rate. | ||
/// Return an error if the requested sample rate is not supported by the device. | ||
fn set_sample_rate( | ||
|
@@ -301,6 +306,10 @@ impl DeviceTrait for Device { | |
Device::name(self) | ||
} | ||
|
||
fn id(&self) -> Result<DeviceId, DeviceIdError> { | ||
Device::id(self) | ||
} | ||
|
||
fn supported_input_configs( | ||
&self, | ||
) -> Result<Self::SupportedInputConfigs, SupportedStreamConfigsError> { | ||
|
@@ -395,6 +404,44 @@ impl Device { | |
}) | ||
} | ||
|
||
fn id(&self) -> Result<DeviceId, DeviceIdError> { | ||
let property_address = AudioObjectPropertyAddress { | ||
mSelector: kAudioDevicePropertyDeviceUID, | ||
mScope: kAudioObjectPropertyScopeGlobal, | ||
mElement: kAudioObjectPropertyElementMain, | ||
}; | ||
|
||
// CFString is retained by the audio object, use wrap_under_get_rule | ||
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. Quote from docs:
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. @xephyris it is opposite. Even if uid is retained by device it will create copy when you get it. |
||
let mut uid: *mut CFString = std::ptr::null_mut(); | ||
let data_size = size_of::<*mut CFString>() as u32; | ||
|
||
// SAFETY: AudioObjectGetPropertyData is documented to write a CFString pointer | ||
// for kAudioDevicePropertyDeviceUID. We check the status code before use. | ||
let status = unsafe { | ||
AudioObjectGetPropertyData( | ||
self.audio_device_id, | ||
NonNull::from(&property_address), | ||
0, | ||
null(), | ||
NonNull::from(&data_size), | ||
NonNull::from(&mut uid).cast(), | ||
) | ||
}; | ||
check_os_status(status)?; | ||
|
||
// SAFETY: We verified uid is non-null and the status was successful | ||
if !uid.is_null() { | ||
let uid_string = unsafe { CFString::wrap_under_get_rule(uid).to_string() }; | ||
Ok(DeviceId::CoreAudio(uid_string)) | ||
} else { | ||
Err(DeviceIdError::BackendSpecific { | ||
err: BackendSpecificError { | ||
description: "Device UID is null".to_string(), | ||
}, | ||
}) | ||
} | ||
} | ||
|
||
// Logic re-used between `supported_input_configs` and `supported_output_configs`. | ||
#[allow(clippy::cast_ptr_alignment)] | ||
fn supported_configs( | ||
|
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.
"default" differs from "Default Device" in
name
. Maybe use shared const?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.
I'm slightly unsure about this change, as in most conventions an id would not have spaces inside of it, so I feel like
"default"
does a better job of this.DeviceId
s andname
s also do not intermix so I don't believe this will be much of an issue either.Of course if the general consensus is that
"Default Device"
is better than"default"
, I would be happy to change it.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.
Good rationale, let’s keep it like it this then.
How do you feel this’ll be for ALSA? Spaces in device names (e.g. for USB devices) would not be impossible.
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.
I think if we are to maintain no spaces in the
id
, then we could maybe just replace all the spaces with-
dashes or_
underscores.From what I've seen and researched though, it seems like the
hw:CARD="CARD_ID",DEV="DEVICE_NUM"
style inself.pcm_id
cannot contain spaces, as they are not permitted in card ids. Do you have any examples of such a case with spaces, or does thealsa
implementation incpal
use card names not card ids?My research in this is very basic, so what I'm saying may not be completely accurate.