fix(client/present): one bogus present stamp no longer wedges the cadence run
Caught on glass during the WP2 baseline, on the first stream the metric ever measured: judder=0 mode=0 cadN=0 disorder=119, every second, on a perfectly healthy 118 fps stream with the panel period correctly learned at 8.33 ms. Every single present was scoring as disordered. The cause was the vendor quirk decode/display.rs already documents — Android's render callback can deliver a garbage far-future system_nano on a session's first frames. The rule "hold the later instant so one reordered delivery cannot corrupt the following spacings" then latched onto that stamp permanently: every real timestamp afterwards was behind it, so nothing was ever scored again for the rest of the session. The rule was right for what it was written for and wrong past a bound. A step backwards of a few refreshes IS a reordered delivery and the later instant should win; a step backwards of an hour is a bogus stamp and the run must re-anchor onto the new sample. One bad sample now costs one sample. Both implementations get the bound and the regression test, since the two must agree; the Swift port would otherwise have shipped the same latch-up. Also makes the statistic self-diagnosing, which is what turned a puzzling result into a five-minute diagnosis: summary() returning None was indistinguishable from a window of perfectly smooth zeros in a log line, so "no cadence is being scored at all" looked exactly like "no judder". The pf.present line now carries the raw sample/stall/disorder counts and the period the run is quantising against, whatever the evidence bar. Gates: punktfunk-core 190 tests green (22 in phase, incl. the new a_garbage_far_future_stamp_does_not_wedge_the_run); swift test --filter PresentIntervalsTests 11/11 green; fmt clean; on-glass re-run against .173 now reports judder 8-36permille with cadN ~119/s.
This commit is contained in:
@@ -288,6 +288,7 @@ impl PresentMeter {
|
||||
Vec<u64>,
|
||||
Vec<u64>,
|
||||
Vec<u64>,
|
||||
(u32, u32, u32),
|
||||
Option<punktfunk_core::phase::PresentCadence>,
|
||||
) {
|
||||
let mut g = self
|
||||
@@ -302,6 +303,7 @@ impl PresentMeter {
|
||||
std::mem::take(&mut g.feed_us),
|
||||
std::mem::take(&mut g.codec_us),
|
||||
std::mem::take(&mut g.e2e_us),
|
||||
g.intervals.pending(),
|
||||
g.intervals.take(),
|
||||
)
|
||||
}
|
||||
@@ -602,7 +604,7 @@ impl Presenter {
|
||||
return None;
|
||||
}
|
||||
self.last_flush = Instant::now();
|
||||
let (latch, displays, feed, codec, e2e, cadence) = meter.drain();
|
||||
let (latch, displays, feed, codec, e2e, cad_raw, cadence) = meter.drain();
|
||||
if self.released == 0 && displays == 0 {
|
||||
return None; // idle stream — nothing worth a line
|
||||
}
|
||||
@@ -626,7 +628,7 @@ impl Presenter {
|
||||
feedMs p50={:.2} max={:.2} codecMs p50={:.2} max={:.2} \
|
||||
e2eMs p50={:.2} max={:.2} circ={:.2}ms coh={} \
|
||||
vsyncMs={:.2} panelMs={:.2} \
|
||||
judder={}permille mode={}vsync stalls={} disorder={}",
|
||||
judder={}permille mode={}vsync cadN={} stalls={} disorder={} cadPeriodMs={:.2}",
|
||||
self.released,
|
||||
displays,
|
||||
self.paced_drops,
|
||||
@@ -651,8 +653,10 @@ impl Presenter {
|
||||
panel_ns as f64 / 1e6,
|
||||
cadence.map(|c| c.judder_permille).unwrap_or(0),
|
||||
cadence.map(|c| c.mode_units).unwrap_or(0),
|
||||
cadence.map(|c| c.stalls).unwrap_or(0),
|
||||
cadence.map(|c| c.disordered).unwrap_or(0),
|
||||
cad_raw.0,
|
||||
cad_raw.1,
|
||||
cad_raw.2,
|
||||
meter.panel_period_ns.load(Ordering::Relaxed) as f64 / 1e6,
|
||||
);
|
||||
self.released = 0;
|
||||
// Margin adaptation, off the MEASURED latch. A release targets the first grid point past
|
||||
|
||||
@@ -595,6 +595,11 @@ struct PresentIntervals {
|
||||
private static let maxUnits = 8
|
||||
/// Minimum intervals before a summary means anything (matches `circularLatch`'s bar).
|
||||
private static let minSamples = 8
|
||||
/// A backwards step larger than this is a bogus timestamp, not a reordered delivery, so the
|
||||
/// run re-anchors rather than holding the old instant. Without it, one garbage far-future
|
||||
/// stamp latches the statistic and every later present scores as disordered for the whole
|
||||
/// session — observed on glass on Android, 2026-08-05.
|
||||
private static let reanchorNs: Int64 = 100_000_000
|
||||
|
||||
private var lastPresentNs: Int64 = 0
|
||||
private var hist = [Int](repeating: 0, count: PresentIntervals.maxUnits + 1)
|
||||
@@ -614,10 +619,14 @@ struct PresentIntervals {
|
||||
guard prev > 0, periodNs > 0 else { return }
|
||||
let spacing = presentNs - prev
|
||||
if spacing <= 0 {
|
||||
// Keep the LATER instant so one disordered delivery cannot corrupt every
|
||||
// following spacing.
|
||||
// Hold the LATER instant so one reordered delivery cannot corrupt every following
|
||||
// spacing — but only when the step back is small enough to BE a reordering. Beyond
|
||||
// that the old instant is the bogus one (see `reanchorNs`) and the run re-anchors
|
||||
// onto the new sample, which `lastPresentNs` already holds.
|
||||
disordered += 1
|
||||
lastPresentNs = max(prev, presentNs)
|
||||
if prev - presentNs < PresentIntervals.reanchorNs {
|
||||
lastPresentNs = prev
|
||||
}
|
||||
return
|
||||
}
|
||||
// Nearest whole refresh: a present is "on the grid" if it is closer to this vblank than
|
||||
|
||||
@@ -103,6 +103,26 @@ final class PresentIntervalsTests: XCTestCase {
|
||||
"keeping the later instant means the following spacings stay on the grid")
|
||||
}
|
||||
|
||||
/// The on-glass failure of 2026-08-05, pinned on both sides. A render callback can deliver a
|
||||
/// garbage far-future timestamp on a session's first frames; holding "the later instant"
|
||||
/// unconditionally latched onto it and scored EVERY subsequent present as disordered for the
|
||||
/// whole session. One bad sample must cost one sample.
|
||||
func testAGarbageFarFutureStampDoesNotWedgeTheRun() {
|
||||
var pi = PresentIntervals()
|
||||
var t: Int64 = 1_000_000_000
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
pi.record(presentNs: t + 60 * 60 * 1_000_000_000, periodNs: Self.P)
|
||||
for _ in 0..<20 {
|
||||
t += Self.P
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
}
|
||||
let s = pi.summary()
|
||||
XCTAssertEqual(s?.disordered, 1, "the garbage stamp cost exactly one sample")
|
||||
XCTAssertEqual(s?.modeUnits, 1)
|
||||
XCTAssertEqual(s?.judderPermille, 0)
|
||||
XCTAssertEqual(s?.samples, 19, "every present after the re-anchor scored")
|
||||
}
|
||||
|
||||
func testAnUnknownGridScoresNothing() {
|
||||
var pi = PresentIntervals()
|
||||
var t: Int64 = 1_000_000_000
|
||||
|
||||
@@ -135,6 +135,13 @@ pub fn circular_latch(samples_us: &[u64], period_ns: i64) -> Option<(u64, u16)>
|
||||
/// single hitch dominate the window.
|
||||
const CADENCE_MAX_UNITS: usize = 8;
|
||||
|
||||
/// A backwards step larger than this is not a reordered delivery, it is a bogus timestamp, and
|
||||
/// the run re-anchors onto the new instant instead of holding the old one. Android's render
|
||||
/// callback is documented to carry a garbage far-future stamp on a session's first frames;
|
||||
/// without this bound, holding "the later instant" latches onto that stamp and every subsequent
|
||||
/// present scores as disordered for the rest of the session (observed on glass, 2026-08-05).
|
||||
const CADENCE_REANCHOR_NS: i64 = 100_000_000;
|
||||
|
||||
/// Minimum intervals before a cadence summary means anything — same evidence bar as
|
||||
/// [`circular_latch`]. At any sane frame rate a 1 s window clears this many times over; it is
|
||||
/// there so a window truncated by a reanchor does not publish a judder figure off three samples.
|
||||
@@ -212,10 +219,15 @@ impl PresentIntervals {
|
||||
}
|
||||
let spacing = present_ns - prev;
|
||||
if spacing <= 0 {
|
||||
// A repeated or out-of-order callback. Keep the LATER instant as the predecessor so
|
||||
// one disordered delivery cannot corrupt every following spacing.
|
||||
// A repeated or out-of-order callback. Hold the LATER instant so one reordered
|
||||
// delivery cannot corrupt every following spacing — but only when the step back is
|
||||
// small enough to BE a reordering. Beyond that the old instant is the bogus one
|
||||
// (see [`CADENCE_REANCHOR_NS`]) and the run re-anchors onto the new sample, which
|
||||
// `last_present_ns` already holds.
|
||||
self.disordered += 1;
|
||||
self.last_present_ns = prev.max(present_ns);
|
||||
if prev - present_ns < CADENCE_REANCHOR_NS {
|
||||
self.last_present_ns = prev;
|
||||
}
|
||||
return;
|
||||
}
|
||||
// Round to the nearest whole refresh: a present is "on the grid" if it is closer to this
|
||||
@@ -229,6 +241,15 @@ impl PresentIntervals {
|
||||
self.samples += 1;
|
||||
}
|
||||
|
||||
/// The window's raw counts `(samples, stalls, disordered)`, whatever the evidence bar.
|
||||
///
|
||||
/// [`summary`](Self::summary) returning `None` is otherwise indistinguishable from a window
|
||||
/// of perfectly smooth zeros in a log line, which makes "no cadence is being scored at all"
|
||||
/// invisible — the exact failure this exists to diagnose.
|
||||
pub fn pending(&self) -> (u32, u32, u32) {
|
||||
(self.samples, self.stalls, self.disordered)
|
||||
}
|
||||
|
||||
/// This window's summary, or `None` under [`CADENCE_MIN_SAMPLES`].
|
||||
pub fn summary(&self) -> Option<PresentCadence> {
|
||||
if self.samples < CADENCE_MIN_SAMPLES {
|
||||
@@ -533,6 +554,27 @@ mod cadence_tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// The on-glass failure of 2026-08-05, pinned. Android's render callback can deliver a
|
||||
/// garbage far-future timestamp on a session's first frames. Holding "the later instant"
|
||||
/// unconditionally latched onto it and scored EVERY subsequent present as disordered —
|
||||
/// `cadN=0 disorder=119` per second, for the whole session, with the period known and the
|
||||
/// stream perfectly healthy. One bad sample must cost one sample, not the session.
|
||||
#[test]
|
||||
fn a_garbage_far_future_stamp_does_not_wedge_the_run() {
|
||||
let mut pi = PresentIntervals::new();
|
||||
let mut t = 1_000_000_000i64;
|
||||
pi.record(t, P);
|
||||
pi.record(t + 60 * 60 * 1_000_000_000, P); // a vendor's epoch-sized first stamp
|
||||
for _ in 0..20 {
|
||||
t += P;
|
||||
pi.record(t, P);
|
||||
}
|
||||
let s = pi.summary().expect("the run recovers instead of wedging");
|
||||
assert_eq!(s.disordered, 1, "the garbage stamp cost exactly one sample");
|
||||
assert_eq!((s.mode_units, s.judder_permille), (1, 0));
|
||||
assert_eq!(s.samples, 19, "every present after the re-anchor scored");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unknown_grid_scores_nothing() {
|
||||
let s = cadence(&[P], 60);
|
||||
|
||||
Reference in New Issue
Block a user