From 2462ba81b43eb4cd0f79c092f258f2f940f23192 Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Fri, 7 Aug 2026 09:05:19 +1000 Subject: [PATCH 1/3] perf(arm64): prefer the NEON nnedi3 over the scalar znedi3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit znedi3's SIMD kernels are x86-only, so the ARM bundles build it with X86=0 and it runs fully scalar (PredictorC / PrescreenerOldC). The bundled dubhater nnedi3 ships real NEON kernels and is 6.3x faster for the same call — measured on an M1, QTGMC Slow, 400 frames of 720x576: 37.8s vs 5.95s of CPU, which is 30% of the whole arm64 QTGMC cost. havsfunc hardcoded `core.znedi3.nnedi3 if hasattr(core, 'znedi3')` at three call sites (daa, santiag, QTGMC), and both our templates named znedi3 directly in the upscale path, so every ARM deinterlace paid it. Both plugins implement the same network from the same nnedi3_weights.bin and their signatures are identical for every argument used, so this is a drop-in swap: measured mean output difference 0.045/255 for the interpolator alone and 0.072/255 end-to-end through QTGMC, against a tolerance of 2.0. Worst single pixel is ~48/255 on hard edges, where the two implementations' float rounding flips a prescreener decision. The choice is made at runtime rather than by the build, so havsfunc patch 6 is byte-identical on every platform and x86 keeps using znedi3 exactly as before. Worth +10% (Preset Faster) to +40% (Slow) end-to-end on arm64. - havsfunc patch 6 in all three download-deps scripts, adding _nnedi3_impl() - a _nnedi3() helper in both templates, replacing the four direct calls - test_92 fails the build if a template names an implementation directly; this is a bug that still produces a correct picture, just slowly, so nothing else would catch it - nnedi3 added to the required-plugin list on ARM only (Windows and macOS-x64 ship only znedi3 + nnedi3cl) and to deps-expected-plugins.json for Linux --- CLAUDE.md | 47 ++++++++++++++++- Scripts/deps-expected-plugins.json | 2 + Scripts/download-deps-linux.sh | 31 +++++++++++ Scripts/download-deps-macos.sh | 31 +++++++++++ Scripts/download-deps-windows.ps1 | 29 +++++++++++ .../integration_filter_parameters_test.dart | 6 ++- app/test/vapoursynth_integration_test.dart | 12 +++++ worker/templates/pipeline_template.vpy | 21 +++++++- worker/templates/preview_template.vpy | 21 +++++++- worker/tests/filter_integration_test.rs | 51 ++++++++++++++++++- 10 files changed, 243 insertions(+), 8 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a69dbf8..94ffc0c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -884,7 +884,7 @@ integration tests. Matrix: macOS **arm64** (`macos-15`), macOS **x64** All three `download-deps-*` scripts apply these automatically, and they must stay **identical across platforms** — a patch applied on only some platforms makes the -same job produce different output per OS. Five patches: +same job produce different output per OS. Six patches: 1. **mvtools API**: Renamed `_lambda`→`lambda`, `_global`→`global` parameters 2. **DFTTest API**: `sstring` parameter removed, replaced with `sigma=10.0` @@ -892,6 +892,7 @@ same job produce different output per OS. Five patches: 4. **EEDI3CL fallback**: modern `eedi3m` dropped `EEDI3CL`, so `opencl=True` fell over; fall back to CPU `EEDI3` (NNEDI3CL still uses the GPU) 5. **`Bob()` 16-bit resample** — see below +6. **ARM nnedi3 preference** — see "The ARM interpolator choice" below Each patch is a **literal string match** against havsfunc r31. If upstream ever changes one of those lines the patch silently does nothing, so behavioural tests @@ -902,6 +903,50 @@ matter more than usual (see `app/test/vapoursynth_integration_test.dart`). > would `AttributeError`. Only reachable with `Denoiser='knlmeanscl'` **and** > `ChromaNoise=True` (both non-default), so it has never been hit. +### The ARM interpolator choice: nnedi3, not znedi3 (patch 6) + +**Never name an nnedi3 implementation directly.** Both templates define a +`_nnedi3()` helper and havsfunc gets an `_nnedi3_impl()` via patch 6; every call +site goes through one of those. `test_92` fails the build if a template +reintroduces a direct `core.znedi3.nnedi3` / `core.nnedi3.nnedi3` call. + +The reason is a pure-performance trap that produces a **correct picture**, so +nothing but a benchmark or that assertion catches it: + +- znedi3's SIMD kernels are x86-only, so `download-deps-{macos,linux}.sh` build it + `make X86=0 X86_AVX512=0` and ARM gets the scalar `PredictorC`/`PrescreenerOldC` + path. The bundled dubhater **`nnedi3` has real NEON kernels** + (`computeNetwork0_neon`, `dotProd_neon`, …). +- Measured on an M1, QTGMC Slow, 400 frames of 720x576: **37.8s of CPU for znedi3 + vs 5.95s for nnedi3 — 6.3x**, and 30% of the entire arm64 QTGMC cost. End to end + the swap is worth **+10% (Faster) to +40% (Slow)**. +- havsfunc hardcoded `core.znedi3.nnedi3 if hasattr(core, 'znedi3')` at three call + sites (daa, santiag, QTGMC), so *every* ARM deinterlace paid it. + +The two plugins implement the same network from the same `nnedi3_weights.bin` and +their signatures are identical for every argument used, so this is a drop-in swap: +measured mean output difference **0.045/255** for the interpolator alone and +**0.072/255** end-to-end through QTGMC (tolerance is 2.0). Worst single pixel is +~48/255 on hard edges, where the two implementations' float rounding flips a +prescreener decision — expected for independent implementations of the same net. + +The choice is made **at runtime** (`platform.machine()`), not by the build, so the +patch text stays identical on every platform and **x86 keeps using znedi3 +unchanged**. Both plugins are therefore required in the deps bundle and both are in +the `vapoursynth_integration_test.dart` plugin list — dropping either breaks +deinterlacing on half the platforms. + +> This is one instance of a much larger arm64 gap: native arm64 QTGMC runs +> **3–4.5x slower than the x64 bundle under Rosetta**. The dominant cause is not +> this but `std.Expr` — VapourSynth's `compile_jit()` +> (`src/core/expr/jitcompiler.cpp`) is wrapped in `#ifdef VS_TARGET_CPU_X86`, so on +> aarch64 it returns no compiler and `exprfilter.cpp` falls back to +> `ExprInterpreter::eval()`, a scalar switch-dispatch interpreter run **once per +> pixel**: 69.5s vs 3.3s of CPU on the same job, **21x**. Most of VS core is +> x86-SIMD-only the same way (genericfilters, mergefilters, averageframes, +> planestats). Fixing that needs a vectorized Expr on ARM and is tracked +> separately. Not a cause: mvtools is *faster* natively (it compiles its SSE2 +> paths through simde). ### Linux builds on ubuntu-24.04, and that sets the glibc floor The Linux **deps** builds and the runners that test against them diff --git a/Scripts/deps-expected-plugins.json b/Scripts/deps-expected-plugins.json index 4dbeded..2b31cb5 100644 --- a/Scripts/deps-expected-plugins.json +++ b/Scripts/deps-expected-plugins.json @@ -93,6 +93,7 @@ "libmiscfilters.so", "libmvtools.so", "libneo-f3kdb.so", + "libnnedi3.so", "libnnedi3cl.so", "libremovegrain.so", "libtcanny.so", @@ -118,6 +119,7 @@ "libmiscfilters.so", "libmvtools.so", "libneo-f3kdb.so", + "libnnedi3.so", "libnnedi3cl.so", "libremovegrain.so", "libtcanny.so", diff --git a/Scripts/download-deps-linux.sh b/Scripts/download-deps-linux.sh index a381b85..aa736eb 100755 --- a/Scripts/download-deps-linux.sh +++ b/Scripts/download-deps-linux.sh @@ -1114,6 +1114,37 @@ if old_bob in content: ) patches.append('Bob 16-bit resample') +# Patch 6: prefer the NEON nnedi3 over the scalar znedi3 on ARM. +# znedi3's SIMD kernels are x86-only, so the ARM bundles build it with X86=0 and +# it runs fully scalar (PredictorC / PrescreenerOldC). The bundled dubhater +# nnedi3 ships real NEON kernels and is 6.3x faster for the same call — measured +# on an M1, QTGMC Slow, 400 frames of 720x576: 37.8s vs 5.95s of CPU, which is +# 30% of the whole arm64 QTGMC cost. havsfunc hardcodes znedi3 whenever it is +# present, so without this every ARM deinterlace pays that. +# Both plugins implement the same network from the same nnedi3_weights.bin and +# their signatures are identical for every argument havsfunc passes, so this is a +# drop-in swap: measured mean output difference 0.045/255 (worst pixel 27/255, on +# edges where the prescreener decision flips). +# The choice is made at runtime rather than by the build, so this patch text +# stays identical on every platform — x86 keeps using znedi3 exactly as before. +if '_nnedi3_impl' not in content: + old_edi = "myNNEDI3 = core.znedi3.nnedi3 if hasattr(core, 'znedi3') else core.nnedi3.nnedi3" + n_edi = content.count(old_edi) + if n_edi: + # Leading whitespace is untouched, so this covers all three call sites + # (daa, santiag, QTGMC) despite their differing indentation. + content = content.replace(old_edi, "myNNEDI3 = _nnedi3_impl()") + content = content.replace('import math\n', 'import math\n' + ''' + +# Prefer the NEON nnedi3 over the scalar znedi3 on ARM (see download-deps-*). +def _nnedi3_impl(): + import platform + if platform.machine().lower() in ('arm64', 'aarch64') and hasattr(core, 'nnedi3'): + return core.nnedi3.nnedi3 + return core.znedi3.nnedi3 if hasattr(core, 'znedi3') else core.nnedi3.nnedi3 +''') + patches.append(f'ARM nnedi3 preference ({n_edi} sites)') + if patches: with open(havsfunc_path, 'w') as f: f.write(content) diff --git a/Scripts/download-deps-macos.sh b/Scripts/download-deps-macos.sh index ab45c6a..17aa695 100755 --- a/Scripts/download-deps-macos.sh +++ b/Scripts/download-deps-macos.sh @@ -1577,6 +1577,37 @@ if old_bob in content: ) patches.append('Bob 16-bit resample') +# Patch 6: prefer the NEON nnedi3 over the scalar znedi3 on ARM. +# znedi3's SIMD kernels are x86-only, so the ARM bundles build it with X86=0 and +# it runs fully scalar (PredictorC / PrescreenerOldC). The bundled dubhater +# nnedi3 ships real NEON kernels and is 6.3x faster for the same call — measured +# on an M1, QTGMC Slow, 400 frames of 720x576: 37.8s vs 5.95s of CPU, which is +# 30% of the whole arm64 QTGMC cost. havsfunc hardcodes znedi3 whenever it is +# present, so without this every ARM deinterlace pays that. +# Both plugins implement the same network from the same nnedi3_weights.bin and +# their signatures are identical for every argument havsfunc passes, so this is a +# drop-in swap: measured mean output difference 0.045/255 (worst pixel 27/255, on +# edges where the prescreener decision flips). +# The choice is made at runtime rather than by the build, so this patch text +# stays identical on every platform — x86 keeps using znedi3 exactly as before. +if '_nnedi3_impl' not in content: + old_edi = "myNNEDI3 = core.znedi3.nnedi3 if hasattr(core, 'znedi3') else core.nnedi3.nnedi3" + n_edi = content.count(old_edi) + if n_edi: + # Leading whitespace is untouched, so this covers all three call sites + # (daa, santiag, QTGMC) despite their differing indentation. + content = content.replace(old_edi, "myNNEDI3 = _nnedi3_impl()") + content = content.replace('import math\n', 'import math\n' + ''' + +# Prefer the NEON nnedi3 over the scalar znedi3 on ARM (see download-deps-*). +def _nnedi3_impl(): + import platform + if platform.machine().lower() in ('arm64', 'aarch64') and hasattr(core, 'nnedi3'): + return core.nnedi3.nnedi3 + return core.znedi3.nnedi3 if hasattr(core, 'znedi3') else core.nnedi3.nnedi3 +''') + patches.append(f'ARM nnedi3 preference ({n_edi} sites)') + if patches: with open(havsfunc_path, 'w') as f: f.write(content) diff --git a/Scripts/download-deps-windows.ps1 b/Scripts/download-deps-windows.ps1 index 458e05e..fd44412 100644 --- a/Scripts/download-deps-windows.ps1 +++ b/Scripts/download-deps-windows.ps1 @@ -692,6 +692,35 @@ def _fix_mv_args(args): $PatchesApplied += "Bob 16-bit resample" } + # Patch 6: prefer the NEON nnedi3 over the scalar znedi3 on ARM. + # znedi3's SIMD kernels are x86-only, so the ARM bundles build it with X86=0 + # and it runs fully scalar; the bundled dubhater nnedi3 ships real NEON + # kernels and is 6.3x faster for the same call (measured on an M1, QTGMC + # Slow: 37.8s vs 5.95s of CPU). havsfunc hardcodes znedi3 whenever it is + # present, so without this every ARM deinterlace pays that cost. + # Windows x64 is NOT affected - the runtime check below keeps it on znedi3 + # exactly as before. This is applied here purely so every platform generates + # identical output from an identical havsfunc, same as patch 5. + $OldEdi = "myNNEDI3 = core.znedi3.nnedi3 if hasattr(core, 'znedi3') else core.nnedi3.nnedi3" + if ($Content -notmatch "_nnedi3_impl" -and $Content.Contains($OldEdi)) { + Write-Host " Applying ARM nnedi3 preference patch..." -ForegroundColor Gray + # Leading whitespace is untouched, so this covers all three call sites + # (daa, santiag, QTGMC) despite their differing indentation. + $Content = $Content.Replace($OldEdi, "myNNEDI3 = _nnedi3_impl()") + $PatchFunction = @" + +# Prefer the NEON nnedi3 over the scalar znedi3 on ARM (see download-deps-*). +def _nnedi3_impl(): + import platform + if platform.machine().lower() in ('arm64', 'aarch64') and hasattr(core, 'nnedi3'): + return core.nnedi3.nnedi3 + return core.znedi3.nnedi3 if hasattr(core, 'znedi3') else core.nnedi3.nnedi3 + +"@ + $Content = $Content -replace "(import math\r?\n)", "`$1$PatchFunction" + $PatchesApplied += "ARM nnedi3 preference" + } + if ($PatchesApplied.Count -gt 0) { Set-Content $HavsfuncPath $Content -NoNewline Write-Host " havsfunc patched ($($PatchesApplied -join ', '))" -ForegroundColor Green diff --git a/app/test/integration_filter_parameters_test.dart b/app/test/integration_filter_parameters_test.dart index 61771da..ffd7e05 100644 --- a/app/test/integration_filter_parameters_test.dart +++ b/app/test/integration_filter_parameters_test.dart @@ -360,8 +360,10 @@ void main() { print(' Parsed ${actual.length} params'); expect(actual['alpha'], '0.4'); expect(actual['mdis'], '30'); - // The nnedi3 sclip that guides it carries the nnedi3 controls. - final sclip = parseFilterParams(script, 'core.znedi3.nnedi3('); + // The nnedi3 sclip that guides it carries the nnedi3 controls. Match the + // assignment, not a bare `_nnedi3(` — the helper's own `def _nnedi3(` line + // appears earlier in the script and would be found first. + final sclip = parseFilterParams(script, 'sclip = _nnedi3('); expect(sclip['nsize'], '4'); expect(sclip['nns'], '3'); // And the half-pixel dh shift is corrected per plane. diff --git a/app/test/vapoursynth_integration_test.dart b/app/test/vapoursynth_integration_test.dart index 5b78bda..3370c25 100644 --- a/app/test/vapoursynth_integration_test.dart +++ b/app/test/vapoursynth_integration_test.dart @@ -122,6 +122,18 @@ required = ['std', 'resize', 'mv', 'znedi3', 'eedi3m', 'fmtc', 'ctmf', 'warp', 'misc', 'grain', 'tcanny', 'zsmooth', 'descratch', 'vivtc', 'ttmpsm', 'tmedian', 'fft3dfilter'] + +# On ARM, `nnedi3` is load-bearing and `znedi3` is only the fallback: znedi3's +# SIMD is x86-only, so the ARM bundles build it scalar and both the templates' +# _nnedi3() helper and havsfunc's patched _nnedi3_impl() pick nnedi3 instead +# (6.3x faster, same network and weights). A bundle that dropped nnedi3 would +# still deinterlace correctly — it would just silently be slow again, which is +# exactly the kind of regression a "does it run" check never catches. +# x86 bundles deliberately do NOT ship plain nnedi3 (Windows and macOS-x64 have +# only znedi3 + nnedi3cl), so this requirement is arch-conditional, not global. +import platform +if platform.machine().lower() in ('arm64', 'aarch64'): + required.append('nnedi3') # nnedi3cl and knlm are deliberately NOT required: both are OpenCL and the app # degrades to a CPU path when the driver is absent. diff --git a/worker/templates/pipeline_template.vpy b/worker/templates/pipeline_template.vpy index 7cc0ed4..2920c02 100644 --- a/worker/templates/pipeline_template.vpy +++ b/worker/templates/pipeline_template.vpy @@ -49,6 +49,23 @@ print(f"INPUT_INFO:frames={total_frames},fps_num={input_fps_num},fps_den={input_ # Import havsfunc for various filters (QTGMC, SMDegrain, chroma fixes) import havsfunc as haf +# znedi3 carries x86-only SIMD, so on ARM it is built with X86=0 and runs fully +# scalar — measured 6.3x slower than the bundled nnedi3, which ships real NEON +# kernels. Both implement the same network from the same weights file and their +# signatures are identical for every argument used here, so preferring nnedi3 on +# ARM is a drop-in swap (measured mean difference 0.045/255). Route every +# interpolator call through this helper rather than naming a plugin directly. +# havsfunc's own three call sites get the same treatment via its patch 6. +import platform as _platform + +_IS_ARM = _platform.machine().lower() in ('arm64', 'aarch64') + + +def _nnedi3(clip, **kwargs): + if _IS_ARM and hasattr(core, 'nnedi3'): + return core.nnedi3.nnedi3(clip, **kwargs) + return core.znedi3.nnedi3(clip, **kwargs) + # ============================================================================ # PASS 1: PRE-CROP (before deinterlacing to reduce processing area) # ============================================================================ @@ -1047,7 +1064,7 @@ if clip.format.color_family == vs.YUV: {{#UPSCALE_EDGE_DIRECTED}} def _upscale_double_height(c): {{#UPSCALE_NNEDI3}} - return core.znedi3.nnedi3( + return _nnedi3( c, field=1, dh=True, @@ -1072,7 +1089,7 @@ def _upscale_double_height(c): # EEDI3 needs a second opinion to bound its cost search. nnedi3 is the # conventional choice and is bundled anyway, so the nnedi3 controls below # shape the sclip that guides EEDI3. - sclip = core.znedi3.nnedi3( + sclip = _nnedi3( c, field=1, dh=True, diff --git a/worker/templates/preview_template.vpy b/worker/templates/preview_template.vpy index 9b2c549..7fca623 100644 --- a/worker/templates/preview_template.vpy +++ b/worker/templates/preview_template.vpy @@ -41,6 +41,23 @@ print(f"INPUT_INFO:frames={total_frames},fps_num={{FPS_NUM}},fps_den={{FPS_DEN}} # Import havsfunc for various filters (QTGMC, SMDegrain, chroma fixes) import havsfunc as haf +# znedi3 carries x86-only SIMD, so on ARM it is built with X86=0 and runs fully +# scalar — measured 6.3x slower than the bundled nnedi3, which ships real NEON +# kernels. Both implement the same network from the same weights file and their +# signatures are identical for every argument used here, so preferring nnedi3 on +# ARM is a drop-in swap (measured mean difference 0.045/255). Route every +# interpolator call through this helper rather than naming a plugin directly. +# havsfunc's own three call sites get the same treatment via its patch 6. +import platform as _platform + +_IS_ARM = _platform.machine().lower() in ('arm64', 'aarch64') + + +def _nnedi3(clip, **kwargs): + if _IS_ARM and hasattr(core, 'nnedi3'): + return core.nnedi3.nnedi3(clip, **kwargs) + return core.znedi3.nnedi3(clip, **kwargs) + # ============================================================================ # PASS 1: PRE-CROP (before deinterlacing to reduce processing area) # ============================================================================ @@ -993,7 +1010,7 @@ if clip.format.color_family == vs.YUV: {{#UPSCALE_EDGE_DIRECTED}} def _upscale_double_height(c): {{#UPSCALE_NNEDI3}} - return core.znedi3.nnedi3( + return _nnedi3( c, field=1, dh=True, @@ -1018,7 +1035,7 @@ def _upscale_double_height(c): # EEDI3 needs a second opinion to bound its cost search. nnedi3 is the # conventional choice and is bundled anyway, so the nnedi3 controls below # shape the sclip that guides EEDI3. - sclip = core.znedi3.nnedi3( + sclip = _nnedi3( c, field=1, dh=True, diff --git a/worker/tests/filter_integration_test.rs b/worker/tests/filter_integration_test.rs index 75ae11d..3f68323 100644 --- a/worker/tests/filter_integration_test.rs +++ b/worker/tests/filter_integration_test.rs @@ -2510,7 +2510,7 @@ fn test_70_upscale_nnedi3_granular_parameters() { }); run_job_and_verify(&job, "NNEDI3 upscale granular", &[ - "core.znedi3.nnedi3", + "return _nnedi3(", "nsize=4", "nns=4", "qual=2", @@ -3383,3 +3383,52 @@ fn test_91_chroma_subsampling_serde_names_match_the_app() { Some("vs.YUV422P10") ); } + +/// Test 92: neither template may name an nnedi3 implementation directly. +/// +/// znedi3's SIMD kernels are x86-only, so the ARM bundles build it with X86=0 +/// and it runs fully scalar — measured 6.3x slower than the bundled nnedi3, +/// which ships real NEON kernels, and 30% of the whole arm64 QTGMC cost. Both +/// implement the same network from the same weights file, so the templates pick +/// at runtime via the `_nnedi3()` helper (havsfunc does the same through its +/// patch 6). A direct `core.znedi3.nnedi3` call reintroduces the slow path on +/// ARM silently — it still produces a correct picture, just far slower, so only +/// an assertion like this one catches it. +#[test] +fn test_92_templates_do_not_hardcode_an_nnedi3_implementation() { + let templates_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("templates"); + + for name in ["pipeline_template.vpy", "preview_template.vpy"] { + let path = templates_dir.join(name); + let body = std::fs::read_to_string(&path) + .unwrap_or_else(|e| panic!("read {}: {e}", path.display())); + + // The helper itself is the one place either plugin may be named. + let helper_start = body + .find("def _nnedi3(") + .unwrap_or_else(|| panic!("{name} is missing the _nnedi3() helper")); + let helper_end = helper_start + + body[helper_start..] + .find("\n\n") + .expect("helper should be followed by a blank line"); + let (before, after) = (&body[..helper_start], &body[helper_end..]); + + for (plugin, call) in [ + ("znedi3", "core.znedi3.nnedi3("), + ("nnedi3", "core.nnedi3.nnedi3("), + ] { + assert!( + !before.contains(call) && !after.contains(call), + "{name} calls {plugin} directly; route it through _nnedi3() so ARM \ + gets the NEON implementation" + ); + } + + // And the helper is actually reached — the upscale path is the only + // caller in the template itself (QTGMC goes through havsfunc). + assert!( + body.contains("_nnedi3(\n"), + "{name} defines _nnedi3() but never calls it" + ); + } +} From d00324b385cd1d07f15f1b9847c6a955a63d8de3 Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Fri, 7 Aug 2026 19:48:11 +1000 Subject: [PATCH 2/3] fix(deps): build nnedi3 on linux-arm64, and stop requiring it on x86 The packaging guard added with the ARM interpolator change was right to fail: libnnedi3.so genuinely was not in either Linux bundle. Two separate reasons, one per arch. linux-arm64 could never have built it. dubhater's build system treats every ARM as 32-bit ARMv7, which breaks aarch64 twice over: -mfpu=neon is an ARMv7 option gcc rejects outright, and cpufeatures.cpp reads HWCAP_ARM_* out of getauxval(), constants that exist only for 32-bit ARM. macOS only ever hit the first, because cpufeatures.cpp has an __APPLE__ branch that skips the hwcap probe entirely -- so the macOS script has carried the -mfpu sed for ages while Linux silently shipped no plugin at all. Fix both in the nnedi3 block, with a guard that hard-fails if either literal match stops applying. That guard matters more than usual here: nnedi3.cpp only does "if (!cpu.neon) d->opt = 0", so a patch that quietly stopped working would produce a correct picture at scalar speed, which is precisely the failure this plugin is bundled to avoid. linux-x64 is a different story -- it fails on a missing yasm, and should not be building it in the first place. Patch 6 prefers nnedi3 only on ARM, so x86 keeps znedi3; macos-x64 and windows-x64 already omit nnedi3 from their expected-plugin lists. Requiring it on linux-x64 was inconsistent with that contract, so drop it. The NEON intrinsics themselves were never the problem: simd_neon.c is pure arm_neon.h with no inline asm, and macOS arm64 -- also aarch64 -- has been compiling that same file all along. --- CLAUDE.md | 20 +++++++++++++++++--- Scripts/deps-expected-plugins.json | 1 - Scripts/download-deps-linux.sh | 24 ++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 94ffc0c..a6cdd19 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -932,9 +932,23 @@ prescreener decision — expected for independent implementations of the same ne The choice is made **at runtime** (`platform.machine()`), not by the build, so the patch text stays identical on every platform and **x86 keeps using znedi3 -unchanged**. Both plugins are therefore required in the deps bundle and both are in -the `vapoursynth_integration_test.dart` plugin list — dropping either breaks -deinterlacing on half the platforms. +unchanged**. nnedi3 is therefore required only on the **ARM** bundles — +`deps-expected-plugins.json` lists it for `macos-arm64` and `linux-arm64` and +deliberately **not** for the three x86 ones, which never call it. Requiring it on +x86 just fails the packaging guard on a plugin nothing would load (nnedi3's x86 +path also needs `yasm`, which the runners don't have). + +> **Building nnedi3 on aarch64 needs two source patches**, because dubhater's +> build system treats every ARM as 32-bit ARMv7. `-mfpu=neon` is an ARMv7 option +> that aarch64 gcc rejects outright, and `cpufeatures.cpp` reads `HWCAP_ARM_*` +> from `getauxval()` — constants that exist only for 32-bit ARM. macOS only ever +> hit the first (it takes the `__APPLE__` branch in `cpufeatures.cpp`), which is +> why Linux arm64 shipped without the plugin until 2026-08-07. Both edits, and a +> guard that hard-fails if either stops matching, are in the nnedi3 block of +> `download-deps-linux.sh`; keep the `-mfpu` expression identical to the macOS +> one. The second patch is the one to be careful with: `nnedi3.cpp` only does +> `if (!cpu.neon) d->opt = 0`, so a wrong answer there yields a **correct picture +> at scalar speed** — the same silent failure this whole section is about. > This is one instance of a much larger arm64 gap: native arm64 QTGMC runs > **3–4.5x slower than the x64 bundle under Rosetta**. The dominant cause is not diff --git a/Scripts/deps-expected-plugins.json b/Scripts/deps-expected-plugins.json index 2b31cb5..60e5248 100644 --- a/Scripts/deps-expected-plugins.json +++ b/Scripts/deps-expected-plugins.json @@ -93,7 +93,6 @@ "libmiscfilters.so", "libmvtools.so", "libneo-f3kdb.so", - "libnnedi3.so", "libnnedi3cl.so", "libremovegrain.so", "libtcanny.so", diff --git a/Scripts/download-deps-linux.sh b/Scripts/download-deps-linux.sh index aa736eb..6cec765 100755 --- a/Scripts/download-deps-linux.sh +++ b/Scripts/download-deps-linux.sh @@ -678,6 +678,30 @@ if [ "$FORCE" = true ] || [ ! -f "$PLUGINS_DIR/libnnedi3.so" ]; then rm -rf nnedi3 git clone --depth 1 https://github.com/dubhater/vapoursynth-nnedi3.git nnedi3 cd nnedi3 + # dubhater's build system treats every ARM as 32-bit ARMv7, so aarch64 fails + # in two unrelated places. Both are build-system bugs, not portability limits: + # the NEON intrinsics themselves compile fine on aarch64 (macOS arm64 builds + # this same simd_neon.c). + # + # 1. -mfpu=neon is an ARMv7 option. NEON is baseline on aarch64 and gcc + # rejects the flag outright. download-deps-macos.sh strips it with this + # same expression -- keep the two identical. + # 2. cpufeatures.cpp reads HWCAP_ARM_* out of getauxval(), and those + # constants exist only for 32-bit ARM. ARMv8-A mandates NEON, so take + # the constant-true path macOS already uses. + # + # (2) is the dangerous one, because getting it wrong is silent: nnedi3.cpp + # only does "if (!cpu.neon) d->opt = 0", so a false negative still produces a + # correct picture -- just at scalar speed, which is the entire thing this + # plugin is bundled to avoid. Both edits are literal string matches, so + # verify they applied rather than shipping an unpatched build. + sed -i 's/ -mfpu=neon//' Makefile.am + sed -i 's/#elif defined(__APPLE__) && defined(NNEDI3_ARM)/#elif (defined(__APPLE__) || defined(__aarch64__)) \&\& defined(NNEDI3_ARM)/' src/cpufeatures.cpp + if grep -q -- '-mfpu=neon' Makefile.am || ! grep -q '__aarch64__' src/cpufeatures.cpp; then + echo " ERROR: the nnedi3 aarch64 build patches no longer apply -- upstream changed." + echo " Refusing to build a silently-scalar nnedi3; fix the patches in this script." + exit 1 + fi if ./autogen.sh && \ PKG_CONFIG_PATH="$PLUGIN_PKG_CONFIG" \ CFLAGS="-I$VS_INCLUDE_DIR" \ From 0a5ad2a643ac1d2966f5dc97f1a68ff1e996921c Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Fri, 7 Aug 2026 21:22:25 +1000 Subject: [PATCH 3/3] fix(test): make test_92 line-ending agnostic The blank-line search after the _nnedi3() helper looked for "\n\n", but git checks the templates out CRLF on Windows, where the file only ever contains "\r\n\r\n" -- so the test panicked with "helper should be followed by a blank line" on that platform alone. Normalise line endings on read. Verified both ways locally by converting the templates to CRLF and re-running. Nightly did not catch this because it runs only the heavy Flutter suite; test_92 is a cargo test and lives in the per-push gate. --- worker/tests/filter_integration_test.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/worker/tests/filter_integration_test.rs b/worker/tests/filter_integration_test.rs index 3f68323..e202ae5 100644 --- a/worker/tests/filter_integration_test.rs +++ b/worker/tests/filter_integration_test.rs @@ -3400,8 +3400,12 @@ fn test_92_templates_do_not_hardcode_an_nnedi3_implementation() { for name in ["pipeline_template.vpy", "preview_template.vpy"] { let path = templates_dir.join(name); + // Normalise line endings: git checks these templates out CRLF on Windows, + // and the blank-line search below is otherwise looking for "\n\n" in a + // file that only ever contains "\r\n\r\n". let body = std::fs::read_to_string(&path) - .unwrap_or_else(|e| panic!("read {}: {e}", path.display())); + .unwrap_or_else(|e| panic!("read {}: {e}", path.display())) + .replace("\r\n", "\n"); // The helper itself is the one place either plugin may be named. let helper_start = body