From 704defe0a795e8468c24d995a3237f51edeede22 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 15 Aug 2026 13:24:35 +0200 Subject: [PATCH 01/10] fix(capture): the phase lock re-engaged on one good report, so a marginal host flapped 41 times in 24 minutes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP-A1 of design/host-source-stutter-fixes.md. The 2026-08-15 Skynet log is a 24-minute session with the bitrate pinned, loss_ppm=0 throughout and zero client recoveries — a fully exonerated transport — and 41 phase-lock engage/disengage cycles, each disengage logging an arrival phase 0.11-8.22 ms off (up to a full 120 Hz period). The controller was manufacturing a good part of that itself. Engagement needed ONE coherent report, and the incoherent disengage asked for NO re-engage backoff (the travel-budget path asks for 10 ticks), so a host whose coherence oscillates around COHERENCE_FLOOR_MILLI re-engaged within a second of every disengage. Each cycle is a timing step in both directions: engaging starts holding submits on a grid by up to a period, disengaging drops the offset to zero and the next frames leave that much earlier. Since every client presents on arrival, all 82 of those steps reached the glass. So engagement now needs five consecutive coherent reports, each incoherent cycle waits longer than the last (10 ticks doubling to 320), and a host that has torn down an engaged grid eight times parks the lock for the session — permanently disengaged is today's default and strictly better than another cycle of steps. A lock that holds for a minute forgives the escalation, and only a disengage that tore down an ENGAGED grid counts toward the fuse: otherwise a launch-time shader storm, which is minutes of genuinely incoherent arrival before the controller ever locks, would fuse a host that then locks perfectly for hours. The disengage line gains coherence_milli, which is the number that says whether a host is marginal or hopeless and was the one thing the log could not show. --- crates/punktfunk-host/src/native/stream.rs | 256 ++++++++++++++++++++- 1 file changed, 250 insertions(+), 6 deletions(-) diff --git a/crates/punktfunk-host/src/native/stream.rs b/crates/punktfunk-host/src/native/stream.rs index aa95b462..3da1e911 100644 --- a/crates/punktfunk-host/src/native/stream.rs +++ b/crates/punktfunk-host/src/native/stream.rs @@ -271,10 +271,21 @@ impl PhaseCtl { /// budget catches any residual chase the statistics miss. New in v3: ANTIPODE DAMPING — an /// error within 1 ms of ±period/2 flips sign on sampling noise (measured as 0↔2↔4 ms offset /// chatter), so near-antipode steps are halved until the error commits to a side. +/// +/// New in v4 (design/host-source-stutter-fixes.md WP-A1, from the 2026-08-15 Skynet log): a host +/// whose coherence oscillates around [`COHERENCE_FLOOR_MILLI`](Self::COHERENCE_FLOOR_MILLI) used +/// to flap forever, because engagement needed ONE coherent report and the incoherent disengage +/// asked for NO backoff — 41 engage/disengage cycles in 24 minutes on a KWin host. Each cycle is +/// a self-inflicted timing step in both directions (engage starts holding submits by up to a +/// period; disengage drops the offset to 0 and the next frames leave that much earlier), so the +/// controller was manufacturing the very arrival jitter it exists to remove. Hence: engagement +/// needs SUSTAINED coherence, each incoherent cycle backs off longer than the last, and a host +/// that cannot hold coherence at all parks for the session. struct PhaseController { /// Grid offset, ns ∈ [0, period). Meaningful only while engaged. offset_ns: i64, - /// The grid's epoch; `None` = disengaged (no submit-grid sleeps, zero cost). + /// The grid's epoch; `None` = disengaged (no submit-grid sleeps, zero cost). Doubles as the + /// lock's age — it is stamped at engage and cleared at disengage, nowhere else. epoch: Option, /// Last adjust instant (~1 Hz cadence). last_adjust: std::time::Instant, @@ -282,6 +293,13 @@ struct PhaseController { cum_travel_ns: i64, /// Consecutive incoherent reports; 3 disengage. incoherent_streak: u32, + /// Consecutive COHERENT reports — the v4 engage gate. + coherent_streak: u32, + /// Incoherent disengages this session: escalates the backoff, and blows the fuse. Forgiven + /// by a lock that holds for [`LOCK_STABLE`](Self::LOCK_STABLE). + incoherent_cycles: u32, + /// Fuse blown — parked for the session, no further engagement and no further log lines. + fused: bool, /// Adjust ticks to sit out after a disengage before re-engaging. reengage_backoff: u32, } @@ -304,6 +322,19 @@ impl PhaseController { const ANTIPODE_GUARD_NS: i64 = 1_000_000; /// Adjust ticks sat out after a disengage (travel exhaustion) before trying again. const REENGAGE_BACKOFF: u32 = 10; + /// Consecutive coherent reports the grid must see before it engages (v4). One was enough in + /// v3, which is why a host hovering at the coherence floor re-engaged within a second of + /// every disengage; at the ~1 Hz report cadence this asks for ~5 s of a phase worth locking. + const ENGAGE_COHERENT_REPORTS: u32 = 5; + /// Incoherent cycles before the grid parks for the session. A host that has failed this many + /// times is telling us its arrival phase is not lockable, and permanently disengaged (today's + /// default, zero added latency) is strictly better than another cycle of steps. + const INCOHERENT_FUSE: u32 = 8; + /// Escalation ceiling — `REENGAGE_BACKOFF << 5` = 320 ticks ≈ 5 min. + const MAX_BACKOFF_SHIFT: u32 = 5; + /// A lock held this long forgives the session's escalation: a transient bad patch (a shader + /// storm, a game launch) must not fuse a host that is otherwise perfectly lockable. + const LOCK_STABLE: std::time::Duration = std::time::Duration::from_secs(60); fn new() -> PhaseController { PhaseController { @@ -312,6 +343,9 @@ impl PhaseController { last_adjust: std::time::Instant::now(), cum_travel_ns: 0, incoherent_streak: 0, + coherent_streak: 0, + incoherent_cycles: 0, + fused: false, reengage_backoff: 0, } } @@ -320,10 +354,14 @@ impl PhaseController { self.epoch.is_some() } - fn disengage(&mut self, reason: &'static str, backoff: u32) { + /// `coherence_milli` is the report that caused the disengage: without it the log showed how + /// far the offset had walked but not how far below the gate the phase actually was, which is + /// the number that says whether the host is marginal or hopeless. + fn disengage(&mut self, reason: &'static str, backoff: u32, coherence_milli: u16) { if self.engaged() { tracing::info!( offset_ms = self.offset_ns as f64 / 1e6, + coherence_milli, reason, "phase lock: disengaging the submit grid" ); @@ -331,6 +369,10 @@ impl PhaseController { self.epoch = None; self.offset_ns = 0; self.cum_travel_ns = 0; + // Both streaks restart: re-engaging costs a fresh ENGAGE_COHERENT_REPORTS of proof, and + // the next disengage costs a fresh three incoherent reports. + self.incoherent_streak = 0; + self.coherent_streak = 0; self.reengage_backoff = backoff; } @@ -338,7 +380,7 @@ impl PhaseController { /// Sign convention: a positive (shortest-way) error means frames arrive too early and wait /// at the client — submit LATER (grow the offset); negative — earlier. fn adjust(&mut self, r: &punktfunk_core::quic::PhaseReport, period_ns: i64) { - if period_ns <= 0 { + if period_ns <= 0 || self.fused { return; } self.last_adjust = std::time::Instant::now(); @@ -349,13 +391,43 @@ impl PhaseController { let coherent = r.coherence_milli == u16::MAX || r.coherence_milli >= Self::COHERENCE_FLOOR_MILLI; if !coherent { + self.coherent_streak = 0; self.incoherent_streak += 1; if self.incoherent_streak >= 3 { - self.disengage("incoherent arrival phase", 0); + // Only a disengage that tore down an ENGAGED grid cost the stream a timing step. + // Counting the others would let a launch-time shader storm — minutes of genuinely + // incoherent arrival before the controller ever locks — blow the fuse on a host + // that then locks perfectly for the next three hours. + if self.engaged() { + self.incoherent_cycles += 1; + if self.incoherent_cycles >= Self::INCOHERENT_FUSE { + self.fused = true; + tracing::info!( + cycles = self.incoherent_cycles, + coherence_milli = r.coherence_milli, + "phase lock: arrival phase incoherent on this host — parked for the \ + session" + ); + } + } + // Each cycle waits longer than the last: 10 ticks (~10 s) doubling to 320 (~5 min). + let backoff = Self::REENGAGE_BACKOFF + << self + .incoherent_cycles + .saturating_sub(1) + .min(Self::MAX_BACKOFF_SHIFT); + self.disengage("incoherent arrival phase", backoff, r.coherence_milli); } return; } self.incoherent_streak = 0; + self.coherent_streak = self.coherent_streak.saturating_add(1); + // A lock this old has proven itself — forgive the escalation so a session that hits one + // bad patch an hour is not slowly fused by it. Checked here rather than at disengage so + // the state decays while the lock is good, not only when it is lost. + if self.epoch.is_some_and(|e| e.elapsed() >= Self::LOCK_STABLE) { + self.incoherent_cycles = 0; + } let target = Self::TARGET_LEAD_FLOOR_NS.max(r.uncertainty_ns as i64 + 1_000_000); // Signed SHORTEST-WAY error around the period. let raw = (r.arrival_lead_ns as i64 - target).rem_euclid(period_ns); @@ -369,8 +441,16 @@ impl PhaseController { return; } if !self.engaged() { + // Hysteresis: an offset worth holding submits for has to be backed by a phase that + // stayed coherent, not by the one report that happened to clear the gate. + if self.coherent_streak < Self::ENGAGE_COHERENT_REPORTS { + return; + } self.epoch = Some(std::time::Instant::now()); - tracing::info!("phase lock: engaging the submit grid"); + tracing::info!( + coherence_milli = r.coherence_milli, + "phase lock: engaging the submit grid" + ); } let mut step = error.clamp(-Self::MAX_STEP_NS, Self::MAX_STEP_NS); // Antipode damping: this error sits where its sign is a coin flip — half steps until @@ -382,7 +462,7 @@ impl PhaseController { self.cum_travel_ns += step.abs(); if self.cum_travel_ns > period_ns + period_ns / 4 { tracing::info!("phase lock: travel budget exhausted without convergence — disengaging"); - self.disengage("travel budget", Self::REENGAGE_BACKOFF); + self.disengage("travel budget", Self::REENGAGE_BACKOFF, r.coherence_milli); } } @@ -5233,6 +5313,170 @@ mod tests { ); } + // ---- v4 flap hygiene (design/host-source-stutter-fixes.md WP-A1) ---- + // + // These exercise the controller's reaction to the COHERENCE STATISTIC rather than the + // statistic itself (`report_from_lead` above covers that), because the shape being replayed + // is "coherence oscillating around the floor" — far easier to state directly than to conjure + // out of a sample spread, and the number under test is the gate, not the estimator. + + /// Just above / just below the gate — the neighbourhood the field host lived in. + const COHERENT: u16 = PhaseController::COHERENCE_FLOOR_MILLI + 40; + const INCOHERENT: u16 = PhaseController::COHERENCE_FLOOR_MILLI - 40; + /// A lead far enough from the target to be worth acting on (error ≫ deadband). + const ACTIONABLE_LEAD: i64 = 7_500_000; + + fn report_at(coherence_milli: u16, lead_ns: i64) -> punktfunk_core::quic::PhaseReport { + punktfunk_core::quic::PhaseReport { + next_latch_host_ns: 0, + latch_period_ns: SIM_P as u32, + uncertainty_ns: 1_000_000, + arrival_lead_ns: lead_ns.rem_euclid(SIM_P) as u32, + coherence_milli, + } + } + + /// One full cycle on a host that keeps losing its phase: engage on a sustained good phase, + /// then lose it. Returns the backoff the disengage asked for, and leaves the controller with + /// that backoff spent so the caller can run another cycle. + fn one_incoherent_cycle(c: &mut PhaseController) -> u32 { + for _ in 0..PhaseController::ENGAGE_COHERENT_REPORTS { + c.adjust(&report_at(COHERENT, ACTIONABLE_LEAD), SIM_P); + } + assert!(c.engaged(), "the cycle must engage before it tears down"); + for _ in 0..3 { + c.adjust(&report_at(INCOHERENT, ACTIONABLE_LEAD), SIM_P); + } + let asked = c.reengage_backoff; + while !c.fused && c.reengage_backoff > 0 { + c.adjust(&report_at(INCOHERENT, ACTIONABLE_LEAD), SIM_P); + } + asked + } + + #[test] + fn engage_requires_sustained_coherence() { + let mut c = PhaseController::new(); + for i in 1..PhaseController::ENGAGE_COHERENT_REPORTS { + c.adjust(&report_at(COHERENT, ACTIONABLE_LEAD), SIM_P); + assert!(!c.engaged(), "engaged on only {i} coherent report(s)"); + } + c.adjust(&report_at(COHERENT, ACTIONABLE_LEAD), SIM_P); + assert!( + c.engaged(), + "sustained coherence must still engage the grid" + ); + + // …and the proof is CONSECUTIVE: one bad report puts it back to zero, which is the whole + // difference from v3 on a host that clears the gate every other second. + let mut c = PhaseController::new(); + for _ in 0..PhaseController::ENGAGE_COHERENT_REPORTS - 1 { + c.adjust(&report_at(COHERENT, ACTIONABLE_LEAD), SIM_P); + } + c.adjust(&report_at(INCOHERENT, ACTIONABLE_LEAD), SIM_P); + for _ in 0..PhaseController::ENGAGE_COHERENT_REPORTS - 1 { + c.adjust(&report_at(COHERENT, ACTIONABLE_LEAD), SIM_P); + } + assert!( + !c.engaged(), + "a broken streak must not count toward engaging" + ); + } + + #[test] + fn incoherent_disengage_backoff_escalates() { + let mut c = PhaseController::new(); + let asked: Vec = (0..4).map(|_| one_incoherent_cycle(&mut c)).collect(); + assert_eq!( + asked, + vec![10, 20, 40, 80], + "each cycle must wait longer than the last (v3 asked for zero, every time)" + ); + } + + #[test] + fn fuse_after_repeated_cycles() { + let mut c = PhaseController::new(); + for _ in 0..PhaseController::INCOHERENT_FUSE { + one_incoherent_cycle(&mut c); + } + assert!( + c.fused, + "a host that never holds a lock must park for the session" + ); + // Parked means parked: even a v1-style bypass report (u16::MAX) cannot wake it. + for _ in 0..50 { + c.adjust(&report_at(u16::MAX, ACTIONABLE_LEAD), SIM_P); + } + assert!(!c.engaged(), "a fused controller must stay disengaged"); + } + + #[test] + fn stable_lock_resets_escalation() { + let mut c = PhaseController::new(); + one_incoherent_cycle(&mut c); + one_incoherent_cycle(&mut c); + assert_eq!(c.incoherent_cycles, 2, "two cycles should have escalated"); + + for _ in 0..PhaseController::ENGAGE_COHERENT_REPORTS { + c.adjust(&report_at(COHERENT, ACTIONABLE_LEAD), SIM_P); + } + assert!(c.engaged()); + // Buy the minute by moving the epoch back, the same way the grid test places its epoch — + // the lock's age IS the epoch's age. + c.epoch = Some( + std::time::Instant::now() + - PhaseController::LOCK_STABLE + - std::time::Duration::from_secs(1), + ); + c.adjust(&report_at(COHERENT, ACTIONABLE_LEAD), SIM_P); + assert_eq!( + c.incoherent_cycles, 0, + "a lock that held past LOCK_STABLE must forgive the escalation" + ); + } + + /// The 2026-08-15 Skynet log in one function: 24 minutes of a host whose arrival phase sits + /// ON the coherence gate, flipping sides every few reports. v3 engaged on the FIRST coherent + /// report after a zero-backoff disengage and logged **41 engage/disengage cycles** — 82 + /// self-inflicted timing steps (each engage holds submits by up to a period, each disengage + /// drops the offset to zero) on a session whose transport was provably clean. + #[test] + fn flap_replay_stops_the_engage_churn() { + let mut c = PhaseController::new(); + let mut rng = Lcg(23); + let (mut engagements, mut reports, mut coherent_side) = (0u32, 0u32, true); + while reports < 24 * 60 { + // Runs of 2-4 reports a side: "every few reports", the logged shape. + let run = 2 + rng.next_noise(3).rem_euclid(3) as u32; + for _ in 0..run { + let was = c.engaged(); + let side = if coherent_side { COHERENT } else { INCOHERENT }; + c.adjust(&report_at(side, ACTIONABLE_LEAD), SIM_P); + engagements += u32::from(!was && c.engaged()); + reports += 1; + } + coherent_side = !coherent_side; + } + assert!( + engagements <= 2, + "24 min of gate-hovering must not churn the grid: {engagements} engagements" + ); + // …and the cure must not be "never engage again": a host that settles still locks. This + // is R1 in the plan's register, asserted rather than argued. + assert!( + !c.fused, + "flapping that never engaged must not blow the fuse" + ); + for _ in 0..PhaseController::ENGAGE_COHERENT_REPORTS { + c.adjust(&report_at(COHERENT, ACTIONABLE_LEAD), SIM_P); + } + assert!( + c.engaged(), + "a phase that finally holds must still get the grid" + ); + } + /// The 2026-08-14 Hyprland field report, in one function: xdph advertises `Hidden|Embedded`, /// so a session that asked for cursor metadata is served **Embedded** — no `SPA_META_Cursor` /// is ever sent, whatever the pointer does. The host must then (a) stop planning a metadata -- 2.54.0 From 5e77ec42c68bcc7ac6f7eab9d527b86c5d972858 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 15 Aug 2026 13:24:56 +0200 Subject: [PATCH 02/10] fix(audio): a capture hole cost more than itself, and delivered_pct could not say how many there were MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP-A2, B1 and B2 of design/host-source-stutter-fixes.md, from the same 24-minute Skynet log. Capture delivered_pct sat at 84-97 — never 100 — through continuous loud gameplay with dropped_chunks=0: 3-16 % of wall-clock where our sink node's process callback simply never ran. Three things were wrong with that, and they compound. The measurement could not distinguish one 2 s hole from three hundred 8 ms ones, which are a device fault and a scheduling fault and want different answers. So CaptureStats gains gaps/max_gap_ms/missed_dequeues: the Linux callback scores its own arrival cadence against the NEGOTIATED quantum (a graph clamped to 1024 frames is slow, not gapping) and drops its stamp across a state transition, so a deliberate Paused span is not one enormous hole. The four silent early-returns in that callback — no buffer, no datas, no mapped memory — are counted instead of vanishing. Windows reaches the same numbers by a different road: it is a polling loop over a tap that stops delivering entirely while the endpoint idles, so a cadence metric would score every quiet moment; it reads WASAPI's own DATA_DISCONTINUITY flag and sizes the hole from the device position, ignoring the flag on the first packet after a packet-less second (that is the tap waking up, not a hole). A hole then cost far more than the audio it swallowed, because audio_thread blocked in next_chunk for its whole duration and NOTHING left the host: the client's de-jitter ring drained, underran, de-primed and had to re-prime, so a 30 ms hole became a much longer artifact. The loop is now deadline-driven — the capturers gained a bounded next_chunk_within — and covers a hole with silence frames on the existing pacer schedule, continuous in seq and pts, for up to 500 ms. Past that the host is not glitching, it is quiet, and the wire stops exactly as it did before. A partial frame straddling a hole is padded out and sent first rather than completed by post-gap samples, which would splice audio from both sides into one frame; the post-gap re-anchor is clamped so wire pts can never step backwards over the frames infill already sent; and a hole the wire could NOT cover clears the redundancy predecessor, because nothing before it may be spliced onto what follows. The capture counters deliberately measure upstream of all this, so infill can never make that line look healthy. Some of those holes we were inflicting on ourselves: the log shows three "audio format negotiated" lines in minute 1, each wrapped in a Paused-Streaming flap. Wine churns its audio device at launch, the sink briefly goes unused, WirePlumber suspends it on its idle timeout and the next app resumes it. The minted sink now sets session.suspend-timeout-seconds=0 — deliberately not node.always-process, which would keep the node scheduled with nothing connected and run this callback 200 times a second on a host sitting between sessions — and a renegotiation resolving to the format we already had says so at DEBUG instead of reading like a third format change. --- crates/punktfunk-host/src/audio.rs | 14 + .../src/audio/capture_policy.rs | 260 ++++++++++++++++++ crates/punktfunk-host/src/audio/linux/mod.rs | 90 +++++- .../src/audio/windows/wasapi_cap.rs | 57 +++- crates/punktfunk-host/src/native/audio.rs | 119 ++++++-- 5 files changed, 507 insertions(+), 33 deletions(-) diff --git a/crates/punktfunk-host/src/audio.rs b/crates/punktfunk-host/src/audio.rs index 2de21c9a..b1fe93bf 100644 --- a/crates/punktfunk-host/src/audio.rs +++ b/crates/punktfunk-host/src/audio.rs @@ -72,6 +72,20 @@ pub trait AudioCapturer: Send { /// reopen. fn next_chunk(&mut self) -> Result>; + /// [`next_chunk`](Self::next_chunk) with a caller-chosen upper bound on the wait. + /// + /// The encode loop owes the wire a frame every 5 ms whether or not capture has anything to + /// say, and blocking here for as long as the capturer feels like is what made a capture hole + /// cost far more than the audio it swallowed: nothing left the host for the hole's whole + /// duration, so the client's de-jitter ring drained, underran and de-primed over a gap it + /// could otherwise have ridden through (WP-B1). Expiry returns an **empty** chunk — the same + /// "no samples right now" the idle timeout reports, and equally not an error. + /// + /// Backends with no bound of their own just delegate; they simply wake less precisely. + fn next_chunk_within(&mut self, _budget: std::time::Duration) -> Result> { + self.next_chunk() + } + /// The interleaved channel count this capturer delivers (what it was opened with). fn channels(&self) -> u32 { CHANNELS as u32 diff --git a/crates/punktfunk-host/src/audio/capture_policy.rs b/crates/punktfunk-host/src/audio/capture_policy.rs index 872f6f83..c233a6d0 100644 --- a/crates/punktfunk-host/src/audio/capture_policy.rs +++ b/crates/punktfunk-host/src/audio/capture_policy.rs @@ -97,6 +97,12 @@ impl FightDamper { /// How often the capture loop reports its vitals (WP0.2). pub(crate) const STATS_EVERY: Duration = Duration::from_secs(30); +/// Shortest callback-to-callback delta that can be called a gap, whatever the quantum. At the +/// 5 ms quantum we ask for, `2 × quantum` would be 10 ms anyway; this floor is what stops a +/// graph running an even smaller quantum (the 2026-08-15 field host negotiated 128 frames = +/// 2.7 ms) from scoring ordinary scheduling noise as a hole. +const GAP_FLOOR: Duration = Duration::from_millis(10); + /// 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 @@ -120,6 +126,18 @@ pub(crate) struct CaptureStats { /// 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, + /// Windows in which the callback simply did not run on time (WP-A2). `delivered_pct` proves + /// audio is missing but structurally cannot say HOW: one 2 s hole and three hundred 8 ms + /// hiccups produce the same percentage and want completely different answers (a device or + /// graph fault vs. a scheduling fault). The 2026-08-15 field log sat at 84–97 % for 24 + /// minutes of loud gameplay with `dropped_chunks=0` and no way to tell those apart. + pub(crate) gaps: u64, + /// The largest of those, µs. Reported in ms; kept in µs so a sub-ms threshold is expressible. + pub(crate) max_gap_us: u64, + /// Callbacks that ran but carried nothing — no buffer to dequeue, no `datas`, no mapped + /// memory. Every one of these used to `return` silently, so a stream that fired its callback + /// on time and handed us nothing looked identical to a stream nobody was feeding. + pub(crate) missed_dequeues: u64, } impl CaptureStats { @@ -135,6 +153,30 @@ impl CaptureStats { } } + /// Score one callback arrival against the previous one. + /// + /// `since_last` is `None` for the first callback of a stream — and, deliberately, for the + /// first after a state transition: the caller drops its stamp when the stream pauses, so a + /// legitimately Paused span is not scored as one enormous hole. (The Paused↔Streaming flaps + /// around a format renegotiation stay visible as the state DEBUG lines next to a small + /// post-resume gap, which is the honest reading of what happened.) + /// + /// `quantum` is the NEGOTIATED buffer duration, not the one we asked for: a graph handing us + /// 21.3 ms buffers is not gapping when its callbacks are 21.3 ms apart — it is doing exactly + /// what it negotiated, and the quantum warning above already said so. + pub(crate) fn observe_callback(&mut self, since_last: Option, quantum: Duration) { + let Some(delta) = since_last else { return }; + if delta > (quantum * 2).max(GAP_FLOOR) { + self.gaps += 1; + self.max_gap_us = self.max_gap_us.max(delta.as_micros() as u64); + } + } + + /// The window's worst gap in whole ms — the unit the log line and the field reports speak. + pub(crate) fn max_gap_ms(&self) -> u64 { + self.max_gap_us / 1_000 + } + /// `(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) { @@ -151,6 +193,79 @@ impl CaptureStats { } } +/// How long a capture hole may run before the wire starts covering it. Two protocol frames: long +/// enough that ordinary quantum jitter never trips it, short enough that the client's ring never +/// notices the hole. +pub(crate) const INFILL_AFTER: Duration = Duration::from_millis(2 * FRAME_MS as u64); +/// How much silence one hole may be covered with. Past this the host is not glitching, it is +/// QUIET — a desktop between games is legitimately silent for hours and paying a few kbps to keep +/// saying so is absurd — so the wire stops, which is exactly the behaviour that shipped before. +pub(crate) const INFILL_MAX: Duration = Duration::from_millis(500); + +/// One protocol audio frame — the wire's unit, and the granularity infill works in. +const FRAME_MS: u32 = punktfunk_core::audio::FRAME_MS; + +/// What the wire owes for the slot that is due now. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Infill { + /// Nothing: audio is flowing, or the hole is still too young to be worth covering. + Wait, + /// One frame of silence, on the pacer's schedule and continuous with what came before. + Silence, + /// The budget is spent. Say nothing — and the next real chunk begins a NEW continuity. + Quiet, +} + +/// Whether, and for how long, the wire covers a capture hole with silence (WP-B1). +/// +/// A hole used to cost far more than the audio it swallowed. `audio_thread` blocked in +/// `next_chunk` for its whole duration, so nothing at all left the host: the client's de-jitter +/// ring drained, underran, de-primed, and then had to re-prime — turning a 30 ms hole into a much +/// longer audible artifact, and doing it 3–16 % of the time on the 2026-08-15 field host. Silence +/// on the same 5 ms schedule, with continuous `seq` and pts, keeps that ring fed and its playout +/// anchored, so what the listener loses shrinks to exactly the audio that was genuinely missing. +/// +/// Time is passed IN, so the policy is pure and its tests run on every platform. +#[derive(Default)] +pub(crate) struct InfillPolicy { + /// Silence already sent for the open hole. Denominated in TIME rather than in frames or + /// callbacks, which is the recorded lesson from the client's de-prime fuse: a count there made + /// an iPad give up three times sooner than a Mac for no reason anyone intended. + filled_ms: u32, + /// Latched once a hole outlives the budget and the wire falls silent. + broke: bool, +} + +impl InfillPolicy { + /// Decide the slot that is due now. Call EXACTLY once per due frame — it consumes budget. + pub(crate) fn decide(&mut self, since_last_chunk: Duration) -> Infill { + if since_last_chunk < INFILL_AFTER { + return Infill::Wait; + } + if self.filled_ms as u64 >= INFILL_MAX.as_millis() as u64 { + self.broke = true; + return Infill::Quiet; + } + self.filled_ms += FRAME_MS; + Infill::Silence + } + + /// True once the budget is spent, so the caller can go back to blocking for real audio + /// instead of waking every few milliseconds to decide to stay quiet. + pub(crate) fn exhausted(&self) -> bool { + self.filled_ms as u64 >= INFILL_MAX.as_millis() as u64 + } + + /// A real chunk arrived. Returns whether the hole it closed BROKE continuity — the wire went + /// silent across it, so the redundancy predecessor and any partial frame straddling the hole + /// both describe audio from before a discontinuity, and neither may be spliced onto what + /// comes next. + pub(crate) fn chunk_arrived(&mut self) -> bool { + self.filled_ms = 0; + std::mem::take(&mut self.broke) + } +} + #[cfg(test)] mod tests { use super::*; @@ -257,4 +372,149 @@ mod tests { let (_, _, pct) = half.summary(Duration::from_secs(1), 48_000); assert!((pct - 50.0).abs() < 1.0, "expected ~50 %, got {pct}"); } + + /// The 5 ms quantum we ask for — the shape all three gap tests are measured against. + const Q: Duration = Duration::from_millis(5); + + /// The product gap the 2026-08-15 field log left open, closed: `delivered_pct` alone reports + /// the SAME 93 % for one 2 s hole and for three hundred 8 ms hiccups, and those are different + /// faults with different fixes. The counters have to separate them without a second log. + #[test] + fn gap_accounting_tells_one_long_hole_from_many_short_ones() { + let mut one = CaptureStats::default(); + one.observe_callback(None, Q); // first callback of the stream — nothing to compare to + one.observe_callback(Some(Duration::from_secs(2)), Q); + assert_eq!(one.gaps, 1); + assert_eq!(one.max_gap_ms(), 2_000); + + let mut many = CaptureStats::default(); + for _ in 0..300 { + many.observe_callback(Some(Duration::from_millis(8)), Q); + } + assert_eq!(many.gaps, 300); + assert_eq!(many.max_gap_ms(), 8); + + // Both shapes lose comparable audio; only the counters tell them apart. + assert!( + one.max_gap_ms() > many.max_gap_ms() * 100, + "the discriminator is the SHAPE, not the total" + ); + } + + /// A stream delivering exactly what it negotiated is never a gap — including the clamped + /// 21.3 ms quantum a VM's `default.clock.min-quantum` forces, which would otherwise score a + /// gap on every single callback and bury the real ones. + #[test] + fn a_negotiated_cadence_is_never_a_gap() { + let mut s = CaptureStats::default(); + for _ in 0..100 { + s.observe_callback(Some(Duration::from_micros(5_100)), Q); // 5 ms + jitter + } + assert_eq!(s.gaps, 0, "ordinary jitter at the negotiated quantum"); + + let clamped = Duration::from_micros(21_333); // 1024 frames @ 48 kHz + let mut vm = CaptureStats::default(); + for _ in 0..100 { + vm.observe_callback(Some(clamped), clamped); + } + assert_eq!(vm.gaps, 0, "a clamped quantum is slow, not gapping"); + // …and a real hole on that same graph still scores. + vm.observe_callback(Some(Duration::from_millis(200)), clamped); + assert_eq!(vm.gaps, 1); + } + + /// Drive one hole from the moment it opens until the policy gives up on it, the way the + /// encode loop does: one decision per due frame slot. + fn cover_a_hole(p: &mut InfillPolicy) -> usize { + let mut silence = 0usize; + let mut open = INFILL_AFTER; + loop { + match p.decide(open) { + Infill::Silence => { + silence += 1; + open += Duration::from_millis(FRAME_MS as u64); + } + Infill::Quiet => return silence, + Infill::Wait => unreachable!("the hole is open — {open:?} is past INFILL_AFTER"), + } + assert!(silence < 10_000, "the budget must be finite"); + } + } + + /// The wire covers a hole for exactly as long as the budget allows, and then admits the host + /// is simply quiet. Both halves matter: without the first, a 30 ms hole costs the client a + /// de-prime and a re-prime; without the second, an idle desktop pays for silence datagrams + /// forever. + #[test] + fn infill_covers_a_hole_and_then_admits_the_host_is_quiet() { + let mut p = InfillPolicy::default(); + // Ordinary quantum jitter, not a hole — nothing owed. + assert_eq!(p.decide(Duration::ZERO), Infill::Wait); + assert_eq!( + p.decide(INFILL_AFTER - Duration::from_millis(1)), + Infill::Wait + ); + + let silence = cover_a_hole(&mut p); + assert_eq!( + silence as u64 * FRAME_MS as u64, + INFILL_MAX.as_millis() as u64, + "the wire must cover exactly the budget, in frames of {FRAME_MS} ms" + ); + assert!(p.exhausted(), "…and then stop asking"); + } + + /// A stream that is flowing must never synthesize anything — this policy is invisible until + /// something is actually wrong. + #[test] + fn a_flowing_stream_never_infills() { + let mut p = InfillPolicy::default(); + for _ in 0..1_000 { + assert_eq!( + p.decide(Duration::from_millis(FRAME_MS as u64)), + Infill::Wait + ); + } + assert!(!p.exhausted()); + assert!(!p.chunk_arrived(), "no hole means no discontinuity"); + } + + /// Only a hole the wire could NOT cover breaks continuity. The distinction is the whole point + /// of the budget: across a covered hole `seq` and pts never broke, so the redundancy + /// predecessor still describes the frame before this one and the client can keep using it. + #[test] + fn only_an_uncovered_hole_breaks_continuity() { + let mut covered = InfillPolicy::default(); + for k in 0..20u64 { + covered.decide(INFILL_AFTER + Duration::from_millis(FRAME_MS as u64 * k)); + } + assert!( + !covered.chunk_arrived(), + "a covered hole is continuous — the client heard silence, not a splice" + ); + + let mut lost = InfillPolicy::default(); + cover_a_hole(&mut lost); + assert!( + lost.chunk_arrived(), + "past the budget the wire went quiet, so nothing before the hole may be spliced on" + ); + // …and the next hole starts from a clean budget rather than an exhausted one. + assert!(!lost.exhausted()); + assert_eq!(cover_a_hole(&mut lost) as u64 * FRAME_MS as u64, 500); + } + + /// A deliberate pause is not a hole. The caller drops its stamp across a state transition, so + /// the span reaches us as `None` — without that rule every Paused↔Streaming flap (three of + /// them in minute 1 of the field log, around each format renegotiation) would report a gap + /// the size of the pause and drown the sub-10 ms ones that actually matter. + #[test] + fn a_paused_span_is_not_scored() { + let mut s = CaptureStats::default(); + s.observe_callback(Some(Duration::from_millis(5)), Q); + s.observe_callback(None, Q); // resumed: the pause spanned an unknowable amount of time + s.observe_callback(Some(Duration::from_millis(5)), Q); + assert_eq!(s.gaps, 0); + assert_eq!(s.max_gap_ms(), 0); + } } diff --git a/crates/punktfunk-host/src/audio/linux/mod.rs b/crates/punktfunk-host/src/audio/linux/mod.rs index 9749e2c9..473fc502 100644 --- a/crates/punktfunk-host/src/audio/linux/mod.rs +++ b/crates/punktfunk-host/src/audio/linux/mod.rs @@ -162,7 +162,11 @@ impl Drop for PwAudioCapturer { impl AudioCapturer for PwAudioCapturer { fn next_chunk(&mut self) -> Result> { - match self.chunks.recv_timeout(Duration::from_secs(5)) { + self.next_chunk_within(Duration::from_secs(5)) + } + + fn next_chunk_within(&mut self, budget: Duration) -> Result> { + match self.chunks.recv_timeout(budget) { Ok(c) => Ok(c), // A quiet sink (paused game, idle desktop) is NOT a failure — return an empty chunk so the // caller keeps the capturer alive. Only a dead capture thread is an Err (→ caller reopens). @@ -740,6 +744,18 @@ fn pw_thread( // default election against real hardware; session routing comes from the // stream_sink claim, not from priority. "priority.session" => "50", + // Never let the session manager suspend this node (WP-B2). The 2026-08-15 + // field log shows three `audio format negotiated` lines in the first minute, + // each wrapped in a Paused↔Streaming flap: Wine churns its audio device at + // launch, the sink goes briefly unused, WirePlumber suspends it on its idle + // timeout, and the next app resumes it — and every one of those round trips + // is a real hole in a stream someone is listening to. + // + // Deliberately NOT `node.always-process`: that keeps the node SCHEDULED with + // nothing connected, so a host sitting between sessions would run this + // callback two hundred times a second forever (risk R5). Disabling the + // suspend keeps the node available without asking anyone to drive it. + "session.suspend-timeout-seconds" => "0", }; p.insert(*pw::keys::NODE_NAME, name.as_str()); p @@ -772,6 +788,19 @@ 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, + /// When the callback last ran (WP-A2), so its CADENCE can be scored and not just its + /// content. Cleared across a state transition — a deliberate Paused span must not + /// read as one enormous hole. Lives here rather than in `stats` because the stats + /// reset every reporting window and the cadence does not. + last_cb: Option, + /// The quantum actually negotiated, which is what a gap is measured against. Seeded + /// with the one we ASK for, and corrected on the first callback that carries data — + /// on a graph that clamped us to 1024 frames, 21.3 ms between callbacks is the deal + /// we got, not a fault. + quantum: Duration, + /// The format currently negotiated, so a renegotiation that resolves to the SAME one + /// can be told from a real change (WP-B2). + negotiated: Option<(spa::param::audio::AudioFormat, u32, u32)>, /// 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, @@ -782,14 +811,24 @@ fn pw_thread( stats: Default::default(), last_stats: std::time::Instant::now(), reported_quantum: false, + last_cb: None, + quantum: Duration::from_micros( + CAPTURE_QUANTUM_FRAMES as u64 * 1_000_000 / SAMPLE_RATE as u64, + ), + negotiated: None, active, }; let _listener = stream .add_local_listener_with_user_data(ud) .state_changed({ let mainloop = mainloop.clone(); - move |_s, _ud, old, new| { + move |_s, ud, old, new| { tracing::debug!(?old, ?new, "pipewire audio stream state"); + // Any transition ends the cadence we were measuring: the span across a + // Paused↔Streaming flap is not a gap in delivery, it is a gap in the stream + // existing. Scoring it would report one huge hole per renegotiation and bury + // the sub-10 ms ones the field log is actually about (WP-A2). + ud.last_cb = None; // A stream error is unrecoverable for this instance — exit so the sessions' // reopen path builds a fresh one (same contract as the core-error path above). if matches!(new, pw::stream::StreamState::Error(_)) { @@ -797,13 +836,29 @@ fn pw_thread( } } }) - .param_changed(move |_stream, _tx, id, param| { + .param_changed(move |_stream, ud, 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() { + // Renegotiating to the format we already had is the graph resuming us, not + // the stream changing (WP-B2): the field log's minute-1 burst was three of + // these, which read as three format changes and were none. Say which it was + // — the flap itself stays visible in the state DEBUG lines and in A2's gap + // counters, where it belongs. + let now = (info.format(), info.rate(), info.channels()); + if ud.negotiated == Some(now) { + tracing::debug!( + format = ?now.0, + rate = now.1, + channels = now.2, + "audio format renegotiated, unchanged (the graph resumed our sink)" + ); + return; + } + ud.negotiated = Some(now); // `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 @@ -824,11 +879,22 @@ fn pw_thread( }) .process(|stream, ud| { let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + // Score the ARRIVAL before anything can make us return: a callback that ran + // and carried nothing still proves the callback ran, and that is a different + // fault from one that never ran at all. Stamped first, so the two are + // counted independently (WP-A2). + let now = std::time::Instant::now(); + let since_last = ud.last_cb.map(|t| now.duration_since(t)); + ud.last_cb = Some(now); + ud.stats.observe_callback(since_last, ud.quantum); + let Some(mut buffer) = stream.dequeue_buffer() else { + ud.stats.missed_dequeues += 1; return; }; let datas = buffer.datas_mut(); if datas.is_empty() { + ud.stats.missed_dequeues += 1; return; } let d = &mut datas[0]; @@ -836,8 +902,12 @@ fn pw_thread( let c = d.chunk(); (c.offset() as usize, c.size() as usize) }; - let Some(buf) = d.data() else { return }; + let Some(buf) = d.data() else { + ud.stats.missed_dequeues += 1; + return; + }; if offset > buf.len() { + ud.stats.missed_dequeues += 1; return; } let region = &buf[offset..(offset + size).min(buf.len())]; @@ -854,6 +924,12 @@ fn pw_thread( // whole field investigation to find; it should cost one log line. let frames = n / (ud.channels.max(1) as usize); let want = CAPTURE_QUANTUM_FRAMES as usize; + // What a gap is measured against from here on — see `CapUd::quantum`. + if frames > 0 { + ud.quantum = Duration::from_micros( + frames as u64 * 1_000_000 / SAMPLE_RATE as u64, + ); + } if frames > want { tracing::warn!( requested_frames = want, @@ -910,6 +986,12 @@ fn pw_thread( peak_db = format!("{peak_db:.1}"), rms_db = format!("{rms_db:.1}"), delivered_pct = format!("{delivered_pct:.0}"), + // The shape of whatever `delivered_pct` is short by (WP-A2): one + // long hole and three hundred short ones read the same in the + // percentage and mean entirely different things. + gaps = ud.stats.gaps, + max_gap_ms = ud.stats.max_gap_ms(), + missed_dequeues = ud.stats.missed_dequeues, dropped_chunks = ud.stats.dropped_chunks, "desktop audio capture" ); diff --git a/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs b/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs index 6ac3891e..8928a4d7 100644 --- a/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs +++ b/crates/punktfunk-host/src/audio/windows/wasapi_cap.rs @@ -117,7 +117,10 @@ impl Drop for WasapiLoopbackCapturer { impl AudioCapturer for WasapiLoopbackCapturer { fn next_chunk(&mut self) -> Result> { - match self.chunks.recv_timeout(Duration::from_secs(5)) { + self.next_chunk_within(Duration::from_secs(5)) + } + fn next_chunk_within(&mut self, budget: Duration) -> Result> { + match self.chunks.recv_timeout(budget) { Ok(c) => Ok(c), // A quiet sink is NOT a failure — return an empty chunk so the caller keeps the capturer // alive. Only a dead capture thread is an Err (→ caller reopens). Matches the Linux path. @@ -185,6 +188,13 @@ const DEFAULT_CHECK_EVERY: Duration = Duration::from_secs(1); const FIRST_OPEN_ATTEMPTS: u32 = 3; /// Pause between first-open attempts (endpoint churn settles in well under a second). const FIRST_OPEN_RETRY_PAUSE: Duration = Duration::from_secs(1); +/// How long the packet loop may go without a packet before the next `DATA_DISCONTINUITY` reads as +/// the endpoint having idled rather than as a capture hole (WP-A2). Classic loopback delivers +/// nothing at all while nothing renders and then flags the packet that resumes, so scoring that +/// flag unconditionally would charge a gap — sized by the quiet — to every notification sound on an +/// otherwise silent host. At the engine's ~10 ms period a whole second without a packet is far +/// past anything this loop can still tell apart from that. +const LOOPBACK_IDLE_AFTER: Duration = Duration::from_secs(1); fn capture_thread( tx: SyncSender>, @@ -596,6 +606,16 @@ fn capture_once( // and a permanent A/V offset, with nothing in any log). let mut stats = CaptureStats::default(); let mut last_stats = Instant::now(); + // WP-A2 — where the gap counters come from on Windows. `delivered_pct` proves audio is missing + // but not whether it went in one hole or three hundred, and no clock can answer that here: this + // is a POLLING loop over a tap that stops delivering entirely while the endpoint idles, so + // "time since the last data" — the rule the Linux capture callback uses — would score every + // quiet moment on the host as a hole. WASAPI says it outright instead: a packet flagged + // discontinuous is one the tap admits is not contiguous with the previous, and the device + // position it carries (`next_index` is where the next packet must start if nothing was lost) + // sizes the missing audio in the device's own clock, not in how late we happened to poll. + let mut last_packet: Option = None; + let mut next_index: u64 = 0; // WP2.4 — damping for the default-playback tug-of-war. let mut fight = FightDamper::new(Instant::now()); loop { @@ -611,9 +631,34 @@ fn capture_once( Ok(Some(0)) | Ok(None) => break, Ok(Some(_n)) => { saw_packets = true; - capture_client + let before = bytes.len(); + let info = capture_client .read_from_device_to_deque(&mut bytes) .context("read loopback")?; + let now = Instant::now(); + // Judged BEFORE the stamp moves: a discontinuity on the first packet after a + // packet-less stretch is the tap waking up, which is what an idle endpoint + // does here, not a hole in anything that was playing. + let flowing = + last_packet.is_some_and(|t| now.duration_since(t) < LOOPBACK_IDLE_AFTER); + let frames = ((bytes.len() - before) / block_align) as u64; + if frames == 0 { + // Told a packet was ready, then handed none: the same fault the Linux twin + // counts when a callback runs with no buffer to dequeue, and equally + // invisible before — a tap spinning like this looked exactly like a quiet + // desktop. + stats.missed_dequeues += 1; + } else { + if info.flags.data_discontinuity && flowing { + stats.gaps += 1; + let lost = info.index.saturating_sub(next_index); + stats.max_gap_us = stats + .max_gap_us + .max(lost.saturating_mul(1_000_000) / SAMPLE_RATE as u64); + } + next_index = info.index.saturating_add(frames); + last_packet = Some(now); + } } Err(e) => return Err(anyhow!("get_next_packet_size: {e}")), } @@ -667,6 +712,14 @@ fn capture_once( peak_db = format!("{peak_db:.1}"), rms_db = format!("{rms_db:.1}"), delivered_pct = format!("{delivered_pct:.0}"), + // The shape of whatever `delivered_pct` is short by (WP-A2): one long hole and + // three hundred short ones read the same in the percentage and mean entirely + // different things. Sourced from WASAPI's own discontinuity flag here rather than + // from the callback cadence the Linux twin measures, so an endpoint that idles and + // resumes is not among them — see [`LOOPBACK_IDLE_AFTER`]. + gaps = stats.gaps, + max_gap_ms = stats.max_gap_ms(), + missed_dequeues = stats.missed_dequeues, dropped_chunks = stats.dropped_chunks, "desktop audio capture" ); diff --git a/crates/punktfunk-host/src/native/audio.rs b/crates/punktfunk-host/src/native/audio.rs index a26d6092..e5510e3c 100644 --- a/crates/punktfunk-host/src/native/audio.rs +++ b/crates/punktfunk-host/src/native/audio.rs @@ -162,9 +162,22 @@ pub(super) fn audio_thread( ); } let mut acc: Vec = Vec::with_capacity(frame_len * 4); + // The frame currently being encoded. Reused rather than collected fresh each time: it is + // filled from `acc` on the normal path and padded out with silence on the infill path, and + // one buffer covers both without allocating 200 times a second. + let mut frame_buf: Vec = Vec::with_capacity(frame_len); // Sized for the largest surround frame (7.1 HQ ≈ 1.3 KB at 5 ms); ample for normal quality. let mut opus_buf = vec![0u8; 4096]; let mut seq: u32 = 0; + // W-B1 — whether the wire covers a capture hole with silence, and for how long. See + // [`InfillPolicy`]: before this, a hole meant the loop simply blocked in `next_chunk` and + // NOTHING left the host for its duration, so the client's ring drained → underran → + // de-primed → re-primed, and a 30 ms hole became a much longer audible artifact. + let mut infill = crate::audio::capture_policy::InfillPolicy::default(); + let mut last_chunk_at = std::time::Instant::now(); + // Nothing may be synthesized before the first real frame: there is no continuity to protect + // yet, and the wire clock has no anchor to continue from. + let mut sent_any = false; // Reopen-with-backoff: hold the capturer in an Option so a mid-session capture-thread death // (device unplug, daemon restart) — or a first open lost to session-start churn above — // reopens instead of muting the rest of a multi-hour session. A quiet sink is NOT a death — @@ -199,7 +212,11 @@ pub(super) fn audio_thread( // Uninitialised on purpose: every read is preceded by the re-anchor at the top of the chunk // loop, and seeding it with a placeholder would just be a value the compiler correctly points // out is never read. - let mut next_pts_ns: u64; + // + // Seeded rather than left uninitialised now that infilled frames advance it too: it is the + // pts of the NEXT frame to leave, real or synthesized, and every send advances it by one + // frame. `sent_any` is what keeps the seed from ever reaching the wire. + let mut next_pts_ns: u64 = 0; let mut pace_due: Option = None; if capturer.is_some() { tracing::info!( @@ -235,7 +252,28 @@ pub(super) fn audio_thread( } } } - let chunk = match capturer.as_mut().unwrap().next_chunk() { + // Wake on whichever comes first: a capture chunk, or the moment the wire next has + // something to say. Waiting only on capture is what made a hole cost more than the audio + // it swallowed — see [`InfillPolicy`]. + let waited = if infill.exhausted() { + // Past the infill budget the host is not glitching, it is QUIET, and there is nothing + // to send until real audio returns. Block the way this loop always did rather than + // waking two hundred times a second to decide to stay silent. + capturer.as_mut().unwrap().next_chunk() + } else { + let now = std::time::Instant::now(); + // A frame that is due but has no audio behind it cannot be acted on until the hole is + // old enough to be worth covering, so wait for the LATER of the two — waiting only for + // the due time would spin through the window between them. + let ready_at = match pace_due { + Some(due) if acc.len() >= frame_len => due, + Some(due) => due.max(last_chunk_at + crate::audio::capture_policy::INFILL_AFTER), + None => now + FRAME_INTERVAL, + }; + let budget = ready_at.saturating_duration_since(now).min(PACE_MAX_SLEEP); + capturer.as_mut().unwrap().next_chunk_within(budget) + }; + let chunk = match waited { Ok(c) => c, Err(e) => { tracing::warn!(error = %format!("{e:#}"), "audio capture lost — reopening"); @@ -244,41 +282,68 @@ pub(super) fn audio_thread( continue; } }; - // Anchor the sample clock on THIS chunk's arrival. PipeWire hands us a buffer of already - // captured audio, so the newest sample in `acc` is ~now and the oldest is one whole - // buffer-occupancy earlier. Re-deriving the anchor every chunk (rather than free-running - // a counter) keeps the stamp tied to the capture device's own cadence, so a drifting or - // resampling graph corrects itself instead of accumulating error over a long session. - let arrival_ns = now_ns(); - acc.extend_from_slice(&chunk); - let queued_frames = (acc.len() / want as usize) as u64; - next_pts_ns = arrival_ns.saturating_sub(queued_frames * 1_000_000_000 / SAMPLE_RATE as u64); - while acc.len() >= frame_len { - // Hold each frame until its slot on the audio clock. The FIRST frame of a chunk is - // already due (its samples are the oldest we hold), so this only ever delays the - // tail of a multi-frame chunk — exactly the burst we are trying not to send. A - // schedule that has fallen more than one frame behind is re-anchored rather than - // chased, so a scheduling hiccup cannot turn into a permanent send-time debt. + if !chunk.is_empty() { + if infill.chunk_arrived() { + // The wire fell silent across that hole. The partial frame in `acc` and the + // redundancy predecessor both describe audio from before a discontinuity, so + // splicing either onto what follows is a click plus a pts that lies about it. + acc.clear(); + prev_frame.clear(); + } + last_chunk_at = std::time::Instant::now(); + // Anchor the sample clock on THIS chunk's arrival. PipeWire hands us a buffer of + // already captured audio, so the newest sample in `acc` is ~now and the oldest is one + // whole buffer-occupancy earlier. Re-deriving the anchor every chunk (rather than + // free-running a counter) keeps the stamp tied to the capture device's own cadence, so + // a drifting or resampling graph corrects itself instead of accumulating error over a + // long session. + let arrival_ns = now_ns(); + acc.extend_from_slice(&chunk); + let queued_frames = (acc.len() / want as usize) as u64; + let anchor = + arrival_ns.saturating_sub(queued_frames * 1_000_000_000 / SAMPLE_RATE as u64); + // Never step backwards. Infilled frames advanced the wire clock while capture was + // away, and an anchor re-derived from this chunk's arrival can land at or before the + // last frame we already sent. + next_pts_ns = anchor.max(next_pts_ns); + } + // Everything the wire owes for the slots that have come due — real or synthesized, one + // schedule, one encoder, one `seq`. A schedule that has fallen more than one frame behind + // is re-anchored rather than chased, so a scheduling hiccup cannot turn into a permanent + // send-time debt. + loop { let now = std::time::Instant::now(); match pace_due { - Some(due) if due > now => { - let wait = due - now; - // Never sleep longer than the audio we are holding: `next_chunk` has to be - // serviced or the capture channel backs up and starts dropping. - std::thread::sleep(wait.min(PACE_MAX_SLEEP)); - } + Some(due) if due > now => break, // this frame's slot has not arrived yet Some(due) if now.duration_since(due) > PACE_REANCHOR => pace_due = None, _ => {} } + frame_buf.clear(); + if acc.len() >= frame_len { + frame_buf.extend(acc.drain(..frame_len)); + } else if !sent_any { + break; + } else { + match infill.decide(last_chunk_at.elapsed()) { + crate::audio::capture_policy::Infill::Silence => { + // Pad the partial frame out with silence and send THAT, rather than + // leaving it for post-gap samples to complete: one frame carrying audio + // from both sides of a hole is a click, and its pts is a lie about when + // half of it was captured. + frame_buf.extend(acc.drain(..)); + frame_buf.resize(frame_len, 0.0); + } + crate::audio::capture_policy::Infill::Wait + | crate::audio::capture_policy::Infill::Quiet => break, + } + } pace_due = Some(pace_due.unwrap_or_else(std::time::Instant::now) + FRAME_INTERVAL); - - let mut frame: Vec = acc.drain(..frame_len).collect(); if gain != 1.0 { - punktfunk_core::audio::apply_gain(&mut frame, gain); + punktfunk_core::audio::apply_gain(&mut frame_buf, gain); } let pts_ns = next_pts_ns; next_pts_ns += FRAME_MS as u64 * 1_000_000; - match enc.encode_float(&frame, &mut opus_buf) { + match enc.encode_float(&frame_buf, &mut opus_buf) { Ok(n) => { let opus = &opus_buf[..n]; let d = if redundancy { -- 2.54.0 From 925b07ec10b06e20f98d80e51208e7aa4be7c9c5 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 15 Aug 2026 13:25:15 +0200 Subject: [PATCH 03/10] fix(capture): the wire pts was stamped downstream of the compositor's own delivery jitter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP-A3 and B3 of design/host-source-stutter-fixes.md. All three Linux publish sites stamped pts_ns with SystemTime::now() inside OUR PipeWire process callback — the instant the buffer was DELIVERED to us, not the instant the compositor produced it. On a host whose screencast delivery is jittery that difference IS the jitter, and it was baked into the timestamps the client plays back from. This is what makes the client-side cure a non-cure on its own: the CadenceClock design's §2.2 invariant is that the loop smooths the offset and never the timestamps, so source-timestamp playout would faithfully REPRODUCE this jitter rather than absorb it. Fixing the host stamp is that work's prerequisite, not an alternative to it. The compositor's spa_meta_header is already located per buffer for the CORRUPTED skip, and only .flags was ever read. Its .pts is the producer's own instant, upstream of the delivery. Whether that stamp is actually cleaner is a question about a specific compositor, not something to assume, so both halves ship: a 30 s provenance line reports each clock's interval MAD about its own median (a shared centre would fold the period-estimation error and any genuine rate difference into a number meant to be about jitter), the empirical period, the sample count behind it, and the header-minus-delivery p50 as a domain check. If the compositor's number is materially tighter its stamp is worth shipping; if both are equally ragged the producer composes irregularly and no choice of stamp can help — which is a real possible outcome the line will state rather than hide. The stamp itself is now taken once, at the top of consume_frame, and used by all three publish paths: they each took their own reading at whatever point they reached the publish, so a CPU de-pad's milliseconds landed inside the timestamp and the paths could drift apart silently. The compositor's monotonic stamp is rebased into the wire's realtime domain from a clock pair re-sampled each window, and a per-frame plausibility gate falls back to the delivery stamp — counted, not silent — for anything more than 50 ms out, so a producer that never fills the header in, or fills it with the wrong clock, is exactly as it was. PUNKTFUNK_CAPTURE_HDR_PTS=0 puts the whole stream back on delivery stamps. Note for the release: stamping at arrival instead of at publish makes host_us and e2e read HIGHER by the delivery delay we previously failed to count. The numbers move because they get truer. --- crates/pf-capture/src/lib.rs | 6 + crates/pf-capture/src/linux/pipewire.rs | 121 +++++++- crates/pf-capture/src/pts_provenance.rs | 391 ++++++++++++++++++++++++ 3 files changed, 505 insertions(+), 13 deletions(-) create mode 100644 crates/pf-capture/src/pts_provenance.rs diff --git a/crates/pf-capture/src/lib.rs b/crates/pf-capture/src/lib.rs index 153026fb..436cb443 100644 --- a/crates/pf-capture/src/lib.rs +++ b/crates/pf-capture/src/lib.rs @@ -564,6 +564,12 @@ pub type CursorForwardSender = std::sync::Arc Result<()> + Send #[cfg(target_os = "linux")] pub mod pwinit; +// Which clock the wire's `pts_ns` comes from, and how clean each candidate is. Only the Linux +// capturer consumes it, but the arithmetic is pure and platform-independent, so its tests run +// everywhere rather than only where the backend does. +#[cfg(any(target_os = "linux", test))] +mod pts_provenance; + // The Windows backend lives under `windows/`, the Linux one under `linux/`. Windows capture is IDD // direct-push only (DXGI Desktop Duplication + the WGC relay were removed). #[cfg(target_os = "windows")] diff --git a/crates/pf-capture/src/linux/pipewire.rs b/crates/pf-capture/src/linux/pipewire.rs index 0dcbf2e5..aeed690d 100644 --- a/crates/pf-capture/src/linux/pipewire.rs +++ b/crates/pf-capture/src/linux/pipewire.rs @@ -71,6 +71,19 @@ struct UserData { linear_nv12_failed: bool, /// Rate-limit counter for the latest-frame-only diagnostic log (see `.process`). dbg_log_n: u64, + /// WP-A3/B3 — which clock the wire's `pts_ns` comes from, and how clean each one is. See + /// [`crate::pts_provenance`]: the delivery stamp this loop used to take unconditionally is + /// downstream of the compositor's delivery jitter, so it bakes that jitter into the + /// timestamps the client plays back from. + pts: crate::pts_provenance::PtsProvenance, + /// When the provenance window opened. + pts_reported: std::time::Instant, + /// `CLOCK_REALTIME − CLOCK_MONOTONIC`, ns — what carries the compositor's monotonic stamp + /// into the wire's realtime domain. Re-sampled each reporting window; the two clocks drift + /// by µs over 30 s, so this is not a per-frame cost. + rt_minus_mono_ns: i64, + /// `PUNKTFUNK_CAPTURE_HDR_PTS=0` puts the wire back on the delivery stamp unconditionally. + hdr_pts_enabled: bool, /// PW4 step 1: the producer-fence wait distribution, measured on this (the PipeWire loop) /// thread. Per-session, like the fall-through tally. fence_wait: FenceWaitStats, @@ -790,10 +803,40 @@ impl Drop for DmabufMap { /// /// `pw_buf` is the buffer's `pw_buffer` handle (`spa_buf`'s owner), used only as the identity a /// raw-passthrough publish withholds via [`UserData::try_defer`] — never dereferenced here. +/// How often the wire-pts provenance line is emitted (WP-A3). Matches the audio plane's stats +/// cadence so a field log reads as one timeline. +const PTS_REPORT_EVERY: std::time::Duration = std::time::Duration::from_secs(30); + +/// `CLOCK_REALTIME − CLOCK_MONOTONIC`, ns. +/// +/// PipeWire stamps `spa_meta_header.pts` in the graph's clock domain (`CLOCK_MONOTONIC`); the wire +/// — and the client's plausibility gate — speak realtime-since-epoch. Sampling the pair back to +/// back is what carries one into the other, and the residual is the few hundred nanoseconds +/// between the two reads against a 50 ms plausibility window. A failed read reports 0, which puts +/// every rebased stamp outside that window and falls the whole stream back to delivery stamps — +/// the safe direction. +fn realtime_minus_monotonic_ns() -> i64 { + let rt = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_nanos() as i64) + .unwrap_or(0); + let mut ts = libc::timespec { + tv_sec: 0, + tv_nsec: 0, + }; + // SAFETY: `clock_gettime` writes one `timespec` through the pointer and touches nothing else; + // `ts` is a live, properly aligned local. A non-zero return leaves it untouched. + if unsafe { libc::clock_gettime(libc::CLOCK_MONOTONIC, &mut ts) } != 0 { + return 0; + } + rt - (ts.tv_sec as i64 * 1_000_000_000 + ts.tv_nsec as i64) +} + fn consume_frame( ud: &mut UserData, spa_buf: *mut spa::sys::spa_buffer, pw_buf: *mut pw::sys::pw_buffer, + hdr_pts_ns: Option, ) { // No active stream: release the buffer without the (expensive at 5K) de-pad. if !ud.signals.active.load(Ordering::Relaxed) { @@ -831,6 +874,54 @@ fn consume_frame( return; // format not negotiated yet } + // ONE stamp for this frame, whichever of the three paths below publishes it (WP-B3). Each + // used to take its own `SystemTime::now()` at the moment it happened to reach the publish, so + // a CPU de-pad's milliseconds landed INSIDE the timestamp and the three paths could drift + // apart without anything saying so. Sampled here, before any of that work, the stamp + // describes the frame's arrival rather than our handling of it. + let delivery_ns = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_nanos() as u64) + .unwrap_or(0); + let hdr_pts_ns = hdr_pts_ns.filter(|&p| p > 0); + // Measured on EVERY capture, whether or not the compositor's stamp is the one we ship: the + // question "which clock is cleaner on this host" is what decides that, and a diagnostic that + // only runs once you already trust the answer cannot inform it. + ud.pts.observe(hdr_pts_ns, delivery_ns); + let stamp = crate::pts_provenance::wire_pts( + ud.hdr_pts_enabled.then_some(hdr_pts_ns).flatten(), + delivery_ns, + ud.rt_minus_mono_ns, + ); + let pts_ns = stamp.pts_ns; + if ud.hdr_pts_enabled && hdr_pts_ns.is_some() && !stamp.from_header { + ud.pts.implausible += 1; + } + if ud.pts_reported.elapsed() >= PTS_REPORT_EVERY { + if let Some(r) = ud.pts.report() { + tracing::info!( + frames = r.frames, + with_hdr = r.with_hdr, + samples = r.samples, + period_us = r.period_us, + // THE pair this whole work package exists to compare. Materially tighter on the + // left ⇒ the compositor's stamp is worth shipping; equally ragged ⇒ the producer + // composes irregularly and no choice of stamp can fix it. + hdr_mad_us = r.hdr_mad_us, + delivery_mad_us = r.delivery_mad_us, + offset_p50_ms = r.offset_p50_ms, + implausible = r.implausible, + hdr_pts_used = ud.hdr_pts_enabled, + "capture wire-pts provenance" + ); + } + ud.pts.reset_window(); + ud.pts_reported = std::time::Instant::now(); + // The two clocks drift by µs over a window; re-pairing them here keeps the rebase honest + // over a multi-hour session without costing anything per frame. + ud.rt_minus_mono_ns = realtime_minus_monotonic_ns(); + } + // Implicit-fence wait: Mutter renders into the dmabuf and hands it over at // GPU-submit time; with no producer explicit sync (Mutter+NVIDIA can't) we snapshot // the buffer's implicit fence and wait the producer's render before sampling — @@ -985,10 +1076,6 @@ fn consume_frame( if dup < 0 { break 'passthrough PassthroughFallback::DupFailed; } - let pts_ns = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|d| d.as_nanos() as u64) - .unwrap_or(0); let hold = ud.try_defer(pw_buf); ud.publish(CapturedFrame { width: w as u32, @@ -1134,10 +1221,6 @@ fn consume_frame( "zero-copy: dmabuf imported to CUDA (no CPU copy)" ); } - let pts_ns = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|d| d.as_nanos() as u64) - .unwrap_or(0); ud.publish(CapturedFrame { width: w as u32, height: h as u32, @@ -1342,10 +1425,6 @@ fn consume_frame( // the layout isn't packed RGB). This is the CPU path's counterpart to the producer's // hardware cursor plane, which stays out of the captured buffer. composite_cursor(&mut tight, w, h, fmt, &ud.cursor); - let pts_ns = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|d| d.as_nanos() as u64) - .unwrap_or(0); let frame = CapturedFrame { width: w as u32, height: h as u32, @@ -1633,6 +1712,10 @@ pub fn pipewire_thread( yuv444: want_444, linear_nv12_failed: false, dbg_log_n: 0, + pts: crate::pts_provenance::PtsProvenance::new(), + pts_reported: std::time::Instant::now(), + rt_minus_mono_ns: realtime_minus_monotonic_ns(), + hdr_pts_enabled: std::env::var("PUNKTFUNK_CAPTURE_HDR_PTS").as_deref() != Ok("0"), fence_wait: FenceWaitStats::default(), pool: PoolCensus::default(), passthrough_fallbacks: PassthroughFallbacks::default(), @@ -1912,6 +1995,18 @@ pub fn pipewire_thread( // while the buffer is still held. unsafe { (*hdr).flags } }; + // The compositor's OWN stamp for this frame (WP-A3/B3), stamped upstream of the + // delivery jitter our `SystemTime::now()` cannot see past. Located here because + // the header already is; whether it is worth shipping is what the provenance + // line measures. + let hdr_pts = if hdr.is_null() { + None + } else { + // SAFETY: as for `.flags` — non-null, from a lookup that demanded at least + // `size_of::()` bytes (so `.pts` is in bounds), read while + // the buffer is still held. + Some(unsafe { (*hdr).pts }) + }; // First data chunk's size + flags (used for the diagnostic + CORRUPTED check) // and its data type (a dmabuf legitimately reports chunk size 0, so the size-0 // stale skip only applies to mappable SHM buffers). @@ -1960,7 +2055,7 @@ pub fn pipewire_thread( return; } - consume_frame(ud, spa_buf, newest); + consume_frame(ud, spa_buf, newest, hdr_pts); })); // Hand `newest` back to the stream exactly once, on EVERY path — normal, corrupted-skip, // or a caught panic in the closure above — UNLESS a raw-passthrough publish withheld it diff --git a/crates/pf-capture/src/pts_provenance.rs b/crates/pf-capture/src/pts_provenance.rs new file mode 100644 index 00000000..56d0e13a --- /dev/null +++ b/crates/pf-capture/src/pts_provenance.rs @@ -0,0 +1,391 @@ +//! Where the wire's presentation timestamp actually comes from +//! (design/host-source-stutter-fixes.md, WP-A3 and WP-B3). +//! +//! Every Linux capture publish stamps `pts_ns` with `SystemTime::now()` inside OUR PipeWire +//! process callback — the instant the buffer was DELIVERED to us, not the instant the compositor +//! produced it. On a host whose screencast delivery is jittery, that difference IS the jitter, and +//! it is baked into the timestamps the client eventually plays back from. (The 2026-08-15 Skynet +//! log: 41 phase-lock disengage cycles in 24 minutes, arrival offsets up to a full 120 Hz period, +//! on a session whose transport was provably clean.) The compositor's own `spa_meta_header.pts` is +//! stamped upstream of that delivery, so it *might* be clean — but "might" is the entire question, +//! and the client-side cure (source-timestamp playout) faithfully REPRODUCES whatever jitter the +//! timestamps carry rather than absorbing it. So: measure both clocks in the same window, against +//! each other, before trusting either. +//! +//! Time is passed IN rather than read here, which keeps this pure and lets its tests run on every +//! platform — the same reason `capture_policy` in the host crate is split out. + +/// Frames sampled per reporting window. ~34 s at 120 Hz, so a 30 s window is covered without the +/// callback ever reallocating: the vectors are built once at their cap and only ever pushed into +/// while short of it. Allocation on the PipeWire loop thread is what this avoids. +const MAX_SAMPLES: usize = 4096; + +/// A rebased compositor stamp further than this from the delivery instant is not a timestamp for +/// this frame: the wrong clock domain, a stale header, or a producer that never fills it in. Fall +/// back to the delivery stamp for that frame, and count it — silently trusting a garbage stamp +/// would put the whole stream's timing on a fiction (risk R3). +const PLAUSIBLE_NS: i64 = 50_000_000; + +/// One frame's stamp, and which clock produced it. +pub(crate) struct WirePts { + pub(crate) pts_ns: u64, + /// False when this frame fell back to the delivery stamp — the honest per-frame answer, and + /// the number that says whether B3 is actually doing anything on this host. + pub(crate) from_header: bool, +} + +/// The wire stamp for one frame. +/// +/// `hdr_pts_ns` is the compositor's `spa_meta_header.pts`, which PipeWire defines in the graph's +/// clock domain (`CLOCK_MONOTONIC`); `delivery_ns` is realtime-since-epoch, which is the domain +/// the wire and the client's plausibility gate both speak. `rt_minus_mono_ns` carries one into the +/// other. A missing or implausible header stamp yields the delivery stamp — today's behaviour — +/// so a producer that fills in no header is unaffected. +pub(crate) fn wire_pts( + hdr_pts_ns: Option, + delivery_ns: u64, + rt_minus_mono_ns: i64, +) -> WirePts { + let fallback = WirePts { + pts_ns: delivery_ns, + from_header: false, + }; + // Producers that have no timestamp write 0 (or leave it negative); neither is a stamp. + let Some(hdr) = hdr_pts_ns.filter(|&p| p > 0) else { + return fallback; + }; + let rebased = hdr.saturating_add(rt_minus_mono_ns); + if rebased <= 0 || (rebased - delivery_ns as i64).abs() >= PLAUSIBLE_NS { + return fallback; + } + WirePts { + pts_ns: rebased as u64, + from_header: true, + } +} + +/// One reporting window of "which clock is cleaner", plus the domain sanity check. +#[derive(Default)] +pub(crate) struct PtsProvenance { + frames: u64, + with_hdr: u64, + /// Intervals between consecutive stamps, ns — one series per clock. THE pair the whole WP + /// exists to compare: if the compositor's is materially tighter than ours, its stamp is worth + /// adopting; if both are equally ragged, the compositor is composing irregularly and no + /// choice of stamp can fix it (risk R7). + hdr_intervals: Vec, + delivery_intervals: Vec, + /// `hdr − delivery` per frame. Expected to be huge and roughly CONSTANT (two clock origins); + /// its variance is the signal, and a wildly varying one means the header is not a per-frame + /// stamp at all. + offsets: Vec, + prev_hdr: Option, + prev_delivery: Option, + /// Frames that asked for the header stamp and were refused by the plausibility gate. + pub(crate) implausible: u64, +} + +/// A window's worth, in the units a log line wants. +pub(crate) struct PtsReport { + pub(crate) frames: u64, + pub(crate) with_hdr: u64, + /// Intervals the deviations were computed from. A MAD over eight samples and one over three + /// thousand deserve different amounts of belief, and the log line should not hide which it is. + pub(crate) samples: u64, + /// Median interval between deliveries — the empirical period. Derived rather than taken from + /// the negotiated refresh on purpose: a median is immune both to a wrong nominal and to the + /// occasional skipped tick, and a skipped tick is exactly what a fixed nominal would + /// mis-score as jitter. + pub(crate) period_us: i64, + /// Median absolute deviation of each clock's intervals about ITS OWN median — "how ragged is + /// this clock's cadence", and nothing else. Judging both against one shared centre sounds + /// tidier and is worse: it folds the period-estimation error, and any genuine rate difference + /// between two clock sources, into a number that is supposed to be about jitter. A perfectly + /// regular producer would then report several µs of dispersion it does not have. + pub(crate) hdr_mad_us: i64, + pub(crate) delivery_mad_us: i64, + pub(crate) offset_p50_ms: i64, + pub(crate) implausible: u64, +} + +impl PtsProvenance { + pub(crate) fn new() -> PtsProvenance { + PtsProvenance { + hdr_intervals: Vec::with_capacity(MAX_SAMPLES), + delivery_intervals: Vec::with_capacity(MAX_SAMPLES), + offsets: Vec::with_capacity(MAX_SAMPLES), + ..Default::default() + } + } + + /// Fold one frame in. `hdr_pts_ns` is `None` when the buffer carried no usable header, which + /// also breaks the header interval chain — an interval spanning a frame we could not stamp + /// would read as one clean long gap rather than the missing measurement it is. + pub(crate) fn observe(&mut self, hdr_pts_ns: Option, delivery_ns: u64) { + self.frames += 1; + if let Some(prev) = self.prev_delivery { + push_capped( + &mut self.delivery_intervals, + delivery_ns as i64 - prev as i64, + ); + } + self.prev_delivery = Some(delivery_ns); + + let Some(hdr) = hdr_pts_ns.filter(|&p| p > 0) else { + self.prev_hdr = None; + return; + }; + self.with_hdr += 1; + push_capped(&mut self.offsets, hdr - delivery_ns as i64); + if let Some(prev) = self.prev_hdr { + push_capped(&mut self.hdr_intervals, hdr - prev); + } + self.prev_hdr = Some(hdr); + } + + /// The window's answer, or `None` if too little arrived to say anything. + pub(crate) fn report(&mut self) -> Option { + if self.delivery_intervals.len() < 8 { + return None; + } + Some(PtsReport { + frames: self.frames, + with_hdr: self.with_hdr, + samples: self.delivery_intervals.len() as u64, + period_us: median(&mut self.delivery_intervals) / 1_000, + hdr_mad_us: mad(&mut self.hdr_intervals) / 1_000, + delivery_mad_us: mad(&mut self.delivery_intervals) / 1_000, + offset_p50_ms: median(&mut self.offsets) / 1_000_000, + implausible: self.implausible, + }) + } + + /// Start a fresh window. The previous stamps survive so the first interval of the new window + /// is a real measurement rather than a hole. + pub(crate) fn reset_window(&mut self) { + self.frames = 0; + self.with_hdr = 0; + self.implausible = 0; + self.hdr_intervals.clear(); + self.delivery_intervals.clear(); + self.offsets.clear(); + } +} + +fn push_capped(v: &mut Vec, x: i64) { + if v.len() < MAX_SAMPLES { + v.push(x); + } +} + +/// Median, in place. Empty reports 0 so a log line stays parseable (the caller has already +/// declined to report on a window this thin). +fn median(v: &mut [i64]) -> i64 { + if v.is_empty() { + return 0; + } + v.sort_unstable(); + v[v.len() / 2] +} + +/// Median absolute deviation about the series' own median — robust to the occasional skipped +/// tick, which a mean would let dominate and a fixed nominal would mis-score as jitter. +fn mad(v: &mut Vec) -> i64 { + if v.is_empty() { + return 0; + } + let centre = median(v); + for x in v.iter_mut() { + *x = (*x - centre).abs(); + } + median(v) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A monotonic clock a few days into an uptime, against a realtime clock in 2026 — the domain + /// gap the rebase exists to close, and the shape the plausibility gate must not mistake for a + /// bad stamp. + const MONO_BASE: i64 = 300_000 * 1_000_000_000; + const RT_BASE: u64 = 1_786_000_000 * 1_000_000_000; + const RT_MINUS_MONO: i64 = RT_BASE as i64 - MONO_BASE; + + #[test] + fn a_rebased_compositor_stamp_is_used_and_a_stale_one_is_not() { + // In domain and on time: adopted. + let w = wire_pts( + Some(MONO_BASE + 1_000_000), + RT_BASE + 1_200_000, + RT_MINUS_MONO, + ); + assert!(w.from_header); + assert_eq!( + w.pts_ns, + RT_BASE + 1_000_000, + "the compositor's own instant" + ); + + // Half a second stale — a header nobody refreshed. Fall back rather than put the stream's + // timing on a fiction. + let w = wire_pts(Some(MONO_BASE - 500_000_000), RT_BASE, RT_MINUS_MONO); + assert!(!w.from_header); + assert_eq!(w.pts_ns, RT_BASE); + + // Never stamped at all (0), and the no-header case: today's behaviour, unchanged. + assert!(!wire_pts(Some(0), RT_BASE, RT_MINUS_MONO).from_header); + assert_eq!(wire_pts(None, RT_BASE, RT_MINUS_MONO).pts_ns, RT_BASE); + } + + /// The wrong clock domain is the failure mode risk R3 names, and it must be *loud in the + /// numbers and silent in the stream*: every frame falls back, nothing is corrupted. + #[test] + fn a_raw_monotonic_stamp_never_reaches_the_wire() { + let w = wire_pts(Some(MONO_BASE), RT_BASE, 0); // rebase forgotten + assert!(!w.from_header); + assert_eq!(w.pts_ns, RT_BASE); + } + + const PERIOD: i64 = 8_333_333; // 120 Hz + + /// Deterministic LCG in ±spread around zero (no OS randomness in tests). Zero-mean noise, not + /// a short repeating cycle: a cycle's interval series has only a handful of distinct values + /// and its median lands on one of the jitter peaks rather than on the period — which is a + /// property of that harness, not of the statistic. + struct Lcg(u64); + impl Lcg { + fn noise(&mut self, spread_ns: i64) -> i64 { + self.0 = self + .0 + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + ((self.0 >> 33) as i64 % (2 * spread_ns)) - spread_ns + } + } + + /// The measurement the branch decision rests on: a compositor stamping a clean 120 Hz grid + /// whose buffers reach us raggedly must show a materially tighter MAD than our delivery + /// stamps. This is the field shape — arrival offsets wandering up to a full period. + #[test] + fn a_clean_producer_behind_a_jittery_delivery_is_visible() { + let mut p = PtsProvenance::new(); + let mut rng = Lcg(7); + for i in 0..600i64 { + let hdr = MONO_BASE + i * PERIOD; // the producer's own stamp is exact + let delivery = (RT_BASE as i64 + i * PERIOD + rng.noise(3_000_000)) as u64; + p.observe(Some(hdr), delivery); + } + let r = p.report().expect("600 frames is plenty"); + assert_eq!(r.frames, 600); + assert_eq!(r.with_hdr, 600); + assert!( + (r.period_us - PERIOD / 1_000).abs() <= 100, + "the empirical period must find the grid, got {} us", + r.period_us + ); + assert_eq!(r.hdr_mad_us, 0, "an exact producer has no deviation"); + assert!( + r.delivery_mad_us > 1_000, + "delivery jitter of milliseconds must show as milliseconds, got {} us", + r.delivery_mad_us + ); + // The domain check: a roughly CONSTANT offset is what "two clock origins" looks like — a + // varying one would mean the header is not a per-frame stamp at all. (The tolerance is + // the delivery jitter itself, which the offset carries by construction.) + let origins_ms = (MONO_BASE - RT_BASE as i64) / 1_000_000; + assert!( + (r.offset_p50_ms - origins_ms).abs() <= 5, + "offset p50 {} vs clock origins {origins_ms}", + r.offset_p50_ms + ); + } + + /// Risk R7 asserted: if the compositor composes irregularly rather than merely delivering + /// late, both clocks are equally ragged and the numbers say so — no stamp swap can help, and + /// the report must not flatter the header into looking like a cure. + #[test] + fn an_irregular_producer_is_not_flattered() { + let mut p = PtsProvenance::new(); + let mut rng = Lcg(11); + for i in 0..600i64 { + // One wobble, carried faithfully by both clocks: the compositor really did compose + // at that instant, and really did deliver it straight away. + let wobble = rng.noise(3_000_000); + p.observe( + Some(MONO_BASE + i * PERIOD + wobble), + (RT_BASE as i64 + i * PERIOD + wobble) as u64, + ); + } + let r = p.report().unwrap(); + assert_eq!( + r.hdr_mad_us, r.delivery_mad_us, + "an irregular producer must look exactly as bad through either clock" + ); + assert!( + r.hdr_mad_us > 1_000, + "…and both must show the wobble, got {} us", + r.hdr_mad_us + ); + } + + /// A producer that fills in no header at all still gets its delivery clock measured, and the + /// header series must not invent intervals across the frames it could not stamp. + #[test] + fn a_producer_without_headers_still_reports_its_delivery_clock() { + let mut p = PtsProvenance::new(); + for i in 0..40i64 { + p.observe(None, (RT_BASE as i64 + i * 8_333_333) as u64); + } + let r = p.report().unwrap(); + assert_eq!(r.with_hdr, 0); + assert_eq!(r.hdr_mad_us, 0, "no samples, not a clean clock"); + assert!((r.period_us - 8_333).abs() <= 1); + } + + /// A new window starts clean but NOT blind: the previous stamps survive the reset, so the + /// first interval after a report is a real measurement rather than a hole. Getting this wrong + /// silently drops one frame's interval every 30 s — invisible, and exactly the kind of slow + /// bias that makes two clocks look more alike than they are. + #[test] + fn a_reset_window_keeps_measuring_across_the_boundary() { + let mut p = PtsProvenance::new(); + let mut rng = Lcg(13); + for i in 0..40i64 { + p.observe( + Some(MONO_BASE + i * PERIOD), + (RT_BASE as i64 + i * PERIOD) as u64, + ); + } + p.implausible = 3; + let first = p.report().unwrap(); + assert_eq!( + first.implausible, 3, + "the window's fallbacks must be reported" + ); + + p.reset_window(); + for i in 40..80i64 { + let delivery = (RT_BASE as i64 + i * PERIOD + rng.noise(2_000_000)) as u64; + p.observe(Some(MONO_BASE + i * PERIOD), delivery); + } + let second = p.report().unwrap(); + assert_eq!(second.frames, 40, "counts start over"); + assert_eq!(second.implausible, 0, "…and so do the fallbacks"); + assert_eq!( + second.samples, 40, + "40 frames across a boundary yield 40 intervals, not 39 — the chain survived" + ); + assert_eq!(second.hdr_mad_us, 0, "the exact producer is still exact"); + } + + /// A window too thin to mean anything says nothing rather than reporting noise as a verdict. + #[test] + fn a_thin_window_reports_nothing() { + let mut p = PtsProvenance::new(); + for i in 0..4i64 { + p.observe(Some(MONO_BASE + i), RT_BASE + i as u64); + } + assert!(p.report().is_none()); + } +} -- 2.54.0 From 424621f1cefd00c92c995dcde4bc5a84b0e298c6 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 15 Aug 2026 13:44:34 +0200 Subject: [PATCH 04/10] fix(audio): what the Linux gate caught in the three preceding commits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Corrections to A1/A2/A3/B1 after running clippy `-D warnings` + the full test suites under the amd64 CI image. Kept separate from the client work rather than folded in, because two of them are worth reading. **The infill path was dead code.** `sent_any` gates synthesis on something having been sent — there is no continuity to protect and no wire clock to continue from before the first frame — and nothing ever set it. So WP-B1 would have compiled, shipped, and concealed exactly nothing. Found by re-reading the loop; clippy's `unused_mut` flagged the same thing from the other direction, which is the only reason it is a footnote rather than a field report. The loop has no thread-level test by design ("the thread stays plumbing"), so this is the class of bug that costs a release. **`max_gap_ms` meant two different things on the two platforms.** Linux was reporting the callback-to-callback DELTA; Windows sizes its holes from the WASAPI device position, so it reports MISSING AUDIO by construction. One number in one field name has to mean one thing, so Linux now subtracts the quantum it was legitimately handed. (The plan's own two statements about this were inconsistent — a >10 ms gap floor cannot score the "8 ms hole" its example asks for. Missing audio is the reading that makes both true.) The rest: a `&mut Vec` that wanted a slice, two `as i64` casts on a `tv_sec` that already is one, `extend(drain(..))` where `append` says it better — and two of my own tests asserting the wrong thing (the 8 ms hole above, and a replay that expected re-engagement without sitting out the backoff the replay itself had just armed, which is precisely the delay the hysteresis is supposed to impose). --- crates/pf-capture/src/linux/pipewire.rs | 2 +- crates/pf-capture/src/pts_provenance.rs | 2 +- crates/punktfunk-host/src/audio/capture_policy.rs | 14 +++++++++++--- crates/punktfunk-host/src/native/audio.rs | 15 ++++++++++----- crates/punktfunk-host/src/native/stream.rs | 5 ++++- 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/crates/pf-capture/src/linux/pipewire.rs b/crates/pf-capture/src/linux/pipewire.rs index aeed690d..3bf6b223 100644 --- a/crates/pf-capture/src/linux/pipewire.rs +++ b/crates/pf-capture/src/linux/pipewire.rs @@ -829,7 +829,7 @@ fn realtime_minus_monotonic_ns() -> i64 { if unsafe { libc::clock_gettime(libc::CLOCK_MONOTONIC, &mut ts) } != 0 { return 0; } - rt - (ts.tv_sec as i64 * 1_000_000_000 + ts.tv_nsec as i64) + rt - (ts.tv_sec * 1_000_000_000 + ts.tv_nsec) } fn consume_frame( diff --git a/crates/pf-capture/src/pts_provenance.rs b/crates/pf-capture/src/pts_provenance.rs index 56d0e13a..e8112d63 100644 --- a/crates/pf-capture/src/pts_provenance.rs +++ b/crates/pf-capture/src/pts_provenance.rs @@ -190,7 +190,7 @@ fn median(v: &mut [i64]) -> i64 { /// Median absolute deviation about the series' own median — robust to the occasional skipped /// tick, which a mean would let dominate and a fixed nominal would mis-score as jitter. -fn mad(v: &mut Vec) -> i64 { +fn mad(v: &mut [i64]) -> i64 { if v.is_empty() { return 0; } diff --git a/crates/punktfunk-host/src/audio/capture_policy.rs b/crates/punktfunk-host/src/audio/capture_policy.rs index c233a6d0..9e88e654 100644 --- a/crates/punktfunk-host/src/audio/capture_policy.rs +++ b/crates/punktfunk-host/src/audio/capture_policy.rs @@ -168,7 +168,13 @@ impl CaptureStats { let Some(delta) = since_last else { return }; if delta > (quantum * 2).max(GAP_FLOOR) { self.gaps += 1; - self.max_gap_us = self.max_gap_us.max(delta.as_micros() as u64); + // The MISSING audio, not the callback delta: one quantum of that delta is the buffer + // we were legitimately handed. Reporting the delta would inflate every gap by the + // quantum and — worse — mean something different from the Windows feed, which sizes + // its holes from the device position and so reports missing audio by construction. + self.max_gap_us = self + .max_gap_us + .max(delta.saturating_sub(quantum).as_micros() as u64); } } @@ -385,11 +391,13 @@ mod tests { one.observe_callback(None, Q); // first callback of the stream — nothing to compare to one.observe_callback(Some(Duration::from_secs(2)), Q); assert_eq!(one.gaps, 1); - assert_eq!(one.max_gap_ms(), 2_000); + // Two seconds between callbacks, one quantum of which was audio we were handed. + assert_eq!(one.max_gap_ms(), 2_000 - Q.as_millis() as u64); + // …versus three hundred 8 ms holes, each arriving as a 13 ms callback delta. let mut many = CaptureStats::default(); for _ in 0..300 { - many.observe_callback(Some(Duration::from_millis(8)), Q); + many.observe_callback(Some(Q + Duration::from_millis(8)), Q); } assert_eq!(many.gaps, 300); assert_eq!(many.max_gap_ms(), 8); diff --git a/crates/punktfunk-host/src/native/audio.rs b/crates/punktfunk-host/src/native/audio.rs index e5510e3c..356add00 100644 --- a/crates/punktfunk-host/src/native/audio.rs +++ b/crates/punktfunk-host/src/native/audio.rs @@ -255,10 +255,12 @@ pub(super) fn audio_thread( // Wake on whichever comes first: a capture chunk, or the moment the wire next has // something to say. Waiting only on capture is what made a hole cost more than the audio // it swallowed — see [`InfillPolicy`]. - let waited = if infill.exhausted() { - // Past the infill budget the host is not glitching, it is QUIET, and there is nothing - // to send until real audio returns. Block the way this loop always did rather than - // waking two hundred times a second to decide to stay silent. + let waited = if infill.exhausted() || !sent_any { + // Nothing is owed until real audio returns: either the infill budget is spent (the + // host is not glitching, it is QUIET) or nothing has ever been sent, so there is no + // continuity to hold. Block the way this loop always did rather than waking two + // hundred times a second to decide to stay silent — a session that starts on a quiet + // desktop would otherwise spin until the first sound. capturer.as_mut().unwrap().next_chunk() } else { let now = std::time::Instant::now(); @@ -330,7 +332,7 @@ pub(super) fn audio_thread( // leaving it for post-gap samples to complete: one frame carrying audio // from both sides of a hole is a click, and its pts is a lie about when // half of it was captured. - frame_buf.extend(acc.drain(..)); + frame_buf.append(&mut acc); frame_buf.resize(frame_len, 0.0); } crate::audio::capture_policy::Infill::Wait @@ -364,6 +366,9 @@ pub(super) fn audio_thread( prev_frame.extend_from_slice(opus); } seq = seq.wrapping_add(1); + // From here there is a continuity worth protecting, and `next_pts_ns` has a + // real anchor to continue from — both preconditions for synthesizing anything. + sent_any = true; } Err(e) => { opus_encode_errs += 1; diff --git a/crates/punktfunk-host/src/native/stream.rs b/crates/punktfunk-host/src/native/stream.rs index 3da1e911..11f3c0d2 100644 --- a/crates/punktfunk-host/src/native/stream.rs +++ b/crates/punktfunk-host/src/native/stream.rs @@ -5468,7 +5468,10 @@ mod tests { !c.fused, "flapping that never engaged must not blow the fuse" ); - for _ in 0..PhaseController::ENGAGE_COHERENT_REPORTS { + // The replay ends mid-backoff — a bad patch always does — so recovery costs that backoff + // plus the engage proof, ~15 s at the report cadence. That delay IS the trade: every + // second of it is spent in today's disengaged default, which adds no latency at all. + for _ in 0..PhaseController::REENGAGE_BACKOFF + PhaseController::ENGAGE_COHERENT_REPORTS { c.adjust(&report_at(COHERENT, ACTIONABLE_LEAD), SIM_P); } assert!( -- 2.54.0 From 7d9d7c3814a5a13aae30d3ffebeb10f1dec9e863 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 15 Aug 2026 13:44:36 +0200 Subject: [PATCH 05/10] fix(client): a packet drought drained the ring into a de-prime, because nothing arrived to reveal it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP-C1 of design/host-source-stutter-fixes.md, for the three Rust playback sites (Linux/PipeWire, Windows/WASAPI, Android/AAudio). The decode path already concealed a SEQ GAP: AudioGapTracker reports the packets missing before the one that arrived and libopus synthesizes each from the decoder's own state. But that only fires when a LATER packet arrives to reveal the gap. When the wire simply goes quiet — a delivery stall on a bunching Wi-Fi link, or a host whose capture stalled — nothing arrives to reveal anything: the ring drains to empty, the callback runs short, and the de-jitter policy de-primes and then re-primes a whole target's worth of fresh silence. The artifact is far longer than the audio actually missing. **The plan's premise for this WP does not hold, and the design changed because of it.** It specified concealment "on a pull that finds the ring short", i.e. in the audio callback. The decoder is not reachable from there at ANY of the four sites: decode runs on its own thread and reaches the callback only through a PCM channel plus AudioSyncCell. Nor is a second decoder instance an answer — PLC extrapolates from the last decoded frame, so a fresh one would conceal from empty state. So concealment lives on the decode thread, which owns exactly the state it needs, and the shape ends up mirroring the host's silence infill: wait one frame rather than blocking, and on a timeout with a draining ring synthesize one frame. DroughtConceal is the shared policy, bounded by JitterTuning::plc_max_ms() — twice the preset's own de-prime fuse, derived rather than a fifth field so it cannot drift from the thing it protects, and per-platform for free. Denominated in TIME, never in frames or callbacks: that is the recorded lesson from this very fuse, where a count gave an iPad a third of a Mac's slack. Concealment is gated on the ring actually running out — a drought a deep ring covers is inaudible, and synthesizing over it would insert audio the late packets are about to duplicate, pushing the stream permanently later for the drift shed to cut back out audibly. Two subtleties worth the reader's time. Frames concealed for a drought are subtracted from the loss concealment the seq path then asks for, or a packet genuinely lost INSIDE a covered drought is concealed twice. And the wait only shortens to one frame once something has decoded: before that there is no state to extrapolate from and no continuity to hold, so a session whose host never sends audio keeps the old long timeout instead of waking 200 times a second. `plc_ms=` joins the 10 s playback line at all three sites, riding AudioSyncCell from the decode thread to the two callbacks that emit it. Concealment that nobody can see is concealment that hides the bug it is covering: a healthy `underruns` bought with a climbing `plc_ms` is a link in trouble, not a link that is fine. Apple's leg is NOT included. Its ring is the Swift AudioRing and its decoder sits behind the C ABI, so the same fix needs a new punktfunk_connection entry point plus AudioRing/Stats changes — a separate piece of work, declared here rather than left to be discovered. --- clients/android/native/src/audio.rs | 48 +++++- crates/pf-client-core/src/audio.rs | 15 +- crates/pf-client-core/src/audio_wasapi.rs | 16 +- crates/pf-client-core/src/session.rs | 49 ++++++- crates/punktfunk-core/src/audio.rs | 169 ++++++++++++++++++++++ 5 files changed, 283 insertions(+), 14 deletions(-) diff --git a/clients/android/native/src/audio.rs b/clients/android/native/src/audio.rs index 1fa4333d..6f449839 100644 --- a/clients/android/native/src/audio.rs +++ b/clients/android/native/src/audio.rs @@ -851,6 +851,15 @@ fn decode_loop( let mut window_peak = 0f32; // loudest |sample| since the last log — tells a tone from silence let mut gaps = punktfunk_core::audio::AudioGapTracker::new(); let mut frame_samples = 0usize; // per-channel samples of the last decoded frame — the PLC unit + // WP-C1 — the drought half of concealment. The loop below already conceals a SEQ GAP, but only + // when a later packet arrives to reveal it; when the wire simply goes quiet — Wi-Fi power-save + // bunching, the shape this preset already runs deeper for — nothing arrives to reveal anything + // and the ring drains into an underrun and a de-prime whose re-prime is a longer artifact than + // the audio that was missing. + let mut drought = punktfunk_core::audio::DroughtConceal::new( + punktfunk_core::audio::JitterTuning::AAUDIO.plc_max_ms(), + ); + let mut last_packet = std::time::Instant::now(); // A/V sync (audio latency overhaul). This thread is the only place holding all three // ingredients at once: the packet's host capture `pts_ns`, the ring depth (via the sync cell) @@ -894,10 +903,15 @@ fn decode_loop( sync.set_target(av.desired_depth(depth)); av_offset_out.store(av.offset_ms() as i64, Ordering::Relaxed); } + last_packet = std::time::Instant::now(); + // Anything the drought path already covered is audio the stream now has; + // concealing it a second time here would insert samples it never carried and push + // everything after them later. + let already = drought.packet(); // Conceal lost packets (a seq gap) with libopus PLC before decoding the one that // arrived: empty input synthesizes `frame_samples` of interpolation per missing // packet — an inaudible fade instead of the click a hard gap makes in the ring. - for _ in 0..gaps.missing_before(pkt.seq) { + for _ in 0..gaps.missing_before(pkt.seq).saturating_sub(already) { let plc = frame_samples * channels; if plc == 0 { break; // no decoded frame yet to size the concealment from @@ -958,13 +972,17 @@ fn decode_loop( // the picture); 0 with sync off, or before it has a video reference. // Logged next to the depth because a deep ring on a jittery link is // correct and only the offset separates that from audio held late. + // `plc_ms` is concealment synthesized for packet droughts: a healthy + // `underruns` bought with a climbing `plc_ms` is a link in trouble, + // not a link that is fine. log::info!( - "audio: opus={count} pcm_frames={} underruns={} buffer_ms={} target_ms={} av_ms={} peak={window_peak:.3}", + "audio: opus={count} pcm_frames={} underruns={} buffer_ms={} target_ms={} av_ms={} plc_ms={} peak={window_peak:.3}", counters.pcm_written.load(Ordering::Relaxed), counters.underruns.load(Ordering::Relaxed), (depth / ms.max(1)) as u64, counters.target_ms.load(Ordering::Relaxed), av.offset_ms(), + drought.total_ms(), ); window_peak = 0.0; } @@ -972,7 +990,31 @@ fn decode_loop( Err(e) => log::debug!("audio: opus decode: {e}"), } } - Err(PunktfunkError::NoFrame) => {} // timeout + Err(PunktfunkError::NoFrame) => { + // Nothing on the wire. If the ring is draining with it, conceal from the decoder's + // own state — the same libopus interpolation the loss path uses, bounded by this + // preset's de-prime fuse so a genuinely dead stream is not papered over. ONE frame + // per tick, not a burst: this arm fires every 5 ms, which is the rate the callback + // drains at, so concealment keeps pace with playout instead of racing ahead of a + // depth reading it has already invalidated. `frame_samples` is 0 until something + // has decoded — there is no state to extrapolate from before then. + let depth_ms = (sync.depth() / ms.max(1)) as u32; + if frame_samples > 0 && drought.conceal(last_packet.elapsed(), depth_ms) { + let plc = frame_samples * channels; + if let Ok(samples) = dec.decode_float(&[], &mut pcm[..plc], false) { + let mut buf = free_rx + .try_recv() + .unwrap_or_else(|_| Vec::with_capacity(pcm_scratch)); + buf.clear(); + buf.extend_from_slice(&pcm[..samples * channels]); + match tx.try_send(buf) { + Ok(()) | Err(TrySendError::Full(_)) => {} + Err(TrySendError::Disconnected(_)) => return DecodeExit::Shutdown, + } + } + sync.publish_plc_ms(drought.total_ms()); + } + } Err(_) => return DecodeExit::SessionClosed, } } diff --git a/crates/pf-client-core/src/audio.rs b/crates/pf-client-core/src/audio.rs index 31e765a7..db96e3d6 100644 --- a/crates/pf-client-core/src/audio.rs +++ b/crates/pf-client-core/src/audio.rs @@ -174,6 +174,12 @@ impl Drop for AudioPlayer { } } +/// This backend's de-jitter tuning. Named once so the decode thread can read the same numbers the +/// callback runs on — its drought concealment is bounded by this preset's de-prime fuse, and the +/// two drifting apart is exactly how one platform quietly ends up with a third of another's slack. +pub(crate) const TUNING: punktfunk_core::audio::JitterTuning = + punktfunk_core::audio::JitterTuning::PIPEWIRE; + /// Producer-side state: incoming decoded PCM and the ring the process callback drains. struct PlayerData { rx: Receiver>, @@ -247,10 +253,7 @@ fn pw_thread( rx: pcm_rx, recycle: recycle_tx, ring: VecDeque::new(), - policy: punktfunk_core::audio::JitterPolicy::new( - punktfunk_core::audio::JitterTuning::PIPEWIRE, - channels as u8, - ), + policy: punktfunk_core::audio::JitterPolicy::new(TUNING, channels as u8), channels, underruns: 0, sheds: 0, @@ -361,6 +364,10 @@ fn pw_thread( target_ms = ud.policy.target_ms(), underruns = ud.underruns, drift_sheds = ud.sheds, + // Concealment must be visible next to the underruns it prevented: a + // healthy `underruns` bought with a climbing `plc_ms` is a link in + // trouble, not a link that is fine. + plc_ms = ud.sync.plc_ms(), "audio playback" ); } diff --git a/crates/pf-client-core/src/audio_wasapi.rs b/crates/pf-client-core/src/audio_wasapi.rs index 5d646ccc..5e535fa0 100644 --- a/crates/pf-client-core/src/audio_wasapi.rs +++ b/crates/pf-client-core/src/audio_wasapi.rs @@ -41,6 +41,13 @@ const CAPT_CHANNELS: usize = 2; /// halves the frame-fill share of mouth-to-ear latency vs the old 20 ms. const MIC_FRAME: usize = 480; +/// This backend's de-jitter tuning. Named once so the decode thread can read the same numbers the +/// render loop runs on — its drought concealment is bounded by this preset's de-prime fuse, and +/// the two drifting apart is exactly how one platform quietly ends up with a third of another's +/// slack. +pub(crate) const TUNING: punktfunk_core::audio::JitterTuning = + punktfunk_core::audio::JitterTuning::WASAPI; + /// A selectable WASAPI endpoint for the settings pickers. #[derive(Clone, Debug)] pub struct AudioDevice { @@ -305,10 +312,7 @@ fn render_thread( // 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 policy = punktfunk_core::audio::JitterPolicy::new(TUNING, channels); let mut out = Vec::new(); // per-quantum scratch, reused across iterations let (mut underruns, mut sheds, mut callbacks) = (0u64, 0u64, 0u64); @@ -367,6 +371,10 @@ fn render_thread( target_ms = policy.target_ms(), underruns, drift_sheds = sheds, + // Concealment must be visible next to the underruns it prevented: a healthy + // `underruns` bought with a climbing `plc_ms` is a link in trouble, not a + // link that is fine. + plc_ms = sync.plc_ms(), "audio playback" ); } diff --git a/crates/pf-client-core/src/session.rs b/crates/pf-client-core/src/session.rs index d4c3bfa2..309b2226 100644 --- a/crates/pf-client-core/src/session.rs +++ b/crates/pf-client-core/src/session.rs @@ -1686,8 +1686,25 @@ fn spawn_audio( if !av_sync_enabled { tracing::info!("A/V sync disabled by PUNKTFUNK_NO_AV_SYNC"); } + // WP-C1 — the drought half of concealment. The loop below already conceals a SEQ GAP, + // but only when a later packet arrives to reveal it; when the wire simply goes quiet + // nothing arrives to reveal anything, and the ring drains into an underrun and a + // de-prime whose re-prime is a longer artifact than the audio that was missing. + let mut drought = + punktfunk_core::audio::DroughtConceal::new(audio::TUNING.plc_max_ms()); + let mut last_packet = std::time::Instant::now(); while !stop.load(Ordering::SeqCst) { - match connector.next_audio(Duration::from_millis(100)) { + // Wait at most one frame WHILE there is a stream to protect: the drought decision + // has to be made on the wire's schedule, not whenever the next packet happens to + // turn up. Before anything has decoded there is no state to conceal from and + // nothing to conceal for, so a session whose host never sends audio keeps the old + // long timeout rather than waking two hundred times a second to do nothing. + let wait_ms = if frame_samples > 0 { + punktfunk_core::audio::FRAME_MS as u64 + } else { + 100 + }; + match connector.next_audio(Duration::from_millis(wait_ms)) { Ok(pkt) => { // Place this frame against the picture it belongs with, BEFORE it is // queued: `buffered_ahead` is everything that must still play first, so @@ -1710,10 +1727,15 @@ fn spawn_audio( sync_cell.set_target(av.desired_depth(depth)); av_offset_out.store(av.offset_ms() as i64, Ordering::Relaxed); } + last_packet = std::time::Instant::now(); + // Anything the drought path already covered is audio the stream now has; + // concealing it a second time here would insert samples it never carried + // and push everything after them later. + let already = drought.packet(); // Conceal lost packets (a seq gap) with libopus PLC before decoding the one // that arrived: empty input synthesizes `frame_samples` of interpolation per // missing packet — an inaudible fade instead of the click a hard gap makes. - for _ in 0..gaps.missing_before(pkt.seq) { + for _ in 0..gaps.missing_before(pkt.seq).saturating_sub(already) { let plc = frame_samples * channels as usize; if plc == 0 { break; // no decoded frame yet to size the concealment from @@ -1736,7 +1758,28 @@ fn spawn_audio( Err(e) => tracing::debug!(error = %e, "opus decode failed"), } } - Err(PunktfunkError::NoFrame) => {} + Err(PunktfunkError::NoFrame) => { + // Nothing on the wire. If the ring is draining with it, conceal from the + // decoder's own state — the same libopus interpolation the loss path uses, + // bounded by this backend's de-prime fuse so a genuinely dead stream is + // not papered over. `frame_samples` is 0 until something has decoded: + // there is no state to extrapolate from before then. + // + // ONE frame per tick, not a burst: this arm fires every `FRAME_MS`, which + // is exactly the rate the callback drains at, so concealment keeps pace + // with playout instead of racing ahead of a depth reading it has already + // invalidated. + let depth_ms = (sync_cell.depth() / per_ms) as u32; + if frame_samples > 0 && drought.conceal(last_packet.elapsed(), depth_ms) { + let plc = frame_samples * channels as usize; + if let Ok(samples) = dec.decode_float(&[], &mut pcm[..plc], false) { + let mut buf = player.take_buffer(); + buf.extend_from_slice(&pcm[..samples * channels as usize]); + player.push(buf); + } + sync_cell.publish_plc_ms(drought.total_ms()); + } + } Err(_) => break, // plane closed — the session is ending } } diff --git a/crates/punktfunk-core/src/audio.rs b/crates/punktfunk-core/src/audio.rs index b8e8ba2d..b049bb95 100644 --- a/crates/punktfunk-core/src/audio.rs +++ b/crates/punktfunk-core/src/audio.rs @@ -470,6 +470,15 @@ impl JitterTuning { deprime_ms: 60, }; + /// How long a packet DROUGHT may be concealed before the ring is allowed to underrun and the + /// de-prime hysteresis is allowed to run (WP-C1). Twice the de-prime window: long enough to + /// ride out the delivery stalls that de-prime rings today, short enough that a genuinely dead + /// stream is not papered over. DERIVED rather than a fifth field, so it cannot drift away from + /// the fuse it exists to protect — and per-platform for free, since `deprime_ms` already is. + pub const fn plc_max_ms(&self) -> u32 { + self.deprime_ms * 2 + } + /// 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 @@ -485,6 +494,79 @@ impl JitterTuning { } } +/// A drought must outlast ordinary arrival jitter before anything is synthesized for it: two +/// protocol frames, the same tolerance the host's capture-hole infill uses at the other end. +const DROUGHT_AFTER: std::time::Duration = std::time::Duration::from_millis(2 * FRAME_MS as u64); +/// …and the ring must actually be running out. A drought a deep ring can cover is not audible, +/// and concealing it would synthesize audio the late packets are about to duplicate — pushing the +/// whole stream later and handing the drift shed a mess to clean up audibly. +const DROUGHT_FLOOR_MS: u32 = 2 * FRAME_MS; + +/// Bounded concealment of a packet DROUGHT — the client-side twin of the host's capture-hole +/// infill (design/host-source-stutter-fixes.md, WP-C1). +/// +/// The decode path already conceals a SEQ GAP: [`AudioGapTracker`] reports the packets missing +/// before the one that arrived and libopus synthesizes each from the decoder's own state. But that +/// only fires when a LATER packet arrives to reveal the gap. When the wire simply goes quiet — a +/// delivery stall on a bunching Wi-Fi link, or a host whose capture stalled — nothing arrives to +/// reveal anything: the ring drains to empty, the callback runs short, and +/// [`JitterPolicy::note_read`] de-primes and then re-primes a whole target's worth of fresh +/// silence. The artifact is far longer than the audio actually missing, and this is the shape the +/// 2026-08-15 field session spent 3–16 % of its wall-clock in. +/// +/// So a drought that is draining the ring gets concealed too, from the same decoder state, for a +/// bounded time. Denominated in TIME, never in frames or callbacks: that is the recorded lesson +/// from the very fuse this protects, where a count gave an iPad a third of a Mac's slack for no +/// reason anyone intended. +/// +/// Time is passed IN, so the policy stays as syscall-free and deterministic as the rest of this +/// module. +pub struct DroughtConceal { + /// Concealed since the last real packet. + concealed_ms: u32, + max_ms: u32, + /// Concealed over the session — what the 10 s `plc_ms=` line reports. Concealment must be + /// visible: a policy that quietly papers over a failing link is a policy that hides the bug. + total_ms: u64, +} + +impl DroughtConceal { + pub fn new(max_ms: u32) -> DroughtConceal { + DroughtConceal { + concealed_ms: 0, + max_ms, + total_ms: 0, + } + } + + /// A packet arrived, ending any drought. Returns how many FRAMES were concealed for it, so the + /// caller can subtract them from the loss concealment [`AudioGapTracker`] is about to ask for: + /// packets genuinely lost inside a drought we already covered must not be covered twice, which + /// would insert audio the stream never had and push everything after it later. + pub fn packet(&mut self) -> u32 { + std::mem::take(&mut self.concealed_ms) / FRAME_MS + } + + /// Should one more frame be concealed? `depth_ms` is the playout ring as the callback last + /// saw it. + pub fn conceal(&mut self, since_last_packet: std::time::Duration, depth_ms: u32) -> bool { + if since_last_packet < DROUGHT_AFTER + || depth_ms > DROUGHT_FLOOR_MS + || self.concealed_ms >= self.max_ms + { + return false; + } + self.concealed_ms += FRAME_MS; + self.total_ms += FRAME_MS as u64; + true + } + + /// Concealment over the session, ms. + pub fn total_ms(&self) -> u64 { + self.total_ms + } +} + /// What one callback should do, from [`JitterPolicy::step`]. #[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] pub struct JitterStep { @@ -1066,6 +1148,11 @@ pub fn spa_positions(channels: u8) -> &'static [u32] { pub struct AudioSyncCell { depth: std::sync::atomic::AtomicUsize, target: std::sync::atomic::AtomicUsize, + /// Concealment the decode side has synthesized this session, ms — telemetry travelling the + /// same way the target does. It rides here because the counter is produced on the decode + /// thread and the 10 s playback line is emitted from the callback, and concealment that + /// nobody can see is concealment that hides the bug it is covering (WP-C1, risk R6). + plc_ms: std::sync::atomic::AtomicU64, } impl Default for AudioSyncCell { @@ -1073,6 +1160,7 @@ impl Default for AudioSyncCell { AudioSyncCell { depth: std::sync::atomic::AtomicUsize::new(0), target: std::sync::atomic::AtomicUsize::new(usize::MAX), + plc_ms: std::sync::atomic::AtomicU64::new(0), } } } @@ -1089,6 +1177,16 @@ impl AudioSyncCell { self.depth.load(std::sync::atomic::Ordering::Relaxed) } + /// Decode side: publish total concealment synthesized for packet droughts. + pub fn publish_plc_ms(&self, ms: u64) { + self.plc_ms.store(ms, std::sync::atomic::Ordering::Relaxed); + } + + /// Callback side: that total, for the periodic playback line. + pub fn plc_ms(&self) -> u64 { + self.plc_ms.load(std::sync::atomic::Ordering::Relaxed) + } + /// Decode side: ask the ring to aim for this depth (`None` = run unsynchronised). pub fn set_target(&self, target: Option) { self.target.store( @@ -1415,6 +1513,77 @@ mod tests { ); } + // ---- drought concealment (WP-C1) ----------------------------------------------------- + + /// Concealment is for a ring that is running OUT. A drought a deep ring can cover is + /// inaudible, and synthesizing over it would insert audio the late packets are about to + /// duplicate — the stream would then run permanently later and the drift shed would have to + /// cut it back out, audibly. + #[test] + fn a_drought_is_concealed_only_while_the_ring_is_running_out() { + let mut c = DroughtConceal::new(JitterTuning::PIPEWIRE.plc_max_ms()); + let stalled = DROUGHT_AFTER + std::time::Duration::from_millis(FRAME_MS as u64); + assert!( + !c.conceal(stalled, 40), + "a 40 ms ring covers this drought by itself" + ); + assert!(c.conceal(stalled, 0), "an empty ring does not"); + assert_eq!(c.total_ms(), FRAME_MS as u64); + } + + /// Ordinary arrival jitter is not a drought — this policy must be invisible until the wire + /// has genuinely stopped. + #[test] + fn ordinary_jitter_is_not_a_drought() { + let mut c = DroughtConceal::new(JitterTuning::AAUDIO.plc_max_ms()); + for _ in 0..1_000 { + assert!(!c.conceal(std::time::Duration::from_millis(FRAME_MS as u64), 0)); + } + assert_eq!(c.total_ms(), 0); + assert_eq!(c.packet(), 0); + } + + /// The window is bounded, and bounded in TIME — the whole reason `deprime_ms` stopped being a + /// callback count. Every preset must get exactly twice its own de-prime fuse, so no platform + /// silently gets a third of another's protection again. + #[test] + fn drought_concealment_is_bounded_at_twice_the_deprime_fuse() { + for t in [ + JitterTuning::PIPEWIRE, + JitterTuning::WASAPI, + JitterTuning::COREAUDIO, + JitterTuning::AAUDIO, + ] { + assert_eq!(t.plc_max_ms(), t.deprime_ms * 2); + let mut c = DroughtConceal::new(t.plc_max_ms()); + let mut ms = 0u32; + while c.conceal(DROUGHT_AFTER, 0) { + ms += FRAME_MS; + assert!(ms <= t.plc_max_ms(), "ran past the budget for {t:?}"); + } + assert_eq!(ms, t.plc_max_ms(), "must use exactly the budget for {t:?}"); + } + } + + /// Packets genuinely lost INSIDE a drought we already covered must not be covered a second + /// time by the loss path: doing both would insert audio the stream never carried and push + /// everything after it later. + #[test] + fn concealment_already_paid_for_is_not_paid_for_twice() { + let mut c = DroughtConceal::new(JitterTuning::WASAPI.plc_max_ms()); + for _ in 0..4 { + assert!(c.conceal(DROUGHT_AFTER, 0)); + } + let mut gaps = AudioGapTracker::new(); + gaps.missing_before(10); + // Four frames concealed; the wire then reveals six were lost. Only two are still owed. + let already = c.packet(); + assert_eq!(already, 4); + assert_eq!(gaps.missing_before(17).saturating_sub(already), 2); + // …and the next drought starts from a full budget. + assert!(c.conceal(DROUGHT_AFTER, 0)); + } + // ---- bitrate tiers ------------------------------------------------------------------- /// `Standard` must reproduce the historical table EXACTLY — that is what makes the tier -- 2.54.0 From e207d0d73e073cedec341d19ca677d2c843af15a Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 15 Aug 2026 13:58:19 +0200 Subject: [PATCH 06/10] feat(client/present): a clock that plays frames out on the source's cadence, not on their arrival MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP3 of design/presenter-cadence-rework-implementation-plan.md — the shared core, no client touched yet. Every client presents a frame the moment it is decoded, so the transport's jitter lands on the glass 1:1. On a host whose compositor delivers raggedly that is the whole defect: the 2026-08-15 Skynet log has KWin's screencast arriving 0.11-8.22 ms off its own grid — up to a full 120 Hz period — for 24 minutes straight, on a session with the bitrate pinned and zero packet loss. CadenceClock estimates the offset between the source clock and the present clock and hands back a due time on the source's own timeline plus a cushion sized to the measured jitter. It is type-2 (offset AND per-frame rate) because two free-running crystals produce a ramp and a proportional-only loop lags a ramp forever; `tracks_a_clock_ramp` asserts that against its own type-1 twin rather than against a threshold I picked. Fixed-point i64 throughout, so it runs identically on every client and in the offline harness. Three properties matter more than the loop itself: It smooths the OFFSET, never the timestamps. Due is `src_pts + offset + cushion`, so genuine variation in the source's own cadence — a variable-rate renderer, an irregular capture tick — passes straight through, and only the transport's contribution to `ready − pts` is filtered. Anything that made due times more evenly spaced than the source would be a bug; `preserves_source_cadence` is the test that says so, and the sim's second case refuses to reward flattening. It is domain-agnostic by construction. A constant offset between clock domains is absorbed by the offset estimator, so each client feeds `ready_ns` and reads `due_ns` in ONE domain with no conversion anywhere in the path — `domain_offset_is_absorbed` shifts a whole trace by 987 seconds and asserts every due time moves by exactly that and the spacings not at all. Suspend/resume breaks the constant, which is what `reset()` is for. A late frame's due time is returned in the PAST, unclamped. Clamping it to `ready_ns` would quietly turn every late frame into a fresh anchor — which is precisely arrival-driven presentation, the thing this exists to stop being. The offline sim (§2.4) imports the real type rather than paraphrasing it, which is R7 and the recorded phase-lock v3 lesson: a Python cross-check there inherited a mis-derived constant and duly "confirmed" a non-bug. Replaying ±6 ms of arrival jitter — the field shape — through both rules onto a synthetic 120 Hz panel: **294‰ judder arrival-driven, 24‰ cadence-driven.** ⚠ The tuning constants are PROVISIONAL and labelled as such. The plan asks for them to be fitted to recorded traces (its spike S2); S2 was never run, and the 2026-08-05 baseline records that omission itself. These are derived from first principles and the first real trace should replace them. `note_off_cadence` is in the API from day one (risk R2) for the frames whose stamps do not lie on the source timeline — a repeat the host anchored at submit, or one its plausibility gate replaced with "now". Folding those in would drag the estimate toward "now" exactly when the stream is idle and the estimate matters most. --- crates/punktfunk-core/src/phase.rs | 585 +++++++++++++++++++++++++++++ 1 file changed, 585 insertions(+) diff --git a/crates/punktfunk-core/src/phase.rs b/crates/punktfunk-core/src/phase.rs index a66f6f1d..17a0ff9f 100644 --- a/crates/punktfunk-core/src/phase.rs +++ b/crates/punktfunk-core/src/phase.rs @@ -129,6 +129,261 @@ pub fn circular_latch(samples_us: &[u64], period_ns: i64) -> Option<(u64, u16)> Some((mean_ns, (r * 1000.0) as u16)) } +// ---- Source-timestamp playout (design/presenter-cadence-rework.md, WP3) -------------------- + +/// Tuning for one cadence loop. Gains are SHIFT COUNTS — the loop is fixed-point i64 throughout, +/// so it runs identically on every client and in the offline harness, and carries no float into a +/// present path. +/// +/// ⚠ **These values are provisional, and saying so is part of the design.** The plan asks for +/// constants fitted to recorded `(src_pts, received, decoded)` traces (its spike S2), and S2 was +/// never run — the 2026-08-05 baseline records that omission itself. What is here is derived from +/// first principles (a proportional time constant of tens of frames, an integral an order slower, +/// a cushion of a few mean-absolute-deviations) and is honest about being a starting point. The +/// first real trace should replace them, with the trace named beside each. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CadenceTuning { + /// Proportional gain on the offset estimate: `1 >> offset_shift` of the residual per frame. + pub offset_shift: u8, + /// Integral gain on the per-frame rate (skew) term: `1 >> skew_shift`. + pub skew_shift: u8, + /// EMA weight for the residual mean-absolute-deviation. + pub jitter_shift: u8, + /// Per-sample residual clamp — one outlier must not yank the estimate. + pub error_clamp_ns: i64, + /// Cushion = `mad * cushion_num / cushion_den`, clamped to + /// `[cushion_floor_ns, frame_interval_ns]`. + pub cushion_num: u16, + pub cushion_den: u16, + pub cushion_floor_ns: i64, + /// Source-timestamp gap beyond which the loop re-anchors instead of tracking. + pub reanchor_gap_ns: i64, +} + +impl CadenceTuning { + /// For callers that snap the due time onto a display grid afterwards: the snap-up itself + /// carries roughly half a refresh of implicit slack, so the cushion can be small. + pub const fn snapping() -> CadenceTuning { + CadenceTuning { + offset_shift: 5, + skew_shift: 10, + jitter_shift: 5, + error_clamp_ns: 20_000_000, + cushion_num: 2, + cushion_den: 1, + cushion_floor_ns: 500_000, + reanchor_gap_ns: 500_000_000, + } + } + + /// For callers presenting at the due time directly (VRR, direct scanout): no implicit slack, + /// so the cushion must cover more of the distribution on its own. + pub const fn free_running() -> CadenceTuning { + CadenceTuning { + cushion_num: 3, + cushion_floor_ns: 2_000_000, + ..CadenceTuning::snapping() + } + } +} + +/// Loop health for the 1 Hz line — the numbers that say whether the cushion is doing its job. +/// +/// Residual PERCENTILES are deliberately absent: this type is allocation-free and holds no +/// histogram, and the client stat paths (the judder metric) are where distributions belong. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct CadenceHealth { + /// Frames folded since the last [`CadenceClock::reset`]. + pub frames: u64, + /// …of which the due time was already past when the frame became presentable. The direct + /// signal that the cushion is too small. + pub late: u64, + /// Times the loop gave up tracking and re-anchored (gap, regression, or explicit reset). + pub reanchors: u64, + pub offset_ns: i64, + pub skew_ns: i64, + pub jitter_ns: i64, + pub cushion_ns: i64, +} + +/// Plays frames out on the SOURCE's cadence instead of on their arrival instant. +/// +/// The defect it exists for: every client presents a frame as soon as it is decoded, so the +/// transport's jitter — and, on a host whose compositor delivers raggedly, the compositor's — +/// lands on the glass 1:1. The loop estimates the offset between the source clock and the present +/// clock, and hands back a due time on the source's own timeline plus a cushion sized to the +/// measured jitter. +/// +/// **Type-2 on purpose.** It tracks offset *and* per-frame rate, because two free-running crystals +/// produce a ramp and a proportional-only loop lags a ramp forever. +/// +/// **It smooths the offset, never the timestamps.** The due time is `src_pts + offset + cushion`, +/// so genuine variation in the source's own cadence — a variable-rate renderer, an irregular +/// capture tick — passes straight through. Only the transport's contribution to `ready − pts` is +/// filtered. Any change that makes due times more evenly spaced than the source is a bug, not an +/// improvement; `preserves_source_cadence` is the test that says so. +/// +/// **Domain-agnostic by construction.** A constant offset between clock domains (monotonic vs +/// realtime) is absorbed by the offset estimator, so a caller feeds `ready_ns` and reads `due_ns` +/// in ONE domain and needs no clock conversion anywhere in this path. Suspend/resume breaks the +/// constant — that is a discontinuity, and [`reset`](Self::reset) covers it. +/// +/// Prior art is ordinary and old: MPEG-2 TS PCR recovery and RTP playout scheduling (RFC 3550 +/// §6.4.1 carries the jitter estimator this MAD mirrors). +#[derive(Debug, Clone)] +pub struct CadenceClock { + tuning: CadenceTuning, + /// `ready − src_pts`, smoothed. Absorbs the clock-domain constant. + offset_ns: i64, + /// Per-frame drift of that offset — the integral term. + skew_ns: i64, + /// EMA of |residual|, the cushion's input. + mad_ns: i64, + /// `None` until the first sample anchors the loop. + last_pts_ns: Option, + /// Last frame interval seen, so [`cushion_ns`](Self::cushion_ns) can apply its ceiling. + frame_interval_ns: i64, + health: CadenceHealth, +} + +impl CadenceClock { + pub fn new(tuning: CadenceTuning) -> CadenceClock { + CadenceClock { + tuning, + offset_ns: 0, + skew_ns: 0, + mad_ns: 0, + last_pts_ns: None, + frame_interval_ns: 0, + health: CadenceHealth::default(), + } + } + + /// Force a re-anchor on the next sample. Call on every discontinuity the client already knows + /// about: reanchor, codec rebuild, surface recreate, jump-to-live, resume. + pub fn reset(&mut self) { + self.last_pts_ns = None; + self.skew_ns = 0; + // `mad_ns` deliberately SURVIVES. It describes the link, not the stream, and a cushion + // that collapsed to its floor at every rebuild would spend the next few hundred frames + // presenting late — the exact failure the cushion exists to prevent. + } + + /// Fold one presentable frame and return when it is due, in the present clock domain. + /// + /// `ready_ns` is when the frame became presentable; `frame_interval_ns` is the nominal source + /// interval and the cushion's ceiling. + /// + /// The result **may be earlier than `ready_ns`** — that is a late frame, and the caller's + /// contract is "already due ⇒ present at the next opportunity", never "drag the grid back to + /// now". Clamping here would quietly turn every late frame into a fresh anchor. + pub fn due_ns(&mut self, src_pts_ns: u64, ready_ns: i64, frame_interval_ns: i64) -> i64 { + self.frame_interval_ns = frame_interval_ns; + self.health.frames += 1; + let pts = src_pts_ns as i64; + let raw = ready_ns - pts; + + let anchored = match self.last_pts_ns { + // Source time going BACKWARDS, or a gap so long the estimate cannot be trusted to + // have tracked across it: re-anchor rather than slew for seconds. + Some(last) + if src_pts_ns < last || src_pts_ns - last > self.tuning.reanchor_gap_ns as u64 => + { + false + } + Some(_) => true, + None => false, + }; + if anchored { + // Advance the estimate one frame on the rate term, then correct it by a bounded + // fraction of what the new sample says. + self.offset_ns = self.offset_ns.saturating_add(self.skew_ns); + let err = (raw - self.offset_ns) + .clamp(-self.tuning.error_clamp_ns, self.tuning.error_clamp_ns); + self.offset_ns = self + .offset_ns + .saturating_add(shr_toward_zero(err, self.tuning.offset_shift)); + self.skew_ns = self + .skew_ns + .saturating_add(shr_toward_zero(err, self.tuning.skew_shift)); + let dev = err.abs() - self.mad_ns; + self.mad_ns += shr_toward_zero(dev, self.tuning.jitter_shift); + } else { + self.offset_ns = raw; + self.skew_ns = 0; + self.health.reanchors += 1; + } + self.last_pts_ns = Some(src_pts_ns); + + let due = pts + .saturating_add(self.offset_ns) + .saturating_add(self.cushion_ns()); + if due < ready_ns { + self.health.late += 1; + } + self.publish(); + due + } + + /// A frame whose timestamp is not on the source cadence — a repeat the host re-anchored at + /// submit, or a stamp its plausibility gate replaced with "now". Those samples do not lie on + /// the source's timeline, and folding them in would drag the offset estimate toward "now" + /// exactly when the stream is idle and the estimate matters most. + /// + /// Returns a due time from the CURRENT estimate, leaving offset, skew and jitter untouched: + /// the frame is simply due once it is ready, cushioned like any other. + pub fn note_off_cadence(&mut self, ready_ns: i64, frame_interval_ns: i64) -> i64 { + self.frame_interval_ns = frame_interval_ns; + ready_ns.saturating_add(self.cushion_ns()) + } + + pub fn jitter_ns(&self) -> i64 { + self.mad_ns + } + + /// How far past the estimate a frame is held, to absorb the measured jitter. + /// + /// The one-frame-interval ceiling is an INVARIANT, not a tunable: a cushion past a whole frame + /// buys latency for smoothness the source cannot supply, and at that point the honest fix is a + /// deeper buffer the user asked for, not a loop quietly holding frames. + pub fn cushion_ns(&self) -> i64 { + let den = self.tuning.cushion_den.max(1) as i64; + let want = self.mad_ns.saturating_mul(self.tuning.cushion_num as i64) / den; + let ceiling = if self.frame_interval_ns > 0 { + self.frame_interval_ns + } else { + i64::MAX + }; + want.clamp(self.tuning.cushion_floor_ns.min(ceiling), ceiling) + } + + pub fn health(&self) -> CadenceHealth { + let mut h = self.health; + h.offset_ns = self.offset_ns; + h.skew_ns = self.skew_ns; + h.jitter_ns = self.mad_ns; + h.cushion_ns = self.cushion_ns(); + h + } + + fn publish(&mut self) { + self.health.offset_ns = self.offset_ns; + self.health.skew_ns = self.skew_ns; + self.health.jitter_ns = self.mad_ns; + } +} + +/// Arithmetic shift that rounds toward ZERO, so a negative residual is damped by exactly as much +/// as its positive twin. A plain `>>` rounds toward −∞, which biases a loop that spends its whole +/// life within a few nanoseconds of zero error. +const fn shr_toward_zero(v: i64, shift: u8) -> i64 { + if v < 0 { + -((-v) >> shift) + } else { + v >> shift + } +} + #[cfg(test)] mod tests { use super::*; @@ -136,6 +391,336 @@ mod tests { const P: i64 = 8_333_333; // 120 Hz in ns const P_US: u64 = 8_333; // …and in µs, the sample unit + // ---- CadenceClock (design/presenter-cadence-rework-implementation-plan.md §2.3) -------- + + /// A source stamping realtime, played out by a client whose present clock is monotonic and + /// therefore a whole different era. The loop must never need to be told about this. + const PTS0: u64 = 1_786_000_000_000_000_000; + const DOMAIN: i64 = -1_785_000_000_000_000_000; + /// Transport + decode: what `ready − pts` sits at once the domain is taken out. + const DELAY: i64 = 12_000_000; + + /// Deterministic LCG in ±spread around zero — no OS randomness in tests. + struct Lcg(u64); + impl Lcg { + fn noise(&mut self, spread_ns: i64) -> i64 { + self.0 = self + .0 + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + if spread_ns == 0 { + return 0; + } + ((self.0 >> 33) as i64 % (2 * spread_ns)) - spread_ns + } + } + + fn pts_at(k: i64) -> u64 { + (PTS0 as i64 + k * P) as u64 + } + + /// Run `n` frames of a well-behaved 120 Hz source and hand back the clock. + fn settled(n: i64, spread_ns: i64) -> CadenceClock { + let mut c = CadenceClock::new(CadenceTuning::snapping()); + let mut rng = Lcg(7); + for k in 0..n { + let ready = pts_at(k) as i64 + DOMAIN + DELAY + rng.noise(spread_ns); + c.due_ns(pts_at(k), ready, P); + } + c + } + + #[test] + fn settles_from_cold() { + let c = settled(400, 1_000_000); + let err = c.health().offset_ns - (DOMAIN + DELAY); + assert!( + err.abs() < 500_000, + "offset must converge on the true transport delay, off by {err} ns" + ); + assert_eq!(c.health().reanchors, 1, "only the cold start anchors"); + } + + /// The type-2 property, and the reason the loop carries a rate term at all: two free-running + /// crystals produce a RAMP, and a proportional-only loop lags a ramp forever. Asserted against + /// its own type-1 twin so the difference is the measurement, not a threshold I chose. + #[test] + fn tracks_a_clock_ramp() { + const RAMP: i64 = 400; // ns per frame ≈ 48 ppm, an ordinary crystal pair + let run = |tuning: CadenceTuning| -> i64 { + let mut c = CadenceClock::new(tuning); + let mut last_err = 0; + for k in 0..4_000i64 { + let ready = pts_at(k) as i64 + DOMAIN + DELAY + k * RAMP; + c.due_ns(pts_at(k), ready, P); + last_err = (ready - pts_at(k) as i64) - c.health().offset_ns; + } + last_err.abs() + }; + let type2 = run(CadenceTuning::snapping()); + // The same loop with its integral gain switched off: a shift this large truncates every + // residual to zero, which is exactly "proportional only". + let type1 = run(CadenceTuning { + skew_shift: 63, + ..CadenceTuning::snapping() + }); + assert!( + type2 * 4 < type1, + "a rate term must beat proportional-only on a ramp: {type2} ns vs {type1} ns" + ); + assert!(type2 < 3_000, "steady-state ramp error {type2} ns"); + } + + #[test] + fn rejects_a_single_outlier() { + let mut c = settled(400, 200_000); + let before = c.health().offset_ns; + // One frame arrives half a second late — a stall, not a new operating point. + let k = 400; + c.due_ns( + pts_at(k), + pts_at(k) as i64 + DOMAIN + DELAY + 500_000_000, + P, + ); + let moved = (c.health().offset_ns - before).abs(); + // The clamped correction, plus the one frame of rate the loop advances by regardless — + // that advance is the estimate doing its job, not the outlier moving it. + let t = CadenceTuning::snapping(); + let bound = (t.error_clamp_ns >> t.offset_shift) + c.health().skew_ns.abs(); + assert!( + moved <= bound, + "one outlier moved the estimate {moved} ns, past the clamp's {bound} ns" + ); + } + + #[test] + fn reanchors_on_a_gap() { + let mut c = settled(400, 200_000); + let anchors = c.health().reanchors; + // The stream was paused for two seconds; the estimate cannot have tracked across that. + let far = pts_at(400) + 2_000_000_000; + let ready = far as i64 + DOMAIN + DELAY + 4_000_000; + c.due_ns(far, ready, P); + assert_eq!(c.health().reanchors, anchors + 1); + assert_eq!( + c.health().offset_ns, + ready - far as i64, + "a re-anchor adopts the new sample outright rather than slewing to it" + ); + } + + #[test] + fn reanchors_on_regression() { + let mut c = settled(400, 200_000); + let anchors = c.health().reanchors; + let back = pts_at(200); // source timestamps went backwards + c.due_ns(back, back as i64 + DOMAIN + DELAY, P); + assert_eq!(c.health().reanchors, anchors + 1); + } + + /// A due time in the past is returned AS IS. Clamping it to `ready_ns` would quietly turn + /// every late frame into a fresh anchor, which is how an arrival-driven presenter behaves — + /// the thing this clock exists to stop being. + #[test] + fn late_frame_returns_past_due() { + let mut c = settled(400, 200_000); + let k = 400; + let ready = pts_at(k) as i64 + DOMAIN + DELAY + 30_000_000; // 30 ms late + let due = c.due_ns(pts_at(k), ready, P); + assert!( + due < ready, + "a frame that arrived 30 ms late must read as already due" + ); + assert_eq!(c.health().late, 1); + } + + #[test] + fn off_cadence_does_not_move_the_loop() { + let mut c = settled(400, 500_000); + let before = c.health(); + let due = c.note_off_cadence(1_000_000, P); + let after = c.health(); + assert_eq!(before.offset_ns, after.offset_ns); + assert_eq!(before.skew_ns, after.skew_ns); + assert_eq!(before.jitter_ns, after.jitter_ns); + assert_eq!( + before.frames, after.frames, + "and it is not a cadence sample" + ); + assert_eq!(due, 1_000_000 + c.cushion_ns()); + } + + /// One domain in, same domain out: shifting the whole present-side trace by an arbitrary + /// constant must change every due time by exactly that constant and nothing else. This is + /// what lets each client feed its own clock without a conversion in the path. + #[test] + fn domain_offset_is_absorbed() { + const SHIFT: i64 = 987_654_321_000; + let run = |extra: i64| -> Vec { + let mut c = CadenceClock::new(CadenceTuning::snapping()); + let mut rng = Lcg(11); + (0..300i64) + .map(|k| { + let ready = pts_at(k) as i64 + DOMAIN + DELAY + extra + rng.noise(2_000_000); + c.due_ns(pts_at(k), ready, P) + }) + .collect() + }; + let a = run(0); + let b = run(SHIFT); + for (i, (x, y)) in a.iter().zip(b.iter()).enumerate() { + assert_eq!(y - x, SHIFT, "frame {i} shifted by {} not {SHIFT}", y - x); + } + } + + /// The invariant that separates this from a metronome: a source that genuinely runs at an + /// irregular rate is REPRODUCED, not evened out. Anything that made these due spacings more + /// uniform than the source's own would be a bug. + #[test] + fn preserves_source_cadence() { + let mut c = CadenceClock::new(CadenceTuning::snapping()); + // A deliberately lumpy source: alternating short and long frames. + let spacings: Vec = (0..300) + .map(|k| if k % 2 == 0 { P / 2 } else { P * 3 / 2 }) + .collect(); + let mut pts = PTS0; + let mut dues = Vec::new(); + let mut ptss = Vec::new(); + let mut rng = Lcg(13); + for &s in &spacings { + pts = (pts as i64 + s) as u64; + let ready = pts as i64 + DOMAIN + DELAY + rng.noise(500_000); + ptss.push(pts as i64); + dues.push(c.due_ns(pts, ready, P)); + } + // Compare the back half, once the loop has settled. + for i in 200..dues.len() { + let d_due = dues[i] - dues[i - 1]; + let d_pts = ptss[i] - ptss[i - 1]; + assert!( + (d_due - d_pts).abs() < 200_000, + "due spacing {d_due} must follow the source's {d_pts}" + ); + } + } + + #[test] + fn cushion_respects_ceiling() { + let mut c = CadenceClock::new(CadenceTuning::free_running()); + let mut rng = Lcg(17); + // Jitter far wider than a frame — the cushion must still never exceed one interval. + for k in 0..500i64 { + let ready = pts_at(k) as i64 + DOMAIN + DELAY + rng.noise(40_000_000); + c.due_ns(pts_at(k), ready, P); + assert!( + c.cushion_ns() <= P, + "cushion {} ns exceeded the frame interval", + c.cushion_ns() + ); + } + assert!( + c.jitter_ns() > P, + "the harness must actually have stressed it" + ); + } + + // ---- Offline sim (§2.4): the real clock, replayed, against today's rule ---------------- + // + // R7, the phase-lock v3 lesson: the harness imports the REAL type. A paraphrase once + // "confirmed" a non-bug and cost a session. + + /// Judder as WP1 defines it: round each consecutive present spacing to whole panel periods, + /// then report the ‰ of intervals that are not the modal count. + fn judder_permille(presents: &[i64], panel_ns: i64) -> u32 { + let counts: Vec = presents + .windows(2) + .map(|w| (w[1] - w[0] + panel_ns / 2) / panel_ns) + .collect(); + if counts.is_empty() { + return 0; + } + let mode = *counts + .iter() + .max_by_key(|c| counts.iter().filter(|x| x == c).count()) + .unwrap(); + let off = counts.iter().filter(|c| **c != mode).count(); + (off * 1000 / counts.len()) as u32 + } + + /// Turn a series of target instants into the refreshes a frame actually reaches glass on. + /// + /// Newest-wins, which is what both presenters really do: a frame whose slot is already + /// claimed REPLACES the one aimed there rather than being delayed to the next refresh. + /// Delaying it instead would ratchet — one clamp puts the sequence permanently ahead of its + /// own targets and every later frame clamps too, producing a flawless metronome out of an + /// arbitrarily jittery input, and scoring zero judder for both rules. + fn present_slots(targets: &[i64], panel_ns: i64) -> Vec { + let mut out: Vec = Vec::new(); + for &t in targets { + let slot = (t + panel_ns - 1) / panel_ns * panel_ns; + if out.last().is_none_or(|&last| slot > last) { + out.push(slot); + } + } + out + } + + #[test] + fn the_clock_beats_arrival_presentation_on_a_jittery_link() { + // ±6 ms — the 2026-08-15 field shape, where KWin's screencast arrival offsets ran + // 0.11–8.22 ms against an 8.33 ms period. Jitter much narrower than the panel period is + // the case snapping absorbs on its own (and, at a lucky grid phase, absorbs entirely), + // which is precisely why the reported host is the one worth simulating. + const JITTER: i64 = 6_000_000; + let mut c = CadenceClock::new(CadenceTuning::snapping()); + let mut rng = Lcg(23); + let (mut arrival, mut cadence) = (Vec::new(), Vec::new()); + for k in 0..1_200i64 { + let ready = pts_at(k) as i64 + DOMAIN + DELAY + rng.noise(JITTER); + let due = c.due_ns(pts_at(k), ready, P); + if k > 200 { + // Today: aim at the frame the moment it is decoded. With the clock: aim at its + // due time, and never before the frame exists. + arrival.push(ready); + cadence.push(ready.max(due)); + } + } + let ja = judder_permille(&present_slots(&arrival, P), P); + let jc = judder_permille(&present_slots(&cadence, P), P); + // The expected-gain figure §2.4 asks this harness to produce. `cargo test -- --nocapture`. + println!( + "sim: arrival {ja}‰ → cadence {jc}‰ (cushion {} ns)", + c.cushion_ns() + ); + assert!( + jc < ja / 2, + "source-timestamp playout must materially beat arrival: {jc}‰ vs {ja}‰" + ); + } + + /// …and it must NOT "win" by flattening a source that is genuinely uneven — the same harness, + /// a variable-rate source, and the clock is expected to reproduce its lumpiness. + #[test] + fn the_sim_does_not_reward_flattening_a_variable_source() { + let mut c = CadenceClock::new(CadenceTuning::snapping()); + let mut rng = Lcg(29); + let mut pts = PTS0; + let (mut dues, mut ptss) = (Vec::new(), Vec::new()); + for k in 0..600i64 { + // A renderer alternating 60 and 120 fps work — real, and not a defect. + pts = (pts as i64 + if k % 3 == 0 { 2 * P } else { P }) as u64; + let ready = pts as i64 + DOMAIN + DELAY + rng.noise(500_000); + ptss.push(pts as i64); + dues.push(c.due_ns(pts, ready, P)); + } + let jd = judder_permille(&dues[300..], P); + let jp = judder_permille(&ptss[300..], P); + assert_eq!( + jd, jp, + "the due-time cadence must score exactly what the source's own does" + ); + } + #[test] fn identical_samples_are_fully_coherent() { let (mean, coh) = circular_latch(&[4_000; 16], P).unwrap(); -- 2.54.0 From ff513a18e1e1a348c591c8f4b1567239e5cad6c9 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 15 Aug 2026 14:08:31 +0200 Subject: [PATCH 07/10] fix(apple): the fourth client had no way to reach the one decoder that could conceal a drought MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes WP-C1 of design/host-source-stutter-fixes.md. The three Rust clients conceal a packet drought on their decode thread; Apple could not, and the reason is structural rather than an oversight: its playout ring is Swift and its Opus decoder lives behind the C ABI, so the drain thread had nothing to call. Standing up a second decoder Swift-side would not have worked either — PLC extrapolates from the LAST DECODED FRAME, so a fresh decoder conceals from empty state. So the ABI grows one function. `punktfunk_connection_audio_plc` synthesizes a single frame from the connection's own live decoder, returning no-frame when nothing has decoded yet (there is no state to extrapolate from) and when libopus declines to interpolate — a timeout's answer, not an error. Frames it returns carry seq and pts of zero, because concealed audio was never on the wire and must not reach an A/V-sync observation; the drain loop skips `av.observe` for them. The double-conceal defence had to move. On the Rust clients the decode loop subtracts frames it already concealed from what AudioGapTracker then asks for; Swift cannot, because the tracker is behind the ABI. So `AudioPcmState` counts its own concealment and `decode_packet` does the subtraction — same invariant, enforced one layer lower, with a Rust test standing in for the Swift half. Swift gets the policy port beside `AudioRing`'s existing one, in the same shape and for the same reason: one budget, `deprimeMS * 2`, denominated in time and not in callbacks. `plc_ms=` joins the 10 s line as it did on the other three. ABI_VERSION 22 → 23. ⚠ That number is the one thing here worth a second look: 21 was claimed twice by concurrent worktrees once already, and 23 is correct only against what is visible from this branch. --- .../PunktfunkKit/Audio/AudioRing.swift | 93 +++++++++- .../PunktfunkKit/Audio/SessionAudio.swift | 57 ++++++- .../Connection/PunktfunkConnection.swift | 40 +++++ .../AudioRingDriftTests.swift | 112 ++++++++++++ crates/punktfunk-core/src/abi.rs | 161 +++++++++++++++++- crates/punktfunk-core/src/lib.rs | 14 +- include/punktfunk_core.h | 54 +++++- 7 files changed, 519 insertions(+), 12 deletions(-) diff --git a/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift b/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift index 5400e3c1..f64f1d22 100644 --- a/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift +++ b/clients/apple/Sources/PunktfunkKit/Audio/AudioRing.swift @@ -48,11 +48,17 @@ final class AudioRing: @unchecked Sendable { /// gaps per 10 minutes at a 5 ms quantum, against 3 at 8 ms and 1 at 16 ms on an identical /// link. Mirrors `JitterTuning::COREAUDIO.deprime_ms`. private static let deprimeMS = 60 + /// How long a packet DROUGHT may be concealed (`DroughtConceal`) before this ring is allowed + /// to underrun and the hysteresis above is allowed to run: twice that window — long enough to + /// ride out the delivery stalls that de-prime rings today, short enough that a genuinely dead + /// stream is not papered over. DERIVED from the fuse rather than written out, so it cannot + /// drift away from the thing it exists to protect. Mirrors `JitterTuning::plc_max_ms`. + static let plcMaxMS = deprimeMS * 2 /// Floor in callbacks under `deprimeMS`, so a large-quantum device keeps real hysteresis /// instead of de-priming on the first short read. Mirrors `MIN_DEPRIME_CALLBACKS`. private static let minDeprimeCallbacks = 2 /// The protocol's frame: the shed unit, and the slack added over a large device quantum. - private static let frameMS = 5 + 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 @@ -151,6 +157,10 @@ final class AudioRing: @unchecked Sendable { /// no timestamps, so the drain thread (which has both a packet's `pts_ns` and the video leg) /// hands the number back for reporting. Mirrors `NativeClient::audio_av_offset_ms`. private var avOffsetMS = 0 + /// Drought concealment the drain thread has synthesized this session, ms — STORED here for the + /// same reason `avOffsetMS` is: the ring cannot compute it, but it is where the numbers a + /// listener's complaint needs can be read under one lock. + private var plcMS = 0 private let channels: Int private let perMS: Int private let lock = OSAllocatedUnfairLock() @@ -224,6 +234,16 @@ final class AudioRing: @unchecked Sendable { avOffsetMS = ms } + /// Store the drain thread's running drought concealment (`DroughtConceal.totalMS`) for + /// reporting. Concealment that nobody can see is concealment that hides the bug it is + /// covering: a healthy `underruns` bought with a climbing `plc_ms` is a link in trouble, not + /// a link that is fine. + func notePlcMS(_ ms: Int) { + lock.lock() + defer { lock.unlock() } + plcMS = ms + } + /// Buffered depth in interleaved samples — what the sync loop measures against (`bufferedMS` /// is the same quantity rounded for humans). Everything queued here must play before the frame /// the drain thread is about to write, which is exactly what delays it. @@ -481,6 +501,10 @@ final class AudioRing: @unchecked Sendable { /// Reported next to the depth, never instead of it: a deep ring on a jittery link is /// CORRECT behaviour, and only the offset separates that from a ring holding audio late. let avOffsetMS: Int + /// Audio synthesized for packet droughts this session (`DroughtConceal`), ms — read next + /// to `underruns`, which it exists to prevent, because the two only mean something + /// together. + let plcMS: Int } var stats: Stats { @@ -491,7 +515,8 @@ final class AudioRing: @unchecked Sendable { targetMS: target / max(perMS, 1), underruns: underrunCount, sheds: shedCount, - avOffsetMS: avOffsetMS) + avOffsetMS: avOffsetMS, + plcMS: plcMS) } } @@ -641,6 +666,70 @@ struct AvSync { } } +// MARK: - Drought concealment + +/// Bounded concealment of a packet DROUGHT — the Apple leg of the policy the three Rust clients +/// share (`punktfunk_core::audio::DroughtConceal`; design/host-source-stutter-fixes.md, WP-C1). +/// +/// The decode path already conceals a SEQ GAP: core's in-ABI decoder synthesizes the packets the +/// sequence says went missing before the one that arrived (`nextAudioPcm`). But that only fires +/// when a LATER packet arrives to reveal the gap. When the wire simply goes quiet — a delivery +/// stall on a bunching Wi-Fi link, or a host whose capture stalled — nothing arrives to reveal +/// anything: `AudioRing` drains to empty, the render callback runs short, and `noteRead` de-primes +/// and then re-primes a whole target's worth of fresh silence. The artifact is far longer than the +/// audio actually missing, and this is the shape the 2026-08-15 field session spent 3–16 % of its +/// wall-clock in. +/// +/// So a drought that is draining the ring gets concealed too, from the same decoder state +/// (`PunktfunkConnection.audioPlc`), for a bounded time. Denominated in TIME, never in frames or +/// callbacks: that is the recorded lesson from the very fuse this protects, where a count gave an +/// iPad a third of a Mac's slack (`AudioRing.deprimeMS`, and +/// `testDeprimeFuseIsADurationNotACallbackCount`). +/// +/// Time is passed IN, so the policy stays as deterministic as the ring's own. +struct DroughtConceal { + /// A drought must outlast ordinary arrival jitter before anything is synthesized for it: two + /// protocol frames, the same tolerance the host's capture-hole infill uses at the other end. + private static let afterMS = 2 * AudioRing.frameMS + /// …and the ring must actually be running out. A drought a deep ring can cover is not audible, + /// and concealing it would synthesize audio the late packets are about to duplicate — pushing + /// the whole stream later and handing the drift shed a mess to clean up audibly. + private static let floorMS = 2 * AudioRing.frameMS + + /// Concealed since the last real packet. + private var concealedMS = 0 + private let maxMS: Int + /// Concealed over the session — what the 10 s `plc_ms=` line reports. Concealment must be + /// visible: a policy that quietly papers over a failing link is a policy that hides the bug. + private(set) var totalMS = 0 + + init(maxMS: Int) { + self.maxMS = maxMS + } + + /// A packet arrived, ending any drought — the next one starts from a full budget. + /// + /// The Rust twin also hands back the frames it concealed, for its caller to subtract from the + /// loss concealment the seq path is about to ask for. Here that subtraction is core's, on the + /// far side of the ABI, because that is where the gap tracker lives (see + /// `punktfunk_connection_audio_plc`) — a packet genuinely lost inside a covered drought must + /// not be concealed twice either way. + mutating func packet() { + concealedMS = 0 + } + + /// Should one more frame be concealed? `depthMS` is the playout ring as the render callback + /// last left it. + mutating func conceal(sinceLastPacketMS: Int, depthMS: Int) -> Bool { + if sinceLastPacketMS < Self.afterMS || depthMS > Self.floorMS || concealedMS >= maxMS { + return false + } + concealedMS += AudioRing.frameMS + totalMS += AudioRing.frameMS + return true + } +} + /// CoreAudio channel layout for the canonical wire order FL FR FC LFE RL RR [SL SR]. nil for /// stereo (the standard layout is correct). For 5.1/7.1 we list explicit channel labels via /// `kAudioChannelLayoutTag_UseChannelDescriptions` — preset tags (DTS_5_1 etc.) don't reliably diff --git a/clients/apple/Sources/PunktfunkKit/Audio/SessionAudio.swift b/clients/apple/Sources/PunktfunkKit/Audio/SessionAudio.swift index c0c38782..0f30ea5b 100644 --- a/clients/apple/Sources/PunktfunkKit/Audio/SessionAudio.swift +++ b/clients/apple/Sources/PunktfunkKit/Audio/SessionAudio.swift @@ -1030,6 +1030,16 @@ public final class SessionAudio { defer { drainDone.signal() } var drained = 0 var av = AvSync(channels: channels) + // WP-C1 — the drought half of concealment. Core heals a SEQ GAP, but only when a later + // packet arrives to reveal it; when the wire simply goes quiet nothing arrives to + // reveal anything, and the ring drains into an underrun and a de-prime whose re-prime + // is a longer artifact than the audio that was missing. + var drought = DroughtConceal(maxMS: AudioRing.plcMaxMS) + var lastPacketNs = DispatchTime.now().uptimeNanoseconds + // Something has decoded, so there is both state to conceal from and continuity to + // hold. Until then a session whose host never sends audio keeps the long timeout below + // rather than waking two hundred times a second to do nothing. + var decoded = false // 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). @@ -1038,11 +1048,48 @@ public final class SessionAudio { alive = autoreleasepool { () -> Bool in let pcm: PunktfunkConnection.AudioPCM? do { - pcm = try connection.nextAudioPcm(timeoutMs: 100) + // Wait at most one frame WHILE there is a stream to protect: the drought + // decision has to be made on the wire's schedule, not whenever the next packet + // happens to turn up. + pcm = try connection.nextAudioPcm( + timeoutMs: decoded ? UInt32(AudioRing.frameMS) : 100) } catch { return false // session closed } - guard let pcm, pcm.frameCount > 0 else { return true } + guard let pcm, pcm.frameCount > 0 else { + // Nothing on the wire. If the ring is draining with it, conceal from the + // decoder's own state — the same libopus interpolation the loss path uses, + // bounded by this ring's de-prime fuse so a genuinely dead stream is not + // papered over. ONE frame per tick, not a burst: this arm runs every frame, + // which is the rate the callback drains at, so concealment keeps pace with + // playout instead of racing ahead of a depth reading it has already + // invalidated. + guard decoded else { return true } + let quietMS = Int( + (DispatchTime.now().uptimeNanoseconds &- lastPacketNs) / 1_000_000) + guard drought.conceal(sinceLastPacketMS: quietMS, depthMS: ring.bufferedMS) + else { + return true + } + let plc: PunktfunkConnection.AudioPCM? + do { + plc = try connection.audioPlc() + } catch { + return false // session closed + } + if let plc { + plc.samples.withUnsafeBufferPointer { p in + if let base = p.baseAddress { + ring.write(base, count: plc.frameCount * plc.channels) + } + } + } + ring.notePlcMS(drought.totalMS) + return true + } + decoded = true + lastPacketNs = DispatchTime.now().uptimeNanoseconds + drought.packet() // Place this frame against the picture it belongs with BEFORE queueing it: the // depth read here is everything that must still play first, which is exactly what // delays it. Skipped wholesale when no meter was wired, so an un-armed session @@ -1070,12 +1117,14 @@ public final class SessionAudio { // 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. + // was in before the 2026-08 audio work. `plc_ms` rides along because a healthy + // `underruns` bought with a climbing `plc_ms` is a link in trouble, not a link + // that is fine. 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) av_offset_ms=\(s.avOffsetMS)" + "audio: buffer_ms=\(s.bufferedMS) target_ms=\(s.targetMS) underruns=\(s.underruns) drift_sheds=\(s.sheds) av_offset_ms=\(s.avOffsetMS) plc_ms=\(s.plcMS)" ) } return true diff --git a/clients/apple/Sources/PunktfunkKit/Connection/PunktfunkConnection.swift b/clients/apple/Sources/PunktfunkKit/Connection/PunktfunkConnection.swift index fb1508a4..416ecb6f 100644 --- a/clients/apple/Sources/PunktfunkKit/Connection/PunktfunkConnection.swift +++ b/clients/apple/Sources/PunktfunkKit/Connection/PunktfunkConnection.swift @@ -1139,6 +1139,46 @@ public final class PunktfunkConnection { } } + /// Synthesize one frame of concealment from the in-core decoder's own state — no packet + /// involved, nothing pulled off the wire. `nil` when there is nothing to extrapolate from + /// (before the first decode, or when libopus declines), which the caller treats exactly like a + /// timeout: write nothing this tick. Throws `.closed` once the session ended. + /// + /// `nextAudioPcm` heals a gap the SEQUENCE reveals; that needs a later packet to arrive and + /// reveal it. This is for the wire simply going quiet, where nothing arrives to reveal + /// anything and the ring drains into an underrun and a de-prime whose re-prime is a longer + /// artifact than the audio that was missing. `DroughtConceal` owns WHEN to ask — bounded in + /// time, and only while the ring is genuinely running out. + /// + /// Same audio thread as `nextAudioPcm`, whose borrowed buffer this call invalidates (they + /// share the slot). The returned `samples` are copied out. `ptsNs`/`seq` read 0: this frame was + /// never on the wire, so it has no capture instant and must not reach an `AvSync` observation. + public func audioPlc() throws -> AudioPCM? { + audioLock.lock() + defer { audioLock.unlock() } + guard let h = liveHandle() else { throw PunktfunkClientError.closed } + + var out = PunktfunkAudioPcm() + let rc = punktfunk_connection_audio_plc(h, &out) + switch rc { + case statusOK: + let channels = Int(out.channels) + let total = Int(out.frame_count) * channels + guard let base = out.samples, total > 0 else { return nil } + // Copy: the pointer borrows connection memory only until the next PCM call. + let samples = Array(UnsafeBufferPointer(start: base, count: total)) + return AudioPCM( + samples: samples, frameCount: Int(out.frame_count), + channels: channels, ptsNs: out.pts_ns, seq: out.seq) + case statusNoFrame: + return nil + case statusClosed: + throw PunktfunkClientError.closed + default: + throw PunktfunkClientError.status(rc) + } + } + /// Pull the next force-feedback update for the GCController haptics engine: /// `(pad, lowFrequency, highFrequency)` with 0...0xFFFF amplitudes, (0, 0) = stop. /// Drain from the (single) feedback thread, alongside `nextHidOutput`. Drops the v2 diff --git a/clients/apple/Tests/PunktfunkKitTests/AudioRingDriftTests.swift b/clients/apple/Tests/PunktfunkKitTests/AudioRingDriftTests.swift index 76566868..a4e03daa 100644 --- a/clients/apple/Tests/PunktfunkKitTests/AudioRingDriftTests.swift +++ b/clients/apple/Tests/PunktfunkKitTests/AudioRingDriftTests.swift @@ -719,5 +719,117 @@ final class AudioRingDriftTests: XCTestCase { XCTAssertEqual(stats.bufferedMS, 30) XCTAssertEqual(stats.avOffsetMS, 37, "positive = audio behind the picture") } + + // MARK: - Drought concealment (WP-C1) + + /// Concealment is for a ring that is running OUT. A drought a deep ring can cover is + /// inaudible, and synthesizing over it would insert audio the late packets are about to + /// duplicate — the stream would then run permanently later and the drift shed would have to + /// cut it back out, audibly. + /// + /// Mirrors `a_drought_is_concealed_only_while_the_ring_is_running_out`. + func testADroughtIsConcealedOnlyWhileTheRingIsRunningOut() { + var c = DroughtConceal(maxMS: AudioRing.plcMaxMS) + let stalledMS = 3 * AudioRing.frameMS + XCTAssertFalse( + c.conceal(sinceLastPacketMS: stalledMS, depthMS: 40), + "a 40 ms ring covers this drought by itself") + XCTAssertTrue( + c.conceal(sinceLastPacketMS: stalledMS, depthMS: 0), "an empty ring does not") + XCTAssertEqual(c.totalMS, AudioRing.frameMS) + } + + /// Ordinary arrival jitter is not a drought — this policy must be invisible until the wire has + /// genuinely stopped. + /// + /// Mirrors `ordinary_jitter_is_not_a_drought`. + func testOrdinaryJitterIsNotADrought() { + var c = DroughtConceal(maxMS: AudioRing.plcMaxMS) + for _ in 0..<1_000 { + XCTAssertFalse(c.conceal(sinceLastPacketMS: AudioRing.frameMS, depthMS: 0)) + } + XCTAssertEqual(c.totalMS, 0) + } + + /// The window is bounded, and bounded in TIME — the whole reason `deprimeMS` stopped being a + /// callback count (`testDeprimeFuseIsADurationNotACallbackCount`). Derived from the fuse, so + /// it cannot drift away from the thing it protects: an edit to one is an edit to both. + /// + /// Mirrors `drought_concealment_is_bounded_at_twice_the_deprime_fuse`. + func testDroughtConcealmentIsBoundedAtTwiceTheDeprimeFuse() { + let deprimeMS = 60 // AudioRing.deprimeMS / JitterTuning::COREAUDIO.deprime_ms + XCTAssertEqual(AudioRing.plcMaxMS, 2 * deprimeMS) + var c = DroughtConceal(maxMS: AudioRing.plcMaxMS) + var ms = 0 + for _ in 0..<1_000 where c.conceal(sinceLastPacketMS: 2 * AudioRing.frameMS, depthMS: 0) { + ms += AudioRing.frameMS + } + XCTAssertEqual(ms, AudioRing.plcMaxMS, "must use exactly the budget, and stop there") + XCTAssertEqual(c.totalMS, AudioRing.plcMaxMS, "and report every millisecond of it") + } + + /// A packet ends the drought and hands back a full budget for the next one — a link that + /// stalls once a minute must be covered every time, not only the first. + /// + /// The other half of the Rust `concealment_already_paid_for_is_not_paid_for_twice` — that + /// frames a drought already covered are subtracted from the loss concealment the seq path then + /// asks for — cannot be tested from here: on this leg the gap tracker lives behind the C ABI, + /// and so does the subtraction (`drought_concealment_is_not_charged_again_by_the_loss_path` in + /// `punktfunk_core::abi`). + func testAPacketEndsTheDroughtAndRefreshesTheBudget() { + var c = DroughtConceal(maxMS: AudioRing.plcMaxMS) + for _ in 0..<1_000 where c.conceal(sinceLastPacketMS: 2 * AudioRing.frameMS, depthMS: 0) {} + XCTAssertEqual(c.totalMS, AudioRing.plcMaxMS, "budget spent") + XCTAssertFalse(c.conceal(sinceLastPacketMS: 2 * AudioRing.frameMS, depthMS: 0)) + c.packet() + XCTAssertTrue( + c.conceal(sinceLastPacketMS: 2 * AudioRing.frameMS, depthMS: 0), + "the next drought must start from a full budget") + XCTAssertEqual( + c.totalMS, AudioRing.plcMaxMS + AudioRing.frameMS, + "the SESSION total keeps counting — it is what the log line reports") + } + + /// THE field scenario this exists for, played against the real ring: the wire goes quiet for + /// longer than the de-prime fuse (a Wi-Fi delivery stall, or a host whose capture stalled). + /// Without concealment the ring drains, starves, and re-primes a whole target's worth of fresh + /// silence — an artifact far longer than the audio that was missing. Fed one synthesized frame + /// per drain tick instead, playback continues through the whole budget and nobody hears the + /// stall at all. + func testConcealmentRidesOutAStallThatWouldOtherwiseDeprime() { + /// Prime, then stall the wire for `ms`, ticking the drain thread's 5 ms loop and the + /// device callback in step. Returns when the first silent callback lands (nil = none). + func stall(ms: Int, concealing: Bool) -> Int? { + let ring = AudioRing(capacity: 48_000 * channels, channels: channels) + let want = 5 * perMS + var scratch = [Float](repeating: 0, count: want) + let feed = [Float](repeating: 0.5, count: 25 * perMS) + feed.withUnsafeBufferPointer { ring.write($0.baseAddress!, count: 25 * perMS) } + var drought = DroughtConceal(maxMS: AudioRing.plcMaxMS) + for tick in 0..<(ms / AudioRing.frameMS) { + if concealing, + drought.conceal( + sinceLastPacketMS: tick * AudioRing.frameMS, depthMS: ring.bufferedMS) { + feed.withUnsafeBufferPointer { + ring.write($0.baseAddress!, count: AudioRing.frameMS * perMS) + } + } + scratch.withUnsafeMutableBufferPointer { + ring.read(into: $0.baseAddress!, count: want) + } + if scratch.allSatisfy({ $0 == 0 }) { return tick * AudioRing.frameMS } + } + return nil + } + // The defect: 25 ms of ring, a 60 ms fuse — the stall is silent well inside the budget. + guard let deprimedAt = stall(ms: AudioRing.plcMaxMS, concealing: false) else { + return XCTFail("the unconcealed stall must still de-prime — the ring changed under us") + } + XCTAssertLessThan(deprimedAt, AudioRing.plcMaxMS) + XCTAssertNil( + stall(ms: AudioRing.plcMaxMS, concealing: true), + "a stall inside the budget must not reach the listener at all (unconcealed: silent " + + "after \(deprimedAt) ms)") + } } #endif diff --git a/crates/punktfunk-core/src/abi.rs b/crates/punktfunk-core/src/abi.rs index 7b4467cf..31f6d972 100644 --- a/crates/punktfunk-core/src/abi.rs +++ b/crates/punktfunk-core/src/abi.rs @@ -710,6 +710,13 @@ struct AudioPcmState { /// frame. 0 until the first decode, which skips concealment (nothing to size it from), /// exactly like the other clients. frame_samples: usize, + /// Frames synthesized for a packet DROUGHT since the last real packet + /// ([`punktfunk_connection_audio_plc`]), subtracted from the loss concealment the next packet + /// asks for: a packet genuinely lost inside a drought this already covered must not be covered + /// twice, which would insert audio the stream never carried and push everything after it + /// later. The subtraction has to happen HERE — the gap tracker is on this side of the ABI, so + /// the embedder driving the drought cannot see what it is about to be charged for. + drought_frames: u32, } #[cfg(feature = "quic")] @@ -748,8 +755,12 @@ impl AudioPcmState { // Conceal lost packets (a seq gap) before decoding the one that arrived: empty input // synthesizes `frame_samples` of interpolation per missing packet — an inaudible fade // instead of the click a hard gap makes in the ring. Mirrors the Linux/Windows session - // pump and the Android native pump; capped by the tracker at 50 ms. - let missing = self.gaps.missing_before(seq); + // pump and the Android native pump; capped by the tracker at 50 ms. Whatever a drought + // already covered comes off the top — that audio is in the embedder's ring already. + let missing = self + .gaps + .missing_before(seq) + .saturating_sub(std::mem::take(&mut self.drought_frames)); let mut filled = 0usize; if self.frame_samples > 0 { for _ in 0..missing { @@ -779,6 +790,32 @@ impl AudioPcmState { Err(_) => Err(PunktfunkStatus::BadPacket), } } + + /// Synthesize ONE concealment frame with no packet involved — the drought half of concealment + /// (see [`punktfunk_connection_audio_plc`] for what asks for it and why). + /// + /// Returns the interleaved sample count now valid at the front of `pcm`, or `Ok(0)` when + /// nothing has decoded yet: libopus PLC extrapolates from the LAST decoded frame, so before + /// there is one there is neither state to extrapolate from nor a frame size to ask for. + fn conceal(&mut self, channels: u8) -> Result { + let ch = channels as usize; + let plc = self.frame_samples * ch; + if plc == 0 { + return Ok(0); + } + let Some(dec) = self.decoder.as_mut() else { + return Ok(0); + }; + match dec.decode_float(&[], &mut self.pcm[..plc], false) { + Ok(samples) => { + self.drought_frames = self.drought_frames.saturating_add(1); + Ok(samples * ch) + } + // libopus declined to interpolate. Nothing to hand out, and the caller's response is + // a timeout's: write nothing and let its ring's own underrun path have the drought. + Err(_) => Ok(0), + } + } } /// `PunktfunkHidOutput::kind` — lightbar RGB (`r`/`g`/`b` valid). @@ -2577,7 +2614,9 @@ pub struct PunktfunkAudioPcm { /// IN FRONT of the arriving frame in the same buffer — `out->frame_count` then covers the /// concealed frames plus the real one (`out->seq`/`out->pts_ns` are the real packet's). The /// embedder just writes the whole buffer to its ring, same as any other frame; gaps arrive -/// pre-healed, exactly as they do on the clients that decode outside core. +/// pre-healed, exactly as they do on the clients that decode outside core. That covers a gap a +/// LATER packet reveals; when the wire goes quiet instead, see +/// [`punktfunk_connection_audio_plc`]. /// /// # Safety /// `c` is a valid connection handle; `out` is writable. At most one thread pulls audio. @@ -2630,6 +2669,78 @@ pub unsafe extern "C" fn punktfunk_connection_next_audio_pcm( }) } +/// Synthesize ONE frame of concealment from the in-core decoder's own state — no packet involved, +/// nothing pulled off the wire (design/host-source-stutter-fixes.md, WP-C1). +/// +/// [`punktfunk_connection_next_audio_pcm`] heals a gap the SEQUENCE reveals, which needs a later +/// packet to arrive and reveal it. When the wire simply goes quiet — a delivery stall on a +/// bunching Wi-Fi link, or a host whose capture stalled — nothing arrives to reveal anything: the +/// embedder's playout ring drains to empty, its callback runs short, and its de-jitter policy +/// de-primes and then re-primes a whole target's worth of fresh silence. The artifact is far +/// longer than the audio actually missing. +/// +/// So on a `NO_FRAME` timeout with a DRAINING ring, ask for this instead. The policy stays on the +/// embedder's side because that is where its two ingredients live — the ring depth and the clock +/// since the last packet — and it must be: bounded in TIME (roughly twice the ring's own de-prime +/// fuse), never in callbacks or frames, and gated on the ring genuinely running out. A drought a +/// deep ring covers is inaudible, and concealing it would insert audio the late packets are about +/// to duplicate, pushing the stream permanently later. Core supplies only the mechanism, one frame +/// per call, at the cadence the embedder drains at. +/// +/// Returns [`PunktfunkStatus::NoFrame`] when nothing has decoded yet — PLC extrapolates from the +/// last decoded frame, so before there is one there is no state to extrapolate from — and if +/// libopus declines to interpolate. Both mean "write nothing this tick", exactly like a timeout. +/// +/// `out->seq` and `out->pts_ns` read 0: this frame was never on the wire, so it has no sequence +/// number and no capture instant, and it must never be fed to an A/V-sync observation. +/// `out->samples` borrows connection memory until the next PCM call on this handle — the SAME +/// slot [`punktfunk_connection_next_audio_pcm`] hands out, so call both from the one audio thread. +/// +/// Frames taken this way are subtracted from the concealment the next arriving packet asks for, so +/// a packet genuinely lost inside a covered drought is not concealed twice. +/// +/// # Safety +/// `c` is a valid connection handle; `out` is writable. At most one thread pulls audio. +#[cfg(feature = "quic")] +#[unsafe(no_mangle)] +pub unsafe extern "C" fn punktfunk_connection_audio_plc( + c: *mut PunktfunkConnection, + out: *mut PunktfunkAudioPcm, +) -> PunktfunkStatus { + guard(|| { + // SAFETY: per the ABI contract - an opaque handle from a `*_new`/`*_pair` that the caller + // has not yet freed, or null, which `as_mut`/`as_ref` reports as `None` and the `match` + // here handles. + let c = match unsafe { c.as_ref() } { + Some(c) => c, + None => return PunktfunkStatus::NullPointer, + }; + if out.is_null() { + return PunktfunkStatus::NullPointer; + } + let channels = crate::audio::normalize_channels(c.inner.audio_channels); + let mut state = lock_recover(&c.audio_pcm); + match state.conceal(channels) { + Ok(0) => PunktfunkStatus::NoFrame, + Ok(samples) => { + // SAFETY: per the ABI contract - `out` is a caller-owned writable slot of the + // matching `#[repr(C)]` type, written once by value. + unsafe { + *out = PunktfunkAudioPcm { + samples: state.pcm.as_ptr(), + frame_count: (samples / channels.max(1) as usize) as u32, + channels, + seq: 0, + pts_ns: 0, + }; + } + PunktfunkStatus::Ok + } + Err(status) => status, + } + }) +} + /// Pull the next pad-audio frame (0xD1) — one Opus frame of DualSense voice-coil haptics /// (`kind` = [`PUNKTFUNK_PAD_AUDIO_KIND_HAPTICS`], 5 ms) or built-in-speaker audio /// ([`PUNKTFUNK_PAD_AUDIO_KIND_SPEAKER`], 10 ms) for gamepad `*out_pad` — waiting up to @@ -5294,4 +5405,48 @@ mod tests { Ok((crate::audio::MAX_CONCEAL_PACKETS as usize + 1) * FRAME * 2) ); } + + /// Drought concealment (WP-C1): the embedder asks for a frame at a time while the wire is + /// quiet, and the loss path must then charge only for what the drought did NOT already cover + /// — concealing a packet lost inside a covered drought a second time would insert audio the + /// stream never carried and push everything after it later. The subtraction lives here + /// because the gap tracker does; the embedder driving the drought cannot see it. + #[test] + fn drought_concealment_is_not_charged_again_by_the_loss_path() { + const FRAME: usize = 240; // 5 ms @ 48 kHz, per channel + let l = crate::audio::LAYOUT_STEREO; + let mut enc = opus::MSEncoder::new( + 48_000, + l.streams, + l.coupled, + l.mapping, + opus::Application::LowDelay, + ) + .expect("MSEncoder"); + enc.set_vbr(false).unwrap(); + let mut packet = |tone: f32| { + let mut frame = vec![0f32; FRAME * 2]; + for (i, s) in frame.iter_mut().enumerate() { + *s = 0.25 * (i as f32 * tone).sin(); + } + let mut out = vec![0u8; 1500]; + let n = enc.encode_float(&frame, &mut out).unwrap(); + out.truncate(n); + out + }; + + let mut state = AudioPcmState::default(); + // Nothing has decoded: PLC has no state to extrapolate from, and the ABI reports NoFrame. + assert_eq!(state.conceal(2), Ok(0)); + + assert_eq!(state.decode_packet(&packet(0.05), 0, 2), Ok(FRAME * 2)); + // The wire goes quiet; the embedder covers four frames of it. + for _ in 0..4 { + assert_eq!(state.conceal(2), Ok(FRAME * 2)); + } + // It comes back at seq 7 — six packets missing, four of them already in the ring. + assert_eq!(state.decode_packet(&packet(0.06), 7, 2), Ok(3 * FRAME * 2)); + // …and the next drought starts from nothing owed, not from a stale credit. + assert_eq!(state.decode_packet(&packet(0.06), 9, 2), Ok(2 * FRAME * 2)); + } } diff --git a/crates/punktfunk-core/src/lib.rs b/crates/punktfunk-core/src/lib.rs index 97ec83d1..61fcc87b 100644 --- a/crates/punktfunk-core/src/lib.rs +++ b/crates/punktfunk-core/src/lib.rs @@ -208,7 +208,19 @@ pub use stats::Stats; /// way). Additive and client-local: the mask, the expiry and the `AccessUpdate` message all /// shipped with the Welcome's trailing-field append (old peers skip them in both directions), /// so [`WIRE_VERSION`] is unchanged. -pub const ABI_VERSION: u32 = 22; +/// v23: added `punktfunk_connection_audio_plc` — one frame of libopus packet-loss concealment, +/// synthesized from the connection's OWN decoder state, for an embedder whose playout ring is +/// draining because nothing is arriving (design/host-source-stutter-fixes.md WP-C1). The three +/// Rust clients conceal a packet drought on their decode thread; Apple's ring is Swift and its +/// decoder sits behind this ABI, so without a call it had no way to reach the one thing that can +/// extrapolate the missing audio — a second decoder would conceal from empty state, because PLC +/// extrapolates from the last decoded frame. A NEW symbol: every existing function keeps its +/// signature and behaviour, and an embedder that never calls it behaves exactly as before (it +/// simply de-primes over droughts, as all four clients used to). Frames it returns carry `seq` +/// and `pts_ns` of `0` — concealed audio was never on the wire and must not reach an A/V-sync +/// observation. Additive and client-local: nothing new is sent or parsed, so [`WIRE_VERSION`] is +/// unchanged. +pub const ABI_VERSION: u32 = 23; /// The punktfunk/1 **wire** version — what `Hello`/`Welcome` carry and hosts equality-check. /// Deliberately its own constant: [`ABI_VERSION`] tracks the embeddable **C surface** diff --git a/include/punktfunk_core.h b/include/punktfunk_core.h index c5f05d83..3d5b4acc 100644 --- a/include/punktfunk_core.h +++ b/include/punktfunk_core.h @@ -137,7 +137,19 @@ // way). Additive and client-local: the mask, the expiry and the `AccessUpdate` message all // shipped with the Welcome's trailing-field append (old peers skip them in both directions), // so [`WIRE_VERSION`] is unchanged. -#define PUNKTFUNK_ABI_VERSION 22 +// v23: added `punktfunk_connection_audio_plc` — one frame of libopus packet-loss concealment, +// synthesized from the connection's OWN decoder state, for an embedder whose playout ring is +// draining because nothing is arriving (design/host-source-stutter-fixes.md WP-C1). The three +// Rust clients conceal a packet drought on their decode thread; Apple's ring is Swift and its +// decoder sits behind this ABI, so without a call it had no way to reach the one thing that can +// extrapolate the missing audio — a second decoder would conceal from empty state, because PLC +// extrapolates from the last decoded frame. A NEW symbol: every existing function keeps its +// signature and behaviour, and an embedder that never calls it behaves exactly as before (it +// simply de-primes over droughts, as all four clients used to). Frames it returns carry `seq` +// and `pts_ns` of `0` — concealed audio was never on the wire and must not reach an A/V-sync +// observation. Additive and client-local: nothing new is sent or parsed, so [`WIRE_VERSION`] is +// unchanged. +#define PUNKTFUNK_ABI_VERSION 23 // The punktfunk/1 **wire** version — what `Hello`/`Welcome` carry and hosts equality-check. // Deliberately its own constant: [`ABI_VERSION`] tracks the embeddable **C surface** @@ -2862,7 +2874,9 @@ PunktfunkStatus punktfunk_connection_end_reason(PunktfunkConnection *c, uint8_t // IN FRONT of the arriving frame in the same buffer — `out->frame_count` then covers the // concealed frames plus the real one (`out->seq`/`out->pts_ns` are the real packet's). The // embedder just writes the whole buffer to its ring, same as any other frame; gaps arrive -// pre-healed, exactly as they do on the clients that decode outside core. +// pre-healed, exactly as they do on the clients that decode outside core. That covers a gap a +// LATER packet reveals; when the wire goes quiet instead, see +// [`punktfunk_connection_audio_plc`]. // // # Safety // `c` is a valid connection handle; `out` is writable. At most one thread pulls audio. @@ -2871,6 +2885,42 @@ PunktfunkStatus punktfunk_connection_next_audio_pcm(PunktfunkConnection *c, uint32_t timeout_ms); #endif +#if defined(PUNKTFUNK_FEATURE_QUIC) +// Synthesize ONE frame of concealment from the in-core decoder's own state — no packet involved, +// nothing pulled off the wire (design/host-source-stutter-fixes.md, WP-C1). +// +// [`punktfunk_connection_next_audio_pcm`] heals a gap the SEQUENCE reveals, which needs a later +// packet to arrive and reveal it. When the wire simply goes quiet — a delivery stall on a +// bunching Wi-Fi link, or a host whose capture stalled — nothing arrives to reveal anything: the +// embedder's playout ring drains to empty, its callback runs short, and its de-jitter policy +// de-primes and then re-primes a whole target's worth of fresh silence. The artifact is far +// longer than the audio actually missing. +// +// So on a `NO_FRAME` timeout with a DRAINING ring, ask for this instead. The policy stays on the +// embedder's side because that is where its two ingredients live — the ring depth and the clock +// since the last packet — and it must be: bounded in TIME (roughly twice the ring's own de-prime +// fuse), never in callbacks or frames, and gated on the ring genuinely running out. A drought a +// deep ring covers is inaudible, and concealing it would insert audio the late packets are about +// to duplicate, pushing the stream permanently later. Core supplies only the mechanism, one frame +// per call, at the cadence the embedder drains at. +// +// Returns [`PunktfunkStatus::NoFrame`] when nothing has decoded yet — PLC extrapolates from the +// last decoded frame, so before there is one there is no state to extrapolate from — and if +// libopus declines to interpolate. Both mean "write nothing this tick", exactly like a timeout. +// +// `out->seq` and `out->pts_ns` read 0: this frame was never on the wire, so it has no sequence +// number and no capture instant, and it must never be fed to an A/V-sync observation. +// `out->samples` borrows connection memory until the next PCM call on this handle — the SAME +// slot [`punktfunk_connection_next_audio_pcm`] hands out, so call both from the one audio thread. +// +// Frames taken this way are subtracted from the concealment the next arriving packet asks for, so +// a packet genuinely lost inside a covered drought is not concealed twice. +// +// # Safety +// `c` is a valid connection handle; `out` is writable. At most one thread pulls audio. +PunktfunkStatus punktfunk_connection_audio_plc(PunktfunkConnection *c, PunktfunkAudioPcm *out); +#endif + #if defined(PUNKTFUNK_FEATURE_QUIC) // Pull the next pad-audio frame (0xD1) — one Opus frame of DualSense voice-coil haptics // (`kind` = [`PUNKTFUNK_PAD_AUDIO_KIND_HAPTICS`], 5 ms) or built-in-speaker audio -- 2.54.0 From a7ef62241bda7e6dfe5b64d69edf592b2e02cffc Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 15 Aug 2026 14:18:14 +0200 Subject: [PATCH 08/10] feat(android): present on the host's cadence under smoothness, not on the frame's arrival MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP4 of design/presenter-cadence-rework-implementation-plan.md — the Android binding of the CadenceClock landed in e207d0d7. The presenter aimed every frame at `next_target(now)`, so a frame's place on the glass was decided by when it happened to finish decoding. On a host whose compositor delivers raggedly that is the defect verbatim: the source's own spacing is discarded and replaced with the transport's. Three things had to be true for the clock to work here. Its `ready_ns` must be in the domain `releaseOutputBufferAtTime` consumes, which is CLOCK_MONOTONIC — so a monotonic twin is now stamped beside the existing realtime `decoded_ns`, at the same instant on the codec's looper thread, and `decoded_ns` keeps its realtime meaning for the latency stats. The cushion's ceiling is one SOURCE frame, so it comes from the negotiated `mode.refresh_hz`, not the panel's. And the loop must re-anchor on the discontinuities the client already knows about — the re-anchor gate's arm count is that seam, and it subsumes every abandoned-AU and codec-error site the plan listed. (The codec is never rebuilt in place here: a rebuild tears the loop down and builds a fresh Presenter, so a reset there would be dead code.) **The preroll gate is retired, and keeping it was never the conservative option.** `prerolled` completes only when the store reaches capacity, but under cadence targeting a frame sits in the store only between its decode and its due time, so with a cushion under one frame interval the depth is about one. Preroll would re-arm on nearly every pass and a buffer=2 session would present only during bursts. `head_is_releasable` replaces both it and the per-vsync drain: a frame leaves once the grid point it aims at is the next one this pump could still submit for. `next_target` now takes a composed `not_before_ns` instead of a separate `margin_ns`. Passing `max(now, due)` while the margin stayed inside made the function require `G > due + margin`, which pushes a frame whose due time sits just under a grid point onto the next one for some phases and not others — that is judder, not latency, and it only appears once the adaptive margin widens off zero. For the latency path the new expression is arithmetically identical to what the old body computed. Smoothness only: under `latency` the clock is `None`, so it is not consulted rather than consulted and ignored, and a test drives 600 frames through the due path asserting every answer is `None` and the target floor is still exactly `now + margin`. ⚠ `qDry` changes meaning and says so on the field: it now counts vsync ticks that found the store empty, with no preroll precondition. Under cadence targeting an empty store is the ordinary steady state — it reads as supply depth, not as an alarm. Verified: aarch64 AND armv7 clippy `--all-targets -D warnings` both exit 0 (the armv7 leg because `vsync.rs`'s 32-bit `timespec` casts are target-dependent and must not be "cleaned up"). No device was attached, so the unit tests were type-checked but not run; their exact synthetic trace was instead replayed through the REAL imported clock, giving frames=600 reanchors=1 late=17 and a worst steady-state due-spacing error of 116 µs against a source grid whose raw arrivals are off by 4 ms. --- .../android/native/src/decode/async_loop.rs | 32 +- .../android/native/src/decode/presenter.rs | 300 ++++++++++++++++-- clients/android/native/src/decode/vsync.rs | 35 +- 3 files changed, 318 insertions(+), 49 deletions(-) diff --git a/clients/android/native/src/decode/async_loop.rs b/clients/android/native/src/decode/async_loop.rs index 0bc4123a..48f99d26 100644 --- a/clients/android/native/src/decode/async_loop.rs +++ b/clients/android/native/src/decode/async_loop.rs @@ -37,6 +37,7 @@ struct OutputReady { index: usize, pts_us: u64, decoded_ns: i128, + decoded_mono_ns: i64, } /// Events the async decode loop reacts to. The codec's async-notify callbacks (which run on its @@ -51,11 +52,12 @@ enum DecodeEvent { Au(Frame, u32), /// An input buffer slot freed (index) — we can queue an AU into it. InputAvailable(usize), - /// A decoded frame is ready (buffer index + echoed pts + the callback-time `decoded` stamp). + /// A decoded frame is ready (buffer index + echoed pts + the callback-time `decoded` stamps). OutputAvailable { index: usize, pts_us: u64, decoded_ns: i128, + decoded_mono_ns: i64, }, /// The output format changed — re-check the stream's colour signalling (HDR DataSpace). FormatChanged, @@ -126,6 +128,12 @@ pub(super) fn run_async( // decode stage ends when the frame actually became available — not after the // channel hop + whatever work the loop coalesces in front of presenting it. decoded_ns: now_realtime_ns(), + // Its monotonic twin, from the same instant. The stats are REALTIME (they + // fold the host's clock offset in), while the cadence loop and + // `releaseOutputBufferAtTime` are both CLOCK_MONOTONIC — and the loop is fed + // and read in one domain, never converted (`punktfunk_core::phase`: a + // constant offset between domains is what its offset estimator absorbs). + decoded_mono_ns: now_monotonic_ns(), }); })), on_format_changed: Some(Box::new(move |_fmt| { @@ -232,7 +240,7 @@ pub(super) fn run_async( PresentPriority::Smooth { buffer } => format!("smoothness, buffer {buffer}"), } ); - Some(Presenter::new(priority)) + Some(Presenter::new(priority, mode.refresh_hz)) }; stats.set_presenter_active(presenter.is_some()); // The vsync clock, started LAZILY on the first decoded frame (see `vsync.rs`); its ticks ride @@ -299,6 +307,7 @@ pub(super) fn run_async( // codec error; `recovery_flags` carries each AU's user_flags from `dispatch_event` (feed) to // `present_ready` (present), keyed by the codec-echoed pts. let mut gate = ReanchorGate::new(client.frames_dropped()); + let mut last_arms = gate.arms(); let mut recovery_flags: VecDeque<(u64, u32)> = VecDeque::new(); let mut last_kf_req: Option = None; // Productive (dispatch+feed+present) time between displayed frames; reported to ADPF once one is @@ -378,6 +387,19 @@ pub(super) fn run_async( &mut queued_stamps, &mut gate, ); + // The cadence loop's re-anchor seam. A fresh arm means a loss was detected: the gate is + // about to freeze on the last good picture and the frames that reach the presenter on the + // far side come through a decoder that has just recovered, so the source→presentable delay + // the loop had measured is not the one it will see. Watched by ARM COUNT rather than at + // each `gate.arm` site because those are spread across the dispatcher, the feeder and this + // loop's own backstops, and the count catches every one of them — including the ones + // `feed_ready` raises for an abandoned partial AU. + if gate.arms() != last_arms { + last_arms = gate.arms(); + if let Some(p) = presenter.as_mut() { + p.reset_cadence(); + } + } let had_output = !ready.is_empty(); let rendered_before = rendered; present_ready( @@ -414,7 +436,7 @@ pub(super) fn run_async( if let (Some(_), Some(c)) = (p.flush_log(&meter, clock), clock) { let period = c.panel_period_ns().max(c.period_ns()); if period > 0 { - if let Some(t) = c.next_target(now_monotonic_ns(), 0) { + if let Some(t) = c.next_target(now_monotonic_ns()) { let mono_now = now_monotonic_ns(); let real_now = now_realtime_ns(); let leads_us: Vec = arrival_stamps @@ -737,10 +759,12 @@ fn dispatch_event( index, pts_us, decoded_ns, + decoded_mono_ns, } => ready.push(OutputReady { index, pts_us, decoded_ns, + decoded_mono_ns, }), DecodeEvent::FormatChanged => *fmt_dirty = true, DecodeEvent::Vsync => *vsync_tick = true, @@ -998,7 +1022,7 @@ fn present_ready( for o in ready.drain(..) { let flags = take_flags(recovery_flags, o.pts_us); if gate.on_decoded(flags, false, now) == GateVerdict::Present { - let dropped = p.submit(codec, o.index, o.pts_us, o.decoded_ns); + let dropped = p.submit(codec, o.index, o.pts_us, o.decoded_ns, o.decoded_mono_ns); skipped += dropped; *discarded += dropped; } else { diff --git a/clients/android/native/src/decode/presenter.rs b/clients/android/native/src/decode/presenter.rs index 5b372712..06ed7232 100644 --- a/clients/android/native/src/decode/presenter.rs +++ b/clients/android/native/src/decode/presenter.rs @@ -14,6 +14,12 @@ //! frame timeline (API 33+, via [`super::vsync`]), so the latch phase is deterministic instead //! of inheriting network + decode jitter. On the 31/32 fallback the release is ASAP — //! identical to the legacy path — and only the budget prediction uses the measured period. +//! * under the SMOOTHNESS intent only, a **due time** per frame from +//! [`punktfunk_core::phase::CadenceClock`]: the release targets the first timeline at or after +//! the instant the SOURCE's own timestamp says the frame is due, rather than the first one +//! after it happened to decode, so the host's cadence reaches glass instead of the network's +//! (`design/presenter-cadence-rework.md` D1). The latency intent holds no clock at all and +//! stays arrival-driven by construction. //! //! The legacy behaviour (release the newest ready buffer immediately, unbudgeted) remains //! selectable at runtime: `adb shell setprop debug.punktfunk.presenter arrival` — the on-device @@ -21,6 +27,7 @@ //! (off = the synchronous pre-overhaul loop, no presenter at all). use ndk::media::media_codec::MediaCodec; +use punktfunk_core::phase::{CadenceClock, CadenceHealth, CadenceTuning}; use std::collections::VecDeque; use std::sync::atomic::{AtomicBool, AtomicI32, Ordering}; use std::sync::Mutex; @@ -103,8 +110,9 @@ const FALLBACK_PERIOD_NS: i64 = 8_333_333; pub(crate) enum PresentPriority { /// Newest-wins, release the instant the budget opens. The default. Latency, - /// A small FIFO (1..=3 frames) drained one per vsync: jitter absorbed at one refresh of - /// added display latency per slot, which the metrics show rather than hide. + /// A small holding store (1..=3 frames) drained on each frame's DUE time from the source's own + /// timestamps: transport jitter absorbed in the cushion the cadence loop sizes from its own + /// measured residual, at a latency the metrics show rather than hide. Smooth { buffer: usize }, } @@ -129,6 +137,9 @@ struct HeldFrame { pts_us: u64, /// The output callback's `CLOCK_REALTIME` stamp — the pace metric's start (decoded→release). decoded_ns: i128, + /// When the source says this frame is due, monotonic — [`CadenceClock::due_ns`] folded at + /// submit. `None` under the latency intent, which has no clock. + due_ns: Option, } /// The one-in-flight glass budget. @@ -287,20 +298,33 @@ fn p50_max_ms(mut v: Vec) -> (f64, f64) { } pub(super) struct Presenter { - /// 0 = newest-wins; 1..=3 = smoothing FIFO capacity. + /// 0 = newest-wins; 1..=3 = the holding store's capacity, a bound against a burst rather than + /// a pacing depth (design §4.3). fifo_capacity: usize, frames: VecDeque, - /// FIFO preroll: `take` withholds until the buffer filled to capacity once, re-armed on a dry - /// run — the Apple `FrameStore` semantics (headroom never builds without it). - prerolled: bool, + /// The source-cadence loop, and the ONLY thing that makes this presenter anything but + /// arrival-driven. `None` under the latency intent — not "a clock we choose to ignore" but no + /// clock at all, so that path cannot drift into cadence targeting by accident (`design/ + /// presenter-cadence-rework.md` §5; the test below is what holds it there). + cadence: Option, + /// The source's nominal frame interval — [`CadenceClock`]'s cushion ceiling, which is a design + /// invariant rather than a tunable (a cushion past one whole frame buys smoothness the source + /// cannot supply). From the negotiated stream rate, which cannot change without tearing this + /// loop down. + frame_interval_ns: i64, inflight: Option, - /// A vsync arrived since the last release — the FIFO's one-per-refresh drain pace. + /// A vsync arrived since the last release — the retry beat for a parked frame, and what the + /// empty-store readout below is counted against. vsync_tick: bool, // -- 1 Hz pf-present window, always on -- released: u64, paced_drops: u64, no_budget: u64, forced: u64, + /// Vsync ticks that found the store empty. Under cadence targeting an empty store is the + /// ordinary state between a frame's decode and its due time, so this reads as supply depth + /// rather than as the underflow alarm it was under the retired per-slot drain — the alarm is + /// the loop's own late count. dry: u64, /// Pump passes that held a frame back because too many earlier releases were still /// unconfirmed ([`UNDISPLAYED_CAP`]) — reads 0 on a healthy device, and a climbing value is @@ -321,7 +345,12 @@ pub(super) struct Presenter { } impl Presenter { - pub(super) fn new(priority: PresentPriority) -> Presenter { + /// `source_hz` is the NEGOTIATED STREAM rate, not the panel's — it sizes the cadence loop's + /// cushion ceiling, and the quantity that must not be exceeded is one source frame. 0 (an + /// unnegotiated rate) falls back to [`FALLBACK_PERIOD_NS`], the tighter of the two plausible + /// answers: a ceiling set too low costs smoothness, one set too high costs latency the user + /// never asked for. + pub(super) fn new(priority: PresentPriority, source_hz: u32) -> Presenter { let pinned = latch_margin_ns(); let (margin_ns, margin_pinned) = match pinned { Some(ns) => (ns, true), @@ -336,13 +365,27 @@ impl Presenter { " (adaptive — widens on latch misses)" } ); + let frame_interval_ns = match source_hz { + 0 => FALLBACK_PERIOD_NS, + hz => 1_000_000_000 / i64::from(hz), + }; + // `snapping()`, because every release here goes through the frame-timeline grid: the + // snap-up carries roughly half a refresh of implicit slack, so the cushion is small. + let cadence = matches!(priority, PresentPriority::Smooth { .. }).then(|| { + log::info!( + "presenter: cadence clock on the source's timeline, frame interval {:.2}ms", + frame_interval_ns as f64 / 1e6 + ); + CadenceClock::new(CadenceTuning::snapping()) + }); Presenter { fifo_capacity: match priority { PresentPriority::Latency => 0, PresentPriority::Smooth { buffer } => buffer, }, frames: VecDeque::new(), - prerolled: false, + cadence, + frame_interval_ns, inflight: None, vsync_tick: false, released: 0, @@ -359,8 +402,9 @@ impl Presenter { } } - /// A vsync pulse from the clock thread's event — the retry tick for a parked frame and the - /// FIFO's drain pace. + /// A vsync pulse from the clock thread's event — the retry tick for a frame parked on a closed + /// budget, and the beat the empty-store readout is counted against. No longer a drain pace: + /// under cadence targeting the due time is what releases a frame. pub(super) fn on_vsync(&mut self) { self.vsync_tick = true; } @@ -368,13 +412,21 @@ impl Presenter { /// Accept one decoded, gate-approved output buffer. Newest-wins evicts everything older /// (released unrendered — the explicit, counted drop); the FIFO evicts its oldest past /// capacity. Returns how many frames were dropped by the policy (the HUD's `skipped`). + /// + /// `decoded_mono_ns` is the monotonic twin of `decoded_ns`, stamped at the same instant on the + /// codec's callback thread: the cadence loop and `releaseOutputBufferAtTime` both live in + /// `CLOCK_MONOTONIC`, while the latency stats live in `CLOCK_REALTIME`, and the loop is fed and + /// read in ONE domain (`phase.rs`: a constant offset between domains is absorbed by the offset + /// estimator, so no conversion belongs anywhere in this path). pub(super) fn submit( &mut self, codec: &MediaCodec, index: usize, pts_us: u64, decoded_ns: i128, + decoded_mono_ns: i64, ) -> u64 { + let due_ns = self.due_at(pts_us, decoded_mono_ns); let mut dropped = 0u64; if self.fifo_capacity == 0 { while let Some(stale) = self.frames.pop_front() { @@ -386,6 +438,7 @@ impl Presenter { index, pts_us, decoded_ns, + due_ns, }); if self.fifo_capacity > 0 && self.frames.len() > self.fifo_capacity { if let Some(stale) = self.frames.pop_front() { @@ -397,6 +450,61 @@ impl Presenter { dropped } + /// When the source says this frame is due, or `None` under the latency intent. + /// + /// `pts_us` IS the host's own stamp — it round-trips through the codec's presentation time, so + /// the source timeline needs no plumbing of its own. The µs the codec API quantises it to + /// costs ±0.5 µs of white noise on an 8.3 ms period, and that passes through to the due time + /// like any other variation in the source's cadence: the loop smooths the offset, never the + /// timestamps. + fn due_at(&mut self, pts_us: u64, decoded_mono_ns: i64) -> Option { + let interval_ns = self.frame_interval_ns; + self.cadence + .as_mut() + .map(|c| c.due_ns(pts_us.saturating_mul(1_000), decoded_mono_ns, interval_ns)) + } + + /// The earliest present a release may target: SurfaceFlinger's latch lead ahead of now, since + /// a present it cannot latch in time is not a target, and never before the frame's own due + /// time. That second half is the whole of the cadence change — `next_target(max(now, due))` + /// where it used to be `next_target(now)` (design §4.2). + fn not_before_ns(&self, now_mono_ns: i64, due_ns: Option) -> i64 { + let submit_floor_ns = now_mono_ns + self.margin_ns; + due_ns.map_or(submit_floor_ns, |due| due.max(submit_floor_ns)) + } + + /// Whether the frame at the head of the smoothing store may leave it yet. + /// + /// The store's job under cadence targeting is to HOLD WHAT IS NOT DUE YET (design §4.3): the + /// due time paces, so capacity is a bound against a burst rather than the clock. A frame is + /// releasable once the grid point it aims at is the next one this pump could still submit for + /// — one `grid_period_ns` of reach past the submit margin. Sooner buys nothing, because the + /// release is timed either way and holding keeps the store's own eviction policy live over the + /// frame; later risks the loop's 5 ms housekeeping wake landing inside the submit lead, which + /// costs the frame a whole refresh. + fn head_is_releasable(&self, now_mono_ns: i64, grid_period_ns: i64) -> bool { + let reach_ns = now_mono_ns + self.margin_ns + grid_period_ns; + self.frames + .front() + .is_some_and(|f| f.due_ns.is_none_or(|due| due <= reach_ns)) + } + + /// Force the cadence loop to re-anchor on the next frame — the discontinuity hook the clock + /// asks its callers for. The Android seam is the re-anchor gate arming: a loss freezes the + /// gate, the decoder recovers behind it, and what reaches this store on the far side comes + /// through a pipeline whose delay is no longer the one the loop measured. Source-timestamp + /// regressions and half-second gaps the loop catches by itself. No-op under latency. + pub(super) fn reset_cadence(&mut self) { + if let Some(c) = self.cadence.as_mut() { + c.reset(); + } + } + + /// The cadence loop's health for the 1 Hz line, or `None` when there is no loop to read. + fn cadence_health(&self) -> Option { + self.cadence.as_ref().map(CadenceClock::health) + } + /// The present decision point — run on every loop pass (frame arrivals, vsync ticks, and the /// 5 ms housekeeping wake all land here). Releases AT MOST one frame (the budget). Returns /// `true` when a frame was released to glass this call. @@ -423,28 +531,28 @@ impl Presenter { // pass — frame waiting or not — so its forgiveness timer measures real elapsed time // rather than how often a frame happened to be ready. let backlogged = self.unconfirmed_backlog(meter, now_mono_ns); + // The grid a release snaps to, for the store's due-time reach below: the panel's own + // period where it is known — the app's choreographer stream can be down-rated below it + // (see `VsyncShared::next_target`) — and the measured callback period otherwise. + let grid_period_ns = clock + .map(|c| c.panel_period_ns().max(c.period_ns())) + .filter(|&p| p > 0) + .unwrap_or(FALLBACK_PERIOD_NS); // Pick the frame this pump may release. let frame = if self.fifo_capacity == 0 { self.frames.pop_back() // submit() kept it a single slot; back == the newest } else { - // FIFO: drain exactly one frame per vsync tick, after preroll; a drain tick that - // finds the buffer dry re-arms preroll (the Apple `FrameStore` underflow semantics — - // the previous frame persists on glass, a repeat by omission, while headroom - // rebuilds). Everything is gated on the tick so an idle stream neither counts - // underflows nor churns the preroll flag 200×/s. - if !self.vsync_tick { - return false; - } - if !self.prerolled { - if self.frames.len() < self.fifo_capacity { - return false; + // The smoothing store releases on the DUE time, so there is no drain tick to gate on + // and no preroll beneath it. One-frame-per-slot was itself the defect: at 60 fps on a + // 120 Hz panel it drains at twice supply, the store empties, preroll re-arms, and the + // intervals become 1,3,1,3 where 2,2,2 is the correct answer — the smoothing mode + // juddering by construction at exactly the rate mismatch it exists to smooth (design + // §4.3, D3). Due times one source period apart snap to every second vblank instead. + if !self.head_is_releasable(now_mono_ns, grid_period_ns) { + if self.vsync_tick && self.frames.is_empty() { + self.dry += 1; } - self.prerolled = true; - } - if self.frames.is_empty() { - self.prerolled = false; - self.dry += 1; - self.vsync_tick = false; // this tick's drain ran (and found nothing) + self.vsync_tick = false; // this tick's evaluation ran (and released nothing) return false; } self.frames.pop_front() @@ -463,8 +571,11 @@ impl Presenter { } return false; } - // Release: timeline-timed when the clock has one, ASAP otherwise. - let target = clock.and_then(|c| c.next_target(now_mono_ns, self.margin_ns)); + // Release: timeline-timed when the clock has one, ASAP otherwise. Under cadence targeting + // the floor is the frame's due time rather than this instant — the source's grid rather + // than the network's. + let target = + clock.and_then(|c| c.next_target(self.not_before_ns(now_mono_ns, frame.due_ns))); let released = match target { Some(t) => codec .release_output_buffer_at_time_by_index(frame.index, t.expected_present_ns) @@ -549,6 +660,12 @@ impl Presenter { /// codec-pure queued→decoded time) / `e2e` (capture→decoded, skew-corrected — the wireless /// A/B headline) / `vsync` (the measured panel period). /// + /// Under the smoothness intent it carries the cadence loop's health as well — `late‰` of all + /// frames folded (a due time already past when the frame became presentable: the direct signal + /// the cushion is too small, and WP8's acceptance criterion), `jitter` (the loop residual's + /// mean absolute deviation, our first honest per-stream jitter number), `cushion` and + /// `reanchors`. Absent under latency, where there is no loop. + /// /// Returns this window's CIRCULAR latch statistics `(vector-mean latch ns mod panel period, /// coherence ‰)` when a window actually flushed — the phase-lock reporter's v2 error signal /// (design/phase-locked-capture.md §6; the v1 median was immovable under jitter). @@ -577,6 +694,21 @@ impl Presenter { let period_ms = clock.map(|c| c.period_ns() as f64 / 1e6).unwrap_or(0.0); let panel_ns = clock.map(|c| c.panel_period_ns()).unwrap_or(0); let (outstanding, _) = meter.outstanding(); + // Cumulative over the session rather than this window (the loop's counters survive + // `reset`): `late` is a RATE question, and one second of it is too few frames to read a + // sub-percent criterion off. + let cadence = self + .cadence_health() + .map(|h| { + format!( + " late={}‰ jitterMs={:.2} cushionMs={:.2} reanchors={}", + h.late.saturating_mul(1000) / h.frames.max(1), + h.jitter_ns as f64 / 1e6, + h.cushion_ns as f64 / 1e6, + h.reanchors, + ) + }) + .unwrap_or_default(); log::info!( target: "pf.present", "released={} displays={} paced={} noBudget={} forced={} qDry={} \ @@ -584,7 +716,7 @@ impl Presenter { paceMs p50={:.2} max={:.2} latchMs p50={:.2} max={:.2} \ feedMs p50={:.2} max={:.2} codecMs p50={:.2} max={:.2} \ e2eMs p50={:.2} max={:.2} circ={:.2}ms coh={} \ - vsyncMs={:.2} panelMs={:.2}", + vsyncMs={:.2} panelMs={:.2}{}", self.released, displays, self.paced_drops, @@ -607,6 +739,7 @@ impl Presenter { circ.map(|(_, c)| c).unwrap_or(0), period_ms, panel_ns as f64 / 1e6, + cadence, ); self.released = 0; // Margin adaptation, off the MEASURED latch. A release targets the first grid point past @@ -671,3 +804,108 @@ pub(super) fn presenter_disabled_by_sysprop() -> bool { }; n > 0 && &buf[..n as usize] == b"arrival" } + +#[cfg(test)] +mod tests { + use super::*; + + const SOURCE_HZ: u32 = 120; + const P: i64 = 8_333_333; // one 120 Hz source interval, ns + /// A host stamping realtime, played out against a monotonic present clock — two different + /// eras, which is the whole reason the loop estimates the offset rather than being told it. + const PTS0_NS: i64 = 1_786_000_000_000_000_000; + const MONO0: i64 = 987_000_000_000; + const PTS0_US: u64 = (PTS0_NS / 1000) as u64; + + /// `k`'s pts as the CODEC echoes it — µs, truncated exactly as `feed_ready` queues it — and + /// the monotonic instant it became presentable, with `jitter_ns` of transport noise on the + /// arrival and none on the source stamp. + fn frame(k: i64, jitter_ns: i64) -> (u64, i64) { + (((PTS0_NS + k * P) / 1000) as u64, MONO0 + k * P + jitter_ns) + } + + #[test] + fn the_latency_intent_never_consults_the_cadence_clock() { + let mut p = Presenter::new(PresentPriority::Latency, SOURCE_HZ); + for k in 0..600 { + let (pts_us, ready) = frame(k, (k % 11) * 400_000); + assert_eq!( + p.due_at(pts_us, ready), + None, + "latency must produce no due time at all" + ); + } + assert!( + p.cadence_health().is_none(), + "latency holds no loop to have health" + ); + // …so the target floor is exactly what it was before the clock existed: this instant plus + // SurfaceFlinger's latch lead, and nothing else. + assert_eq!(p.not_before_ns(MONO0, None), MONO0 + p.margin_ns); + } + + #[test] + fn the_smooth_intent_puts_every_decoded_frame_through_the_loop() { + let mut p = Presenter::new(PresentPriority::Smooth { buffer: 2 }, SOURCE_HZ); + for k in 0..600 { + let (pts_us, ready) = frame(k, (k % 11) * 400_000); + assert!(p.due_at(pts_us, ready).is_some()); + } + let h = p.cadence_health().expect("smooth holds a loop"); + assert_eq!(h.frames, 600); + assert_eq!( + h.reanchors, 1, + "only the cold start anchors on a clean trace" + ); + } + + #[test] + fn a_due_time_ahead_of_now_moves_the_target_and_one_behind_it_does_not() { + let p = Presenter::new(PresentPriority::Smooth { buffer: 2 }, SOURCE_HZ); + let floor = MONO0 + p.margin_ns; + // Due ahead: the release aims at the source's grid, which is the entire change. + assert_eq!(p.not_before_ns(MONO0, Some(floor + P)), floor + P); + // Due already past — a late frame, which the loop deliberately reports unclamped: present + // at the next opportunity, never drag the grid back to the frame. + assert_eq!(p.not_before_ns(MONO0, Some(floor - 5 * P)), floor); + } + + #[test] + fn the_store_holds_a_frame_that_is_not_due_yet_and_releases_it_within_reach_of_its_slot() { + let mut p = Presenter::new(PresentPriority::Smooth { buffer: 2 }, SOURCE_HZ); + let due = MONO0 + 4 * P; + p.frames.push_back(HeldFrame { + index: 0, + pts_us: PTS0_US, + decoded_ns: 0, + due_ns: Some(due), + }); + assert!(!p.head_is_releasable(MONO0, P), "four refreshes early"); + assert!( + !p.head_is_releasable(due - P - p.margin_ns - 1, P), + "one nanosecond outside the submit reach of its own slot" + ); + assert!( + p.head_is_releasable(due - P - p.margin_ns, P), + "exactly one panel period of reach, the earliest that still buys nothing to wait" + ); + assert!( + p.head_is_releasable(due + 10 * P, P), + "late frames go at once" + ); + } + + #[test] + fn a_frame_carrying_no_due_time_is_always_releasable() { + // `HeldFrame` is shared with the latency intent, where the due time is always absent, so + // the predicate has to drain on a frame it cannot answer for rather than wedge behind it. + let mut p = Presenter::new(PresentPriority::Smooth { buffer: 2 }, SOURCE_HZ); + p.frames.push_back(HeldFrame { + index: 0, + pts_us: PTS0_US, + decoded_ns: 0, + due_ns: None, + }); + assert!(p.head_is_releasable(MONO0, P)); + } +} diff --git a/clients/android/native/src/decode/vsync.rs b/clients/android/native/src/decode/vsync.rs index a0ccfc55..b92c1f1c 100644 --- a/clients/android/native/src/decode/vsync.rs +++ b/clients/android/native/src/decode/vsync.rs @@ -93,16 +93,24 @@ impl VsyncShared { self.panel_period_ns.load(Ordering::Relaxed) } - /// The release target for a frame submitted at `now`: the earliest stored timeline whose - /// EXPECTED PRESENT is still `margin` away, extrapolated forward by whole periods once the - /// stored set has aged out (timelines refresh once per vsync callback; a frame can decode - /// anywhere inside that window). `None` on the 31/32 fallback — the caller releases ASAP. + /// The release target for a frame that must not be presented before `not_before_ns`: the + /// earliest stored timeline whose EXPECTED PRESENT is past that instant, extrapolated forward + /// by whole periods once the stored set has aged out (timelines refresh once per vsync + /// callback; a frame can decode anywhere inside that window). `None` on the 31/32 fallback — + /// the caller releases ASAP. + /// + /// The floor is the CALLER's to compose because two constraints meet in it and only the + /// caller knows the second: SurfaceFlinger's latch lead (`now + margin`, always) and, under + /// cadence targeting, the frame's own due time on the source's timeline. Adding the submit + /// lead here to a due time that is already an absolute present instant would push a frame due + /// just under a grid point onto the next one for some phases and not others — judder rather + /// than latency, which is the defect the due time exists to remove. /// /// Gated on `expected_present`, NOT the timeline's `deadline`, on purpose: the deadline /// budgets for GPU rendering the app has yet to submit (`presDeadline` — 11.3 ms on the /// A024, more than a full 120 Hz period), but a video buffer is already fully rendered — - /// the only real constraint is SurfaceFlinger's own latch lead, which is what the caller's - /// `margin` represents. Targeting by deadline cost every frame an extra refresh of waiting + /// the only real constraint is SurfaceFlinger's own latch lead, which is what the caller + /// folds into the floor. Targeting by deadline cost every frame an extra refresh of waiting /// (measured: latch p50 ~21 ms vs the ~2-interval floor); a mis-gamble here just means the /// frame presents one vsync later — exactly what the conservative gate always paid. /// @@ -110,10 +118,9 @@ impl VsyncShared { /// at the app's assigned render rate, but the panel latches at its own — when the app is /// down-rated (60 Hz callbacks on a 120 Hz panel) the reported timelines are a whole panel /// period apart or more, and pacing to them would cap the video. Pulling the target earlier - /// by whole panel periods (while its present still clears the margin) restores the true - /// grid; when callbacks run at the panel rate the pull condition is never true and this is - /// a no-op. - pub(super) fn next_target(&self, now_ns: i64, margin_ns: i64) -> Option { + /// by whole panel periods (while its present still clears the floor) restores the true grid; + /// when callbacks run at the panel rate the pull condition is never true and this is a no-op. + pub(super) fn next_target(&self, not_before_ns: i64) -> Option { let mut t = { let g = self .timelines @@ -121,7 +128,7 @@ impl VsyncShared { .unwrap_or_else(std::sync::PoisonError::into_inner); let found = g .iter() - .find(|t| t.expected_present_ns > now_ns + margin_ns) + .find(|t| t.expected_present_ns > not_before_ns) .copied(); match found { Some(t) => t, @@ -132,8 +139,8 @@ impl VsyncShared { return None; } // All stored timelines have passed — step the last one forward whole - // periods until its present clears `now + margin` again. - let behind = (now_ns + margin_ns).saturating_sub(last.expected_present_ns); + // periods until its present clears the floor again. + let behind = not_before_ns.saturating_sub(last.expected_present_ns); let k = behind / period + 1; FrameTimeline { expected_present_ns: last.expected_present_ns + k * period, @@ -144,7 +151,7 @@ impl VsyncShared { }; let panel = self.panel_period_ns.load(Ordering::Relaxed); if panel > 0 { - while t.expected_present_ns - panel > now_ns + margin_ns { + while t.expected_present_ns - panel > not_before_ns { t.deadline_ns -= panel; t.expected_present_ns -= panel; } -- 2.54.0 From 08a06ff295e2f3a99a405d7039188baef84c464e Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 15 Aug 2026 14:19:15 +0200 Subject: [PATCH 09/10] feat(apple): present on the host's cadence under smoothness, and prove the port agrees with the original MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP6 of design/presenter-cadence-rework-implementation-plan.md — the Apple binding of the CadenceClock landed in e207d0d7. Apple's video pipeline is Swift and does not link the Rust core, so this is a hand-written twin the way AudioRing is of JitterPolicy. That kind of port is only as good as its evidence of agreement, so beyond the ten mirrored unit tests the two implementations were run against each other numerically: disposable harnesses on both sides, importing the REAL Rust type, printing eight vectors — LCG output, the 400-frame settle state, head and tail due times, the ramp's type-2 versus type-1 error, a lumpy source's dues, cushion and jitter under wide jitter, and an outlier + gap + regression + off-cadence event sequence. All eight lines diffed identical, down to `offset=-1784999999987888109 skew=946 jitter=514645`. Both scaffolds are deleted; the doc comments name the Rust type and those vectors as the contract. The clock-domain trap is the whole risk here and it is worth stating plainly: `presentAtMediaTime` consumes `CACurrentMediaTime`, while `decodedNs` is CLOCK_REALTIME, and §2.2's rule is one domain in, same domain out. So `mediaTimeNs(forRealtimeNs:)` is written as the exact inverse of the existing media→realtime bridge, reading the two clocks in the same order so the sub-µs skew between them cancels on a round trip. The conversion happens ONCE, on the way in; the due time comes back in media time and is consumed by `nextVsync` and the store predicate with no second conversion — which is precisely what `domainOffsetIsAbsorbed` licenses. It is per frame rather than per session because the two clocks diverge across device sleep, the one case where the offset is not constant. The deadline presenter (iPhone/iPad) is included even though WP6 names only `presentAt`, which that loop has no equivalent of: its link vend IS the grid snap, so the due-gated `take` alone makes it cadence-driven. Leaving it out would have made WP8's "Mac + iPhone" leg test nothing on half its hardware. Preroll retires only on the due-gated path; plain `take()` and its tests are untouched. On the cadence path `underflows` is deliberately not counted — an empty store is the normal steady state once frames are held until due, and `CadenceHealth.late` is the honest starvation signal instead. Smoothness only: the clock exists iff the store policy is FIFO, so under `latency` `takeReady` is the old `ring.take()`, `dueMediaTime` is nil and `max(now, now)` is the old expression, and the stats line has no cadence segment. Verified: `swift test` 319 passing, 0 failures, including the ten new ones. ⚠ iOS/tvOS could not be typechecked — the checked-in xcframework's Info.plist declares only `macos-arm64`, which is pre-existing and unrelated. Confirmed instead that no added line sits inside any `#if os(...)`, so the macOS build typechecks the entire diff. --- .../PunktfunkKit/Video/Stage2Pipeline.swift | 440 +++++++++++++++++- .../PunktfunkKit/Video/VideoDecoder.swift | 6 + .../PunktfunkKitTests/CadenceClockTests.swift | 216 +++++++++ 3 files changed, 648 insertions(+), 14 deletions(-) create mode 100644 clients/apple/Tests/PunktfunkKitTests/CadenceClockTests.swift diff --git a/clients/apple/Sources/PunktfunkKit/Video/Stage2Pipeline.swift b/clients/apple/Sources/PunktfunkKit/Video/Stage2Pipeline.swift index 3a621ee7..bdf33e0b 100644 --- a/clients/apple/Sources/PunktfunkKit/Video/Stage2Pipeline.swift +++ b/clients/apple/Sources/PunktfunkKit/Video/Stage2Pipeline.swift @@ -162,6 +162,28 @@ public final class FrameStore: @unchecked Sendable { return f } + /// Cadence-driven take: hand back the oldest frame only once its DUE time has arrived, so the + /// store's job becomes "hold what is not due yet" instead of "release one per present + /// opportunity" (design/presenter-cadence-rework.md §4.3). `due` projects the frame's due + /// instant on the same clock as `now`; nil (no cadence estimate for that frame) means due + /// immediately. + /// + /// The preroll gate does not apply here. It exists only to build headroom for a per-slot + /// drain, and under cadence targeting the cushion IS the headroom — prerolling on top would + /// stack `capacity − 1` frames of standing latency the user never asked for. `underflows` is + /// not counted either: an empty store is the normal steady state once frames are held until + /// due, so the honest starvation signal is `CadenceHealth.late` (the due time had already + /// passed when the frame became presentable), not a run-dry count. + func take(dueBy now: CFTimeInterval, due: (Frame) -> CFTimeInterval?) -> Frame? { + lock.lock() + defer { lock.unlock() } + guard let oldest = frames.first else { return nil } + if let at = due(oldest), at > now { return nil } // held — not due yet + if isFifo { return frames.removeFirst() } + frames.removeAll(keepingCapacity: true) + return oldest + } + /// Return a frame the render thread took but could not present (no drawable yet, or a /// transient render failure). Newest-wins keeps it only while the slot is still empty — a /// newer decoded frame wins; FIFO reinserts it at the FRONT (it is the oldest; a transient @@ -205,6 +227,269 @@ private final class VsyncClock: @unchecked Sendable { } } +/// Tuning for one cadence loop. Gains are SHIFT COUNTS — the loop is fixed-point Int64 +/// throughout, so it runs identically on every client and in the offline harness, and carries no +/// float into a present path. +/// +/// ⚠ **These values are provisional, and saying so is part of the design.** The plan asks for +/// constants fitted to recorded `(src_pts, received, decoded)` traces (its spike S2), and S2 was +/// never run. What is here is derived from first principles — a proportional time constant of tens +/// of frames, an integral an order slower, a cushion of a few mean-absolute-deviations — and the +/// first real trace should replace them. +/// +/// ⚠ A hand-written port of `punktfunk_core::phase::CadenceTuning`, exactly as `AudioRing` is a +/// port of `JitterPolicy`: this pipeline is Swift and does not link that Rust type. **Every +/// constant and rule here must stay in lockstep with it** — `CadenceClockTests` and the Rust +/// `phase::tests` assert the two against the same synthetic input, and that agreement IS the +/// contract. +struct CadenceTuning: Equatable { + /// Proportional gain on the offset estimate: `1 >> offsetShift` of the residual per frame. + var offsetShift: UInt8 + /// Integral gain on the per-frame rate (skew) term: `1 >> skewShift`. + var skewShift: UInt8 + /// EMA weight for the residual mean-absolute-deviation. + var jitterShift: UInt8 + /// Per-sample residual clamp — one outlier must not yank the estimate. + var errorClampNs: Int64 + /// Cushion = `mad * cushionNum / cushionDen`, clamped to + /// `[cushionFloorNs, frameIntervalNs]`. + var cushionNum: UInt16 + var cushionDen: UInt16 + var cushionFloorNs: Int64 + /// Source-timestamp gap beyond which the loop re-anchors instead of tracking. + var reanchorGapNs: Int64 + + /// For callers that snap the due time onto a display grid afterwards: the snap-up itself + /// carries roughly half a refresh of implicit slack, so the cushion can be small. Every Apple + /// present snaps — onto `VsyncClock.nextVsync` under arrival/glass pacing, onto the + /// CAMetalDisplayLink's own vend under deadline pacing — so this is the one this client runs. + static func snapping() -> CadenceTuning { + CadenceTuning( + offsetShift: 5, skewShift: 10, jitterShift: 5, errorClampNs: 20_000_000, + cushionNum: 2, cushionDen: 1, cushionFloorNs: 500_000, + reanchorGapNs: 500_000_000) + } + + /// For callers presenting at the due time directly (VRR, direct scanout): no implicit slack, + /// so the cushion must cover more of the distribution on its own. + static func freeRunning() -> CadenceTuning { + var t = snapping() + t.cushionNum = 3 + t.cushionFloorNs = 2_000_000 + return t + } +} + +/// Loop health for the pf-present line — the numbers that say whether the cushion is doing its +/// job. Mirrors `punktfunk_core::phase::CadenceHealth`. +/// +/// Residual PERCENTILES are deliberately absent: this type holds no histogram, and the client +/// stat paths (the latency meters) are where distributions belong. +struct CadenceHealth: Equatable { + /// Frames folded since the last `reset`. + var frames: UInt64 = 0 + /// …of which the due time was already past when the frame became presentable. The direct + /// signal that the cushion is too small. + var late: UInt64 = 0 + /// Times the loop gave up tracking and re-anchored (gap, regression, or explicit reset). + var reanchors: UInt64 = 0 + var offsetNs: Int64 = 0 + var skewNs: Int64 = 0 + var jitterNs: Int64 = 0 + var cushionNs: Int64 = 0 +} + +/// Plays frames out on the SOURCE's cadence instead of on their arrival instant. +/// +/// The defect it exists for: every client presents a frame as soon as it is decoded, so the +/// transport's jitter — and, on a host whose compositor delivers raggedly, the compositor's — +/// lands on the glass 1:1. The 2026-08-15 Skynet field log has KWin's screencast arriving +/// 0.11–8.22 ms off its own grid (up to a full 120 Hz period) for 24 minutes on a session with the +/// bitrate pinned and zero loss. The loop estimates the offset between the source clock and the +/// present clock and hands back a due time on the source's own timeline plus a cushion sized to +/// the measured jitter. +/// +/// **Type-2 on purpose.** It tracks offset *and* per-frame rate, because two free-running crystals +/// produce a ramp and a proportional-only loop lags a ramp forever. +/// +/// **It smooths the offset, never the timestamps.** Due is `srcPts + offset + cushion`, so genuine +/// variation in the source's own cadence — a variable-rate renderer, an irregular capture tick — +/// passes straight through, and only the transport's contribution to `ready − pts` is filtered. +/// Anything that made due times more evenly spaced than the source would be a bug. +/// +/// **Domain-agnostic by construction.** A constant offset between clock domains is absorbed by the +/// offset estimator, so a caller feeds `readyNs` and reads the due time in ONE domain with no +/// conversion anywhere in this path. On Apple that domain is `CACurrentMediaTime` — the clock +/// `presentAtMediaTime` consumes — so the decode-output instant is converted ONCE on the way in +/// (`Stage2Pipeline.mediaTimeNs(forRealtimeNs:)`) and the due time comes back needing none. +/// Suspend/resume breaks the constant; the gap re-anchor below is what covers it. +/// +/// A late frame's due time is returned in the PAST, unclamped: clamping it to `readyNs` would +/// quietly turn every late frame into a fresh anchor, which is precisely the arrival-driven +/// presentation this exists to stop being. +/// +/// Prior art is ordinary and old: MPEG-2 TS PCR recovery and RTP playout scheduling (RFC 3550 +/// §6.4.1 carries the jitter estimator this MAD mirrors). +/// +/// ⚠ A hand-written port of `punktfunk_core::phase::CadenceClock` — see `CadenceTuning` for the +/// lockstep contract. Sendable; lock-guarded — the decode-completion thread folds frames while the +/// render thread reads health. +final class CadenceClock: @unchecked Sendable { + private let lock = NSLock() + private let tuning: CadenceTuning + /// `ready − srcPts`, smoothed. Absorbs the clock-domain constant. + private var offsetNs: Int64 = 0 + /// Per-frame drift of that offset — the integral term. + private var skewNs: Int64 = 0 + /// EMA of |residual|, the cushion's input. + private var madNs: Int64 = 0 + /// nil until the first sample anchors the loop. + private var lastPtsNs: UInt64? + /// Last frame interval seen, so `cushionNs` can apply its ceiling. + private var frameIntervalNs: Int64 = 0 + private var counters = CadenceHealth() + + init(tuning: CadenceTuning) { + self.tuning = tuning + } + + /// Force a re-anchor on the next sample. Call on every discontinuity the client already knows + /// about: reanchor, codec rebuild, surface recreate, jump-to-live, resume. + func reset() { + lock.lock() + lastPtsNs = nil + skewNs = 0 + // `madNs` deliberately SURVIVES. It describes the link, not the stream, and a cushion that + // collapsed to its floor at every rebuild would spend the next few hundred frames + // presenting late — the exact failure the cushion exists to prevent. + lock.unlock() + } + + /// Fold one presentable frame and return when it is due, in the present clock domain. + /// + /// `readyNs` is when the frame became presentable; `frameIntervalNs` is the nominal source + /// interval and the cushion's ceiling. + /// + /// The result **may be earlier than `readyNs`** — that is a late frame, and the caller's + /// contract is "already due ⇒ present at the next opportunity", never "drag the grid back to + /// now". + func dueNs(srcPtsNs: UInt64, readyNs: Int64, frameIntervalNs: Int64) -> Int64 { + lock.lock() + defer { lock.unlock() } + self.frameIntervalNs = frameIntervalNs + counters.frames += 1 + let pts = Int64(bitPattern: srcPtsNs) + let raw = saturatingSub(readyNs, pts) + + let anchored: Bool + if let last = lastPtsNs { + // Source time going BACKWARDS, or a gap so long the estimate cannot be trusted to + // have tracked across it: re-anchor rather than slew for seconds. + anchored = + !(srcPtsNs < last + || srcPtsNs - last > UInt64(bitPattern: tuning.reanchorGapNs)) + } else { + anchored = false + } + if anchored { + // Advance the estimate one frame on the rate term, then correct it by a bounded + // fraction of what the new sample says. + offsetNs = saturatingAdd(offsetNs, skewNs) + let err = min( + max(saturatingSub(raw, offsetNs), -tuning.errorClampNs), tuning.errorClampNs) + offsetNs = saturatingAdd(offsetNs, shrTowardZero(err, tuning.offsetShift)) + skewNs = saturatingAdd(skewNs, shrTowardZero(err, tuning.skewShift)) + let dev = abs(err) - madNs + madNs = saturatingAdd(madNs, shrTowardZero(dev, tuning.jitterShift)) + } else { + offsetNs = raw + skewNs = 0 + counters.reanchors += 1 + } + lastPtsNs = srcPtsNs + + let due = saturatingAdd(saturatingAdd(pts, offsetNs), lockedCushionNs()) + if due < readyNs { counters.late += 1 } + return due + } + + /// A frame whose timestamp is not on the source cadence — a repeat the host re-anchored at + /// submit, a stamp its plausibility gate replaced with "now", or one that reached us with no + /// usable pts at all. Those samples do not lie on the source's timeline, and folding them in + /// would drag the offset estimate toward "now" exactly when the stream is idle and the + /// estimate matters most. + /// + /// Returns a due time from the CURRENT estimate, leaving offset, skew and jitter untouched: + /// the frame is simply due once it is ready, cushioned like any other. + func noteOffCadence(readyNs: Int64, frameIntervalNs: Int64) -> Int64 { + lock.lock() + defer { lock.unlock() } + self.frameIntervalNs = frameIntervalNs + return saturatingAdd(readyNs, lockedCushionNs()) + } + + func jitterNs() -> Int64 { + lock.lock() + defer { lock.unlock() } + return madNs + } + + /// How far past the estimate a frame is held, to absorb the measured jitter. + /// + /// The one-frame-interval ceiling is an INVARIANT, not a tunable: a cushion past a whole frame + /// buys latency for smoothness the source cannot supply, and at that point the honest fix is a + /// deeper buffer the user asked for, not a loop quietly holding frames. + func cushionNs() -> Int64 { + lock.lock() + defer { lock.unlock() } + return lockedCushionNs() + } + + func health() -> CadenceHealth { + lock.lock() + defer { lock.unlock() } + var h = counters + h.offsetNs = offsetNs + h.skewNs = skewNs + h.jitterNs = madNs + h.cushionNs = lockedCushionNs() + return h + } + + private func lockedCushionNs() -> Int64 { + let den = Int64(max(tuning.cushionDen, 1)) + let want = saturatingMul(madNs, Int64(tuning.cushionNum)) / den + let ceiling = frameIntervalNs > 0 ? frameIntervalNs : Int64.max + return min(max(want, min(tuning.cushionFloorNs, ceiling)), ceiling) + } +} + +/// Arithmetic shift that rounds toward ZERO, so a negative residual is damped by exactly as much +/// as its positive twin. A plain `>>` rounds toward −∞, which biases a loop that spends its whole +/// life within a few nanoseconds of zero error. +private func shrTowardZero(_ v: Int64, _ shift: UInt8) -> Int64 { + v < 0 ? -((-v) >> shift) : v >> shift +} + +/// The Rust loop is `saturating_*` throughout — a garbage timestamp must clamp the estimate, never +/// trap the render thread. Swift's operators trap instead of saturating, so the port carries its +/// own; for every reachable input these are plain `+`, `−`, `×`. +private func saturatingAdd(_ a: Int64, _ b: Int64) -> Int64 { + let (v, overflow) = a.addingReportingOverflow(b) + return overflow ? (b > 0 ? Int64.max : Int64.min) : v +} + +private func saturatingSub(_ a: Int64, _ b: Int64) -> Int64 { + let (v, overflow) = a.subtractingReportingOverflow(b) + return overflow ? (b > 0 ? Int64.min : Int64.max) : v +} + +private func saturatingMul(_ a: Int64, _ b: Int64) -> Int64 { + let (v, overflow) = a.multipliedReportingOverflow(by: b) + guard overflow else { return v } + return (a > 0) == (b > 0) ? Int64.max : Int64.min +} + /// When a ready frame is pushed to the layer — the stage-2 vs stage-3 presenter split. Same decode /// half, same newest-wins ring; only the present cadence differs. /// @@ -329,7 +614,8 @@ public final class PresentLinkInfo: @unchecked Sendable { } } -/// Deadline pacing's staged frame-rate hint. SessionPresenter pushes the stream rate from the +/// Deadline pacing's staged frame-rate hint, and — on every pacing — the session's nominal source +/// interval (`sourceIntervalNs`). SessionPresenter pushes the stream rate from the /// MAIN thread (session start + every layout/Reconfigure); the link's own thread drains and /// applies it, so the CAMetalDisplayLink is only ever touched from the thread that runs it. The /// floor is PINNED at the stream rate — no idle ramp-down: with a low floor the link idles toward @@ -369,6 +655,16 @@ private final class FrameRateHint: @unchecked Sendable { pending = nil return p } + /// The nominal SOURCE interval in nanoseconds — the cadence clock's cushion ceiling. Read on + /// every pacing, not just deadline: this box is where the negotiated stream rate already + /// lives, staged from main on session start and every Reconfigure, and the decode-completion + /// thread needs it under a lock. 0 = not known yet, which the clock handles by running its + /// cushion uncapped (the shared core's own behaviour for a zero interval). + func sourceIntervalNs() -> Int64 { + lock.lock() + defer { lock.unlock() } + return streamHz > 0 ? Int64(1_000_000_000.0 / Double(streamHz)) : 0 + } private static func range(hz: Float, boosted: Bool) -> CAFrameRateRange { #if os(tvOS) // A TV is a FIXED-rate display: there is no ProMotion panel to lift and no Pencil to @@ -663,6 +959,11 @@ final class PresentGate: @unchecked Sendable { /// between system-reported on-glass times (vsync-aligned presents show clean refresh-period /// multiples; immediate flips scatter). Lock-guarded — `presented` lands on a Metal callback thread. private final class PresentDebugStats: @unchecked Sendable { + /// The session's cadence loop, for the line's `cadence` segment — `nil` under the latency + /// intent, and then the line is emitted exactly as it was before source-timestamp playout + /// existed. `late` is the number WP8 gates on: a due time already past when the frame became + /// presentable is the direct signal that the cushion is too small. + private let cadence: CadenceClock? private let lock = NSLock() private var last = CACurrentMediaTime() private var ok = 0, failed = 0, empty = 0, dropped = 0, gated = 0, noDrawable = 0 @@ -685,6 +986,10 @@ private final class PresentDebugStats: @unchecked Sendable { private var inFlight = 0 private var maxInFlight = 0 + init(cadence: CadenceClock?) { + self.cadence = cadence + } + func emptyWake() { lock.lock(); empty += 1; lock.unlock() } /// A wake that found the stage-3 gate closed (a present still in flight) — the frame stays in @@ -743,6 +1048,17 @@ private final class PresentDebugStats: @unchecked Sendable { let vendP50 = vends.isEmpty ? 0 : vends[vends.count / 2] let vendMax = vends.last ?? 0 let inflightMax = maxInFlight + // Loop health, appended only where a loop exists — `late`/`frames` is WP8's cushion + // criterion and `reanchor` says whether the estimate is tracking at all. + let loop = cadence?.health() + let cadenceLine = + loop.map { + String( + format: " cadence late=%llu/%llu reanchor=%llu jitterUs=%lld cushionUs=%lld " + + "skewNs=%lld", + $0.late, $0.frames, $0.reanchors, $0.jitterNs / 1000, $0.cushionNs / 1000, + $0.skewNs) + } ?? "" let line = String( format: "pf-present decoded=%d ok=%d fail=%d empty=%d gated=%d noDrawable=%d " + "dropped=%d qDrop=%d qDry=%d maxRenderMs=%.1f inflightMax=%d forced=%d " @@ -751,7 +1067,7 @@ private final class PresentDebugStats: @unchecked Sendable { decoded, ok, failed, empty, gated, noDrawable, dropped, smoothing.overflowDrops, smoothing.underflows, maxRenderMs, inflightMax, gate?.drainForced() ?? 0, p50, dMax, deltas.count, latchP50, latchMax, - vendP50, vendMax) + vendP50, vendMax) + cadenceLine ok = 0; failed = 0; empty = 0; dropped = 0; gated = 0; noDrawable = 0 maxRenderMs = 0 maxInFlight = inFlight // the window peak restarts from the live depth @@ -806,6 +1122,11 @@ public final class Stage2Pipeline { /// at most one per vsync, so the FIFO store drains on the display's cadence rather than on /// arrival. Ignored under `.deadline` (the link IS the cadence there). private let vsyncPaced: Bool + /// Source-timestamp playout for the SMOOTHNESS intent: every decoded frame is stamped with + /// when it is due on the host's own cadence, and the present decision aims there instead of at + /// the moment the frame happened to decode. `nil` under `latency`, whose path keeps no cadence + /// arithmetic in it at all — see the intent gate in `init`. + private let cadence: CadenceClock? private let endToEndMeter: LatencyMeter? private let decodeMeter: LatencyMeter? private let displayMeter: LatencyMeter? @@ -885,12 +1206,23 @@ public final class Stage2Pipeline { self.decodeMeter = decodeMeter self.displayMeter = displayMeter self.presentFloorMeter = presentFloorMeter + // The intent gate: source-timestamp playout is what `smooth` MEANS now, and `latency` is + // defined as arrival-driven with no cushion — so the store policy, which is the intent's + // only other expression (`PresentPriority.storePolicy`: smooth → fifo, latency → + // newest-wins), is what decides whether a clock exists at all. A latency session runs the + // same present path it ran before this existed. + switch storePolicy { + case .newestWins: self.cadence = nil + case .fifo: self.cadence = CadenceClock(tuning: .snapping()) + } let ring = ring let recovery = recovery let renderSignal = renderSignal let gate = gate let decodeReport = decodeReport let phaseReporter = phaseReporter + let cadence = cadence + let rateHint = frameRateHint self.decoder = VideoDecoder( onDecoded: { frame in // Decode stage = received→decoded, both client CLOCK_REALTIME (offset 0 — no @@ -911,7 +1243,11 @@ public final class Stage2Pipeline { // present) on a proven clean re-anchor (IDR / RFI anchor / 2nd recovery mark) or the // bounded backstop. decoderKeyframe=false: VT doesn't flag IDRs, the wire FLAG_SOF does. guard gate.onDecoded(flags: frame.flags) else { return } - ring.submit(frame) + // Decoder OUTPUT is where the cadence loop is sampled — the instant the frame + // becomes presentable. Receipt would not model decode at all and could hand back a + // due time already past by the moment the frame exists; dequeue would fold the + // present path's own wait into the estimate and make the loop chase its output. + ring.submit(Stage2Pipeline.dated(frame, by: cadence, hint: rateHint)) // FRAME ARRIVAL is the render trigger (never the display link — see the header). renderSignal.signal() }, @@ -936,6 +1272,11 @@ public final class Stage2Pipeline { decodeReport.bind(connection) // arm the Automatic-bitrate decode signal for this session phaseReporter.bind(connection) // arm phase reports (flushed only by the deadline link) gate.reseed(framesDropped: connection.framesDropped()) // baseline the freeze to this session + // A fresh session is a fresh source clock: re-anchor on its first frame rather than slew + // for seconds off the previous host's offset. (Mid-session discontinuities — background + // resume, a stream idle under the infinite GOP — arrive as a source-timestamp gap the loop + // re-anchors on by itself; this seam covers the one it cannot see.) + cadence?.reset() token = StopFlag() // fresh token per start — a stop is permanent (like StreamPump) // Configure the decoder's chroma + the layer's initial colorimetry before the first frame. The @@ -962,7 +1303,7 @@ public final class Stage2Pipeline { connection: connection, token: token, pumpStopped: pumpStopped, ring: ring, renderSignal: renderSignal, device: presenter.metalDevice, queue: presenter.metalQueue, - decodeMeter: decodeMeter, + decodeMeter: decodeMeter, cadence: cadence, rateHint: frameRateHint, onFrame: onFrame, onSessionEnd: onSessionEnd, onDecodedSize: onDecodedSize) } else { thread = Thread { @@ -1088,7 +1429,8 @@ public final class Stage2Pipeline { // startDeadlinePresenter. The V-Sync policy below doesn't apply there (the link deadline- // times every present). Deadline sessions ALWAYS carry the stats (their pf-present line // streams to Console.app via presentLog — the on-device pacing decomposition). - let debugStats = (presentDebug || pacing == .deadline) ? PresentDebugStats() : nil + let debugStats = + (presentDebug || pacing == .deadline) ? PresentDebugStats(cadence: cadence) : nil if pacing == .deadline { startDeadlinePresenter(debugStats: debugStats) return @@ -1118,6 +1460,12 @@ public final class Stage2Pipeline { // Stage-3's bounded in-flight present gate; nil = stage-2's present-on-arrival. A local // (like the ring) so neither the render thread nor the presented handlers capture `self`. let gate: PresentGate? = pacing == .glass ? PresentGate(capacity: gateDepth) : nil + // Cadence targeting turns the store into a holding buffer: a frame comes out once it is + // DUE, not once a present opportunity exists (§4.3). The latency intent has no clock and + // keeps the unconditional take, byte for byte. + let takeReady: () -> ReadyFrame? = cadence == nil + ? { ring.take() } + : { ring.take(dueBy: CACurrentMediaTime(), due: { $0.dueMediaTime }) } let renderThread = Thread { defer { renderStopped.signal() } // macOS smoothness: the vsync this thread last presented onto — at most ONE present @@ -1150,7 +1498,7 @@ public final class Stage2Pipeline { debugStats?.flushIfDue(ring: ring, gate: gate) return } - guard !token.isStopped, let frame = ring.take() else { + guard !token.isStopped, let frame = takeReady() else { gate?.release() // armed but nothing to render — don't hold the gate stale debugStats?.emptyWake() debugStats?.flushIfDue(ring: ring, gate: gate) @@ -1158,8 +1506,14 @@ public final class Stage2Pipeline { } // V-Sync ON: flip on the next predicted vsync (< one period out, stale link ⇒ // immediate — see VsyncClock). OFF: flip as soon as the GPU finishes. + // + // Under cadence targeting the grid is entered at the frame's DUE time rather than + // at this instant, so two frames the host emitted one period apart land one period + // apart on glass however unevenly they arrived. Never before `now`: a due time in + // the past means the frame is late, not that the grid moves back. + let now = CACurrentMediaTime() let presentAt = vsyncEnabled - ? vsyncClock.nextVsync(after: CACurrentMediaTime()) : nil + ? vsyncClock.nextVsync(after: max(now, frame.dueMediaTime ?? now)) : nil let renderStarted = CACurrentMediaTime() let issuedNs = Stage2Pipeline.realtimeNs(forDisplayLinkTimestamp: renderStarted) let onGlass: (Int64?) -> Void = { presentedNs in @@ -1238,6 +1592,13 @@ public final class Stage2Pipeline { let hint = frameRateHint let layer = presenter.layer let stash = LatestBox() + // Cadence targeting under deadline pacing: the link's vend IS the grid snap, so the clock + // only has to hold a frame back until it is due and the next update presents it — at most + // one refresh later. Same holding-buffer rule as the arrival/glass loop (§4.3); latency + // sessions have no clock and take unconditionally. + let takeReady: () -> ReadyFrame? = cadence == nil + ? { ring.take() } + : { ring.take(dueBy: CACurrentMediaTime(), due: { $0.dueMediaTime }) } // ⭐ Shrink the drawable pool to 2 for THIS pacing — the measured fix for a present floor // stuck at two refreshes (field 2026-08-13, Apple TV 4K / tvOS 27: `os present +32.5` at @@ -1328,7 +1689,7 @@ public final class Stage2Pipeline { // layer's CURRENT config, so drawableSize/format have to be right before a vend // can succeed at all (see reconcileLayer — the session-start bootstrap, where // the layer still has its initial 0×0 size and every vend fails allocation). - guard !token.isStopped, let frame = ring.take() else { + guard !token.isStopped, let frame = takeReady() else { debugStats?.emptyWake() debugStats?.flushIfDue(ring: ring, gate: nil) return @@ -1409,8 +1770,9 @@ public final class Stage2Pipeline { /// MAIN thread (SessionPresenter — session start + every layout/Reconfigure): hint the /// deadline link with the stream cadence. Staged; the link's own thread applies it (see - /// `FrameRateHint`). No-op under arrival/glass pacing, where the hosting view's CADisplayLink - /// is the hinted link. + /// `FrameRateHint`). Under arrival/glass pacing no link reads it — the hosting view's + /// CADisplayLink is the hinted one there — but the stored rate is still the cadence clock's + /// nominal source interval, and hence its cushion ceiling, on every pacing. public func setFrameRateHint(hz: Float) { frameRateHint.stage(hz: hz) } @@ -1489,7 +1851,7 @@ public final class Stage2Pipeline { connection: PunktfunkConnection, token: StopFlag, pumpStopped: DispatchSemaphore, ring: FrameStore, renderSignal: DispatchSemaphore, device: MTLDevice, queue: MTLCommandQueue, - decodeMeter: LatencyMeter?, + decodeMeter: LatencyMeter?, cadence: CadenceClock?, rateHint: FrameRateHint, onFrame: (@Sendable (AccessUnit) -> Void)?, onSessionEnd: (@Sendable () -> Void)?, onDecodedSize: (@Sendable (Int, Int) -> Void)? @@ -1547,10 +1909,15 @@ public final class Stage2Pipeline { Int64(ts.tv_sec) * 1_000_000_000 + Int64(ts.tv_nsec) decodeMeter?.record( ptsNs: UInt64(receivedNs), atNs: decodedNs, offsetNs: 0) + // Same cadence sample as the VideoToolbox half: the wavelet decode's + // completion IS this frame's presentable instant. ring.submit( - ReadyFrame( - ptsNs: ptsNs, receivedNs: receivedNs, decodedNs: decodedNs, - image: .planar(planes), flags: flags)) + Stage2Pipeline.dated( + ReadyFrame( + ptsNs: ptsNs, receivedNs: receivedNs, + decodedNs: decodedNs, image: .planar(planes), + flags: flags), + by: cadence, hint: rateHint)) renderSignal.signal() } if submitted { @@ -1585,5 +1952,50 @@ public final class Stage2Pipeline { let realtimeNow = Int64(ts.tv_sec) * 1_000_000_000 + Int64(ts.tv_nsec) return realtimeNow + Int64((t - caNow) * 1_000_000_000) } + + /// The exact inverse: a client `CLOCK_REALTIME` nanosecond instant (`ReadyFrame.decodedNs`) + /// expressed on the `CACurrentMediaTime` timeline the present path schedules against. + /// + /// It reads the two clocks in the SAME ORDER as `realtimeNs(forDisplayLinkTimestamp:)` above + /// and forms the same difference, so the sub-microsecond skew between the two reads is the + /// same sign in both and cancels on a round trip. + /// + /// The cadence loop needs this because its rule is one domain in, SAME domain out: it is fed + /// the decode-output instant in media time and its due time comes back in media time, with no + /// second conversion anywhere downstream. (A constant realtime↔media offset would be absorbed + /// by the loop's own offset estimator and need no conversion at all — but the two clocks + /// diverge across device sleep, which is exactly why the conversion is done per frame here + /// rather than once per session.) + static func mediaTimeNs(forRealtimeNs t: Int64) -> Int64 { + let caNow = CACurrentMediaTime() + var ts = timespec() + clock_gettime(CLOCK_REALTIME, &ts) + let realtimeNow = Int64(ts.tv_sec) * 1_000_000_000 + Int64(ts.tv_nsec) + return Int64(caNow * 1_000_000_000) + (t - realtimeNow) + } + + /// Stamp a decoded frame with when it is DUE on the source's cadence, at the moment it enters + /// the ready store. Returns the frame untouched when the session has no clock (the latency + /// intent). + /// + /// A frame whose wire pts did not survive (`ptsNs == 0` — the decoder's "unknown" value) is + /// not on the source's timeline at all, so it is folded through `noteOffCadence`: due as soon + /// as it is ready, and the estimate left alone. Folding "now" in would drag the offset toward + /// this instant precisely when the loop has the least evidence. + private static func dated( + _ frame: ReadyFrame, by clock: CadenceClock?, hint: FrameRateHint + ) -> ReadyFrame { + guard let clock else { return frame } + let readyNs = mediaTimeNs(forRealtimeNs: frame.decodedNs) + let interval = hint.sourceIntervalNs() + let dueNs = + frame.ptsNs > 0 + ? clock.dueNs( + srcPtsNs: frame.ptsNs, readyNs: readyNs, frameIntervalNs: interval) + : clock.noteOffCadence(readyNs: readyNs, frameIntervalNs: interval) + var dated = frame + dated.dueMediaTime = Double(dueNs) / 1_000_000_000 + return dated + } } #endif diff --git a/clients/apple/Sources/PunktfunkKit/Video/VideoDecoder.swift b/clients/apple/Sources/PunktfunkKit/Video/VideoDecoder.swift index c8d3be40..f845b726 100644 --- a/clients/apple/Sources/PunktfunkKit/Video/VideoDecoder.swift +++ b/clients/apple/Sources/PunktfunkKit/Video/VideoDecoder.swift @@ -47,6 +47,12 @@ public struct ReadyFrame: @unchecked Sendable { /// context so the re-anchor gate can classify this decoded frame (IDR / RFI anchor / recovery /// mark) at present time — the async decode callback has no other access to it. 0 when unknown. public let flags: UInt32 + /// When this frame is DUE on the SOURCE's cadence, in `CACurrentMediaTime` seconds — the + /// domain the present path schedules against (`presentAtMediaTime`, `VsyncClock`). Stamped + /// where the frame enters the ready store, by the pipeline's `CadenceClock`; `nil` under the + /// latency intent, which has no clock and presents on arrival. May be in the PAST: that is a + /// late frame, and the contract is "already due ⇒ present at the next opportunity". + public var dueMediaTime: CFTimeInterval? /// The VideoToolbox path's buffer; nil for a PyroWave planar frame. (Kept as the accessor /// the decode round-trip tests assert against.) diff --git a/clients/apple/Tests/PunktfunkKitTests/CadenceClockTests.swift b/clients/apple/Tests/PunktfunkKitTests/CadenceClockTests.swift new file mode 100644 index 00000000..fe2ece60 --- /dev/null +++ b/clients/apple/Tests/PunktfunkKitTests/CadenceClockTests.swift @@ -0,0 +1,216 @@ +import XCTest + +#if canImport(Metal) +@testable import PunktfunkKit + +/// Source-timestamp playout: the Swift `CadenceClock` against the SAME synthetic inputs its Rust +/// original runs (`punktfunk_core::phase::tests`) — one test per Rust test, matching names, the +/// same constants, the same deterministic LCG. The port is hand-written because this pipeline does +/// not link the Rust type, so agreement on these vectors is the whole lockstep contract: a +/// constant or a rounding rule that drifts on either side fails here, not on a user's screen. +final class CadenceClockTests: XCTestCase { + /// 120 Hz in ns. + private static let p: Int64 = 8_333_333 + + /// A source stamping realtime, played out by a client whose present clock is monotonic and + /// therefore a whole different era. The loop must never need to be told about this. + private static let pts0: UInt64 = 1_786_000_000_000_000_000 + private static let domain: Int64 = -1_785_000_000_000_000_000 + /// Transport + decode: what `ready − pts` sits at once the domain is taken out. + private static let delay: Int64 = 12_000_000 + + /// Deterministic LCG in ±spread around zero — no OS randomness in tests. The multiplier, + /// increment and the `>> 33` fold are the Rust harness's, so both sides replay the identical + /// jitter sequence for a given seed. + private struct Lcg { + private var state: UInt64 + init(_ seed: UInt64) { state = seed } + mutating func noise(_ spreadNs: Int64) -> Int64 { + state = state &* 6_364_136_223_846_793_005 &+ 1_442_695_040_888_963_407 + guard spreadNs != 0 else { return 0 } + return Int64(state >> 33) % (2 * spreadNs) - spreadNs + } + } + + private static func ptsAt(_ k: Int64) -> UInt64 { + UInt64(bitPattern: Int64(pts0) + k * p) + } + + /// Run `n` frames of a well-behaved 120 Hz source and hand back the clock. + private static func settled(_ n: Int64, spread: Int64) -> CadenceClock { + let c = CadenceClock(tuning: .snapping()) + var rng = Lcg(7) + for k in 0.. Int64 { + let c = CadenceClock(tuning: tuning) + var lastErr: Int64 = 0 + for k in Int64(0)..<4_000 { + let pts = Int64(bitPattern: Self.ptsAt(k)) + let ready = pts + Self.domain + Self.delay + k * ramp + _ = c.dueNs(srcPtsNs: Self.ptsAt(k), readyNs: ready, frameIntervalNs: Self.p) + lastErr = (ready - pts) - c.health().offsetNs + } + return abs(lastErr) + } + let type2 = run(.snapping()) + // The same loop with its integral gain switched off: a shift this large truncates every + // residual to zero, which is exactly "proportional only". + var type1Tuning = CadenceTuning.snapping() + type1Tuning.skewShift = 63 + let type1 = run(type1Tuning) + XCTAssertLessThan( + type2 * 4, type1, + "a rate term must beat proportional-only on a ramp: \(type2) ns vs \(type1) ns") + XCTAssertLessThan(type2, 3_000, "steady-state ramp error \(type2) ns") + } + + func testRejectsASingleOutlier() { + let c = Self.settled(400, spread: 200_000) + let before = c.health().offsetNs + // One frame arrives half a second late — a stall, not a new operating point. + let k: Int64 = 400 + _ = c.dueNs( + srcPtsNs: Self.ptsAt(k), + readyNs: Int64(bitPattern: Self.ptsAt(k)) + Self.domain + Self.delay + 500_000_000, + frameIntervalNs: Self.p) + let moved = abs(c.health().offsetNs - before) + // The clamped correction, plus the one frame of rate the loop advances by regardless — + // that advance is the estimate doing its job, not the outlier moving it. + let t = CadenceTuning.snapping() + let bound = (t.errorClampNs >> t.offsetShift) + abs(c.health().skewNs) + XCTAssertLessThanOrEqual( + moved, bound, "one outlier moved the estimate \(moved) ns, past the clamp's \(bound)") + } + + func testReanchorsOnAGap() { + let c = Self.settled(400, spread: 200_000) + let anchors = c.health().reanchors + // The stream was paused for two seconds; the estimate cannot have tracked across that. + let far = Self.ptsAt(400) + 2_000_000_000 + let ready = Int64(bitPattern: far) + Self.domain + Self.delay + 4_000_000 + _ = c.dueNs(srcPtsNs: far, readyNs: ready, frameIntervalNs: Self.p) + XCTAssertEqual(c.health().reanchors, anchors + 1) + XCTAssertEqual( + c.health().offsetNs, ready - Int64(bitPattern: far), + "a re-anchor adopts the new sample outright rather than slewing to it") + } + + func testReanchorsOnRegression() { + let c = Self.settled(400, spread: 200_000) + let anchors = c.health().reanchors + let back = Self.ptsAt(200) // source timestamps went backwards + _ = c.dueNs( + srcPtsNs: back, readyNs: Int64(bitPattern: back) + Self.domain + Self.delay, + frameIntervalNs: Self.p) + XCTAssertEqual(c.health().reanchors, anchors + 1) + } + + /// A due time in the past is returned AS IS. Clamping it to `readyNs` would quietly turn every + /// late frame into a fresh anchor, which is how an arrival-driven presenter behaves — the + /// thing this clock exists to stop being. + func testLateFrameReturnsPastDue() { + let c = Self.settled(400, spread: 200_000) + let k: Int64 = 400 + let ready = Int64(bitPattern: Self.ptsAt(k)) + Self.domain + Self.delay + 30_000_000 + let due = c.dueNs(srcPtsNs: Self.ptsAt(k), readyNs: ready, frameIntervalNs: Self.p) + XCTAssertLessThan(due, ready, "a frame that arrived 30 ms late must read as already due") + XCTAssertEqual(c.health().late, 1) + } + + func testOffCadenceDoesNotMoveTheLoop() { + let c = Self.settled(400, spread: 500_000) + let before = c.health() + let due = c.noteOffCadence(readyNs: 1_000_000, frameIntervalNs: Self.p) + let after = c.health() + XCTAssertEqual(before.offsetNs, after.offsetNs) + XCTAssertEqual(before.skewNs, after.skewNs) + XCTAssertEqual(before.jitterNs, after.jitterNs) + XCTAssertEqual(before.frames, after.frames, "and it is not a cadence sample") + XCTAssertEqual(due, 1_000_000 + c.cushionNs()) + } + + /// One domain in, same domain out: shifting the whole present-side trace by an arbitrary + /// constant must change every due time by exactly that constant and nothing else. This is what + /// lets each client feed its own clock without a conversion in the path — and on Apple it is + /// what makes `mediaTimeNs(forRealtimeNs:)` the ONE conversion in the whole loop. + func testDomainOffsetIsAbsorbed() { + let shift: Int64 = 987_654_321_000 + func run(_ extra: Int64) -> [Int64] { + let c = CadenceClock(tuning: .snapping()) + var rng = Lcg(11) + return (Int64(0)..<300).map { k in + let ready = + Int64(bitPattern: Self.ptsAt(k)) + Self.domain + Self.delay + extra + + rng.noise(2_000_000) + return c.dueNs(srcPtsNs: Self.ptsAt(k), readyNs: ready, frameIntervalNs: Self.p) + } + } + let a = run(0) + let b = run(shift) + for (i, pair) in zip(a, b).enumerated() { + XCTAssertEqual( + pair.1 - pair.0, shift, "frame \(i) shifted by \(pair.1 - pair.0) not \(shift)") + } + } + + /// The invariant that separates this from a metronome: a source that genuinely runs at an + /// irregular rate is REPRODUCED, not evened out. Anything that made these due spacings more + /// uniform than the source's own would be a bug. + func testPreservesSourceCadence() { + let c = CadenceClock(tuning: .snapping()) + // A deliberately lumpy source: alternating short and long frames. + let spacings = (0..<300).map { $0 % 2 == 0 ? Self.p / 2 : Self.p * 3 / 2 } + var pts = Self.pts0 + var dues: [Int64] = [] + var ptss: [Int64] = [] + var rng = Lcg(13) + for s in spacings { + pts = UInt64(bitPattern: Int64(bitPattern: pts) + s) + let ready = Int64(bitPattern: pts) + Self.domain + Self.delay + rng.noise(500_000) + ptss.append(Int64(bitPattern: pts)) + dues.append(c.dueNs(srcPtsNs: pts, readyNs: ready, frameIntervalNs: Self.p)) + } + // Compare the back half, once the loop has settled. + for i in 200.. Date: Sat, 15 Aug 2026 14:24:27 +0200 Subject: [PATCH 10/10] feat(desktop): present on the host's cadence under smoothness, and let VRR finally be worth something MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP5 of design/presenter-cadence-rework-implementation-plan.md — the desktop binding of the CadenceClock landed in e207d0d7. `target != last_target_ns` is gone, and `last_target_ns` with it. That equality was defect D3 verbatim: one present per latch slot, decided by slot identity rather than by when the frame was actually due. In its place a frame is served when its own due time says so — under snapping, `due < next_slot_after(now + margin)`. That `<` is derived rather than picked: `next_slot_after` is monotone, so "this frame's target slot is no later than the earliest slot still reachable" reduces to exactly it. Desktop is §5's simplest row and the code says so — `decoded_ns` and `session::now_ns()` are both CLOCK_REALTIME, so `ready_ns` goes in raw and no conversion appears anywhere in the path. The domain check is still asserted, with source stamps 56 years away from the present clock. **VRR stops being merely not-worse.** Where the presenter has MEASURED variable refresh — the existing `CadenceProbe` verdict, not a capability bit — the snap is skipped and the frame is presented at its due time under `free_running()` tuning. `Unknown` reverts to snapping: the absence of a measurement is not a measurement. Two consequences worth carrying into WP8 rather than discovering there. The preroll gate STAYS on desktop (WP4 retires Android's; the `smooth_buffer` reinterpretation is WP7's), so preroll now stacks on top of the cushion and criterion 3's e2e cost is cushion + up to half a refresh + preroll. And one-present-per-slot is now `PresentGate`'s job: where present timing is live that is strictly better, because the gate spaces presents by real glass completions — but where it is unavailable the gate is inert and a faster-than- panel stream can submit two presents into one vblank, throttled only by the blocking acquire. That is already the `latency` intent's behaviour on those boxes, but it is new for `smooth`, and `.221` Intel Arc / AMD are where to watch. `wake_timeout` is rewritten as the exact mirror of the decision, including the branch that matters: before the first glass stamp the clock is unanchored and `next_slot_after` answers "one period from the query" rather than from a grid, so the servable instant moves with `now`. Missing that would have parked a session's opening frames for a refresh they never owed. The cushion ceiling is one SOURCE frame, from the negotiated stream `Mode.refresh_hz` — never the panel period. The measured fps was rejected deliberately: it sags exactly when the transport is struggling, which is when a ceiling derived from it would start licensing a bigger hold. ⚠ `note_off_cadence` is unused here. Nothing on this path can identify a repeat or a host-anchored implausible stamp without S1's wire bit, so the plan's R2 stays open for the desktop leg — recorded rather than worked around. Verified: clippy `--all-targets -D warnings` exit 0 with `Checking pf-presenter` confirmed present (a warm shared target dir can otherwise print Finished having compiled nothing), 53 tests pass, and the same clean under `--no-default-features` because the intake edit sits beside the cfg-gated PyroWave collapse. --- crates/pf-presenter/src/present_pace.rs | 346 +++++++++++++++++++++--- crates/pf-presenter/src/run.rs | 176 +++++++++--- 2 files changed, 449 insertions(+), 73 deletions(-) diff --git a/crates/pf-presenter/src/present_pace.rs b/crates/pf-presenter/src/present_pace.rs index a853551c..4e7e4507 100644 --- a/crates/pf-presenter/src/present_pace.rs +++ b/crates/pf-presenter/src/present_pace.rs @@ -12,11 +12,18 @@ //! swapchain's own queue can never become a standing queue (+1 refresh per slot, //! forever — the law every bounded-FIFO pacing rediscovered on Apple). MAILBOX cannot //! queue and never needs it. +//! * [`SourcePacer`] — the shared [`punktfunk_core::phase::CadenceClock`] bound to this +//! client: under the smoothness intent frames are played out on the SOURCE's cadence +//! instead of on their arrival instant, so a raggedly-delivering host stops landing its +//! jitter on the glass 1:1. //! //! Everything here is pure state + arithmetic on `CLOCK_REALTIME` ns (the -//! `pf_client_core::session::now_ns` domain the on-glass stamps live in); the run loop -//! owns all clocks and Vulkan calls, which is what keeps this testable. +//! `pf_client_core::session::now_ns` domain the on-glass stamps live in) — +//! `DecodedFrame::decoded_ns` included, which is what lets the cadence clock run with no +//! domain conversion anywhere in this path. The run loop owns all clocks and Vulkan calls, +//! which is what keeps this testable. +use punktfunk_core::phase::{CadenceClock, CadenceHealth, CadenceTuning}; use std::collections::VecDeque; /// Stale-present force-open: an undisplayed present older than this is presumed lost @@ -64,10 +71,6 @@ impl FrameStore { self.capacity > 0 } - pub(crate) fn is_empty(&self) -> bool { - self.frames.is_empty() - } - pub(crate) fn submit(&mut self, f: T) { if self.capacity == 0 { if self.frames.pop_front().is_some() { @@ -85,7 +88,13 @@ impl FrameStore { } } - pub(crate) fn take(&mut self) -> Option { + /// The frame this pass will present, if any. + /// + /// `due` answers "has this frame's due time arrived?" for the front of a smoothing + /// FIFO. Newest-wins never asks it — under the latency intent a frame is due the + /// instant it exists — which is what keeps the cadence clock out of that path + /// entirely. + pub(crate) fn take(&mut self, due: impl FnOnce(&T) -> bool) -> Option { if self.capacity == 0 { return self.frames.pop_front(); } @@ -97,14 +106,24 @@ impl FrameStore { } self.prerolled = true; } - match self.frames.pop_front() { - Some(f) => Some(f), - None => { - self.underflows += 1; - self.prerolled = false; - None - } + let Some(f) = self.frames.front() else { + self.underflows += 1; + self.prerolled = false; + return None; + }; + // Not yet due is the smoothing intent WORKING: the store has a frame and is + // holding it for its slot. No counter moves and the preroll stands — reading this + // as a dry buffer would re-arm the preroll on every well-paced frame. + if !due(f) { + return None; } + self.frames.pop_front() + } + + /// The frame `take` would consider next, without consuming it — the run loop sizes its + /// event-wait from that frame's due time. + pub(crate) fn front(&self) -> Option<&T> { + self.frames.front() } /// A frame taken but not presented (gate closed, present failed before consuming @@ -403,6 +422,97 @@ impl CadenceProbe { } } +/// Plays frames out on the SOURCE's cadence: the shared +/// [`CadenceClock`](punktfunk_core::phase::CadenceClock) plus the two policy calls that +/// belong to this client rather than to the loop — which intent it applies to, and which +/// cushion the panel's measured refresh behaviour asks for. +/// +/// The defect it exists for is a host that delivers raggedly. The 2026-08-15 Skynet trace +/// has KWin's screencast arriving 0.11-8.22 ms off its own grid — up to a full 120 Hz +/// period — for 24 minutes, with the bitrate pinned and zero packet loss; presented on +/// arrival, every one of those milliseconds lands on the glass. +/// +/// The invariant to hold onto when touching this: the loop smooths the OFFSET, never the +/// timestamps. Genuine variation in the source's own cadence passes straight through to the +/// due time, so anything that made the due times more evenly spaced than the source would be +/// a bug and not an improvement (design/presenter-cadence-rework-implementation-plan.md +/// §2.2). +pub(crate) struct SourcePacer { + clock: CadenceClock, + /// Running the free-running tuning — i.e. the last verdict [`follow`](Self::follow) + /// saw was [`Cadence::Variable`]. + free_running: bool, +} + +impl SourcePacer { + pub(crate) fn new() -> SourcePacer { + SourcePacer { + clock: CadenceClock::new(CadenceTuning::snapping()), + free_running: false, + } + } + + /// Fold a frame arriving at the store and answer when it is due, in the same clock + /// domain `ready_ns` came in. + /// + /// `None` under the latency intent, which is arrival-driven by definition: it costs + /// what it always did, and the loop never carries an estimate built from samples it + /// then ignored. + /// + /// Called at SUBMIT rather than at take, so the estimate sees the arrival process the + /// transport actually produced — the frames the store goes on to drop are part of it, + /// and folding what survived the store would hide exactly the jitter being measured. + pub(crate) fn due_ns( + &mut self, + smoothing: bool, + src_pts_ns: u64, + ready_ns: u64, + frame_interval_ns: i64, + ) -> Option { + smoothing.then(|| { + self.clock + .due_ns(src_pts_ns, ready_ns as i64, frame_interval_ns) + }) + } + + /// Follow the measured refresh verdict. Snapping a due time onto the latch grid carries + /// roughly half a refresh of implicit slack, presenting at it directly carries none, so + /// the two want different cushions. + /// + /// Re-tuning costs a re-anchor (the tuning is fixed at construction), which is why this + /// is keyed to the probe's PUBLISHED verdict — agreed across rounds — and not to a + /// per-window reading that was measured flapping on glass. + pub(crate) fn follow(&mut self, verdict: Cadence) { + let free = verdict == Cadence::Variable; + if free != self.free_running { + self.free_running = free; + self.clock = CadenceClock::new(if free { + CadenceTuning::free_running() + } else { + CadenceTuning::snapping() + }); + } + } + + /// Present at the due time itself instead of snapping it to the latch grid. True only + /// where variable refresh is MEASURED live, which is the one case where the panel + /// refreshes when we present and there is no grid to aim at. + pub(crate) fn free_running(&self) -> bool { + self.free_running + } + + /// Re-anchor on the next frame — every discontinuity this loop already knows about (a + /// display change, an accepted mid-session mode switch). The measured jitter survives + /// it by design: it describes the link, not the stream. + pub(crate) fn reset(&mut self) { + self.clock.reset(); + } + + pub(crate) fn health(&self) -> CadenceHealth { + self.clock.health() + } +} + /// The FIFO glass budget: at most one undisplayed present in flight, measured by the /// present-wait waiter's outstanding count. Never consulted under MAILBOX/IMMEDIATE /// (they cannot queue) or without present-wait (nothing to count with — behavior is @@ -455,24 +565,24 @@ mod tests { fn newest_wins_replaces_and_putback_never_clobbers() { let mut s: FrameStore = FrameStore::new(0); assert!(!s.is_smoothing()); - assert_eq!(s.take(), None); + assert_eq!(s.take(|_| true), None); s.submit(1); s.submit(2); s.submit(3); - assert_eq!(s.take(), Some(3), "only the newest survives"); - assert_eq!(s.take(), None); + assert_eq!(s.take(|_| true), Some(3), "only the newest survives"); + assert_eq!(s.take(|_| true), None); // A taken-but-unpresented frame returns — unless a fresher one arrived. s.submit(4); - let f = s.take().unwrap(); + let f = s.take(|_| true).unwrap(); s.put_back(f); - assert_eq!(s.take(), Some(4)); - let f = s.take(); + assert_eq!(s.take(|_| true), Some(4)); + let f = s.take(|_| true); assert_eq!(f, None); s.submit(5); - let f = s.take().unwrap(); + let f = s.take(|_| true).unwrap(); s.submit(6); s.put_back(f); // 6 arrived while 5 was out — 6 wins - assert_eq!(s.take(), Some(6)); + assert_eq!(s.take(|_| true), Some(6)); assert_eq!( s.take_counters(), (2, 0, 0), @@ -486,26 +596,30 @@ mod tests { let mut s: FrameStore = FrameStore::new(2); assert!(s.is_smoothing()); s.submit(1); - assert_eq!(s.take(), None, "prerolling: below capacity, nothing vends"); - s.submit(2); - assert_eq!(s.take(), Some(1), "preroll reached — FIFO order"); assert_eq!( - s.take(), + s.take(|_| true), + None, + "prerolling: below capacity, nothing vends" + ); + s.submit(2); + assert_eq!(s.take(|_| true), Some(1), "preroll reached — FIFO order"); + assert_eq!( + s.take(|_| true), Some(2), "once prerolled the buffer drains normally" ); // Dry after preroll = one underflow, preroll re-arms. - assert_eq!(s.take(), None); + assert_eq!(s.take(|_| true), None); s.submit(3); - assert_eq!(s.take(), None, "re-armed preroll holds again"); + assert_eq!(s.take(|_| true), None, "re-armed preroll holds again"); s.submit(4); - assert_eq!(s.take(), Some(3)); + assert_eq!(s.take(|_| true), Some(3)); // Overflow drops the OLDEST: [4] → [4,5] → 6 evicts 4 → 7 evicts 5. s.submit(5); s.submit(6); s.submit(7); - assert_eq!(s.take(), Some(6)); - assert_eq!(s.take(), Some(7)); + assert_eq!(s.take(|_| true), Some(6)); + assert_eq!(s.take(|_| true), Some(7)); let (replaced, drops, dry) = s.take_counters(); assert_eq!(replaced, 0); assert_eq!(drops, 2, "6 evicted 4, 7 evicted 5"); @@ -519,9 +633,57 @@ mod tests { let mut s: FrameStore = FrameStore::new(2); s.submit(1); s.submit(2); - let f = s.take().unwrap(); + let f = s.take(|_| true).unwrap(); s.put_back(f); - assert_eq!(s.take(), Some(1), "the put-back frame is still first"); + assert_eq!( + s.take(|_| true), + Some(1), + "the put-back frame is still first" + ); + } + + /// A frame held for its due time is the smoothing intent working, not the store + /// running dry: nothing is counted and the preroll it built stays armed. Counting it + /// as an underflow would re-arm the preroll on every well-paced frame and stall the + /// stream for a buffer's worth of frames each time. + #[test] + fn a_frame_held_for_its_due_time_is_not_an_underflow() { + let mut s: FrameStore = FrameStore::new(2); + s.submit(10); + s.submit(20); + assert_eq!(s.take(|_| false), None, "prerolled, but nothing is due yet"); + assert_eq!(s.take(|&v| v >= 10), Some(10)); + assert_eq!(s.take(|&v| v >= 30), None, "20 is not due yet either"); + assert_eq!(s.take(|_| true), Some(20)); + // NOW it is genuinely dry, which is an underflow and does re-arm the preroll. + assert_eq!(s.take(|_| true), None); + s.submit(30); + assert_eq!(s.take(|_| true), None, "re-armed preroll holds again"); + assert_eq!( + s.take_counters(), + (0, 0, 1), + "one dry, nothing from the holds" + ); + } + + /// The desktop half of the intent split: a newest-wins store vends on arrival and + /// never so much as ASKS for a due time, so no cadence clock can end up gating the + /// latency intent even if a caller handed it one. + #[test] + fn the_latency_store_vends_without_ever_asking_a_due_time() { + let mut s: FrameStore = FrameStore::new(0); + s.submit(7); + let mut asked = false; + let got = s.take(|_| { + asked = true; + false + }); + assert_eq!( + got, + Some(7), + "arrival-driven: the frame goes out regardless" + ); + assert!(!asked, "…and the due time was never consulted"); } /// force_latency collapses a smoothing store to a newest-wins slot mid-stream. @@ -534,10 +696,14 @@ mod tests { s.submit(3); s.force_latency(); assert!(!s.is_smoothing()); - assert_eq!(s.take(), Some(3), "only the newest survives the collapse"); + assert_eq!( + s.take(|_| true), + Some(3), + "only the newest survives the collapse" + ); s.submit(4); s.submit(5); - assert_eq!(s.take(), Some(5)); + assert_eq!(s.take(|_| true), Some(5)); } /// The clock learns the min positive spacing (capped at the mode refresh), anchors @@ -732,6 +898,116 @@ mod tests { assert_eq!(drip.verdict(), bulk.verdict(), "batching must not matter"); } + /// 120 Hz, and a source stamping a clock domain far from the present clock's — the + /// pacer must never need to be told about the difference. + const SRC_P: i64 = 8_333_333; + const SRC_PTS0: u64 = 1_786_000_000_000_000_000; + const SRC_READY0: u64 = 1_000_000_000; + + /// Fold `n` frames of a clean 120 Hz source through the pacer. + fn fold(p: &mut SourcePacer, smoothing: bool, n: u64) { + for k in 0..n { + p.due_ns( + smoothing, + SRC_PTS0 + k * SRC_P as u64, + SRC_READY0 + k * SRC_P as u64, + SRC_P, + ); + } + } + + /// The clock half of the intent split: under latency the pacer folds NOTHING. The + /// estimate exists only where it is used, so the latency path costs exactly what it + /// cost before this work and a stream that collapses to latency mid-flight (PyroWave) + /// leaves no half-built loop behind it. + #[test] + fn the_latency_intent_folds_no_frames_into_the_cadence_clock() { + let mut p = SourcePacer::new(); + for k in 0..64u64 { + assert_eq!( + p.due_ns( + false, + SRC_PTS0 + k * SRC_P as u64, + SRC_READY0 + k * SRC_P as u64, + SRC_P + ), + None, + "latency has no due time to answer with" + ); + } + let h = p.health(); + assert_eq!(h.frames, 0, "not one sample reached the loop"); + assert_eq!((h.offset_ns, h.skew_ns, h.jitter_ns), (0, 0, 0)); + // …and the very same frames under smoothness do reach it. + assert!(p.due_ns(true, SRC_PTS0, SRC_READY0, SRC_P).is_some()); + assert_eq!(p.health().frames, 1); + } + + /// One domain in, same domain out (the clock's own invariant, asserted here because + /// this binding is the one that feeds `decoded_ns` and reads back a `now_ns` deadline + /// with no conversion between them): the due time lands on the PRESENT clock's + /// timeline, however far the source's stamps are from it. + #[test] + fn a_due_time_comes_back_on_the_present_clocks_timeline() { + let mut p = SourcePacer::new(); + fold(&mut p, true, 400); + let k = 400u64; + let due = p + .due_ns( + true, + SRC_PTS0 + k * SRC_P as u64, + SRC_READY0 + k * SRC_P as u64, + SRC_P, + ) + .unwrap(); + let ready = (SRC_READY0 + k * SRC_P as u64) as i64; + assert!( + (due - ready).abs() <= p.health().cushion_ns, + "due {due} is not within a cushion of the present-clock ready {ready}" + ); + } + + /// The VRR half: where variable refresh is MEASURED live there is no grid to snap to, + /// so the due time is presented directly and the cushion has to cover the distribution + /// on its own. Re-tuning is a fresh loop, which is why it follows the probe's published + /// verdict — agreed across rounds — and not a per-window reading. + #[test] + fn a_measured_vrr_verdict_switches_the_cushion_policy() { + let mut p = SourcePacer::new(); + assert!(!p.free_running(), "snapping until the panel says otherwise"); + fold(&mut p, true, 200); + p.follow(Cadence::Fixed); + assert!(!p.free_running()); + assert_eq!( + p.health().frames, + 200, + "a verdict that changes nothing must not re-anchor" + ); + p.follow(Cadence::Variable); + assert!(p.free_running()); + assert_eq!(p.health().frames, 0, "re-tuning is a fresh loop"); + p.follow(Cadence::Unknown); + assert!( + !p.free_running(), + "Unknown is the absence of a measurement, not a measurement of VRR" + ); + + // The two tunings differ where it matters: with no jitter measured yet, the + // free-running cushion already holds a frame back by more than the snapping one, + // which is riding on the half-refresh the snap-up gives it for free. + let mut snap = SourcePacer::new(); + snap.due_ns(true, SRC_PTS0, SRC_READY0, SRC_P); + let mut free = SourcePacer::new(); + free.follow(Cadence::Variable); + free.due_ns(true, SRC_PTS0, SRC_READY0, SRC_P); + assert!( + free.health().cushion_ns > snap.health().cushion_ns, + "free-running {} must cushion past snapping {}", + free.health().cushion_ns, + snap.health().cushion_ns + ); + } + /// Gate: open at zero outstanding, closed at one, force-open past the stale bound. #[test] fn gate_budgets_one_undisplayed_present() { diff --git a/crates/pf-presenter/src/run.rs b/crates/pf-presenter/src/run.rs index d7d695db..c3491179 100644 --- a/crates/pf-presenter/src/run.rs +++ b/crates/pf-presenter/src/run.rs @@ -21,7 +21,8 @@ use crate::overlay::{ FrameCtx, Overlay, OverlayAction, OverlayFrame, PointerButton, PointerInput, SessionPhase, }; use crate::present_pace::{ - Cadence, CadenceProbe, FrameStore, LatchClock, PresentGate, MARGIN_MAX_NS, MARGIN_STEP_NS, + Cadence, CadenceProbe, FrameStore, LatchClock, PresentGate, SourcePacer, MARGIN_MAX_NS, + MARGIN_STEP_NS, }; use crate::touch::Abs; use crate::vk::{FrameInput, Presenter}; @@ -209,6 +210,17 @@ enum ModeCtl<'a> { /// pure wake-up — the loop drains the frame channel regardless of why it woke. struct FrameWake; +/// A decoded frame and when the source cadence says it is due on glass — the pair the +/// intent store holds, so the due time is decided from the arrival process the store SAW +/// rather than from whatever survived it. +struct Paced { + frame: DecodedFrame, + /// In `session::now_ns`'s domain (`DecodedFrame::decoded_ns` is the same clock, which + /// is what lets this path run with no conversion in it). `0` under the latency intent, + /// which never asks. + due_ns: i64, +} + /// Everything one stream session accumulates — created at session start, dropped at /// session end (browse mode cycles through several per process lifetime). struct StreamState { @@ -265,10 +277,18 @@ struct StreamState { /// smoothness. NOTE: a smoothing store holds decoder-pool frames (Vulkan-Video /// AVFrames) up to `buffer` deep on top of the depth-2 wake channels — within pool /// headroom for 1..=3, but any deeper store must revisit pool sizing. - store: FrameStore, + store: FrameStore, /// The panel latch grid (present-wait glass stamps; submit-anchored fallback) — the /// smoothness slot clock, and the values published to the host-facing `latch_grid`. clock: LatchClock, + /// Plays smoothness frames out on the SOURCE's cadence instead of on their arrival + /// instant — inert under latency, which never folds a frame into it. + pacer: SourcePacer, + /// The SOURCE's nominal frame interval: the negotiated STREAM mode's refresh, never + /// the panel's. It is the cushion's ceiling, so it has to describe the cadence the host + /// produces — on a 120 fps stream shown on a 60 Hz panel the panel's period would + /// license twice the hold the source's own cadence can justify. + source_interval_ns: i64, /// The FIFO glass budget (one undisplayed present in flight) — inert off FIFO modes /// or without present timing. gate: PresentGate, @@ -279,9 +299,6 @@ struct StreamState { /// VRR is off, and so the cadence probe's reference. Deliberately not the learned /// period (see the probe's call site). mode_period_ns: u64, - /// The latch slot the last smoothness present served (one present per slot); 0 = - /// none yet. - last_target_ns: u64, /// Smoothness slot-pick margin: starts 0 (a fixed lead is pure display tax — /// measured on Android), widens +500 µs per >2-miss window toward 2.5 ms. margin_ns: u64, @@ -385,6 +402,9 @@ impl StreamState { native_refresh_hz: u32, ) -> StreamState { let profile = params.profile.clone(); + // The rate we ASKED for, until the Welcome resolves it (`Connected` below). No + // frames flow before that, so this only ever has to be sane, not right. + let source_interval_ns = frame_interval_ns(params.mode.refresh_hz, native_refresh_hz); // The presenter's half of phase-locked capture: it writes the latch grid the // pump reads (see `LatchGrid`), so keep the Arc before the params move. `None` // when the session didn't advertise the cap — the 1 Hz fold then skips the work. @@ -431,10 +451,11 @@ impl StreamState { presented: PresentedWindow::default(), store: FrameStore::new(usize::from(priority.fifo_capacity())), clock: LatchClock::new(native_refresh_hz), + pacer: SourcePacer::new(), + source_interval_ns, gate: PresentGate::default(), cadence: CadenceProbe::new(), mode_period_ns: 1_000_000_000 / u64::from(native_refresh_hz.max(1)), - last_target_ns: 0, margin_ns: 0, win_misses: 0, win_out_max: 0, @@ -481,26 +502,61 @@ impl StreamState { self.handle.stop.store(true, Ordering::SeqCst); } - /// The event-loop wait bound: a smoothness stream with buffered frames sleeps only - /// to its next latch-slot deadline; everything else keeps the 15 ms housekeeping - /// tick (frames, input, and present completions all wake the loop early anyway). + /// The event-loop wait bound: a smoothness stream with a frame in hand sleeps only to + /// the pass that can still serve it; everything else keeps the 15 ms housekeeping tick + /// (frames, input, and present completions all wake the loop early anyway). + /// + /// ⚠ This is the present decision's mirror and has to stay one — it answers "when does + /// that decision first say yes?", so a rule changed on one side and not the other + /// oversleeps a smooth stream straight past its own due time. fn wake_timeout(&self) -> Duration { const TICK: Duration = Duration::from_millis(15); - if !self.store.is_smoothing() || self.store.is_empty() { + if !self.store.is_smoothing() { return TICK; } - let now = session::now_ns(); - let mut target = self - .clock - .next_slot_after(now.saturating_add(self.margin_ns)); - if target == self.last_target_ns { - // This slot is already served — the next boundary is the deadline. - target += self.clock.period_ns(); - } - Duration::from_nanos(target.saturating_sub(now)).clamp(Duration::from_millis(1), TICK) + let Some(p) = self.store.front() else { + return TICK; + }; + // Free-running presents at the due time itself. Snapping presents once the slot the + // frame is aimed at is the next one still reachable — one period, less the submit + // lead, before it. Before the first on-glass stamp there is no grid and + // `next_slot_after` answers "one period from the query" instead, so the decision's + // slot moves with `now` and the frame is servable a period sooner; mirror that too, + // or a session's opening frames sit in the store for a refresh they never owed. + let lead_ns = self.clock.period_ns() as i64 + self.margin_ns as i64; + let wake_ns = if self.pacer.free_running() { + p.due_ns + } else if self.clock.anchor_ns() == 0 { + p.due_ns - lead_ns + } else { + self.clock.next_slot_after(p.due_ns.max(0) as u64) as i64 - lead_ns + }; + Duration::from_nanos(wake_ns.saturating_sub(session::now_ns() as i64).max(0) as u64) + .clamp(Duration::from_millis(1), TICK) } } +/// One frame at `refresh_hz`, in ns — the SOURCE's nominal interval, and so the cadence +/// cushion's ceiling. +/// +/// The negotiated stream mode's refresh is the only source-rate signal a client has, and it +/// is the right one: it is the rate the host's virtual output runs at, so it bounds the +/// cadence the capture can produce. The MEASURED fps would be the tempting alternative and +/// is the wrong answer — it sags exactly when the transport is struggling, which is when a +/// ceiling derived from it would start licensing a bigger hold. +/// +/// `0` = the mode asked for "native", which the host resolves to the display this client +/// reported; that display's rate is therefore what it will produce. Neither known falls +/// back to 60 Hz, the same last resort [`native_mode`]'s caller takes. +fn frame_interval_ns(refresh_hz: u32, fallback_hz: u32) -> i64 { + let hz = match (refresh_hz, fallback_hz) { + (0, 0) => 60, + (0, f) => f, + (r, _) => r, + }; + 1_000_000_000 / i64::from(hz) +} + /// Whether a present error is `VK_ERROR_DEVICE_LOST` anywhere in its chain. A lost /// device is unrecoverable by spec — every object on it (decoder frames, swapchain, /// the Skia context) is dead, and the demote-to-software path would rebuild the @@ -857,7 +913,11 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result st.mode_period_ns = 1_000_000_000 / u64::from(hz); } st.cadence.reset(); - st.last_target_ns = 0; + // The estimate was built against a panel this stream is no + // longer on, and the verdict its cushion policy came from has + // just been thrown away with it. Re-anchoring costs one frame; + // the measured jitter survives, because that describes the link. + st.pacer.reset(); tracing::info!( refresh_hz = hz, "display changed — relearning the latch grid" @@ -1394,6 +1454,9 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result continue; } st.mode_line = format!("{}×{}@{}", m.width, m.height, m.refresh_hz); + // The RESOLVED rate — a `0 = native` request becomes a real number + // here, and this is the last moment before frames start arriving. + st.source_interval_ns = frame_interval_ns(m.refresh_hz, native.refresh_hz); tracing::info!(mode = %st.mode_line, "connected"); window .set_title(&format!("{} · {}", opts.window_title, st.mode_line)) @@ -1904,26 +1967,39 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result ); } } - st.store.submit(f); + // The intent AFTER any PyroWave collapse above, so a wavelet stream folds + // nothing into a loop it will never consult. + let smoothing = st.store.is_smoothing(); + let due_ns = st + .pacer + .due_ns(smoothing, f.pts_ns, f.decoded_ns, st.source_interval_ns) + .unwrap_or(0); + st.store.submit(Paced { frame: f, due_ns }); } // One frame out, by intent: latency takes the newest whenever the glass - // gate allows; smoothness serves at most one frame per latch slot (the + // gate allows; smoothness serves the frame whose due time has come (the // preroll/underflow behavior lives in the store). let now_ns = session::now_ns(); - let mut slot_target = 0u64; + st.pacer.follow(st.cadence.verdict()); let mut to_present = if st.store.is_smoothing() { - let target = st - .clock - .next_slot_after(now_ns.saturating_add(st.margin_ns)); - if target != st.last_target_ns { - slot_target = target; - st.store.take() + if st.pacer.free_running() { + // Variable refresh, measured: the panel refreshes when we present, so + // there is no grid to aim at and the due time IS the target. + st.store.take(|p| p.due_ns <= now_ns as i64) } else { - None + // The first latch still reachable from here, given the submit lead. A + // frame due before it cannot be shown any sooner by waiting; one due + // after it would land a slot early, which is the judder this exists to + // remove. (`next_slot_after` is monotone, so "its own target slot is + // not later than this one" reduces to the comparison below.) + let slot = st + .clock + .next_slot_after(now_ns.saturating_add(st.margin_ns)); + st.store.take(|p| p.due_ns < slot as i64) } } else { - st.store.take() + st.store.take(|_| true) }; // The FIFO glass budget: one undisplayed present in flight, so the // swapchain's own FIFO can never become a standing queue (a measured @@ -1942,7 +2018,7 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result } } } - if let Some(f) = to_present { + if let Some(Paced { frame: f, .. }) = to_present { // Resize END: a frame at the steered target size means the sharp new-mode // picture is here — lift the scrim. A no-op unless a switch is in flight. let (fw, fh) = f.image.dimensions(); @@ -2168,12 +2244,6 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result }; if did_present { presented_video = true; - // Smoothness: this latch slot is served — one present per slot. - // (Set only on success: a gated or failed present leaves the slot - // open for the retry.) - if slot_target != 0 { - st.last_target_ns = slot_target; - } if opts.json_status && !st.ready_announced { st.ready_announced = true; println!("{{\"ready\":true}}"); @@ -2261,6 +2331,7 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result // when anything moved, or always under PUNKTFUNK_PRESENT_DEBUG=1 — // the field-triage instrument for the intent engine. if pacing_active && (present_debug || q_drop + q_dry + gated + forced > 0) { + let cadence_health = st.pacer.health(); tracing::info!( smoothing = st.presented.smoothing, mode = st.presented.mode, @@ -2276,6 +2347,15 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result latch_ms = st.presented.latch_ms, period_us = st.clock.period_ns() / 1000, margin_us = st.margin_ns / 1000, + // The cadence loop's current hold and the jitter it is sized from, + // plus the frames whose due time had already passed when they + // arrived — the direct read on whether the cushion is big enough. + // All three are cumulative/instantaneous, NOT window sums like the + // counters above: the loop's state is what a triage session wants, + // and a one-second slice of a running estimate says nothing. + cushion_us = cadence_health.cushion_ns / 1000, + jitter_us = cadence_health.jitter_ns / 1000, + late = cadence_health.late, "presenter window" ); } @@ -2391,6 +2471,11 @@ fn hud_mode_tick(st: &mut StreamState, window: &mut sdl3::video::Window, title_b st.mode_line = format!("{}×{}@{}", m.width, m.height, m.refresh_hz); tracing::info!(mode = %st.mode_line, "stream mode switched"); let _ = window.set_title(&format!("{title_base} · {}", st.mode_line)); + // A switch is a full host-side rebuild of the virtual display and the encoder: the + // interval the cushion is bounded by can change, and the gap the rebuild leaves is + // a hole the cadence estimate must re-anchor across rather than slew over. + st.source_interval_ns = frame_interval_ns(m.refresh_hz, 0); + st.pacer.reset(); } st.shown_mode = Some(m); } @@ -3216,6 +3301,21 @@ mod tests { assert_eq!((m.width, m.height), (0, 0)); } + /// The cadence cushion is bounded by the SOURCE's frame interval, and the negotiated + /// stream mode is where that number comes from. Substituting the panel's period — + /// the tempting simplification, since the presenter has one at hand — would let a + /// 120 fps stream on a 60 Hz panel hold a frame for twice the source's own cadence. + #[test] + fn the_cadence_interval_comes_from_the_stream_mode_not_the_panel() { + assert_eq!(frame_interval_ns(120, 60), 8_333_333); + assert_eq!(frame_interval_ns(60, 165), 16_666_666); + // A `0 = native` request is resolved by the host to the display this client + // reported, so that display's rate is what it will produce. + assert_eq!(frame_interval_ns(0, 165), 6_060_606); + // Neither known: 60 Hz, never an unbounded ceiling. + assert_eq!(frame_interval_ns(0, 0), 16_666_666); + } + #[test] fn overlay_scale_follows_dpi_and_survives_a_bogus_display() { // 100 % / 96 dpi is the identity — the chrome keeps the size it always had. -- 2.54.0