From 9fa3aa84e11b6c54c31053238c3eaad0a49580f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?= <malu.bertsch@gmail.com>
Date: Thu, 16 Dec 2021 23:18:49 +0100
Subject: [PATCH] LibAudio: Add an adjustable buffer size to FlacLoader

This makes it easier to fine-tune the optimal input buffer size.
---
 Userland/Libraries/LibAudio/FlacLoader.cpp | 14 +++++---------
 Userland/Libraries/LibAudio/FlacLoader.h   | 15 +++++++++++----
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/Userland/Libraries/LibAudio/FlacLoader.cpp b/Userland/Libraries/LibAudio/FlacLoader.cpp
index ff3774d04b..53f74d0ee0 100644
--- a/Userland/Libraries/LibAudio/FlacLoader.cpp
+++ b/Userland/Libraries/LibAudio/FlacLoader.cpp
@@ -4,12 +4,12 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include "AK/StdLibExtras.h"
 #include <AK/Debug.h>
 #include <AK/FlyString.h>
 #include <AK/Format.h>
 #include <AK/Math.h>
 #include <AK/ScopeGuard.h>
+#include <AK/StdLibExtras.h>
 #include <AK/String.h>
 #include <AK/StringBuilder.h>
 #include <AK/Try.h>
@@ -30,19 +30,15 @@ FlacLoaderPlugin::FlacLoaderPlugin(StringView path)
         return;
     }
 
-    auto maybe_stream = Core::InputFileStream::open_buffered(path);
-    if (maybe_stream.is_error()) {
-        m_error = LoaderError { "Can't open file stream" };
-        return;
-    }
-    m_stream = make<FlacInputStream>(maybe_stream.release_value());
+    auto maybe_stream = Buffered<Core::InputFileStream, FLAC_BUFFER_SIZE> { MUST(Core::File::open(path, Core::OpenMode::ReadOnly)) };
+    m_stream = make<FlacInputStream<FLAC_BUFFER_SIZE>>(move(maybe_stream));
     if (!m_stream)
         m_error = LoaderError { "Can't open file stream" };
 }
 
 FlacLoaderPlugin::FlacLoaderPlugin(const ByteBuffer& buffer)
 {
-    m_stream = make<FlacInputStream>(InputMemoryStream(buffer));
+    m_stream = make<FlacInputStream<FLAC_BUFFER_SIZE>>(InputMemoryStream(buffer));
     if (!m_stream)
         m_error = LoaderError { "Can't open memory stream" };
 }
@@ -61,7 +57,7 @@ MaybeLoaderError FlacLoaderPlugin::parse_header()
 {
     InputBitStream bit_input = [&]() -> InputBitStream {
         if (m_file) {
-            return InputBitStream(m_stream->get<Buffered<Core::InputFileStream>>());
+            return InputBitStream(m_stream->get<Buffered<Core::InputFileStream, FLAC_BUFFER_SIZE>>());
         }
         return InputBitStream(m_stream->get<InputMemoryStream>());
     }();
diff --git a/Userland/Libraries/LibAudio/FlacLoader.h b/Userland/Libraries/LibAudio/FlacLoader.h
index af8ee0608c..0f552934c3 100644
--- a/Userland/Libraries/LibAudio/FlacLoader.h
+++ b/Userland/Libraries/LibAudio/FlacLoader.h
@@ -19,15 +19,22 @@
 
 namespace Audio {
 
-class FlacInputStream : public Variant<Buffered<Core::InputFileStream>, InputMemoryStream> {
+// Experimentally determined to be a decent buffer size on i686:
+// 4K (the default) is slightly worse, and 64K is much worse.
+// At sufficiently large buffer sizes, the advantage of infrequent read() calls is outweighed by the memmove() overhead.
+// There was no intensive fine-tuning done to determine this value, so improvements may definitely be possible.
+constexpr size_t FLAC_BUFFER_SIZE = 8 * KiB;
+
+template<size_t Size = FLAC_BUFFER_SIZE>
+class FlacInputStream : public Variant<Buffered<Core::InputFileStream, Size>, InputMemoryStream> {
 
 public:
-    using Variant<Buffered<Core::InputFileStream>, InputMemoryStream>::Variant;
+    using Variant<Buffered<Core::InputFileStream, Size>, InputMemoryStream>::Variant;
 
     bool seek(size_t pos)
     {
         return this->visit(
-            [&](Buffered<Core::InputFileStream>& buffered) {
+            [&](Buffered<Core::InputFileStream, Size>& buffered) {
                 // Discard the buffer, then seek normally.
                 if (!buffered.discard_or_error(buffered.buffered()))
                     return false;
@@ -142,7 +149,7 @@ private:
 
     // keep track of the start of the data in the FLAC stream to seek back more easily
     u64 m_data_start_location { 0 };
-    OwnPtr<FlacInputStream> m_stream;
+    OwnPtr<FlacInputStream<FLAC_BUFFER_SIZE>> m_stream;
     Optional<FlacFrameHeader> m_current_frame;
     Vector<Sample> m_current_frame_data;
     u64 m_current_sample_or_frame { 0 };