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

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 OutputBuffers* sStreams = nullptr;
-DRAM_ATTR static PcmBuffer* sStream2 = 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;
+  OutputBuffers* streams = sStreams;
-  PcmBuffer* stream2 = sStream2;
+  if (streams == nullptr) {
-  if (stream1 == nullptr || stream2 == nullptr) {
     return 0;
   }
 
   int16_t* samples = reinterpret_cast<int16_t*>(buf);
-  stream1->receive({samples, static_cast<size_t>(buf_size / 2)}, false, false);
+  streams->first.receive({samples, static_cast<size_t>(buf_size / 2)}, false,
-  stream2->receive({samples, static_cast<size_t>(buf_size / 2)}, true, 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;
+  OutputBuffers* cur = bluetooth::BluetoothState::sources();
-  std::tie(cur1, cur2) = bluetooth::BluetoothState::sources();
+  if (src == cur) {
-  if (src1 == cur1 && src2 == cur2) {
     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*> {
+auto BluetoothState::sources() -> OutputBuffers* {
-  return {sStream1, sStream2};
+  return sStreams;
 }
 
-auto BluetoothState::sources(PcmBuffer* src1, PcmBuffer* src2) -> void {
+auto BluetoothState::sources(OutputBuffers* src) -> void {
-  sStream1 = src1;
+  sStreams = src;
-  sStream2 = src2;
 }
 
 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 {

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 =
+  BaseType_t ret1 = src->first.receive(
-      src->receive({reinterpret_cast<int16_t*>(buf), event->size / 2}, true);
+      {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)),

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() -> OutputBuffers*;
-  static auto sources(PcmBuffer*, PcmBuffer*) -> void;
+  static auto sources(OutputBuffers*) -> void;
 
   static auto event_handler(std::function<void(Event)>) -> void;
 

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_;
+  OutputBuffers& buffers_;
-  PcmBuffer& buffer2_;
   i2s_chan_handle_t i2s_handle_;
 
   bool i2s_active_;

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

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);
   }
 

src/tangara/audio/audio_fsm.cpp

@@ -59,10 +59,16 @@ std::shared_ptr<IAudioOutput> AudioState::sOutput;
 std::shared_ptr<I2SAudioOutput> AudioState::sI2SOutput;
 std::shared_ptr<BluetoothAudioOutput> AudioState::sBtOutput;
 
-// Two seconds of samples for two channels, at a representative sample rate.
+// For tracks, keep about two seconds' worth of samples at 2ch 48kHz. This
-constexpr size_t kDrainLatencySamples = 48000 * 2 * 2;
+// is more headroom than we need for small playback, but it doesn't hurt to
+// keep some PSRAM in our pockets for a rainy day.
+constexpr size_t kTrackDrainLatencySamples = 48000 * 2 * 2;
 
-std::unique_ptr<drivers::PcmBuffer> AudioState::sDrainBuffer;
+// For system sounds, we intentionally choose codecs that are very fast to
+// decode. This lets us get away with a much smaller drain buffer.
+constexpr size_t kSystemDrainLatencySamples = 48000;
+
+std::unique_ptr<drivers::OutputBuffers> AudioState::sDrainBuffers;
 std::optional<IAudioOutput::Format> AudioState::sDrainFormat;
 
 StreamCues AudioState::sStreamCues;
@@ -237,11 +243,11 @@ void AudioState::react(const system_fsm::BluetoothEvent& ev) {
         break;
     }
   }
-  if (std::holds_alternative<drivers::bluetooth::RemoteVolumeChanged>(ev.event)) {
+  if (std::holds_alternative<drivers::bluetooth::RemoteVolumeChanged>(
-    auto volume_chg = std::get<drivers::bluetooth::RemoteVolumeChanged>(ev.event).new_vol;
+          ev.event)) {
-        events::Ui().Dispatch(RemoteVolumeChanged{
+    auto volume_chg =
-          .value = volume_chg
+        std::get<drivers::bluetooth::RemoteVolumeChanged>(ev.event).new_vol;
-        });
+    events::Ui().Dispatch(RemoteVolumeChanged{.value = volume_chg});
   }
 }
 
@@ -354,12 +360,13 @@ namespace states {
 void Uninitialised::react(const system_fsm::BootComplete& ev) {
   sServices = ev.services;
 
-  sDrainBuffer = std::make_unique<drivers::PcmBuffer>(kDrainLatencySamples);
+  sDrainBuffers = std::make_unique<drivers::OutputBuffers>(
+      kTrackDrainLatencySamples, kSystemDrainLatencySamples);
 
   sStreamFactory.reset(new FatfsStreamFactory(*sServices));
-  sI2SOutput.reset(new I2SAudioOutput(sServices->gpios(), *sDrainBuffer));
+  sI2SOutput.reset(new I2SAudioOutput(sServices->gpios(), *sDrainBuffers));
   sBtOutput.reset(new BluetoothAudioOutput(
-      sServices->bluetooth(), *sDrainBuffer, sServices->bg_worker()));
+      sServices->bluetooth(), *sDrainBuffers, sServices->bg_worker()));
 
   auto& nvs = sServices->nvs();
   sI2SOutput->SetMaxVolume(nvs.AmpMaxVolume());
@@ -390,7 +397,7 @@ void Uninitialised::react(const system_fsm::BootComplete& ev) {
       .left_bias = nvs.AmpLeftBias(),
   });
 
-  sSampleProcessor.reset(new SampleProcessor(*sDrainBuffer));
+  sSampleProcessor.reset(new SampleProcessor(sDrainBuffers->first));
   sSampleProcessor->SetOutput(sOutput);
 
   sDecoder.reset(Decoder::Start(sSampleProcessor));
@@ -507,7 +514,7 @@ void Playback::react(const system_fsm::SdStateChanged& ev) {
 }
 
 void Playback::react(const internal::StreamHeartbeat& ev) {
-  sStreamCues.update(sDrainBuffer->totalReceived());
+  sStreamCues.update(sDrainBuffers->first.totalReceived());
 
   if (sStreamCues.hasStream()) {
     emitPlaybackUpdate(false);

src/tangara/audio/audio_fsm.hpp

@@ -81,7 +81,7 @@ class AudioState : public tinyfsm::Fsm<AudioState> {
   static std::shared_ptr<BluetoothAudioOutput> sBtOutput;
   static std::shared_ptr<IAudioOutput> sOutput;
 
-  static std::unique_ptr<drivers::PcmBuffer> sDrainBuffer;
+  static std::unique_ptr<drivers::OutputBuffers> sDrainBuffers;
 
   static StreamCues sStreamCues;
   static std::optional<IAudioOutput::Format> sDrainFormat;

src/tangara/audio/bt_audio_output.cpp

@@ -33,11 +33,11 @@ namespace audio {
 static constexpr uint16_t kVolumeRange = 60;
 
 BluetoothAudioOutput::BluetoothAudioOutput(drivers::Bluetooth& bt,
-                                           drivers::PcmBuffer& buffer,
+                                           drivers::OutputBuffers& bufs,
                                            tasks::WorkerPool& p)
     : IAudioOutput(),
       bluetooth_(bt),
-      buffer_(buffer),
+      buffers_(bufs),
       bg_worker_(p),
       volume_() {}
 
@@ -45,9 +45,9 @@ BluetoothAudioOutput::~BluetoothAudioOutput() {}
 
 auto BluetoothAudioOutput::changeMode(Modes mode) -> void {
   if (mode == Modes::kOnPlaying) {
-    bluetooth_.SetSource(&buffer_);
+    bluetooth_.SetSources(&buffers_);
   } else {
-    bluetooth_.SetSource(nullptr);
+    bluetooth_.SetSources(nullptr);
   }
 }
 

src/tangara/audio/bt_audio_output.hpp

@@ -25,7 +25,7 @@ namespace audio {
 class BluetoothAudioOutput : public IAudioOutput {
  public:
   BluetoothAudioOutput(drivers::Bluetooth& bt,
-                       drivers::PcmBuffer& buf,
+                       drivers::OutputBuffers& bufs,
                        tasks::WorkerPool&);
   ~BluetoothAudioOutput();
 
@@ -54,7 +54,7 @@ class BluetoothAudioOutput : public IAudioOutput {
 
  private:
   drivers::Bluetooth& bluetooth_;
-  drivers::PcmBuffer& buffer_;
+  drivers::OutputBuffers& buffers_;
   tasks::WorkerPool& bg_worker_;
 
   uint16_t volume_;

src/tangara/audio/i2s_audio_output.cpp

@@ -42,10 +42,10 @@ static constexpr uint16_t kLineLevelVolume = 0x13d;
 static constexpr uint16_t kDefaultVolume = 0x100;
 
 I2SAudioOutput::I2SAudioOutput(drivers::IGpios& expander,
-                               drivers::PcmBuffer& buffer)
+                               drivers::OutputBuffers& buffers)
     : IAudioOutput(),
       expander_(expander),
-      buffer_(buffer),
+      buffers_(buffers),
       dac_(),
       current_mode_(Modes::kOff),
       current_config_(),
@@ -72,7 +72,7 @@ auto I2SAudioOutput::changeMode(Modes mode) -> void {
   if (was_off) {
     // Ensure an I2SDac instance actually exists.
     if (!dac_) {
-      auto instance = drivers::I2SDac::create(expander_, buffer_);
+      auto instance = drivers::I2SDac::create(expander_, buffers_);
       if (!instance) {
         return;
       }

src/tangara/audio/i2s_audio_output.hpp

@@ -21,7 +21,7 @@ namespace audio {
 
 class I2SAudioOutput : public IAudioOutput {
  public:
-  I2SAudioOutput(drivers::IGpios&, drivers::PcmBuffer&);
+  I2SAudioOutput(drivers::IGpios&, drivers::OutputBuffers&);
 
   auto SetMaxVolume(uint16_t) -> void;
   auto SetVolumeDb(uint16_t) -> void;
@@ -51,7 +51,7 @@ class I2SAudioOutput : public IAudioOutput {
 
  private:
   drivers::IGpios& expander_;
-  drivers::PcmBuffer& buffer_;
+  drivers::OutputBuffers& buffers_;
 
   std::unique_ptr<drivers::I2SDac> dac_;