Repository navigation
feat(core): lighter preview copies of heavy videos below full quality - #6365
Conversation
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 important · 1 nit
The runtime, server and core changes look sound. I found no correctness defect in the swap and restore logic, and the PR's core and studio-server suites pass locally (65 + 243). The head is still red on two required checks, though:
- [blocker] Required checks fail at
6fb77f5e.- Preflight
oxfmt --checkflagsstudio-servermediaCodecMap.test.ts,mediaMetadata.test.ts,mediaProxyPreview.test.ts,mediaProxyPreview.tsandroutes/preview.ts. - The comment ratchet flags
core/src/previewProxyBox.ts(11.1% → 14.9%, 7 lines) andstudio-server/src/helpers/proxyTranscoder.ts(+1 line, theKEYFRAME_ARGScomment). - regression, preview-regression, player-perf and Windows all fail only because preflight did not pass.
- Preflight
- [nit] The PR body lists the levels as 1, 0.75, 0.5, 0.25, but
PREVIEW_QUALITY_LEVELSis[1, 0.5, 0.25, 0.125].
Scope
Read the whole diff. Traced the light-swap lifecycle: the level change in flight (live → applyPreviewQuality), keepSized resizing and restoring the original, a hostile copy growing back at level 1, and the render-mode guards. Checked that the client and server agree on isHeavyForPreview for the same quantized box, and that a project with no hostile files still omits __HF_MEDIA_CODEC_MAP__. No sender of set-preview-quality lands in this PR (#6366 adds it).
Edit accuracy: accurate 271 (base branch 271), smooth 127 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (2)
|
…keyframe every second
… for the complexity gate
2655700 to
2d5faf8
Compare
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 resolved, 1 open, 1 new · 6fb77f5e → 2d5faf88
The format and comment checks are green now. CI on this head still fails, and it passed on main at the merge base f32e2a6f.
- [blocker] Typecheck fails. The output is
mediaProxy.ts(380,64): TS2345 Argument of type 'number' is not assignable to parameter of type '1 | 0.5 | 0.25 | 0.125'. The split-outCopyJob.levelis typednumber, butscalePreviewProxyBoxneeds aPreviewQuality. Typing the fieldlevel: PreviewQualityfixes it. The same error also failsTest: runtime contract. - [blocker]
verify:packed-manifestsfails.@hyperframes/producerpacks to 789459 bytes against a budget of 781312 (+1.04%). Trim the package, or raise the budget in the script with a reason.
There was a problem hiding this comment.
Verdict: APPROVE · 2 resolved, 0 open, 0 new · 2d5faf88 → b3dc2150
The changes since my last review are CopyJob.level: PreviewQuality and the producer budget raised to 828932, measured size plus 5%, with the reason in the body. Every workflow has a passing run on this head: CI, regression, Windows, both perf runs, preview-regression, Comments and CodeQL. The cancelled runs are duplicate triggers that were superseded.
What
Adds a preview quality level (1, 0.5, 0.25, 0.125). Below full quality, videos that are heavy for a browser to play (4K and other very large frames, 10-bit or higher, or 40 Mbit/s and up) are swapped for a lighter copy at that fraction of the frame size. A file Chrome plays easily is never copied at full quality.
Why
Heavy source videos make the preview stutter on modest machines. A lighter copy keeps scrubbing and playback smooth while render output is unchanged.
How
core: one rule decides which files are heavy (copy box at most half the source pixels, or 10-bit or higher, or at least 40 Mbit/s). The constants live in one place.studio-server: probes width, height, bit depth and bitrate, and serves a light copy when the request asks for one and a box is given. Copies get a keyframe every second so seeking stays quick.PROXY_PARAMS_VERSIONis bumped so old copies are rebuilt.set-preview-qualitybridge action swaps heavy elements to their light copy and back to the original, keeping the element's layout size.Tests
Runtime, server and core tests cover the rule, the probe, the route and the swap. Four runtime tests fail when the level control is disabled.
Package size
The runtime bundle that the producer package ships grows with this change (the producer tarball is 789459 bytes against a budget of 781312). The producer budget is raised to the measured size plus 5%; the added code is the preview-only quality handling in the runtime, and the tarball holds no maps, tests or media.