Skip to content
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .github/workflows/cpal.yml
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,6 @@ jobs:
fail-fast: false
matrix:
include:
- windows-version: "0.59.0"
- windows-version: "0.60.0"
- windows-version: "0.61.3"
# Skip 0.62.x since we already test current version in test-native
name: test-windows-v${{ matrix.windows-version }}
Expand Down Expand Up @@ -349,6 +347,12 @@ jobs:
# Use cargo update --precise to lock the windows crate to a specific version
cargo update --precise ${{ matrix.windows-version }} windows

# Remove and re-add `windows-core` to force Cargo to re-resolve it
# I'd prefer a more surgical approach, but things get complicated once multiple `windows-core`
# versions are in play
cargo rm --target='cfg(target_os = "windows")' windows-core
cargo add --target='cfg(target_os = "windows")' windows-core@*

# Verify the version was locked correctly
echo "Locked windows crate version:"
cargo tree | grep "windows v" || echo "Windows crate not found in dependency tree"
Expand Down
18 changes: 17 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,20 @@ edition = "2021"
rust-version = "1.70"

[features]
default = ["wasapi-virtual-default-devices"]

asio = [
"asio-sys",
"num-traits",
] # Only available on Windows. See README for setup instructions.

# Enable virtual default devices for WASAPI, so that audio will be
# automatically rerouted when the default input or output device is changed.
#
# Note that this only works on Windows 8 and above. It is turned on by default,
# but consider turning it off if you are supporting an older version of Windows.
wasapi-virtual-default-devices = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, a documentation suggestion:

# Enable virtual default devices for WASAPI. When enabled:
# - Audio automatically reroutes when the default device changes
# - Streams survive device changes (e.g., plugging in headphones)
# - Requires Windows 8 or later
#
# Disable this feature if supporting Windows 7 or earlier.

Second, I wonder if we should invert the feature and rename it to windows-legacy (disabled by default). I'm not so sure if "virtual default devices" clearly communicates its intention. What do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good shout on the docs, will do first.

I think inverting the feature would be more ergonomic, but the Cargo docs suggest that features should always be additive:

A consequence of this is that features should be additive. That is, enabling a feature should not disable functionality, and it should usually be safe to enable any combination of features. A feature should not introduce a SemVer-incompatible change.

I think it's probably fine - I don't see a way for this to cause a conflict with another package, as you'd want this to be controlled at the application level anyway - but I'm not sure. For now, I'll just action the doc change, but I'm open to the inversion if you think it's compatible with the norms of the ecosystem.

Feature name: Hmm, yeah, I'm not sold on it either; it was what first came to mind to cover the concept of activating the output with a virtual default device, but it's not the most self-descriptive thing. Maybe wasapi-default-device-autorouting or something? (I'm using wasapi because it looks like cpal supports ASIO as well, but I'm happy to change to windows if that sounds better)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can still argue it's additive: it enables legacy support.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, will action!


# Deprecated, the `oboe` backend has been removed
oboe-shared-stdcxx = []

