Skip to content

audio: flow control never starts if the host asks for the sample rate before selecting alt 1 - #3820

Closed
mikeysklar wants to merge 1 commit into
hathach:masterfrom
mikeysklar:audio-recompute-tx-packet-size
Closed

audio: flow control never starts if the host asks for the sample rate before selecting alt 1#3820
mikeysklar wants to merge 1 commit into
hathach:masterfrom
mikeysklar:audio-recompute-tx-packet-size

Conversation

@mikeysklar

@mikeysklar mikeysklar commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

I jumped in with @FoamyGuy and @relic-se on the usb_audio module for
CircuitPython, adafruit/circuitpython#11102, and trying to get UAC2 microphone
support going on the ESP32-S2 and S3. That PR already works on RP2040, RP2350 and
nRF52840, so I went looking for what was different on ESP32 and found this along
the way. It is not ESP32 specific.

packet_sz_tx[] is never computed, so EP IN flow control silently never engages.

audiod_calc_tx_packet_sz() asserts packet_sz_tx_max <= audio->ep_in_sz, but it
runs from the clock source SAM_FREQ CUR handlers, audio_device.c:1288 and
:1697. Hosts send that before SET_INTERFACE alt=1, which is where ep_in_sz
gets set. So it is still 0, the assert fails, and nothing recomputes once the
endpoint opens.

audiod_tx_packet_size() then checks nominal_size[1], finds 0, skips flow
control entirely and returns tu_min16(data_count, max_depth).

Read off the device at 48 kHz stereo 16 bit:

before after
sample_rate_tx 48000 48000
ep_in_sz at calc time 0 196
packet_sz_tx[0] 0 188
packet_sz_tx[1] 0 192

With the fix the endpoint sends the expected mix. A bus capture of a running
stream shows 3671 packets of 192 bytes and 335 of 188.

The fix recomputes once ep_in_sz is known.

Tested on ESP32-S3 and ESP32-S2 against the TinyUSB commit CircuitPython currently pins, 5453ed09f.

audiod_calc_tx_packet_sz() asserts packet_sz_tx_max <= audio->ep_in_sz, but it
runs from the clock source SAM_FREQ CUR handlers. Hosts send that request before
SET_INTERFACE alt=1, which is where ep_in_sz is assigned, so ep_in_sz is still 0,
the assert fails, and packet_sz_tx[] stays zeroed. Nothing recomputes it once the
endpoint is open.

audiod_tx_packet_size() then sees nominal_size[1] == 0, skips EP IN flow control
entirely and returns tu_min16(data_count, max_depth).

Recompute once the endpoint size is known.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an early audio TX packet-size recalculation after opening an IN endpoint.

Changes:

  • Recomputes flow-control packet sizes once endpoint and format data are available.
  • Guards recalculation on a known sample rate.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +1185 to +1186
if (audio->sample_rate_tx) {
audiod_calc_tx_packet_sz(audio);
@mikeysklar

Copy link
Copy Markdown
Contributor Author

Good catch from the review, this is a duplicate and the behavior I reported does
not happen.

audiod_set_interface() already calls audiod_calc_tx_packet_sz(audio) at line
1261, after ep_in_sz and the format parameters are populated, so the sizes do
get computed correctly.

My counters sampled inside the SAM_FREQ handler. ep_in_sz really is 0 there
and the calc really does bail, but I never sampled after line 1261, so I read a
transient as the final state. The evidence was in my own bus capture and I walked
past it: the packets were already 192 and 188 bytes, which is the correct
flow controlled mix.

Closing. Sorry for the noise, and thanks for the careful read.

@mikeysklar mikeysklar closed this Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants