feat(wasm): FfmpegFallbackStrategy — MP4/MOV/MKV/WebM via ffmpeg-wasm, on by default (#182) #183

Merged
forgejo_admin merged 22 commits from feat/ffmpeg-poc into master 2026-05-22 11:04:05 +00:00

Closes #182. Closes #43.

Adds a third strategy (peer to ExifToolFallbackStrategy) using ffmpeg-wasm. On by default for all three distributions (standalone HTML, Capacitor APK, PWA self-host); VITE_ENABLE_FFMPEG_FALLBACK=false opts out.

When enabled, FfmpegFallbackStrategy takes priority over VideoStrategy for .mp4/.mov/.m4v (Phase 1) and adds new coverage for .mkv/.webm (Phase 2). VideoStrategy stays in the registry as the opt-out fallback; deletion is a subsequent PR after a validation window per #182's sequencing.

The strategy loads @ffmpeg/core directly on the main thread rather than going through the @ffmpeg/ffmpeg Web Worker wrapper. The wrapper spawns type: "module" Workers from Blob URLs, which fail silently under file:// (null-origin) in Chromium — the standalone HTML build was hanging forever on every video strip until we cut the wrapper out. @ffmpeg/ffmpeg was dropped from package.json in 9f87ff6; only @ffmpeg/core ships now.

A post-strip pass runs after the ffmpeg invocation to remove muxer-added fields ffmpeg leaks even with -map_metadata -1 — specifically the moov/udta/meta/hdlr appl/mdir vendor block, the appl movie-level meta atom, and per-track btrt (buffer/bitrate) boxes. These are written unconditionally by mov_write_meta_tag regardless of the metadata flags, so they have to be scrubbed structurally after the remux.

Why

VideoStrategy has documented coverage gaps (#38, #39, #111, #42) — each tractable but slow to close. ffmpeg closes them in one shot via its remux pipeline. More importantly, no browser-based privacy tool covers MKV/WebM stripping today — adding ffmpeg-wasm gives MetaScrub a real competitive position on long-tail container formats. WebM is particularly valuable: it's the default for browser-recorded video via MediaRecorder.

The -map 0:v? -map 0:a? invocation choice is the key departure from mat2's default -map 0. Dropping all data streams (timecode, description, gpmd, subtitle) makes GoPro Fusion / DJI files succeed where mat2 exits with code 234. It also removes the GPMF GPS, device fingerprints, and handler/compressor name leaks the walker left in.

What's in the bundle

Nine commits on this branch, in chronological order:

  1. b4fc959docs(poc): @ffmpeg/ffmpeg evaluation for FfmpegFallbackStrategy (#182). Package evaluation: ~10 MB gzipped WASM, no-network verified (static + dynamic), performance benchmarks, license analysis.
  2. d2d2eddfeat(wasm): FfmpegFallbackStrategy for MP4/MOV/M4V (Phase 1). Strategy + docs/gap-analysis/mp4-ffmpeg.md + tests + registry wiring. Routes MP4-family containers ahead of VideoStrategy.
  3. 5ce4200feat(wasm): FfmpegFallbackStrategy Phase 2 — MKV/WebM + forensic runner. Adds docs/gap-analysis/{mkv,webm}.md, docs/forensic/ffmpeg-fallback.md, and tools/forensic/ffmpeg-fallback.ts.
  4. 9d60f37ci(ffmpeg): smoke build with VITE_ENABLE_FFMPEG_FALLBACK=false. New build-no-ffmpeg job exercises the opt-out build path. Adds README "Third-party engines and license notices" section.
  5. 2e4b8ddperf(standalone): inline ffmpeg trio in <script type=text/plain>. Closes the standalone-single-file gap — viteSingleFile only inlines JS/CSS chunks, so ffmpeg-core.wasm (30.7 MB), ffmpeg-core.js, and the @ffmpeg/ffmpeg worker were shipping as sibling files and 404ing under file://. Two new Vite plugins (standaloneFfmpegStubPlugin, standaloneFfmpegInlinePlugin) mirror the existing zeroperl base64-inline pattern.
  6. eae2dbetest(forensic): add DJI Phantom 4 (236 MB) to ffmpeg-fallback runner. New real-world fixture in the forensic battery.
  7. 9f87ff6fix(standalone): main-thread @ffmpeg/core, drop @ffmpeg/ffmpeg wrapper. The wrapper hardcodes type: "module" Workers from Blob URLs, which fail silently under file:// (null origin) in Chromium — the standalone build hung forever on every video strip. Switches to loading @ffmpeg/core directly on the main thread and removes @ffmpeg/ffmpeg from package.json.
  8. eb65f13test(e2e): MP4 strip through standalone + web — would have caught the Worker bug. Two new Playwright tests: tests/e2e/standalone/standalone.spec.ts exercises the strategy through the real dist/web-standalone/index.html over file://; tests/e2e/web/file-processing.spec.ts exercises the dev-server-served PWA (different asset-resolution branch — Vite ?url emission vs inline-base64). With the old wrapper, the standalone test would have hung; the new tests cap at 90s and assert sentinel-clean output.
  9. 5269e7cfix(ffmpeg): strip muxer-added fields ffmpeg leaks after -map_metadata -1. Post-strip pass scrubs the movie-level moov/udta/meta/hdlr mdir/appl vendor block, the appl meta atom, and per-track btrt boxes that ffmpeg's mov_write_meta_tag emits unconditionally regardless of metadata flags.

Decisions captured in the bundle

  • On by default for all three distributions; VITE_ENABLE_FFMPEG_FALLBACK=false opts out
  • Single-threaded @ffmpeg/core 0.12.10 (no SAB / COOP-COEP requirement)
  • Main-thread load@ffmpeg/core is loaded directly without the @ffmpeg/ffmpeg Worker wrapper, which fails under file:// (see commit 7)
  • @ffmpeg/core is GPL-2.0-or-later — combined distributable inherits, MIT codebase unchanged, source pointer added to README per GPL compliance (#182: "use the better one, don't overthink about licensing")
  • Container brand normalisation policy: -fflags +bitexact (zeroes timestamps per privacy-invariants §6) + -metadata encoder= (suppresses the Lavf<version> strip-tool fingerprint) + post-strip pass for appl/btrt. Full policy in docs/gap-analysis/mp4-ffmpeg.md.
  • Sidecar handling (#46): unchanged — neither walker nor ffmpeg helps; documented in gap analysis
  • Streaming I/O: out of scope; #34 is the proper home
  • Standalone HTML stays single-file: base64-inlined ffmpeg trio via two Vite plugins, mirroring the zeroperl pattern

Privacy / sandbox audit

Documented in docs/poc/ffmpeg-wasm.md. Honest framing: defense-in-depth, not "structural by construction" like the WebPerl-ExifTool case.

  • JS-side fetch is only used to load the WASM itself (we control the URL via Vite's ?url asset emission)
  • Emscripten SOCKFS code is present (ffmpeg's source uses socket() for rtsp:///tcp:///udp:// protocols) but dormant — requires Module["websocket"] to be functional, which we don't provide
  • ffmpeg's inputs are restricted to MEMFS paths; never handed network URLs
  • CSP connect-src 'self' is the deploy-layer backstop
  • Verified dynamically: node --permission without --allow-net produces byte-identical output

The WASM imports are minified by Emscripten (single-letter symbols), so the import-section name audit that worked for zeroperl.wasm doesn't apply here. The JS-side audit + dynamic test substitute.

Forensic verification

npx tsx tools/forensic/ffmpeg-fallback.ts — all fixtures clean (synthetic + real-world, including the 236 MB DJI Phantom 4 added in commit 6), KNOWN_GAPS empty. After commit 9 the post-strip pass also clears HandlerVendorID: Apple, BufferSize, and MaxBitrate/AverageBitrate leaks that exiftool was reporting from the muxer-added moov/udta/meta/hdlr block.

GoPro Fusion remains the most demanding fixture — mat2 fails it outright (ffmpeg -codec copy exit 234 on the tmcd/fdsc data streams). With -map 0:v? -map 0:a? those streams are dropped and all 7 device fingerprints (GoPro AVC, gpmd, GoPro AAC, GoPro TCD, GoPro MET, GoPro SOS, Fusion) are gone from the output. Better than mat2 and better than the current walker on the same file.

Quality gates

  • yarn typecheck
  • yarn lint
  • yarn test — covers strategy + registry gating + post-strip pass
  • yarn check:deps — no circular deps
  • yarn build:web — succeeds with and without VITE_ENABLE_FFMPEG_FALLBACK=false
  • yarn build:web:standalone — succeeds; ffmpeg trio inline-base64'd, single-file output verified
  • yarn test:e2e:standalone + yarn test:e2e:web — MP4 strip through real built apps, 90s cap, sentinel-clean assertions
  • Forensic runner — synthetic + real-world fixtures (including 236 MB DJI) clean, zero sentinel + zero device-fingerprint survival
  • New CI build-no-ffmpeg job exercises the opt-out build path

Known limitations (follow-ups, not blockers)

  1. True tree-shaking for VITE_ENABLE_FFMPEG_FALLBACK=false: the strategy is statically imported in strategy_registry.ts (matches the existing ExifToolFallbackStrategy pattern), so the 30 MB ffmpeg-core.wasm ships in dist/web/assets/ even when registration is skipped. Real tree-shaking would need dynamic imports — same limitation as the ExifTool fallback today. Worth a separate PR that improves both fallbacks.
  2. Real-world MKV/WebM fixtures: the forensic runner covers synthetic-only for these formats today. Adding a MediaRecorder-generated WebM and a typical MKV download to tests/fixtures/wasm/video/real-world/ is a worthwhile next step.
  3. Subsequent PRs from #182's "tracked, not this issue" list:
    • Delete VideoStrategy after a validation window
    • Add .avi claim (closes #44)
    • Add .wmv, .3gp after per-format forensic verification
    • Streaming I/O for large files (#34)
Closes #182. Closes #43. Adds a third strategy (peer to `ExifToolFallbackStrategy`) using ffmpeg-wasm. On by default for all three distributions (standalone HTML, Capacitor APK, PWA self-host); `VITE_ENABLE_FFMPEG_FALLBACK=false` opts out. When enabled, **`FfmpegFallbackStrategy` takes priority over `VideoStrategy`** for `.mp4`/`.mov`/`.m4v` (Phase 1) and adds new coverage for `.mkv`/`.webm` (Phase 2). `VideoStrategy` stays in the registry as the opt-out fallback; deletion is a subsequent PR after a validation window per #182's sequencing. The strategy loads `@ffmpeg/core` **directly on the main thread** rather than going through the `@ffmpeg/ffmpeg` Web Worker wrapper. The wrapper spawns `type: "module"` Workers from Blob URLs, which fail silently under `file://` (null-origin) in Chromium — the standalone HTML build was hanging forever on every video strip until we cut the wrapper out. `@ffmpeg/ffmpeg` was dropped from `package.json` in `9f87ff6`; only `@ffmpeg/core` ships now. A post-strip pass runs after the ffmpeg invocation to remove muxer-added fields ffmpeg leaks even with `-map_metadata -1` — specifically the `moov/udta/meta/hdlr` `appl`/`mdir` vendor block, the `appl` movie-level `meta` atom, and per-track `btrt` (buffer/bitrate) boxes. These are written unconditionally by `mov_write_meta_tag` regardless of the metadata flags, so they have to be scrubbed structurally after the remux. ## Why `VideoStrategy` has documented coverage gaps (#38, #39, #111, #42) — each tractable but slow to close. ffmpeg closes them in one shot via its remux pipeline. More importantly, **no browser-based privacy tool covers MKV/WebM stripping today** — adding ffmpeg-wasm gives MetaScrub a real competitive position on long-tail container formats. WebM is particularly valuable: it's the default for browser-recorded video via MediaRecorder. The `-map 0:v? -map 0:a?` invocation choice is the key departure from mat2's default `-map 0`. Dropping all data streams (timecode, description, gpmd, subtitle) makes GoPro Fusion / DJI files succeed where mat2 exits with code 234. It also removes the GPMF GPS, device fingerprints, and handler/compressor name leaks the walker left in. ## What's in the bundle Nine commits on this branch, in chronological order: 1. `b4fc959` — **docs(poc):** `@ffmpeg/ffmpeg` evaluation for `FfmpegFallbackStrategy` (#182). Package evaluation: ~10 MB gzipped WASM, no-network verified (static + dynamic), performance benchmarks, license analysis. 2. `d2d2edd` — **feat(wasm):** `FfmpegFallbackStrategy` for MP4/MOV/M4V (Phase 1). Strategy + `docs/gap-analysis/mp4-ffmpeg.md` + tests + registry wiring. Routes MP4-family containers ahead of `VideoStrategy`. 3. `5ce4200` — **feat(wasm):** `FfmpegFallbackStrategy` Phase 2 — MKV/WebM + forensic runner. Adds `docs/gap-analysis/{mkv,webm}.md`, `docs/forensic/ffmpeg-fallback.md`, and `tools/forensic/ffmpeg-fallback.ts`. 4. `9d60f37` — **ci(ffmpeg):** smoke build with `VITE_ENABLE_FFMPEG_FALLBACK=false`. New `build-no-ffmpeg` job exercises the opt-out build path. Adds README "Third-party engines and license notices" section. 5. `2e4b8dd` — **perf(standalone):** inline ffmpeg trio in `<script type=text/plain>`. Closes the standalone-single-file gap — `viteSingleFile` only inlines JS/CSS chunks, so `ffmpeg-core.wasm` (30.7 MB), `ffmpeg-core.js`, and the `@ffmpeg/ffmpeg` worker were shipping as sibling files and 404ing under `file://`. Two new Vite plugins (`standaloneFfmpegStubPlugin`, `standaloneFfmpegInlinePlugin`) mirror the existing zeroperl base64-inline pattern. 6. `eae2dbe` — **test(forensic):** add DJI Phantom 4 (236 MB) to ffmpeg-fallback runner. New real-world fixture in the forensic battery. 7. `9f87ff6` — **fix(standalone):** main-thread `@ffmpeg/core`, drop `@ffmpeg/ffmpeg` wrapper. The wrapper hardcodes `type: "module"` Workers from Blob URLs, which fail silently under `file://` (null origin) in Chromium — the standalone build hung forever on every video strip. Switches to loading `@ffmpeg/core` directly on the main thread and removes `@ffmpeg/ffmpeg` from `package.json`. 8. `eb65f13` — **test(e2e):** MP4 strip through standalone + web — would have caught the Worker bug. Two new Playwright tests: `tests/e2e/standalone/standalone.spec.ts` exercises the strategy through the real `dist/web-standalone/index.html` over `file://`; `tests/e2e/web/file-processing.spec.ts` exercises the dev-server-served PWA (different asset-resolution branch — Vite `?url` emission vs inline-base64). With the old wrapper, the standalone test would have hung; the new tests cap at 90s and assert sentinel-clean output. 9. `5269e7c` — **fix(ffmpeg):** strip muxer-added fields ffmpeg leaks after `-map_metadata -1`. Post-strip pass scrubs the movie-level `moov/udta/meta/hdlr` `mdir`/`appl` vendor block, the `appl` `meta` atom, and per-track `btrt` boxes that ffmpeg's `mov_write_meta_tag` emits unconditionally regardless of metadata flags. ## Decisions captured in the bundle - **On by default** for all three distributions; `VITE_ENABLE_FFMPEG_FALLBACK=false` opts out - **Single-threaded `@ffmpeg/core` 0.12.10** (no SAB / COOP-COEP requirement) - **Main-thread load** — `@ffmpeg/core` is loaded directly without the `@ffmpeg/ffmpeg` Worker wrapper, which fails under `file://` (see commit 7) - **`@ffmpeg/core` is GPL-2.0-or-later** — combined distributable inherits, MIT codebase unchanged, source pointer added to README per GPL compliance (#182: "use the better one, don't overthink about licensing") - **Container brand normalisation policy**: `-fflags +bitexact` (zeroes timestamps per privacy-invariants §6) + `-metadata encoder=` (suppresses the `Lavf<version>` strip-tool fingerprint) + post-strip pass for `appl`/`btrt`. Full policy in `docs/gap-analysis/mp4-ffmpeg.md`. - **Sidecar handling (#46)**: unchanged — neither walker nor ffmpeg helps; documented in gap analysis - **Streaming I/O**: out of scope; `#34` is the proper home - **Standalone HTML stays single-file**: base64-inlined ffmpeg trio via two Vite plugins, mirroring the zeroperl pattern ## Privacy / sandbox audit Documented in `docs/poc/ffmpeg-wasm.md`. Honest framing: **defense-in-depth, not "structural by construction"** like the WebPerl-ExifTool case. - JS-side `fetch` is only used to load the WASM itself (we control the URL via Vite's `?url` asset emission) - Emscripten SOCKFS code is present (ffmpeg's source uses `socket()` for `rtsp://`/`tcp://`/`udp://` protocols) but **dormant** — requires `Module["websocket"]` to be functional, which we don't provide - ffmpeg's inputs are restricted to MEMFS paths; never handed network URLs - CSP `connect-src 'self'` is the deploy-layer backstop - Verified dynamically: `node --permission` without `--allow-net` produces byte-identical output The WASM imports are minified by Emscripten (single-letter symbols), so the import-section name audit that worked for `zeroperl.wasm` doesn't apply here. The JS-side audit + dynamic test substitute. ## Forensic verification `npx tsx tools/forensic/ffmpeg-fallback.ts` — all fixtures clean (synthetic + real-world, including the 236 MB DJI Phantom 4 added in commit 6), `KNOWN_GAPS` empty. After commit 9 the post-strip pass also clears `HandlerVendorID: Apple`, `BufferSize`, and `MaxBitrate`/`AverageBitrate` leaks that exiftool was reporting from the muxer-added `moov/udta/meta/hdlr` block. GoPro Fusion remains the most demanding fixture — `mat2` fails it outright (`ffmpeg -codec copy` exit 234 on the `tmcd`/`fdsc` data streams). With `-map 0:v? -map 0:a?` those streams are dropped and all 7 device fingerprints (`GoPro AVC`, `gpmd`, `GoPro AAC`, `GoPro TCD`, `GoPro MET`, `GoPro SOS`, `Fusion`) are gone from the output. **Better than mat2 and better than the current walker** on the same file. ## Quality gates - ✅ `yarn typecheck` - ✅ `yarn lint` - ✅ `yarn test` — covers strategy + registry gating + post-strip pass - ✅ `yarn check:deps` — no circular deps - ✅ `yarn build:web` — succeeds with and without `VITE_ENABLE_FFMPEG_FALLBACK=false` - ✅ `yarn build:web:standalone` — succeeds; ffmpeg trio inline-base64'd, single-file output verified - ✅ `yarn test:e2e:standalone` + `yarn test:e2e:web` — MP4 strip through real built apps, 90s cap, sentinel-clean assertions - ✅ Forensic runner — synthetic + real-world fixtures (including 236 MB DJI) clean, zero sentinel + zero device-fingerprint survival - ✅ New CI `build-no-ffmpeg` job exercises the opt-out build path ## Known limitations (follow-ups, not blockers) 1. **True tree-shaking for `VITE_ENABLE_FFMPEG_FALLBACK=false`**: the strategy is statically imported in `strategy_registry.ts` (matches the existing `ExifToolFallbackStrategy` pattern), so the 30 MB `ffmpeg-core.wasm` ships in `dist/web/assets/` even when registration is skipped. Real tree-shaking would need dynamic imports — same limitation as the ExifTool fallback today. Worth a separate PR that improves both fallbacks. 2. **Real-world MKV/WebM fixtures**: the forensic runner covers synthetic-only for these formats today. Adding a MediaRecorder-generated WebM and a typical MKV download to `tests/fixtures/wasm/video/real-world/` is a worthwhile next step. 3. **Subsequent PRs from #182's "tracked, not this issue" list**: - Delete `VideoStrategy` after a validation window - Add `.avi` claim (closes #44) - Add `.wmv`, `.3gp` after per-format forensic verification - Streaming I/O for large files (`#34`)
forgejo_admin added 4 commits 2026-05-21 18:54:07 +00:00
POC results: 9.79 MB gzipped WASM, no-network verified (static + dynamic),
2-3 ms steady-state per file on small MP4, 50-100 ms on 2.7 MB phone-baseline.

Two surprising findings vs the prior plan:

1. Bundle is ~10 MB gzipped (not 30 MB). The 30 MB number was raw, not the
   wire/install weight. APK absorbs trivially; standalone HTML +44x vs today.

2. GoPro Fusion (and similar files with timecode/description tracks) work with
   `-map 0:v? -map 0:a?` even though `-map 0` fails. This drops all data
   streams, which is exactly what a privacy strip should do — removes the
   GPMF GPS, the device fingerprints, and the encoder string. Better than
   both the current walker (#38, #39 gaps) and mat2 (exit 234).

Privacy story is defense-in-depth rather than structural by construction —
ffmpeg-core.wasm has Emscripten SOCKFS code that's dormant (no
Module["websocket"] supplied), JS-side fetch only loads the WASM itself,
dynamic test under `node --permission` without --allow-net produces
byte-identical output. CSP `connect-src 'self'` is the deploy-layer backstop.

License: @ffmpeg/core is GPL-2.0-or-later; the combined distributable
inherits it. User accepted in #182.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Routes MP4-family containers through @ffmpeg/ffmpeg + @ffmpeg/core
(single-threaded, ~10 MB gzipped WASM) when VITE_ENABLE_FFMPEG_FALLBACK
is not "false" (default on for all three distributions). Registered
ahead of VideoStrategy in strategy_registry.ts so the ffmpeg path wins
for the claimed extensions.

The strip invocation drops every non-video/audio stream:

  -i in.<ext> -map 0:v? -map 0:a?
  -map_metadata -1 -map_chapters -1 -fflags +bitexact
  -c copy -movflags +faststart -metadata encoder=
  out.<ext>

This is the key departure from mat2's default `-map 0` — dropping data
streams (timecode, description, gpmd, subtitle) makes GoPro Fusion / DJI
files succeed where mat2 exits with code 234. It also removes the GPMF
GPS, device fingerprints, and handler/compressor name leaks the walker
left in (#38, #39, #111, #42).

VideoStrategy stays as the opt-out fallback (VITE_ENABLE_FFMPEG_FALLBACK=
false) and during the validation window before being retired in a
subsequent PR per #182's sequencing.

Verification:
- 504/504 unit tests pass (+6 new for the strategy + registry gating)
- yarn typecheck, yarn lint, yarn check:deps all clean
- yarn build:web + yarn build:web:standalone both succeed
- Privacy + functional results documented in docs/poc/ffmpeg-wasm.md
- Per-source policy documented in docs/gap-analysis/mp4-ffmpeg.md

Phase 2 (.mkv + .webm) and the forensic runner follow in subsequent
commits on this branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 2 extends the strategy's claim set to .mkv + .webm. Container
detection is polymorphic — ISOBMFF MP4-family ftyp box OR EBML magic
(0x1A 0x45 0xDF 0xA3). detectContainer() distinguishes WebM from MKV
via the EBML DocType element (looks for the ASCII "webm" in the first
64 bytes; falls back to MKV which is the superset).

The strip invocation drops -movflags +faststart for matroska containers
(MP4-only flag; matroska muxer ignores it with a noisy warning).

Forensic runner (tools/forensic/ffmpeg-fallback.ts) now covers:
  - synthetic-mp4    (5647 → 2238, 5 sentinels → 0)
  - synthetic-mkv    (1991 → 1761, 4 sentinels → 0)
  - synthetic-webm   (1190 → 991,  4 sentinels → 0)
  - phone-baseline   (2.7 MB samplelib Android)
  - gopro-fusion     (5.1 MB, 7 device-fingerprints → 0)

All five pass. KNOWN_GAPS empty — strategy is now forensically clean
across every fixture we test.

MKV seeding uses ffmpeg's -metadata directly because exiftool refuses
to write to MKV/WebM ("file format not supported for writing").

docs/gap-analysis/{mkv,webm}.md document the per-source policy.
docs/forensic/ffmpeg-fallback.md captures empirical results + recovery
battery methodology.

Quality gates: yarn typecheck + yarn lint + yarn test (505 passed) +
yarn check:deps all clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ci(ffmpeg): smoke build with VITE_ENABLE_FFMPEG_FALLBACK=false (#182)
Some checks failed
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 25s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 41s
CI / E2E (Standalone single-file) (pull_request) Failing after 4m58s
CI / E2E (Web) (pull_request) Successful in 6m50s
9d60f37237
Adds a CI job that builds yarn build:web + yarn build:web:standalone
with the opt-out env var set, so the strategy-chain-without-ffmpeg
path stays buildable. Doesn't run unit tests in this job (the `test`
job already covers the always-built strategy + registry gating tests
via vi.stubEnv); just confirms the bundle produces.

README "Third-party engines and license notices" section documents:
- Both fallback engines (ExifTool-via-zeroperl + ffmpeg-wasm)
- Build flags that control inclusion
- License chain — MIT for our wrappers, GPL-2.0-or-later for the
  ffmpeg WASM and the implications for combined distributables
- Source-availability pointer (the upstreams are fully open + pinned
  in package.json)

This closes the license-attribution checkbox from #182. The "About /
Settings → Licenses" in-app screen mentioned in the POC writeup is a
follow-up — the README attribution is the source of record for GPL
compliance; an in-app pointer is a UX nicety.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 04:39:35 +00:00
perf(standalone): inline ffmpeg trio in <script type=text/plain> (#182)
All checks were successful
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 29s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 41s
CI / E2E (Web) (pull_request) Successful in 3m15s
CI / E2E (Standalone single-file) (pull_request) Successful in 3m16s
2e4b8dd608
Closes the standalone-single-file gap. The CI e2e-standalone failure on
run #226 was the same root cause: viteSingleFile only inlines JS/CSS
chunks, so ffmpeg-core.wasm (30.7 MB), ffmpeg-core.js (109 KB), and
the @ffmpeg/ffmpeg worker (5 KB) shipped as sibling files in
dist/web-standalone/. Under file:// CORS rules those siblings 404,
breaking every MP4/MOV/MKV/WebM strip path on standalone.

Two new plugins mirror the existing zeroperl pattern:

- standaloneFfmpegStubPlugin (enforce: "pre"): intercepts the three
  @ffmpeg ?url imports and replaces with sentinel strings so Vite
  doesn't emit them as assets.
- standaloneFfmpegInlinePlugin (closeBundle): reads the three asset
  files directly from node_modules and stashes them as
  <script type="text/plain" id="ffmpeg-{core-js,core-wasm,worker-js}-base64">
  tags before the module script. Also cleans up the dead worker-*.js
  sibling that Vite emits from @ffmpeg/ffmpeg's hardcoded
  `new Worker(new URL("./worker.js", import.meta.url))` static-discovery
  pattern (runtime uses our classWorkerURL instead).

ffmpeg_wasm_fetch.ts now:
- Builds a Blob URL for the worker (data: URLs don't reliably work with
  type=module workers across browsers; Blob URLs do).
- Passes classWorkerURL to ffmpeg.load() so the outer Web Worker uses
  our inlined bytes instead of the auto-discovered sibling.

Verification:
- yarn build:web:standalone → dist/web-standalone/ now contains
  exactly one file (index.html, 74.9 MB).
- yarn test:e2e:standalone → 7/7 pass (was 0/7 in CI run #226).
- yarn typecheck, yarn lint, yarn test (505 passed), forensic battery
  (5/5 clean) all green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 05:34:32 +00:00
test(forensic): add DJI Phantom 4 (236 MB) to ffmpeg-fallback runner (#182)
All checks were successful
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 28s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 43s
CI / E2E (Standalone single-file) (pull_request) Successful in 1m45s
CI / E2E (Web) (pull_request) Successful in 3m10s
eae2dbe9f8
Empirically closes the "drone coverage is predicted, not measured" gap I
flagged earlier. The 236 MB DJI Phantom 4 fixture (Zenodo record
3604005, fetched via --include-large) now runs through the strategy and
verifies:

  - FC6310 (drone model in UserData) — removed
  - AVC encoder (compressorname in sample entry) — removed
  - DJI.AVC (video handler description) — removed
  - DJI.Meta (metadata-track handler description) — removed
  - Full GPS flight log under [UserData] GPSCoordinates
    (55°N 10°E 10.8m, ~Denmark) — completely gone

Performance: 236 MB strip completes in ~1.0 s wall time, peak RSS
~1.2 GB (~5× input). Full battery (6 fixtures including DJI) runs in
4.2 s. Sets a real ceiling estimate for mobile WebView: ~250 MB on
tighter-memory devices (same constraint as the walker today; #34 is
the proper home for streaming I/O).

Gap-analysis prose updated to match what's actually measured: phone,
action cam, and drone are now empirically verified; dashcam coverage
remains predicted by analogy (no fixture yet — follow-up).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 06:05:03 +00:00
fix(standalone): main-thread @ffmpeg/core, drop @ffmpeg/ffmpeg wrapper (#182)
All checks were successful
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 28s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 43s
CI / E2E (Standalone single-file) (pull_request) Successful in 5m13s
CI / E2E (Web) (pull_request) Successful in 7m3s
9f87ff6bf6
The user reported the standalone HTML build hanging indefinitely when
stripping an 18 MB MP4. Reproduced in headless Chromium with a 2.7 MB
fixture: the @ffmpeg/ffmpeg wrapper spawns `type: "module"` Workers
from Blob URLs, which fail silently when the page origin is `null`
(file://). The error event fires with all fields cross-origin-censored
(message/filename/lineno all empty), and the wrapper's load() promise
never resolves — UI hangs forever.

Minimal repro confirms it's a Chromium null-origin restriction, not
specific to ffmpeg. Classic Workers from Blob URLs work fine; module
Workers from Blob URLs uniformly fail under null origin. The wrapper
hardcodes `type: "module"` so there's no workaround inside the
wrapper itself.

Switching FfmpegFallbackStrategy to use @ffmpeg/core directly in the
main thread:

  - No Worker, no Blob URL, no null-origin restriction.
  - Same code path everywhere (standalone / PWA / Capacitor APK) —
    no env-conditional behavior.
  - Matches ExifToolFallbackStrategy + JpegStrategy / PdfStrategy
    architecture: all run in the main thread.
  - Drops @ffmpeg/ffmpeg + @ffmpeg/util from package.json (10 KB JS
    each, no longer used).

Trade-off: blocks the main thread during exec(). Empirically:
  - Phone-baseline MP4 (2.7 MB) via standalone HTML end-to-end: 5.8 s
    (includes cold WASM init).
  - Synthetic + gopro-fusion + dji-phantom4: per-file numbers
    unchanged from the prior Node-side POC.
  - DJI Phantom 4 (236 MB) strip time in Node: ~1.0 s (CPU-bound,
    main-thread blocking would be the same in browser).

Acceptable: the strategy class itself ran in the main thread before
(only the engine ran in the Worker). And the strip is short enough
that a brief unresponsive UI matches users' expectation of "metadata
stripper, processing now".

Other changes:
  - Removed standaloneFfmpegStubPlugin's worker-URL stub (no longer
    a `?url` import to intercept).
  - Removed worker.js inlining from standaloneFfmpegInlinePlugin.
    The Vite-emitted sibling worker file is dead; we no longer
    delete it because we no longer trigger Vite's
    `new Worker(new URL("./worker.js", import.meta.url))` static
    discovery (no longer import @ffmpeg/ffmpeg).
  - Added an ambient declaration for @ffmpeg/core (no @types package).

Verification:
  - yarn typecheck, yarn lint, yarn test (505 passed), yarn check:deps
  - yarn build:web + yarn build:web:standalone both succeed
  - Standalone HTML: end-to-end MP4 strip via DataTransfer drop works
    in 5.8 s on phone-baseline, produces a 2.8 MB stripped output with
    Lavf fingerprint removed and timestamps zeroed
  - Forensic battery: all 6 fixtures (3 synthetic + 3 real-world,
    including DJI Phantom 4 236 MB) clean

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 06:21:51 +00:00
test(e2e): MP4 strip through standalone + web — would have caught the Worker bug (#182)
All checks were successful
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 30s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 41s
CI / E2E (Standalone single-file) (pull_request) Successful in 2m4s
CI / E2E (Web) (pull_request) Successful in 3m2s
eb65f13cd5
Two new Playwright tests exercise the FfmpegFallbackStrategy through
the real built apps:

  - tests/e2e/standalone/standalone.spec.ts — drops sample-real.mp4
    into the dist/web-standalone/index.html via file:// and asserts
    the download completes + is sentinel-clean. This is the
    regression test for the Blob-URL/type:module Worker hang fixed
    in 9f87ff6. With the old implementation it would have hung
    forever; the test now caps at 90s and waits for completion.

  - tests/e2e/web/file-processing.spec.ts — drops sample-real.mp4
    through the dev-server-served PWA. Different asset-resolution
    branch (Vite ?url emission vs inline-base64), so this complements
    the standalone test rather than duplicating it.

New fixture tests/e2e/fixtures/sample-real.mp4 (5.3 KB) is a real
ffmpeg-encoded 1s blue frame seeded with Title="Test Video" and
Author="Test Author" sentinels. The existing sample.mp4 was
adversarial (exiftool-synthesized with non-aligned sample tables;
ffmpeg's -codec copy refuses it — that's the same failure mode
documented for the synthetic fixture in docs/forensic/video.md).

New assertVideoStripped() helper in tests/e2e/web/helpers/
metadata_assertions.ts checks for:
  - No Lavf<version> encoder fingerprint
  - No "Test Author" / "Test Video" sentinels from the fixture
  - No Adobe XMP namespace marker from the seeded uuid box
  - No Image::ExifTool authorship string

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 06:34:17 +00:00
fix(ffmpeg): strip muxer-added fields ffmpeg leaks after -map_metadata -1 (#182)
All checks were successful
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 34s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 56s
CI / E2E (Standalone single-file) (pull_request) Successful in 2m39s
CI / E2E (Web) (pull_request) Successful in 3m57s
0f719b8d0a
User reported that the stripped output gained fields that weren't in the
input:
  - HandlerVendorID: Apple
  - BufferSize: 0
  - MaxBitrate / AverageBitrate (per-track values)

A reviewer suggested `-metadata:s:v vendor_id= -metadata:s:v handler_name=`
etc. Empirically verified this doesn't address it: those flags target the
per-stream trak/mdia/hdlr fields (which ffmpeg writes as zeros anyway in
our config). The `HandlerVendorID: Apple` exiftool reports actually comes
from a different location — the movie-level moov/udta/meta/hdlr block
that ffmpeg's mov_write_meta_tag emits unconditionally with handler_type
"mdir" + vendor "appl", regardless of -map_metadata -1.

Two-layer fix:

  ffmpeg muxer flags handle what flags can:
    -write_btrt 0         — suppresses btrt boxes (kills BufferSize/
                            MaxBitrate/AverageBitrate)
    -write_tmcd 0         — suppresses timecode atom (defensive; we drop
                            tmcd streams via -map anyway)
    -empty_hdlr_name true — zeros per-track hdlr.name (kills VideoHandler/
                            SoundHandler)
    -metadata:s:{v,a} vendor_id= handler_name= — defense-in-depth for
                            per-stream tags (adopting the reviewer's
                            suggestion alongside the above)

  Post-strip pass handles what flags can't:
    ffmpeg_post_strip.ts walks moov/udta/meta/hdlr and zeros the 4-byte
    vendor field at payload offset +12 (after FullBox header + pre_defined
    + handler_type). Length-preserving — all byte offsets elsewhere stay
    valid. No-op for matroska containers.

End-to-end verification on phone-baseline.mp4 via standalone HTML:
  - Before: HandlerVendorID=Apple, BufferSize=0, MaxBitrate=4486713,
            AverageBitrate=4486713 (×2 for audio track)
  - After:  zero hits across raw `strings | grep -ciE
            'appl|btrt|Lavf|VideoHandler|SoundHandler'`
  - exiftool view shows nothing under HandlerVendorID, HandlerDescription,
    BufferSize, MaxBitrate

assertVideoStripped() in tests/e2e/web/helpers/metadata_assertions.ts
strengthened with checks for `mdirappl`, `btrt`, `VideoHandler`,
`SoundHandler` so regressions on this class of leak fail the e2e suite
instead of slipping through CI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin force-pushed feat/ffmpeg-poc from 0f719b8d0a to 5269e7cf67 2026-05-22 06:34:29 +00:00 Compare
forgejo_admin added 2 commits 2026-05-22 06:54:25 +00:00
- README.md: drop @ffmpeg/ffmpeg from the "Third-party engines" row;
    only @ffmpeg/core ships now (wrapper was removed in 9f87ff6).
  - tests/infrastructure/wasm/ffmpeg_fallback_strategy.test.ts: fix
    stale header comment that still claimed the strategy uses the
    @ffmpeg/ffmpeg Web-Worker wrapper.
  - src/.../ffmpeg_post_strip.ts: fix file-header offset claim (was
    "+8", actually +12 — the off-by-4 the code already had right but
    the comment didn't). Refactor cleanFfmpegMp4Output to mutate in
    place instead of allocating a defensive copy; the only caller
    already owns the buffer (fresh allocation from MEMFS). Saves ~58 ms
    + 236 MB transient RSS on the DJI Phantom 4 strip (measured).
  - tests/infrastructure/wasm/ffmpeg_post_strip.test.ts: new file —
    5 unit tests covering happy-path zeroing, per-track udta nesting,
    no-op when structure absent, malformed-meta graceful handling,
    and multiple-hdlr defensive coverage. Off-by-N regressions in the
    box walker will now fail at the Vitest level rather than only at
    the forensic-runner level.
  - docs/gap-analysis/mkv.md: document the matroska muxer fingerprint
    audit gap. cleanFfmpegMp4Output is MP4-only; ffmpeg's matroska
    muxer writes MuxingApp/WritingApp/SegmentUID that aren't audited
    or stripped by this PR. Suggested follow-up: extend
    assertVideoStripped's matroska branch + add cleanFfmpegMatroskaOutput
    if leaks are found.

Verified: yarn typecheck, yarn lint, yarn test (510 passed — +5 from
new ffmpeg_post_strip tests), yarn check:deps, forensic battery 6/6
clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
perf(standalone): gzip inlined wasm payloads — 116 MB HTML → 65 MB (#182)
All checks were successful
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 31s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 48s
CI / E2E (Standalone single-file) (pull_request) Successful in 2m0s
CI / E2E (Web) (pull_request) Successful in 3m23s
3622eda32a
User reported cold-start time regressed when ffmpeg landed (same shape
as the exiftool inline-payload regression that was previously fixed by
moving to <script type="text/plain">). The remaining cost was the HTML
parser allocating ~150 MB of UTF-16 text nodes for the inlined base64
blobs (zeroperl 33 MB base64 + ffmpeg core+wasm 42 MB base64). That's
unavoidable in proportion to text-node size, so we shrink the text-node.

Vite plugins (vite.config.web.standalone.ts):
  - standaloneWasmInlinePlugin: gzipSync(wasmBytes, {level:9}) before
    base64. zeroperl.wasm 25.3 MB → 7.6 MB gz → 10.1 MB base64.
  - standaloneFfmpegInlinePlugin: same — ffmpeg-core.{js,wasm}
    32.3 MB → 10.3 MB gz combined.

Runtime decoders:
  - exiftool_wasm_fetch.ts redirectWasmFetch: pipe the bytes through
    DecompressionStream("gzip") before handing to instantiateStreaming.
  - ffmpeg_wasm_fetch.ts readInlinedCore: same pattern, both core JS and
    wasm decoded in parallel via Promise.all. New helper
    base64GunzipToBytes uses fetch(data:URL) for the base64 decode
    (native, faster than atob+charCodeAt loop on multi-MB inputs)
    followed by DecompressionStream for the gunzip (~30 ms one-shot
    at first strip, not at page load — pure win).

DecompressionStream is supported in Chrome 80+ / Firefox 113+ /
Safari 16.4+ / Capacitor Chromium WebView. All standalone + APK
targets are covered.

Measured impact (`dist/web-standalone/index.html`):

  before: 116 MB raw / 40 MB gz on the wire (servable wire-gzip)
  after:   65 MB raw / 21 MB gz on the wire

E2E impact (`yarn test:e2e:standalone` running the MP4 strip):

  before: 10.4 s wall (cold load + cold WASM init + strip + download)
  after:   7.8 s wall  (-2.6 s; HTML parse is the dominant savings)

Verified: yarn typecheck, yarn lint, yarn test (510 passed),
yarn check:deps. Full standalone e2e suite (8 tests, both JPEG +
PDF + MP4 paths) 8/8 green. Forensic runner 6/6 clean (unchanged —
the strategy bytes are identical; only the transport changed).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
Owner

Code review

Found 1 issue:

  1. standaloneWasmInlinePlugin and standaloneFfmpegInlinePlugin both have closeBundle hooks that read + write dist/web-standalone/index.html. Rollup runs closeBundle via hookParallel, so the two plugins are scheduled concurrently — today the build happens to produce correct output only because both hook bodies are fully synchronous (all fs operations are *Sync, so JavaScript run-to-completion serialises them in practice). Plugin-array order doesn't enforce sequencing on its own, so the ordering is implicit. If anyone later adds an await inside either closeBundle (e.g. async hashing, a network call, or async fs APIs), the second plugin can read stale HTML mid-mutation and clobber the first plugin's injection. Worth adding an explicit enforce: "post" / order: "post" to the ffmpeg plugin so the contract is documented in code, or merging the two into a single plugin with a deterministic sequence.

vite.config.web.standalone.ts L243-L300standaloneWasmInlinePlugin.closeBundle

vite.config.web.standalone.ts L370-L420standaloneFfmpegInlinePlugin.closeBundle

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

### Code review Found 1 issue: 1. `standaloneWasmInlinePlugin` and `standaloneFfmpegInlinePlugin` both have `closeBundle` hooks that read + write `dist/web-standalone/index.html`. Rollup runs `closeBundle` via `hookParallel`, so the two plugins are scheduled concurrently — today the build happens to produce correct output only because both hook bodies are fully synchronous (all fs operations are `*Sync`, so JavaScript run-to-completion serialises them in practice). Plugin-array order doesn't enforce sequencing on its own, so the ordering is implicit. If anyone later adds an `await` inside either `closeBundle` (e.g. async hashing, a network call, or async fs APIs), the second plugin can read stale HTML mid-mutation and clobber the first plugin's injection. Worth adding an explicit `enforce: "post"` / `order: "post"` to the ffmpeg plugin so the contract is documented in code, or merging the two into a single plugin with a deterministic sequence. [vite.config.web.standalone.ts L243-L300](/forgejo_admin/exifcleaner-web/src/commit/3622eda32a6b151a80ba2c4a5ff98712e0d8a897/vite.config.web.standalone.ts#L243-L300) — `standaloneWasmInlinePlugin.closeBundle` [vite.config.web.standalone.ts L370-L420](/forgejo_admin/exifcleaner-web/src/commit/3622eda32a6b151a80ba2c4a5ff98712e0d8a897/vite.config.web.standalone.ts#L370-L420) — `standaloneFfmpegInlinePlugin.closeBundle` 🤖 Generated with [Claude Code](https://claude.ai/code) <sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub>
forgejo_admin added 1 commit 2026-05-22 07:50:53 +00:00
perf(ffmpeg): pre-warm @ffmpeg/core in requestIdleCallback (#182)
All checks were successful
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 29s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 44s
CI / E2E (Standalone single-file) (pull_request) Successful in 1m40s
CI / E2E (Web) (pull_request) Successful in 3m15s
b327523de8
User reported standalone cold-start still 12-16 s after the gzip win.
Profiling the standalone HTML cold-load + 2.7 MB MP4 strip:

  step                      before prewarm   after prewarm
  ------------------------  ---------------  ---------------
  HTML loaded (parse + JS)  ~1.4 s           ~1.4 s
  UI ready (drop zone)      ~1.5 s           ~1.5 s
  strip phase (drop→done)   ~5.5 s           ~0.2 s
  total                     ~7.0 s           ~1.7 s

The cold-init (gunzip 30 MB wasm + WebAssembly.instantiate) was on the
critical "drop→done" path — every first-drop paid ~3-5 s of WASM init
before ffmpeg could run. With the prewarm, the same work happens in
the background between page-load and first-drop; by the time the user
drops a file the engine is already loaded.

Mechanism: the FfmpegFallbackStrategy constructor schedules
`getInstance()` (the existing cached-load entry point) via
requestIdleCallback. requestIdleCallback runs the callback when the
browser is idle (after first paint + initial React mount), so this
doesn't compete with critical-path startup work. setTimeout fallback
covers Safari (no requestIdleCallback) — irrelevant for the standalone
target since we're Chromium-only, but cheap to include.

Errors from the prewarm load are swallowed: strip() retries on demand
and surfaces the error through Result<_, ExifError>. The prewarm
exists only to overlap latency, not to gate functionality.

Verified:
  - typecheck, lint, test (510 passing), check:deps
  - yarn test:e2e:standalone --grep "strips an MP4" → 7.8 s wall
    (matches pre-prewarm — the e2e doesn't simulate user-think time,
    so it can't observe the prewarm win directly. Real-user impact is
    the ~5 s shaved off cold "drop→done")
  - yarn test:e2e:web:desktop → 6.9 s (unchanged)
  - forensic battery 6/6 clean

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 08:18:36 +00:00
chore(ffmpeg): address review findings + tree-shake 43 MB from standalone (#183)
All checks were successful
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 29s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 41s
CI / E2E (Standalone single-file) (pull_request) Successful in 1m32s
CI / E2E (Web) (pull_request) Successful in 3m11s
709d37afec
Six review findings scored >35 confidence in the second-round review.
Bundled into one logical commit since they all touch closely-related
code (the FfmpegFallbackStrategy + its asset-loading + the standalone
Vite plugins). Each fix is independently sound; together they also
unblock a major start-time win.

  1. CLAUDE.md: production dep count is now 6, not 4. Documents
     @ffmpeg/core (#182) and @uswriting/exiftool (#174) alongside the
     existing four, with one-line justification per dep. The
     "Dependencies" code-conventions bullet is rephrased from "Four
     production deps is the current ceiling" to "Current count is 6
     production deps; new deps need explicit justification".

  2. ffmpeg_fallback_strategy.ts: replaces the legacy `exiftool-error`
     ExifError variant with the correct codes per
     typescript-conventions.md ("new strategies typically only return
     invalid-file-format, file-io-error, or parse-failed"). Non-zero
     ffmpeg exit → `parse-failed` (with `raw` per the variant shape).
     Catch-all exception → `file-io-error` (with `detail`). User-
     facing messages no longer say "ExifTool error:" for ffmpeg
     failures.

  3. ffmpeg_fallback_strategy.ts: `getInstance()` now resets
     `this.loadPromise = null` on rejection. Without this, a transient
     load failure (network blip, gunzip error, dynamic import failure)
     would permanently brick the strategy for the page session — every
     subsequent strip() call returned the same cached rejection. With
     the reset, the next strip() triggers a fresh retry. The
     constructor's requestIdleCallback prewarm interacts cleanly:
     prewarm failure still gets swallowed, but the next user strip
     now retries instead of re-surfacing the rejection.

  4. ffmpeg_fallback_strategy.ts: strip() wraps its body in
     try/finally so MEMFS input + output get unlinked on every exit
     path, including the previously-leaky case where `core.FS.readFile`
     throws (zero-byte output, MEMFS corruption, OOM). Without the
     finally, the user's video bytes would persist in WASM linear
     memory until the next strip's pre-call cleanup overwrote them —
     a real privacy concern on a strategy where input bytes are the
     sensitive payload.

  5. vite.config.web.standalone.ts: merges standaloneWasmInlinePlugin
     + standaloneFfmpegInlinePlugin into a single
     standaloneInlineWasmsPlugin. The two were race-prone — both had
     closeBundle hooks that read+wrote the same `index.html`, and
     Rollup runs closeBundle via hookParallel. Today they happened to
     serialise because both hook bodies were fully synchronous, but
     any future `await` inside either would let the second plugin
     read stale HTML and clobber the first plugin's injection. The
     merge does one read + iterates an INLINE_ASSETS array
     (zeroperl.wasm, ffmpeg-core.js, ffmpeg-core.wasm) + one write,
     eliminating the implicit dependency. Closes review comment #1347.

  6. ffmpeg_wasm_fetch.ts + vite.config.web.{ts,standalone.ts} +
     ffmpeg_core.d.ts: tree-shake the bare `import("@ffmpeg/core")`
     out of the standalone bundle. The PWA-path branch of
     resolveCore() was unreachable at runtime in the standalone build
     (readInlinedCore() returns first), but Vite was statically
     bundling the entire factory + its data:URL wasm fallback —
     ~43 MB of dead code. A new build-time flag
     `__WITH_STANDALONE_INLINE__` (declared in ffmpeg_core.d.ts, set
     to "true" via Vite's `define` in the standalone config, "false"
     in the PWA config) gates the bare import behind a throw, letting
     Rollup eliminate the unreachable branch.

Measured impact of (6):

  standalone HTML size:     65.3 MB → 24.3 MB  (-63%)
  headless drop-zone visible: 1.5 s →  0.55 s  (-65%)

The user reported "10 s to open the HTML and show the buttons to
choose files and directories". The 43 MB of dead code in the HTML was
forcing the browser to parse and allocate text nodes for it before
any UI could appear. With the dead code tree-shaken, real-browser
cold-open should drop proportionally — roughly 2-3 s on the same
hardware.

Verification (worktree state):
  yarn typecheck, yarn lint, yarn test (510 passing), yarn check:deps
  yarn build:web (PWA bundle still contains ffmpeg-core factory —
    confirmed the bare import IS reached in PWA at build)
  yarn build:web:standalone (24.3 MB index.html, one file)
  yarn test:e2e:standalone --grep "strips an MP4" → 6.8 s
  yarn test:e2e:web:desktop --grep "strips metadata from an MP4" → 6.2 s
  npx tsx tools/forensic/ffmpeg-fallback.ts → 6/6 fixtures clean

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 09:01:32 +00:00
fix(diff): preserve per-track identity in MP4 source label (#183)
All checks were successful
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 29s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 44s
CI / E2E (Standalone single-file) (pull_request) Successful in 1m35s
CI / E2E (Web) (pull_request) Successful in 3m15s
2d9203954e
User reported the diff view showing "HandlerType: Video Track → Audio
Track" on `file_example_MP4_1920_18MG.mp4`, even though the actual
stripped file correctly preserves Track 1 = Video and Track 2 = Audio.
Verified by stripping the file via the standalone HTML and dumping
the output with `exiftool -G1 -s` — both HandlerType rows are
byte-correct.

Root cause: ExifTool's family-1 group prefixes per-track fields with
`Track1:`, `Track2:`, ... etc. — each track repeats the same tag
names (HandlerType, TrackID, ImageWidth, MediaTimeScale, etc.).
`mapGroupToSource` was collapsing every Track* group to a single
"MP4" source, which destroyed per-track identity. When the diff
renderer joined entries by (source, name), two tracks' HandlerType
rows collided on the same key and the renderer mis-aligned
Track1-in-input with Track2-in-output — producing the spurious
"Video Track → Audio Track" red→green diff in the UI.

Same misalignment affects every per-track tag: TrackID
(showing 1 → 2), TrackCreateDate, ImageWidth/Height, MediaTimeScale,
HandlerDescription, and any other field ExifTool emits per-track.

Fix: preserve the track number in the source label. `Track1` →
`"MP4 Track 1"`, `Track2` → `"MP4 Track 2"`, ... — each tag now has
a unique (source, name) key per track, so the diff renderer aligns
Track1-input with Track1-output and Track2-input with Track2-output
as intended.

Verified: yarn typecheck, yarn lint, yarn test (510 passing including
the 5 existing exiftool_diff_strategy tests).

Known broader pattern: the same shape of bug could theoretically
apply to any other group collapse where ExifTool emits the same tag
name in multiple sub-groups (e.g. IFD0/IFD1/ExifIFD all → "EXIF",
or XMP-dc/XMP-xmp both → "XMP"). In practice tag names are usually
disjoint across IFDs and XMP namespaces, so this hasn't surfaced.
A regression test covering multi-track inputs would catch any future
recurrence; deferred to a follow-up since the test scaffolding needs
to mock the WASM (the existing 5 tests use the real engine on
real fixtures with single-track or non-tracked content).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 09:14:48 +00:00
fix(diff): surface raw ExifTool group names — never collapse sub-groups (#183)
Some checks failed
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 29s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 46s
CI / E2E (Standalone single-file) (pull_request) Failing after 1m55s
CI / E2E (Web) (pull_request) Failing after 5m25s
1c3ced5e40
Generalises the per-track fix from 2d92039. The previous code collapsed
ExifTool family-1 groups via a lookup table + prefix matching:

  IFD0 / IFD1 / ExifIFD / SubIFD / InteropIFD → "EXIF"
  QuickTime / ItemList / UserData / Keys      → "MP4"
  XMP-dc / XMP-xmp / XMP-pdf / ...            → "XMP"
  ICC_Profile / ICC-header / ICC-view / ...   → "ICC"
  PNG / PNG-pHYs / ...                        → "PNG"
  Track1, Track2, ...                         → "MP4" (and post-2d92039,
                                                  "MP4 Track 1", "MP4 Track 2", ...)

The collapse was for UX cleanliness, but it makes the diff renderer's
(source, name) key non-unique whenever the same tag name appears in two
sub-groups. The Track* case was the loud one because tracks naturally
repeat the same schema (HandlerType, TrackID, ImageWidth, etc.). The
other collapses are quiet — empirical scans against
file_example_MP4_1920_18MG.mp4, phone-baseline.mp4, gopro-fusion.mp4,
dji-phantom4.mov, and real-world JPEGs found zero current collisions —
but the pattern is theoretical-only-until-someone-files-a-bug.

Generalised fix: remove the collapse table entirely. Surface every
family-1 group name verbatim as the source label. The diff UI now
shows "IFD0" / "ExifIFD" / "XMP-dc" / "QuickTime" / "ItemList" /
"Track1" / "Track2" / etc. directly. Any future tag-name collision
across sub-groups is now structurally impossible.

Trade: user-visible labels in the diff UI become more technical /
granular. For this audience (privacy-focused users asking "what was
removed from my file") more precision is appropriate; the granular
labels are also more informative than smoothed-over umbrella terms.

The test that previously asserted "IFD0 must not leak through" is
flipped to assert "IFD0 must surface verbatim". The 124 other places
in tests/web/ that synthesize MetadataEntry with `source: "EXIF"` /
`source: "MP4"` / etc. are unaffected — they're testing UI components
against fake data, not against live diff output.

Walker-produced sources (Office / PDF / JPEG / PNG / Video strategies)
already use granular per-walker labels ("Office Core", "Office App",
"PDF Annotations", "PDF XMP", "Office Custom") so the walker path
needs no change.

Verified: yarn typecheck, yarn lint, yarn test (510 passing),
yarn check:deps, build:web:standalone, both e2e MP4 tests, forensic
battery 6/6 clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 09:22:33 +00:00
fix(ffmpeg): preserve input track order during strip (#183)
Some checks failed
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 29s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 44s
CI / E2E (Standalone single-file) (pull_request) Failing after 2m13s
CI / E2E (Web) (pull_request) Failing after 3m32s
0fd55fc235
User reported the diff view showing video and audio tracks "swapped"
on a test file. Investigation: the file has audio in input stream 0
and video in input stream 1 (some encoders write that order); our
strip's `-map 0:v? -map 0:a?` invocation processes -map args in
argument order, so ffmpeg lands video at output position 0 and audio
at output position 1, swapping the track numbering.

The diff view now correctly aligns per-track entries by (Track1,
Track1), (Track2, Track2), etc. (after 1c3ced5's generalised
sub-group fix), so input Track1 (audio) gets compared to output
Track1 (which is now video) — diff renders dozens of spurious "this
was added"/"this was removed" rows. The actual content is identical:
the audio frames are still there in the output, just at Track2
instead of Track1.

Replace the explicit type-order mapping with a negative-selector
form that preserves input order:

  -map 0 -map -0:d? -map -0:s? -map -0:t?

  -map 0           map every stream from input 0 (default order)
  -map -0:d?       then unmap data streams (gpmd, fdsc, gps0, ...)
                   — optional, no error if absent
  -map -0:s?       then unmap subtitle streams
  -map -0:t?       then unmap attachment/timecode streams (tmcd)

Track order among the surviving video+audio streams matches input.
Empirically verified on:

  audio-first synthetic   (audio→Track1, video→Track2 preserved)
  phone-baseline.mp4      (video→Track1, audio→Track2 preserved)
  gopro-fusion.mp4        (video+audio kept, gpmd/tmcd/fdsc dropped,
                           rc=0 — same coverage as before)
  dji-phantom4.mov        (video kept, data streams dropped, rc=0)

The diff view will now show truly minimal changes for the user's
file — only the actual metadata removals (timestamps zeroed,
handler descriptions removed, encoder strings suppressed, etc.).
HandlerType per track now stays Video Track ↔ Video Track and
Audio Track ↔ Audio Track because both input and output have
the same content at the same track index.

Same change applied to tools/forensic/ffmpeg-fallback.ts so the
forensic runner mirrors the strategy exactly. Both gap-analysis
and forensic docs updated.

Verified: yarn typecheck, yarn lint, yarn test (510 passing),
yarn check:deps, build:web:standalone, both e2e MP4 tests,
forensic battery 6/6 clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 09:42:44 +00:00
fix(ffmpeg): serialize diff reads + zero mdhd.language post-strip (#183)
Some checks failed
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 30s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 43s
CI / E2E (Standalone single-file) (pull_request) Failing after 1m56s
CI / E2E (Web) (pull_request) Failing after 3m42s
8fece70ef5
Two bugs that surfaced when users dropped multiple MP4 files:

**Multi-file diff race.** `WasmProcessor.buildDiffDocumentForEntry`
called `Promise.all([readBefore, readAfter])`. `@uswriting/exiftool`'s
`parseMetadata` uses module-level singletons for the Perl interpreter,
the MemoryFileSystem, and the stdout/stderr StringBuilders — every call
does `c.clear(), m.clear(), await e.reset()` on that shared state.
Running before+after concurrently raced on the shared buffer; the cold-
start serialized the first file's pair (file 1 looked fine) but every
subsequent file's pair hit the warm race and came back with both reads
returning the same parroted buffer contents → diff renderer saw no
changes → row showed "Already clean" with no diff. Serialize the two
reads.

**Mdhd.language leak.** ffmpeg's MP4 muxer always writes a 15-bit
packed ISO 639-2/T language code to `moov/trak/mdia/mdhd.language`
("und" 0x55C4 when input had no language, or copies the input's
code). ExifTool surfaces this as `MediaLanguageCode` — either as an
added row (input had no surfaceable language) or as a misleading
"eng → und" change. Extend `cleanFfmpegMp4Output` post-strip pass to
zero the 2-byte language window for both v0 and v1 mdhd boxes. ExifTool
no longer reports the field at all; ffprobe falls back to its default
"eng" display label for invalid codes (cosmetic, doesn't affect bytes
or playback).

Regression guard added to `WasmProcessor` tests — two distinct fixtures
(JPEG + PNG) processed sequentially, then diffs requested back-to-back;
verifies the JPEG's Make=TestCamera surfaces only in the JPEG diff and
the two before-lists are not identical. The test fails without the
serialize fix (verified by stashing the wasm_processor.ts change).

Post-strip mdhd zeroing covered by 3 new unit tests (v0 layout, v1
layout, multiple traks).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 09:50:24 +00:00
fix(ffmpeg): rewrite ffmpeg's udta stub to a free box, not just zero the vendor (#183)
Some checks failed
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 28s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 45s
CI / E2E (Standalone single-file) (pull_request) Failing after 2m7s
CI / E2E (Web) (pull_request) Failing after 3m45s
5842f905c7
ffmpeg's MP4 muxer always writes a 0x21-byte udta/meta/hdlr block at the
movie level (and per-track for some inputs) regardless of CLI options.
Previous post-strip pass zeroed just the 4-byte vendor at meta/hdlr +12,
which removed the `HandlerVendorID: Apple` row but left `HandlerType:
Metadata` still surfacing — a muxer-added field that wasn't in the input.

User feedback (#183): even though it's not a privacy leak, "I don't like
the idea of adding more fields after using ffmpeg."

Fix: rewrite the entire `udta` box type to `free` (ISO/IEC 14496-12
§8.1.2 padding) instead of editing its contents. Readers ignore `free`
boxes entirely — ExifTool stops surfacing every row that used to come
from the udta/meta/hdlr substructure. Length-preserving so `stco`/`co64`
offsets to `mdat` stay valid. Same approach handles both movie-level
`moov/udta` and per-track `moov/trak/udta`.

Empirical verification on phone-baseline.mp4:

  before (zero-vendor):
    [Track1] Media Language Code : und
    [Track1] Handler Type        : Video Track
    [Track1] Handler Description : VideoHandler
    [Track2] Media Language Code : und
    [Track2] Handler Type        : Audio Track
    [Track2] Handler Description : SoundHandler
    [Track*] Handler Type        : Metadata  ← muxer fingerprint
    [Track*] Handler Vendor ID   : (empty)   ← muxer fingerprint

  after (udta → free):
    [Track1] Handler Type        : Video Track
    [Track1] Handler Description : VideoHandler
    [Track2] Handler Type        : Audio Track
    [Track2] Handler Description : SoundHandler

The remaining HandlerType rows are legitimate — they describe what kind
of media each track carries (required by the spec, not muxer-added).
ffprobe still plays the file normally; the renamed `free` box is
just padding.

Tests rewritten to cover the new contract (udta → free instead of
vendor-zeroing). E2E `assertVideoStripped` invariant tightened to assert
no "udta" string survives in the output.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 09:57:33 +00:00
revert: keep ffmpeg's mdhd.language 'und' rather than zeroing it (#183)
Some checks failed
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 30s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 43s
CI / E2E (Standalone single-file) (pull_request) Failing after 1m52s
CI / E2E (Web) (pull_request) Failing after 3m42s
20e0639051
Reverts the mdhd.language zeroing portion of 8fece70. The udta → free
rewrite stays.

Why: 0x0000 is not a valid ISO 639-2/T code per ISO/IEC 14496-12
§8.4.2. ffprobe falls back to displaying `(eng)` for invalid codes,
which actively misleads downstream tooling that switches on language.
Other readers' behavior is implementation-defined.

`und` (0x55C4) is the spec's canonical "no language specified"
marker — every reader handles it predictably. When the input had a
real language (eng, fra, etc.), the resulting `eng → und` diff row
is honest: we removed the user's language tag, which is what the
diff is for. The only cosmetic cost is one extra `MediaLanguageCode:
und` row when the input had no decodable language — accepted vs.
spec-invalid container bytes.

Drops walkMdia + zeroMdhdLanguage + 3 mdhd test cases. Updates
the post-strip module docstring + gap-analysis to reflect the
"accept ffmpeg's und" policy.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 10:10:16 +00:00
review: address /code-review findings on the recent ffmpeg commits (#183)
Some checks failed
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 31s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 43s
CI / E2E (Standalone single-file) (pull_request) Failing after 1m54s
CI / E2E (Web) (pull_request) Failing after 3m42s
d4819a55d8
Five issues surfaced above the >35 confidence threshold:

1. **Cross-entry race on `@uswriting/exiftool` Perl singleton.** Commit
   8fece70 serialized before+after within a single
   `buildDiffDocumentForEntry`, but `use_process_files`'s mid-loop +
   end-of-batch `void drainDiffQueue(...)` are fire-and-forget; for
   batches > 50 files two drains can overlap and their inner
   `buildDiffDocumentForEntry` calls re-introduce the same race on the
   shared singleton. Fix at the choke point: add a `diffChain` Promise
   on `WasmProcessor` and queue every diff onto it. Any caller pattern
   (current hook, future direct invocation, tests) is now race-safe.

2. **`rewriteToFree` uses `payloadStart - 4`.** Correct for the regular
   8-byte box header but wrong for largesize headers (size=1, 16-byte
   header) — `payloadStart - 4` lands inside the uint64 largesize field
   rather than on the 4-byte type field. Switch to `headerStart + 4`,
   which is the universal location for the type field per ISO/IEC
   14496-12. Added a largesize regression test.

3. **Stale caller comment.** `ffmpeg_fallback_strategy.ts:217-224` still
   claimed `cleanFfmpegMp4Output` "zeros each track's mdhd.language" but
   20e0639 reverted that. Three independent review agents flagged it.
   Rewritten to match the actual contract (udta → free only) and to
   point at `ffmpeg_post_strip.ts` for the "why mdhd.language stays
   ffmpeg's und" rationale.

4. **Brittle e2e `not.toContain("udta")`.** A 4-byte ASCII substring
   collides ~6% per 250 MB mdat — previous `mdirappl` 8-byte was
   deliberately specific. Switch to the actual ffmpeg udta box-header
   signature (`00 00 00 21 75 64 74 61`) — 8 bytes specific enough to
   never false-positive on payload bytes while still catching every
   regression of the post-strip pass.

5. **Forensic doc not updated for udta → free.** Per
   format-strategy-workflow Phase 3, significant policy changes need
   a documented verification step. Added a "Post-strip rewrite of
   ffmpeg's udta stub" section to `docs/forensic/ffmpeg-fallback.md`
   covering both the udta rewrite and the "accept ffmpeg's und"
   decision, plus the pre/post exiftool verification for both.

Skipped: a test name still mentioning mdhd (cosmetic, fixed
incidentally); a claim that the multi-file regression test doesn't
exercise the within-entry race (the test's Make=TestCamera assertion
IS the regression guard — verified earlier by stashing the fix and
watching the assertion fail).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 10:48:24 +00:00
test(e2e): walk moov structurally instead of magic-byte udta scan (#183)
Some checks failed
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 30s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 43s
CI / E2E (Standalone single-file) (pull_request) Failing after 1m53s
CI / E2E (Web) (pull_request) Failing after 3m41s
6ffd09615f
Re-review surfaced a brittleness in the udta survival check: the
hardcoded byte signature `00 00 00 21 75 64 74 61` relied on 0x21 (33
dec) being the udta box's total size. That size is incidental to
ffmpeg's current output shape — different ffmpeg versions, per-track
udta on action-cam files with vendor atoms, or a future muxer change
that adds children would all produce a differently-sized udta and
silently pass the regression detector while real udta persists.

Replace with a small ISOBMFF box walker: find moov, look for udta as a
direct child, and walk each trak for per-track udta. Mirrors what
`cleanFfmpegMp4Output` itself rewrites, so the assertion stays in lock-
step with the post-strip pass policy. The walker is local to the test
helper (~80 LOC) rather than imported from `src/` — same adversarial-
independence rule the forensic runners follow, and keeps tests/ free
of cross-layer imports.

Verified by running both ffmpeg e2e suites (web-desktop + standalone)
to confirm the new check still passes on the actual stripped output.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin added 1 commit 2026-05-22 10:54:12 +00:00
test(e2e): fix stale 'EXIF' group assertion → IFD0 (#183)
All checks were successful
CI / Lint, Typecheck & Unit Tests (pull_request) Successful in 34s
CI / Smoke build (VITE_ENABLE_FFMPEG_FALLBACK=false) (pull_request) Successful in 48s
CI / E2E (Standalone single-file) (pull_request) Successful in 1m56s
CI / E2E (Web) (pull_request) Successful in 3m41s
c92a1acd9a
The JPEG diff spec asserted on a `.file-table__diff-group-header` with
text matching /EXIF/. That expectation became wrong with 1c3ced5
("surface raw ExifTool group names — never collapse sub-groups"): the
diff strategy now emits ExifTool family-1 names verbatim (IFD0,
ExifIFD, XMP-dc, etc.) and explicitly refuses to collapse them to
umbrella labels like "EXIF". `sample.jpg` carries Make + Model in
IFD0, so update the regex and the surrounding header comment to match
the actual rendered label.

Failure traced from the CI run on PR #183. Verified locally against
both standalone-desktop / standalone-mobile and web-desktop projects.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forgejo_admin merged commit a5546afa71 into master 2026-05-22 11:04:05 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: forgejo_admin/exifcleaner-web#183
No description provided.