Build opus with NEON intrinsics on non-Android 64-bit ARM#3833
Open
mcfnord wants to merge 1 commit into
Open
Conversation
…tware#2806) The opus arch-optimisation block only matched the Android ABI names `armeabi-v7a` and `arm64-v8a`. Everywhere else Qt reports 64-bit ARM as `QT_ARCH=arm64` (Apple Silicon macOS, iOS, Linux aarch64), so those builds fell through to the plain-C opus path with no NEON acceleration. In fact the ARM NEON sources were never compiled on any target: the `SOURCES += $$SOURCES_OPUS_ARCH` line only lived in the x86 branch, so even the Android arch match populated the variable but never built it. Add an `arm64` branch that: - defines OPUS_ARM_MAY_HAVE_NEON_INTR (required for opus to include its arm/*.h headers) plus the PRESUME_NEON_INTR / PRESUME_AARCH64_NEON_INTR defines (NEON is part of the base AArch64 ISA, so it is always present); - compiles the NEON intrinsic sources, which need no special compiler flags, straight into SOURCES. Split SOURCES_OPUS_ARM into the self-contained NEON intrinsic files and the two NE10 files, which #include <NE10_dsp.h> from the external Ne10 library that Jamulus does not bundle; only the NEON subset is compiled. The x86 and Android arch matches are untouched: their generated Makefiles are byte-identical before and after this change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Note
📡 STAND BY FOR AN LLM-AUTHORED MESSAGE.
Fixes #2806.
Follows up on the root-cause analysis posted in #2806 (comment): the opus arch-optimisation block in
Jamulus.promatches only the Android ABI names (armeabi-v7a,arm64-v8a). Everywhere else, Qt reports 64-bit ARM asQT_ARCH=arm64— Apple Silicon macOS (mac/deploy_mac.shpasses it explicitly per architecture slice), iOS, and Linux aarch64 — so all of those builds compile opus as plain C with no NEON acceleration, even though the NEON intrinsic sources ship in-tree.While implementing the fix, a deeper problem surfaced: the ARM NEON sources have never been compiled on any target, including Android. Two independent gaps:
SOURCES += $$SOURCES_OPUS_ARCHonly exists in the x86 consumption branch (around line 1150), so the Android arch match populatedSOURCES_OPUS_ARCHbut nothing ever built it.arm/*.hheaders onOPUS_ARM_MAY_HAVE_NEON_INTR(seelibs/opus/celt/pitch.h:49andlibs/opus/celt/cpu_support.h), which the block never defines — thePRESUMEdefines alone are inert.That the Android builds link cleanly is itself the proof: with the defines inert, the generic C paths are used and the never-compiled NEON objects are never referenced.
What this PR does
else:contains(QT_ARCH, arm64)branch to the arch block that definesOPUS_ARM_MAY_HAVE_NEON_INTR=1 OPUS_ARM_PRESUME_NEON_INTR=1 OPUS_ARM_PRESUME_AARCH64_NEON_INTR=1. NEON is part of the base AArch64 ISA, so presuming it is always safe on this arch — withPRESUMEset, opus calls the NEON functions directly and no runtime CPU detection machinery is engaged.SOURCES. Unlike the x86 SSE files, they need no special compiler flags on AArch64, so noQMAKE_EXTRA_COMPILERSdance is required.SOURCES_OPUS_ARMinto the nine self-contained NEON intrinsic files and the two NE10 files (celt_fft_ne10.c,celt_mdct_ne10.c). The NE10 pair#include <NE10_dsp.h>from the external Ne10 library, which Jamulus does not bundle, so only the NEON subset is compiled.Deliberately not included: activating NEON for the Android ABIs. That would change currently-shipping builds and
armeabi-v7aneeds its own decision about NEON baseline guarantees (#2475 territory). This PR leaves the Android and x86 branches untouched — their generated Makefiles are byte-identical before and after this change (verified by diffingqmake-generatedMakefile.ReleaseforQT_ARCH=x86_64andQT_ARCH=arm64-v8a).Why it's worth having
Opus encode/decode is the dominant per-channel CPU cost of a Jamulus server and a large share of the client's audio-thread budget. The NEON intrinsics cover the hot inner loops: pitch cross-correlation and inner products in CELT (
celt_pitch_xcorr_float_neon,celt_inner_prod_neon,dual_inner_prod_neon) plus the SILK NSQ/LPC kernels. Every Apple Silicon Mac client, every iOS build, and every Linux aarch64 server (a rapidly growing hosting class — AWS Graviton, Oracle Ampere, Raspberry Pi 5) currently runs the scalar C fallback. This change turns the optimizations that already ship in the tree on for that hardware, with zero effect on any other platform.Testing (cross-verified on Linux, aarch64-linux-gnu-gcc 13.3):
qmake "QT_ARCH=arm64"on the patched tree selects the nine NEON sources and the three defines; the NE10 files are correctly absent.DEFINES_OPUSfrom the generated Makefile, archives into a static library with all nine*_neonsymbols defined and zero unresolved NEON references.fmla v1.4schains incelt_pitch_xcorr_float_neon) — genuinely vectorized, not scalar fallback.QT_ARCH=x86_64andQT_ARCH=arm64-v8aMakefiles are byte-identical pre/post patch (regression guard).🤖 Generated with Claude Code