fix(diagnostics): measure present judder on delivering intervals only (#392) - #392
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A 30-minute recording of a mostly still 144 Hz desktop raised the judder finding 203 times, worst "judder" 1045 ms against an 8 ms budget. One second of nothing happening, reported as the worst judder of the session -- and every saved recording carried "1 problem observed" in its toast because of it.
source_present_jitter_mswaspeak - averageover every present interval in the 2 s window. A source that stops contributes one enormous interval, and that interval is not pacing: it is the measure of how long nothing happened.What changed
Intervals longer than four output periods are excluded from the jitter statistic. They mean the source stopped delivering, which is the duplication check's subject, not this one's. What remains is the spread of the intervals that were actual deliveries, reported as
p95 - p5so a single late frame cannot speak for a whole window. When too few delivering intervals are left, the metric goes Unavailable rather than 0 -- zero would read as perfect pacing instead of as no measurement.The 8 ms threshold is unchanged and deliberately so. It belongs to the OUTPUT period -- half a 60 fps interval -- not to the source. Scaling it with the source would make a 240 Hz capture stricter than a 144 Hz one, which is backwards: a faster source is easier for frame selection to smooth, not harder.
The quantile pair is not incidental
p95 - medianwas the first attempt and scores the WORST case at zero: a source alternating 4 ms and 22 ms puts half its samples in the long mode, so the median sits up there with the p95.PresentCadenceReportsSustainedIrregularDeliverycaught it. Mirroring the quantile at the low end measures how far apart the intervals actually lie.Verification
Four new cases, each naming a situation the old measurement got wrong or a regression the new one must not introduce:
102 tests in
test_pipeline_diagnosticspass. Live on the same 144 Hz desktop overdxgi_od: no present cadence reported at all, where the previous build produced 203 findings.Not in this change
rec.pacing.duplicationhas the same blind spot one level down -- it cannot tell a game capped at 30 fps from a still picture, and its own remediation text says as much. The analysis and the fix (the age of the last real frame at the point the duplicate is produced distinguishes them) are written up in.workspace. It touches the video thread, so it deserves its own pass. Until then it at least says something true about a still desktop, which the judder alarm did not.