fix(audio): accept one data endpoint per AS alternate

Collect one audio data endpoint for each alternate setting and ignore explicit feedback endpoints until feedback scheduling is implemented. Validate the maximum packet for every discrete rate against the endpoint, transfer buffer, and FIFO.

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

2 files changed, 127 insertions, 97 deletions

diff --git a/src/class/audio/audio_host.c b/src/class/audio/audio_host.c
index 722b6fcbc..57aaf7d52 100644
--- a/src/class/audio/audio_host.c
+++ b/src/class/audio/audio_host.c
@@ -554,9 +554,9 @@ static const uint8_t *audioh_parse_as(audioh_interface_t *p_audio, const tusb_de
   uint8_t  sam_freq_count                       = 0;
   uint32_t sam_freq[CFG_TUH_AUDIO_MAX_SAM_FREQ] = {0};
 
-  // An alternate setting can expose an endpoint in each direction. Explicit
-  // feedback endpoints are skipped; implicit-feedback data endpoints remain
-  // normal audio endpoints.
+  // An AS alternate setting has one audio data endpoint and may have one
+  // explicit feedback endpoint. Implicit-feedback endpoints are data endpoints
+  // and are handled normally when they are the AS interface's data endpoint.
   typedef struct {
     uint8_t  ep_addr;
     uint16_t ep_size;
@@ -565,8 +565,8 @@ static const uint8_t *audioh_parse_as(audioh_interface_t *p_audio, const tusb_de
     uint8_t  ep_usage;
     bool     sam_freq_ctrl;
   } audioh_ep_info_t;
-  audioh_ep_info_t ep_info[2] = {0};
-  uint8_t          ep_count   = 0;
+  audioh_ep_info_t ep_info     = {0};
+  bool             has_data_ep = false;
   // The CS_ENDPOINT descriptor carries the sampling-frequency control bit of
   // its endpoint. Devices differ in whether it precedes or follows the
   // standard endpoint descriptor, so attribute it in either order.
@@ -612,8 +612,8 @@ static const uint8_t *audioh_parse_as(audioh_interface_t *p_audio, const tusb_de
           const bool                              sam_freq_ctrl = (desc_ep->bmAttributes & 0x01) != 0;
           if (unassigned_ep) {
             // Standard order: the CS_ENDPOINT follows its endpoint descriptor
-            ep_info[ep_count - 1].sam_freq_ctrl = sam_freq_ctrl;
-            unassigned_ep                       = false;
+            ep_info.sam_freq_ctrl = sam_freq_ctrl;
+            unassigned_ep         = false;
           } else {
             // Non-standard order: the CS_ENDPOINT precedes its endpoint descriptor
             pending_sam_freq_ctrl = sam_freq_ctrl;
@@ -640,21 +640,26 @@ static const uint8_t *audioh_parse_as(audioh_interface_t *p_audio, const tusb_de
           break;
         }
 
-        if ((usage == (TUSB_ISO_EP_ATT_DATA >> 4) || implicit_feedback) && ep_count < 2) {
-          audioh_ep_info_t *ep = &ep_info[ep_count];
-          ep->ep_addr          = desc_endpoint->bEndpointAddress;
-          ep->ep_size          = tu_edpt_packet_size(desc_endpoint);
-          ep->ep_interval      = desc_endpoint->bInterval;
+        if (usage == (TUSB_ISO_EP_ATT_DATA >> 4) || implicit_feedback) {
+          if (has_data_ep) {
+            TU_LOG_DRV("  AUDIO AS itf %u alt %u: extra data ep %02x ignored\r\n", itf_num, alt,
+                       desc_endpoint->bEndpointAddress);
+            break;
+          }
+
+          ep_info.ep_addr     = desc_endpoint->bEndpointAddress;
+          ep_info.ep_size     = tu_edpt_packet_size(desc_endpoint);
+          ep_info.ep_interval = desc_endpoint->bInterval;
           // bInterval must be in [1, 16] for isochronous endpoints
-          if (ep->ep_interval == 0 || ep->ep_interval > 16) {
-            ep->ep_interval = 1;
+          if (ep_info.ep_interval == 0 || ep_info.ep_interval > 16) {
+            ep_info.ep_interval = 1;
           }
-          ep->ep_sync           = desc_endpoint->bmAttributes.sync;
-          ep->ep_usage          = desc_endpoint->bmAttributes.usage;
-          ep->sam_freq_ctrl     = pending_sam_freq_ctrl;
+          ep_info.ep_sync       = desc_endpoint->bmAttributes.sync;
+          ep_info.ep_usage      = desc_endpoint->bmAttributes.usage;
+          ep_info.sam_freq_ctrl = pending_sam_freq_ctrl;
           pending_sam_freq_ctrl = false;
-          unassigned_ep         = !ep->sam_freq_ctrl;
-          ep_count++;
+          unassigned_ep         = !ep_info.sam_freq_ctrl;
+          has_data_ep           = true;
         }
         break;
       }
@@ -664,7 +669,7 @@ static const uint8_t *audioh_parse_as(audioh_interface_t *p_audio, const tusb_de
     p_desc = tu_desc_next(p_desc);
   }
 
-  if (ep_count == 0) {
+  if (!has_data_ep) {
     return p_desc;
   }
 
@@ -692,77 +697,73 @@ static const uint8_t *audioh_parse_as(audioh_interface_t *p_audio, const tusb_de
   }
   const uint16_t frame_bytes = (uint16_t)frame_bytes_32;
 
-  // Register one configuration per (endpoint, discrete sampling frequency)
-  for (uint8_t e = 0; e < ep_count; e++) {
-    const audioh_ep_info_t *ep     = &ep_info[e];
-    tuh_audio_stream_t     *stream = audioh_get_stream(p_audio, tu_edpt_dir(ep->ep_addr));
-    if (stream == NULL) {
-      continue;
-    }
+  // Register one configuration per discrete sampling frequency
+  const audioh_ep_info_t *ep     = &ep_info;
+  tuh_audio_stream_t     *stream = audioh_get_stream(p_audio, tu_edpt_dir(ep->ep_addr));
+  TU_ASSERT(stream != NULL, p_desc);
+
+  const uint16_t epbuf_size = (stream->dir == TUSB_DIR_IN) ? CFG_TUH_AUDIO_EPIN_BUFSIZE : CFG_TUH_AUDIO_EPOUT_BUFSIZE;
+
+  if (ep->ep_size == 0 || ep->ep_size > iso_xfer_size) {
+    TU_LOG_DRV("  AUDIO AS itf %u alt %u: invalid isochronous ep size %u\r\n", itf_num, alt, ep->ep_size);
+    return p_desc;
+  }
 
-    const uint16_t epbuf_size = (stream->dir == TUSB_DIR_IN) ? CFG_TUH_AUDIO_EPIN_BUFSIZE : CFG_TUH_AUDIO_EPOUT_BUFSIZE;
+  // Capture: the device can deliver up to its max packet size per poll
+  // interval, the transfer buffer must fit it
+  if (stream->dir == TUSB_DIR_IN && (ep->ep_size > epbuf_size || ep->ep_size > CFG_TUH_AUDIO_STREAM_BUFSIZE)) {
+    TU_LOG_DRV("  AUDIO AS itf %u alt %u: capture ep size %u exceeds buffer capacity\r\n", itf_num, alt, ep->ep_size);
+    return p_desc;
+  }
 
-    if (ep->ep_size == 0 || ep->ep_size > iso_xfer_size) {
-      TU_LOG_DRV("  AUDIO AS itf %u alt %u: invalid isochronous ep size %u\r\n", itf_num, alt, ep->ep_size);
+  for (uint8_t i = 0; i < sam_freq_count; i++) {
+    if (sam_freq[i] == 0) {
       continue;
     }
 
-    // Capture: the device can deliver up to its max packet size per poll
-    // interval, the transfer buffer must fit it
-    if (stream->dir == TUSB_DIR_IN && (ep->ep_size > epbuf_size || ep->ep_size > CFG_TUH_AUDIO_STREAM_BUFSIZE)) {
-      TU_LOG_DRV("  AUDIO AS itf %u alt %u: capture ep size %u exceeds buffer capacity\r\n", itf_num, alt, ep->ep_size);
+    // The largest whole-frame packet for one poll interval must fit the
+    // endpoint. Playback must also stage it in the transfer buffer and FIFO.
+    const uint64_t frames_numerator = (uint64_t)sam_freq[i] * audioh_interval_us(ep->ep_interval, p_audio->daddr);
+    const uint64_t max_frames       = (frames_numerator + 999999u) / 1000000u;
+    const uint64_t packet_bytes     = max_frames * frame_bytes;
+    if (packet_bytes == 0 || packet_bytes > ep->ep_size ||
+        (stream->dir == TUSB_DIR_OUT && (packet_bytes > epbuf_size || packet_bytes > CFG_TUH_AUDIO_STREAM_BUFSIZE))) {
+      TU_LOG_DRV("  AUDIO AS itf %u alt %u: packet per interval does not fit endpoint/buffers (ep size %u)\r\n",
+                 itf_num, alt, ep->ep_size);
       continue;
     }
-
-    for (uint8_t i = 0; i < sam_freq_count; i++) {
-      if (sam_freq[i] == 0) {
-        continue;
-      }
-
-      // The largest whole-frame packet for one poll interval must fit the
-      // endpoint. Playback must also stage it in the transfer buffer and FIFO.
-      const uint64_t frames_numerator = (uint64_t)sam_freq[i] * audioh_interval_us(ep->ep_interval, p_audio->daddr);
-      const uint64_t max_frames       = (frames_numerator + 999999u) / 1000000u;
-      const uint64_t packet_bytes     = max_frames * frame_bytes;
-      if (packet_bytes == 0 || packet_bytes > ep->ep_size ||
-          (stream->dir == TUSB_DIR_OUT && (packet_bytes > epbuf_size || packet_bytes > CFG_TUH_AUDIO_STREAM_BUFSIZE))) {
-        TU_LOG_DRV("  AUDIO AS itf %u alt %u: packet per interval does not fit endpoint/buffers (ep size %u)\r\n",
-                   itf_num, alt, ep->ep_size);
-        continue;
-      }
-      // Skip duplicate configurations
-      bool duplicate = false;
-      for (uint8_t j = 0; j < stream->config_count; j++) {
-        if (stream->config[j].format == format && stream->config[j].sample_rate == sam_freq[i] &&
-            stream->config[j].channels == num_channels) {
-          duplicate = true;
-          break;
-        }
-      }
-      if (duplicate) {
-        continue;
-      }
-
-      if (stream->config_count >= AUDIOH_MAX_CONFIGS) {
-        TU_LOG_DRV("  AUDIO AS itf %u alt %u: reach max configurations %u\r\n", itf_num, alt, AUDIOH_MAX_CONFIGS);
-        return p_desc;
+    // Skip duplicate configurations
+    bool duplicate = false;
+    for (uint8_t j = 0; j < stream->config_count; j++) {
+      if (stream->config[j].format == format && stream->config[j].sample_rate == sam_freq[i] &&
+          stream->config[j].channels == num_channels) {
+        duplicate = true;
+        break;
       }
+    }
+    if (duplicate) {
+      continue;
+    }
 
-      stream->config[stream->config_count].dir =
-        (stream->dir == TUSB_DIR_IN) ? TUH_AUDIO_STREAM_CAPTURE : TUH_AUDIO_STREAM_PLAYBACK;
-      stream->config[stream->config_count].format      = format;
-      stream->config[stream->config_count].sample_rate = sam_freq[i];
-      stream->config[stream->config_count].channels    = num_channels;
-      stream->map[stream->config_count].itf_num        = itf_num;
-      stream->map[stream->config_count].alt_setting    = alt;
-      stream->map[stream->config_count].ep_addr        = ep->ep_addr;
-      stream->map[stream->config_count].ep_size        = ep->ep_size;
-      stream->map[stream->config_count].ep_interval    = ep->ep_interval;
-      stream->map[stream->config_count].ep_sync        = ep->ep_sync;
-      stream->map[stream->config_count].ep_usage       = ep->ep_usage;
-      stream->map[stream->config_count].sam_freq_ctrl  = ep->sam_freq_ctrl;
-      stream->config_count++;
+    if (stream->config_count >= AUDIOH_MAX_CONFIGS) {
+      TU_LOG_DRV("  AUDIO AS itf %u alt %u: reach max configurations %u\r\n", itf_num, alt, AUDIOH_MAX_CONFIGS);
+      return p_desc;
     }
+
+    stream->config[stream->config_count].dir =
+      (stream->dir == TUSB_DIR_IN) ? TUH_AUDIO_STREAM_CAPTURE : TUH_AUDIO_STREAM_PLAYBACK;
+    stream->config[stream->config_count].format      = format;
+    stream->config[stream->config_count].sample_rate = sam_freq[i];
+    stream->config[stream->config_count].channels    = num_channels;
+    stream->map[stream->config_count].itf_num        = itf_num;
+    stream->map[stream->config_count].alt_setting    = alt;
+    stream->map[stream->config_count].ep_addr        = ep->ep_addr;
+    stream->map[stream->config_count].ep_size        = ep->ep_size;
+    stream->map[stream->config_count].ep_interval    = ep->ep_interval;
+    stream->map[stream->config_count].ep_sync        = ep->ep_sync;
+    stream->map[stream->config_count].ep_usage       = ep->ep_usage;
+    stream->map[stream->config_count].sam_freq_ctrl  = ep->sam_freq_ctrl;
+    stream->config_count++;
   }
 
   return p_desc;
@@ -1022,7 +1023,6 @@ bool tuh_audio_configure(uint8_t dev_idx, uint8_t stream_idx, uint8_t config_idx
     TU_VERIFY(!usbh_edpt_busy(s->daddr, s->edpt.ep_addr), false);
   }
 
-  // A shared AS interface must not be left in two different alternate settings
   tuh_audio_stream_t *other = (s == &p_audio->out_stream) ? &p_audio->in_stream : &p_audio->out_stream;
   if (other->active_config != TUSB_INDEX_INVALID_8) {
     const tuh_audio_stream_config_t *other_cfg = &other->config[other->active_config];
@@ -1031,13 +1031,6 @@ bool tuh_audio_configure(uint8_t dev_idx, uint8_t stream_idx, uint8_t config_idx
                  (unsigned long)cfg->sample_rate, (unsigned long)other_cfg->sample_rate);
       return false;
     }
-
-    const audioh_stream_map_t *m1 = &s->map[config_idx];
-    const audioh_stream_map_t *m2 = &other->map[other->active_config];
-    if (m1->itf_num == m2->itf_num && m1->alt_setting != m2->alt_setting) {
-      TU_LOG_DRV("  AUDIO configure failed: shared AS itf %u in conflicting alt settings\r\n", m1->itf_num);
-      return false;
-    }
   }
 
   // The HCD endpoint must be reopened even when the new configuration uses
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 4137642b3..63f59c454 100644
--- a/test/unit-test/test/host/audio/test_audio_host.c
+++ b/test/unit-test/test/host/audio/test_audio_host.c
@@ -10,9 +10,10 @@ TEST_SOURCE_FILE("audio_host.c")
 TEST_SOURCE_FILE("tusb_fifo.c")
 
 enum {
-  AUDIO_DEV_ADDR = 1,
-  AUDIO_AC_ITF   = 0,
-  AUDIO_AS_ITF   = 1,
+  AUDIO_DEV_ADDR  = 1,
+  AUDIO_AC_ITF    = 0,
+  AUDIO_AS_ITF    = 1,
+  AUDIO_AS_IN_ITF = 2,
 
   PLAYBACK_INPUT_TERM  = 1,
   PLAYBACK_FU          = 2,
@@ -177,6 +178,11 @@ uint32_t tu_edpt_stream_read_xfer(tu_edpt_stream_t *s) {
   9, TUSB_DESC_INTERFACE, AUDIO_AC_ITF, 0, 0, TUSB_CLASS_AUDIO, AUDIO_SUBCLASS_CONTROL, AUDIO_INT_PROTOCOL_CODE_V1, 0, \
     9, TUSB_DESC_CS_INTERFACE, AUDIO10_CS_AC_INTERFACE_HEADER, 0x00, 0x01, 0x09, 0x00, 1, AUDIO_AS_ITF
 
+#define TEST_UAC1_AC_HEADER_2                                                                                          \
+  9, TUSB_DESC_INTERFACE, AUDIO_AC_ITF, 0, 0, TUSB_CLASS_AUDIO, AUDIO_SUBCLASS_CONTROL, AUDIO_INT_PROTOCOL_CODE_V1, 0, \
+    10, TUSB_DESC_CS_INTERFACE, AUDIO10_CS_AC_INTERFACE_HEADER, 0x00, 0x01, 0x34, 0x00, 2, AUDIO_AS_ITF,               \
+    AUDIO_AS_IN_ITF
+
 #define TEST_UAC1_INPUT_TERM(_id, _type, _channels)                                                            \
   12, TUSB_DESC_CS_INTERFACE, AUDIO10_CS_AC_INTERFACE_INPUT_TERMINAL, _id, U16_TO_U8S_LE(_type), 0, _channels, \
     U16_TO_U8S_LE(AUDIO10_CHANNEL_CONFIG_NON_PREDEFINED), 0, 0
@@ -188,11 +194,13 @@ uint32_t tu_edpt_stream_read_xfer(tu_edpt_stream_t *s) {
 #define TEST_UAC1_OUTPUT_TERM(_id, _type, _source_id)                                                             \
   9, TUSB_DESC_CS_INTERFACE, AUDIO10_CS_AC_INTERFACE_OUTPUT_TERMINAL, _id, U16_TO_U8S_LE(_type), 0, _source_id, 0
 
-#define TEST_UAC1_AS_INTERFACE(_alt, _ep_count)                                                      \
-  9, TUSB_DESC_INTERFACE, AUDIO_AS_ITF, _alt, _ep_count, TUSB_CLASS_AUDIO, AUDIO_SUBCLASS_STREAMING, \
+#define TEST_UAC1_AS_INTERFACE_NUM(_itf, _alt, _ep_count)                                    \
+  9, TUSB_DESC_INTERFACE, _itf, _alt, _ep_count, TUSB_CLASS_AUDIO, AUDIO_SUBCLASS_STREAMING, \
     AUDIO_INT_PROTOCOL_CODE_V1, 0
 
-#define TEST_UAC1_AS_ALT0 TEST_UAC1_AS_INTERFACE(0, 0)
+#define TEST_UAC1_AS_INTERFACE(_alt, _ep_count) TEST_UAC1_AS_INTERFACE_NUM(AUDIO_AS_ITF, _alt, _ep_count)
+
+#define TEST_UAC1_AS_ALT0                       TEST_UAC1_AS_INTERFACE(0, 0)
 
 #define TEST_UAC1_AS_GENERAL(_term_id)                                        \
   7, TUSB_DESC_CS_INTERFACE, AUDIO10_CS_AS_INTERFACE_AS_GENERAL, _term_id, 1, \
@@ -203,8 +211,10 @@ uint32_t tu_edpt_stream_read_xfer(tu_edpt_stream_t *s) {
     AUDIO10_FORMAT_TYPE_I, _channels, _bytes, _bits, TU_ARGS_NUM(__VA_ARGS__),                     \
     TU_ARGS_APPLY_EXPAND(U24_TO_U8S_LE, __VA_ARGS__)
 
-#define TEST_UAC1_DATA_EP(_ep, _attr, _size, _interval)                                                \
-  9, TUSB_DESC_ENDPOINT, _ep, (TUSB_XFER_ISOCHRONOUS | (_attr)), U16_TO_U8S_LE(_size), _interval, 0, 0
+#define TEST_UAC1_DATA_EP_SYNC(_ep, _attr, _size, _interval, _sync_ep)                                        \
+  9, TUSB_DESC_ENDPOINT, _ep, (TUSB_XFER_ISOCHRONOUS | (_attr)), U16_TO_U8S_LE(_size), _interval, 0, _sync_ep
+
+#define TEST_UAC1_DATA_EP(_ep, _attr, _size, _interval) TEST_UAC1_DATA_EP_SYNC(_ep, _attr, _size, _interval, 0)
 
 #define TEST_UAC1_CS_DATA_EP_ATTR(_attr)                                                                               \
   7, TUSB_DESC_CS_ENDPOINT, AUDIO10_CS_EP_SUBTYPE_GENERAL, _attr, AUDIO10_CS_AS_ISO_DATA_EP_LOCK_DELAY_UNIT_UNDEFINED, \
@@ -240,6 +250,26 @@ static const uint8_t midi_only_collection[] = {
 };
 
 static const uint8_t playback_with_implicit_feedback[] = {
+  TEST_UAC1_AC_HEADER_2,
+  TEST_UAC1_INPUT_TERM(PLAYBACK_INPUT_TERM, AUDIO_TERM_TYPE_USB_STREAMING, 2),
+  TEST_UAC1_OUTPUT_TERM(PLAYBACK_OUTPUT_TERM, AUDIO_TERM_TYPE_OUT_HEADPHONES, PLAYBACK_INPUT_TERM),
+  TEST_UAC1_INPUT_TERM(CAPTURE_INPUT_TERM, AUDIO_TERM_TYPE_IN_GENERIC_MIC, 1),
+  TEST_UAC1_OUTPUT_TERM(CAPTURE_OUTPUT_TERM, AUDIO_TERM_TYPE_USB_STREAMING, CAPTURE_INPUT_TERM),
+  TEST_UAC1_AS_ALT0,
+  TEST_UAC1_AS_INTERFACE(1, 1),
+  TEST_UAC1_AS_GENERAL(PLAYBACK_INPUT_TERM),
+  TEST_UAC1_FORMAT(2, 2, 16, 48000),
+  TEST_UAC1_DATA_EP_SYNC(0x01, TUSB_ISO_EP_ATT_ADAPTIVE, 192, 1, 0x81),
+  TEST_UAC1_CS_DATA_EP,
+  TEST_UAC1_AS_INTERFACE_NUM(AUDIO_AS_IN_ITF, 0, 0),
+  TEST_UAC1_AS_INTERFACE_NUM(AUDIO_AS_IN_ITF, 1, 1),
+  TEST_UAC1_AS_GENERAL(CAPTURE_OUTPUT_TERM),
+  TEST_UAC1_FORMAT(1, 2, 16, 48000),
+  TEST_UAC1_DATA_EP(0x81, TUSB_ISO_EP_ATT_IMPLICIT_FB, 96, 1),
+  TEST_UAC1_CS_DATA_EP,
+};
+
+static const uint8_t playback_with_extra_data_endpoint[] = {
   TEST_UAC1_AC_HEADER,
   TEST_UAC1_INPUT_TERM(PLAYBACK_INPUT_TERM, AUDIO_TERM_TYPE_USB_STREAMING, 2),
   TEST_UAC1_OUTPUT_TERM(PLAYBACK_OUTPUT_TERM, AUDIO_TERM_TYPE_OUT_HEADPHONES, PLAYBACK_INPUT_TERM),
@@ -450,6 +480,13 @@ void test_audio_host_treats_implicit_feedback_as_audio_in_endpoint(void) {
   TEST_ASSERT_EQUAL(TUH_AUDIO_STREAM_CAPTURE, tuh_audio_stream_direction(0, 1));
 }
 
+void test_audio_host_ignores_second_data_endpoint_in_same_as_interface(void) {
+  open_descriptors(playback_with_extra_data_endpoint, sizeof(playback_with_extra_data_endpoint));
+
+  TEST_ASSERT_EQUAL_UINT8(1, tuh_audio_stream_count(0));
+  TEST_ASSERT_EQUAL(TUH_AUDIO_STREAM_PLAYBACK, tuh_audio_stream_direction(0, 0));
+}
+
 void test_audio_host_maps_playback_fu_declared_before_usb_input_terminal(void) {
   open_descriptors(playback_fu_before_terminal, sizeof(playback_fu_before_terminal));