Feed the render data to MediaStreamAudioProcessor and used AudioBus in render callback

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 4422b506233fc37398e9ed1aa72bf6c341694368..e13fd005b009df986e337424f89f8e61fc8492bf 100644
--- a/content/renderer/media/media_stream_audio_processor.cc
+++ b/content/renderer/media/media_stream_audio_processor.cc
@@ -141,8 +141,10 @@ class MediaStreamAudioProcessor::MediaStreamAudioConverter
 MediaStreamAudioProcessor::MediaStreamAudioProcessor(
     const media::AudioParameters& source_params,
     const blink::WebMediaConstraints& constraints,
-    int effects)
+    int effects,
+    WebRtcPlayoutDataSource* playout_data_source)
     : render_delay_ms_(0),
+      playout_data_source_(playout_data_source),
       audio_mirroring_(false) {
   capture_thread_checker_.DetachFromThread();
   render_thread_checker_.DetachFromThread();
@@ -151,7 +153,6 @@ MediaStreamAudioProcessor::MediaStreamAudioProcessor(
 }
 
 MediaStreamAudioProcessor::~MediaStreamAudioProcessor() {
-  DCHECK(main_thread_checker_.CalledOnValidThread());

[tommi (sloooow) - chröme, 2014/02/06 16:47:01]

why removing?
(it seems like a good thing to make sure the object is destructed on the same
thread it was constructed on)

[no longer working on chromium, 2014/02/06 17:20:14]

MediaStreamAudioProcessor is reference counted, and the audio thread can grab a
reference and release it there.

Normally this is not a problem since we usually stop the audio thread before we
destuct the MediaStreamAudioProcessor. But I am afraid we don't really guarantee
it.
I removed this thread check since it does not matter in which thread this
destruction happens.

[tommi (sloooow) - chröme, 2014/02/06 17:58:44]

Can we move construction to the audio thread then as well?

[no longer working on chromium, 2014/02/17 13:27:28]

WebRtcAudioCapturer today creates/deletes its resources in the main render
thread since AudioInputDevice needs to be created/deleted on the main render
thread. And I would like to do this to all the resources instead of splitting
them into parts.

I removed the thread check here since I thought it was not a problem to have the
destruction happens in another thread. But since this introduces concerns, I
added it back.

   StopAudioProcessing();
 }
 
@@ -160,42 +161,11 @@ void MediaStreamAudioProcessor::PushCaptureData(media::AudioBus* audio_source) {
   capture_converter_->Push(audio_source);
 }
 
-void MediaStreamAudioProcessor::PushRenderData(
-    const int16* render_audio, int sample_rate, int number_of_channels,
-    int number_of_frames, base::TimeDelta render_delay) {
-  DCHECK(render_thread_checker_.CalledOnValidThread());
-
-  // Return immediately if the echo cancellation is off.
-  if (!audio_processing_ ||
-      !audio_processing_->echo_cancellation()->is_enabled()) {
-    return;
-  }
-
-  TRACE_EVENT0("audio",
-               "MediaStreamAudioProcessor::FeedRenderDataToAudioProcessing");
-  int64 new_render_delay_ms = render_delay.InMilliseconds();
-  DCHECK_LT(new_render_delay_ms,
-            std::numeric_limits<base::subtle::Atomic32>::max());
-  base::subtle::Release_Store(&render_delay_ms_, new_render_delay_ms);
-
-  InitializeRenderConverterIfNeeded(sample_rate, number_of_channels,
-                                    number_of_frames);
-
-  // TODO(xians): Avoid this extra interleave/deinterleave.
-  render_data_bus_->FromInterleaved(render_audio,
-                                    render_data_bus_->frames(),
-                                    sizeof(render_audio[0]));
-  render_converter_->Push(render_data_bus_.get());
-  while (render_converter_->Convert(&render_frame_))
-    audio_processing_->AnalyzeReverseStream(&render_frame_);
-}
-
 bool MediaStreamAudioProcessor::ProcessAndConsumeData(
     base::TimeDelta capture_delay, int volume, bool key_pressed,
     int* new_volume, int16** out) {
   DCHECK(capture_thread_checker_.CalledOnValidThread());
-  TRACE_EVENT0("audio",
-               "MediaStreamAudioProcessor::ProcessAndConsumeData");
+  TRACE_EVENT0("audio", "MediaStreamAudioProcessor::ProcessAndConsumeData");
 
   if (!capture_converter_->Convert(&capture_frame_))
     return false;
@@ -215,6 +185,25 @@ const media::AudioParameters& MediaStreamAudioProcessor::OutputFormat() const {
   return capture_converter_->sink_parameters();
 }
 
+void MediaStreamAudioProcessor::OnPlayoutData(media::AudioBus* audio_bus,
+                                              int sample_rate,
+                                              int audio_delay_milliseconds) {
+  DCHECK(render_thread_checker_.CalledOnValidThread());
+  DCHECK(audio_processing_->echo_cancellation()->is_enabled());

[tommi (sloooow) - chröme, 2014/02/06 16:47:01]

is this correct?  will we only ever get this callback if aec is enabled?  What
if only agc is turned on etc?

previously there was a check for it that handled it as a supported case but
having a DCHECK means this is a state that should be impossible for us to reach.

[no longer working on chromium, 2014/02/06 17:20:14]

the playout data is only needed when AEC is enabled, and it does nothing to AGC
or any other component.
In Line 264, we add this processor as sink for the playout data only when the
AEC is enabled.

[tommi (sloooow) - chröme, 2014/02/06 17:58:44]

OK, cool.

+
+  TRACE_EVENT0("audio", "MediaStreamAudioProcessor::OnPlayoutData");
+  DCHECK_LT(audio_delay_milliseconds,
+            std::numeric_limits<base::subtle::Atomic32>::max());
+  base::subtle::Release_Store(&render_delay_ms_, audio_delay_milliseconds);
+
+  InitializeRenderConverterIfNeeded(sample_rate, audio_bus->channels(),
+                                    audio_bus->frames());
+
+  render_converter_->Push(audio_bus);
+  while (render_converter_->Convert(&render_frame_))
+    audio_processing_->AnalyzeReverseStream(&render_frame_);
+}
+
 void MediaStreamAudioProcessor::InitializeAudioProcessingModule(
     const blink::WebMediaConstraints& constraints, int effects) {
   DCHECK(!audio_processing_);
@@ -276,6 +265,9 @@ void MediaStreamAudioProcessor::InitializeAudioProcessingModule(
     EnableEchoCancellation(audio_processing_.get());
     if (enable_experimental_aec)
       EnableExperimentalEchoCancellation(audio_processing_.get());
+
+    if (playout_data_source_)
+      playout_data_source_->AddPlayoutSink(this);
   }
 
   if (enable_ns)
@@ -408,6 +400,9 @@ void MediaStreamAudioProcessor::StopAudioProcessing() {
   if (!audio_processing_.get())
     return;
 
+  if (playout_data_source_)
+    playout_data_source_->RemovePlayoutSink(this);
+
   audio_processing_.reset();
 }