fix(runtime): silent-first-play + loading overlay for preview

packages/core/src/runtime/adapters/lottie.ts

@@ -187,3 +187,24 @@ interface LottieWindow extends Window {
   /** Compositions register their Lottie animation instances here for the adapter to seek. */
   __hfLottie?: Array<LottieWebAnimation | DotLottiePlayer>;
 }
+
+/**
+ * Whether a registered Lottie animation has finished loading its JSON source.
+ *
+ * Handles both supported player shapes:
+ * - `lottie-web` exposes a boolean `isLoaded` property on `AnimationItem`.
+ * - `@dotlottie/player-component` doesn't have `isLoaded`; we infer readiness
+ *   from `totalFrames > 0` since that value is only populated once the
+ *   manifest/animation JSON has been parsed.
+ *
+ * Exported so consumers (e.g. the studio "Loading assets…" overlay) share a
+ * single source of truth instead of re-implementing the duck-typing per call
+ * site.
+ */
+export function isLottieAnimationLoaded(anim: unknown): boolean {
+  if (typeof anim !== "object" || anim === null) return true; // unknown shape — don't block
+  const maybe = anim as { isLoaded?: boolean; totalFrames?: number };
+  if (maybe.isLoaded === true) return true;
+  if (typeof maybe.totalFrames === "number" && maybe.totalFrames > 0) return true;
+  return false;
+}

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

