-
-
Notifications
You must be signed in to change notification settings - Fork 2k
build: pin win/linux to x64 and guard bundled ffmpeg architecture #899
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
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 |
|---|---|---|
| @@ -0,0 +1,181 @@ | ||
| import { closeSync, existsSync, openSync, readSync, statSync } from "node:fs"; | ||
| import path from "node:path"; | ||
|
|
||
| const projectRoot = process.cwd(); | ||
| const ffmpegPath = path.join(projectRoot, "node_modules", "ffmpeg-static", "ffmpeg"); | ||
|
|
||
| function relativePath(filePath) { | ||
| return path.relative(projectRoot, filePath).replaceAll("\\", "/"); | ||
| } | ||
|
|
||
| function fail(message) { | ||
| console.error(`[verify-bundled-ffmpeg] ${message}`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| function parseArgs(argv) { | ||
| const parsed = {}; | ||
| for (let index = 0; index < argv.length; index += 1) { | ||
| const arg = argv[index]; | ||
| if (arg === "--platform" || arg === "--arch") { | ||
| const value = argv[index + 1]; | ||
| if (!value || value.startsWith("--")) { | ||
| fail(`Missing value for ${arg}.`); | ||
| } | ||
| parsed[arg.slice(2)] = value; | ||
| index += 1; | ||
| continue; | ||
| } | ||
| fail(`Unknown argument: ${arg}`); | ||
| } | ||
| return parsed; | ||
| } | ||
|
|
||
| const args = parseArgs(process.argv.slice(2)); | ||
| const targetPlatform = args.platform ?? process.platform; | ||
| // `--arch` accepts a comma-separated list because a single electron-builder | ||
| // invocation can emit several architectures (e.g. `--mac` builds x64 and | ||
| // arm64), while ffmpeg-static only ever has one binary staged. | ||
| const targetArches = (args.arch ?? process.arch).split(",").map((value) => value.trim()); | ||
|
|
||
| // ffmpeg-static ships a single binary for whichever platform/arch was active | ||
| // at npm install time, so a cross-platform `npm run build:win` on macOS will | ||
| // silently package a Mach-O binary into a Windows app. Read the file's magic | ||
| // bytes and compare against the platform we are actually packaging for. | ||
| function readHeader(filePath, length) { | ||
| const buffer = Buffer.alloc(length); | ||
| const fd = openSync(filePath, "r"); | ||
| try { | ||
| readSync(fd, buffer, 0, length, 0); | ||
| } finally { | ||
| closeSync(fd); | ||
| } | ||
| return buffer; | ||
| } | ||
|
|
||
| const MACHO_CPU_TYPES = new Map([ | ||
| [0x01000007, "x64"], | ||
| [0x0100000c, "arm64"], | ||
| [0x00000007, "ia32"], | ||
| ]); | ||
|
|
||
| const ELF_MACHINES = new Map([ | ||
| [0x3e, "x64"], | ||
| [0xb7, "arm64"], | ||
| [0x03, "ia32"], | ||
| ]); | ||
|
|
||
| const PE_MACHINES = new Map([ | ||
| [0x8664, "x64"], | ||
| [0xaa64, "arm64"], | ||
| [0x014c, "ia32"], | ||
| ]); | ||
|
|
||
| function identifyBinary(header) { | ||
| // ELF: 0x7F 'E' 'L' 'F' | ||
| if (header[0] === 0x7f && header[1] === 0x45 && header[2] === 0x4c && header[3] === 0x46) { | ||
| const machine = header.readUInt16LE(0x12); | ||
| return { | ||
| platform: "linux", | ||
| arch: ELF_MACHINES.get(machine) ?? `unknown(0x${machine.toString(16)})`, | ||
| }; | ||
| } | ||
|
|
||
| // PE: 'MZ', with the COFF header located at the offset stored in e_lfanew. | ||
| if (header[0] === 0x4d && header[1] === 0x5a) { | ||
| const peOffset = header.readUInt32LE(0x3c); | ||
| if (peOffset + 6 <= header.length && header.readUInt32LE(peOffset) === 0x00004550) { | ||
| const machine = header.readUInt16LE(peOffset + 4); | ||
| return { | ||
| platform: "win32", | ||
| arch: PE_MACHINES.get(machine) ?? `unknown(0x${machine.toString(16)})`, | ||
| }; | ||
| } | ||
| return { platform: "win32", arch: "unknown" }; | ||
| } | ||
|
|
||
| // Mach-O universal ("fat") binary. | ||
| const fatMagic = header.readUInt32BE(0); | ||
| if (fatMagic === 0xcafebabe || fatMagic === 0xcafebabf) { | ||
| return { platform: "darwin", arch: "universal" }; | ||
|
Comment on lines
+99
to
+100
Contributor
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. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Validate Mach-O fat architecture records. A fat Mach-O magic value identifies only the container. Its 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| // Mach-O thin binary, little-endian on every arch we ship. | ||
| const machoMagic = header.readUInt32LE(0); | ||
| if (machoMagic === 0xfeedface || machoMagic === 0xfeedfacf) { | ||
| const cpuType = header.readUInt32LE(4); | ||
| return { | ||
| platform: "darwin", | ||
| arch: MACHO_CPU_TYPES.get(cpuType) ?? `unknown(0x${cpuType.toString(16)})`, | ||
| }; | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| const installHint = (arch) => | ||
| `npm_config_platform=${targetPlatform} npm_config_arch=${arch} node scripts/install-ffmpeg-static.mjs`; | ||
|
|
||
| if (!existsSync(ffmpegPath) || !statSync(ffmpegPath).isFile()) { | ||
| fail( | ||
| `Bundled ffmpeg binary is missing at ${relativePath(ffmpegPath)}.\n` + | ||
| ` Install it with:\n` + | ||
| ` ${installHint(targetArches[0])}`, | ||
| ); | ||
| } | ||
|
|
||
| const header = readHeader(ffmpegPath, 1024); | ||
| const detected = identifyBinary(header); | ||
|
|
||
| if (!detected) { | ||
| fail( | ||
| `Could not identify the format of the bundled ffmpeg binary at ${relativePath(ffmpegPath)}.\n` + | ||
| ` It may be truncated or corrupt. Reinstall it with:\n` + | ||
| ` ${installHint(targetArches[0])}`, | ||
| ); | ||
| } | ||
|
|
||
| // A universal Mach-O covers both macOS architectures, so only the platform | ||
| // needs to match for it. | ||
| const unsatisfied = targetArches.filter( | ||
| (arch) => | ||
| detected.platform !== targetPlatform || | ||
| (detected.arch !== arch && detected.arch !== "universal"), | ||
| ); | ||
|
|
||
| if (unsatisfied.length > 0) { | ||
| const targetLabel = targetArches.map((arch) => `${targetPlatform}/${arch}`).join(", "); | ||
| const multiArch = targetArches.length > 1; | ||
|
|
||
| fail( | ||
| `Bundled ffmpeg binary does not match the build target.\n` + | ||
| ` Packaging for: ${targetLabel}\n` + | ||
| ` Binary is: ${detected.platform}/${detected.arch} (${relativePath(ffmpegPath)})\n` + | ||
| ` Unsatisfied: ${unsatisfied.map((arch) => `${targetPlatform}/${arch}`).join(", ")}\n` + | ||
| `\n` + | ||
| ` ffmpeg-static stages exactly one binary, chosen at npm install time, so a\n` + | ||
| ` build cannot cover a platform or architecture other than that one.\n` + | ||
| ` Shipping this would break every feature that shells out to ffmpeg.\n` + | ||
| `\n` + | ||
| (multiArch | ||
| ? ` This target spans ${targetArches.length} architectures, which one staged binary can\n` + | ||
| ` never satisfy. Build one architecture at a time, reinstalling ffmpeg in\n` + | ||
| ` between, for example:\n` + | ||
| targetArches | ||
| .map( | ||
| (arch) => | ||
| ` ${installHint(arch)} && npx electron-builder --${targetPlatform === "darwin" ? "mac" : targetPlatform} --${arch}`, | ||
| ) | ||
| .join("\n") + | ||
| `\n` | ||
| : ` Install the matching binary with:\n ${installHint(targetArches[0])}\n`) + | ||
| `\n` + | ||
| ` A correct ffmpeg is still not sufficient for a shippable cross-platform build:\n` + | ||
| ` native helpers and the whisper runtime are built for the host only. Prefer the\n` + | ||
| ` platform-specific CI jobs in .github/workflows/release.yml.`, | ||
| ); | ||
| } | ||
|
|
||
| console.log( | ||
| `[verify-bundled-ffmpeg] OK: ${detected.platform}/${detected.arch} satisfies build target ${targetArches.map((arch) => `${targetPlatform}/${arch}`).join(", ")} (${relativePath(ffmpegPath)})`, | ||
| ); | ||
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.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: webadderallorg/Recordly
Length of output: 16353
🏁 Script executed:
Repository: webadderallorg/Recordly
Length of output: 16398
🏁 Script executed:
Repository: webadderallorg/Recordly
Length of output: 16331
🌐 Web query:
ffmpeg-static 5.3.0 index.js win32 ffmpeg.exe💡 Result:
The ffmpeg-static package, including version 5.3.0, provides a programmatic way to access a statically linked FFmpeg binary within a Node.js project [1][2][3]. When you require the package in your code—e.g., const pathToFfmpeg = require('ffmpeg-static')—it returns the absolute path to the FFmpeg binary installed in your node_modules directory [4][5][6]. Regarding your query about win32 and ffmpeg.exe: 1. Installation Behavior: When you run npm install ffmpeg-static on a Windows environment (win32), the package installation script automatically downloads the appropriate precompiled FFmpeg binary [1][3][4]. 2. Path and Filename: The index.js file in ffmpeg-static is designed to resolve and provide the path to this binary [4][7]. On Windows systems, this path will point to the ffmpeg.exe file located within the node_modules/ffmpeg-static directory [8][4]. 3. Potential Issues: Some users have reported issues where the binary may not correctly append or resolve the.exe extension in certain cross-platform build environments (like Electron) [8]. If you find that the file exists but lacks the.exe extension, or that the path provided does not match the actual file on disk, it is often due to environment-specific configuration or cached node_modules [8][4]. To use it in your application, you typically pass this path to a library like fluent-ffmpeg or use it directly with Node.js' child_process [7][5]: const ffmpeg = require('fluent-ffmpeg'); const pathToFfmpeg = require('ffmpeg-static'); ffmpeg.setFfmpegPath(pathToFfmpeg);
Citations:
Resolve the platform-specific executable and inspect fat Mach-O architectures.
ffmpeg-staticusesffmpeg.exeforwin32, but Line 5 always checksffmpeg. Move path construction aftertargetPlatformis parsed.Proposed fix
Lines 97-100 also classify every fat Mach-O as
universalwithout reading its architecture records. Since Lines 140-143 then acceptuniversalfor every target architecture, a fat binary without the requestedx64orarm64slice can pass verification. Parse the records and require every target architecture to be present.📝 Committable suggestion
🤖 Prompt for AI Agents