fix(bsp): audio tasks wait for a whole block instead of padding with silence (all 10 BSPs)

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
 }

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<size_t>(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<size_t>(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<size_t>(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<std::mutex> lock(m);

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<size_t>(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<size_t>(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<size_t>(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;
 }

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<size_t>(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<size_t>(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<size_t>(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<std::mutex> 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