-
Notifications
You must be signed in to change notification settings - Fork 237
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
Comments
I tried to update it #194. |
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? |
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.
Are these the new SIMD enhancements? I think they might be important, if so. Sadly, I was unable to build from your fork:
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 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
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 |
Yes that one works for me :) Sorry I took so long to reply, I had a big build running. |
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. |
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? And your new version with 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 Plz try:
|
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 :) |
@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 🤣 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
The text was updated successfully, but these errors were encountered: