Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion electron-builder.json5
Original file line number Diff line number Diff line change
Expand Up @@ -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-*/**"]
}
]
Expand Down
8 changes: 7 additions & 1 deletion electron/media/audioPeaks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/<tag>/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-"));
Expand Down
15 changes: 10 additions & 5 deletions electron/media/audioPeaks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/<tag>/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/<tag>/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 `<tag>/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
Expand Down
11 changes: 9 additions & 2 deletions electron/native/pipewire-capture/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand Down
17 changes: 5 additions & 12 deletions scripts/before-pack.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
{
Expand Down Expand Up @@ -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
Expand Down
15 changes: 12 additions & 3 deletions scripts/build-linux-pipewire-helper.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ for (const dir of [outDir, devDir]) {
}

/**
* Copies the vendored ffmpeg shared libraries into `<dir>/ffmpeg/`.
* Copies the vendored ffmpeg shared libraries into `<dir>/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
Expand All @@ -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 `<dir>/ffmpeg/` until a third claimant
* turned up on the same path: fetch-ffmpeg.mjs vendors the static ffmpeg
* EXECUTABLE to `<dir>/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");
Expand All @@ -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
Expand Down
28 changes: 20 additions & 8 deletions scripts/fetch-ffmpeg.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down Expand Up @@ -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 `<binDir>/ffmpeg` as a FILE, while
// build-linux-pipewire-helper.mjs stages the helper's libraries into
// `<binDir>/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 `<binDir>/ffmpeg` as a FILE while
// build-linux-pipewire-helper.mjs wanted `<binDir>/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);
Expand Down
Loading