Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Xiph's RNNoise v0.2 #192

Closed
alfureu opened this issue Apr 16, 2024 · 17 comments
Closed

New Xiph's RNNoise v0.2 #192

alfureu opened this issue Apr 16, 2024 · 17 comments

Comments

@alfureu
Copy link

alfureu commented Apr 16, 2024

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

@pengzhendong
Copy link

pengzhendong commented Apr 23, 2024

I tried to update it #194. but maybe I still need to modify the CMakeLists.txt to support SSE4_1 and AVX2.

https://github.com/pengzhendong/noise-suppression-for-voice

@rejedai
Copy link

rejedai commented Apr 26, 2024

I tried to update it, but maybe I still need to modify the CMakeLists.txt to support SSE4_1 and AVX2.

https://github.com/pengzhendong/noise-suppression-for-voice

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?

@pengzhendong
Copy link

pengzhendong commented Apr 26, 2024

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

@rejedai
Copy link

rejedai commented Apr 26, 2024

Thanks. I'll look into it.

@pallaswept
Copy link
Contributor

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 !

@pengzhendong
Copy link

pengzhendong commented May 2, 2024

@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
Copy link
Contributor

@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.

@pengzhendong
Copy link

@rejedai @pallaswept It supports SIMD now.

https://github.com/pengzhendong/noise-suppression-for-voice/commit/4e208c14939a569fa772fb06b806142d5997b440

@pallaswept
Copy link
Contributor

@rejedai @pallaswept It supports SIMD now.

pengzhendong@4e208c1

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.

@pallaswept
Copy link
Contributor

@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
Screenshot_2_PZD

And your new version with -DBUILD_RTCD:BOOL=ON (I checked to confirm that the files were built)
Screenshot_4_PZD

For reference, here is JellyBrick's fork of the old rnnoise 0.1 with SIMD enhancements.
Screenshot_4_JB

And here is the original werman noise suppression plugin:
Screenshot_4_werman

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.

@pengzhendong
Copy link

pengzhendong commented May 7, 2024

@pallaswept It may be necessary to explicitly specify the level of optimization.

Plz try:

set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O3")

@pallaswept
Copy link
Contributor

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.

@pengzhendong
Copy link

pengzhendong commented May 8, 2024

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.

https://github.com/pengzhendong/simd/blob/master/run.sh

@pallaswept
Copy link
Contributor

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.

https://github.com/pengzhendong/simd/blob/master/run.sh

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 :)

@pengzhendong
Copy link

pengzhendong commented May 8, 2024

Would it be helpful if I send you a PR for the LV2 bug?

@pallaswept It's very helpful, thanks.

@pallaswept
Copy link
Contributor

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 🤣

Thanks for porting this! Our hero :)

@werman
Copy link
Owner

werman commented May 19, 2024

https://github.com/werman/noise-suppression-for-voice/releases/tag/v1.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants