Repository navigation
feat(studio): prompt to install FFmpeg before Export, not after #3314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
fbfd320
571832d
4c1a783
8e8db08
f11e10b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,19 +43,44 @@ export function findFFprobe(): string | undefined { | |
| return findFfBinary("ffprobe", { configuredMustExist: true }); | ||
| } | ||
|
|
||
| export function getFFmpegInstallHint(): string { | ||
| const FFMPEG_DOWNLOAD_URL = "https://ffmpeg.org/download.html"; | ||
|
|
||
| /** | ||
| * The one command that installs FFmpeg on this machine, or `undefined` when | ||
| * the platform has no single command worth pasting. | ||
| * | ||
| * Separate from `getFFmpegInstallHint` because Studio renders this behind a | ||
| * copy button, and a copy button over prose ("download the build from ... and | ||
| * add its bin/ directory to PATH") copies something that is not a command. | ||
| * This is the only place that maps a platform to an install command; the hint | ||
| * below is derived from it. | ||
| */ | ||
| export function getFFmpegInstallCommand(): string | undefined { | ||
| switch (process.platform) { | ||
| case "darwin": | ||
| return "brew install ffmpeg"; | ||
| case "linux": { | ||
| // 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; | ||
| return ffmpegInstallCommand(distro.family); | ||
| } | ||
| // winget ships with Windows 10 1809+ and Windows 11. Machines without it | ||
| // still get the manual download route from the hint below. | ||
| case "win32": | ||
| return "Download the 64-bit Windows build from https://ffmpeg.org/download.html#build-windows and add its bin/ directory to PATH."; | ||
| return "winget install --id Gyan.FFmpeg -e"; | ||
| default: | ||
| return "https://ffmpeg.org/download.html"; | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| export function getFFmpegInstallHint(): string { | ||
| const command = getFFmpegInstallCommand(); | ||
| if (process.platform === "win32") { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The doc comment makes
|
||
| return `${command}, or download the 64-bit build from ${FFMPEG_DOWNLOAD_URL}#build-windows and add its bin/ directory to PATH.`; | ||
| } | ||
| if (command) return command; | ||
| if (process.platform === "linux") return ffmpegInstallCommand("unknown"); | ||
| return FFMPEG_DOWNLOAD_URL; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This guard is the thing standing between the Copy button and a sentence of prose, and nothing tests it —
ffmpeg.test.tssetsdarwin,win32andsunos, neverlinux.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), whichFfmpegRequiredNotice.tsxrenders inside<code>behind Copy — precisely the failure the doc comment above says the command/hint split exists to prevent. Whole suite stays green.One
itwithsetPlatform("linux")and a stubbeddetectLinuxDistrocloses it.