diff --git a/.github/workflows/usbhost.yml b/.github/workflows/usbhost.yml index 94862d59..e4c82274 100644 --- a/.github/workflows/usbhost.yml +++ b/.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 diff --git a/modules/usbhost/src/shared/usbhost_audio_interval.h b/modules/usbhost/src/shared/usbhost_audio_interval.h new file mode 100644 index 00000000..1a58c284 --- /dev/null +++ b/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 +#include +#include + +#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 diff --git a/modules/usbhost/src/usbhost_host_stream.c b/modules/usbhost/src/usbhost_host_stream.c index 90273fcc..ff344ad0 100644 --- a/modules/usbhost/src/usbhost_host_stream.c +++ b/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); diff --git a/modules/usbhost/src/usbhost_host_xfer.c b/modules/usbhost/src/usbhost_host_xfer.c index e10d6f5e..d6190100 100644 --- a/modules/usbhost/src/usbhost_host_xfer.c +++ b/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; diff --git a/modules/usbhost/tests/test_audio_interval.c b/modules/usbhost/tests/test_audio_interval.c new file mode 100644 index 00000000..2b716af0 --- /dev/null +++ b/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 +#include + +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; +}