diff --git a/crates/pf-client-core/src/audio_wasapi.rs b/crates/pf-client-core/src/audio_wasapi.rs index 2df9f3b8..2b01f22b 100644 --- a/crates/pf-client-core/src/audio_wasapi.rs +++ b/crates/pf-client-core/src/audio_wasapi.rs @@ -97,13 +97,43 @@ pub fn devices() -> Result<(Vec, Vec)> { /// Settings device pickers via session main), or the OS default. A picked device that's /// gone (unplugged USB DAC, remote session) falls back to the default with a warning — /// audio keeps working, like the PipeWire twin's `target.object` behavior. +/// Resolve an active endpoint by id WITHOUT `DeviceEnumerator::get_device`. +/// +/// That helper builds its argument as `PCWSTR::from_raw(HSTRING::from(id).as_ptr())` — the +/// `HSTRING` is a temporary, dropped at the end of that statement, so `GetDevice` reads freed +/// memory and misses ids that are perfectly valid. Scanning the active collection touches only +/// safe crate APIs, so it cannot regress the same way. (`punktfunk-host` fixes the same bug with +/// raw COM instead; this crate cannot, because it pins a different `windows` revision than +/// `wasapi` does, making the two `IMMDevice` types incompatible.) +pub(crate) fn device_by_id( + enumerator: &DeviceEnumerator, + direction: &Direction, + id: &str, +) -> Result { + let devices = enumerator + .get_device_collection(direction) + .map_err(|e| anyhow!("enumerate {direction:?} endpoints: {e}"))?; + let count = devices + .get_nbr_devices() + .map_err(|e| anyhow!("endpoint count: {e}"))?; + for i in 0..count { + let dev = devices + .get_device_at_index(i) + .map_err(|e| anyhow!("endpoint {i}: {e}"))?; + if dev.get_id().is_ok_and(|got| got == id) { + return Ok(dev); + } + } + anyhow::bail!("no active {direction:?} endpoint with id {id}") +} + fn pick_device( enumerator: &DeviceEnumerator, direction: &Direction, var: &str, ) -> Result { if let Some(id) = std::env::var(var).ok().filter(|v| !v.is_empty()) { - match enumerator.get_device(&id) { + match device_by_id(enumerator, direction, &id) { Ok(d) => { tracing::info!( var, diff --git a/crates/pf-client-core/src/pad_audio.rs b/crates/pf-client-core/src/pad_audio.rs index a5222b04..4f29c2e7 100644 --- a/crates/pf-client-core/src/pad_audio.rs +++ b/crates/pf-client-core/src/pad_audio.rs @@ -920,9 +920,11 @@ fn pad_render_thread( let res = (|| -> anyhow::Result<()> { const BLOCK_ALIGN: usize = PAD_CHANNELS * 4; // f32 interleaved let enumerator = wasapi::DeviceEnumerator::new().context("DeviceEnumerator")?; - let device = enumerator - .get_device(endpoint_id) - .map_err(|e| anyhow!("correlated endpoint not found: {e}"))?; + // Not `get_device`: that helper resolves through a freed string — see + // [`crate::audio_wasapi::device_by_id`]. + let device = + crate::audio_wasapi::device_by_id(&enumerator, &Direction::Render, endpoint_id) + .map_err(|e| anyhow!("correlated endpoint not found: {e:#}"))?; let mut audio_client = device.get_iaudioclient().context("IAudioClient")?; // FL|FR|BL|BR: front pair = the pad's speaker, back pair = the voice coils. let desired = WaveFormat::new(32, 32, &SampleType::Float, 48_000, PAD_CHANNELS, Some(0x33)); diff --git a/crates/punktfunk-host/src/audio/windows/audio_control.rs b/crates/punktfunk-host/src/audio/windows/audio_control.rs index a39ec594..9dd46fde 100644 --- a/crates/punktfunk-host/src/audio/windows/audio_control.rs +++ b/crates/punktfunk-host/src/audio/windows/audio_control.rs @@ -304,11 +304,13 @@ pub(crate) fn restore_default_playback() { } /// Open a device by endpoint id, with a name for error context. +/// +/// Resolves through [`super::pad_endpoint::open_wasapi_device`], NOT the `wasapi` crate's +/// `DeviceEnumerator::get_device` — that one hands `GetDevice` a freed string (see the helper's +/// docs), so it fails at random on ids that are perfectly valid. pub(crate) fn open_endpoint(ep: &Endpoint) -> Result { - wasapi::DeviceEnumerator::new() - .map_err(|e| anyhow!("DeviceEnumerator: {e}"))? - .get_device(&ep.1) - .map_err(|e| anyhow!("open endpoint {:?}: {e}", ep.0)) + super::pad_endpoint::open_wasapi_device(&ep.1) + .map_err(|e| anyhow!("open endpoint {:?}: {e:#}", ep.0)) } // --- IPolicyConfig (undocumented): set a default audio endpoint by id, for all three roles. --- diff --git a/crates/punktfunk-host/src/audio/windows/pad_endpoint.rs b/crates/punktfunk-host/src/audio/windows/pad_endpoint.rs index 374ac6ca..6269a089 100644 --- a/crates/punktfunk-host/src/audio/windows/pad_endpoint.rs +++ b/crates/punktfunk-host/src/audio/windows/pad_endpoint.rs @@ -95,6 +95,12 @@ const MMDEV_RENDER_PATH: &str = r"SOFTWARE\Microsoft\Windows\CurrentVersion\MMDe const ENDPOINT_ID_PREFIX: &str = "{0.0.0.00000000}."; /// How long [`ensure`] waits for the new render endpoint to materialise after driver install. const ENDPOINT_WAIT: Duration = Duration::from_secs(10); +/// How many times [`ensure`] re-stamps before giving up and asking for an AudioEndpointBuilder +/// kick (AEB reverts the format keys behind a fresh endpoint — see the loop in [`ensure`]). +const STAMP_ATTEMPTS: usize = 5; +/// How long to let AudioEndpointBuilder settle after a stamp BEFORE checking whether it held. +/// Checking immediately always reports success, including on the passes that get reverted. +const STAMP_SETTLE: Duration = Duration::from_millis(1200); /// One provisioned pad-audio endpoint. Persistent by design (endpoints survive host restarts); /// [`remove`] exists for tests + the `pad-endpoint remove` escape hatch only. @@ -844,7 +850,7 @@ fn open_mmdevice(endpoint_id: &str) -> Result { /// leave behind — a heisenbug whose failure mode is `0x80070002` (ERROR_FILE_NOT_FOUND) for an id /// that is perfectly valid. [`open_mmdevice`] keeps its wide buffer alive across the call, so /// resolve there and only borrow the crate's wrapper around the resulting interface. -fn open_wasapi_device(endpoint_id: &str) -> Result { +pub(crate) fn open_wasapi_device(endpoint_id: &str) -> Result { let dev = open_mmdevice(endpoint_id)?; wasapi::Device::from_immdevice(dev) .map_err(|e| anyhow!("wrap IMMDevice {endpoint_id} as a wasapi Device: {e}")) @@ -1162,8 +1168,35 @@ pub fn ensure(pad_index: u8) -> Result { wait_for_endpoint(&device_instance)? } }; - stamp_endpoint(&endpoint_id, pad_index) - .with_context(|| format!("stamp pad endpoint {endpoint_id}"))?; + // Stamp until it STAYS stamped. On a freshly created endpoint the write lands — a check run + // straight afterwards reports all seven served — and AudioEndpointBuilder then quietly reverts + // the three format keys behind us, leaving 4/7 for good. So the check has to happen after a + // settle, not immediately: verifying too early is exactly how this looked like "the stamps + // never took" for one debugging session and "the stamps took fine" for the next. + // + // Converging here matters beyond tidiness: `needs_aeb_kick` is what makes + // [`provision_at_startup`] restart AudioEndpointBuilder + Audiosrv, so latching the transient + // means bouncing the whole machine's audio stack on EVERY host start, forever, chasing stamps + // a re-pass would have landed. Once the endpoint has finished settling a re-stamp sticks + // permanently (measured stable over 30 s), and `stamp_endpoint` skips already-served keys, so + // the extra passes cost nothing once it has taken. + let mut served = false; + for attempt in 0..STAMP_ATTEMPTS { + stamp_endpoint(&endpoint_id, pad_index) + .with_context(|| format!("stamp pad endpoint {endpoint_id}"))?; + thread::sleep(STAMP_SETTLE); + served = all_served(&endpoint_id, pad_index); + if served { + if attempt > 0 { + tracing::debug!( + pad = pad_index, + attempt = attempt + 1, + "pad endpoint stamps held after a re-pass" + ); + } + break; + } + } // Default-device guard: a freshly registered render endpoint can grab the default. A pad // "speaker" as default playback would swallow ALL desktop audio — put the previous default // back via the IPolicyConfig machinery audio_control already owns. @@ -1181,7 +1214,6 @@ pub fn ensure(pad_index: u8) -> Result { "default playback moved to the new pad endpoint and no previous default is known"), } } - let served = all_served(&endpoint_id, pad_index); Ok(PadEndpoint { endpoint_id, device_instance, diff --git a/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs b/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs index 2dfcbfe2..db2d5374 100644 --- a/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs +++ b/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs @@ -349,7 +349,7 @@ fn capture_once( if assert_plan { if let Some(d) = seen_default.as_deref() { if d != dev_id { - match judge_default(&en, &wiring, d) { + match judge_default(&wiring, d) { DefaultKind::Capturable(name) => { tracing::info!(default = %name, planned = %dev_name, "could not park the default playback on the planned endpoint — \ @@ -428,7 +428,7 @@ fn capture_once( ); return Ok(Next::Reopen(TargetMode::Follow)); } - return Ok(match judge_default(&en, &wiring, &nid) { + return Ok(match judge_default(&wiring, &nid) { DefaultKind::Capturable(name) => { tracing::info!(device = %name, "operator changed the output device mid-stream — following \ @@ -461,8 +461,11 @@ enum DefaultKind { Unknown, } -fn judge_default(en: &DeviceEnumerator, wiring: &wiring_plan::Wiring, id: &str) -> DefaultKind { - let Ok(dev) = en.get_device(id) else { +/// Resolves through [`super::pad_endpoint::open_wasapi_device`], NOT the `wasapi` crate's +/// `DeviceEnumerator::get_device` — that one hands `GetDevice` a freed string (see the helper's +/// docs), and a spurious miss here silently downgrades a capturable default to `Unknown`. +fn judge_default(wiring: &wiring_plan::Wiring, id: &str) -> DefaultKind { + let Ok(dev) = super::pad_endpoint::open_wasapi_device(id) else { return DefaultKind::Unknown; }; let name = dev.get_friendlyname().unwrap_or_default();