CRAS: iodev - Avoid repeated reset request

When severe underrun is triggered, audio thread will send multiple reset
request in a short time, while main thread has not handle the first
request yet. Add a flag in cras_iodev so we can ignore multiple reset
request until device is actually opened.

BUG=chrome-os-partner:60497
TEST=modify SEVERE_UNDERRUN_MS to 0.1 and trigger underrun by
cras_test_client. Check messages and see there is only one reset request
works.

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

5 files changed, 81 insertions, 12 deletions

diff --git a/cras/src/server/audio_thread.c b/cras/src/server/audio_thread.c
index f45f45a4..447818b7 100644
--- a/cras/src/server/audio_thread.c
+++ b/cras/src/server/audio_thread.c
@@ -15,7 +15,6 @@
 #include "cras_audio_area.h"
 #include "audio_thread_log.h"
 #include "cras_config.h"
-#include "cras_device_monitor.h"
 #include "cras_fmt_conv.h"
 #include "cras_iodev.h"
 #include "cras_rstream.h"
@@ -1211,7 +1210,7 @@ static int do_playback(struct audio_thread *thread)
 				/* Handle severe underrun. */
 				ATLOG(atlog, AUDIO_THREAD_SEVERE_UNDERRUN,
 				      adev->dev->info.idx, 0, 0);
