Handle the dual microphones and dual cameras cases. Activate the proper microphone when user activa…

chromeos/audio/cras_audio_handler.cc

 // Copyright (c) 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.
 
 #include "chromeos/audio/cras_audio_handler.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 23 matching lines @@
 const int kMuteThresholdPercent = 1;
 
 // The duration of HDMI output re-discover grace period in milliseconds.
 const int kHDMIRediscoverGracePeriodDurationInMs = 2000;
 
 // Mixer matrix, [0.5, 0.5; 0.5, 0.5]
 const std::vector<double> kStereoToMono = {0.5, 0.5, 0.5, 0.5};
 // Mixer matrix, [1, 0; 0, 1]
 const std::vector<double> kStereoToStereo = {1, 0, 0, 1};
 
-static CrasAudioHandler* g_cras_audio_handler = NULL;
+static CrasAudioHandler* g_cras_audio_handler = nullptr;
 
 bool IsSameAudioDevice(const AudioDevice& a, const AudioDevice& b) {
   return a.stable_device_id == b.stable_device_id && a.is_input == b.is_input &&
          a.type == b.type && a.device_name == b.device_name;
 }
 
 bool IsDeviceInList(const AudioDevice& device, const AudioNodeList& node_list) {
   for (const AudioNode& node : node_list) {
     if (device.stable_device_id == node.StableDeviceId())
       return true;
@@ skipping 49 matching lines @@
 // static
 void CrasAudioHandler::InitializeForTesting() {
   CHECK(!g_cras_audio_handler);
   CrasAudioHandler::Initialize(new AudioDevicesPrefHandlerStub());
 }
 
 // static
 void CrasAudioHandler::Shutdown() {
   CHECK(g_cras_audio_handler);
   delete g_cras_audio_handler;
-  g_cras_audio_handler = NULL;
+  g_cras_audio_handler = nullptr;
 }
 
 // static
 bool CrasAudioHandler::IsInitialized() {
-  return g_cras_audio_handler != NULL;
+  return g_cras_audio_handler != nullptr;
 }
 
 // static
 CrasAudioHandler* CrasAudioHandler::Get() {
   CHECK(g_cras_audio_handler)
       << "CrasAudioHandler::Get() called before Initialize().";
   return g_cras_audio_handler;
 }
 
 void CrasAudioHandler::OnVideoCaptureStarted(media::VideoFacingMode facing) {
-  // TODO(jennyz): Switch active audio device according to video facing.
+  // Do nothing if the device doesn't have both front and rear microphones.
+  if (!HasDualInternalMic())
+    return;
+  switch (facing) {
+    case media::MEDIA_VIDEO_FACING_USER:
+      front_camera_on_ = true;
+      break;
+    case media::MEDIA_VIDEO_FACING_ENVIRONMENT:
+      rear_camera_on_ = true;
+      break;
+    default:
+      NOTREACHED();

[shenghao, 2017/03/01 11:25:55]

I am not 100% sure but I think it's possible to reach here. External camera
should have facing=MEDIA_VIDEO_FACING_NONE.

[jennyz, 2017/03/02 17:31:40]

I need to know in which scenario it will pass MEDIA_VIDEO_FACING_NONE in under
dual internal mic mode, and how we are going to handle it? For now, I would just
log a warning and do nothing just in case it happens.

[shenghao, 2017/03/03 03:45:21]

Since external camera has MEDIA_VIDEO_FACING_NONE, I don't think we need to log
warning for it. Only log warning for NUM_MEDIA_VIDEO_FACING_MODES case.

[jennyz, 2017/03/03 19:35:15]

Done.

+  }
+
+  // If the current active input is an external device, keep it.
+  const AudioDevice* active_input = GetDeviceFromId(active_input_node_id_);
+  if (active_input && active_input->IsExternalDevice())
+    return;
+
+  // Activate the correct mic for the current active camera.
+  ActivateMicForCamera(facing);
 }
 
 void CrasAudioHandler::OnVideoCaptureStopped(media::VideoFacingMode facing) {
-  // TODO(jennyz): Switch active audio device according to video facing.
+  // Do nothing if the device doesn't have both front and rear microphones.
+  if (!HasDualInternalMic())
+    return;
+
+  switch (facing) {
+    case media::MEDIA_VIDEO_FACING_USER:
+      front_camera_on_ = false;
+      break;
+    case media::MEDIA_VIDEO_FACING_ENVIRONMENT:
+      rear_camera_on_ = false;
+      break;
+    default:
+      NOTREACHED();
+  }
+
+  // If the current active input is an external device, keep it.
+  const AudioDevice* active_input = GetDeviceFromId(active_input_node_id_);
+  if (active_input && active_input->IsExternalDevice())
+    return;
+
+  // Switch to front mic properly.

[shenghao, 2017/03/01 11:25:55]

Why is it "front mic"? Do you mean "correct mic"?

[jennyz, 2017/03/02 17:31:41]

In R58, chrome code will only handle the dual camera/dual mic case with only one
camera active at a time, which is the typical chrome book and camera app case.
For the developers, they can use audio extension api to set both mic to be
active if they need to do so for their app.

For our use case I mentioned above, the camera stop, typically, user close the
camera app, we would switch back to internal mic and the front mic is the
default mic to set active. 

+  DeviceActivateType activate_by =

[shenghao, 2017/03/01 11:25:55]

s/activate_by/activated_by/

[jennyz, 2017/03/02 17:31:41]

Done.

+      HasExternalDevice(true) ? ACTIVATE_BY_USER : ACTIVATE_BY_PRIORITY;
+  SwitchToDevice(*GetDeviceByType(AUDIO_TYPE_FRONT_MIC), true, activate_by);

[shenghao, 2017/03/01 11:25:55]

Why do we also switch to AUDIO_TYPE_FRONT_MIC? Is it possible to switch to
AUDIO_TYPE_BACK_MIC?

[shenghao, 2017/03/01 11:27:54]

typo: also => always

[jennyz, 2017/03/02 17:31:40]

Acknowledged.

[jennyz, 2017/03/02 17:31:41]

See comments above.

 }
 
 void CrasAudioHandler::AddAudioObserver(AudioObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void CrasAudioHandler::RemoveAudioObserver(AudioObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
 bool CrasAudioHandler::HasKeyboardMic() {
-  return GetKeyboardMic() != NULL;
+  return GetKeyboardMic() != nullptr;
 }
 
 bool CrasAudioHandler::IsOutputMuted() {
   return output_mute_on_;
 }
 
 bool CrasAudioHandler::IsOutputMutedForDevice(uint64_t device_id) {
   const AudioDevice* device = GetDeviceFromId(device_id);
   if (!device)
     return false;
@@ skipping 440 matching lines @@
   if (!chromeos::DBusThreadManager::IsInitialized() ||
       !chromeos::DBusThreadManager::Get() ||
       !chromeos::DBusThreadManager::Get()->GetCrasAudioClient())
     return;
   chromeos::DBusThreadManager::Get()->GetCrasAudioClient()->
       RemoveObserver(this);
   chromeos::DBusThreadManager::Get()->GetSessionManagerClient()->
       RemoveObserver(this);
   if (audio_pref_handler_.get())
     audio_pref_handler_->RemoveAudioPrefObserver(this);
-  audio_pref_handler_ = NULL;
+  audio_pref_handler_ = nullptr;
 }
 
 void CrasAudioHandler::AudioClientRestarted() {
   // Make sure the logging is enabled in case cras server
   // restarts after crashing.
   LogErrors();
   InitializeAudioState();
 }
 
 void CrasAudioHandler::NodesChanged() {
@@ skipping 87 matching lines @@
 
 void CrasAudioHandler::EmitLoginPromptVisibleCalled() {
   // Enable logging after cras server is started, which will be after
   // EmitLoginPromptVisible.
   LogErrors();
 }
 
 const AudioDevice* CrasAudioHandler::GetDeviceFromId(uint64_t device_id) const {
   AudioDeviceMap::const_iterator it = audio_devices_.find(device_id);
   if (it == audio_devices_.end())
-    return NULL;
+    return nullptr;
 
   return &(it->second);
 }
 
 const AudioDevice* CrasAudioHandler::GetDeviceFromStableDeviceId(
     uint64_t stable_device_id) const {
   for (AudioDeviceMap::const_iterator it = audio_devices_.begin();
        it != audio_devices_.end(); ++it) {
     if (it->second.stable_device_id == stable_device_id)
       return &(it->second);
   }
-  return NULL;
+  return nullptr;
 }
 
 const AudioDevice* CrasAudioHandler::GetKeyboardMic() const {
   for (AudioDeviceMap::const_iterator it = audio_devices_.begin();
        it != audio_devices_.end(); it++) {
     if (it->second.is_input && it->second.type == AUDIO_TYPE_KEYBOARD_MIC)
       return &(it->second);
   }
-  return NULL;
+  return nullptr;
 }
 
 void CrasAudioHandler::SetupAudioInputState() {
   // Set the initial audio state to the ones read from audio prefs.
   const AudioDevice* device = GetDeviceFromId(active_input_node_id_);
   if (!device) {
     LOG_IF(ERROR, log_errors_)
         << "Can't set up audio state for unknown input device id ="
         << "0x" << std::hex << active_input_node_id_;
     return;
@@ skipping 429 matching lines @@
   } else {
     // Do not active the device if its previous state is inactive.
     VLOG(1) << "Hotplug device remains inactive as its previous state:"
             << hotplug_device.ToString();
   }
 }
 
 void CrasAudioHandler::SwitchToTopPriorityDevice(bool is_input) {
   AudioDevice top_device =
       is_input ? input_devices_pq_.top() : output_devices_pq_.top();
-  if (top_device.is_for_simple_usage())
-    SwitchToDevice(top_device, true, ACTIVATE_BY_PRIORITY);
+  if (!top_device.is_for_simple_usage())
+    return;
+
+  // For the dual camera and dual microphone case, choose microphone
+  // that is consistent to the active camera.
+  if (IsFrontOrRearMic(top_device) && HasDualInternalMic() && IsCameraOn()) {

[shenghao, 2017/03/01 11:25:55]

I think you can write:

if (IsFrontOrRearMic(top_device)) {
  ActivateInternalMicForActiveCamera();
  return;
}

[jennyz, 2017/03/02 17:31:40]

The above code is not equivalent to the original lines. But I did simplify it by
calling ActivateInternalMicForActiveCamera() inside the if block.

+    media::VideoFacingMode facing = front_camera_on_
+                                        ? media::MEDIA_VIDEO_FACING_USER
+                                        : media::MEDIA_VIDEO_FACING_ENVIRONMENT;
+    ActivateMicForCamera(facing);
+    return;
+  }
+
+  SwitchToDevice(top_device, true, ACTIVATE_BY_PRIORITY);
 }
 
 void CrasAudioHandler::SwitchToPreviousActiveDeviceIfAvailable(bool is_input) {
   AudioDevice previous_active_device;
   if (GetActiveDeviceFromUserPref(is_input, &previous_active_device)) {
     DCHECK(previous_active_device.is_for_simple_usage());
     // Switch to previous active device stored in user prefs.
     SwitchToDevice(previous_active_device, true,
                    ACTIVATE_BY_RESTORE_PREVIOUS_STATE);
   } else {
@@ skipping 218 matching lines @@
       FROM_HERE, base::TimeDelta::FromMilliseconds(
                      hdmi_rediscover_grace_period_duration_in_ms_),
       this, &CrasAudioHandler::UpdateAudioAfterHDMIRediscoverGracePeriod);
 }
 
 void CrasAudioHandler::SetHDMIRediscoverGracePeriodForTesting(
     int duration_in_ms) {
   hdmi_rediscover_grace_period_duration_in_ms_ = duration_in_ms;
 }
 
+void CrasAudioHandler::ActivateMicForCamera(
+    media::VideoFacingMode camera_facing) {
+  const AudioDevice* mic = GetMicForCamera(camera_facing);
+  if (!mic || mic->active)
+    return;
+
+  SwitchToDevice(*mic, true, ACTIVATE_BY_CAMERA);
+}
+
+void CrasAudioHandler::ActivateInternalMicForActiveCamera() {
+  if (HasDualInternalMic() && IsCameraOn()) {
+    media::VideoFacingMode facing = front_camera_on_
+                                        ? media::MEDIA_VIDEO_FACING_USER
+                                        : media::MEDIA_VIDEO_FACING_ENVIRONMENT;
+    ActivateMicForCamera(facing);
+  }
+}
+
+// For the dual microphone case, from user point of view, they only see internal
+// microphone in UI. Chrome will make the best decision on which one to pick.
+// If the camera is off, the front microphone should be picked as the default
+// active microphone. Otherwise, it will switch to the microphone that
+// matches the active camera, i.e. front microphone for front camera and
+// reaa microphone for rear camera.

[shenghao, 2017/03/01 11:25:55]

s/reaa/rear/

[jennyz, 2017/03/02 17:31:41]

Done.

+void CrasAudioHandler::SwitchToFrontOrRearMic() {
+  DCHECK(HasDualInternalMic());
+  if (IsCameraOn()) {

[shenghao, 2017/03/01 11:25:55]

Why do we need to check IsCameraOn() twice here and line 1491?

[jennyz, 2017/03/02 17:31:40]

Removed IsCameraOn() from 1491.

+    ActivateInternalMicForActiveCamera();
+  } else {
+    SwitchToDevice(*GetDeviceByType(AUDIO_TYPE_FRONT_MIC), true,
+                   ACTIVATE_BY_USER);
+  }
+}
+
+const AudioDevice* CrasAudioHandler::GetMicForCamera(
+    media::VideoFacingMode camera_facing) {
+  switch (camera_facing) {
+    case media::MEDIA_VIDEO_FACING_USER:
+      return GetDeviceByType(AUDIO_TYPE_FRONT_MIC);
+    case media::MEDIA_VIDEO_FACING_ENVIRONMENT:
+      return GetDeviceByType(AUDIO_TYPE_REAR_MIC);
+      break;

[shenghao, 2017/03/01 11:25:55]

You don't need "break" here because of the return statement.

[jennyz, 2017/03/02 17:31:40]

Done.

+    default:
+      NOTREACHED();

[shenghao, 2017/03/01 11:25:55]

Same as line 141

[jennyz, 2017/03/02 17:31:41]

This code path is not reachable, the code path to call this function would check
and filter out the case that facing is not the first two cases.

+  }
+  return nullptr;
+}
+
+const AudioDevice* CrasAudioHandler::GetDeviceByType(AudioDeviceType type) {
+  for (AudioDeviceMap::const_iterator it = audio_devices_.begin();

[shenghao, 2017/03/01 11:25:55]

For all the "AudioDeviceMap::const_iterator", can we use "const auto" to replace
them?

[jennyz, 2017/03/02 17:31:40]

Done.

+       it != audio_devices_.end(); ++it) {
+    if (it->second.type == type)
+      return &(it->second);
+  }
+  return nullptr;
+}
+
+bool CrasAudioHandler::HasDualInternalMic() const {
+  bool has_front_mic = false;
+  bool has_rear_mic = false;
+  for (AudioDeviceMap::const_iterator it = audio_devices_.begin();
+       it != audio_devices_.end(); ++it) {
+    if (it->second.type == AUDIO_TYPE_FRONT_MIC)
+      has_front_mic = true;
+    else if (it->second.type == AUDIO_TYPE_REAR_MIC)
+      has_rear_mic = true;
+    if (has_front_mic && has_rear_mic)
+      break;
+  }
+  return has_front_mic && has_rear_mic;
+}
+
+bool CrasAudioHandler::IsFrontOrRearMic(const AudioDevice& device) const {
+  return device.is_input && (device.type == AUDIO_TYPE_FRONT_MIC ||
+                             device.type == AUDIO_TYPE_REAR_MIC);
+}
+
+bool CrasAudioHandler::IsCameraOn() const {
+  return front_camera_on_ || rear_camera_on_;
+}
+
+bool CrasAudioHandler::HasExternalDevice(bool is_input) const {

[shenghao, 2017/03/01 11:25:55]

It seems that this method is only called in one place, and |is_input| is true.
Can we remove the parameter?

[jennyz, 2017/03/02 17:31:40]

I would rather keep it just in case it could be used by output device in the
future.

+  for (AudioDeviceMap::const_iterator it = audio_devices_.begin();
+       it != audio_devices_.end(); ++it) {
+    if (is_input == it->second.is_input && it->second.IsExternalDevice())
+      return true;
+  }
+  return false;
+}
+
 }  // namespace chromeos