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

- Test the high bits of STREAMINFO's total sample count, and that a total
  past 36 bits is left unknown.
- README limits: the usual 5- and 6-channel layouts depend on the mask
  comment too; say what is outside the streamable subset.
- Comments: 32-bit code 0 does not decode everywhere and leaves the
  subset; the oracle test helper is in craft_media_testkit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
alehelandClaude Opus 5.5 committed 2026-11-04 10:35:44 +01:00
1 parent 430ad15683
commit c008e8ee07
4 files changed
+26 -5

No files matched your search

+6 -3
View File
@@ -261,9 +261,12 @@ match craft_audio::decode(&bytes, None) {
- AIFF with more than four channels decodes with an unordered layout, in the file's order.
- A FLAC layout that decoders would not assume for the channel count depends on its
`WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment, and RFC 9639 §8.6.2 calls such a file not
streamable. This includes the usual 7-channel layout: without the comment, its back left, back
right and back centre are read as back centre, side left and side right.
`WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment. This includes the usual layouts for 5 and 6 channels:
without the comment, back surrounds are read as side surrounds. It includes the usual 7-channel
layout too: without the comment, its back left, back right and back centre are read as back
centre, side left and side right.
- Outside RFC 9639's streamable subset (§7), which some decoders require: FLAC at 32 bits, at a
sample rate the frame header cannot name, or with a layout that needs the mask comment.
- `ChannelLayout::unordered` is written to FLAC as mask `0x0000`, as RFC 9639 §8.6.2 allows, but
ffmpeg and `flac -d` ignore that mask and use their default layout for the channel count.
- Not decoded at all: AC-3, E-AC-3, Opus and Speex.
+17
View File
@@ -234,3 +234,20 @@ impl Encoder {
data
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn streaminfo_holds_a_36_bit_total_and_leaves_a_larger_one_unknown() {
let enc = Encoder::new(EncoderConfig::new(48_000, 1, 16)).unwrap();
let total = |t| {
let si = enc.streaminfo_with(0, 0, t, [0; 16]);
(u64::from(si[13] & 0x0F) << 32) | u64::from(u32::from_be_bytes([si[14], si[15], si[16], si[17]]))
};
assert_eq!(total((1 << 36) - 1), (1 << 36) - 1);
assert_eq!(total((1 << 35) + 5), (1 << 35) + 5);
assert_eq!(total(1 << 36), 0);
}
}
+2 -1
View File
@@ -59,7 +59,8 @@ fn sample_size_code(bps: u8) -> u8 {
20 => 5,
24 => 6,
// 32 bits has code 7 since RFC 9639, which earlier decoders (symphonia 0.5, FilmCraft's own
// FLAC import) reject as reserved; 0 ("as STREAMINFO says") decodes everywhere
// FLAC import) reject as reserved; 0 ("as STREAMINFO says") decodes in those as well, but is
// outside the streamable subset (§7)
_ => 0,
}
}
+1 -1
View File
@@ -1,4 +1,4 @@
//! ffmpeg as an external decoder oracle (skipped when absent, see `tests/common`): every stream must
//! ffmpeg as an external decoder oracle (skipped when absent, see `craft_media_testkit`): every stream must
//! decode without an error to exactly the samples that went in, with the STREAMINFO MD5 matching
//! ffmpeg's hash of the decoded audio. From FilmCraft's `filmcraft-flac` tests.