Skip to content

perf(player): coalesce _mirrorParentMediaTime writes#396

Open
vanceingalls wants to merge 2 commits intoperf/p1-2-scope-media-mutation-observerfrom
perf/p1-4-coalesce-mirror-parent-media-time
Open

perf(player): coalesce _mirrorParentMediaTime writes#396
vanceingalls wants to merge 2 commits intoperf/p1-2-scope-media-mutation-observerfrom
perf/p1-4-coalesce-mirror-parent-media-time

Conversation

@vanceingalls
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls commented Apr 21, 2026

Summary

Coalesce writes to el.currentTime inside _mirrorParentMediaTime so a single jitter sample no longer triggers a parent-media seek. A drift correction now requires two consecutive samples above the threshold (~MIRROR_DRIFT_THRESHOLD_SECONDS) before the player writes back. One-shot alignment paths (promoteToParentProxy, _onIframeMediaAdded) opt out via force: true so initial alignment stays immediate.

Why

Step P1-4 of the player perf proposal. _mirrorParentMediaTime is called every animation frame on parent media proxies. Even without true drift, browser internals report tiny jitter on currentTime reads — typically below 30 ms but occasionally crossing the threshold for a frame. Writing to currentTime triggers a seek, which is expensive and invalidates pipeline buffers, which causes the next frame's reading to jitter further. The result was unnecessary seek thrash on otherwise-aligned media.

By requiring two consecutive over-threshold samples, transient jitter is filtered out while real drift (a sustained offset) still corrects within ~1 frame of latency. This eliminates the most common cause of dropped frames on the studio thumbnail grid.

What changed

  • Each _parentMedia entry gains a driftSamples counter that increments while the absolute drift is above MIRROR_DRIFT_THRESHOLD_SECONDS and resets to 0 on the first sample below.
  • _mirrorParentMediaTime(el, opts) only writes back when driftSamples >= 2, except when opts.force === true.
  • promoteToParentProxy and _onIframeMediaAdded pass force: true so the first alignment after registration is still immediate (these are user-visible state transitions, not steady-state telemetry).

Test plan

  • 11 new unit/integration tests in hyperframes-player.test.ts covering:
    • Single-sample jitter does not trigger a write.
    • Two-sample sustained drift does trigger a write.
    • Trending drift correction (gradually increasing offset) is detected within 2 samples.
    • force: true override bypasses the sample requirement.
    • Out-of-range proxies (proxies whose source has been removed) do not panic.
    • Multiple proxies maintain independent counters — drift on one does not affect the other.
    • _promoteToParentProxy alignment is immediate.

Stack

Step P1-4 of the player perf proposal. Builds on P1-1 (shared adopted stylesheets) and P1-2 (scoped media observer). Together these three target the studio multi-player render path — P0-1* perf gate scenarios will pick up the wins automatically.

Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right design. Two-sample gate absorbs single-tick jitter (bridge interval, GC pause, throttled rAF) without thrashing currentTime, and the force: true escape hatch preserves zero-tolerance semantics at ownership promotion and proxy initialization — the two moments where a misaligned audible sample would be perceptually unacceptable.

Counter-reset-on-threshold-recovery is the critical invariant and it's exercised directly (a sample coming back within threshold clears the counter, so a later spike can't piggy-back on stale state). Similarly, out-of-range samples zero the counter — a proxy dropping out of its active window and returning later starts fresh.

The integration test (_promoteToParentProxy invokes _mirrorParentMediaTime with force: true) is the kind of call-site pin I'd want on a subtle perf contract like this: without it, a future refactor could remove { force: true } from the promotion path and every test still passes even though ownership flips would audibly drift.

Worth-calling-out-but-not-blocking: MIRROR_REQUIRED_CONSECUTIVE_DRIFT_SAMPLES = 2 is a constant — if bridge cadence ever changes (the comment cites bridgeMaxPostIntervalMs: 80, so worst-case correction latency is ~160ms), 2 may need to be re-tuned. Consider a doc-comment cross-reference so whoever touches the bridge cadence sees the coupling.

Approved.

Rames Jusso

Require two consecutive readings above MIRROR_DRIFT_THRESHOLD_SECONDS
before writing back to el.currentTime, eliminating seek thrash from
transient jitter on the parent timeline. Each _parentMedia entry
gains a driftSamples counter; promoteToParentProxy and
_onIframeMediaAdded pass force:true to keep one-shot alignments
immediate.

Adds 11 unit/integration tests covering jitter, trending drift, force
override, out-of-range proxies, multi-proxy independence, and
_promoteToParentProxy alignment.
@vanceingalls vanceingalls force-pushed the perf/p1-4-coalesce-mirror-parent-media-time branch from 7600cd7 to 62e91f3 Compare April 22, 2026 00:43
Per reviewer feedback on #396: surface the coupling between the
mirror-loop's required-consecutive-drift-samples constant and the
timeline-control bridge's max post interval, in both directions, so a
future change to either side immediately points at the other.

The coupling is:
  worst_case_correction_latency_ms
    ≈ MIRROR_REQUIRED_CONSECUTIVE_DRIFT_SAMPLES × bridgeMaxPostIntervalMs

At today's values (2 × 80 ms = 160 ms) this stays under the perceptual
A/V re-sync tolerance. Doc-only change — no behavior delta.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants