From b3b512f10e0570f7dc8a04e1613f1234e5532728 Mon Sep 17 00:00:00 2001
From: jacqueline <me@jacqueline.id.au>
Date: Mon, 20 Nov 2023 11:04:20 +1100
Subject: [PATCH] Don't start readahead until parsing the stream's header is
 finished

...but also disable readahead for now anyway, since it's unstable
---
 src/audio/audio_decoder.cpp            |  1 +
 src/audio/fatfs_audio_input.cpp        |  3 +-
 src/audio/include/readahead_source.hpp |  5 +++
 src/audio/readahead_source.cpp         | 61 +++++++++++++++-----------
 src/codecs/include/codec.hpp           |  9 ++++
 5 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/src/audio/audio_decoder.cpp b/src/audio/audio_decoder.cpp
index 03402cfc..a58268b0 100644
--- a/src/audio/audio_decoder.cpp
+++ b/src/audio/audio_decoder.cpp
@@ -133,6 +133,7 @@ auto Decoder::BeginDecoding(std::shared_ptr<codecs::IStream> stream) -> bool {
              codecs::ICodec::ErrorString(open_res.error()).c_str());
     return false;
   }
+  stream->SetPreambleFinished();
 
   if (open_res->total_samples) {
     timer_.reset(new Timer(open_res.value()));
diff --git a/src/audio/fatfs_audio_input.cpp b/src/audio/fatfs_audio_input.cpp
index e13ae793..6580f301 100644
--- a/src/audio/fatfs_audio_input.cpp
+++ b/src/audio/fatfs_audio_input.cpp
@@ -147,7 +147,8 @@ auto FatfsAudioInput::OpenFile(const std::pmr::string& path) -> bool {
 
   auto source =
       std::make_unique<FatfsSource>(stream_type.value(), std::move(file));
-  new_stream_.reset(new ReadaheadSource(bg_worker_, std::move(source)));
+  // new_stream_.reset(new ReadaheadSource(bg_worker_, std::move(source)));
+  new_stream_ = std::move(source);
   return true;
 }
 
diff --git a/src/audio/include/readahead_source.hpp b/src/audio/include/readahead_source.hpp
index dea3ff3f..bbc0da12 100644
--- a/src/audio/include/readahead_source.hpp
+++ b/src/audio/include/readahead_source.hpp
@@ -38,13 +38,18 @@ class ReadaheadSource : public codecs::IStream {
 
   auto CurrentPosition() -> int64_t override;
 
+  auto SetPreambleFinished() -> void override;
+
   ReadaheadSource(const ReadaheadSource&) = delete;
   ReadaheadSource& operator=(const ReadaheadSource&) = delete;
 
  private:
+  auto BeginReadahead() -> void;
+
   tasks::Worker& worker_;
   std::unique_ptr<codecs::IStream> wrapped_;
 
+  bool readahead_enabled_;
   std::atomic<bool> is_refilling_;
   StreamBufferHandle_t buffer_;
   int64_t tell_;
diff --git a/src/audio/readahead_source.cpp b/src/audio/readahead_source.cpp
index 9ef6c43f..41d38eee 100644
--- a/src/audio/readahead_source.cpp
+++ b/src/audio/readahead_source.cpp
@@ -32,6 +32,7 @@ ReadaheadSource::ReadaheadSource(tasks::Worker& worker,
     : IStream(wrapped->type()),
       worker_(worker),
       wrapped_(std::move(wrapped)),
+      readahead_enabled_(false),
       is_refilling_(false),
       buffer_(xStreamBufferCreateWithCaps(kBufferSize, 1, MALLOC_CAP_SPIRAM)),
       tell_(wrapped_->CurrentPosition()) {}
@@ -71,32 +72,9 @@ auto ReadaheadSource::Read(cpp::span<std::byte> dest) -> ssize_t {
 
   // If we're here, then there is more data to be read from the wrapped stream.
   // Ensure the readahead is running.
-  if (!is_refilling_ &&
+  if (!is_refilling_ && readahead_enabled_ &&
       xStreamBufferBytesAvailable(buffer_) < kBufferSize / 4) {
-    is_refilling_ = true;
-    std::function<void(void)> refill = [this]() {
-      // Try to keep larger than most reasonable FAT sector sizes for more
-      // efficient disk reads.
-      constexpr size_t kMaxSingleRead = 1024 * 16;
-      std::byte working_buf[kMaxSingleRead];
-      for (;;) {
-        size_t bytes_to_read = std::min<size_t>(
-            kMaxSingleRead, xStreamBufferSpacesAvailable(buffer_));
-        if (bytes_to_read == 0) {
-          break;
-        }
-        size_t read = wrapped_->Read({working_buf, bytes_to_read});
-        if (read > 0) {
-          xStreamBufferSend(buffer_, working_buf, read, 0);
-        }
-        if (read < bytes_to_read) {
-          break;
-        }
-      }
-      is_refilling_ = false;
-      is_refilling_.notify_all();
-    };
-    worker_.Dispatch(refill);
+    BeginReadahead();
   }
 
   return bytes_written;
@@ -123,4 +101,37 @@ auto ReadaheadSource::SeekTo(int64_t destination, SeekFrom from) -> void {
 auto ReadaheadSource::CurrentPosition() -> int64_t {
   return tell_;
 }
+
+auto ReadaheadSource::SetPreambleFinished() -> void {
+  readahead_enabled_ = true;
+  BeginReadahead();
+}
+
+auto ReadaheadSource::BeginReadahead() -> void {
+  is_refilling_ = true;
+  std::function<void(void)> refill = [this]() {
+    // Try to keep larger than most reasonable FAT sector sizes for more
+    // efficient disk reads.
+    constexpr size_t kMaxSingleRead = 1024 * 16;
+    std::byte working_buf[kMaxSingleRead];
+    for (;;) {
+      size_t bytes_to_read = std::min<size_t>(
+          kMaxSingleRead, xStreamBufferSpacesAvailable(buffer_));
+      if (bytes_to_read == 0) {
+        break;
+      }
+      size_t read = wrapped_->Read({working_buf, bytes_to_read});
+      if (read > 0) {
+        xStreamBufferSend(buffer_, working_buf, read, 0);
+      }
+      if (read < bytes_to_read) {
+        break;
+      }
+    }
+    is_refilling_ = false;
+    is_refilling_.notify_all();
+  };
+  worker_.Dispatch(refill);
+}
+
 }  // namespace audio
diff --git a/src/codecs/include/codec.hpp b/src/codecs/include/codec.hpp
index 67358d54..87f6637c 100644
--- a/src/codecs/include/codec.hpp
+++ b/src/codecs/include/codec.hpp
@@ -49,6 +49,15 @@ class IStream {
 
   virtual auto CurrentPosition() -> int64_t = 0;
 
+  /*
+   * Called by codecs to indicate that they've finished parsing any header data
+   * within this stream, and are about to begin decoding.
+   * 
+   * Currently used as a hint to the readahead stream to begin prefetching file
+   * data.
+   */
+  virtual auto SetPreambleFinished() -> void {}
+
  protected:
   StreamType t_;
 };