Skip to content

Commit af8cb75

Browse files
authored
Accept PCM8/32, float64, A-law, mu-law and WAVEFORMATEXTENSIBLE in the WAV reader (#319)
* 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. * 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. * 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.
1 parent 3ce15a6 commit af8cb75

2 files changed

Lines changed: 643 additions & 13 deletions

File tree

src/framework/audio/wav_reader.cpp

Lines changed: 226 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
#include "engine/framework/audio/wav_reader.h"
22

3+
#include <algorithm>
4+
#include <array>
35
#include <cstdint>
6+
#include <cstring>
47
#include <fstream>
58
#include <stdexcept>
69
#include <string>
@@ -63,24 +66,127 @@ void skip_bytes(std::istream & input, std::streamoff count) {
6366
}
6467
}
6568

69+
// WAVE format tags. EXTENSIBLE is the one that matters in practice: many
70+
// encoders emit it for ordinary PCM16 whenever there are more than two channels
71+
// or a channel mask is set, and the real codec then lives in a SubFormat GUID
72+
// rather than in the format tag itself.
73+
constexpr uint16_t kFormatPcm = 0x0001;
74+
constexpr uint16_t kFormatFloat = 0x0003;
75+
constexpr uint16_t kFormatALaw = 0x0006;
76+
constexpr uint16_t kFormatMuLaw = 0x0007;
77+
constexpr uint16_t kFormatExtensible = 0xFFFE;
78+
79+
// Bytes 2..15 of every KSDATAFORMAT_SUBTYPE_* GUID:
80+
// XXXXXXXX-0000-0010-8000-00aa00389b71.
81+
constexpr std::array<char, 14> kKsDataFormatSubtypeTail = {
82+
0x00, 0x00, 0x00, 0x00, 0x10, 0x00, static_cast<char>(0x80),
83+
0x00, 0x00, static_cast<char>(0xAA), 0x00, 0x38, static_cast<char>(0x9B), 0x71,
84+
};
85+
86+
// Names a container we can recognise but not decode, so the error can say what
87+
// the file actually is instead of "invalid WAV RIFF header".
88+
const char * identify_foreign_container(const std::array<char, 12> & header) {
89+
const auto * bytes = reinterpret_cast<const uint8_t *>(header.data());
90+
if (std::memcmp(header.data(), "fLaC", 4) == 0) {
91+
return "FLAC";
92+
}
93+
if (std::memcmp(header.data(), "OggS", 4) == 0) {
94+
return "Ogg (Vorbis/Opus)";
95+
}
96+
if (std::memcmp(header.data(), "ID3", 3) == 0) {
97+
return "MP3";
98+
}
99+
// MPEG audio frame sync: 11 set bits.
100+
if (bytes[0] == 0xFF && (bytes[1] & 0xE0) == 0xE0) {
101+
return "MP3";
102+
}
103+
if (std::memcmp(header.data() + 4, "ftyp", 4) == 0) {
104+
return "MP4/M4A (AAC or ALAC)";
105+
}
106+
if (std::memcmp(header.data(), "FORM", 4) == 0) {
107+
return "AIFF";
108+
}
109+
if (std::memcmp(header.data(), "RF64", 4) == 0) {
110+
return "RF64";
111+
}
112+
if (std::memcmp(header.data(), "caff", 4) == 0) {
113+
return "CAF";
114+
}
115+
if (bytes[0] == 0x1A && bytes[1] == 0x45 && bytes[2] == 0xDF && bytes[3] == 0xA3) {
116+
return "Matroska/WebM";
117+
}
118+
return nullptr;
119+
}
120+
121+
// G.711 expansion. Both are 8-bit logarithmic codings still common in
122+
// telephony recordings and in WAVs produced by conferencing tools.
123+
float decode_mu_law(uint8_t value) {
124+
value = static_cast<uint8_t>(~value);
125+
const int sign = (value & 0x80) != 0 ? -1 : 1;
126+
const int exponent = (value >> 4) & 0x07;
127+
const int mantissa = value & 0x0F;
128+
const int magnitude = ((mantissa << 3) + 0x84) << exponent;
129+
return static_cast<float>(sign * (magnitude - 0x84)) / 32768.0F;
130+
}
131+
132+
float decode_a_law(uint8_t value) {
133+
value ^= 0x55;
134+
// Note the inversion relative to mu-law above: in A-law the sign bit marks a
135+
// POSITIVE sample. Getting this backwards is silent -- the audio decodes at
136+
// the right amplitude, just phase-inverted -- so it is pinned by an
137+
// exhaustive 256-code table in the tests rather than by spot checks.
138+
const int sign = (value & 0x80) != 0 ? 1 : -1;
139+
const int exponent = (value >> 4) & 0x07;
140+
const int mantissa = value & 0x0F;
141+
int magnitude = 0;
142+
if (exponent == 0) {
143+
magnitude = (mantissa << 4) + 8;
144+
} else {
145+
magnitude = ((mantissa << 4) + 0x108) << (exponent - 1);
146+
}
147+
return static_cast<float>(sign * magnitude) / 32768.0F;
148+
}
149+
150+
std::string describe_encoding(uint16_t format, uint16_t bits) {
151+
std::string name;
152+
switch (format) {
153+
case kFormatPcm: name = "PCM"; break;
154+
case kFormatFloat: name = "IEEE float"; break;
155+
case kFormatALaw: name = "A-law"; break;
156+
case kFormatMuLaw: name = "mu-law"; break;
157+
case kFormatExtensible: name = "extensible"; break;
158+
default: name = "format tag " + std::to_string(format); break;
159+
}
160+
return name + ", " + std::to_string(bits) + "-bit";
161+
}
162+
66163
} // namespace
67164

