From c837a1a8de492f42d5b5fd0e4b61da4681328ccf Mon Sep 17 00:00:00 2001 From: Lucas CHOLLET Date: Thu, 13 Oct 2022 16:05:57 +0200 Subject: [PATCH] LibAudio: Factorize stream initialisation to base class `LoaderPlugin` All actual plugins follow the same logic to initialize their stream, this commit factorizes all of this to their base class: `LoaderPlugin`. --- Meta/Lagom/Fuzzers/FuzzFlacLoader.cpp | 2 +- Meta/Lagom/Fuzzers/FuzzMP3Loader.cpp | 2 +- Userland/Libraries/LibAudio/FlacLoader.cpp | 11 ++++------- Userland/Libraries/LibAudio/FlacLoader.h | 6 +----- Userland/Libraries/LibAudio/Loader.cpp | 20 ++++++++++++++++++++ Userland/Libraries/LibAudio/Loader.h | 9 +++++++++ Userland/Libraries/LibAudio/MP3Loader.cpp | 2 +- Userland/Libraries/LibAudio/MP3Loader.h | 3 --- Userland/Libraries/LibAudio/WavLoader.cpp | 11 ++++------- Userland/Libraries/LibAudio/WavLoader.h | 7 +------ 10 files changed, 42 insertions(+), 31 deletions(-) diff --git a/Meta/Lagom/Fuzzers/FuzzFlacLoader.cpp b/Meta/Lagom/Fuzzers/FuzzFlacLoader.cpp index d3ca3e5aade..5056f0899ec 100644 --- a/Meta/Lagom/Fuzzers/FuzzFlacLoader.cpp +++ b/Meta/Lagom/Fuzzers/FuzzFlacLoader.cpp @@ -11,7 +11,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size) { auto flac_data = ByteBuffer::copy(data, size).release_value(); - auto flac = make(flac_data); + auto flac = make(flac_data.bytes()); if (flac->initialize().is_error()) return 1; diff --git a/Meta/Lagom/Fuzzers/FuzzMP3Loader.cpp b/Meta/Lagom/Fuzzers/FuzzMP3Loader.cpp index f9b8a92c646..4d1b751e57f 100644 --- a/Meta/Lagom/Fuzzers/FuzzMP3Loader.cpp +++ b/Meta/Lagom/Fuzzers/FuzzMP3Loader.cpp @@ -11,7 +11,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size) { auto flac_data = ByteBuffer::copy(data, size).release_value(); - auto mp3 = make(flac_data); + auto mp3 = make(flac_data.bytes()); if (mp3->initialize().is_error()) return 1; diff --git a/Userland/Libraries/LibAudio/FlacLoader.cpp b/Userland/Libraries/LibAudio/FlacLoader.cpp index 19ea92d51d2..b91c8433f57 100644 --- a/Userland/Libraries/LibAudio/FlacLoader.cpp +++ b/Userland/Libraries/LibAudio/FlacLoader.cpp @@ -26,21 +26,18 @@ namespace Audio { FlacLoaderPlugin::FlacLoaderPlugin(StringView path) - : m_path(path) + : LoaderPlugin(path) { } -FlacLoaderPlugin::FlacLoaderPlugin(Bytes& buffer) - : m_backing_memory(buffer) +FlacLoaderPlugin::FlacLoaderPlugin(Bytes buffer) + : LoaderPlugin(buffer) { } MaybeLoaderError FlacLoaderPlugin::initialize() { - if (m_backing_memory.has_value()) - m_stream = LOADER_TRY(Core::Stream::MemoryStream::construct(m_backing_memory.value())); - else - m_stream = LOADER_TRY(Core::Stream::File::open(m_path, Core::Stream::OpenMode::Read)); + LOADER_TRY(LoaderPlugin::initialize()); TRY(parse_header()); TRY(reset()); diff --git a/Userland/Libraries/LibAudio/FlacLoader.h b/Userland/Libraries/LibAudio/FlacLoader.h index 812d5e4cbbe..4f94d58e7b2 100644 --- a/Userland/Libraries/LibAudio/FlacLoader.h +++ b/Userland/Libraries/LibAudio/FlacLoader.h @@ -48,7 +48,7 @@ ALWAYS_INLINE ErrorOr decode_unsigned_exp_golomb(u8 order, BigEndianInputBi class FlacLoaderPlugin : public LoaderPlugin { public: explicit FlacLoaderPlugin(StringView path); - explicit FlacLoaderPlugin(Bytes& buffer); + explicit FlacLoaderPlugin(Bytes buffer); ~FlacLoaderPlugin() { } @@ -95,8 +95,6 @@ private: ALWAYS_INLINE ErrorOr convert_sample_rate_code(u8 sample_rate_code); ALWAYS_INLINE ErrorOr convert_bit_depth_code(u8 bit_depth_code); - StringView m_path; - // Data obtained directly from the FLAC metadata: many values have specific bit counts u32 m_sample_rate { 0 }; // 20 bit u8 m_num_channels { 0 }; // 3 bit @@ -113,8 +111,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 }; - OwnPtr m_stream; - Optional m_backing_memory; Optional m_current_frame; // Whatever the last get_more_samples() call couldn't return gets stored here. Vector m_unread_data; diff --git a/Userland/Libraries/LibAudio/Loader.cpp b/Userland/Libraries/LibAudio/Loader.cpp index f98212f0abe..163d48e20e4 100644 --- a/Userland/Libraries/LibAudio/Loader.cpp +++ b/Userland/Libraries/LibAudio/Loader.cpp @@ -11,6 +11,26 @@ namespace Audio { +LoaderPlugin::LoaderPlugin(StringView path) + : m_path(path) +{ +} + +LoaderPlugin::LoaderPlugin(Bytes buffer) + : m_backing_memory(buffer) +{ +} + +MaybeLoaderError LoaderPlugin::initialize() +{ + if (m_backing_memory.has_value()) + m_stream = LOADER_TRY(Core::Stream::MemoryStream::construct(m_backing_memory.value())); + else + m_stream = LOADER_TRY(Core::Stream::File::open(m_path, Core::Stream::OpenMode::Read)); + + return {}; +} + Loader::Loader(NonnullOwnPtr plugin) : m_plugin(move(plugin)) { diff --git a/Userland/Libraries/LibAudio/Loader.h b/Userland/Libraries/LibAudio/Loader.h index 737d7dfe966..4f66a5be1c9 100644 --- a/Userland/Libraries/LibAudio/Loader.h +++ b/Userland/Libraries/LibAudio/Loader.h @@ -18,6 +18,7 @@ #include #include #include +#include namespace Audio { @@ -28,6 +29,8 @@ using MaybeLoaderError = Result; class LoaderPlugin { public: + explicit LoaderPlugin(StringView path); + explicit LoaderPlugin(Bytes buffer); virtual ~LoaderPlugin() = default; virtual MaybeLoaderError initialize() = 0; @@ -53,6 +56,12 @@ public: // Human-readable name of the file format, of the form (.) virtual String format_name() = 0; virtual PcmSampleFormat pcm_format() = 0; + +protected: + StringView m_path; + OwnPtr m_stream; + // The constructor might set this so that we can initialize the data stream later. + Optional m_backing_memory; }; class Loader : public RefCounted { diff --git a/Userland/Libraries/LibAudio/MP3Loader.cpp b/Userland/Libraries/LibAudio/MP3Loader.cpp index e44a5a4f74b..61340c00a78 100644 --- a/Userland/Libraries/LibAudio/MP3Loader.cpp +++ b/Userland/Libraries/LibAudio/MP3Loader.cpp @@ -20,7 +20,7 @@ MP3LoaderPlugin::MP3LoaderPlugin(StringView path) } MP3LoaderPlugin::MP3LoaderPlugin(Bytes buffer) - : m_backing_memory(buffer) + : LoaderPlugin(buffer) { } diff --git a/Userland/Libraries/LibAudio/MP3Loader.h b/Userland/Libraries/LibAudio/MP3Loader.h index 7e83c0e929f..a0ef5ea8eb4 100644 --- a/Userland/Libraries/LibAudio/MP3Loader.h +++ b/Userland/Libraries/LibAudio/MP3Loader.h @@ -72,9 +72,6 @@ private: AK::Optional m_current_frame; u32 m_current_frame_read; - StringView m_path; - OwnPtr m_stream; - Optional m_backing_memory; OwnPtr m_bitstream; DuplexMemoryStream m_bit_reservoir; }; diff --git a/Userland/Libraries/LibAudio/WavLoader.cpp b/Userland/Libraries/LibAudio/WavLoader.cpp index 458eb68eba2..62d0d06753a 100644 --- a/Userland/Libraries/LibAudio/WavLoader.cpp +++ b/Userland/Libraries/LibAudio/WavLoader.cpp @@ -19,23 +19,20 @@ namespace Audio { static constexpr size_t const maximum_wav_size = 1 * GiB; // FIXME: is there a more appropriate size limit? WavLoaderPlugin::WavLoaderPlugin(StringView path) - : m_path(path) + : LoaderPlugin(path) { } MaybeLoaderError WavLoaderPlugin::initialize() { - if (m_backing_memory.has_value()) - m_stream = LOADER_TRY(Core::Stream::MemoryStream::construct(m_backing_memory.value())); - else - m_stream = LOADER_TRY(Core::Stream::File::open(m_path, Core::Stream::OpenMode::Read)); + LOADER_TRY(LoaderPlugin::initialize()); TRY(parse_header()); return {}; } -WavLoaderPlugin::WavLoaderPlugin(Bytes const& buffer) - : m_backing_memory(buffer) +WavLoaderPlugin::WavLoaderPlugin(Bytes buffer) + : LoaderPlugin(buffer) { } diff --git a/Userland/Libraries/LibAudio/WavLoader.h b/Userland/Libraries/LibAudio/WavLoader.h index ff96120d575..437f7eb54fe 100644 --- a/Userland/Libraries/LibAudio/WavLoader.h +++ b/Userland/Libraries/LibAudio/WavLoader.h @@ -30,7 +30,7 @@ static constexpr unsigned const WAVE_FORMAT_EXTENSIBLE = 0xFFFE; // Determined b class WavLoaderPlugin : public LoaderPlugin { public: explicit WavLoaderPlugin(StringView path); - explicit WavLoaderPlugin(Bytes const& buffer); + explicit WavLoaderPlugin(Bytes buffer); virtual MaybeLoaderError initialize() override; @@ -56,11 +56,6 @@ private: template MaybeLoaderError read_samples_from_stream(Core::Stream::Stream& stream, SampleReader read_sample, FixedArray& samples) const; - StringView m_path; - OwnPtr m_stream; - // The constructor might set this so that we can initialize the data stream later. - Optional m_backing_memory; - u32 m_sample_rate { 0 }; u16 m_num_channels { 0 }; PcmSampleFormat m_sample_format;