Give PcmBuffer pairs a name, and wire them up in the audio stack

6 files changed, 47 insertions, 36 deletions

diff --git a/src/drivers/bluetooth.cpp b/src/drivers/bluetooth.cpp
index 23c4f8d8..acb38ce4 100644
--- a/src/drivers/bluetooth.cpp
+++ b/src/drivers/bluetooth.cpp
@@ -37,8 +37,7 @@ namespace drivers {
 
 [[maybe_unused]] static constexpr char kTag[] = "bluetooth";
 
-DRAM_ATTR static PcmBuffer* sStream1 = nullptr;
-DRAM_ATTR static PcmBuffer* sStream2 = nullptr;
+DRAM_ATTR static OutputBuffers* sStreams = nullptr;
 DRAM_ATTR static std::atomic<float> sVolumeFactor = 1.f;
 
 static tasks::WorkerPool* sBgWorker;
@@ -97,15 +96,16 @@ IRAM_ATTR auto a2dp_data_cb(uint8_t* buf, int32_t buf_size) -> int32_t {
   if (buf == nullptr || buf_size <= 0) {
     return 0;
   }
-  PcmBuffer* stream1 = sStream1;
-  PcmBuffer* stream2 = sStream2;
-  if (stream1 == nullptr || stream2 == nullptr) {
+  OutputBuffers* streams = sStreams;
+  if (streams == nullptr) {
     return 0;
   }
 
   int16_t* samples = reinterpret_cast<int16_t*>(buf);
-  stream1->receive({samples, static_cast<size_t>(buf_size / 2)}, false, false);
-  stream2->receive({samples, static_cast<size_t>(buf_size / 2)}, true, false);
+  streams->first.receive({samples, static_cast<size_t>(buf_size / 2)}, false,
+                         false);
+  streams->second.receive({samples, static_cast<size_t>(buf_size / 2)}, true,
+                          false);
 
   // Apply software volume scaling.
   float factor = sVolumeFactor.load();
@@ -184,14 +184,13 @@ auto Bluetooth::PreferredDevice() -> std::optional<bluetooth::MacAndName> {
   return bluetooth::BluetoothState::preferred_device();
 }
 
-auto Bluetooth::SetSources(PcmBuffer* src1, PcmBuffer* src2) -> void {
+auto Bluetooth::SetSources(OutputBuffers* src) -> void {
   auto lock = bluetooth::BluetoothState::lock();
-  PcmBuffer *cur1, *cur2;
-  std::tie(cur1, cur2) = bluetooth::BluetoothState::sources();
-  if (src1 == cur1 && src2 == cur2) {
+  OutputBuffers* cur = bluetooth::BluetoothState::sources();
+  if (src == cur) {
     return;
   }
-  bluetooth::BluetoothState::sources(src1, src2);
+  bluetooth::BluetoothState::sources(src);
   tinyfsm::FsmList<bluetooth::BluetoothState>::dispatch(
       bluetooth::events::SourcesChanged{});
 }
@@ -381,13 +380,12 @@ auto BluetoothState::preferred_device(std::optional<MacAndName> addr) -> void {
   sPreferredDevice_ = addr;
 }
 
-auto BluetoothState::sources() -> std::pair<PcmBuffer*, PcmBuffer*> {
-  return {sStream1, sStream2};
+auto BluetoothState::sources() -> OutputBuffers* {
+  return sStreams;
 }
 
-auto BluetoothState::sources(PcmBuffer* src1, PcmBuffer* src2) -> void {
-  sStream1 = src1;
-  sStream2 = src2;
+auto BluetoothState::sources(OutputBuffers* src) -> void {
+  sStreams = src;
 }
 
 auto BluetoothState::event_handler(std::function<void(Event)> cb) -> void {
@@ -715,7 +713,6 @@ void Connected::entry() {
                        sPreferredDevice_->mac != stored_pref->mac)) {
     sStorage_->PreferredBluetoothDevice(sPreferredDevice_);
   }
-  // TODO: if we already have a source, immediately start playing
 }
 
 void Connected::exit() {
@@ -733,7 +730,7 @@ void Connected::react(const events::PreferredDeviceChanged& ev) {
 }
 
 void Connected::react(const events::SourcesChanged& ev) {
-  if (sStream1 != nullptr && sStream2 != nullptr) {
+  if (sStreams != nullptr) {
     ESP_LOGI(kTag, "checking source is ready");
     esp_a2d_media_ctrl(ESP_A2D_MEDIA_CTRL_CHECK_SRC_RDY);
   } else {
diff --git a/src/drivers/i2s_dac.cpp b/src/drivers/i2s_dac.cpp
index b1044896..4e2e171a 100644
--- a/src/drivers/i2s_dac.cpp
+++ b/src/drivers/i2s_dac.cpp
@@ -52,10 +52,12 @@ extern "C" IRAM_ATTR auto callback(i2s_chan_handle_t handle,
   assert(event->size % 4 == 0);
 
   uint8_t* buf = *reinterpret_cast<uint8_t**>(event->data);
-  auto* src = reinterpret_cast<PcmBuffer*>(user_ctx);
+  auto* src = reinterpret_cast<OutputBuffers*>(user_ctx);
 
-  BaseType_t ret =
-      src->receive({reinterpret_cast<int16_t*>(buf), event->size / 2}, true);
+  BaseType_t ret1 = src->first.receive(
+      {reinterpret_cast<int16_t*>(buf), event->size / 2}, false, true);
+  BaseType_t ret2 = src->second.receive(
+      {reinterpret_cast<int16_t*>(buf), event->size / 2}, true, true);
 
   // The ESP32's I2S peripheral has a different endianness to its processors.
   // ESP-IDF handles this difference for stereo channels, but not for mono
@@ -70,10 +72,10 @@ extern "C" IRAM_ATTR auto callback(i2s_chan_handle_t handle,
     }
   }
 
-  return ret;
+  return ret1 || ret2;
 }
 
-auto I2SDac::create(IGpios& expander, PcmBuffer& buf)
+auto I2SDac::create(IGpios& expander, OutputBuffers& bufs)
     -> std::optional<I2SDac*> {
   i2s_chan_handle_t i2s_handle;
   i2s_chan_config_t channel_config{
@@ -90,7 +92,7 @@ auto I2SDac::create(IGpios& expander, PcmBuffer& buf)
   // First, instantiate the instance so it can do all of its power on
   // configuration.
   std::unique_ptr<I2SDac> dac =
-      std::make_unique<I2SDac>(expander, buf, i2s_handle);
+      std::make_unique<I2SDac>(expander, bufs, i2s_handle);
 
   // Whilst we wait for the initial boot, we can work on installing the I2S
   // driver.
@@ -122,14 +124,14 @@ auto I2SDac::create(IGpios& expander, PcmBuffer& buf)
       .on_sent = callback,
       .on_send_q_ovf = NULL,
   };
-  i2s_channel_register_event_callback(i2s_handle, &callbacks, &buf);
+  i2s_channel_register_event_callback(i2s_handle, &callbacks, &bufs);
 
   return dac.release();
 }
 
-I2SDac::I2SDac(IGpios& gpio, PcmBuffer& buf, i2s_chan_handle_t i2s_handle)
+I2SDac::I2SDac(IGpios& gpio, OutputBuffers& bufs, i2s_chan_handle_t i2s_handle)
     : gpio_(gpio),
-      buffer_(buf),
+      buffers_(bufs),
       i2s_handle_(i2s_handle),
       i2s_active_(false),
       clock_config_(I2S_STD_CLK_DEFAULT_CONFIG(48000)),
diff --git a/src/drivers/include/drivers/bluetooth.hpp b/src/drivers/include/drivers/bluetooth.hpp
index b3b12ffc..eaecfb2b 100644
--- a/src/drivers/include/drivers/bluetooth.hpp
+++ b/src/drivers/include/drivers/bluetooth.hpp
@@ -43,7 +43,7 @@ class Bluetooth {
   auto SetPreferredDevice(std::optional<bluetooth::MacAndName> dev) -> void;
   auto PreferredDevice() -> std::optional<bluetooth::MacAndName>;
 
-  auto SetSources(PcmBuffer*, PcmBuffer*) -> void;
+  auto SetSources(OutputBuffers*) -> void;
   auto SetVolumeFactor(float) -> void;
 
   auto SetEventHandler(std::function<void(bluetooth::Event)> cb) -> void;
@@ -118,8 +118,8 @@ class BluetoothState : public tinyfsm::Fsm<BluetoothState> {
   static auto discovery() -> bool;
   static auto discovery(bool) -> void;
 
-  static auto sources() -> std::pair<PcmBuffer*, PcmBuffer*>;
-  static auto sources(PcmBuffer*, PcmBuffer*) -> void;
+  static auto sources() -> OutputBuffers*;
+  static auto sources(OutputBuffers*) -> void;
 
   static auto event_handler(std::function<void(Event)>) -> void;
 
diff --git a/src/drivers/include/drivers/i2s_dac.hpp b/src/drivers/include/drivers/i2s_dac.hpp
index 0fe462b4..891acb56 100644
--- a/src/drivers/include/drivers/i2s_dac.hpp
+++ b/src/drivers/include/drivers/i2s_dac.hpp
@@ -40,9 +40,10 @@ constexpr size_t kI2SBufferLengthFrames = 1024;
  */
 class I2SDac {
  public:
-  static auto create(IGpios& expander, PcmBuffer&, PcmBuffer&) -> std::optional<I2SDac*>;
+  static auto create(IGpios& expander, OutputBuffers&)
+      -> std::optional<I2SDac*>;
 
-  I2SDac(IGpios& gpio, PcmBuffer&, i2s_chan_handle_t i2s_handle);
+  I2SDac(IGpios& gpio, OutputBuffers&, i2s_chan_handle_t i2s_handle);
   ~I2SDac();
 
   auto SetPaused(bool) -> void;
@@ -77,8 +78,7 @@ class I2SDac {
   auto set_channel(bool) -> void;
 
   IGpios& gpio_;
-  PcmBuffer& buffer1_;
-  PcmBuffer& buffer2_;
+  OutputBuffers& buffers_;
   i2s_chan_handle_t i2s_handle_;
 
   bool i2s_active_;
diff --git a/src/drivers/include/drivers/pcm_buffer.hpp b/src/drivers/include/drivers/pcm_buffer.hpp
index 27e9eec6..968c3398 100644
--- a/src/drivers/include/drivers/pcm_buffer.hpp
+++ b/src/drivers/include/drivers/pcm_buffer.hpp
@@ -74,4 +74,16 @@ class PcmBuffer {
   RingbufHandle_t ringbuf_;
 };
 
+/*
+ * Convenience type for a pair of PcmBuffers. Each audio output handles mixing
+ * streams together to ensure that low-latency sounds in one channel (e.g. a
+ * system notification bleep) aren't delayed by a large audio buffer in the
+ * other channel (e.g. a long-running track).
+ *
+ * By convention, the first buffer of this pair is used for tracks, whilst the
+ * second is reserved for 'system sounds'; usually TTS, but potentially maybe
+ * other informative noises.
+ */
+using OutputBuffers = std::pair<PcmBuffer, PcmBuffer>;
+
 }  // namespace drivers
diff --git a/src/drivers/pcm_buffer.cpp b/src/drivers/pcm_buffer.cpp
index b619cefb..1d2bab1e 100644
--- a/src/drivers/pcm_buffer.cpp
+++ b/src/drivers/pcm_buffer.cpp
@@ -56,7 +56,7 @@ IRAM_ATTR auto PcmBuffer::receive(std::span<int16_t> dest, bool mix, bool isr)
   }
 
   size_t total_read = first_read + second_read;
-  if (total_read < dest.size()) {
+  if (total_read < dest.size() && !mix) {
     std::fill_n(dest.begin() + total_read, dest.size() - total_read, 0);
   }