craft-audio: review fixes, round 1 (#442)

- FileEncoder/encode for FLAC hand the encoder one 4096-frame block at a
  time: a whole-buffer encode drained the pending samples quadratically
  (480 s of stereo took 19 s, now 5 s) and copied the whole input twice.
- Frame headers name sample rates outside the table (codes 12-14), so
  streams at rates like 11025 or 37800 Hz stay in the streamable subset.
- The decode doctest needs symphonia to read FLAC at this point.
- README: FLAC output is verified against ffmpeg, not the flac tool.
- Tests: keep the AIFF force-RF64 check; exact 2^36 frame boundary.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
alehel
2026-11-04 10:35:44 +01:00
co-authored by Claude Opus 5.5
parent 550732201c
commit 7c88c201a5
7 changed files with 57 additions and 19 deletions
+1 -1
View File
@@ -271,7 +271,7 @@ match craft_audio::decode(&bytes, None) {
| Codec | Spec | Scope | Verified against |
|---|---|---|---|
| WAV, AIFF | Microsoft RIFF, Apple AIFF/AIFF-C | read: integer 8–32-bit, float 32/64-bit, `WAVE_FORMAT_EXTENSIBLE` masks, RF64/BW64, BWF `bext`, truncated files; write: 16/24/32-bit integer and 32-bit float, RF64 above 4 GiB, any layout, TPDF dither | round trips; hostile-input tests |
| FLAC encoder | RFC 9639 | 16, 24, 32 bits; 1–8 channels; up to 655,350 Hz; LPC, stereo decorrelation, STREAMINFO MD5 | bit-exact through ffmpeg and the reference `flac` decoder |
| FLAC encoder | RFC 9639 | 16, 24, 32 bits; 1–8 channels; up to 655,350 Hz; LPC, stereo decorrelation, STREAMINFO MD5 | bit-exact through ffmpeg, STREAMINFO MD5 checked against ffmpeg |
## symphonia
+1 -1
View File
@@ -364,7 +364,7 @@ pub fn probe_with(bytes: &[u8], ext_hint: Option<&str>, opts: &DecodeOptions) ->
/// file instead, and [`decode_with`] for another track or size limit.
///
/// ```
/// # #[cfg(feature = "flac-encode")] {
/// # #[cfg(all(feature = "flac-encode", feature = "symphonia"))] {
/// use craft_audio::{AudioBuffer, EncodeFormat, EncodeOptions, decode, encode};
///
/// let tone: Vec<f32> = (0..4800).map(|i| (i as f32 * 0.05).sin() * 0.5).collect();
+25 -6
View File
@@ -27,9 +27,10 @@ fn block_size_code(n: u16) -> (u8, Option<(u32, u32)>) {
}
}
/// Frame header sample rate code; 0 refers to STREAMINFO.
fn sample_rate_code(rate: u32) -> u8 {
match rate {
/// Frame header sample rate code, with the value stored after the header for codes 12–14 (kHz,
/// Hz, tens of Hz); 0 refers to STREAMINFO and is left for rates none of those can name.
fn sample_rate_code(rate: u32) -> (u8, Option<(u32, u32)>) {
let code = match rate {
88_200 => 1,
176_400 => 2,
192_000 => 3,
@@ -41,8 +42,12 @@ fn sample_rate_code(rate: u32) -> u8 {
44_100 => 9,
48_000 => 10,
96_000 => 11,
r if r % 1000 == 0 && r / 1000 <= 0xFF => return (12, Some((r / 1000, 8))),
r if r <= 0xFFFF => return (13, Some((r, 16))),
r if r % 10 == 0 && r / 10 <= 0xFFFF => return (14, Some((r / 10, 16))),
_ => 0,
}
};
(code, None)
}
/// Frame header sample size code; 0 refers to STREAMINFO.
@@ -160,9 +165,11 @@ pub fn write(spec: &FrameSpec) -> Vec<u8> {
Some(Stereo::MidSide) => 10,
_ => spec.channels.len().saturating_sub(1) as u8,
};
let mut h = vec![0xFF, 0xF8, bs_code << 4 | sample_rate_code(spec.sample_rate), channel_code << 4 | sample_size_code(spec.bps) << 1];
let (sr_code, sr_extra) = sample_rate_code(spec.sample_rate);
let mut h = vec![0xFF, 0xF8, bs_code << 4 | sr_code, channel_code << 4 | sample_size_code(spec.bps) << 1];
coded_number(spec.number, &mut h);
if let Some((v, n)) = bs_extra {
// the uncommon block size, then the uncommon sample rate (§9.1.6, §9.1.7)
for (v, n) in [bs_extra, sr_extra].into_iter().flatten() {
if n == 8 {
h.push(v as u8);
} else {
@@ -202,4 +209,16 @@ mod tests {
assert_eq!(code(0x7FFF_FFFF), [0xFD, 0xBF, 0xBF, 0xBF, 0xBF, 0xBF]);
assert_eq!(code(0xF_FFFF_FFFF), [0xFE, 0xBF, 0xBF, 0xBF, 0xBF, 0xBF, 0xBF]);
}
#[test]
fn sample_rates_outside_the_table_are_named_in_the_header() {
assert_eq!(sample_rate_code(48_000), (10, None));
assert_eq!(sample_rate_code(64_000), (12, Some((64, 8))));
assert_eq!(sample_rate_code(11_025), (13, Some((11_025, 16))));
assert_eq!(sample_rate_code(37_800), (13, Some((37_800, 16))));
assert_eq!(sample_rate_code(352_800), (14, Some((35_280, 16))));
assert_eq!(sample_rate_code(655_350), (14, Some((65_535, 16))));
// neither whole kHz, at most 65535 Hz, nor whole tens of Hz
assert_eq!(sample_rate_code(100_001), (0, None));
}
}
+18 -10
View File
@@ -36,6 +36,9 @@ fn check(channels: usize, sample_rate: u32, depth: BitDepth) -> Result<u8> {
}
}
/// Frames quantized and handed to the encoder at a time: its block size.
const CHUNK: usize = 4096;
fn encoder_error(e: encoder::Error) -> AudioError {
AudioError::Encode(format!("FLAC: {e}"))
}
@@ -88,16 +91,21 @@ impl<W: Write + Seek> Writer<W> {
return Err(super::failed_write_error());
}
let total = self.check_room(frames as u64)?;
for (c, (p, q)) in self.planar.iter_mut().zip(self.quantizers.iter_mut()).enumerate() {
let src = block.get(c).map(AsRef::as_ref).unwrap_or_default();
p.clear();
p.extend((0..frames).map(|i| q.quantize(src.get(i).copied().unwrap_or(0.0))));
}
let refs: Vec<&[i32]> = self.planar.iter().map(Vec::as_slice).collect();
let written = self.encoder.encode(&refs).map_err(encoder_error).and_then(|bytes| Ok(self.out.write_all(&bytes)?));
if let Err(e) = written {
self.failed = true;
return Err(e);
// One FLAC block at a time: the encoder then holds less than two blocks, and the copies
// stay small however long `block` is.
for at in (0..frames).step_by(CHUNK) {
let end = frames.min(at + CHUNK);
for (c, (p, q)) in self.planar.iter_mut().zip(self.quantizers.iter_mut()).enumerate() {
let src = block.get(c).map(AsRef::as_ref).unwrap_or_default();
p.clear();
p.extend((at..end).map(|i| q.quantize(src.get(i).copied().unwrap_or(0.0))));
}
let refs: Vec<&[i32]> = self.planar.iter().map(Vec::as_slice).collect();
let written = self.encoder.encode(&refs).map_err(encoder_error).and_then(|bytes| Ok(self.out.write_all(&bytes)?));
if let Err(e) = written {
self.failed = true;
return Err(e);
}
}
self.frames = total;
Ok(())
+5 -1
View File
@@ -133,7 +133,7 @@ fn every_signal_round_trips_exactly() {
#[test]
fn channel_layouts_rates_and_sizes() {
let cases: [(&str, u8, u8, u32, usize); 8] = [
let cases: [(&str, u8, u8, u32, usize); 12] = [
("mono-8bit", 1, 8, 8_000, 3_001),
("mono-24-44k1", 1, 24, 44_100, 9_999),
("stereo-20bit", 2, 20, 96_000, 10_000),
@@ -141,6 +141,10 @@ fn channel_layouts_rates_and_sizes() {
("mono-32", 1, 32, 48_000, 7_777),
("5.1-32", 6, 32, 96_000, 12_345),
("odd-rate", 2, 16, 37_800, 5_000),
("rate-khz", 2, 16, 64_000, 5_000),
("rate-tens", 2, 24, 352_800, 5_000),
("rate-max", 1, 16, 655_350, 5_000),
("rate-from-streaminfo", 1, 16, 100_001, 5_000),
("tiny", 2, 16, 48_000, 1),
];
for (name, ch, bps, rate, n) in cases {
+2
View File
@@ -140,6 +140,8 @@ fn bwf_roundtrip() {
.with_time_reference(0x1_2345_6789); // > 32 bits
let src = sine(96_000, 2, 1000);
// RF64 is a WAV layout: other formats ignore it.
let aiff = encode(&src, &EncodeOptions::new(EncodeFormat::Aiff).with_force_rf64(true)).unwrap();
assert_eq!(&aiff[..4], b"FORM");
let flac = encode(&src, &EncodeOptions::new(EncodeFormat::Flac).with_force_rf64(true)).unwrap();
assert_eq!(&flac[..4], b"fLaC");
for depth in DEPTHS {
+5
View File
@@ -108,6 +108,11 @@ fn blocks_past_a_format_limit_are_refused_before_anything_is_written() {
enc.write(&buf.channels, 100).unwrap();
assert!(enc.check_room(limit / 2).is_ok(), "{format:?}");
assert!(matches!(enc.check_room(limit), Err(AudioError::TooLarge(_))), "{format:?}");
if format == EncodeFormat::Flac {
// 100 frames are in: STREAMINFO counts up to 2^36 - 1 in all.
assert!(enc.check_room(limit - 101).is_ok());
assert!(matches!(enc.check_room(limit - 100), Err(AudioError::TooLarge(_))));
}
// A block that does not hold the frames it claims is refused before the limit is checked.
if let Ok(frames) = usize::try_from(limit) {
assert!(matches!(enc.write::<Vec<f32>>(&[], frames), Err(AudioError::Encode(_))), "{format:?}");