Switching AudioInputDeviceManager from using AudioManager interface to AudioSystem one.

content/browser/renderer_host/media/audio_input_device_manager.cc

 // Copyright (c) 2012 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.
 
 #include "content/browser/renderer_host/media/audio_input_device_manager.h"
 
 #include <memory>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/metrics/histogram_macros.h"
 #include "build/build_config.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/media_stream_request.h"
 #include "media/audio/audio_input_ipc.h"
-#include "media/audio/audio_manager_base.h"
+#include "media/audio/audio_system.h"
 #include "media/base/audio_parameters.h"
 #include "media/base/channel_layout.h"
 #include "media/base/media_switches.h"
 
 #if defined(OS_CHROMEOS)
 #include "chromeos/audio/cras_audio_handler.h"
 #endif
 
 namespace content {
 
 const int AudioInputDeviceManager::kFakeOpenSessionId = 1;
 
 namespace {
 // Starting id for the first capture session.
 const int kFirstSessionId = AudioInputDeviceManager::kFakeOpenSessionId + 1;
 }
 
 AudioInputDeviceManager::AudioInputDeviceManager(
-    media::AudioManager* audio_manager)
+    media::AudioSystem* audio_system)
     : next_capture_session_id_(kFirstSessionId),
 #if defined(OS_CHROMEOS)
       keyboard_mic_streams_count_(0),
 #endif
-      audio_manager_(audio_manager),
-      device_task_runner_(audio_manager_->GetTaskRunner()) {
+      audio_system_(audio_system) {
 }
 
 AudioInputDeviceManager::~AudioInputDeviceManager() {
 }
 
 const StreamDeviceInfo* AudioInputDeviceManager::GetOpenedDeviceInfoById(
     int session_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   StreamDeviceList::iterator device = GetDevice(session_id);
   if (device == devices_.end())
     return nullptr;
 
   return &(*device);
 }
 
 void AudioInputDeviceManager::RegisterListener(
     MediaStreamProviderListener* listener) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(listener);
-  DCHECK(device_task_runner_);
   listeners_.AddObserver(listener);
 }
 
 void AudioInputDeviceManager::UnregisterListener(
     MediaStreamProviderListener* listener) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(listener);
   listeners_.RemoveObserver(listener);
 }
 
 int AudioInputDeviceManager::Open(const MediaStreamDevice& device) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   // Generate a new id for this device.
   int session_id = next_capture_session_id_++;
-  device_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioInputDeviceManager::OpenOnDeviceThread,
-                 this, session_id, device));
+
+  // base::Unretained(this) is safe, because the IO thread is stopped before
+  // AudioInputDeviceManager destruction on the main thread.

[Guido Urdaneta, 2017/04/06 14:26:41]

Update the comment: AIDM is not destroyed on the main thread.
The intent is to destroy it on the IO thread, during destruction of the
IO-thread's message loop. It is still safe to use base::Unretained.

Also note that since the object is ref counted, destruction could be somewhere
else (but hopefully it is not).

[o1ka, 2017/04/07 12:22:17]

Done.

+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kUseFakeDeviceForMediaStream)) {
+    audio_system_->GetAssociatedOutputDeviceID(

[Guido Urdaneta, 2017/04/06 14:26:41]

Why get associated output device ID of a fake device? What will it return?

[o1ka, 2017/04/07 12:22:17]

This is how the code works now
(https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/audi...).


AudioSystem goes to AudioManager to ask for an associated output device ID.
kUseFakeDeviceForMediaStream is "under tests" situation. Some tests provide
fake/mock audio managers. So it all depends on how the test is set up and what
mocks do.

I'm intentionally not changing this behavior, because I don't want to break
tests unrelated to this CL.
We can explore improvements of this for-test logic separately.

[Guido Urdaneta, 2017/04/11 09:44:55]

Acknowledged.

+        device.id,
+        base::Bind(&AudioInputDeviceManager::OpenedOnIOThread,
+                   base::Unretained(this), session_id, device,
+                   base::TimeTicks::Now(),
+                   media::AudioParameters::UnavailableDeviceParams(),
+                   media::AudioParameters::UnavailableDeviceParams()));
+  } else {
+    // TODO(tommi): As is, we hit this code path when device.type is
+    // MEDIA_TAB_AUDIO_CAPTURE and the device id is not a device that
+    // the AudioManager can know about. This currently does not fail because
+    // the implementation of GetInputStreamParameters returns valid parameters
+    // by default for invalid devices. That behavior is problematic because it
+    // causes other parts of the code to attempt to open truly invalid or
+    // missing devices and falling back on alternate devices (and likely fail
+    // twice in a row). Tab audio capture should not pass through here and
+    // GetInputStreamParameters should return invalid parameters for invalid
+    // devices.
+
+    audio_system_->GetInputDeviceInfo(
+        device.id, base::Bind(&AudioInputDeviceManager::OpenedOnIOThread,
+                              base::Unretained(this), session_id, device,
+                              base::TimeTicks::Now()));
+  }
 
   return session_id;
 }
 
 void AudioInputDeviceManager::Close(int session_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   StreamDeviceList::iterator device = GetDevice(session_id);
   if (device == devices_.end())
     return;
   const MediaStreamType stream_type = device->device.type;
@@ skipping 38 matching lines @@
         BrowserThread::UI,
         FROM_HERE,
         base::Bind(
             &AudioInputDeviceManager::SetKeyboardMicStreamActiveOnUIThread,
             this,
             false));
   }
 }
 #endif
 
