fix(player): the interactive playback clock absorbs a main-thread stall instead of skipping past it (#4053)

* fix(player): the interactive playback clock absorbs a main-thread stall instead of skipping past it

TransportClock's monotonic time is a bare wall-clock read with no
protection against a long gap between reads. On a composition whose
first real paint blocks the main thread for several seconds, nothing
reads the clock during that time; the next read reported the whole
gap as elapsed playback, so the composition jumped forward instead of
continuing from where it froze.

A gap over 500ms is now treated as a stall and corrected to a 33ms
advance instead of reported whole, matching GSAP's own ticker default.
The correction lives inside now() itself so every reader benefits,
since this clock is read every real frame by more than one driver.
Render/export capture is unaffected: it always pauses and seeks to an
exact time rather than playing, so the correction never engages there.

* chore(player): drop internal tracker references from stall-policy comments

* fix(player): stop the stall correction from corrupting the clock while audio is authoritative

An independent review found that the previous commit's correction ran
unconditionally on every read, including while an attached audio
source was providing the time. Audio keeps playing through a
JS-thread stall, so a gap between reads there is not a stall — but the
correction still shifted the clock's internal reference point, and
that corruption only surfaced later, on an unrelated read, once the
clock fell back to monotonic timing (audio pausing or buffering).

The correction now runs only in the monotonic branch, after audio has
had its chance to answer. Added a regression test that fails against
the previous placement and passes against this one.

* fix(player): keep the stall correction's read timestamp fresh through an audio-authoritative stretch

A second independent review found the previous commit's fix was
incomplete: it stopped a stall correction from firing while audio was
answering a read, but the internal timestamp it compares against only
moved on monotonic reads. Any monotonic read from before audio took
over stayed the comparison point for as long as audio kept answering,
so the whole audio-authoritative stretch read as one giant stall the
moment audio dropped out again — reproducing the exact corruption the
previous commit set out to fix, through a different door. The same
gap also meant a real, non-stalled audio outage got smoothed away to
almost nothing instead of advancing normally.

The read timestamp now moves on every read, audio-answered or not;
only the correction itself stays scoped to the monotonic branch. Two
regression tests reproduce both failure modes and pass here; both were
confirmed to fail against the prior placement.

* fix(player): stop comparing a monotonic read against a timestamp audio made meaningless

A third review found the previous commit's fix was still incomplete:
keeping the read timestamp fresh during audio-authoritative reads
stops it going stale, but a real gap with no reads at all — the main
thread genuinely frozen while an attached audio element kept playing
on its own, unaffected by JS scheduling — still read as a stall the
moment audio next failed to answer, discarding real elapsed time that
audio had been correctly tracking the whole way.

An audio-answered read now clears the timestamp instead of stamping
it, the same as a seek. The next monotonic read then starts from
`_playStartMs`, which nothing but a genuine discontinuity (play, seek,
rate change, detach) ever touches — an anchor that already reconstructs
real elapsed time correctly on its own, without needing the correction
to reason about a comparison point it has no way to make meaningful
once audio was involved. A stall between two purely monotonic reads,
with no audio in between, is unaffected and still corrected exactly as
before.

Two regression tests cover the transition: a real unread gap while
audio was tracking correctly must not be discarded, and a sustained
audio outage must still advance at real speed rather than being
smoothed to nothing. Both were confirmed to fail against the prior
commit's placement.

* chore(core): trim clock comments

packages/core/src/runtime/clock-stall.test.ts

@@ -0,0 +1,225 @@
+import { describe, it, expect } from "vitest";
+import { TransportClock } from "./clock";
+
+// Stall policy for TransportClock's interactive playback — see the PR body.
+
+function createClock(opts?: ConstructorParameters<typeof TransportClock>[0]) {
+  let ms = 0;
+  const clock = new TransportClock({ nowMs: () => ms, ...opts });
+  const advance = (deltaMs: number) => {
+    ms += deltaMs;
+  };
+  return { clock, advance };
+}
+
+describe("TransportClock stall policy — interactive playback", () => {
+  it("a gap under the 500ms threshold advances exactly, unsmoothed", () => {
+    const { clock, advance } = createClock();
+    clock.play();
+    advance(16); // one ordinary frame at 60fps
+    expect(clock.now()).toBeCloseTo(0.016, 10);
+    advance(480); // still under threshold
+    expect(clock.now()).toBeCloseTo(0.496, 10);
+  });
+
+  it("blocking the thread for 4.6s (the measured frost-sequence-camera-orbit stall) advances by at most the adjusted lag, not the whole gap", () => {
+    const { clock, advance } = createClock();
+    clock.play();
+    advance(16);
+    const before = clock.now(); // establishes the read baseline the stall is measured against
+    advance(4600); // the rAF loop never got a chance to run during this
+    const after = clock.now(); // the first read after the stall is what applies the correction
+    expect(after - before).toBeCloseTo(0.033, 10);
+    expect(after - before).toBeLessThan(4.6);
+  });
+
+  it("a stall this severe (8s, the measured WebKit-engine case) is still corrected, not just the smaller 4.6s one", () => {
+    const { clock, advance } = createClock();
+    clock.play();
+    advance(16);
+    const before = clock.now();
+    advance(8000);
+    expect(clock.now() - before).toBeCloseTo(0.033, 10);
+  });
+
+  it("playback continues advancing normally after a stall — the correction does not stick the clock", () => {
+    const { clock, advance } = createClock();
+    clock.play();
+    advance(16);
+    clock.now(); // read baseline, so the next gap is actually measured as a stall
+    advance(4600); // stall
+    const afterStall = clock.now();
+    advance(16); // one more ordinary frame
+    expect(clock.now() - afterStall).toBeCloseTo(0.016, 10);
+  });
+
+  it("many reads within the same real frame do not each get treated as a separate stall window", () => {
+    const { clock, advance } = createClock();
+    clock.play();
+    advance(16);
+    // Several callers read `now()` within the same real frame (state sync,
+    // media sync, audio timing, plus a second driver like the parent-tick
+    // bridge) — none of those back-to-back reads should see each other as a
+    // gap.
+    const a = clock.now();
+    const b = clock.now();
+    const c = clock.now();
+    expect(a).toBe(b);
+    expect(b).toBe(c);
+    advance(4600); // a real stall before the next frame's reads
+    expect(clock.now() - a).toBeCloseTo(0.033, 10);
+  });
+
+  it("a second reader (simulating the parent-tick bridge) is protected by the same correction, with no wiring of its own", () => {
+    const { clock, advance } = createClock();
+    clock.play();
+    // Two independent "drivers" reading the same clock, exactly as
+    // packages/core/src/runtime/init.ts's transportTick and
+    // packages/player/src/hyperframes-player.ts's parent-tick bridge do —
+    // neither is a distinguished caller; both just call now().
+    const driverA = () => clock.now();
+    const driverB = () => clock.now();
+    advance(16);
+    driverA();
+    advance(4600); // stall, discovered by whichever driver reads first
+    const readByB = driverB(); // the OTHER driver — not the one that established the baseline
+    expect(readByB).toBeCloseTo(0.049, 10); // 0.016 + 0.033, corrected regardless of who asks
+  });
+
+  it("a long gap between reads while audio is authoritative does not corrupt the later monotonic fallback", () => {
+    const { clock, advance } = createClock({ duration: 30 });
+    const audioEl = { currentTime: 2, paused: false } as HTMLMediaElement;
+    clock.play();
+    clock.attachAudioSource({ el: audioEl, compositionStart: 0, mediaStart: 0 });
+    expect(clock.now()).toBe(2); // audio-authoritative, correct
+    advance(5000); // audio itself doesn't stall; nothing here reads the monotonic side
+    audioEl.currentTime = 5;
+    expect(clock.now()).toBe(5); // still audio-authoritative, still correct
+    // Audio becomes unavailable without a formal detachAudioSource() call —
+    // now() falls through to monotonic within the same read.
+    Object.assign(audioEl, { paused: true });
+    // First monotonic-branch read: must not be held back by the 5s audio gap.
+    // A corrupted `_playStartMs` would report ~33ms instead of the true ~5s.
+    expect(clock.now()).toBeCloseTo(5, 2);
+  });
+
+  it("a monotonic read before audio takes over does not go stale across the whole audio-authoritative stretch", () => {
+    const { clock, advance } = createClock({ duration: 30 });
+    const audioEl = { currentTime: 0, paused: true } as HTMLMediaElement;
+    clock.play();
+    clock.attachAudioSource({ el: audioEl, compositionStart: 0, mediaStart: 0 });
+    expect(clock.now()).toBeCloseTo(0, 5); // audio not ready yet, falls to monotonic
+    advance(10); // audio starts an instant later
+    Object.assign(audioEl, { paused: false, currentTime: 0.01 });
+    expect(clock.now()).toBeCloseTo(0.01, 2); // now audio-authoritative
+    advance(10_000); // ten real seconds pass, entirely under audio's own clock
+    audioEl.currentTime = 10.01;
+    expect(clock.now()).toBe(10.01); // still audio-authoritative, still correct
+    Object.assign(audioEl, { paused: true }); // audio drops out, no detachAudioSource()
+    // The stale pre-audio read must not be what this gets compared against:
+    // ~10.01s of real time genuinely passed and none of it was a stall.
+    expect(clock.now()).toBeCloseTo(10.01, 1);
+  });
+
+  it("real elapsed time during a sustained audio outage advances normally, not smoothed away as a stall", () => {
+    const { clock, advance } = createClock({ duration: 30 });
+    const audioEl = { currentTime: 0, paused: true } as HTMLMediaElement;
+    clock.play();
+    clock.attachAudioSource({ el: audioEl, compositionStart: 0, mediaStart: 0 });
+    // Audio stays unavailable (buffering) for two real seconds, polled at a
+    // normal frame cadence — no JS-thread stall anywhere in this window.
+    let reported = clock.now();
+    for (let frame = 0; frame < 120; frame++) {
+      advance(16.666);
+      reported = clock.now();
+    }
+    expect(reported).toBeCloseTo(2, 1);
+  });
+
+  it("a real unread gap while audio was tracking correctly is not mistaken for a stall once audio drops", () => {
+    const { clock, advance } = createClock({ duration: 30 });
+    const audioEl = { currentTime: 0, paused: false } as HTMLMediaElement;
+    clock.play();
+    clock.attachAudioSource({ el: audioEl, compositionStart: 0, mediaStart: 0 });
+    expect(clock.now()).toBe(0);
+    advance(100);
+    audioEl.currentTime = 0.1;
+    expect(clock.now()).toBeCloseTo(0.1, 5); // audio-authoritative, last confirmed position
+    // Nothing reads the clock for 5 real seconds — audio itself was still
+    // playing the whole time (native audio isn't gated by JS scheduling),
+    // there was no render stall here, and the composition should read as
+    // if 5s of real playback happened, not ~33ms of corrected "stall".
+    advance(5000);
+    Object.assign(audioEl, { paused: true }); // audio becomes unavailable right as reads resume
+    expect(clock.now()).toBeCloseTo(5.1, 1);
+  });
+
+  it("a long real gap while genuinely paused is not a stall — resuming play() does not inherit it", () => {
+    const { clock, advance } = createClock();
+    clock.play();
+    advance(16);
+    clock.pause();
+    advance(10_000); // ten real seconds paused — a user just left the tab
+    clock.play();
+    advance(16);
+    expect(clock.now()).toBeCloseTo(0.032, 10);
+  });
+
+  it("seeking right after a corrected stall does not inherit or re-trigger the correction", () => {
+    const { clock, advance } = createClock({ duration: 30 });
+    clock.play();
+    advance(16);
+    clock.now(); // read baseline
+    advance(4600); // stall
+    expect(clock.now()).toBeCloseTo(0.049, 10); // this read is what applies the correction
+    clock.seek(10); // an explicit user scrub — must override cleanly
+    expect(clock.now()).toBe(10); // the seek's target, not something the stall's math leaked into
+    advance(16); // ordinary next frame after the seek
+    expect(clock.now()).toBeCloseTo(10.016, 10);
+  });
+
+  it("a rate change right after a corrected stall applies the new rate from the corrected point, not the raw gap", () => {
+    const { clock, advance } = createClock();
+    clock.play();
+    advance(16);
+    clock.now(); // read baseline
+    advance(4600); // stall; setRate below is what next reads the clock
+    clock.setRate(2);
+    // setRate reads now() before switching rate, which is where the 4.6s gap
+    // gets corrected to the 33ms adjusted lag: 0.016 + 0.033 = 0.049.
+    advance(16); // one more frame, now at the new 2x rate
+    expect(clock.now()).toBeCloseTo(0.049 + 0.032, 5);
+  });
+});
+
+describe("TransportClock stall policy — render/export capture is untouched", () => {
+  // packages/producer's `renderSeek` (init.ts) always pauses then seeks; never plays.
+  it("a seek while paused reports exactly the seeked time, regardless of any elapsed wall time before it", () => {
+    const { clock, advance } = createClock({ duration: 30 });
+    clock.seek(1 / 30);
+    expect(clock.now()).toBe(1 / 30);
+    advance(60_000); // an arbitrarily long real gap between two render frames
+    clock.seek(2 / 30);
+    expect(clock.now()).toBe(2 / 30);
+  });
+
+  it("a full frame-by-frame render pass, each frame preceded by a large real-time gap, advances exactly — never smoothed", () => {
+    const { clock, advance } = createClock({ duration: 1 });
+    const fps = 30;
+    for (let frame = 0; frame <= fps; frame++) {
+      advance(5000); // rendering one frame is slow in wall-clock terms; irrelevant to the result
+      const target = Math.min(frame / fps, 1);
+      clock.seek(target);
+      expect(clock.now()).toBe(target);
+    }
+  });
+
+  it("isPlaying() is false throughout a render pass — the stall policy's gate is provably never open", () => {
+    const { clock, advance } = createClock({ duration: 1 });
+    for (let frame = 0; frame <= 30; frame++) {
+      advance(1000);
+      clock.seek(frame / 30);
+      expect(clock.isPlaying()).toBe(false);
+    }
+  });
+});

packages/core/src/runtime/clock.test.ts

@@ -10,6 +10,14 @@ function createClock(opts?: ConstructorParameters<typeof TransportClock>[0]) {
   return { clock, advance, getMs: () => ms };
 }
 
+/** Advances `totalMs` in sub-threshold steps, reading `now()` each time (see clock.ts's stall policy). */
+function advancePolled(clock: TransportClock, advance: (deltaMs: number) => void, totalMs: number) {
+  for (let remaining = totalMs; remaining > 0; remaining -= 250) {
+    advance(Math.min(250, remaining));
+    clock.now();
+  }
+}
+
 describe("TransportClock", () => {
   describe("initial state", () => {
     it("starts paused at time 0", () => {
@@ -108,8 +116,8 @@ describe("TransportClock", () => {
       clock.seek(10);
       expect(clock.isPlaying()).toBe(true);
       expect(clock.now()).toBe(10);
-      advance(1000);
-      expect(clock.now()).toBe(11);
+      advancePolled(clock, advance, 1000);
+      expect(clock.now()).toBeCloseTo(11, 5);
     });
 
     it("clamps to 0", () => {
@@ -235,8 +243,8 @@ describe("TransportClock", () => {
       advance(5000);
       clock.seek(0);
       expect(clock.now()).toBe(0);
-      advance(1000);
-      expect(clock.now()).toBe(1);
+      advancePolled(clock, advance, 1000);
+      expect(clock.now()).toBeCloseTo(1, 5);
     });
   });
 
@@ -288,7 +296,7 @@ describe("TransportClock", () => {
       expect(clock.now()).toBe(5);
       clock.detachAudioSource();
       expect(clock.now()).toBeCloseTo(5, 1);
-      advance(1000);
+      advancePolled(clock, advance, 1000);
       expect(clock.now()).toBeCloseTo(6, 1);
       expect(clock.getSource()).toBe("monotonic");
     });

packages/core/src/runtime/clock.ts

@@ -16,6 +16,10 @@ export type AudioClockSource =
       currentTimeSeconds: number;
     };
 
+/** GSAP's own `lagSmoothing` default — see the PR body for why this clock needs its own copy. */
+const STALL_THRESHOLD_MS = 500;
+const STALL_ADJUSTED_LAG_MS = 33;
+
 export class TransportClock {
   private _baseTime = 0;
   private _playStartMs: number | null = null;
@@ -23,6 +27,8 @@ export class TransportClock {
   private _duration = Infinity;
   private _nowMs: () => number;
   private _audioSource: AudioClockSource | null = null;
+  /** Wall-clock time of the last `now()` read while playing; null while paused. */
+  private _lastReadMs: number | null = null;
 
   constructor(opts?: {
     initialTime?: number;
@@ -39,8 +45,10 @@ export class TransportClock {
   now(): number {
     if (this._playStartMs === null) return this._baseTime;
 
-    // Audio-master: when an audio source is attached, derive time
-    // from it. Drift is impossible because audio IS the clock.
+    // Audio-master: drift is impossible because audio IS the clock. Clearing
+    // `_lastReadMs` (not stamping it) means the next monotonic read starts
+    // fresh from `_playStartMs` instead of a timestamp audio just made
+    // meaningless — see the PR body.
     if (this._audioSource) {
       let audioTime: number | null = null;
       if ("currentTimeSeconds" in this._audioSource) {
@@ -55,6 +63,7 @@ export class TransportClock {
         }
       }
       if (audioTime !== null) {
+        this._lastReadMs = null;
         if (Number.isFinite(this._duration) && audioTime >= this._duration) {
           return this._duration;
         }
@@ -63,6 +72,7 @@ export class TransportClock {
     }
 
     // Monotonic fallback
+    this._applyStallCorrection();
     const elapsed = (this._nowMs() - this._playStartMs) / 1000;
     const t = this._baseTime + elapsed * this._rate;
     if (Number.isFinite(this._duration) && t >= this._duration) {
@@ -71,10 +81,28 @@ export class TransportClock {
     return Math.max(0, t);
   }
 
+  /** Folds a >500ms gap since the last read into `_playStartMs` so it's never reported as played time. */
+  private _applyStallCorrection(): void {
+    if (this._playStartMs === null) return;
+    const nowMs = this._nowMs();
+    if (this._lastReadMs !== null) {
+      const gapMs = nowMs - this._lastReadMs;
+      if (gapMs > STALL_THRESHOLD_MS) {
+        this._playStartMs += gapMs - STALL_ADJUSTED_LAG_MS;
+      }
+    }
+    this._lastReadMs = nowMs;
+  }
+
   play(): boolean {
     if (this._playStartMs !== null) return false;
     if (Number.isFinite(this._duration) && this._baseTime >= this._duration) return false;
     this._playStartMs = this._nowMs();
+    // Not a stall: the gap since the clock was last read (possibly a long
+    // paused idle) says nothing about lost playback time, since none was
+    // playing. `_applyStallCorrection` treats null as "nothing to compare
+    // against yet" and starts the window fresh from the next read.
+    this._lastReadMs = null;
     return true;
   }
 
@@ -92,6 +120,9 @@ export class TransportClock {
     this._baseTime = clamped;
     if (this._playStartMs !== null) {
       this._playStartMs = this._nowMs();
+      // Same reasoning as `play()`: the seek itself, not any elapsed gap
+      // since the last read, is why the reported time is moving now.
+      this._lastReadMs = null;
     }
   }
 
@@ -104,6 +135,7 @@ export class TransportClock {
     if (this._playStartMs !== null) {
       this._baseTime = this.now();
       this._playStartMs = this._nowMs();
+      this._lastReadMs = null;
     }
     this._rate = safe;
   }
@@ -131,6 +163,9 @@ export class TransportClock {
     if (this._audioSource && this._playStartMs !== null) {
       this._baseTime = this.now();
       this._playStartMs = this._nowMs();
+      // Falling back to monotonic timing fresh, same reasoning as `play()`:
+      // any gap while audio was the time source is not a monotonic stall.
+      this._lastReadMs = null;
     }
     this._audioSource = null;
   }

packages/core/src/runtime/init.test.ts

@@ -2532,7 +2532,8 @@ describe("initSandboxRuntimeModular", () => {
     expect(player).toBeDefined();
 
     player?.play();
-    raf.step(1_000);
+    // Sub-threshold steps: the stall policy treats one big unread jump as a stall.
+    for (let steps = 0; steps < 4; steps++) raf.step(250);
 
     expect(player?.isPlaying()).toBe(true);
     expect(player?.getTime()).toBeCloseTo(1, 1);