usbhost: poll a full-speed audio stream's isochronous endpoint every frame

.github/workflows/usbhost.yml

@@ -73,6 +73,15 @@ jobs:
           cc -std=c11 -Wall -Wextra -Werror -O1 -g -fsanitize=address,undefined -Isrc \
              -o /tmp/test_iso_stream tests/test_iso_stream.c
           /tmp/test_iso_stream
+          cc -std=c11 -Wall -Wextra -Werror -Isrc -o /tmp/test_audio_interval tests/test_audio_interval.c
+          /tmp/test_audio_interval
+          cc -std=c11 -Wall -Wextra -Werror -Isrc -DUSBHOST_AI_PLANT_NO_FIX \
+             -o /tmp/test_audio_interval_planted tests/test_audio_interval.c
+          if /tmp/test_audio_interval_planted > /tmp/planted.log 2>&1; then
+            cat /tmp/planted.log
+            echo "leaving the interval alone was NOT caught"; exit 1
+          fi
+          echo "planted fault caught:"; grep FAIL /tmp/planted.log | head -3
       - name: The pacing, framing and downmix tests can fail
         run: |
           for fault in USBHOST_UVC_FRAME_PLANT_NO_SYNC USBHOST_ISO_PACE_PLANT_DROP_REMAINDER USBHOST_DOWNMIX_PLANT_LEFT_ONLY; do

modules/usbhost/src/shared/usbhost_audio_interval.h

@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: MIT
+//
+// The poll interval of a full-speed audio stream's isochronous endpoint.
+//
+// An isochronous endpoint's bInterval is an exponent: 2^(bInterval-1) bus
+// slots, which is microframes (125 us) at high speed and frames (1 ms) at
+// full speed. A webcam's microphone has one configuration for both speeds and
+// says bInterval 4: eight microframes, 1 ms, at high speed, but eight frames,
+// 8 ms, at full speed. ESP-IDF's host driver polls what the descriptor says,
+// so behind a hub that runs the P4's host at full speed the microphone is
+// polled once in eight packets and a 16 kHz stereo stream arrives as 126
+// packets a second instead of 999 (workspace#16).
+//
+// USB Audio 1.0 (4.6.1.1) and 2.0 give an audio streaming endpoint a period
+// of one frame at full speed, and a host that serves audio, Linux's among
+// them, polls every frame whatever the field says. This sets the field to 1
+// for the audio streaming interface about to be claimed, in the copy of the
+// configuration descriptor the host library keeps, before the library reads
+// it to open the pipes.
+//
+// Header-only and free of ESP-IDF so the host-side tests can check it.
+#ifndef USBHOST_AUDIO_INTERVAL_H
+#define USBHOST_AUDIO_INTERVAL_H
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+
+#define USBHOST_AI_DT_INTERFACE   (4)
+#define USBHOST_AI_DT_ENDPOINT    (5)
+#define USBHOST_AI_CLASS_AUDIO    (1)
+#define USBHOST_AI_SUBCLASS_AS    (2)   // AudioStreaming
+#define USBHOST_AI_XFER_ISOC      (1)
+
+// Walk the configuration descriptor `cfg` (`len` bytes) and set bInterval to 1
+// on every isochronous endpoint of interface `itf`, alternate setting `alt`,
+// when that interface is an AudioStreaming one. Returns the number of
+// endpoints changed. The caller has already checked the bus is full speed.
+static inline int usbhost_audio_fs_interval_fix(uint8_t *cfg, size_t len, uint8_t itf, uint8_t alt) {
+    int changed = 0;
+    bool ours = false;
+    size_t off = 0;
+    while (off + 2 <= len && cfg[off] >= 2 && off + cfg[off] <= len) {
+        uint8_t *d = cfg + off;
+        if (d[1] == USBHOST_AI_DT_INTERFACE && d[0] >= 9) {
+            ours = d[2] == itf && d[3] == alt
+                && d[5] == USBHOST_AI_CLASS_AUDIO && d[6] == USBHOST_AI_SUBCLASS_AS;
+        } else if (d[1] == USBHOST_AI_DT_ENDPOINT && d[0] >= 7 && ours
+            && (d[3] & 0x03) == USBHOST_AI_XFER_ISOC && d[6] > 1) {
+#ifndef USBHOST_AI_PLANT_NO_FIX
+            d[6] = 1;
+#endif
+            changed++;
+        }
+        off += d[0];
+    }
+    return changed;
+}
+
+#endif // USBHOST_AUDIO_INTERVAL_H