-void AudioInputDeviceManager::OpenOnDeviceThread(
+void AudioInputDeviceManager::OpenedOnIOThread(
     int session_id,
-    const MediaStreamDevice& device) {
-  SCOPED_UMA_HISTOGRAM_TIMER(
-      "Media.AudioInputDeviceManager.OpenOnDeviceThreadTime");
-  DCHECK(IsOnDeviceThread());
+    const MediaStreamDevice& device,
+    base::TimeTicks start_time,
+    const media::AudioParameters& input_params,
+    const media::AudioParameters& matched_output_params,
+    const std::string& matched_output_device_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(GetDevice(session_id) == devices_.end());
 
-  StreamDeviceInfo out(device.type, device.name, device.id, 0, 0, 0);
-  out.session_id = session_id;
+  UMA_HISTOGRAM_TIMES("Media.AudioInputDeviceManager.OpenOnDeviceThreadTime",
+                      base::TimeTicks::Now() - start_time);
 
-  MediaStreamDevice::AudioDeviceParameters& input_params = out.device.input;
-
-  // Add preferred output device information if a matching output device
-  // exists.
-  out.device.matched_output_device_id =
-      audio_manager_->GetAssociatedOutputDeviceID(device.id);
-
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kUseFakeDeviceForMediaStream)) {
-    // Don't need to query the hardware information if using fake device.
-    input_params.sample_rate = 44100;
-    input_params.channel_layout = media::CHANNEL_LAYOUT_STEREO;
-    if (!out.device.matched_output_device_id.empty()) {
-      out.device.matched_output.sample_rate = 44100;
-      out.device.matched_output.channel_layout = media::CHANNEL_LAYOUT_STEREO;
-    }
-  } else {
-    // TODO(tommi): As is, we hit this code path when device.type is
-    // MEDIA_TAB_AUDIO_CAPTURE and the device id is not a device that
-    // the AudioManager can know about. This currently does not fail because
-    // the implementation of GetInputStreamParameters returns valid parameters
-    // by default for invalid devices. That behavior is problematic because it
-    // causes other parts of the code to attempt to open truly invalid or
-    // missing devices and falling back on alternate devices (and likely fail
-    // twice in a row). Tab audio capture should not pass through here and
-    // GetInputStreamParameters should return invalid parameters for invalid
-    // devices.
-
-    // Get the preferred sample rate and channel configuration for the
-    // audio device.
-    media::AudioParameters params =
-        audio_manager_->GetInputStreamParameters(device.id);
-    input_params.sample_rate = params.sample_rate();
-    input_params.channel_layout = params.channel_layout();
-    input_params.frames_per_buffer = params.frames_per_buffer();
-    input_params.effects = params.effects();
-    input_params.mic_positions = params.mic_positions();
-    if (!out.device.matched_output_device_id.empty()) {
-      params = audio_manager_->GetOutputStreamParameters(
-          out.device.matched_output_device_id);
-      MediaStreamDevice::AudioDeviceParameters& matched_output_params =
-          out.device.matched_output;
-      matched_output_params.sample_rate = params.sample_rate();
-      matched_output_params.channel_layout = params.channel_layout();
-      matched_output_params.frames_per_buffer = params.frames_per_buffer();
-    }
-  }
-
-  // Return the |session_id| through the listener by posting a task on
-  // IO thread since MediaStreamManager handles the callback asynchronously.
-  BrowserThread::PostTask(BrowserThread::IO,
-                          FROM_HERE,
-                          base::Bind(&AudioInputDeviceManager::OpenedOnIOThread,
-                                     this, session_id, out));
-}
-
-void AudioInputDeviceManager::OpenedOnIOThread(int session_id,
-                                               const StreamDeviceInfo& info) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK_EQ(session_id, info.session_id);
-  DCHECK(GetDevice(session_id) == devices_.end());
+  StreamDeviceInfo info(device.type, device.name, device.id);
+  info.session_id = session_id;
+  info.device.input.sample_rate = input_params.sample_rate();

[DaleCurtis, 2017/04/06 18:37:17]

We have the copy constructor enabled for AudioParameters, can you use that here?

[o1ka, 2017/04/07 12:22:17]

These are not AudioParameters :(
(https://cs.chromium.org/chromium/src/content/public/common/media_stream_reque...)

+  info.device.input.channel_layout = input_params.channel_layout();
+  info.device.input.frames_per_buffer = input_params.frames_per_buffer();
+  info.device.input.effects = input_params.effects();
+  info.device.input.mic_positions = input_params.mic_positions();
+  info.device.matched_output_device_id = matched_output_device_id;
+  info.device.matched_output.sample_rate = matched_output_params.sample_rate();
+  info.device.matched_output.channel_layout =
+      matched_output_params.channel_layout();
+  info.device.matched_output.frames_per_buffer =
+      matched_output_params.frames_per_buffer();
+  info.device.matched_output.effects = matched_output_params.effects();
 
   devices_.push_back(info);
 
   for (auto& listener : listeners_)
     listener.Opened(info.device.type, session_id);
 }
 
 void AudioInputDeviceManager::ClosedOnIOThread(MediaStreamType stream_type,
                                                int session_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   for (auto& listener : listeners_)
     listener.Closed(stream_type, session_id);
 }
 
-bool AudioInputDeviceManager::IsOnDeviceThread() const {
-  return device_task_runner_->BelongsToCurrentThread();
-}
 
 AudioInputDeviceManager::StreamDeviceList::iterator
 AudioInputDeviceManager::GetDevice(int session_id) {
   for (StreamDeviceList::iterator i(devices_.begin()); i != devices_.end();
        ++i) {
     if (i->session_id == session_id)
       return i;
   }
 
   return devices_.end();
 }
 
 #if defined(OS_CHROMEOS)
 void AudioInputDeviceManager::SetKeyboardMicStreamActiveOnUIThread(
     bool active) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   chromeos::CrasAudioHandler::Get()->SetKeyboardMicActive(active);
 }
 #endif
 
-
 }  // namespace content