Repository navigation
fix(cli): check and snapshot free video clips the playhead left, halving memory on cut films - #6929
Conversation
…ing memory on cut films
There was a problem hiding this comment.
Requesting changes at ecce7be3 because the body has no fails without the fix: line.
This is a fix PR, and the lane's gate needs a test named on a fails without the fix: line. The Test section does say "the flag case fails with the old call", but it's in a parenthetical, not on that line.
I checked that the claim holds. With init.ts and preloadMedia.ts set back to the merge base, two tests fail and the other 69 in those files pass. At the head, all 71 pass. So the fix should be just this to the body:
fails without the fix:
- entry.test.ts › "with a capture page flag true, a fully buffered clip the playhead left is reloaded 2 times"
- preloadMedia.test.ts › "frees a clip whose whole file is buffered when asked to"
I don't have any concerns about the code. The flag is opt-in and only set by openSettledCompositionPage, and Studio and render never set it. releaseMedia still skips clips with a <source> child and puts src back after freeing. The body is upfront that a clip scrubbed back to gets fetched again, and the byte-identical snapshot run in the body includes backward seeks. Once the line is in, I'll approve.
Edit accuracy: accurate 271 (base branch 271), smooth 98 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (3)
|
There was a problem hiding this comment.
Approving at 3af6fe58. The blocker from my last review is fixed.
- Body: the
fails without the fix:line now names both tests. I confirmed atecce7be3that both fail with the runtime change undone and pass with it. - What changed since
ecce7be3: only the one-line doc comment onreleaseMediawas removed. That clears the earlier redCommentsrun, which is green on this head. The behaviour is the same code I reviewed last time. - Earlier
regressionred (shard-10,sub-composition-video, distributed-simulated): this can't be caused by the PR. It's an FFmpeg chunk-encode failure on a captured JPEG frame with an unspecified size, in a render. Render never sets__HF_RELEASE_PASSED_MEDIA; onlyopenSettledCompositionPagein the CLI does. So the render callsreleaseMedia(el, false), which does exactly what main does. CI on this head was still running when I approved.
The 1a9bb89 port of heygen-com#7402 and heygen-com#7378 brought in releaseMedia(el, flag) with window.__HF_RELEASE_PASSED_MEDIA (heygen-com#6929, needs the heygen-com#6785 buffered guard) and registerSeekCompletion(landed, "image-decode") (the SeekHold union from heygen-com#6596). Both upstream commits stay deferred, so typecheck:runtime failed in the Windows render verification workflow. The fork's releaseMedia already frees every clip, so the call keeps its behaviour. Add tsconfig.runtime.json to the pre-commit typecheck, which only covered tsconfig.json (it excludes src/runtime), and pin it in the hook test. Co-Authored-By: Claude Sonnet <noreply@anthropic.com>
What
check,snapshotandcomparefree each video clip the playhead has left, so a film cut from many 4K clips no longer keeps a decoder for every clip it visited. Studio's preview is unchanged.Why
A project of 22 cut pieces of one 4K source made
checkandsnapshotexit mid-run on a 16 GB Mac. The runtime frees a clip that leaves its window, but skips a clip whose whole file is already buffered ("a loaded clip stays for a scrub back"). These tools only seek forward, so the clips stay loaded and memory grows with every clip visited.Measured on a Linux box, 22 three-second cuts of one 3840x2160 H.264 file inside a sub-composition, Chrome process-tree RSS summed, two runs each:
The same project with 2 clips peaks near 2000 MB, so what is left is the fixed cost of the page, not the clips.
Change
window.__HF_RELEASE_PASSED_MEDIA(one line beside the existing capture flag inopenSettledCompositionPage).releaseMediaalso frees a fully buffered clip that leaves the window. What stays loaded is the clip on screen plus the 4 s look-ahead and the prefetch slots, as in Studio. Without the flag nothing changes.Test
fails without the fix:
Also:
entry.test.ts: with the flag off the same clip is kept, as in Studio.preloadMedia.test.ts:releaseMedia(video, true)frees a fully buffered clip.captureCompositionFrame.browserFallback.test.ts: the capture page sets the flag.check --jsonis identical with the flag on and off apart from the per-run saved id and path, andsnapshot --at 1,40,7.5,64,2,31writes byte-identical PNGs.Not covered: the Mac failure itself was not reproduced; the box never failed, only used more memory. A clip scrubbed back to in a capture page is fetched again.
Size
A lone fix of about 60 changed lines across the runtime and the capture page; nothing open touches this path, so there is no PR to fold it into.