Replace ReNameNoise with RNNoise version 0.2

jmvalin commented 5 months ago

Context

Noise suppression

Description

I'm the RNNoise author. Just to let you know that version 0.2 is out, which should significantly improve audio quality, including for reverberant speech. As an additional benefit, the new models shipped with 0.2 are entirely free (trained with publicly available data). Also, the symbols should no longer clash with Opus.

Mumble component

Client

OS-specific?

Yes

Additional information

No response

Krzmbrzl commented 1 month ago

@Hartmnt what's your take on this?

Hartmnt commented 1 month ago

My take on this is:

I am open for reviewing a PR for this. The PR must

compile properly on all of our supported platforms https://github.com/xiph/rnnoise/issues/222 https://github.com/xiph/rnnoise/issues/223
compile properly with CMake
not contain any duplicated opus/celt/util symbols https://gitlab.xiph.org/xiph/rnnoise/-/issues/2
Since the model has changed, the PR needs to prove that the audio quality of the new version is as good or better than the old model. Using multiple audio devices on multiple platforms. Providing examples. https://gitlab.xiph.org/xiph/rnnoise/-/issues/12
to be completed before #6299 or #5408 is done, and therefore only serves as a stop gap

When a PR is submitted for this I will happily verify all of the above. Sadly, I lack the time and also motivation to do it myself. :)

jmvalin commented 1 month ago

* compile properly on all of our supported platforms [RNNoise v0.2 does not compile on Mac and Linux xiph/rnnoise#222](https://github.com/xiph/rnnoise/issues/222) [Possible to support basic (non-VSX) Altivec? xiph/rnnoise#223](https://github.com/xiph/rnnoise/issues/223)

AFAIK the build issues in v0.2 are fixed now (let me know otherwise). There's optimizations for SSE/AVX and Neon, otherwise it falls back to C. Altivec is unlikely to happen unless someone submits patches.

* compile properly with CMake

Patches welcome. AFAIK v0.1 and ReNameNoise don't have CMake support either. Though if it does, then a patch for RNNoise probably isn't hard to do.

* not contain any duplicated opus/celt/util symbols https://gitlab.xiph.org/xiph/rnnoise/-/issues/2

That was fixed in v0.2.

* Since the model has changed, the PR needs to prove that the audio quality of the new version is as good or better than the old model. Using multiple audio devices on multiple platforms. Providing examples. https://gitlab.xiph.org/xiph/rnnoise/-/issues/12

That issue is now fixed in main and general quality was already up compared to 0.1. That being said there is no such thing as a proof here.

* to be completed before [Add DeepFilterNet noise suppression #6299](https://github.com/mumble-voip/mumble/issues/6299) or [Rewrite audio system using libcrossaudio #5408](https://github.com/mumble-voip/mumble/issues/5408) is done, and therefore only serves as a stop gap

AFAIK (haven't tried it myself) DeepFilterNet should be pretty good, but unless you have someone with 1) audio ML knowledge and 2) C/C++ skills and 3) time to contribute to OSS, then I wouldn't hold my breath on that.

As for libcrossaudio, seems orthogonal to noise suppression, no?

When a PR is submitted for this I will happily verify all of the above. Sadly, I lack the time and also motivation to do it myself. :)

I don't know the Mumble codebase, but considering that RNNoise main is both API and ABI compatible with v0.1, seems like the patch is just "revert switch to ReNameNoise and bump RNNoise version", no?

Anyway, feel free to close this issue if the answers above are incompatible with the Mumble directions.

mumble-voip / mumble