From 0476e8f7ce4563ef04cdeba341ddc7ece8d17165 Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Thu, 27 Aug 2026 00:49:05 +0000 Subject: [PATCH 1/3] feat(audio): accept PCM8/32, float64, A-law, mu-law and WAVEFORMATEXTENSIBLE The reader handled PCM16, PCM24 and float32. Everything else -- including ordinary PCM16 that happens to be tagged 0xFFFE -- came back as "unsupported WAV encoding (need PCM16, PCM24, or float32)", which is confusing when PCM16 is exactly what is inside. Encoders emit WAVEFORMATEXTENSIBLE routinely for more than two channels or whenever a channel mask is set, and the real format tag then lives in the SubFormat GUID rather than in wFormatTag. Adds PCM8 (unsigned, biased by 128), PCM32, float64, G.711 A-law and mu-law, and unwraps WAVEFORMATEXTENSIBLE to whatever its GUID names. Files that still cannot be decoded now say what they are: a FLAC, Ogg, MP3, MP4, AIFF, RF64 or CAF given to the reader is named as such instead of failing with "invalid WAV RIFF header". Tests cover each added format. The G.711 cases are pinned to the published decode values (mu-law 0x00 -> -32124, A-law 0x2A -> +32256, and A-law's absence of an exact zero) rather than to a re-derivation of the same bit manipulation, which would prove nothing. Two negative cases check that widening the accepted set did not turn into accepting everything: ADPCM is still rejected, and a FLAC is still identified as a FLAC. Verified by execution: the new cases fail against the previous reader with the old message and pass against this one; wav_reader_chunk_bounds_test still passes. This lands separately from #180 at @0xShug0's request. The decoders originate from @dignome's contributed tree, where they existed so the CLI and server path would accept the same files the web UI already did. --- src/framework/audio/wav_reader.cpp | 176 +++++++++++++++++++++-- tests/unittests/test_wav_reader.cpp | 213 ++++++++++++++++++++++++++++ 2 files changed, 377 insertions(+), 12 deletions(-) diff --git a/src/framework/audio/wav_reader.cpp b/src/framework/audio/wav_reader.cpp index d55a9f843..a831eaacc 100644 --- a/src/framework/audio/wav_reader.cpp +++ b/src/framework/audio/wav_reader.cpp @@ -1,6 +1,9 @@ #include "engine/framework/audio/wav_reader.h" +#include +#include #include +#include #include #include #include @@ -63,6 +66,89 @@ void skip_bytes(std::istream & input, std::streamoff count) { } } +// WAVE format tags. EXTENSIBLE is the one that matters in practice: many +// encoders emit it for ordinary PCM16 whenever there are more than two channels +// or a channel mask is set, and the real codec then lives in a SubFormat GUID +// rather than in the format tag itself. +constexpr uint16_t kFormatPcm = 0x0001; +constexpr uint16_t kFormatFloat = 0x0003; +constexpr uint16_t kFormatALaw = 0x0006; +constexpr uint16_t kFormatMuLaw = 0x0007; +constexpr uint16_t kFormatExtensible = 0xFFFE; + +// Names a container we can recognise but not decode, so the error can say what +// the file actually is instead of "invalid WAV RIFF header". +const char * identify_foreign_container(const std::array & header) { + const auto * bytes = reinterpret_cast(header.data()); + if (std::memcmp(header.data(), "fLaC", 4) == 0) { + return "FLAC"; + } + if (std::memcmp(header.data(), "OggS", 4) == 0) { + return "Ogg (Vorbis/Opus)"; + } + if (std::memcmp(header.data(), "ID3", 3) == 0) { + return "MP3"; + } + // MPEG audio frame sync: 11 set bits. + if (bytes[0] == 0xFF && (bytes[1] & 0xE0) == 0xE0) { + return "MP3"; + } + if (std::memcmp(header.data() + 4, "ftyp", 4) == 0) { + return "MP4/M4A (AAC or ALAC)"; + } + if (std::memcmp(header.data(), "FORM", 4) == 0) { + return "AIFF"; + } + if (std::memcmp(header.data(), "RF64", 4) == 0) { + return "RF64"; + } + if (std::memcmp(header.data(), "caff", 4) == 0) { + return "CAF"; + } + if (bytes[0] == 0x1A && bytes[1] == 0x45 && bytes[2] == 0xDF && bytes[3] == 0xA3) { + return "Matroska/WebM"; + } + return nullptr; +} + +// G.711 expansion. Both are 8-bit logarithmic codings still common in +// telephony recordings and in WAVs produced by conferencing tools. +float decode_mu_law(uint8_t value) { + value = static_cast(~value); + const int sign = (value & 0x80) != 0 ? -1 : 1; + const int exponent = (value >> 4) & 0x07; + const int mantissa = value & 0x0F; + const int magnitude = ((mantissa << 3) + 0x84) << exponent; + return static_cast(sign * (magnitude - 0x84)) / 32768.0F; +} + +float decode_a_law(uint8_t value) { + value ^= 0x55; + const int sign = (value & 0x80) != 0 ? -1 : 1; + const int exponent = (value >> 4) & 0x07; + const int mantissa = value & 0x0F; + int magnitude = 0; + if (exponent == 0) { + magnitude = (mantissa << 4) + 8; + } else { + magnitude = ((mantissa << 4) + 0x108) << (exponent - 1); + } + return static_cast(sign * magnitude) / 32768.0F; +} + +std::string describe_encoding(uint16_t format, uint16_t bits) { + std::string name; + switch (format) { + case kFormatPcm: name = "PCM"; break; + case kFormatFloat: name = "IEEE float"; break; + case kFormatALaw: name = "A-law"; break; + case kFormatMuLaw: name = "mu-law"; break; + case kFormatExtensible: name = "extensible"; break; + default: name = "format tag " + std::to_string(format); break; + } + return name + ", " + std::to_string(bits) + "-bit"; +} + } // namespace WavData read_wav_f32(std::istream & input) { @@ -70,17 +156,22 @@ WavData read_wav_f32(std::istream & input) { throw std::runtime_error("could not open WAV input"); } - char riff[4]; - input.read(riff, 4); - if (!input || std::string(riff, 4) != "RIFF") { + std::array header{}; + input.read(header.data(), static_cast(header.size())); + const auto header_read = static_cast(input.gcount()); + input.clear(); + input.seekg(static_cast(header_read), std::ios::beg); + + if (header_read < 12 || std::memcmp(header.data(), "RIFF", 4) != 0 || + std::memcmp(header.data() + 8, "WAVE", 4) != 0) { + if (const char * container = identify_foreign_container(header)) { + throw std::runtime_error( + std::string("input is ") + container + + ", not WAV; convert it first, e.g. " + "`ffmpeg -i input -ac 1 -ar 44100 -c:a pcm_s16le output.wav`"); + } throw std::runtime_error("invalid WAV RIFF header"); } - skip_bytes(input, 4); - char wave[4]; - input.read(wave, 4); - if (!input || std::string(wave, 4) != "WAVE") { - throw std::runtime_error("invalid WAV WAVE header"); - } uint16_t audio_format = 0; uint16_t channels = 0; @@ -102,8 +193,18 @@ WavData read_wav_f32(std::istream & input) { sample_rate = read_scalar(input); skip_bytes(input, 6); bits_per_sample = read_scalar(input); - if (chunk_size > 16) { - skip_bytes(input, static_cast(chunk_size - 16)); + std::streamoff consumed = 16; + if (audio_format == kFormatExtensible && chunk_size >= 40) { + skip_bytes(input, 2); // cbSize + skip_bytes(input, 2); // wValidBitsPerSample + skip_bytes(input, 4); // dwChannelMask + // The SubFormat GUID begins with the real format tag. + audio_format = read_scalar(input); + skip_bytes(input, 14); // remainder of the GUID + consumed = 40; + } + if (chunk_size > consumed) { + skip_bytes(input, static_cast(chunk_size) - consumed); } } else if (id == "data") { // chunk_size is a 32-bit field read straight from the file, so a @@ -143,6 +244,54 @@ WavData read_wav_f32(std::istream & input) { wav.sample_rate = static_cast(sample_rate); wav.channels = static_cast(channels); + if (audio_format == kFormatPcm && bits_per_sample == 8) { + // 8-bit PCM in WAV is unsigned, offset by 128. + wav.samples.resize(data.size()); + const auto * pcm = reinterpret_cast(data.data()); + for (size_t i = 0; i < data.size(); ++i) { + wav.samples[i] = (static_cast(pcm[i]) - 128.0F) / 128.0F; + } + return wav; + } + + if (audio_format == kFormatMuLaw && bits_per_sample == 8) { + wav.samples.resize(data.size()); + const auto * pcm = reinterpret_cast(data.data()); + for (size_t i = 0; i < data.size(); ++i) { + wav.samples[i] = decode_mu_law(pcm[i]); + } + return wav; + } + + if (audio_format == kFormatALaw && bits_per_sample == 8) { + wav.samples.resize(data.size()); + const auto * pcm = reinterpret_cast(data.data()); + for (size_t i = 0; i < data.size(); ++i) { + wav.samples[i] = decode_a_law(pcm[i]); + } + return wav; + } + + if (audio_format == kFormatPcm && bits_per_sample == 32) { + const size_t sample_count = data.size() / sizeof(int32_t); + wav.samples.resize(sample_count); + const auto * pcm = reinterpret_cast(data.data()); + for (size_t i = 0; i < sample_count; ++i) { + wav.samples[i] = static_cast(pcm[i]) / 2147483648.0F; + } + return wav; + } + + if (audio_format == kFormatFloat && bits_per_sample == 64) { + const size_t sample_count = data.size() / sizeof(double); + wav.samples.resize(sample_count); + const auto * pcm = reinterpret_cast(data.data()); + for (size_t i = 0; i < sample_count; ++i) { + wav.samples[i] = static_cast(pcm[i]); + } + return wav; + } + if (audio_format == 1 && bits_per_sample == 16) { const size_t sample_count = data.size() / sizeof(int16_t); wav.samples.resize(sample_count); @@ -184,7 +333,10 @@ WavData read_wav_f32(std::istream & input) { return wav; } - throw std::runtime_error("unsupported WAV encoding (need PCM16, PCM24, or float32)"); + throw std::runtime_error( + "unsupported WAV encoding (" + describe_encoding(audio_format, bits_per_sample) + + "); supported: PCM 8/16/24/32-bit, float 32/64-bit, A-law and mu-law. " + "Convert with `ffmpeg -i input -ac 1 -ar 44100 -c:a pcm_s16le output.wav`"); } WavData read_wav_f32(std::string_view input) { diff --git a/tests/unittests/test_wav_reader.cpp b/tests/unittests/test_wav_reader.cpp index 80380bb2a..59e66fbd4 100644 --- a/tests/unittests/test_wav_reader.cpp +++ b/tests/unittests/test_wav_reader.cpp @@ -2,8 +2,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -79,6 +81,91 @@ void require_near(float actual, float expected, const std::string & label) { } } +// Writes a fmt chunk of `format_tag`/`bits` plus a data chunk of raw bytes. When +// `extensible` is set the chunk is the 40-byte WAVEFORMATEXTENSIBLE layout and +// `format_tag` moves into the SubFormat GUID, exactly as encoders emit it for +// multichannel or channel-masked PCM. +void write_wav( + const std::filesystem::path & path, + uint16_t format_tag, + uint16_t bits, + int sample_rate, + int channels, + const std::vector & payload, + bool extensible = false) { + const uint16_t block_align = static_cast(channels * ((bits + 7) / 8)); + const uint32_t byte_rate = static_cast(sample_rate) * block_align; + const uint32_t data_bytes = static_cast(payload.size()); + const uint32_t fmt_bytes = extensible ? 40u : 16u; + + std::ofstream output(path, std::ios::binary); + if (!output) { + throw std::runtime_error("failed to open test WAV: " + path.string()); + } + write_bytes(output, "RIFF", 4); + write_le(output, 20u + fmt_bytes + data_bytes); + write_bytes(output, "WAVE", 4); + write_bytes(output, "fmt ", 4); + write_le(output, fmt_bytes); + write_le(output, extensible ? uint16_t{0xFFFE} : format_tag); + write_le(output, static_cast(channels)); + write_le(output, static_cast(sample_rate)); + write_le(output, byte_rate); + write_le(output, block_align); + write_le(output, bits); + if (extensible) { + write_le(output, uint16_t{22}); // cbSize + write_le(output, bits); // wValidBitsPerSample + write_le(output, uint32_t{0x3}); // dwChannelMask + write_le(output, format_tag); // SubFormat GUID, first field + // Remainder of KSDATAFORMAT_SUBTYPE_*: 0000-0010-8000-00aa00389b71 + const char guid_tail[14] = { + 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, static_cast(0x80), + 0x00, 0x00, static_cast(0xAA), 0x00, 0x38, static_cast(0x9B), + 0x71, + }; + write_bytes(output, guid_tail, 14); + } + write_bytes(output, "data", 4); + write_le(output, data_bytes); + if (data_bytes > 0) { + write_bytes(output, payload.data(), static_cast(data_bytes)); + } +} + +template +std::vector to_bytes(const std::vector & values) { + std::vector out(values.size() * sizeof(T)); + if (!values.empty()) { + std::memcpy(out.data(), values.data(), out.size()); + } + return out; +} + +void require_near(float actual, float expected, float tolerance, const std::string & label) { + if (!std::isfinite(actual)) { + throw std::runtime_error(label + " is not finite"); + } + if (std::fabs(actual - expected) > tolerance) { + throw std::runtime_error(label + " mismatch"); + } +} + +// Runs `body` and requires it to throw with `needle` in the message. A silent +// success here would mean the reader accepted something it cannot decode. +void require_throws_containing( + const std::function & body, const std::string & needle, const std::string & label) { + try { + body(); + } catch (const std::exception & ex) { + if (std::string(ex.what()).find(needle) == std::string::npos) { + throw std::runtime_error(label + ": wrong message: " + ex.what()); + } + return; + } + throw std::runtime_error(label + ": expected a throw, got none"); +} + } // namespace int main() { @@ -107,6 +194,132 @@ int main() { require_near(wav.samples[2], -1.0F, "PCM24 min negative"); require_near(wav.samples[3], -1.0F / 8388608.0F, "PCM24 negative one"); + // --- WAVEFORMATEXTENSIBLE wrapping ordinary PCM16 --------------------- + // The case that actually bites: the payload is plain PCM16, but the + // format tag says 0xFFFE and the real tag lives in the SubFormat GUID. + // A reader that stops at the tag rejects a file it can decode. + { + const auto ext = root / "extensible_pcm16.wav"; + write_wav(ext, 0x0001, 16, 44100, 2, + to_bytes({0, 16384, -16384, -32768}), true); + const auto wav = engine::audio::read_wav_f32(ext); + require(wav.sample_rate == 44100, "EXTENSIBLE PCM16 sample rate mismatch"); + require(wav.channels == 2, "EXTENSIBLE PCM16 channel count mismatch"); + require(wav.samples.size() == 4, "EXTENSIBLE PCM16 sample count mismatch"); + require_near(wav.samples[0], 0.0F, 1.0e-7F, "EXTENSIBLE PCM16 zero"); + require_near(wav.samples[1], 0.5F, 1.0e-7F, "EXTENSIBLE PCM16 half"); + require_near(wav.samples[2], -0.5F, 1.0e-7F, "EXTENSIBLE PCM16 negative half"); + require_near(wav.samples[3], -1.0F, 1.0e-7F, "EXTENSIBLE PCM16 full negative"); + } + + // --- WAVEFORMATEXTENSIBLE wrapping float32 --------------------------- + // Proves the GUID is actually read rather than assumed to be PCM. + { + const auto ext = root / "extensible_f32.wav"; + write_wav(ext, 0x0003, 32, 48000, 1, + to_bytes({0.0F, 0.25F, -0.75F}), true); + const auto wav = engine::audio::read_wav_f32(ext); + require(wav.channels == 1, "EXTENSIBLE float32 channel count mismatch"); + require(wav.samples.size() == 3, "EXTENSIBLE float32 sample count mismatch"); + require_near(wav.samples[1], 0.25F, 1.0e-7F, "EXTENSIBLE float32 quarter"); + require_near(wav.samples[2], -0.75F, 1.0e-7F, "EXTENSIBLE float32 negative"); + } + + // --- PCM8 is unsigned, biased by 128 --------------------------------- + // The sign convention differs from every other PCM width, so a decoder + // that treats it as signed silently inverts the waveform. + { + const auto path8 = root / "pcm8.wav"; + write_wav(path8, 0x0001, 8, 8000, 1, + std::vector{static_cast(128), static_cast(255), + static_cast(0), static_cast(64)}); + const auto wav = engine::audio::read_wav_f32(path8); + require(wav.samples.size() == 4, "PCM8 sample count mismatch"); + require_near(wav.samples[0], 0.0F, 1.0e-7F, "PCM8 midpoint is silence"); + require_near(wav.samples[1], 127.0F / 128.0F, 1.0e-7F, "PCM8 max positive"); + require_near(wav.samples[2], -1.0F, 1.0e-7F, "PCM8 min negative"); + require_near(wav.samples[3], -0.5F, 1.0e-7F, "PCM8 quarter scale"); + } + + // --- PCM32 ----------------------------------------------------------- + { + const auto path32 = root / "pcm32.wav"; + write_wav(path32, 0x0001, 32, 96000, 1, + to_bytes({0, 1073741824, -2147483647 - 1})); + const auto wav = engine::audio::read_wav_f32(path32); + require(wav.sample_rate == 96000, "PCM32 sample rate mismatch"); + require(wav.samples.size() == 3, "PCM32 sample count mismatch"); + require_near(wav.samples[0], 0.0F, 1.0e-7F, "PCM32 zero"); + require_near(wav.samples[1], 0.5F, 1.0e-7F, "PCM32 half"); + require_near(wav.samples[2], -1.0F, 1.0e-7F, "PCM32 full negative"); + } + + // --- float64 --------------------------------------------------------- + { + const auto path64 = root / "float64.wav"; + write_wav(path64, 0x0003, 64, 44100, 2, + to_bytes({0.0, 0.125, -0.875, 1.0})); + const auto wav = engine::audio::read_wav_f32(path64); + require(wav.channels == 2, "float64 channel count mismatch"); + require(wav.samples.size() == 4, "float64 sample count mismatch"); + require_near(wav.samples[1], 0.125F, 1.0e-7F, "float64 eighth"); + require_near(wav.samples[2], -0.875F, 1.0e-7F, "float64 negative"); + require_near(wav.samples[3], 1.0F, 1.0e-7F, "float64 unity"); + } + + // --- G.711 mu-law ---------------------------------------------------- + // Anchored on the published G.711 decode values, not on our own + // implementation: 0x00 -> -32124, 0x80 -> +32124, and both 0x7F and 0xFF + // -> 0. Pinning against a re-derivation of the same bit-twiddling would + // prove nothing. + { + const auto path_mu = root / "mulaw.wav"; + write_wav(path_mu, 0x0007, 8, 8000, 1, + std::vector{static_cast(0xFF), static_cast(0x7F), + static_cast(0x00), static_cast(0x80)}); + const auto wav = engine::audio::read_wav_f32(path_mu); + require(wav.samples.size() == 4, "mu-law sample count mismatch"); + require_near(wav.samples[0], 0.0F, 1.0e-7F, "mu-law 0xFF is silence"); + require_near(wav.samples[1], 0.0F, 1.0e-7F, "mu-law 0x7F is silence"); + require_near(wav.samples[2], -32124.0F / 32768.0F, 1.0e-7F, "mu-law 0x00 minimum"); + require_near(wav.samples[3], 32124.0F / 32768.0F, 1.0e-7F, "mu-law 0x80 maximum"); + } + + // --- G.711 A-law ----------------------------------------------------- + // Published anchors again: 0x55 -> +8, 0xD5 -> -8, 0x2A -> +32256, + // 0xAA -> -32256. A-law has no exact zero, which is itself worth pinning. + { + const auto path_a = root / "alaw.wav"; + write_wav(path_a, 0x0006, 8, 8000, 1, + std::vector{static_cast(0x55), static_cast(0xD5), + static_cast(0x2A), static_cast(0xAA)}); + const auto wav = engine::audio::read_wav_f32(path_a); + require(wav.samples.size() == 4, "A-law sample count mismatch"); + require_near(wav.samples[0], 8.0F / 32768.0F, 1.0e-7F, "A-law 0x55 smallest positive"); + require_near(wav.samples[1], -8.0F / 32768.0F, 1.0e-7F, "A-law 0xD5 smallest negative"); + require_near(wav.samples[2], 32256.0F / 32768.0F, 1.0e-7F, "A-law 0x2A maximum"); + require_near(wav.samples[3], -32256.0F / 32768.0F, 1.0e-7F, "A-law 0xAA minimum"); + } + + // --- Still rejects what it genuinely cannot decode ------------------- + // Widening the accepted set must not turn into accepting everything. + { + const auto path_bad = root / "unsupported.wav"; + write_wav(path_bad, 0x0011, 4, 8000, 1, std::vector{0x01, 0x02}); + require_throws_containing( + [&] { (void)engine::audio::read_wav_f32(path_bad); }, + "unsupported WAV encoding", "ADPCM rejection"); + + const auto path_flac = root / "actually.flac"; + { + std::ofstream output(path_flac, std::ios::binary); + write_bytes(output, "fLaC\0\0\0\x22\0\0\0\0", 12); + } + require_throws_containing( + [&] { (void)engine::audio::read_wav_f32(path_flac); }, + "FLAC", "FLAC container identification"); + } + std::cout << "wav_reader_test passed\n"; } catch (const std::exception & ex) { std::cerr << "wav_reader_test failed: " << ex.what() << "\n"; From 3a50144d07b5395d9055b97f61e42b3dc776c0d1 Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Thu, 27 Aug 2026 16:18:44 +0000 Subject: [PATCH 2/3] fix(audio): correct the A-law sign and validate extensible headers Four defects found by review before merge. **A-law polarity was inverted on all 256 codes.** G.711 sets the sign bit for *positive* samples in A-law and for *negative* ones in mu-law; this treated both the same way. Decoded audio came out at the right amplitude, phase-inverted -- which is inaudible on its own and survives every spot check. The tests did not catch it because they were circular: four "published" anchors whose expected values had been worked out from the same shift-and-bias arithmetic under test, so they confirmed the bug rather than finding it. They are replaced with the full 256-entry decode tables for both codings, taken from outside this codebase -- ffmpeg 9.0.1 decoding a 256-byte file, cross-checked against the values implied by the ITU-T G.711 segment definitions. Reinstating the old sign now fails on `A-law code 0`. **The extensible SubFormat GUID was trusted on its first two bytes.** Only those carry the format tag; the remaining fourteen are a fixed suffix shared by every KSDATAFORMAT_SUBTYPE_*. Without checking them, an unrelated codec whose GUID merely starts 0x0001 decoded as PCM16. Now compared, and cbSize is required to be at least 22 as the structure demands. A sub-40-byte extensible fmt chunk gets a clear error instead of falling through with a stale format tag. **PCM32 and float64 silently dropped a trailing partial sample.** Integer division trimmed it, so a truncated download decoded as valid audio. Both now reject it, matching what PCM24 already did. PCM16 and float32 keep their existing behaviour -- tightening those is not this PR's business. Verified: full ctest green, an A-law file transcoded by ffmpeg works as a CLI --voice-ref end to end, and each new negative case fails against the code as it stood before this commit. --- src/framework/audio/wav_reader.cpp | 55 +++++++- tests/unittests/test_wav_reader.cpp | 198 +++++++++++++++++++++++----- 2 files changed, 215 insertions(+), 38 deletions(-) diff --git a/src/framework/audio/wav_reader.cpp b/src/framework/audio/wav_reader.cpp index a831eaacc..c22f35a1f 100644 --- a/src/framework/audio/wav_reader.cpp +++ b/src/framework/audio/wav_reader.cpp @@ -76,6 +76,13 @@ constexpr uint16_t kFormatALaw = 0x0006; constexpr uint16_t kFormatMuLaw = 0x0007; constexpr uint16_t kFormatExtensible = 0xFFFE; +// Bytes 2..15 of every KSDATAFORMAT_SUBTYPE_* GUID: +// XXXXXXXX-0000-0010-8000-00aa00389b71. +constexpr std::array kKsDataFormatSubtypeTail = { + 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, static_cast(0x80), + 0x00, 0x00, static_cast(0xAA), 0x00, 0x38, static_cast(0x9B), 0x71, +}; + // Names a container we can recognise but not decode, so the error can say what // the file actually is instead of "invalid WAV RIFF header". const char * identify_foreign_container(const std::array & header) { @@ -124,7 +131,11 @@ float decode_mu_law(uint8_t value) { float decode_a_law(uint8_t value) { value ^= 0x55; - const int sign = (value & 0x80) != 0 ? -1 : 1; + // Note the inversion relative to mu-law above: in A-law the sign bit marks a + // POSITIVE sample. Getting this backwards is silent -- the audio decodes at + // the right amplitude, just phase-inverted -- so it is pinned by an + // exhaustive 256-code table in the tests rather than by spot checks. + const int sign = (value & 0x80) != 0 ? 1 : -1; const int exponent = (value >> 4) & 0x07; const int mantissa = value & 0x0F; int magnitude = 0; @@ -194,13 +205,39 @@ WavData read_wav_f32(std::istream & input) { skip_bytes(input, 6); bits_per_sample = read_scalar(input); std::streamoff consumed = 16; - if (audio_format == kFormatExtensible && chunk_size >= 40) { - skip_bytes(input, 2); // cbSize + if (audio_format == kFormatExtensible) { + if (chunk_size < 40) { + throw std::runtime_error( + "malformed WAVEFORMATEXTENSIBLE fmt chunk (needs 40 bytes, got " + + std::to_string(chunk_size) + ")"); + } + const uint16_t cb_size = read_scalar(input); + if (cb_size < 22) { + throw std::runtime_error( + "malformed WAVEFORMATEXTENSIBLE fmt chunk (cbSize " + + std::to_string(cb_size) + ", needs at least 22)"); + } skip_bytes(input, 2); // wValidBitsPerSample skip_bytes(input, 4); // dwChannelMask - // The SubFormat GUID begins with the real format tag. - audio_format = read_scalar(input); - skip_bytes(input, 14); // remainder of the GUID + // Only the first two bytes of the SubFormat GUID carry the real + // format tag. The remaining fourteen are a fixed suffix shared by + // every KSDATAFORMAT_SUBTYPE_*; checking them is what separates a + // genuine format tag from an unrelated codec whose GUID merely + // happens to start with the same two bytes. + const uint16_t sub_format = read_scalar(input); + std::array guid_tail{}; + input.read(guid_tail.data(), static_cast(guid_tail.size())); + if (!input) { + throw std::runtime_error("truncated WAVEFORMATEXTENSIBLE SubFormat GUID"); + } + if (std::memcmp(guid_tail.data(), kKsDataFormatSubtypeTail.data(), + kKsDataFormatSubtypeTail.size()) != 0) { + throw std::runtime_error( + "unsupported WAV encoding (extensible SubFormat is not a " + "KSDATAFORMAT_SUBTYPE_* GUID); convert with " + "`ffmpeg -i input -ac 1 -ar 44100 -c:a pcm_s16le output.wav`"); + } + audio_format = sub_format; consumed = 40; } if (chunk_size > consumed) { @@ -273,6 +310,9 @@ WavData read_wav_f32(std::istream & input) { } if (audio_format == kFormatPcm && bits_per_sample == 32) { + if (data.size() % sizeof(int32_t) != 0) { + throw std::runtime_error("malformed PCM32 WAV data chunk"); + } const size_t sample_count = data.size() / sizeof(int32_t); wav.samples.resize(sample_count); const auto * pcm = reinterpret_cast(data.data()); @@ -283,6 +323,9 @@ WavData read_wav_f32(std::istream & input) { } if (audio_format == kFormatFloat && bits_per_sample == 64) { + if (data.size() % sizeof(double) != 0) { + throw std::runtime_error("malformed float64 WAV data chunk"); + } const size_t sample_count = data.size() / sizeof(double); wav.samples.resize(sample_count); const auto * pcm = reinterpret_cast(data.data()); diff --git a/tests/unittests/test_wav_reader.cpp b/tests/unittests/test_wav_reader.cpp index 59e66fbd4..501a86ae9 100644 --- a/tests/unittests/test_wav_reader.cpp +++ b/tests/unittests/test_wav_reader.cpp @@ -75,6 +75,88 @@ void write_pcm24_wav( } } +// G.711 decode tables, all 256 codes each. +// +// These are ground truth from OUTSIDE this codebase: produced by decoding a +// 256-byte A-law and mu-law file with ffmpeg 9.0.1, and independently matching +// the values reconstructed from the ITU-T G.711 segment definitions. They are +// deliberately NOT a re-derivation of the shift-and-bias arithmetic in +// wav_reader.cpp -- an earlier revision of this test spot-checked four codes +// whose expected values had been worked out from that same arithmetic, and so +// happily confirmed an A-law sign inversion across every one of the 256 codes. +// +// The two codings do not share a sign convention: mu-law sets the top bit for +// negative samples, A-law for positive ones. That is the trap. +constexpr int16_t kALawExpected[256] = { + -5504, -5248, -6016, -5760, -4480, -4224, -4992, -4736, + -7552, -7296, -8064, -7808, -6528, -6272, -7040, -6784, + -2752, -2624, -3008, -2880, -2240, -2112, -2496, -2368, + -3776, -3648, -4032, -3904, -3264, -3136, -3520, -3392, + -22016, -20992, -24064, -23040, -17920, -16896, -19968, -18944, + -30208, -29184, -32256, -31232, -26112, -25088, -28160, -27136, + -11008, -10496, -12032, -11520, -8960, -8448, -9984, -9472, + -15104, -14592, -16128, -15616, -13056, -12544, -14080, -13568, + -344, -328, -376, -360, -280, -264, -312, -296, + -472, -456, -504, -488, -408, -392, -440, -424, + -88, -72, -120, -104, -24, -8, -56, -40, + -216, -200, -248, -232, -152, -136, -184, -168, + -1376, -1312, -1504, -1440, -1120, -1056, -1248, -1184, + -1888, -1824, -2016, -1952, -1632, -1568, -1760, -1696, + -688, -656, -752, -720, -560, -528, -624, -592, + -944, -912, -1008, -976, -816, -784, -880, -848, + 5504, 5248, 6016, 5760, 4480, 4224, 4992, 4736, + 7552, 7296, 8064, 7808, 6528, 6272, 7040, 6784, + 2752, 2624, 3008, 2880, 2240, 2112, 2496, 2368, + 3776, 3648, 4032, 3904, 3264, 3136, 3520, 3392, + 22016, 20992, 24064, 23040, 17920, 16896, 19968, 18944, + 30208, 29184, 32256, 31232, 26112, 25088, 28160, 27136, + 11008, 10496, 12032, 11520, 8960, 8448, 9984, 9472, + 15104, 14592, 16128, 15616, 13056, 12544, 14080, 13568, + 344, 328, 376, 360, 280, 264, 312, 296, + 472, 456, 504, 488, 408, 392, 440, 424, + 88, 72, 120, 104, 24, 8, 56, 40, + 216, 200, 248, 232, 152, 136, 184, 168, + 1376, 1312, 1504, 1440, 1120, 1056, 1248, 1184, + 1888, 1824, 2016, 1952, 1632, 1568, 1760, 1696, + 688, 656, 752, 720, 560, 528, 624, 592, + 944, 912, 1008, 976, 816, 784, 880, 848, +}; + +constexpr int16_t kMuLawExpected[256] = { + -32124, -31100, -30076, -29052, -28028, -27004, -25980, -24956, + -23932, -22908, -21884, -20860, -19836, -18812, -17788, -16764, + -15996, -15484, -14972, -14460, -13948, -13436, -12924, -12412, + -11900, -11388, -10876, -10364, -9852, -9340, -8828, -8316, + -7932, -7676, -7420, -7164, -6908, -6652, -6396, -6140, + -5884, -5628, -5372, -5116, -4860, -4604, -4348, -4092, + -3900, -3772, -3644, -3516, -3388, -3260, -3132, -3004, + -2876, -2748, -2620, -2492, -2364, -2236, -2108, -1980, + -1884, -1820, -1756, -1692, -1628, -1564, -1500, -1436, + -1372, -1308, -1244, -1180, -1116, -1052, -988, -924, + -876, -844, -812, -780, -748, -716, -684, -652, + -620, -588, -556, -524, -492, -460, -428, -396, + -372, -356, -340, -324, -308, -292, -276, -260, + -244, -228, -212, -196, -180, -164, -148, -132, + -120, -112, -104, -96, -88, -80, -72, -64, + -56, -48, -40, -32, -24, -16, -8, 0, + 32124, 31100, 30076, 29052, 28028, 27004, 25980, 24956, + 23932, 22908, 21884, 20860, 19836, 18812, 17788, 16764, + 15996, 15484, 14972, 14460, 13948, 13436, 12924, 12412, + 11900, 11388, 10876, 10364, 9852, 9340, 8828, 8316, + 7932, 7676, 7420, 7164, 6908, 6652, 6396, 6140, + 5884, 5628, 5372, 5116, 4860, 4604, 4348, 4092, + 3900, 3772, 3644, 3516, 3388, 3260, 3132, 3004, + 2876, 2748, 2620, 2492, 2364, 2236, 2108, 1980, + 1884, 1820, 1756, 1692, 1628, 1564, 1500, 1436, + 1372, 1308, 1244, 1180, 1116, 1052, 988, 924, + 876, 844, 812, 780, 748, 716, 684, 652, + 620, 588, 556, 524, 492, 460, 428, 396, + 372, 356, 340, 324, 308, 292, 276, 260, + 244, 228, 212, 196, 180, 164, 148, 132, + 120, 112, 104, 96, 88, 80, 72, 64, + 56, 48, 40, 32, 24, 16, 8, 0, +}; + void require_near(float actual, float expected, const std::string & label) { if (std::fabs(actual - expected) > 1.0e-7F) { throw std::runtime_error(label + " mismatch"); @@ -92,7 +174,9 @@ void write_wav( int sample_rate, int channels, const std::vector & payload, - bool extensible = false) { + bool extensible = false, + bool valid_guid_tail = true, + uint16_t cb_size = 22) { const uint16_t block_align = static_cast(channels * ((bits + 7) / 8)); const uint32_t byte_rate = static_cast(sample_rate) * block_align; const uint32_t data_bytes = static_cast(payload.size()); @@ -114,17 +198,22 @@ void write_wav( write_le(output, block_align); write_le(output, bits); if (extensible) { - write_le(output, uint16_t{22}); // cbSize + write_le(output, cb_size); // cbSize write_le(output, bits); // wValidBitsPerSample write_le(output, uint32_t{0x3}); // dwChannelMask write_le(output, format_tag); // SubFormat GUID, first field // Remainder of KSDATAFORMAT_SUBTYPE_*: 0000-0010-8000-00aa00389b71 - const char guid_tail[14] = { + const char valid_tail[14] = { 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, static_cast(0x80), 0x00, 0x00, static_cast(0xAA), 0x00, 0x38, static_cast(0x9B), 0x71, }; - write_bytes(output, guid_tail, 14); + const char foreign_tail[14] = { + static_cast(0xBE), static_cast(0xEF), static_cast(0xDE), + static_cast(0xAD), 0x42, 0x41, 0x44, 0x47, 0x55, 0x49, 0x44, 0x21, + 0x00, 0x00, + }; + write_bytes(output, valid_guid_tail ? valid_tail : foreign_tail, 14); } write_bytes(output, "data", 4); write_le(output, data_bytes); @@ -267,38 +356,83 @@ int main() { require_near(wav.samples[3], 1.0F, 1.0e-7F, "float64 unity"); } - // --- G.711 mu-law ---------------------------------------------------- - // Anchored on the published G.711 decode values, not on our own - // implementation: 0x00 -> -32124, 0x80 -> +32124, and both 0x7F and 0xFF - // -> 0. Pinning against a re-derivation of the same bit-twiddling would - // prove nothing. + // --- G.711 mu-law and A-law, every code --------------------------- + // One byte per code, decoded in one pass and compared against the + // external tables above. A spot check cannot catch a whole-table sign + // inversion; this can. { - const auto path_mu = root / "mulaw.wav"; - write_wav(path_mu, 0x0007, 8, 8000, 1, - std::vector{static_cast(0xFF), static_cast(0x7F), - static_cast(0x00), static_cast(0x80)}); - const auto wav = engine::audio::read_wav_f32(path_mu); - require(wav.samples.size() == 4, "mu-law sample count mismatch"); - require_near(wav.samples[0], 0.0F, 1.0e-7F, "mu-law 0xFF is silence"); - require_near(wav.samples[1], 0.0F, 1.0e-7F, "mu-law 0x7F is silence"); - require_near(wav.samples[2], -32124.0F / 32768.0F, 1.0e-7F, "mu-law 0x00 minimum"); - require_near(wav.samples[3], 32124.0F / 32768.0F, 1.0e-7F, "mu-law 0x80 maximum"); + std::vector codes(256); + for (int i = 0; i < 256; ++i) { + codes[static_cast(i)] = static_cast(i); + } + + const auto path_mu = root / "mulaw_all.wav"; + write_wav(path_mu, 0x0007, 8, 8000, 1, codes); + const auto mu = engine::audio::read_wav_f32(path_mu); + require(mu.samples.size() == 256, "mu-law sample count mismatch"); + for (int i = 0; i < 256; ++i) { + require_near( + mu.samples[static_cast(i)], + static_cast(kMuLawExpected[i]) / 32768.0F, + 1.0e-7F, + "mu-law code " + std::to_string(i)); + } + + const auto path_a = root / "alaw_all.wav"; + write_wav(path_a, 0x0006, 8, 8000, 1, codes); + const auto alaw = engine::audio::read_wav_f32(path_a); + require(alaw.samples.size() == 256, "A-law sample count mismatch"); + for (int i = 0; i < 256; ++i) { + require_near( + alaw.samples[static_cast(i)], + static_cast(kALawExpected[i]) / 32768.0F, + 1.0e-7F, + "A-law code " + std::to_string(i)); + } } - // --- G.711 A-law ----------------------------------------------------- - // Published anchors again: 0x55 -> +8, 0xD5 -> -8, 0x2A -> +32256, - // 0xAA -> -32256. A-law has no exact zero, which is itself worth pinning. + // --- Extensible headers are validated, not trusted ------------------- + // Only the first two bytes of the SubFormat GUID are the format tag. The + // other fourteen are a fixed suffix; without checking them, any codec + // whose GUID happens to start with 0x0001 decodes as PCM. { - const auto path_a = root / "alaw.wav"; - write_wav(path_a, 0x0006, 8, 8000, 1, - std::vector{static_cast(0x55), static_cast(0xD5), - static_cast(0x2A), static_cast(0xAA)}); - const auto wav = engine::audio::read_wav_f32(path_a); - require(wav.samples.size() == 4, "A-law sample count mismatch"); - require_near(wav.samples[0], 8.0F / 32768.0F, 1.0e-7F, "A-law 0x55 smallest positive"); - require_near(wav.samples[1], -8.0F / 32768.0F, 1.0e-7F, "A-law 0xD5 smallest negative"); - require_near(wav.samples[2], 32256.0F / 32768.0F, 1.0e-7F, "A-law 0x2A maximum"); - require_near(wav.samples[3], -32256.0F / 32768.0F, 1.0e-7F, "A-law 0xAA minimum"); + const auto bad_guid = root / "extensible_foreign_guid.wav"; + write_wav(bad_guid, 0x0001, 16, 44100, 1, + to_bytes({0, 16384}), true, /*valid_guid_tail=*/false); + require_throws_containing( + [&] { (void)engine::audio::read_wav_f32(bad_guid); }, + "KSDATAFORMAT_SUBTYPE", "foreign SubFormat GUID rejection"); + + // WAVEFORMATEXTENSIBLE requires cbSize >= 22; anything less is an + // internally inconsistent header. + const auto bad_cb = root / "extensible_bad_cbsize.wav"; + write_wav(bad_cb, 0x0001, 16, 44100, 1, + to_bytes({0, 16384}), true, true, /*cb_size=*/0); + require_throws_containing( + [&] { (void)engine::audio::read_wav_f32(bad_cb); }, + "cbSize", "extensible cbSize rejection"); + } + + // --- Truncated sample data is an error, not a silent trim ------------ + // PCM24 already rejected a partial trailing sample; PCM32 and float64 + // divided and dropped it, which turns a truncated download into audio + // that looks fine. + { + auto pcm32 = to_bytes({1073741824}); + pcm32.push_back(0x7F); + const auto path32 = root / "pcm32_partial.wav"; + write_wav(path32, 0x0001, 32, 44100, 1, pcm32); + require_throws_containing( + [&] { (void)engine::audio::read_wav_f32(path32); }, + "malformed PCM32", "PCM32 partial sample rejection"); + + auto f64 = to_bytes({0.25}); + f64.push_back(static_cast(0xAA)); + const auto path64 = root / "float64_partial.wav"; + write_wav(path64, 0x0003, 64, 44100, 1, f64); + require_throws_containing( + [&] { (void)engine::audio::read_wav_f32(path64); }, + "malformed float64", "float64 partial sample rejection"); } // --- Still rejects what it genuinely cannot decode ------------------- From 6d31e3285baa0ce4839224b1c228635755e6a5f4 Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Thu, 27 Aug 2026 16:50:26 +0000 Subject: [PATCH 3/3] fix(audio): stop the header sniff and the RIFF pad from breaking valid files Two defects a second reviewer found in the parse loop, both introduced by the first commit in this PR, and both invisible to tests that only ever read from a file path. **The 12-byte header sniff rewound absolutely.** After reading the RIFF header for container identification it did `seekg(header_read, beg)`, which is redundant for a stream that started at offset 0, wrong for one that did not, and impossible for one that cannot seek. `read_wav_f32(std::istream &)` is public; handed a pipe it set failbit and reported `incomplete WAV file` for a perfectly good WAV. The bytes are already consumed, so the rewind is simply removed. **The RIFF pad byte was required at EOF.** After an odd-sized chunk the reader always seeks one byte. Seeking past the end is legal on an ifstream but not on the in-memory buffer behind the string_view overload, so the same bytes parsed from disk and threw from an upload. Plenty of writers omit the final pad, and this PR is what makes it matter: PCM8, A-law and mu-law are one byte per sample, so odd data chunks go from rare to routine. A pad byte carries no data, so a missing one at EOF now ends the chunk loop instead of failing. Also rejects a `fmt ` chunk shorter than 16 bytes, which previously read on into whatever followed. The PCM8, PCM32 and float64 expectations were still derived from the implementation, the same construction that hid the A-law inversion. They are now frozen from `ffmpeg -f f32le` output and extended to the endpoints that distinguish a correct conversion from a plausible one: INT32_MAX, which float32 rounding maps to exactly 1.0, and a float64 value not representable in float32. Verified: reinstating any of the three defects fails the suite with the matching message; full ctest green; and our decode of a real ffmpeg-transcoded A-law file matches ffmpeg on all 153280 samples with zero mismatches. --- src/framework/audio/wav_reader.cpp | 22 ++++- tests/unittests/test_wav_reader.cpp | 124 ++++++++++++++++++++++------ 2 files changed, 117 insertions(+), 29 deletions(-) diff --git a/src/framework/audio/wav_reader.cpp b/src/framework/audio/wav_reader.cpp index c22f35a1f..807f1994a 100644 --- a/src/framework/audio/wav_reader.cpp +++ b/src/framework/audio/wav_reader.cpp @@ -167,11 +167,15 @@ WavData read_wav_f32(std::istream & input) { throw std::runtime_error("could not open WAV input"); } + // Consume the 12-byte RIFF header once and keep it: it doubles as the magic + // for naming a non-WAV container below. Deliberately no rewind afterwards -- + // these bytes are spent, and an absolute seek back to 12 would be wrong for + // an istream that did not begin at offset 0 and impossible for one that + // cannot seek at all, such as a pipe. std::array header{}; input.read(header.data(), static_cast(header.size())); const auto header_read = static_cast(input.gcount()); input.clear(); - input.seekg(static_cast(header_read), std::ios::beg); if (header_read < 12 || std::memcmp(header.data(), "RIFF", 4) != 0 || std::memcmp(header.data() + 8, "WAVE", 4) != 0) { @@ -199,6 +203,11 @@ WavData read_wav_f32(std::istream & input) { const uint32_t chunk_size = read_scalar(input); const std::string id(chunk_id, 4); if (id == "fmt ") { + if (chunk_size < 16) { + throw std::runtime_error( + "malformed WAV fmt chunk (needs 16 bytes, got " + + std::to_string(chunk_size) + ")"); + } audio_format = read_scalar(input); channels = read_scalar(input); sample_rate = read_scalar(input); @@ -269,7 +278,16 @@ WavData read_wav_f32(std::istream & input) { skip_bytes(input, chunk_size); } if (chunk_size % 2 == 1) { - skip_bytes(input, 1); + // RIFF pads an odd-sized chunk to an even boundary, but plenty of + // writers omit that byte when the chunk is the last thing in the + // file. It carries no data, so a missing one at EOF is not an error. + // This matters more than it used to: PCM8, A-law and mu-law are one + // byte per sample, so odd data chunks are now common. + input.seekg(1, std::ios::cur); + if (!input) { + input.clear(); + break; + } } } diff --git a/tests/unittests/test_wav_reader.cpp b/tests/unittests/test_wav_reader.cpp index 501a86ae9..e174a119f 100644 --- a/tests/unittests/test_wav_reader.cpp +++ b/tests/unittests/test_wav_reader.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -75,18 +76,14 @@ void write_pcm24_wav( } } -// G.711 decode tables, all 256 codes each. +// G.711 decode tables, all 256 codes each. Ground truth from outside this +// codebase: ffmpeg 9.0.1 decoding a 256-byte file of every code, matching the +// values implied by the ITU-T G.711 segment definitions. // -// These are ground truth from OUTSIDE this codebase: produced by decoding a -// 256-byte A-law and mu-law file with ffmpeg 9.0.1, and independently matching -// the values reconstructed from the ITU-T G.711 segment definitions. They are -// deliberately NOT a re-derivation of the shift-and-bias arithmetic in -// wav_reader.cpp -- an earlier revision of this test spot-checked four codes -// whose expected values had been worked out from that same arithmetic, and so -// happily confirmed an A-law sign inversion across every one of the 256 codes. -// -// The two codings do not share a sign convention: mu-law sets the top bit for -// negative samples, A-law for positive ones. That is the trap. +// Every expected value in this file must come from an external decoder, never +// from the arithmetic in wav_reader.cpp. The two codings do not share a sign +// convention -- mu-law sets the top bit for negative samples, A-law for +// positive ones -- and a table derived from the implementation cannot see that. constexpr int16_t kALawExpected[256] = { -5504, -5248, -6016, -5760, -4480, -4224, -4992, -4736, -7552, -7296, -8064, -7808, -6528, -6272, -7040, -6784, @@ -321,39 +318,52 @@ int main() { const auto path8 = root / "pcm8.wav"; write_wav(path8, 0x0001, 8, 8000, 1, std::vector{static_cast(128), static_cast(255), - static_cast(0), static_cast(64)}); + static_cast(0), static_cast(64), + static_cast(1)}); const auto wav = engine::audio::read_wav_f32(path8); - require(wav.samples.size() == 4, "PCM8 sample count mismatch"); - require_near(wav.samples[0], 0.0F, 1.0e-7F, "PCM8 midpoint is silence"); - require_near(wav.samples[1], 127.0F / 128.0F, 1.0e-7F, "PCM8 max positive"); - require_near(wav.samples[2], -1.0F, 1.0e-7F, "PCM8 min negative"); - require_near(wav.samples[3], -0.5F, 1.0e-7F, "PCM8 quarter scale"); + require(wav.samples.size() == 5, "PCM8 sample count mismatch"); + // Frozen from `ffmpeg -i pcm8.wav -f f32le -`. Note the endpoints are + // asymmetric: 255 is 127/128, not 1.0. ffmpeg agrees. + const float expected8[5] = {0.0F, 0.9921875F, -1.0F, -0.5F, -0.9921875F}; + for (int i = 0; i < 5; ++i) { + require_near(wav.samples[static_cast(i)], expected8[i], 1.0e-7F, + "PCM8 sample " + std::to_string(i)); + } } // --- PCM32 ----------------------------------------------------------- { const auto path32 = root / "pcm32.wav"; write_wav(path32, 0x0001, 32, 96000, 1, - to_bytes({0, 1073741824, -2147483647 - 1})); + to_bytes({0, 1073741824, -2147483647 - 1, 2147483647, 1})); const auto wav = engine::audio::read_wav_f32(path32); require(wav.sample_rate == 96000, "PCM32 sample rate mismatch"); - require(wav.samples.size() == 3, "PCM32 sample count mismatch"); - require_near(wav.samples[0], 0.0F, 1.0e-7F, "PCM32 zero"); - require_near(wav.samples[1], 0.5F, 1.0e-7F, "PCM32 half"); - require_near(wav.samples[2], -1.0F, 1.0e-7F, "PCM32 full negative"); + require(wav.samples.size() == 5, "PCM32 sample count mismatch"); + // Frozen from ffmpeg. INT32_MAX lands on exactly 1.0 rather than + // 2147483647/2^31, because float32's ULP at that magnitude is 256 and + // the cast rounds up before the divide. ffmpeg does the same. + const float expected32[5] = {0.0F, 0.5F, -1.0F, 1.0F, 4.656612873077393e-10F}; + for (int i = 0; i < 5; ++i) { + require_near(wav.samples[static_cast(i)], expected32[i], 1.0e-12F, + "PCM32 sample " + std::to_string(i)); + } } // --- float64 --------------------------------------------------------- { const auto path64 = root / "float64.wav"; write_wav(path64, 0x0003, 64, 44100, 2, - to_bytes({0.0, 0.125, -0.875, 1.0})); + to_bytes({0.0, 0.125, -0.875, 1.0, 1.0 + 0x1p-30})); const auto wav = engine::audio::read_wav_f32(path64); require(wav.channels == 2, "float64 channel count mismatch"); - require(wav.samples.size() == 4, "float64 sample count mismatch"); - require_near(wav.samples[1], 0.125F, 1.0e-7F, "float64 eighth"); - require_near(wav.samples[2], -0.875F, 1.0e-7F, "float64 negative"); - require_near(wav.samples[3], 1.0F, 1.0e-7F, "float64 unity"); + require(wav.samples.size() == 5, "float64 sample count mismatch"); + // Frozen from ffmpeg. The last one is deliberately not representable + // in float32 and collapses to 1.0 on both sides. + const float expected64[5] = {0.0F, 0.125F, -0.875F, 1.0F, 1.0F}; + for (int i = 0; i < 5; ++i) { + require_near(wav.samples[static_cast(i)], expected64[i], 1.0e-7F, + "float64 sample " + std::to_string(i)); + } } // --- G.711 mu-law and A-law, every code --------------------------- @@ -435,6 +445,66 @@ int main() { "malformed float64", "float64 partial sample rejection"); } + // --- All three overloads agree, including on a missing final pad ----- + // RIFF pads odd-sized chunks, but writers routinely omit the byte when + // the chunk ends the file, and one-byte-per-sample formats make odd data + // chunks common. The path overload tolerated this because seeking past + // EOF is legal on an ifstream; the in-memory overload did not, so the + // same bytes parsed from a file and rejected from an upload buffer. + { + const auto odd = root / "pcm8_odd_no_pad.wav"; + write_wav(odd, 0x0001, 8, 8000, 1, + std::vector{static_cast(0), static_cast(128), + static_cast(254)}); + std::ifstream in(odd, std::ios::binary); + const std::string blob((std::istreambuf_iterator(in)), + std::istreambuf_iterator()); + require(blob.size() % 2 == 1, "test file should end without the pad byte"); + + const auto from_path = engine::audio::read_wav_f32(odd); + const auto from_memory = engine::audio::read_wav_f32(std::string_view(blob)); + require(from_path.samples.size() == 3, "odd-size PCM8 sample count from path"); + require(from_memory.samples.size() == 3, "odd-size PCM8 sample count from memory"); + for (size_t i = 0; i < from_path.samples.size(); ++i) { + require_near(from_memory.samples[i], from_path.samples[i], 0.0F, + "path and memory overloads disagree at " + std::to_string(i)); + } + + // The istream overload must not assume the stream began at offset 0, + // which an absolute rewind after the header sniff would. + std::istringstream prefixed(std::string("PREFIX!!") + blob, std::ios::binary); + prefixed.seekg(8); + const auto from_stream = engine::audio::read_wav_f32(prefixed); + require(from_stream.samples.size() == 3, "offset istream sample count"); + require_near(from_stream.samples[0], from_path.samples[0], 0.0F, + "offset istream disagrees with path"); + } + + // --- A short fmt chunk is an error, not a read into the next chunk ---- + { + const auto short_fmt = root / "short_fmt.wav"; + { + std::ofstream out(short_fmt, std::ios::binary); + const std::vector payload = to_bytes({0, 16384}); + write_bytes(out, "RIFF", 4); + write_le(out, 20u + 14u + static_cast(payload.size())); + write_bytes(out, "WAVE", 4); + write_bytes(out, "fmt ", 4); + write_le(out, 14u); // one field short of the minimum + write_le(out, uint16_t{1}); + write_le(out, uint16_t{1}); + write_le(out, uint32_t{44100}); + write_le(out, uint32_t{88200}); + write_le(out, uint16_t{2}); + write_bytes(out, "data", 4); + write_le(out, static_cast(payload.size())); + write_bytes(out, payload.data(), static_cast(payload.size())); + } + require_throws_containing( + [&] { (void)engine::audio::read_wav_f32(short_fmt); }, + "malformed WAV fmt chunk", "short fmt chunk rejection"); + } + // --- Still rejects what it genuinely cannot decode ------------------- // Widening the accepted set must not turn into accepting everything. {