Migrate to use SetOutputNodeVolume and SetInputNodeGain cras apis.

Based on our discussion about cras apis with cras team, chrome will switch to use per node volume/gain set apis, and do not use the apis for setting system output volume and input gain. This should simplify the flow for switching active input/output audio devices and solve the racing problems during the switch related to previous work around.


BUG=244597

Review URL: https://chromiumcodereview.appspot.com/18641003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@211282 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 64 insertions, 97 deletions

diff --git a/chromeos/audio/audio_device.cc b/chromeos/audio/audio_device.cc
index 7ebdb78..6e1f1b3 100644
--- a/chromeos/audio/audio_device.cc
+++ b/chromeos/audio/audio_device.cc
@@ -106,8 +106,8 @@ std::string AudioDevice::ToString() const {
                       "is_input = %s ",
                       is_input ? "true" : "false");
   base::StringAppendF(&result,
-                      "id = %s ",
-                      base::Uint64ToString(id).c_str());
+                      "id = 0x%"PRIx64" ",
+                      id);
   base::StringAppendF(&result,
                       "display_name = %s ",
                       display_name.c_str());
diff --git a/chromeos/audio/cras_audio_handler.cc b/chromeos/audio/cras_audio_handler.cc
index b94c1c5..8a4829a 100644
--- a/chromeos/audio/cras_audio_handler.cc
+++ b/chromeos/audio/cras_audio_handler.cc
@@ -194,7 +194,7 @@ void CrasAudioHandler::SetOutputVolumePercent(int volume_percent) {
   if (const AudioDevice* device = GetDeviceFromId(active_output_node_id_))
     audio_pref_handler_->SetVolumeGainValue(*device, output_volume_);
 
-  SetOutputVolumeInternal(output_volume_);
+  SetOutputNodeVolume(active_output_node_id_, output_volume_);
   FOR_EACH_OBSERVER(AudioObserver, observers_, OnOutputVolumeChanged());
 }
 
@@ -207,7 +207,7 @@ void CrasAudioHandler::SetInputGainPercent(int gain_percent) {
   if (const AudioDevice* device = GetDeviceFromId(active_input_node_id_))
     audio_pref_handler_->SetVolumeGainValue(*device, input_gain_);
 
-  SetInputGainInternal(input_gain_);
+  SetInputNodeGain(active_input_node_id_, input_gain_);
   FOR_EACH_OBSERVER(AudioObserver, observers_, OnInputGainChanged());
 }
 
@@ -334,21 +334,6 @@ void CrasAudioHandler::AudioClientRestarted() {
   InitializeAudioState();
 }
 