68165
WavData read_wav_f32(std::istream & input) {
69166
if (!input) {
70167
throw std::runtime_error("could not open WAV input");
71168
}
72169

73-
char riff[4];
74-
input.read(riff, 4);
75-
if (!input || std::string(riff, 4) != "RIFF") {
170+
// Consume the 12-byte RIFF header once and keep it: it doubles as the magic
171+
// for naming a non-WAV container below. Deliberately no rewind afterwards --
172+
// these bytes are spent, and an absolute seek back to 12 would be wrong for
173+
// an istream that did not begin at offset 0 and impossible for one that
174+
// cannot seek at all, such as a pipe.
175+
std::array<char, 12> header{};
176+
input.read(header.data(), static_cast<std::streamsize>(header.size()));
177+
const auto header_read = static_cast<size_t>(input.gcount());
178+
input.clear();
179+
180+
if (header_read < 12 || std::memcmp(header.data(), "RIFF", 4) != 0 ||
181+
std::memcmp(header.data() + 8, "WAVE", 4) != 0) {
182+
if (const char * container = identify_foreign_container(header)) {
183+
throw std::runtime_error(
184+
std::string("input is ") + container +
185+
", not WAV; convert it first, e.g. "
186+
"`ffmpeg -i input -ac 1 -ar 44100 -c:a pcm_s16le output.wav`");
187+
}
76188
throw std::runtime_error("invalid WAV RIFF header");
77189
}
78-
skip_bytes(input, 4);
79-
char wave[4];
80-
input.read(wave, 4);
81-
if (!input || std::string(wave, 4) != "WAVE") {
82-
throw std::runtime_error("invalid WAV WAVE header");
83-
}
84190

85191
uint16_t audio_format = 0;
86192
uint16_t channels = 0;
@@ -97,13 +203,54 @@ WavData read_wav_f32(std::istream & input) {
97203
const uint32_t chunk_size = read_scalar<uint32_t>(input);
98204
const std::string id(chunk_id, 4);
99205
if (id == "fmt ") {
206+
if (chunk_size < 16) {
207+
throw std::runtime_error(
208+
"malformed WAV fmt chunk (needs 16 bytes, got " +
209+
std::to_string(chunk_size) + ")");
210+
}
100211
audio_format = read_scalar<uint16_t>(input);
101212
channels = read_scalar<uint16_t>(input);
102213
sample_rate = read_scalar<uint32_t>(input);
103214
skip_bytes(input, 6);
104215
bits_per_sample = read_scalar<uint16_t>(input);
105-
if (chunk_size > 16) {
106-
skip_bytes(input, static_cast<std::streamoff>(chunk_size - 16));
216+
std::streamoff consumed = 16;
217+
if (audio_format == kFormatExtensible) {
218+
if (chunk_size < 40) {
219+
throw std::runtime_error(
220+
"malformed WAVEFORMATEXTENSIBLE fmt chunk (needs 40 bytes, got " +
221+
std::to_string(chunk_size) + ")");
222+
}
223+
const uint16_t cb_size = read_scalar<uint16_t>(input);
224+
if (cb_size < 22) {
225+
throw std::runtime_error(
226+
"malformed WAVEFORMATEXTENSIBLE fmt chunk (cbSize " +
227+
std::to_string(cb_size) + ", needs at least 22)");
228+
}
229+
skip_bytes(input, 2); // wValidBitsPerSample
230+
skip_bytes(input, 4); // dwChannelMask
231+
// Only the first two bytes of the SubFormat GUID carry the real
232+
// format tag. The remaining fourteen are a fixed suffix shared by
233+
// every KSDATAFORMAT_SUBTYPE_*; checking them is what separates a
234+
// genuine format tag from an unrelated codec whose GUID merely
235+
// happens to start with the same two bytes.
236+
const uint16_t sub_format = read_scalar<uint16_t>(input);
237+
std::array<char, 14> guid_tail{};
238+
input.read(guid_tail.data(), static_cast<std::streamsize>(guid_tail.size()));
239+
if (!input) {
240+
throw std::runtime_error("truncated WAVEFORMATEXTENSIBLE SubFormat GUID");
241+
}
242+
if (std::memcmp(guid_tail.data(), kKsDataFormatSubtypeTail.data(),
243+
kKsDataFormatSubtypeTail.size()) != 0) {
244+
throw std::runtime_error(
245+
"unsupported WAV encoding (extensible SubFormat is not a "
246+
"KSDATAFORMAT_SUBTYPE_* GUID); convert with "
247+
"`ffmpeg -i input -ac 1 -ar 44100 -c:a pcm_s16le output.wav`");
248+
}
249+
audio_format = sub_format;
250+
consumed = 40;
251+
}
252+
if (chunk_size > consumed) {
253+
skip_bytes(input, static_cast<std::streamoff>(chunk_size) - consumed);
107254
}
108255
} else if (id == "data") {
109256
// chunk_size is a 32-bit field read straight from the file, so a
@@ -131,7 +278,16 @@ WavData read_wav_f32(std::istream & input) {
131278
skip_bytes(input, chunk_size);
132279
}
133280
if (chunk_size % 2 == 1) {
134-
skip_bytes(input, 1);
281+
// RIFF pads an odd-sized chunk to an even boundary, but plenty of
282+
// writers omit that byte when the chunk is the last thing in the
283+
// file. It carries no data, so a missing one at EOF is not an error.
284+
// This matters more than it used to: PCM8, A-law and mu-law are one
285+
// byte per sample, so odd data chunks are now common.
286+
input.seekg(1, std::ios::cur);
287+
if (!input) {
288+
input.clear();
289+
break;
290+
}
135291
}
136292
}
137293

@@ -143,6 +299,60 @@ WavData read_wav_f32(std::istream & input) {
143299
wav.sample_rate = static_cast<int>(sample_rate);
144300
wav.channels = static_cast<int>(channels);
145301

302+
if (audio_format == kFormatPcm && bits_per_sample == 8) {
303+
// 8-bit PCM in WAV is unsigned, offset by 128.
304+
wav.samples.resize(data.size());
305+
const auto * pcm = reinterpret_cast<const uint8_t *>(data.data());
306+
for (size_t i = 0; i < data.size(); ++i) {
307+
wav.samples[i] = (static_cast<float>(pcm[i]) - 128.0F) / 128.0F;
308+
}
309+
return wav;
310+
}
311+
312+
if (audio_format == kFormatMuLaw && bits_per_sample == 8) {
313+
wav.samples.resize(data.size());
314+
const auto * pcm = reinterpret_cast<const uint8_t *>(data.data());
315+
for (size_t i = 0; i < data.size(); ++i) {
316+
wav.samples[i] = decode_mu_law(pcm[i]);
317+
}
318+
return wav;
319+
}
320+
321+
if (audio_format == kFormatALaw && bits_per_sample == 8) {
322+
wav.samples.resize(data.size());
323+
const auto * pcm = reinterpret_cast<const uint8_t *>(data.data());
324+
for (size_t i = 0; i < data.size(); ++i) {
325+
wav.samples[i] = decode_a_law(pcm[i]);
326+
}
327+
return wav;
328+
}
329+
330+
if (audio_format == kFormatPcm && bits_per_sample == 32) {
331+
if (data.size() % sizeof(int32_t) != 0) {
332+
throw std::runtime_error("malformed PCM32 WAV data chunk");
333+
}
334+
const size_t sample_count = data.size() / sizeof(int32_t);
335+
wav.samples.resize(sample_count);
336+
const auto * pcm = reinterpret_cast<const int32_t *>(data.data());
337+
for (size_t i = 0; i < sample_count; ++i) {
338+
wav.samples[i] = static_cast<float>(pcm[i]) / 2147483648.0F;
339+
}
340+
return wav;
341+
}
342+
343+
if (audio_format == kFormatFloat && bits_per_sample == 64) {
344+
if (data.size() % sizeof(double) != 0) {
345+
throw std::runtime_error("malformed float64 WAV data chunk");
346+
}
347+
const size_t sample_count = data.size() / sizeof(double);
348+
wav.samples.resize(sample_count);
349+
const auto * pcm = reinterpret_cast<const double *>(data.data());
350+
for (size_t i = 0; i < sample_count; ++i) {
351+
wav.samples[i] = static_cast<float>(pcm[i]);
352+
}
353+
return wav;
354+
}
355+
146356
if (audio_format == 1 && bits_per_sample == 16) {
147357
const size_t sample_count = data.size() / sizeof(int16_t);
148358
wav.samples.resize(sample_count);
@@ -184,7 +394,10 @@ WavData read_wav_f32(std::istream & input) {
184394
return wav;
185395
}
186396

187-
throw std::runtime_error("unsupported WAV encoding (need PCM16, PCM24, or float32)");
397+
throw std::runtime_error(
398+
"unsupported WAV encoding (" + describe_encoding(audio_format, bits_per_sample) +
399+
"); supported: PCM 8/16/24/32-bit, float 32/64-bit, A-law and mu-law. "
400+
"Convert with `ffmpeg -i input -ac 1 -ar 44100 -c:a pcm_s16le output.wav`");
188401
}
189402

190403
WavData read_wav_f32(std::string_view input) {

0 commit comments

Comments
 (0)