Repository navigation
build: modernize emsdk and third-party library toolchain - #5
Merged
Merged
Conversation
|
Warning Review limit reachedNext included review available in 43 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (13)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Project516
force-pushed
the
build/toolchain-upgrade
branch
from
September 24, 2026 13:42
7794813 to
d66fd6b
Compare
Project516
marked this pull request as ready for review
September 24, 2026 14:41
Bumps every vendored third-party library to its latest stable tag, switching mirrored libraries (x265, libvpx, ogg, theora, opus, vorbis, zlib, libwebp, freetype2) to canonical upstream where a diff showed the ffmpegwasm/* mirror carries no emscripten-specific patch. x264 stays on the ffmpegwasm mirror (diverged too far from upstream to reconcile) and lame stays on its mirror (upstream has no maintained git tags). harfbuzz is pinned at 8.5.0, the last release with an autotools build; 9.0+ is meson-only and needs build/harfbuzz.sh rewritten separately. Also demotes a few C dialect checks the newer clang in emsdk 6.0.10 turned into hard errors, and lets the core-mt build's wasm memory grow (bounded at 2GB) instead of a fixed 1GB, so decoding a single 4K frame does not abort with OOM (ffmpegwasm#948). Co-authored-by: Ben Younes <2910651+ousamabenyounes@users.noreply.github.com>
ffmpeg and ffprobe exit via exit(), which Emscripten implements by throwing. The exception unwinds the JS frames but not the WebAssembly stack, so every call leaked stack space (ffmpegwasm#943); separately, the malloc'd argv strings and argv array from stringsToPtr() were never freed. Repeated calls in one session exhausted the heap and stack until 'memory access out of bounds'. Both are now cleaned up in a finally block. Needs a new core build to take effect. Co-authored-by: Mrmaxmeier <Mrmaxmeier@gmail.com>
emsdk >= 3.1.58 stopped needing a separate worker.js for multi-threaded cores (folded into the main core script), and >= 3.1.68 stops emitting even a stub. bind.js's _locateFile no longer routes .worker.js lookups, worker.ts no longer computes or forwards a workerURL to the core, and core-mt's package.json drops the now-nonexistent './worker' export. FFMessageLoadConfig.workerURL stays as an accepted-but-unused, deprecated option so existing callers do not break.
Emscripten's toolchain file reports CMAKE_SYSTEM_PROCESSOR=x86 for legacy
bitness-check compatibility. x265 4.x's CMakeLists.txt treats any 32-bit
x86 target as real ia32 and force-adds -march=i686, which emcc's clang
rejects outright for wasm32 ('unsupported option -march= for target
wasm32-unknown-emscripten'). Report a processor name x265 has no x86
special case for instead.
Canonical upstream's autogen.sh (autoreconf -if) no longer runs configure itself the way the old ffmpegwasm mirror's did, so passing configure flags straight to autogen.sh silently dropped them and left no Makefile.
Canonical vorbis (and likely others) now list transitive deps under Requires.private in their .pc files instead of the old ffmpegwasm mirror's plain Requires:. Plain `pkg-config --libs` ignores private requires, so linking against e.g. vorbisenc alone dropped -lvorbis/-logg and ffmpeg's configure reported 'vorbisenc not found using pkg-config'. --pkg-config-flags=--static makes pkg-config include private deps, which is what every one of these libraries is built as (static-only).
FFmpeg's configure runs pkg-config with --static, which pulls in Libs.private. x265 lists gcc_s, rt, and dl there; emscripten has none of them, so the x265 link check failed.
Project516
force-pushed
the
build/toolchain-upgrade
branch
from
September 24, 2026 14:42
442e64f to
2ebd1f3
Compare
Project516
added a commit
that referenced
this pull request
Sep 24, 2026
This PR stacks on build/toolchain-upgrade (#5, not yet merged) instead of master, so the pull_request trigger needs that base branch listed too or CI never runs on it.
Owner
Author
|
claude-opus-5-5 responding on behalf of project516 project516-review-bot timed out twice on this PR (free models returned empty replies, then hit rate limits), so a Sonnet review agent reviewed head 2ebd1f3 instead. Verdict: approve, no blocking issues. It hand-checked the bind.js argv free and stack restore, the x265 build ordering, and the upstream source hosts. Two nits, both deferred on purpose:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes
Step 1 of a staged FFmpeg upgrade (stays on FFmpeg 5.1.x; the fftools
scheduler rewrite in FFmpeg 6+ needs real threads and is out of scope
here, see "FFmpeg upgrade plan" in AGENTS.md). This PR modernizes the wasm core build toolchain:
stable release.
ffmpegwasm/*mirror, switches tocanonical upstream wherever a diff of the mirror against the matching
upstream tag showed no emscripten-specific patch (x265, libvpx, ogg,
theora, opus, vorbis, zlib, libwebp, freetype2). x264 and lame stay on
their mirrors; see "pinned, not bumped" below.
dialect checks clang now errors on by default, and drops the
ffmpeg-core.worker.jscodepath that current emsdk no longer emits formulti-threaded cores.
co-authored-by): freeing exec()/ffprobe()'s argv and restoring the wasm
stack pointer (Restore the wasm stack pointer after exec() and ffprobe() ffmpegwasm/ffmpeg.wasm#943), and letting the core-mt
build's wasm memory grow up to 2GB instead of a fixed 1GB so a single
4K frame does not OOM (fix: allow core-mt WASM memory to grow to avoid OOM on 4K frames (#946) ffmpegwasm/ffmpeg.wasm#948).
Versions
4-cores3.44.2v1.13.1v1.17.0masterv1.3.4v1.3.6v1.1.1v1.1.1v1.3.1v1.6.1v1.3.3v1.3.7v1.2.11v1.3.2v1.3.2v1.6.0VER-2-10-4VER-2-14-3v1.0.9v1.0.175.2.08.5.00.15.00.17.5release-3.0.5release-3.0.6Pinned, not bumped
stablebranch). A shallow-clone diff of the ffmpegwasm mirror's4-coresbranch against upstreamstableshows it has divergedyears' worth of upstream history around its own patch, so there is no
small patch to re-derive and reapply on top of current upstream. Kept
pinned to the mirror as-is.
has no maintained git remote with tags; the ffmpegwasm mirror's
masteris the only usable git source. Kept pinned.autotools
configure.ac(9.0.0 onward is meson-only). Jumping tolatest (14.5.0) needs
build/harfbuzz.shrewritten around meson plusan emscripten cross file, which is a bigger, separable change than a
version bump; left for a follow-up PR so this one stays focused on the
toolchain move.
Non-obvious fixes
Newer clang errors on old C: emsdk 6.0.10's clang defaults implicit
function declarations, mismatched function pointer types, and
int/pointer conversions to hard errors. n5.1.10 and its bundled libs
still use that looser dialect in places, so
CFLAGSnow demotes thosethree checks back to warnings (
-Wno-error=...) instead of patchingevery call site.
bind.js heap/stack leak:
exec()/ffprobe()callModule["_ffmpeg"]/Module["_ffprobe"], which exit via a thrown exception. That leaves thewasm stack pointer wherever the C code left it and never frees the
malloc'd argv strings/array, so repeated calls exhausted memory over a
session. Both are now cleaned up in a
finallyblock, ported fromRestore the wasm stack pointer after exec() and ffprobe() ffmpegwasm/ffmpeg.wasm#943 with the argv
free()half added on top(that half was never fixed upstream). Needs the new core build in this
PR to take effect.
core-mt memory growth: the multi-threaded build had a fixed
INITIAL_MEMORY=1024MBwith no growth, so a single high-resolutionframe could OOM even with nothing else wrong.
-sALLOW_MEMORY_GROWTHwith pthreads used to be discouraged, but current emscripten supports
growable shared memory; added
-sMAXIMUM_MEMORY=2GBas the cap sosmall jobs still start from the same 1GB initial allocation and only
grow when a job actually needs more (fix: allow core-mt WASM memory to grow to avoid OOM on 4K frames (#946) ffmpegwasm/ffmpeg.wasm#948).
worker.js removal: emsdk >= 3.1.68 no longer emits
ffmpeg-core.worker.jsfor multi-threaded builds (folded into the maincore script). Removed the
.worker.jsbranch from bind.js's_locateFile, the worker.js computation inpackages/ffmpeg/src/worker.ts,and the
./workerexport frompackages/core-mt/package.json.FFMessageLoadConfig.workerURLstays as an accepted-but-unused,@deprecatedoption so existing callers do not break.pkg-config and static libs: current
.pcfiles list transitive depsunder
Requires.private, so FFmpeg's configure now runs pkg-config with--static. That pulled x265's host-only libs (-lgcc_s,-lrt,-ldl,...) into the link, so
build/x265.shstrips them fromx265.pc.libc++ for x265: current emcc no longer links libc++ unless asked, so
configure and the final link pass
-sDEFAULT_TO_CXX.sem_close/sem_unlink in the st core: x265 4.x references them for
cross-process shared memory, and Emscripten's single-thread libc lacks
them. The st build adds two stubs to
libx265.a. ffmpeg.wasm never enablesthat x265 feature.
zlib 1.3.2: its new CMake builds a shared library and skips
zlib.pcunder
SKIP_INSTALL_FILES. It is now built static only, with its.pc.ENVIRONMENT=web,worker: with
workeralone, current emsdk loads thewasm with a synchronous XHR, which browsers block on a page. Adding
weblets the core load directly on a page again.
UMD pthreads: current emsdk spawns pthreads from
self.location.hrefin a worker and ignores
mainScriptUrlOrBlob. When the UMD core isloaded with
importScripts(), that URL is the@project516/ffmpegworker, so every pthread ran the wrapper and
load()hung. The UMD buildnow patches that one line to prefer
mainScriptUrlOrBlob, and fails ifthe line changes.
build/ffmpeg.shprintsffbuild/config.logwhen configure fails, so CIshows why.
Verification
pnpm lintandpnpm buildpass locally.test pages (
ffmpeg-core-st,ffmpeg-st,ffmpeg-mt).Noticed, not fixed
build/harfbuzz.sh(see "pinned, not bumped" above).ffmpegwasm/x264mirror; no safe path tocanonical upstream without hand-porting an unknown-scope patch.
tracked in "FFmpeg upgrade plan" in AGENTS.md; not attempted here.