Clear the drain buffer when skipping between tracks

10 files changed, 106 insertions, 39 deletions

diff --git a/src/audio/audio_fsm.cpp b/src/audio/audio_fsm.cpp
index ea0315eb..08a0941a 100644
--- a/src/audio/audio_fsm.cpp
+++ b/src/audio/audio_fsm.cpp
@@ -51,6 +51,10 @@ std::shared_ptr<I2SAudioOutput> AudioState::sI2SOutput;
 std::shared_ptr<BluetoothAudioOutput> AudioState::sBtOutput;
 std::shared_ptr<IAudioOutput> AudioState::sOutput;
 
+// Two seconds of samples for two channels, at a representative sample rate.
+constexpr size_t kDrainBufferSize = sizeof(sample::Sample) * 48000 * 4;
+StreamBufferHandle_t AudioState::sDrainBuffer;
+
 std::optional<database::TrackId> AudioState::sCurrentTrack;
 bool AudioState::sIsPlaybackAllowed;
 
@@ -129,7 +133,7 @@ void AudioState::react(const SetVolumeBalance& ev) {
 void AudioState::react(const OutputModeChanged& ev) {
   ESP_LOGI(kTag, "output mode changed");
   auto new_mode = sServices->nvs().OutputMode();
-  sOutput->SetMode(IAudioOutput::Modes::kOff);
+  sOutput->mode(IAudioOutput::Modes::kOff);
   switch (new_mode) {
     case drivers::NvsStorage::Output::kBluetooth:
       sOutput = sBtOutput;
@@ -138,7 +142,7 @@ void AudioState::react(const OutputModeChanged& ev) {
       sOutput = sI2SOutput;
       break;
   }
-  sOutput->SetMode(IAudioOutput::Modes::kOnPaused);
+  sOutput->mode(IAudioOutput::Modes::kOnPaused);
   sSampleConverter->SetOutput(sOutput);
 
   // Bluetooth volume isn't 'changed' until we've connected to a device.
@@ -150,6 +154,32 @@ void AudioState::react(const OutputModeChanged& ev) {
   }
 }
 
+auto AudioState::clearDrainBuffer() -> void {
+  // Tell the decoder to stop adding new samples. This might not take effect
+  // immediately, since the decoder might currently be stuck waiting for space
+  // to become available in the drain buffer.
+  sFileSource->SetPath();
+
+  auto mode = sOutput->mode();
+  if (mode == IAudioOutput::Modes::kOnPlaying) {
+    // If we're currently playing, then the drain buffer will be actively
+    // draining on its own. Just keep trying to reset until it works.
+    while (xStreamBufferReset(sDrainBuffer) != pdPASS) {
+    }
+  } else {
+    // If we're not currently playing, then we need to actively pull samples
+    // out of the drain buffer to unblock the decoder.
+    while (!xStreamBufferIsEmpty(sDrainBuffer)) {
+      // Read a little to unblock the decoder.
+      uint8_t drain[2048];
+      xStreamBufferReceive(sDrainBuffer, drain, sizeof(drain), 0);
+
+      // Try to quickly discard the rest.
+      xStreamBufferReset(sDrainBuffer);
+    }
+  }
+}
+
 auto AudioState::playTrack(database::TrackId id) -> void {
   sCurrentTrack = id;
   sServices->bg_worker().Dispatch<void>([=]() {
@@ -194,10 +224,6 @@ void AudioState::react(const TogglePlayPause& ev) {
 
 namespace states {
 
-// Two seconds of samples for two channels, at a representative sample rate.
-constexpr size_t kDrainBufferSize = sizeof(sample::Sample) * 48000 * 4;
-static StreamBufferHandle_t sDrainBuffer;
-
 void Uninitialised::react(const system_fsm::BootComplete& ev) {
   sServices = ev.services;
 
@@ -229,7 +255,7 @@ void Uninitialised::react(const system_fsm::BootComplete& ev) {
   } else {
     sOutput = sBtOutput;
   }
-  sOutput->SetMode(IAudioOutput::Modes::kOnPaused);
+  sOutput->mode(IAudioOutput::Modes::kOnPaused);
 
   events::Ui().Dispatch(VolumeLimitChanged{
       .new_limit_db =
@@ -272,6 +298,7 @@ void Standby::react(const QueueUpdate& ev) {
   if (!current_track || (sCurrentTrack && (*sCurrentTrack == *current_track))) {
     return;
   }
+  clearDrainBuffer();
   playTrack(*current_track);
 }
 
@@ -315,7 +342,7 @@ void Standby::react(const system_fsm::StorageMounted& ev) {
 
 void Playback::entry() {
   ESP_LOGI(kTag, "beginning playback");
-  sOutput->SetMode(IAudioOutput::Modes::kOnPlaying);
+  sOutput->mode(IAudioOutput::Modes::kOnPlaying);
 
   events::System().Dispatch(PlaybackStarted{});
   events::Ui().Dispatch(PlaybackStarted{});
@@ -323,10 +350,10 @@ void Playback::entry() {
 
 void Playback::exit() {
   ESP_LOGI(kTag, "finishing playback");
-  sOutput->SetMode(IAudioOutput::Modes::kOnPaused);
+  sOutput->mode(IAudioOutput::Modes::kOnPaused);
 
-  // Stash the current volume now, in case it changed during playback, since we
-  // might be powering off soon.
+  // Stash the current volume now, in case it changed during playback, since
+  // we might be powering off soon.
   commitVolume();
 
   events::System().Dispatch(PlaybackStopped{});
@@ -343,6 +370,10 @@ void Playback::react(const QueueUpdate& ev) {
   if (!ev.current_changed) {
     return;
   }
+  // Cut the current track immediately.
+  if (ev.reason == QueueUpdate::Reason::kExplicitUpdate) {
+    clearDrainBuffer();
+  }
   auto current_track = sServices->track_queue().current();
   if (!current_track) {
     sFileSource->SetPath();
diff --git a/src/audio/bt_audio_output.cpp b/src/audio/bt_audio_output.cpp
index 41c89069..dff98e36 100644
--- a/src/audio/bt_audio_output.cpp
+++ b/src/audio/bt_audio_output.cpp
@@ -35,7 +35,7 @@ BluetoothAudioOutput::BluetoothAudioOutput(StreamBufferHandle_t s,
 
 BluetoothAudioOutput::~BluetoothAudioOutput() {}
 
-auto BluetoothAudioOutput::SetMode(Modes mode) -> void {
+auto BluetoothAudioOutput::changeMode(Modes mode) -> void {
   if (mode == Modes::kOnPlaying) {
     bluetooth_.SetSource(stream());
   } else {
diff --git a/src/audio/i2s_audio_output.cpp b/src/audio/i2s_audio_output.cpp
index 4043574e..cd61d97f 100644
--- a/src/audio/i2s_audio_output.cpp
+++ b/src/audio/i2s_audio_output.cpp
@@ -58,7 +58,7 @@ I2SAudioOutput::~I2SAudioOutput() {
   dac_->SetSource(nullptr);
 }
 
-auto I2SAudioOutput::SetMode(Modes mode) -> void {
+auto I2SAudioOutput::changeMode(Modes mode) -> void {
   if (mode == current_mode_) {
     return;
   }
diff --git a/src/audio/include/audio_events.hpp b/src/audio/include/audio_events.hpp
index 03584062..a79ca4ec 100644
--- a/src/audio/include/audio_events.hpp
+++ b/src/audio/include/audio_events.hpp
@@ -14,7 +14,6 @@
 #include "tinyfsm.hpp"
 
 #include "track.hpp"
-#include "track_queue.hpp"
 #include "types.hpp"
 
 namespace audio {
@@ -39,6 +38,13 @@ struct PlaybackStopped : tinyfsm::Event {};
 
 struct QueueUpdate : tinyfsm::Event {
   bool current_changed;
+
+  enum Reason {
+    kExplicitUpdate,
+    kRepeatingLastTrack,
+    kTrackFinished,
+  };
+  Reason reason;
 };
 
 struct PlayFile : tinyfsm::Event {
diff --git a/src/audio/include/audio_fsm.hpp b/src/audio/include/audio_fsm.hpp
index 29ec489a..2d335e74 100644
--- a/src/audio/include/audio_fsm.hpp
+++ b/src/audio/include/audio_fsm.hpp
@@ -52,7 +52,7 @@ class AudioState : public tinyfsm::Fsm<AudioState> {
   void react(const OutputModeChanged&);
 
   virtual void react(const system_fsm::BootComplete&) {}
-  virtual void react(const system_fsm::KeyLockChanged&) {};
+  virtual void react(const system_fsm::KeyLockChanged&){};
   virtual void react(const system_fsm::StorageMounted&) {}
   virtual void react(const system_fsm::BluetoothEvent&);
 
@@ -67,6 +67,7 @@ class AudioState : public tinyfsm::Fsm<AudioState> {
   virtual void react(const internal::AudioPipelineIdle&) {}
 
  protected:
+  auto clearDrainBuffer() -> void;
   auto playTrack(database::TrackId id) -> void;
   auto commitVolume() -> void;
 
@@ -79,6 +80,8 @@ class AudioState : public tinyfsm::Fsm<AudioState> {
   static std::shared_ptr<BluetoothAudioOutput> sBtOutput;
   static std::shared_ptr<IAudioOutput> sOutput;
 
+  static StreamBufferHandle_t sDrainBuffer;
+
   static std::optional<database::TrackId> sCurrentTrack;
 
   auto readyToPlay() -> bool;
@@ -91,7 +94,7 @@ class Uninitialised : public AudioState {
  public:
   void react(const system_fsm::BootComplete&) override;
 
-  void react(const system_fsm::BluetoothEvent&) override {};
+  void react(const system_fsm::BluetoothEvent&) override{};
 
   using AudioState::react;
 };
diff --git a/src/audio/include/audio_sink.hpp b/src/audio/include/audio_sink.hpp
index eba55eb5..85c23f5c 100644
--- a/src/audio/include/audio_sink.hpp
+++ b/src/audio/include/audio_sink.hpp
@@ -27,7 +27,8 @@ class IAudioOutput {
   StreamBufferHandle_t stream_;
 
  public:
-  IAudioOutput(StreamBufferHandle_t stream) : stream_(stream) {}
+  IAudioOutput(StreamBufferHandle_t stream)
+      : stream_(stream), mode_(Modes::kOff) {}
 
   virtual ~IAudioOutput() {}
 
@@ -41,7 +42,14 @@ class IAudioOutput {
    * Indicates whether this output is currently being sent samples. If this is
    * false, the output should place itself into a low power state.
    */
-  virtual auto SetMode(Modes) -> void = 0;
+  auto mode(Modes m) -> void {
+    if (mode_ == m) {
+      return;
+    }
+    changeMode(m);
+    mode_ = m;
+  }
+  auto mode() -> Modes { return mode_; }
 
   virtual auto SetVolumeImbalance(int_fast8_t balance) -> void = 0;
 
@@ -67,6 +75,11 @@ class IAudioOutput {
   virtual auto Configure(const Format& format) -> void = 0;
 
   auto stream() -> StreamBufferHandle_t { return stream_; }
+
+ protected:
+  Modes mode_;
+
+  virtual auto changeMode(Modes new_mode) -> void = 0;
 };
 
 }  // namespace audio
diff --git a/src/audio/include/bt_audio_output.hpp b/src/audio/include/bt_audio_output.hpp
index f6d2200c..a61e718a 100644
--- a/src/audio/include/bt_audio_output.hpp
+++ b/src/audio/include/bt_audio_output.hpp
@@ -28,8 +28,6 @@ class BluetoothAudioOutput : public IAudioOutput {
                        tasks::WorkerPool&);
   ~BluetoothAudioOutput();
 
-  auto SetMode(Modes) -> void override;
-
   auto SetVolumeImbalance(int_fast8_t balance) -> void override;
 
   auto SetVolume(uint16_t) -> void override;
@@ -48,6 +46,9 @@ class BluetoothAudioOutput : public IAudioOutput {
   BluetoothAudioOutput(const BluetoothAudioOutput&) = delete;
   BluetoothAudioOutput& operator=(const BluetoothAudioOutput&) = delete;
 
+ protected:
+  auto changeMode(Modes) -> void override;
+
  private:
   drivers::Bluetooth& bluetooth_;
   tasks::WorkerPool& bg_worker_;
diff --git a/src/audio/include/i2s_audio_output.hpp b/src/audio/include/i2s_audio_output.hpp
index 7c297106..5f3fc3ff 100644
--- a/src/audio/include/i2s_audio_output.hpp
+++ b/src/audio/include/i2s_audio_output.hpp
@@ -23,8 +23,6 @@ class I2SAudioOutput : public IAudioOutput {
   I2SAudioOutput(StreamBufferHandle_t, drivers::IGpios& expander);
   ~I2SAudioOutput();
 
-  auto SetMode(Modes) -> void override;
-
   auto SetMaxVolume(uint16_t) -> void;
   auto SetVolumeDb(uint16_t) -> void;
 
@@ -46,6 +44,9 @@ class I2SAudioOutput : public IAudioOutput {
   I2SAudioOutput(const I2SAudioOutput&) = delete;
   I2SAudioOutput& operator=(const I2SAudioOutput&) = delete;
 
+ protected:
+  auto changeMode(Modes) -> void override;
+
  private:
   drivers::IGpios& expander_;
   std::unique_ptr<drivers::I2SDac> dac_;
diff --git a/src/audio/include/track_queue.hpp b/src/audio/include/track_queue.hpp
index e4fd7881..5b7c9448 100644
--- a/src/audio/include/track_queue.hpp
+++ b/src/audio/include/track_queue.hpp
@@ -12,6 +12,7 @@
 #include <shared_mutex>
 #include <vector>
 
+#include "audio_events.hpp"
 #include "cppbor_parse.h"
 #include "database.hpp"
 #include "tasks.hpp"
@@ -120,6 +121,8 @@ class TrackQueue {
   TrackQueue& operator=(const TrackQueue&) = delete;
 
  private:
+  auto next(QueueUpdate::Reason r) -> void;
+
   mutable std::shared_mutex mutex_;
 
   tasks::WorkerPool& bg_worker_;
diff --git a/src/audio/track_queue.cpp b/src/audio/track_queue.cpp
index b75230fc..ccadd3a6 100644
--- a/src/audio/track_queue.cpp
+++ b/src/audio/track_queue.cpp
@@ -33,6 +33,8 @@ namespace audio {
 
 [[maybe_unused]] static constexpr char kTag[] = "tracks";
 
+using Reason = QueueUpdate::Reason;
+
 RandomIterator::RandomIterator()
     : seed_(0), pos_(0), size_(0), replay_(false) {}
 
@@ -72,8 +74,11 @@ auto RandomIterator::replay(bool r) -> void {
   replay_ = r;
 }
 
-auto notifyChanged(bool current_changed) -> void {
-  QueueUpdate ev{.current_changed = current_changed};
+auto notifyChanged(bool current_changed, Reason reason) -> void {
+  QueueUpdate ev{
+      .current_changed = current_changed,
+      .reason = reason,
+  };
   events::Ui().Dispatch(ev);
   events::Audio().Dispatch(ev);
 }
@@ -157,7 +162,7 @@ auto TrackQueue::insert(Item i, size_t index) -> void {
         update_shuffler();
       }
     }
-    notifyChanged(current_changed);
+    notifyChanged(current_changed, Reason::kExplicitUpdate);
   } else if (std::holds_alternative<database::TrackIterator>(i)) {
     // Iterators can be very large, and retrieving items from them often
     // requires disk i/o. Handle them asynchronously so that inserting them
@@ -185,7 +190,7 @@ auto TrackQueue::insert(Item i, size_t index) -> void {
         const std::unique_lock<std::shared_mutex> lock(mutex_);
         update_shuffler();
       }
-      notifyChanged(current_changed);
+      notifyChanged(current_changed, Reason::kExplicitUpdate);
     });
   }
 }
@@ -200,6 +205,10 @@ auto TrackQueue::append(Item i) -> void {
 }
 
 auto TrackQueue::next() -> void {
+  next(Reason::kExplicitUpdate);
+}
+
+auto TrackQueue::next(Reason r) -> void {
   bool changed = true;
 
   {
@@ -221,7 +230,7 @@ auto TrackQueue::next() -> void {
     }
   }
 
-  notifyChanged(changed);
+  notifyChanged(changed, r);
 }
 
 auto TrackQueue::previous() -> void {
@@ -245,22 +254,22 @@ auto TrackQueue::previous() -> void {
     }
   }
 
-  notifyChanged(changed);
+  notifyChanged(changed, Reason::kExplicitUpdate);
 }
 
 auto TrackQueue::finish() -> void {
   if (repeat_) {
-    notifyChanged(true);
+    notifyChanged(true, Reason::kRepeatingLastTrack);
   } else {
-    next();
+    next(Reason::kTrackFinished);
   }
 }
 
 auto TrackQueue::skipTo(database::TrackId id) -> void {
   // Defer this work to the background not because it's particularly
-  // long-running (although it could be), but because we want to ensure we only
-  // search for the given id after any previously pending iterator insertions
-  // have finished.
+  // long-running (although it could be), but because we want to ensure we
+  // only search for the given id after any previously pending iterator
+  // insertions have finished.
   bg_worker_.Dispatch<void>([=, this]() {
     bool found = false;
     {
@@ -274,7 +283,7 @@ auto TrackQueue::skipTo(database::TrackId id) -> void {
       }
     }
     if (found) {
-      notifyChanged(true);
+      notifyChanged(true, Reason::kExplicitUpdate);
     }
   });
 }
@@ -294,7 +303,7 @@ auto TrackQueue::clear() -> void {
     }
   }
 
-  notifyChanged(true);
+  notifyChanged(true, Reason::kExplicitUpdate);
 }
 
 auto TrackQueue::random(bool en) -> void {
@@ -311,7 +320,7 @@ auto TrackQueue::random(bool en) -> void {
   }
 
   // Current track doesn't get randomised until next().
-  notifyChanged(false);
+  notifyChanged(false, Reason::kExplicitUpdate);
 }
 
 auto TrackQueue::random() const -> bool {
@@ -325,7 +334,7 @@ auto TrackQueue::repeat(bool en) -> void {
     repeat_ = en;
   }
 
-  notifyChanged(false);
+  notifyChanged(false, Reason::kExplicitUpdate);
 }
 
 auto TrackQueue::repeat() const -> bool {
@@ -341,7 +350,7 @@ auto TrackQueue::replay(bool en) -> void {
       shuffle_->replay(en);
     }
   }
-  notifyChanged(false);
+  notifyChanged(false, Reason::kExplicitUpdate);
 }
 
 auto TrackQueue::replay() const -> bool {
@@ -477,7 +486,7 @@ auto TrackQueue::deserialise(const std::string& s) -> void {
   QueueParseClient client{*this};
   const uint8_t* data = reinterpret_cast<const uint8_t*>(s.data());
   cppbor::parse(data, data + s.size(), &client);
-  notifyChanged(true);
+  notifyChanged(true, Reason::kExplicitUpdate);
 }
 
 }  // namespace audio