From 2cc1c11f7042e7ca2e80dc9fb8e0c4cd1a56e70a Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Fri, 7 Aug 2026 13:40:34 +0200 Subject: [PATCH] fix(linux): give the capture helper's ffmpeg libraries their own directory name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three artifacts claimed electron/native/bin/linux-x64/ffmpeg: - fetch-ffmpeg.mjs vendors the static ffmpeg EXECUTABLE there (PINNED["linux-x64"].exe) - build-linux-pipewire-helper.mjs mkdir'd it as a DIRECTORY for the helper's unrenamed libav*/libsw*.so, the name its $ORIGIN/ffmpeg RUNPATH is compiled against - audioPeaks.ts resolves that same path expecting a binary to spawn Whoever ran last won, and every loser failed obliquely: `EEXIST: mkdir .../linux-x64/ffmpeg` from the helper build, `spawn … EACCES` from audioPeaks when it found a directory where a binary belonged. CI never saw it because build:linux runs only fetch:ffmpeg:sdk on a fresh checkout — a flag that exists, per its own comment, to dodge this very collision. The helper's directory moves to helper-ffmpeg/ rather than the executable moving, because a bare `ffmpeg` means the executable to every other reader of this tree (win32 has ffmpeg.exe at the same level), and a directory of shared objects under that name is a lie about its contents. RUNPATH follows in build.rs. The staleness/payload guard in before-pack.cjs follows too, and its file-instead-of-directory branch loses the explanation of a collision that can no longer happen. Also corrects the --sdk-only comment, which justified itself with that collision, and the claim in assertLgpl's note that nothing in the app spawns the ffmpeg CLI any more — audioPeaks does, falling back to the browser decode paths when it is absent, which on Linux is always. Whether the CLI should ship on Linux at all is a separate question (~110 MB) and this change does not touch it. --- electron-builder.json5 | 6 ++++- electron/media/audioPeaks.test.ts | 8 ++++++- electron/media/audioPeaks.ts | 15 ++++++++---- electron/native/pipewire-capture/build.rs | 11 +++++++-- scripts/before-pack.cjs | 17 ++++---------- scripts/build-linux-pipewire-helper.mjs | 15 +++++++++--- scripts/fetch-ffmpeg.mjs | 28 ++++++++++++++++------- 7 files changed, 68 insertions(+), 32 deletions(-) diff --git a/electron-builder.json5 b/electron-builder.json5 index 13c1cb2c5..b1d394ac9 100644 --- a/electron-builder.json5 +++ b/electron-builder.json5 @@ -109,11 +109,15 @@ "from": "electron/native/bin", "to": "electron/native/bin", // `**`, not `*`: the capture helper's ffmpeg libraries live one level - // deeper, in `linux-x64/ffmpeg/`. They are in their own directory + // deeper, in `linux-x64/helper-ffmpeg/`. They are in their own directory // because `linux-x64/` also holds the compositor addon's copies of the // same sonames with every symbol renamed to `osff_*`, and the helper // must not find those — see scripts/build-linux-pipewire-helper.mjs. // A single-level filter ships a helper that cannot start. + // + // The name is `helper-ffmpeg` rather than `ffmpeg` because that shorter + // name is taken: fetch-ffmpeg.mjs vendors the static ffmpeg executable + // there, and audioPeaks.ts resolves it by that path. "filter": ["linux-*/**"] } ] diff --git a/electron/media/audioPeaks.test.ts b/electron/media/audioPeaks.test.ts index a49c28b06..2a455bfc3 100644 --- a/electron/media/audioPeaks.test.ts +++ b/electron/media/audioPeaks.test.ts @@ -55,10 +55,16 @@ describe("ffmpeg resolution", () => { }); /** - * The shape that slipped through. A Linux dev checkout can have + * The shape that slipped through. A Linux dev checkout used to have * `electron/native/bin//ffmpeg` as a DIRECTORY of shared libraries * rather than the binary; `existsSync` accepted it, resolution stopped * there, and the failure only surfaced later as `spawn … EACCES`. + * + * That particular collision is gone — the helper's libraries moved to + * `helper-ffmpeg/` — but the assertion stays, because it is really about + * `resolveFfmpeg` not confusing existence with executability, and the next + * thing to land a directory on a candidate path will not announce itself + * either. */ it("skips a candidate that is a directory rather than the binary", () => { const here = mkdtempSync(path.join(tmpdir(), "openscreen-ffmpeg-")); diff --git a/electron/media/audioPeaks.ts b/electron/media/audioPeaks.ts index f1beabeb5..604afd051 100644 --- a/electron/media/audioPeaks.ts +++ b/electron/media/audioPeaks.ts @@ -103,11 +103,16 @@ let cachedFfmpeg: string | null | undefined; * * EXISTENCE IS NOT ENOUGH, and the difference is not academic. `existsSync` was * the test here, and it answers true for a DIRECTORY: on a Linux dev machine - * `electron/native/bin//ffmpeg` is a folder holding the shared libraries - * (`libavcodec.so.62` and friends) rather than the binary, so resolution picked - * the folder, every later candidate was skipped, and the failure surfaced much - * later as `spawn … EACCES` — a message that blames permissions rather than - * saying the wrong candidate was chosen. + * `electron/native/bin//ffmpeg` used to be a folder holding the capture + * helper's shared libraries (`libavcodec.so.62` and friends) rather than the + * binary, so resolution picked the folder, every later candidate was skipped, + * and the failure surfaced much later as `spawn … EACCES` — a message that + * blames permissions rather than saying the wrong candidate was chosen. + * + * Those libraries have since moved to `/helper-ffmpeg/`, so this path is + * the executable's alone again. The check stays regardless: it costs one stat, + * and it is the difference between a clear null and an EACCES half a subsystem + * away. * * Every failure mode is swallowed on purpose. A candidate that is absent, not a * regular file, or not executable is simply not this one; throwing out of diff --git a/electron/native/pipewire-capture/build.rs b/electron/native/pipewire-capture/build.rs index 5aecdb941..d9aa33662 100644 --- a/electron/native/pipewire-capture/build.rs +++ b/electron/native/pipewire-capture/build.rs @@ -115,7 +115,7 @@ fn link_ffmpeg(root: &Path) { for name in ["avcodec", "avformat", "avutil", "swscale", "swresample"] { println!("cargo:rustc-link-lib={name}"); } - // `$ORIGIN/ffmpeg`, NOT `$ORIGIN`. The helper is staged into + // A SUBDIRECTORY, NOT `$ORIGIN`. The helper is staged into // electron/native/bin/linux-x64/, and that directory ALREADY contains // libavcodec.so.62 and friends — the copies whose every symbol was renamed // to `osff_*` by scripts/build-linux-compositor-addon.mjs so the compositor @@ -125,12 +125,19 @@ fn link_ffmpeg(root: &Path) { // renaming trick is what makes the ADDON work and what would break the // HELPER, so the two sets of libraries must not share a directory. // + // `helper-ffmpeg` and not `ffmpeg`, which is what this used to be: that name + // is also where fetch-ffmpeg.mjs vendors the static ffmpeg EXECUTABLE, and + // where audioPeaks.ts looks for it. Three artifacts, one path — whichever + // ran last won, and the loser failed with EEXIST from mkdir or EACCES from + // spawn, neither of which names the real problem. A directory called + // `ffmpeg` full of shared objects is also simply a lie about its contents. + // // The absolute vendored path comes second so `cargo run` works straight out // of the repo. `--disable-new-dtags` is what makes these RUNPATH entries // apply to the transitive ffmpeg libs too; with the default DT_RUNPATH they // would not. println!("cargo:rustc-link-arg=-Wl,--disable-new-dtags"); - println!("cargo:rustc-link-arg=-Wl,-rpath,$ORIGIN/ffmpeg"); + println!("cargo:rustc-link-arg=-Wl,-rpath,$ORIGIN/helper-ffmpeg"); println!("cargo:rustc-link-arg=-Wl,-rpath,{}", lib.display()); let mut builder = bindgen::Builder::default(); diff --git a/scripts/before-pack.cjs b/scripts/before-pack.cjs index e017b4014..58a9e17bc 100644 --- a/scripts/before-pack.cjs +++ b/scripts/before-pack.cjs @@ -112,11 +112,11 @@ const MAC_REQUIRED = [ * `linux.extraResources` ships this directory wholesale (`filter: ["linux-*​/**"]`), * so "present here" is the same thing as "present in the installed app". * - * Note the two ffmpeg sets, which is why `ffmpeg/` is required separately below: + * Note the two ffmpeg sets, which is why `helper-ffmpeg/` is required separately below: * the `.so` files sitting directly in this directory are the compositor's copies, * with every symbol renamed to `osff_*` so the addon cannot bind to Chromium's * bundled ffmpeg. The helper needs the *unrenamed* originals, which is what the - * `ffmpeg/` subdirectory holds. + * `helper-ffmpeg/` subdirectory holds. */ const LINUX_REQUIRED = [ { @@ -226,20 +226,13 @@ function checkLinuxNativePayload(context) { // property that matters — it has to be a directory holding the *unrenamed* libraries. // An empty one, or the wrong kind of entry, passes a name match and still ships a // helper that cannot start. - const helperFfmpeg = path.join(dir, "ffmpeg"); + const helperFfmpeg = path.join(dir, "helper-ffmpeg"); const isDir = fs.existsSync(helperFfmpeg) && fs.statSync(helperFfmpeg).isDirectory(); if (fs.existsSync(helperFfmpeg) && !isDir) { - // `fetch:ffmpeg` vendors the *static* ffmpeg binary to exactly this path, while - // `build:native:linux` wants a directory here. They collide, and the loser is - // whichever ran first. CI never sees it — `build:linux` only runs - // `fetch:ffmpeg:sdk`, which does not write the executable — so this fires on - // local packaging after someone has run the full fetch by hand. throw new Error( `Refusing to package: ${path.relative(ROOT, helperFfmpeg)} is a file, not a directory.\n\n` + - "That path is where the PipeWire helper's ffmpeg libraries live, but the static\n" + - "ffmpeg binary that `npm run fetch:ffmpeg` vendors lands on the same name and\n" + - "overwrote it. Delete it and re-run:\n\n npm run build:native:linux\n\n" + - "(`npm run build:linux` uses fetch:ffmpeg:sdk, which does not write that file.)", + "It should hold the PipeWire helper's unrenamed ffmpeg shared objects.\n" + + "Delete it and re-run:\n\n npm run build:native:linux", ); } const libs = isDir diff --git a/scripts/build-linux-pipewire-helper.mjs b/scripts/build-linux-pipewire-helper.mjs index e5c147827..d761db6f7 100644 --- a/scripts/build-linux-pipewire-helper.mjs +++ b/scripts/build-linux-pipewire-helper.mjs @@ -97,7 +97,7 @@ for (const dir of [outDir, devDir]) { } /** - * Copies the vendored ffmpeg shared libraries into `/ffmpeg/`. + * Copies the vendored ffmpeg shared libraries into `/helper-ffmpeg/`. * * THE SUBDIRECTORY IS THE WHOLE POINT. `electron/native/bin/linux-x64/` already * holds libavcodec.so.62 and friends — but those are the copies whose every @@ -107,11 +107,20 @@ for (const dir of [outDir, devDir]) { * different consumers, and only one of them can win a directory. * * The addon needs the renamed set next to itself; the helper needs the ordinary - * set. So the helper's RUNPATH is `$ORIGIN/ffmpeg` (see build.rs) and its + * set. So the helper's RUNPATH is `$ORIGIN/helper-ffmpeg` (see build.rs) and its * libraries live here. Putting them side by side produced exactly one symptom, * which the probe below catches: * * undefined symbol: avcodec_send_frame, version LIBAVCODEC_62 + * + * AND THE NAME IS PART OF IT. This was `/ffmpeg/` until a third claimant + * turned up on the same path: fetch-ffmpeg.mjs vendors the static ffmpeg + * EXECUTABLE to `/ffmpeg` (PINNED["linux-x64"].exe), and audioPeaks.ts + * resolves that same path expecting a binary. Whoever ran last won, and the + * loser failed obliquely — `EEXIST: mkdir .../ffmpeg` here, or `spawn … + * EACCES` from audioPeaks when it found a directory where a binary belonged. + * Two artifacts of different shapes cannot share a name; this one moved because + * a bare `ffmpeg` means the executable to every other reader of this tree. */ function stageFfmpeg(dir) { const source = path.join(root, "crates", "thirdparty", "ffmpeg-linux64-lgpl-shared", "lib"); @@ -123,7 +132,7 @@ function stageFfmpeg(dir) { return; } - const target = path.join(dir, "ffmpeg"); + const target = path.join(dir, "helper-ffmpeg"); fs.mkdirSync(target, { recursive: true }); // Only the sonames the helper actually links, and only the real files — // the tree also holds unversioned `.so` symlinks that the loader never diff --git a/scripts/fetch-ffmpeg.mjs b/scripts/fetch-ffmpeg.mjs index 40ab59481..331457da4 100644 --- a/scripts/fetch-ffmpeg.mjs +++ b/scripts/fetch-ffmpeg.mjs @@ -450,8 +450,10 @@ async function fetchSharedDlls(tag, binDir) { // copies belong in binDir. On Linux that same directory is owned by the two // native build scripts: build-linux-compositor-addon.mjs puts SYMBOL-RENAMED // (osff_*) copies there so the addon cannot bind to Chromium's ffmpeg, and - // build-linux-pipewire-helper.mjs stages unrenamed ones in `binDir/ffmpeg/` - // for the helper's `$ORIGIN/ffmpeg` RUNPATH. Dropping a third, unrenamed set + // build-linux-pipewire-helper.mjs stages unrenamed ones in + // `binDir/helper-ffmpeg/` for the helper's `$ORIGIN/helper-ffmpeg` RUNPATH + // (named to stay clear of `binDir/ffmpeg`, which is the static executable + // this script vendors). Dropping a third, unrenamed set // in binDir would overwrite the renamed ones under identical filenames and // break the addon at load time. Linux takes the SDK below and nothing else. if (process.platform !== "win32") { @@ -510,12 +512,22 @@ async function main() { const dest = path.join(binDir, spec.exe); // `--sdk-only` skips the standalone ffmpeg CLI and vendors just the build-time - // SDK. Linux needs it: the CLI lands at `/ffmpeg` as a FILE, while - // build-linux-pipewire-helper.mjs stages the helper's libraries into - // `/ffmpeg/` as a DIRECTORY — the name its `$ORIGIN/ffmpeg` RUNPATH is - // compiled against. One clobbers the other (`EEXIST: mkdir .../linux-x64/ffmpeg`). - // Nothing in the app spawns the CLI any more (see assertLgpl's note), and v1.7.0 - // shipped Linux packages without it, so on Linux it is dead weight AND a conflict. + // SDK, which is what `build:linux` uses. + // + // It was introduced because the CLI landed at `/ffmpeg` as a FILE while + // build-linux-pipewire-helper.mjs wanted `/ffmpeg/` as a DIRECTORY, and + // one clobbered the other (`EEXIST: mkdir .../linux-x64/ffmpeg`). That conflict + // is gone — the helper's libraries live in `helper-ffmpeg/` now — so this flag + // no longer avoids a collision. What is left is a size argument: the static CLI + // is ~110 MB, `linux.extraResources` has no exclusion for it (unlike Windows' + // "!win32-*/ffmpeg.exe"), and Linux packages have shipped without it since + // v1.7.0. + // + // One caveat if that is ever revisited: the app is NOT entirely done with the + // CLI, contrary to assertLgpl's note below. electron/media/audioPeaks.ts spawns + // it to decode waveform peaks ~6x faster than the renderer can, and falls back + // to the browser pipelines when it is absent — so on Linux that fallback is + // always the one taken. Degraded, cached after the first decode, not broken. if (process.argv.includes("--sdk-only")) { console.log(`Skipping the standalone ffmpeg CLI (--sdk-only).`); await fetchSharedDlls(tag, binDir);