forked from unom/punktfunk
fix(pad-audio): close the twelve findings the sweep left open on this branch
Everything the 2026-08-03 haptics sweep filed against the pad-audio branch (P2 + P3). Four of them are the difference between a feature that works and one that fails silently. **B6 — nothing ever un-muted the coils.** Every rumble report asserts `HAPTICS_SELECT`, which is SDL's "disable audio haptics" bit: the firmware mutes the very voice coils the 0xD1 stream drives. No code anywhere cleared it again, so ONE rumble left tier-A haptics silent for the rest of that pad's life — no error, nothing in a log, and the host happily streaming into a muted actuator. `DsDevice.ds5AudioHapticsReport` is the documented undo (flag0 with both bits clear); written EP0-direct when the stream starts and again after a rumble stop while a stream is live, because the stop report re-mutes on its way past. **B10 — the desktop mix could reach a controller's coils.** Pad endpoints were filtered out inside `plan()` only. The watchdog, Follow mode and the parked default all go through `judge_default`, which classifies by NAME — and a pad endpoint is deliberately stamped "DualSense Wireless Controller" so games treat it as the pad's speaker. No name rule could ever catch one. It now refuses them by identity. **B27 — an out-of-range pad aliased onto a real slot.** The 0xCD plane's pad is the only u16 index and every consumer narrowed it with `as u8` on an assumption nothing enforced, so wire pad 256 steered pad 0's speaker volumes. Rejected at the decoder, which makes the narrowings lossless by construction. An existing test had pinned the bug in place, asserting that wire pad 513 round-trips; corrected, plus a test for the 256→0 alias specifically. **B7 — caps that arrived late were never announced.** The renderer commits the tier-A trade only once its sink opens, which is well past the arrival burst's two 100 ms ticks, and `set_pad_audio_caps` only stored an atomic. The client believed it had pad audio while the host emitted nothing. The input task now compares the live registry against what the last arrival actually carried and re-arms the burst itself — no new plumbing, and no extra traffic when nothing changed. The rest: `needs_aeb_kick` is finally ACTED on (R4) — a stored-but-not-served endpoint is declined rather than opened, because `AUTOCONVERTPCM` makes it succeed and mis-route; a failed provisioning no longer latches `PROVISIONED` for the process lifetime (R5), and `host_cap` retries, so a host that started while the audio stack was busy recovers at the next connect instead of the next reboot; the loopback init timeout reaps its thread instead of detaching one per ~2 s reopen (R6); kind-change restarts are bounded (R3) since the trigger is a client-sent arrival; the devtest uses the endpoint's real channel mask (B11) instead of letting wasapi derive 0x0F against the endpoint's 0x33; the render loop asks `is_session_ended()` rather than spinning at nice -16 (R12); short writes are counted and reported instead of dropping the tail in silence (R13); and a frame addressed to another pad is dropped before it can seed the gap tracker from a foreign sequence space (R14). Verified: punktfunk-host clippy -D warnings **0 on a real Windows box**; Linux/amd64 clippy 0 with **589 tests** (pf-client-core 114, pf-inject 101, punktfunk-client-android 20, punktfunk-core 345+1+8); Android :kit: tests + :app: compile green; fmt clean. Six punktfunk-host tests fail on that Windows box. FIVE fail identically on a tree with no pad-audio code at all (QUIC `Rejected(SetupFailed)` — the box's network environment); the sixth passes 3/3 in isolation and only failed under the parallel run, on a locally-bound ephemeral port. Neither is this change. Still owed: on-glass. This is a hardware feature and none of it has been on a real DualSense since the merge.
This commit is contained in:
@@ -230,10 +230,28 @@ class DsCapture(
|
||||
Log.i(TAG, "pad audio self-test → ${if (r > 0) "PASS ($r frames)" else "FAIL ($r)"}")
|
||||
}, "pf-pad-selftest").start()
|
||||
} else {
|
||||
// B6: hand the coils back before the first haptics frame. Any rumble earlier in this
|
||||
// session asserted HAPTICS_SELECT, which firmware-mutes them, and nothing else ever
|
||||
// clears it — so without this the stream renders into a muted actuator and looks for
|
||||
// all the world like the host is sending nothing.
|
||||
restoreAudioHaptics()
|
||||
hook.start(index, fd)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* B6: clear the rumble/haptics-select bits so the pad's voice coils answer the audio-haptics
|
||||
* path again. EP0-direct, like the other out-of-band writes here: this has to land even when
|
||||
* the interrupt-OUT queue is busy or draining, and it is idempotent.
|
||||
*/
|
||||
private fun restoreAudioHaptics() {
|
||||
val m = model ?: return
|
||||
if (m == DsDevice.Model.DUALSHOCK4) return // no voice coils, no audio-haptics path
|
||||
if (!usb.writeControl(DsDevice.ds5AudioHapticsReport(m))) {
|
||||
Log.w(TAG, "pad audio: could not hand the coils back to audio haptics")
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Stop the renderer, then close the connection whose descriptor it borrows — in that order.
|
||||
*
|
||||
@@ -349,6 +367,10 @@ class DsCapture(
|
||||
// write — as this used to — meant a discarded stop left the motors running with
|
||||
// nothing scheduled to try again; a USB pad holds its last level until told zero.
|
||||
if (sent) disarmBackstop() else armBackstop(STOP_RETRY_MS)
|
||||
// B6: the stop report just re-asserted HAPTICS_SELECT on its way past, so if a
|
||||
// haptics stream is live the coils it drives were muted by the very write that
|
||||
// silenced the motors. Give them back.
|
||||
if (sent && padAudioStarted) restoreAudioHaptics()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -276,6 +276,21 @@ object DsDevice {
|
||||
* the classic compat-vibration path AND `VIBRATION2` (firmware ≥ 2.24's full-range replot;
|
||||
* older firmware ignores the unknown flag2 bit) — the host parser accepts either.
|
||||
*/
|
||||
/**
|
||||
* B6: hand the voice coils back to the audio-haptics path.
|
||||
*
|
||||
* Every [ds5RumbleReport] asserts `HAPTICS_SELECT` (flag0 bit1), which is SDL's
|
||||
* "disable audio haptics" bit — the firmware mutes the coils the 0xD1 haptics stream drives.
|
||||
* Until now NOTHING ever cleared it again, so a single rumble anywhere in a session left tier-A
|
||||
* haptics silent for the rest of that pad's life, with no error and nothing in a log.
|
||||
*
|
||||
* The undo is a report whose flag0 has BOTH bits clear (SDL's own comment: "Leaving emulated
|
||||
* rumble bits off will restore audio haptics"). No other valid flag is set, so nothing else
|
||||
* about the pad's state is touched. Mirrors `Ds5Feedback::audio_haptics_packet` on the desktop
|
||||
* client, which is the same packet one transport over.
|
||||
*/
|
||||
fun ds5AudioHapticsReport(model: Model): ByteArray = newDs5(model)
|
||||
|
||||
fun ds5RumbleReport(model: Model, low: Int, high: Int): ByteArray = newDs5(model).also {
|
||||
it[1] = (DS5_FLAG0_COMPAT_VIBRATION or DS5_FLAG0_HAPTICS_SELECT).toByte()
|
||||
it[39] = DS5_FLAG2_VIBRATION2.toByte()
|
||||
|
||||
@@ -606,7 +606,7 @@ fn render(
|
||||
fn drain_until_stop(client: &NativeClient, stop: &AtomicBool) {
|
||||
while !stop.load(Ordering::Relaxed) {
|
||||
if client.next_pad_audio(Duration::from_millis(20)).is_none()
|
||||
&& stop.load(Ordering::Relaxed)
|
||||
&& (stop.load(Ordering::Relaxed) || client.is_session_ended())
|
||||
{
|
||||
return;
|
||||
}
|
||||
@@ -632,6 +632,10 @@ fn pump(
|
||||
let mut samples_in = 0u64;
|
||||
let mut peak = 0i32;
|
||||
let mut last_report = std::time::Instant::now();
|
||||
// R13: caller-side short-write accounting (distinct from `st.short_bytes`, which is a
|
||||
// URB-level statistic from inside the transport).
|
||||
let mut st_short = 0u64;
|
||||
let mut st_short_logged = std::time::Instant::now();
|
||||
let mut pcm: Vec<i16> = Vec::with_capacity(MAX_FRAME_SAMPLES * 2);
|
||||
let mut out: Vec<i16> = Vec::with_capacity(MAX_BUFFER_FRAMES * PAD_CHANNELS);
|
||||
|
||||
@@ -644,7 +648,7 @@ fn pump(
|
||||
let st = playback.stats();
|
||||
log::info!(
|
||||
"pad audio: {frames_in} frames in, {samples_in} samples, peak={peak}, \
|
||||
{} written, {} underruns, {} short",
|
||||
{} written, {} underruns, {} short, {st_short} dropped to back-pressure",
|
||||
playback.frames_written(),
|
||||
st.underruns,
|
||||
st.short_bytes
|
||||
@@ -654,9 +658,31 @@ fn pump(
|
||||
}
|
||||
|
||||
let Some(frame) = client.next_pad_audio(Duration::from_millis(10)) else {
|
||||
// R12: `next_pad_audio` collapses a DISCONNECTED channel into the same `None` as an
|
||||
// ordinary timeout, so this arm cannot tell "nothing arrived in 10 ms" from "the
|
||||
// session is gone and nothing will ever arrive again". Left to `continue`, a closed
|
||||
// session span this loop at nice -16 until the owner's stop flag caught up — roughly
|
||||
// a second of a real-time-priority thread doing nothing. Ask the connection directly.
|
||||
if client.is_session_ended() {
|
||||
log::debug!("pad audio: session ended, leaving the render loop");
|
||||
break;
|
||||
}
|
||||
continue;
|
||||
};
|
||||
|
||||
// R14: `PadAudioFrame` carries the wire pad it was addressed to, and this renderer serves
|
||||
// exactly one. A frame for another pad — a queue still holding the previous occupant's
|
||||
// when a slot is re-used, or a host bug — would otherwise be decoded here AND seed the
|
||||
// gap tracker from a foreign sequence space, which shows up as a burst of phantom
|
||||
// concealment rather than as anything obviously wrong.
|
||||
if frame.pad != pad {
|
||||
log::debug!(
|
||||
"pad audio: dropping frame for pad {} on pad {pad}",
|
||||
frame.pad
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
// The settings gate each kind independently: haptics off but speaker on is a legitimate
|
||||
// configuration, and the host may still be sending both.
|
||||
let wanted = match frame.kind {
|
||||
@@ -731,13 +757,35 @@ fn pump(
|
||||
// chunk is never padded with silence mid-stream.
|
||||
out.clear();
|
||||
if mixer.pop(&mut out) > 0 {
|
||||
if let Err(e) = playback.write_interleaved(&out) {
|
||||
if is_fatal(&e) {
|
||||
log::warn!("pad audio: stream lost: {e}");
|
||||
return;
|
||||
match playback.write_interleaved(&out) {
|
||||
// R13: a SHORT write is back-pressure, not success — the endpoint took `n` frames
|
||||
// and the rest is ours to deal with. Discarding the return value dropped the tail
|
||||
// with nothing said, so a stalled endpoint sounded like clipped audio with a clean
|
||||
// log. We cannot retry from here without unbounded buffering (the mixer's whole
|
||||
// point is to stay ahead of the device), so the tail is still dropped — but it is
|
||||
// now COUNTED and reported by the 1 s line, which is the difference between a
|
||||
// diagnosable stall and a mystery.
|
||||
Ok(n) if n < out.len() => {
|
||||
st_short += (out.len() - n) as u64;
|
||||
if st_short_logged.elapsed() >= Duration::from_secs(5) {
|
||||
log::warn!(
|
||||
"pad audio: endpoint short-wrote {} of {} samples ({st_short} total) \
|
||||
— the device is not keeping up",
|
||||
n,
|
||||
out.len()
|
||||
);
|
||||
st_short_logged = std::time::Instant::now();
|
||||
}
|
||||
}
|
||||
Ok(_) => {}
|
||||
Err(e) => {
|
||||
if is_fatal(&e) {
|
||||
log::warn!("pad audio: stream lost: {e}");
|
||||
return;
|
||||
}
|
||||
log::debug!("pad audio: write hiccup: {e}");
|
||||
mixer.discard();
|
||||
}
|
||||
log::debug!("pad audio: write hiccup: {e}");
|
||||
mixer.discard();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2196,7 +2196,8 @@ fn hidout_pad(h: &HidOutput) -> u8 {
|
||||
| HidOutput::Trigger { pad, .. }
|
||||
| HidOutput::TrackpadHaptic { pad, .. }
|
||||
| HidOutput::HidRaw { pad, .. } => *pad,
|
||||
// AudioCtl's pad is u16 on the wire; the index space is 0..MAX_PADS end to end.
|
||||
// AudioCtl's pad is the plane's only u16. `HidOutput::decode` rejects anything at or
|
||||
// above MAX_PADS (B27), so by the time one reaches here the narrowing is lossless.
|
||||
HidOutput::AudioCtl { pad, .. } => *pad as u8,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -334,21 +334,22 @@ mod tests {
|
||||
#[test]
|
||||
fn audio_ctl_dedups_by_value() {
|
||||
let mut d = HidoutDedup::default();
|
||||
let t = Instant::now();
|
||||
let audio = |flags, vol| HidOutput::AudioCtl {
|
||||
pad: 0,
|
||||
flags,
|
||||
raw: [vol, 0, 0, 0, 0, 0],
|
||||
};
|
||||
// Identical twice → exactly one emission.
|
||||
assert!(d.should_forward(&audio(0x17, 0x50)));
|
||||
assert!(!d.should_forward(&audio(0x17, 0x50)));
|
||||
assert!(d.should_forward(&audio(0x17, 0x50), t));
|
||||
assert!(!d.should_forward(&audio(0x17, 0x50), t));
|
||||
// Either half changing (flags, or the raw region) forwards again.
|
||||
assert!(d.should_forward(&audio(0x16, 0x50)));
|
||||
assert!(d.should_forward(&audio(0x16, 0x60)));
|
||||
assert!(d.should_forward(&audio(0x16, 0x50), t));
|
||||
assert!(d.should_forward(&audio(0x16, 0x60), t));
|
||||
// The other kinds' state is untouched by audio traffic.
|
||||
assert!(d.should_forward(&HidOutput::PlayerLeds { pad: 0, bits: 1 }));
|
||||
assert!(d.should_forward(&HidOutput::PlayerLeds { pad: 0, bits: 1 }, t));
|
||||
// `clear` (pad re-plug) re-arms the value dedup.
|
||||
d.clear();
|
||||
assert!(d.should_forward(&audio(0x16, 0x60)));
|
||||
assert!(d.should_forward(&audio(0x16, 0x60), t));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -770,8 +770,9 @@ impl PunktfunkHidOutput {
|
||||
HidOutput::HidRaw { .. } => return None,
|
||||
HidOutput::AudioCtl { pad, flags, raw } => {
|
||||
// Same packing idiom as TrackpadHaptic: `which` carries the flags byte,
|
||||
// `effect[0..6]` the raw audio region. The u16 wire pad narrows losslessly —
|
||||
// pads are 0..16 (`input::MAX_PADS`) end to end.
|
||||
// `effect[0..6]` the raw audio region. The u16 wire pad narrows losslessly
|
||||
// because `HidOutput::decode` refuses one at or above `input::MAX_PADS` (B27) —
|
||||
// it is enforced there, not merely assumed here.
|
||||
out.kind = PUNKTFUNK_HIDOUT_AUDIO_CTL;
|
||||
out.pad = *pad as u8;
|
||||
out.which = *flags;
|
||||
|
||||
@@ -48,12 +48,23 @@ pub(super) async fn run(
|
||||
// An arrival's outgoing flags word: the pad index, plus the pad's audio-render bits (8/9)
|
||||
// toward a HOST_CAP_PAD_AUDIO host. With no declared caps (or an older host) this is
|
||||
// byte-identical to the plain index — the pre-pad-audio wire.
|
||||
let arrival_flags = |idx: usize| -> u32 {
|
||||
let caps = if pad_audio {
|
||||
// B7: the caps a pad's LAST arrival actually carried. `set_pad_audio_caps` only stores into
|
||||
// the registry — it cannot reach this task — so a declaration that lands after the arrival
|
||||
// burst has drained (the renderer commits the trade only once its sink opens, which is well
|
||||
// past the two 100 ms ticks) used to never reach the host at all: the client believed it had
|
||||
// pad audio and the host emitted nothing on 0xD1, silently, forever. Comparing this against
|
||||
// the live registry on every tick re-arms the burst by itself, with no new plumbing and no
|
||||
// extra traffic when nothing changed.
|
||||
let mut arrival_caps_sent: [u8; MAX_PADS] = [0; MAX_PADS];
|
||||
let caps_now = |idx: usize| -> u8 {
|
||||
if pad_audio {
|
||||
pad_audio_caps[idx].load(Ordering::Relaxed)
|
||||
} else {
|
||||
0
|
||||
};
|
||||
}
|
||||
};
|
||||
let arrival_flags = |idx: usize| -> u32 {
|
||||
let caps = caps_now(idx);
|
||||
crate::input::encode_gamepad_arrival(idx as u8, caps)
|
||||
};
|
||||
let mut refresh = tokio::time::interval(Duration::from_millis(100));
|
||||
@@ -115,6 +126,7 @@ pub(super) async fn run(
|
||||
// burst so the host learns it before the pad's first frame even under loss.
|
||||
arrival[idx] = Some(ev.code as u8);
|
||||
arrival_owed[idx] = ARRIVAL_RESENDS;
|
||||
arrival_caps_sent[idx] = caps_now(idx);
|
||||
let arr = crate::input::InputEvent {
|
||||
flags: arrival_flags(idx),
|
||||
..ev
|
||||
@@ -127,11 +139,21 @@ pub(super) async fn run(
|
||||
}
|
||||
_ = refresh.tick() => {
|
||||
for idx in 0..MAX_PADS {
|
||||
// B7: caps declared after the burst drained — re-announce this pad's arrival.
|
||||
// Only for a pad that HAS an arrival (so it is a live, declared controller),
|
||||
// and only when the value actually moved, so a steady session sends nothing.
|
||||
if arrival[idx].is_some()
|
||||
&& arrival_owed[idx] == 0
|
||||
&& caps_now(idx) != arrival_caps_sent[idx]
|
||||
{
|
||||
arrival_owed[idx] = ARRIVAL_RESENDS;
|
||||
}
|
||||
// Re-send an owed kind declaration (independent of whether the pad has state
|
||||
// yet — it may be idle-but-connected). Idempotent on the host.
|
||||
if arrival_owed[idx] > 0 {
|
||||
if let Some(kind) = arrival[idx] {
|
||||
arrival_owed[idx] -= 1;
|
||||
arrival_caps_sent[idx] = caps_now(idx);
|
||||
let arr = crate::input::InputEvent {
|
||||
kind: InputKind::GamepadArrival,
|
||||
_pad: [0; 3],
|
||||
|
||||
@@ -560,11 +560,22 @@ impl HidOutput {
|
||||
// Bounded: at most HID_REPORT_MAX bytes are kept from the (attacker-sized) tail.
|
||||
data: b[4..b.len().min(4 + HID_REPORT_MAX)].to_vec(),
|
||||
}),
|
||||
HIDOUT_AUDIO_CTL if b.len() >= 11 => Some(HidOutput::AudioCtl {
|
||||
pad: u16::from_le_bytes([b[2], b[3]]),
|
||||
flags: b[4],
|
||||
raw: b[5..11].try_into().unwrap(),
|
||||
}),
|
||||
// B27: the pad is the only u16 index on this plane, and every consumer narrows it
|
||||
// with `as u8` on the stated assumption that pads are 0..MAX_PADS. Nothing enforced
|
||||
// that, so wire pad 256 silently ALIASED onto slot 0 — a malformed or hostile
|
||||
// datagram steering a real controller's speaker volumes. Rejected here, at the one
|
||||
// place the u16 exists, so the narrowings downstream are lossless by construction
|
||||
// (the same fix R10 applied to the rumble plane).
|
||||
HIDOUT_AUDIO_CTL
|
||||
if b.len() >= 11
|
||||
&& u16::from_le_bytes([b[2], b[3]]) < crate::input::MAX_PADS as u16 =>
|
||||
{
|
||||
Some(HidOutput::AudioCtl {
|
||||
pad: u16::from_le_bytes([b[2], b[3]]),
|
||||
flags: b[4],
|
||||
raw: b[5..11].try_into().unwrap(),
|
||||
})
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
@@ -1400,13 +1411,15 @@ mod tests {
|
||||
#[test]
|
||||
fn audio_ctl_wire_layout_and_truncation() {
|
||||
// The exact 11-byte layout: [0xCD][0x06][u16 pad LE][u8 flags][6 raw bytes].
|
||||
// The pad is deliberately a REPRESENTABLE one: this used to assert that 0x0201 (513)
|
||||
// round-tripped, which pinned B27's aliasing in place as if it were the contract.
|
||||
let a = HidOutput::AudioCtl {
|
||||
pad: 0x0201,
|
||||
pad: 0x000B,
|
||||
flags: 0x17,
|
||||
raw: [1, 2, 3, 4, 5, 6],
|
||||
};
|
||||
let d = a.encode();
|
||||
assert_eq!(d, [0xCD, 0x06, 0x01, 0x02, 0x17, 1, 2, 3, 4, 5, 6]);
|
||||
assert_eq!(d, [0xCD, 0x06, 0x0B, 0x00, 0x17, 1, 2, 3, 4, 5, 6]);
|
||||
assert_eq!(HidOutput::decode(&d), Some(a));
|
||||
// Truncated buffers are rejected outright (fixed length — never a partial read).
|
||||
for n in 2..d.len() {
|
||||
@@ -1437,6 +1450,49 @@ mod tests {
|
||||
assert!(decode_pad_audio_datagram(&hdr).unwrap().opus.is_empty());
|
||||
}
|
||||
|
||||
/// B27: the pad is the only u16 index on the 0xCD plane and every consumer narrows it with
|
||||
/// `as u8`. An out-of-range one used to alias onto a real slot instead of being refused —
|
||||
/// wire pad 256 steering pad 0's speaker volumes.
|
||||
#[test]
|
||||
fn audio_ctl_rejects_a_pad_outside_the_index_space() {
|
||||
let ok = HidOutput::AudioCtl {
|
||||
pad: (crate::input::MAX_PADS - 1) as u16,
|
||||
flags: 0x12,
|
||||
raw: [1, 2, 3, 4, 5, 6],
|
||||
};
|
||||
assert_eq!(
|
||||
HidOutput::decode(&ok.encode()),
|
||||
Some(ok),
|
||||
"the last valid pad must still decode"
|
||||
);
|
||||
|
||||
// Anything at or above MAX_PADS is refused outright, not truncated.
|
||||
for pad in [crate::input::MAX_PADS as u16, 256, u16::MAX] {
|
||||
let d = HidOutput::AudioCtl {
|
||||
pad,
|
||||
flags: 0x12,
|
||||
raw: [1, 2, 3, 4, 5, 6],
|
||||
}
|
||||
.encode();
|
||||
assert_eq!(HidOutput::decode(&d), None, "pad {pad} must not decode");
|
||||
}
|
||||
|
||||
// The specific alias the bug produced: 256 as u8 == 0.
|
||||
let d = HidOutput::AudioCtl {
|
||||
pad: 256,
|
||||
flags: 0,
|
||||
raw: [0; 6],
|
||||
}
|
||||
.encode();
|
||||
assert!(
|
||||
!matches!(
|
||||
HidOutput::decode(&d),
|
||||
Some(HidOutput::AudioCtl { pad: 0, .. })
|
||||
),
|
||||
"wire pad 256 must never surface as pad 0"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cursor_state_roundtrip() {
|
||||
for (flags, x, y) in [
|
||||
|
||||
@@ -1387,9 +1387,14 @@ fn pad_audio_slots() -> u8 {
|
||||
.clamp(1, 4)
|
||||
}
|
||||
|
||||
/// The endpoints provisioned at startup, set exactly once by the worker thread.
|
||||
/// The endpoints provisioned at startup, set exactly once by the worker thread — and only on
|
||||
/// SUCCESS. See [`provision_at_startup`] for why the failure path deliberately leaves it unset.
|
||||
static PROVISIONED: OnceLock<Arc<Vec<PadEndpoint>>> = OnceLock::new();
|
||||
|
||||
/// A provisioning attempt is in flight. Guards the retry in [`ensure_provisioned`] against
|
||||
/// spawning a second COM worker while the first is still enumerating.
|
||||
static PROVISIONING: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false);
|
||||
|
||||
/// Host-startup pre-provisioning (Windows, env-gated): spawn a COM worker that `ensure()`s
|
||||
/// endpoints for slots `0..N`, performs at most ONE AudioEndpointBuilder+Audiosrv restart if
|
||||
/// any stamp is stored-but-not-served (then re-verifies), and publishes the results for the
|
||||
@@ -1403,6 +1408,10 @@ pub(crate) fn provision_at_startup() {
|
||||
if PROVISIONED.get().is_some() {
|
||||
return;
|
||||
}
|
||||
// R5: one attempt at a time. Without this the retry below could stack COM workers.
|
||||
if PROVISIONING.swap(true, std::sync::atomic::Ordering::SeqCst) {
|
||||
return;
|
||||
}
|
||||
let slots = pad_audio_slots();
|
||||
let spawned = thread::Builder::new()
|
||||
.name("punktfunk-pad-audio".into())
|
||||
@@ -1441,9 +1450,24 @@ pub(crate) fn provision_at_startup() {
|
||||
stored-but-not-served until the next reboot"),
|
||||
}
|
||||
}
|
||||
let _ = PROVISIONED.set(Arc::new(eps));
|
||||
// R5: latch the result ONLY if we actually provisioned something. This used to store
|
||||
// whatever `eps` held even when the loop broke on the first error — an empty vec —
|
||||
// and `OnceLock` made that permanent: one transient failure (a busy audio stack, a
|
||||
// service mid-restart) disabled pad audio for the entire life of the host process,
|
||||
// with the only evidence a single warning at startup. An empty result now leaves the
|
||||
// cell unset so `ensure_provisioned` can try again when a session next asks.
|
||||
if eps.is_empty() {
|
||||
tracing::warn!(
|
||||
"pad-audio provisioning produced no endpoints — leaving it unlatched so the \
|
||||
next session retries rather than disabling pad audio for this process"
|
||||
);
|
||||
} else {
|
||||
let _ = PROVISIONED.set(Arc::new(eps));
|
||||
}
|
||||
PROVISIONING.store(false, std::sync::atomic::Ordering::SeqCst);
|
||||
});
|
||||
if let Err(e) = spawned {
|
||||
PROVISIONING.store(false, std::sync::atomic::Ordering::SeqCst);
|
||||
tracing::warn!(error = %e, "could not spawn the pad-audio provisioning thread");
|
||||
}
|
||||
}
|
||||
@@ -1454,6 +1478,17 @@ pub(crate) fn provisioned_endpoints() -> Option<Arc<Vec<PadEndpoint>>> {
|
||||
PROVISIONED.get().cloned()
|
||||
}
|
||||
|
||||
/// R5: ask for provisioning again if the startup attempt produced nothing. Cheap and idempotent —
|
||||
/// a successful latch returns immediately, and `PROVISIONING` keeps concurrent askers to one
|
||||
/// worker. Called where a session first wants to know whether pad audio exists, so a host that
|
||||
/// started while the audio stack was busy recovers on the next connect instead of at the next
|
||||
/// reboot.
|
||||
pub(crate) fn ensure_provisioned() {
|
||||
if PROVISIONED.get().is_none() {
|
||||
provision_at_startup();
|
||||
}
|
||||
}
|
||||
|
||||
/// The provisioned endpoint for one pad slot — what a session queries when a client pad with
|
||||
/// speaker support arrives, to attach a [`PadLoopbackCapturer`].
|
||||
#[allow(dead_code)]
|
||||
@@ -1571,13 +1606,42 @@ impl PadLoopbackCapturer {
|
||||
}),
|
||||
Ok(Err(e)) => Err(e),
|
||||
Err(_) => {
|
||||
// R6: signal AND reap. Dropping `join` here detached the WASAPI thread, and the
|
||||
// streamer's reopen loop retries this every ~2 s — so a wedged activation leaked
|
||||
// one thread (each holding COM apartment state and a channel end) per attempt,
|
||||
// indefinitely. The join is bounded in practice because the thread's own loop
|
||||
// observes `stop` between waits; give it a moment and, if it is genuinely stuck
|
||||
// inside a blocking WASAPI call, say so rather than leaking in silence.
|
||||
stop.store(true, Ordering::SeqCst);
|
||||
Err(anyhow!("pad loopback init timed out"))
|
||||
match reap_with_timeout(join, Duration::from_secs(2)) {
|
||||
true => Err(anyhow!("pad loopback init timed out")),
|
||||
false => Err(anyhow!(
|
||||
"pad loopback init timed out and its thread did not exit — the audio \
|
||||
stack is wedged; not retrying into a thread leak"
|
||||
)),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Join `join`, giving it `budget` to notice a stop flag. `true` if it exited.
|
||||
///
|
||||
/// A detached thread is the wrong answer at a retry point (see [`PadLoopbackCapturer::open`]):
|
||||
/// the caller reopens on a timer, so "leak one and move on" compounds. Waiting is bounded, and a
|
||||
/// thread that outlasts the budget is reported instead of silently accumulating.
|
||||
fn reap_with_timeout(join: JoinHandle<()>, budget: Duration) -> bool {
|
||||
let deadline = std::time::Instant::now() + budget;
|
||||
while !join.is_finished() {
|
||||
if std::time::Instant::now() >= deadline {
|
||||
return false;
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(20));
|
||||
}
|
||||
let _ = join.join();
|
||||
true
|
||||
}
|
||||
|
||||
/// Render a test tone straight into a pad's audio endpoint.
|
||||
///
|
||||
/// The point is iteration speed. Without this, exercising the pad-audio chain means launching a
|
||||
@@ -1645,13 +1709,18 @@ pub(crate) fn render_test_tone(
|
||||
let device = open_wasapi_device(endpoint_id)
|
||||
.with_context(|| format!("pad endpoint {endpoint_id} not found"))?;
|
||||
let mut audio_client = device.get_iaudioclient().context("IAudioClient")?;
|
||||
// B11: the SAME mask the endpoint and the loopback capture use. Passing `None` let wasapi
|
||||
// derive `(1 << 4) - 1` = 0x0F (FL FR FC LFE) instead of 0x33 (FL FR BL BR), so this devtest
|
||||
// — the instrument for "which coil is which" — put its tone on a different channel pairing
|
||||
// than the real path. It could not exercise the coil route at all, and read as an inverted
|
||||
// pair when it appeared to.
|
||||
let desired = WaveFormat::new(
|
||||
32,
|
||||
32,
|
||||
&SampleType::Float,
|
||||
SAMPLE_RATE as usize,
|
||||
PAD_CHANNELS as usize,
|
||||
None,
|
||||
Some(PAD_CHANNEL_MASK),
|
||||
);
|
||||
let (default_period, _min) = audio_client.get_device_period().context("device period")?;
|
||||
audio_client
|
||||
|
||||
@@ -511,7 +511,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 — \
|
||||
@@ -639,7 +639,7 @@ fn capture_once(
|
||||
);
|
||||
return Ok(Next::Reopen(TargetMode::Follow));
|
||||
}
|
||||
match judge_default(&en, wiring, &nid) {
|
||||
match judge_default(wiring, &nid) {
|
||||
DefaultKind::Capturable(name) => {
|
||||
audio_client.stop_stream().ok();
|
||||
tracing::info!(device = %name,
|
||||
@@ -739,7 +739,15 @@ fn judge_default(wiring: &wiring_plan::Wiring, id: &str) -> DefaultKind {
|
||||
.mic_render
|
||||
.as_ref()
|
||||
.is_some_and(|(_, mic_id)| mic_id == id);
|
||||
if is_mic || wiring_plan::excluded_from_loopback(&ln) {
|
||||
// B10: a pad's audio endpoint is not ordinary hardware, and the name rules cannot see that —
|
||||
// it is deliberately stamped with the controller's own name ("DualSense Wireless Controller")
|
||||
// so games treat it as the pad's speaker, which means `excluded_from_loopback` passes it
|
||||
// straight through as `Capturable`. The pure plan filtered these out, but the plan is not the
|
||||
// only reader: this classifier drives the watchdog, Follow mode and the parked default, so a
|
||||
// pad endpoint that happened to be the system default could be adopted as the desktop capture
|
||||
// source — sending the whole desktop mix to a controller's voice coils. Identity, not name.
|
||||
let is_pad = super::pad_endpoint::is_pad_render_endpoint(id);
|
||||
if is_mic || is_pad || wiring_plan::excluded_from_loopback(&ln) {
|
||||
DefaultKind::Dud(name)
|
||||
} else {
|
||||
DefaultKind::Capturable(name)
|
||||
|
||||
@@ -724,7 +724,7 @@ mod tests {
|
||||
("steam streaming microphone", fmt(24_000, 1)),
|
||||
("odyssey", fmt(48_000, 2)),
|
||||
]);
|
||||
let w = plan_with_formats(&renders, &captures, None, false, &p, 2);
|
||||
let w = plan_with_formats(&renders, &captures, None, false, &p, 2, &[]);
|
||||
assert_eq!(
|
||||
w.loopback_render.as_ref().unwrap().0,
|
||||
"1 - Odyssey G60SD (AMD High Definition Audio Device)",
|
||||
@@ -754,7 +754,7 @@ mod tests {
|
||||
("steam streaming microphone", fmt(48_000, 2)),
|
||||
("realtek", fmt(48_000, 2)),
|
||||
]);
|
||||
let w = plan_with_formats(&renders, &[], None, false, &p, 2);
|
||||
let w = plan_with_formats(&renders, &[], None, false, &p, 2, &[]);
|
||||
assert_eq!(
|
||||
w.loopback_render.unwrap().0,
|
||||
"Speakers (Steam Streaming Microphone)"
|
||||
@@ -770,7 +770,7 @@ mod tests {
|
||||
ep("Speakers (Steam Streaming Microphone)"),
|
||||
];
|
||||
let p = probe(vec![("steam streaming microphone", fmt(16_000, 1))]);
|
||||
let w = plan_with_formats(&renders, &[], None, false, &p, 2);
|
||||
let w = plan_with_formats(&renders, &[], None, false, &p, 2, &[]);
|
||||
assert_eq!(
|
||||
w.loopback_render.as_ref().unwrap().0,
|
||||
"Speakers (Steam Streaming Microphone)"
|
||||
@@ -786,7 +786,7 @@ mod tests {
|
||||
fn narrowing_is_reported_for_real_hardware_too() {
|
||||
let renders = [ep("Headset (Hands-Free AG Audio)")];
|
||||
let p = probe(vec![("headset", fmt(16_000, 1))]);
|
||||
let w = plan_with_formats(&renders, &[], None, false, &p, 2);
|
||||
let w = plan_with_formats(&renders, &[], None, false, &p, 2, &[]);
|
||||
assert_eq!(
|
||||
w.loopback_render.as_ref().unwrap().0,
|
||||
"Headset (Hands-Free AG Audio)"
|
||||
@@ -806,8 +806,8 @@ mod tests {
|
||||
];
|
||||
let captures = [ep("CABLE Output (VB-Audio Virtual Cable)")];
|
||||
for host_audio in [false, true] {
|
||||
let a = plan(&renders, &captures, None, host_audio);
|
||||
let b = plan_with_formats(&renders, &captures, None, host_audio, &no_formats, 2);
|
||||
let a = plan(&renders, &captures, None, host_audio, &[]);
|
||||
let b = plan_with_formats(&renders, &captures, None, host_audio, &no_formats, 2, &[]);
|
||||
assert_eq!(a, b, "host_audio={host_audio}");
|
||||
assert!(a.loopback_narrowing.is_none());
|
||||
}
|
||||
@@ -825,7 +825,7 @@ mod tests {
|
||||
("steam streaming microphone", fmt(24_000, 1)),
|
||||
("realtek", fmt(48_000, 2)),
|
||||
]);
|
||||
let w = plan_with_formats(&renders, &[], None, true, &p, 2);
|
||||
let w = plan_with_formats(&renders, &[], None, true, &p, 2, &[]);
|
||||
assert_eq!(w.loopback_render.unwrap().0, "Speakers (Realtek HD Audio)");
|
||||
}
|
||||
|
||||
|
||||
@@ -523,12 +523,22 @@ struct PadAudioSlots {
|
||||
/// `(kinds, handle)` per running pad — `kinds` is the arrival's audio-caps mask, kept so
|
||||
/// an identical re-arrival (they are re-sent against datagram loss) is a no-op.
|
||||
slots: [Option<(u8, pad_audio::PadAudioHandle)>; MAX_WIRE_PADS],
|
||||
/// Kind-change restarts spent per pad this session (R3). The trigger is a client-sent
|
||||
/// arrival, so without a ceiling the client decides how many WASAPI captures the host opens.
|
||||
restarts: [u8; MAX_WIRE_PADS],
|
||||
}
|
||||
|
||||
/// R3: how many times one pad may change its declared audio kinds before the host stops
|
||||
/// obliging. A real controller declares once at open and never again; the re-sent arrivals are
|
||||
/// identical and take the no-op path above, so this is only reached by a client that keeps
|
||||
/// changing its mind.
|
||||
const MAX_PAD_AUDIO_RESTARTS: u8 = 8;
|
||||
|
||||
impl PadAudioSlots {
|
||||
fn new() -> PadAudioSlots {
|
||||
PadAudioSlots {
|
||||
slots: std::array::from_fn(|_| None),
|
||||
restarts: [0; MAX_WIRE_PADS],
|
||||
}
|
||||
}
|
||||
|
||||
@@ -544,8 +554,23 @@ impl PadAudioSlots {
|
||||
if *have == kinds {
|
||||
return; // identical re-arrival — keep the running streamer
|
||||
}
|
||||
// R3: the restart trigger is a CLIENT-sent arrival, so the count is client-driven.
|
||||
// Nothing bounded it: a client alternating its declared kinds could make the host
|
||||
// tear down and re-spawn a WASAPI loopback capture indefinitely, each cycle paying a
|
||||
// thread spawn and an endpoint activation. Cheap to bound, and a pad that has already
|
||||
// changed its mind this many times in one session is not doing anything legitimate.
|
||||
if self.restarts[idx] >= MAX_PAD_AUDIO_RESTARTS {
|
||||
tracing::warn!(
|
||||
pad = idx,
|
||||
"pad-audio kinds changed again after {MAX_PAD_AUDIO_RESTARTS} restarts — \
|
||||
ignoring; the streamer keeps its current kinds for this session"
|
||||
);
|
||||
return;
|
||||
}
|
||||
self.restarts[idx] += 1;
|
||||
tracing::info!(
|
||||
pad = idx,
|
||||
restarts = self.restarts[idx],
|
||||
"pad-audio kinds changed — restarting the streamer"
|
||||
);
|
||||
self.stop(idx);
|
||||
|
||||
@@ -247,6 +247,11 @@ pub(super) fn host_cap(client_caps: u8) -> bool {
|
||||
let asked = client_caps & punktfunk_core::quic::CLIENT_CAP_PAD_AUDIO != 0;
|
||||
#[cfg(target_os = "windows")]
|
||||
{
|
||||
// R5: a startup attempt that failed transiently leaves nothing latched, so retry here —
|
||||
// this is the first moment in a session's life that anyone asks whether pad audio exists.
|
||||
if asked {
|
||||
crate::audio::pad_endpoint::ensure_provisioned();
|
||||
}
|
||||
asked
|
||||
&& std::env::var_os("PUNKTFUNK_PAD_AUDIO").is_none_or(|v| v != "0")
|
||||
&& crate::audio::pad_endpoint::provisioned_endpoints()
|
||||
@@ -288,6 +293,22 @@ pub(super) fn spawn(
|
||||
// cheap to refuse rather than spin the open/backoff loop on an empty id.
|
||||
return None;
|
||||
}
|
||||
if ep.needs_aeb_kick {
|
||||
// R4: this flag was computed on every path and consulted nowhere past startup. It means
|
||||
// the endpoint's stamps are STORED but not SERVED — the audio stack never picked up the
|
||||
// DualSense identity — and startup's one restart did not fix it. Opening anyway is worse
|
||||
// than refusing: `AUTOCONVERTPCM` makes a wrong-format endpoint initialize *successfully*,
|
||||
// so the stream runs, the logs look healthy, and the haptics/speaker pair is mis-routed
|
||||
// with nothing to point at. Decline, and say which reboot-shaped problem it is.
|
||||
tracing::warn!(
|
||||
pad,
|
||||
endpoint = %ep.endpoint_id,
|
||||
"pad endpoint stamps are stored but not served — the audio stack has not adopted the \
|
||||
DualSense identity (a reboot, or a manual AudioEndpointBuilder+Audiosrv restart, \
|
||||
clears it). Not streaming: the endpoint would open and mis-route."
|
||||
);
|
||||
return None;
|
||||
}
|
||||
let stop_t = stop.clone();
|
||||
match std::thread::Builder::new()
|
||||
.name(format!("punktfunk1-pad{pad}"))
|
||||
|
||||
Reference in New Issue
Block a user