Repository navigation
fix(snapshot): video frames keep the colours render gives the same footage - #6425
Conversation
Edit accuracy: accurate 271 (base branch 271), smooth 131 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 1 nit · head df780410
I checked that render's filter chain is unchanged for SDR, tone-mapped HDR and kept HDR. Snapshot now gets the same SDR retag, HDR stays untouched, and with no ffprobe it warns once.
- nit: the
colours may differassertion in the negative-TS test depends on module-levelwarnedColourUnknown. Today it is the first test in the file to hit the no-ffprobe path, so it passes. Run after theextracts without ffprobetest (-tfilter, reorder, retry), it sees 0 warnings and fails.
CI notes
All required checks are green. Flake gate failed 0/10, and 6/10 on a rerun. The failing test each time was the existing seeks into a stream-copied HEVC cut hitting its 20 s timeout. I ran that test with 8 busy cores: main dc56b3d2 takes ~15 s, 54bc274c ~17 s, df780410 ~15 s. So it was already close to the limit under contention, and this PR doesn't slow it. Comments (comment ratchet, not required) is red on snapshot.ts and videoFrameExtractor.ts.
…stays inside its timeout
Brings in 10 upstream commits (ad4d3fa..15367a9): heygen-com#6425, heygen-com#6427, heygen-com#6429, heygen-com#6431, heygen-com#6436, heygen-com#6432, heygen-com#6453, heygen-com#6457, heygen-com#6450, heygen-com#6451. One conflict, packages/core/src/compiler/compositionScoping.ts (heygen-com#6436). This fork passes authored composition code to Function() as a JSON string literal, so upstream's markers around the raw source would sit outside the text its page-side scan reads and the fix would silently not apply: the wrapper's own __timelines use would make every composition look able to register a timeline. The markers go inside the literal instead, begin marker on the first source line so error line numbers do not shift. The pageBuild goldens change only by those two marker comments. A new engine test runs upstream's scan on this fork's real wrapper output and fails without the markers. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Fixes #6157
What was wrong
Render retags SDR footage as sRGB when it extracts frames, and
snapshotdid not, so a snapshot's video frames kept the source's BT.709 transfer tag. Chrome 152 colour-manages that tag (Chrome 131 ignored it), so snapshot stills came out about 4% darker than the same composition rendered.Fix
The retag now has one owner,
sdrCanvasColourFiltersin the engine's frame extractor. Render uses it where it used to inline the filters, andsnapshotapplies it to the frame it injects over each<video>. The snapshot frame is probed first, and the helper owns the HDR decision: HDR yields no retag unless it was tone-mapped, which render does and snapshot does not.Design calls:
Test
New test: a BT.709 clip's snapshot frame carries the sRGB tag. With the snapshot call removed it fails (
expected false to be true); with it,snapshot.test.tsis 89 passed, 0 failed.videoFrameExtractorengine tests: 272 passed, 1 failed, and that failure isremoteTailasserting extraction concurrency above the host's CPU count (16 on the runner), which this change does not touch.Why under 100 lines
One function moves to a shared export and one call site gains four lines; the rest of the diff is the test helper and the byte-compare to pixel-compare swap in existing tests.