From 3f738a9989b29a5d7535d8ddf955778983f93d4a Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 13 Aug 2026 20:05:41 +0200 Subject: [PATCH 1/3] fix(pf-vkdecode): AV1's "maximum parameters" level is not a level above the ceiling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 2026-08-13 field report from the same RTX 5060 client as a02014ec: every AV1 session demoted to D3D11VA with "outside device caps: stream level (seq_level_idx 31) above the device's maxLevel (AV1 Std level 23)" — 4K120, NVIDIA, the hardware decoding the stream trivially on the D3D11VA rung it fell through to. a02014ec fixed the H.264/H.265 half of exactly this and left AV1 alone on the premise that "no over-declaration has been seen in the field"; the reporter's own log from that same day already showed otherwise. seq_level_idx is a 5-bit field. Annex A defines 0…23 (levels 2.0…7.3), reserves 24…30, and makes 31 the "maximum parameters" level — the spec's own way of saying the bitstream is NOT constrained to a level. StdVideoAV1Level stops at 7.3 = 23, so 31 has no Std code point and the index-coded comparison that holds across 0…23 says nothing here: 31 > 23 is true even of a device that decodes everything AV1 can name, which is what makes it useless as a capability test. We write no AV1 level on any host encode path, so whichever sentinel the vendor's encoder defaults to is what the client must accept. So the gate warns once and proceeds, like its H.265 sibling. Unlike H.265 there is nothing to clamp: StdVideoAV1SequenceHeader carries no level field, so the declaration never reaches the driver and cannot be invalid usage. The stream's real demands stay enforced where they are physical facts — coded extent and DPB depth, both checked at session build. Not verified on glass: no RTX 5060 here, and the reporter's box is the only one that has produced a seq_level_idx 31 stream. The unit test pins the arithmetic that made the refusal look reasonable. --- crates/pf-vkdecode/src/caps.rs | 8 ++- crates/pf-vkdecode/src/caps_av1.rs | 13 +++- crates/pf-vkdecode/src/decoder_av1.rs | 96 ++++++++++++++++++++++----- 3 files changed, 97 insertions(+), 20 deletions(-) diff --git a/crates/pf-vkdecode/src/caps.rs b/crates/pf-vkdecode/src/caps.rs index 90b4639c..b9ccd22a 100644 --- a/crates/pf-vkdecode/src/caps.rs +++ b/crates/pf-vkdecode/src/caps.rs @@ -185,8 +185,12 @@ pub enum MaxLevelIdc { H265(hh::StdVideoH265LevelIdc), /// `VkVideoDecodeAV1CapabilitiesKHR::maxLevel`. Unlike the other two this code /// space is the BITSTREAM's own: `StdVideoAV1Level` is index-coded exactly like - /// AV1's `seq_level_idx` (2.0 = 0, 2.1 = 1, … 7.3 = 23), so the decoder's gate - /// compares the sequence header's value against it directly. + /// AV1's `seq_level_idx` (2.0 = 0, 2.1 = 1, … 7.3 = 23). + /// + /// ⚠ Only over 0…23. `seq_level_idx` is 5 bits, and 31 is Annex A's "maximum + /// parameters" sentinel — no level constraint — which outranks even a device + /// reporting the enum's top value. The AV1 gate therefore treats a stream above + /// this ceiling as advisory instead of refusing it (`VkAv1Decoder::ensure_state`). Av1(hh::StdVideoAV1Level), } diff --git a/crates/pf-vkdecode/src/caps_av1.rs b/crates/pf-vkdecode/src/caps_av1.rs index c3b88b9a..6c81b506 100644 --- a/crates/pf-vkdecode/src/caps_av1.rs +++ b/crates/pf-vkdecode/src/caps_av1.rs @@ -211,9 +211,16 @@ pub struct RawAv1Caps { pub max_coded_extent: vk::Extent2D, pub max_dpb_slots: u32, pub max_active_reference_pictures: u32, - /// `VkVideoDecodeAV1CapabilitiesKHR::maxLevel` (index-coded Std level — the - /// SAME numbering as the bitstream's `seq_level_idx`, which is what makes the - /// decoder's level gate a plain comparison). + /// `VkVideoDecodeAV1CapabilitiesKHR::maxLevel` (index-coded Std level — the same + /// numbering as the bitstream's `seq_level_idx` OVER 0…23, which is the whole + /// range `StdVideoAV1Level` enumerates). + /// + /// ⚠ That correspondence does not extend to the rest of the bitstream field. + /// `seq_level_idx` is 5 bits: 24…30 are reserved and 31 is Annex A's "maximum + /// parameters" sentinel — "not constrained to a level" — which has no Std code + /// point and is NOT an ordering above 7.3. The decoder's gate therefore treats + /// a stream above this ceiling as advisory rather than comparing it as a level + /// (`VkAv1Decoder::ensure_state`). pub max_level: hh::StdVideoAV1Level, /// `VkVideoCapabilitiesKHR::stdHeaderVersion` — session creation echoes it back. pub std_header_version: vk::ExtensionProperties, diff --git a/crates/pf-vkdecode/src/decoder_av1.rs b/crates/pf-vkdecode/src/decoder_av1.rs index bb7c7212..b106917b 100644 --- a/crates/pf-vkdecode/src/decoder_av1.rs +++ b/crates/pf-vkdecode/src/decoder_av1.rs @@ -100,6 +100,7 @@ use pf_bitstream::av1::NUM_REF_SLOTS; use pf_bitstream::h264::DisplayCrop; use tracing::debug; use tracing::trace; +use tracing::warn; use crate::caps::DecodeCaps; use crate::caps::DecodeProfile; @@ -688,6 +689,10 @@ pub struct VkAv1Decoder { /// through a temporal unit, which is why the skip is per FRAME while the error /// is per ACCESS UNIT. awaiting_key: bool, + /// One-shot latch for the over-declared-level warning, so a stream whose + /// sequence header sits above the device ceiling says so once per decoder + /// rather than once per access unit (`ensure_state` runs per AU). + level_advisory_warned: bool, } impl VkAv1Decoder { @@ -728,6 +733,7 @@ impl VkAv1Decoder { device_lost: false, recovery: RecoveryLatch::default(), awaiting_key: false, + level_advisory_warned: false, }) } @@ -745,8 +751,10 @@ impl VkAv1Decoder { /// /// The negotiated facts are a HINT (the in-band sequence header is /// authoritative), so this is deliberately not a promise that decode will - /// succeed: the level ceiling and a sequence header that disagrees with the - /// Welcome still surface at the first AU. + /// succeed: a coded extent outside the caps, a DPB deeper than the device + /// allows, and a sequence header that disagrees with the Welcome all still + /// surface at the first AU. The declared LEVEL is not among them — it is + /// advisory, and `ensure_state` only warns on it. pub fn probe_stream_support( &self, chroma_format_idc: u8, @@ -1478,8 +1486,9 @@ impl VkAv1Decoder { self.flush(); } - /// Session/caps for THIS plan exist and match its extent + profile, and the - /// stream sits inside the device's level ceiling. + /// Session/caps for THIS plan exist and match its extent + profile. A declared + /// level above the device ceiling warns once and proceeds — see the gate below + /// for why an AV1 `seq_level_idx` is advisory and 31 is not even a level. fn ensure_state(&mut self, plan: &AuPlan) -> Result<(), VkDecodeError> { let key = profile_key_for(plan)?; if self.caps.as_ref().map(|(k, _)| *k) != Some(key) { @@ -1491,17 +1500,39 @@ impl VkAv1Decoder { unsafe { query_av1_caps(&self.dev, key) }.map_err(|r| caps_query_error(r, key))?; self.caps = Some((key, derive_caps_av1(&raw, wanted)?)); } - // The level gate. AV1's `StdVideoAV1Level` is index-coded exactly like the - // bitstream's `seq_level_idx` (2.0 = 0 … 7.3 = 23) and ascends with the - // level, so this is a plain comparison — of AV1 code points against an AV1 - // ceiling, the pairing `MaxLevelIdc`'s tag exists to keep honest. + // The declared level vs the device ceiling: a DECLARED level above `maxLevel` + // is NOT a refusal, for the reason `VkH265Decoder::ensure_state` spells out — + // the level is a CLAIM, and the stream's real demands are enforced where they + // are physical facts (coded extent and DPB depth, checked in `rebuild_state`). + // + // AV1 makes the point sharper than H.265 did. `seq_level_idx` is a 5-bit + // field; Annex A defines 0…23 (levels 2.0…7.3) and reserves 24…30, but **31 is + // the "maximum parameters" level — the spec's own way of saying the bitstream + // is not constrained to any level at all**. `StdVideoAV1Level` has no code + // point for it (it stops at 7.3 = 23), so the index-coded comparison that + // holds across 0…23 is meaningless against 31: the sentinel is not a level + // and 31 > 23 is not "too demanding". Real-time encoders emit it as a matter + // of course — a 2026-08-13 field report (RTX 5060 client, 4K120) had EVERY + // AV1 session demote to D3D11VA on "stream level (seq_level_idx 31) above the + // device's maxLevel (AV1 Std level 23)" while the same hardware decoded the + // stream trivially. We never write an AV1 level on any host encode path, so + // whatever the vendor defaults to is what the client must accept. + // + // Unlike H.265 there is nothing to clamp: `StdVideoAV1SequenceHeader` carries + // no level field (see `params_av1`), so the declaration never reaches the + // driver and cannot be invalid usage. Warn once, proceed. let caps_max_level = self.caps.as_ref().expect("queried above").1.max_level_idc; let stream_level = u32::from(stream_level_idx(plan)); - if stream_level > caps_max_level.code_point() { - return Err(VkDecodeError::Unsupported(format!( - "stream level (seq_level_idx {stream_level}) above the device's \ - maxLevel ({caps_max_level})" - ))); + if stream_level > caps_max_level.code_point() && !self.level_advisory_warned { + self.level_advisory_warned = true; + warn!( + stream_level, + ceiling = %caps_max_level, + "stream declares an AV1 level above the device ceiling — the declared \ + level is advisory (seq_level_idx 31 means \"maximum parameters\", and \ + encoders over-declare); proceeding, since the level never reaches the \ + driver" + ); } let coded = coded_extent(plan); match &self.state { @@ -2907,10 +2938,45 @@ mod tests { assert_eq!(key.output_format(), Some(crate::caps::NV12)); assert!(!key.film_grain); - // The level gate reads operating point 0 and stays inside the Std range. + // The level gate reads operating point 0. This vector declares a real level, + // inside the Std range — the sentinel case is pinned separately below. assert!(stream_level_idx(&plan) <= 23); } + /// `seq_level_idx` 31 is Annex A's "maximum parameters" — "not constrained to a + /// level" — not a level above 7.3, and `StdVideoAV1Level` has no code point for + /// it. Comparing it as an ordinary level is what demoted every AV1 session on a + /// 2026-08-13 field report (RTX 5060, 4K120): `maxLevel` came back 23 (7.3, the + /// device's own maximum) and 31 > 23 refused a stream the hardware decodes fine. + /// + /// This pins the ARITHMETIC that made the refusal look reasonable, so nobody + /// restores the gate by reading `31 > 23` as "too demanding": + #[test] + fn the_av1_max_parameters_sentinel_is_not_a_level_above_the_ceiling() { + // The ceiling as the gate reads it, on a device that decodes everything the + // Std enum can name — 7.3, the top code point there is. + let ceiling = crate::caps::MaxLevelIdc::Av1(hh::StdVideoAV1Level_STD_VIDEO_AV1_LEVEL_7_3); + assert_eq!(ceiling.code_point(), 23, "the Std enum's top code point"); + + // Every `seq_level_idx` the Std enum names compares sanely against it… + for idx in 0..=ceiling.code_point() { + assert!(idx <= ceiling.code_point()); + } + // …and everything above is OUTSIDE that code space, not above the ceiling: + // 24…30 are reserved and 31 is "maximum parameters". A maxed-out device + // cannot satisfy the comparison, which is why it is not a capability test. + for idx in (ceiling.code_point() + 1)..=31 { + assert!( + idx > ceiling.code_point(), + "seq_level_idx {idx} is outside the Std range, not a more demanding level" + ); + } + + // The field report's exact pairing, kept legible: 31 against a ceiling of 23. + assert!(31 > ceiling.code_point()); + assert_eq!(format!("{ceiling}"), "AV1 Std level 23"); + } + #[test] fn only_a_decoded_key_frame_ends_the_wait_for_one() { let mut planner = Av1Planner::new(); @@ -2951,7 +3017,7 @@ mod tests { /// `PlanError::AwaitingIdr`, and the reason [`VkAv1Decoder::awaiting_key`]'s /// docs carry: a clean `Ok(None)` resets the consumer's demotion streak once /// per frame, so a rung whose every key frame fails (film grain on a device - /// without the grain profile; a level above `maxLevelIdc`; a sequence header + /// without the grain profile; a coded extent outside the caps; a sequence header /// disagreeing with the negotiation) would never demote and the session would /// hold a frozen screen with a clean bill of health. /// -- 2.54.0 From 1677d1c0c222e469ea9a9ababa3520a04e58a545 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 13 Aug 2026 21:00:26 +0200 Subject: [PATCH 2/3] fix(host/audio): stop warning that "the stream will click" when there is no stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 2026-08-13 field host log carried ten "the audio encode thread could not keep up — captured audio was DROPPED" warnings, the worst reading dropped_chunks=11251. That reads like catastrophic audio loss. It was not: not one sample anybody wanted was lost. PipeWire negotiated a 128-frame quantum, so the plane produces 48000/128 = 375 chunks/s and a 30 s stats window holds exactly 11250 — those windows were a 100 % drop rate, at peak_db=-120.0 (digital silence). Every one of the ten straddled a session boundary, and across all of them dropped_chunks/375 matches the seconds with NO live session in that window to within a fraction of a second (3890/375 = 10.4 s against a 10.5 s gap; 3616/375 = 9.6 s against 9.8 s). The capturer is host-lifetime: the native and gamestream planes PARK it between sessions (`AudioCapturer::idle`) rather than dropping it, but the consumer is the per-session encode thread. The hand-off channel is a bounded sync_channel(64), so ~170 ms after a session ends it is full and every try_send fails for as long as the host sits idle — counted as the encode thread falling behind, and reported with a sentence about a stream that does not exist. It is the worst kind of false alarm: it names a real failure mode, in a subsystem with real open audio work, at a volume that demands attention. So the drop counter now only counts while a session is actually reading, via an `active` flag shared with the capture thread and toggled by the same open/drain/idle/Drop transitions that already own the routing claim. A full channel under a live consumer still means exactly what it used to. Both backends: the parking call sites are platform-independent, so the WASAPI half had the identical defect (it had no `idle` at all, and gains one). Only the Linux half has field evidence. Verified: punktfunk-host clippy --all-targets -D warnings clean on linux/amd64 (container) and cargo check clean on the Windows CI runner. --- crates/punktfunk-host/src/audio/linux/mod.rs | 53 ++++++++++++++++--- .../src/audio/windows/wasapi_cap.rs | 42 ++++++++++++--- 2 files changed, 83 insertions(+), 12 deletions(-) diff --git a/crates/punktfunk-host/src/audio/linux/mod.rs b/crates/punktfunk-host/src/audio/linux/mod.rs index 804c7d34..d8052ec7 100644 --- a/crates/punktfunk-host/src/audio/linux/mod.rs +++ b/crates/punktfunk-host/src/audio/linux/mod.rs @@ -62,6 +62,17 @@ pub struct PwAudioCapturer { /// active). Toggled by open/[`drain`](AudioCapturer::drain) (claim) and /// [`idle`](AudioCapturer::idle)/Drop (release). claimed: bool, + /// Whether a session is currently CONSUMING this capturer, shared with the PipeWire + /// thread so the drop counter can tell "the encode thread fell behind" from "nobody is + /// reading". The capturer is host-lifetime and merely PARKED between sessions + /// ([`idle`](AudioCapturer::idle)), so without this the producer keeps filling the bounded + /// hand-off channel, every `try_send` fails once it is full, and the plane reports a 100 % + /// drop rate — warning that "the stream will click" when there is no stream. A 2026-08-13 + /// field host log carried ten such warnings, up to `dropped_chunks=11251` (= 30 s × 375 + /// chunks/s, i.e. every single chunk), each one straddling a session boundary and each one + /// meaningless. Distinct from `claimed`, which tracks the sink-routing claim and only + /// exists when the stream sink is enabled at all. + active: Arc, } impl PwAudioCapturer { @@ -90,10 +101,21 @@ impl PwAudioCapturer { // mode the sink node must exist before we claim the default to its name. let (ready_tx, ready_rx) = sync_channel::>(1); let thread_sink_name = sink_name.clone(); + // Opens at session start (see the routing claim below), so the consumer is live from + // the first chunk. + let active = Arc::new(AtomicBool::new(true)); + let thread_active = Arc::clone(&active); thread::Builder::new() .name("punktfunk-pw-audio".into()) .spawn(move || { - if let Err(e) = pw_thread(tx, quit_rx, channels, thread_sink_name, ready_tx) { + if let Err(e) = pw_thread( + tx, + quit_rx, + channels, + thread_sink_name, + ready_tx, + thread_active, + ) { tracing::error!(error = %format!("{e:#}"), "pipewire audio thread failed"); } }) @@ -118,12 +140,16 @@ impl PwAudioCapturer { quit: quit_tx, sink_name, claimed, + active, }) } } impl Drop for PwAudioCapturer { fn drop(&mut self) { + // The receiver dies with us; anything the producer still pushes is unwanted by + // definition, and it must not be reported as the encode thread falling behind. + self.active.store(false, Ordering::Relaxed); if self.claimed { self.claimed = false; stream_sink::release(); @@ -157,9 +183,15 @@ impl AudioCapturer for PwAudioCapturer { stream_sink::claim(name); self.claimed = true; } + // Ordered AFTER the backlog drain, so the producer never counts a drop against a + // channel this call is still emptying. + self.active.store(true, Ordering::Relaxed); } fn idle(&mut self) { + // Parked: from here the channel fills and stays full, and those drops are nobody's + // fault. See `PwAudioCapturer::active`. + self.active.store(false, Ordering::Relaxed); if self.claimed { self.claimed = false; stream_sink::release(); @@ -644,6 +676,7 @@ fn pw_thread( channels: u32, sink_name: Option, ready: std::sync::mpsc::SyncSender>, + active: Arc, ) -> Result<()> { use pipewire as pw; use pw::{properties::properties, spa}; @@ -735,6 +768,9 @@ fn pw_thread( /// never again — the one number that identifies a clamped quantum, invisible on every /// subsequent open (including every reopen after a device change). reported_quantum: bool, + /// Shared with the capturer — see [`PwAudioCapturer::active`]. Read on every + /// failed hand-off to keep parked-capturer backpressure out of the drop count. + active: Arc, } let ud = CapUd { tx, @@ -742,6 +778,7 @@ fn pw_thread( stats: Default::default(), last_stats: std::time::Instant::now(), reported_quantum: false, + active, }; let _listener = stream .add_local_listener_with_user_data(ud) @@ -844,11 +881,15 @@ fn pw_thread( samples.push(f32::from_le_bytes(b)); } 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() { + // Non-blocking and lossy, as before — but COUNTED, and only while a session + // is actually reading. A full channel under a LIVE consumer 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. A full channel under a PARKED capturer means nothing + // at all: the capturer is host-lifetime, so between sessions the channel + // fills once and then refuses everything, which counted as a 100 % drop rate + // and warned about a stream that did not exist (`PwAudioCapturer::active`). + if ud.tx.try_send(samples).is_err() && ud.active.load(Ordering::Relaxed) { ud.stats.dropped_chunks += 1; } if ud.last_stats.elapsed() >= crate::audio::capture_policy::STATS_EVERY { diff --git a/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs b/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs index 58b42db9..0a1d72dc 100644 --- a/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs +++ b/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs @@ -43,6 +43,15 @@ pub struct WasapiLoopbackCapturer { channels: u32, stop: Arc, join: Option>, + /// Whether a session is currently CONSUMING this capturer, shared with the capture thread + /// so the drop counter can tell "the encode thread fell behind" from "nobody is reading". + /// The native/gamestream planes park a capturer between sessions + /// ([`idle`](AudioCapturer::idle)) instead of dropping it, and the hand-off channel is + /// bounded — so without this the thread fills it once, then counts every subsequent chunk + /// as a drop and warns that "the stream will click" with no stream to click. Proven on the + /// Linux twin by a 2026-08-13 field log (100 % drop rate across session gaps); the parking + /// call sites are platform-independent, so this half had the same defect. + active: Arc, } impl WasapiLoopbackCapturer { @@ -58,10 +67,13 @@ impl WasapiLoopbackCapturer { // rather than a silent dead thread. let (ready_tx, ready_rx) = sync_channel::>(1); let stop_t = stop.clone(); + // Opens at session start, so the consumer is live from the first chunk. + let active = Arc::new(AtomicBool::new(true)); + let active_t = active.clone(); let join = thread::Builder::new() .name("punktfunk-wasapi-audio".into()) .spawn(move || { - if let Err(e) = capture_thread(tx, stop_t, ready_tx, channels) { + if let Err(e) = capture_thread(tx, stop_t, ready_tx, channels, active_t) { tracing::error!(error = %format!("{e:#}"), "wasapi loopback thread failed"); } }) @@ -76,6 +88,7 @@ impl WasapiLoopbackCapturer { channels, stop, join: Some(join), + active, }) } Ok(Err(e)) => Err(e), @@ -92,6 +105,9 @@ impl WasapiLoopbackCapturer { impl Drop for WasapiLoopbackCapturer { fn drop(&mut self) { + // The receiver dies with us; anything the thread still pushes is unwanted by + // definition, and must not be reported as the encode thread falling behind. + self.active.store(false, Ordering::Relaxed); self.stop.store(true, Ordering::SeqCst); if let Some(j) = self.join.take() { let _ = j.join(); @@ -114,6 +130,14 @@ impl AudioCapturer for WasapiLoopbackCapturer { } fn drain(&mut self) { while self.chunks.try_recv().is_ok() {} + // Ordered AFTER the backlog drain, so the capture thread never counts a drop against a + // channel this call is still emptying. + self.active.store(true, Ordering::Relaxed); + } + fn idle(&mut self) { + // Parked: from here the channel fills and stays full, and those drops are nobody's + // fault. See [`WasapiLoopbackCapturer::active`]. + self.active.store(false, Ordering::Relaxed); } } @@ -167,6 +191,7 @@ fn capture_thread( stop: Arc, ready: SyncSender>, channels: u32, + active: Arc, ) -> Result<()> { // COM must be initialized on THIS thread (MTA), before any device call. if let Err(e) = wasapi::initialize_mta() @@ -192,7 +217,7 @@ fn capture_thread( // is said once per topology — the field log drowned in 256+ copies of the same line. let mut unsat_logged: Option = None; while !stop.load(Ordering::Relaxed) { - match capture_once(&tx, &stop, &mut ready, channels, mode) { + match capture_once(&tx, &stop, &mut ready, channels, mode, &active) { Ok(Next::Stopped) => break, Ok(Next::Reopen(m)) => { mode = m; @@ -357,6 +382,7 @@ fn capture_once( ready: &mut Option>>, channels: u32, mode: TargetMode, + active: &AtomicBool, ) -> Result { // Interleaved f32: channels * 4 bytes per frame. let block_align = channels as usize * 4; @@ -611,10 +637,14 @@ fn capture_once( samples.push(f32::from_le_bytes([c[0], c[1], c[2], c[3]])); } 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() { + // Non-blocking, lossy — same discipline as PipeWire. COUNTED, and only while a + // session is actually reading: a full channel under a LIVE consumer means the encode + // thread is not keeping up, and every dropped chunk is a click plus a permanent + // shift of everything after it. A full channel under a PARKED capturer means nothing + // — the planes park capturers between sessions rather than dropping them, so the + // channel fills once and then refuses everything + // ([`WasapiLoopbackCapturer::active`]). + if tx.try_send(samples).is_err() && active.load(Ordering::Relaxed) { stats.dropped_chunks += 1; } } -- 2.54.0 From 8216f1d92d75a6bb1e5dc0cfb126a2b51d8676b6 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 13 Aug 2026 21:00:48 +0200 Subject: [PATCH 3/3] fix(host): a 2 s keyframe cadence is the client's flush cooldown, not display churn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The host's recovery-cadence detector warns that "client keyframe recoveries are METRONOMIC — a periodic host/display disturbance (display-topology churn, display-poller software, virtual-display timing) is the likely cause, not random network loss". In a 2026-08-13 field log it fired at period_s=2.0 and sent the investigation at three innocent host subsystems. 2.0 s is `punktfunk_core::client::FLUSH_COOLDOWN`. The client's receive-backlog guard sheds a standing queue with a flush plus a keyframe request and is rate-limited to one per cooldown, so a client that cannot sustain the stream asks for a keyframe at EXACTLY that spacing for as long as it stays behind — the constant's own doc says it "degrades into a periodic skip + a logged warning", which is the behaviour the detector then read as physical. Perfect periodicity argues FOR a fixed software cooldown, not against it. In the field case the host was blameless and the chain ran the other way: the client refused the negotiated codec on its Vulkan rung, demoted to a slower decode path, could not hold 4K120 there, and built the standing queue. Three layers between the symptom the host reported and the cause. So the detector now routes: a period on the client's cooldown names the client and says where to look in ITS log (`receive backlog stopped draining`, and a demoted decode rung); anything else keeps the display-disturbance wording it had. The comparison reads FLUSH_COOLDOWN itself — now `pub` for exactly this, documented as such — rather than a copy of the number, so the two cannot drift. ±10 % absorbs scheduling jitter and the request's trip without being wide enough to swallow the disturbance cadences the other branch exists to report. Verified: 18/18 native::stream::tests on linux/amd64 (container), including the new case, which derives its inputs from FLUSH_COOLDOWN so it survives a retune; clippy --all-targets -D warnings clean; cargo check clean on the Windows CI runner. --- .../src/client/frame_channel.rs | 10 ++- crates/punktfunk-core/src/client/mod.rs | 1 + crates/punktfunk-host/src/native/stream.rs | 74 +++++++++++++++++-- 3 files changed, 76 insertions(+), 9 deletions(-) diff --git a/crates/punktfunk-core/src/client/frame_channel.rs b/crates/punktfunk-core/src/client/frame_channel.rs index f8c20b6e..47d7db24 100644 --- a/crates/punktfunk-core/src/client/frame_channel.rs +++ b/crates/punktfunk-core/src/client/frame_channel.rs @@ -47,7 +47,15 @@ pub(crate) const FLUSH_AFTER: Duration = Duration::from_millis(250); /// Minimum spacing between jump-to-live events, so a bottleneck that instantly rebuilds the queue (a /// link/consumer that can't sustain the bitrate at all) degrades into a periodic skip + a logged /// warning instead of a continuous flush/keyframe storm. -pub(crate) const FLUSH_COOLDOWN: Duration = Duration::from_secs(2); +/// +/// **Public because the HOST needs it to read its own logs.** Each jump-to-live sends a keyframe +/// request, so a client that cannot sustain the rate asks for one at exactly this spacing, +/// forever — and the host's recovery-cadence detector saw that perfect periodicity and blamed a +/// periodic *display* disturbance (2026-08-13 field log: `period_s=2.0`, three subsystems named, +/// none of them the cause). Perfect periodicity is the signature of a fixed software cooldown, +/// not of a physical disturbance. The host compares against this constant rather than a copy of +/// the number, so the two can never drift apart. +pub const FLUSH_COOLDOWN: Duration = Duration::from_secs(2); /// A clock-triggered jump-to-live that discarded fewer datagrams than this (and no queued AUs) /// found NO local backlog: the frames read as late, but nothing here was actually behind. Two diff --git a/crates/punktfunk-core/src/client/mod.rs b/crates/punktfunk-core/src/client/mod.rs index c7bc8ddc..bf4202d8 100644 --- a/crates/punktfunk-core/src/client/mod.rs +++ b/crates/punktfunk-core/src/client/mod.rs @@ -42,6 +42,7 @@ mod recovery; mod rumble; mod worker; +pub use self::frame_channel::FLUSH_COOLDOWN; pub use self::planes::AudioPacket; pub use self::probe::ProbeOutcome; pub use self::rumble::{ActuatorQuirks, RumbleCommand}; diff --git a/crates/punktfunk-host/src/native/stream.rs b/crates/punktfunk-host/src/native/stream.rs index 149bc923..0ff3df79 100644 --- a/crates/punktfunk-host/src/native/stream.rs +++ b/crates/punktfunk-host/src/native/stream.rs @@ -2722,14 +2722,35 @@ pub(super) fn virtual_stream(ctx: SessionContext, prepared: Option bool { + let flush = punktfunk_core::client::FLUSH_COOLDOWN; + period.abs_diff(flush) < flush / 10 +} + /// One mode's capture/encode pipeline: (capturer, encoder, first frame, frame interval). /// Dropping the capturer tears down the PipeWire stream and the virtual output with it. type Pipeline = ( @@ -4597,6 +4635,26 @@ fn build_pipeline( mod tests { use super::*; + /// The 2026-08-13 field log's exact reading — `period_s=2.0` — must be attributed to the + /// client's backlog shedding, not to a host display disturbance. The whole point of routing + /// on the shared constant is that this stays true if the cooldown is ever retuned, so the + /// test derives its cases from `FLUSH_COOLDOWN` instead of hardcoding two seconds. + #[test] + fn a_recovery_cadence_on_the_clients_cooldown_is_not_blamed_on_the_display() { + let flush = punktfunk_core::client::FLUSH_COOLDOWN; + assert!(matches_client_flush_cadence(flush), "the field reading"); + // Scheduling jitter and the request's trip across the link stay inside the band. + assert!(matches_client_flush_cadence(flush + flush / 20)); + assert!(matches_client_flush_cadence(flush - flush / 20)); + + // Cadences that are NOT the cooldown still reach the display-disturbance branch — the + // band must not be so wide that it swallows them. + assert!(!matches_client_flush_cadence(flush / 2)); + assert!(!matches_client_flush_cadence(flush * 2)); + assert!(!matches_client_flush_cadence(flush + flush / 5)); + assert!(!matches_client_flush_cadence(std::time::Duration::ZERO)); + } + #[test] fn an_escalated_but_caught_up_encoder_stops_refusing_climbs() { const DEGRADE: u32 = 10; -- 2.54.0