Repository navigation
fix(cli): cloud render --json reports re-encode progress and times out a stalled cache check - #7755
miguel-heygen wants to merge 1 commit into
Conversation
…t a stalled cache check
Edit accuracy: accurate 267 (base branch 267), smooth 79 of thoseThe gate passes. Quarantined, measured but not gated (8)
Unstable (1)
|
![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 blocker · 0 important · 1 nit · 0fc9cd61
The code is sound, and both "fails without the fix" claims reproduce locally: the progress test fails with expected 0 to be greater than 0, and the cache test times out at 20 s. Two required checks are red on this head, though:
- blocker: Build ›
verify:packed-manifestsfails because@hyperframes/studio-server packed size 170334 bytes is over its budget of 168500 by more than 1%. Either shrink what the new entry adds to the tarball, or raisescripts/package-size-budgets.jsonand give the reason in the body. - blocker: Lint and Preflight fail with
studio-server/package.json is out of sync. Details are inline. The Windows lanes and preview-regression are red only because Preflight failed.
Scope
Read the whole diff (9 files). Locally, the new and changed tests pass, along with studio-server's 80 proxyTranscoder tests. Five tests in render.test.ts --dry-run and offload.undecodableTrack fail here because of the environment, and they fail the same way on main.
— Claude
| "import": "./src/helpers/outsideFile.ts", | ||
| "types": "./src/helpers/outsideFile.ts" | ||
| }, | ||
| "./ffmpeg-progress": { |
There was a problem hiding this comment.
blocker: bun run sync:package-subpaths builds these exports from packages/studio-server/package-subpaths.json, so it deletes this hand-written entry, and Lint/Preflight fail with "package.json is out of sync". Add "./ffmpeg-progress": { "source": "./src/helpers/ffmpegProgress.ts", "runtime": "./dist/helpers/ffmpegProgress.js", "types": "./dist/helpers/ffmpegProgress.d.ts" } next to ./outside-file in that file, then rerun the sync.
| const probe = preset.contentType.startsWith("video/") ? await probeVideo(sourcePath) : undefined; | ||
| const decoder = probe ? alphaDecoderArgs(probe) : []; | ||
| const color = presetName === "hdrVideo" && probe ? hdrColorArgs(probe) : []; | ||
| const totalSeconds = onProgress ? await outputSeconds(sourcePath, segments) : null; |
There was a problem hiding this comment.
nit: uploadPlanned always passes a callback here, so onProgress is never undefined. Every re-encode of an untrimmed clip therefore runs an extra ffprobe, even without --json. To skip it when nobody is listening, pass the callback only when ctx.opts.onReencodeProgress is set.
What
hyperframes cloud render --jsonnow writes areencode_progressline on stderr while it re-encodes a video before upload, in the same shape as the existingupload_progresslines:{"event":"reencode_progress","file":"take.mkv","percent":42,"done_seconds":12.6,"total_seconds":30}. One line per whole percent, or one per second when the clip's length is unknown (percentandtotal_secondsare thennull).HEADon its asset URL) now gives up after 10 s and uploads the file again, instead of waiting forever.Why
A big or unaccepted video is re-encoded with libx264
-preset slow -crf 18before upload, which takes minutes on a long 4K take. Until it finished the command printed nothing, so a--jsoncaller (an agent, Desktop) could not tell a long encode from a dead one. And the cache check'sHEADhad no timeout, so an asset host that accepted the connection but never answered held the whole render.Related work
None.
How
-progress pipe:1reader that Studio's preview copies already use moves tostudio-server/src/helpers/ffmpegProgress.ts(exported as@hyperframes/studio-server/ffmpeg-progress), and both the preview-copy encoder and the cloud re-encode use it, so there is one parser of those lines.spawnwith-progress pipe:1 -nostatsinstead ofexecFile; on failure it reports ffmpeg's own stderr. Its total length comes from the same place the size limit already reads it (the played segments, else the probed duration).stillServedaborts itsHEADafterCACHE_CHECK_TIMEOUT_MS(10 s). A timed-out check counts as "not served", so the cost is one extra upload, never a wrong cached URL.Test plan
offload.progress.test.ts(real ffmpeg): a 2 s clip under a name the asset API refuses is re-encoded, and progress arrives withdoneSecondsabove 1.5 against atotalSecondsof 2.offload.test.ts: a cached asset whoseHEADnever answers is uploaded again once the timeout passes (fake timers).render.test.ts:reencodeProgressLineswrites one line per whole percent, and one per second when the length is unknown.expected 0 to be greater than 0); without theHEADtimeout, the cache test hangs until its 20 s limit.offload.test.ts,offload.progress.test.ts,offload.undecodableTrack.test.ts(real ffmpeg re-encode end to end),offload.sizeLimit.test.ts,render.test.ts: 75 passed. studio-serverproxyTranscodertests: 80 passed.tsc --noEmitclean in cli and studio-server.