diff --git a/components/esp-box/src/audio.cpp b/components/esp-box/src/audio.cpp index 48d154e3bc..8843ac4ce3 100644 --- a/components/esp-box/src/audio.cpp +++ b/components/esp-box/src/audio.cpp @@ -114,9 +114,15 @@ bool EspBox::initialize_i2s(uint32_t default_audio_rate) { ESP_ERROR_CHECK(i2s_channel_init_std_mode(audio_tx_handle, &audio_std_cfg)); auto buffer_size = calc_audio_buffer_size(default_audio_rate); + // the block (and so the stream's trigger level) must be a whole number of + // frames: the sample-rate formula can land between frames, and a block that + // split one would desynchronize the framing of everything received after it + buffer_size -= buffer_size % (NUM_CHANNELS * NUM_BYTES_PER_CHANNEL); audio_tx_buffer.resize(buffer_size); - audio_tx_stream = xStreamBufferCreate(buffer_size * 4, 0); + // Trigger level = one block: the consumer waits for a whole block instead of + // padding a partial one with silence (see audio_task_callback). + audio_tx_stream = xStreamBufferCreate(buffer_size * 4, buffer_size); xStreamBufferReset(audio_tx_stream); @@ -157,19 +163,43 @@ bool EspBox::initialize_sound(uint32_t default_audio_rate, void EspBox::enable_sound(bool enable) { gpio_set_level(sound_power_pin, enable); } +// How long the audio task waits for a whole block before writing a partial one +// (or silence). Longer than one block at every rate the board supports (a block +// is 1/60 s at the default rate, 30 ms at 26.6 kHz), so partial writes only +// happen when the producer is idle or genuinely behind, never because its next +// frame simply had not arrived yet; short enough that idle->silence latency is +// not noticeable. +static constexpr uint32_t AUDIO_TX_RECEIVE_TIMEOUT_MS = 40; + bool EspBox::audio_task_callback(std::mutex &m, std::condition_variable &cv, bool &task_notified) { - // Queue the next I2S out frame to write - size_t available = xStreamBufferBytesAvailable(audio_tx_stream); + // Queue the next I2S out block. Wait (bounded) for a whole block rather than + // taking a partial one and padding it with silence: producers such as the + // emulators push one video frame of audio at a time with frame-to-frame + // jitter, and the stream (4 blocks) exists to absorb exactly that. Taking a + // partial block injected up to a block of silence every time the stream + // momentarily held less than one -- audible as stutter whenever the producer's + // cadence was uneven. If the producer is idle (menu, pause) the timeout keeps + // the DMA fed with silence; a partial block after the timeout is written as + // is (the I2S driver concatenates partial writes without gaps). size_t buffer_size = audio_tx_buffer.size(); - available = std::min(available, buffer_size); uint8_t *buffer = &audio_tx_buffer[0]; - memset(buffer, 0, buffer_size); - - if (available == 0) { + size_t got = xStreamBufferReceive(audio_tx_stream, buffer, buffer_size, + pdMS_TO_TICKS(AUDIO_TX_RECEIVE_TIMEOUT_MS)); + // the block is frame-aligned and producers queue whole frames, so this cannot + // happen; if it ever does, do not silently drop bytes (that would desynchronize + // the framing of everything after them) -- say so + constexpr size_t frame_bytes = NUM_CHANNELS * NUM_BYTES_PER_CHANNEL; + if (got % frame_bytes != 0) { + static uint32_t misaligned_count = 0; + if ((misaligned_count++ % 100) == 0) { + logger_.warn("audio stream holds a partial frame ({} bytes)", got); + } + } + if (got == 0) { + memset(buffer, 0, buffer_size); i2s_channel_write(audio_tx_handle, buffer, buffer_size, NULL, portMAX_DELAY); } else { - xStreamBufferReceive(audio_tx_stream, buffer, available, 0); - i2s_channel_write(audio_tx_handle, buffer, buffer_size, NULL, portMAX_DELAY); + i2s_channel_write(audio_tx_handle, buffer, got, NULL, portMAX_DELAY); } return false; // don't stop the task } diff --git a/components/esp32-p4-eth/src/audio.cpp b/components/esp32-p4-eth/src/audio.cpp index 1669e1258c..4a0097f289 100644 --- a/components/esp32-p4-eth/src/audio.cpp +++ b/components/esp32-p4-eth/src/audio.cpp @@ -112,8 +112,15 @@ bool Esp32P4Eth::initialize_audio(uint32_t sample_rate, const espp::Task::BaseCo // The audio task drains this stream buffer to I2S. Size it generously so a // whole UI sound clip fits and play_audio() can enqueue it without blocking. auto tx_buf_size = calc_audio_buffer_size(sample_rate); + // the block (and so the stream's trigger level) must be a whole number of + // frames: the sample-rate formula can land between frames, and a block that + // split one would desynchronize the framing of everything received after it + tx_buf_size -= tx_buf_size % (NUM_CHANNELS * NUM_BYTES_PER_CHANNEL); audio_tx_buffer.resize(tx_buf_size); - audio_tx_stream = xStreamBufferCreate(std::max(tx_buf_size * 4, 64 * 1024), 0); + // Trigger level = one block: the audio task waits for a whole block instead of + // padding a partial one with silence (see audio_task_callback). + audio_tx_stream = + xStreamBufferCreate(std::max(tx_buf_size * 4, 64 * 1024), tx_buf_size); if (audio_tx_stream == nullptr) { return fail_audio_init("Failed to allocate the audio TX stream buffer"); } @@ -294,29 +301,46 @@ void Esp32P4Eth::microphone_volume(float volume) { float Esp32P4Eth::microphone_volume() const { return mic_volume_; } +// How long the audio task waits for a whole block before writing a partial one +// (or silence). Longer than one block at every rate the board supports (a block +// is 1/60 s at the default rate), so partial writes only happen when the +// producer is idle or genuinely behind, never because its next frame simply had +// not arrived yet; short enough that idle->silence latency is not noticeable. +static constexpr uint32_t AUDIO_TX_RECEIVE_TIMEOUT_MS = 40; + bool Esp32P4Eth::audio_task_callback(std::mutex &m, std::condition_variable &cv, bool &task_notified) { - size_t available = xStreamBufferBytesAvailable(audio_tx_stream); size_t buffer_size = audio_tx_buffer.size(); - available = std::min(available, buffer_size); - // only ever hand whole 16-bit samples to I2S; a partial sample would shift - // the framing of everything after it - available &= ~static_cast(1); uint8_t *tx_buf = audio_tx_buffer.data(); - memset(tx_buf, 0, buffer_size); - // Use a finite write timeout (not portMAX_DELAY) so this task returns - // periodically and can observe a stop request; an infinite write would block - // Task::stop() from joining during teardown if the I2S sink ever stalls. - if (available == 0) { + // Wait (bounded) for a whole block rather than taking whatever is queued and + // zero-padding it to a block: producers push audio in bursts (one video frame + // at a time, with frame-to-frame jitter), and the stream buffer exists to + // absorb exactly that. Taking a partial block padded it with silence every + // time the stream momentarily held less than one block, which is audible as + // stutter whenever the producer's cadence is uneven. The DMA is still fed + // whole blocks at a constant cadence while audio is flowing (the stream's + // trigger level is one block); only after the timeout -- producer idle or + // slower than real time -- is a partial block written as is (the I2S driver + // concatenates partial writes without gaps), or silence when there is none. + // Finite timeouts (not portMAX_DELAY) so this task returns periodically and + // can observe a stop request during teardown. + size_t got = xStreamBufferReceive(audio_tx_stream, tx_buf, buffer_size, + pdMS_TO_TICKS(AUDIO_TX_RECEIVE_TIMEOUT_MS)); + // the block is frame-aligned and producers queue whole frames, so this cannot + // happen; if it ever does, do not silently drop bytes (that would desynchronize + // the framing of everything after them) -- say so + constexpr size_t frame_bytes = NUM_CHANNELS * NUM_BYTES_PER_CHANNEL; + if (got % frame_bytes != 0) { + static uint32_t misaligned_count = 0; + if ((misaligned_count++ % 100) == 0) { + logger_.warn("audio stream holds a partial frame ({} bytes)", got); + } + } + if (got == 0) { + memset(tx_buf, 0, buffer_size); i2s_channel_write(audio_tx_handle, tx_buf, buffer_size, NULL, pdMS_TO_TICKS(100)); } else { - xStreamBufferReceive(audio_tx_stream, tx_buf, available, 0); - // Always write a full, frame-aligned buffer (queued samples zero-padded to - // buffer_size) so the I2S DMA is fed at a constant cadence - matching the - // esp-box / t-deck / m5stack-tab5 playback path. Writing only `available` - // bytes makes the drain cadence variable and interleaves whole frames of - // silence into bursty streams, which sounds choppy/glitchy. - i2s_channel_write(audio_tx_handle, tx_buf, buffer_size, NULL, pdMS_TO_TICKS(100)); + i2s_channel_write(audio_tx_handle, tx_buf, got, NULL, pdMS_TO_TICKS(100)); } // honor a stop request per the Task contract: check/clear notified under m std::unique_lock lock(m); diff --git a/components/esp32-p4-function-ev-board/src/audio.cpp b/components/esp32-p4-function-ev-board/src/audio.cpp index 511082d4fa..7a6b3d4093 100644 --- a/components/esp32-p4-function-ev-board/src/audio.cpp +++ b/components/esp32-p4-function-ev-board/src/audio.cpp @@ -94,8 +94,15 @@ bool Esp32P4FunctionEvBoard::initialize_audio(uint32_t sample_rate, // The audio task drains this stream buffer to I2S. Size it generously so a // whole UI sound clip fits and play_audio() can enqueue it without blocking. auto tx_buf_size = calc_audio_buffer_size(sample_rate); + // the block (and so the stream's trigger level) must be a whole number of + // frames: the sample-rate formula can land between frames, and a block that + // split one would desynchronize the framing of everything received after it + tx_buf_size -= tx_buf_size % (NUM_CHANNELS * NUM_BYTES_PER_CHANNEL); audio_tx_buffer.resize(tx_buf_size); - audio_tx_stream = xStreamBufferCreate(std::max(tx_buf_size * 4, 64 * 1024), 0); + // Trigger level = one block: the audio task waits for a whole block instead of + // padding a partial one with silence (see audio_task_callback). + audio_tx_stream = + xStreamBufferCreate(std::max(tx_buf_size * 4, 64 * 1024), tx_buf_size); xStreamBufferReset(audio_tx_stream); using namespace std::placeholders; @@ -259,26 +266,44 @@ void Esp32P4FunctionEvBoard::microphone_volume(float volume) { float Esp32P4FunctionEvBoard::microphone_volume() const { return mic_volume_; } +// How long the audio task waits for a whole block before writing a partial one +// (or silence). Longer than one block at every rate the board supports (a block +// is 1/60 s at the default rate), so partial writes only happen when the +// producer is idle or genuinely behind, never because its next frame simply had +// not arrived yet; short enough that idle->silence latency is not noticeable. +static constexpr uint32_t AUDIO_TX_RECEIVE_TIMEOUT_MS = 40; + bool Esp32P4FunctionEvBoard::audio_task_callback(std::mutex &m, std::condition_variable &cv, bool &task_notified) { - size_t available = xStreamBufferBytesAvailable(audio_tx_stream); size_t buffer_size = audio_tx_buffer.size(); - available = std::min(available, buffer_size); - // only ever hand whole 16-bit samples to I2S; a partial sample would shift - // the framing of everything after it - available &= ~static_cast(1); uint8_t *tx_buf = audio_tx_buffer.data(); - memset(tx_buf, 0, buffer_size); - if (available == 0) { + // Wait (bounded) for a whole block rather than taking whatever is queued and + // zero-padding it to a block: producers push audio in bursts (one video frame + // at a time, with frame-to-frame jitter), and the stream buffer exists to + // absorb exactly that. Taking a partial block padded it with silence every + // time the stream momentarily held less than one block, which is audible as + // stutter whenever the producer's cadence is uneven. The DMA is still fed + // whole blocks at a constant cadence while audio is flowing (the stream's + // trigger level is one block); only after the timeout -- producer idle or + // slower than real time -- is a partial block written as is (the I2S driver + // concatenates partial writes without gaps), or silence when there is none. + size_t got = xStreamBufferReceive(audio_tx_stream, tx_buf, buffer_size, + pdMS_TO_TICKS(AUDIO_TX_RECEIVE_TIMEOUT_MS)); + // the block is frame-aligned and producers queue whole frames, so this cannot + // happen; if it ever does, do not silently drop bytes (that would desynchronize + // the framing of everything after them) -- say so + constexpr size_t frame_bytes = NUM_CHANNELS * NUM_BYTES_PER_CHANNEL; + if (got % frame_bytes != 0) { + static uint32_t misaligned_count = 0; + if ((misaligned_count++ % 100) == 0) { + logger_.warn("audio stream holds a partial frame ({} bytes)", got); + } + } + if (got == 0) { + memset(tx_buf, 0, buffer_size); i2s_channel_write(audio_tx_handle, tx_buf, buffer_size, NULL, portMAX_DELAY); } else { - xStreamBufferReceive(audio_tx_stream, tx_buf, available, 0); - // Always write a full, frame-aligned buffer (queued samples zero-padded to - // buffer_size) so the I2S DMA is fed at a constant cadence - matching the - // esp-box / t-deck / m5stack-tab5 playback path. Writing only `available` - // bytes makes the drain cadence variable and interleaves whole frames of - // silence into bursty streams, which sounds choppy/glitchy. - i2s_channel_write(audio_tx_handle, tx_buf, buffer_size, NULL, portMAX_DELAY); + i2s_channel_write(audio_tx_handle, tx_buf, got, NULL, portMAX_DELAY); } return false; } diff --git a/components/esp32-p4-module-dev-kit/src/audio.cpp b/components/esp32-p4-module-dev-kit/src/audio.cpp index c219f9beea..33f2da58df 100644 --- a/components/esp32-p4-module-dev-kit/src/audio.cpp +++ b/components/esp32-p4-module-dev-kit/src/audio.cpp @@ -188,8 +188,15 @@ bool Esp32P4ModuleDevKit::initialize_audio(uint32_t sample_rate, // how much it queued, so callers stream clips longer than the buffer in // chunks (see the example's playback loop). auto tx_buf_size = calc_audio_buffer_size(sample_rate, TX_NUM_CHANNELS); + // the block (and so the stream's trigger level) must be a whole number of + // frames: the sample-rate formula can land between frames, and a block that + // split one would desynchronize the framing of everything received after it + tx_buf_size -= tx_buf_size % (TX_NUM_CHANNELS * NUM_BYTES_PER_CHANNEL); audio_tx_buffer.resize(tx_buf_size); - audio_tx_stream = xStreamBufferCreate(std::max(tx_buf_size * 4, 4 * 1024), 0); + // Trigger level = one block: the audio task waits for a whole block instead of + // padding a partial one with silence (see audio_task_callback). + audio_tx_stream = + xStreamBufferCreate(std::max(tx_buf_size * 4, 4 * 1024), tx_buf_size); if (audio_tx_stream == nullptr) { return fail_audio_init("Failed to allocate the audio TX stream buffer"); } @@ -416,26 +423,48 @@ void Esp32P4ModuleDevKit::microphone_volume(float volume) { float Esp32P4ModuleDevKit::microphone_volume() const { return mic_volume_; } +// How long the audio task waits for a whole block before writing a partial one +// (or silence). Longer than one block at every rate the board supports (a block +// is 1/60 s at the default rate), so partial writes only happen when the +// producer is idle or genuinely behind, never because its next frame simply had +// not arrived yet; short enough that idle->silence latency is not noticeable. +static constexpr uint32_t AUDIO_TX_RECEIVE_TIMEOUT_MS = 40; + bool Esp32P4ModuleDevKit::audio_task_callback(std::mutex &m, std::condition_variable &cv, bool &task_notified) { - (void)cv; // unused: this task paces itself on the finite-timeout I2S write below - size_t available = xStreamBufferBytesAvailable(audio_tx_stream); + (void)cv; // unused: this task paces itself on the finite-timeout receive/write below size_t buffer_size = audio_tx_buffer.size(); - available = std::min(available, buffer_size); - // only ever hand whole 16-bit samples to I2S; a partial sample would shift - // the framing of everything after it - available &= ~static_cast(1); uint8_t *tx_buf = audio_tx_buffer.data(); - memset(tx_buf, 0, buffer_size); - if (available > 0) { - xStreamBufferReceive(audio_tx_stream, tx_buf, available, 0); - } - // Always write a full, frame-aligned buffer (queued samples zero-padded to - // buffer_size) so the I2S DMA is fed at a constant cadence - matching the - // esp-box / t-deck / m5stack-tab5 playback path. Writing only `available` - // bytes makes the drain cadence variable and interleaves whole frames of - // silence into bursty streams, which sounds choppy/glitchy. - // + // Wait (bounded) for a whole block rather than taking whatever is queued and + // zero-padding it to a block: producers push audio in bursts (one video frame + // at a time, with frame-to-frame jitter), and the stream buffer exists to + // absorb exactly that. Taking a partial block padded it with silence every + // time the stream momentarily held less than one block, which is audible as + // stutter whenever the producer's cadence is uneven. The DMA is still fed + // whole blocks at a constant cadence while audio is flowing (the stream's + // trigger level is one block); only after the timeout -- producer idle or + // slower than real time -- is a partial block written as is (the I2S driver + // concatenates partial writes without gaps), or silence when there is none. + // The wait happens before taking audio_tx_mutex_ so a sample-rate change is + // never held off by it. + size_t got = xStreamBufferReceive(audio_tx_stream, tx_buf, buffer_size, + pdMS_TO_TICKS(AUDIO_TX_RECEIVE_TIMEOUT_MS)); + // the block is frame-aligned and producers queue whole frames, so this cannot + // happen; if it ever does, do not silently drop bytes (that would desynchronize + // the framing of everything after them) -- say so + constexpr size_t frame_bytes = TX_NUM_CHANNELS * NUM_BYTES_PER_CHANNEL; + if (got % frame_bytes != 0) { + static uint32_t misaligned_count = 0; + if ((misaligned_count++ % 100) == 0) { + logger_.warn("audio stream holds a partial frame ({} bytes)", got); + } + } + size_t to_write = buffer_size; + if (got == 0) { + memset(tx_buf, 0, buffer_size); + } else { + to_write = got; + } // Use a finite write timeout (not portMAX_DELAY) so this task returns // periodically and can observe a stop request; an infinite write would block // Task::stop() from joining during teardown if the I2S sink ever stalls. @@ -445,15 +474,14 @@ bool Esp32P4ModuleDevKit::audio_task_callback(std::mutex &m, std::condition_vari // Serialize with set_audio_sample_rate(), which may disable/reconfigure the // channel; writing to a disabled channel would race the reconfig. std::lock_guard lk(audio_tx_mutex_); - err = - i2s_channel_write(audio_tx_handle, tx_buf, buffer_size, &bytes_written, pdMS_TO_TICKS(100)); + err = i2s_channel_write(audio_tx_handle, tx_buf, to_write, &bytes_written, pdMS_TO_TICKS(100)); } - if (err != ESP_OK || bytes_written != buffer_size) { + if (err != ESP_OK || bytes_written != to_write) { // Rate-limit: a stalled I2S sink would otherwise emit this every ~100 ms. static uint32_t write_warn_count = 0; if ((write_warn_count++ % 100) == 0) { logger_.warn("i2s_channel_write: {} ({} of {} bytes written, occurrence {})", - esp_err_to_name(err), bytes_written, buffer_size, write_warn_count); + esp_err_to_name(err), bytes_written, to_write, write_warn_count); } } // honor a stop request per the Task contract: check/clear notified under m diff --git a/components/esp32-p4-nano/src/audio.cpp b/components/esp32-p4-nano/src/audio.cpp index 4c9d0da15c..11afbdcfb4 100644 --- a/components/esp32-p4-nano/src/audio.cpp +++ b/components/esp32-p4-nano/src/audio.cpp @@ -113,8 +113,15 @@ bool Esp32P4Nano::initialize_audio(uint32_t sample_rate, // The audio task drains this stream buffer to I2S. Size it generously so a // whole UI sound clip fits and play_audio() can enqueue it without blocking. auto tx_buf_size = calc_audio_buffer_size(sample_rate); + // the block (and so the stream's trigger level) must be a whole number of + // frames: the sample-rate formula can land between frames, and a block that + // split one would desynchronize the framing of everything received after it + tx_buf_size -= tx_buf_size % (NUM_CHANNELS * NUM_BYTES_PER_CHANNEL); audio_tx_buffer.resize(tx_buf_size); - audio_tx_stream = xStreamBufferCreate(std::max(tx_buf_size * 4, 64 * 1024), 0); + // Trigger level = one block: the audio task waits for a whole block instead of + // padding a partial one with silence (see audio_task_callback). + audio_tx_stream = + xStreamBufferCreate(std::max(tx_buf_size * 4, 64 * 1024), tx_buf_size); if (audio_tx_stream == nullptr) { return fail_audio_init("Failed to allocate the audio TX stream buffer"); } @@ -295,29 +302,46 @@ void Esp32P4Nano::microphone_volume(float volume) { float Esp32P4Nano::microphone_volume() const { return mic_volume_; } +// How long the audio task waits for a whole block before writing a partial one +// (or silence). Longer than one block at every rate the board supports (a block +// is 1/60 s at the default rate), so partial writes only happen when the +// producer is idle or genuinely behind, never because its next frame simply had +// not arrived yet; short enough that idle->silence latency is not noticeable. +static constexpr uint32_t AUDIO_TX_RECEIVE_TIMEOUT_MS = 40; + bool Esp32P4Nano::audio_task_callback(std::mutex &m, std::condition_variable &cv, bool &task_notified) { - size_t available = xStreamBufferBytesAvailable(audio_tx_stream); size_t buffer_size = audio_tx_buffer.size(); - available = std::min(available, buffer_size); - // only ever hand whole 16-bit samples to I2S; a partial sample would shift - // the framing of everything after it - available &= ~static_cast(1); uint8_t *tx_buf = audio_tx_buffer.data(); - memset(tx_buf, 0, buffer_size); - // Use a finite write timeout (not portMAX_DELAY) so this task returns - // periodically and can observe a stop request; an infinite write would block - // Task::stop() from joining during teardown if the I2S sink ever stalls. - if (available == 0) { + // Wait (bounded) for a whole block rather than taking whatever is queued and + // zero-padding it to a block: producers push audio in bursts (one video frame + // at a time, with frame-to-frame jitter), and the stream buffer exists to + // absorb exactly that. Taking a partial block padded it with silence every + // time the stream momentarily held less than one block, which is audible as + // stutter whenever the producer's cadence is uneven. The DMA is still fed + // whole blocks at a constant cadence while audio is flowing (the stream's + // trigger level is one block); only after the timeout -- producer idle or + // slower than real time -- is a partial block written as is (the I2S driver + // concatenates partial writes without gaps), or silence when there is none. + // Finite timeouts (not portMAX_DELAY) so this task returns periodically and + // can observe a stop request during teardown. + size_t got = xStreamBufferReceive(audio_tx_stream, tx_buf, buffer_size, + pdMS_TO_TICKS(AUDIO_TX_RECEIVE_TIMEOUT_MS)); + // the block is frame-aligned and producers queue whole frames, so this cannot + // happen; if it ever does, do not silently drop bytes (that would desynchronize + // the framing of everything after them) -- say so + constexpr size_t frame_bytes = NUM_CHANNELS * NUM_BYTES_PER_CHANNEL; + if (got % frame_bytes != 0) { + static uint32_t misaligned_count = 0; + if ((misaligned_count++ % 100) == 0) { + logger_.warn("audio stream holds a partial frame ({} bytes)", got); + } + } + if (got == 0) { + memset(tx_buf, 0, buffer_size); i2s_channel_write(audio_tx_handle, tx_buf, buffer_size, NULL, pdMS_TO_TICKS(100)); } else { - xStreamBufferReceive(audio_tx_stream, tx_buf, available, 0); - // Always write a full, frame-aligned buffer (queued samples zero-padded to - // buffer_size) so the I2S DMA is fed at a constant cadence - matching the - // esp-box / t-deck / m5stack-tab5 playback path. Writing only `available` - // bytes makes the drain cadence variable and interleaves whole frames of - // silence into bursty streams, which sounds choppy/glitchy. - i2s_channel_write(audio_tx_handle, tx_buf, buffer_size, NULL, pdMS_TO_TICKS(100)); + i2s_channel_write(audio_tx_handle, tx_buf, got, NULL, pdMS_TO_TICKS(100)); } // honor a stop request per the Task contract: check/clear notified under m std::unique_lock lock(m); diff --git a/components/esp32-p4-wifi6-dev-kit/src/audio.cpp b/components/esp32-p4-wifi6-dev-kit/src/audio.cpp index 1d410ae28e..83a1c6c682 100644 --- a/components/esp32-p4-wifi6-dev-kit/src/audio.cpp +++ b/components/esp32-p4-wifi6-dev-kit/src/audio.cpp @@ -169,8 +169,15 @@ bool Esp32P4Wifi6DevKit::initialize_audio(uint32_t sample_rate, // how much it queued, so callers stream clips longer than the buffer in // chunks (see the example's playback loop). auto tx_buf_size = calc_audio_buffer_size(sample_rate, TX_NUM_CHANNELS); + // the block (and so the stream's trigger level) must be a whole number of + // frames: the sample-rate formula can land between frames, and a block that + // split one would desynchronize the framing of everything received after it + tx_buf_size -= tx_buf_size % (TX_NUM_CHANNELS * NUM_BYTES_PER_CHANNEL); audio_tx_buffer.resize(tx_buf_size); - audio_tx_stream = xStreamBufferCreate(std::max(tx_buf_size * 4, 4 * 1024), 0); + // Trigger level = one block: the audio task waits for a whole block instead of + // padding a partial one with silence (see audio_task_callback). + audio_tx_stream = + xStreamBufferCreate(std::max(tx_buf_size * 4, 4 * 1024), tx_buf_size); if (audio_tx_stream == nullptr) { return fail_audio_init("Failed to allocate the audio TX stream buffer"); } @@ -380,26 +387,48 @@ void Esp32P4Wifi6DevKit::microphone_volume(float volume) { float Esp32P4Wifi6DevKit::microphone_volume() const { return mic_volume_; } +// How long the audio task waits for a whole block before writing a partial one +// (or silence). Longer than one block at every rate the board supports (a block +// is 1/60 s at the default rate), so partial writes only happen when the +// producer is idle or genuinely behind, never because its next frame simply had +// not arrived yet; short enough that idle->silence latency is not noticeable. +static constexpr uint32_t AUDIO_TX_RECEIVE_TIMEOUT_MS = 40; + bool Esp32P4Wifi6DevKit::audio_task_callback(std::mutex &m, std::condition_variable &cv, bool &task_notified) { - (void)cv; // unused: this task paces itself on the finite-timeout I2S write below - size_t available = xStreamBufferBytesAvailable(audio_tx_stream); + (void)cv; // unused: this task paces itself on the finite-timeout receive/write below size_t buffer_size = audio_tx_buffer.size(); - available = std::min(available, buffer_size); - // only ever hand whole 16-bit samples to I2S; a partial sample would shift - // the framing of everything after it - available &= ~static_cast(1); uint8_t *tx_buf = audio_tx_buffer.data(); - memset(tx_buf, 0, buffer_size); - if (available > 0) { - xStreamBufferReceive(audio_tx_stream, tx_buf, available, 0); - } - // Always write a full, frame-aligned buffer (queued samples zero-padded to - // buffer_size) so the I2S DMA is fed at a constant cadence - matching the - // esp-box / t-deck / m5stack-tab5 playback path. Writing only `available` - // bytes makes the drain cadence variable and interleaves whole frames of - // silence into bursty streams, which sounds choppy/glitchy. - // + // Wait (bounded) for a whole block rather than taking whatever is queued and + // zero-padding it to a block: producers push audio in bursts (one video frame + // at a time, with frame-to-frame jitter), and the stream buffer exists to + // absorb exactly that. Taking a partial block padded it with silence every + // time the stream momentarily held less than one block, which is audible as + // stutter whenever the producer's cadence is uneven. The DMA is still fed + // whole blocks at a constant cadence while audio is flowing (the stream's + // trigger level is one block); only after the timeout -- producer idle or + // slower than real time -- is a partial block written as is (the I2S driver + // concatenates partial writes without gaps), or silence when there is none. + // The wait happens before taking audio_tx_mutex_ so a sample-rate change is + // never held off by it. + size_t got = xStreamBufferReceive(audio_tx_stream, tx_buf, buffer_size, + pdMS_TO_TICKS(AUDIO_TX_RECEIVE_TIMEOUT_MS)); + // the block is frame-aligned and producers queue whole frames, so this cannot + // happen; if it ever does, do not silently drop bytes (that would desynchronize + // the framing of everything after them) -- say so + constexpr size_t frame_bytes = TX_NUM_CHANNELS * NUM_BYTES_PER_CHANNEL; + if (got % frame_bytes != 0) { + static uint32_t misaligned_count = 0; + if ((misaligned_count++ % 100) == 0) { + logger_.warn("audio stream holds a partial frame ({} bytes)", got); + } + } + size_t to_write = buffer_size; + if (got == 0) { + memset(tx_buf, 0, buffer_size); + } else { + to_write = got; + } // Use a finite write timeout (not portMAX_DELAY) so this task returns // periodically and can observe a stop request; an infinite write would block // Task::stop() from joining during teardown if the I2S sink ever stalls. @@ -409,15 +438,14 @@ bool Esp32P4Wifi6DevKit::audio_task_callback(std::mutex &m, std::condition_varia // Serialize with audio_sample_rate(), which may disable/reconfigure the // channel; writing to a disabled channel would race the reconfig. std::lock_guard lk(audio_tx_mutex_); - err = - i2s_channel_write(audio_tx_handle, tx_buf, buffer_size, &bytes_written, pdMS_TO_TICKS(100)); + err = i2s_channel_write(audio_tx_handle, tx_buf, to_write, &bytes_written, pdMS_TO_TICKS(100)); } - if (err != ESP_OK || bytes_written != buffer_size) { + if (err != ESP_OK || bytes_written != to_write) { // Rate-limit: a stalled I2S sink would otherwise emit this every ~100 ms. static uint32_t write_warn_count = 0; if ((write_warn_count++ % 100) == 0) { logger_.warn("i2s_channel_write: {} ({} of {} bytes written, occurrence {})", - esp_err_to_name(err), bytes_written, buffer_size, write_warn_count); + esp_err_to_name(err), bytes_written, to_write, write_warn_count); } } // honor a stop request per the Task contract: check/clear notified under m diff --git a/components/m5stack-cardputer/src/audio.cpp b/components/m5stack-cardputer/src/audio.cpp index d664c7452d..acbc4327e1 100644 --- a/components/m5stack-cardputer/src/audio.cpp +++ b/components/m5stack-cardputer/src/audio.cpp @@ -119,8 +119,14 @@ bool M5StackCardputer::initialize_sound(uint32_t default_audio_rate, // size the transmit buffering from the actual (possibly shared) rate size_t buffer_size = calc_audio_buffer_size(audio_sample_rate()); + // the block (and so the stream's trigger level) must be a whole number of + // frames: the sample-rate formula can land between frames, and a block that + // split one would desynchronize the framing of everything received after it + buffer_size -= buffer_size % (NUM_CHANNELS * NUM_BYTES_PER_CHANNEL); audio_tx_buffer.resize(buffer_size); - audio_tx_stream = xStreamBufferCreate(buffer_size * 4, 0); + // Trigger level = one block: the audio task waits for a whole block instead of + // padding a partial one with silence (see audio_task_callback). + audio_tx_stream = xStreamBufferCreate(buffer_size * 4, buffer_size); if (audio_tx_stream == nullptr) { logger_.error("Could not allocate the audio stream buffer"); audio_tx_buffer.clear(); @@ -177,27 +183,54 @@ bool M5StackCardputer::initialize_sound(uint32_t default_audio_rate, return audio_task_->start(); } +// How long the audio task waits for a whole block before writing a partial one +// (or silence). Longer than one block at every rate the board supports (a block +// is 1/60 s at the default rate), so partial writes only happen when the +// producer is idle or genuinely behind, never because its next frame simply had +// not arrived yet; short enough that idle->silence latency is not noticeable. +static constexpr uint32_t AUDIO_TX_RECEIVE_TIMEOUT_MS = 40; + bool M5StackCardputer::audio_task_callback(std::mutex &m, std::condition_variable &cv, bool &task_notified) { - // Queue the next I2S out frame to write - size_t available = xStreamBufferBytesAvailable(audio_tx_stream); + // Queue the next I2S out block. Wait (bounded) for a whole block rather than + // taking whatever is queued and zero-padding it to a block: producers push + // audio in bursts (one video frame at a time, with frame-to-frame jitter), + // and the stream buffer exists to absorb exactly that. Taking a partial block + // padded it with silence every time the stream momentarily held less than + // one block, which is audible as stutter whenever the producer's cadence is + // uneven. The DMA is still fed whole blocks while audio is flowing (the + // stream's trigger level is one block); only after the timeout -- producer + // idle or slower than real time -- is a partial block written as is (the I2S + // driver concatenates partial writes without gaps), or silence when there is + // none. size_t buffer_size = audio_tx_buffer.size(); - available = std::min(available, buffer_size); uint8_t *buffer = &audio_tx_buffer[0]; - memset(buffer, 0, buffer_size); - - if (available > 0) { - xStreamBufferReceive(audio_tx_stream, buffer, available, 0); - // The amplifier has no volume control, so scale the samples in software - // according to the current volume / mute state - float scale = mute_ ? 0.0f : (volume_ / 100.0f); - auto *samples = reinterpret_cast(buffer); - size_t num_samples = available / sizeof(int16_t); - for (size_t i = 0; i < num_samples; i++) { - samples[i] = static_cast(samples[i] * scale); + size_t got = xStreamBufferReceive(audio_tx_stream, buffer, buffer_size, + pdMS_TO_TICKS(AUDIO_TX_RECEIVE_TIMEOUT_MS)); + // the block is frame-aligned and producers queue whole frames, so this cannot + // happen; if it ever does, do not silently drop bytes (that would desynchronize + // the framing of everything after them) -- say so + constexpr size_t frame_bytes = NUM_CHANNELS * NUM_BYTES_PER_CHANNEL; + if (got % frame_bytes != 0) { + static uint32_t misaligned_count = 0; + if ((misaligned_count++ % 100) == 0) { + logger_.warn("audio stream holds a partial frame ({} bytes)", got); } } - i2s_channel_write(audio_tx_handle, buffer, buffer_size, NULL, portMAX_DELAY); + if (got == 0) { + memset(buffer, 0, buffer_size); + i2s_channel_write(audio_tx_handle, buffer, buffer_size, NULL, portMAX_DELAY); + return false; // don't stop the task + } + // The amplifier has no volume control, so scale the samples in software + // according to the current volume / mute state + float scale = mute_ ? 0.0f : (volume_ / 100.0f); + auto *samples = reinterpret_cast(buffer); + size_t num_samples = got / sizeof(int16_t); + for (size_t i = 0; i < num_samples; i++) { + samples[i] = static_cast(samples[i] * scale); + } + i2s_channel_write(audio_tx_handle, buffer, got, NULL, portMAX_DELAY); return false; // don't stop the task } diff --git a/components/m5stack-tab5/src/audio.cpp b/components/m5stack-tab5/src/audio.cpp index 5013885962..ee10957a24 100644 --- a/components/m5stack-tab5/src/audio.cpp +++ b/components/m5stack-tab5/src/audio.cpp @@ -86,8 +86,14 @@ bool M5StackTab5::initialize_audio(uint32_t sample_rate, // Stream buffers and task auto tx_buf_size = calc_audio_buffer_size(sample_rate); + // the block (and so the stream's trigger level) must be a whole number of + // frames: the sample-rate formula can land between frames, and a block that + // split one would desynchronize the framing of everything received after it + tx_buf_size -= tx_buf_size % (NUM_CHANNELS * NUM_BYTES_PER_CHANNEL); audio_tx_buffer.resize(tx_buf_size); - audio_tx_stream = xStreamBufferCreate(tx_buf_size * 4, 0); + // Trigger level = one block: the audio task waits for a whole block instead of + // padding a partial one with silence (see audio_task_callback). + audio_tx_stream = xStreamBufferCreate(tx_buf_size * 4, tx_buf_size); xStreamBufferReset(audio_tx_stream); // RX buffer for recording audio_rx_buffer.resize(tx_buf_size); @@ -332,22 +338,45 @@ void M5StackTab5::stop_audio_recording() { logger_.info("Audio recording stopped"); } +// How long the audio task waits for a whole block before writing a partial one +// (or silence). Longer than one block at every rate the board supports (a block +// is 1/60 s at the default rate), so partial writes only happen when the +// producer is idle or genuinely behind, never because its next frame simply had +// not arrived yet; short enough that idle->silence latency is not noticeable. +static constexpr uint32_t AUDIO_TX_RECEIVE_TIMEOUT_MS = 40; + bool M5StackTab5::audio_task_callback(std::mutex &m, std::condition_variable &cv, bool &task_notified) { - // Playback: write next buffer worth of audio from stream buffer - // Queue the next I2S out frame to write (matches the esp-box / t-deck path): - // always write a full, frame-aligned buffer with the queued samples zero- - // padded up to buffer_size so the I2S DMA is fed at a constant cadence. - size_t available = xStreamBufferBytesAvailable(audio_tx_stream); + // Playback: write the next block of audio from the stream buffer. Wait + // (bounded) for a whole block rather than taking whatever is queued and + // zero-padding it to a block: producers push audio in bursts (one video frame + // at a time, with frame-to-frame jitter), and the stream buffer exists to + // absorb exactly that. Taking a partial block padded it with silence every + // time the stream momentarily held less than one block, which is audible as + // stutter whenever the producer's cadence is uneven. The DMA is still fed + // whole blocks at a constant cadence while audio is flowing (the stream's + // trigger level is one block); only after the timeout -- producer idle or + // slower than real time -- is a partial block written as is (the I2S driver + // concatenates partial writes without gaps), or silence when there is none. size_t buffer_size = audio_tx_buffer.size(); - available = std::min(available, buffer_size); uint8_t *tx_buf = audio_tx_buffer.data(); - memset(tx_buf, 0, buffer_size); - if (available == 0) { + size_t got = xStreamBufferReceive(audio_tx_stream, tx_buf, buffer_size, + pdMS_TO_TICKS(AUDIO_TX_RECEIVE_TIMEOUT_MS)); + // the block is frame-aligned and producers queue whole frames, so this cannot + // happen; if it ever does, do not silently drop bytes (that would desynchronize + // the framing of everything after them) -- say so + constexpr size_t frame_bytes = NUM_CHANNELS * NUM_BYTES_PER_CHANNEL; + if (got % frame_bytes != 0) { + static uint32_t misaligned_count = 0; + if ((misaligned_count++ % 100) == 0) { + logger_.warn("audio stream holds a partial frame ({} bytes)", got); + } + } + if (got == 0) { + memset(tx_buf, 0, buffer_size); i2s_channel_write(audio_tx_handle, tx_buf, buffer_size, NULL, portMAX_DELAY); } else { - xStreamBufferReceive(audio_tx_stream, tx_buf, available, 0); - i2s_channel_write(audio_tx_handle, tx_buf, buffer_size, NULL, portMAX_DELAY); + i2s_channel_write(audio_tx_handle, tx_buf, got, NULL, portMAX_DELAY); } return false; diff --git a/components/smartpanlee-sc01-plus/src/smartpanlee-sc01-plus.cpp b/components/smartpanlee-sc01-plus/src/smartpanlee-sc01-plus.cpp index 89d559e786..332b008086 100644 --- a/components/smartpanlee-sc01-plus/src/smartpanlee-sc01-plus.cpp +++ b/components/smartpanlee-sc01-plus/src/smartpanlee-sc01-plus.cpp @@ -163,8 +163,14 @@ bool SmartPanleeSc01Plus::initialize_i2s(uint32_t default_audio_rate) { ESP_ERROR_CHECK(i2s_channel_init_std_mode(audio_tx_handle_, &audio_std_cfg_)); auto buffer_size = calc_audio_buffer_size(default_audio_rate); + // the block (and so the stream's trigger level) must be a whole number of + // frames: the sample-rate formula can land between frames, and a block that + // split one would desynchronize the framing of everything received after it + buffer_size -= buffer_size % (audio_num_channels * audio_num_bytes_per_channel); audio_tx_buffer_.resize(buffer_size); - audio_tx_stream_ = xStreamBufferCreate(buffer_size * 4, 0); + // Trigger level = one block: the audio task waits for a whole block instead of + // padding a partial one with silence (see audio_task_callback). + audio_tx_stream_ = xStreamBufferCreate(buffer_size * 4, buffer_size); if (!audio_tx_stream_) { logger_.error("Failed to create audio stream buffer"); return false; @@ -518,22 +524,47 @@ bool SmartPanleeSc01Plus::update_touch() { return changed; } +// How long the audio task waits for a whole block before writing a partial one +// (or silence). Longer than one block at every rate the board supports (a block +// is 1/60 s at the default rate), so partial writes only happen when the +// producer is idle or genuinely behind, never because its next frame simply had +// not arrived yet; short enough that idle->silence latency is not noticeable. +static constexpr uint32_t AUDIO_TX_RECEIVE_TIMEOUT_MS = 40; + bool SmartPanleeSc01Plus::audio_task_callback(std::mutex &m, std::condition_variable &cv, bool &task_notified) { (void)m; (void)cv; (void)task_notified; - auto available = xStreamBufferBytesAvailable(audio_tx_stream_); + // Wait (bounded) for a whole block rather than taking whatever is queued and + // zero-padding it to a block: producers push audio in bursts (one video frame + // at a time, with frame-to-frame jitter), and the stream buffer exists to + // absorb exactly that. Taking a partial block padded it with silence every + // time the stream momentarily held less than one block, which is audible as + // stutter whenever the producer's cadence is uneven. The DMA is still fed + // whole blocks at a constant cadence while audio is flowing (the stream's + // trigger level is one block); only after the timeout -- producer idle or + // slower than real time -- is a partial block written as is (the I2S driver + // concatenates partial writes without gaps), or silence when there is none. auto buffer_size = audio_tx_buffer_.size(); - available = std::min(available, buffer_size); - uint8_t *buffer = audio_tx_buffer_.data(); - std::memset(buffer, 0, buffer_size); - - size_t bytes_received = 0; - if (available > 0) { - bytes_received = xStreamBufferReceive(audio_tx_stream_, buffer, available, 0); + size_t bytes_received = xStreamBufferReceive(audio_tx_stream_, buffer, buffer_size, + pdMS_TO_TICKS(AUDIO_TX_RECEIVE_TIMEOUT_MS)); + // the block is frame-aligned and producers queue whole frames, so this cannot + // happen; if it ever does, do not silently drop bytes (that would desynchronize + // the framing of everything after them) -- say so + constexpr size_t frame_bytes = audio_num_channels * audio_num_bytes_per_channel; + if (bytes_received % frame_bytes != 0) { + static uint32_t misaligned_count = 0; + if ((misaligned_count++ % 100) == 0) { + logger_.warn("audio stream holds a partial frame ({} bytes)", bytes_received); + } + } + if (bytes_received == 0) { + std::memset(buffer, 0, buffer_size); + i2s_channel_write(audio_tx_handle_, buffer, buffer_size, nullptr, portMAX_DELAY); + return false; } if (mute_) { @@ -551,7 +582,7 @@ bool SmartPanleeSc01Plus::audio_task_callback(std::mutex &m, std::condition_vari } } - i2s_channel_write(audio_tx_handle_, buffer, buffer_size, nullptr, portMAX_DELAY); + i2s_channel_write(audio_tx_handle_, buffer, bytes_received, nullptr, portMAX_DELAY); return false; } diff --git a/components/t-deck/src/audio.cpp b/components/t-deck/src/audio.cpp index d2657c107c..79b0530f36 100644 --- a/components/t-deck/src/audio.cpp +++ b/components/t-deck/src/audio.cpp @@ -110,9 +110,15 @@ bool TDeck::initialize_i2s(uint32_t default_audio_rate) { ESP_ERROR_CHECK(i2s_channel_init_std_mode(audio_tx_handle, &audio_std_cfg)); auto buffer_size = calc_audio_buffer_size(default_audio_rate); + // the block (and so the stream's trigger level) must be a whole number of + // frames: the sample-rate formula can land between frames, and a block that + // split one would desynchronize the framing of everything received after it + buffer_size -= buffer_size % (NUM_CHANNELS * NUM_BYTES_PER_CHANNEL); audio_tx_buffer.resize(buffer_size); - audio_tx_stream = xStreamBufferCreate(buffer_size * 4, 0); + // Trigger level = one block: the audio task waits for a whole block instead of + // padding a partial one with silence (see audio_task_callback). + audio_tx_stream = xStreamBufferCreate(buffer_size * 4, buffer_size); xStreamBufferReset(audio_tx_stream); @@ -146,35 +152,59 @@ bool TDeck::initialize_sound(uint32_t default_audio_rate, return audio_task_->start(); } +// How long the audio task waits for a whole block before writing a partial one +// (or silence). Longer than one block at every rate the board supports (a block +// is 1/60 s at the default rate), so partial writes only happen when the +// producer is idle or genuinely behind, never because its next frame simply had +// not arrived yet; short enough that idle->silence latency is not noticeable. +static constexpr uint32_t AUDIO_TX_RECEIVE_TIMEOUT_MS = 40; + bool TDeck::audio_task_callback(std::mutex &m, std::condition_variable &cv, bool &task_notified) { - // Queue the next I2S out frame to write - size_t available = xStreamBufferBytesAvailable(audio_tx_stream); + // Queue the next I2S out block. Wait (bounded) for a whole block rather than + // taking whatever is queued and zero-padding it to a block: producers push + // audio in bursts (one video frame at a time, with frame-to-frame jitter), + // and the stream buffer exists to absorb exactly that. Taking a partial block + // padded it with silence every time the stream momentarily held less than + // one block, which is audible as stutter whenever the producer's cadence is + // uneven. The DMA is still fed whole blocks while audio is flowing (the + // stream's trigger level is one block); only after the timeout -- producer + // idle or slower than real time -- is a partial block written as is (the I2S + // driver concatenates partial writes without gaps), or silence when there is + // none. size_t buffer_size = audio_tx_buffer.size(); - available = std::min(available, buffer_size); uint8_t *buffer = &audio_tx_buffer[0]; - memset(buffer, 0, buffer_size); + size_t got = xStreamBufferReceive(audio_tx_stream, buffer, buffer_size, + pdMS_TO_TICKS(AUDIO_TX_RECEIVE_TIMEOUT_MS)); + // the block is frame-aligned and producers queue whole frames, so this cannot + // happen; if it ever does, do not silently drop bytes (that would desynchronize + // the framing of everything after them) -- say so + constexpr size_t frame_bytes = NUM_CHANNELS * NUM_BYTES_PER_CHANNEL; + if (got % frame_bytes != 0) { + static uint32_t misaligned_count = 0; + if ((misaligned_count++ % 100) == 0) { + logger_.warn("audio stream holds a partial frame ({} bytes)", got); + } + } - if (available == 0) { + if (got == 0) { + memset(buffer, 0, buffer_size); i2s_channel_write(audio_tx_handle, buffer, buffer_size, NULL, portMAX_DELAY); } else { - // get the data from the buffer - xStreamBufferReceive(audio_tx_stream, buffer, available, 0); - // apply volume and mute if (mute_) { - memset(buffer, 0, available); + memset(buffer, 0, got); } else { // NOTE: we are actually using int16_t samples, so we need to adjust the // pointers to the buffer to the correct positions. int16_t *ptr = reinterpret_cast(buffer); - for (size_t i = 0; i < (available / 2); ++i) { + for (size_t i = 0; i < (got / 2); ++i) { int32_t sample = (ptr[i] * volume_) / 100.0f; ptr[i] = static_cast(std::clamp(sample, INT16_MIN, INT16_MAX)); } } // write the buffer to the I2S channel - i2s_channel_write(audio_tx_handle, buffer, buffer_size, NULL, portMAX_DELAY); + i2s_channel_write(audio_tx_handle, buffer, got, NULL, portMAX_DELAY); } return false; // don't stop the task }