-void CrasAudioHandler::OutputVolumeChanged(int volume) {
-  if (output_volume_ != volume) {
-    LOG(WARNING) << "Output volume state inconsistent, internal volume="
-        << output_volume_ << ", dbus signal volume=" << volume;
-    return;
-  }
-}
-
-void CrasAudioHandler::InputGainChanged(int gain) {
-  if (input_gain_ != gain) {
-    LOG(WARNING) << "input gain state inconsistent, internal gain="
-        << input_gain_  << ", dbus signal gain=" << gain;
-  }
-}
-
 void CrasAudioHandler::OutputMuteChanged(bool mute_on) {
   if (output_mute_on_ != mute_on) {
     LOG(WARNING) << "output mute state inconsistent, internal mute="
@@ -372,18 +357,18 @@ void CrasAudioHandler::ActiveOutputNodeChanged(uint64 node_id) {
   if (active_output_node_id_ == node_id)
     return;
 
-  active_output_node_id_ = node_id;
-  SetupAudioOutputState();
-  FOR_EACH_OBSERVER(AudioObserver, observers_, OnActiveOutputNodeChanged());
+  // Active audio output device should always be changed by chrome.
+  LOG(WARNING) << "Active output node changed unexpectedly by system node_id="
+      << "0x" << std::hex << node_id;
 }
 
 void CrasAudioHandler::ActiveInputNodeChanged(uint64 node_id) {
   if (active_input_node_id_ == node_id)
     return;
 
-  active_input_node_id_ = node_id;
-  SetupAudioInputState();
-  FOR_EACH_OBSERVER(AudioObserver, observers_, OnActiveInputNodeChanged());
+  // Active audio input device should always be changed by chrome.
+  LOG(WARNING) << "Active input node changed unexpectedly by system node_id="
+      << "0x" << std::hex << node_id;
 }
 
 void CrasAudioHandler::OnAudioPolicyPrefChanged() {
@@ -401,30 +386,28 @@ const AudioDevice* CrasAudioHandler::GetDeviceFromId(uint64 device_id) const {
 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) {
-    input_mute_on_ = audio_pref_handler_->GetMuteValue(*device);
-    input_gain_ = audio_pref_handler_->GetVolumeGainValue(*device);
-  } else {
-    input_mute_on_ = kPrefMuteOff;
-    input_gain_ = kDefaultVolumeGainPercent;
+  if (!device) {
+    LOG(ERROR) << "Can't set up audio state for unknow input device id ="
+        << "0x" << std::hex << active_input_node_id_;
+    return;
   }
-
+  input_mute_on_ = audio_pref_handler_->GetMuteValue(*device);
+  input_gain_ = audio_pref_handler_->GetVolumeGainValue(*device);
   SetInputMuteInternal(input_mute_on_);
-  SetInputGainInternal(input_gain_);
+  SetInputNodeGain(active_input_node_id_, input_gain_);
 }
 
 void CrasAudioHandler::SetupAudioOutputState() {
   const AudioDevice* device = GetDeviceFromId(active_output_node_id_);
-  if (device) {
-    output_mute_on_ = audio_pref_handler_->GetMuteValue(*device);
-    output_volume_ = audio_pref_handler_->GetVolumeGainValue(*device);
-  } else {
-    output_mute_on_ = kPrefMuteOff;
-    output_volume_ = kDefaultVolumeGainPercent;
+  if (!device) {
+      LOG(ERROR) << "Can't set up audio state for unknow output device id ="
+        << "0x" << std::hex << active_output_node_id_;
+    return;
   }
-
+  output_mute_on_ = audio_pref_handler_->GetMuteValue(*device);
+  output_volume_ = audio_pref_handler_->GetVolumeGainValue(*device);
   SetOutputMuteInternal(output_mute_on_);
-  SetOutputVolumeInternal(output_volume_);
+  SetOutputNodeVolume(active_output_node_id_, output_volume_);
 }
 
 void CrasAudioHandler::InitializeAudioState() {
@@ -448,9 +431,9 @@ void CrasAudioHandler::ApplyAudioPolicy() {
   }
 }
 
-void CrasAudioHandler::SetOutputVolumeInternal(int volume) {
+void CrasAudioHandler::SetOutputNodeVolume(uint64 node_id, int volume) {
   chromeos::DBusThreadManager::Get()->GetCrasAudioClient()->
-      SetOutputVolume(volume);
+      SetOutputNodeVolume(node_id, volume);
 }
 
 bool  CrasAudioHandler::SetOutputMuteInternal(bool mute_on) {
@@ -462,9 +445,9 @@ bool  CrasAudioHandler::SetOutputMuteInternal(bool mute_on) {
   return true;
 }
 
-void CrasAudioHandler::SetInputGainInternal(int gain) {
+void CrasAudioHandler::SetInputNodeGain(uint64 node_id, int gain) {
   chromeos::DBusThreadManager::Get()->GetCrasAudioClient()->
-      SetInputGain(gain);
+      SetInputNodeGain(node_id, gain);
 }
 
 bool CrasAudioHandler::SetInputMuteInternal(bool mute_on) {
@@ -482,42 +465,44 @@ void CrasAudioHandler::GetNodes() {
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
+bool CrasAudioHandler::ChangeActiveDevice(const AudioDevice& new_active_device,
+                                          uint64* current_active_node_id) {
+  // If the device we want to switch to is already the current active device,
+  // do nothing.
+  if (new_active_device.active &&
+      new_active_device.id == *current_active_node_id) {
+    return false;
+  }
+  if (GetDeviceFromId(*current_active_node_id))
+    audio_devices_[*current_active_node_id].active = false;
+  *current_active_node_id = new_active_device.id;
+  audio_devices_[*current_active_node_id].active = true;
+  return true;
+}
+
 void CrasAudioHandler::SwitchToDevice(const AudioDevice& device) {
-  // The flow we follow is this,
-  // .) Global mute.
-  // .) Switch to active device.
-  // .) Once device is switched, set sound state for new device.
-  // We do this since during the state from when a device is plugged in or out,
-  // we are in between devices. We cannot switch to the new device with the
-  // old devices volume, since in certain situations it might be a very jarring
-  // or disturbing sound (for example, plugging out headphones, which were set
-  // to high volume, and switching to speakers, that were set to low volume).
-  // To avoid this, we mute all sound, do our switch, and then directly set
-  // the volume and mute to that of the new device. This way the user never has
-  // to hear the wrong volume for a device.
-  LOG(INFO) << "Switching active device to: " << device.ToString();
   if (device.is_input) {
-    SetInputMuteInternal(true);
+    if (!ChangeActiveDevice(device, &active_input_node_id_))
+      return;
+
+    SetupAudioInputState();
     DBusThreadManager::Get()->GetCrasAudioClient()->SetActiveInputNode(
         device.id);
+    FOR_EACH_OBSERVER(AudioObserver, observers_, OnActiveInputNodeChanged());
   } else {
-    SetOutputMuteInternal(true);
+    if (!ChangeActiveDevice(device, &active_output_node_id_))
+      return;
+
+    SetupAudioOutputState();
     DBusThreadManager::Get()->GetCrasAudioClient()->SetActiveOutputNode(
         device.id);
+    FOR_EACH_OBSERVER(AudioObserver, observers_, OnActiveOutputNodeChanged());
   }
 }
 
 void CrasAudioHandler::UpdateDevicesAndSwitchActive(
     const AudioNodeList& nodes) {
-  bool input_device_removed = false;
-  bool output_device_removed = false;
-
-  size_t num_previous_input_devices = input_devices_pq_.size();
-  size_t num_previous_output_devices = output_devices_pq_.size();
-
   audio_devices_.clear();
-  active_input_node_id_ = 0;
-  active_output_node_id_ = 0;
   has_alternative_input_ = false;
   has_alternative_output_ = false;
 
@@ -527,10 +512,6 @@ void CrasAudioHandler::UpdateDevicesAndSwitchActive(
     output_devices_pq_.pop();
 
   for (size_t i = 0; i < nodes.size(); ++i) {
-    if (nodes[i].is_input && nodes[i].active)
-      active_input_node_id_ = nodes[i].id;
-    else if (!nodes[i].is_input && nodes[i].active)
-      active_output_node_id_ = nodes[i].id;
     AudioDevice device(nodes[i]);
     audio_devices_[device.id] = device;
 
@@ -550,28 +531,11 @@ void CrasAudioHandler::UpdateDevicesAndSwitchActive(
       output_devices_pq_.push(device);
   }
 
-  if (num_previous_input_devices > input_devices_pq_.size())
-    input_device_removed = true;
-  if (num_previous_output_devices > output_devices_pq_.size())
-    output_device_removed = true;
-
-  // If either,
-  // .) the top input/output device is already active, or,
-  // .) an input/output device was removed but not the active device,
-  // then we don't need to switch the device, otherwise we do need to switch.
-  if (!input_devices_pq_.empty() &&
-      (!(input_devices_pq_.top().active || (input_device_removed &&
-          active_input_node_id_))))
+  if (!input_devices_pq_.empty())
     SwitchToDevice(input_devices_pq_.top());
-  else
-    SetupAudioInputState();
 
-  if (!output_devices_pq_.empty() &&
-      (!(output_devices_pq_.top().active || (output_device_removed &&
-          active_output_node_id_))))
+  if (!output_devices_pq_.empty())
     SwitchToDevice(output_devices_pq_.top());
-  else
-    SetupAudioOutputState();
 }
 
 void CrasAudioHandler::HandleGetNodes(const chromeos::AudioNodeList& node_list,
diff --git a/chromeos/audio/cras_audio_handler.h b/chromeos/audio/cras_audio_handler.h
index cb26f23..99025f3 100644
--- a/chromeos/audio/cras_audio_handler.h
+++ b/chromeos/audio/cras_audio_handler.h
@@ -177,9 +177,7 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
  private:
   // Overriden from CrasAudioClient::Observer.
   virtual void AudioClientRestarted() OVERRIDE;
-  virtual void OutputVolumeChanged(int volume) OVERRIDE;
   virtual void OutputMuteChanged(bool mute_on) OVERRIDE;
-  virtual void InputGainChanged(int gain) OVERRIDE;
   virtual void InputMuteChanged(bool mute_on) OVERRIDE;
   virtual void NodesChanged() OVERRIDE;
   virtual void ActiveOutputNodeChanged(uint64 node_id) OVERRIDE;
@@ -203,15 +201,15 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
   // change notification is received.
   void ApplyAudioPolicy();
 
-  // Sets output volume to specified value of |volume|.
-  void SetOutputVolumeInternal(int volume);
+  // Sets output volume of |node_id| to |volume|.
+  void SetOutputNodeVolume(uint64 node_id, int volume);
 
   // Sets output mute state to |mute_on| internally, returns true if output mute
   // is set.
   bool SetOutputMuteInternal(bool mute_on);
 
-  // Sets output volume to specified value and notifies observers.
-  void SetInputGainInternal(int gain);
+  // Sets input gain of |node_id| to |gain|.
+  void SetInputNodeGain(uint64 node_id, int gain);
 
   // Sets input mute state to |mute_on| internally, returns true if input mute
   // is set.
@@ -224,6 +222,11 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
   // if needed.
   void UpdateDevicesAndSwitchActive(const AudioNodeList& nodes);
 
+  // Returns true if *|current_active_node_id| device is changed to
+  // |new_active_device|.
+  bool ChangeActiveDevice(const AudioDevice& new_active_device,
+                          uint64* current_active_node_id);
+
   void SwitchToDevice(const AudioDevice& device);
 
   // Handles dbus callback for GetNodes.