@@ -145,6 +145,17 @@ describe("refreshRuntimeMediaCache", () => {
 });
 
 describe("syncRuntimeMedia", () => {
+  function fakePlayedRanges(el: HTMLMediaElement, ranges: Array<[number, number]>): void {
+    Object.defineProperty(el, "played", {
+      configurable: true,
+      get: () => ({
+        length: ranges.length,
+        start: (i: number) => ranges[i][0],
+        end: (i: number) => ranges[i][1],
+      }),
+    });
+  }
+
   function createMockClip(overrides?: Partial<RuntimeMediaClip>): RuntimeMediaClip {
     const el = document.createElement("video") as HTMLVideoElement;
     document.body.appendChild(el);
@@ -153,6 +164,9 @@ describe("syncRuntimeMedia", () => {
     el.pause = vi.fn();
     Object.defineProperty(el, "currentTime", { value: 0, writable: true, configurable: true });
     Object.defineProperty(el, "playbackRate", { value: 1, writable: true, configurable: true });
+    // Default: audio has been playing — so drift-seek forward is allowed.
+    // Tests that exercise the "cold first play" guard call fakePlayedRanges(el, []).
+    fakePlayedRanges(el, [[0, 1]]);
     return {
       el,
       start: 0,
@@ -178,35 +192,42 @@ describe("syncRuntimeMedia", () => {
     expect(clip.el.play).toHaveBeenCalled();
   });
 
-  it("defers play on unbuffered media and calls load()", () => {
+  it("plays synchronously even when media is unbuffered (preserves user gesture)", () => {
+    // Calling play() synchronously inside the user-gesture call chain lets the
+    // browser queue playback until data buffers, while consuming the transient
+    // user activation. Deferring to an async canplay handler would let the
+    // activation expire and the autoplay policy silently reject — producing
+    // the "silent first play, audio only after second click" bug.
     const clip = createMockClip({ start: 0, end: 10 });
     Object.defineProperty(clip.el, "readyState", { value: 0, writable: true });
-    const loadSpy = vi.spyOn(clip.el, "load").mockImplementation(() => {});
-    const addEventSpy = vi.spyOn(clip.el, "addEventListener");
     syncRuntimeMedia({ clips: [clip], timeSeconds: 5, playing: true, playbackRate: 1 });
-    expect(clip.el.play).not.toHaveBeenCalled();
-    expect(loadSpy).toHaveBeenCalledOnce();
-    expect(addEventSpy).toHaveBeenCalledWith("canplay", expect.any(Function), { once: true });
+    expect(clip.el.play).toHaveBeenCalled();
   });
 
-  it("plays when canplay fires after deferred play", () => {
+  it("nudges preload to auto AND calls play() on unbuffered media in one pass", () => {
+    // Assets added after the runtime already bound its metadata listeners
+    // (e.g. a sub-composition that injects a late <audio>) need both the
+    // preload nudge and the play() call from the same sync tick — the
+    // reviewer flagged that these two concerns must not drift.
     const clip = createMockClip({ start: 0, end: 10 });
     Object.defineProperty(clip.el, "readyState", { value: 0, writable: true });
-    vi.spyOn(clip.el, "load").mockImplementation(() => {});
+    Object.defineProperty(clip.el, "preload", { value: "metadata", writable: true });
     syncRuntimeMedia({ clips: [clip], timeSeconds: 5, playing: true, playbackRate: 1 });
-    expect(clip.el.play).not.toHaveBeenCalled();
-    clip.el.dispatchEvent(new Event("canplay"));
+    expect(clip.el.preload).toBe("auto");
     expect(clip.el.play).toHaveBeenCalled();
   });
 
-  it("does not re-register listener on repeated ticks while unbuffered", () => {
+  it("does not re-fire play() while a previous play() is in flight", () => {
+    // Without a play-request dedup, the 50ms runtime poll would fire 20–40
+    // spurious play() calls per element during the 1–2s initial buffer, each
+    // with a catch() that would swallow any real AbortError / NotAllowedError
+    // the developer needs to see.
     const clip = createMockClip({ start: 0, end: 10 });
-    Object.defineProperty(clip.el, "readyState", { value: 0, writable: true });
-    const loadSpy = vi.spyOn(clip.el, "load").mockImplementation(() => {});
+    Object.defineProperty(clip.el, "readyState", { value: 4, writable: true });
     syncRuntimeMedia({ clips: [clip], timeSeconds: 5, playing: true, playbackRate: 1 });
-    syncRuntimeMedia({ clips: [clip], timeSeconds: 5.1, playing: true, playbackRate: 1 });
-    syncRuntimeMedia({ clips: [clip], timeSeconds: 5.2, playing: true, playbackRate: 1 });
-    expect(loadSpy).toHaveBeenCalledOnce();
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 5.02, playing: true, playbackRate: 1 });
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 5.04, playing: true, playbackRate: 1 });
+    expect(clip.el.play).toHaveBeenCalledTimes(1);
   });
 
   it("pauses active clip when not playing", () => {
@@ -229,19 +250,78 @@ describe("syncRuntimeMedia", () => {
     expect(clip.el.volume).toBe(0.7);
   });
 
-  it("seeks when currentTime drifts > 0.3s", () => {
+  it("hard-syncs on the first active tick (sub-composition activation, mediaStart offsets)", () => {
     const clip = createMockClip({ start: 0, end: 10, mediaStart: 0 });
     Object.defineProperty(clip.el, "currentTime", { value: 0, writable: true });
     syncRuntimeMedia({ clips: [clip], timeSeconds: 5, playing: false, playbackRate: 1 });
     expect(clip.el.currentTime).toBe(5);
   });
 
-  it("does not seek when currentTime is close enough", () => {
+  it("does not seek on sub-0.5s drift in steady-state — avoids pause/play hiccups", () => {
     const clip = createMockClip({ start: 0, end: 10, mediaStart: 0 });
-    Object.defineProperty(clip.el, "currentTime", { value: 5.1, writable: true });
-    const original = clip.el.currentTime;
-    syncRuntimeMedia({ clips: [clip], timeSeconds: 5, playing: false, playbackRate: 1 });
-    expect(clip.el.currentTime).toBe(original);
+    Object.defineProperty(clip.el, "currentTime", { value: 5.4, writable: true });
+    // Establish a baseline offset of 0 with a steady-state tick first.
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 5.4, playing: true, playbackRate: 1 });
+    // Now a small transient drift: timeline backs up 0.4s (typical of
+    // pause/play ordering). Below the 0.5s threshold — don't seek.
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 5, playing: true, playbackRate: 1 });
+    expect(clip.el.currentTime).toBe(5.4);
+  });
+
+  it("does not force audio forward while it's still buffering (gradual drift growth)", () => {
+    // Cold-play: audio stuck buffering at 0, timeline advances ~16ms per tick.
+    // The offset grows gradually; no single tick jumps by 0.5s, so the
+    // drift-correction seek must NOT fire. Without this guard the runtime
+    // would force-seek audio forward and the user would miss the opening
+    // words of the narration.
+    const clip = createMockClip({ start: 0, end: 10, mediaStart: 0 });
+    Object.defineProperty(clip.el, "currentTime", { value: 0, writable: true });
+    // First tick: timeline at 0, audio at 0, no drift — first-tick hard-sync is a no-op.
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 0, playing: true, playbackRate: 1 });
+    // Subsequent ticks: timeline advances, audio stays buffering at 0.
+    for (let t = 0.016; t < 0.7; t += 0.016) {
+      syncRuntimeMedia({ clips: [clip], timeSeconds: t, playing: true, playbackRate: 1 });
+    }
+    expect(clip.el.currentTime).toBe(0);
+  });
+
+  it("re-syncs on a scrub — offset jumps in one tick", () => {
+    const clip = createMockClip({ start: 0, end: 20, mediaStart: 0 });
+    Object.defineProperty(clip.el, "currentTime", { value: 2, writable: true });
+    // Steady-state.
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 2, playing: true, playbackRate: 1 });
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 2.02, playing: true, playbackRate: 1 });
+    // User scrubs forward to 15 — offset jumps from ~0 to ~13 in one tick.
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 15, playing: true, playbackRate: 1 });
+    expect(clip.el.currentTime).toBe(15);
+  });
+
+  it("catastrophic-drift safety valve eventually resyncs a stuck element", () => {
+    const clip = createMockClip({ start: 0, end: 100, mediaStart: 0 });
+    Object.defineProperty(clip.el, "currentTime", { value: 0, writable: true });
+    // Establish baseline at t=0.
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 0, playing: true, playbackRate: 1 });
+    // Gradually advance timeline by 0.3s per tick without audio moving.
+    // Each tick's offset delta is 0.3 (< 0.5s jump threshold), so only the
+    // >3s catastrophic-drift safety valve can trigger the resync.
+    for (let t = 0.3; t <= 4; t += 0.3) {
+      syncRuntimeMedia({ clips: [clip], timeSeconds: t, playing: true, playbackRate: 1 });
+    }
+    expect(clip.el.currentTime).toBeGreaterThan(3);
+  });
+
+  it("clears offset baseline when clip deactivates — re-entry hard-syncs", () => {
+    const clip = createMockClip({ start: 0, end: 5, mediaStart: 0 });
+    Object.defineProperty(clip.el, "currentTime", { value: 0, writable: true });
+    // Active pass: establish baseline at t=2.
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 2, playing: true, playbackRate: 1 });
+    // Deactivate: timeline moves past the clip window.
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 6, playing: true, playbackRate: 1 });
+    // Re-activate at t=3 — first-tick hard-sync should fire despite having
+    // a previous baseline, because the clip was inactive in between.
+    Object.defineProperty(clip.el, "currentTime", { value: 0, writable: true });
+    syncRuntimeMedia({ clips: [clip], timeSeconds: 3, playing: true, playbackRate: 1 });
+    expect(clip.el.currentTime).toBe(3);
   });
 
   it("sets per-element playbackRate × global rate", () => {

packages/core/src/runtime/media.ts

@@ -66,9 +66,26 @@ export function refreshRuntimeMediaCache(params?: {
   return { timedMediaEls: mediaEls, mediaClips, videoClips, maxMediaEnd };
 }
 
-// Elements with a pending deferred play — prevents re-calling load()/addEventListener
-// on every tick while the media is still buffering.
-const pendingPlay = new WeakSet<HTMLMediaElement>();
+// Per-element timeline→media offset from the previous tick. Used to tell a
+// gradual drift (initial buffer catch-up, where offset grows ~16ms/tick) from
+// a scrub (where offset jumps in one tick). Cleared when a clip becomes
+// inactive so the next activation gets a hard resync on its first tick.
+const lastOffset = new WeakMap<HTMLMediaElement, number>();
+
+// Elements whose play() is in flight. The sync runs on a 50 ms poll and with
+// a 1–2 s buffer that would fire 20–40 spurious play() calls per element —
+// noise in devtools and, worse, each `.catch(() => {})` would swallow a real
+// AbortError / NotAllowedError that should surface. Cleared on the `playing`
+// event (actual playback started) or on `pause`/`error` (state ended).
+const playRequested = new WeakSet<HTMLMediaElement>();
+function markPlayRequested(el: HTMLMediaElement): void {
+  if (playRequested.has(el)) return;
+  playRequested.add(el);
+  const clear = () => playRequested.delete(el);
+  el.addEventListener("playing", clear, { once: true });
+  el.addEventListener("pause", clear, { once: true });
+  el.addEventListener("error", clear, { once: true });
+}
 
 export function syncRuntimeMedia(params: {
   clips: RuntimeMediaClip[];
@@ -97,36 +114,73 @@ export function syncRuntimeMedia(params: {
       } catch {
         // ignore unsupported playbackRate
       }
-      if (Math.abs((el.currentTime || 0) - relTime) > 0.3) {
+      // Drift correction. Forcing `el.currentTime = relTime` every frame
+      // causes an audible seek+rebuffer hiccup (readyState drops briefly).
+      //
+      // We only want to correct drift that came from an *event* — an explicit
+      // user seek, a sub-composition activation, or a timeline jump — not
+      // drift that grew naturally from initial-buffer latency. Telling them
+      // apart by timing: scrubs move the timeline-to-media offset by seconds
+      // in a single tick; buffer catch-up grows the offset by ~one frame
+      // (<20ms) per tick.
+      //
+      // The first tick a clip is active we don't have a previous offset to
+      // compare against — treat that as a hard resync so sub-compositions
+      // with non-zero `mediaStart` land on the right frame.
+      //
+      // Tradeoff: the 3 s catastrophic-drift valve means an unnoticed
+      // steady-state drift can accumulate up to ~3 s before we correct.
+      // For music / motion graphics this is inaudible; for lip-synced
+      // dialogue it is not. If that becomes a target use case, switch to
+      // a short-window tight threshold (e.g. tighten to 0.15 s when the
+      // last play/pause transition was >500 ms ago).
+      const currentElTime = el.currentTime || 0;
+      const drift = Math.abs(currentElTime - relTime);
+      const offset = relTime - currentElTime;
+      const prevOffset = lastOffset.get(el);
+      lastOffset.set(el, offset);
+      const firstTickOfClip = prevOffset === undefined;
+      const offsetJumped = !firstTickOfClip && Math.abs(offset - prevOffset!) > 0.5;
+      const catastrophicDrift = drift > 3;
+      if (drift > 0.5 && (firstTickOfClip || offsetJumped || catastrophicDrift)) {
         try {
           el.currentTime = relTime;
         } catch {
           // ignore browser seek restrictions
         }
       }
-      if (params.playing && el.paused && !pendingPlay.has(el)) {
-        if (el.readyState >= HTMLMediaElement.HAVE_FUTURE_DATA) {
-          void el.play().catch(() => {});
-        } else {
-          pendingPlay.add(el);
-          if (el.preload !== "auto") el.preload = "auto";
-          el.addEventListener(
-            "canplay",
-            () => {
-              pendingPlay.delete(el);
-              if (!el.paused) return;
-              void el.play().catch(() => {});
-            },
-            { once: true },
-          );
-          el.addEventListener("error", () => pendingPlay.delete(el), { once: true });
-          el.load();
-        }
+      if (params.playing && el.paused && !playRequested.has(el)) {
+        // `HTMLMediaElement.play()` is spec'd to queue playback and resolve
+        // once enough data is buffered, so we can unconditionally call it —
+        // no need to gate on `readyState` or defer to a `canplay` listener.
+        //
+        // The old `readyState < HAVE_FUTURE_DATA` branch called `el.load()`
+        // inside the listener, which *aborts* the in-flight fetch that
+        // `bindMediaMetadataListeners` already started at init time and
+        // restarts from zero. On slow networks this delayed playback by
+        // seconds. The canplay listener was also racey — the event could
+        // fire between `load()` and `addEventListener` attachment, wedging
+        // the element waiting for a callback that never came.
+        //
+        // preload="auto" is already set at bind time in init.ts; the
+        // re-assignment here is defensive for media elements that were
+        // inserted after the runtime bound its listeners.
+        if (el.preload !== "auto") el.preload = "auto";
+        markPlayRequested(el);
+        void el.play().catch(() => {
+          // If play() rejects — e.g. autoplay blocked, element removed
+          // mid-flight — drop the in-flight flag so a future sync tick can
+          // retry rather than getting stuck waiting for `playing`/`pause`.
+          playRequested.delete(el);
+        });
       } else if (!params.playing && !el.paused) {
         el.pause();
       }
       continue;
     }
+    // Clip left its active window — drop the offset baseline so the next
+    // activation (e.g. re-entering a sub-composition) gets a hard resync.
+    lastOffset.delete(el);
     if (!el.paused) el.pause();
   }
 }