CHANGE(client): Exclude and discourage RNNoise

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 #6041
mumble-voip · Dec 30, 2023 · 694c4fb · 694c4fb
1 parent fc3a16b
commit 694c4fb
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 5 deletions.
diff --git a/docs/dev/build-instructions/cmake_options.md b/docs/dev/build-instructions/cmake_options.md
@@ -192,7 +192,7 @@ Build redacted (outdated) plugins as well
 ### rnnoise
 
 Use RNNoise for machine learning noise reduction.
-(Default: ON)
+(Default: OFF)
 
 ### server
 

diff --git a/installer/ClientInstaller.cs b/installer/ClientInstaller.cs
@@ -18,6 +18,7 @@
 public struct Features {
 	public bool overlay;
 	public bool g15;
+	public bool rnnoise;
 }
 
 public class ClientInstaller : MumbleInstall {
@@ -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");
@@ -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");
@@ -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) {

diff --git a/src/mumble/CMakeLists.txt b/src/mumble/CMakeLists.txt
@@ -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)
 
@@ -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)
@@ -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

diff --git a/src/tests/TestSettingsJSONSerialization/generate_test_case.py b/src/tests/TestSettingsJSONSerialization/generate_test_case.py
@@ -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"]: