Work around for HDMI audio output rediscovering transistion loss.

When audio output is playing on HDMI device, each time user opens or closes the lid of the device, changes the resolution of the hdmi display, or the device resumes after suspension, there is a short period of time the audio will be played on internal speaker, then goes back to hdmi device., The root cause is due to the fact that cras will lose hdmi output audio device for a short period of time and rediscover it later during the transition period of these scenarios. The hot plug cause the internal speaker be selected as an alternative active output device during the transition period. For some platform, the transition period is quite long as several seconds and it is an annoying user experience.

I've made a workaround in CrasAudioHandler to mute the output during the hdmi rediscovering grace period, so that the audio will NOT be leaked.

BUG=503667

Review URL: https://codereview.chromium.org/1199413008

Cr-Commit-Position: refs/heads/master@{#336681}

3 files changed, 193 insertions, 1 deletions

diff --git a/chromeos/audio/cras_audio_handler.cc b/chromeos/audio/cras_audio_handler.cc
index 3a86d86..1cf03d1 100644
--- a/chromeos/audio/cras_audio_handler.cc
+++ b/chromeos/audio/cras_audio_handler.cc
@@ -28,6 +28,9 @@ const int kDefaultUnmuteVolumePercent = 4;
 // Volume value which should be considered as muted in range [0, 100].
 const int kMuteThresholdPercent = 1;
 
+// The duration of HDMI output re-discover grace period in milliseconds.
+const int kHDMIRediscoverGracePeriodDurationInMs = 2000;
+
 static CrasAudioHandler* g_cras_audio_handler = NULL;
 
 bool IsSameAudioDevice(const AudioDevice& a, const AudioDevice& b) {
@@ -432,6 +435,26 @@ void CrasAudioHandler::LogErrors() {
   log_errors_ = true;
 }
 
+// If the HDMI device is the active output device, when the device enters/exits
+// docking mode, or HDMI display changes resolution, or chromeos device
+// suspends/resumes, cras will lose the HDMI output node for a short period of
+// time, then rediscover it. This hotplug behavior will cause the audio output
+// be leaked to the alternatvie active audio output during HDMI re-discovering
+// period. See crbug.com/503667.
+void CrasAudioHandler::SetActiveHDMIOutoutRediscoveringIfNecessary(
+    bool force_rediscovering) {
+  if (!GetDeviceFromId(active_output_node_id_))
+    return;
+
+  // Marks the start of the HDMI re-discovering grace period, during which we
+  // will mute the audio output to prevent it to be be leaked to the
+  // alternative output device.
+  if ((hdmi_rediscovering_ && force_rediscovering) ||
+      (!hdmi_rediscovering_ && IsHDMIPrimaryOutputDevice())) {
+    StartHDMIRediscoverGracePeriod();
+  }
+}
+
 CrasAudioHandler::CrasAudioHandler(
     scoped_refptr<AudioDevicesPrefHandler> audio_pref_handler)
     : audio_pref_handler_(audio_pref_handler),
@@ -445,6 +468,9 @@ CrasAudioHandler::CrasAudioHandler(
       has_alternative_output_(false),
       output_mute_locked_(false),
       log_errors_(false),
+      hdmi_rediscover_grace_period_duration_in_ms_(
+          kHDMIRediscoverGracePeriodDurationInMs),
+      hdmi_rediscovering_(false),
       weak_ptr_factory_(this) {
   if (!audio_pref_handler.get())
     return;
@@ -464,6 +490,7 @@ CrasAudioHandler::CrasAudioHandler(
 }
 
 CrasAudioHandler::~CrasAudioHandler() {
+  hdmi_rediscover_timer_.Stop();
   if (!chromeos::DBusThreadManager::IsInitialized() ||
       !chromeos::DBusThreadManager::Get() ||
       !chromeos::DBusThreadManager::Get()->GetCrasAudioClient())
@@ -570,7 +597,13 @@ void CrasAudioHandler::SetupAudioOutputState() {
     return;
   }
   DCHECK(!device->is_input);
-  output_mute_on_ = audio_pref_handler_->GetMuteValue(*device);
+  // Mute the output during HDMI re-discovering grace period.
+  if (hdmi_rediscovering_ && !IsHDMIPrimaryOutputDevice()) {
+    VLOG(1) << "Mute the output during HDMI re-discovering grace period";
+    output_mute_on_ = true;
+  } else {
+    output_mute_on_ = audio_pref_handler_->GetMuteValue(*device);
+  }
   output_volume_ = audio_pref_handler_->GetOutputVolumeValue(device);
 
   SetOutputMuteInternal(output_mute_on_);
@@ -976,4 +1009,34 @@ void CrasAudioHandler::RemoveActiveNodeInternal(uint64_t node_id, bool notify) {
   }
 }
 
+void CrasAudioHandler::UpdateAudioAfterHDMIRediscoverGracePeriod() {
+  VLOG(1) << "HDMI output re-discover grace period ends.";
+  hdmi_rediscovering_ = false;
+  if (!IsOutputMutedForDevice(active_output_node_id_)) {
+    // Unmute the audio output after the HDMI transition period.
+    VLOG(1) << "Unmute output after HDMI rediscovring grace period.";
+    SetOutputMuteInternal(false);
+  }
+}
+
+bool CrasAudioHandler::IsHDMIPrimaryOutputDevice() const {
+  const AudioDevice* device = GetDeviceFromId(active_output_node_id_);
+  return (device && device->type == chromeos::AUDIO_TYPE_HDMI);
+}
+
+void CrasAudioHandler::StartHDMIRediscoverGracePeriod() {
+  VLOG(1) << "Start HDMI rediscovering grace period.";
+  hdmi_rediscovering_ = true;
+  hdmi_rediscover_timer_.Stop();
+  hdmi_rediscover_timer_.Start(
+      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;
+}
+
 }  // namespace chromeos
diff --git a/chromeos/audio/cras_audio_handler.h b/chromeos/audio/cras_audio_handler.h
index 633e30a..10c77a4 100644
--- a/chromeos/audio/cras_audio_handler.h
+++ b/chromeos/audio/cras_audio_handler.h
@@ -11,6 +11,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "base/timer/timer.h"
 #include "chromeos/audio/audio_device.h"
 #include "chromeos/audio/audio_pref_observer.h"
 #include "chromeos/dbus/audio_node.h"
@@ -188,6 +189,15 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
   // Enables error logging.
   virtual void LogErrors();
 
+  // If necessary, sets the starting point for re-discovering the active HDMI
+  // output device caused by device entering/exiting docking mode, HDMI display
+  // changing resolution, or chromeos device suspend/resume. If
+  // |force_rediscovering| is true, it will force to set the starting point for
+  // re-discovering the active HDMI output device again if it has been in the
+  // middle of rediscovering the HDMI active output device.
+  virtual void SetActiveHDMIOutoutRediscoveringIfNecessary(
+      bool force_rediscovering);
+
  protected:
   explicit CrasAudioHandler(
       scoped_refptr<AudioDevicesPrefHandler> audio_pref_handler);
@@ -293,6 +303,16 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
   // Removes |node_id| from additional active nodes.
   void RemoveActiveNodeInternal(uint64_t node_id, bool notify);
 
+  void UpdateAudioAfterHDMIRediscoverGracePeriod();
+
+  bool IsHDMIPrimaryOutputDevice() const;
+
+  void StartHDMIRediscoverGracePeriod();
+
+  bool hdmi_rediscovering() const { return hdmi_rediscovering_; }
+
+  void SetHDMIRediscoverGracePeriodForTesting(int duration_in_ms);
+
   enum DeviceStatus {
     OLD_DEVICE,
     NEW_DEVICE,
@@ -328,6 +348,11 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
   // Failures are not logged at startup, since CRAS may not be running yet.
   bool log_errors_;
 
+  // Timer for HDMI re-discovering grace period.
+  base::OneShotTimer<CrasAudioHandler> hdmi_rediscover_timer_;
+  int hdmi_rediscover_grace_period_duration_in_ms_;
+  bool hdmi_rediscovering_;
+
   base::WeakPtrFactory<CrasAudioHandler> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(CrasAudioHandler);
diff --git a/chromeos/audio/cras_audio_handler_unittest.cc b/chromeos/audio/cras_audio_handler_unittest.cc
index fa5dbc3..7472a7f 100644
--- a/chromeos/audio/cras_audio_handler_unittest.cc
+++ b/chromeos/audio/cras_audio_handler_unittest.cc
@@ -4,9 +4,12 @@
 
 #include "chromeos/audio/cras_audio_handler.h"
 
+#include "base/bind.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "chromeos/audio/audio_devices_pref_handler_stub.h"
 #include "chromeos/dbus/audio_node.h"
@@ -343,6 +346,18 @@ class CrasAudioHandlerTest : public testing::Test {
     return num_active_nodes;
   }
 
+  void SetActiveHDMIRediscover() {
+    cras_audio_handler_->SetActiveHDMIOutoutRediscoveringIfNecessary(true);
+  }
+
+  void SetHDMIRediscoverGracePeriodDuration(int duration_in_ms) {
+    cras_audio_handler_->SetHDMIRediscoverGracePeriodForTesting(duration_in_ms);
+  }
+
+  bool IsDuringHDMIRediscoverGracePeriod() {
+    return cras_audio_handler_->hdmi_rediscovering();
+  }
+
  protected:
   base::MessageLoopForUI message_loop_;
   CrasAudioHandler* cras_audio_handler_;  // Not owned.
@@ -354,6 +369,46 @@ class CrasAudioHandlerTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(CrasAudioHandlerTest);
 };
 
+class HDMIRediscoverWaiter {
+ public:
+  HDMIRediscoverWaiter(CrasAudioHandlerTest* cras_audio_handler_test,
+                       int grace_period_duration_in_ms)
+      : cras_audio_handler_test_(cras_audio_handler_test),
+        grace_period_duration_in_ms_(grace_period_duration_in_ms) {}
+
+  void WaitUntilTimeOut(int wait_duration_in_ms) {
+    base::RunLoop run_loop;
+    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE, run_loop.QuitClosure(),
+        base::TimeDelta::FromMilliseconds(wait_duration_in_ms));
+    run_loop.Run();
+  }
+
+  void CheckHDMIRediscoverGracePeriodEnd(const base::Closure& quit_loop_func) {
+    if (!cras_audio_handler_test_->IsDuringHDMIRediscoverGracePeriod()) {
+      quit_loop_func.Run();
+      return;
+    }
+    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&HDMIRediscoverWaiter::CheckHDMIRediscoverGracePeriodEnd,
+                   base::Unretained(this), quit_loop_func),
+        base::TimeDelta::FromMilliseconds(grace_period_duration_in_ms_ / 4));
+  }
+
+  void WaitUntilHDMIRediscoverGracePeriodEnd() {
+    base::RunLoop run_loop;
+    CheckHDMIRediscoverGracePeriodEnd(run_loop.QuitClosure());
+    run_loop.Run();
+  }
+
+ private:
+  CrasAudioHandlerTest* cras_audio_handler_test_;  // not owned
+  int grace_period_duration_in_ms_;
+
+  DISALLOW_COPY_AND_ASSIGN(HDMIRediscoverWaiter);
+};
+
 TEST_F(CrasAudioHandlerTest, InitializeWithOnlyDefaultAudioDevices) {
   AudioNodeList audio_nodes;
   audio_nodes.push_back(kInternalSpeaker);
@@ -2493,4 +2548,53 @@ TEST_F(CrasAudioHandlerTest, ActiveNodeLostDuringLoginSession) {
   EXPECT_TRUE(headphone_resumed->active);
 }
 
+// This test HDMI output rediscovering case in crbug.com/503667.
+TEST_F(CrasAudioHandlerTest, HDMIOutputRediscover) {
+  AudioNodeList audio_nodes;
+  audio_nodes.push_back(kInternalSpeaker);
+  audio_nodes.push_back(kHDMIOutput);
+  SetUpCrasAudioHandler(audio_nodes);
+
+  // Verify the HDMI device has been selected as the active output, and audio
+  // output is not muted.
+  AudioDevice active_output;
+  EXPECT_TRUE(
+      cras_audio_handler_->GetPrimaryActiveOutputDevice(&active_output));
+  EXPECT_EQ(kHDMIOutput.id, active_output.id);
+  EXPECT_EQ(kHDMIOutput.id, cras_audio_handler_->GetPrimaryActiveOutputNode());
+  EXPECT_TRUE(cras_audio_handler_->has_alternative_output());
+  EXPECT_FALSE(cras_audio_handler_->IsOutputMuted());
+
+  // Trigger HDMI rediscovering grace period, and remove the HDMI node.
+  const int grace_period_in_ms = 200;
+  SetHDMIRediscoverGracePeriodDuration(grace_period_in_ms);
+  SetActiveHDMIRediscover();
+  AudioNodeList audio_nodes_lost_hdmi;
+  audio_nodes_lost_hdmi.push_back(kInternalSpeaker);
+  ChangeAudioNodes(audio_nodes_lost_hdmi);
+
+  // Verify the active output is switched to internal speaker, it is not muted
+  // by preference, but the system output is muted during the grace period.
+  EXPECT_TRUE(
+      cras_audio_handler_->GetPrimaryActiveOutputDevice(&active_output));
+  EXPECT_EQ(kInternalSpeaker.id, active_output.id);
+  EXPECT_FALSE(
+      cras_audio_handler_->IsOutputMutedForDevice(kInternalSpeaker.id));
+  EXPECT_TRUE(cras_audio_handler_->IsOutputMuted());
+
+  // Re-attach the HDMI device after a little delay.
+  HDMIRediscoverWaiter waiter(this, grace_period_in_ms);
+  waiter.WaitUntilTimeOut(grace_period_in_ms / 4);
+  ChangeAudioNodes(audio_nodes);
+
+  // After HDMI re-discover grace period, verify HDMI output is selected as the
+  // active device and not muted.
+  waiter.WaitUntilHDMIRediscoverGracePeriodEnd();
+  EXPECT_TRUE(
+      cras_audio_handler_->GetPrimaryActiveOutputDevice(&active_output));
+  EXPECT_EQ(kHDMIOutput.id, active_output.id);
+  EXPECT_EQ(kHDMIOutput.id, cras_audio_handler_->GetPrimaryActiveOutputNode());
+  EXPECT_FALSE(cras_audio_handler_->IsOutputMuted());
+}
+
 }  // namespace chromeos