Closed alfureu closed 5 months ago
I tried to update it https://github.com/werman/noise-suppression-for-voice/pull/194. ~but maybe I still need to modify the CMakeLists.txt
to support SSE4_1 and AVX2.~
I tried to update it, but maybe I still need to modify the
CMakeLists.txt
to support SSE4_1 and AVX2.
How's it going? Did you get it to compile?
As I understand the RNNoise contains trained model, but I do not understand where it is stored, how it becomes part of the library?
How's it going? Did you get it to compile?
As I understand the RNNoise contains trained model, but I do not understand where it is stored, how it becomes part of the library?
~Supporting SSE4_1 and AVX2 is not in my plans now.~ Do you need SSE4_1 or AVX2?
Yes, you should download the model and convert it to header file. Or you can download the headr file from the releases directly.
Maybe the repo can give you some helps: https://github.com/pengzhendong/pyrnnoise/blob/master/CMakeLists.txt
Thanks. I'll look into it.
Thanks for looking at this, @pengzhendong !
I had a couple of questions but you don't have issues enabled on your repo, so I hope it's OK to reply here.
Do you need SSE4_1 or AVX2?
Are these the new SIMD enhancements? I think they might be important, if so.
Sadly, I was unable to build from your fork:
[ 83s] cd /home/abuild/rpmbuild/BUILD/noise-suppression-for-voice-0+git20240424.a6af0b4/build/src/rnnoise && /usr/bin/cc -I/home/abuild/rpmbuild/BUILD/noise-suppression-for-voice-0+git20240424.a6af0b4/src/rnnoise/include -I/home/abuild/rpmbuild/BUILD/noise-suppression-for-voice-0+git20240424.a6af0b4/src/rnnoise/src -O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -O2 -g -DNDEBUG -fPIC -w -MD -MT src/rnnoise/CMakeFiles/RnNoise.dir/src/nnet.c.o -MF CMakeFiles/RnNoise.dir/src/nnet.c.o.d -o CMakeFiles/RnNoise.dir/src/nnet.c.o -c /home/abuild/rpmbuild/BUILD/noise-suppression-for-voice-0+git20240424.a6af0b4/src/rnnoise/src/nnet.c
[ 83s] In file included from /home/abuild/rpmbuild/BUILD/noise-suppression-for-voice-0+git20240424.a6af0b4/src/rnnoise/include/rnnoise/vec_avx.h:38,
[ 83s] from /home/abuild/rpmbuild/BUILD/noise-suppression-for-voice-0+git20240424.a6af0b4/src/rnnoise/include/rnnoise/vec.h:40,
[ 83s] from /home/abuild/rpmbuild/BUILD/noise-suppression-for-voice-0+git20240424.a6af0b4/src/rnnoise/src/nnet.c:40:
[ 83s] /home/abuild/rpmbuild/BUILD/noise-suppression-for-voice-0+git20240424.a6af0b4/src/rnnoise/include/rnnoise/x86/x86cpu.h:40:10: fatal error: common.h: No such file or directory
[ 83s] 40 | #include "common.h"
[ 83s] | ^~~~~~~~~~
[ 83s] compilation terminated.
Of course, the common.h file is present, but I notice the directory is not included in the linker call, so perhaps this is why it cannot find it.
I don't know cmake really at all, so I'm not sure if I did it right, but I do have a working fix for this. I had to add the include directories to the config. . I would drop you a PR, but I honestly have no idea if this is 'correct'? Maybe the relevant files should be added to RN_NOISE_SRC
, I just don't know anything about cmake.
There is one outstanding error I receive during the build now, because I am packaging it, the packaging system notices a dupe, which is the result of an old bug, where the lv2 plugins aren't named separately, so the stereo one doesn't work - and apparently it's actually just building the mono .so
again, because rpm builds say it's binary identical:
[ 223s] lv2-rnnoise.x86_64: W: files-duplicate /usr/lib64/lv2/rnnoise_stereo.lv2/ui.ttl /usr/lib64/lv2/rnnoise_mono.lv2/ui.ttl
[ 223s] Your package contains duplicated files that are not hard- or symlinks. You
[ 223s] should use the %fdupes macro to link the files to one.
[ 223s]
I have a quick-fix patch for this which I submitted (and was merged) to the simd fork and posted here, which simply appends the plugin URL with the channel count. This is not as elegant as some nice logic which appends it with a different text string so it's called -mono and -stereo or something, but -1ch and -2ch works fine :)
Note that this is not only a packaging issue, the plugin really is broken without a fix like this, you will get errors from lv2 when attempting to use the stereo plugin, as per the linked issues. If you like, I could file a PR on your repo for this, too?
Let me know if I can help in any way! :) And thanks again @pengzhendong !
@pallaswept I think it's better to remove the header files from SRC, and add them to the include directories.
Could you run this repo?
https://github.com/pengzhendong/pyrnnoise/blob/master/CMakeLists.txt
@pallaswept I think it's better to remove the header files from SRC, and add them to the include directories.
Could you run this report?
https://github.com/pengzhendong/pyrnnoise/blob/master/CMakeLists.txt
Yes that one works for me :)
Sorry I took so long to reply, I had a big build running.
@rejedai @pallaswept It supports SIMD now.
@rejedai @pallaswept It supports SIMD now.
Fantastic! I have an OpenSUSE package for this, I'll update right away. I also did some performance testing, comparing with the simd fork of rnnoise 0.1. with interesting results, this will make it even more interesting :D
Thanks so much for this one @pengzhendong , really excellent that someone would keep this alive.
@rejedai @pallaswept It supports SIMD now.
Oddly, I do not see any performance change with this new version, compared to your previous. I wonder if the code is correctly detecting the CPU capabilities?
Here is your previous version
And your new version with -DBUILD_RTCD:BOOL=ON
(I checked to confirm that the files were built)
For reference, here is JellyBrick's fork of the old rnnoise 0.1 with SIMD enhancements.
And here is the original werman noise suppression plugin:
So, there seems to be some performance regression between 0.1 and 0.2, but, there is no performance gain with your new SIMD patches, nor really with JellyBrick's. I wonder if maybe it is failing to detect my CPU's SIMD extensions? It's a 5900X.
I hope this is interesting and helpful for you.
@pallaswept ~It may be necessary to explicitly specify the level of optimization.~
Plz try:
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O3")
I'm afraid the result was the same, when building with -O3.
Sorry for the delay in replying (again!), I thought I had replied already but it must not have posted.
I'm afraid the result was the same, when building with -O3.
Sorry for the delay in replying (again!), I thought I had replied already but it must not have posted.
@pallaswept Modern compilers will fully optimize the code when compiling with -O3, so that there is not much difference in performance without using SIMD instructions. You may be able to see the difference in debug mode.
I'm afraid the result was the same, when building with -O3. Sorry for the delay in replying (again!), I thought I had replied already but it must not have posted.
@pallaswept Modern compilers will fully optimize the code when compiling with -O3, so that there is not much difference in performance without using SIMD instructions. You may be able to see the difference in debug mode.
Thanks so much for making that. Like you said, I only saw a measurable difference in debug mode. Even -O2 had the same effect.
That's a super cool demo, very straightforward, clearly illustrates results. gcc behaves in a similar way with FPU instructions and the like, on microcontrollers, so this is familiar to me now. I can see why you initially did not bother with it!
Would it be helpful if I send you a PR for the LV2 bug? I feel like maybe you have a better fix in mind :)
Would it be helpful if I send you a PR for the LV2 bug?
@pallaswept It's very helpful, thanks.
Done! The patch is a little different, I made the extra effort to make the URI have "mono" and "stereo" this time. I have tested it and it works very nicely. I am terrible at cmake, though :rofl:
Thanks for porting this! Our hero :)
There is a new RNNoise v0.2 released 2 days ago. Is there any plan to update the VST plugin to this version?
Link: https://github.com/xiph/rnnoise