Add a virtual beamforming audio device on ChromeOS.

media/audio/cras/audio_manager_cras.cc

Index: media/audio/cras/audio_manager_cras.cc
diff --git a/media/audio/cras/audio_manager_cras.cc b/media/audio/cras/audio_manager_cras.cc
index 86d12fc39e5d504808b002083b85f9fb15a4b802..be2b550b1d1e9cd07a21bbd883e761217e3f06c9 100644
--- a/media/audio/cras/audio_manager_cras.cc
+++ b/media/audio/cras/audio_manager_cras.cc
@@ -23,28 +23,46 @@
 #undef max
 
 namespace media {
+namespace {
 
-static void AddDefaultDevice(AudioDeviceNames* device_names) {
-  DCHECK(device_names->empty());

[Henrik Grunell, 2015/08/27 08:38:18]

Why remove the dcheck?

-
+void AddDefaultDevice(AudioDeviceNames* device_names) {
   // Cras will route audio from a proper physical device automatically.
   device_names->push_back(
       AudioDeviceName(AudioManagerBase::kDefaultDeviceName,
                       AudioManagerBase::kDefaultDeviceId));
 }
 
+std::string GetMicPositions() {

[Henrik Grunell, 2015/08/27 08:38:18]

Add comment.

Can the function be called on any thread?

+  // Get the list of devices from CRAS. An internal mic with a non-empty
+  // positions field indicates the machine has a beamforming capable mic array.
+  chromeos::AudioDeviceList devices;
+  chromeos::CrasAudioHandler::Get()->GetAudioDevices(&devices);
+  for (const auto& device : devices) {
+    if (device.type == chromeos::AUDIO_TYPE_INTERNAL_MIC) {
+      // There should be only one internal mic device.
+      return device.mic_positions;
+    }
+  }
+  return "";
+}
+
 // Maximum number of output streams that can be open simultaneously.
-static const int kMaxOutputStreams = 50;
+const int kMaxOutputStreams = 50;
 
 // Default sample rate for input and output streams.
-static const int kDefaultSampleRate = 48000;
+const int kDefaultSampleRate = 48000;
 
 // Define bounds for the output buffer size.
-static const int kMinimumOutputBufferSize = 512;
-static const int kMaximumOutputBufferSize = 8192;
+const int kMinimumOutputBufferSize = 512;
+const int kMaximumOutputBufferSize = 8192;
 
 // Default input buffer size.
-static const int kDefaultInputBufferSize = 1024;
+const int kDefaultInputBufferSize = 1024;
+
+const char kBeamformingDeviceNameSuffix[] = " with beamforming";
+const char kBeamformingDeviceIdSuffix[] = "-beamforming";

[aluebs-chromium, 2015/08/28 19:14:30]

Since they are user-facing strings, is this the right thing for these strings?
Isn't there a standard way of handling them? I am thinking specially about
internationalization, etc.

[ajm, 2015/08/28 20:17:18]

Great question, I wonder the same thing! The existing "Default" doesn't appear
to have any i8ln. Dylan do you know anything about this case? If not, we need to
find someone that does.

[dgreid, 2015/08/28 20:24:55]

I don't think we ever bothered to translate "Default" because without another
option it didn't matter.  That might have to be revisited now. 

+
+}  // namespace
 
 bool AudioManagerCras::HasAudioOutputDevices() {
   return true;
@@ -62,7 +80,13 @@ bool AudioManagerCras::HasAudioInputDevices() {
 
 AudioManagerCras::AudioManagerCras(AudioLogFactory* audio_log_factory)
     : AudioManagerBase(audio_log_factory),
-      has_keyboard_mic_(false) {
+      has_keyboard_mic_(false),
+      beamforming_device_name_(
+          std::string(AudioManagerBase::kDefaultDeviceName) +
+              kBeamformingDeviceNameSuffix,
+          std::string(AudioManagerBase::kDefaultDeviceId) +
+              kBeamformingDeviceIdSuffix),
+      mic_positions_(GetMicPositions()) {
   SetMaxOutputStreamsAllowed(kMaxOutputStreams);
 }
 
@@ -76,6 +100,13 @@ void AudioManagerCras::ShowAudioInputSettings() {
 
 void AudioManagerCras::GetAudioInputDeviceNames(
     AudioDeviceNames* device_names) {
+  // A non-empty mic_positions indicates we have a beamforming capable mic
+  // array. Add the virtual beamforming device to the list. When this device is
+  // queried through GetInputStreamParameters, provide the cached mic positions.
+  if (!mic_positions_.empty()) {

[aluebs-chromium, 2015/08/28 19:14:30]

Shouldn't we be more restrictive here? For instance check that tha
mic_positions_ represents a valid geometry and has more than one mic, for
example? Else we will be surfacing this device and then don't know how to handle
it later.

[ajm, 2015/08/28 23:38:31]

That's a good point. I have those checks in MSAP for uniformity with the media
constraint, but you're right that we shouldn't expose a beamforming device if
it's not possible.

My thinking was that the CRAS configs would be responsible for this. Don't
provide non-empty mic positions if we don't have a beamforming capable device.

If we do the string parsing here, it's a stronger argument for doing checks here
as well. Will consider.

[aluebs-chromium, 2015/08/29 00:00:26]

Yes, I know it was in MSAP, but if we do it here we are protected against
potential string bugs in CRAS config.

[ajm, 2015/09/02 20:38:41]

Forgot to switch the check to > 1 mic. Done in PS#11.

+    device_names->push_back(beamforming_device_name_);
+  }
+
   AddDefaultDevice(device_names);
 }
 
@@ -94,12 +125,15 @@ AudioParameters AudioManagerCras::GetInputStreamParameters(
   AudioParameters::PlatformEffectsMask effects =
       has_keyboard_mic_ ? AudioParameters::KEYBOARD_MIC
                         : AudioParameters::NO_EFFECTS;
+  // Return the cached mic positions in the case of the beamforming device.
+  const std::string& mic_positions =
+      device_id == beamforming_device_name_.unique_id ? mic_positions_ : "";
 
   // TODO(hshi): Fine-tune audio parameters based on |device_id|. The optimal
   // parameters for the loopback stream may differ from the default.
-  return AudioParameters(
-      AudioParameters::AUDIO_PCM_LOW_LATENCY, CHANNEL_LAYOUT_STEREO,
-      kDefaultSampleRate, 16, buffer_size, effects);
+  return AudioParameters(AudioParameters::AUDIO_PCM_LOW_LATENCY,
+                         CHANNEL_LAYOUT_STEREO, kDefaultSampleRate, 16,

[aluebs-chromium, 2015/08/28 19:14:30]

What does CHANNEL_LAYOUT_STEREO represent? Is this the number of channels before
or after AudioProcessing? Because when beamforming this is going to be
different. Also , does this mean that it doesn't support higher number of
channels than 2? What about mono?

[ajm, 2015/08/28 23:38:31]

This represents the channel layout for the audio coming from the device.

We'll eventually have to refactor this to support > 2 mics. We may use the
CHANNEL_LAYOUT_DISCRETE format for that, which lets us specify an arbitrary
number of channels. I don't want to do that in this CL though.

[aluebs-chromium, 2015/08/29 00:00:26]

Agreed that this CL is big enough. But it needs to be added at some point.

+                         buffer_size, mic_positions, effects);
 }
 
 void AudioManagerCras::SetHasKeyboardMic() {
@@ -156,9 +190,8 @@ AudioParameters AudioManagerCras::GetPreferredOutputStreamParameters(
   if (user_buffer_size)
     buffer_size = user_buffer_size;
 
-  return AudioParameters(
-      AudioParameters::AUDIO_PCM_LOW_LATENCY, channel_layout,
-      sample_rate, bits_per_sample, buffer_size, AudioParameters::NO_EFFECTS);
+  return AudioParameters(AudioParameters::AUDIO_PCM_LOW_LATENCY, channel_layout,
+                         sample_rate, bits_per_sample, buffer_size);
 }
 
 AudioOutputStream* AudioManagerCras::MakeOutputStream(