Fix non-looping audio playback across espressif, nordic, raspberrypi, zephyr-cp#11085
Conversation
audiobusio.I2SOut dropped the final buffer that audiosample_get_buffer returns together with GET_BUFFER_DONE, breaking before it was copied. A single-buffer RawSample played with loop=False returns its entire buffer with GET_BUFFER_DONE on the first call, so nothing was ever output. - Add a last_buffer flag so the final buffer is copied out before stopping, instead of breaking before the copy. - Silence any unfilled remainder of the DMA buffer so a sample that ends mid-buffer (or a zero-length sample) does not play stale data. - Fix the preload cap, which compared a byte count against a frame count and left the DMA only ~1/4 primed, splitting playback into a short tone, a gap, then the rest. Addresses adafruit#10539. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
audiobusio.I2SOut dropped the final buffer that audiosample_get_buffer returns together with GET_BUFFER_DONE, breaking before it was copied. A single-buffer RawSample played with loop=False returns its entire buffer with GET_BUFFER_DONE on the first call, so nothing was ever output. Add a last_buffer flag so the final buffer is copied out before stopping, instead of breaking before the copy. The existing hold_value tail fill and external stopping/paused handling are unchanged. Addresses adafruit#10539. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A single-buffer sample plays entirely in hardware via DMA chaining with no completion interrupt (the interrupt is only enabled for the multi-buffer path). With loop=False, channel[0] chains to itself and stops after one pass, but nothing ever ran audio_dma_stop, so playing_in_progress stayed true forever and I2SOut.playing (and AudioOut/PWMAudioOut) never became false. Detect this in audio_dma_get_playing: when a single-buffer, non-looping, non-paused sample's DMA channel has drained, stop and report not playing. Addresses adafruit#10539. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fill_buffer set stopping and returned on GET_BUFFER_DONE with loop=False before copying the returned data. A single-buffer RawSample returns its entire buffer with GET_BUFFER_DONE on the first call, so the block was filled with silence and nothing was heard. Copy the returned buffer first, then handle GET_BUFFER_DONE (drain and stop). Add a native_sim regression test that plays a single-buffer, non-looping sample and asserts the sine wave reaches the I2S output. Addresses adafruit#10539. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| // Sample ended mid-buffer (or was empty/errored): silence the rest so the | ||
| // DMA doesn't play stale data. | ||
| if (output_buffer_size > 0) { | ||
| memset(output_buffer, 0, output_buffer_size); |
There was a problem hiding this comment.
Do we want to duplicate the last valid value? Won't this click instead?
There was a problem hiding this comment.
I discussed this with Claude during the session. In nordic there is a hold value.
> ... question is whether the espressif port should use the hold_value fill rather than zeros. Is that better?
Short answer: no — for espressif, zeros are the better fit. The hold_value trick is a nordic-specific necessity, not a general improvement. Here's the reasoning:
Why nordic needs hold_value: nordic's pause (common_hal_audiobusio_i2sout_pause, line 297) just sets a flag — it does not stop the I2S clock. So the DMA keeps running during a pause, and the fill loop has to keep emitting something that freezes the waveform. Replicating the last sample holds the output steady so a pause→resume is seamless (no gap/click mid-waveform). Nordic then reuses that same fill for end-of-sample as a side effect.
Why espressif doesn't: espressif pauses by disabling the channel in hardware (port_i2s_pause → i2s_channel_disable) — the clock stops and the line holds on its own. So espressif's tail fill is only ever reached at end of sample, where the goal is genuine silence. There, zero is the right target:
- Zero returns the output to the true quiescent level. hold_value parks a DC offset on the line until the stream actually stops — no tone, but DC through an AC-coupled output/speaker decays with a thump.
- hold_value doesn't remove the end-of-sample click, it just relocates it: instead of stepping last-sample→0 at the sample boundary, you step last-sample→(whatever-on-stop) when the channel finally disables. Same magnitude, arguably worse because of the DC in between.
- For the zero-length / empty case, there is no "last sample," so a hold approach would emit a stale hold_value from a previous play (nordic guards this with if (buffer != buffer_start), leaving it stale). Zero-fill is unconditionally correct.
The one thing neither does: a true click-free end needs a short fade-out ramp. That's a real enhancement, but it's separate, applies to all ports equally, and I'd keep it out of this bugfix branch.
So I'd leave espressif on zeros. If you ever want click-free endings, the right move is a shared fade-out, not adopting nordic's DC-hold. ...
2 participants
Tagging @deshipu
🤖 Generated with Claude Code
Playing a
RawSamplewithloop=Falsemisbehaved across several ports. The common cause:audiosample_get_bufferreturns the final chunk of data together withGET_BUFFER_DONE, and a single-bufferRawSample(the default) returns its entire buffer withGET_BUFFER_DONEon the very first call. Ports that acted onDONEbefore copying that buffer dropped the audio entirely.Per-port changes
common-hal/audiobusio/__init__.clast_bufferflag so the finalGET_BUFFER_DONEbuffer is copied out instead of dropped (single-bufferloop=Falsewas completely silent).common-hal/audiobusio/I2SOut.c— samelast_bufferdeferral so the final buffer is played; existinghold_valuetail fill unchanged.audio_dma.c— a single-buffer, non-looping sample plays entirely in hardware via DMA chaining with no completion interrupt, soplayingnever cleared.audio_dma_get_playingnow detects the drained channel and stops. (Different symptom: the audio played once, butplayingstayedTrueforever.)common-hal/audiobusio/I2SOut.c— copy the returned buffer before handlingGET_BUFFER_DONE; adds anative_simregression test (tests/test_audiobusio.py) asserting the sine wave reaches the I2S output.Testing
playingclears).Out of scope / follow-ups
I2SOut.playingnever clears on its own after a non-looping sample finishes (no drain-aware completion detection). Tracked separately in zephyr-cp: audiobusio.I2SOut.playing never becomes False after a non-looping sample finishes #11084.single_buffer=False)RawSampleplayed withloop=Falsehangs on raspberrypi (plays briefly, thenplayingnever clears) — this combination has no natural end and needs aRawSample-layer decision. To be reproduced on other ports and filed separately. Tracked separately in Double-buffer RawSample (single_buffer=False) played with loop=False hangs; playing never clears #11086.