fix(audio): keep stream state consistent on control failures

Route asynchronous activation and sampling-frequency failures through the stream error callback. Keep a running stream active when SET_INTERFACE alt 0 cannot be submitted so stop can be retried without diverging from device state.

Signed-off-by: HiFiPHile <[email protected]>

4 files changed, 85 insertions, 17 deletions

diff --git a/examples/host/audio_host/src/audio_app.c b/examples/host/audio_host/src/audio_app.c
index 941248e99..0c4ed8a23 100644
--- a/examples/host/audio_host/src/audio_app.c
+++ b/examples/host/audio_host/src/audio_app.c
@@ -348,12 +348,18 @@ static void audio_app_restart_stream(uintptr_t param) {
   }
 }
 
-// Invoked when an isochronous transfer fails: the stream was stopped by the
-// driver, re-open it after a short delay so the device can recover.
+// Invoked when stream activation or an isochronous transfer fails: the stream
+// was stopped by the driver, re-open it after a short delay so the device can
+// recover.
 void tuh_audio_err_cb(uint8_t idx, uint8_t stream_idx, uint16_t xferred_bytes) {
   (void)xferred_bytes;
   err_cb_count++;
-  printf("  AUDIO transfer error: addr=%u stream=%u xferred_bytes=%u\r\n", idx, stream_idx, (unsigned)xferred_bytes);
+  if (stream_idx == cap_stream_idx) {
+    mic_ready = false;
+  } else if (stream_idx == spk_stream_idx) {
+    spk_ready = false;
+  }
+  printf("  AUDIO stream error: idx=%u stream=%u xferred_bytes=%u\r\n", idx, stream_idx, (unsigned)xferred_bytes);
   app_defer_ms_async(100, (app_defer_func_t)audio_app_restart_stream, ((uintptr_t)idx << 8) | stream_idx);
 }
 
diff --git a/src/class/audio/audio_host.c b/src/class/audio/audio_host.c
index 57aaf7d52..78ecd9525 100644
--- a/src/class/audio/audio_host.c
+++ b/src/class/audio/audio_host.c
@@ -373,6 +373,12 @@ static void audioh_stream_fail(tuh_audio_stream_t *s) {
   s->running       = false;
 }
 
