Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
19 changes: 18 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,21 @@ 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. 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.
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)


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

Expand All @@ -31,7 +41,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 +59,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

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
12 changes: 11 additions & 1 deletion src/host/wasapi/com.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use super::IoError;
use std::marker::PhantomData;

use windows::Win32::Foundation::RPC_E_CHANGED_MODE;
use windows::Win32::System::Com::{CoInitializeEx, CoUninitialize, COINIT_APARTMENTTHREADED};
use windows::Win32::System::Com::{
CoInitializeEx, CoTaskMemFree, CoUninitialize, COINIT_APARTMENTTHREADED,
};

thread_local!(static COM_INITIALIZED: ComInitialized = {
unsafe {
Expand Down Expand Up @@ -50,6 +52,14 @@ impl Drop for ComInitialized {
}
}

/// RAII for COM-originating strings that need to be freed with `CoTaskMemFree`.
pub struct ComString(pub windows::core::PWSTR);
impl Drop for ComString {
fn drop(&mut self) {
unsafe { CoTaskMemFree(Some(self.0.as_ptr() as *mut _)) }
}
}

/// Ensures that COM is initialized in this thread.
#[inline]
pub fn com_initialized() {
Expand Down
Loading
Loading