feat(studio): prompt to install FFmpeg before Export, not after - #3314
Conversation
Studio asked the render server to encode and, when FFmpeg was missing,
showed the user "Server error (503). Check the terminal for details." The
server had already diagnosed the problem and sent back both the cause and
a per-platform install command; the client threw that body away and
printed the status code. It was the most reported Studio failure by a
wide margin, and it surfaced only after the composition was finished.
- New GET /api/environment/ffmpeg reports encoder availability. It calls
runEnvironmentChecks with every optional check off, which is exactly the
FFmpeg and ffprobe pair `doctor` runs, so Studio and the CLI cannot
disagree about whether a render can start. Only a passing result is
cached, so Recheck sees a freshly installed FFmpeg.
- The Renders panel shows the cause plus a one-click-copy install command
when the encoder is missing, and offers Recheck so installing it does
not mean restarting Studio.
- The refusal lives in `startRender`, not in a button. Studio has two
Export controls and the header one calls `startRender` directly, so a
check inside the panel's button left the other free to queue a render
this machine could not finish. The header button now routes to the
prompt rather than being disabled, which would be a dead end.
- Failed render responses render the server's `{ error, hint }` when it
sends one, so the status code is the fallback rather than the message.
- `getFFmpegInstallCommand` becomes the single owner of "what installs
FFmpeg here", with the prose hint derived from it. Windows gains a
winget command and keeps the manual download route in the hint.
Also folds in three cleanups this change ran into: the Renders tab moves
out of StudioRightPanel (every field it needs was already in context, and
the file was at the size cap), StudioContextInput stops keeping a second
copy of the renderQueue shape, and the three server-test describes share
one temp-project helper instead of three drifting copies.
Verified against a real Studio on both paths: with FFmpeg unresolvable
the panel prompts, both Export controls refuse, Copy and Recheck work;
with FFmpeg present there is no prompt and a render completes to MP4.
Polish pass on the card that blocks Export, driven by measuring it rather than looking at it. Contrast was the real defect. The card sits on an amber wash, which lifts the background well above the panel's normal surface, so the panel's usual greys for secondary copy stop working on it. The line explaining why Export is disabled measured 2.2:1 against a 4.5:1 minimum, and so did the recheck result. Those move to a lighter step and measure 6.6:1. The amber title and buttons were already fine at 11.8:1 and 13.7:1. Recheck had no answer for its most likely outcome. Clicking it while FFmpeg is still missing repainted nothing, so the button read as broken at the exact moment the user is least sure of themselves. It now reports "Still not found" for a beat. The cue sits last in its row so appearing and vanishing moves nothing before it, and the Copy button gets a fixed width so swapping its label cannot shift the command out from under the pointer. Also: - Keyboard focus was invisible on all three controls. They now carry the same focus ring every other button in this panel uses. - Nested radii were not concentric (6px outer, 4px inner, 10px padding). Now 12 outer minus 10 padding equals the 2 inner. - Dropped "The render cannot proceed without it" from the missing-FFmpeg detail. The error it accompanies already says so, and in Studio the disabled Export button says it louder. Shortens `doctor` output too. - Body copy gets text-wrap: pretty so the narrow column stops orphaning. Verified in a real Studio: the card reads clearly, Recheck reports its failure and retires the cue, and with FFmpeg present there is still no card and both Export controls stay enabled.
The smoke run fails on any unmocked API request, which is the point: it
catches calls the shell makes by accident. This one is deliberate, so it
needs a fixture rather than an exemption.
Answers `{ ok: true }`. A usable encoder is the case this run wants, since
what it asserts is that the shell mounts clean, not that a blocking notice
renders; the notice has its own tests.
terencecho
left a comment
There was a problem hiding this comment.
LGTM. Prompt-before-Export gate is centralized in useRenderQueue::startRender, so both the panel's Export button and the header's route through the same refusal — the class of bug where one control forgets to check the signal is closed by construction (comment in useRenderQueue.ts even calls this out).
Verified:
- Server
/api/environment/ffmpegreusesrunEnvironmentChecks()(same code asdoctor), so no new probe surface. Only success is cached — Recheck after installing FFmpeg gets a fresh probe. - Client cache in
useFfmpegStatusis dropped byrecheck()before re-running, and null status ("no answer") deliberately fails open, so an older or unreachable dev server does not lock a working setup. FfmpegRequiredNoticeshows title + detail + copyable command, falls back to prose hint when no per-platform command exists, and the "Other install options" link is unconditional.- Header Export click on
ffmpegMissingopens the Renders panel first, then returns — the user lands on the prompt with the install command instead of on a mystery no-op. readServerErrorretires the "Server error (503)" black hole: the render route's{error, hint}body is now shown with cause + remediation.- Test coverage: platform detection (darwin/win32/sunos), server endpoint (missing + present + skip-if-real-binary), notice recheck cue including the "still missing" retraction,
startRenderrefusal from any caller, null-probe fail-open. Cross-platform CI (macOS/Windows/Linux shards) all green at8e8db08.
Nits (non-blocking):
- On Windows without winget,
FfmpegRequiredNoticehideshintbecausecommandis truthy — the "or download the 64-bit build from ffmpeg.org" tail thatgetFFmpegInstallHint()deliberately appends never reaches the user. They still have the "Other install options" link, but the combined-hint intent fromffmpeg.tsis inert on-screen. Consider surfacing the tail alongsidecommandon win32, or making the download link more prominent when the platform is Windows. FFMPEG_DOWNLOAD_URL/DOWNLOAD_URLis defined identically inpackages/cli/src/browser/ffmpeg.tsandpackages/studio/src/components/renders/FfmpegRequiredNotice.tsx. Trivial, but a shared const would keep them from drifting.- Server-side
ffmpegReadypersists for the process lifetime — a mid-session uninstall would leave the cache stale. Documented in the comment; noting for completeness.
— Review by tai (pr-review)
jrusso1020
left a comment
There was a problem hiding this comment.
Read the full files on the preflight/install path at 8e8db087, not just the diff. All 8 required contexts are green at that head, so BLOCKED is the reviewer gate only.
What holds up
- The refusal lives in
startRender, not in a button (useRenderQueue.ts:167). This is the decision the whole change rests on, and it holds up better than the description claims — see the first finding. - The endpoint's central claim checks out.
runEnvironmentChecks()with no options pushes ffmpeg and ffprobe unconditionally and gates browser/disk/UNC behind explicit flags (preflight.ts:267-297), so "exactly the FFmpeg and ffprobe pairdoctorruns" is accurate, and Studio and the CLI genuinely cannot disagree. - The gate test pins the fail-open case (
useRenderQueueFfmpegGate.test.tsx): missing refuses, present starts,nullstarts. The third is the one that matters — asserting it makes "unknown fails open" a guarantee rather than a sentence in the description.
important — the third Export control reaches the gate but not the explanation
There are three render entry points, not two. Besides the header and the panel button, every composition card in the left sidebar carries a Render control: CompositionsTab.tsx:376 -> StudioLeftSidebar.tsx:65-71 -> renderQueue.startRender(...).
Because the refusal lives in startRender, this path is safe — no unfinishable render is queued, and the no-bypass property holds for all three. The gap is feedback. The header deliberately opens the Renders panel before returning (StudioHeader.tsx:398-405) so the click lands on the prompt. The sidebar path only appends an "Export blocked" row to a panel that may be collapsed or sitting on another tab, so the user clicks Render and observes nothing happen at all.
The description rejects disabling the header button because it "would be a dead end with no path to the explanation". This is that dead end without the disabled styling — and it is the one path where the user has named a specific composition they want rendered.
StudioLeftSidebar already calls usePanelLayoutContext(), so it is about three lines in handleRenderComposition:
if (renderQueue.ffmpegMissing) {
setRightPanelTab("renders");
setRightCollapsed(false);
return;
}Worth correcting the count in the description and in the header comment of useRenderQueueFfmpegGate.test.tsx — both say "two Export controls".
nit — the one branch added to protect the Copy button has no test
ffmpeg.ts:66, if (distro.family === "unknown") return undefined;. ffmpeg.test.ts sets darwin, win32 and sunos; it never sets linux, so neither linux branch is exercised.
Delete that line and getFFmpegInstallCommand() returns ffmpegInstallCommand("unknown") — "Install ffmpeg (which includes ffprobe) via your distro package manager, then re-run." (linuxDeps.ts:199-201) — which FfmpegRequiredNotice.tsx renders inside <code> behind a Copy button. That is exactly the "a copy button over prose copies something that is not a command" failure the function's own doc comment says the split exists to prevent, and the entire suite stays green through it. One it with setPlatform("linux") and a stubbed detectLinuxDistro pins it.
nit — getFFmpegInstallHint() interpolates a possibly-undefined command on Windows
ffmpeg.ts:79-82. command is string | undefined and the platform check cannot narrow it. Correct today, because the win32 case always returns a string. But the doc comment makes getFFmpegInstallCommand the single owner of platform-to-command, so the day someone decides winget is not universal enough and returns undefined there, the hint silently becomes "undefined, or download the 64-bit build...". if (command && process.platform === "win32") costs nothing and makes the coupling explicit.
On the existing review
tai got here first and its three nits are real; I have not restated them. One correction: its summary says "test coverage on all platform branches" — verified at source, there is no setPlatform("linux") anywhere in ffmpeg.test.ts, which is what the second nit above is about.
Verdict: COMMENT
Reasoning: No blockers. The gate is correct and every path routes through it, so the sidebar item is an incomplete application of this PR's own principle rather than a defect, and tai's approval already satisfies the gate. Happy to approve on request.
— Rames Jusso
| // The server would answer this with a 503 anyway. Refusing here keeps | ||
| // the reason and the fix in the message, and keeps a control that | ||
| // forgot to disable itself from producing a mystery failure. | ||
| if (ffmpegMissing) { |
There was a problem hiding this comment.
This is the right home for the refusal, and it covers more than the description credits it for.
There is a third caller: every composition card in the left sidebar has a Render control (CompositionsTab.tsx:376 -> StudioLeftSidebar.tsx:65-71). Because the gate is here rather than in a button, that path is safe too — nothing unfinishable gets queued.
What it does not get is the explanation. The header opens the Renders panel before returning; the sidebar path just appends an "Export blocked" row to a panel that may be collapsed or on another tab, so the click looks like it did nothing. Details and a three-line fix in the summary.
| // Distro-aware so WSL/Fedora/Arch/Alpine users get a command that | ||
| // actually works instead of a Debian-only `apt` line. | ||
| const distro = detectLinuxDistro(); | ||
| if (distro.family === "unknown") return undefined; |
There was a problem hiding this comment.
This guard is the thing standing between the Copy button and a sentence of prose, and nothing tests it — ffmpeg.test.ts sets darwin, win32 and sunos, never linux.
Remove this line and the function returns ffmpegInstallCommand("unknown") = "Install ffmpeg (which includes ffprobe) via your distro package manager, then re-run." (linuxDeps.ts:199-201), which FfmpegRequiredNotice.tsx renders inside <code> behind Copy — precisely the failure the doc comment above says the command/hint split exists to prevent. Whole suite stays green.
One it with setPlatform("linux") and a stubbed detectLinuxDistro closes it.
|
|
||
| export function getFFmpegInstallHint(): string { | ||
| const command = getFFmpegInstallCommand(); | ||
| if (process.platform === "win32") { |
There was a problem hiding this comment.
command is string | undefined here and the platform check cannot narrow it, so this is correct only because the win32 case above always returns a string.
The doc comment makes getFFmpegInstallCommand the single owner of platform-to-command. The day that decision changes — someone concludes winget is not universal enough and returns undefined — this silently renders "undefined, or download the 64-bit build from ...".
if (command && process.platform === "win32") costs nothing and makes the dependency explicit.
Review found a third render entry point I had missed. Besides the header and the Renders panel button, every composition card in the left sidebar carries its own Render control that calls startRender directly. Because the refusal lives in startRender, that path was already safe: no unfinishable render was ever queued. The gap was feedback. The header deliberately opens the Renders panel before returning so the click lands on the prompt; the sidebar just appended a blocked row to a panel that may be collapsed or sitting on another tab, so the click looked like nothing happened at all. That is the same dead end this change rejected for the header, minus the disabled styling, and it is the one path where the user named a specific composition they wanted rendered. Also from review: - Pinned the guard that keeps prose out of the Copy button. On an unrecognised distro `ffmpegInstallCommand` returns a sentence, and the card renders whatever it gets inside <code> behind Copy. Deleting the guard left the whole suite green, which is exactly the coverage hole: `ffmpeg.test.ts` never set platform to linux. Two cases now do, and removing the guard fails them. - `getFFmpegInstallHint()` guards on the command rather than the platform. Correct either way today, but the win32 branch interpolated a value typed `string | undefined`, so the day that returns undefined the hint would have read "undefined, or download the ...". - Corrected "two Export controls" to three in the comments that said it.
|
Thanks both. Addressed in The third Export control — fixed. Confirmed at source: Fixed as suggested, in Verified in a real Studio with FFmpeg off PATH: with the right panel on Design, clicking a composition card's Render now switches to Renders and shows the prompt, and queues nothing. The untested linux branch — fixed, and it was worse than "untested". I reproduced the claim before fixing it: deleting The Control count corrected to three in both comments and in the PR description. Deliberately not changed:
One thing to flag rather than have you find it: the branch is 1164 changed lines against the repo's 1000-line cap. The overage is the gate-forced cleanups (the Renders tab extraction, the duplicate context type, the shared server-test helper) plus the polish and review passes. Each is independently revertable, but I did not split them into a second PR. |
First sync under the TAB-782 merge model: a pinned release tag on `tabario`, not a rebase and not moving `upstream/main` (3 unreleased commits past v0.8.1). 17 commits, 2 conflicts, both predicted by TAB-782's dry run. ## v0.8.0 is a minor bump that does not apply to us Its migration notes are entirely about distributed cloud rendering — redeploying Lambda/Cloud Run infra and Plan v2 becoming the default transport. Tabario installs only the `packages/cli` tarball and renders locally with in-container Chromium; nothing in `src/hyperframes/` or the Dockerfile references planProtocol, aws-lambda, cloud-run or distributed. ## The conflict that mattered: an egress regression from an upstream refactor `StudioHeader.tsx` was pure adjacency — upstream's new `ffmpegMissing` line landed where TAB-703's return-to-Tabario handler sits. Both kept. `composition-probe.ts` was not. TAB-746 had replaced upstream's jsdelivr runtime fallback with a same-origin URL, next to the single injection site. v0.8.1 (heygen-com#3316) extracted that URL into a new `runtime-url.ts` *and added a second injection site* — `prepareSrcdocForElement` in `shader-options.ts` injects the runtime into a srcdoc at parse time, reading the same constant. Resolving the conflict as written would have left that new path pointing at `cdn.jsdelivr.net`: third-party egress from a customer's browser, reintroduced by a refactor in a file our patch never touched, in the exact surface TAB-697 was opened to clear. So the patch moved to `runtime-url.ts`, which is where upstream now keeps the shared constant precisely so the two sites "cannot drift onto different URLs". One change covers both, and our footprint in `composition-probe.ts` shrinks from ~45 lines to a thin accessor — less to conflict with next time. `forkEgressGuard.test.ts` is what caught this, which is what it exists for. ## Also carried - `ffmpeg.org` added to the egress guard's justified list. Unlike every other entry it is not a dependency that could be re-pointed at our origin: it is an `<a href target="_blank">` in upstream's new FfmpegRequiredNotice (heygen-com#3314), so no request happens without a user click. It is dead code here regardless — the notice renders only when the server reports no FFmpeg, and the compositor image ships it. - Two upstream player tests asserted the CDN bundle filename in the srcdoc. Rewritten to assert the intent (a runtime IS injected) plus the fork's own guarantee (same-origin), which the upstream form cannot express. ## Verified typecheck clean · studio-server 489/489 (incl. the TAB-780/781 agent tests) · studio 4288 · cli 2686 · player 338/338 · egress guard 5/5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What
Studio now checks whether this machine can encode video before it offers
Export, and prompts with a copy-paste install command when it cannot.
Previously, exporting without FFmpeg installed produced:
The server already knew the exact cause and shipped a per-platform install
hint in the response body; Studio discarded that body and printed the status
code instead. The user found out only after the composition was finished.
Why
This is the most common way a Studio export fails, and the message named
neither the cause nor the fix. Two things made it worse than a bad string:
exists, so the failure never produced a
render_errorevent. It wasinvisible in the render telemetry that would normally rank it.
response Studio was throwing away.
How
GET /api/environment/ffmpegreports encoder availability. It callsrunEnvironmentChecks()with every optional check off, which is exactly theFFmpeg + ffprobe pair
doctorruns, so Studio and the CLI cannot disagreeabout whether a render can start. ffprobe is included deliberately: it ships
with FFmpeg but is a separate binary, and any project with a media asset
fails at probe time without it. Only a passing result is cached, so
"Recheck" sees a freshly installed FFmpeg without restarting Studio.
The refusal lives in
startRender, not in a button. Studio renders fromthree places: the Renders panel's Export button, the header's, and the Render
control on every composition card in the left sidebar. The last two call
startRenderdirectly, so a check inside one button leaves the rest free toqueue a render this machine cannot finish. Any fourth caller is covered by
construction. The header and sidebar controls additionally reveal the prompt
rather than being disabled, which would be a dead end with no path to the
explanation.
Unknown fails open. A null status means the probe gave no answer (older
server, failed request). That is not evidence of a missing encoder, and
blocking Export on no answer would lock out setups that render fine.
Failed render responses surface the server's
{ error, hint }when itsends one. The status code is now the fallback, not the message.
getFFmpegInstallCommand()becomes the single owner of "what installsFFmpeg here", with the prose hint derived from it. Windows gains a winget
command and keeps the manual download route in the hint for machines
without winget.
Three cleanups this change ran into are folded in, because repo gates
(file-size cap, unused-export audit, duplication audit) block on them:
StudioRightPanelinto its own component.Every field it needed was already on the shell context, and the file was at
the 600-line cap.
StudioContextInputstops keeping a second structural copy of therenderQueueshape, which had to be edited in lockstep with the first.describeblocks share one temp-project helperinstead of three copies that had already drifted apart.
What it looks like
Captured from a real Studio run on a machine with no FFmpeg on PATH, so this
is the message an actual user gets, not a forced error state.
The prompt in context. Export is disabled underneath it, and the header's
Export routes here rather than queueing a render that cannot finish.
The command is the loudest element, because it is the part you act on.
Recheck answers its most likely outcome. Without this it repaints nothing and
reads as a dead button at the moment the user is least sure of themselves. The
cue sits last in its row so appearing and vanishing moves nothing before it.
Test plan
Unit tests cover the parts that are easy to get wrong:
serverError.test.ts— the server's cause+hint wins over the status code;non-JSON, missing, and non-string error bodies fall back to the status.
useRenderQueueFfmpegGate.test.tsx—startRenderrefuses and writes a rowcarrying the install command; it proceeds when FFmpeg is present; it
proceeds when the probe gave no answer.
RenderQueue.test.tsx— the prompt and the command render, Export isdisabled, Recheck is wired, and none of it appears when FFmpeg is present.
studioServer.test.ts— the endpoint reports cause + command when theencoder is unusable, and a plain
{ ok: true }when it resolves.ffmpeg.test.ts— per-platform install command, and that the hint staysderived from it (Windows keeps the manual download URL).
Manually verified against a real Studio, on the final build, both paths:
brew install ffmpeg; the Renders panel shows the prompt; the panel'sExport button is disabled; the header's Export opens the panel instead of
queueing a doomed render; Copy copies; Recheck re-hits the server.
{ ok: true }; no prompt; bothExport controls enabled; a render runs end to end and produces an MP4.
Not covered
render_errorevent. This change makes thatpath much rarer rather than fixing the telemetry gap; closing it means
emitting a render outcome for a render that never started, which is a
separate decision about that event's contract.
doctorruns (Chrome launchability) is a larger surface and is untouchedhere, even though it is also a common cause of failed exports.
@hyperframes/studioinstalled locally runs Studio through Vite instead,where
/api/*is served by the shared studio API and this route 404s. Theprobe treats that as "no answer" and fails open, so that mode keeps its
existing behaviour rather than regressing. Scaffolded projects do not
install the studio package, so this is the contributor dev loop, not the
user path.