search

avaneev / r8brain-free-src

High-quality pro audio resampler / sample rate conversion C++ library. Very fast, for both audio resampling and time-series interpolation.
MIT License
572 stars 61 forks source link

CDSPHBUpsampler.inc NEON is limited to ARM64, document that? #21

Closed peterdk closed 2 years ago

peterdk commented 2 years ago

Hi,

I am using your library in my Mini Piano Lite Android app. Today I tried updating to the 5.6 version of r8brain. However, I got compile errors that eluded me for a while. It turns out that the CDSPHBUpsampler.inc NEON code uses ARM64 only NEON types, and won't compile on ARMv7a NEON. PFFFT works with ARMv7a NEON, but CDSPHBUpsampler.inc doesn't.

I got it working by removing my check for ARMv7A in the upsampler, but I thought it would be useful to add some info about supported NEON versions in the readme?

Thanks for the great library.

Peter

avaneev commented 2 years ago

PFFFT uses similar macros to CDSPHBUpsampler (defined( aarch64 ) || defined( arm64 ) || defined( __ARM_NEON )). However, upsampler may use intrinsics available for ARM64 only. I will remove the defined( __ARM_NEON ) so that ARM support is limited to ARM64.

peterdk commented 2 years ago

Yes, I normally modified the macro's to allow armv7a also to use neon, but the upsampler indeed uses intrinsics not available to them.

avaneev commented 2 years ago

I've updated to v5.7 - please check it out.

peterdk commented 2 years ago

Thanks, that one compiles without issues and no changes needed at my side!

avaneev commented 2 years ago

Thanks for checking!

mgood7123 commented 2 years ago

@avaneev @peterdk

please note that

https://github.com/android/ndk/wiki/Changelog-r23 https://github.com/android/ndk/wiki/Changelog-r24

"NDK r23 is the last release that will support non-Neon. Beginning with NDK r24, the armeabi-v7a libraries in the sysroot will be built with Neon. A very small number of very old devices do not support Neon"

avaneev commented 2 years ago

r8brain-free always enables NEON on AArch64 builds. But NEON code does not offer much benefit overall, so nothing to worry about.

peterdk commented 2 years ago

@mgood7123 It doesn't mean that you can't limit NEON to arm64 in the library as now is the case. Just that the flag is always set, which didn't work and is now fixed with this ticket.

mgood7123 commented 2 years ago

r8brain-free always enables NEON on AArch64 builds. But NEON code does not offer much benefit overall, so nothing to worry about.

hmm alright

mgood7123 commented 2 years ago

@mgood7123 It doesn't mean that you can't limit NEON to arm64 in the library as now is the case. Just that the flag is always set, which didn't work and is now fixed with this ticket.

yes, but only if compiling with ndk r24

peterdk commented 2 years ago

Not really, I enabled NEON for armv7a in my own app, since that's configurable. The default indeed is not to use NEON on armv7 in ndk < r24

mgood7123 commented 2 years ago

Not really, I enabled NEON for armv7a in my own app, since that's configurable. The default indeed is not to use NEON on armv7 in ndk < r24

yes, but i think in r24 neon is enabled by default now