CRAS: iodev_list - Skip ramping if device volume is zero

When adjusted active node volume is zero, or system volume is zero,
samples of a device will be suppressed to zero.
Since the device is already playing zeros, there is no need to ramp down
or up for mute/unmute change.

BUG=chromium:697511
TEST=on kevin, check there is no extra ramp when pressing volume down
when volume is 0.
TEST=make check

Change-Id: I648fab83f2e6dcfbd2db5370db0509d513d3b40d
Reviewed-on: https://chromium-review.googlesource.com/447852
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

2 files changed, 75 insertions, 4 deletions

diff --git a/cras/src/server/cras_iodev_list.c b/cras/src/server/cras_iodev_list.c
index 7490de78..5eac3e68 100644
--- a/cras/src/server/cras_iodev_list.c
+++ b/cras/src/server/cras_iodev_list.c
@@ -294,6 +294,25 @@ static void sys_vol_change(void *context, int32_t volume)
 	}
 }
 
+/*
+ * Checks if a device should start ramping for mute/unmute change.
+ * Device must meet all the conditions:
+ *
+ * - Device is enabled in iodev_list.
+ * - Device has ramp support.
+ * - Device is in normal run state, that is, it must be running with valid
+ *   streams.
+ * - Device volume, which considers both system volume and adjusted active
+ *   node volume, is not zero. If device volume is zero, all the samples are
+ *   suppressed to zero and there is no need to ramp.
+ */
+static int device_should_start_ramp_for_mute(const struct cras_iodev *dev)
+{
+	return (cras_iodev_list_dev_is_enabled(dev) && dev->ramp &&
+		cras_iodev_state(dev) == CRAS_IODEV_STATE_NORMAL_RUN &&
+		!cras_iodev_is_zero_volume(dev));
+}
+
 /* Called when the system mute state changes.  Pass the current mute setting
  * to the default output if it is active. */
 static void sys_mute_change(void *context, int muted, int user_muted,
@@ -303,9 +322,9 @@ static void sys_mute_change(void *context, int muted, int user_muted,
 	int should_mute = muted || user_muted;
 
 	DL_FOREACH(devs[CRAS_STREAM_OUTPUT].iodevs, dev) {
-		if (cras_iodev_list_dev_is_enabled(dev) && dev->ramp &&
-		    cras_iodev_state(dev) == CRAS_IODEV_STATE_NORMAL_RUN) {
-			/* Start ramping in audio thread and set mute/unmute
+		if (device_should_start_ramp_for_mute(dev)) {
+			/*
+			 * Start ramping in audio thread and set mute/unmute
 			 * state on device. This should only be done when
 			 * device is running with valid streams.
 			 *
@@ -314,7 +333,8 @@ static void sys_mute_change(void *context, int muted, int user_muted,
 			 * 2. Unmute -> Mute: Set device mute state after
 			 *                    ramping is done.
 			 *
-			 * The above transition will be handled by cras_iodev_ramp_start.
+			 * The above transition will be handled by
+			 * cras_iodev_ramp_start.
 			 */
 			audio_thread_dev_start_ramp(
 					audio_thread,
diff --git a/cras/src/tests/iodev_list_unittest.cc b/cras/src/tests/iodev_list_unittest.cc
index 4169bbf8..55509ded 100644
--- a/cras/src/tests/iodev_list_unittest.cc
+++ b/cras/src/tests/iodev_list_unittest.cc
@@ -79,6 +79,7 @@ static struct cras_iodev *audio_thread_dev_start_ramp_dev;
 static int audio_thread_dev_start_ramp_called;
 static enum CRAS_IODEV_RAMP_REQUEST audio_thread_dev_start_ramp_req ;
 static std::map<const struct cras_iodev*, enum CRAS_IODEV_STATE> cras_iodev_state_ret;
+static int cras_iodev_is_zero_volume_ret;
 
 void dummy_update_active_node(struct cras_iodev *iodev,
                               unsigned node_idx,
@@ -197,6 +198,7 @@ class IoDevTestSuite : public testing::Test {
       audio_thread_dev_start_ramp_called = 0;
       audio_thread_dev_start_ramp_req =
           CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK;
+      cras_iodev_is_zero_volume_ret = 0;
     }
 
     static void set_volume_1(struct cras_iodev* iodev) {
@@ -756,6 +758,28 @@ TEST_F(IoDevTestSuite, OutputMuteChangedToMute) {
   ASSERT_TRUE(device_in_vector(set_mute_dev_vector, &d2_));
   ASSERT_TRUE(device_in_vector(set_mute_dev_vector, &d3_));
 
+  // Assume d1_ should mute for volume.
+  // It should not use ramp.
+  cras_iodev_is_zero_volume_ret = 1;
+
+  // Clear stub data of interest.
+  audio_thread_dev_start_ramp_dev = NULL;
+  audio_thread_dev_start_ramp_called = 0;
+  set_mute_called = 0;
+  set_mute_dev_vector.clear();
+
+  // Execute the callback.
+  observer_ops->output_mute_changed(NULL, 0, 1, 0);
+
+  // Verify three devices all set mute state right away.
+  EXPECT_EQ(NULL, audio_thread_dev_start_ramp_dev);
+  EXPECT_EQ(0, audio_thread_dev_start_ramp_called);
+  EXPECT_EQ(3, set_mute_called);
+  EXPECT_EQ(3, set_mute_dev_vector.size());
+  ASSERT_TRUE(device_in_vector(set_mute_dev_vector, &d1_));
+  ASSERT_TRUE(device_in_vector(set_mute_dev_vector, &d2_));
+  ASSERT_TRUE(device_in_vector(set_mute_dev_vector, &d3_));
+
   // Assume d1_ is changed to no_stream run state
   // It should not use ramp.
   cras_iodev_state_ret[&d1_] = CRAS_IODEV_STATE_NO_STREAM_RUN;
@@ -818,6 +842,28 @@ TEST_F(IoDevTestSuite, OutputMuteChangedToUnmute) {
   ASSERT_TRUE(device_in_vector(set_mute_dev_vector, &d2_));
   ASSERT_TRUE(device_in_vector(set_mute_dev_vector, &d3_));
 
+  // Assume d1_ should mute for volume.
+  // It should not use ramp.
+  cras_iodev_is_zero_volume_ret = 1;
+
+  // Clear stub data of interest.
+  audio_thread_dev_start_ramp_dev = NULL;
+  audio_thread_dev_start_ramp_called = 0;
+  set_mute_called = 0;
+  set_mute_dev_vector.clear();
+
+  // Execute the callback.
+  observer_ops->output_mute_changed(NULL, 0, 1, 0);
+
+  // Verify three devices all set mute state right away.
+  EXPECT_EQ(NULL, audio_thread_dev_start_ramp_dev);
+  EXPECT_EQ(0, audio_thread_dev_start_ramp_called);
+  EXPECT_EQ(3, set_mute_called);
+  EXPECT_EQ(3, set_mute_dev_vector.size());
+  ASSERT_TRUE(device_in_vector(set_mute_dev_vector, &d1_));
+  ASSERT_TRUE(device_in_vector(set_mute_dev_vector, &d2_));
+  ASSERT_TRUE(device_in_vector(set_mute_dev_vector, &d3_));
+
   // Assume d1_ is changed to no_stream run state
   // It should not use ramp.
   cras_iodev_state_ret[&d1_] = CRAS_IODEV_STATE_NO_STREAM_RUN;
@@ -1413,6 +1459,11 @@ int cras_iodev_set_mute(struct cras_iodev* iodev) {
   return 0;
 }
 
+int cras_iodev_is_zero_volume(const struct cras_iodev *iodev)
+{
+  return cras_iodev_is_zero_volume_ret;
+}
+
 enum CRAS_IODEV_STATE cras_iodev_state(const struct cras_iodev *iodev)
 {
 	return cras_iodev_state_ret[iodev];