Skip to content

Commit

Permalink
Merge PR mumble-voip#6292: CHANGE(client): Exclude and discourage RNN…
Browse files Browse the repository at this point in the history
…oise

Due to RNNoise being unmaintained and the library's code being in very poor shape, this commit excludes the RNNoise feature from Mumble by default and discourages its use.

The library contains Opus and CELT symbols (probably due to copy&paste of code) that can end up being called from Opus instead of its own versions of these functions. This can lead to completely unforeseen behavior, including crashes.
An example of this is as of writing this, enabling RNNoise on macOS leads to a crash of Mumble pretty much as soon as it starts up with an "invalid instruction" error. The reason being that part of RNNoise's implementation of one of Opus's symbols contains a code path that produces an invalid instruction in optimized builds (and a segfault in debug builds) and this code path is taken when Opus (wrongly) uses this function instead of its own.

Fixes mumble-voip#6041
  • Loading branch information
Krzmbrzl authored Dec 31, 2023
2 parents ead6fc9 + 694c4fb commit dac3337
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 5 deletions.
2 changes: 1 addition & 1 deletion docs/dev/build-instructions/cmake_options.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ Build redacted (outdated) plugins as well
### rnnoise

Use RNNoise for machine learning noise reduction.
(Default: ON)
(Default: OFF)

### server

Expand Down
15 changes: 13 additions & 2 deletions installer/ClientInstaller.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
public struct Features {
public bool overlay;
public bool g15;
public bool rnnoise;
}

public class ClientInstaller : MumbleInstall {
Expand Down Expand Up @@ -86,11 +87,14 @@ public ClientInstaller(string version, string arch, Features features) {
// 64 bit
this.Platform = WixSharp.Platform.x64;
binaries = new List<string>() {
"rnnoise.dll",
"speexdsp.dll",
"mumble.exe",
};

if (features.rnnoise) {
binaries.Add("rnnoise.dll");
}

if (features.overlay) {
binaries.Add("mumble_ol.dll");
binaries.Add("mumble_ol_helper.exe");
Expand All @@ -105,11 +109,14 @@ public ClientInstaller(string version, string arch, Features features) {
// 32 bit
this.Platform = WixSharp.Platform.x86;
binaries = new List<string>() {
"rnnoise.dll",
"speexdsp.dll",
"mumble.exe",
};

if (features.rnnoise) {
binaries.Add("rnnoise.dll");
}

if (features.overlay) {
binaries.Add("mumble_ol.dll");
binaries.Add("mumble_ol_helper.exe");
Expand Down Expand Up @@ -214,6 +221,10 @@ public static void Main(string[] args) {
if (args[i] == "--overlay") {
features.overlay = true;
}

if (args[i] == "--rnnoise") {
features.rnnoise = true;
}
}

if (version != null && arch != null) {
Expand Down
9 changes: 8 additions & 1 deletion src/mumble/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ option(translations "Include languages other than English." ON)
option(bundle-qt-translations "Bundle Qt's translations as well" ${static})

option(bundled-speex "Build the included version of Speex instead of looking for one on the system." ON)
option(rnnoise "Use RNNoise for machine learning noise reduction." ON)
option(rnnoise "Use RNNoise for machine learning noise reduction." OFF)
option(bundled-rnnoise "Build the included version of RNNoise instead of looking for one on the system." ${rnnoise})
option(bundled-json "Build the included version of nlohmann_json instead of looking for one on the system" ON)

Expand Down Expand Up @@ -726,6 +726,7 @@ else()
endif()

if(rnnoise)
message(WARNING "Using RNNoise is discouraged and can lead to unforeseen behavior, including crashes at runtime due to the RNNoise library exposing Opus/CELT symbols")
target_compile_definitions(mumble_client_object_lib PRIVATE "USE_RNNOISE")

if(bundled-rnnoise)
Expand Down Expand Up @@ -1165,6 +1166,12 @@ if(packaging AND WIN32)
)
endif()

if(rnnoise)
list(APPEND installer_vars
"--rnnoise"
)
endif()

file(COPY
${CMAKE_SOURCE_DIR}/installer/MumbleInstall.cs
${CMAKE_SOURCE_DIR}/installer/ClientInstaller.cs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def getDefaultValueForType(dataType):
elif dataType in ["IdleAction"]:
return "Settings::Deafen"
elif dataType in ["NoiseCancel"]:
return "Settings::NoiseCancelBoth"
return "Settings::NoiseCancelOff"
elif dataType in ["EchoCancelOptionID"]:
return "EchoCancelOptionID::SPEEX_MULTICHANNEL"
elif dataType in ["QuitBehavior"]:
Expand Down

0 comments on commit dac3337

Please sign in to comment.