diff --git a/crates/punktfunk-host/src/capture/windows/idd_push.rs b/crates/punktfunk-host/src/capture/windows/idd_push.rs index 5a915049..d3c08d01 100644 --- a/crates/punktfunk-host/src/capture/windows/idd_push.rs +++ b/crates/punktfunk-host/src/capture/windows/idd_push.rs @@ -24,7 +24,8 @@ use super::{CapturedFrame, Capturer, FramePayload, PixelFormat}; use anyhow::{bail, Context, Result}; use pf_driver_proto::{control, frame}; use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle}; -use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; +use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use windows::core::{w, Interface, PCWSTR, PWSTR}; use windows::Win32::Foundation::{ @@ -451,6 +452,119 @@ impl ChannelBroker { } /// Creates + owns the shared ring; yields the driver's frames as [`FramePayload::D3d11`]. +/// The display descriptor the capture loop follows: live HDR state + active resolution of the +/// virtual target. +#[derive(Clone, Copy, PartialEq, Eq)] +struct DisplayDescriptor { + hdr: bool, + width: u32, + height: u32, +} + +/// Off-thread poller for [`DisplayDescriptor`]. The CCD queries behind it (`QueryDisplayConfig`, +/// twice per sample) serialize on the session-global display-configuration lock, which display- +/// topology events and third-party display-poller software (the SteelSeries-GG class) can hold +/// for tens-to-hundreds of milliseconds at a time. Polled inline — the old design — that stall +/// landed ON the capture/encode thread: a periodic frame hitch on an otherwise healthy host, and +/// invisible in any log. Now a dedicated thread samples every [`Self::INTERVAL`] and publishes a +/// snapshot; the capture thread's per-frame cost is one uncontended mutex read, and a slow CCD +/// sample is *measured and logged* instead of silently stalling the stream. +/// +/// Failure policy is last-known-good, per field: a transient CCD failure — including the target +/// briefly missing from the active-path list during a topology re-probe — keeps the previous +/// value instead of reading as `hdr = false` (the old behavior, which on an HDR session turned +/// every blip into TWO ring recreates: false, then true again a poll later). `seq` bumps only +/// when at least one query succeeded, so the consumer's debounce counts real observations, never +/// failures. +struct DescriptorPoller { + /// Latest merged sample + its sequence number; the poller holds the lock only to copy it. + snap: Arc>, + stop: Arc, + thread: Option>, +} + +impl DescriptorPoller { + /// Poll cadence — the old inline throttle. With the consumer's two-strikes debounce on top, a + /// real "Use HDR" flip or mode-set is acted on within ~2 samples (≈ ½ s). + const INTERVAL: Duration = Duration::from_millis(250); + /// A sample slower than this means something is sitting on the display-config lock (topology + /// churn / display-poller software) — the disturbance class behind periodic virtual-display + /// stream hitches. Logged (rate-limited) so an affected host self-diagnoses. + const SLOW: Duration = Duration::from_millis(50); + + fn spawn(target_id: u32, initial: DisplayDescriptor) -> Self { + let snap = Arc::new(Mutex::new((initial, 0u64))); + let stop = Arc::new(AtomicBool::new(false)); + let (snap_t, stop_t) = (snap.clone(), stop.clone()); + let thread = std::thread::Builder::new() + .name("pf-idd-desc-poll".into()) + .spawn(move || { + let mut last = initial; + let mut seq = 0u64; + let mut last_slow_log: Option = None; + while !stop_t.load(Ordering::Relaxed) { + let t = Instant::now(); + // SAFETY: both are read-only CCD queries taking only a copy of the plain `u32` + // target id (see their own SAFETY docs); nothing is borrowed across the calls. + let (hdr, res) = unsafe { + ( + crate::win_display::advanced_color_enabled(target_id), + crate::win_display::active_resolution(target_id), + ) + }; + let took = t.elapsed(); + if took >= Self::SLOW + && last_slow_log.is_none_or(|t| t.elapsed() >= Duration::from_secs(10)) + { + last_slow_log = Some(Instant::now()); + tracing::warn!( + took_ms = took.as_millis() as u64, + target_id, + "slow display-descriptor poll — something is holding the Windows \ + display-config lock (topology churn / display-poller software); on \ + a host with periodic stream hitches, correlate this cadence" + ); + } + if hdr.is_some() || res.is_some() { + if let Some(hdr) = hdr { + last.hdr = hdr; + } + if let Some((width, height)) = res { + last.width = width; + last.height = height; + } + seq += 1; + *snap_t.lock().unwrap() = (last, seq); + } + // Park (not sleep) so `drop` wakes the thread immediately via `unpark`. + std::thread::park_timeout(Self::INTERVAL); + } + }) + .map_err(|e| { + // Degraded, not fatal: the session streams, it just never follows a mid-session + // HDR flip / mode-set (seq stays 0 → the consumer sees no changes). + tracing::error!(error = %e, "IDD push: descriptor-poller thread failed to spawn"); + }) + .ok(); + Self { snap, stop, thread } + } + + /// The latest sample (lock held only for the copy — the poller writes at 4 Hz). + fn snapshot(&self) -> (DisplayDescriptor, u64) { + *self.snap.lock().unwrap() + } +} + +impl Drop for DescriptorPoller { + fn drop(&mut self) { + self.stop.store(true, Ordering::Relaxed); + if let Some(t) = self.thread.take() { + t.thread().unpark(); + let _ = t.join(); + } + } +} + pub struct IddPushCapturer { device: ID3D11Device, context: ID3D11DeviceContext, @@ -480,9 +594,15 @@ pub struct IddPushCapturer { /// Windows mid-session. Drives the ring format (HDR → FP16 surfaces, SDR → BGRA) and the conversion. /// Polled in the capture loop; a change recreates the ring (see [`Self::recreate_ring`]). display_hdr: bool, - /// Throttle for the `advanced_color_enabled` poll (a CCD `QueryDisplayConfig`, ~ms — too costly per - /// frame at 240 Hz). - last_acm_poll: Instant, + /// Off-thread display-descriptor sampler (see [`DescriptorPoller`]) — the capture loop reads + /// its snapshot instead of running CCD queries inline on the frame path. + desc_poller: DescriptorPoller, + /// Sequence of the last poller sample the capture loop consumed (0 = none yet). + desc_seq: u64, + /// Two-strikes debounce for descriptor changes: the first differing sample arms this; only a + /// SECOND consecutive sample with the same new descriptor triggers the recreate, so a + /// single-sample transient (a topology re-probe blip) never tears the ring down. + pending_desc: Option, /// Set when a display-descriptor change triggered a ring recreate (recovery, game-capture bug GB1); /// cleared when a fresh frame resumes. If it stays set past the recovery window, `try_consume` drops /// the session (recover-or-drop, no DDA). @@ -700,8 +820,10 @@ impl IddPushCapturer { // Let the colorspace change settle before the driver composes + we size the ring. std::thread::sleep(Duration::from_millis(250)); } - let display_hdr = - enabled_hdr || crate::win_display::advanced_color_enabled(target.target_id); + // A failed open-time read defaults to SDR (unless the 10-bit path enabled HDR above) — + // there is no "last known" yet; the descriptor poller corrects a wrong guess mid-session. + let display_hdr = enabled_hdr + || crate::win_display::advanced_color_enabled(target.target_id).unwrap_or(false); let ring_fmt = if display_hdr { DXGI_FORMAT_R16G16B16A16_FLOAT } else { @@ -809,7 +931,16 @@ impl IddPushCapturer { generation, client_10bit, display_hdr, - last_acm_poll: Instant::now(), + desc_poller: DescriptorPoller::spawn( + target.target_id, + DisplayDescriptor { + hdr: display_hdr, + width: w, + height: h, + }, + ), + desc_seq: 0, + pending_desc: None, recovering_since: None, last_fresh: Instant::now(), last_liveness: Instant::now(), @@ -1034,36 +1165,43 @@ impl IddPushCapturer { Ok(()) } - /// Throttled poll of the display's live HDR state; recreate the ring if the user flipped "Use HDR". - /// Called from the capture loop (incl. while frozen on a format mismatch) so a toggle recovers within - /// a poll interval. + /// Follow the [`DescriptorPoller`]'s snapshot of the display's live HDR state + resolution; + /// recreate the ring when the display REALLY changed (a "Use HDR" flip, or a fullscreen game + /// mode-setting the virtual display out from under the negotiated size — game-capture bug + /// GB1). Called from the capture loop (incl. while frozen on a format mismatch); cheap — one + /// mutex read, the CCD queries run off-thread. Two-strikes debounce: a change is acted on + /// only when TWO consecutive samples agree on the same new descriptor (~½ s), so a + /// single-sample transient during a topology re-probe never costs a ring recreate. fn poll_display_hdr(&mut self) { - if self.last_acm_poll.elapsed() < Duration::from_millis(250) { + let (now, seq) = self.desc_poller.snapshot(); + if seq == self.desc_seq { + return; // no new sample since last consume + } + self.desc_seq = seq; + let current = DisplayDescriptor { + hdr: self.display_hdr, + width: self.width, + height: self.height, + }; + if now == current { + self.pending_desc = None; // steady (or a blip reverted before its second strike) return; } - self.last_acm_poll = Instant::now(); - // SAFETY: `advanced_color_enabled` is an `unsafe fn` taking only a copy of the plain `u32` target - // id; it performs a read-only CCD query and returns an owned `bool`, borrowing nothing from us. - let now_hdr = unsafe { crate::win_display::advanced_color_enabled(self.target_id) }; - // Follow the display's ACTUAL resolution too — a fullscreen game can mode-set the virtual display - // out from under the negotiated size (game-capture bug GB1). Unknown read → keep our current size. - // SAFETY: `active_resolution` is an `unsafe fn` taking only a copy of the plain `u32` target id; it - // performs a read-only CCD query and returns owned `(w, h)` values, borrowing nothing from us. - let (now_w, now_h) = unsafe { crate::win_display::active_resolution(self.target_id) } - .unwrap_or((self.width, self.height)); - if now_hdr == self.display_hdr && now_w == self.width && now_h == self.height { + if self.pending_desc != Some(now) { + self.pending_desc = Some(now); // first strike — arm, act on confirmation return; } + self.pending_desc = None; tracing::info!( target_id = self.target_id, from = format!("{}x{} hdr={}", self.width, self.height, self.display_hdr), - to = format!("{now_w}x{now_h} hdr={now_hdr}"), + to = format!("{}x{} hdr={}", now.width, now.height, now.hdr), "IDD push: display descriptor changed — recreating the ring at the new mode" ); // Start the recovery clock (if not already running): if a fresh frame doesn't resume within the // window, try_consume drops the session rather than freeze. self.recovering_since.get_or_insert_with(Instant::now); - if let Err(e) = self.recreate_ring(now_hdr, now_w, now_h) { + if let Err(e) = self.recreate_ring(now.hdr, now.width, now.height) { tracing::warn!(error = %format!("{e:#}"), "IDD push: ring recreate failed"); } } diff --git a/crates/punktfunk-host/src/punktfunk1.rs b/crates/punktfunk-host/src/punktfunk1.rs index 1327cb70..2f62bc91 100644 --- a/crates/punktfunk-host/src/punktfunk1.rs +++ b/crates/punktfunk-host/src/punktfunk1.rs @@ -3163,6 +3163,82 @@ struct SessionContext { launch: Option, } +/// Detector for METRONOMIC client keyframe-recovery cycles — the "periodic double-jolt" symptom +/// class field reports keep describing: a host/display-side disturbance repeating every few +/// seconds (display-topology churn, display-poller software, virtual-display timing), where each +/// cycle ends in a client keyframe request the host serves. Random network loss is bursty and +/// irregular; a stable period is a machine, and saying so in the host log turns a "nothing in the +/// logs :/" report into a self-diagnosis. +/// +/// Served forced IDRs within [`Self::COALESCE`] count as ONE event (a double-jolt's paired IDRs — +/// the cooldown re-issue of a lost keyframe — are one user-visible disturbance). When the gaps +/// between the last [`Self::STREAK`] events are all within ±[`Self::TOLERANCE`] of their mean, +/// [`Self::note`] returns the mean period for the caller to warn with, then stays quiet for +/// [`Self::REWARN`] while the cycle persists. Pure logic — unit-tested below. +struct RecoveryCadence { + events: std::collections::VecDeque, + last_warn: Option, +} + +impl RecoveryCadence { + /// Serves closer together than this are the same user-visible disturbance. + const COALESCE: std::time::Duration = std::time::Duration::from_millis(1500); + /// Consecutive evenly-spaced events before the cycle counts as metronomic. + const STREAK: usize = 4; + /// "Evenly spaced" = every gap within this fraction of the mean gap. + const TOLERANCE: f64 = 0.2; + /// Once warned, re-warn at most this often while the cycle persists. + const REWARN: std::time::Duration = std::time::Duration::from_secs(30); + + fn new() -> Self { + Self { + events: std::collections::VecDeque::new(), + last_warn: None, + } + } + + /// Record a served client-recovery IDR at `now`; `Some(mean period)` exactly when the + /// metronomic-cycle warning should fire. + fn note(&mut self, now: std::time::Instant) -> Option { + if self + .events + .back() + .is_some_and(|last| now.duration_since(*last) < Self::COALESCE) + { + return None; + } + self.events.push_back(now); + if self.events.len() > Self::STREAK { + self.events.pop_front(); + } + if self.events.len() < Self::STREAK { + return None; + } + let gaps: Vec = self + .events + .iter() + .zip(self.events.iter().skip(1)) + .map(|(a, b)| b.duration_since(*a).as_secs_f64()) + .collect(); + let mean = gaps.iter().sum::() / gaps.len() as f64; + if mean <= 0.0 + || gaps + .iter() + .any(|g| (g - mean).abs() > mean * Self::TOLERANCE) + { + return None; + } + if self + .last_warn + .is_some_and(|t| now.duration_since(t) < Self::REWARN) + { + return None; + } + self.last_warn = Some(now); + Some(std::time::Duration::from_secs_f64(mean)) + } +} + fn virtual_stream(ctx: SessionContext) -> Result<()> { // This thread runs the capture+encode loop (single-process — the only topology: Linux portal / // synthetic, Windows in-process IDD-push). Elevate it so a CPU-heavy game can't deschedule our GPU @@ -3377,6 +3453,9 @@ fn virtual_stream(ctx: SessionContext) -> Result<()> { // clock now — that coalesces the keyframe storm a client fires while its decoder wedges on the cold // opening GOP, instead of answering it with a redundant second IDR. let mut last_forced_idr: Option = Some(std::time::Instant::now()); + // Self-diagnosis for the periodic-stutter class: warns when the served recovery IDRs settle + // into a stable multi-second rhythm (see [`RecoveryCadence`]). + let mut recovery_cadence = RecoveryCadence::new(); // Per-stage latency breakdown (PUNKTFUNK_PERF): per-call µs for the GPU-bound stages so we see // exactly where the capture→encoded latency goes — cap=try_latest (ring read + colour convert), // submit=encode_picture launch, wait=lock_bitstream (the scheduling wait + ASIC encode, the one @@ -3578,7 +3657,18 @@ fn virtual_stream(ctx: SessionContext) -> Result<()> { } else { tracing::debug!("forcing keyframe (client decode recovery)"); enc.request_keyframe(); - last_forced_idr = Some(std::time::Instant::now()); + let now = std::time::Instant::now(); + last_forced_idr = Some(now); + if let Some(period) = recovery_cadence.note(now) { + tracing::warn!( + period_s = format!("{:.1}", period.as_secs_f64()), + "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; \ + correlate with 'slow display-descriptor poll' / 'display descriptor \ + changed' lines" + ); + } } } // Measure the per-stage split when `PUNKTFUNK_PERF` is set OR a web-console stats capture is @@ -4209,6 +4299,69 @@ fn build_pipeline( mod tests { use super::*; + /// Feed [`RecoveryCadence`] a schedule of event offsets (ms from a common origin) and return + /// what each `note` produced. + fn cadence_run(offsets_ms: &[u64]) -> Vec> { + let base = std::time::Instant::now(); + let mut c = RecoveryCadence::new(); + offsets_ms + .iter() + .map(|ms| c.note(base + std::time::Duration::from_millis(*ms))) + .collect() + } + + #[test] + fn cadence_detects_metronomic_recoveries() { + // Four IDR serves ~4 s apart (±5%) → the fourth trips the detector at ~4 s. + let out = cadence_run(&[0, 4_000, 8_100, 11_950]); + assert_eq!(out[..3], [None, None, None]); + let period = out[3].expect("metronomic series must be detected"); + assert!( + (period.as_secs_f64() - 3.98).abs() < 0.2, + "period={period:?}" + ); + } + + #[test] + fn cadence_coalesces_double_jolt_pairs() { + // The field signature: a jolt pair (second IDR ~0.7 s after the first, the cooldown + // re-issue) every ~4 s. Each pair is ONE event; detection still lands on the ~4 s cycle. + let out = cadence_run(&[ + 0, 700, // pair 1 + 4_000, 4_700, // pair 2 + 8_000, 8_650, // pair 3 + 12_000, // pair 4 (first serve trips it) + ]); + assert!(out[..6].iter().all(Option::is_none)); + let period = out[6].expect("coalesced pairs must still read as a 4 s cycle"); + assert!( + (period.as_secs_f64() - 4.0).abs() < 0.2, + "period={period:?}" + ); + } + + #[test] + fn cadence_ignores_irregular_bursts() { + // Genuine Wi-Fi-style loss: irregular gaps → never flagged. + assert!(cadence_run(&[0, 2_000, 9_000, 12_500, 21_000]) + .iter() + .all(Option::is_none)); + } + + #[test] + fn cadence_rewarns_at_most_every_30s() { + // A persisting 4 s cycle: warn on the 4th event (t=12 s), then stay quiet until ≥30 s + // past the warn — the t=44 s event (index 11) is the first at or beyond t=42 s. + let offsets: Vec = (0..12).map(|i| i * 4_000).collect(); + let out = cadence_run(&offsets); + let warned: Vec = out + .iter() + .enumerate() + .filter_map(|(i, o)| o.map(|_| i)) + .collect(); + assert_eq!(warned, vec![3, 11], "warn indices"); + } + #[test] fn adapt_fec_maps_loss_to_recovery_band() { // A perfectly clean window (0 loss) lands on the floor. diff --git a/crates/punktfunk-host/src/windows/win_display.rs b/crates/punktfunk-host/src/windows/win_display.rs index 731d96ef..c748e82c 100644 --- a/crates/punktfunk-host/src/windows/win_display.rs +++ b/crates/punktfunk-host/src/windows/win_display.rs @@ -169,12 +169,15 @@ pub(crate) unsafe fn set_advanced_color(target_id: u32, enable: bool) -> bool { /// actually ON for the virtual display right now (e.g. because the user toggled it in Windows display /// settings). The capture/encode pipeline follows the monitor's real colorspace (WGC → FP16 → NVENC /// Main10 BT.2020 PQ), so this is the authoritative "is this an HDR session" signal — NOT the -/// handshake-negotiated bit depth. Returns false if the target isn't found / the query fails. -pub(crate) unsafe fn advanced_color_enabled(target_id: u32) -> bool { +/// handshake-negotiated bit depth. `None` when the query fails or the target isn't in the active-path +/// list (both happen transiently during a display-topology re-probe): the caller decides the fallback — +/// the capture loop's poller keeps the last known value, since reading a blip as "HDR off" used to cost +/// an HDR session TWO spurious ring recreates (false, then true again a poll later). +pub(crate) unsafe fn advanced_color_enabled(target_id: u32) -> Option { let mut np = 0u32; let mut nm = 0u32; if GetDisplayConfigBufferSizes(QDC_ONLY_ACTIVE_PATHS, &mut np, &mut nm).is_err() { - return false; + return None; } let mut paths = vec![DISPLAYCONFIG_PATH_INFO::default(); np as usize]; let mut modes = vec![DISPLAYCONFIG_MODE_INFO::default(); nm as usize]; @@ -188,7 +191,7 @@ pub(crate) unsafe fn advanced_color_enabled(target_id: u32) -> bool { ) .is_err() { - return false; + return None; } for p in paths.iter().take(np as usize) { if p.targetInfo.id == target_id { @@ -199,12 +202,12 @@ pub(crate) unsafe fn advanced_color_enabled(target_id: u32) -> bool { info.header.id = p.targetInfo.id; if DisplayConfigGetDeviceInfo(&mut info.header) == 0 { // value bit 1 = advancedColorEnabled (bit 0 = advancedColorSupported). - return (info.Anonymous.value & 0x2) != 0; + return Some((info.Anonymous.value & 0x2) != 0); } - return false; + return None; } } - false + None } /// Force the freshly-added SudoVDA monitor to the client's exact `WxH@Hz`. The ADD IOCTL only