CRAS: mute the new enabled device for 0.5s after device switch

1. Let all different kinds of ramp request for device state transits
from open or no stream into normal be triggered in
cras_iodev_output_event_sample_ready() by audio_thread. In that case
some popped noise due to device switch or resume can be surely
prevented.

2. When active_node is updated, mute the new enabled device for 0.5s to
reduce the popped noise afer device switch.

BUG=b:144892934
TEST=1. Play music and perform suspend/ resume, there should be no
popped noise.
2. Play music with headphone plugged and then unplugg the headphone,
there should be no popped noise.

Change-Id: Ib26291e33048876f28aa16d98d47408fecc7f362
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/2082971
Tested-by: Judy Hsiao <judyhsiao@chromium.org>
Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org>
Commit-Queue: Judy Hsiao <judyhsiao@chromium.org>

6 files changed, 73 insertions, 71 deletions

diff --git a/cras/src/server/cras_alsa_io.c b/cras/src/server/cras_alsa_io.c
index aeb6739b..6ccbc91e 100644
--- a/cras/src/server/cras_alsa_io.c
+++ b/cras/src/server/cras_alsa_io.c
@@ -1997,6 +1997,7 @@ alsa_iodev_create(size_t card_index, const char *card_name, size_t device_index,
 	iodev->ramp = cras_ramp_create();
 	if (iodev->ramp == NULL)
 		goto cleanup_iodev;
+	iodev->initial_ramp_request = CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK;
 
 	aio->mixer = mixer;
 	aio->config = config;
diff --git a/cras/src/server/cras_iodev.c b/cras/src/server/cras_iodev.c
index 4924491a..87287efd 100644
--- a/cras/src/server/cras_iodev.c
+++ b/cras/src/server/cras_iodev.c
@@ -37,6 +37,8 @@
 static const float RAMP_UNMUTE_DURATION_SECS = 0.5;
 static const float RAMP_NEW_STREAM_DURATION_SECS = 0.01;
 static const float RAMP_MUTE_DURATION_SECS = 0.1;
+static const float RAMP_RESUME_MUTE_DURATION_SECS = 1;
+static const float RAMP_SWITCH_MUTE_DURATION_SECS = 0.5;
 static const float RAMP_VOLUME_CHANGE_DURATION_SECS = 0.1;
 
 /*
@@ -258,14 +260,11 @@ static int cras_iodev_output_event_sample_ready(struct cras_iodev *odev)
 {
 	if (odev->state == CRAS_IODEV_STATE_OPEN ||
 	    odev->state == CRAS_IODEV_STATE_NO_STREAM_RUN) {
-		int ramp_mute = odev->ramp_mute;
 		/* Starts ramping up if device should not be muted.
-		 * Both mute, ramp_mute and volume are taken into consideration.
+		 * Both mute and volume are taken into consideration.
 		 */
-		if (odev->ramp && !output_should_mute(odev) && !ramp_mute) {
-			cras_iodev_start_ramp(
-				odev,
-				CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK);
+		if (odev->ramp && !output_should_mute(odev)) {
+			cras_iodev_start_ramp(odev, odev->initial_ramp_request);
 		}
 	}
 
@@ -1474,12 +1473,6 @@ static void ramp_down_mute_callback(void *data)
 	cras_device_monitor_set_device_mute_state(odev->info.idx);
 }
 
-static void ramp_mute_callback(void *data)
-{
-	struct cras_iodev *odev = (struct cras_iodev *)data;
-	cras_iodev_set_ramp_mute(odev, 0);
-}
-
 /* Used in audio thread. Check the docstrings of CRAS_IODEV_RAMP_REQUEST. */
 int cras_iodev_start_ramp(struct cras_iodev *odev,
 			  enum CRAS_IODEV_RAMP_REQUEST request)
@@ -1513,12 +1506,19 @@ int cras_iodev_start_ramp(struct cras_iodev *odev,
 		cb = ramp_down_mute_callback;
 		cb_data = (void *)odev;
 		break;
-	case CRAS_IODEV_RAMP_REQUEST_MUTE:
+	case CRAS_IODEV_RAMP_REQUEST_RESUME_MUTE:
 		from = 0;
 		to = 0;
-		duration_secs = RAMP_MUTE_DURATION_SECS;
-		cb = ramp_mute_callback;
-		cb_data = (void *)odev;
+		duration_secs = RAMP_RESUME_MUTE_DURATION_SECS;
+		odev->initial_ramp_request =
+			CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK;
+		break;
+	case CRAS_IODEV_RAMP_REQUEST_SWITCH_MUTE:
+		from = 0;
+		to = 0;
+		duration_secs = RAMP_SWITCH_MUTE_DURATION_SECS;
+		odev->initial_ramp_request =
+			CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK;
 		break;
 	default:
 		return -EINVAL;
@@ -1586,21 +1586,6 @@ int cras_iodev_set_mute(struct cras_iodev *iodev)
 	return 0;
 }
 
-int cras_iodev_set_ramp_mute(struct cras_iodev *odev, int ramp_mute)
-{
-	if (!odev->ramp)
-		return 0;
-
-	if (ramp_mute) {
-		if (output_should_mute(odev))
-			return 0;
-
-		cras_iodev_start_ramp(odev, CRAS_IODEV_RAMP_REQUEST_MUTE);
-	}
-	odev->ramp_mute = ramp_mute;
-	return 0;
-}
-
 void cras_iodev_update_highest_hw_level(struct cras_iodev *iodev,
 					unsigned int hw_level)
 {
diff --git a/cras/src/server/cras_iodev.h b/cras/src/server/cras_iodev.h
index 0dd29b06..062024b3 100644
--- a/cras/src/server/cras_iodev.h
+++ b/cras/src/server/cras_iodev.h
@@ -239,8 +239,8 @@ struct cras_ionode {
  *                    been processed by the input DSP.
  * input_data - Used to pass audio input data to streams with or without
  *              stream side processing.
- * ramp_mute - The flag indicates if the device is in ramp_mute state (using
- *             ramp to mute for RAMP_MUTE_DURATION_SECS seconds)
+ * initial_ramp_request - The value indicates which type of ramp the device
+ * should perform when some samples are ready for playback.
  *
  */
 struct cras_iodev {
@@ -314,7 +314,7 @@ struct cras_iodev {
 	int input_streaming;
 	unsigned int input_frames_read;
 	unsigned int input_dsp_offset;
-	int ramp_mute;
+	unsigned int initial_ramp_request;
 	struct input_data *input_data;
 	struct cras_iodev *prev, *next;
 };
@@ -344,16 +344,23 @@ struct cras_iodev {
  *   first sample of new stream is ready, there is no need to change mute/unmute
  *   state.
  *
- * - CRAS_IODEV_RAMP_REQUEST_MUTE: Mute the device for RAMP_MUTE_DURATION_SECS
- *   seconds
+ * - CRAS_IODEV_RAMP_REQUEST_RESUME_MUTE: To prevent popped noise, mute the
+ *   device for RAMP_RESUME_MUTE_DURATION_SECS seconds on sample ready after
+ *   resume if there were playback stream before suspend.
+ *
+ * - CRAS_IODEV_RAMP_REQUEST_SWITCH_MUTE: To prevent popped noise, mute the
+ *   device for RAMP_SWITCH_MUTE_DURATION_SECS seconds on sample ready after
+ *   device switch if there were playback stream before switch.
  *
  */
 
 enum CRAS_IODEV_RAMP_REQUEST {
-	CRAS_IODEV_RAMP_REQUEST_UP_UNMUTE = 0,
-	CRAS_IODEV_RAMP_REQUEST_DOWN_MUTE = 1,
-	CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK = 2,
-	CRAS_IODEV_RAMP_REQUEST_MUTE = 3,
+	CRAS_IODEV_RAMP_REQUEST_NONE = 0,
+	CRAS_IODEV_RAMP_REQUEST_UP_UNMUTE = 1,
+	CRAS_IODEV_RAMP_REQUEST_DOWN_MUTE = 2,
+	CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK = 3,
+	CRAS_IODEV_RAMP_REQUEST_RESUME_MUTE = 4,
+	CRAS_IODEV_RAMP_REQUEST_SWITCH_MUTE = 5,
 };
 
 /*
@@ -798,18 +805,6 @@ int cras_iodev_start_volume_ramp(struct cras_iodev *odev,
  */
 int cras_iodev_set_mute(struct cras_iodev *iodev);
 
-/* Start/stop iodev ramp_mute state.
- * Args:
- *    odev[in] - The output device.
- *    ramp_mute - 1 to set ramp_mute flag and start the ramp. The device which
- *                is not muted will be muted for RAMP_MUTE_DURATION_SECS
- *                seconds.
- *              - 0 to reset the ramp_mute flag.
- * Returns:
- *    0 on success. Negative error code on failure.
- */
-int cras_iodev_set_ramp_mute(struct cras_iodev *odev, int ramp_mute);
-
 /*
  * Checks if an output iodev's volume is zero.
  * If there is an active node, check the adjusted node volume.
diff --git a/cras/src/server/cras_iodev_list.c b/cras/src/server/cras_iodev_list.c
index 504e3920..6c154af8 100644
--- a/cras/src/server/cras_iodev_list.c
+++ b/cras/src/server/cras_iodev_list.c
@@ -530,22 +530,28 @@ static void resume_devs()
 
 	int has_output_stream = 0;
 	stream_list_suspended = 0;
+
+	/*
+	 * To remove the short popped noise caused by applications that can not
+	 * stop playback "right away" after resume, we mute all output devices
+	 * for a short time if there is any output stream.
+	 */
 	DL_FOREACH (stream_list_get(stream_list), rstream) {
-		if ((rstream->flags & HOTWORD_STREAM) == HOTWORD_STREAM)
-			continue;
-		stream_added_cb(rstream);
 		if (rstream->direction == CRAS_STREAM_OUTPUT)
 			has_output_stream++;
 	}
-
-	/* To remove the short popped noise caused by applications that can not
-         * stop playback "right away" after resume, we mute all output devices
-         * for a short time if there is any output stream.*/
 	if (has_output_stream) {
 		DL_FOREACH (enabled_devs[CRAS_STREAM_OUTPUT], edev) {
-			cras_iodev_set_ramp_mute(edev->dev, 1);
+			edev->dev->initial_ramp_request =
+				CRAS_IODEV_RAMP_REQUEST_RESUME_MUTE;
 		}
 	}
+
+	DL_FOREACH (stream_list_get(stream_list), rstream) {
+		if ((rstream->flags & HOTWORD_STREAM) == HOTWORD_STREAM)
+			continue;
+		stream_added_cb(rstream);
+	}
 }
 
 /* Called when the system audio is suspended or resumed. */
@@ -1498,6 +1504,8 @@ void cras_iodev_list_select_node(enum CRAS_STREAM_DIRECTION direction,
 	struct cras_iodev *new_dev = NULL;
 	struct enabled_dev *edev;
 	int new_node_already_enabled = 0;
+	struct cras_rstream *rstream;
+	int has_output_stream = 0;
 	int rc;
 
 	/* find the devices for the id. */
@@ -1555,6 +1563,19 @@ void cras_iodev_list_select_node(enum CRAS_STREAM_DIRECTION direction,
 
 	if (new_dev && !new_node_already_enabled) {
 		new_dev->update_active_node(new_dev, node_index_of(node_id), 1);
+
+		/* To reduce the popped noise of active device change, mute
+		 * new_dev's for RAMP_SWITCH_MUTE_DURATION_SECS s.
+		 */
+		DL_FOREACH (stream_list_get(stream_list), rstream) {
+			if (rstream->direction == CRAS_STREAM_OUTPUT)
+				has_output_stream++;
+		}
+		if (direction == CRAS_STREAM_OUTPUT && has_output_stream) {
+			new_dev->initial_ramp_request =
+				CRAS_IODEV_RAMP_REQUEST_SWITCH_MUTE;
+		}
+
 		rc = enable_device(new_dev);
 		if (rc == 0) {
 			/* Disable fallback device after new device is enabled.
diff --git a/cras/src/tests/iodev_list_unittest.cc b/cras/src/tests/iodev_list_unittest.cc
index 1ca8cf46..f9644faa 100644
--- a/cras/src/tests/iodev_list_unittest.cc
+++ b/cras/src/tests/iodev_list_unittest.cc
@@ -360,10 +360,12 @@ TEST_F(IoDevTestSuite, RampMuteAfterResume) {
   cras_iodev_list_init();
 
   d1_.direction = CRAS_STREAM_OUTPUT;
+  d1_.initial_ramp_request = CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK;
   rc = cras_iodev_list_add_output(&d1_);
   ASSERT_EQ(0, rc);
 
   d2_.direction = CRAS_STREAM_INPUT;
+  d2_.initial_ramp_request = CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK;
   rc = cras_iodev_list_add_input(&d2_);
   ASSERT_EQ(0, rc);
 
@@ -387,11 +389,12 @@ TEST_F(IoDevTestSuite, RampMuteAfterResume) {
   observer_ops->suspend_changed(NULL, 0);
 
   /* Test only output device that has stream will be muted after resume */
-  EXPECT_EQ(1, d1_.ramp_mute);
-  EXPECT_EQ(0, d2_.ramp_mute);
+  EXPECT_EQ(d1_.initial_ramp_request, CRAS_IODEV_RAMP_REQUEST_RESUME_MUTE);
+  EXPECT_EQ(CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK,
+            d2_.initial_ramp_request);
 
   /* Reset d1 ramp_mute and remove output stream to test again */
-  d1_.ramp_mute = 0;
+  d1_.initial_ramp_request = CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK;
   DL_DELETE(stream_list, &rstream);
   stream_list_get_ret = stream_list;
   stream_rm_cb(&rstream);
@@ -401,8 +404,10 @@ TEST_F(IoDevTestSuite, RampMuteAfterResume) {
   stream_list_get_ret = stream_list;
   observer_ops->suspend_changed(NULL, 0);
 
-  EXPECT_EQ(0, d1_.ramp_mute);
-  EXPECT_EQ(0, d2_.ramp_mute);
+  EXPECT_EQ(CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK,
+            d1_.initial_ramp_request);
+  EXPECT_EQ(CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK,
+            d2_.initial_ramp_request);
 
   cras_iodev_list_deinit();
 }
@@ -1792,11 +1797,6 @@ int cras_iodev_close(struct cras_iodev* iodev) {
   return 0;
 }
 
-int cras_iodev_set_ramp_mute(struct cras_iodev* odev, int ramp_mute) {
-  odev->ramp_mute = ramp_mute;
-  return 0;
-}
-
 int cras_iodev_set_format(struct cras_iodev* iodev,
                           const struct cras_audio_format* fmt) {
   return 0;
diff --git a/cras/src/tests/iodev_unittest.cc b/cras/src/tests/iodev_unittest.cc
index 7adb36cf..bb57e119 100644
--- a/cras/src/tests/iodev_unittest.cc
+++ b/cras/src/tests/iodev_unittest.cc
@@ -1722,7 +1722,7 @@ TEST(IoDev, PrepareOutputBeforeWriteSamples) {
 
   // Assume device has ramp member.
   iodev.ramp = reinterpret_cast<struct cras_ramp*>(0x1);
-
+  iodev.initial_ramp_request = CRAS_IODEV_RAMP_REQUEST_UP_START_PLAYBACK;
   // Case 4.1: Assume device with ramp is started and is in no stream state.
   iodev.state = CRAS_IODEV_STATE_NO_STREAM_RUN;
   // Assume sample is ready.