Expand All @@ -31,7 +40,12 @@ clap = { version = "4.5", features = ["derive"] }
# versions when bumping to a new release, and only increase the minimum when absolutely necessary.
# When updating this, also update the "windows-version" matrix in the CI workflow.
[target.'cfg(target_os = "windows")'.dependencies]
windows = { version = ">=0.58, <=0.62", features = [
# The `implement` feature was removed in windows-0.61, which means that we can't
# use older versions of the `windows` crate without explicitly activating `implement`
# for them, which will cause problems for >=0.61.
#
# See <https://github.com/microsoft/windows-rs/pull/3333>.
windows = { version = ">=0.61, <=0.62", features = [
"Win32_Media_Audio",
"Win32_Foundation",
"Win32_Devices_Properties",
Expand All @@ -44,6 +58,8 @@ windows = { version = ">=0.58, <=0.62", features = [
"Win32_Media_Multimedia",
"Win32_UI_Shell_PropertiesSystem",
] }
# Explicitly depend on windows-core for use with the `windows::core::implement` macro.
windows-core = "*"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cargo publish will reject wildcard versions, please specify what we need.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... yes, that is a problem. I'll see what I can do; the problem I saw was that the CI would fail to resolve windows-core to the correct version, but that might be fine with the bodge I put in there. Will test.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this is a problem. cargo will not resolve windows-core to the same version of windows's windows-core if I match the version constraints or use a lax constraint like ^0.61.

I can see three options here:

  1. Restrict the version to a single known version (0.61 or 0.62)
  2. Ask windows-rs to add support for referencing windows::core, instead of windows_core, somehow: Cannot use implement macro microsoft/windows-rs#3568 (comment). Even if this happened, it would have to be another windows-rs release.
  3. Ask a Cargo wizard if there are any solutions for getting *-like behaviour in a way that can still be cargo publish-ed

I'll ask around regarding 3, and ask in that windows-rs issue for future support, but 1 may be necessary in the meantime :S

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for asking in windows-rs, how did that go?

The frequent API breakage of these windows crates sure is a problem when trying to support a ~12 month window of them. I'm not a Windows user myself, and wouldn't mind "bumping" the Minimum Supported Windows Version. But from what I gather from the community here, historical support is a desired thing to have.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But from what I gather from the community here, historical support is a desired thing to have.

Yeah especially with how Microsoft is treating their users on windows 11 we will want to support windows 10 as long as we can. Thank you soo much for your work in that regard philpax and roderick. I know its a pain to support te windows platform, but the impact is so high. Especially for gamedev (bevy).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

windows-rs doesn't have any immediate solutions; they're planning on refactoring their crate structure, but as far as I can tell, the result would have a similar problem. (It also wouldn't help us with our historical support of crate versions, even if they did make a change: we'd still have to bump to that version onwards)

Yeah especially with how Microsoft is treating their users on windows 11 we will want to support windows 10 as long as we can.

Hmm, I'm pretty sure newer versions of the windows crate support older Windows versions, it's mostly just that you're likely to have multiple versions of the crate without some mechanism to synchronise them (e.g. windows v0.58 and windows v0.62, etc)


Speaking of, I was thinking of another solution: catpuccin-egui uses individual features for each supported version of egui, so the user can control which version of egui is used by switching out the feature in use.

We could do something similar for both windows and windows-core, so that the user can select which version to target, defaulting to either the earliest or the latest supported by us (e.g. default on windows-0-62). I think this would also solve the issue with implement, so we could extend the range of support once more.

Of course, this comes with the downside that the user needs to opt into an older version of windows-as-used-by-cpal-as-maybe-used-by-rodio, but I suspect it's the only viable solution given the constraints at hand (outside of locking down to one windows version and potentially bloating users' builds). Thoughts? Do we know of any users who have strong feelings on the matter?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, wait, just realised that wouldn't work... the implement macro would still look for a global windows_core, bypassing any aliases we set up. What a headache 😭

audio_thread_priority = { version = "0.34.0", optional = true }
asio-sys = { version = "0.2", path = "asio-sys", optional = true }
num-traits = { version = "0.2.6", optional = true }
Expand Down
239 changes: 183 additions & 56 deletions src/host/wasapi/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,17 @@ struct IAudioClientWrapper(Audio::IAudioClient);
unsafe impl Send for IAudioClientWrapper {}
unsafe impl Sync for IAudioClientWrapper {}

#[derive(Debug, Clone)]
enum DeviceType {
DefaultOutput,
DefaultInput,
Specific(Audio::IMMDevice),
}

/// An opaque type that identifies an end point.
#[derive(Clone)]
pub struct Device {
device: Audio::IMMDevice,
device: DeviceType,
/// We cache an uninitialized `IAudioClient` so that we can call functions from it without
/// having to create/destroy audio clients all the time.
future_audio_client: Arc<Mutex<Option<IAudioClientWrapper>>>, // TODO: add NonZero around the ptr
Expand Down Expand Up @@ -266,15 +273,70 @@ unsafe fn format_from_waveformatex_ptr(
Some(format)
}

#[cfg(feature = "wasapi-virtual-default-devices")]
unsafe fn activate_audio_interface_sync(
deviceinterfacepath: windows::core::PWSTR,
) -> windows::core::Result<Audio::IAudioClient> {
use windows::core::IUnknown;

#[windows::core::implement(Audio::IActivateAudioInterfaceCompletionHandler)]
struct CompletionHandler(std::sync::mpsc::Sender<windows::core::Result<IUnknown>>);

fn retrieve_result(
operation: &Audio::IActivateAudioInterfaceAsyncOperation,
) -> windows::core::Result<IUnknown> {
let mut result = windows::core::HRESULT::default();
let mut interface: Option<IUnknown> = None;
unsafe {
operation.GetActivateResult(&mut result, &mut interface)?;
}
result.ok()?;
interface.ok_or_else(|| {
windows::core::Error::new(
Audio::AUDCLNT_E_DEVICE_INVALIDATED,
"audio interface could not be retrieved during activation",
)
})
}

impl Audio::IActivateAudioInterfaceCompletionHandler_Impl for CompletionHandler_Impl {
fn ActivateCompleted(
&self,
operation: windows::core::Ref<Audio::IActivateAudioInterfaceAsyncOperation>,
) -> windows::core::Result<()> {
let result = operation.ok().and_then(retrieve_result);
let _ = self.0.send(result);
Ok(())
}
}

let (sender, receiver) = std::sync::mpsc::channel();
let completion: Audio::IActivateAudioInterfaceCompletionHandler =
CompletionHandler(sender).into();
Audio::ActivateAudioInterfaceAsync(
deviceinterfacepath,
&Audio::IAudioClient::IID,
None,
&completion,
)?;
let result = receiver.recv_timeout(Duration::from_secs(2)).unwrap()?;
result.cast()
}

unsafe impl Send for Device {}
unsafe impl Sync for Device {}

impl Device {
pub fn name(&self) -> Result<String, DeviceNameError> {
let device = self
.immdevice()
.ok_or(DeviceNameError::from(BackendSpecificError {
description: "device not found while getting name".to_string(),
}))?;

unsafe {
// Open the device's property store.
let property_store = self
.device
let property_store = device
.OpenPropertyStore(STGM_READ)
.expect("could not open property store");

Expand Down Expand Up @@ -325,13 +387,43 @@ impl Device {
#[inline]
fn from_immdevice(device: Audio::IMMDevice) -> Self {
Device {
device,
device: DeviceType::Specific(device),
future_audio_client: Arc::new(Mutex::new(None)),
}
}

#[inline]
fn default_output() -> Self {
Device {
device: DeviceType::DefaultOutput,
future_audio_client: Arc::new(Mutex::new(None)),
}
}

#[inline]
fn default_input() -> Self {
Device {
device: DeviceType::DefaultInput,
future_audio_client: Arc::new(Mutex::new(None)),
}
}

pub fn immdevice(&self) -> &Audio::IMMDevice {
&self.device
pub fn immdevice(&self) -> Option<Audio::IMMDevice> {
match &self.device {
DeviceType::DefaultOutput => unsafe {
get_enumerator()
.0
.GetDefaultAudioEndpoint(Audio::eRender, Audio::eConsole)
.ok()
},
DeviceType::DefaultInput => unsafe {
get_enumerator()
.0
.GetDefaultAudioEndpoint(Audio::eCapture, Audio::eConsole)
.ok()
},
DeviceType::Specific(device) => Some(device.clone()),
}
}

/// Ensures that `future_audio_client` contains a `Some` and returns a locked mutex to it.
Expand All @@ -343,10 +435,43 @@ impl Device {
return Ok(lock);
}

// When using virtual default devices, we use `ActivateAudioInterfaceAsync` to get
// an `IAudioClient` for the default output or input device. When retrieved this way,
// streams will be automatically rerouted if the default device is changed.
//
// Otherwise, we use `Activate` to get an `IAudioClient` for the current device.

#[cfg(feature = "wasapi-virtual-default-devices")]
let audio_client: Audio::IAudioClient = unsafe {
// can fail if the device has been disconnected since we enumerated it, or if
// the device doesn't support playback for some reason
self.device.Activate(Com::CLSCTX_ALL, None)?
match &self.device {
DeviceType::DefaultOutput => {
let default_audio = Com::StringFromIID(&Audio::DEVINTERFACE_AUDIO_RENDER)?;
let result = activate_audio_interface_sync(default_audio);
Com::CoTaskMemFree(Some(default_audio.as_ptr() as _));
result?
}
DeviceType::DefaultInput => {
let default_audio = Com::StringFromIID(&Audio::DEVINTERFACE_AUDIO_CAPTURE)?;
let result = activate_audio_interface_sync(default_audio);
Com::CoTaskMemFree(Some(default_audio.as_ptr() as _));
result?
}
DeviceType::Specific(device) => {
// can fail if the device has been disconnected since we enumerated it, or if
// the device doesn't support playback for some reason
device.Activate(Com::CLSCTX_ALL, None)?
}
}
};

#[cfg(not(feature = "wasapi-virtual-default-devices"))]
let audio_client = unsafe {
self.immdevice()
.ok_or(windows::core::Error::new(
Audio::AUDCLNT_E_DEVICE_INVALIDATED,
"device not found while getting audio client",
))?
.Activate(Com::CLSCTX_ALL, None)?
};

*lock = Some(IAudioClientWrapper(audio_client));
Expand Down Expand Up @@ -515,8 +640,14 @@ impl Device {
}

pub(crate) fn data_flow(&self) -> Audio::EDataFlow {
let endpoint = Endpoint::from(self.device.clone());
endpoint.data_flow()
match &self.device {
DeviceType::DefaultOutput => Audio::eRender,
DeviceType::DefaultInput => Audio::eCapture,
DeviceType::Specific(device) => {
let endpoint = Endpoint::from(device.clone());
endpoint.data_flow()
}
}
}

pub fn default_input_config(&self) -> Result<SupportedStreamConfig, DefaultStreamConfigError> {
Expand Down Expand Up @@ -766,40 +897,47 @@ impl Device {
impl PartialEq for Device {
#[inline]
fn eq(&self, other: &Device) -> bool {
// Use case: In order to check whether the default device has changed
// the client code might need to compare the previous default device with the current one.
// The pointer comparison (`self.device == other.device`) don't work there,
// because the pointers are different even when the default device stays the same.
//
// In this code section we're trying to use the GetId method for the device comparison, cf.
// https://docs.microsoft.com/en-us/windows/desktop/api/mmdeviceapi/nf-mmdeviceapi-immdevice-getid
unsafe {
struct IdRAII(windows::core::PWSTR);
/// RAII for device IDs.
impl Drop for IdRAII {
fn drop(&mut self) {
unsafe { Com::CoTaskMemFree(Some(self.0 .0 as *mut _)) }
}
}
// GetId only fails with E_OUTOFMEMORY and if it does, we're probably dead already.
// Plus it won't do to change the device comparison logic unexpectedly.
let id1 = self.device.GetId().expect("cpal: GetId failure");
let id1 = IdRAII(id1);
let id2 = other.device.GetId().expect("cpal: GetId failure");
let id2 = IdRAII(id2);
// 16-bit null-terminated comparison.
let mut offset = 0;
loop {
let w1: u16 = *(id1.0).0.offset(offset);
let w2: u16 = *(id2.0).0.offset(offset);
if w1 == 0 && w2 == 0 {
return true;
}
if w1 != w2 {
return false;
match (&self.device, &other.device) {
(DeviceType::DefaultOutput, DeviceType::DefaultOutput) => true,
(DeviceType::DefaultInput, DeviceType::DefaultInput) => true,
(DeviceType::Specific(dev1), DeviceType::Specific(dev2)) => {
// Use case: In order to check whether the default device has changed
// the client code might need to compare the previous default device with the current one.
// The pointer comparison (`self.device == other.device`) don't work there,
// because the pointers are different even when the default device stays the same.
//
// In this code section we're trying to use the GetId method for the device comparison, cf.
// https://docs.microsoft.com/en-us/windows/desktop/api/mmdeviceapi/nf-mmdeviceapi-immdevice-getid
unsafe {
struct IdRAII(windows::core::PWSTR);
/// RAII for device IDs.
impl Drop for IdRAII {
fn drop(&mut self) {
unsafe { Com::CoTaskMemFree(Some(self.0 .0 as *mut _)) }
}
}
// GetId only fails with E_OUTOFMEMORY and if it does, we're probably dead already.
// Plus it won't do to change the device comparison logic unexpectedly.
let id1 = dev1.GetId().expect("cpal: GetId failure");
let id1 = IdRAII(id1);
let id2 = dev2.GetId().expect("cpal: GetId failure");
let id2 = IdRAII(id2);
// 16-bit null-terminated comparison.
let mut offset = 0;
loop {
let w1: u16 = *(id1.0).0.offset(offset);
let w2: u16 = *(id2.0).0.offset(offset);
if w1 == 0 && w2 == 0 {
return true;
}
if w1 != w2 {
return false;
}
offset += 1;
}
}
offset += 1;
}
_ => false,
}
}
}
Expand Down Expand Up @@ -911,23 +1049,12 @@ impl Iterator for Devices {
}
}

fn default_device(data_flow: Audio::EDataFlow) -> Option<Device> {
unsafe {
let device = get_enumerator()
.0
.GetDefaultAudioEndpoint(data_flow, Audio::eConsole)
.ok()?;
// TODO: check specifically for `E_NOTFOUND`, and panic otherwise
Some(Device::from_immdevice(device))
}
}

pub fn default_input_device() -> Option<Device> {
default_device(Audio::eCapture)
Some(Device::default_input())
}

pub fn default_output_device() -> Option<Device> {
default_device(Audio::eRender)
Some(Device::default_output())
}

/// Get the audio clock used to produce `StreamInstant`s.
Expand Down
Loading