modules/usbhost/src/usbhost_host_stream.c

@@ -388,6 +388,8 @@ static int usbhost_stream_open_locked(uint32_t dev_id, uint8_t ep, const usbhost
     usb_device_info_t info;
     const bool high_speed = usb_host_device_info(dev, &info) == ESP_OK && info.speed == USB_SPEED_HIGH;
     // An isochronous bInterval is an exponent on either bus: 2^(b-1) slots.
+    // A full-speed audio endpoint has had its changed to 1 at the claim
+    // (usbhost_audio_interval.h), so this is the interval IDF polls at.
     const uint32_t interval = (binterval >= 1 && binterval <= 16) ? 1u << (binterval - 1) : 1;
     const usbhost_iso_queue_t q = usbhost_iso_queue_size(high_speed, interval, portTICK_PERIOD_MS);
 

modules/usbhost/src/usbhost_host_xfer.c

@@ -43,6 +43,7 @@
 #include "usb/usb_host.h"
 #include "usbhost_ticks.h"
 #include "usbhost_host_stream.h"
+#include "shared/usbhost_audio_interval.h"
 #include "usbhost_host.h"
 
 
@@ -169,6 +170,22 @@ static usbhost_xfer_claim_t *usbhost_xfer_find_claim(usb_device_handle_t dev, ui
 
 static void usbhost_xfer_drop_pipes(usb_device_handle_t dev, int itf, bool device_gone);
 
+// A full-speed audio stream is polled every frame, whatever its endpoint's
+// bInterval says (usbhost_audio_interval.h). IDF reads the interval from its
+// copy of the configuration descriptor when the interface is claimed, so this
+// corrects the copy first. The copy is heap memory the library keeps for the
+// device; the const is the library's promise not to change it, ours to keep
+// for everything but this one byte.
+static void usbhost_xfer_audio_fs_interval(usb_device_handle_t dev, uint8_t itf, uint8_t alt) {
+    usb_device_info_t info;
+    const usb_config_desc_t *cfg;
+    if (usb_host_device_info(dev, &info) != ESP_OK || info.speed != USB_SPEED_FULL
+        || usb_host_get_active_config_descriptor(dev, &cfg) != ESP_OK) {
+        return;
+    }
+    usbhost_audio_fs_interval_fix((uint8_t *)cfg, cfg->wTotalLength, itf, alt);
+}
+
 // 1 for a fresh claim (the host's pipes start at DATA0), 0 when the
 // interface was already claimed at that alternate, or a negative errno.
 int usbhost_xfer_claim(uint32_t dev_id, uint8_t itf, uint8_t alt) {
@@ -200,6 +217,7 @@ int usbhost_xfer_claim(uint32_t dev_id, uint8_t itf, uint8_t alt) {
             goto out;
         }
     }
+    usbhost_xfer_audio_fs_interval(dev, itf, alt);
     esp_err_t err = usb_host_interface_claim(usbhost_host_client_get(), dev, itf, alt);
     if (err != ESP_OK) {
         r = err == ESP_ERR_NOT_FOUND ? -MP_ENOENT : -MP_EBUSY;

modules/usbhost/tests/test_audio_interval.c

@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: MIT
+//
+// Host-side tests for the full-speed audio endpoint interval fix
+// (workspace#16): a webcam's microphone says bInterval 4 for both speeds,
+// which is 1 ms at high speed and 8 ms at full speed, and the P4's host polled
+// it at the 8 ms. Only the audio streaming interface being claimed, at the
+// alternate setting being claimed, has its isochronous endpoint changed.
+#include "shared/usbhost_audio_interval.h"
+
+#include <stdio.h>
+#include <string.h>
+
+static int failures = 0;
+
+#define CHECK(cond) do { \
+    if (!(cond)) { \
+        printf("FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); \
+        failures++; \
+    } \
+} while (0)
+
+static size_t put_itf(uint8_t *p, uint8_t n, uint8_t alt, uint8_t cls, uint8_t sub) {
+    uint8_t d[9] = { 9, 4, n, alt, 1, cls, sub, 0, 0 };
+    memcpy(p, d, 9);
+    return 9;
+}
+
+static size_t put_ep(uint8_t *p, uint8_t addr, uint8_t attr, uint16_t wmax, uint8_t interval) {
+    uint8_t d[7] = { 7, 5, addr, attr, (uint8_t)(wmax & 0xff), (uint8_t)(wmax >> 8), interval };
+    memcpy(p, d, 7);
+    return 7;
+}
+
+// Interface 1 is video (alt 1 an isochronous endpoint at bInterval 4, a
+// legitimate slow one), interface 3 audio: alt 0 nothing, alt 1 the mic's
+// isochronous endpoint at bInterval 4, alt 2 an interrupt endpoint at 4, and
+// a class-specific descriptor (type 0x25) between them.
+static size_t build(uint8_t *c, size_t *ep_video, size_t *ep_a1, size_t *ep_a2) {
+    size_t n = 0;
+    uint8_t cfg[9] = { 9, 2, 0, 0, 4, 1, 0, 0x80, 50 };
+    memcpy(c, cfg, 9);
+    n = 9;
+    n += put_itf(c + n, 1, 1, 14, 2);
+    *ep_video = n;
+    n += put_ep(c + n, 0x81, 0x05, 944, 4);
+    n += put_itf(c + n, 3, 0, 1, 2);
+    n += put_itf(c + n, 3, 1, 1, 2);
+    uint8_t cs[7] = { 7, 0x25, 1, 0, 0, 0, 0 };
+    memcpy(c + n, cs, 7);
+    n += 7;
+    *ep_a1 = n;
+    n += put_ep(c + n, 0x83, 0x05, 68, 4);
+    n += put_itf(c + n, 3, 2, 1, 2);
+    *ep_a2 = n;
+    n += put_ep(c + n, 0x83, 0x03, 68, 4);
+    c[2] = (uint8_t)n;
+    return n;
+}
+
+int main(void) {
+    uint8_t c[200];
+    size_t v, a1, a2;
+    size_t len = build(c, &v, &a1, &a2);
+
+    // The mic's alt 1: its isochronous endpoint goes to every frame.
+    CHECK(usbhost_audio_fs_interval_fix(c, len, 3, 1) == 1);
+    CHECK(c[a1 + 6] == 1);
+    // Nothing else moved: the video endpoint, and the other alternate's.
+    CHECK(c[v + 6] == 4);
+    CHECK(c[a2 + 6] == 4);
+
+    // Claiming the video interface changes nothing: it isn't audio.
+    len = build(c, &v, &a1, &a2);
+    CHECK(usbhost_audio_fs_interval_fix(c, len, 1, 1) == 0);
+    CHECK(c[v + 6] == 4 && c[a1 + 6] == 4);
+
+    // An audio alternate with only an interrupt endpoint: not isochronous.
+    CHECK(usbhost_audio_fs_interval_fix(c, len, 3, 2) == 0);
+    CHECK(c[a2 + 6] == 4);
+
+    // Alt 0 has no endpoints; an interface that isn't there changes nothing.
+    CHECK(usbhost_audio_fs_interval_fix(c, len, 3, 0) == 0);
+    CHECK(usbhost_audio_fs_interval_fix(c, len, 9, 1) == 0);
+    CHECK(c[a1 + 6] == 4);
+
+    // Already 1: left alone and not counted.
+    c[a1 + 6] = 1;
+    CHECK(usbhost_audio_fs_interval_fix(c, len, 3, 1) == 0);
+    CHECK(c[a1 + 6] == 1);
+
+    // A truncated descriptor stops at the end instead of reading past it.
+    len = build(c, &v, &a1, &a2);
+    CHECK(usbhost_audio_fs_interval_fix(c, a1 + 3, 3, 1) == 0);
+    CHECK(c[a1 + 6] == 4);
+    // A zero-length descriptor can't loop forever.
+    c[a1] = 0;
+    CHECK(usbhost_audio_fs_interval_fix(c, len, 3, 1) == 0);
+
+    if (failures) {
+        printf("%d failures\n", failures);
+        return 1;
+    }
+    printf("ok\n");
+    return 0;
+}