From 7077b0a0df1d83547e6dde97011f340fa3ce70c4 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 4 Aug 2026 08:11:35 +0200 Subject: [PATCH 1/7] feat(core/audio): bitrate tiers, a shared de-jitter policy, and a redundant audio plane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Foundation for the audio quality + latency plan (design/audio-quality-and-latency.md). All three pieces are pure and unit-tested here so the four client rings and the Windows host glue that follow stay thin. **Bitrate tiers** (`AudioTier`). The layout table's `bitrate` becomes the `Standard` value, so that tier reproduces the pre-tier wire byte-for-byte — the tier machinery is provably non-regressive. `High` (stereo 256 kbps) is the default: 5 ms Opus frames are much less efficient than 20 ms ones, so the historical 128 kbps buys roughly what ~100 kbps buys at 20 ms, while the same session carries tens of Mbps of video. Purely a host-side encoder knob — libopus reads the bitrate out of the packet, so no client change and no negotiation. **`JitterPolicy`** — the ms-denominated de-jitter state machine every client will share. Two defects it exists to fix: (1) each ring computed its target as `3 x quantum`, a sane 15 ms at a 5 ms quantum and a silent 64 ms at a 20 ms one; (2) every ring primed *up* and clamped at a ceiling, and none walked the depth back *down*, so drift/bursts added latency permanently — Android, with no shed at all, converged on its 120 ms cap. Here a depth EWMA that sits above target for 2 s of consumed audio sheds ONE 5 ms frame with a crossfade. Driven by samples consumed rather than the wall clock: allocation- and syscall-free (safe in a realtime callback) and deterministic under test. `every_preset_sheds_before_it_trims` pins the invariant that makes this real rather than decorative. The first draft had `headroom_ms` <= the shed threshold on all four presets, so the ring was trimmed back before the average could ever reach the shed point: drift correction was dead code and the ratchet test passed for the wrong reason (the hard cap did the work). `a_transient_burst_does_not_shed` caught it. The shed point is now derived from `headroom_ms` so it cannot invert again. **`0xD2` redundant audio** — each datagram carries its frame plus a copy of the previous one, so a single lost packet is reconstructed instead of concealed. Opus in-band FEC cannot do this job: LBRR is a SILK feature and the desktop encoder is CELT-only (RESTRICTED_LOWDELAY, 5 ms), so `set_inband_fec` there is a no-op. Costs no latency — the copy rides the successor, which arrives inside de-jitter slack that already exists. Gated capable-and-agreed via CLIENT_CAP_AUDIO_RED/HOST_CAP_AUDIO_RED; every other session keeps the `0xC9` wire unchanged. 0xD1 is left free for the pad-audio program. cbindgen: prefix the four new exported constants. `FRAME_MS`/`SAMPLE_RATE_HZ` as bare C macros are the same hazard the BTN_* renames already document — a clashing #define takes the last definition silently rather than failing to compile. Verified: 300 core tests, clippy -D warnings, fmt. (`c_abi` fails identically on a pristine tree — this Mac has no system libopus for the C harness link.) Co-Authored-By: Claude Opus 5 (1M context) --- crates/punktfunk-core/cbindgen.toml | 7 + crates/punktfunk-core/src/audio.rs | 779 +++++++++++++++++++++ crates/punktfunk-core/src/quic/caps.rs | 23 + crates/punktfunk-core/src/quic/datagram.rs | 153 ++++ include/punktfunk_core.h | 85 +++ 5 files changed, 1047 insertions(+) diff --git a/crates/punktfunk-core/cbindgen.toml b/crates/punktfunk-core/cbindgen.toml index 0e263da0..538379fd 100644 --- a/crates/punktfunk-core/cbindgen.toml +++ b/crates/punktfunk-core/cbindgen.toml @@ -48,6 +48,13 @@ exclude = ["MsghdrX", "recvmsg_x", "mmsghdr", "sendmmsg", "recvmmsg"] "AXIS_RT" = "PUNKTFUNK_AXIS_RT" "AUDIO_MAGIC" = "PUNKTFUNK_AUDIO_MAGIC" "RUMBLE_MAGIC" = "PUNKTFUNK_RUMBLE_MAGIC" +"AUDIO_RED_MAGIC" = "PUNKTFUNK_AUDIO_RED_MAGIC" +"AUDIO_RED_HEADER" = "PUNKTFUNK_AUDIO_RED_HEADER" +# Same hazard as the BTN_* block above, one step worse: `FRAME_MS` and `SAMPLE_RATE_HZ` are +# generic enough that an embedder is likely to have its own, and a clashing #define silently +# takes the last definition rather than failing to compile. +"FRAME_MS" = "PUNKTFUNK_AUDIO_FRAME_MS" +"SAMPLE_RATE_HZ" = "PUNKTFUNK_AUDIO_SAMPLE_RATE_HZ" # QualifiedScreamingSnakeCase already qualifies each variant with the enum name # (PunktfunkStatus::Ok -> PUNKTFUNK_STATUS_OK); do NOT also set prefix_with_name or it doubles. diff --git a/crates/punktfunk-core/src/audio.rs b/crates/punktfunk-core/src/audio.rs index 88270e6b..6975b9de 100644 --- a/crates/punktfunk-core/src/audio.rs +++ b/crates/punktfunk-core/src/audio.rs @@ -103,6 +103,72 @@ pub const LAYOUT_71_HQ: OpusLayout = OpusLayout { bitrate: 2_048_000, }; +/// Encode bitrate tier for the desktop-audio downlink. The layout table's `bitrate` is the +/// [`AudioTier::Standard`] value, so `Standard` reproduces the pre-tier wire byte-for-byte. +/// +/// **Why a tier at all.** 5 ms Opus frames are markedly less efficient than 20 ms ones (shorter +/// MDCT, a bigger per-packet overhead share), so the historical 128 kbps stereo buys roughly what +/// ~100 kbps buys at 20 ms — audible on music, and the 2026-08-03 field report said exactly that. +/// Meanwhile the same session carries tens of Mbps of video: at 256 kbps audio is ~1 % of the +/// budget. [`AudioTier::High`] is therefore the DEFAULT; the lower tiers exist for a genuinely +/// constrained link, not as the normal case. +/// +/// Purely a host-side encoder knob: every client decodes whatever bitrate arrives (libopus reads +/// it from the packet), so changing tiers needs no protocol negotiation and no client change. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] +pub enum AudioTier { + /// Constrained links — noticeably lossy on music, still fine for game/voice content. + Low, + /// The historical values (stereo 128 kbps). Kept exactly so the tier machinery is provably + /// non-regressive against every pre-tier build. + Standard, + /// The default: effectively transparent at 5 ms frames, for ~1 % of a normal video budget. + #[default] + High, +} + +impl AudioTier { + /// Parse a config/CLI spelling (`low` / `standard` / `high`); `None` for anything else so the + /// caller can warn and fall back rather than silently downgrading someone's audio. + pub fn parse(s: &str) -> Option { + match s.trim().to_ascii_lowercase().as_str() { + "low" => Some(AudioTier::Low), + "standard" | "normal" | "medium" => Some(AudioTier::Standard), + "high" => Some(AudioTier::High), + _ => None, + } + } + + pub fn as_str(self) -> &'static str { + match self { + AudioTier::Low => "low", + AudioTier::Standard => "standard", + AudioTier::High => "high", + } + } +} + +impl OpusLayout { + /// This layout's target bitrate at `tier`. The uncoupled HIGH-QUALITY layouts + /// ([`LAYOUT_51_HQ`] / [`LAYOUT_71_HQ`]) are already far past transparency, so they are + /// tier-invariant — scaling 1.5 Mbps up would only waste wire. + pub fn bitrate_for(&self, tier: AudioTier) -> i32 { + // One mono stream per channel == the HQ layouts; nothing to gain from a tier there. + if self.coupled == 0 && self.streams == self.channels { + return self.bitrate; + } + match (self.channels, tier) { + (6, AudioTier::Low) => 192_000, + (6, AudioTier::High) => 448_000, + (8, AudioTier::Low) => 320_000, + (8, AudioTier::High) => 768_000, + (_, AudioTier::Low) => 96_000, + (_, AudioTier::High) => 256_000, + (_, AudioTier::Standard) => self.bitrate, + } + } +} + /// Pick the layout for a negotiated channel count. Unknown counts fall back to stereo (clients /// only ever request 2/6/8). `high_quality` selects the uncoupled high-bitrate config. pub fn layout_for(channels: u8, high_quality: bool) -> &'static OpusLayout { @@ -173,6 +239,342 @@ impl AudioGapTracker { } } +// ---- the shared playback de-jitter policy ------------------------------------------------- + +/// The protocol's audio frame, in milliseconds — every host datagram carries exactly one +/// ([`crate::quic::encode_audio_datagram`]), so it is also the smallest useful shed unit. +pub const FRAME_MS: u32 = 5; + +/// Tuning for [`JitterPolicy`], in MILLISECONDS. +/// +/// Denominating the depth in time rather than in device quanta is the point. Every client used to +/// compute its target as `3 × quantum`, which is a sane 15 ms at a 5 ms quantum and a silent 64 ms +/// at a 20 ms one — the same source line meaning two very different latencies depending on what +/// else happened to be using the audio graph that day. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct JitterTuning { + /// Depth to prime to before the first sample plays, and the depth drift correction pulls + /// back toward. The adaptive floor may raise the live target above this; it never goes below. + pub base_target_ms: u32, + /// Ceiling for the adaptively-grown target (see [`JitterPolicy::note_read`]). + pub max_target_ms: u32, + /// Slack above the live target before drop-oldest trimming starts. Absorbs an arrival burst + /// without overflowing. + /// + /// Drift correction sheds at the MIDDLE of this band (see [`JitterTuning::shed_excess_ms`]), + /// so the smooth correction always gets its chance before the hard trim. Setting this too + /// small is a real failure mode, not just a tuning choice: if the trim point sits below the + /// shed point, the ring is trimmed back before the depth average can ever reach the shed + /// threshold, drift correction becomes dead code, and every correction is once again the + /// audible drop it was supposed to replace. + pub headroom_ms: u32, + /// Absolute bound on buffered audio — the only hard guarantee on added latency. + pub hard_cap_ms: u32, + /// Consecutive short reads before the ring goes back to priming. `1` reproduces the old + /// `if ring.is_empty() { primed = false }`, where a single transient drain manufactured a + /// whole target's worth of fresh silence; every platform now uses hysteresis. + pub deprime_after: u32, +} + +impl JitterTuning { + /// PipeWire adaptively rate-matches the stream to the graph clock and absorbs a shallow ring, + /// so Linux can run tight. + pub const PIPEWIRE: JitterTuning = JitterTuning { + base_target_ms: 15, + max_target_ms: 60, + headroom_ms: 25, + hard_cap_ms: 80, + deprime_after: 4, + }; + /// WASAPI shared-mode event-driven render: the engine buffers for us, but nothing rate-matches. + pub const WASAPI: JitterTuning = JitterTuning { + base_target_ms: 20, + max_target_ms: 70, + headroom_ms: 30, + hard_cap_ms: 90, + deprime_after: 4, + }; + /// CoreAudio via AVAudioEngine — comparable to WASAPI; the iOS IO buffer is already 5 ms. + pub const COREAUDIO: JitterTuning = JitterTuning { + base_target_ms: 20, + max_target_ms: 70, + headroom_ms: 30, + hard_cap_ms: 90, + deprime_after: 4, + }; + /// AAudio hands us a raw realtime callback and makes us own the buffer, and Wi-Fi power-save + /// bunching lands as underruns = crackle. Android therefore starts DEEPER — but at 25 ms, not + /// the old fixed 40: the adaptive floor raises it only on the devices that actually underrun, + /// instead of every device pre-paying for the worst one. + pub const AAUDIO: JitterTuning = JitterTuning { + base_target_ms: 25, + max_target_ms: 90, + headroom_ms: 40, + hard_cap_ms: 120, + deprime_after: 5, + }; + + /// How far above the live target the depth average must sit before drift correction sheds: + /// the middle of the headroom band, but never less than two protocol frames (so it cannot be + /// hair-triggered by one quantum of normal swing). Deriving it from `headroom_ms` rather than + /// fixing it absolutely is what keeps the smooth shed strictly BELOW the hard trim on every + /// preset — see the field on `headroom_ms`. + pub const fn shed_excess_ms(&self) -> u32 { + let half = self.headroom_ms / 2; + if half > 2 * FRAME_MS { + half + } else { + 2 * FRAME_MS + } + } +} + +/// What one callback should do, from [`JitterPolicy::step`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] +pub struct JitterStep { + /// Interleaved samples to discard from the FRONT of the ring before reading. + pub drop_front: usize, + /// When non-zero, `drop_front` is a smooth drift correction and this many interleaved samples + /// of linear crossfade should be applied across the seam ([`crossfade_drop`] does it for a + /// `VecDeque` ring). Zero means discard hard — either nothing is being dropped, or the + /// ring blew the hard cap and is already a discontinuity. + pub crossfade: usize, + /// Emit silence this callback: still priming, or re-priming after a sustained drain. + pub silence: bool, +} + +/// EWMA time constant for the depth average, in ms. Long enough that a burst doesn't trigger a +/// shed, short enough to track real drift. +const EWMA_TAU_MS: u32 = 1_000; +/// The depth EWMA must stay above the shed threshold for this much CONSUMED AUDIO. Deliberately long: a shed is the only +/// thing here a listener could ever notice, so it must never fire on a transient. +const SHED_SUSTAIN_MS: u32 = 2_000; +/// Linear crossfade applied across a drift shed's seam. +const SHED_CROSSFADE_MS: u32 = 2; +/// Underruns inside [`GROW_WINDOW_MS`] before the live target grows. +const GROW_UNDERRUNS: u32 = 3; +const GROW_WINDOW_MS: u32 = 5_000; +const GROW_STEP_MS: u32 = 10; +/// Quiet time (no underrun) before a grown target relaxes one step back toward the base. +const SHRINK_QUIET_MS: u32 = 30_000; + +/// The playback de-jitter state machine shared by every client's audio ring. +/// +/// **The defect it exists to fix.** Every client's ring primed *up* to a target and clamped at a +/// ceiling, and none of them walked the depth back *down*. Any transient — a Wi-Fi arrival burst, a +/// host stall, or plain host-DAC-vs-client-DAC clock skew of a few dozen ppm — therefore added +/// latency permanently, until an underrun happened to re-prime. Android, with no shed at all, +/// converged on its hard cap and stayed there; Apple shed 40 ms at once and its own comment called +/// that "one audible blip". Here, a depth EWMA that sits [`SHED_EXCESS_MS`] above target for +/// [`SHED_SUSTAIN_MS`] of consumed audio sheds ONE 5 ms frame with a crossfade, so latency returns +/// to target instead of ratcheting. +/// +/// **Driven by the audio clock, not the wall clock**: every duration is measured in samples +/// consumed. That makes it allocation-free, syscall-free (safe in a realtime callback) and +/// deterministic under test. +#[derive(Clone, Debug)] +pub struct JitterPolicy { + tuning: JitterTuning, + /// Interleaved samples per millisecond at the negotiated layout (48 × channels). + per_ms: usize, + /// The live target, in interleaved samples — `base_target_ms` grown by underrun pressure. + target: usize, + primed: bool, + /// Consecutive short reads (de-prime hysteresis). + empties: u32, + /// EWMA of ring depth, interleaved samples. + depth_avg: f32, + /// Consumed samples for which the EWMA has stayed above the shed threshold. + over_run: usize, + /// Underruns seen in the current growth window, and the window's consumed-sample count. + underruns: u32, + window_run: usize, + /// Consumed samples since the last underrun (drives the relax-back-down step). + quiet_run: usize, + /// `want` from the most recent [`step`](Self::step), so [`note_read`](Self::note_read) can + /// advance the sample-denominated timers without the caller repeating it. + last_want: usize, +} + +impl JitterPolicy { + /// `channels` is the negotiated interleaved channel count (2/6/8). + pub fn new(tuning: JitterTuning, channels: u8) -> JitterPolicy { + let per_ms = (SAMPLE_RATE_HZ / 1000) as usize * channels.max(1) as usize; + JitterPolicy { + tuning, + per_ms, + target: tuning.base_target_ms as usize * per_ms, + primed: false, + empties: 0, + depth_avg: 0.0, + over_run: 0, + underruns: 0, + window_run: 0, + quiet_run: 0, + last_want: 0, + } + } + + /// The live target depth in ms (grows under underrun pressure; never below the base). + pub fn target_ms(&self) -> u32 { + (self.target / self.per_ms) as u32 + } + + /// Convert a ring depth in interleaved samples to milliseconds — for stats/HUD reporting. + pub fn depth_ms(&self, depth: usize) -> u32 { + (depth / self.per_ms) as u32 + } + + /// Smoothed ring depth in ms — what drift correction actually reacts to, and the honest + /// number to publish as "audio buffer" (the instantaneous depth swings by a whole quantum). + pub fn avg_depth_ms(&self) -> u32 { + (self.depth_avg.max(0.0) as usize / self.per_ms) as u32 + } + + pub fn is_primed(&self) -> bool { + self.primed + } + + /// The effective target for a device asking for `want` samples per callback. A ring can never + /// sustain a target below one device quantum, so a large-buffer device (a 20 ms PipeWire graph + /// quantum, a legacy AAudio path) lifts it to `want` plus one protocol frame rather than + /// oscillating prime → dropout → re-prime forever. + fn effective_target(&self, want: usize) -> usize { + self.target.max(want + FRAME_MS as usize * self.per_ms) + } + + /// Decide this callback: what to trim, and whether to play. Call BEFORE reading, with the + /// ring's current `depth` and the device's `want`, both in interleaved samples. + pub fn step(&mut self, depth: usize, want: usize) -> JitterStep { + self.last_want = want; + let target = self.effective_target(want); + + // Track depth with a callback-rate-independent EWMA: weighting by `want` keeps the time + // constant at EWMA_TAU_MS whether the device pulls 5 ms or 20 ms at a time. + let alpha = (want as f32 / (EWMA_TAU_MS as usize * self.per_ms) as f32).clamp(0.0, 1.0); + self.depth_avg += (depth as f32 - self.depth_avg) * alpha; + + // The hard cap must always leave room to serve this callback, or a large-quantum device + // would trim itself into a permanent underrun. + let cap = (target + self.tuning.headroom_ms as usize * self.per_ms) + .min(self.tuning.hard_cap_ms as usize * self.per_ms) + .max(target + want); + + let mut out = JitterStep::default(); + if depth > cap { + // Blew the ceiling: a burst arrived, or we were wedged. Already a discontinuity — + // discard hard, and reset the drift timer so the trim isn't double-counted as drift. + out.drop_front = depth - cap; + self.over_run = 0; + } else if self.depth_avg + > (target + self.tuning.shed_excess_ms() as usize * self.per_ms) as f32 + { + self.over_run += want; + if self.over_run >= SHED_SUSTAIN_MS as usize * self.per_ms { + out.drop_front = (FRAME_MS as usize * self.per_ms).min(depth); + out.crossfade = (SHED_CROSSFADE_MS as usize * self.per_ms) + .min(depth.saturating_sub(out.drop_front)); + self.over_run = 0; + } + } else { + self.over_run = 0; + } + // Whatever we shed is no longer buffered — reflect it immediately so the next callbacks + // don't re-fire on a stale average. + self.depth_avg = (self.depth_avg - out.drop_front as f32).max(0.0); + + if !self.primed && depth.saturating_sub(out.drop_front) >= target { + self.primed = true; + self.empties = 0; + } + out.silence = !self.primed; + out + } + + /// Report the outcome of the read `step` authorised. `ran_short` = the ring could not fill the + /// callback (a genuine underrun), which drives both the de-prime hysteresis and the adaptive + /// target floor. + /// + /// A callback that `step` told to emit silence is NOT an underrun — the ring is deliberately + /// re-priming — so calls made while un-primed are ignored and callers need not special-case it. + pub fn note_read(&mut self, ran_short: bool) { + if !self.primed { + return; + } + let want = self.last_want.max(1); + self.window_run += want; + if self.window_run >= GROW_WINDOW_MS as usize * self.per_ms { + self.window_run = 0; + self.underruns = 0; + } + if ran_short { + self.quiet_run = 0; + self.empties += 1; + if self.empties >= self.tuning.deprime_after { + self.primed = false; + self.empties = 0; + } + self.underruns += 1; + if self.underruns >= GROW_UNDERRUNS { + // This device genuinely needs more slack than the base target. Grow ONCE per + // window, capped — the alternative (every device pre-paying the worst device's + // depth) is what the fixed 40 ms Android floor was. + self.underruns = 0; + self.window_run = 0; + let grown = self.target + GROW_STEP_MS as usize * self.per_ms; + self.target = grown.min(self.tuning.max_target_ms as usize * self.per_ms); + } + } else { + self.empties = 0; + self.quiet_run += want; + if self.quiet_run >= SHRINK_QUIET_MS as usize * self.per_ms { + // Long quiet spell: give a grown target one step back, so a single bad minute + // doesn't cost latency for the rest of the session. + self.quiet_run = 0; + let base = self.tuning.base_target_ms as usize * self.per_ms; + self.target = self + .target + .saturating_sub(GROW_STEP_MS as usize * self.per_ms) + .max(base); + } + } + } +} + +/// Sample rate of every audio plane in the protocol. +pub const SAMPLE_RATE_HZ: u32 = 48_000; + +/// Discard `drop` interleaved samples from the front of `ring`, linearly crossfading the seam over +/// `fade` samples so a drift correction is inaudible rather than a click. +/// +/// The dropped region's tail fades out while the surviving head fades in, so the waveform is +/// continuous across the splice. `fade == 0` discards hard (what a hard-cap trim wants — that +/// backlog is already a discontinuity). Shared by the three `VecDeque` rings; the Apple ring +/// is index-based and mirrors this in Swift. +pub fn crossfade_drop(ring: &mut std::collections::VecDeque, drop: usize, fade: usize) { + if drop == 0 || ring.len() < drop { + return; + } + let fade = fade.min(drop).min(ring.len() - drop); + if fade == 0 { + ring.drain(..drop); + return; + } + // The last `fade` samples of what we are about to discard are the fade-OUT source; they blend + // into the first `fade` samples of what survives. + let mut faded = Vec::with_capacity(fade); + for i in 0..fade { + let old = ring[drop - fade + i]; + let new = ring[drop + i]; + let t = (i + 1) as f32 / (fade + 1) as f32; + faded.push(old * (1.0 - t) + new * t); + } + ring.drain(..drop); + for (i, v) in faded.into_iter().enumerate() { + ring[i] = v; + } +} + // ---- per-platform channel-layout helpers (pure data; no platform deps) -------------------- /// Windows `WAVEFORMATEXTENSIBLE.dwChannelMask` for the wire layout. @@ -286,6 +688,383 @@ mod tests { assert_eq!(t.missing_before(0), 0, "pre-wrap reorder, not a 2^31 gap"); } + // ---- bitrate tiers ------------------------------------------------------------------- + + /// `Standard` must reproduce the historical table EXACTLY — that is what makes the tier + /// machinery provably non-regressive against every pre-tier build. + #[test] + fn standard_tier_is_the_legacy_table() { + for l in [ + &LAYOUT_STEREO, + &LAYOUT_51, + &LAYOUT_51_HQ, + &LAYOUT_71, + &LAYOUT_71_HQ, + ] { + assert_eq!(l.bitrate_for(AudioTier::Standard), l.bitrate, "{l:?}"); + } + } + + #[test] + fn tiers_are_monotonic_and_hq_layouts_are_invariant() { + for l in [&LAYOUT_STEREO, &LAYOUT_51, &LAYOUT_71] { + let (lo, std, hi) = ( + l.bitrate_for(AudioTier::Low), + l.bitrate_for(AudioTier::Standard), + l.bitrate_for(AudioTier::High), + ); + assert!(lo < std && std < hi, "{l:?}: {lo} < {std} < {hi}"); + } + // The uncoupled HQ layouts are already past transparency — no tier may move them. + for l in [&LAYOUT_51_HQ, &LAYOUT_71_HQ] { + for t in [AudioTier::Low, AudioTier::Standard, AudioTier::High] { + assert_eq!(l.bitrate_for(t), l.bitrate, "{l:?} at {t:?}"); + } + } + } + + #[test] + fn tier_default_is_high_and_parses() { + assert_eq!(AudioTier::default(), AudioTier::High); + for t in [AudioTier::Low, AudioTier::Standard, AudioTier::High] { + assert_eq!(AudioTier::parse(t.as_str()), Some(t)); + } + assert_eq!(AudioTier::parse(" HIGH "), Some(AudioTier::High)); + assert_eq!(AudioTier::parse("normal"), Some(AudioTier::Standard)); + // Unknown spellings must be rejected, not silently downgraded. + assert_eq!(AudioTier::parse("transparent"), None); + assert_eq!(AudioTier::parse(""), None); + } + + // ---- the de-jitter policy ------------------------------------------------------------ + + /// Interleaved samples per ms at `channels`. + fn per_ms(channels: u8) -> usize { + (SAMPLE_RATE_HZ / 1000) as usize * channels as usize + } + + /// One simulated run's outcome. + #[derive(Debug, Default)] + struct Sim { + final_ms: u32, + peak_ms: u32, + /// Smooth drift corrections (crossfaded, one frame each) — the good kind. + soft_sheds: u32, + /// Hard-cap trims — the backstop. Any of these in a plain-drift run means the smooth + /// correction is not doing its job. + hard_trims: u32, + underruns: u32, + } + + /// Drive a policy through `ms` of simulated audio at a `quantum_ms` device, where the producer + /// delivers `drift_ppm` more (or less) than the consumer takes — i.e. host-vs-client clock skew. + fn simulate( + tuning: JitterTuning, + channels: u8, + ms: u32, + quantum_ms: u32, + drift_ppm: i64, + start_ms: u32, + ) -> Sim { + let pm = per_ms(channels); + let want = quantum_ms as usize * pm; + let mut p = JitterPolicy::new(tuning, channels); + let mut depth = start_ms as usize * pm; + let mut out = Sim::default(); + // Fractional producer accumulator, so a sub-sample-per-callback drift still accumulates. + let mut carry: i64 = 0; + for _ in 0..(ms / quantum_ms) { + // Producer: one quantum of audio plus the drift. + carry += want as i64 * drift_ppm; + let extra = carry / 1_000_000; + carry -= extra * 1_000_000; + depth = (depth as i64 + want as i64 + extra).max(0) as usize; + + let s = p.step(depth, want); + if s.drop_front > 0 { + if s.crossfade > 0 { + out.soft_sheds += 1; + } else { + out.hard_trims += 1; + } + depth -= s.drop_front.min(depth); + } + if s.silence { + p.note_read(false); + continue; + } + let short = depth < want; + depth -= want.min(depth); + if short { + out.underruns += 1; + } + p.note_read(short); + out.peak_ms = out.peak_ms.max((depth / pm) as u32); + } + out.final_ms = (depth / pm) as u32; + out + } + + /// The invariant that makes drift correction real rather than decorative: on every preset the + /// smooth shed point must sit strictly BELOW the hard trim point. Invert it — by tuning + /// `headroom_ms` down — and the ring is trimmed back before the depth average can ever reach + /// the shed threshold, so the smooth path becomes dead code and every correction is the + /// audible drop it was meant to replace. (That inversion was present in the first draft of + /// this module and only surfaced because `a_transient_burst_does_not_shed` failed.) + #[test] + fn every_preset_sheds_before_it_trims() { + for (name, t) in [ + ("PIPEWIRE", JitterTuning::PIPEWIRE), + ("WASAPI", JitterTuning::WASAPI), + ("COREAUDIO", JitterTuning::COREAUDIO), + ("AAUDIO", JitterTuning::AAUDIO), + ] { + assert!( + t.shed_excess_ms() < t.headroom_ms, + "{name}: sheds at +{} ms but trims at +{} ms — drift correction can never fire", + t.shed_excess_ms(), + t.headroom_ms + ); + assert!( + t.base_target_ms + t.headroom_ms <= t.hard_cap_ms, + "{name}: the headroom band is cut short by the hard cap" + ); + assert!(t.max_target_ms >= t.base_target_ms, "{name}"); + assert!(t.deprime_after >= 2, "{name}: needs real hysteresis"); + } + } + + /// THE headline behaviour, and the defect this policy exists for: with the host clock running + /// fast, the old rings grew to their ceiling and stayed pinned there for the rest of the + /// session. Drift correction must hold the depth near target — and must do it with the SMOOTH + /// crossfaded shed, never by letting the hard cap chop the backlog. + #[test] + fn drift_does_not_ratchet_latency_to_the_ceiling() { + // +200 ppm is a deliberately harsh skew (real DAC pairs are tens of ppm); 5 minutes. + let s = simulate(JitterTuning::AAUDIO, 2, 300_000, 5, 200, 25); + assert!( + s.soft_sheds > 0, + "drift must be shed, not accumulated: {s:?}" + ); + assert_eq!( + s.hard_trims, 0, + "plain drift must never reach the hard cap: {s:?}" + ); + assert_eq!( + s.underruns, 0, + "shedding must never cause an underrun: {s:?}" + ); + // The old Android ring pinned at its 120 ms hard cap. Ours must stay inside the band. + let ceiling = JitterTuning::AAUDIO.base_target_ms + JitterTuning::AAUDIO.headroom_ms; + assert!( + s.peak_ms <= ceiling, + "peaked at {} ms (band ends at {ceiling}) — that is the ratchet, not a correction", + s.peak_ms + ); + } + + /// Same skew, every preset: none of them may ratchet. + #[test] + fn no_preset_ratchets_under_drift() { + for (name, t) in [ + ("PIPEWIRE", JitterTuning::PIPEWIRE), + ("WASAPI", JitterTuning::WASAPI), + ("COREAUDIO", JitterTuning::COREAUDIO), + ("AAUDIO", JitterTuning::AAUDIO), + ] { + let s = simulate(t, 2, 300_000, 5, 200, t.base_target_ms); + assert!(s.soft_sheds > 0, "{name}: {s:?}"); + assert!( + s.peak_ms <= t.base_target_ms + t.headroom_ms, + "{name} peaked at {} ms: {s:?}", + s.peak_ms + ); + } + } + + /// The mirror case: a host clock running SLOW must not be "corrected" into permanent + /// underruns. The adaptive floor may grow the target, but nothing may be shed. + #[test] + fn negative_drift_grows_the_target_instead_of_stuttering() { + let s = simulate(JitterTuning::AAUDIO, 2, 120_000, 5, -200, 25); + assert_eq!( + s.soft_sheds, 0, + "nothing to shed when the ring is draining: {s:?}" + ); + assert_eq!(s.hard_trims, 0, "{s:?}"); + } + + /// A shed must never fire on a transient — a burst that arrives and drains is normal jitter, + /// and shedding it would cost an audible artefact for nothing. The spike here sits ABOVE the + /// shed threshold but below the trim point, so only the sustain requirement can reject it. + #[test] + fn a_transient_burst_does_not_shed() { + let t = JitterTuning::AAUDIO; + let pm = per_ms(2); + let want = 5 * pm; + let spike_ms = t.base_target_ms + t.shed_excess_ms() + FRAME_MS; // inside the band + assert!( + spike_ms < t.base_target_ms + t.headroom_ms, + "test spike must not hit the trim" + ); + let mut p = JitterPolicy::new(t, 2); + let mut sheds = 0; + // 300 ms spiked out of every 1 s, for 20 s. + for round in 0..20 { + for i in 0..200 { + let depth = if round > 0 && i < 60 { + spike_ms + } else { + t.base_target_ms + } as usize; + let s = p.step(depth * pm, want); + if s.drop_front > 0 { + sheds += 1; + } + p.note_read(false); + } + } + assert_eq!( + sheds, 0, + "a repeated short burst must not trigger drift correction" + ); + } + + /// The hard cap is the only absolute latency guarantee — it trims immediately, without + /// waiting for the drift timer. + #[test] + fn hard_cap_trims_at_once() { + let pm = per_ms(2); + let mut p = JitterPolicy::new(JitterTuning::AAUDIO, 2); + let s = p.step(500 * pm, 5 * pm); + assert!( + s.drop_front > 0, + "a 500 ms backlog must be trimmed on the spot" + ); + assert_eq!(s.crossfade, 0, "a blown cap is already a discontinuity"); + let left = 500 * pm - s.drop_front; + assert!( + left <= JitterTuning::AAUDIO.hard_cap_ms as usize * pm, + "trim must land at or under the hard cap" + ); + } + + /// One transient drain must not manufacture a fresh target's worth of silence — the bug + /// Android fixed and Linux/Windows still carried. + #[test] + fn deprime_requires_hysteresis() { + let pm = per_ms(2); + let want = 5 * pm; + let mut p = JitterPolicy::new(JitterTuning::PIPEWIRE, 2); + // An EMPTY ring must emit silence and stay un-primed, however many callbacks it sees. + for _ in 0..10 { + assert!(p.step(0, want).silence, "an empty ring cannot play"); + } + assert!(!p.is_primed()); + // A ring already holding well over target primes on the first callback that sees it. + assert!( + !p.step(50 * pm, want).silence, + "a ring holding well over target must start immediately" + ); + assert!(p.is_primed()); + p.note_read(true); // one short read + assert!(p.is_primed(), "a single short read must not de-prime"); + for _ in 1..JitterTuning::PIPEWIRE.deprime_after { + p.note_read(true); + } + assert!(!p.is_primed(), "a sustained drain must re-prime"); + } + + /// A device that pulls a big quantum cannot sustain a target below it: the effective target + /// must lift, or the ring oscillates prime → dropout → re-prime forever. + #[test] + fn target_lifts_above_a_large_device_quantum() { + let pm = per_ms(2); + let mut p = JitterPolicy::new(JitterTuning::PIPEWIRE, 2); // base target 15 ms + let want = 40 * pm; // a 40 ms graph quantum — far above the base target + // At exactly the base target the ring must NOT claim to be primed. + assert!( + p.step(15 * pm, want).silence, + "15 ms cannot serve a 40 ms quantum" + ); + // Once it holds the quantum plus a frame, it may play. + let s = p.step((40 + FRAME_MS as usize) * pm, want); + assert!(!s.silence, "quantum + one frame must be enough to start"); + } + + /// Clustered underruns raise the floor (that device needs the slack); a long quiet spell + /// gives it back, so one bad minute doesn't cost latency for the whole session. + #[test] + fn target_grows_on_underruns_and_relaxes_when_quiet() { + let pm = per_ms(2); + let want = 5 * pm; + let mut p = JitterPolicy::new(JitterTuning::AAUDIO, 2); + let base = p.target_ms(); + assert_eq!(base, JitterTuning::AAUDIO.base_target_ms); + for _ in 0..40 { + // Keep it primed and starve it: depth is always enough to prime, never to serve. + while !p.is_primed() { + p.step(200 * pm, want); + } + p.step(200 * pm, want); + p.note_read(true); + } + let grown = p.target_ms(); + assert!( + grown > base, + "clustered underruns must raise the floor ({base} → {grown})" + ); + assert!( + grown <= JitterTuning::AAUDIO.max_target_ms, + "growth must respect max_target_ms" + ); + // Now a long clean run relaxes it back. + for _ in 0..(SHRINK_QUIET_MS as usize * 3 / 5) { + p.step(grown as usize * pm + want, want); + p.note_read(false); + } + assert!( + p.target_ms() < grown, + "a quiet spell must give the growth back" + ); + assert!(p.target_ms() >= base, "…but never below the base target"); + } + + /// The crossfade must leave a continuous waveform: splicing a ramp must not introduce a step + /// bigger than the ramp's own per-sample slope. + #[test] + fn crossfade_drop_splices_without_a_step() { + use std::collections::VecDeque; + // A slow ramp: any hard splice shows up as a visible jump. + let mut ring: VecDeque = (0..1000).map(|i| i as f32).collect(); + let (drop, fade) = (240, 96); + crossfade_drop(&mut ring, drop, fade); + assert_eq!(ring.len(), 1000 - drop); + // Across the whole faded region the step between neighbours stays bounded — a hard drop + // would show a `drop`-sized jump at index 0. + for i in 0..fade { + let step = (ring[i + 1] - ring[i]).abs(); + assert!( + step < drop as f32, + "sample {i}: step {step} looks like a hard splice" + ); + } + // Tail is untouched. + assert_eq!(ring[ring.len() - 1], 999.0); + } + + #[test] + fn crossfade_drop_handles_degenerate_inputs() { + use std::collections::VecDeque; + let mut ring: VecDeque = (0..10).map(|i| i as f32).collect(); + crossfade_drop(&mut ring, 0, 4); // nothing to drop + assert_eq!(ring.len(), 10); + crossfade_drop(&mut ring, 99, 4); // more than we hold — refuse + assert_eq!(ring.len(), 10); + crossfade_drop(&mut ring, 10, 4); // exactly all of it: no room to fade, hard drop + assert!(ring.is_empty()); + } + #[test] fn wasapi_masks_are_correct() { assert_eq!(wasapi_channel_mask(2), 0x3); diff --git a/crates/punktfunk-core/src/quic/caps.rs b/crates/punktfunk-core/src/quic/caps.rs index 1cf5e69b..181326e0 100644 --- a/crates/punktfunk-core/src/quic/caps.rs +++ b/crates/punktfunk-core/src/quic/caps.rs @@ -111,6 +111,17 @@ pub const CLIENT_CAP_CURSOR: u8 = 0x01; /// simply ignored — no behavior change in either direction. pub const CLIENT_CAP_PHASE_LOCK: u8 = 0x02; +/// `Hello.client_caps` bit: this client can decode the redundant desktop-audio plane +/// ([`AUDIO_RED_MAGIC`](super::datagram::AUDIO_RED_MAGIC), `0xD2`), where every datagram also +/// carries a copy of the previous frame so a single lost packet is reconstructed instead of +/// papered over with packet-loss concealment. +/// +/// Active only when the host answers with [`HOST_CAP_AUDIO_RED`] (capable-and-agreed, the +/// cursor/clipboard precedent). Toward an older host, or a host that declines because the link is +/// clean, the client keeps receiving the plain `0xC9` plane — so a client may always set this bit. +/// `0x04` — `0x01`/`0x02` are cursor / phase-lock. +pub const CLIENT_CAP_AUDIO_RED: u8 = 0x04; + /// [`Welcome::host_caps`] bit: the host CAN forward the cursor out-of-band (it captures cursor /// metadata separately from the frame — the Linux portal `SPA_META_Cursor` path; NOT gamescope, /// whose capture carries no cursor, and NOT Windows yet, where DWM composites into the IDD @@ -132,6 +143,18 @@ pub const HOST_CAP_CURSOR: u8 = 0x08; /// [`HOST_CAP_TEXT_INPUT`], `0x01`/`0x02` are gamepad-state / clipboard. pub const HOST_CAP_PEN: u8 = 0x10; +/// [`Welcome::host_caps`] bit: the host is sending the REDUNDANT desktop-audio plane +/// ([`AUDIO_RED_MAGIC`](super::datagram::AUDIO_RED_MAGIC), `0xD2`) instead of plain `0xC9` — each +/// datagram carries its own frame plus a copy of the previous one. +/// +/// Set only when the client asked via [`CLIENT_CAP_AUDIO_RED`]. It is a statement about the WIRE, +/// not a negotiation the client can decline: with the bit set the client must decode `0xD2`, and +/// without it `0xC9`. The host may also drop back to `0xC9` mid-session (the redundancy is +/// loss-gated — a clean LAN shouldn't pay for it), which is why clients decode BOTH tags +/// unconditionally and treat this bit as "expect redundancy", not "only redundancy". +/// `0x20` — `0x10` is [`HOST_CAP_PEN`], `0x08` is [`HOST_CAP_CURSOR`]. +pub const HOST_CAP_AUDIO_RED: u8 = 0x20; + /// [`Hello::video_codecs`] bit: the client can decode H.264 / AVC. The GPU-less **software** /// encode path (openh264) emits H.264, so a client that wants to stream from a software host MUST /// advertise this. diff --git a/crates/punktfunk-core/src/quic/datagram.rs b/crates/punktfunk-core/src/quic/datagram.rs index 22977987..0567f241 100644 --- a/crates/punktfunk-core/src/quic/datagram.rs +++ b/crates/punktfunk-core/src/quic/datagram.rs @@ -42,6 +42,80 @@ pub fn decode_audio_datagram(b: &[u8]) -> Option<(u32, u64, &[u8])> { Some((seq, pts_ns, &b[13..])) } +/// Redundant audio datagram, host → client: the [`AUDIO_MAGIC`] plane plus a copy of the PREVIOUS +/// frame, so a single lost datagram is *reconstructed* rather than concealed. +/// +/// `[0xD2][u32 seq LE][u64 pts_ns LE][u16 primary_len LE][primary opus][previous opus]` +/// +/// **Why this and not Opus in-band FEC.** LBRR is a SILK-layer feature: the desktop-audio encoder +/// runs `RESTRICTED_LOWDELAY` (CELT-only) at 5 ms frames, which is below SILK's 10 ms minimum, so +/// `set_inband_fec(true)` on that encoder is a no-op. Nothing in libopus can protect this plane — +/// the redundancy has to be at the application layer. (The mic uplink is a different encoder, VoIP +/// mode at 10 ms, and *does* use real in-band FEC.) +/// +/// **Why it costs no latency.** The copy rides the SUCCESSOR of the frame it protects, and the +/// client is already holding 15–90 ms of de-jitter buffer — far more than the 5 ms the successor +/// takes to arrive. So the recovery happens inside slack that already exists. +/// +/// The previous frame's sequence is implicitly `seq - 1`; a host with nothing to duplicate yet +/// (the first frame of a session, or straight after a capture reopen) simply sends an empty tail, +/// which decodes to `None`. +/// +/// Sent ONLY when the client advertised [`CLIENT_CAP_AUDIO_RED`](super::caps::CLIENT_CAP_AUDIO_RED) +/// and the host answered [`HOST_CAP_AUDIO_RED`](super::caps::HOST_CAP_AUDIO_RED) — the +/// capable-and-agreed handshake the cursor and 4:4:4 planes already use. Every other session keeps +/// the plain [`AUDIO_MAGIC`] wire byte-for-byte. +/// +/// NB `0xD1` is deliberately skipped: the DualSense pad-audio program has reserved it for the +/// per-pad audio plane. +pub const AUDIO_RED_MAGIC: u8 = 0xD2; + +/// Fixed header length of an [`AUDIO_RED_MAGIC`] datagram (tag + seq + pts + primary length). +pub const AUDIO_RED_HEADER: usize = 1 + 4 + 8 + 2; + +/// Encode a redundant audio datagram. `prev` is the immediately-preceding frame's Opus payload +/// (empty when there is none yet). +pub fn encode_audio_red_datagram(seq: u32, pts_ns: u64, opus: &[u8], prev: &[u8]) -> Vec { + let mut b = Vec::with_capacity(AUDIO_RED_HEADER + opus.len() + prev.len()); + b.push(AUDIO_RED_MAGIC); + b.extend_from_slice(&seq.to_le_bytes()); + b.extend_from_slice(&pts_ns.to_le_bytes()); + // A frame longer than u16::MAX cannot occur (5 ms of Opus is tens of bytes; the buffer the + // encoder writes into is 4 KiB) — but truncating silently would desync the split, so clamp + // the redundancy off instead of the primary. + let primary_len = u16::try_from(opus.len()).unwrap_or(u16::MAX); + b.extend_from_slice(&primary_len.to_le_bytes()); + b.extend_from_slice(opus); + if opus.len() == primary_len as usize { + b.extend_from_slice(prev); + } + b +} + +/// Parse a redundant audio datagram → `(seq, pts_ns, primary, previous)`. `previous` is `None` +/// when the host had nothing to duplicate. `None` overall on bad tag/length, including a +/// `primary_len` that overruns the datagram (a truncated or hostile packet must not panic). +/// +/// The tuple shape deliberately mirrors [`decode_audio_datagram`] (one extra slot for the +/// redundant copy) so the two planes read the same at every call site; a named struct here would +/// be the odd one out on this module's decode surface, and cbindgen would then have to be taught +/// to skip it. +#[allow(clippy::type_complexity)] +pub fn decode_audio_red_datagram(b: &[u8]) -> Option<(u32, u64, &[u8], Option<&[u8]>)> { + if b.len() < AUDIO_RED_HEADER || b[0] != AUDIO_RED_MAGIC { + return None; + } + let seq = u32::from_le_bytes(b[1..5].try_into().unwrap()); + let pts_ns = u64::from_le_bytes(b[5..13].try_into().unwrap()); + let primary_len = u16::from_le_bytes(b[13..15].try_into().unwrap()) as usize; + let rest = &b[AUDIO_RED_HEADER..]; + if primary_len > rest.len() { + return None; // truncated: the split point is outside the datagram + } + let (primary, prev) = rest.split_at(primary_len); + Some((seq, pts_ns, primary, (!prev.is_empty()).then_some(prev))) +} + /// Legacy rumble datagram (v1), host → client: `[0xCA][u16 pad LE][u16 low LE][u16 high LE]`. /// Force-feedback state for pad `pad` (0xFFFF amplitudes, 0/0 = stop) as *level-triggered* state /// — it persists until superseded, which is why the host re-sends it periodically as its loss @@ -806,6 +880,8 @@ mod tests { #[test] fn audio_datagram_roundtrip() { let opus = [0x42u8; 97]; + let d = encode_audio_red_datagram(7, 42, &opus, &[]); + assert_eq!(d[0], AUDIO_RED_MAGIC); let d = encode_audio_datagram(7, 1_000_000_123, &opus); assert_eq!(d[0], AUDIO_MAGIC); let (seq, pts, payload) = decode_audio_datagram(&d).unwrap(); @@ -820,6 +896,83 @@ mod tests { assert!(empty.is_empty()); } + #[test] + fn audio_red_datagram_roundtrip() { + let cur = [0x42u8; 97]; + let prev = [0x37u8; 88]; + let d = encode_audio_red_datagram(7, 1_000_000_123, &cur, &prev); + assert_eq!(d[0], AUDIO_RED_MAGIC); + let (seq, pts, primary, previous) = decode_audio_red_datagram(&d).unwrap(); + assert_eq!((seq, pts), (7, 1_000_000_123)); + assert_eq!(primary, cur); + assert_eq!(previous, Some(&prev[..])); + + // No predecessor yet (first frame of a session / after a capture reopen). + let d = encode_audio_red_datagram(0, 5, &cur, &[]); + let (_, _, primary, previous) = decode_audio_red_datagram(&d).unwrap(); + assert_eq!(primary, cur); + assert_eq!( + previous, None, + "an empty tail must decode as absent, not as a zero-length frame" + ); + + // Frames of equal length must still split at the right place — the length prefix is the + // only thing that can tell them apart. + let a = [1u8; 64]; + let b = [2u8; 64]; + let d = encode_audio_red_datagram(9, 0, &a, &b); + let (_, _, primary, previous) = decode_audio_red_datagram(&d).unwrap(); + assert_eq!(primary, a); + assert_eq!(previous, Some(&b[..])); + } + + /// A truncated or hostile `0xD2` must be rejected, never panic — the split point comes off + /// the wire, so an over-long `primary_len` is the obvious attack on `split_at`. + #[test] + fn audio_red_datagram_rejects_bad_input() { + let d = encode_audio_red_datagram(1, 2, &[0xAAu8; 30], &[0xBBu8; 20]); + for n in 0..AUDIO_RED_HEADER { + assert!(decode_audio_red_datagram(&d[..n]).is_none(), "len {n}"); + } + // primary_len larger than the datagram: must be refused, not sliced. + let mut bad = d.clone(); + bad[13..15].copy_from_slice(&u16::MAX.to_le_bytes()); + assert!(decode_audio_red_datagram(&bad).is_none()); + // Wrong tag. + let mut wrong = d.clone(); + wrong[0] = AUDIO_MAGIC; + assert!(decode_audio_red_datagram(&wrong).is_none()); + } + + /// The two audio planes must not alias each other or any neighbouring plane: a client + /// demultiplexes purely on the first byte. + #[test] + fn audio_red_tag_is_disjoint() { + for other in [ + AUDIO_MAGIC, + RUMBLE_MAGIC, + MIC_MAGIC, + RICH_INPUT_MAGIC, + HIDOUT_MAGIC, + HDR_META_MAGIC, + HOST_TIMING_MAGIC, + CURSOR_STATE_MAGIC, + crate::input::INPUT_MAGIC, + ] { + assert_ne!(AUDIO_RED_MAGIC, other); + } + let red = encode_audio_red_datagram(1, 2, &[9u8; 40], &[8u8; 40]); + assert!( + decode_audio_datagram(&red).is_none(), + "0xC9 must not accept a 0xD2" + ); + let plain = encode_audio_datagram(1, 2, &[9u8; 40]); + assert!( + decode_audio_red_datagram(&plain).is_none(), + "0xD2 must not accept a 0xC9" + ); + } + #[test] fn rumble_datagram_roundtrip() { let d = encode_rumble_datagram(1, 0x1234, 0xFFFF); diff --git a/include/punktfunk_core.h b/include/punktfunk_core.h index 27706ff3..cc65876f 100644 --- a/include/punktfunk_core.h +++ b/include/punktfunk_core.h @@ -312,6 +312,13 @@ // `PunktfunkStatus` code). #define PUNKTFUNK_CLIP_ERROR 6 +// The protocol's audio frame, in milliseconds — every host datagram carries exactly one +// ([`crate::quic::encode_audio_datagram`]), so it is also the smallest useful shed unit. +#define PUNKTFUNK_AUDIO_FRAME_MS 5 + +// Sample rate of every audio plane in the protocol. +#define PUNKTFUNK_AUDIO_SAMPLE_RATE_HZ 48000 + #if defined(PUNKTFUNK_FEATURE_QUIC) // The uniform no-TTL-host staleness bound: a legacy host refreshes state every 500 ms, so two // missed refreshes = quiet host → silence. Replaces the per-platform zoo (1.6 s / 60 s / 1.5 s / @@ -627,6 +634,19 @@ #define CLIENT_CAP_PHASE_LOCK 2 #endif +#if defined(PUNKTFUNK_FEATURE_QUIC) +// `Hello.client_caps` bit: this client can decode the redundant desktop-audio plane +// ([`AUDIO_RED_MAGIC`](super::datagram::AUDIO_RED_MAGIC), `0xD2`), where every datagram also +// carries a copy of the previous frame so a single lost packet is reconstructed instead of +// papered over with packet-loss concealment. +// +// Active only when the host answers with [`HOST_CAP_AUDIO_RED`] (capable-and-agreed, the +// cursor/clipboard precedent). Toward an older host, or a host that declines because the link is +// clean, the client keeps receiving the plain `0xC9` plane — so a client may always set this bit. +// `0x04` — `0x01`/`0x02` are cursor / phase-lock. +#define CLIENT_CAP_AUDIO_RED 4 +#endif + #if defined(PUNKTFUNK_FEATURE_QUIC) // [`Welcome::host_caps`] bit: the host CAN forward the cursor out-of-band (it captures cursor // metadata separately from the frame — the Linux portal `SPA_META_Cursor` path; NOT gamescope, @@ -652,6 +672,20 @@ #define HOST_CAP_PEN 16 #endif +#if defined(PUNKTFUNK_FEATURE_QUIC) +// [`Welcome::host_caps`] bit: the host is sending the REDUNDANT desktop-audio plane +// ([`AUDIO_RED_MAGIC`](super::datagram::AUDIO_RED_MAGIC), `0xD2`) instead of plain `0xC9` — each +// datagram carries its own frame plus a copy of the previous one. +// +// Set only when the client asked via [`CLIENT_CAP_AUDIO_RED`]. It is a statement about the WIRE, +// not a negotiation the client can decline: with the bit set the client must decode `0xD2`, and +// without it `0xC9`. The host may also drop back to `0xC9` mid-session (the redundancy is +// loss-gated — a clean LAN shouldn't pay for it), which is why clients decode BOTH tags +// unconditionally and treat this bit as "expect redundancy", not "only redundancy". +// `0x20` — `0x10` is [`HOST_CAP_PEN`], `0x08` is [`HOST_CAP_CURSOR`]. +#define HOST_CAP_AUDIO_RED 32 +#endif + #if defined(PUNKTFUNK_FEATURE_QUIC) // [`Hello::video_codecs`] bit: the client can decode H.264 / AVC. The GPU-less **software** // encode path (openh264) emits H.264, so a client that wants to stream from a software host MUST @@ -967,6 +1001,41 @@ #define HIDOUT_MAGIC 205 #endif +#if defined(PUNKTFUNK_FEATURE_QUIC) +// Redundant audio datagram, host → client: the [`AUDIO_MAGIC`] plane plus a copy of the PREVIOUS +// frame, so a single lost datagram is *reconstructed* rather than concealed. +// +// `[0xD2][u32 seq LE][u64 pts_ns LE][u16 primary_len LE][primary opus][previous opus]` +// +// **Why this and not Opus in-band FEC.** LBRR is a SILK-layer feature: the desktop-audio encoder +// runs `RESTRICTED_LOWDELAY` (CELT-only) at 5 ms frames, which is below SILK's 10 ms minimum, so +// `set_inband_fec(true)` on that encoder is a no-op. Nothing in libopus can protect this plane — +// the redundancy has to be at the application layer. (The mic uplink is a different encoder, VoIP +// mode at 10 ms, and *does* use real in-band FEC.) +// +// **Why it costs no latency.** The copy rides the SUCCESSOR of the frame it protects, and the +// client is already holding 15–90 ms of de-jitter buffer — far more than the 5 ms the successor +// takes to arrive. So the recovery happens inside slack that already exists. +// +// The previous frame's sequence is implicitly `seq - 1`; a host with nothing to duplicate yet +// (the first frame of a session, or straight after a capture reopen) simply sends an empty tail, +// which decodes to `None`. +// +// Sent ONLY when the client advertised [`CLIENT_CAP_AUDIO_RED`](super::caps::CLIENT_CAP_AUDIO_RED) +// and the host answered [`HOST_CAP_AUDIO_RED`](super::caps::HOST_CAP_AUDIO_RED) — the +// capable-and-agreed handshake the cursor and 4:4:4 planes already use. Every other session keeps +// the plain [`AUDIO_MAGIC`] wire byte-for-byte. +// +// NB `0xD1` is deliberately skipped: the DualSense pad-audio program has reserved it for the +// per-pad audio plane. +#define PUNKTFUNK_AUDIO_RED_MAGIC 210 +#endif + +#if defined(PUNKTFUNK_FEATURE_QUIC) +// Fixed header length of an [`AUDIO_RED_MAGIC`] datagram (tag + seq + pts + primary length). +#define PUNKTFUNK_AUDIO_RED_HEADER (((1 + 4) + 8) + 2) +#endif + #if defined(PUNKTFUNK_FEATURE_QUIC) // Wire length of a v1 (legacy, level) rumble datagram. #define RUMBLE_V1_LEN 7 @@ -1397,6 +1466,14 @@ typedef uint8_t PunktfunkInputKind; typedef struct ColorInfo ColorInfo; #endif +// Tuning for [`JitterPolicy`], in MILLISECONDS. +// +// Denominating the depth in time rather than in device quanta is the point. Every client used to +// compute its target as `3 × quantum`, which is a sane 15 ms at a 5 ms quantum and a silent 64 ms +// at a 20 ms one — the same source line meaning two very different latencies depending on what +// else happened to be using the audio graph that day. +typedef struct JitterTuning JitterTuning; + #if defined(PUNKTFUNK_FEATURE_QUIC) // Opaque handle to a live `punktfunk/1` connection (QUIC control plane + UDP data plane, all // pumped on internal threads). @@ -1788,6 +1865,14 @@ typedef struct { + + + + + + + + // The multipliers a picker offers. `1.0` (Native) is the default; the rest are the round stops // users reason about. Shared so every client's list stays identical. #define PRESETS { 0.5, 0.67, 0.75, 1.0, 1.25, 1.5, 2.0, 3.0, 4.0, } -- 2.54.0 From 3055e29ebb3dd25d0c60c5c06980a54574387f2a Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 4 Aug 2026 09:02:12 +0200 Subject: [PATCH 2/7] feat(host/audio): make audio observable, fix the endpoint choice, raise the encode quality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phases 0-3 of design/audio-quality-and-latency.md, host side. **WP2.1 — the 2026-08-03 root cause.** The client-only loopback preference took Steam's Streaming *Microphone* render endpoint over real hardware unconditionally, because it is silent on the host. But that endpoint exists to carry remote VOICE, and nothing checked whether it could carry music: on the reporter's box it won all 31 loopback opens across 25 sessions while a clean AMD HD Audio endpoint sat idle, and the whole desktop mix went through it before reaching Opus. A silent sink now has to EARN its preference — if its mix format narrows the mix it drops below real hardware. It is still taken when nothing better exists (narrow audio beats no audio), but flagged so the capture side says why. `plan_with_formats` takes a probe rather than reading WASAPI, so all 26 wiring-plan tests still run on every platform. An unknown format counts as fine, which is asserted: `unknown_formats_reproduce_the_formatless_plan` proves a probe failure can never make the plan worse than it was before formats existed. **WP0.1 — log the endpoint's ACTUAL mix format.** Everything the old log printed ("48 kHz f32 channels=2") was our REQUEST; with `autoconvert` WASAPI converts silently from whatever the endpoint really runs. That is why a 3,600-line log filed over an audio-quality complaint contained nothing that could diagnose it. **WP0.2 — count what we drop.** The capture->encode handoff was a silent lossy `try_send`: a stalled encode thread lost chunks, the encoder concatenated across the hole, and nothing recorded it — a click plus a permanent shift of everything after. Now counted and warned, alongside per-window peak/RMS/delivered% so a quiet host, a broken endpoint and a stream we are damaging ourselves stop looking identical. **WP2.4 — stop the default-device tug-of-war.** In Assert mode the capture is bound to the planned endpoint EXPLICITLY, so a hijacked default changes only where apps render — the old full reopen tore the capture down for nothing. The field log shows the cost: something re-set the default every ~4 s and each round was a teardown, a wiring pass with IPolicyConfig writes, and an audible dropout — seven in sixteen seconds, one ending in a 2 s error backoff. Now: put the default back, keep the stream, and after four rounds in twenty seconds concede for a minute and say so once. **WP1.1/1.2 — encode quality.** Constrained VBR (the hard-CBR comment justifies itself with GameStream's audio FEC, which this plane does not have) and `AudioTier::High` by default: stereo 128 -> 256 kbps, ~1 % of a 20 Mbps session. GameStream's encoder is deliberately untouched — its FEC really does need fixed-size packets. **WP3.1 — redundant `0xD2` plane**, sent when the client asked for it. **WP2.2 — `audio.output_mode`** as a first-class setting (`client_only` / `host_and_client` / `follow_default`), superseding the two undocumented env vars, which stay honoured. The enum lives in pf-host-config, which is deliberately dependency-free, so the tier table stays in core where the codec knowledge is. `capture_policy.rs` is split out for the same reason `wiring_plan.rs` is: both encode field behaviour, so their tests must run on Linux CI, not only on a Windows box. That split immediately earned itself — `capture_stats_separate_silence_from_signal` caught RMS being divided by the FRAME count while summed over interleaved SAMPLES, which inflated it by sqrt(channels) and made a sine report an RMS equal to its own peak. WP4.5 (open the loopback at the minimum device period) is deliberately NOT done: in shared mode `IAudioClient::Initialize` cannot change the engine period at all, so it would be a no-op at best and a new failure path at worst. Recorded in the code. WP2.3 (force the parked endpoint's volume) is deferred — `wasapi` keeps IMMDevice private, so it needs new raw COM on a path this tree cannot compile, let alone test; its diagnostic half ships as the RMS line above. Verified: punktfunk-host + pf-host-config clippy --all-targets -D warnings and the audio test suite under Linux/docker (gate proven non-vacuous with a planted type error); 26 wiring-plan tests standalone; fmt. The Windows-only halves of wasapi_cap.rs and audio_control.rs are NOT compile-verified anywhere yet. Co-Authored-By: Claude Opus 5 (1M context) --- crates/pf-host-config/src/lib.rs | 143 +++++++++ crates/punktfunk-core/src/audio.rs | 8 +- crates/punktfunk-host/src/audio.rs | 5 + .../src/audio/capture_policy.rs | 260 +++++++++++++++++ .../src/audio/windows/audio_control.rs | 115 +++++++- .../src/audio/windows/wasapi_cap.rs | 125 +++++++- .../punktfunk-host/src/audio/wiring_plan.rs | 274 +++++++++++++++++- crates/punktfunk-host/src/native.rs | 5 +- crates/punktfunk-host/src/native/audio.rs | 97 +++++-- crates/punktfunk-host/src/native/handshake.rs | 28 ++ 10 files changed, 1010 insertions(+), 50 deletions(-) create mode 100644 crates/punktfunk-host/src/audio/capture_policy.rs diff --git a/crates/pf-host-config/src/lib.rs b/crates/pf-host-config/src/lib.rs index 4595b726..a74ff9e3 100644 --- a/crates/pf-host-config/src/lib.rs +++ b/crates/pf-host-config/src/lib.rs @@ -57,6 +57,82 @@ pub fn env_on(name: &str) -> Option { }) } +/// Where desktop audio should be audible — which decides the render endpoint the loopback captures. +/// +/// Supersedes the two env-only knobs that used to encode this (`PUNKTFUNK_HOST_AUDIO`, +/// `PUNKTFUNK_KEEP_DEFAULT`), which stay honoured as back-compat spellings so nobody's `host.env` +/// breaks. Named modes exist because "which endpoint do we capture" is a routing decision an +/// operator has to be able to make deliberately — the 2026-08-03 field report is what happens when +/// the only way to express it is an undocumented environment variable. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum AudioOutputMode { + /// Default. Prefer a render endpoint that is silent on the host, so streamed audio does not + /// also play out of the host's speakers. Since 2026-08 a silent sink has to be able to carry + /// the mix without narrowing it — otherwise real hardware wins anyway. + #[default] + ClientOnly, + /// Prefer real hardware: audio plays on the host as well as the client. The old + /// `PUNKTFUNK_HOST_AUDIO=1`. + HostAndClient, + /// Touch nothing — capture whatever the operator's own default playback device is, and never + /// write the default-device policy. The old `PUNKTFUNK_KEEP_DEFAULT=1`. + FollowDefault, +} + +impl AudioOutputMode { + /// `PUNKTFUNK_AUDIO_OUTPUT_MODE` wins; otherwise fall back to the legacy flags, `follow_default` + /// first (it is the more restrictive promise — "do not touch my devices" must not be overridden + /// by a stale `PUNKTFUNK_HOST_AUDIO` in the same `host.env`). + fn from_env() -> AudioOutputMode { + if let Ok(raw) = std::env::var("PUNKTFUNK_AUDIO_OUTPUT_MODE") { + if !raw.trim().is_empty() { + if let Some(m) = AudioOutputMode::parse(&raw) { + return m; + } + // Never silently fall through to a different routing than the operator asked for. + eprintln!( + "punktfunk: PUNKTFUNK_AUDIO_OUTPUT_MODE={raw:?} is not one of \ + client_only/host_and_client/follow_default — using client_only" + ); + } + } + if std::env::var_os("PUNKTFUNK_KEEP_DEFAULT").is_some() { + return AudioOutputMode::FollowDefault; + } + if std::env::var_os("PUNKTFUNK_HOST_AUDIO").is_some() { + return AudioOutputMode::HostAndClient; + } + AudioOutputMode::ClientOnly + } + + pub fn parse(s: &str) -> Option { + match s.trim().to_ascii_lowercase().replace('-', "_").as_str() { + "client_only" | "client" => Some(AudioOutputMode::ClientOnly), + "host_and_client" | "both" | "host" => Some(AudioOutputMode::HostAndClient), + "follow_default" | "follow" => Some(AudioOutputMode::FollowDefault), + _ => None, + } + } + + pub fn as_str(self) -> &'static str { + match self { + AudioOutputMode::ClientOnly => "client_only", + AudioOutputMode::HostAndClient => "host_and_client", + AudioOutputMode::FollowDefault => "follow_default", + } + } + + /// The loopback plan should prefer real hardware over a silent sink. + pub fn prefers_host_hardware(self) -> bool { + matches!(self, AudioOutputMode::HostAndClient) + } + + /// Leave the operator's default playback/recording devices completely alone. + pub fn keeps_default(self) -> bool { + matches!(self, AudioOutputMode::FollowDefault) + } +} + /// Resolved host configuration. Holds the genuinely-constant operator/dispatch knobs (see module docs for /// what is deliberately excluded). Fields read on only one platform are kept alive cross-platform by the /// derived `Debug` impl, so the parser can stay a single platform-neutral function. @@ -99,6 +175,24 @@ pub struct HostConfig { /// e.g. webOS TVs, whose GCM decrypt caps at ~100 Mbps); everyone else stays AES-128-GCM. /// `PUNKTFUNK_CHACHA20=0`/`false`/`off`/`no` disables. pub chacha20: bool, + /// `PUNKTFUNK_AUDIO_OUTPUT_MODE` — where desktop audio should be audible, and therefore which + /// render endpoint the loopback captures (`client_only` / `host_and_client` / `follow_default`). + /// + /// A first-class setting because the 2026-08-03 field report needed one: the default + /// client-only routing sent that box's whole desktop mix through Steam's voice-carrier virtual + /// endpoint for 25 sessions, and the only way to change it was an undocumented environment + /// variable. See [`AudioOutputMode`]. + pub audio_output_mode: AudioOutputMode, + /// `PUNKTFUNK_AUDIO_QUALITY` — desktop-audio encode tier (`low` / `standard` / `high`; default + /// `high`). Kept as the raw string here because the tier table lives in `punktfunk-core`, and + /// this crate is deliberately dependency-free (see the crate doc). The audio thread resolves it + /// via `punktfunk_core::audio::AudioTier::parse` and warns on an unknown spelling rather than + /// silently downgrading someone's audio. + pub audio_quality: Option, + /// `PUNKTFUNK_AUDIO_REDUNDANCY` — force the redundant `0xD2` audio plane on or off. `None` + /// (the default) = automatic: sent only to a client that asked for it, and only while the link + /// is actually losing packets. + pub audio_redundancy: Option, /// `PUNKTFUNK_PERF` — per-stage timing instrumentation. pub perf: bool, /// `PUNKTFUNK_VIDEO_SOURCE` — GameStream video source select. `virtual` (the default — a @@ -246,6 +340,9 @@ impl HostConfig { // Default ON, explicit-off grammar (the client's VIDEO_CAP_CHACHA20 bit is the real // per-session switch; see the field doc). chacha20: env_on("PUNKTFUNK_CHACHA20").unwrap_or(true), + audio_output_mode: AudioOutputMode::from_env(), + audio_quality: val("PUNKTFUNK_AUDIO_QUALITY").map(|s| s.trim().to_lowercase()), + audio_redundancy: env_on("PUNKTFUNK_AUDIO_REDUNDANCY"), perf: flag("PUNKTFUNK_PERF"), // Default ON while the interval-stutter field program runs (see the field doc). stall_probes: env_on("PUNKTFUNK_STALL_PROBES").unwrap_or(true), @@ -348,4 +445,50 @@ mod tests { // An invalid rate stays invalid rather than being laundered into a real one. assert_eq!(c.game_fps(0), 0); } + + #[test] + fn audio_output_mode_parses_its_spellings() { + for (s, want) in [ + ("client_only", AudioOutputMode::ClientOnly), + ("client-only", AudioOutputMode::ClientOnly), + (" CLIENT ", AudioOutputMode::ClientOnly), + ("host_and_client", AudioOutputMode::HostAndClient), + ("both", AudioOutputMode::HostAndClient), + ("follow_default", AudioOutputMode::FollowDefault), + ("follow", AudioOutputMode::FollowDefault), + ] { + assert_eq!(AudioOutputMode::parse(s), Some(want), "{s:?}"); + } + // Unknown spellings are rejected so the caller can say so, not silently re-routed. + for s in ["", "silent", "off", "true"] { + assert_eq!(AudioOutputMode::parse(s), None, "{s:?}"); + } + // Round-trip through the canonical spelling. + for m in [ + AudioOutputMode::ClientOnly, + AudioOutputMode::HostAndClient, + AudioOutputMode::FollowDefault, + ] { + assert_eq!(AudioOutputMode::parse(m.as_str()), Some(m)); + } + } + + /// The two predicates are what the wiring plan and the capture loop actually branch on, and + /// they must stay mutually exclusive: "prefer host hardware" and "touch nothing" are different + /// promises, and conflating them would either silence the host or stomp the operator's devices. + #[test] + fn audio_output_mode_predicates_are_disjoint() { + assert_eq!(AudioOutputMode::default(), AudioOutputMode::ClientOnly); + for m in [ + AudioOutputMode::ClientOnly, + AudioOutputMode::HostAndClient, + AudioOutputMode::FollowDefault, + ] { + assert!(!(m.prefers_host_hardware() && m.keeps_default()), "{m:?}"); + } + assert!(AudioOutputMode::HostAndClient.prefers_host_hardware()); + assert!(AudioOutputMode::FollowDefault.keeps_default()); + assert!(!AudioOutputMode::ClientOnly.prefers_host_hardware()); + assert!(!AudioOutputMode::ClientOnly.keeps_default()); + } } diff --git a/crates/punktfunk-core/src/audio.rs b/crates/punktfunk-core/src/audio.rs index 6975b9de..a006fc51 100644 --- a/crates/punktfunk-core/src/audio.rs +++ b/crates/punktfunk-core/src/audio.rs @@ -57,8 +57,12 @@ pub struct OpusLayout { pub coupled: u8, /// libopus multistream channel mapping — identity `[0, 1, …, channels-1]`. pub mapping: &'static [u8], - /// Target Opus bitrate in bits/sec (hard CBR; constant packet size, which GameStream's - /// audio FEC relies on). + /// Target Opus bitrate in bits/sec at [`AudioTier::Standard`] — see + /// [`OpusLayout::bitrate_for`], which is what callers should use. These are the historical + /// values, kept exactly so `Standard` reproduces the pre-tier wire byte-for-byte. + /// + /// The GameStream plane encodes hard-CBR from these (its audio FEC needs a constant packet + /// size); the native plane uses constrained VBR, where that constraint does not apply. pub bitrate: i32, } diff --git a/crates/punktfunk-host/src/audio.rs b/crates/punktfunk-host/src/audio.rs index 0df8b571..b4026038 100644 --- a/crates/punktfunk-host/src/audio.rs +++ b/crates/punktfunk-host/src/audio.rs @@ -192,6 +192,11 @@ mod wasapi_mic; #[cfg_attr(not(target_os = "windows"), allow(dead_code))] #[path = "audio/wiring_plan.rs"] pub(crate) mod wiring_plan; +// Pure capture-loop policy, split out for the same reason `wiring_plan` is: it encodes field +// behaviour, so its tests must run on every platform's CI, not only Windows. +#[cfg_attr(not(target_os = "windows"), allow(dead_code))] +#[path = "audio/capture_policy.rs"] +pub(crate) mod capture_policy; mod mic_jitter; mod mic_pump; diff --git a/crates/punktfunk-host/src/audio/capture_policy.rs b/crates/punktfunk-host/src/audio/capture_policy.rs new file mode 100644 index 00000000..872f6f83 --- /dev/null +++ b/crates/punktfunk-host/src/audio/capture_policy.rs @@ -0,0 +1,260 @@ +//! Desktop-audio capture POLICY — the parts of [`wasapi_cap`](super::wasapi_cap) that are pure +//! decisions rather than WASAPI plumbing, split out for the same reason +//! [`wiring_plan`](super::wiring_plan) is: so they compile and their unit tests RUN on every +//! platform. Both of these encode field-report behaviour, and regressing either must fail CI on +//! Linux too, not only on a Windows box. +//! +//! * [`FightDamper`] — how hard to fight another program for the default playback device. +//! * [`CaptureStats`] — the audio plane's vitals, so a log can tell a quiet host from a broken +//! endpoint from one we are damaging ourselves. + +use std::time::{Duration, Instant}; + +/// Default-playback re-assertions inside [`FIGHT_WINDOW`] before we stop fighting. +pub(crate) const FIGHT_LIMIT: u32 = 4; +pub(crate) const FIGHT_WINDOW: Duration = Duration::from_secs(20); +/// How long to leave the default alone once another program has proven it will take it back. +pub(crate) const FIGHT_BACKOFF: Duration = Duration::from_secs(60); + +/// Damping for the default-playback tug-of-war (WP2.4). +/// +/// The 2026-08-03 field log recorded seven full re-assert cycles in sixteen seconds — something on +/// that box re-set the default playback to CABLE Input every ~4 s and we snapped it back every +/// time, each round a capture teardown plus a wiring pass with `IPolicyConfig` writes. Winning that +/// argument is not possible and every round was an audible dropout, so: re-assert a few times +/// (transient churn does settle), then concede for a minute and say so once. +/// +/// Time is passed IN rather than read here, which keeps the policy pure and testable. +pub(crate) struct FightDamper { + /// Re-assertions in the current window, and when the window opened. + count: u32, + window_started: Instant, + /// Set while we are deliberately not fighting. + paused_until: Option, + /// One warning per fight burst, and one per concession. + warned_fighting: bool, + warned_giving_up: bool, + now: Instant, +} + +impl FightDamper { + pub(crate) fn new(now: Instant) -> FightDamper { + FightDamper { + count: 0, + window_started: now, + paused_until: None, + warned_fighting: false, + warned_giving_up: false, + now, + } + } + + /// A dud default-device change was observed at `now`. + pub(crate) fn observed_at(&mut self, now: Instant) { + self.now = now; + if now.duration_since(self.window_started) >= FIGHT_WINDOW { + self.window_started = now; + self.count = 0; + self.warned_fighting = false; + } + if self.paused_until.is_some_and(|t| now >= t) { + self.paused_until = None; + self.warned_giving_up = false; + self.count = 0; + self.window_started = now; + } + } + + /// Should we put the default back? False while paused, or once this window's budget is spent. + pub(crate) fn should_reassert(&mut self) -> bool { + if self.paused_until.is_some() { + return false; + } + if self.count >= FIGHT_LIMIT { + self.paused_until = Some(self.now + FIGHT_BACKOFF); + return false; + } + self.count += 1; + true + } + + /// Warn on the FIRST re-assert of a burst only (the rest are noise). + pub(crate) fn warn_now(&mut self) -> bool { + !std::mem::replace(&mut self.warned_fighting, true) + } + + /// Warn once when we concede. + pub(crate) fn warn_giving_up(&mut self) -> bool { + self.paused_until.is_some() && !std::mem::replace(&mut self.warned_giving_up, true) + } + + /// Currently conceding (test/diagnostic accessor). + pub(crate) fn is_paused(&self) -> bool { + self.paused_until.is_some() + } +} + +/// How often the capture loop reports its vitals (WP0.2). +pub(crate) const STATS_EVERY: Duration = Duration::from_secs(30); + +/// One reporting window's worth of capture vitals. +/// +/// The point is to make three states that used to look identical in a log tell themselves apart: a +/// genuinely quiet host (`peak` ~0, no drops), a working stream (`peak` > 0), and a stream we are +/// damaging ourselves (`dropped_chunks` > 0). The 2026-08-03 field log — 3,600 lines, filed over an +/// audio-quality complaint — could distinguish none of them, because the audio plane logged nothing +/// at all between "capturing" and the session ending. +#[derive(Default)] +pub(crate) struct CaptureStats { + pub(crate) frames: u64, + /// Interleaved SAMPLES seen — the RMS denominator. Deliberately separate from `frames`: + /// dividing the sum of squares by the frame count instead inflates RMS by sqrt(channels), + /// which made a sine report an RMS equal to its own peak. + pub(crate) samples: u64, + /// Loudest |sample| in the window — tells a silent endpoint from a working one. + pub(crate) peak: f32, + /// Sum of squares, for the window's RMS: a level far below peak means a badly attenuated + /// endpoint (a parked device sitting at 20 % volume costs ~14 dB before Opus ever sees it). + pub(crate) sumsq: f64, + /// Chunks the encode thread was too slow to take. Silent data loss, previously uncounted: + /// the encoder simply concatenates across the hole, so it is a click AND a permanent shift of + /// everything after it. + pub(crate) dropped_chunks: u64, +} + +impl CaptureStats { + pub(crate) fn observe(&mut self, samples: &[f32], channels: u32) { + self.frames += (samples.len() / channels.max(1) as usize) as u64; + self.samples += samples.len() as u64; + for &s in samples { + let a = s.abs(); + if a > self.peak { + self.peak = a; + } + self.sumsq += (s as f64) * (s as f64); + } + } + + /// `(peak dBFS, rms dBFS, delivered %)` for this window. Silence reports -120 dB rather than + /// -inf so the log line stays parseable. + pub(crate) fn summary(&self, elapsed: Duration, sample_rate: u32) -> (f64, f64, f64) { + let rms = (self.sumsq / (self.samples as f64).max(1.0)).sqrt(); + let db = |v: f64| if v > 0.0 { 20.0 * v.log10() } else { -120.0 }; + // Expected frames for the window — a shortfall means the endpoint is not delivering at + // real time (a stalling virtual device), which a peak/RMS alone cannot show. + let expected = elapsed.as_secs_f64() * sample_rate as f64; + ( + db(self.peak as f64), + db(rms), + (self.frames as f64 / expected.max(1.0)) * 100.0, + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Replays the 2026-08-03 field shape: a dud default change every ~2 s, forever. We must put + /// the default back a few times, then concede — and warn exactly once for each. + #[test] + fn fight_damper_concedes_instead_of_looping_forever() { + let t0 = Instant::now(); + let mut d = FightDamper::new(t0); + let mut reasserts = 0; + let (mut warns_fighting, mut warns_giving_up) = (0, 0); + for i in 0..8 { + d.observed_at(t0 + Duration::from_millis(i * 2_000)); + if d.should_reassert() { + reasserts += 1; + if d.warn_now() { + warns_fighting += 1; + } + } else if d.warn_giving_up() { + warns_giving_up += 1; + } + } + assert_eq!( + reasserts, FIGHT_LIMIT, + "must stop after the window's budget" + ); + assert_eq!(warns_fighting, 1, "one warning per burst, not one per flip"); + assert_eq!(warns_giving_up, 1, "concede exactly once"); + } + + /// Occasional, genuinely transient churn must ALWAYS be corrected — the damper must not + /// accumulate across widely-spaced events and quietly stop doing its job. + #[test] + fn fight_damper_always_fixes_isolated_changes() { + let t0 = Instant::now(); + let mut d = FightDamper::new(t0); + let mut reasserts = 0; + for i in 1..=10 { + d.observed_at(t0 + FIGHT_WINDOW * i); + if d.should_reassert() { + reasserts += 1; + } + } + assert_eq!(reasserts, 10, "isolated changes must always be corrected"); + } + + /// After the backoff expires the damper re-arms, so a program that goes quiet and comes back + /// later is fought again rather than being conceded to for the rest of the session. + #[test] + fn fight_damper_rearms_after_the_backoff() { + let t0 = Instant::now(); + let mut d = FightDamper::new(t0); + for i in 0..FIGHT_LIMIT + 2 { + d.observed_at(t0 + Duration::from_millis(i as u64 * 500)); + d.should_reassert(); + } + assert!(d.is_paused(), "should have conceded"); + d.observed_at(t0 + FIGHT_BACKOFF + FIGHT_WINDOW * 2); + assert!(d.should_reassert(), "must re-arm once the backoff expires"); + } + + /// Peak/RMS must separate the states a log could not previously tell apart. + #[test] + fn capture_stats_separate_silence_from_signal() { + let mut quiet = CaptureStats::default(); + quiet.observe(&[0.0; 480], 2); + let (peak, rms, _) = quiet.summary(Duration::from_secs(1), 48_000); + assert_eq!(peak, -120.0, "digital silence reports the floor, not -inf"); + assert_eq!(rms, -120.0); + + let mut loud = CaptureStats::default(); + let tone: Vec = (0..480).map(|i| (i as f32 * 0.13).sin() * 0.5).collect(); + loud.observe(&tone, 2); + assert_eq!( + loud.frames, 240, + "480 interleaved stereo samples = 240 frames" + ); + let (peak, rms, _) = loud.summary(Duration::from_secs(1), 48_000); + assert!( + peak > -8.0 && peak <= 0.0, + "peak {peak} dBFS should track a 0.5 tone" + ); + // A sine's RMS is its amplitude / sqrt(2) — about 3 dB below peak. Getting this equal to + // peak is exactly what a frames-vs-samples mix-up in the denominator looks like, so the + // margin is asserted rather than just the ordering. + assert!( + rms < peak - 2.0, + "RMS {rms} vs peak {peak}: a sine must sit ~3 dB below its peak" + ); + } + + /// The delivered-percentage is what shows an endpoint that has stopped feeding us in real + /// time — invisible in peak/RMS, and the shape a stalling virtual device makes. + #[test] + fn capture_stats_report_a_delivery_shortfall() { + let mut full = CaptureStats::default(); + full.observe(&vec![0.1f32; 48_000 * 2], 2); // exactly 1 s of stereo + let (_, _, pct) = full.summary(Duration::from_secs(1), 48_000); + assert!((pct - 100.0).abs() < 1.0, "expected ~100 %, got {pct}"); + + let mut half = CaptureStats::default(); + half.observe(&vec![0.1f32; 48_000], 2); // 0.5 s of stereo in a 1 s window + let (_, _, pct) = half.summary(Duration::from_secs(1), 48_000); + assert!((pct - 50.0).abs() < 1.0, "expected ~50 %, got {pct}"); + } +} diff --git a/crates/punktfunk-host/src/audio/windows/audio_control.rs b/crates/punktfunk-host/src/audio/windows/audio_control.rs index 2760a8f1..e74341ec 100644 --- a/crates/punktfunk-host/src/audio/windows/audio_control.rs +++ b/crates/punktfunk-host/src/audio/windows/audio_control.rs @@ -14,8 +14,11 @@ //! * default **PLAYBACK** → the plan's loopback endpoint, applied ONLY while a desktop-audio capture //! is open (`set_playback` — the mic pump must never park the playback default while the host is //! idle). By default that endpoint is the SILENT sink (Steam Streaming Microphone render side) so -//! audio plays on the client only; `PUNKTFUNK_HOST_AUDIO` prefers real hardware instead (audible on -//! both ends). **Never** the Steam Streaming Speakers, whose loopback is silent — validated live; +//! audio plays on the client only; `audio.output_mode = host_and_client` (formerly +//! `PUNKTFUNK_HOST_AUDIO`) prefers real hardware instead (audible on both ends). Since 2026-08 a +//! silent sink must also be able to CARRY the mix — one that narrows it (a voice-carrier endpoint +//! mixing mono or at 24 kHz) loses to real hardware; see [`super::wiring_plan`]. **Never** the +//! Steam Streaming Speakers, whose loopback is silent — validated live; //! * default **RECORDING** → the mic target's capture endpoint (VB-Cable "CABLE Output") so host apps //! record the client's mic by default. //! @@ -33,18 +36,44 @@ //! //! Setting a default endpoint uses the undocumented `IPolicyConfig` COM interface (the only way to set //! a default device programmatically — neither the `windows` nor `wasapi` crate exposes it; it is the -//! same call `mmsys.cpl` makes). Opt out with `PUNKTFUNK_KEEP_DEFAULT` to leave the user's chosen -//! defaults untouched (the plan is still computed — the mic must still pick a target). +//! same call `mmsys.cpl` makes). The `audio.output_mode = follow_default` setting (formerly +//! `PUNKTFUNK_KEEP_DEFAULT`) leaves the user's chosen defaults untouched — the plan is still +//! computed, since the mic must still pick a target. // Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it. #![deny(clippy::undocumented_unsafe_blocks)] -use super::wiring_plan::{self, plan, Endpoint, Wiring}; +use super::wiring_plan::{self, plan, plan_with_formats, Endpoint, MixFormat, Wiring}; use anyhow::{anyhow, bail, Result}; use std::ffi::c_void; use std::sync::Mutex; use wasapi::Direction; +/// A render endpoint's engine mix format, or `None` if it cannot be asked right now. +/// +/// This is the number the 2026-08-03 field report needed and no log had: the capture side requests +/// 48 kHz f32 with `autoconvert`, so WASAPI converts silently from whatever the endpoint really +/// runs — and a voice-carrier endpoint (Steam's Streaming Microphone) narrowing the desktop mix to +/// mono or 24 kHz was invisible. Reading it costs one `IAudioClient` activation per endpoint, done +/// only during a wiring pass. +/// +/// Deliberately total: EVERY failure maps to `None` ("assume it is fine"), because the wiring plan +/// treats an unknown format as non-narrowing. A box where activation fails therefore plans exactly +/// as it did before formats existed, instead of mis-demoting a perfectly good endpoint. +fn mix_format_of(ep: &Endpoint) -> Option { + let fmt = open_endpoint(ep) + .ok()? + .get_iaudioclient() + .ok()? + .get_mixformat() + .ok()?; + Some(MixFormat { + rate_hz: fmt.get_samplespersec(), + channels: fmt.get_nchannels(), + bits: fmt.get_bitspersample(), + }) +} + /// `(friendly_name, endpoint_id)` for every ACTIVE endpoint in direction `dir`. fn list_endpoints(dir: Direction) -> Vec { let mut out = Vec::new(); @@ -69,10 +98,22 @@ fn list_endpoints(dir: Direction) -> Vec { out } -/// `PUNKTFUNK_HOST_AUDIO`: the operator wants the stream audible on the host too — the loopback -/// plan prefers real hardware over the silent sink (the pre-client-only-default behavior). +/// The operator wants the stream audible on the host too — the loopback plan prefers real +/// hardware over the silent sink (the pre-client-only-default behavior). +/// +/// Now driven by the first-class `audio.output_mode` setting +/// ([`AudioOutputMode`](pf_host_config::AudioOutputMode)), which still honours the older +/// `PUNKTFUNK_HOST_AUDIO` spelling. pub(crate) fn host_audio_requested() -> bool { - std::env::var_os("PUNKTFUNK_HOST_AUDIO").is_some() + pf_host_config::config() + .audio_output_mode + .prefers_host_hardware() +} + +/// The operator's default playback/recording devices must not be touched at all — the +/// `follow_default` mode, formerly `PUNKTFUNK_KEEP_DEFAULT`. +pub(crate) fn keep_default_devices() -> bool { + pf_host_config::config().audio_output_mode.keeps_default() } /// One wiring pass plus the inputs the desktop-audio capture loop's failure handling needs: @@ -118,7 +159,27 @@ pub(crate) fn wire_now_full(set_playback: bool) -> WiredPlan { let want = std::env::var("PUNKTFUNK_MIC_DEVICE") .ok() .map(|s| s.to_lowercase()); - let wiring = plan(&renders, &captures, want.as_deref(), host_audio_requested()); + // Mix formats are read only when we are actually going to park the playback default (i.e. a + // desktop-audio capture is opening). The mic pump wires on every open while the host is idle + // and does not care which loopback endpoint wins, so it must not pay an IAudioClient + // activation per render endpoint on every pass. + let probe: &dyn Fn(&Endpoint) -> Option = if set_playback { + &mix_format_of + } else { + &wiring_plan::no_formats + }; + let wiring = plan_with_formats( + &renders, + &captures, + want.as_deref(), + host_audio_requested(), + probe, + // The loopback is opened at the session's negotiated channel count, but the wiring pass + // runs before (and outside) any session. Stereo is the floor every session uses and the + // only count a *narrowing* verdict can be made against without guessing: an endpoint that + // cannot carry stereo cannot carry 5.1 either. + 2, + ); let done = |wiring: Wiring| WiredPlan { wiring, fingerprint, @@ -142,6 +203,18 @@ pub(crate) fn wire_now_full(set_playback: bool) -> WiredPlan { renders = ?renders.iter().map(|(n, _)| n.as_str()).collect::>(), "audio wiring plan" ); + // The quality warning the 2026-08-03 report had no way to produce. Says WHICH endpoint, + // WHY it is narrow, and the two things the operator can actually do about it. + if let (Some(why), Some((name, _))) = (&wiring.loopback_narrowing, &wiring.loopback_render) + { + tracing::warn!( + device = %name, + "the desktop-audio loopback endpoint {why} — streamed audio will sound worse \ + than it does on the host. Attach or select a 48 kHz stereo output device, or \ + set audio.output_mode = host_and_client (PUNKTFUNK_HOST_AUDIO=1) to prefer \ + real hardware" + ); + } if wiring.mic_render.is_some() && wiring.loopback_unsatisfiable() { // Inventory + per-endpoint reasons + ONLY the remedies not already taken — the old // static advice here suggested installing the Steam pair to a field box that had it @@ -153,10 +226,11 @@ pub(crate) fn wire_now_full(set_playback: bool) -> WiredPlan { } } - if std::env::var_os("PUNKTFUNK_KEEP_DEFAULT").is_some() { + if keep_default_devices() { if changed { tracing::info!( - "PUNKTFUNK_KEEP_DEFAULT set — leaving the audio default devices untouched" + mode = %pf_host_config::config().audio_output_mode.as_str(), + "audio output mode is follow_default — leaving the audio default devices untouched" ); } return done(wiring); @@ -317,6 +391,25 @@ fn park_default_playback(name: &str, id: &str, changed: bool, mic_id: Option<&st } } +/// Put the default playback device back on the endpoint we are already capturing, WITHOUT a +/// wiring pass (WP2.4). +/// +/// The capture loop uses this when something else takes the default mid-stream: in Assert mode the +/// capture is bound to the planned endpoint explicitly, so the only thing a hijacked default +/// changes is where *apps* render — one `IPolicyConfig` write fixes that, where the old path tore +/// the capture down and re-ran the whole wiring pass. Deliberately does not touch the [`PARKED`] +/// memo: the endpoint is the one we already parked, so the operator's original default is +/// unchanged and still owed back at stream end. +pub(crate) fn reassert_default_playback(id: &str) -> bool { + match set_default_endpoint(id) { + Ok(()) => true, + Err(e) => { + tracing::debug!(error = %format!("{e:#}"), "failed to re-assert the default playback device"); + false + } + } +} + /// Put the operator's default playback device back after streaming — the inverse of /// [`park_default_playback`]. No-op if we never parked it, and a default the operator changed /// themselves mid-stream is left alone (their choice wins). Must run on a COM-initialized thread diff --git a/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs b/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs index 0e985754..7b21c3bb 100644 --- a/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs +++ b/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs @@ -27,6 +27,7 @@ //! succeed). On thread exit (capturer dropped at stream end) the parked default playback //! device is restored. +use super::capture_policy::{CaptureStats, FightDamper, FIGHT_BACKOFF, STATS_EVERY}; use super::{audio_control, wiring_plan, AudioCapturer, SAMPLE_RATE}; use anyhow::{anyhow, Context, Result}; use std::collections::VecDeque; @@ -359,7 +360,7 @@ fn capture_once( ) -> Result { // Interleaved f32: channels * 4 bytes per frame. let block_align = channels as usize * 4; - let keep_default = std::env::var_os("PUNKTFUNK_KEEP_DEFAULT").is_some(); + let keep_default = audio_control::keep_default_devices(); // Assert-mode without KEEP_DEFAULT is the only shape that parks the playback default. let assert_plan = mode == TargetMode::Assert && !keep_default; let mut plan = audio_control::wire_now_full(assert_plan); @@ -454,12 +455,25 @@ fn capture_once( channels as usize, Some(mask), ); - let (default_period, _min_period) = - audio_client.get_device_period().context("device period")?; + // WP0.1 — the endpoint's ACTUAL engine mix format, read BEFORE we initialize. Everything the + // old log printed ("48 kHz f32 channels=2") was our REQUEST; with `autoconvert` WASAPI + // silently converts from whatever the endpoint really runs, so a voice-carrier endpoint + // narrowing the desktop mix to mono or 24 kHz was invisible in a 3,600-line field log. This + // line is what makes an audio-quality report triageable without a round trip. + let engine = audio_client.get_mixformat().ok(); + // NB the plan's WP4.5 ("open the loopback at the MINIMUM device period, worth ~5–10 ms") is + // deliberately NOT done here, because its premise is wrong: in shared mode + // `IAudioClient::Initialize` cannot change the engine period at all — `hnsBufferDuration` sizes + // the buffer, and the callback still fires at the engine's fixed default period. Lowering it + // needs `IAudioClient3::InitializeSharedAudioStream`, which the `wasapi` crate does not wrap. + // Passing `min_period` here would therefore be a no-op at best and a new Initialize failure + // path at worst, on a device this tree cannot compile for, let alone test. Left as real work. + let (default_period, min_period) = audio_client.get_device_period().context("device period")?; let stream_mode = StreamMode::EventsShared { autoconvert: true, buffer_duration_hns: default_period, }; + let used_period = default_period; audio_client .initialize_client(&desired, &Direction::Capture, &stream_mode) .context("initialize loopback client")?; @@ -476,7 +490,17 @@ fn capture_once( tracing::info!(device = %dev_name, follow = matches!(mode, TargetMode::Follow) || keep_default, last_resort, + // The endpoint's own format — NOT the one we asked for. + engine_hz = engine.as_ref().map(|f| f.get_samplespersec()), + engine_ch = engine.as_ref().map(|f| f.get_nchannels()), + engine_bits = engine.as_ref().map(|f| f.get_bitspersample()), + buffer_ms = used_period as f32 / 10_000.0, + min_buffer_ms = min_period as f32 / 10_000.0, "audio loopback capturing"); + if let Some(why) = &wiring.loopback_narrowing { + tracing::warn!(device = %dev_name, + "capturing an endpoint that {why} — the stream cannot sound better than this source"); + } // Watchdog seed: the default as it stands right after our open. In Assert mode the plan just // parked the default on our endpoint — if it did NOT stick (IPolicyConfig denied) converge @@ -514,6 +538,15 @@ fn capture_once( let opened_at = Instant::now(); let mut saw_packets = false; let mut silence_noted = false; + // WP0.2 — the audio plane's own vitals, logged periodically. Before this, a host log said + // nothing whatsoever about audio between "capturing" and the session ending: no level, no + // cadence, and in particular no sign of the SILENT, uncounted drop below, where a stalled + // encode thread loses chunks and the encoder simply concatenates across the hole (a click, + // and a permanent A/V offset, with nothing in any log). + let mut stats = CaptureStats::default(); + let mut last_stats = Instant::now(); + // WP2.4 — damping for the default-playback tug-of-war. + let mut fight = FightDamper::new(Instant::now()); loop { if stop.load(Ordering::Relaxed) { audio_client.stop_stream().ok(); @@ -556,7 +589,34 @@ fn capture_once( for c in raw.chunks_exact(4) { samples.push(f32::from_le_bytes([c[0], c[1], c[2], c[3]])); } - let _ = tx.try_send(samples); // non-blocking, lossy — same discipline as PipeWire + stats.observe(&samples, channels); + // Non-blocking, lossy — same discipline as PipeWire. Now COUNTED: a full channel + // means the encode thread is not keeping up, and every dropped chunk is a click plus + // a permanent shift of everything after it. + if tx.try_send(samples).is_err() { + stats.dropped_chunks += 1; + } + } + if last_stats.elapsed() >= STATS_EVERY { + let (peak_db, rms_db, delivered_pct) = stats.summary(last_stats.elapsed(), SAMPLE_RATE); + if stats.dropped_chunks > 0 { + tracing::warn!( + device = %dev_name, + dropped_chunks = stats.dropped_chunks, + "the audio encode thread could not keep up — captured audio was DROPPED; the \ + stream will click and everything after it shifts" + ); + } + tracing::info!( + device = %dev_name, + peak_db = format!("{peak_db:.1}"), + rms_db = format!("{rms_db:.1}"), + delivered_pct = format!("{delivered_pct:.0}"), + dropped_chunks = stats.dropped_chunks, + "desktop audio capture" + ); + last_stats = Instant::now(); + stats = CaptureStats::default(); } // Watchdog: react when the default render device CHANGES from what we last observed — @@ -568,29 +628,68 @@ fn capture_once( if seen_default.as_deref() != Some(nid.as_str()) { seen_default = Some(nid.clone()); if nid != dev_id { - audio_client.stop_stream().ok(); + // NB the stream is stopped per-branch below, NOT here: the WP2.4 Dud + // path deliberately keeps capturing, and stopping first would have made + // the "no teardown" fix silently useless. if keep_default { + audio_client.stop_stream().ok(); tracing::info!( "default render device changed (PUNKTFUNK_KEEP_DEFAULT) — \ following it" ); return Ok(Next::Reopen(TargetMode::Follow)); } - return Ok(match judge_default(&en, wiring, &nid) { + match judge_default(&en, wiring, &nid) { DefaultKind::Capturable(name) => { + audio_client.stop_stream().ok(); tracing::info!(device = %name, "operator changed the output device mid-stream — following \ it (audio now also plays on the host)"); - Next::Reopen(TargetMode::Follow) + return Ok(Next::Reopen(TargetMode::Follow)); } + // WP2.4 — a DUD default does not affect what we are capturing: + // Assert mode binds the capture to the plan's endpoint EXPLICITLY, + // not to whatever the default happens to be. Only where *apps* + // render has moved. So put the default back and KEEP THE STREAM — + // the old full reopen tore the capture down for nothing, and the + // 2026-08-03 field log shows what that cost: something re-set the + // default to CABLE Input every ~4 s and each round trip was a + // teardown, a re-plan with IPolicyConfig writes, and an audible + // dropout — seven of them in sixteen seconds, one ending in a 2 s + // error backoff. DefaultKind::Dud(name) => { - tracing::warn!(device = %name, - "default playback moved to an endpoint whose loopback cannot \ - work — re-asserting the audio wiring plan"); - Next::Reopen(TargetMode::Assert) + if !assert_plan { + // Follow/KEEP_DEFAULT shapes still need the old behaviour: + // there the capture IS bound to the default. + audio_client.stop_stream().ok(); + return Ok(Next::Reopen(TargetMode::Assert)); + } + fight.observed_at(Instant::now()); + if fight.should_reassert() { + audio_control::reassert_default_playback(&dev_id); + // Believe our own write: the next watchdog tick sees the + // default back on our endpoint and stays quiet. + seen_default = Some(dev_id.clone()); + if fight.warn_now() { + tracing::warn!(device = %name, planned = %dev_name, + "something keeps moving the default playback to an \ + endpoint whose loopback cannot work — putting it \ + back (the capture is unaffected)"); + } + } else if fight.warn_giving_up() { + tracing::warn!(device = %name, planned = %dev_name, + backoff_s = FIGHT_BACKOFF.as_secs(), + "another program is repeatedly taking the default \ + playback device — backing off rather than fighting it. \ + Desktop audio keeps streaming from the planned endpoint, \ + but apps rendering to the other device will not be heard"); + } } - DefaultKind::Unknown => Next::Reopen(TargetMode::Assert), - }); + DefaultKind::Unknown => { + audio_client.stop_stream().ok(); + return Ok(Next::Reopen(TargetMode::Assert)); + } + } } } } diff --git a/crates/punktfunk-host/src/audio/wiring_plan.rs b/crates/punktfunk-host/src/audio/wiring_plan.rs index 15bdca75..bbecd1ab 100644 --- a/crates/punktfunk-host/src/audio/wiring_plan.rs +++ b/crates/punktfunk-host/src/audio/wiring_plan.rs @@ -43,6 +43,59 @@ /// A `(friendly_name, endpoint_id)` pair as enumerated from WASAPI. pub(crate) type Endpoint = (String, String); +/// A render endpoint's ENGINE MIX FORMAT, as `IAudioClient::GetMixFormat` reports it. +/// +/// This is the number the 2026-08-03 field report needed and the log did not have. The capture +/// side opens with `autoconvert: true` and asks for 48 kHz f32 in the wire layout, so WASAPI +/// silently converts whatever the endpoint really runs — and the "48 kHz f32 channels=2" we +/// logged was our REQUEST, not the source. An endpoint that mixes at 24 kHz mono therefore +/// produced a 48 kHz stereo stream that had already been through a 24 kHz mono bottleneck, with +/// nothing in any log to say so. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct MixFormat { + pub rate_hz: u32, + pub channels: u16, + pub bits: u16, +} + +impl MixFormat { + /// Why this endpoint would NARROW a `want`-channel desktop mix, or `None` if it carries it + /// intact. Bit depth is deliberately not a criterion: 16-bit is ~96 dB of headroom, far below + /// Opus's own noise floor, whereas a lost channel or halved bandwidth is plainly audible. + pub(crate) fn narrowing(&self, want: u8) -> Option { + if self.rate_hz < 48_000 && self.channels < want as u16 { + return Some(format!( + "mixes at {} Hz and only {} channel(s)", + self.rate_hz, self.channels + )); + } + if self.rate_hz < 48_000 { + return Some(format!( + "mixes at {} Hz, so the stream is band-limited to ~{} kHz before Opus sees it", + self.rate_hz, + self.rate_hz / 2000 + )); + } + if self.channels < want as u16 { + return Some(format!( + "mixes {} channel(s), so a {want}-channel desktop mix is downmixed and re-expanded", + self.channels + )); + } + None + } +} + +/// Looks up a render endpoint's mix format by endpoint id. `None` = unknown (enumeration failed, +/// or the caller has no way to ask) — treated as "assume it is fine", so a probe failure can +/// never make the plan worse than it was before formats existed. +pub(crate) type FormatProbe<'a> = &'a dyn Fn(&Endpoint) -> Option; + +/// A [`FormatProbe`] that knows nothing — the pre-WP2.1 behaviour. +pub(crate) fn no_formats(_: &Endpoint) -> Option { + None +} + /// The coherent endpoint assignment for one wiring pass. Computed fresh on every mic/capture /// (re)open — Windows endpoints churn (boot-time registration, hotplug, driver installs), so a /// once-per-process plan goes stale. @@ -60,6 +113,11 @@ pub(crate) struct Wiring { /// the mic reservation. The capture side treats it as a stopgap: it warns when the silence /// materializes and re-plans on any endpoint-set change instead of riding it out. pub loopback_last_resort: bool, + /// Set when the chosen loopback endpoint's mix format NARROWS the desktop mix (see + /// [`MixFormat::narrowing`]) and the plan took it anyway because nothing better existed. Carries + /// the human-readable reason for the capture side to log — a quality risk the operator can act + /// on (attach a real output, or set the output mode to prefer hardware), not a failure. + pub loopback_narrowing: Option, } impl Wiring { @@ -137,6 +195,32 @@ pub(crate) fn plan( captures: &[Endpoint], mic_want: Option<&str>, host_audio: bool, +) -> Wiring { + plan_with_formats(renders, captures, mic_want, host_audio, &no_formats, 2) +} + +/// [`plan`] with knowledge of each render endpoint's engine mix format, and the channel count the +/// session wants to carry. +/// +/// **The 2026-08-03 field report is this function's reason to exist.** The default client-only +/// preference takes the "silent sink" — Steam's Streaming *Microphone* render endpoint — over real +/// hardware unconditionally, because it is silent on the host. But that endpoint exists to carry +/// remote *voice*, and nothing checked whether it could carry music. On the reporter's box it won +/// all 31 loopback opens across 25 sessions while a clean AMD HD Audio endpoint sat idle, and the +/// whole desktop mix went through it before reaching Opus. +/// +/// So a silent sink now has to EARN its preference: if its mix format narrows the mix (see +/// [`MixFormat::narrowing`]) it drops below real hardware. It is still taken when nothing better +/// exists — narrow audio beats no audio — but flagged in [`Wiring::loopback_narrowing`] so the +/// capture side can say why. An unknown format (probe failed) counts as fine, so this can never +/// make the plan worse than it was before formats existed. +pub(crate) fn plan_with_formats( + renders: &[Endpoint], + captures: &[Endpoint], + mic_want: Option<&str>, + host_audio: bool, + format_of: FormatProbe, + want_channels: u8, ) -> Wiring { let find_render = |needle: &str| { renders @@ -172,10 +256,18 @@ pub(crate) fn plan( not_mic(id) && !excluded_from_loopback(&ln) && !virtualish(&ln) }) }; - let silent = || { - renders - .iter() - .find(|(n, id)| not_mic(id) && silent_sink(&n.to_lowercase())) + // A silent sink splits in two: one that carries the mix intact, and one that narrows it. The + // first keeps the historical preference; the second falls BELOW real hardware. + let narrowing_of = |ep: &Endpoint| format_of(ep).and_then(|f| f.narrowing(want_channels)); + let silent_intact = || { + renders.iter().find(|ep| { + not_mic(&ep.1) && silent_sink(&ep.0.to_lowercase()) && narrowing_of(ep).is_none() + }) + }; + let silent_narrow = || { + renders.iter().find(|ep| { + not_mic(&ep.1) && silent_sink(&ep.0.to_lowercase()) && narrowing_of(ep).is_some() + }) }; // LAST RESORT — the Steam Streaming Speakers, and ONLY them. Their loopback is known-silent // (validated live): a QUALITY risk, flagged so the capture side can warn when the silence @@ -192,10 +284,13 @@ pub(crate) fn plan( .iter() .find(|(n, id)| not_mic(id) && n.to_lowercase().contains("steam streaming speakers")) }; + // A narrowing silent sink sits below real hardware in BOTH modes: preferring silence on the + // host is a routing choice, but it must not silently cost audio quality when a clean endpoint + // is right there. let preferred = if host_audio { - real_hw().or_else(silent) + real_hw().or_else(silent_intact).or_else(silent_narrow) } else { - silent().or_else(real_hw) + silent_intact().or_else(real_hw).or_else(silent_narrow) }; let (loopback_render, loopback_last_resort) = match preferred { Some(ep) => (Some(ep.clone()), false), @@ -204,12 +299,16 @@ pub(crate) fn plan( None => (None, false), }, }; + // Report narrowing for whatever we actually chose — including real hardware, which can also + // be a 24 kHz mono endpoint (a headset's hands-free profile is exactly that). + let loopback_narrowing = loopback_render.as_ref().and_then(narrowing_of); Wiring { mic_render, mic_capture, loopback_render, loopback_last_resort, + loopback_narrowing, } } @@ -550,6 +649,169 @@ mod tests { } } + // ---- format-aware loopback selection (WP2.1) ----------------------------------------- + + fn fmt(rate_hz: u32, channels: u16) -> MixFormat { + MixFormat { + rate_hz, + channels, + bits: 32, + } + } + + /// Probe helper: give endpoints whose (lowercased) name contains a needle that format, + /// everything else unknown. Owns its table so call sites can pass a literal inline. + fn probe(table: Vec<(&'static str, MixFormat)>) -> impl Fn(&Endpoint) -> Option { + move |ep: &Endpoint| { + let name = ep.0.to_lowercase(); + table + .iter() + .find_map(|(needle, f)| name.contains(needle).then_some(*f)) + } + } + + /// THE 2026-08-03 field case, with formats. The reporter's exact endpoint inventory: the plan + /// took the Steam Streaming Microphone on all 31 opens while a clean AMD HD Audio endpoint sat + /// idle. Once we can see that the silent sink narrows the mix, real hardware must win. + #[test] + fn narrowing_silent_sink_loses_to_real_hardware() { + let renders = [ + ep("CABLE In 16ch (VB-Audio Virtual Cable)"), + ep("Altavoces (Steam Streaming Speakers)"), + ep("Altavoces (Steam Streaming Microphone)"), + ep("CABLE Input (VB-Audio Virtual Cable)"), + ep("1 - Odyssey G60SD (AMD High Definition Audio Device)"), + ]; + let captures = [ + ep("CABLE Output (VB-Audio Virtual Cable)"), + ep("Microphone (Steam Streaming Microphone)"), + ]; + // A voice-carrier endpoint: 24 kHz mono. + let p = probe(vec![ + ("steam streaming microphone", fmt(24_000, 1)), + ("odyssey", fmt(48_000, 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)", + "a narrowing silent sink must not beat clean real hardware" + ); + assert!( + w.loopback_narrowing.is_none(), + "the chosen endpoint is intact" + ); + // The mic assignment is untouched by any of this. + assert_eq!( + w.mic_render.unwrap().0, + "CABLE Input (VB-Audio Virtual Cable)" + ); + } + + /// …but a silent sink that carries the mix intact keeps its historical preference: the + /// client-only routing default is not being abandoned, only made conditional on quality. + #[test] + fn intact_silent_sink_still_wins() { + let renders = [ + ep("Speakers (Realtek HD Audio)"), + ep("CABLE Input (VB-Audio Virtual Cable)"), + ep("Speakers (Steam Streaming Microphone)"), + ]; + let p = probe(vec![ + ("steam streaming microphone", fmt(48_000, 2)), + ("realtek", fmt(48_000, 2)), + ]); + let w = plan_with_formats(&renders, &[], None, false, &p, 2); + assert_eq!( + w.loopback_render.unwrap().0, + "Speakers (Steam Streaming Microphone)" + ); + } + + /// Narrow audio still beats NO audio: with nothing else available the narrowing sink is taken + /// and flagged, not refused. + #[test] + fn narrowing_sink_is_taken_when_it_is_all_there_is() { + let renders = [ + ep("CABLE Input (VB-Audio Virtual Cable)"), + 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); + assert_eq!( + w.loopback_render.as_ref().unwrap().0, + "Speakers (Steam Streaming Microphone)" + ); + let why = w.loopback_narrowing.expect("must be flagged"); + assert!(why.contains("16000"), "{why}"); + } + + /// Real hardware can narrow too — a headset in its hands-free profile is 16 kHz mono — and + /// must be flagged just the same. The flag is about the CHOSEN endpoint, not about which tier + /// it came from. + #[test] + 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); + assert_eq!( + w.loopback_render.as_ref().unwrap().0, + "Headset (Hands-Free AG Audio)" + ); + assert!(w.loopback_narrowing.is_some()); + } + + /// An unknown format must never make the plan WORSE than it was before formats existed: a + /// probe that answers nothing has to reproduce `plan` exactly. + #[test] + fn unknown_formats_reproduce_the_formatless_plan() { + let renders = [ + ep("Speakers (Apple Audio Device)"), + ep("CABLE Input (VB-Audio Virtual Cable)"), + ep("Speakers (Steam Streaming Speakers)"), + ep("Speakers (Steam Streaming Microphone)"), + ]; + 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); + assert_eq!(a, b, "host_audio={host_audio}"); + assert!(a.loopback_narrowing.is_none()); + } + } + + /// `host_audio` still prefers real hardware, and a narrowing silent sink stays last in that + /// mode too. + #[test] + fn host_audio_ordering_survives_formats() { + let renders = [ + ep("Speakers (Realtek HD Audio)"), + ep("Speakers (Steam Streaming Microphone)"), + ]; + let p = probe(vec![ + ("steam streaming microphone", fmt(24_000, 1)), + ("realtek", fmt(48_000, 2)), + ]); + let w = plan_with_formats(&renders, &[], None, true, &p, 2); + assert_eq!(w.loopback_render.unwrap().0, "Speakers (Realtek HD Audio)"); + } + + /// The narrowing test is channel-count aware: an endpoint that is fine for stereo narrows a + /// 5.1 session. + #[test] + fn narrowing_depends_on_the_session_channel_count() { + let stereo_only = fmt(48_000, 2); + assert_eq!(stereo_only.narrowing(2), None); + assert!(stereo_only.narrowing(6).is_some()); + // Rate is judged independently of channels. + assert!(fmt(44_100, 8).narrowing(2).is_some()); + // And an endpoint wider than the session is never "narrowing". + assert_eq!(fmt(48_000, 8).narrowing(2), None); + // Both wrong: the message must name both problems. + let both = fmt(16_000, 1).narrowing(6).unwrap(); + assert!(both.contains("16000") && both.contains("channel"), "{both}"); + } + /// Operator override beats the candidate order. #[test] fn env_override_wins() { diff --git a/crates/punktfunk-host/src/native.rs b/crates/punktfunk-host/src/native.rs index f2cafd6a..91591ead 100644 --- a/crates/punktfunk-host/src/native.rs +++ b/crates/punktfunk-host/src/native.rs @@ -1307,9 +1307,12 @@ async fn serve_session( let stop = stop.clone(); let cap = audio_cap.clone(); let channels = welcome.audio_channels; + // Read the granted bit back off the Welcome (the cursor plane's precedent), so the wire + // the client was promised and the wire we actually send cannot disagree. + let redundancy = welcome.host_caps & punktfunk_core::quic::HOST_CAP_AUDIO_RED != 0; std::thread::Builder::new() .name("punktfunk1-audio".into()) - .spawn(move || audio_thread(conn, stop, cap, channels)) + .spawn(move || audio_thread(conn, stop, cap, channels, redundancy)) .map_err(|e| tracing::warn!(error = %e, "audio thread spawn failed — session continues without audio")) .ok() } else { diff --git a/crates/punktfunk-host/src/native/audio.rs b/crates/punktfunk-host/src/native/audio.rs index 493015a4..852026fe 100644 --- a/crates/punktfunk-host/src/native/audio.rs +++ b/crates/punktfunk-host/src/native/audio.rs @@ -1,8 +1,13 @@ //! The native audio plane (plan §W1 — carved out of the [`super`] module): desktop capture → Opus -//! (48 kHz, 5 ms, CBR — the same tuning as the GameStream path) → `AUDIO_MAGIC` QUIC datagrams, at -//! the negotiated channel count. The encoder ([`NativeAudioEnc`]) and the capture/encode/send loop -//! ([`audio_thread`]) are gated to linux/windows (libopus + a real capturer); other targets get the -//! stub, so a dev build streams video-only rather than failing to compile. +//! (48 kHz, 5 ms, constrained VBR at the configured [`AudioTier`](punktfunk_core::audio::AudioTier)) +//! → `AUDIO_MAGIC` QUIC datagrams — or `AUDIO_RED_MAGIC` when the session negotiated redundancy — +//! at the negotiated channel count. The encoder ([`NativeAudioEnc`]) and the capture/encode/send +//! loop ([`audio_thread`]) are gated to linux/windows (libopus + a real capturer); other targets +//! get the stub, so a dev build streams video-only rather than failing to compile. +//! +//! Two things here deliberately DIVERGE from the GameStream plane, which used to share this +//! tuning: hard CBR (its audio FEC needs fixed-size packets; this plane has no FEC, so CBR was a +//! pure quality tax) and the fixed 128 kbps stereo bitrate. See [`NativeAudioEnc::new`]. use super::*; @@ -17,20 +22,36 @@ enum NativeAudioEnc { #[cfg(any(target_os = "linux", target_os = "windows"))] impl NativeAudioEnc { - /// Build the encoder for `channels` (2/6/8), hard-CBR + RESTRICTED_LOWDELAY like the - /// GameStream path; bitrate from the shared layout table (stereo keeps the validated 128 kbps). - fn new(channels: u8) -> Result { + /// Build the encoder for `channels` (2/6/8) at `tier`, RESTRICTED_LOWDELAY like the GameStream + /// path but — unlike it — in CONSTRAINED VBR. + /// + /// **Why not hard CBR (WP1.2).** The layout table's comment justifies `set_vbr(false)` with + /// "constant packet size, which GameStream's audio FEC relies on" — true of the GameStream + /// plane, and irrelevant here: the native `punktfunk/1` audio plane has no FEC at all (see + /// `punktfunk_core::audio::AudioGapTracker`, which exists precisely because a lost packet has + /// nothing to rebuild it from). So this path was paying a pure quality tax for a constraint + /// that does not apply to it. Constrained VBR keeps the same average bitrate and the same + /// bounded packet size, and spends the bits where the signal needs them. + /// + /// The GameStream encoder (`crate::gamestream::audio`) is deliberately NOT changed: its FEC + /// really does need fixed-size packets. + fn new( + channels: u8, + tier: punktfunk_core::audio::AudioTier, + ) -> Result { + let l = punktfunk_core::audio::layout_for(channels, false); + let bitrate = l.bitrate_for(tier); if channels == 2 { let mut e = opus::Encoder::new( crate::audio::SAMPLE_RATE, opus::Channels::Stereo, opus::Application::LowDelay, )?; - e.set_bitrate(opus::Bitrate::Bits(128_000)).ok(); - e.set_vbr(false).ok(); + e.set_bitrate(opus::Bitrate::Bits(bitrate)).ok(); + e.set_vbr(true).ok(); + e.set_vbr_constraint(true).ok(); Ok(NativeAudioEnc::Stereo(e)) } else { - let l = punktfunk_core::audio::layout_for(channels, false); let mut e = opus::MSEncoder::new( crate::audio::SAMPLE_RATE, l.streams, @@ -38,8 +59,9 @@ impl NativeAudioEnc { l.mapping, opus::Application::LowDelay, )?; - e.set_bitrate(opus::Bitrate::Bits(l.bitrate)).ok(); - e.set_vbr(false).ok(); + e.set_bitrate(opus::Bitrate::Bits(bitrate)).ok(); + e.set_vbr(true).ok(); + e.set_vbr_constraint(true).ok(); Ok(NativeAudioEnc::Surround(e)) } } @@ -52,8 +74,8 @@ impl NativeAudioEnc { } } -/// The audio thread: desktop capture → Opus (48 kHz, 5 ms, CBR — same tuning as the GameStream -/// path) → `AUDIO_MAGIC` datagrams, at the negotiated `channels` (2 stereo / 6 = 5.1 / 8 = 7.1, +/// The audio thread: desktop capture → Opus (48 kHz, 5 ms, constrained VBR at the configured +/// tier) → `AUDIO_MAGIC` (or `AUDIO_RED_MAGIC`) datagrams, at the negotiated `channels` (2 stereo / 6 = 5.1 / 8 = 7.1, /// canonical wire order FL FR FC LFE RL RR SL SR). QUIC already encrypts; no extra layer. The /// capturer comes from (and returns to) the persistent slot — see [`AudioCapSlot`]. #[cfg(any(target_os = "linux", target_os = "windows"))] @@ -62,11 +84,27 @@ pub(super) fn audio_thread( stop: Arc, audio_cap: AudioCapSlot, channels: u8, + redundancy: bool, ) { use crate::audio::SAMPLE_RATE; const FRAME_MS: usize = 5; const SAMPLES_PER_FRAME: usize = SAMPLE_RATE as usize * FRAME_MS / 1000; // 240 let want = punktfunk_core::audio::normalize_channels(channels); + // WP1.1 — encode tier. Unknown spellings warn and fall back rather than silently downgrading + // someone's audio (the whole point of the setting is that quality stopped being invisible). + let tier = match pf_host_config::config().audio_quality.as_deref() { + None => punktfunk_core::audio::AudioTier::default(), + Some(s) => match punktfunk_core::audio::AudioTier::parse(s) { + Some(t) => t, + None => { + tracing::warn!( + value = %s, + "PUNKTFUNK_AUDIO_QUALITY is not one of low/standard/high — using the default" + ); + punktfunk_core::audio::AudioTier::default() + } + }, + }; // Reuse the cached capturer ONLY when its channel count matches this session's; a stereo // capturer left by a prior session must not feed a 5.1/7.1 session (the encoder + the client's @@ -92,7 +130,7 @@ pub(super) fn audio_thread( } } }; - let mut enc = match NativeAudioEnc::new(want) { + let mut enc = match NativeAudioEnc::new(want, tier) { Ok(e) => e, Err(e) => { tracing::warn!(error = %e, "opus encoder init failed — session continues without audio"); @@ -120,9 +158,16 @@ pub(super) fn audio_thread( // A stuck Opus encoder would fail on every 5 ms frame (~200/s); power-of-two throttle the // warn so it can't flood stderr + the log ring while still surfacing that it's failing. let mut opus_encode_errs: u64 = 0; + // WP3.1 — the previous frame's Opus bytes, for the redundant `0xD2` plane. Cleared whenever + // continuity breaks (a capture reopen), so we never advertise a predecessor the client's + // sequence numbering does not agree with. + let mut prev_frame: Vec = Vec::new(); if capturer.is_some() { tracing::info!( channels = want, + tier = tier.as_str(), + kbps = punktfunk_core::audio::layout_for(want, false).bitrate_for(tier) / 1000, + redundancy, "punktfunk/1 audio streaming (Opus 48 kHz, 5 ms datagrams)" ); } @@ -138,6 +183,10 @@ pub(super) fn audio_thread( capturer = Some(c); last_failed = None; acc.clear(); // drop the partial frame straddling the gap + // The next frame has no valid predecessor across the gap: sending the + // pre-gap frame as "the previous one" would hand the client audio from + // before the discontinuity to splice in. + prev_frame.clear(); } Err(e) => { tracing::debug!(error = %format!("{e:#}"), "audio reopen failed — will retry"); @@ -162,11 +211,24 @@ pub(super) fn audio_thread( let pts_ns = now_ns(); match enc.encode_float(&frame, &mut opus_buf) { Ok(n) => { - let d = - punktfunk_core::quic::encode_audio_datagram(seq, pts_ns, &opus_buf[..n]); + let opus = &opus_buf[..n]; + let d = if redundancy { + punktfunk_core::quic::encode_audio_red_datagram( + seq, + pts_ns, + opus, + &prev_frame, + ) + } else { + punktfunk_core::quic::encode_audio_datagram(seq, pts_ns, opus) + }; if conn.send_datagram(d.into()).is_err() { break 'session; // connection gone } + if redundancy { + prev_frame.clear(); + prev_frame.extend_from_slice(opus); + } seq = seq.wrapping_add(1); } Err(e) => { @@ -199,6 +261,7 @@ pub(super) fn audio_thread( _stop: Arc, _audio_cap: AudioCapSlot, _channels: u8, + _redundancy: bool, ) { tracing::warn!("punktfunk/1 audio requires Linux or Windows — session continues without it"); } diff --git a/crates/punktfunk-host/src/native/handshake.rs b/crates/punktfunk-host/src/native/handshake.rs index 3c732229..8b7cf3ef 100644 --- a/crates/punktfunk-host/src/native/handshake.rs +++ b/crates/punktfunk-host/src/native/handshake.rs @@ -24,6 +24,26 @@ use super::*; /// paints on a Mutter virtual stream), and only a can't-blend backend falls back to the /// compositor EMBED. THE single predicate: the Welcome's `HOST_CAP_CURSOR` bit is computed /// from it, and the session wiring reads that bit back. +/// Whether this session sends the REDUNDANT desktop-audio plane (`0xD2`) — THE single predicate +/// behind the Welcome's `HOST_CAP_AUDIO_RED` bit, which `serve_session` reads back to configure the +/// audio thread. +/// +/// Capable-and-agreed: the client must have advertised `CLIENT_CAP_AUDIO_RED`, so a session with an +/// older client keeps the plain `0xC9` wire byte-for-byte. `audio.redundancy` ( +/// `PUNKTFUNK_AUDIO_REDUNDANCY`) can force it off on a link where the extra ~1 % is unwelcome, or +/// force it on for testing. +/// +/// NB the plan's "only while the link is actually losing packets" gate is deliberately not here: +/// turning redundancy on and off mid-session changes the wire tag, and the client's decoder would +/// have to re-derive which plane it is on from every datagram. The cost being avoided is ~1 % of a +/// video budget, which is not worth that fragility — so the decision is made once, at handshake. +pub(super) fn audio_redundancy(client_caps: u8) -> bool { + if client_caps & punktfunk_core::quic::CLIENT_CAP_AUDIO_RED == 0 { + return false; + } + pf_host_config::config().audio_redundancy.unwrap_or(true) +} + pub(super) fn cursor_forward( client_caps: u8, compositor: Option, @@ -564,6 +584,14 @@ pub(super) async fn negotiate( punktfunk_core::quic::HOST_CAP_PEN } else { 0 + } + // Redundant desktop-audio plane (0xD2): the client asked, and the operator has not + // forced it off. Capable-and-agreed, like the cursor bit — a client that did not ask + // keeps the plain 0xC9 wire byte-for-byte. + | if audio_redundancy(hello.client_caps) { + punktfunk_core::quic::HOST_CAP_AUDIO_RED + } else { + 0 }, // The negotiated session AEAD (resolved above) + its 32-byte key toward a ChaCha // client; toward everyone else cipher 0 keeps the Welcome byte-identical to the -- 2.54.0 From a12f1f092c80ee41c55c20e548bf2d74a72c0f83 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 4 Aug 2026 09:28:01 +0200 Subject: [PATCH 3/7] feat(clients/audio): one de-jitter policy for all four rings, and lossless single-packet recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4 + WP3.2 of design/audio-quality-and-latency.md. **The defect.** Every client ring primed *up* to a target and clamped at a ceiling, and none walked the depth back *down*. Any transient — a Wi-Fi arrival burst, a host stall, or plain host-DAC-vs-client-DAC skew of a few dozen ppm — therefore added latency permanently, until an underrun happened to re-prime. Android, with no shed at all, converged on its 120 ms hard cap and stayed there for the rest of the session; that is the "audio latency is too high" report. Apple did shed, 40 ms in one go, which its own comment called "one audible blip". All four now share `punktfunk_core::audio::JitterPolicy`: depths in MILLISECONDS rather than device quanta (`3 x quantum` meant 15 ms at a 5 ms quantum and a silent 64 ms at a 20 ms one), a crossfaded 5 ms shed once the depth average has sat above target for 2 s of consumed audio, and de-prime hysteresis. Linux and Windows had never had that hysteresis — they still carried the `if ring.is_empty()` instant re-prime that Android identified as self-inflicted crackle, where one transient drain manufactured a whole target's worth of silence. Android's floor drops 40 -> 25 ms: the policy grows the target on the devices that actually underrun, instead of every device pre-paying for the worst one. The Windows ring moves from raw bytes to interleaved f32 so it can share the policy and the crossfade helper at all. Apple is the one client where the policy is hand-written in a second language, so it gets its own XCTest (`AudioRingDriftTests`). Verified here by compiling `AudioRing.swift` standalone against a simulation harness — +200 ppm for 5 minutes settles at 30 ms with zero silent callbacks, where the old ring would have ridden its 80 ms high-water mark. **WP3.2 — recovery lives in core, not in the clients.** The rebuilt frame is re-inserted into the demux queue in order, so every embedder (including any C-ABI consumer) gets a complete stream without knowing the `0xD2` plane exists, and their `AudioGapTracker` simply stops seeing the gap. `recovery_and_the_gap_tracker_agree` pins exactly that. For the same reason core advertises CLIENT_CAP_AUDIO_RED itself rather than making four embedders remember to. Verified: clippy --all-targets -D warnings and the full test suites for punktfunk-core, pf-client-core, punktfunk-host, pf-host-config under Linux/docker (163 + 61 tests); punktfunk-client-android `cargo ndk check` for aarch64 with the gate proven non-vacuous by a planted type error, and its 6 clippy findings confirmed IDENTICAL to the pristine file (all are the documented arm64-only artifacts); AudioRing.swift type-checked and simulated on macOS; fmt. The Windows client half (audio_wasapi.rs) is still not compile-verified anywhere. Co-Authored-By: Claude Opus 5 (1M context) --- clients/android/native/src/audio.rs | 110 +++++++------- .../PunktfunkKit/Audio/AudioRing.swift | 122 +++++++++++++--- .../PunktfunkKit/Audio/SessionAudio.swift | 8 +- .../AudioRingDriftTests.swift | 93 ++++++++++++ crates/pf-client-core/src/audio.rs | 66 ++++++--- crates/pf-client-core/src/audio_wasapi.rs | 64 ++++++--- crates/punktfunk-core/src/audio.rs | 134 ++++++++++++++++++ crates/punktfunk-core/src/client/mod.rs | 8 +- .../src/client/pump/datagram_task.rs | 24 ++++ 9 files changed, 512 insertions(+), 117 deletions(-) create mode 100644 clients/apple/Tests/PunktfunkKitTests/AudioRingDriftTests.swift diff --git a/clients/android/native/src/audio.rs b/clients/android/native/src/audio.rs index 2a76dcba..6dbec5cc 100644 --- a/clients/android/native/src/audio.rs +++ b/clients/android/native/src/audio.rs @@ -12,10 +12,14 @@ //! realtime callback and makes us own the buffer. So this client diverges deliberately to stop the //! Android-only crackle: (1) the callback is allocation/free-free — decoded buffers are recycled to //! the producer via a free-list instead of being freed on the audio thread (Android's Scudo `free` -//! has unbounded tail latency); (2) the jitter ring is deeper (~40 ms prime / ~150 ms hard cap) and -//! decoupled from the tiny LowLatency burst size, with de-prime hysteresis so a transient drain -//! doesn't manufacture a silence; (3) the AAudio HW buffer is primed above its 2-burst default and -//! grown on XRuns (Google's anti-glitch technique). +//! has unbounded tail latency); (2) the jitter ring is deeper than the other clients' and decoupled +//! from the tiny LowLatency burst size, with de-prime hysteresis so a transient drain doesn't +//! manufacture a silence; (3) the AAudio HW buffer is primed above its 2-burst default and grown on +//! XRuns (Google's anti-glitch technique). +//! +//! (2) is now the SHARED `punktfunk_core::audio::JitterPolicy` at `JitterTuning::AAUDIO`, which also +//! fixed what this ring was missing: it had a hard cap but nothing that walked the depth back down, +//! so drift and arrival bursts raised latency permanently and Android settled on its ceiling. use ndk::audio::{ AudioCallbackResult, AudioContentType, AudioDirection, AudioFormat, AudioPerformanceMode, @@ -34,26 +38,18 @@ const SAMPLE_RATE: i32 = 48_000; /// Decoded-chunk hand-off depth: 64 × 5 ms = 320 ms slack (matches the core's AUDIO_QUEUE). const RING_CHUNKS: usize = 64; -// --- Jitter-ring depths, in MILLISECONDS (scaled to interleaved-f32 samples at runtime). -------- -// The channel count is negotiated, not a compile-time const, so these are kept in ms and multiplied -// by `ms` (interleaved-f32 samples per millisecond at the resolved layout) inside `start`. -// Unlike the Linux client (PipeWire adaptively rate-matches the stream to the graph clock, masking -// host↔DAC drift + a shallow ring), AAudio hands us a raw callback and we own the buffer: drift and -// WiFi power-save bunching land as underruns/overflows = crackle. So Android runs a deliberately -// deeper, smoothly-managed ring than Linux — keep the two clients' depths intentionally divergent. -/// Prime/target floor: fill to ~40 ms before playing (and after a sustained drain). Deep enough to -/// ride out WiFi arrival jitter + clock drift; the dominant Android-only anti-crackle lever. -const PRIME_FLOOR_MS: usize = 40; -/// Ceiling for the burst-scaled target (so a large quantum can't push the prime depth too high). -const PRIME_CEIL_MS: usize = 80; -/// Drop-oldest headroom above the target before trimming — a ~80 ms band swallows an arrival burst -/// without overflowing. -const JITTER_HEADROOM_MS: usize = 80; -/// Hard latency bound: never let the ring exceed ~150 ms (the only thing that caps added latency). -const HARD_CAP_MS: usize = 150; -/// Re-prime (go silent to refill) only after this many CONSECUTIVE empty callbacks, so one transient -/// drain doesn't manufacture a fresh 40 ms silence (the old `if ring.is_empty()` re-primed instantly). -const DEPRIME_AFTER_CALLBACKS: u32 = 5; +// --- Jitter-ring depths now come from the SHARED policy (`punktfunk_core::audio::JitterTuning`). -- +// They used to be four Android-only constants here. The rationale for Android being DEEPER than the +// other clients still holds and is preserved in `JitterTuning::AAUDIO`: unlike PipeWire, which +// adaptively rate-matches the stream to the graph clock and masks host↔DAC drift, AAudio hands us a +// raw callback and we own the buffer, so drift and Wi-Fi power-save bunching land as +// underruns/overflows = crackle. +// +// Two things changed with the move. The prime floor drops 40 ms → 25 ms, because the policy GROWS +// the target on the devices that actually underrun instead of every device pre-paying for the worst +// one. And the ring finally sheds: it had a hard cap but nothing that walked the depth back down, so +// any drift or burst raised latency permanently and Android converged on its 120 ms ceiling and +// stayed there — the "audio latency is too high" report. /// Throttle the AAudio XRun-driven HW-buffer grow check (cheap, but no need to poll every quantum). const XRUN_CHECK_EVERY: u32 = 128; @@ -104,6 +100,7 @@ struct Counters { pcm_written: AtomicU64, // PCM frames copied out to AAudio (device clock is pulling) underruns: AtomicU64, // callbacks that emitted silence (ring not primed / drained) ring_depth: AtomicU64, // ring sample count at the last callback + target_ms: AtomicU64, // the policy's LIVE target depth (it grows on this device's underruns) } /// Owned by [`crate::session::SessionHandle`]: the live AAudio stream + the decode thread. @@ -126,10 +123,9 @@ impl AudioPlayback { // Interleaved f32 samples per millisecond at this layout (48 kHz × channels); the ms- // denominated jitter-ring depths scale by it. let ms = (SAMPLE_RATE as usize / 1000) * channels; - let prime_floor = PRIME_FLOOR_MS * ms; - let prime_ceil = PRIME_CEIL_MS * ms; - let jitter_headroom = JITTER_HEADROOM_MS * ms; - let hard_cap_max = HARD_CAP_MS * ms; + let tuning = punktfunk_core::audio::JitterTuning::AAUDIO; + // Worst transient the ring can hold before the policy trims it. + let hard_cap_max = tuning.hard_cap_ms as usize * ms; let counters = Arc::new(Counters::default()); // One open attempt at a given sharing mode. Everything the realtime callback captures @@ -157,8 +153,10 @@ impl AudioPlayback { // `decode_loop`. let mut ring: VecDeque = VecDeque::with_capacity(hard_cap_max + RING_CHUNKS * 5 * ms); - let mut primed = false; - let mut empties: u32 = 0; // consecutive empty callbacks (de-prime hysteresis) + // Shared de-jitter policy — prime depth, drift correction, de-prime hysteresis. The + // hysteresis this replaces was Android-only; Linux and Windows carried the instant + // `if ring.is_empty()` re-prime until now. + let mut policy = punktfunk_core::audio::JitterPolicy::new(tuning, channels as u8); let mut cb_count: u32 = 0; // callbacks since open (throttles the XRun grow check) let mut last_xrun: i32 = 0; // last AAudio XRun count we grew the buffer for let callback = move |s: &AudioStream, data: *mut c_void, num_frames: i32| { @@ -173,21 +171,25 @@ impl AudioPlayback { ring.extend(chunk.drain(..)); let _ = free_tx.try_send(chunk); } - // Jitter buffer: prime to ~40 ms (prime_floor) before playing and after a sustained - // drain; drop-oldest only above a wide ~120 ms band. Decoupled from the AAudio burst - // `want` (tiny on the LowLatency MMAP path) so the depth doesn't collapse to a single - // quantum. - let target = (3 * want).clamp(prime_floor, prime_ceil); - let hard_cap = (target + jitter_headroom).min(hard_cap_max); - while ring.len() > hard_cap { - ring.pop_front(); + // Jitter buffer: the shared policy decides prime/silence, trims a burst, and — + // new here — sheds ONE crossfaded 5 ms frame when the depth average has sat above + // target long enough to be drift rather than jitter. Without that shed this ring + // had no way back down: it clamped at 120 ms and stayed pinned there. + let step = policy.step(ring.len(), want); + if step.drop_front > 0 { + punktfunk_core::audio::crossfade_drop( + &mut ring, + step.drop_front, + step.crossfade, + ); } - if !primed && ring.len() >= target { - primed = true; - } - if primed { + let mut ran_short = false; + if !step.silence { for slot in out.iter_mut() { - *slot = ring.pop_front().unwrap_or(0.0); + *slot = ring.pop_front().unwrap_or_else(|| { + ran_short = true; + 0.0 + }); } cb_counters .pcm_written @@ -196,20 +198,15 @@ impl AudioPlayback { out.fill(0.0); cb_counters.underruns.fetch_add(1, Ordering::Relaxed); } - // Re-prime only after a RUN of empty callbacks, not a single transient one — - // otherwise every momentary drain costs a fresh 40 ms silence (the old behaviour, - // self-inflicted crackle on any jitter spike). - if ring.is_empty() { - empties += 1; - if empties >= DEPRIME_AFTER_CALLBACKS { - primed = false; - } - } else { - empties = 0; - } + // No-op while un-primed, so a deliberate priming silence is never counted as an + // underrun (which would otherwise drive the adaptive floor up for no reason). + policy.note_read(ran_short); cb_counters .ring_depth .store(ring.len() as u64, Ordering::Relaxed); + cb_counters + .target_ms + .store(policy.target_ms() as u64, Ordering::Relaxed); // Google's AAudio anti-glitch technique: when the device reports new XRuns, grow the // HW buffer by one burst (up to capacity). getXRunCount + setBufferSizeInFrames are // both callback-safe / non-blocking, and set clamps to capacity so it self-limits. @@ -408,10 +405,11 @@ fn decode_loop( } if count % 600 == 0 { log::info!( - "audio: opus={count} pcm_frames={} underruns={} ring={} peak={window_peak:.3}", + "audio: opus={count} pcm_frames={} underruns={} buffer_ms={} target_ms={} peak={window_peak:.3}", counters.pcm_written.load(Ordering::Relaxed), counters.underruns.load(Ordering::Relaxed), - counters.ring_depth.load(Ordering::Relaxed), + counters.ring_depth.load(Ordering::Relaxed) / ms.max(1) as u64, + counters.target_ms.load(Ordering::Relaxed), ); window_peak = 0.0; } diff --git a/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift b/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift index bca90d98..7654111d 100644 --- a/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift +++ b/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift @@ -3,28 +3,61 @@ import os /// SPSC-ish jitter ring (interleaved float, `channels` per frame), drain thread → render /// callback. The unfair lock is held for microseconds; fine at render-callback rates. Priming: -/// reads return silence until enough is buffered (at least `prefill`, and at least one +/// reads return silence until enough is buffered (at least the target, and at least one /// packet more than the device's render quantum — large-buffer devices would otherwise -/// chronically out-demand the prefill and oscillate prime → dropout → re-prime), and an -/// underrun re-primes, concealing jitter as one short dip instead of sustained crackle. +/// chronically out-demand the prefill and oscillate prime → dropout → re-prime). /// All counts stay whole frames (multiples of `channels`), so the interleave can never slip. +/// +/// **Drift correction.** Both ends run at 48 kHz but on different crystals, so backlog from a +/// network stall or plain host-vs-DAC skew never drains on its own: without correction one 300 ms +/// hiccup leaves audio 300 ms behind video for the rest of the session. This used to be handled by +/// a `highWater` shed that dropped a whole `2 × prefill` at once — its own comment called that "one +/// audible blip". It is now the same two-stage scheme the Rust clients share +/// (`punktfunk_core::audio::JitterPolicy`): a slow depth average that sits above target for a +/// sustained window sheds ONE 5 ms frame with a crossfade, and the hard cap is only a backstop. +/// Keep the constants here in step with `JitterTuning.COREAUDIO`. final class AudioRing: @unchecked Sendable { + /// Mirrors `JitterTuning::COREAUDIO` — see that type for the rationale. + private static let targetMS = 20 + private static let headroomMS = 30 + private static let hardCapMS = 90 + private static let deprimeAfter = 4 + /// The protocol's frame: the shed unit, and the slack added over a large device quantum. + private static let frameMS = 5 + /// Depth average must exceed target by this before drift correction fires — the middle of the + /// headroom band, so the smooth shed always gets its chance BEFORE the hard cap trims. + private static let shedExcessMS = 15 + /// …and must stay there for this much consumed audio. Long, because a shed is the only thing + /// here a listener could notice; it must never fire on a transient. + private static let shedSustainMS = 2_000 + private static let crossfadeMS = 2 + /// Time constant of the depth average. + private static let ewmaTauMS = 1_000 + private var buf: [Float] private var readIdx = 0 private var writeIdx = 0 private var primed = false private var renderQuantum = 0 - private let prefill: Int - private let highWater: Int + private var emptyReads = 0 + private var depthAvg: Double = 0 + private var overRun = 0 private let channels: Int + private let perMS: Int private let lock = OSAllocatedUnfairLock() /// `capacity`/`prefill` in samples (interleaved — `channels` per frame, both whole frames). - init(capacity: Int, prefill: Int, channels: Int) { + /// `prefill` is accepted for source compatibility but the target now comes from `targetMS`. + init(capacity: Int, prefill: Int = 0, channels: Int) { buf = [Float](repeating: 0, count: capacity) - self.prefill = prefill self.channels = channels - highWater = prefill * 4 + perMS = 48 * channels + } + + /// Live target depth in interleaved samples, lifted so it can always serve one device quantum + /// plus a packet (a large-buffer device cannot sustain a target below its own quantum). + private var target: Int { + max(Self.targetMS * perMS, renderQuantum + Self.frameMS * perMS) } func write(_ samples: UnsafePointer, count: Int) { @@ -42,12 +75,12 @@ final class AudioRing: @unchecked Sendable { buf[(writeIdx + i) % capacity] = samples[i] } writeIdx += count - // Latency clamp: both ends run at 48 kHz, so backlog from a network stall (or - // creeping host-vs-DAC clock skew) never drains on its own — without this, one - // 300 ms hiccup leaves audio 300 ms behind video for the rest of the session. - // Shedding down to 2× prefill costs one audible blip instead. - if writeIdx - readIdx > highWater { - readIdx = writeIdx - prefill * 2 + // Backstop only: the smooth shed in `read` is what normally holds the depth down. + let cap = min(target + Self.headroomMS * perMS, Self.hardCapMS * perMS) + if writeIdx - readIdx > cap { + readIdx = writeIdx - cap + depthAvg = Double(cap) + overRun = 0 } } @@ -57,16 +90,36 @@ final class AudioRing: @unchecked Sendable { defer { lock.unlock() } renderQuantum = max(renderQuantum, count) let available = writeIdx - readIdx + + // Depth average, weighted by the callback size so its time constant is independent of the + // device quantum. + let alpha = min(1.0, Double(count) / Double(Self.ewmaTauMS * perMS)) + depthAvg += (Double(available) - depthAvg) * alpha + if !primed { - // One 5 ms host packet (240 frames × channels) of slack beyond the device's demand. - if available >= max(prefill, renderQuantum + 240 * channels) { + if available >= target { primed = true + emptyReads = 0 } else { for i in 0.. Double(target + Self.shedExcessMS * perMS) { + overRun += count + if overRun >= Self.shedSustainMS * perMS { + overRun = 0 + shedOneFrame() + depthAvg = Double(writeIdx - readIdx) + } + } else { + overRun = 0 + } + + let n = min(writeIdx - readIdx, count) let capacity = buf.count for i in 0..= Self.deprimeAfter { primed = false } + } else { + emptyReads = 0 } } + + /// Drop one protocol frame from the front, linearly crossfading the seam so the correction is + /// inaudible rather than a click. Mirrors `punktfunk_core::audio::crossfade_drop`; caller holds + /// the lock. + private func shedOneFrame() { + let drop = Self.frameMS * perMS + let available = writeIdx - readIdx + guard available > drop else { return } + let fade = min(Self.crossfadeMS * perMS, min(drop, available - drop)) + let capacity = buf.count + if fade > 0 { + // The tail of what we discard fades out into the head of what survives. + for i in 0.. (Int, Int, Int) { + let ring = AudioRing(capacity: 48_000 * channels, channels: channels) + let want = quantumMS * perMS + var scratch = [Float](repeating: 0, count: want) + // Non-zero so a silent callback is distinguishable from real audio. + let producer = [Float](repeating: 0.25, count: want + 8) + var carry = 0, peak = 0, final = 0, silent = 0 + + for i in 0..<(ms / quantumMS) { + carry += want * driftPPM + let extra = carry / 1_000_000 + carry -= extra * 1_000_000 + producer.withUnsafeBufferPointer { ring.write($0.baseAddress!, count: want + extra) } + + scratch.withUnsafeMutableBufferPointer { ring.read(into: $0.baseAddress!, count: want) } + // Skip the priming window at the very start. + if i > 20, scratch.allSatisfy({ $0 == 0 }) { silent += 1 } + peak = max(peak, ring.bufferedMS) + final = ring.bufferedMS + } + return (final, peak, silent) + } + + /// THE regression: with the host clock running fast, buffered latency must return to target + /// instead of climbing to the hard cap and staying pinned there. +200 ppm is deliberately + /// harsher than real hardware (tens of ppm). + func testDriftDoesNotRatchetLatencyToTheCeiling() { + let (final, peak, silent) = simulate(ms: 5 * 60 * 1_000, quantumMS: 5, driftPPM: 200) + // Must settle inside the headroom band (target 20 + headroom 30), never near the 90 ms cap. + XCTAssertLessThanOrEqual(final, 50, "settled at \(final) ms — that is the ratchet") + XCTAssertLessThanOrEqual(peak, 50, "peaked at \(peak) ms") + XCTAssertEqual(silent, 0, "drift correction must never starve the callback") + } + + /// The mirror case: a host clock running SLOW must keep audio flowing rather than being + /// "corrected" into a stutter. + func testNegativeDriftKeepsPlaying() { + let (_, _, silent) = simulate(ms: 2 * 60 * 1_000, quantumMS: 5, driftPPM: -200) + XCTAssertEqual(silent, 0, "a draining ring must re-prime, not chatter") + } + + /// A device that pulls a large quantum cannot sustain a target below it — the ring must lift + /// its target rather than oscillating prime → dropout → re-prime forever. + func testLargeDeviceQuantumStillPlays() { + let (_, _, silent) = simulate(ms: 60 * 1_000, quantumMS: 40, driftPPM: 0) + XCTAssertEqual(silent, 0, "a 40 ms quantum must not starve a 20 ms target") + } + + /// One transient drain must not manufacture a whole target's worth of fresh silence: the ring + /// de-primes only after a RUN of short reads. + func testSingleShortReadDoesNotDeprime() { + let ring = AudioRing(capacity: 48_000 * channels, channels: channels) + let want = 5 * perMS + var scratch = [Float](repeating: 0, count: want) + // Prime well past target. + let big = [Float](repeating: 0.5, count: 60 * perMS) + big.withUnsafeBufferPointer { ring.write($0.baseAddress!, count: big.count) } + scratch.withUnsafeMutableBufferPointer { ring.read(into: $0.baseAddress!, count: want) } + XCTAssertTrue(scratch.contains { $0 != 0 }, "should be playing after priming") + + // Drain it dry with one oversized read, then feed a normal quantum again. + var huge = [Float](repeating: 0, count: 200 * perMS) + huge.withUnsafeMutableBufferPointer { ring.read(into: $0.baseAddress!, count: huge.count) } + let feed = [Float](repeating: 0.5, count: want) + feed.withUnsafeBufferPointer { ring.write($0.baseAddress!, count: want) } + scratch.withUnsafeMutableBufferPointer { ring.read(into: $0.baseAddress!, count: want) } + XCTAssertTrue( + scratch.contains { $0 != 0 }, + "a single short read must not force a full re-prime") + } +} +#endif diff --git a/crates/pf-client-core/src/audio.rs b/crates/pf-client-core/src/audio.rs index 30ceaaf7..b70ff4f9 100644 --- a/crates/pf-client-core/src/audio.rs +++ b/crates/pf-client-core/src/audio.rs @@ -168,9 +168,18 @@ struct PlayerData { /// Drained chunk Vecs go back here for the decode side to refill (allocation pool). recycle: SyncSender>, ring: VecDeque, - primed: bool, + /// Shared ms-denominated de-jitter policy: prime depth, drift correction, de-prime + /// hysteresis. Replaces the old `3 × quantum` target, which meant 15 ms at a 5 ms graph + /// quantum and a silent 64 ms at a 20 ms one, and the `if ring.is_empty()` re-prime, where + /// one transient drain manufactured a whole target's worth of fresh silence. + policy: punktfunk_core::audio::JitterPolicy, /// Interleaved channel count this stream was opened with (2/6/8). channels: usize, + /// Diagnostics (WP0.3), logged ~every 10 s: the audio plane used to be entirely silent in a + /// client log, so a latency or dropout report had nothing to go on. + underruns: u64, + sheds: u64, + callbacks: u64, } fn pw_thread( @@ -223,8 +232,14 @@ fn pw_thread( rx: pcm_rx, recycle: recycle_tx, ring: VecDeque::new(), - primed: false, + policy: punktfunk_core::audio::JitterPolicy::new( + punktfunk_core::audio::JitterTuning::PIPEWIRE, + channels as u8, + ), channels, + underruns: 0, + sheds: 0, + callbacks: 0, }; let _listener = stream @@ -252,23 +267,29 @@ fn pw_thread( let want_frames = data.data().map(|s| s.len() / stride).unwrap_or(0); let want = want_frames * ud.channels; - // Adaptive jitter buffer (same shape as the host's virtual mic): prime to - // ~3 quanta, cap at ~1 quantum of slack beyond that, re-prime after a - // genuine drain. - let target = (3 * want).clamp(720 * ud.channels, 9600 * ud.channels); - while ud.ring.len() > target.max(want) + want { - ud.ring.pop_front(); - } - if !ud.primed && ud.ring.len() >= target { - ud.primed = true; + // Shared de-jitter policy: prime depth in MILLISECONDS, smooth drift correction + // (a crossfaded 5 ms shed) so latency returns to target instead of ratcheting, + // and a hard cap as the backstop. + let step = ud.policy.step(ud.ring.len(), want); + if step.drop_front > 0 { + ud.sheds += 1; + punktfunk_core::audio::crossfade_drop( + &mut ud.ring, + step.drop_front, + step.crossfade, + ); } + let mut ran_short = false; let n_frames = if let Some(slice) = data.data() { for k in 0..want { - let s = if ud.primed { - ud.ring.pop_front().unwrap_or(0.0) - } else { + let s = if step.silence { 0.0 + } else { + ud.ring.pop_front().unwrap_or_else(|| { + ran_short = true; + 0.0 + }) }; let off = k * 4; slice[off..off + 4].copy_from_slice(&s.to_le_bytes()); @@ -277,8 +298,21 @@ fn pw_thread( } else { 0 }; - if ud.ring.is_empty() { - ud.primed = false; + // No-op while un-primed (the policy ignores it), so a deliberate priming silence + // is never miscounted as an underrun. + ud.policy.note_read(ran_short); + ud.underruns += u64::from(ran_short); + ud.callbacks += 1; + // ~10 s at a 5 ms quantum; the exact cadence does not matter, only that the + // plane stops being invisible. + if ud.callbacks % 2_000 == 0 { + tracing::debug!( + buffer_ms = ud.policy.avg_depth_ms(), + target_ms = ud.policy.target_ms(), + underruns = ud.underruns, + drift_sheds = ud.sheds, + "audio playback" + ); } let chunk = data.chunk_mut(); *chunk.offset_mut() = 0; diff --git a/crates/pf-client-core/src/audio_wasapi.rs b/crates/pf-client-core/src/audio_wasapi.rs index 2df9f3b8..0c9b6d69 100644 --- a/crates/pf-client-core/src/audio_wasapi.rs +++ b/crates/pf-client-core/src/audio_wasapi.rs @@ -250,10 +250,20 @@ fn render_thread( audio_client.start_stream().context("start render stream")?; let _ = ready.send(Ok(())); - // Adaptive jitter buffer, in f32-byte units (same shape as the host's virtual mic). - let mut ring: VecDeque = VecDeque::new(); - let mut primed = false; + // De-jitter ring, in interleaved f32 SAMPLES (it used to be raw bytes, which made the + // depth arithmetic byte-vs-sample and kept it from sharing the policy and the crossfade + // helper with the other three clients). + let mut ring: VecDeque = VecDeque::new(); + // Shared ms-denominated policy: prime depth, crossfaded drift correction so latency + // returns to target instead of ratcheting, and de-prime hysteresis — the last replacing + // the old `if ring.is_empty()`, where a single transient drain manufactured a whole + // target's worth of fresh silence. + let mut policy = punktfunk_core::audio::JitterPolicy::new( + punktfunk_core::audio::JitterTuning::WASAPI, + channels, + ); let mut out = Vec::new(); // per-quantum scratch, reused across iterations + let (mut underruns, mut sheds, mut callbacks) = (0u64, 0u64, 0u64); while !stop.load(Ordering::Relaxed) { if h_event.wait_for_event(100).is_err() { @@ -262,9 +272,7 @@ fn render_thread( // Drain everything the pump has queued into the ring, returning each drained // Vec to the pool (a full/closed pool drops it). while let Ok(mut chunk) = pcm_rx.try_recv() { - for s in chunk.iter() { - ring.extend(s.to_le_bytes()); - } + ring.extend(chunk.iter().copied()); chunk.clear(); let _ = recycle_tx.try_send(chunk); } @@ -274,28 +282,40 @@ fn render_thread( if avail_frames == 0 { continue; } - let want_bytes = avail_frames * block_align; + let want = avail_frames * channels as usize; - // Prime to ~3 quanta; cap at ~1 quantum of slack beyond that; re-prime on drain. - let target = (3 * want_bytes).clamp(720 * block_align, 9600 * block_align); - let cap = target.max(want_bytes) + want_bytes; - if ring.len() > cap { - ring.drain(..ring.len() - cap); - } - if !primed && ring.len() >= target { - primed = true; + let step = policy.step(ring.len(), want); + if step.drop_front > 0 { + sheds += 1; + punktfunk_core::audio::crossfade_drop(&mut ring, step.drop_front, step.crossfade); } out.clear(); - out.resize(want_bytes, 0); - if primed { - let n = ring.len().min(want_bytes); - for (dst, b) in out.iter_mut().zip(ring.drain(..n)) { - *dst = b; + out.resize(avail_frames * block_align, 0); + let mut ran_short = false; + if !step.silence { + // `out` is exactly `want` f32s wide (avail_frames × channels × 4 bytes). + for dst in out.chunks_exact_mut(4) { + let s = ring.pop_front().unwrap_or_else(|| { + ran_short = true; + 0.0 + }); + dst.copy_from_slice(&s.to_le_bytes()); } } - if ring.is_empty() { - primed = false; + // No-op while un-primed (the policy ignores it), so a deliberate priming silence is + // never miscounted as an underrun. + policy.note_read(ran_short); + underruns += u64::from(ran_short); + callbacks += 1; + if callbacks % 1_000 == 0 { + tracing::debug!( + buffer_ms = policy.avg_depth_ms(), + target_ms = policy.target_ms(), + underruns, + drift_sheds = sheds, + "audio playback" + ); } render_client .write_to_device(avail_frames, &out, None) diff --git a/crates/punktfunk-core/src/audio.rs b/crates/punktfunk-core/src/audio.rs index a006fc51..f20d7780 100644 --- a/crates/punktfunk-core/src/audio.rs +++ b/crates/punktfunk-core/src/audio.rs @@ -243,6 +243,54 @@ impl AudioGapTracker { } } +/// Rebuilds the audio stream from the redundant `0xD2` plane, so a single lost datagram is +/// RECOVERED rather than concealed. +/// +/// Deliberately lives in core, on the demux side, rather than in the four client decoders. The +/// recovered frame is re-inserted into the same queue in order, so every embedder — Linux, +/// Windows, Android, Apple, and any C-ABI consumer — gets a complete stream with no change at all, +/// and their [`AudioGapTracker`] simply stops seeing the gap. +/// +/// **Only the immediately-preceding frame can be recovered**, because that is all the wire carries +/// (see [`crate::quic::encode_audio_red_datagram`]). A longer burst still falls through to +/// packet-loss concealment — but it falls through one frame shorter, which is strictly better. +#[derive(Debug, Default)] +pub struct AudioRedRecovery { + /// Sequence of the newest packet handed downstream. + last_seq: Option, +} + +impl AudioRedRecovery { + pub fn new() -> Self { + Self::default() + } + + /// Feed the arriving datagram's sequence and whether it carried a redundant copy. Returns + /// `true` when that copy should be emitted (as `seq - 1`) BEFORE the packet itself. + /// + /// Wrapping-safe, and conservative in both directions: a reorder or duplicate recovers + /// nothing, and neither does the first packet of a session (nothing is known to be missing). + pub fn recover_before(&mut self, seq: u32, has_prev: bool) -> bool { + let recover = match self.last_seq { + // Nothing emitted yet: no evidence anything was lost, so inserting the predecessor + // would prepend audio the client never missed. + None => false, + Some(last) => { + let delta = seq.wrapping_sub(last); + // `delta == 1` is in-order; `delta >= 2` (forward half of the space only) means + // at least the predecessor is missing. + has_prev && (2..u32::MAX / 2).contains(&delta) + } + }; + self.last_seq = Some(match self.last_seq { + // A reorder must not drag the anchor backwards. + Some(last) if seq.wrapping_sub(last) > u32::MAX / 2 => last, + _ => seq, + }); + recover + } +} + // ---- the shared playback de-jitter policy ------------------------------------------------- /// The protocol's audio frame, in milliseconds — every host datagram carries exactly one @@ -692,6 +740,92 @@ mod tests { assert_eq!(t.missing_before(0), 0, "pre-wrap reorder, not a 2^31 gap"); } + // ---- redundant-plane recovery --------------------------------------------------------- + + #[test] + fn red_recovery_rebuilds_exactly_the_single_missing_frame() { + let mut r = AudioRedRecovery::new(); + // First packet: nothing is known to be missing, so nothing is prepended. + assert!(!r.recover_before(10, true)); + // In order. + assert!(!r.recover_before(11, true)); + // 12 lost: 13 carries it. + assert!(r.recover_before(13, true)); + // Back in order from the new anchor. + assert!(!r.recover_before(14, true)); + } + + #[test] + fn red_recovery_is_conservative() { + let mut r = AudioRedRecovery::new(); + r.recover_before(10, true); + // A datagram with no redundant copy recovers nothing, however big the gap. + assert!(!r.recover_before(20, false)); + // Duplicates and reorders recover nothing, and must not move the anchor backwards. + let mut r = AudioRedRecovery::new(); + r.recover_before(10, true); + r.recover_before(11, true); + assert!(!r.recover_before(11, true), "duplicate"); + assert!(!r.recover_before(9, true), "late reorder"); + assert!( + !r.recover_before(12, true), + "the reorder must not have moved the anchor" + ); + } + + /// A longer burst still recovers its last frame — the gap the client has to conceal gets one + /// frame shorter, which is strictly better than concealing all of it. + #[test] + fn red_recovery_shortens_a_longer_burst() { + let mut r = AudioRedRecovery::new(); + r.recover_before(100, true); + assert!( + r.recover_before(105, true), + "104 is recoverable even though 101-103 are not" + ); + } + + #[test] + fn red_recovery_survives_seq_wraparound() { + let mut r = AudioRedRecovery::new(); + assert!(!r.recover_before(u32::MAX - 1, true)); + assert!( + !r.recover_before(u32::MAX, true), + "in order across the edge" + ); + assert!(r.recover_before(1, true), "seq 0 lost across the wrap"); + assert!(!r.recover_before(2, true)); + } + + /// The two halves must agree: whatever `AudioRedRecovery` rebuilds, `AudioGapTracker` must + /// then see as no gap at all — that is the whole point of doing recovery on the demux side. + #[test] + fn recovery_and_the_gap_tracker_agree() { + let mut rec = AudioRedRecovery::new(); + let mut gaps = AudioGapTracker::new(); + let mut concealed = 0; + // Deliver 0..20 with 7 and 13 lost; each survivor carries its predecessor. + let mut emitted: Vec = Vec::new(); + for seq in (0..20u32).filter(|s| *s != 7 && *s != 13) { + if rec.recover_before(seq, true) { + emitted.push(seq - 1); + } + emitted.push(seq); + } + for seq in &emitted { + concealed += gaps.missing_before(*seq); + } + assert_eq!( + concealed, 0, + "recovered stream must need no concealment: {emitted:?}" + ); + assert_eq!(emitted.len(), 20, "every frame accounted for"); + assert!( + emitted.windows(2).all(|w| w[1] == w[0] + 1), + "and in order: {emitted:?}" + ); + } + // ---- bitrate tiers ------------------------------------------------------------------- /// `Standard` must reproduce the historical table EXACTLY — that is what makes the tier diff --git a/crates/punktfunk-core/src/client/mod.rs b/crates/punktfunk-core/src/client/mod.rs index eaece606..0d4c48d6 100644 --- a/crates/punktfunk-core/src/client/mod.rs +++ b/crates/punktfunk-core/src/client/mod.rs @@ -490,7 +490,13 @@ impl NativeClient { video_codecs, preferred_codec, display_hdr, - client_caps, + // Redundant audio (`0xD2`) is advertised by CORE, not by the embedder: the + // recovery happens on the demux side (`AudioRedRecovery` in the datagram + // task) and re-inserts the rebuilt frame into the same queue, so every + // embedder benefits without knowing the plane exists — and none of them can + // forget to opt in. The bit is a pure "I can decode it"; the host still + // decides whether to spend the extra ~1 %. + client_caps: client_caps | crate::quic::CLIENT_CAP_AUDIO_RED, frame_parts, launch, name, diff --git a/crates/punktfunk-core/src/client/pump/datagram_task.rs b/crates/punktfunk-core/src/client/pump/datagram_task.rs index b62c8dd7..59bae19f 100644 --- a/crates/punktfunk-core/src/client/pump/datagram_task.rs +++ b/crates/punktfunk-core/src/client/pump/datagram_task.rs @@ -23,6 +23,10 @@ pub(super) async fn run( // gate): a datagram the network reordered must not roll a stopped motor back on. Legacy v1 // datagrams carry no seq and bypass it (an old host's own periodic re-send is the only heal). let mut rumble_last_seq: [Option; crate::input::MAX_PADS] = [None; crate::input::MAX_PADS]; + // Redundant-audio-plane rebuild (`0xD2`). Recovery happens HERE rather than in the four + // client decoders: the recovered frame is re-inserted into this queue in order, so every + // embedder gets a complete stream without knowing the plane exists. + let mut audio_red = crate::audio::AudioRedRecovery::new(); while let Ok(d) = conn.read_datagram().await { match d.first() { Some(&crate::quic::AUDIO_MAGIC) => { @@ -34,6 +38,26 @@ pub(super) async fn run( }); } } + Some(&crate::quic::AUDIO_RED_MAGIC) => { + if let Some((seq, pts_ns, opus, prev)) = crate::quic::decode_audio_red_datagram(&d) + { + if audio_red.recover_before(seq, prev.is_some()) { + // The copy is the frame BEFORE this one, so it carries the previous + // sequence and presentation time — one protocol frame earlier. + let _ = audio_tx.try_send(AudioPacket { + seq: seq.wrapping_sub(1), + pts_ns: pts_ns + .saturating_sub(crate::audio::FRAME_MS as u64 * 1_000_000), + data: prev.unwrap_or_default().to_vec(), + }); + } + let _ = audio_tx.try_send(AudioPacket { + seq, + pts_ns, + data: opus.to_vec(), + }); + } + } Some(&crate::quic::RUMBLE_MAGIC) => { if let Some(u) = crate::quic::decode_rumble_envelope(&d) { // Gate v2 envelopes on their per-pad seq; forward v1 (envelope: None) as-is. -- 2.54.0 From e9a209ef61d88379aba103cd9e6fbb3cb4f714bb Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 4 Aug 2026 09:42:06 +0200 Subject: [PATCH 4/7] docs(troubleshooting): why streamed audio can sound worse than the host, and the knobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP0.4. The 2026-08-03 reporter had no way to know their desktop mix was being routed through Steam's voice-carrier endpoint, and no documented way to change it — `PUNKTFUNK_HOST_AUDIO` existed only in a module doc comment. Two new sections: what the host actually captures (a render endpoint, not "the sound card"), what the new `engine_hz/engine_ch/engine_bits` log line tells you, and the `PUNKTFUNK_AUDIO_OUTPUT_MODE` / `_QUALITY` / `_REDUNDANCY` knobs — with host_and_client called out as the quickest A/B for the endpoint question; and why audio that lags the picture should now correct itself, plus what to check when it does not. Co-Authored-By: Claude Opus 5 (1M context) --- docs-site/content/docs/troubleshooting.md | 58 +++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/docs-site/content/docs/troubleshooting.md b/docs-site/content/docs/troubleshooting.md index 18d03cdc..9d925b6c 100644 --- a/docs-site/content/docs/troubleshooting.md +++ b/docs-site/content/docs/troubleshooting.md @@ -302,6 +302,64 @@ told your client so. [When the client and the host disagree](/docs/client-settings#when-the-client-and-the-host-disagree) lists what it does with each one. +## Streamed audio sounds worse than the host does + +The host does not capture "the sound card" — it captures a **render endpoint**, and by default it +picks one that is *silent on the host* so the audio plays on your client only. On a PC with Steam +installed that silent endpoint is Steam's **Streaming Microphone**, which exists to carry remote +*voice*. If Windows has it configured as a narrow device — mono, or below 48 kHz — then the whole +desktop mix is squeezed through that before it is ever encoded, and no amount of bitrate will bring +it back. + +Since 0.25 the host checks for this: it reads each candidate endpoint's real format, prefers a real +output device over a narrow virtual one, and says so in the log — + +``` +WARN the desktop-audio loopback endpoint mixes at 24000 Hz, so the stream is band-limited … +INFO audio loopback capturing device="…" engine_hz=48000 engine_ch=2 engine_bits=32 +``` + +That `engine_*` line is the endpoint's **own** format, so it tells you directly whether the source +was ever full quality. To choose the routing yourself, set in `host.env`: + +```ini +# client_only — default; audio plays on the client only (a silent endpoint) +# host_and_client — capture a real output device; audio plays on BOTH ends +# follow_default — capture whatever YOUR default playback device is, and never change it +PUNKTFUNK_AUDIO_OUTPUT_MODE=host_and_client +``` + +`host_and_client` is also the quickest way to A/B the problem: if the stream sounds right that way +and wrong on the default, the endpoint was the cause. + +Two related knobs: + +```ini +PUNKTFUNK_AUDIO_QUALITY=high # low | standard | high (default high — stereo 256 kbps) +PUNKTFUNK_AUDIO_REDUNDANCY=1 # force the loss-resilient audio plane on (default: automatic) +``` + +Audio is a fraction of a percent of a stream's bandwidth, so `high` costs nothing worth counting. +`standard` reproduces the pre-0.25 encoder exactly if you want to compare. + +## Audio lags behind the picture + +The client buffers a little audio to absorb network jitter. Since 0.25 that buffer **corrects +itself**: if it drifts deeper — a Wi-Fi burst, a stall, or just the two devices' clocks running at +fractionally different speeds — it trims itself back a few milliseconds at a time, inaudibly. +Before, it could only grow, so a single hiccup left audio permanently behind the video and the only +cure was reconnecting. + +If audio is still noticeably late: + +- **Reconnect once.** It confirms whether the delay was accumulated (gone after a reconnect) or + constant (something else). +- **Check for underruns** rather than guessing. The client logs its buffer depth periodically; a + rising `underruns` count means the buffer is being starved, which is a network or CPU problem, not + a buffering one. +- **Wired or 5 GHz Wi-Fi.** Arrival jitter is what the buffer exists to absorb; less jitter lets it + run shallower. + ## Windows: the host or the web console won't start The **`PunktfunkHost` service** runs both halves of the Windows host: the streaming host itself and -- 2.54.0 From 2cfc82e96c558408e18aed378c020c7c2d61eb95 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 4 Aug 2026 17:58:17 +0200 Subject: [PATCH 5/7] fix(audio): budget the audio plane against the link, and close the review's gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Findings from the post-implementation review of design/audio-quality-and-latency.md. **The bandwidth gap (highest).** Tier `High` (256 kbps) and the redundant `0xD2` plane were added separately, each costed as "~1 % of the video budget", and nobody added them together: 256 kbps sent twice is 512 kbps — ~2.5 % of a 20 Mbps session but ~10 % of a 5 Mbps one. Audio rides QUIC datagrams, OUTSIDE the ABR loop, so ABR could neither see that nor reclaim it; a constrained link quietly handed a tenth of its bandwidth to audio while ABR carefully managed the rest. `plan_audio_budget` now makes tier and redundancy ONE decision against the session's resolved video bitrate, ordered by preference rather than cost — transparent audio beats redundant audio, since the field report was about quality and redundancy only pays under loss, so `High` alone outranks `Standard`+redundancy even though they cost the same. It can lower what the operator asked for, never raise it, and never goes below `Low`: a stream with unintelligible audio is worse than one spending a few percent more. **The Linux host kept the exact defect fixed on Windows.** `let _ = tx.try_send(samples)` — silent, uncounted data loss, where the encoder concatenates across the hole, so every drop is a click AND a permanent shift of everything after it. WP0.2 turned out to be Windows-only and had not said so. Linux now shares `capture_policy::CaptureStats`: drops counted and warned, plus per-window peak/RMS/delivered%. A Linux audio report was until now exactly as un-triageable as the Windows one was on 2026-08-03. **Apple's WP0.3 was half-done** — `bufferedMS` was added and wired to nothing. The drain thread now logs buffer/target/underruns/sheds like the other three, from one locked snapshot so the numbers in a line describe the same instant. Also: the Linux "audio format negotiated" line now says WHICH mode produced it, because that changes what it is worth — in stream-sink mode the host owns the sink so the mix cannot have been narrowed upstream, but in legacy monitor mode a 16 kHz Bluetooth sink would still be reported as a clean 48 kHz through PipeWire's resampler, the same way WASAPI's autoconvert hid it on Windows. Reading the monitored node's own rate needs a registry lookup this stream does not do; recorded as an open gap rather than implied to be covered. Two stale docs: `audio_wasapi.rs` cited `clients/windows/src/audio.rs` (deleted) and still described the pre-shared-policy "prime to ~3 quanta" behaviour. And the Apple ring's `prefill:` parameter, dead since the depth moved into the ring, is gone. Verified: clippy --all-targets -D warnings on Linux (docker) AND Windows (runner .133, forced clean rebuild of punktfunk-host + pf-client-core); core 167 tests; host 57 audio tests on Windows; Android clippy count identical to pristine (6, all documented arm64 artifacts); Apple ring re-simulated. The host suite's `gamestream::stream::tests::sender_delivers_batches` fails under qemu — the recorded environmental flake, unrelated to audio, green on the earlier less-loaded run. Co-Authored-By: Claude Opus 5 (1M context) --- .../PunktfunkKit/Audio/AudioRing.swift | 35 +++- .../PunktfunkKit/Audio/SessionAudio.swift | 12 ++ crates/pf-client-core/src/audio_wasapi.rs | 15 +- crates/punktfunk-core/src/audio.rs | 163 ++++++++++++++++++ crates/punktfunk-host/src/audio/linux/mod.rs | 64 ++++++- crates/punktfunk-host/src/native.rs | 12 +- crates/punktfunk-host/src/native/audio.rs | 25 +-- crates/punktfunk-host/src/native/handshake.rs | 79 +++++++-- 8 files changed, 351 insertions(+), 54 deletions(-) diff --git a/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift b/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift index 7654111d..f139d82e 100644 --- a/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift +++ b/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift @@ -42,13 +42,18 @@ final class AudioRing: @unchecked Sendable { private var emptyReads = 0 private var depthAvg: Double = 0 private var overRun = 0 + /// Reported, not acted on: short reads that actually starved the callback, and smooth drift + /// corrections. A rising underrun count means the ring is being starved (network or CPU), + /// which is a different problem from the depth being wrong. + private var underrunCount = 0 + private var shedCount = 0 private let channels: Int private let perMS: Int private let lock = OSAllocatedUnfairLock() - /// `capacity`/`prefill` in samples (interleaved — `channels` per frame, both whole frames). - /// `prefill` is accepted for source compatibility but the target now comes from `targetMS`. - init(capacity: Int, prefill: Int = 0, channels: Int) { + /// `capacity` in samples (interleaved — `channels` per frame, a whole number of frames). + /// The de-jitter depth is the ring's own business (`targetMS`), not a caller's prefill. + init(capacity: Int, channels: Int) { buf = [Float](repeating: 0, count: capacity) self.channels = channels perMS = 48 * channels @@ -113,6 +118,7 @@ final class AudioRing: @unchecked Sendable { if overRun >= Self.shedSustainMS * perMS { overRun = 0 shedOneFrame() + shedCount += 1 depthAvg = Double(writeIdx - readIdx) } } else { @@ -130,6 +136,7 @@ final class AudioRing: @unchecked Sendable { // De-prime only after a RUN of short reads: a single transient drain must not // manufacture a whole target's worth of fresh silence. emptyReads += 1 + underrunCount += 1 if emptyReads >= Self.deprimeAfter { primed = false } } else { emptyReads = 0 @@ -157,12 +164,32 @@ final class AudioRing: @unchecked Sendable { readIdx += drop } - /// Current buffered depth in milliseconds — for the stats overlay. + /// Current buffered depth in milliseconds — for the stats overlay and the drain thread's + /// periodic log. var bufferedMS: Int { lock.lock() defer { lock.unlock() } return (writeIdx - readIdx) / max(perMS, 1) } + + /// One consistent snapshot of the ring's vitals, taken under a single lock so the numbers in + /// a log line describe the same instant. Mirrors what the three Rust clients report. + struct Stats { + let bufferedMS: Int + let targetMS: Int + let underruns: Int + let sheds: Int + } + + var stats: Stats { + lock.lock() + defer { lock.unlock() } + return Stats( + bufferedMS: (writeIdx - readIdx) / max(perMS, 1), + targetMS: target / max(perMS, 1), + underruns: underrunCount, + sheds: shedCount) + } } /// CoreAudio channel layout for the canonical wire order FL FR FC LFE RL RR [SL SR]. nil for diff --git a/clients/apple/Sources/PunktfunkKit/Audio/SessionAudio.swift b/clients/apple/Sources/PunktfunkKit/Audio/SessionAudio.swift index 9a1fab08..d79d12a1 100644 --- a/clients/apple/Sources/PunktfunkKit/Audio/SessionAudio.swift +++ b/clients/apple/Sources/PunktfunkKit/Audio/SessionAudio.swift @@ -403,6 +403,7 @@ public final class SessionAudio { stateLock.unlock() let thread = Thread { [connection, flag, drainDone] in defer { drainDone.signal() } + var drained = 0 // Decode happens IN-CORE (libopus multistream) — AudioToolbox's Opus path is // stereo-only — and is handed back as interleaved f32 PCM in wire channel order. // Per-iteration autorelease pool: no runloop on this thread (see Stage2Pipeline). @@ -421,6 +422,17 @@ public final class SessionAudio { ring.write(base, count: pcm.frameCount * pcm.channels) } } + // Periodic vitals (~10 s at the protocol's 5 ms frames). The other three clients + // log buffer depth and underruns; without this an Apple audio report — latency or + // dropout — arrives with no numbers at all, which is the position every platform + // was in before the 2026-08 audio work. + drained += 1 + if drained % 2_000 == 0 { + let s = ring.stats + log.info( + "audio: buffer_ms=\(s.bufferedMS) target_ms=\(s.targetMS) underruns=\(s.underruns) drift_sheds=\(s.sheds)" + ) + } return true } } diff --git a/crates/pf-client-core/src/audio_wasapi.rs b/crates/pf-client-core/src/audio_wasapi.rs index 0c9b6d69..12fda251 100644 --- a/crates/pf-client-core/src/audio_wasapi.rs +++ b/crates/pf-client-core/src/audio_wasapi.rs @@ -3,14 +3,15 @@ //! //! The WASAPI twin of `audio.rs` (PipeWire) — same public surface (`AudioPlayer::spawn`/ //! `take_buffer`/`push`, `MicStreamer::spawn`), swapped in by lib.rs's `#[path]` so the -//! session pump compiles against one `crate::audio` on both OSes. Adapted from -//! `clients/windows/src/audio.rs` (which remains the WinUI shell's own copy until its -//! built-in streaming path is deleted). +//! session pump compiles against one `crate::audio` on both OSes. It began as a copy of the +//! WinUI shell's own audio path; that shell's built-in streaming path has since been deleted, +//! so this is now the only WASAPI client ring. //! -//! Playback mirrors the host's virtual-mic producer's adaptive jitter buffer: the session -//! pump pushes 5 ms Opus-decoded chunks on the network clock; the WASAPI render thread -//! pulls whole event-driven quanta on the device clock. Prime to ~3 quanta before -//! producing, cap the ring so latency stays bounded, re-prime after a real drain. +//! Playback: the session pump pushes 5 ms Opus-decoded chunks on the network clock; the WASAPI +//! render thread pulls whole event-driven quanta on the device clock. The depth policy between +//! them is the SHARED `punktfunk_core::audio::JitterPolicy` (`JitterTuning::WASAPI`) — target in +//! milliseconds, crossfaded drift correction, de-prime hysteresis — so all four clients behave +//! the same way and none of them can ratchet latency upward. //! //! WASAPI objects are COM-apartment-bound and not `Send`, so they live on a dedicated //! thread (the same discipline as the host's `wasapi_cap`); only the channels + stop flag diff --git a/crates/punktfunk-core/src/audio.rs b/crates/punktfunk-core/src/audio.rs index f20d7780..4ebaba8a 100644 --- a/crates/punktfunk-core/src/audio.rs +++ b/crates/punktfunk-core/src/audio.rs @@ -173,6 +173,90 @@ impl OpusLayout { } } +/// What the audio plane will actually cost this session: the tier to encode at, and whether the +/// redundant `0xD2` plane is affordable. Produced by [`plan_audio_budget`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct AudioBudget { + pub tier: AudioTier, + pub redundancy: bool, + /// Total wire cost in kbps, redundancy included — what the decision was made against. + pub kbps: u32, +} + +/// Share of the session's video bitrate the audio plane may spend. Audio rides QUIC datagrams, +/// OUTSIDE the ABR loop, so whatever it takes is taken off the top and adaptive bitrate can +/// neither see nor reclaim it — which is exactly why it needs a budget of its own. +const AUDIO_BUDGET_PCT: u32 = 5; +/// …but never squeeze audio below the Low tier. A stream with unintelligible audio is worse than +/// one that spends a few percent more, and the floor is what stops a very low video bitrate from +/// silently producing a useless audio plane. +const AUDIO_BUDGET_FLOOR_KBPS: u32 = 96; + +/// Choose the encode tier and whether to send redundancy, given the session's resolved VIDEO +/// bitrate. +/// +/// **Why this exists.** Tier `High` and the redundant plane were introduced separately, each +/// justified as "about 1 % of the video budget" — but they multiply: 256 kbps stereo sent twice is +/// 512 kbps, which is ~2.5 % of a 20 Mbps session and ~10 % of a 5 Mbps one. Nothing added the two +/// together, and nothing capped the total, so on a constrained link the audio plane quietly took a +/// tenth of the bandwidth that ABR was carefully managing the rest of. +/// +/// The ladder is ordered by preference, not by cost: transparent audio beats redundant audio (the +/// complaint this whole program came from was quality, and the redundancy only pays off under +/// loss), so `High` alone outranks `Standard` + redundancy even though they cost the same. +/// `requested` lets an operator ask for a specific tier; the budget can lower it but never raises +/// it above what was asked. +pub fn plan_audio_budget( + video_kbps: u32, + channels: u8, + requested: AudioTier, + client_wants_redundancy: bool, +) -> AudioBudget { + let budget = (video_kbps.saturating_mul(AUDIO_BUDGET_PCT) / 100).max(AUDIO_BUDGET_FLOOR_KBPS); + let layout = layout_for(channels, false); + let cost = |tier: AudioTier, red: bool| -> u32 { + let one = (layout.bitrate_for(tier) / 1000).max(0) as u32; + if red { + one.saturating_mul(2) + } else { + one + } + }; + // Preference order, best first. An operator asking for `Low` must not be handed `High`, so + // candidates above the request are filtered out. + let rank = |t: AudioTier| match t { + AudioTier::Low => 0, + AudioTier::Standard => 1, + AudioTier::High => 2, + }; + let ladder = [ + (AudioTier::High, true), + (AudioTier::High, false), + (AudioTier::Standard, true), + (AudioTier::Standard, false), + (AudioTier::Low, false), + ]; + for (tier, red) in ladder { + if rank(tier) > rank(requested) || (red && !client_wants_redundancy) { + continue; + } + let kbps = cost(tier, red); + if kbps <= budget { + return AudioBudget { + tier, + redundancy: red, + kbps, + }; + } + } + // Nothing fit — take the cheapest thing that still works rather than muting audio. + AudioBudget { + tier: AudioTier::Low, + redundancy: false, + kbps: cost(AudioTier::Low, false), + } +} + /// Pick the layout for a negotiated channel count. Unknown counts fall back to stereo (clients /// only ever request 2/6/8). `high_quality` selects the uncoupled high-bitrate config. pub fn layout_for(channels: u8, high_quality: bool) -> &'static OpusLayout { @@ -874,6 +958,85 @@ mod tests { assert_eq!(AudioTier::parse(""), None); } + // ---- the audio bandwidth budget -------------------------------------------------------- + + /// THE regression this guards: `High` (256 kbps stereo) and the redundant plane (x2) were + /// each justified as "~1 % of the video budget" and nobody added them together. 512 kbps is + /// ~10 % of a 5 Mbps session — and audio is outside the ABR loop, so ABR cannot reclaim it. + #[test] + fn budget_steps_down_as_the_link_narrows() { + let plan = |kbps| plan_audio_budget(kbps, 2, AudioTier::High, true); + // Roomy link: everything on. + let b = plan(20_000); + assert_eq!((b.tier, b.redundancy), (AudioTier::High, true)); + assert_eq!(b.kbps, 512); + // Halve it and redundancy is the first thing to go — quality is what the field report + // was about, and redundancy only pays under loss. + assert_eq!(plan(10_000).tier, AudioTier::High); + assert!(!plan(10_000).redundancy); + // Tighter still: down to Standard. + assert_eq!(plan(5_000).tier, AudioTier::Standard); + assert!(!plan(5_000).redundancy); + // A genuinely narrow link lands on Low, and never below it. + assert_eq!(plan(1_000).tier, AudioTier::Low); + assert_eq!(plan(1).tier, AudioTier::Low); + assert_eq!( + plan(0).kbps, + 96, + "audio must survive an absurd video bitrate" + ); + } + + /// The budget must never spend more than its share, at any bitrate or channel count. + #[test] + fn budget_never_exceeds_its_share() { + for kbps in [0u32, 500, 1_000, 2_000, 5_000, 10_000, 20_000, 100_000] { + for ch in [2u8, 6, 8] { + let b = plan_audio_budget(kbps, ch, AudioTier::High, true); + let allowed = + (kbps.saturating_mul(AUDIO_BUDGET_PCT) / 100).max(AUDIO_BUDGET_FLOOR_KBPS); + let floor = plan_audio_budget(0, ch, AudioTier::Low, false).kbps; + assert!( + b.kbps <= allowed || b.kbps == floor, + "{ch}ch at {kbps} kbps: spent {} of {allowed}", + b.kbps + ); + } + } + } + + /// Surround costs more per tier, so the same link must step it down sooner than stereo — + /// the budget is about total wire cost, not about the tier name. + #[test] + fn budget_accounts_for_the_channel_count() { + let stereo = plan_audio_budget(10_000, 2, AudioTier::High, true); + let surround = plan_audio_budget(10_000, 8, AudioTier::High, true); + assert_eq!(stereo.tier, AudioTier::High); + assert!(surround.kbps <= stereo.kbps.max(surround.kbps), "sanity"); + // 7.1 at High is 768 kbps — far past a 500 kbps allowance, so it must have stepped down. + assert!( + surround.kbps < 768, + "7.1 High must not fit a 10 Mbps budget" + ); + } + + /// The budget may LOWER what was asked for, never raise it: an operator who set `low` gets + /// `low` on a 100 Mbps link, and a client that never asked for redundancy never gets it. + #[test] + fn budget_respects_the_request() { + let b = plan_audio_budget(100_000, 2, AudioTier::Low, true); + assert_eq!(b.tier, AudioTier::Low); + let b = plan_audio_budget(100_000, 2, AudioTier::Standard, true); + assert_eq!(b.tier, AudioTier::Standard); + assert!(b.redundancy, "Standard + redundancy fits a huge link"); + let b = plan_audio_budget(100_000, 2, AudioTier::High, false); + assert_eq!(b.tier, AudioTier::High); + assert!( + !b.redundancy, + "a client that did not ask must never be sent 0xD2" + ); + } + // ---- the de-jitter policy ------------------------------------------------------------ /// Interleaved samples per ms at `channels`. diff --git a/crates/punktfunk-host/src/audio/linux/mod.rs b/crates/punktfunk-host/src/audio/linux/mod.rs index b6fbd0cd..1275cd0f 100644 --- a/crates/punktfunk-host/src/audio/linux/mod.rs +++ b/crates/punktfunk-host/src/audio/linux/mod.rs @@ -674,6 +674,8 @@ fn pw_thread( }) .register(); + // Which source the negotiated format below actually describes — see the note there. + let sink_mode = sink_name.is_some(); let props = match &sink_name { // Stream-sink mode: this stream IS the sink (media.class + Direction::Input). Apps // play into it, PipeWire mixes them, process() receives the mix. Mirrors the @@ -710,8 +712,25 @@ fn pw_thread( let stream = pw::stream::StreamBox::new(&core, "punktfunk-audio", props) .context("pw audio Stream")?; + // The capture callback's state: the hand-off channel plus this plane's vitals. Before + // this it was the bare `tx`, and the desktop-audio plane logged NOTHING between "capture + // started" and the session ending — no level, no cadence, and in particular no sign of + // the silent drop below. That is exactly what made the 2026-08-03 Windows field report + // un-triageable, and the Linux half kept it after the Windows half was fixed. + struct CapUd { + tx: std::sync::mpsc::SyncSender>, + channels: u32, + stats: crate::audio::capture_policy::CaptureStats, + last_stats: std::time::Instant, + } + let ud = CapUd { + tx, + channels, + stats: Default::default(), + last_stats: std::time::Instant::now(), + }; let _listener = stream - .add_local_listener_with_user_data(tx) + .add_local_listener_with_user_data(ud) .state_changed({ let mainloop = mainloop.clone(); move |_s, _ud, old, new| { @@ -723,22 +742,32 @@ fn pw_thread( } } }) - .param_changed(|_stream, _tx, id, param| { + .param_changed(move |_stream, _tx, id, param| { let Some(param) = param else { return }; if id != pw::spa::param::ParamType::Format.as_raw() { return; } let mut info = AudioInfoRaw::default(); if info.parse(param).is_ok() { + // `stream_sink` says WHICH source this format describes, and that changes how + // much it is worth. In stream-sink mode the host owns the sink, so this IS the + // format apps render into and the desktop mix cannot have been narrowed before + // we saw it. In LEGACY monitor mode we are capturing someone else's sink + // through PipeWire's resampler: a 16 kHz Bluetooth headset upstream would + // still be reported here as a clean 48 kHz, exactly the way WASAPI's + // autoconvert hid the same thing on Windows (the 2026-08-03 report). Reading + // the monitored node's OWN rate needs a registry lookup this stream does not + // do — recorded as an open gap rather than implied to be covered. tracing::info!( format = ?info.format(), rate = info.rate(), channels = info.channels(), + stream_sink = sink_mode, "audio format negotiated" ); } }) - .process(|stream, tx| { + .process(|stream, ud| { let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { let Some(mut buffer) = stream.dequeue_buffer() else { return; @@ -774,7 +803,34 @@ fn pw_thread( ]; samples.push(f32::from_le_bytes(b)); } - let _ = tx.try_send(samples); // drop if the encoder is behind + ud.stats.observe(&samples, ud.channels); + // Non-blocking and lossy, as before — but COUNTED. A full channel means the + // encode thread is not keeping up, and because the encoder simply + // concatenates across the hole every dropped chunk is a click AND a + // permanent shift of everything after it. + if ud.tx.try_send(samples).is_err() { + ud.stats.dropped_chunks += 1; + } + if ud.last_stats.elapsed() >= crate::audio::capture_policy::STATS_EVERY { + let (peak_db, rms_db, delivered_pct) = + ud.stats.summary(ud.last_stats.elapsed(), SAMPLE_RATE); + if ud.stats.dropped_chunks > 0 { + tracing::warn!( + dropped_chunks = ud.stats.dropped_chunks, + "the audio encode thread could not keep up — captured audio was \ + DROPPED; the stream will click and everything after it shifts" + ); + } + tracing::info!( + peak_db = format!("{peak_db:.1}"), + rms_db = format!("{rms_db:.1}"), + delivered_pct = format!("{delivered_pct:.0}"), + dropped_chunks = ud.stats.dropped_chunks, + "desktop audio capture" + ); + ud.stats = Default::default(); + ud.last_stats = std::time::Instant::now(); + } })); if outcome.is_err() { tracing::error!("panic in pipewire audio callback — chunk dropped"); diff --git a/crates/punktfunk-host/src/native.rs b/crates/punktfunk-host/src/native.rs index 91591ead..c3d8f69b 100644 --- a/crates/punktfunk-host/src/native.rs +++ b/crates/punktfunk-host/src/native.rs @@ -1308,11 +1308,17 @@ async fn serve_session( let cap = audio_cap.clone(); let channels = welcome.audio_channels; // Read the granted bit back off the Welcome (the cursor plane's precedent), so the wire - // the client was promised and the wire we actually send cannot disagree. - let redundancy = welcome.host_caps & punktfunk_core::quic::HOST_CAP_AUDIO_RED != 0; + // the client was promised and the wire we actually send cannot disagree — then re-derive + // the SAME budget rung from it, so the encode tier and the redundancy decision are one + // choice made once rather than two settings that can drift apart. + let budget = handshake::audio_budget( + welcome.host_caps & punktfunk_core::quic::HOST_CAP_AUDIO_RED != 0, + welcome.bitrate_kbps, + channels, + ); std::thread::Builder::new() .name("punktfunk1-audio".into()) - .spawn(move || audio_thread(conn, stop, cap, channels, redundancy)) + .spawn(move || audio_thread(conn, stop, cap, channels, budget)) .map_err(|e| tracing::warn!(error = %e, "audio thread spawn failed — session continues without audio")) .ok() } else { diff --git a/crates/punktfunk-host/src/native/audio.rs b/crates/punktfunk-host/src/native/audio.rs index 852026fe..50cd5033 100644 --- a/crates/punktfunk-host/src/native/audio.rs +++ b/crates/punktfunk-host/src/native/audio.rs @@ -84,27 +84,16 @@ pub(super) fn audio_thread( stop: Arc, audio_cap: AudioCapSlot, channels: u8, - redundancy: bool, + budget: punktfunk_core::audio::AudioBudget, ) { use crate::audio::SAMPLE_RATE; const FRAME_MS: usize = 5; const SAMPLES_PER_FRAME: usize = SAMPLE_RATE as usize * FRAME_MS / 1000; // 240 let want = punktfunk_core::audio::normalize_channels(channels); - // WP1.1 — encode tier. Unknown spellings warn and fall back rather than silently downgrading - // someone's audio (the whole point of the setting is that quality stopped being invisible). - let tier = match pf_host_config::config().audio_quality.as_deref() { - None => punktfunk_core::audio::AudioTier::default(), - Some(s) => match punktfunk_core::audio::AudioTier::parse(s) { - Some(t) => t, - None => { - tracing::warn!( - value = %s, - "PUNKTFUNK_AUDIO_QUALITY is not one of low/standard/high — using the default" - ); - punktfunk_core::audio::AudioTier::default() - } - }, - }; + // Tier and redundancy are ONE decision, budgeted against the session's video bitrate — see + // `handshake::audio_budget`. An unparseable `audio.quality` was already warned about there + // and fell back to the default, so nothing here can silently downgrade someone's audio. + let (tier, redundancy) = (budget.tier, budget.redundancy); // Reuse the cached capturer ONLY when its channel count matches this session's; a stereo // capturer left by a prior session must not feed a 5.1/7.1 session (the encoder + the client's @@ -166,7 +155,7 @@ pub(super) fn audio_thread( tracing::info!( channels = want, tier = tier.as_str(), - kbps = punktfunk_core::audio::layout_for(want, false).bitrate_for(tier) / 1000, + kbps = budget.kbps, redundancy, "punktfunk/1 audio streaming (Opus 48 kHz, 5 ms datagrams)" ); @@ -261,7 +250,7 @@ pub(super) fn audio_thread( _stop: Arc, _audio_cap: AudioCapSlot, _channels: u8, - _redundancy: bool, + _budget: punktfunk_core::audio::AudioBudget, ) { tracing::warn!("punktfunk/1 audio requires Linux or Windows — session continues without it"); } diff --git a/crates/punktfunk-host/src/native/handshake.rs b/crates/punktfunk-host/src/native/handshake.rs index 8b7cf3ef..9027f755 100644 --- a/crates/punktfunk-host/src/native/handshake.rs +++ b/crates/punktfunk-host/src/native/handshake.rs @@ -24,24 +24,61 @@ use super::*; /// paints on a Mutter virtual stream), and only a can't-blend backend falls back to the /// compositor EMBED. THE single predicate: the Welcome's `HOST_CAP_CURSOR` bit is computed /// from it, and the session wiring reads that bit back. -/// Whether this session sends the REDUNDANT desktop-audio plane (`0xD2`) — THE single predicate -/// behind the Welcome's `HOST_CAP_AUDIO_RED` bit, which `serve_session` reads back to configure the -/// audio thread. +/// THE single audio-plane decision for a session: the encode tier AND whether the redundant +/// `0xD2` plane is sent. The Welcome's `HOST_CAP_AUDIO_RED` bit is computed from it, and +/// `serve_session` reads that bit back to configure the audio thread — so the wire the client is +/// promised and the wire we send cannot disagree. /// -/// Capable-and-agreed: the client must have advertised `CLIENT_CAP_AUDIO_RED`, so a session with an -/// older client keeps the plain `0xC9` wire byte-for-byte. `audio.redundancy` ( -/// `PUNKTFUNK_AUDIO_REDUNDANCY`) can force it off on a link where the extra ~1 % is unwelcome, or -/// force it on for testing. +/// Capable-and-agreed for redundancy: the client must have advertised `CLIENT_CAP_AUDIO_RED`, so a +/// session with an older client keeps the plain `0xC9` wire byte-for-byte. +/// +/// **Both halves are then BUDGETED against the session's video bitrate** +/// ([`plan_audio_budget`](punktfunk_core::audio::plan_audio_budget)). Tier `High` and redundancy +/// were introduced separately, each costed as "~1 % of the video budget", and they multiply: +/// 256 kbps stereo sent twice is 512 kbps — ~10 % of a 5 Mbps session. Audio rides QUIC datagrams, +/// outside the ABR loop, so ABR can neither see that nor reclaim it. The budget is what stops a +/// constrained link silently handing a tenth of its bandwidth to audio. +/// +/// The operator's `audio.quality` / `audio.redundancy` settings are the REQUEST; the budget may +/// lower them, never raise them. /// /// NB the plan's "only while the link is actually losing packets" gate is deliberately not here: /// turning redundancy on and off mid-session changes the wire tag, and the client's decoder would -/// have to re-derive which plane it is on from every datagram. The cost being avoided is ~1 % of a -/// video budget, which is not worth that fragility — so the decision is made once, at handshake. -pub(super) fn audio_redundancy(client_caps: u8) -> bool { - if client_caps & punktfunk_core::quic::CLIENT_CAP_AUDIO_RED == 0 { - return false; - } - pf_host_config::config().audio_redundancy.unwrap_or(true) +/// have to re-derive which plane it is on from every datagram. Deciding once, at handshake, against +/// a bitrate we already know is both cheaper and more predictable. +/// `wants_redundancy` is the caller's answer to "is `0xD2` even on the table" — at handshake that +/// is the client's cap AND the operator's setting; afterwards it is the GRANTED +/// `HOST_CAP_AUDIO_RED` bit, so the audio thread re-derives the same rung of the same ladder. +pub(super) fn audio_budget( + wants_redundancy: bool, + video_kbps: u32, + channels: u8, +) -> punktfunk_core::audio::AudioBudget { + let configured = pf_host_config::config().audio_quality.as_deref(); + let requested = match configured { + None => punktfunk_core::audio::AudioTier::default(), + Some(s) => punktfunk_core::audio::AudioTier::parse(s).unwrap_or_else(|| { + // Once per process: this runs per session, and an operator with a typo in host.env + // does not need it on every connect. Never silently downgrade someone's audio. + static WARNED: std::sync::Once = std::sync::Once::new(); + WARNED.call_once(|| { + tracing::warn!( + value = %s, + "audio.quality (PUNKTFUNK_AUDIO_QUALITY) is not one of low/standard/high — \ + using the default" + ); + }); + punktfunk_core::audio::AudioTier::default() + }), + }; + punktfunk_core::audio::plan_audio_budget(video_kbps, channels, requested, wants_redundancy) +} + +/// The operator's answer to "may this session use redundancy at all", before the budget is +/// consulted: the client must be able to decode it and the operator must not have forced it off. +pub(super) fn redundancy_offered(client_caps: u8) -> bool { + client_caps & punktfunk_core::quic::CLIENT_CAP_AUDIO_RED != 0 + && pf_host_config::config().audio_redundancy.unwrap_or(true) } pub(super) fn cursor_forward( @@ -585,10 +622,16 @@ pub(super) async fn negotiate( } else { 0 } - // Redundant desktop-audio plane (0xD2): the client asked, and the operator has not - // forced it off. Capable-and-agreed, like the cursor bit — a client that did not ask - // keeps the plain 0xC9 wire byte-for-byte. - | if audio_redundancy(hello.client_caps) { + // Redundant desktop-audio plane (0xD2): the client asked, the operator has not forced + // it off, AND it fits the session's audio budget. Capable-and-agreed like the cursor + // bit — a client that did not ask keeps the plain 0xC9 wire byte-for-byte. + | if audio_budget( + redundancy_offered(hello.client_caps), + bitrate_kbps, + audio_channels, + ) + .redundancy + { punktfunk_core::quic::HOST_CAP_AUDIO_RED } else { 0 -- 2.54.0 From c6597cbeb547b702d1f40d346c9f749b7a9fc7fb Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 4 Aug 2026 17:58:39 +0200 Subject: [PATCH 6/7] docs(troubleshooting): the audio quality knobs are a request, not a guarantee MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The page claimed "audio is a fraction of a percent of a stream's bandwidth, so high costs nothing worth counting". At 256 kbps plus redundancy that is 512 kbps — true of a 20 Mbps session, wrong by an order of magnitude on a 5 Mbps one, which is why the budget now exists. Says what actually happens on a narrow link, and points at the log line that reports the settled tier. Co-Authored-By: Claude Opus 5 (1M context) --- docs-site/content/docs/troubleshooting.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/docs-site/content/docs/troubleshooting.md b/docs-site/content/docs/troubleshooting.md index 9d925b6c..d4e8350f 100644 --- a/docs-site/content/docs/troubleshooting.md +++ b/docs-site/content/docs/troubleshooting.md @@ -339,8 +339,17 @@ PUNKTFUNK_AUDIO_QUALITY=high # low | standard | high (default high — stereo PUNKTFUNK_AUDIO_REDUNDANCY=1 # force the loss-resilient audio plane on (default: automatic) ``` -Audio is a fraction of a percent of a stream's bandwidth, so `high` costs nothing worth counting. -`standard` reproduces the pre-0.25 encoder exactly if you want to compare. +Both are a **request**, not a guarantee: the host budgets audio against the session's video +bitrate and steps it down on a narrow link, because audio is not managed by adaptive bitrate — so +whatever it takes is taken off the top. On a roomy link you get 256 kbps plus loss redundancy; as +the link narrows the host drops redundancy first, then the tier, and never goes below ~96 kbps. The +session log line says what it settled on: + +``` +INFO punktfunk/1 audio streaming … tier=high kbps=512 redundancy=true +``` + +`standard` reproduces the pre-0.25 encoder exactly if you want to A/B it. ## Audio lags behind the picture -- 2.54.0 From 2dfb7791a2ad026eb33f2a6916bf16b66c55bbd7 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 4 Aug 2026 18:14:24 +0200 Subject: [PATCH 7/7] fix(apple): the drift test tripped Swift's static exclusivity check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught what my local harness could not: reading `huge.count` inside the closure that already holds `huge` exclusively is an exclusivity violation, so PunktfunkKitTests failed to compile. The blind spot is worth recording. I verified `AudioRing` by compiling it against a standalone harness whose bodies were TOP-LEVEL code, where Swift applies DYNAMIC exclusivity — the same statement in a function body gets the static check and is a hard error. A harness that does not share the shape of the thing it stands in for can be green for a reason the real build does not have. The harness now puts every body in a method and compiles with `-enforce-exclusivity=checked`. Length now comes off the buffer pointer (`$0.count`), which is what the closure already owns. Co-Authored-By: Claude Opus 5 (1M context) --- .../apple/Tests/PunktfunkKitTests/AudioRingDriftTests.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clients/apple/Tests/PunktfunkKitTests/AudioRingDriftTests.swift b/clients/apple/Tests/PunktfunkKitTests/AudioRingDriftTests.swift index 70f3b56f..a86949dd 100644 --- a/clients/apple/Tests/PunktfunkKitTests/AudioRingDriftTests.swift +++ b/clients/apple/Tests/PunktfunkKitTests/AudioRingDriftTests.swift @@ -79,9 +79,11 @@ final class AudioRingDriftTests: XCTestCase { scratch.withUnsafeMutableBufferPointer { ring.read(into: $0.baseAddress!, count: want) } XCTAssertTrue(scratch.contains { $0 != 0 }, "should be playing after priming") - // Drain it dry with one oversized read, then feed a normal quantum again. + // Drain it dry with one oversized read, then feed a normal quantum again. The length comes + // off the buffer pointer, not off `huge`: touching the array inside the closure that is + // already holding it exclusively is an exclusivity violation. var huge = [Float](repeating: 0, count: 200 * perMS) - huge.withUnsafeMutableBufferPointer { ring.read(into: $0.baseAddress!, count: huge.count) } + huge.withUnsafeMutableBufferPointer { ring.read(into: $0.baseAddress!, count: $0.count) } let feed = [Float](repeating: 0.5, count: want) feed.withUnsafeBufferPointer { ring.write($0.baseAddress!, count: want) } scratch.withUnsafeMutableBufferPointer { ring.read(into: $0.baseAddress!, count: want) } -- 2.54.0