Support multiple files for AEC dump.

content/renderer/media/webrtc_audio_device_impl.cc

Index: content/renderer/media/webrtc_audio_device_impl.cc
diff --git a/content/renderer/media/webrtc_audio_device_impl.cc b/content/renderer/media/webrtc_audio_device_impl.cc
index 293ff36c15cc52bb58e0edc8c0f0ae7ef91255a0..9dddd4289349a1d23f6a2a551a559e4fc525baab 100644
--- a/content/renderer/media/webrtc_audio_device_impl.cc
+++ b/content/renderer/media/webrtc_audio_device_impl.cc
@@ -18,6 +18,10 @@
 using media::AudioParameters;
 using media::ChannelLayout;
 
+namespace {
+  int g_capturer_id_counter = 0;

[no longer working on chromium, 2014/06/16 18:30:55]

why? can't it be a member instead?

[Henrik Grunell, 2014/06/17 20:21:41]

Right, it can. Done.

+}
+
 namespace content {
 
 WebRtcAudioDeviceImpl::WebRtcAudioDeviceImpl()
@@ -227,6 +231,16 @@ int32_t WebRtcAudioDeviceImpl::Init() {
   DVLOG(1) << "WebRtcAudioDeviceImpl::Init()";
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  if (MediaStreamAudioProcessor::IsAudioTrackProcessingEnabled()) {

[no longer working on chromium, 2014/06/16 18:30:55]

WebRtcAudioDeviceImpl is not the right place for all these AEC dump code, they
should be in MediaStreamAudioProcessor, that says, we should allocate one aec
dump file descriptor for each MediaStreamAudioProcessor. Right now you code
allocates one dump file desc for each capturer, this is not completely correct.

[Henrik Grunell, 2014/06/17 20:21:41]

After discussing offline I understand that this is required for future planned
changes of MSAP and the capturer. Done.

+    aec_dump_message_filter_ = AecDumpMessageFilter::Get();
+    DCHECK(aec_dump_message_filter_);

[no longer working on chromium, 2014/06/16 18:30:55]

you don't need this DCHECK(), the following line will crash if it is NULL

[Henrik Grunell, 2014/06/17 20:21:41]

Done.

+    aec_dump_message_filter_->SetObserver(this);
+    for (CapturerMap::const_iterator iter = capturers_.begin();
+         iter != capturers_.end(); ++iter) {
+        RegisterAecDumpForId(iter->first);

[no longer working on chromium, 2014/06/16 18:30:55]

indentation.

[Henrik Grunell, 2014/06/17 20:21:41]

Done.

+    }
+  }
+
   // We need to return a success to continue the initialization of WebRtc VoE
   // because failure on the capturer_ initialization should not prevent WebRTC
   // from working. See issue http://crbug.com/144421 for details.
@@ -249,17 +263,22 @@ int32_t WebRtcAudioDeviceImpl::Terminate() {
   DCHECK(!renderer_.get() || !renderer_->IsStarted())
       << "The shared audio renderer shouldn't be running";
 
+  if (aec_dump_message_filter_) {
+    aec_dump_message_filter_->SetObserver(NULL);

[no longer working on chromium, 2014/06/16 18:30:55]

if you have more than 1 tab using webrtc, this SetObserver method will try to
overwrite each other, is it OK?

[Henrik Grunell, 2014/06/17 20:21:41]

This is changed now. MSAP is a delegate instead.

+    aec_dump_message_filter_ = NULL;
+  }
+
   DisableAecDump();
 
   // Stop all the capturers to ensure no further OnData() and
   // RemoveAudioCapturer() callback.
   // Cache the capturers in a local list since WebRtcAudioCapturer::Stop()
   // will trigger RemoveAudioCapturer() callback.
-  CapturerList capturers;
+  CapturerMap capturers;
   capturers.swap(capturers_);
-  for (CapturerList::const_iterator iter = capturers.begin();
+  for (CapturerMap::const_iterator iter = capturers.begin();
        iter != capturers.end(); ++iter) {
-    (*iter)->Stop();
+    iter->second->Stop();
   }
 
   initialized_ = false;
@@ -450,6 +469,32 @@ int32_t WebRtcAudioDeviceImpl::PlayoutSampleRate(
   return 0;
 }
 
+void WebRtcAudioDeviceImpl::OnAecDumpFile(
+    int id,
+    const IPC::PlatformFileForTransit& file_handle) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  base::File file = IPC::PlatformFileForTransitToFile(file_handle);
+  DCHECK(file.IsValid());
+
+  CapturerMap::iterator iter = capturers_.find(id);
+  if (iter != capturers_.end()) {
+    iter->second->StartAecDump(file.Pass());
+  } else {
+    // The capturer has been removed.
+    file.Close();
+  }
+}
+
+void WebRtcAudioDeviceImpl::OnDisableAecDump() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DisableAecDump();
+}
+
+void WebRtcAudioDeviceImpl::OnIpcClosed() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  aec_dump_message_filter_ = NULL;
+}
+
 bool WebRtcAudioDeviceImpl::SetAudioRenderer(WebRtcAudioRenderer* renderer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(renderer);
@@ -471,17 +516,18 @@ void WebRtcAudioDeviceImpl::AddAudioCapturer(
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(capturer.get());
   DCHECK(!capturer->device_id().empty());
+
+  int id = g_capturer_id_counter++;

[Tom Sepez, 2014/06/16 17:33:18]

why is it OK to increment this outside the lock?

[Henrik Grunell, 2014/06/17 20:21:41]

This has been removed, ids are handled in the message filter.

   {
     base::AutoLock auto_lock(lock_);
-    DCHECK(std::find(capturers_.begin(), capturers_.end(), capturer) ==
-        capturers_.end());
-    capturers_.push_back(capturer);
+    DCHECK(capturers_.find(id) == capturers_.end());
+    for (CapturerMap::const_iterator iter = capturers_.begin();
+         iter != capturers_.end(); ++iter) {
+      DCHECK(capturer != iter->second);
+    }
+    capturers_[id] = capturer;
   }
-
-  // Start the Aec dump if the Aec dump has been enabled and has not been
-  // started.
-  if (aec_dump_file_.IsValid())
-    MaybeStartAecDump();
+  RegisterAecDumpForId(id);
 }
 
 void WebRtcAudioDeviceImpl::RemoveAudioCapturer(
@@ -489,16 +535,38 @@ void WebRtcAudioDeviceImpl::RemoveAudioCapturer(
   DVLOG(1) << "WebRtcAudioDeviceImpl::AddAudioCapturer()";
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(capturer.get());
-  base::AutoLock auto_lock(lock_);
-  capturers_.remove(capturer);
+
+  int id = -1;
+  {
+    base::AutoLock auto_lock(lock_);
+    for (CapturerMap::iterator iter = capturers_.begin();
+         iter != capturers_.end(); ++iter) {
+      if (iter->second == capturer) {
+        id = iter->first;
+        capturers_.erase(iter);
+        break;
+      }
+    }
+  }
+  DCHECK_GE(id, 0);
+
+  if (aec_dump_message_filter_) {
+    aec_dump_message_filter_->io_message_loop()->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &AecDumpMessageFilter::UnregisterAecDumpConsumer,
+            aec_dump_message_filter_,
+            id));
+  }
 }
 
 scoped_refptr<WebRtcAudioCapturer>
 WebRtcAudioDeviceImpl::GetDefaultCapturer() const {
   base::AutoLock auto_lock(lock_);
   // Use the last |capturer| which is from the latest getUserMedia call as
-  // the default capture device.
-  return capturers_.empty() ? NULL : capturers_.back();
+  // the default capture device. The last capturer will have the largest key,
+  // so it will be last entry in the map.
+  return capturers_.empty() ? NULL : capturers_.rbegin()->second;
 }
 
 void WebRtcAudioDeviceImpl::AddPlayoutSink(
@@ -533,48 +601,24 @@ bool WebRtcAudioDeviceImpl::GetAuthorizedDeviceInfoForAudioRenderer(
       session_id, output_sample_rate, output_frames_per_buffer);
 }
 
-void WebRtcAudioDeviceImpl::EnableAecDump(base::File aec_dump_file) {
+void WebRtcAudioDeviceImpl::RegisterAecDumpForId(int id) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(aec_dump_file.IsValid());
-
-  // Close the previous AEC dump file description if it has not been consumed.
-  // This can happen if no getUserMedia has been made yet.
-  // TODO(xians): DCHECK(!aec_dump_file_.IsValid()) after the browser
-  // guarantees it won't call EnableAecDump() more than once in a row.
-  if (aec_dump_file_.IsValid())
-    aec_dump_file_.Close();
-
-  aec_dump_file_ = aec_dump_file.Pass();
-  MaybeStartAecDump();
+  if (aec_dump_message_filter_) {
+    aec_dump_message_filter_->io_message_loop()->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &AecDumpMessageFilter::RegisterAecDumpConsumer,
+            aec_dump_message_filter_,
+            id));
+  }
 }
 
 void WebRtcAudioDeviceImpl::DisableAecDump() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  // Simply invalidate the |aec_dump_file_| if we have not pass the ownership
-  // to WebRtc.
-  if (aec_dump_file_.IsValid()) {
-    aec_dump_file_.Close();
-    return;
-  }
-
-  // We might have call StartAecDump() on one of the capturer. Loop
-  // through all the capturers and call StopAecDump() on each of them.
-  for (CapturerList::const_iterator iter = capturers_.begin();
+  for (CapturerMap::const_iterator iter = capturers_.begin();
        iter != capturers_.end(); ++iter) {
-    (*iter)->StopAecDump();
+    iter->second->StopAecDump();
   }
 }
 
-void WebRtcAudioDeviceImpl::MaybeStartAecDump() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(aec_dump_file_.IsValid());
-
-  // Start the Aec dump on the current default capturer.
-  scoped_refptr<WebRtcAudioCapturer> default_capturer(GetDefaultCapturer());
-  if (!default_capturer)
-    return;
-
-  default_capturer->StartAecDump(aec_dump_file_.Pass());
-}
-
 }  // namespace content