LibAudio: Modernize WAV loader

With this, the WAV loader is a completely modern LibAudio loader:
- Own type header for RIFF data structures
- custom stream read functions for the types
- Final removal of legacy I/O error checking
- clearer error messages
- clean handling of header chunks

The latter will allow proper handling of other chunks (before "data") in
the future, such as metadata :^)

Userland/Libraries/LibAudio/CMakeLists.txt

@@ -2,6 +2,7 @@ set(SOURCES
     GenericTypes.cpp
     SampleFormats.cpp
     Loader.cpp
+    RIFFTypes.cpp
     WavLoader.cpp
     FlacLoader.cpp
     WavWriter.cpp

Userland/Libraries/LibAudio/RIFFTypes.cpp

@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2023, kleines Filmröllchen <filmroellchen@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include "RIFFTypes.h"
+#include <AK/Endian.h>
+#include <AK/Stream.h>
+#include <AK/Try.h>
+
+namespace Audio::RIFF {
+
+ErrorOr<ChunkID> ChunkID::read_from_stream(Stream& stream)
+{
+    Array<u8, chunk_id_size> id;
+    TRY(stream.read_until_filled(id.span()));
+    return ChunkID { id };
+}
+
+ErrorOr<Chunk> Chunk::read_from_stream(Stream& stream)
+{
+    auto id = TRY(stream.read_value<ChunkID>());
+
+    u32 size = TRY(stream.read_value<LittleEndian<u32>>());
+    auto data = TRY(FixedArray<u8>::create(size));
+    TRY(stream.read_until_filled(data.span()));
+
+    return Chunk {
+        id,
+        size,
+        move(data),
+    };
+}
+
+StringView ChunkID::as_ascii_string() const
+{
+    return StringView { id_data.span() };
+}
+
+bool ChunkID::operator==(StringView const& other_string) const
+{
+    return as_ascii_string() == other_string;
+}
+
+FixedMemoryStream Chunk::data_stream()
+{
+    return FixedMemoryStream { data.span() };
+}
+
+}

Userland/Libraries/LibAudio/RIFFTypes.h

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2018-2023, the SerenityOS developers.
+ * Copyright (c) 2023, kleines Filmröllchen <filmroellchen@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/FixedArray.h>
+#include <AK/Forward.h>
+#include <AK/MemoryStream.h>
+#include <AK/Types.h>
+
+// RIFF-specific type definitions necessary for handling WAVE files.
+namespace Audio::RIFF {
+
+static constexpr StringView const riff_magic = "RIFF"sv;
+static constexpr StringView const wave_subformat_id = "WAVE"sv;
+static constexpr StringView const data_chunk_id = "data"sv;
+static constexpr StringView const list_chunk_id = "LIST"sv;
+static constexpr StringView const info_chunk_id = "INFO"sv;
+static constexpr StringView const format_chunk_id = "fmt "sv;
+
+// Constants for handling WAVE header data.
+enum class WaveFormat : u32 {
+    Pcm = 0x0001,        // WAVE_FORMAT_PCM
+    IEEEFloat = 0x0003,  // WAVE_FORMAT_IEEE_FLOAT
+    ALaw = 0x0006,       // 8-bit ITU-T G.711 A-law
+    MuLaw = 0x0007,      // 8-bit ITU-T G.711 µ-law
+    Extensible = 0xFFFE, // Determined by SubFormat
+};
+
+static constexpr size_t const chunk_id_size = 4;
+
+struct ChunkID {
+    static ErrorOr<ChunkID> read_from_stream(Stream& stream);
+    StringView as_ascii_string() const;
+    bool operator==(ChunkID const&) const = default;
+    bool operator==(StringView const&) const;
+
+    Array<u8, chunk_id_size> id_data;
+};
+
+// http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/Docs/riffmci.pdf page 11 (Chunks)
+struct Chunk {
+    static ErrorOr<Chunk> read_from_stream(Stream& stream);
+    FixedMemoryStream data_stream();
+
+    ChunkID id;
+    u32 size;
+    FixedArray<u8> data;
+};
+
+}

Userland/Libraries/LibAudio/WavLoader.cpp

@@ -1,12 +1,13 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
- * Copyright (c) 2021, kleines Filmröllchen <filmroellchen@serenityos.org>
+ * Copyright (c) 2021-2023, kleines Filmröllchen <filmroellchen@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #include "WavLoader.h"
 #include "LoaderError.h"
+#include "RIFFTypes.h"
 #include <AK/Debug.h>
 #include <AK/Endian.h>
 #include <AK/FixedArray.h>
@@ -183,95 +184,56 @@ MaybeLoaderError WavLoaderPlugin::seek(int sample_index)
 // Specification reference: http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/WAVE.html
 MaybeLoaderError WavLoaderPlugin::parse_header()
 {
-    bool ok = true;
+#define CHECK(check, category, msg)                                                                                                                 \
-    size_t bytes_read = 0;
+    do {                                                                                                                                            \
-
+        if (!(check)) {                                                                                                                             \
-    auto read_u8 = [&]() -> ErrorOr<u8, LoaderError> {
+            return LoaderError { category, static_cast<size_t>(LOADER_TRY(m_stream->tell())), DeprecatedString::formatted("WAV header: {}", msg) }; \
-        u8 value = LOADER_TRY(m_stream->read_value<LittleEndian<u8>>());
+        }                                                                                                                                           \
-        bytes_read += 1;
-        return value;
-    };
-
-    auto read_u16 = [&]() -> ErrorOr<u16, LoaderError> {
-        u16 value = LOADER_TRY(m_stream->read_value<LittleEndian<u16>>());
-        bytes_read += 2;
-        return value;
-    };
-
-    auto read_u32 = [&]() -> ErrorOr<u32, LoaderError> {
-        u32 value = LOADER_TRY(m_stream->read_value<LittleEndian<u32>>());
-        bytes_read += 4;
-        return value;
-    };
-
-#define CHECK_OK(category, msg)                                                                      \
-    do {                                                                                             \
-        if (!ok)                                                                                     \
-            return LoaderError { category, DeprecatedString::formatted("Parsing failed: {}", msg) }; \
     } while (0)
 
-    u32 riff = TRY(read_u32());
+    auto riff = TRY(m_stream->read_value<RIFF::ChunkID>());
-    ok = ok && riff == 0x46464952; // "RIFF"
+    CHECK(riff == RIFF::riff_magic, LoaderError::Category::Format, "RIFF header magic invalid");
-    CHECK_OK(LoaderError::Category::Format, "RIFF header");
 
-    u32 sz = TRY(read_u32());
+    u32 size = TRY(m_stream->read_value<LittleEndian<u32>>());
-    ok = ok && sz < maximum_wav_size;
+    CHECK(size < maximum_wav_size, LoaderError::Category::Format, "File size too large");
-    CHECK_OK(LoaderError::Category::Format, "File size");
 
-    u32 wave = TRY(read_u32());
+    auto wave = TRY(m_stream->read_value<RIFF::ChunkID>());
-    ok = ok && wave == 0x45564157; // "WAVE"
+    CHECK(wave == RIFF::wave_subformat_id, LoaderError::Category::Format, "WAVE subformat id invalid");
-    CHECK_OK(LoaderError::Category::Format, "WAVE header");
 
-    u32 fmt_id = TRY(read_u32());
+    auto format_chunk = TRY(m_stream->read_value<RIFF::Chunk>());
-    ok = ok && fmt_id == 0x20746D66; // "fmt "
+    CHECK(format_chunk.id.as_ascii_string() == RIFF::format_chunk_id, LoaderError::Category::Format, "FMT chunk id invalid");
-    CHECK_OK(LoaderError::Category::Format, "FMT header");
 
-    u32 fmt_size = TRY(read_u32());
+    auto format_stream = format_chunk.data_stream();
-    ok = ok && (fmt_size == 16 || fmt_size == 18 || fmt_size == 40);
+    u16 audio_format = TRY(format_stream.read_value<LittleEndian<u16>>());
-    CHECK_OK(LoaderError::Category::Format, "FMT size");
+    CHECK(audio_format == to_underlying(RIFF::WaveFormat::Pcm) || audio_format == to_underlying(RIFF::WaveFormat::IEEEFloat) || audio_format == to_underlying(RIFF::WaveFormat::Extensible),
+        LoaderError::Category::Unimplemented, "Audio format not supported");
 
-    u16 audio_format = TRY(read_u16());
+    m_num_channels = TRY(format_stream.read_value<LittleEndian<u16>>());
-    CHECK_OK(LoaderError::Category::Format, "Audio format"); // incomplete read check
+    CHECK(m_num_channels == 1 || m_num_channels == 2, LoaderError::Category::Unimplemented, "Channel count");
-    ok = ok && (audio_format == WAVE_FORMAT_PCM || audio_format == WAVE_FORMAT_IEEE_FLOAT || audio_format == WAVE_FORMAT_EXTENSIBLE);
-    CHECK_OK(LoaderError::Category::Unimplemented, "Audio format PCM/Float"); // value check
 
-    m_num_channels = TRY(read_u16());
+    m_sample_rate = TRY(format_stream.read_value<LittleEndian<u32>>());
-    ok = ok && (m_num_channels == 1 || m_num_channels == 2);
+    // Data rate; can be ignored.
-    CHECK_OK(LoaderError::Category::Unimplemented, "Channel count");
+    TRY(format_stream.read_value<LittleEndian<u32>>());
+    u16 block_size_bytes = TRY(format_stream.read_value<LittleEndian<u16>>());
 
-    m_sample_rate = TRY(read_u32());
+    u16 bits_per_sample = TRY(format_stream.read_value<LittleEndian<u16>>());
-    CHECK_OK(LoaderError::Category::IO, "Sample rate");
 
-    TRY(read_u32());
+    if (audio_format == to_underlying(RIFF::WaveFormat::Extensible)) {
-    CHECK_OK(LoaderError::Category::IO, "Data rate");
+        CHECK(format_chunk.size == 40, LoaderError::Category::Format, "Extensible fmt size is not 40 bytes");
-
-    u16 block_size_bytes = TRY(read_u16());
-    CHECK_OK(LoaderError::Category::IO, "Block size");
-
-    u16 bits_per_sample = TRY(read_u16());
-    CHECK_OK(LoaderError::Category::IO, "Bits per sample");
-
-    if (audio_format == WAVE_FORMAT_EXTENSIBLE) {
-        ok = ok && (fmt_size == 40);
-        CHECK_OK(LoaderError::Category::Format, "Extensible fmt size"); // value check
 
         // Discard everything until the GUID.
         // We've already read 16 bytes from the stream. The GUID starts in another 8 bytes.
-        TRY(read_u32());
+        TRY(format_stream.read_value<LittleEndian<u64>>());
-        TRY(read_u32());
-        CHECK_OK(LoaderError::Category::IO, "Discard until GUID");
 
         // Get the underlying audio format from the first two bytes of GUID
-        u16 guid_subformat = TRY(read_u16());
+        u16 guid_subformat = TRY(format_stream.read_value<LittleEndian<u16>>());
-        ok = ok && (guid_subformat == WAVE_FORMAT_PCM || guid_subformat == WAVE_FORMAT_IEEE_FLOAT);
+        CHECK(guid_subformat == to_underlying(RIFF::WaveFormat::Pcm) || guid_subformat == to_underlying(RIFF::WaveFormat::IEEEFloat), LoaderError::Category::Unimplemented, "GUID SubFormat not supported");
-        CHECK_OK(LoaderError::Category::Unimplemented, "GUID SubFormat");
 
         audio_format = guid_subformat;
     }
 
-    if (audio_format == WAVE_FORMAT_PCM) {
+    if (audio_format == to_underlying(RIFF::WaveFormat::Pcm)) {
-        ok = ok && (bits_per_sample == 8 || bits_per_sample == 16 || bits_per_sample == 24);
+        CHECK(bits_per_sample == 8 || bits_per_sample == 16 || bits_per_sample == 24, LoaderError::Category::Unimplemented, "PCM bits per sample not supported");
-        CHECK_OK(LoaderError::Category::Unimplemented, "Bits per sample (PCM)"); // value check
 
         // We only support 8-24 bit audio right now because other formats are uncommon
         if (bits_per_sample == 8) {
@@ -281,9 +243,8 @@ MaybeLoaderError WavLoaderPlugin::parse_header()
         } else if (bits_per_sample == 24) {
             m_sample_format = PcmSampleFormat::Int24;
         }
-    } else if (audio_format == WAVE_FORMAT_IEEE_FLOAT) {
+    } else if (audio_format == to_underlying(RIFF::WaveFormat::IEEEFloat)) {
-        ok = ok && (bits_per_sample == 32 || bits_per_sample == 64);
+        CHECK(bits_per_sample == 32 || bits_per_sample == 64, LoaderError::Category::Unimplemented, "Float bits per sample not supported");
-        CHECK_OK(LoaderError::Category::Unimplemented, "Bits per sample (Float)"); // value check
 
         // Again, only the common 32 and 64 bit
         if (bits_per_sample == 32) {
@@ -293,42 +254,28 @@ MaybeLoaderError WavLoaderPlugin::parse_header()
         }
     }
 
-    ok = ok && (block_size_bytes == (m_num_channels * (bits_per_sample / 8)));
+    CHECK(block_size_bytes == (m_num_channels * (bits_per_sample / 8)), LoaderError::Category::Format, "Block size invalid");
-    CHECK_OK(LoaderError::Category::Format, "Block size sanity check");
 
     dbgln_if(AWAVLOADER_DEBUG, "WAV format {} at {} bit, {} channels, rate {}Hz ",
         sample_format_name(m_sample_format), pcm_bits_per_sample(m_sample_format), m_num_channels, m_sample_rate);
 
-    // Read chunks until we find DATA
+    // Read all chunks before DATA.
     bool found_data = false;
-    u32 data_size = 0;
+    while (!found_data) {
-    u8 search_byte = 0;
+        auto chunk_header = TRY(m_stream->read_value<RIFF::ChunkID>());
-    while (true) {
+        if (chunk_header == RIFF::data_chunk_id) {
-        search_byte = TRY(read_u8());
+            found_data = true;
-        CHECK_OK(LoaderError::Category::IO, "Reading byte searching for data");
+        } else {
-        if (search_byte != 0x64) // D
+            TRY(m_stream->seek(-RIFF::chunk_id_size, SeekMode::FromCurrentPosition));
-            continue;
+            auto chunk = TRY(m_stream->read_value<RIFF::Chunk>());
-
+            dbgln_if(AWAVLOADER_DEBUG, "Unhandled WAV chunk of type {}, size {} bytes", chunk.id.as_ascii_string(), chunk.size);
-        search_byte = TRY(read_u8());
+            // TODO: Handle LIST INFO chunks.
-        CHECK_OK(LoaderError::Category::IO, "Reading next byte searching for data");
+        }
-        if (search_byte != 0x61) // A
-            continue;
-
-        u16 search_remaining = TRY(read_u16());
-        CHECK_OK(LoaderError::Category::IO, "Reading remaining bytes searching for data");
-        if (search_remaining != 0x6174) // TA
-            continue;
-
-        data_size = TRY(read_u32());
-        found_data = true;
-        break;
     }
 
-    ok = ok && found_data;
+    u32 data_size = TRY(m_stream->read_value<LittleEndian<u32>>());
-    CHECK_OK(LoaderError::Category::Format, "Found no data chunk");
+    CHECK(found_data, LoaderError::Category::Format, "Found no data chunk");
-
+    CHECK(data_size < maximum_wav_size, LoaderError::Category::Format, "Data too large");
-    ok = ok && data_size < maximum_wav_size;
-    CHECK_OK(LoaderError::Category::Format, "Data was too large");
 
     m_total_samples = data_size / block_size_bytes;
 
@@ -337,7 +284,8 @@ MaybeLoaderError WavLoaderPlugin::parse_header()
         block_size_bytes,
         m_total_samples);
 
-    m_byte_offset_of_data_samples = bytes_read;
+    m_byte_offset_of_data_samples = TRY(m_stream->tell());
     return {};
 }
+
 }

Userland/Libraries/LibAudio/WavLoader.h

@@ -17,14 +17,10 @@
 
 namespace Audio {
 
-// constants for handling the WAV header data
+// Loader for the WAVE (file extension .wav) uncompressed audio file format.
-static constexpr unsigned const WAVE_FORMAT_PCM = 0x0001;        // PCM
+// WAVE uses the Microsoft RIFF container.
-static constexpr unsigned const WAVE_FORMAT_IEEE_FLOAT = 0x0003; // IEEE float
+// Original RIFF Spec, without later extensions: https://www.aelius.com/njh/wavemetatools/doc/riffmci.pdf
-static constexpr unsigned const WAVE_FORMAT_ALAW = 0x0006;       // 8-bit ITU-T G.711 A-law
+// More concise WAVE information plus various spec links: http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/WAVE.html
-static constexpr unsigned const WAVE_FORMAT_MULAW = 0x0007;      // 8-bit ITU-T G.711 µ-law
-static constexpr unsigned const WAVE_FORMAT_EXTENSIBLE = 0xFFFE; // Determined by SubFormat
-
-// Parses and reads audio data from a WAV file.
 class WavLoaderPlugin : public LoaderPlugin {
 public:
     explicit WavLoaderPlugin(NonnullOwnPtr<SeekableStream> stream);