Added a kEchoCancellation constraint to turn off the audio processing.

content/renderer/media/media_stream_audio_processor.cc

Index: content/renderer/media/media_stream_audio_processor.cc
diff --git a/content/renderer/media/media_stream_audio_processor.cc b/content/renderer/media/media_stream_audio_processor.cc
index c83ffff7eb408417dd9a641fc3c93f92a032119f..eb56c4dfcd75ae675c425c175ea16783385ae672 100644
--- a/content/renderer/media/media_stream_audio_processor.cc
+++ b/content/renderer/media/media_stream_audio_processor.cc
@@ -24,7 +24,6 @@ namespace content {
 namespace {
 
 using webrtc::AudioProcessing;
-using webrtc::MediaConstraintsInterface;
 
 #if defined(OS_ANDROID)
 const int kAudioProcessingSampleRate = 16000;
@@ -157,15 +156,14 @@ class MediaStreamAudioProcessor::MediaStreamAudioConverter
 MediaStreamAudioProcessor::MediaStreamAudioProcessor(
     const blink::WebMediaConstraints& constraints,
     int effects,
-    MediaStreamType type,
     WebRtcPlayoutDataSource* playout_data_source)
     : render_delay_ms_(0),
       playout_data_source_(playout_data_source),
-      audio_mirroring_(false),
-      typing_detected_(false) {
+      goog_audio_mirroring_(false),
+      goog_typing_detected_(false) {
   capture_thread_checker_.DetachFromThread();
   render_thread_checker_.DetachFromThread();
-  InitializeAudioProcessingModule(constraints, effects, type);
+  InitializeAudioProcessingModule(constraints, effects);
 }
 
 MediaStreamAudioProcessor::~MediaStreamAudioProcessor() {
@@ -193,7 +191,7 @@ void MediaStreamAudioProcessor::PushCaptureData(media::AudioBus* audio_source) {
   DCHECK_EQ(audio_source->frames(),
             capture_converter_->source_parameters().frames_per_buffer());
 
-  if (audio_mirroring_ &&
+  if (goog_audio_mirroring_ &&
       capture_converter_->source_parameters().channel_layout() ==
           media::CHANNEL_LAYOUT_STEREO) {
     // Swap the first and second channels.
@@ -271,73 +269,68 @@ void MediaStreamAudioProcessor::OnPlayoutDataSourceChanged() {
 
 void MediaStreamAudioProcessor::GetStats(AudioProcessorStats* stats) {
   stats->typing_noise_detected =
-      (base::subtle::Acquire_Load(&typing_detected_) != false);
+      (base::subtle::Acquire_Load(&goog_typing_detected_) != false);
   GetAecStats(audio_processing_.get(), stats);
 }
 
 void MediaStreamAudioProcessor::InitializeAudioProcessingModule(
-    const blink::WebMediaConstraints& constraints, int effects,
-    MediaStreamType type) {
+    const blink::WebMediaConstraints& constraints, int effects) {
   DCHECK(!audio_processing_);
 
-  RTCMediaConstraints native_constraints(constraints);
+  MediaAudioConstraints audio_constraints(constraints, effects);
 
   // Audio mirroring can be enabled even though audio processing is otherwise
   // disabled.
-  audio_mirroring_ = GetPropertyFromConstraints(
-      &native_constraints, webrtc::MediaConstraintsInterface::kAudioMirroring);
+  goog_audio_mirroring_ = audio_constraints.GetProperty(
+      MediaAudioConstraints::kGoogAudioMirroring);
 
   if (!IsAudioTrackProcessingEnabled()) {
     RecordProcessingState(AUDIO_PROCESSING_IN_WEBRTC);
     return;
   }
 
-  // Only apply the fixed constraints for gUM of MEDIA_DEVICE_AUDIO_CAPTURE.
-  DCHECK(IsAudioMediaType(type));
-  if (type == MEDIA_DEVICE_AUDIO_CAPTURE)
-    ApplyFixedAudioConstraints(&native_constraints);
-
-  if (effects & media::AudioParameters::ECHO_CANCELLER) {
-    // If platform echo canceller is enabled, disable the software AEC.
-    native_constraints.AddMandatory(
-        MediaConstraintsInterface::kEchoCancellation,
-        MediaConstraintsInterface::kValueFalse, true);
+  // |kEchoCancellation| is used as a master control on enabling/disabling
+  // the audio processing.
+  const bool echo_cancellation = audio_constraints.GetProperty(

[ajm, 2014/04/15 01:29:58]

Is this defaulted to true, and only overridden if the user sets it? Otherwise
apps that don't currently set this will suddenly have their audio processing
disabled.

Also, shouldn't enabling echo_cancellation also enable the AEC? Currently,
kGoogEchoCancellation still needs to be specified to do this.

[no longer working on chromium, 2014/04/15 12:43:46]

Yes.
Yes, but this needs to be done after our "user" switch to echo_cancellation.
Otherwise we will run into problem in music mode, where all the goog*
constraints are set to false but echo_cancellation is unset.

[ajm, 2014/04/16 05:17:43]

Ah, OK. I see you have a TODO below to deal with this.

+      MediaAudioConstraints::kEchoCancellation);

[tommi (sloooow) - chröme, 2014/04/17 10:56:41]

It's not clear from the code that the return value will be |true| when the
constraint doesn't exist.  Can you elaborate in the comments here how this
works?

[no longer working on chromium, 2014/04/23 14:59:06]

Done.

FYI, there is also comment in MediaAudioConstraints::GetProperty
// Gets the property of the constraint named by |key| in |constraints_|.
// Returns the constraint's value if the key is found; Otherwise returns the
// default value of the constraint.

+  if (!echo_cancellation) {
+    RecordProcessingState(AUDIO_PROCESSING_DISABLED);
+    return;
   }
 
 #if defined(OS_IOS)
   // On iOS, VPIO provides built-in AEC and AGC.
-  const bool enable_aec = false;
-  const bool enable_agc = false;
+  const bool goog_aec = false;

[ajm, 2014/04/15 01:29:58]

I'm not sure that having these match the constraint name is the best approach.
This variable determines if the AEC will be enabled, so "enable_aec" seems more
clear to me.

[no longer working on chromium, 2014/04/15 12:43:46]

The change is to avoid the confusion between standardized constraint and goog
constraints.
Tommi had a comment in his first review:
If you don't like goog_aec, how about enable_goog_aec?

[ajm, 2014/04/16 05:17:43]

OK, I can buy it.
No, I think that's worse ;)

+  const bool goog_agc = false;
 #else
-  const bool enable_aec = GetPropertyFromConstraints(
-      &native_constraints, MediaConstraintsInterface::kEchoCancellation);
-  const bool enable_agc = GetPropertyFromConstraints(
-      &native_constraints, webrtc::MediaConstraintsInterface::kAutoGainControl);
+  // TODO(xians): goog_aec should be just echo_cancellation.

[tommi (sloooow) - chröme, 2014/04/17 10:56:41]

Here I think we need to only check the value of kGoogEchoCancellation if
kEchoCancellation was *not* explicitly set.  If kEchoCancellation is explicitly
set, then goog_aec can be set to the same value as echo_cancellation.

[no longer working on chromium, 2014/04/23 14:59:06]

Done with taking care the case where kEchoCancellation is set to true, while
kGoogEchoCancellation is false.

FYI, I was hoping our client could switch over to kEchoCancellation, so we don't
need to take care of this conner case. But it turned out that not much code was
required to cover it.

+  const bool goog_aec = audio_constraints.GetProperty(
+      MediaAudioConstraints::kGoogEchoCancellation);
+  const bool goog_agc = audio_constraints.GetProperty(
+      MediaAudioConstraints::kGoogAutoGainControl);
 #endif
 
 #if defined(OS_IOS) || defined(OS_ANDROID)
-  const bool enable_experimental_aec = false;
-  const bool enable_typing_detection = false;
+  const bool goog_experimental_aec = false;
+  const bool goog_typing_detection = false;
 #else
-  const bool enable_experimental_aec = GetPropertyFromConstraints(
-      &native_constraints,
-      MediaConstraintsInterface::kExperimentalEchoCancellation);
-  const bool enable_typing_detection = GetPropertyFromConstraints(
-      &native_constraints, MediaConstraintsInterface::kTypingNoiseDetection);
+  const bool goog_experimental_aec = audio_constraints.GetProperty(
+      MediaAudioConstraints::kGoogExperimentalEchoCancellation);
+  const bool goog_typing_detection = audio_constraints.GetProperty(
+      MediaAudioConstraints::kGoogTypingNoiseDetection);
 #endif
 
-  const bool enable_ns = GetPropertyFromConstraints(
-      &native_constraints, MediaConstraintsInterface::kNoiseSuppression);
-  const bool enable_experimental_ns = GetPropertyFromConstraints(
-        &native_constraints,
-        MediaConstraintsInterface::kExperimentalNoiseSuppression);
-  const bool enable_high_pass_filter = GetPropertyFromConstraints(
-      &native_constraints, MediaConstraintsInterface::kHighpassFilter);
-
-  // Return immediately if no audio processing component is enabled.
-  if (!enable_aec && !enable_experimental_aec && !enable_ns &&
-      !enable_high_pass_filter && !enable_typing_detection && !enable_agc &&
-      !enable_experimental_ns) {
+  const bool goog_ns = audio_constraints.GetProperty(
+      MediaAudioConstraints::kGoogNoiseSuppression);
+  const bool goog_experimental_ns = audio_constraints.GetProperty(
+      MediaAudioConstraints::kGoogExperimentalNoiseSuppression);
+ const bool goog_high_pass_filter = audio_constraints.GetProperty(
+     MediaAudioConstraints::kGoogHighpassFilter);
+
+  // Return immediately if no goog constraint is enabled.

[tommi (sloooow) - chröme, 2014/04/17 10:56:41]

Can you add that in this case the goog constraints would have been explicitly
set to false and that |echo_cancellation| must *not* have been set to false but
must be missing from the set of constraints?
- can we DCHECK that at least?
..actually, if |echo_cancellation| was set to true explicitly, then goog_aec
should be set to true (since the former overrides the latter), but I don't see
that handled here.

[no longer working on chromium, 2014/04/23 14:59:06]

No, we can't. The constraints can be whatever JS developers set to.
now GetPropery() handles it.

+  if (!goog_aec && !goog_experimental_aec && !goog_ns &&
+      !goog_high_pass_filter && !goog_typing_detection &&
+      !goog_agc && !goog_experimental_ns) {
     RecordProcessingState(AUDIO_PROCESSING_DISABLED);
     return;
   }
@@ -346,32 +339,33 @@ void MediaStreamAudioProcessor::InitializeAudioProcessingModule(
   audio_processing_.reset(webrtc::AudioProcessing::Create(0));
 
   // Enable the audio processing components.
-  if (enable_aec) {
+  if (goog_aec) {
     EnableEchoCancellation(audio_processing_.get());
-    if (enable_experimental_aec)
+
+    if (goog_experimental_aec)
       EnableExperimentalEchoCancellation(audio_processing_.get());
 
     if (playout_data_source_)
       playout_data_source_->AddPlayoutSink(this);
   }
 
-  if (enable_ns)
+  if (goog_ns)
     EnableNoiseSuppression(audio_processing_.get());
 
-  if (enable_experimental_ns)
+  if (goog_experimental_ns)
     EnableExperimentalNoiseSuppression(audio_processing_.get());
 
-  if (enable_high_pass_filter)
+  if (goog_high_pass_filter)
     EnableHighPassFilter(audio_processing_.get());
 
-  if (enable_typing_detection) {
+  if (goog_typing_detection) {
     // TODO(xians): Remove this |typing_detector_| after the typing suppression
     // is enabled by default.
     typing_detector_.reset(new webrtc::TypingDetection());
     EnableTypingDetection(audio_processing_.get(), typing_detector_.get());
   }
 
-  if (enable_agc)
+  if (goog_agc)
     EnableAutomaticGainControl(audio_processing_.get());
 
   // Configure the audio format the audio processing is running on. This
@@ -488,7 +482,7 @@ int MediaStreamAudioProcessor::ProcessData(webrtc::AudioFrame* audio_frame,
     bool vad_active =
         (audio_frame->vad_activity_ == webrtc::AudioFrame::kVadActive);
     bool typing_detected = typing_detector_->Process(key_pressed, vad_active);
-    base::subtle::Release_Store(&typing_detected_, typing_detected);
+    base::subtle::Release_Store(&goog_typing_detected_, typing_detected);
   }
 
   // Return 0 if the volume has not been changed, otherwise return the new