Added an "enable-audio-processor" flag and WebRtcAudioProcessor class

content/renderer/media/webrtc_audio_processor.h

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_RENDERER_MEDIA_WEBRTC_AUDIO_PROCESSOR_H_
+#define CONTENT_RENDERER_MEDIA_WEBRTC_AUDIO_PROCESSOR_H_
+
+#include "base/synchronization/lock.h"
+#include "content/common/content_export.h"
+#include "media/base/audio_converter.h"
+#include "third_party/libjingle/source/talk/app/webrtc/mediaconstraintsinterface.h"
+#include "third_party/webrtc/modules/audio_processing/include/audio_processing.h"
+#include "third_party/webrtc/modules/interface/module_common_types.h"
+
+namespace media {
+class AudioBus;
+class AudioFifo;
+class AudioParameters;
+}  // namespace media
+
+namespace webrtc {
+class AudioFrame;
+}
+
+namespace content {
+
+// This class owns an object of webrtc::AudioProcessing, it enables the audio
+// processing components based on the constraints, process the data and output

[Henrik Grunell, 2013/11/01 07:22:36]

I would suggest: "...processes the data and outputs it in a unit of 10 ms data
chunk."

(Language corrections + suggest to simple write "it" instead of repeating.)

[no longer working on chromium, 2013/11/01 11:44:43]

Done.

+// the post-processed data in a unit of 10ms data chunk.
+class CONTENT_EXPORT WebRtcAudioProcessor {

[Henrik Grunell, 2013/11/01 07:22:36]

Is this class thread safe? Does it live on one thread only?

[no longer working on chromium, 2013/11/01 11:44:43]

No, I can add comment to explain which thread the methods are called.

+ public:
+  explicit WebRtcAudioProcessor(
+      const webrtc::MediaConstraintsInterface* constraints);
+  ~WebRtcAudioProcessor();
+
+  // Pushes in capture data for processing.

[Henrik Grunell, 2013/11/01 07:22:36]

How much data? (In ms.) 10 ms? Everything in |audio_bus|?

[no longer working on chromium, 2013/11/01 11:44:43]

everything in the |audio_bus|

+  void PushCaptureData(media::AudioBus* audio_source);
+
+  // Processes a block of 10ms data and output the post-processed data via

[Henrik Grunell, 2013/11/01 07:22:36]

"Processes a block of 10 ms data and outputs it via"

[no longer working on chromium, 2013/11/01 11:44:43]

Done.

+  // |out|. The output data format is exposed via |sample_rate|,

[Henrik Grunell, 2013/11/01 07:22:36]

I don't understand "The output data format is exposed via...". What are the
variables sample_rate etc. they referring to? They are not in this function.

[no longer working on chromium, 2013/11/01 11:44:43]

Old comment, I removed the code but forgot the update the comment.
Removed now.

+  // |number_of_channels| and |number_of_frames|.
+  // Returns true if it has 10ms data for processing, otherwise false.

[Henrik Grunell, 2013/11/01 07:22:36]

"10 ms"

It's a bit unclear. I assume returning false means that it hasn't processed
anything since it doesn't have enough input data? And there is nothing written
to the output? Or has a part been processed or written? Please clarify the
comment.

[no longer working on chromium, 2013/11/01 11:44:43]

Done.

+  bool ProcessAndConsume10MsData(int capture_audio_delay_ms,
+                                 int volume,
+                                 bool key_pressed,
+                                 int16** out);
+
+  // Called when the format of the capture data has changed.
+  void SetCaptureFormat(const media::AudioParameters& source_params);
+
+  // Feed render audio to AudioProcessing for analysis. This is needed
+  // iff echo processing is enabled.
+  void FeedRenderDataToAudioProcessing(const int16* render_audio,

[Henrik Grunell, 2013/11/01 07:22:36]

Maybe it should be named as for capture: PushRenderData?

"ToAudioProcessing" is redundant since that's was the class is about.

How much data? (In ms.) Is it determined by |number_of_frames|? Is a frame
always 10 ms?

[no longer working on chromium, 2013/11/01 11:44:43]

Done.

+                                       int sample_rate,
+                                       int number_of_channels,
+                                       int number_of_frames,
+                                       int render_delay_ms);
+
+  // The audio format of the output from the processor.
+  const media::AudioParameters& OutputFormat() const;
+
+  // Accessor to check if the audio processing is enabled or not.

[Henrik Grunell, 2013/11/01 07:22:36]

It can't be set, right? So, in what cases is it enabled and in what cases not?
"has_audio_processing()" sounds to me that, if returns false, it doesn't have
the capabilities to do processing at all (i.e. not possible to enable). Maybe
clarify name and/or comment.

[no longer working on chromium, 2013/11/01 11:44:43]

When the constraints are all set to be false, there is no audio processing in
this class, and this class will work like a FIFO.

+  bool has_audio_processing() const { return audio_processing_.get() != NULL; }
+
+ private:
+  class WebRtcAudioConverter;
+
+  // Helper to initialize the WebRtc AudioProcessing.
+  void InitializeAudioProcessingModule(
+      const webrtc::MediaConstraintsInterface* constraints);
+
+  // Helper to initialize the render converter.
+  void InitializeRenderConverterIfNeeded(int sample_rate,
+                                         int number_of_channels,
+                                         int frames_per_buffer);
+
+  // Called by ProcessAndConsume10MsData().
+  void ProcessData(int audio_delay_milliseconds,
+                   int volume,
+                   bool key_pressed);
+
+  // Called when the processor is going away.
+  void StopAudioProcessing();
+
+  // Cached value for the render delay latency.
+  int render_delay_ms_;
+
+  // Protects |render_delay_ms_|.
+  // TODO(xians): Can we get rid of the lock?
+  mutable base::Lock lock_;
+
+  scoped_ptr<webrtc::AudioProcessing> audio_processing_;

[Henrik Grunell, 2013/11/01 07:22:36]

Comment.

[no longer working on chromium, 2013/11/01 11:44:43]

Done.

+
+  // Converter used for the down-mixing and resampling of the capture data.
+  scoped_ptr<WebRtcAudioConverter> capture_converter_;
+
+  // Converter used for the down-mixing and resampling of the render data when
+  // the AEC is enabled.
+  scoped_ptr<WebRtcAudioConverter> render_converter_;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_RENDERER_MEDIA_WEBRTC_AUDIO_PROCESSOR_H_