From cabfd4b75ecc733bdf36997606a686c4d2bc277d Mon Sep 17 00:00:00 2001
From: jacqueline <me@jacqueline.id.au>
Date: Sun, 5 Feb 2023 14:27:06 +1100
Subject: [PATCH] fix pipeline heap corruption and chunk ignores

---
 src/audio/audio_decoder.cpp    |  2 +-
 src/audio/audio_task.cpp       |  3 +++
 src/audio/chunk.cpp            | 17 +++++++++++++----
 src/audio/i2s_audio_output.cpp |  1 +
 src/audio/include/chunk.hpp    |  7 +++++++
 5 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/audio/audio_decoder.cpp b/src/audio/audio_decoder.cpp
index b3415647..27b9cca3 100644
--- a/src/audio/audio_decoder.cpp
+++ b/src/audio/audio_decoder.cpp
@@ -40,7 +40,7 @@ auto AudioDecoder::ProcessStreamInfo(const StreamInfo& info)
   stream_info_ = info;
 
   if (info.chunk_size) {
-    chunk_reader_ = ChunkReader(info.chunk_size.value());
+    chunk_reader_.emplace(info.chunk_size.value());
   } else {
     ESP_LOGE(kTag, "no chunk size given");
     return cpp::fail(UNSUPPORTED_STREAM);
diff --git a/src/audio/audio_task.cpp b/src/audio/audio_task.cpp
index 8af3e7ff..47b115cc 100644
--- a/src/audio/audio_task.cpp
+++ b/src/audio/audio_task.cpp
@@ -151,6 +151,9 @@ void AudioTaskMain(void* args) {
             ESP_LOGE(kTag, "failed to process chunk");
             continue;
           }
+
+          // TODO: think about whether to do the whole queue
+          break;
         }
       }
     }
diff --git a/src/audio/chunk.cpp b/src/audio/chunk.cpp
index abc42bc7..61c0dc2b 100644
--- a/src/audio/chunk.cpp
+++ b/src/audio/chunk.cpp
@@ -14,10 +14,14 @@
 
 namespace audio {
 
+static const std::size_t kWorkingBufferMultiple = 2;
+
 ChunkReader::ChunkReader(std::size_t chunk_size)
     : raw_working_buffer_(static_cast<std::byte*>(
-          heap_caps_malloc(chunk_size * 2, MALLOC_CAP_SPIRAM))),
-      working_buffer_(raw_working_buffer_, chunk_size * 1.5) {}
+          heap_caps_malloc(chunk_size * kWorkingBufferMultiple,
+                           MALLOC_CAP_SPIRAM))),
+      working_buffer_(raw_working_buffer_,
+                      chunk_size * kWorkingBufferMultiple) {}
 
 ChunkReader::~ChunkReader() {
   free(raw_working_buffer_);
@@ -29,8 +33,8 @@ auto ChunkReader::HandleNewData(cpp::span<std::byte> data)
   // Copy the new data onto the front for anything that was left over from the
   // last portion. Note: this could be optimised for the '0 leftover bytes'
   // case, which technically shouldn't need a copy.
-  cpp::span<std::byte> new_data_dest = working_buffer_.subspan(leftover_bytes_);
-  std::copy(data.begin(), data.end(), new_data_dest.begin());
+  std::copy(data.begin(), data.end(),
+            working_buffer_.begin() + leftover_bytes_);
   last_data_in_working_buffer_ =
       working_buffer_.first(leftover_bytes_ + data.size());
   leftover_bytes_ = 0;
@@ -39,6 +43,11 @@ auto ChunkReader::HandleNewData(cpp::span<std::byte> data)
 
 auto ChunkReader::HandleLeftovers(std::size_t bytes_used) -> void {
   leftover_bytes_ = last_data_in_working_buffer_.size() - bytes_used;
+
+  // Ensure that we don't have more than a chunk of leftever bytes. This is
+  // bad, because we probably won't have enough data to store the next chunk.
+  assert(leftover_bytes_ <= working_buffer_.size() / kWorkingBufferMultiple);
+
   if (leftover_bytes_ > 0) {
     auto data_to_keep = last_data_in_working_buffer_.last(leftover_bytes_);
     std::copy(data_to_keep.begin(), data_to_keep.end(),
diff --git a/src/audio/i2s_audio_output.cpp b/src/audio/i2s_audio_output.cpp
index ca943904..b00e31d3 100644
--- a/src/audio/i2s_audio_output.cpp
+++ b/src/audio/i2s_audio_output.cpp
@@ -92,6 +92,7 @@ auto I2SAudioOutput::ProcessStreamInfo(const StreamInfo& info)
 
 auto I2SAudioOutput::ProcessChunk(const cpp::span<std::byte>& chunk)
     -> cpp::result<std::size_t, AudioProcessingError> {
+  ESP_LOGI(kTag, "playing samples");
   SetSoftMute(false);
   // TODO(jacqueline): write smaller parts with a small delay so that we can
   // be responsive to pause and seek commands.
diff --git a/src/audio/include/chunk.hpp b/src/audio/include/chunk.hpp
index 6154ab25..0ece1ed6 100644
--- a/src/audio/include/chunk.hpp
+++ b/src/audio/include/chunk.hpp
@@ -18,6 +18,10 @@
 
 namespace audio {
 
+/**
+ * Utility for handling an input stream of chunk data, which simplifies needing
+ * access to blocks of data spanning two chunks.
+ */
 class ChunkReader {
  public:
   explicit ChunkReader(std::size_t chunk_size);
@@ -39,6 +43,9 @@ class ChunkReader {
    */
   auto HandleNewData(cpp::span<std::byte> data) -> cpp::span<std::byte>;
 
+  ChunkReader(const ChunkReader&) = delete;
+  ChunkReader& operator=(const ChunkReader&) = delete;
+
  private:
   std::byte* raw_working_buffer_;
   cpp::span<std::byte> working_buffer_;