-				cras_device_monitor_reset_device(adev->dev);
+				cras_iodev_reset_request(adev->dev);
 			} else {
 				/* Device error, close it. */
 				thread_rm_open_adev(thread, adev);
diff --git a/cras/src/server/cras_iodev.c b/cras/src/server/cras_iodev.c
index 60cb30d2..3f152e3a 100644
--- a/cras/src/server/cras_iodev.c
+++ b/cras/src/server/cras_iodev.c
@@ -14,6 +14,7 @@
 #include "audio_thread_log.h"
 #include "buffer_share.h"
 #include "cras_audio_area.h"
+#include "cras_device_monitor.h"
 #include "cras_dsp.h"
 #include "cras_dsp_pipeline.h"
 #include "cras_fmt_conv.h"
@@ -796,6 +797,7 @@ int cras_iodev_open(struct cras_iodev *iodev, unsigned int cb_level)
 	iodev->min_cb_level = MIN(iodev->buffer_size / 2, cb_level);
 	iodev->max_cb_level = 0;
 
+	iodev->reset_request_pending = 0;
 	iodev->state = CRAS_IODEV_STATE_OPEN;
 
 	if (iodev->direction == CRAS_STREAM_OUTPUT) {
@@ -1121,3 +1123,21 @@ unsigned int cras_iodev_get_num_severe_underruns(const struct cras_iodev *iodev)
 		return iodev->get_num_severe_underruns(iodev);
 	return 0;
 }
+
+int cras_iodev_reset_request(struct cras_iodev* iodev)
+{
+	/* Ignore requests if there is a pending request.
+	 * This function sends the request from audio thread to main
+	 * thread when audio thread finds a device is in a bad state
+	 * e.g. severe underrun. Before main thread receives the
+	 * request and resets device, audio thread might try to send
+	 * multiple requests because it finds device is still in bad
+	 * state. We should ignore requests in this cause. Otherwise,
+	 * main thread will reset device multiple times.
+	 * The flag is cleared in cras_iodev_open.
+	 * */
+	if (iodev->reset_request_pending)
+		return 0;
+	iodev->reset_request_pending = 1;
+	return cras_device_monitor_reset_device(iodev);
+}
diff --git a/cras/src/server/cras_iodev.h b/cras/src/server/cras_iodev.h
index 11923b26..dd7dcd94 100644
--- a/cras/src/server/cras_iodev.h
+++ b/cras/src/server/cras_iodev.h
@@ -162,6 +162,7 @@ struct cras_ionode {
  *     reference.
  * pre_dsp_hook_cb_data - Callback data that will be passing to pre_dsp_hook.
  * post_dsp_hook_cb_data - Callback data that will be passing to post_dsp_hook.
+ * reset_request_pending - The flag for pending reset request.
  */
 struct cras_iodev {
 	void (*set_volume)(struct cras_iodev *iodev);
@@ -220,6 +221,7 @@ struct cras_iodev {
 	loopback_hook_t post_dsp_hook;
 	void *pre_dsp_hook_cb_data;
 	void *post_dsp_hook_cb_data;
+	int reset_request_pending;
 	struct cras_iodev *prev, *next;
 };
 
@@ -611,4 +613,14 @@ unsigned int cras_iodev_get_num_underruns(const struct cras_iodev *iodev);
 unsigned int cras_iodev_get_num_severe_underruns(
 		const struct cras_iodev *iodev);
 
+/* Request main thread to re-open device. This should be used in audio thread
+ * when it finds device is in a bad state. The request will be ignored if
+ * there is still a pending request.
+ * Args:
+ *    iodev[in] - The device.
+ * Returns:
+ *    0 on success. Negative error code on failure.
+ */
+int cras_iodev_reset_request(struct cras_iodev* iodev);
+
 #endif /* CRAS_IODEV_H_ */
diff --git a/cras/src/tests/audio_thread_unittest.cc b/cras/src/tests/audio_thread_unittest.cc
index 829d1445..fd29739c 100644
--- a/cras/src/tests/audio_thread_unittest.cc
+++ b/cras/src/tests/audio_thread_unittest.cc
@@ -31,8 +31,8 @@ static unsigned int cras_iodev_prepare_output_before_write_samples_called;
 static enum CRAS_IODEV_STATE cras_iodev_prepare_output_before_write_samples_state;
 static unsigned int cras_iodev_get_output_buffer_called;
 static int cras_iodev_prepare_output_before_write_samples_ret;
-static int cras_device_monitor_reset_device_called;
-static struct cras_iodev *cras_device_monitor_reset_device_iodev;
+static int cras_iodev_reset_request_called;
+static struct cras_iodev *cras_iodev_reset_request_iodev;
 
 void ResetGlobalStubData() {
   cras_rstream_dev_offset_called = 0;
@@ -58,8 +58,8 @@ void ResetGlobalStubData() {
   cras_iodev_prepare_output_before_write_samples_state = CRAS_IODEV_STATE_OPEN;
   cras_iodev_get_output_buffer_called = 0;
   cras_iodev_prepare_output_before_write_samples_ret = 0;
-  cras_device_monitor_reset_device_called = 0;
-  cras_device_monitor_reset_device_iodev = NULL;
+  cras_iodev_reset_request_called = 0;
+  cras_iodev_reset_request_iodev = NULL;
 }
 
 // Test streams and devices manipulation.
@@ -552,9 +552,9 @@ TEST_F(StreamDeviceSuite, DoPlaybackUnderrun) {
 
   do_playback(thread_);
 
-  // Audio thread should trigger device monitor to reset device.
-  EXPECT_EQ(1, cras_device_monitor_reset_device_called);
-  EXPECT_EQ(&iodev, cras_device_monitor_reset_device_iodev);
+  // Audio thread should ask main thread to reset device.
+  EXPECT_EQ(1, cras_iodev_reset_request_called);
+  EXPECT_EQ(&iodev, cras_iodev_reset_request_iodev);
 
   thread_rm_open_dev(thread_, &iodev);
   TearDownRstream(&rstream);
@@ -939,10 +939,10 @@ unsigned int cras_iodev_get_num_underruns(const struct cras_iodev *iodev)
   return 0;
 }
 
-int cras_device_monitor_reset_device(struct cras_iodev *iodev)
+int cras_iodev_reset_request(struct cras_iodev *iodev)
 {
-  cras_device_monitor_reset_device_called++;
-  cras_device_monitor_reset_device_iodev = iodev;
+  cras_iodev_reset_request_called++;
+  cras_iodev_reset_request_iodev = iodev;
   return 0;
 }
 
diff --git a/cras/src/tests/iodev_unittest.cc b/cras/src/tests/iodev_unittest.cc
index efafe778..42b7c948 100644
--- a/cras/src/tests/iodev_unittest.cc
+++ b/cras/src/tests/iodev_unittest.cc
@@ -77,6 +77,7 @@ static unsigned int simple_no_stream_called;
 static int simple_no_stream_enable;
 static int dev_stream_playback_frames_ret;
 static int get_num_underruns_ret;
+static int device_monitor_reset_device_called;
 
 
 // Iodev callback
@@ -150,6 +151,7 @@ void ResetStubData() {
   if (!atlog)
     atlog = audio_thread_event_log_init();
   get_num_underruns_ret = 0;
+  device_monitor_reset_device_called = 0;
 }
 
 namespace {
@@ -1383,6 +1385,37 @@ TEST(IoDev, GetNumUnderruns) {
   EXPECT_EQ(10, cras_iodev_get_num_underruns(&iodev));
 }
 
+TEST(IoDev, RequestReset) {
+  struct cras_iodev iodev;
+  memset(&iodev, 0, sizeof(iodev));
+
+  ResetStubData();
+
+  iodev.open_dev = open_dev;
+  iodev.direction = CRAS_STREAM_OUTPUT;
+
+  iodev.state = CRAS_IODEV_STATE_CLOSE;
+  iodev_buffer_size = 1024;
+
+  // Open device.
+  cras_iodev_open(&iodev, 240);
+
+  // The first reset request works.
+  EXPECT_EQ(0, cras_iodev_reset_request(&iodev));
+  EXPECT_EQ(1, device_monitor_reset_device_called);
+
+  // The second reset request will do nothing.
+  EXPECT_EQ(0, cras_iodev_reset_request(&iodev));
+  EXPECT_EQ(1, device_monitor_reset_device_called);
+
+  // Assume device is opened again.
+  cras_iodev_open(&iodev, 240);
+
+  // The reset request works.
+  EXPECT_EQ(0, cras_iodev_reset_request(&iodev));
+  EXPECT_EQ(2, device_monitor_reset_device_called);
+}
+
 extern "C" {
 
 //  From libpthread.
@@ -1677,6 +1710,11 @@ int dev_stream_playback_frames(const struct dev_stream *dev_stream) {
   return dev_stream_playback_frames_ret;
 }
 
+int cras_device_monitor_reset_device(struct cras_iodev *iodev) {
+  device_monitor_reset_device_called++;
+  return 0;
+}
+
 }  // extern "C"
 }  //  namespace