breakfastquay / rubberband

Official mirror of Rubber Band Library, an audio time-stretching and pitch-shifting library.
http://breakfastquay.com/rubberband/
GNU General Public License v2.0
561 stars 89 forks source link

android real time pitch shift output values boost 50x~100x, same code compiled for ios and macos are ok #56

Closed pangwong closed 1 year ago

pangwong commented 2 years ago

the problem only occurs on rubberband 1.9.2 and 2.0.2, but 1.8.2 works fine. android NDK version 21.3.6528147, compiled on macos 12.2.1

for example, input pcm buffer are floats range from -1 to 1. the Average Absolute Value of input pcm buffer is 0.17, the Average Absolute Value of output pcm buffer can be 10, 17 the output pcm buffer values boost 50x ~ 100x

cannam commented 2 years ago

Interesting, I haven't seen this reported before.

How is the library configured? Which FFT and resampler implementations and compiler flags are you using? And are these the same on 1.8.2 and 1.9.2?

The library doesn't do anything intrinsically differently on Android - there's no obvious reason it should behave differently there as compared to any other arm64 architecture with similar compilers, except perhaps due to different build options or library dependencies.

pangwong commented 2 years ago

@cannam Hi, I just compiled the library as config in Android.mk using ndk-build. Used Android.mk of both version (1.8.2 and 1.9.2) are all copied from downloaded released package, I just modified some relative path.

No other customization of Android.mk except when build version 1.8.2, because of a compile error :

jni/rubberband/src/pommier/sse_mathfun.h:73:32: error: constant expression evaluates to 2147483648 which cannot be narrowed to type 'int' [-Wc++11-narrowing]
_PS_CONST_TYPE(sign_mask, int, 0x80000000);
                               ^~~~~~~~~~

and I add a CXX_FLAG in Android.mk and it compiles.

LOCAL_CXXFLAGS := -std=c++98
cannam commented 2 years ago

The Android.mk is quite old, it dates from a time with significantly less powerful 32-bit-only processors in Android phones. Some of the defines in there are probably worth removing for test purposes at least.

That could include USE_POMMIER_MATHFUN, LACK_SINCOS, arguably the -ffast-math, -freciprocal-math and -fsingle-precision-constant flags, and even PROCESS_SAMPLE_TYPE=float (since on 64-bit architectures double is often faster). You could also replace USE_KISSFFT with USE_BUILTIN_FFT on 1.9.2 and see what happens. Please do report back here, if you try these.

It might also be instructive to try the releases between 1.8.2 and 1.9.2 (i.e. 1.9.0 and 1.9.1).

I can make a note to look into this, but I don't have an Android development environment to hand and can't easily schedule time for Android things at the moment (except in the case of commercial projects with development time attached) so it is almost certainly quicker to try some variations yourself I'm afraid.

pangwong commented 2 years ago

@cannam Hi sorry for reply late. I have tested 2 cases

  1. as your follow, I remove all the flags listed above and others not change, kissfft is used
  2. substitude kissfft with default fft

the 1st case, the pitch shift result is ok the 2nd case, the app will crash everytime started

cannam commented 2 years ago

Good to hear that it works in one of these cases at least!

In the second case, when you say it crashes at the start, do you mean it crashes when you first create a Rubber Band stretcher object?

pangwong commented 2 years ago

Yes,when the app started, a Rubber Band stretcher object was created, and the app crashed

cannam commented 2 years ago

That's very odd. Can you get a stack trace?

The built-in FFT has had quite a lot of testing by now, and has been through all the usual memory sanitisation. I rebuilt it a moment ago with a series of different combinations of the other arguments used in the Android.mk, in case there was some problem in combination with e.g. a specific aligned memory allocator, but I couldn't find any issues.

pangwong commented 2 years ago

My bad, I did not reproduce the crash when using default fft, maybe some other problem resulted in the crash. Thanks for your in time response, this is a really helpful project.