Wire up AGC to the MediaStreamAudioProcessor

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 35b8d9fa24ebc69814515ac97a65f10dfd01821e..1c3906fe0242313d5086720451b155168118a369 100644
--- a/content/renderer/media/media_stream_audio_processor.cc
+++ b/content/renderer/media/media_stream_audio_processor.cc
@@ -142,7 +142,8 @@ MediaStreamAudioProcessor::MediaStreamAudioProcessor(
     const media::AudioParameters& source_params,
     const blink::WebMediaConstraints& constraints,
     int effects)
-    : render_delay_ms_(0) {
+    : render_delay_ms_(0),
+      stereo_channels_swapping_(false) {
   capture_thread_checker_.DetachFromThread();
   render_thread_checker_.DetachFromThread();
   InitializeAudioProcessingModule(constraints, effects);
@@ -191,15 +192,17 @@ void MediaStreamAudioProcessor::PushRenderData(
 
 bool MediaStreamAudioProcessor::ProcessAndConsumeData(
     base::TimeDelta capture_delay, int volume, bool key_pressed,
-    int16** out) {
+    int* new_volume, int16** out) {
   DCHECK(capture_thread_checker_.CalledOnValidThread());
   TRACE_EVENT0("audio",
                "MediaStreamAudioProcessor::ProcessAndConsumeData");
+  *new_volume = 0;
 
   if (!capture_converter_->Convert(&capture_frame_))
     return false;
 
-  ProcessData(&capture_frame_, capture_delay, volume, key_pressed);
+  *new_volume = ProcessData(&capture_frame_, capture_delay, volume,
+                            key_pressed);
   *out = capture_frame_.data_;
 
   return true;
@@ -246,10 +249,19 @@ void MediaStreamAudioProcessor::InitializeAudioProcessingModule(
   const bool enable_typing_detection = GetPropertyFromConstraints(
       &native_constraints, MediaConstraintsInterface::kTypingNoiseDetection);
 #endif
+  const bool enable_agc = GetPropertyFromConstraints(
+      &native_constraints, webrtc::MediaConstraintsInterface::kAutoGainControl);
+  // TODO(xians): Comments in mediachannel.h claim that SetAgcConfig is only
+  // available for old AGC, check with ajm@ to see if we support
+  // set_target_level_dbfs(), set_compression_gain_db() and enable_limiter()
+  // from the constraints.

[no longer working on chromium, 2014/01/21 09:05:47]

Andrew, do you know if SetAgcConfig is needed or not? I don't how it is hooked
up with the constraints.

[ajm, 2014/01/22 21:50:55]

No, we don't support these currently. You can remove this TODO.

[no longer working on chromium, 2014/01/23 12:46:08]

Done.

+
+  stereo_channels_swapping_ = GetPropertyFromConstraints(
+      &native_constraints, webrtc::MediaConstraintsInterface::kAudioMirroring);
 
   // 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_high_pass_filter && !enable_typing_detection && !enable_agc) {
     return;
   }
 
@@ -272,6 +284,8 @@ void MediaStreamAudioProcessor::InitializeAudioProcessingModule(
   if (enable_typing_detection)
     EnableTypingDetection(audio_processing_.get());
 
+  if (enable_agc)
+    EnableAutomaticGainControl(audio_processing_.get());
 
   // Configure the audio format the audio processing is running on. This
   // has to be done after all the needed components are enabled.

[ajm, 2014/01/22 21:50:55]

Why is that? Did you hit an error? Typically it would be better to do it first.

And just FYI:
Although until recently it was necessary to explicitly reinitialize,
ProcessStream is now able to handle changes on the fly (it initializes
internally).

set_sample_rate_hz and friends are basically deprecated; instead I'd like to add
an overloaded Initialize() to AudioProcessing which will take these parameters,
which can optionally be called if the user knows the parameters ahead of time.

In your case here it doesn't really matter since you're hardcoding the sample
rates.

[no longer working on chromium, 2014/01/23 12:46:08]

I never hit any problem here. I think the reason why I call set_sample_rate_hz()
after the configuration is because we call set_sample_rate_hz() and
set_num_channels() for each packet. So I assume it is better to do it after the
configuration.

I moved it up after the creation.
Do you mean that I can remove these two setters instead? It will be great if I
can.
How about passing those values via the constructor instead of Initialize() ?

@@ -341,15 +355,15 @@ void MediaStreamAudioProcessor::InitializeRenderConverterIfNeeded(
                                              frames_per_buffer);
 }
 
-void MediaStreamAudioProcessor::ProcessData(webrtc::AudioFrame* audio_frame,
-                                            base::TimeDelta capture_delay,
-                                            int volume,
-                                            bool key_pressed) {
+int MediaStreamAudioProcessor::ProcessData(webrtc::AudioFrame* audio_frame,
+                                           base::TimeDelta capture_delay,
+                                           int volume,
+                                           bool key_pressed) {
   DCHECK(capture_thread_checker_.CalledOnValidThread());
   if (!audio_processing_)
-    return;
+    return 0;
 
-  TRACE_EVENT0("audio", "MediaStreamAudioProcessor::Process10MsData");
+  TRACE_EVENT0("audio", "MediaStreamAudioProcessor::ProcessData");
   DCHECK_EQ(audio_processing_->sample_rate_hz(),
             capture_converter_->sink_parameters().sample_rate());
   DCHECK_EQ(audio_processing_->num_input_channels(),
@@ -370,13 +384,28 @@ void MediaStreamAudioProcessor::ProcessData(webrtc::AudioFrame* audio_frame,
 
   audio_processing_->set_stream_delay_ms(total_delay_ms);
   webrtc::GainControl* agc = audio_processing_->gain_control();
+  // TODO(xians): We used to have a problem with the truncation of the volume.
+  // For example, if the OS has 25 volume steps, and the current volume is 7,
+  // which will be scaled to 70 in [0, 255]. When the AGC tries to adjust to
+  // volume to 76, SetVolume will fail updating the volume due to truncating
+  // the new volume back to 7. WebRtc works around this problem by keeping
+  // track values and forcing AGC to continue its trend.
+  // Check with ajm@ on if we still need the workaround.
   int err = agc->set_stream_analog_level(volume);

[no longer working on chromium, 2014/01/21 09:05:47]

Andrew, could you please take a look at this comment. The question is about a
old issue that the OS volume's precision is too small to let AGC change the
volume, we made a workaround in voe_base_impl but I was told by bjorn that you
guys would fix the problem inside AGC.

[ajm, 2014/01/23 17:30:06]

As discussed off review, you can remove this TODO.

[no longer working on chromium, 2014/01/24 09:10:49]

Done. Thanks.

   DCHECK_EQ(err, 0) << "set_stream_analog_level() error: " << err;
   err = audio_processing_->ProcessStream(audio_frame);
   DCHECK_EQ(err, 0) << "ProcessStream() error: " << err;
 
-  // TODO(xians): Add support for AGC, typing detection, audio level
-  // calculation, stereo swapping.
+  // TODO(xians): Add support for typing detection, audio level calculation.
+
+  if (stereo_channels_swapping_ && audio_frame->num_channels_ == 2) {
+    // TODO(xians): Swap the stereo channels after switching to media::AudioBus.
+  }
+
+  // Return 0 if the volume has not been changed, otherwise return the new
+  // volume.
+  return (agc->stream_analog_level() == volume) ?
+      0 : agc->stream_analog_level();
 }
 
 void MediaStreamAudioProcessor::StopAudioProcessing() {