blue-debug opened a new pull request, #19686: URL: https://github.com/apache/nuttx/pull/19686
## Summary This PR contains two commits: a fix for a playback regression, and the documentation for the audio upper half interface. ### 1. Fix: buffer allocation fails when the lower half is silent about its buffer preference Playback is silent on a Raspberry Pi Pico 2 with the `raspberrypi-pico-2:spisd` configuration. Reported and bisected to two commits from #18348 on the dev mailing list: https://www.mail-archive.com/[email protected]/msg14859.html This PR fixes the `nuttx` side; the `nuttx-apps` side is fixed separately. `audio_allocbuffer()` began with: ```c if (upper->periods >= upper->nbuffers) { return 0; } ``` `upper->nbuffers` is only ever assigned in the `AUDIOIOC_GETBUFFERINFO` handler, and only when the lower half answers successfully. There is no default, so a lower half that does not implement that ioctl leaves it at zero. The check is also in the wrong place. `upper->periods` is only incremented on the shared ring path (`bufdesc->u.pbuffer == NULL`), so for private-buffer callers it stays zero for the lifetime of the device and the comparison degenerates into `upper->nbuffers == 0`. It was never counting anything for them. The result is that the first `AUDIOIOC_ALLOCBUFFER` returns 0 without ever calling `apb_alloc()`, and nxplayer aborts with "Could not allocate buffer 0". Move the guard inside the shared ring branch, so private buffers - which never enter `upper->apbs[]` and are unrelated to the ring depth - stay allocatable regardless of `upper->nbuffers`. No default value is introduced, so nothing can disagree with the fallback applications already apply when the ioctl fails. The zero return value is kept rather than turned into an error code: a second application attaching to the same device relies on it to skip allocation and go straight to `AUDIOIOC_ENQUEUEBUFFER`. ### 2. Documentation: the audio upper half interface Addresses #18354. The audio subsystem page listed the source files and the configuration options, but nothing about the interface the upper half presents to applications. #18348 added a device state machine, a second buffer allocation mode, poll and mmap support and several new ioctls, none of which were described anywhere, so the only way to learn the expected call sequence was to read `audio/audio.c`. Added to `Documentation/components/audio/index.rst`: - the device state machine, and the fact that `AUDIOIOC_START` is rejected until `AUDIOIOC_CONFIGURE` has moved the device out of `AUDIO_STATE_OPEN`; - the normal open/configure/allocate/enqueue/start sequence; - the two `AUDIOIOC_ALLOCBUFFER` modes selected by `u.pbuffer`, who owns the buffers in each, and that a shared ring request may return zero when the ring is already populated; - that `AUDIOIOC_GETBUFFERINFO` also establishes the shared ring depth, so a lower half which does not implement it disables that mode; - the poll event semantics, and how `mmap()` selects between a ring buffer and the device status by requested length; - all ioctls handled by the upper half, grouped by purpose; - how per-open state is aggregated into the device state when several applications share one device. Documenting the two allocation modes also makes the contract restored by the first commit explicit. depends-on: apache/nuttx-apps/pull/<APPS_PR> ## Impact - Restores the pre-#18348 behaviour for private-buffer callers (`nxplayer`, `nxrecorder`, `nxlooper`) on any board whose lower half does not implement `AUDIOIOC_GETBUFFERINFO`. In-tree that is anything reaching the hardware through `audio_i2s.c`, since `audio_i2s_ioctl()` forwards blindly to `I2S_IOCTL()` and an I2S driver has no obligation to recognise audio-layer commands. - Shared ring allocation is unchanged: same condition, same point in the flow, `return 0` keeps its meaning. - No API, ABI or configuration change. - Documentation only adds text; no existing section is modified. Pre-existing limitation, not addressed here: when private and shared enqueue paths are used on one device concurrently, both bump `upper->status->head` while only the shared path uses it as the ring read index, so the index can skew. This is reachable today regardless of where the guard sits, so this change does not introduce it. ## Testing Runtime verification was kindly done by the reporter on `raspberrypi-pico-2:spisd`, together with the companion nuttx-apps change: https://www.mail-archive.com/[email protected]/msg14859.html Before: ``` nsh> nxplayer nxplayer> play /mnt/sd0/test.wav nxplayer_playthread: ERROR: Could not allocate buffer 0 ``` After: allocation succeeds and playback runs. Local checks on Ubuntu 22.04.5 LTS x86_64: - `tools/checkpatch.sh` - clean on both commits - `tools/nxstyle audio/audio.c` - no warnings -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
