Repository navigation
fix(core): snapshot and preview show a clip on the frame render shows it - #7238
Conversation
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 2 blockers · 0 important · 0 nits · head a323ae94
- blocker (Rule 2, a site this PR didn't change):
packages/cli/src/commands/snapshot.ts:252resolveSnapshotVideoFrameTimestill usesisClipVisibleAton the authored seconds. Snapshot uses it to decide which<video>gets an FFmpeg frame (activeVideoAt, used at :1263/:1307). Take a video atdata-start="100.00002"andsnapshot --at 100: the element is now visible throughisClipOnScreenAt, butactiveVideoAtreturns null, so no frame is pulled and the still at 100 s has an empty video. Render shows the frame there (videoFrameExtractor.ts:3089usesexportClipWindow). A start chained after a measured media length is the PR's own motivating case, so snapshot still keeps two rules. Fix: route it throughisClipOnScreenAtwith the canonical fps, and add aresolveSnapshotVideoFrameTimecase at 100 / 100.00002. - blocker (CI): the
Commentscheck fails, and the PR body doesn't explain why:RAISED comment share: packages/core/src/runtime/clipWindow.ts is 12.5% (5/40), at the base 11.5% (3/26). The 2-line JSDoc onisClipOnScreenAtraises the file's comment share. Shorten it to one line, or let the name carry it.
Scope
Read the full diff and every caller of isClipVisibleAt/exportClipWindow. media.ts:508 (terminal-video hold) on authored seconds is fine, since it only fires at or past the composition end. The fails without the fix: line is present. I didn't run the tests locally (bun isn't in this sandbox); the rest of CI was still running when I reviewed. No earlier reviews.
— Genesis V2 (Lane B)
There was a problem hiding this comment.
Verdict: REQUEST CHANGES — 1 resolved, 1 open, 0 new · a323ae94 → 4edf543a
✅ The snapshot video frame pick now uses isClipOnScreenAt at the composition's fps, which matches the runtime's compositionFps(), and the new resolveSnapshotVideoFrameTime case covers it. ❌ Comments still fails, now on packages/cli/src/commands/snapshot.ts (6.6%, base 6.5%): the two new doc lines on fps? and compositionFps push it up. Cut both; the names already carry them.
— Genesis V2 (Lane B)
19f7d8f to
6c32cf6
Compare
…view Adopts heygen-com#7250 (SVG stroke drawing is not a dead stretch), heygen-com#7248 (contract check reads a timeline registered in a local script), heygen-com#7252 (comma-list selectors hidden by authored CSS), heygen-com#7238 (snapshot and preview show a clip on the render frame), heygen-com#7237 (stalled cloud upload retries) and heygen-com#7251 (per-scene SVG id namespacing), with prerequisite heygen-com#6829. Defers heygen-com#7246 (needs the delivery limiter from heygen-com#6940) and heygen-com#7232 (needs the WebKit decode-first pipeline), with conditions in the ledger. Watermark advances to b108f85. Co-authored-by: Miguel Ángel <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
What
hyperframes snapshotand Studio preview now decide whether a clip is on screen by the same rule render uses. A title held across two back-to-back clips is now on screen at the exact second the second clip takes over, as it is in the render.Why
A report:
snapshot --at 91,93,95,98,100,102showed a held chapter title and rule in every still except the one at 100 s, while the render's frames at 99.875, 100 and 100.125 s all had them.There were two rules for "is this clip on screen". Render snaps each clip's edges onto the frame grid when they sit within a thousandth of a frame of it (
exportClipWindow). Snapshot and preview used the authored seconds, with only float-noise tolerance. A clip whose start lands a hair past a frame, such as one chained after a media clip with a measured length, starts on that frame in the render but a hair later in snapshot. If the clip before it ends exactly on the frame, snapshot shows neither of them there.Related work
None.
How
isClipOnScreenAtinruntime/clipWindow.tsis now the one rule: it snaps the clip's window as render does, then applies the half-open window with the terminal hold.isRuntimeElementVisibleAt(element visibility for preview, snapshot and render) and the frame-source adapter (video frames) both call it. TheexportRenderSeekswitch that chose between the two rules is gone, along with its plumbing ininit.tsandcompositionReadiness.ts.Snapshot's own pick of which
<video>gets an FFmpeg frame (resolveSnapshotVideoFrameTime, used byactiveVideoAt) calls the same rule, with the composition's frame grid (its rootdata-fps, else 30, as the runtime reads it). Without that, a video starting a hair past a frame was shown but given no frame there.Preview changes by at most a thousandth of a frame (1/30000 s at 30 fps), and only at a clip edge that sits that close to a frame. There it now agrees with the render.
The frame-source test that pinned the old split ("retains unsnapped authored timing during interactive preview") is replaced by one test that preview follows render's window at near-frame starts and ends.
Test plan
snapshot.heldBoundary.browser.test.ts(new): a red title in clips 90 to 100 s and 100.00002 to 110 s, snapshotted at 98, 100 and 102 s. It reads the centre pixel of each still.fails without the fix: on main, the still at 100 s is black (
[100, false]); with the fix all three are red.snapshot.test.ts"pulls a frame for a video starting a hair past 100 s at exactly 100 s, as render does": with the previous snapshot code it returns no frame; now frame 0.timeline.test.ts"puts a clip edge a hair off a frame on that frame for preview and snapshot, as render does": the first clip at 99.875 s, the second at 100 and 100.125 s. On main the 100 s case shows neither.bun vitest run src/runtime src/compositionReadiness.test.tsin core: 119 files, 2838 tests pass.snapshot.test.tsand the browser test: 100 pass. oxlint,tsc --noEmitfor core, studio and scripts, fallow audit andcheck:cli-process-ownershipare clean.Unit tests added/updated
Manual testing performed
Documentation updated (if applicable)
Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug
Not exercised: the reporter's own project; the fixture reproduces the shape the report describes (98 and 102 s keep the title, 100 s drops it, render keeps it).