Repository navigation
fix(cli): check copies only the stretch of a video it plays and stops after 2 minutes of copies - #7182
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
Edit accuracy: accurate 267 (base branch 267), smooth 117 of thoseThe gate passes. Quarantined, measured but not gated (8)
Unstable (1)
|
There was a problem hiding this comment.
Requesting changes at cd44b0ae. The approach is sound, but the finding-capture page still starts a whole-file copy. That's the copy this PR says check no longer makes.
Blocker 1: crop and frame capture still copy the whole video
runBrowserCheck passes limits to serveStaticProjectHtml, but withFindingCapturePage (checkBrowser.ts:738, used by captureFindingCrops and captureFindingFrames) opens its own server with no proxyLimits. When that page asks for the HEVC/ProRes clip, serveProxyRequest calls resolveProxy with limitSeconds: undefined. That looks up the whole-file cache key, and only the windowed copy exists (h264@30s). So it starts a full transcode.
Before this PR, the pre-resolve step had already made the whole copy, so this page got a cache hit. Now any check that reports findings needing crops or frames, on a project with a long hostile video, starts the full 4K copy in a phase the 2-minute copy budget doesn't cover. The crops and frames also wait on that copy or come out without the video's pixels. This contradicts the body's claim that "the page never starts a second full copy".
Fix: carry the limits from the pre-resolve step, or recompute them, into withFindingCapturePage's serveStaticProjectHtml call. Then add a test that the capture server resolves with the same limitSeconds.
Blocker 2: the Comments workflow is red on this head
RAISED comment share: packages/cli/src/utils/staticProjectServer.ts is 11.1% (44/395), at the base 11.1% (43/388)
RAISED comment share: packages/studio-server/src/helpers/proxyTranscoder.ts is 6.7% (82/1219), at the base 6.7% (79/1177)
Checked, no issue
- The
Fails without the fix:line is present and names specific tests. mediaUsedSecondscovers the runtime's offset and rate attributes (data-media-start,data-playback-start,data-playback-rate). It rounds rate < 1 up to 1 and leavesloop, untimed, non-<video>and<source>uses out of windowing. All of these err toward a longer copy, which is the safe direction.--prepare-mediacomputes the same windows with the same cache key, so its output satisfies the next check.- Studio and other callers pass no window and get the whole copy as before.
Nit (non-blocking)
If the budget timer fires just as the last copy settles, overBudget is set but no result is rejected. Check then throws media_copies_unfinished with an empty file list. Only throw when unfinished.length > 0.
There was a problem hiding this comment.
Approving at b62be740. Both blockers from my review at cd44b0ae are fixed.
- Capture pages get the copy windows.
withFindingCapturePagenow passescopyWindows(project.dir, html, …)toserveStaticProjectHtml. That is the samemediaUsedSeconds+proxyWindowSecondscalculation the pre-resolve step uses, keyed byresolve(projectDir, file)on both sides. So a crop or frame capture hits the windowed cache entry and starts no full copy. The new test "serves the page the same copy windows the check made" pins this down. The extra ffprobe on non-hostile timed videos can't throw, becauseprobeMediaMetadataturns errors intoprobeError. A missing or remote src just gets no window. - Comments gate. It's green on this head.
- Nit from my last review. Also fixed: the error now needs
unfinished.length > 0. - Later heads.
5912a32cremoves only a comment.b62be740extractscopiedSecondsandlengthArgs. These keep the old behaviour:range?.lengthSeconds ?? limitSecondspicks the same-tas the old ternary.
CI is green on b62be740, and the cli and studio-server test files touched here all ran. Windows render verification was still running when I approved.
What
hyperframes checkmakes a browser-playable copy of any video Chrome cannot decode (HEVC, ProRes) before it opens the page. It copied the whole file, outside the check's deadline, ended only by a 3 minute no-progress stall. A long source that kept moving held the check for 8+ minutes with no result, which agents read as a hang.Two changes:
<video>with a knowndata-duration(on the video or the element that times it, noloop), check copies only that stretch from the start (plus offset, rate rounded up, rounded to whole 15 s pieces). Anything else still copies whole and shares the preview's copy. The window is part of the copy's cache key, and the check's local server serves the same windowed copy when the page asks for the video, so the page never starts a second full copy. The finding crop and frame capture pages get the same windows, computed from the same page HTML. A window is used only when it saves at least one piece.media_copies_unfinishedfinding naming each unfinished file with its progress andhyperframes check --prepare-media(no time limit); finished pieces are kept, so the next run continues.Studio preview and the other callers are unchanged: they pass no window and get the whole-file copy as before.
Measured
A 90 s 4K portrait HEVC clip, composition plays 5 s of it (devbox, 4 cores): copy 6.5 s windowed (one copy in the cache, page requests served from it) against 32.7 s whole when the composition gives no duration.
Fails without the fix:
packages/cli/src/utils/checkBrowser.test.ts> "copies only the stretch of a long video the composition plays" and "stops a check whose copies outrun the copy budget, naming each file and the way to finish";packages/studio-server/src/helpers/proxyTranscoder.test.ts> "copies only the first stretch when asked, as a copy of its own";checkBrowser.test.ts> "serves the page the same copy windows the check made, so a capture starts no full copy";packages/cli/src/utils/mediaUsedSeconds.test.ts(new file, 5 cases).Not covered
The craft-sampling stop and the queue-wait budget from the same reports are separate follow-ups. A video with no known duration (no
data-durationanywhere above it) still copies whole.