LibAudio: Move audio stream buffering into the loader

Before, some loader plugins implemented their own buffering (FLAC&MP3),
some didn't require any (WAV), and some didn't buffer at all (QOA). This
meant that in practice, while you could load arbitrary amounts of
samples from some loader plugins, you couldn't do that with some others.
Also, it was ill-defined how many samples you would actually get back
from a get_more_samples call.

This commit fixes that by introducing a layer of abstraction between the
loader and its plugins (because that's the whole point of having the
extra class!). The plugins now only implement a load_chunks() function,
which is much simpler to implement and allows plugins to play fast and
loose with what they actually return. Basically, they can return many
chunks of samples, where one chunk is simply a convenient block of
samples to load. In fact, some loaders such as FLAC and QOA have
separate internal functions for loading exactly one chunk. The loaders
*should* load as many chunks as necessary for the sample count to be
reached or surpassed (the latter simplifies loading loops in the
implementations, since you don't need to know how large your next chunk
is going to be; a problem for e.g. FLAC). If a plugin has no problems
returning data of arbitrary size (currently WAV), it can return a single
chunk that exactly (or roughly) matches the requested sample count. If a
plugin is at the stream end, it can also return less samples than was
requested! The loader can handle all of these cases and may call into
load_chunk multiple times. If the plugin returns an empty chunk list (or
only empty chunks; again, they can play fast and loose), the loader
takes that as a stream end signal. Otherwise, the loader will always
return exactly as many samples as the user requested. Buffering is
handled by the loader, allowing any underlying plugin to deal with any
weird sample count requirement the user throws at it (looking at you,
SoundPlayer!).

This (not accidentally!) makes QOA work in SoundPlayer.

Meta/Lagom/Fuzzers/FuzzFlacLoader.cpp

