From e2968ac864eec52bd6b424cc503d75b988899fb3 Mon Sep 17 00:00:00 2001 From: philmoz Date: Wed, 16 Sep 2026 15:12:41 +1000 Subject: [PATCH] fix(firmware): audio logic does not use all of the available buffers (#7689) Co-authored-by: Peter Feerick <5500713+pfeerick@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 --- radio/src/audio.h | 60 +++++++++++++++++++++++++++++++++-------------- radio/src/cli.cpp | 16 +++++++++---- 2 files changed, 54 insertions(+), 22 deletions(-) diff --git a/radio/src/audio.h b/radio/src/audio.h index 234e8c1f0..0fc1eb030 100644 --- a/radio/src/audio.h +++ b/radio/src/audio.h @@ -98,6 +98,11 @@ constexpr uint8_t AUDIO_FILENAME_MAXLEN = (AUDIO_LUA_FILENAME_MAXLEN > AUDIO_MOD #define AUDIO_BUFFER_COUNT (3) #endif +static_assert((AUDIO_QUEUE_LENGTH & (AUDIO_QUEUE_LENGTH - 1)) == 0, + "AUDIO_QUEUE_LENGTH must be a power of 2"); +static_assert(2 * AUDIO_BUFFER_COUNT <= 255, + "AUDIO_BUFFER_COUNT free-running counter must fit in uint8_t"); + #define BEEP_MIN_FREQ (150) #define BEEP_MAX_FREQ (15000) #define BEEP_DEFAULT_FREQ (2250) @@ -302,17 +307,28 @@ class AudioBufferFifo { #endif private: + // free-running counters in [0, 2*AUDIO_BUFFER_COUNT); each written by only one side volatile uint8_t readIdx; volatile uint8_t writeIdx; - inline uint8_t nextBufferIdx(uint8_t idx) const + inline uint8_t nextIndex(uint8_t idx) const { - return (idx >= AUDIO_BUFFER_COUNT - 1 ? 0 : idx + 1); + return (idx >= 2 * AUDIO_BUFFER_COUNT - 1 ? 0 : idx + 1); } - bool full() const { return readIdx == nextBufferIdx(writeIdx); } + inline uint8_t slot(uint8_t idx) const + { + return idx >= AUDIO_BUFFER_COUNT ? idx - AUDIO_BUFFER_COUNT : idx; + } + + uint8_t used() const + { + uint8_t w = writeIdx, r = readIdx; + return w >= r ? w - r : w + 2 * AUDIO_BUFFER_COUNT - r; + } + + bool full() const { return used() == AUDIO_BUFFER_COUNT; } bool empty() const { return readIdx == writeIdx; } - uint8_t used() const { return (writeIdx - readIdx) % AUDIO_BUFFER_COUNT; } public: AudioBufferFifo() : readIdx(0), writeIdx(0) @@ -324,22 +340,20 @@ class AudioBufferFifo { // with audioPushBuffer() AudioBuffer *getEmptyBuffer() const { - return full() ? nullptr : &audioBuffers[writeIdx]; + return full() ? nullptr : &audioBuffers[slot(writeIdx)]; } // puts filled buffer into FIFO - void audioPushBuffer() { writeIdx = nextBufferIdx(writeIdx); } + void audioPushBuffer() { writeIdx = nextIndex(writeIdx); } // frees the last played buffer - void freeNextFilledBuffer() { readIdx = nextBufferIdx(readIdx); } + void freeNextFilledBuffer() { readIdx = nextIndex(readIdx); } // returns a pointer to the audio buffer to be played const AudioBuffer *getNextFilledBuffer() { - return empty() ? nullptr : &audioBuffers[readIdx]; + return empty() ? nullptr : &audioBuffers[slot(readIdx)]; } - - bool filledAtleast(int noBuffers) const { return used() >= noBuffers; } }; class AudioFragmentFifo @@ -348,13 +362,25 @@ class AudioFragmentFifo friend void printAudioVars(); #endif private: + // free-running counters in [0, 2*AUDIO_QUEUE_LENGTH); each written by only one side volatile uint8_t ridx; volatile uint8_t widx; AudioFragment fragments[AUDIO_QUEUE_LENGTH]; uint8_t nextIdx(uint8_t idx) const { - return (idx + 1) & (AUDIO_QUEUE_LENGTH - 1); + return (idx + 1) & (2 * AUDIO_QUEUE_LENGTH - 1); + } + + uint8_t slot(uint8_t idx) const + { + return idx & (AUDIO_QUEUE_LENGTH - 1); + } + + uint8_t used() const + { + uint8_t w = widx, r = ridx; + return w >= r ? w - r : w + 2 * AUDIO_QUEUE_LENGTH - r; } public: @@ -364,7 +390,7 @@ class AudioFragmentFifo { uint8_t i = ridx; while (i != widx) { - AudioFragment & fragment = fragments[i]; + AudioFragment & fragment = fragments[slot(i)]; if (fragment.id == id) return true; i = nextIdx(i); } @@ -375,7 +401,7 @@ class AudioFragmentFifo { uint8_t i = ridx; while (i != widx) { - AudioFragment & fragment = fragments[i]; + AudioFragment & fragment = fragments[slot(i)]; if (fragment.id == id) fragment.clear(); i = nextIdx(i); } @@ -389,7 +415,7 @@ class AudioFragmentFifo bool full() const { - return ridx == nextIdx(widx); + return used() == AUDIO_QUEUE_LENGTH; } void clear() @@ -400,8 +426,8 @@ class AudioFragmentFifo const AudioFragment * get() { if (!empty()) { - const AudioFragment * result = &fragments[ridx]; - if (!fragments[ridx].repeat--) { + const AudioFragment * result = &fragments[slot(ridx)]; + if (!fragments[slot(ridx)].repeat--) { // repeat is done, move to the next fragment ridx = nextIdx(ridx); } @@ -414,7 +440,7 @@ class AudioFragmentFifo { if (!full()) { // TRACE("fragment %d at %d", fragment.type, widx); - fragments[widx] = fragment; + fragments[slot(widx)] = fragment; widx = nextIdx(widx); } } diff --git a/radio/src/cli.cpp b/radio/src/cli.cpp index 372348ae8..65c4b48ef 100644 --- a/radio/src/cli.cpp +++ b/radio/src/cli.cpp @@ -1432,11 +1432,17 @@ void printAudioVars() } } - cliSerialPrint("FragmentFifo: ridx: %d, widx: %d", - audioQueue.fragmentsFifo.ridx, audioQueue.fragmentsFifo.widx); - cliSerialPrint("audioQueue: readIdx: %d, writeIdx: %d, full: %d", - audioQueue.buffersFifo.readIdx, audioQueue.buffersFifo.writeIdx, - audioQueue.buffersFifo.bufferFull); + cliSerialPrint("FragmentFifo: ridx: %d (slot %d), widx: %d (slot %d)", + audioQueue.fragmentsFifo.ridx, + audioQueue.fragmentsFifo.slot(audioQueue.fragmentsFifo.ridx), + audioQueue.fragmentsFifo.widx, + audioQueue.fragmentsFifo.slot(audioQueue.fragmentsFifo.widx)); + cliSerialPrint("audioQueue: readIdx: %d (slot %d), writeIdx: %d (slot %d), full: %d", + audioQueue.buffersFifo.readIdx, + audioQueue.buffersFifo.slot(audioQueue.buffersFifo.readIdx), + audioQueue.buffersFifo.writeIdx, + audioQueue.buffersFifo.slot(audioQueue.buffersFifo.writeIdx), + audioQueue.buffersFifo.full()); cliSerialPrint("normalContext: %u", (uint32_t)audioQueue.normalContext.fragment.type);