perf(arm64): prefer the NEON nnedi3 over scalar znedi3 - #61
Merged
Conversation
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
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On ARM, znedi3 falls back to a pure scalar path —
download-deps-{macos,linux}.shbuild it
X86=0 X86_AVX512=0and its SIMD kernels are x86-only. The bundleddubhater
nnedi3has real NEON kernels, implements the same network from the samennedi3_weights.bin, and has an identical signature for every argument QTGMC uses.Measured on an M1 against the rebuilt R78 bundle, QTGMC Slow: the interpolator goes
from 138.2 to 29.7 CPU-seconds (~4.7x), worth ~13% end-to-end.
The choice is made at runtime via
platform.machine(), so x86 keeps znedi3unchanged and the patch text stays identical on every platform. Delivered two ways:
a
_nnedi3()helper in both templates, and havsfunc patch 6 rewriting the threehardcoded call sites (daa, santiag, QTGMC).
Also fixes the Linux side of the deps build, which the new packaging guard caught:
as 32-bit ARMv7 —
-mfpu=neonis rejected by aarch64 gcc, andcpufeatures.cppreads
HWCAP_ARM_*constants that only exist on 32-bit. macOS only ever hit thefirst, via its
__APPLE__branch. Both patched, with a guard that hard-fails ifeither stops matching —
nnedi3.cpponly doesif (!cpu.neon) d->opt = 0, so alapsed patch yields a correct picture at scalar speed.
macos-x64andwindows-x64already omit nnedi3, so requiring it on linux-x64 was inconsistent.Verification
test_92fails the build if a template reintroduces a direct implementation calldeps-v1.8.0-rc1; nightly green on all 4 platformshavsfunc._nnedi3_impl()resolves tonnedi3.nnedi3