@@ -19,7 +19,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
     auto flac = flac_or_error.release_value();
 
     for (;;) {
-        auto samples = flac->get_more_samples();
+        auto samples = flac->load_chunks(10 * KiB);
         if (samples.is_error())
             return 0;
         if (samples.value().size() > 0)

Meta/Lagom/Fuzzers/FuzzMP3Loader.cpp

@@ -19,7 +19,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
     auto mp3 = mp3_or_error.release_value();
 
     for (;;) {
-        auto samples = mp3->get_more_samples();
+        auto samples = mp3->load_chunks(1 * KiB);
         if (samples.is_error())
             return 0;
         if (samples.value().size() > 0)

Meta/Lagom/Fuzzers/FuzzQOALoader.cpp

@@ -19,7 +19,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
     auto qoa = qoa_or_error.release_value();
 
     for (;;) {
-        auto samples = qoa->get_more_samples();
+        auto samples = qoa->load_chunks(5 * KiB);
         if (samples.is_error())
             return 0;
         if (samples.value().size() > 0)

Meta/Lagom/Fuzzers/FuzzWAVLoader.cpp

@@ -22,7 +22,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
     auto wav = wav_or_error.release_value();
 
     for (;;) {
-        auto samples = wav->get_more_samples();
+        auto samples = wav->load_chunks(4 * KiB);
         if (samples.is_error())
             return 0;
         if (samples.value().size() > 0)

Tests/LibAudio/TestFLACSpec.cpp

@@ -28,11 +28,12 @@ struct FlacTest : Test::TestCase {
         auto loader = result.release_value();
 
         while (true) {
-            auto maybe_samples = loader->get_more_samples(2 * MiB);
+            auto maybe_samples = loader->load_chunks(2 * MiB);
             if (maybe_samples.is_error()) {
                 FAIL(DeprecatedString::formatted("{} (at {})", maybe_samples.error().description, maybe_samples.error().index));
                 return;
             }
+            maybe_samples.value().remove_all_matching([](auto& chunk) { return chunk.is_empty(); });
             if (maybe_samples.value().is_empty())
                 return;
         }

Userland/Libraries/LibAudio/FlacLoader.cpp

@@ -266,14 +266,14 @@ MaybeLoaderError FlacLoaderPlugin::seek(int int_sample_index)
         if (to_read == 0)
             return {};
         dbgln_if(AFLACLOADER_DEBUG, "Seeking {} samples manually", to_read);
-        (void)TRY(get_more_samples(to_read));
+        (void)TRY(load_chunks(to_read));
     } else {
         auto target_seekpoint = maybe_target_seekpoint.release_value();
 
         // When a small seek happens, we may already be closer to the target than the seekpoint.
         if (sample_index - target_seekpoint.sample_index > sample_index - m_loaded_samples) {
             dbgln_if(AFLACLOADER_DEBUG, "Close enough to target: seeking {} samples manually", sample_index - m_loaded_samples);
-            (void)TRY(get_more_samples(sample_index - m_loaded_samples));
+            (void)TRY(load_chunks(sample_index - m_loaded_samples));
             return {};
         }
 
@@ -284,47 +284,34 @@ MaybeLoaderError FlacLoaderPlugin::seek(int int_sample_index)
 
         auto remaining_samples_after_seekpoint = sample_index - m_data_start_location;
         if (remaining_samples_after_seekpoint > 0)
-            (void)TRY(get_more_samples(remaining_samples_after_seekpoint));
+            (void)TRY(load_chunks(remaining_samples_after_seekpoint));
         m_loaded_samples = target_seekpoint.sample_index;
     }
     return {};
 }
 
-LoaderSamples FlacLoaderPlugin::get_more_samples(size_t max_bytes_to_read_from_input)
+ErrorOr<Vector<FixedArray<Sample>>, LoaderError> FlacLoaderPlugin::load_chunks(size_t samples_to_read_from_input)
 {
     ssize_t remaining_samples = static_cast<ssize_t>(m_total_samples - m_loaded_samples);
     if (remaining_samples <= 0)
-        return FixedArray<Sample> {};
+        return Vector<FixedArray<Sample>> {};
 
-    // FIXME: samples_to_read is calculated wrong, because when seeking not all samples are loaded.
-    size_t samples_to_read = min(max_bytes_to_read_from_input, remaining_samples);
-    auto samples = FixedArray<Sample>::must_create_but_fixme_should_propagate_errors(samples_to_read);
+    size_t samples_to_read = min(samples_to_read_from_input, remaining_samples);
+    Vector<FixedArray<Sample>> frames;
     size_t sample_index = 0;
 
-    if (m_unread_data.size() > 0) {
-        size_t to_transfer = min(m_unread_data.size(), samples_to_read);
-        dbgln_if(AFLACLOADER_DEBUG, "Reading {} samples from unread sample buffer (size {})", to_transfer, m_unread_data.size());
-        AK::TypedTransfer<Sample>::move(samples.data(), m_unread_data.data(), to_transfer);
-        if (to_transfer < m_unread_data.size())
-            m_unread_data.remove(0, to_transfer);
-        else
-            m_unread_data.clear_with_capacity();
-
-        sample_index += to_transfer;
-    }
-
     while (sample_index < samples_to_read) {
-        TRY(next_frame(samples.span().slice(sample_index)));
+        TRY(frames.try_append(TRY(next_frame())));
         sample_index += m_current_frame->sample_count;
     }
 
     m_loaded_samples += sample_index;
 
-    return samples;
+    return frames;
 }
 
 // 11.21. FRAME
-MaybeLoaderError FlacLoaderPlugin::next_frame(Span<Sample> target_vector)
+LoaderSamples FlacLoaderPlugin::next_frame()
 {
 #define FLAC_VERIFY(check, category, msg)                                                                                                         \
     do {                                                                                                                                          \
@@ -399,6 +386,7 @@ MaybeLoaderError FlacLoaderPlugin::next_frame(Span<Sample> target_vector)
     for (u8 i = 0; i < subframe_count; ++i) {
         FlacSubframeHeader new_subframe = TRY(next_subframe_header(bit_stream, i));
         Vector<i32> subframe_samples = TRY(parse_subframe(new_subframe, bit_stream));
+        VERIFY(subframe_samples.size() == m_current_frame->sample_count);
         current_subframes.unchecked_append(move(subframe_samples));
     }
 
@@ -410,12 +398,15 @@ MaybeLoaderError FlacLoaderPlugin::next_frame(Span<Sample> target_vector)
     [[maybe_unused]] u16 footer_checksum = LOADER_TRY(bit_stream.read_bits<u16>(16));
     dbgln_if(AFLACLOADER_DEBUG, "Subframe footer checksum: {}", footer_checksum);
 
-    Vector<i32> left;
-    Vector<i32> right;
+    float sample_rescale = 1 / static_cast<float>(1 << (pcm_bits_per_sample(m_current_frame->bit_depth) - 1));
+    dbgln_if(AFLACLOADER_DEBUG, "Sample rescaled from {} bits: factor {:.1f}", pcm_bits_per_sample(m_current_frame->bit_depth), sample_rescale);
+
+    FixedArray<Sample> samples = TRY(FixedArray<Sample>::create(m_current_frame->sample_count));
 
     switch (channel_type) {
     case FlacFrameChannelType::Mono:
-        left = right = current_subframes[0];
+        for (size_t i = 0; i < m_current_frame->sample_count; ++i)
+            samples[i] = Sample { static_cast<float>(current_subframes[0][i]) * sample_rescale };
         break;
     case FlacFrameChannelType::Stereo:
     // TODO mix together surround channels on each side?
@@ -425,64 +416,39 @@ MaybeLoaderError FlacLoaderPlugin::next_frame(Span<Sample> target_vector)
     case FlacFrameChannelType::Surround5p1:
     case FlacFrameChannelType::Surround6p1:
     case FlacFrameChannelType::Surround7p1:
-        left = current_subframes[0];
-        right = current_subframes[1];
+        for (size_t i = 0; i < m_current_frame->sample_count; ++i)
+            samples[i] = { static_cast<float>(current_subframes[0][i]) * sample_rescale, static_cast<float>(current_subframes[1][i]) * sample_rescale };
         break;
     case FlacFrameChannelType::LeftSideStereo:
         // channels are left (0) and side (1)
-        left = current_subframes[0];
-        right.ensure_capacity(left.size());
-        for (size_t i = 0; i < left.size(); ++i) {
+        for (size_t i = 0; i < m_current_frame->sample_count; ++i) {
             // right = left - side
-            right.unchecked_append(left[i] - current_subframes[1][i]);
+            samples[i] = { static_cast<float>(current_subframes[0][i]) * sample_rescale,
+                static_cast<float>(current_subframes[0][i] - current_subframes[1][i]) * sample_rescale };
         }
         break;
     case FlacFrameChannelType::RightSideStereo:
         // channels are side (0) and right (1)
-        right = current_subframes[1];
-        left.ensure_capacity(right.size());
-        for (size_t i = 0; i < right.size(); ++i) {
+        for (size_t i = 0; i < m_current_frame->sample_count; ++i) {
             // left = right + side
-            left.unchecked_append(right[i] + current_subframes[0][i]);
+            samples[i] = { static_cast<float>(current_subframes[1][i] + current_subframes[0][i]) * sample_rescale,
+                static_cast<float>(current_subframes[1][i]) * sample_rescale };
         }
         break;
     case FlacFrameChannelType::MidSideStereo:
         // channels are mid (0) and side (1)
-        left.ensure_capacity(current_subframes[0].size());
-        right.ensure_capacity(current_subframes[0].size());
         for (size_t i = 0; i < current_subframes[0].size(); ++i) {
             i64 mid = current_subframes[0][i];
             i64 side = current_subframes[1][i];
             mid *= 2;
             // prevent integer division errors
-            left.unchecked_append(static_cast<i32>((mid + side) / 2));
-            right.unchecked_append(static_cast<i32>((mid - side) / 2));
+            samples[i] = { static_cast<float>((mid + side) * .5f) * sample_rescale,
+                static_cast<float>((mid - side) * .5f) * sample_rescale };
         }
         break;
     }
 
-    VERIFY(left.size() == right.size() && left.size() == m_current_frame->sample_count);
-
-    float sample_rescale = static_cast<float>(1 << (pcm_bits_per_sample(m_current_frame->bit_depth) - 1));
-    dbgln_if(AFLACLOADER_DEBUG, "Sample rescaled from {} bits: factor {:.1f}", pcm_bits_per_sample(m_current_frame->bit_depth), sample_rescale);
-
-    // zip together channels
-    auto samples_to_directly_copy = min(target_vector.size(), m_current_frame->sample_count);
-    for (size_t i = 0; i < samples_to_directly_copy; ++i) {
-        Sample frame = { left[i] / sample_rescale, right[i] / sample_rescale };
-        target_vector[i] = frame;
-    }
-    // move superfluous data into the class buffer instead
-    auto result = m_unread_data.try_grow_capacity(m_current_frame->sample_count - samples_to_directly_copy);
-    if (result.is_error())
-        return LoaderError { LoaderError::Category::Internal, static_cast<size_t>(samples_to_directly_copy + m_current_sample_or_frame), "Couldn't allocate sample buffer for superfluous data" };
-
-    for (size_t i = samples_to_directly_copy; i < m_current_frame->sample_count; ++i) {
-        Sample frame = { left[i] / sample_rescale, right[i] / sample_rescale };
-        m_unread_data.unchecked_append(frame);
-    }
-
-    return {};
+    return samples;
 #undef FLAC_VERIFY
 }
 

Userland/Libraries/LibAudio/FlacLoader.h

@@ -15,12 +15,6 @@
 
 namespace Audio {
 
-// 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;
-
 ALWAYS_INLINE u8 frame_channel_type_to_channel_count(FlacFrameChannelType channel_type);
 // Sign-extend an arbitrary-size signed number to 64 bit signed
 ALWAYS_INLINE i64 sign_extend(u32 n, u8 size);
@@ -49,7 +43,7 @@ public:
     static Result<NonnullOwnPtr<FlacLoaderPlugin>, LoaderError> create(StringView path);
     static Result<NonnullOwnPtr<FlacLoaderPlugin>, LoaderError> create(Bytes buffer);
 
-    virtual LoaderSamples get_more_samples(size_t max_bytes_to_read_from_input = 128 * KiB) override;
+    virtual ErrorOr<Vector<FixedArray<Sample>>, LoaderError> load_chunks(size_t samples_to_read_from_input) override;
 
     virtual MaybeLoaderError reset() override;
     virtual MaybeLoaderError seek(int sample_index) override;
@@ -70,8 +64,8 @@ private:
     // Either returns the metadata block or sets error message.
     // Additionally, increments m_data_start_location past the read meta block.
     ErrorOr<FlacRawMetadataBlock, LoaderError> next_meta_block(BigEndianInputBitStream& bit_input);
-    // Fetches and writes the next FLAC frame
-    MaybeLoaderError next_frame(Span<Sample>);
+    // Fetches and returns the next FLAC frame.
+    LoaderSamples next_frame();
     // Helper of next_frame that fetches a sub frame's header
     ErrorOr<FlacSubframeHeader, LoaderError> next_subframe_header(BigEndianInputBitStream& bit_input, u8 channel_index);
     // Helper of next_frame that decompresses a subframe
@@ -108,8 +102,6 @@ private:
     // keep track of the start of the data in the FLAC stream to seek back more easily
     u64 m_data_start_location { 0 };
     Optional<FlacFrameHeader> m_current_frame;
-    // Whatever the last get_more_samples() call couldn't return gets stored here.
-    Vector<Sample, FLAC_BUFFER_SIZE> m_unread_data;
     u64 m_current_sample_or_frame { 0 };
     Vector<FlacSeekPoint> m_seektable;
 };

Userland/Libraries/LibAudio/Loader.cpp

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <AK/TypedTransfer.h>
 #include <LibAudio/FlacLoader.h>
 #include <LibAudio/Loader.h>
 #include <LibAudio/MP3Loader.h>
@@ -80,4 +81,49 @@ Result<NonnullOwnPtr<LoaderPlugin>, LoaderError> Loader::create_plugin(Bytes buf
     return LoaderError { "No loader plugin available" };
 }
 
+LoaderSamples Loader::get_more_samples(size_t samples_to_read_from_input)
+{
+    size_t remaining_samples = total_samples() - loaded_samples();
+    size_t samples_to_read = min(remaining_samples, samples_to_read_from_input);
+    auto samples = LOADER_TRY(FixedArray<Sample>::create(samples_to_read));
+
+    size_t sample_index = 0;
+
+    if (m_buffer.size() > 0) {
+        size_t to_transfer = min(m_buffer.size(), samples_to_read);
+        AK::TypedTransfer<Sample>::move(samples.data(), m_buffer.data(), to_transfer);
+        if (to_transfer < m_buffer.size())
+            m_buffer.remove(0, to_transfer);
+        else
+            m_buffer.clear_with_capacity();
+
+        sample_index += to_transfer;
+    }
+
+    while (sample_index < samples_to_read) {
+        auto chunk_data = TRY(m_plugin->load_chunks(samples_to_read - sample_index));
+        chunk_data.remove_all_matching([](auto& chunk) { return chunk.is_empty(); });
+        if (chunk_data.is_empty())
+            break;
+        for (auto& chunk : chunk_data) {
+            if (sample_index < samples_to_read) {
+                auto count = min(samples_to_read - sample_index, chunk.size());
+                AK::TypedTransfer<Sample>::move(samples.span().offset(sample_index), chunk.data(), count);
+                // We didn't read all of the chunk; transfer the rest into the buffer.
+                if (count < chunk.size()) {
+                    auto remaining_samples_count = chunk.size() - count;
+                    // We will always have an empty buffer at this point!
+                    LOADER_TRY(m_buffer.try_append(chunk.span().offset(count), remaining_samples_count));
+                }
+            } else {
+                // We're now past what the user requested. Transfer the entirety of the data into the buffer.
+                LOADER_TRY(m_buffer.try_append(chunk.data(), chunk.size()));
+            }
+            sample_index += chunk.size();
+        }
+    }
+
+    return samples;
+}
+
 }

Userland/Libraries/LibAudio/Loader.h

@@ -25,6 +25,12 @@ namespace Audio {
 
 static constexpr StringView no_plugin_error = "No loader plugin available"sv;
 
+// 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 const loader_buffer_size = 8 * KiB;
+
 using LoaderSamples = ErrorOr<FixedArray<Sample>, LoaderError>;
 using MaybeLoaderError = ErrorOr<void, LoaderError>;
 
@@ -33,7 +39,13 @@ public:
     explicit LoaderPlugin(NonnullOwnPtr<SeekableStream> stream);
     virtual ~LoaderPlugin() = default;
 
-    virtual LoaderSamples get_more_samples(size_t max_bytes_to_read_from_input = 128 * KiB) = 0;
+    // Load as many audio chunks as necessary to get up to the required samples.
+    // A chunk can be anything that is convenient for the plugin to load in one go without requiring to move samples around different buffers.
+    // For example: A FLAC, MP3 or QOA frame.
+    // The chunks are returned in a vector, so the loader can simply add chunks until the requested sample amount is reached.
+    // The sample count MAY be surpassed, but only as little as possible. It CAN be undershot when the end of the stream is reached.
+    // If the loader has no chunking limitations (e.g. WAV), it may return a single exact-sized chunk.
+    virtual ErrorOr<Vector<FixedArray<Sample>>, LoaderError> load_chunks(size_t samples_to_read_from_input) = 0;
 
     virtual MaybeLoaderError reset() = 0;
 
@@ -68,10 +80,15 @@ public:
     static Result<NonnullRefPtr<Loader>, LoaderError> create(StringView path) { return adopt_ref(*new Loader(TRY(create_plugin(path)))); }
     static Result<NonnullRefPtr<Loader>, LoaderError> create(Bytes buffer) { return adopt_ref(*new Loader(TRY(create_plugin(buffer)))); }
 
-    LoaderSamples get_more_samples(size_t max_samples_to_read_from_input = 128 * KiB) const { return m_plugin->get_more_samples(max_samples_to_read_from_input); }
+    // Will only read less samples if we're at the end of the stream.
+    LoaderSamples get_more_samples(size_t samples_to_read_from_input = 128 * KiB);
 
     MaybeLoaderError reset() const { return m_plugin->reset(); }
-    MaybeLoaderError seek(int const position) const { return m_plugin->seek(position); }
+    MaybeLoaderError seek(int const position) const
+    {
+        m_buffer.clear_with_capacity();
+        return m_plugin->seek(position);
+    }
 
     int loaded_samples() const { return m_plugin->loaded_samples(); }
     int total_samples() const { return m_plugin->total_samples(); }
@@ -88,6 +105,7 @@ private:
     explicit Loader(NonnullOwnPtr<LoaderPlugin>);
 
     mutable NonnullOwnPtr<LoaderPlugin> m_plugin;
+    mutable Vector<Sample, loader_buffer_size> m_buffer;
 };
 
 }

Userland/Libraries/LibAudio/MP3Loader.cpp

@@ -65,7 +65,6 @@ MaybeLoaderError MP3LoaderPlugin::reset()
 {
     TRY(seek(0));
     m_current_frame = {};
-    m_current_frame_read = 0;
     m_synthesis_buffer = {};
     m_loaded_samples = 0;
     LOADER_TRY(m_bit_reservoir.discard(m_bit_reservoir.used_buffer_size()));
@@ -83,53 +82,52 @@ MaybeLoaderError MP3LoaderPlugin::seek(int const position)
         }
     }
     m_current_frame = {};
-    m_current_frame_read = 0;
     m_synthesis_buffer = {};
     LOADER_TRY(m_bit_reservoir.discard(m_bit_reservoir.used_buffer_size()));
     m_bitstream->align_to_byte_boundary();
     return {};
 }
 
-LoaderSamples MP3LoaderPlugin::get_more_samples(size_t max_samples_to_read_from_input)
+ErrorOr<Vector<FixedArray<Sample>>, LoaderError> MP3LoaderPlugin::load_chunks(size_t samples_to_read_from_input)
 {
-    FixedArray<Sample> samples = LOADER_TRY(FixedArray<Sample>::create(max_samples_to_read_from_input));
-
-    size_t samples_to_read = max_samples_to_read_from_input;
+    int samples_to_read = samples_to_read_from_input;
+    Vector<FixedArray<Sample>> frames;
     while (samples_to_read > 0) {
+        FixedArray<Sample> samples = LOADER_TRY(FixedArray<Sample>::create(1152));
+
         if (!m_current_frame.has_value()) {
             auto maybe_frame = read_next_frame();
             if (maybe_frame.is_error()) {
                 if (m_stream->is_eof()) {
-                    return FixedArray<Sample> {};
+                    return Vector<FixedArray<Sample>> {};
                 }
                 return maybe_frame.release_error();
             }
             m_current_frame = maybe_frame.release_value();
             if (!m_current_frame.has_value())
                 break;
-            m_current_frame_read = 0;
         }
 
         bool const is_stereo = m_current_frame->header.channel_count() == 2;
-        for (; m_current_frame_read < 576 && samples_to_read > 0; m_current_frame_read++) {
-            auto const left_sample = m_current_frame->channels[0].granules[0].pcm[m_current_frame_read / 32][m_current_frame_read % 32];
-            auto const right_sample = is_stereo ? m_current_frame->channels[1].granules[0].pcm[m_current_frame_read / 32][m_current_frame_read % 32] : left_sample;
-            samples[samples.size() - samples_to_read] = Sample { left_sample, right_sample };
+        auto current_frame_read = 0;
+        for (; current_frame_read < 576; current_frame_read++) {
+            auto const left_sample = m_current_frame->channels[0].granules[0].pcm[current_frame_read / 32][current_frame_read % 32];
+            auto const right_sample = is_stereo ? m_current_frame->channels[1].granules[0].pcm[current_frame_read / 32][current_frame_read % 32] : left_sample;
+            samples[current_frame_read] = Sample { left_sample, right_sample };
             samples_to_read--;
         }
-        for (; m_current_frame_read < 1152 && samples_to_read > 0; m_current_frame_read++) {
-            auto const left_sample = m_current_frame->channels[0].granules[1].pcm[(m_current_frame_read - 576) / 32][(m_current_frame_read - 576) % 32];
-            auto const right_sample = is_stereo ? m_current_frame->channels[1].granules[1].pcm[(m_current_frame_read - 576) / 32][(m_current_frame_read - 576) % 32] : left_sample;
-            samples[samples.size() - samples_to_read] = Sample { left_sample, right_sample };
+        for (; current_frame_read < 1152; current_frame_read++) {
+            auto const left_sample = m_current_frame->channels[0].granules[1].pcm[(current_frame_read - 576) / 32][(current_frame_read - 576) % 32];
+            auto const right_sample = is_stereo ? m_current_frame->channels[1].granules[1].pcm[(current_frame_read - 576) / 32][(current_frame_read - 576) % 32] : left_sample;
+            samples[current_frame_read] = Sample { left_sample, right_sample };
             samples_to_read--;
         }
-        if (m_current_frame_read == 1152) {
-            m_current_frame = {};
-        }
+        m_loaded_samples += samples.size();
+        TRY(frames.try_append(move(samples)));
+        m_current_frame = {};
     }
 
-    m_loaded_samples += samples.size();
-    return samples;
+    return frames;
 }
 
 MaybeLoaderError MP3LoaderPlugin::build_seek_table()

Userland/Libraries/LibAudio/MP3Loader.h

@@ -27,7 +27,7 @@ public:
     static Result<NonnullOwnPtr<MP3LoaderPlugin>, LoaderError> create(StringView path);
     static Result<NonnullOwnPtr<MP3LoaderPlugin>, LoaderError> create(Bytes buffer);
 
-    virtual LoaderSamples get_more_samples(size_t max_bytes_to_read_from_input = 128 * KiB) override;
+    virtual ErrorOr<Vector<FixedArray<Sample>>, LoaderError> load_chunks(size_t samples_to_read_from_input) override;
 
     virtual MaybeLoaderError reset() override;
     virtual MaybeLoaderError seek(int const position) override;
@@ -70,7 +70,6 @@ private:
     size_t m_loaded_samples { 0 };
 
     AK::Optional<MP3::MP3Frame> m_current_frame;
-    u32 m_current_frame_read;
     OwnPtr<BigEndianInputBitStream> m_bitstream;
     AllocatingMemoryStream m_bit_reservoir;
 };

Userland/Libraries/LibAudio/QOALoader.cpp

@@ -142,38 +142,36 @@ MaybeLoaderError QOALoaderPlugin::load_one_frame(Span<Sample>& target, IsFirstFr
     return {};
 }
 
-LoaderSamples QOALoaderPlugin::get_more_samples(size_t max_samples_to_read_from_input)
+ErrorOr<Vector<FixedArray<Sample>>, LoaderError> QOALoaderPlugin::load_chunks(size_t samples_to_read_from_input)
 {
-    // Because each frame can only have so many inter-channel samples (quite a low number),
-    // we just load frames until the limit is reached, but at least one.
-    // This avoids caching samples in the QOA loader, simplifying state management.
-
-    if (max_samples_to_read_from_input < QOA::max_frame_samples)
-        return LoaderError { LoaderError::Category::Internal, LOADER_TRY(m_stream->tell()), "QOA loader is not capable of reading less than one frame of samples"sv };
-
     ssize_t const remaining_samples = static_cast<ssize_t>(m_total_samples - m_loaded_samples);
     if (remaining_samples <= 0)
-        return FixedArray<Sample> {};
-    size_t const samples_to_read = min(max_samples_to_read_from_input, remaining_samples);
+        return Vector<FixedArray<Sample>> {};
+    size_t const samples_to_read = min(samples_to_read_from_input, remaining_samples);
     auto is_first_frame = m_loaded_samples == 0 ? IsFirstFrame::Yes : IsFirstFrame::No;
 
-    auto samples = LOADER_TRY(FixedArray<Sample>::create(samples_to_read));
+    Vector<FixedArray<Sample>> frames;
     size_t current_loaded_samples = 0;
 
     while (current_loaded_samples < samples_to_read) {
-        auto slice_to_load_into = samples.span().slice(current_loaded_samples, min(QOA::max_frame_samples, samples.size() - current_loaded_samples));
+        auto samples = LOADER_TRY(FixedArray<Sample>::create(QOA::max_frame_samples));
+        auto slice_to_load_into = samples.span();
         TRY(this->load_one_frame(slice_to_load_into, is_first_frame));
         is_first_frame = IsFirstFrame::No;
         VERIFY(slice_to_load_into.size() <= QOA::max_frame_samples);
         current_loaded_samples += slice_to_load_into.size();
-        // The buffer wasn't large enough for the next frame.
-        if (slice_to_load_into.size() == 0)
+        if (slice_to_load_into.size() != samples.size()) {
+            auto smaller_samples = LOADER_TRY(FixedArray<Sample>::create(slice_to_load_into));
+            samples.swap(smaller_samples);
+        }
+        LOADER_TRY(frames.try_append(move(samples)));
+
+        if (slice_to_load_into.size() != samples.size())
             break;
     }
     m_loaded_samples += current_loaded_samples;
-    auto trimmed_samples = LOADER_TRY(FixedArray<Sample>::create(samples.span().trim(current_loaded_samples)));
 
-    return trimmed_samples;
+    return frames;
 }
 
 MaybeLoaderError QOALoaderPlugin::reset()

Userland/Libraries/LibAudio/QOALoader.h

@@ -27,7 +27,7 @@ public:
     static Result<NonnullOwnPtr<QOALoaderPlugin>, LoaderError> create(StringView path);
     static Result<NonnullOwnPtr<QOALoaderPlugin>, LoaderError> create(Bytes buffer);
 
-    virtual LoaderSamples get_more_samples(size_t max_samples_to_read_from_input = 128 * KiB) override;
+    virtual ErrorOr<Vector<FixedArray<Sample>>, LoaderError> load_chunks(size_t samples_to_read_from_input) override;
 
     virtual MaybeLoaderError reset() override;
     virtual MaybeLoaderError seek(int sample_index) override;

Userland/Libraries/LibAudio/WavLoader.cpp

@@ -137,20 +137,18 @@ LoaderSamples WavLoaderPlugin::samples_from_pcm_data(Bytes const& data, size_t s
     return samples;
 }
 
-LoaderSamples WavLoaderPlugin::get_more_samples(size_t max_samples_to_read_from_input)
+ErrorOr<Vector<FixedArray<Sample>>, LoaderError> WavLoaderPlugin::load_chunks(size_t samples_to_read_from_input)
 {
     auto remaining_samples = m_total_samples - m_loaded_samples;
     if (remaining_samples <= 0)
-        return FixedArray<Sample> {};
+        return Vector<FixedArray<Sample>> {};
 
     // One "sample" contains data from all channels.
     // In the Wave spec, this is also called a block.
     size_t bytes_per_sample
         = m_num_channels * pcm_bits_per_sample(m_sample_format) / 8;
 
-    // Might truncate if not evenly divisible by the sample size
-    auto max_samples_to_read = max_samples_to_read_from_input / bytes_per_sample;
-    auto samples_to_read = min(max_samples_to_read, remaining_samples);
+    auto samples_to_read = min(samples_to_read_from_input, remaining_samples);
     auto bytes_to_read = samples_to_read * bytes_per_sample;
 
     dbgln_if(AWAVLOADER_DEBUG, "Read {} bytes WAV with num_channels {} sample rate {}, "
@@ -163,7 +161,9 @@ LoaderSamples WavLoaderPlugin::get_more_samples(size_t max_samples_to_read_from_
 
     // m_loaded_samples should contain the amount of actually loaded samples
     m_loaded_samples += samples_to_read;
-    return samples_from_pcm_data(sample_data.bytes(), samples_to_read);
+    Vector<FixedArray<Sample>> samples;
+    TRY(samples.try_append(TRY(samples_from_pcm_data(sample_data.bytes(), samples_to_read))));
+    return samples;
 }
 
 MaybeLoaderError WavLoaderPlugin::seek(int sample_index)

Userland/Libraries/LibAudio/WavLoader.h

@@ -31,7 +31,7 @@ public:
     static Result<NonnullOwnPtr<WavLoaderPlugin>, LoaderError> create(StringView path);
     static Result<NonnullOwnPtr<WavLoaderPlugin>, LoaderError> create(Bytes buffer);
 
-    virtual LoaderSamples get_more_samples(size_t max_samples_to_read_from_input = 128 * KiB) override;
+    virtual ErrorOr<Vector<FixedArray<Sample>>, LoaderError> load_chunks(size_t samples_to_read_from_input) override;
 
     virtual MaybeLoaderError reset() override { return seek(0); }