Skip to content

fix(core): a resized preview video keeps its picture while its new copy loads - #6922

Merged
miguel-heygen merged 3 commits into
mainfrom
fix/preview-proxy-swap-keeps-frame
Nov 1, 2026
Merged

miguel-heygen merged 3 commits into
mainfrom
fix/preview-proxy-swap-keeps-frame

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

What

When the preview resizes a video and switches it to a proxy copy made for its new size, the video keeps showing its last frame until the new copy can draw. Before, it showed black for about 100-250 ms.

Why

A preview proxy is sized to the box the video is shown at. When the stage changes size (a panel opening or folding beside it, a window resize), keepSized asks for a copy at the new size and loadProxy swaps it in with el.src = href; el.load(). That empties the element (emptied, readyState 0), and the video draws nothing until the new file's first frame decodes. At a lowered preview quality any change of box asks for a new copy, so a panel that settles over a few frames swaps several times and the picture blinks black each time. Putting the original back when quality returns to full (restoreOriginal) does the same.

How to reproduce: open a project with a video clip, lower the preview quality, then change the preview's size a few times. A per-frame recording shows the picture area black for 3-7 frames after each change. A text-only composition never does: there is no media element to empty.

Related work

None found.

How

  • holdShownFrame (in mediaProxy.ts, called by both swaps): before the source changes, a video showing a frame gets a still of that frame as its poster. The still is a JPEG at most 640 px wide, drawn through a canvas. A video with no data paints its poster with the same object-fit, so the old picture stays up through the gap. The authored poster (or none) comes back once the new file has a frame and is not seeking, or at once if it errors.
  • Nothing is held when there is no frame yet (the first swap of a file the browser cannot decode), when the canvas is unavailable, or when the frame cannot be read (a cross-origin frame taints the canvas). Those swap exactly as before.
  • heldPosters/unheldPoster sit beside swappedElements/unproxiedSrc in proxySrc.ts, and a scene swap's writtenShape reads the poster as authored, as it already reads src. A video that is mid-swap still counts as unchanged markup.
  • Putting the original back with no src to restore (the video's file came from a <source> child) loads nothing, so no event would release a held still. That path lets go of any held still at once instead of holding a new one.
  • The still is released on loadeddata or seeked only when the video can draw and is not seeking. In six recorded runs the runtime started its seek to the playhead at loadedmetadata, before loadeddata, so the release waited through it: every loadeddata after a swap arrived at the playhead time, never at frame 0. No seek followed a loadeddata within the swap.
  • Not changed: at a lowered quality a copy must still match the box exactly, so a panel settling over a few frames still asks for several copies. Each swap now keeps its picture; asking for fewer copies is a separate change.

Size budget

@hyperframes/core's packed budget goes from 1,661,748 to 1,762,752 bytes: the size CI measured at this PR's head, 1,678,811, × 1.05, as #6865 did for the unpacked budget. Main already packs at 0.95-0.98% over the old budget (1,677,535 bytes when built locally). This PR adds 818 packed bytes, about 1 KB of minified code that ships twice (the runtime IIFE and its inlined copy), which tipped it past the 1% tolerance. The minified hold is already lean; dropping bytes would mean dropping the still.

Test plan

  • Unit tests added/updated: mediaProxy.test.ts
    • A resize swap shows the still while the copy loads, keeps it through a seek, then puts the authored poster back.
    • A failed copy lets go of the still.
    • A video with no frame holds nothing.
    • Putting the original back at full quality holds the frame too.
    • Putting the original back with no src to restore lets go of a held still.
    • A scene swap reads the authored poster.
  • fails without the fix: packages/core/src/runtime/mediaProxy.test.ts › a copy sized to how large the video shows › shows a still of its frame while a resized copy loads, then its own poster again
  • Each new test fails without its line. Removing the hold from the resize swap fails 2; from the restore, 1; releasing only on a drawable frame (not on error), 1; reading poster as written, 1; not letting go when nothing loads back, 1.
  • packages/core: vitest run src/runtime 2697 passed (112 files); mediaProxy.test.ts passed 3 runs in a row; tsc --noEmit, oxlint and oxfmt clean on the changed files.
  • Manual testing performed: a desktop app preview playing a 1080p film at preview quality 1/2, with the panel beside it folded and opened 5 times per run, recorded at 30 fps and each frame's picture luma measured. The runtime was patched with this change in place, toggled on or off per run.
    • Dark picture frames per run: before 20, 19, 15; after 0, 2, 0, 0, 0, 0. The 2 frames in one run came at a swap that landed as the film reached its end; three more runs with per-frame logging did not reproduce it, and in those every swap either took a still or already held one.
    • Main-thread time per swap (machine with software rendering, no GPU): before median 0.4 ms, max 1.8 ms; after median 5.1 ms, p90 9.4 ms, max 15.1 ms when a still is drawn, under 1 ms when there is no frame to hold. The cost is the canvas draw and JPEG encode of a still at most 640 px wide.
  • Comments follow CONTRIBUTING.md "Comments"

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at c620429b.

Blocker (process): this is a fix PR, and the body has no fails without the fix: line naming a test. The test plan says what breaks when each line is removed ("fails 2… 1… 1… 1"), but it names no test on that line. Please add one, for example:

fails without the fix: packages/core/src/runtime/mediaProxy.test.ts › a copy sized to how large the video shows › shows a still of its frame while a resized copy loads, then its own poster again

I'll approve at the new head once that's there and nothing else changes.

Code: I found no blockers. Notes, none blocking:

  • restoreOriginal with original === null removes src and calls load(), so no loadeddata/seeked/error follows. The held still then stays as the poster for good, which is a frozen frame where the element used to be empty. That needs a <video> with no src attribute that was still proxied, so it's unlikely, but you could skip holdShownFrame when original === null or also release on emptied there.
  • loadeddata with seeking === false releases before the runtime seeks the new file to the playhead. On a paused swap away from t=0 that can flash frame 0 for a frame instead of showing black. That's still better than before, but the manual recording is worth checking for it.
  • writtenShape now drops an authored poster="", the same way it drops an empty class. That's fine.
  • Manual testing is still unchecked ("follows in a comment").

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 104 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (1)

  • seqplainkeys-none-center-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo false, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed at a1aedfed. Approving.

  • The body now has a fails without the fix: line. The named test (… shows a still of its frame while a resized copy loads, then its own poster again) expects posters to be [STILL] at load, and without holdShownFrame in loadProxy that can't happen, so it does fail without the fix.
  • Putting the original back with no src no longer leaves a still stuck: letGoOfStill puts the authored poster back right away, and a new test covers it. The release condition is the earlier early-return turned inside out, so nothing else about it changed.
  • My earlier frame-0 concern is answered by the recorded runs: the seek starts at loadedmetadata, before loadeddata.
  • The Comments check passes at this head.

Nit (not blocking): when the original's file comes from a <source> child, load() does pick that child and load it. So this path still blinks black instead of holding a still; it just can't get stuck any more. You could hold here and release on loadeddata, but that can be a follow-up.

…py loads

Swapping a video to a copy made for its new size emptied it until the copy
decoded, so the preview showed black for a moment. The swap now shows a still
of the frame it had, as the video's poster, until the new copy can draw.
@miguel-heygen
miguel-heygen force-pushed the fix/preview-proxy-swap-keeps-frame branch from a1aedfe to 8b1c1d3 Compare November 1, 2026 11:40

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed at 8b1c1d38. Approving.

  • This head was rebased onto main. The engine and streaming-encoder changes in the compare come from #6920 and #6926, not from this PR. The PR's own diff is the same as a1aedfed, which I approved, plus scripts/package-size-budgets.json.
  • The budget figure checks out: 1,678,811 × 1.05 = 1,762,751.55, rounded up to 1,762,752. That's the same rule #6865 used for unpacked. The 818 bytes this PR adds are the runtime's hold code, which ships twice.
  • The fails without the fix: line is still present and names a real test.

Nit (not blocking): resetting to measured +5% leaves about 84 KB of headroom on @hyperframes/core packed. The next few size jumps won't trip the 1% tolerance, so whoever owns the budget may want to tighten it again later.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit af0d68d Nov 1, 2026
144 of 145 checks passed
@miguel-heygen
miguel-heygen deleted the fix/preview-proxy-swap-keeps-frame branch November 1, 2026 12:16

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Post-merge advisory review (this PR is already merged into main). Findings flagged for follow-up, not gating.

Reviewed at 8b1c1d3891a890622e3207f00be4633e5efee03c.

No new correctness finding. Both source-swap paths use the same hold/release helper (packages/core/src/runtime/mediaProxy.ts:205-290), and scene reconciliation compares the authored poster rather than the temporary still (init.ts:384-398). The release condition requires a drawable, non-seeking frame, or an error; release removes its three listeners. Rapid resizes reuse one hold; tainted-canvas failure leaves the original poster and proceeds with the swap.

Genesis's earlier process concern is resolved: the body names a real regression on its fails without the fix: line. The no-src restore releases an existing hold immediately. Genesis already notes that a <source> child can load after this branch, so that path can still blink; this review adds no duplicate finding. The packed budget arithmetic is consistent with the stated measured-size-plus-5% rule; I did not independently reproduce the package-size or recorded preview timing measurements.

Validation: core build passed. node ../../node_modules/vitest/vitest.mjs run src/runtime/mediaProxy.test.ts src/runtime/init.swapScenes.test.ts passed 234 tests. Four independent controls also passed: repeated swaps and listener cleanup, exact absent/empty poster restoration, and tainted-canvas fallback (238 combined). Removing the two hold calls made three relevant regression tests fail. No Desktop recording, physical-browser playback or visual alpha-video acceptance was rerun.

Need: yes, avoid blank preview frames during source swaps · Size: ok · Reuse: existing proxy-source identity bookkeeping and one hold helper · Simpler: already minimal · Structure: fine
Risk: medium — source-swap timing and authored-media identity affect preview continuity.

Verdict: COMMENT (post-merge: Ready)
Reasoning: No new defect found at this head; lifecycle controls and scene-reconciliation tests pass. Existing source-child and size-budget notes remain follow-ups.

— hf-review

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