feat(studio): scrub full-quality video with decoded preview frames - #7379

Review at faa7546 (stacked on #7378). Changes requested before this merges. Not verified in a real browser yet.

Blocking: black-after-Play is still reachable. previewSeek.ts:244-249 (and :150) publishes at the 5 s cap whether or not the native element is ready. frames.clear() then removes the decoded canvas while the <video> may have no decoded frame, and transport.play() runs over a transparent layer. Readiness is only re-checked on loadedmetadata/loadeddata/seeked/error (previewSeek.ts:10). A native element already within 0.02 s of the target gets no seek from syncRuntimeMedia, so seeked never fires; at readyState 1 it waits the full 5 s and then clears. A unit-level probe (handoff(2), native currentTime 2, readyState 1) shows exactly that. Likely real triggers:

• Play at a clip's start on an element that only loaded metadata (the frame path sets preload="metadata")
• a native seek slower than 5 s

It's intermittent because the 120 ms warm timer (init.ts:5064-5074) usually seeks first. Suggestion: never clear the decoded canvas on a timeout. Keep each video's canvas until its native element presents a frame after play() (rVFC, or playing plus a first frame). Add canplay to EVENTS, and force a real seek when the element sits at the target with readyState < 2.

Blocking: required CI didn't run on this head (32 check runs vs 100 on #7378, with no Test/Typecheck/Build/Lint/Studio/SDK checks), presumably because the base isn't main. "Heavy films: Safari vs Chrome" failed with an isPlaying timeout. That's probably the known flake, but it needs a green rerun.

High: Play can be silently dropped. init.ts:5126 bails if nativePlayGeneration moved. Any seekWhenReady/seek/setPausedVideoFrameProvider during the ~278 ms handoff (e.g. an incidental timeline time-sync) cancels it after buffering was already posted. Should only an explicit pause or a user seek cancel a pending Play?

Medium

• Two stacked 5 s caps (decode never settles, then native fallback): up to ~10 s frozen preview (previewSeek.ts:244-247).
• init.ts:3711-3716: a provider swap at rest clears the presented canvases without a native resync, so a stale or blank native frame shows.
• pausedVideoFrames.ts:82-83: any currentTime write differing by ≥ 1e-8 permanently marks an authored seek, which disables decoded frames for that element for the life of the page. The runtime's own mediaElementTime adds 1e-5. I found no current path that trips it while paused, but a looser tolerance and a non-permanent mark would be safer.
• studio-server/src/routes/preview.ts:155,687: the runtime is now injected at head start (ahead of authored head scripts) for every preview and the sub-composition route, replacing insertRuntimeTag and its accelerated-canvas branch. That's a global load-order change worth its own PR or explicit sign-off.

Low

• Abort isn't wired to cancelReads/decode, so the newest seek can wait behind one stale decode.
• The source === request.source branch in sourceTiming is dead.
• el.src || el.currentSrc can be "", which adds a cooldown keyed "".
• "Original source detail" holds at quality 1 only. Hostile codecs decode the served transcode and lowered quality decodes the lighter copy; worth saying in the description.

Checked OK:

• abort and late-decode release, and the pending/runId/live() guards
• native fallback paths
• every graphics consumer (color grading, 2D drawImage patch, WebGL, WebGPU importExternalTexture with VideoFrame lifetime, rVFC)
• preview canvases are stripped from saved markup
• paused decode honours display size, SAR, rotation and flip
• frame-grid timing matches export
• arrow and drag-release settle

Locally, the 8 touched core suites pass 401/401. The Studio decoder tests weren't run (mediabunny ^1.65.2).

Verdict: REQUEST CHANGES · 1 blocker · 1 important · 0 nits · head faa7546b

I agree with @miguel-heygen's blocker: the native handoff can uncover a frameless video at the 5 s cap and keep playing over it. An independent unit repro confirms it: at 5.1 s, playing=true, canvases=0, native readyState=1, seeking=true. Ways to make it less likely: keep each decoded canvas until its native element presents a frame, and warm native elements (preload=auto plus a seek) while the provider owns the picture, because media.ts:666 skips them while paused. The gaps below are additive.

The live plate-8 report (readyState 4, zero canvases) is not explained by this diff. remove() restores the marker, the currentTime override, the callbacks and the canvas, and nothing here writes filter or ancestor styles. The reporter has since ruled out the filter and scene parking. One caution on that evidence: readyState 4 with currentTime advancing doesn't prove frames are being presented, because the media clock keeps running while the video renderer stalls. getVideoPlaybackQuality().totalVideoFrames and chrome://media-internals in the preserved tab would tell a decoder/renderer stall apart from a compositing problem.

— HyperFrames Team Review Agent (Genesis)

Verdict: REQUEST CHANGES — 2 resolved, 0 open, 3 new · faa7546b → aca914de

• Blocker: the dispatched Checks run 42414660582 is red because of the fix commit. I reproduced it locally: the same suites pass 230/230 at e220e96f and fail 4 at aca914de.
  • init.swapScenes.test.ts: "holds a parked destination…" fails, and "waits for the scene a seek lands in…" times out.
  • entry.test.ts: "resolves a mixed image/video seek only after both have decoded" fails.
  • mediaClock.test.ts: the currentTime-writer census fails (↓ inline).
  • Build also fails: the @hyperframes/producer packed size is 867,161 bytes, over its 857,000 budget.

— HyperFrames Team Review Agent (Genesis)

Re-review at 90555ab, covering aca914d in full plus the delta. Still changes needed: one High. Otherwise close.

Fixed:

• Black at the cap: fixed on the Play-handoff path and the paused path. My old probe (element at the target, readyState 1) no longer clears.
• CI: now runs on the stacked head. The aca914d reds are fixed: the producer size, the 3 core tests, and the raw currentTime writer at init.ts:5069.
  • The size budget was raised to 870000; please put the reason in the description, as the budget script asks.
• Echo seeks: they no longer drop a pending Play.
• Provider swap: it re-seeks instead of clearing.
• Authored-seek tolerance: fixed.
• Audio-only <video>: fixed.
• Pick mode: fixed.
• Decoder release before Play: fixed.
• Failed decoder load in suspend: now caught.

High, the handoff never times out. In previewSeek.ts expire, request.native → check() has no fallback. isUnplayable's stopped/never-started detection only arms on a play request (media.ts:212-213, 262-264), and the handoff runs before play(). A native element that Chrome stops loading after the header (readyState 1, network idle, no events) never counts as unplayable.

• Probe: still unresolved after 60 s, buffering, and the whole composition never plays. On main it plays and skips that clip after 5 s.
• Suggestion: after the cap, treat still-unready elements as unplayable for the handoff (start Play with the held or decoded picture still covering them), or count the cap as the play request.

Medium, a paused-seek expiry leaves a stale picture with no retry. The expiry resolves the request and removes the listeners, but never publishes or releases the hold.

• When the element later fires canplay/seeked, nothing commits. The picture stays old while the playhead shows the target.
• The hold attribute also stays, so grading's drawableFrame stays null and the held videos stay hidden until the next interaction.
• Suggestion: resolve, but keep the listeners so the target publishes once it's ready.

Low

• handoff cancel() runs before awaiting suspend. With only a hold snapshot showing (the init.ts:5137 branch), the mid-seek native video is uncovered during the await. Code reading only.
• No test covers the provider-swap resume-Play branch.
• The authored-seek mark is still permanent per element; fine if intended.
• swapScenes "waits for the scene a seek lands in…" was intermittent at aca914d. It passed at 90555ab; watch it.

Medium, unchanged: studio-server/src/routes/preview.ts:155 still injects the runtime at head start and drops insertRuntimeTag's accelerated-canvas tracker (htmlDocument.ts:396-400). Please confirm that's intended.

Locally the 338 core tests in the touched suites pass. Not run: a real browser, Safari, and the Studio vitest suite.

Verdict: REQUEST CHANGES — 3 resolved, 0 open, 1 new · aca914de → 7e43269d. The exact-head Checks run 42417875143 failed only the required Test: runtime contract job (↓ inline). Every other job is green.

— HyperFrames Team Review Agent (Genesis)

Re-review at a1f2a76 (base #7378 @ 4ffd675).

The isVideoElement swap is correct and matters: Studio can create body elements with the editor window's prototypes, so under instanceof the keep-set came out empty. NEW-2, NEW-3, the head-start injection and the provider swap are fixed, and each has a test that fails when the fix is reverted.

Still blocking: Play can wait forever when the element is ready at the moment Play is pressed.

run() calls notePlaybackIntent(el) before host.prepare seeks the element. If the element is at readyState >= 2 at that point, noteFirstPlayAsk stores firstPlayAskedAt = null, and stallSinceFirstAsk returns null forever (askedAt == null). If the handoff seek then leaves the element metadata-only with the network idle, isUnplayable never becomes true, so ready() never passes. The request.native branch of expire only re-runs check() every 5 s, so the request never publishes and the whole composition buffers until Pause. That is the common path: Play after a decoded-frame scrub, where the element is ready and gets seeked to the target. Probed handoffs to t=3, watched for 60 s:

• already metadata-only and idle at the target: resolves at 5 s
• ready at Play, seek leaves it metadata-only and idle: never resolves
• ready at Play, seek hangs while loading: never resolves
• metadata-only, network loading forever: never resolves

main plays the rest of the composition in all three hanging cases.

Simplest fix: on the Play path, once the cap has expired a second time, publish anyway and treat every clip that is still not ready the way clearFrames already treats stalled ones (keep its picture). Alternatively, start the stall clock after the handoff seek instead of before it.

Smaller items:

• After a paused expiry the request stays pending, so previewSeek.time stays set. transportTick returns early, paused media sync stops, and timed-visibility updates and the metadata handler are skipped until the source becomes ready or the user interacts. Before 7e43269 the expiry cleared the request.
• Every Play waits the full 5 s again for a source already skipped as stalled, because notePlaybackIntent resets the clock whenever the element is metadata-only. main skips it straight away after the first detection.
• No test covers the cap re-arm on the Play path or the picture kept by cancel(pending.expired); reverting either one breaks nothing.
• On recovery, the held picture is removed on loadeddata/canplay/seeked without checking the element's time against the transport, so a stale native frame can show briefly before media sync seeks it.

The readyState-4 no-picture stall is not addressed by this delta. Only a real-browser pass can settle it.

Verdict: REQUEST CHANGES — 1 resolved, 0 open, 0 new · 7e43269d → a1f2a762. Exact-head Checks 42418767367 are green. I'm holding the verdict for @miguel-heygen's Play-path blocker, which I confirmed by reading the code. If an element is at readyState ≥ 2 when notePlaybackIntent runs, firstPlayAskedAt becomes null, so isUnplayable can never trip. The request.native branch of expire (previewSeek.ts:277-279) then only re-checks and never publishes. If the handoff seek then stalls at readyState 1, the whole composition waits for Pause, whereas main plays the other layers. The Play path needs a hard publish deadline.

— HyperFrames Team Review Agent (Genesis)

Verdict: REQUEST CHANGES — 1 resolved (@miguel-heygen's unbounded Play: the request-wide 10 s deadline bounds it, including decoder release), 0 open, 1 new · a1f2a762 → 3f1dd2f4. Exact-head Checks 42423260935 are green. The new finding comes from reading the code; I haven't reproduced it in a browser.

— HyperFrames Team Review Agent (Genesis)

Re-review at 3f1dd2f (base #7378 @ 4ffd675).

The hard Play deadline works. The four stalled-handoff cases that hung on a1f2a76 (ready at Play then the seek stays metadata-only, seek hangs while loading, metadata-only loading forever, seek hangs after metadata) now all resolve at 10 s. Removing the deadline fails 7 tests, and reverting the keep-set fails 6. Background recovery for expired paused work and the no-re-arm for skipped sources also hold, each with a test that fails when the fix is reverted. CI on this head is green.

Blocker: a recovered video doesn't reappear while its clip is playing. This confirms the issue Genesis raised.

The recovery check (the clear callback in init.ts plus ready in pausedVideoFrames.ts) reveals the video only once currentTime is within 0.02 s of the moving transport, and otherwise seeks again. Each seeked lands one seek-latency behind the clock, so with more than about 20 ms of seek latency it re-seeks on every seeked. Probe: video ready at Play, handoff seek loads forever, the 10 s deadline passes, then the network delivers.

• 0 ms seek latency: revealed immediately.
• 40 ms: 100 seeks between t=1.05 and t=5.01, revealed only when the clip ended.
• 100 ms: 41 seeks, revealed at clip end.
• Error-then-recover path at 25 ms: 61 seeks, never revealed before the clip ended.

The result is a frozen picture for the rest of the clip, plus a seek loop on that element, which also disrupts its audio. The existing test passes only because land() completes the seek in 0 ms.

Suggested fix: during playback, reveal on the first frame presented after the video is playing (rVFC, or playing plus a timeupdate) and let media sync correct the drift. At minimum, do one corrective seek and then reveal within MEDIA_SYNC_TOLERANCE (0.04 s) instead of re-seeking on every seeked.

Smaller items:

• With a provider available, the seek hold is now a holdNative presented entry, but cancel() releases only the old-style hold. A cancel with no follow-up publish (for example onStopMedia) leaves the frozen snapshot over a hidden video until the next seek or publish.
• Not covered by any test: ownsRecovery (the transportMovableClips filter), the new !pausedFrames.retained(el) gate in the transport tick, and wallClearTimeout(deadline) in retireRequest. Removing any of them breaks nothing.
• Alpha reader: the change is correct. Each sample is closed exactly once on the finite-request path, opaque CFR prefetch is unchanged, and dimensions are unchanged. Worth saying in the description that transparent tracks lose forward prefetch, so each scrub step on an alpha video is a random-access decode. Minor: canBeTransparent() sits inside the Promise.all that opens the reader, so a rejection there fails the whole reader; consider .catch(() => true).

The readyState-4 no-picture stall still needs a real-browser pass.

Verdict: COMMENT (needs rebase) — 1 resolved, 0 open, 0 new · 3f1dd2f4 → 8a1c6ba7. I have no remaining code findings, and the dispatched Checks run 42425930335 is green. #7378 has merged and this PR now targets main, but it conflicts (mergeable_state: dirty), so the required checks can't run against main on this head. I'll approve the rebased head once its required CI is green and the patch matches this one. The intermittent VideoSample GC warning stays an open item for the author.

— HyperFrames Team Review Agent (Genesis)

Re-review at c1b235b (rebased onto main b87a01f). No blockers; this looks good to merge.

Rebase: the range-diff of 4ffd6759..8a1c6ba7 against b87a01f6..17b19cab2 shows all 7 commits identical, with no conflict-resolution edits. The only new commit is c1b235b.

Playback reveal (17b19ca): the earlier blocker is fixed. In the probe, Play's handoff stalls past the 10 s deadline and then the network delivers; transport sync is real, not stubbed.

The error-then-recover path also reveals with 0 seeks. The stalled-handoff cases still all resolve at the 10 s deadline. Snapshots are released on stop-media and kept when a provider replaces the request. Composition-registered frame callbacks still go through staging: the observer uses the unpatched rVFC, and routing it through the patched path fails the new test.

Alpha drain (c1b235b): every sample is closed exactly once on the normal, abort, iterator-error and dispose paths.

Medium, worth doing if you push again: the drain has no bound. If a transparent-video read stalls during paused scrubbing and the user then presses Play, suspend() never resolves on this head; on 8a1c6ba it resolved immediately. Play then waits out the full 10 s deadline instead of starting at once, and the stalled reader keeps its input and connection open until the stall clears. Racing reader.tail against ~1–2 s, then cancelling and disposing, keeps the leak fix for normal drains and restores immediate Play on a stall.

Medium, fine as a follow-up: a recovered video is revealed at the position where it stalled, and nothing seeks it to the playhead. Media sync only speeds it up to 1.25×, and hard-seeks only past 3 s of drift. Delivered 1.5 s after the deadline, it is still −0.91 s behind at clip end; delivered 2.5 s after, −2.15 s, which is visible as a plate out of step with its matte. One corrective seek at reveal when |drift| > 0.5 s fixes it, and a single seek can't loop.

Low:

• These still have no tests: the video.paused check on the progress path, the stale-entry guard in reveal, !driving on hold release, cancel(true) on a mode switch and on pause(), the deadline clear on retire, ownsRecovery, and the retained(el) transport gate.
• Without a frame provider, a mode switch now keeps the old snapshot, and new clip videos aren't held until publish.

Required checks on this head are green. The missing-plate stall at readyState 4 is still unproven either way.

Verdict: APPROVE — 0 open, 0 new · 8a1c6ba7 → c1b235be. I compared the old and rebased ranges: all 7 rebased commits are identical to their previous versions, with no conflict edits. The new alpha drain (c1b235be) keeps getSample semantics through a one-timestamp samplesAtTimestamps query and waits for in-flight alpha reads to finish before disposing the input. Every required workflow on this head is green, including CI, Windows render/tests and regression. The approval covers this SHA only. Known tradeoff, documented in the PR: if an alpha read stalls forever, its reader stays open, because cancelReads runs only after the drain finishes; the 10 s Play deadline still bounds playback.

— HyperFrames Team Review Agent (Genesis)