Host-source stutter: both stutters fixed at the source, and frames finally play on the host's cadence #249

Merged

enricobuehler merged 10 commits from worktree-host-source-stutter-fixes into main 2026-08-15 13:13:08 +00:00

Conversation 0 Commits 10 Files Changed 30

+4080 -214

10 Commits

Author	SHA1	Message	Date
enricobuehler	3fd0a94528	feat(desktop): present on the host's cadence under smoothness, and let VRR finally be worth something ... 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.	2026-08-15 14:24:27 +02:00
enricobuehler	08a06ff295	feat(apple): present on the host's cadence under smoothness, and prove the port agrees with the original ... 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.	2026-08-15 14:19:15 +02:00
enricobuehler	a7ef62241b	feat(android): present on the host's cadence under smoothness, not on the frame's arrival ... 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.	2026-08-15 14:18:14 +02:00
enricobuehler	ff513a18e1	fix(apple): the fourth client had no way to reach the one decoder that could conceal a drought ... 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.	2026-08-15 14:08:31 +02:00
enricobuehler	e207d0d73e	feat(client/present): a clock that plays frames out on the source's cadence, not on their arrival ... 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.	2026-08-15 13:58:19 +02:00
enricobuehler	7d9d7c3814	fix(client): a packet drought drained the ring into a de-prime, because nothing arrived to reveal it ... 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.	2026-08-15 13:44:36 +02:00
enricobuehler	424621f1ce	fix(audio): what the Linux gate caught in the three preceding commits ... 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).	2026-08-15 13:44:34 +02:00
enricobuehler	925b07ec10	fix(capture): the wire pts was stamped downstream of the compositor's own delivery jitter ... 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.	2026-08-15 13:25:15 +02:00
enricobuehler	5e77ec42c6	fix(audio): a capture hole cost more than itself, and delivered_pct could not say how many there were ... 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.	2026-08-15 13:24:56 +02:00
enricobuehler	704defe0a7	fix(capture): the phase lock re-engaged on one good report, so a marginal host flapped 41 times in 24 minutes ... 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.	2026-08-15 13:24:35 +02:00