+static void audioh_stream_error(tuh_audio_stream_t *s, uint16_t xferred_bytes) {
+  s->running = false;
+  tu_edpt_stream_clear(&s->edpt);
+  tuh_audio_err_cb(s->idx, s->stream_idx, xferred_bytes);
+}
+
 // Set the endpoint sampling frequency (3 bytes little-endian)
 static bool audioh_stream_set_freq(tuh_audio_stream_t *s, tuh_xfer_cb_t complete_cb) {
   const audioh_stream_map_t       *map  = &s->map[s->active_config];
@@ -496,9 +502,7 @@ bool audioh_xfer_cb(uint8_t dev_addr, uint8_t ep_addr, xfer_result_t result, uin
   // Failed, stalled, or aborted transfers never carry valid audio data
   if (result != XFER_RESULT_SUCCESS) {
     TU_LOG_DRV("  AUDIO transfer failed: addr=%u ep=%02x result=%u\r\n", dev_addr, ep_addr, result);
-    s->running = false;
-    tu_edpt_stream_clear(&s->edpt); // discard queued data
-    tuh_audio_err_cb(s->idx, s->stream_idx, (uint16_t)xferred_bytes);
+    audioh_stream_error(s, (uint16_t)xferred_bytes);
     return true;
   }
 
@@ -1080,7 +1084,7 @@ static void audioh_stream_start_set_freq_complete(tuh_xfer_t *xfer) {
   }
   if (xfer->result != XFER_RESULT_SUCCESS) {
     TU_LOG_DRV("  AUDIO set sampling frequency failed: result=%u\r\n", xfer->result);
-    s->running = false;
+    audioh_stream_error(s, 0);
     return;
   }
   audioh_stream_start_xfer(s);
@@ -1090,7 +1094,7 @@ static bool audioh_stream_start_active(tuh_audio_stream_t *s) {
   const audioh_stream_map_t *map = &s->map[s->active_config];
   if (map->sam_freq_ctrl) {
     if (!audioh_stream_set_freq(s, audioh_stream_start_set_freq_complete)) {
-      s->running = false;
+      audioh_stream_error(s, 0);
       return false;
     }
   } else {
@@ -1106,7 +1110,7 @@ static void audioh_stream_start_complete(tuh_xfer_t *xfer) {
   }
   if (xfer->result != XFER_RESULT_SUCCESS) {
     TU_LOG_DRV("  AUDIO SET_INTERFACE activate failed: result=%u\r\n", xfer->result);
-    s->running = false;
+    audioh_stream_error(s, 0);
     return;
   }
   (void)audioh_stream_start_active(s);
@@ -1152,16 +1156,18 @@ bool tuh_audio_stop(uint8_t dev_idx, uint8_t stream_idx) {
   tuh_audio_stream_t *s = audioh_get_stream_by_idx(p_audio, stream_idx);
   TU_VERIFY(s && s->state == STREAM_STATE_READY, false);
 
+  const audioh_stream_map_t *map = &s->map[s->active_config];
+  // Leave the stream running if SET_INTERFACE cannot be submitted, so the
+  // caller can retry without the host and device states diverging.
+  TU_VERIFY(tuh_interface_set(s->daddr, map->itf_num, 0, audioh_stream_stop_complete, (uintptr_t)s), false);
+
   // The in-flight transfer (if any) completes and its data is discarded;
-  // queued frames are dropped as well. The interface is deactivated (alt 0)
-  // so the device stops transferring.
+  // queued frames are dropped as well. The interface is being deactivated so
+  // the device stops transferring.
   s->running = false;
   tu_edpt_stream_clear(&s->edpt);
   s->rem_acc = 0; // restart the pacing accumulator on the next tuh_audio_start()
-
-  // Keep stop retryable if EP0 is busy and SET_INTERFACE cannot be submitted.
-  const audioh_stream_map_t *map = &s->map[s->active_config];
-  return tuh_interface_set(s->daddr, map->itf_num, 0, audioh_stream_stop_complete, (uintptr_t)s);
+  return true;
 }
 
 uint32_t tuh_audio_write(uint8_t dev_idx, uint8_t stream_idx, const void *buffer, uint32_t frame_count) {
diff --git a/src/class/audio/audio_host.h b/src/class/audio/audio_host.h
index b1e2a9169..98e103208 100644
--- a/src/class/audio/audio_host.h
+++ b/src/class/audio/audio_host.h
@@ -239,8 +239,9 @@ void tuh_audio_capture_cb(uint8_t idx, uint8_t stream_idx, uint16_t xferred_byte
 // a complete packet is not queued.
 void tuh_audio_playback_cb(uint8_t idx, uint8_t stream_idx, uint16_t xferred_bytes);
 
-// Invoked when an isochronous transfer fails. The stream is stopped
-// (tuh_audio_start() must be called again to resume).
+// Invoked when asynchronous stream activation or an isochronous transfer
+// fails. The stream is stopped (tuh_audio_start() must be called again to
+// resume). xferred_bytes is zero for an activation failure.
 void tuh_audio_err_cb(uint8_t idx, uint8_t stream_idx, uint16_t xferred_bytes);
 
 //--------------------------------------------------------------------+
diff --git a/test/unit-test/test/host/audio/test_audio_host.c b/test/unit-test/test/host/audio/test_audio_host.c
index 63f59c454..51342bfac 100644
--- a/test/unit-test/test/host/audio/test_audio_host.c
+++ b/test/unit-test/test/host/audio/test_audio_host.c
@@ -50,6 +50,18 @@ static uint8_t  edpt_xfer_data[16][8];
 static uint8_t *edpt_xfer_buffer[16];
 static uint8_t  edpt_xfer_count;
 
+static uint8_t  err_cb_count;
+static uint8_t  err_cb_idx;
+static uint8_t  err_cb_stream_idx;
+static uint16_t err_cb_xferred_bytes;
+
+void tuh_audio_err_cb(uint8_t idx, uint8_t stream_idx, uint16_t xferred_bytes) {
+  err_cb_count++;
+  err_cb_idx           = idx;
+  err_cb_stream_idx    = stream_idx;
+  err_cb_xferred_bytes = xferred_bytes;
+}
+
 tusb_speed_t tuh_speed_get(uint8_t daddr) {
   (void)daddr;
   return TUSB_SPEED_FULL;
@@ -439,6 +451,11 @@ void setUp(void) {
   memset(edpt_xfer_buffer, 0, sizeof(edpt_xfer_buffer));
   edpt_xfer_count = 0;
 
+  err_cb_count         = 0;
+  err_cb_idx           = TUSB_INDEX_INVALID_8;
+  err_cb_stream_idx    = TUSB_INDEX_INVALID_8;
+  err_cb_xferred_bytes = 0;
+
   fu_cb_count     = 0;
   fu_cb_user_data = 0;
 
@@ -558,6 +575,44 @@ void test_audio_host_sets_sampling_frequency_after_each_stream_activation(void)
   TEST_ASSERT_EQUAL_UINT8(2, edpt_xfer_count);
 }
 
+void test_audio_host_reports_asynchronous_start_failures(void) {
+  mount_descriptors(capture_fu_before_usb_output, sizeof(capture_fu_before_usb_output));
+  TEST_ASSERT_TRUE(tuh_audio_configure(0, 0, 0));
+
+  TEST_ASSERT_TRUE(tuh_audio_start(0, 0));
+  complete_interface_set(XFER_RESULT_FAILED);
+  TEST_ASSERT_EQUAL_UINT8(1, err_cb_count);
+  TEST_ASSERT_EQUAL_UINT8(0, err_cb_idx);
+  TEST_ASSERT_EQUAL_UINT8(0, err_cb_stream_idx);
+  TEST_ASSERT_EQUAL_UINT16(0, err_cb_xferred_bytes);
+
+  TEST_ASSERT_TRUE(tuh_audio_start(0, 0));
+  complete_interface_set(XFER_RESULT_SUCCESS);
+  complete_control_xfer(XFER_RESULT_STALLED);
+  TEST_ASSERT_EQUAL_UINT8(2, err_cb_count);
+
+  control_xfer_result = false;
+  TEST_ASSERT_TRUE(tuh_audio_start(0, 0));
+  complete_interface_set(XFER_RESULT_SUCCESS);
+  TEST_ASSERT_EQUAL_UINT8(3, err_cb_count);
+}
+
+void test_audio_host_keeps_running_when_stop_cannot_be_submitted(void) {
+  mount_descriptors(capture_fu_before_usb_output, sizeof(capture_fu_before_usb_output));
+  TEST_ASSERT_TRUE(tuh_audio_configure(0, 0, 0));
+  TEST_ASSERT_TRUE(tuh_audio_start(0, 0));
+  complete_interface_set(XFER_RESULT_SUCCESS);
+  complete_control_xfer(XFER_RESULT_SUCCESS);
+  TEST_ASSERT_EQUAL_UINT8(1, edpt_xfer_count);
+
+  interface_set_result = false;
+  TEST_ASSERT_FALSE(tuh_audio_stop(0, 0));
+
+  edpt_busy = false;
+  TEST_ASSERT_TRUE(audioh_xfer_cb(AUDIO_DEV_ADDR, 0x81, XFER_RESULT_SUCCESS, 96));
+  TEST_ASSERT_EQUAL_UINT8(2, edpt_xfer_count);
+}
+
 void test_audio_host_parses_discrete_frequencies_with_interval_greater_than_one(void) {
   tuh_audio_stream_config_t config;
   open_descriptors(playback_with_two_frequencies, sizeof(playback_with_two_frequencies));