Bug 1274392 - When echo cancellation is disabled, disable automatic gain control and noise suppression as well. r=jib
authorPaul Adenot <paul@paul.cx>
Wed, 28 Nov 2018 13:41:02 +0000
changeset 504927 33a1a7271fe37f0e5118dc71381f7e29d3809a89
parent 504926 4e55f708271a1394f850d3bbd5a853b283f32184
child 504928 e374d095d1e2d4c5d9c7c1cc412bb1798149d003
child 504955 84081d5487039fdb4aeca8245ed58bfa34d5051b
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjib
bugs1274392
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1274392 - When echo cancellation is disabled, disable automatic gain control and noise suppression as well. r=jib Differential Revision: https://phabricator.services.mozilla.com/D12902
dom/media/tests/mochitest/test_defaultAudioConstraints.html
dom/media/webrtc/MediaEngineWebRTCAudio.cpp
--- a/dom/media/tests/mochitest/test_defaultAudioConstraints.html
+++ b/dom/media/tests/mochitest/test_defaultAudioConstraints.html
@@ -9,33 +9,72 @@
 "use strict";
 
 createHTML({
   title: "Test that the audio constraints that observe at the audio constraints we expect.",
   bug: "1509842"
 });
 
 runTest(async () => {
+  // We need a real device to get a MediaEngine supporting constraints
   let audioDevice = SpecialPowers.getCharPref("media.audio_loopback_dev", "");
   if (!audioDevice) {
-    ok(false, "No device set by framework. Try --use-test-media-devices");
+    todo(false, "No device set by framework. Try --use-test-media-devices");
     return;
   }
 
   // Get a gUM track with the default settings, check that they are what we
   // expect.
   let stream = await navigator.mediaDevices.getUserMedia({ audio: true });
   let track = stream.getAudioTracks()[0];
-  let defaultConstraints = track.getSettings();
+  let defaultSettings = track.getSettings();
 
-  is(defaultConstraints.echoCancellation, true,
+  is(defaultSettings.echoCancellation, true,
       "Echo cancellation should be ON by default.");
-  is(defaultConstraints.noiseSuppression, true,
+  is(defaultSettings.noiseSuppression, true,
       "Noise suppression should be ON by default.");
-  is(defaultConstraints.autoGainControl, true,
+  is(defaultSettings.autoGainControl, true,
       "Automatic gain control should be ON by default.");
 
   track.stop();
+
+  // This is UA-dependant, and belongs in a Mochitest, not in a WPT.
+  // When a gUM track has been requested with `echoCancellation` OFF, check that
+  // `noiseSuppression` and `autoGainControl` are off as well.
+  stream =
+    await navigator.mediaDevices.getUserMedia({audio:{echoCancellation: false}});
+  track = stream.getAudioTracks()[0];
+  defaultSettings = track.getSettings();
+
+  is(defaultSettings.echoCancellation, false,
+      "Echo cancellation should be OFF when requested.");
+  is(defaultSettings.noiseSuppression, false,
+      `Noise suppression should be OFF when echoCancellation is the only
+      constraint and is OFF.`);
+  is(defaultSettings.autoGainControl, false,
+      `Automatic gain control should be OFF when echoCancellation is the only
+      constraint and is OFF.`);
+
+  track.stop();
+
+  // When a gUM track has been requested with `echoCancellation` OFF, check that
+  // `noiseSuppression` and `autoGainControl` are not OFF as well if another
+  // constraint has been specified.
+  stream =
+    await navigator.mediaDevices.getUserMedia({audio:{echoCancellation: false,
+                                                      autoGainControl: true}});
+  track = stream.getAudioTracks()[0];
+  defaultSettings = track.getSettings();
+
+  is(defaultSettings.echoCancellation, false,
+      "Echo cancellation should be OFF when requested.");
+  is(defaultSettings.noiseSuppression, false,
+      `Noise suppression should be OFF when echoCancellation is OFF and another
+      constraint has been specified.`);
+  is(defaultSettings.autoGainControl, true,
+      "Auto gain control should be ON when requested.");
+
+  track.stop();
 });
 </script>
 </pre>
 </body>
 </html>
--- a/dom/media/webrtc/MediaEngineWebRTCAudio.cpp
+++ b/dom/media/webrtc/MediaEngineWebRTCAudio.cpp
@@ -109,23 +109,22 @@ uint32_t MediaEngineWebRTCMicrophoneSour
 }
 
 nsresult MediaEngineWebRTCMicrophoneSource::EvaluateSettings(
     const NormalizedConstraints& aConstraintsUpdate,
     const MediaEnginePrefs& aInPrefs, MediaEnginePrefs* aOutPrefs,
     const char** aOutBadConstraint) {
   AssertIsOnOwningThread();
 
+  FlattenedConstraints c(aConstraintsUpdate);
   MediaEnginePrefs prefs = aInPrefs;
 
-  FlattenedConstraints c(aConstraintsUpdate);
-
   prefs.mAecOn = c.mEchoCancellation.Get(aInPrefs.mAecOn);
-  prefs.mAgcOn = c.mAutoGainControl.Get(aInPrefs.mAgcOn);
-  prefs.mNoiseOn = c.mNoiseSuppression.Get(aInPrefs.mNoiseOn);
+  prefs.mAgcOn = c.mAutoGainControl.Get(aInPrefs.mAgcOn && prefs.mAecOn);
+  prefs.mNoiseOn = c.mNoiseSuppression.Get(aInPrefs.mNoiseOn && prefs.mAecOn);
 
   // Determine an actual channel count to use for this source. Three factors at
   // play here: the device capabilities, the constraints passed in by content,
   // and a pref that can force things (for testing)
   int32_t maxChannels = mDeviceInfo->MaxChannels();
 
   // First, check channelCount violation wrt constraints. This fails in case of
   // error.