From 550732201cb52d34829fe5d21a45ad7566f3fa4e Mon Sep 17 00:00:00 2001 From: alehel Date: Fri, 30 Oct 2026 12:18:29 +0100 Subject: [PATCH 01/20] craft-audio: FLAC encoder FilmCraft's FLAC encoder behind `flac-encode`, kept over SoundCraft's: linear prediction, stereo decorrelation and the STREAMINFO MD5 give 0.1-37% smaller files on 12 test signals, and the output is bit-exact through `flac` and ffmpeg. It is 3-5x slower than SoundCraft's, still about 56x real time. `lossless-encode` is now `pcm-encode` plus `flac-encode`, and `EncodeFormat::Flac` writes it through `encode` and `FileEncoder`. With only one of the two features, the other's formats are refused as `Unsupported`. The tests that use FLAC files as decode fixtures (truncation, corruption, metadata bounds, zero-frame files, container preflight) come with it, as do the FLAC oracle tests. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/audio.yml | 3 +- README.md | 4 +- crates/audio/Cargo.toml | 8 +- crates/audio/README.md | 24 +- crates/audio/examples/encode.rs | 6 +- crates/audio/src/lib.rs | 21 +- crates/audio/src/read/mod.rs | 6 +- crates/audio/src/write/flac/crc.rs | 57 ++++ crates/audio/src/write/flac/encoder.rs | 232 ++++++++++++++ crates/audio/src/write/flac/frame.rs | 205 ++++++++++++ crates/audio/src/write/flac/md5.rs | 119 +++++++ crates/audio/src/write/flac/mod.rs | 130 ++++++++ crates/audio/src/write/flac/oracle_tests.rs | 206 ++++++++++++ crates/audio/src/write/flac/predict.rs | 327 ++++++++++++++++++++ crates/audio/src/write/mod.rs | 106 ++++++- crates/audio/src/write/pcm.rs | 2 +- crates/audio/tests/audio/encode_format.rs | 3 +- crates/audio/tests/audio/float_headroom.rs | 2 +- crates/audio/tests/audio/malformed.rs | 101 +++++- crates/audio/tests/audio/roundtrip.rs | 98 +++++- crates/audio/tests/audio/stream.rs | 12 +- 21 files changed, 1619 insertions(+), 53 deletions(-) create mode 100644 crates/audio/src/write/flac/crc.rs create mode 100644 crates/audio/src/write/flac/encoder.rs create mode 100644 crates/audio/src/write/flac/frame.rs create mode 100644 crates/audio/src/write/flac/md5.rs create mode 100644 crates/audio/src/write/flac/mod.rs create mode 100644 crates/audio/src/write/flac/oracle_tests.rs create mode 100644 crates/audio/src/write/flac/predict.rs diff --git a/.github/workflows/audio.yml b/.github/workflows/audio.yml index 1dc76d17..f2d7d49e 100644 --- a/.github/workflows/audio.yml +++ b/.github/workflows/audio.yml @@ -35,7 +35,7 @@ jobs: # Each feature on its own, so code that needs another feature fails here. - name: Clippy, each feature alone run: | - for f in "" decode decode-native symphonia encode lossless-encode pcm-encode peaks serde; do + for f in "" decode decode-native symphonia encode lossless-encode pcm-encode flac-encode peaks serde; do echo "::group::features: ${f:-none}" cargo clippy -p craft-audio --all-targets --locked --no-default-features ${f:+--features "$f"} -- -D warnings echo "::endgroup::" @@ -82,6 +82,7 @@ jobs: key: craft-audio-features - run: cargo test -p craft-audio --no-default-features --features lossless-encode --locked - run: cargo test -p craft-audio --no-default-features --features pcm-encode --locked --lib + - run: cargo test -p craft-audio --no-default-features --features flac-encode --locked --lib - run: cargo test -p craft-audio --no-default-features --features decode-native --locked test: timeout-minutes: 20 diff --git a/README.md b/README.md index 6c004096..e45d6e0f 100644 --- a/README.md +++ b/README.md @@ -78,8 +78,8 @@ craft- = { git = "https://github.com/storytold/craft-libs", rev = " = encode(&buf, &opts)?; +let opts = EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(BitDepth::Int24).with_dither(true); +let flac: Vec = encode(&buf, &opts)?; // 5.1 with side surrounds: a layout other than the default for 6 channels, recorded in the WAV. let wav = encode(&surround, &EncodeOptions::default().with_layout(ChannelLayout::SURROUND_5_1_SIDE))?; @@ -240,7 +242,7 @@ match craft_audio::decode(&bytes, None) { |---|---|---| | WAV, BWF, RF64/BW64 | native | native | | AIFF, AIFF-C | native | native | -| FLAC | symphonia | — | +| FLAC | symphonia | native | | AAC-LC, HE-AAC | symphonia | — | | MP1, MP2, MP3 (MPEG audio Layer I/II/III) | symphonia | — | | ALAC, Vorbis, ADPCM, A-law, µ-law | symphonia | — | @@ -269,6 +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 | ## symphonia @@ -320,12 +323,14 @@ the full tests with ffmpeg required. | Part | Source | |---|---| | WAV, AIFF, `decode`/`probe`, peaks | SoundCraft `soundcraft-audio-io` [7dc56a4](https://github.com/storytold/soundcraft/commit/7dc56a4dbeebf57eacece09536726c57844fc59d) | +| FLAC encoder | FilmCraft [8fda2f2](https://github.com/storytold/filmcraft/commit/8fda2f232198c4c79ff9cfe5b6a347bb1375fa9e) | Where apps had overlapping implementations, the one that measured better was kept, with the tests of both: | Format | Kept | Reason | |---|---|---| +| FLAC encoding | FilmCraft | 0.1–37% smaller output than SoundCraft's on 12 test signals; STREAMINFO MD5 | | WAV/AIFF writing | SoundCraft | only one with BWF, dither, arbitrary speaker masks, streaming without known length | ## Migration notes for apps @@ -350,11 +355,12 @@ What an app adjusts: `#[non_exhaustive]`. `Speaker::ALL` is a slice. - `TrackInfo::sample_rate` and `channels` are `Option`s. -- Features are per format. SoundCraft's `encode` is now `lossless-encode`, which turns on - `pcm-encode` (WAV, AIFF). `encode` turns on every encoder. Writing a format +- Features are per format. SoundCraft's `encode` (WAV, AIFF, FLAC) is now `lossless-encode`, made + of `pcm-encode` (WAV, AIFF) and `flac-encode`. `encode` turns on every encoder. Writing a format whose feature is off returns `AudioError::Unsupported`. - `FileEncoder::write` takes any per-channel slices, but refuses a block with missing channels or short slices instead of filling them with silence. - `DecodeOptions` is not `Copy`. +- The FLAC encoder's vendor string is `craft-audio `. - `decode` tries the native path first (WAV, AIFF, and PCM in MP4 and Matroska), which decodes MP4 and Matroska files to presentation length. A failed source read is `AudioError::Io`. diff --git a/crates/audio/examples/encode.rs b/crates/audio/examples/encode.rs index a1c7b89d..6d49d55d 100644 --- a/crates/audio/examples/encode.rs +++ b/crates/audio/examples/encode.rs @@ -1,4 +1,4 @@ -//! Write a tone as WAV and AIFF, whole and streamed. +//! Write a tone as WAV and FLAC, whole and streamed. //! //! `cargo run -p craft-audio --example encode --features lossless-encode -- out` @@ -21,8 +21,8 @@ fn main() -> Result<(), Box> { std::fs::write(dir.join("tone-5.1.wav"), wav)?; // Streamed: the same bytes, block by block into a file. - let opts = EncodeOptions::new(EncodeFormat::Aiff); - let mut enc = FileEncoder::new(std::fs::File::create(dir.join("tone.aiff"))?, &opts, rate as u32, 2)?; + let opts = EncodeOptions::new(EncodeFormat::Flac); + let mut enc = FileEncoder::new(std::fs::File::create(dir.join("tone.flac"))?, &opts, rate as u32, 2)?; for chunk in tone.chunks(4096) { enc.write(&[chunk, chunk], chunk.len())?; } diff --git a/crates/audio/src/lib.rs b/crates/audio/src/lib.rs index 942a141b..cbdc3c07 100644 --- a/crates/audio/src/lib.rs +++ b/crates/audio/src/lib.rs @@ -15,8 +15,9 @@ //! - [`TrackDecoder`] (feature `decode-native`): one track's packets from any demuxer to planar //! `f32` in WAV channel order. //! - [`encode`], [`FileEncoder`]: WAV/BWF/RF64 and AIFF/AIFF-C (feature `pcm-encode`, which also -//! gives [`wav_float_header`]), whole-buffer or incremental into any `Write + Seek`. Features -//! `lossless-encode` and `encode` turn it on. +//! gives [`wav_float_header`]) and FLAC (feature `flac-encode`), whole-buffer or incremental into +//! any `Write + Seek`. Feature `lossless-encode` turns on both; feature `encode` is +//! the same as `lossless-encode`. //! - [`peaks`] (feature `peaks`): min/max/RMS pyramid for waveform drawing. //! //! The shared types below build with no features. @@ -33,7 +34,7 @@ mod read; mod track; #[cfg(any(feature = "decode-native", feature = "pcm-encode"))] mod wav; -#[cfg(feature = "pcm-encode")] +#[cfg(any(feature = "pcm-encode", feature = "flac-encode"))] mod write; pub use buffer::AudioBuffer; @@ -49,7 +50,9 @@ pub use read::{DEFAULT_MAX_SAMPLES, DecodeOptions, Decoder, TrackInfo, decode, d #[cfg(feature = "decode-native")] pub use track::{LOOKAHEAD_PACKETS, TrackDecoder}; #[cfg(feature = "pcm-encode")] -pub use write::{ENCODE_EXTENSIONS, FileEncoder, encode, encode_format_for, wav_float_header}; +pub use write::wav_float_header; +#[cfg(any(feature = "pcm-encode", feature = "flac-encode"))] +pub use write::{ENCODE_EXTENSIONS, FileEncoder, encode, encode_format_for}; /// Errors from probing, decoding and encoding. #[non_exhaustive] @@ -76,7 +79,8 @@ pub enum AudioError { /// Result alias for this module. pub type Result = std::result::Result; -/// The file formats [`encode`] writes (feature `pcm-encode`). +/// The file formats [`encode`] writes. Each needs its feature: WAV and AIFF `pcm-encode`, FLAC +/// `flac-encode`; asking for one that is not built returns [`AudioError::Unsupported`]. #[non_exhaustive] #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -85,6 +89,8 @@ pub enum EncodeFormat { Wav, /// AIFF (integer depths) or AIFF-C `fl32` (float). Aiff, + /// FLAC (integer depths). + Flac, } impl EncodeFormat { @@ -93,6 +99,7 @@ impl EncodeFormat { match self { EncodeFormat::Wav => Container::Wav, EncodeFormat::Aiff => Container::Aiff, + EncodeFormat::Flac => Container::Flac, } } } @@ -244,8 +251,8 @@ pub enum BitDepth { /// ``` /// use craft_audio::{BitDepth, EncodeFormat, EncodeOptions}; /// -/// let opts = EncodeOptions::new(EncodeFormat::Aiff).with_bit_depth(BitDepth::Int16).with_dither(true); -/// assert_eq!(opts.format, EncodeFormat::Aiff); +/// let opts = EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(BitDepth::Int16).with_dither(true); +/// assert_eq!(opts.format, EncodeFormat::Flac); /// ``` #[non_exhaustive] #[derive(Debug, Clone, PartialEq)] diff --git a/crates/audio/src/read/mod.rs b/crates/audio/src/read/mod.rs index 99798120..2a62ef1e 100644 --- a/crates/audio/src/read/mod.rs +++ b/crates/audio/src/read/mod.rs @@ -364,12 +364,12 @@ 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 = "pcm-encode")] { +/// # #[cfg(feature = "flac-encode")] { /// use craft_audio::{AudioBuffer, EncodeFormat, EncodeOptions, decode, encode}; /// /// let tone: Vec = (0..4800).map(|i| (i as f32 * 0.05).sin() * 0.5).collect(); -/// let aiff = encode(&AudioBuffer { sample_rate: 48_000, channels: vec![tone] }, &EncodeOptions::new(EncodeFormat::Aiff)).unwrap(); -/// let (info, buf) = decode(&aiff, None).unwrap(); +/// let flac = encode(&AudioBuffer { sample_rate: 48_000, channels: vec![tone] }, &EncodeOptions::new(EncodeFormat::Flac)).unwrap(); +/// let (info, buf) = decode(&flac, None).unwrap(); /// assert_eq!((info.channels, buf.frames()), (1, 4800)); /// # } /// ``` diff --git a/crates/audio/src/write/flac/crc.rs b/crates/audio/src/write/flac/crc.rs new file mode 100644 index 00000000..0cc4869b --- /dev/null +++ b/crates/audio/src/write/flac/crc.rs @@ -0,0 +1,57 @@ +//! The two frame checksums of RFC 9639 §9.1.8 and §9.3: CRC-8 (polynomial x⁸+x²+x+1) over the +//! frame header and CRC-16 (x¹⁶+x¹⁵+x²+1) over the whole frame, both MSB-first with a zero start. + +const fn table8() -> [u8; 256] { + let mut t = [0u8; 256]; + let mut i = 0; + while i < 256 { + let mut c = i as u8; + let mut b = 0; + while b < 8 { + c = if c & 0x80 != 0 { (c << 1) ^ 0x07 } else { c << 1 }; + b += 1; + } + t[i] = c; + i += 1; + } + t +} + +const fn table16() -> [u16; 256] { + let mut t = [0u16; 256]; + let mut i = 0; + while i < 256 { + let mut c = (i as u16) << 8; + let mut b = 0; + while b < 8 { + c = if c & 0x8000 != 0 { (c << 1) ^ 0x8005 } else { c << 1 }; + b += 1; + } + t[i] = c; + i += 1; + } + t +} + +static T8: [u8; 256] = table8(); +static T16: [u16; 256] = table16(); + +pub fn crc8(data: &[u8]) -> u8 { + data.iter().fold(0u8, |c, &b| T8[usize::from(c ^ b)]) +} + +pub fn crc16(data: &[u8]) -> u16 { + data.iter().fold(0u16, |c, &b| (c << 8) ^ T16[usize::from((c >> 8) as u8 ^ b)]) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn check_values() { + // the standard check value of each polynomial over "123456789" + assert_eq!(crc8(b"123456789"), 0xF4); + assert_eq!(crc16(b"123456789"), 0xFEE8); + } +} diff --git a/crates/audio/src/write/flac/encoder.rs b/crates/audio/src/write/flac/encoder.rs new file mode 100644 index 00000000..508bbf59 --- /dev/null +++ b/crates/audio/src/write/flac/encoder.rs @@ -0,0 +1,232 @@ +//! The FLAC encoder (RFC 9639): planar integer samples (8–32 bit, 1–8 channels) → `fLaC`, STREAMINFO, +//! a VORBIS_COMMENT naming the encoder, then fixed-size frames. Each channel of each frame is coded +//! as constant, verbatim, a fixed polynomial predictor or linear prediction, whichever is smallest, +//! with partitioned Rice codes; stereo frames also try left/side, side/right and mid/side. The +//! STREAMINFO sizes and MD5 are only known at the end: [`Encoder::streaminfo`] is written over +//! [`STREAMINFO_OFFSET`] once [`Encoder::finish`] has run. +//! +//! From FilmCraft's `filmcraft-flac` (storytold/filmcraft@8fda2f2). + +use super::frame::{FrameSpec, Stereo}; +use super::predict::{Effort, Subframe}; + +/// Byte offset of the 34-byte STREAMINFO body in [`Encoder::header`]. +pub const STREAMINFO_OFFSET: u64 = 8; + +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum Error { + #[error("invalid configuration: {0}")] + InvalidConfig(&'static str), + #[error("expected {expected} channels, got {got}")] + Channels { expected: usize, got: usize }, + #[error("channels of unequal length")] + UnequalChannels, + #[error("sample {0} does not fit the configured sample size")] + SampleRange(i32), +} + +#[derive(Clone, Debug)] +pub struct EncoderConfig { + /// 1 to 1 048 575 Hz. + pub sample_rate: u32, + /// 1–8 (FLAC channel order: L R C LFE Ls Rs … for 5.1). + pub channels: u8, + /// 8–32. At 32 bits stereo channels are coded independently: the side channel of + /// left/side, side/right and mid/side coding would need 33 bits. + pub bits_per_sample: u8, + /// Samples per channel per frame, 16–65535 (4096 by default). + pub block_size: u16, + /// 0 (fastest) to 8 (smallest); 5 by default. + pub level: u8, +} + +impl EncoderConfig { + pub fn new(sample_rate: u32, channels: u8, bits_per_sample: u8) -> Self { + EncoderConfig { sample_rate, channels, bits_per_sample, block_size: 4096, level: 5 } + } +} + +pub struct Encoder { + cfg: EncoderConfig, + effort: Effort, + /// Samples waiting for a full block, per channel. + pending: Vec>, + frame_number: u64, + total_samples: u64, + min_frame: u32, + max_frame: u32, + md5: super::md5::Md5, +} + +impl Encoder { + pub fn new(cfg: EncoderConfig) -> Result { + if !(1..=8).contains(&cfg.channels) { + return Err(Error::InvalidConfig("1 to 8 channels")); + } + if !(8..=32).contains(&cfg.bits_per_sample) { + return Err(Error::InvalidConfig("8 to 32 bits per sample")); + } + if !(1..=1_048_575).contains(&cfg.sample_rate) { + return Err(Error::InvalidConfig("sample rate 1 to 1048575 Hz")); + } + if cfg.block_size < 16 { + return Err(Error::InvalidConfig("block size 16 to 65535")); + } + let effort = Effort::level(cfg.level); + Ok(Encoder { + pending: vec![Vec::new(); usize::from(cfg.channels)], + cfg, + effort, + frame_number: 0, + total_samples: 0, + min_frame: 0, + max_frame: 0, + // RFC 9639 requires an MD5 signature of the unencoded audio in STREAMINFO; this is an + // integrity checksum, not a password hash or a security boundary. + md5: super::md5::Md5::default(), // NOSONAR + }) + } + + /// `fLaC`, the STREAMINFO block (sizes and MD5 unknown yet: see [`Encoder::streaminfo`]) and a + /// VORBIS_COMMENT block with the vendor string. + pub fn header(&self) -> Vec { + let mut v = b"fLaC".to_vec(); + v.extend_from_slice(&[0, 0, 0, 34]); + v.extend_from_slice(&self.streaminfo_with(0, 0, 0, [0; 16])); + let vendor = concat!("craft-audio ", env!("CARGO_PKG_VERSION")); + let len = 4 + vendor.len() + 4; + // last metadata block, type 4 (VORBIS_COMMENT), little-endian lengths inside + v.push(0x84); + v.extend_from_slice(&(len as u32).to_be_bytes()[1..]); + v.extend_from_slice(&(vendor.len() as u32).to_le_bytes()); + v.extend_from_slice(vendor.as_bytes()); + v.extend_from_slice(&0u32.to_le_bytes()); + v + } + + /// The final STREAMINFO body: frame sizes, total samples and MD5 of what has been encoded. + pub fn streaminfo(&self) -> [u8; 34] { + self.streaminfo_with(self.min_frame, self.max_frame, self.total_samples, self.md5.digest()) + } + + fn streaminfo_with(&self, min_frame: u32, max_frame: u32, total: u64, md5: [u8; 16]) -> [u8; 34] { + let mut w = craft_media::bitstream::BitWriter::new(); + let bs = u32::from(self.cfg.block_size); + w.write_bits(bs, 16); + w.write_bits(bs, 16); + w.write_bits(min_frame.min(0xFF_FFFF), 24); + w.write_bits(max_frame.min(0xFF_FFFF), 24); + w.write_bits(self.cfg.sample_rate, 20); + w.write_bits(u32::from(self.cfg.channels) - 1, 3); + w.write_bits(u32::from(self.cfg.bits_per_sample) - 1, 5); + // 36 bits; 0 means unknown, and a count that does not fit is left unknown + let total = if total >> 36 == 0 { total } else { 0 }; + w.write_bits((total >> 32) as u32, 4); + w.write_bits(total as u32, 32); + w.write_bytes(&md5); + let mut out = [0u8; 34]; + out.copy_from_slice(w.bytes().get(..34).unwrap_or(&[0; 34])); + out + } + + /// Add samples (one slice per channel, equal lengths); returns the frames completed. + pub fn encode(&mut self, planar: &[&[i32]]) -> Result, Error> { + Ok(self.encode_frames(planar)?.concat()) + } + + /// [`Encoder::encode`] with each completed frame on its own (one MP4 / QuickTime sample each). + pub fn encode_frames(&mut self, planar: &[&[i32]]) -> Result>, Error> { + if planar.len() != self.pending.len() { + return Err(Error::Channels { expected: self.pending.len(), got: planar.len() }); + } + let n = planar.first().map_or(0, |c| c.len()); + if planar.iter().any(|c| c.len() != n) { + return Err(Error::UnequalChannels); + } + // i64: at 32 bits the range is all of i32 (1 << 31 overflows i32) + let max = (1i64 << (self.cfg.bits_per_sample - 1)) - 1; + if let Some(&bad) = planar.iter().flat_map(|c| c.iter()).find(|&&s| i64::from(s) > max || i64::from(s) < -max - 1) { + return Err(Error::SampleRange(bad)); + } + for (p, c) in self.pending.iter_mut().zip(planar) { + p.extend_from_slice(c); + } + let bs = usize::from(self.cfg.block_size); + let mut out = Vec::new(); + while self.pending.first().is_some_and(|p| p.len() >= bs) { + let block: Vec> = self.pending.iter_mut().map(|p| p.drain(..bs).collect()).collect(); + out.push(self.frame(&block)); + } + Ok(out) + } + + /// Encode what is left as a final, shorter frame. + pub fn finish(&mut self) -> Vec { + if self.pending.first().is_none_or(Vec::is_empty) { + return Vec::new(); + } + let block: Vec> = self.pending.iter_mut().map(std::mem::take).collect(); + self.frame(&block) + } + + fn frame(&mut self, block: &[Vec]) -> Vec { + let n = block.first().map_or(0, Vec::len); + let bps = self.cfg.bits_per_sample; + // the MD5 of the unencoded audio: interleaved, little-endian, whole bytes per sample + let bytes = usize::from(bps.div_ceil(8)); + let mut raw = Vec::with_capacity(n * block.len() * bytes); + for i in 0..n { + for c in block { + raw.extend_from_slice(c.get(i).copied().unwrap_or(0).to_le_bytes().get(..bytes).unwrap_or_default()); + } + } + self.md5.update(&raw); + + let effort = self.effort; + let code = |x: &[i32], b: u8| super::predict::choose(x, b, effort); + let data = match block { + // the side channel takes one bit more than the samples: at most 32 (i32) + [l, r] if effort.stereo && bps < 32 => { + let side: Vec = l.iter().zip(r).map(|(&a, &b)| a - b).collect(); + let mid: Vec = l.iter().zip(r).map(|(&a, &b)| (a + b) >> 1).collect(); + let (cl, cr, cs, cm) = (code(l, bps), code(r, bps), code(&side, bps + 1), code(&mid, bps)); + let options = [ + (Stereo::Independent, cl.1 + cr.1), + (Stereo::LeftSide, cl.1 + cs.1), + (Stereo::SideRight, cs.1 + cr.1), + (Stereo::MidSide, cm.1 + cs.1), + ]; + let best = options.iter().min_by_key(|o| o.1).map_or(Stereo::Independent, |o| o.0); + let channels: Vec<(&[i32], Subframe, u8)> = match best { + Stereo::Independent => vec![(l, cl.0, bps), (r, cr.0, bps)], + Stereo::LeftSide => vec![(l, cl.0, bps), (&side, cs.0, bps + 1)], + Stereo::SideRight => vec![(&side, cs.0, bps + 1), (r, cr.0, bps)], + Stereo::MidSide => vec![(&mid, cm.0, bps), (&side, cs.0, bps + 1)], + }; + super::frame::write(&FrameSpec { + number: self.frame_number, + sample_rate: self.cfg.sample_rate, + bps, + stereo: Some(best), + channels: &channels, + }) + } + _ => { + let channels: Vec<(&[i32], Subframe, u8)> = block.iter().map(|c| (c.as_slice(), code(c, bps).0, bps)).collect(); + super::frame::write(&FrameSpec { + number: self.frame_number, + sample_rate: self.cfg.sample_rate, + bps, + stereo: None, + channels: &channels, + }) + } + }; + self.frame_number += 1; + self.total_samples += n as u64; + let size = u32::try_from(data.len()).unwrap_or(u32::MAX); + self.min_frame = if self.min_frame == 0 { size } else { self.min_frame.min(size) }; + self.max_frame = self.max_frame.max(size); + data + } +} diff --git a/crates/audio/src/write/flac/frame.rs b/crates/audio/src/write/flac/frame.rs new file mode 100644 index 00000000..6b3d28e8 --- /dev/null +++ b/crates/audio/src/write/flac/frame.rs @@ -0,0 +1,205 @@ +//! Writing one frame (RFC 9639 §9): header with CRC-8, one subframe per channel, zero padding to a +//! byte, CRC-16. + +use craft_media::bitstream::BitWriter; + +use super::crc::{crc8, crc16}; +use super::predict::{Residual, Subframe}; + +/// Inter-channel decorrelation of a stereo frame (§9.1.3). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Stereo { + Independent, + /// Left, then side (left − right). + LeftSide, + /// Side, then right. + SideRight, + /// Mid ((left + right) >> 1), then side. + MidSide, +} + +/// Frame header block size bits: 4096 has its own code, others are stored after the header. +fn block_size_code(n: u16) -> (u8, Option<(u32, u32)>) { + match n { + 4096 => (0b1100, None), + 1..=256 => (0b0110, Some((u32::from(n - 1), 8))), + _ => (0b0111, Some((u32::from(n.saturating_sub(1)), 16))), + } +} + +/// Frame header sample rate code; 0 refers to STREAMINFO. +fn sample_rate_code(rate: u32) -> u8 { + match rate { + 88_200 => 1, + 176_400 => 2, + 192_000 => 3, + 8_000 => 4, + 16_000 => 5, + 22_050 => 6, + 24_000 => 7, + 32_000 => 8, + 44_100 => 9, + 48_000 => 10, + 96_000 => 11, + _ => 0, + } +} + +/// Frame header sample size code; 0 refers to STREAMINFO. +fn sample_size_code(bps: u8) -> u8 { + match bps { + 8 => 1, + 12 => 2, + 16 => 4, + 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 + _ => 0, + } +} + +/// The frame number in the UTF-8-like coding of §9.1.5 (up to 36 bits). +fn coded_number(v: u64, out: &mut Vec) { + if v < 0x80 { + out.push(v as u8); + return; + } + let tail = match v { + 0..0x800 => 1, + 0x800..0x1_0000 => 2, + 0x1_0000..0x20_0000 => 3, + 0x20_0000..0x400_0000 => 4, + 0x400_0000..0x8000_0000 => 5, + _ => 6, + }; + let lead_mask: u8 = !(0xFFu8 >> (tail + 1)); + out.push(lead_mask | (v >> (6 * tail)) as u8 & (0x7F >> (tail + 1))); + for i in (0..tail).rev() { + out.push(0x80 | ((v >> (6 * i)) & 0x3F) as u8); + } +} + +pub struct FrameSpec<'a> { + pub number: u64, + pub sample_rate: u32, + pub bps: u8, + pub stereo: Option, + /// The signals as coded (mid/side already formed), with their subframes. + pub channels: &'a [(&'a [i32], Subframe, u8)], +} + +fn write_residual(w: &mut BitWriter, residual: &Residual, r: &[i64], block: usize, order: usize) { + let wide = residual.wide(); + w.write_bits(u32::from(wide), 2); + w.write_bits(u32::from(residual.partition_order), 4); + let parts = 1usize << residual.partition_order; + let len = block / parts; + let mut start = 0usize; + for (p, &k) in residual.params.iter().enumerate() { + w.write_bits(u32::from(k), if wide { 5 } else { 4 }); + let n = if p == 0 { len.saturating_sub(order) } else { len }; + for &v in r.get(start..start + n).unwrap_or_default() { + let u = if v >= 0 { (v as u64) << 1 } else { ((-v - 1) as u64) << 1 | 1 }; + let mut q = u >> k; + while q >= 32 { + w.write_bits(0, 32); + q -= 32; + } + w.write_bits(1, q as u32 + 1); + w.write_bits((u & ((1u64 << k) - 1)) as u32, u32::from(k)); + } + start += n; + } +} + +fn write_subframe(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8) { + let b = u32::from(bps); + let sample = |w: &mut BitWriter, v: i32| w.write_bits(v as u32, b); + w.write_bit(false); + match sub { + Subframe::Constant(v) => { + w.write_bits(0, 6); + w.write_bit(false); + sample(w, *v); + } + Subframe::Verbatim => { + w.write_bits(1, 6); + w.write_bit(false); + x.iter().for_each(|&v| sample(w, v)); + } + Subframe::Fixed { order, residual } => { + w.write_bits(0b001000 | u32::from(*order), 6); + w.write_bit(false); + let o = usize::from(*order); + x.iter().take(o).for_each(|&v| sample(w, v)); + let r = super::predict::fixed_residual(x, o).unwrap_or_default(); + write_residual(w, residual, &r, x.len(), o); + } + Subframe::Lpc { coefs, precision, shift, residual } => { + let o = coefs.len(); + w.write_bits(0b100000 | (o as u32 - 1), 6); + w.write_bit(false); + x.iter().take(o).for_each(|&v| sample(w, v)); + w.write_bits(u32::from(*precision) - 1, 4); + w.write_bits(u32::from(*shift), 5); + coefs.iter().for_each(|&c| w.write_bits(c as u32, u32::from(*precision))); + let r = super::predict::lpc_residual(x, coefs, *shift).unwrap_or_default(); + write_residual(w, residual, &r, x.len(), o); + } + } +} + +/// One complete frame. +pub fn write(spec: &FrameSpec) -> Vec { + let block = spec.channels.first().map_or(0, |c| c.0.len()); + let (bs_code, bs_extra) = block_size_code(u16::try_from(block).unwrap_or(u16::MAX)); + let channel_code = match spec.stereo { + Some(Stereo::LeftSide) => 8, + Some(Stereo::SideRight) => 9, + 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]; + coded_number(spec.number, &mut h); + if let Some((v, n)) = bs_extra { + if n == 8 { + h.push(v as u8); + } else { + h.extend_from_slice(&(v as u16).to_be_bytes()); + } + } + h.push(crc8(&h)); + let mut w = BitWriter::new(); + w.write_bytes(&h); + for (x, sub, bps) in spec.channels { + write_subframe(&mut w, x, sub, *bps); + } + let mut out = w.finish(); + let crc = crc16(&out); + out.extend_from_slice(&crc.to_be_bytes()); + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn coded_numbers() { + let code = |v| { + let mut o = Vec::new(); + coded_number(v, &mut o); + o + }; + assert_eq!(code(0), [0]); + assert_eq!(code(0x7F), [0x7F]); + assert_eq!(code(0x80), [0xC2, 0x80]); + assert_eq!(code(0x7FF), [0xDF, 0xBF]); + assert_eq!(code(0x800), [0xE0, 0xA0, 0x80]); + assert_eq!(code(0xFFFF), [0xEF, 0xBF, 0xBF]); + assert_eq!(code(0x1_0000), [0xF0, 0x90, 0x80, 0x80]); + assert_eq!(code(0x7FFF_FFFF), [0xFD, 0xBF, 0xBF, 0xBF, 0xBF, 0xBF]); + assert_eq!(code(0xF_FFFF_FFFF), [0xFE, 0xBF, 0xBF, 0xBF, 0xBF, 0xBF, 0xBF]); + } +} diff --git a/crates/audio/src/write/flac/md5.rs b/crates/audio/src/write/flac/md5.rs new file mode 100644 index 00000000..8e71d86f --- /dev/null +++ b/crates/audio/src/write/flac/md5.rs @@ -0,0 +1,119 @@ +//! MD5 (RFC 1321), for the STREAMINFO signature of the unencoded audio (RFC 9639 §8.2): every +//! sample, interleaved, little-endian, in the fewest whole bytes that hold the sample size. + +const S: [u32; 64] = [ + 7, 12, 17, 22, 7, 12, 17, 22, 7, 12, 17, 22, 7, 12, 17, 22, 5, 9, 14, 20, 5, 9, 14, 20, 5, 9, 14, 20, 5, 9, 14, 20, 4, 11, 16, 23, 4, 11, 16, 23, + 4, 11, 16, 23, 4, 11, 16, 23, 6, 10, 15, 21, 6, 10, 15, 21, 6, 10, 15, 21, 6, 10, 15, 21, +]; + +/// `floor(abs(sin(i + 1)) × 2³²)`, RFC 1321 §3.4. +const K: [u32; 64] = [ + 0xd76aa478, 0xe8c7b756, 0x242070db, 0xc1bdceee, 0xf57c0faf, 0x4787c62a, 0xa8304613, 0xfd469501, 0x698098d8, 0x8b44f7af, 0xffff5bb1, 0x895cd7be, + 0x6b901122, 0xfd987193, 0xa679438e, 0x49b40821, 0xf61e2562, 0xc040b340, 0x265e5a51, 0xe9b6c7aa, 0xd62f105d, 0x02441453, 0xd8a1e681, 0xe7d3fbc8, + 0x21e1cde6, 0xc33707d6, 0xf4d50d87, 0x455a14ed, 0xa9e3e905, 0xfcefa3f8, 0x676f02d9, 0x8d2a4c8a, 0xfffa3942, 0x8771f681, 0x6d9d6122, 0xfde5380c, + 0xa4beea44, 0x4bdecfa9, 0xf6bb4b60, 0xbebfbc70, 0x289b7ec6, 0xeaa127fa, 0xd4ef3085, 0x04881d05, 0xd9d4d039, 0xe6db99e5, 0x1fa27cf8, 0xc4ac5665, + 0xf4292244, 0x432aff97, 0xab9423a7, 0xfc93a039, 0x655b59c3, 0x8f0ccc92, 0xffeff47d, 0x85845dd1, 0x6fa87e4f, 0xfe2ce6e0, 0xa3014314, 0x4e0811a1, + 0xf7537e82, 0xbd3af235, 0x2ad7d2bb, 0xeb86d391, +]; + +#[derive(Clone)] +pub struct Md5 { + state: [u32; 4], + block: [u8; 64], + filled: usize, + len: u64, +} + +impl Default for Md5 { + fn default() -> Self { + Md5 { state: [0x67452301, 0xefcdab89, 0x98badcfe, 0x10325476], block: [0; 64], filled: 0, len: 0 } + } +} + +impl Md5 { + pub fn update(&mut self, mut data: &[u8]) { + self.len = self.len.wrapping_add(data.len() as u64); + while !data.is_empty() { + let take = (64 - self.filled).min(data.len()); + let (head, rest) = data.split_at(take); + if let Some(dst) = self.block.get_mut(self.filled..self.filled + take) { + dst.copy_from_slice(head); + } + self.filled += take; + data = rest; + if self.filled == 64 { + let block = self.block; + self.compress(&block); + self.filled = 0; + } + } + } + + fn compress(&mut self, block: &[u8; 64]) { + let mut m = [0u32; 16]; + for (w, c) in m.iter_mut().zip(block.as_chunks::<4>().0) { + *w = u32::from_le_bytes(*c); + } + let [mut a, mut b, mut c, mut d] = self.state; + for i in 0..64 { + let (f, g) = match i / 16 { + 0 => ((b & c) | (!b & d), i), + 1 => ((d & b) | (!d & c), (5 * i + 1) % 16), + 2 => (b ^ c ^ d, (3 * i + 5) % 16), + _ => (c ^ (b | !d), (7 * i) % 16), + }; + let f = f.wrapping_add(a).wrapping_add(K[i]).wrapping_add(m[g]); + a = d; + d = c; + c = b; + b = b.wrapping_add(f.rotate_left(S[i])); + } + for (s, v) in self.state.iter_mut().zip([a, b, c, d]) { + *s = s.wrapping_add(v); + } + } + + /// The digest so far (the state is not consumed: more data may follow). + pub fn digest(&self) -> [u8; 16] { + let mut h = self.clone(); + let bits = h.len.wrapping_mul(8); + h.update(&[0x80]); + while h.filled != 56 { + h.update(&[0]); + } + h.update(&bits.to_le_bytes()); + let mut out = [0u8; 16]; + for (o, s) in out.as_chunks_mut::<4>().0.iter_mut().zip(h.state) { + o.copy_from_slice(&s.to_le_bytes()); + } + out + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn hex(d: [u8; 16]) -> String { + d.iter().map(|b| format!("{b:02x}")).collect() + } + + #[test] + fn rfc_1321_test_suite() { + let md5 = |s: &[u8]| { + let mut m = Md5::default(); + m.update(s); + hex(m.digest()) + }; + assert_eq!(md5(b""), "d41d8cd98f00b204e9800998ecf8427e"); + assert_eq!(md5(b"abc"), "900150983cd24fb0d6963f7d28e17f72"); + assert_eq!(md5(b"message digest"), "f96b697d7cb7938d525a2f31aaf161d0"); + assert_eq!(md5(b"12345678901234567890123456789012345678901234567890123456789012345678901234567890"), "57edf4a22be3c955ac49da2e2107b67a"); + // fed in pieces across block boundaries + let mut m = Md5::default(); + for c in b"12345678901234567890123456789012345678901234567890123456789012345678901234567890".chunks(7) { + m.update(c); + } + assert_eq!(hex(m.digest()), "57edf4a22be3c955ac49da2e2107b67a"); + } +} diff --git a/crates/audio/src/write/flac/mod.rs b/crates/audio/src/write/flac/mod.rs new file mode 100644 index 00000000..b4069474 --- /dev/null +++ b/crates/audio/src/write/flac/mod.rs @@ -0,0 +1,130 @@ +//! FLAC writing (RFC 9639): [`Writer`] quantizes planar `f32` and streams it through [`encoder`] +//! (linear prediction, stereo decorrelation, STREAMINFO MD5) into any `Write + Seek`. + +mod crc; +mod encoder; +mod frame; +mod md5; +mod predict; + +#[cfg(test)] +use craft_media_testkit as common; +#[cfg(test)] +mod oracle_tests; + +use super::pcm::{Quantizer, validate_buffer, validate_layout}; +use crate::{AudioBuffer, AudioError, BitDepth, Result}; +use encoder::{Encoder, EncoderConfig, STREAMINFO_OFFSET}; +use std::io::{Cursor, Seek, SeekFrom, Write}; + +/// Checks that FLAC can hold the layout; returns the bits per sample. +fn check(channels: usize, sample_rate: u32, depth: BitDepth) -> Result { + validate_layout(channels, sample_rate)?; + if channels > 8 { + return Err(AudioError::Unsupported(format!("FLAC supports at most 8 channels, got {channels}"))); + } + // STREAMINFO could say up to 1048575 Hz, but a frame header names at most 655350 Hz, and + // symphonia (our decoder) refuses streams above that. + if sample_rate > 655_350 { + return Err(AudioError::Unsupported(format!("FLAC sample rate {sample_rate} exceeds 655350 Hz"))); + } + match depth { + BitDepth::Int16 => Ok(16), + BitDepth::Int24 => Ok(24), + BitDepth::Int32 => Ok(32), + BitDepth::Float32 => Err(AudioError::Unsupported("FLAC cannot store 32-bit float samples".into())), + } +} + +fn encoder_error(e: encoder::Error) -> AudioError { + AudioError::Encode(format!("FLAC: {e}")) +} + +/// A FLAC stream written block by block; [`Writer::finish`] fills in the STREAMINFO sizes, sample +/// count and MD5. +pub(crate) struct Writer { + out: W, + start: u64, + encoder: Encoder, + quantizers: Vec, + planar: Vec>, + frames: u64, + /// Set when a write has failed: the encoder may hold samples the sink never received, so the + /// writer refuses every later call. + failed: bool, +} + +impl Writer { + pub(crate) fn new(mut out: W, channels: usize, sample_rate: u32, depth: BitDepth, dither: bool) -> Result { + let bps = check(channels, sample_rate, depth)?; + // `check` bounds channels to 1..=8. + let encoder = Encoder::new(EncoderConfig::new(sample_rate, channels as u8, bps)).map_err(encoder_error)?; + let start = out.stream_position()?; + out.write_all(&encoder.header())?; + Ok(Writer { + out, + start, + encoder, + quantizers: Quantizer::per_channel(u32::from(bps), dither, channels), + planar: vec![Vec::new(); channels], + frames: 0, + failed: false, + }) + } + + /// The frame count after `frames` more, or an error past what FLAC can count (2^36 frames). + pub(crate) fn check_room(&self, frames: u64) -> Result { + let total = self.frames.saturating_add(frames); + if total >= 1u64 << 36 { + return Err(AudioError::TooLarge("too many frames for FLAC".into())); + } + Ok(total) + } + + /// Appends `frames` frames of planar audio (missing samples are silence), unless they don't + /// fit (see [`Writer::check_room`]). + pub(crate) fn write>(&mut self, block: &[C], frames: usize) -> Result<()> { + if self.failed { + 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); + } + self.frames = total; + Ok(()) + } + + pub(crate) fn finish(mut self) -> Result { + if self.failed { + return Err(super::failed_write_error()); + } + let last = self.encoder.finish(); + self.out.write_all(&last)?; + self.out.seek(SeekFrom::Start(self.start.saturating_add(STREAMINFO_OFFSET)))?; + self.out.write_all(&self.encoder.streaminfo())?; + self.out.seek(SeekFrom::End(0))?; + self.out.flush()?; + Ok(self.out) + } +} + +pub(crate) fn encode(buf: &AudioBuffer, depth: BitDepth, dither: bool) -> Result> { + validate_buffer(buf)?; + let bps = check(buf.channels.len(), buf.sample_rate, depth)?; + let frames = buf.frames(); + let mut out = Vec::new(); + out.try_reserve(frames.saturating_mul(buf.channels.len()).saturating_mul(usize::from(bps) / 8) / 2) + .map_err(|_| AudioError::TooLarge("cannot allocate FLAC output".into()))?; + let mut w = Writer::new(Cursor::new(out), buf.channels.len(), buf.sample_rate, depth, dither)?; + w.write(&buf.channels, frames)?; + Ok(w.finish()?.into_inner()) +} diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs new file mode 100644 index 00000000..500365df --- /dev/null +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -0,0 +1,206 @@ +//! ffmpeg as an external decoder oracle (skipped when absent, see `tests/common`): 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. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use super::common; +use super::encoder::{Encoder, EncoderConfig, Error, STREAMINFO_OFFSET}; + +fn ffmpeg() -> Option { + common::ffmpeg() +} + +struct Rng(u64); +impl Rng { + fn next(&mut self) -> f64 { + self.0 ^= self.0 << 13; + self.0 ^= self.0 >> 7; + self.0 ^= self.0 << 17; + (self.0 >> 11) as f64 / (1u64 << 53) as f64 * 2.0 - 1.0 + } +} + +/// Test signals at `bps`, full scale: music-like (two sines and noise), a sweep, white noise, +/// a full-scale square, silence and a lone click. +fn signal(kind: &str, n: usize, bps: u8, seed: u64) -> Vec { + let max = ((1i64 << (bps - 1)) - 1) as f64; + let mut r = Rng(seed | 1); + (0..n) + .map(|i| { + let t = i as f64 / 48_000.0; + let v = match kind { + "music" => { + 0.5 * (2.0 * std::f64::consts::PI * 220.0 * t).sin() + 0.3 * (2.0 * std::f64::consts::PI * 1375.0 * t).sin() + 0.05 * r.next() + } + "sweep" => (2.0 * std::f64::consts::PI * (40.0 + 4000.0 * t) * t).sin() * 0.9, + "noise" => r.next(), + "square" => { + if (i / 37) % 2 == 0 { + 1.0 + } else { + -1.0 + } + } + "click" => f64::from(u8::from(i == n / 2)), + _ => 0.0, + }; + ((v * max).round() as i64).clamp(-(max as i64) - 1, max as i64) as i32 + }) + .collect() +} + +fn encode(planar: &[Vec], cfg: EncoderConfig, chunk: usize) -> Result, Error> { + let mut enc = Encoder::new(cfg)?; + let mut file = enc.header(); + let n = planar.first().map_or(0, Vec::len); + let mut at = 0; + while at < n { + let end = (at + chunk).min(n); + let parts: Vec<&[i32]> = planar.iter().map(|c| &c[at..end]).collect(); + file.extend(enc.encode(&parts)?); + at = end; + } + file.extend(enc.finish()); + let o = STREAMINFO_OFFSET as usize; + file[o..o + 34].copy_from_slice(&enc.streaminfo()); + Ok(file) +} + +fn run(ffmpeg: &Path, args: &[&str], input: &Path) -> (Vec, String) { + let out = Command::new(ffmpeg).args(["-v", "error", "-i"]).arg(input).args(args).output().unwrap(); + assert!(out.status.success(), "ffmpeg failed: {}", String::from_utf8_lossy(&out.stderr)); + (out.stdout, String::from_utf8_lossy(&out.stderr).into_owned()) +} + +/// Encode, decode with ffmpeg, compare every sample and the MD5; returns the file size. +fn round_trip(name: &str, planar: &[Vec], cfg: EncoderConfig, chunk: usize) -> Option { + let ffmpeg = ffmpeg()?; + let bps = cfg.bits_per_sample; + let rate = cfg.sample_rate; + let file = encode(planar, cfg, chunk).unwrap(); + let path = common::fixture_subdir("flac").join(format!("{name}.flac")); + std::fs::write(&path, &file).unwrap(); + + // samples: s32 holds each one shifted to the top bits + let (raw, err) = run(&ffmpeg, &["-f", "s32le", "-c:a", "pcm_s32le", "-"], &path); + assert!(err.is_empty(), "{name}: decoder errors: {err}"); + let decoded: Vec = raw.as_chunks::<4>().0.iter().map(|b| i32::from_le_bytes(*b) >> (32 - bps)).collect(); + let n = planar[0].len(); + assert_eq!(decoded.len(), n * planar.len(), "{name}: sample count"); + for (i, frame) in decoded.chunks_exact(planar.len()).enumerate() { + for (c, &v) in frame.iter().enumerate() { + assert_eq!(v, planar[c][i], "{name}: channel {c} sample {i}"); + } + } + + // STREAMINFO: total samples, rate, and the MD5 against ffmpeg's hash of the decoded audio. FLAC + // hashes samples unscaled; ffmpeg rescales other sizes (20-bit becomes 24-bit), so only 8, 16, + // 24 and 32 bit compare + let si = &file[STREAMINFO_OFFSET as usize..STREAMINFO_OFFSET as usize + 34]; + let pcm = match bps { + 8 => Some("s8"), + 16 => Some("s16le"), + 24 => Some("s24le"), + 32 => Some("s32le"), + _ => None, + }; + if let Some(pcm) = pcm { + let (hash, _) = run(&ffmpeg, &["-c:a", &format!("pcm_{pcm}"), "-f", "hash", "-hash", "md5", "-"], &path); + let hash = String::from_utf8_lossy(&hash); + let ours: String = si[18..34].iter().map(|b| format!("{b:02x}")).collect(); + assert_eq!(hash.trim().strip_prefix("MD5=").unwrap_or(""), ours, "{name}: MD5"); + } + let total = (u64::from(si[13] & 0x0F) << 32) | u64::from(u32::from_be_bytes([si[14], si[15], si[16], si[17]])); + assert_eq!(total, n as u64, "{name}: total samples"); + let si_rate = (u32::from(si[10]) << 12) | (u32::from(si[11]) << 4) | (u32::from(si[12]) >> 4); + assert_eq!(si_rate, rate); + Some(file.len()) +} + +#[test] +fn every_signal_round_trips_exactly() { + for (kind, seed) in [("music", 1), ("sweep", 2), ("noise", 3), ("square", 4), ("silence", 5), ("click", 6)] { + for bps in [16u8, 24, 32] { + let planar = vec![signal(kind, 20_000, bps, seed), signal(kind, 20_000, bps, seed + 100)]; + if round_trip(&format!("{kind}-{bps}"), &planar, EncoderConfig::new(48_000, 2, bps), 4800).is_none() { + return; + } + } + } +} + +#[test] +fn channel_layouts_rates_and_sizes() { + let cases: [(&str, u8, u8, u32, usize); 8] = [ + ("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), + ("5.1-24", 6, 24, 48_000, 12_345), + ("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), + ("tiny", 2, 16, 48_000, 1), + ]; + for (name, ch, bps, rate, n) in cases { + let planar: Vec> = (0..ch).map(|c| signal(if c == 3 { "silence" } else { "music" }, n, bps, u64::from(c) + 7)).collect(); + if round_trip(name, &planar, EncoderConfig::new(rate, ch, bps), 1000).is_none() { + return; + } + } +} + +#[test] +fn levels_and_block_sizes_trade_time_for_size() { + let planar = vec![signal("music", 48_000, 16, 11), signal("sweep", 48_000, 16, 12)]; + let mut sizes = Vec::new(); + for level in 0..=8 { + let cfg = EncoderConfig { level, ..EncoderConfig::new(48_000, 2, 16) }; + let Some(size) = round_trip(&format!("level{level}"), &planar, cfg, 48_000) else { return }; + sizes.push(size); + } + // the effortful levels are never larger than the fastest, and all beat PCM + assert!(sizes[8] <= sizes[0] && sizes[5] <= sizes[0], "{sizes:?}"); + assert!(sizes.iter().all(|&s| s < 48_000 * 4), "{sizes:?}"); + for bs in [16u16, 192, 1152, 4608, 65_535] { + let cfg = EncoderConfig { block_size: bs, ..EncoderConfig::new(48_000, 2, 16) }; + round_trip(&format!("block{bs}"), &planar, cfg, 777); + } +} + +#[test] +fn bad_input_is_an_error_not_a_panic() { + assert!(Encoder::new(EncoderConfig::new(48_000, 0, 16)).is_err()); + assert!(Encoder::new(EncoderConfig::new(48_000, 9, 16)).is_err()); + assert!(Encoder::new(EncoderConfig::new(48_000, 2, 33)).is_err()); + assert!(Encoder::new(EncoderConfig::new(48_000, 2, 7)).is_err()); + assert!(Encoder::new(EncoderConfig::new(0, 2, 16)).is_err()); + assert!(Encoder::new(EncoderConfig { block_size: 8, ..EncoderConfig::new(48_000, 2, 16) }).is_err()); + let mut enc = Encoder::new(EncoderConfig::new(48_000, 2, 16)).unwrap(); + assert_eq!(enc.encode(&[&[0, 1]]), Err(Error::Channels { expected: 2, got: 1 })); + assert_eq!(enc.encode(&[&[0, 1], &[0]]), Err(Error::UnequalChannels)); + assert_eq!(enc.encode(&[&[40_000], &[0]]), Err(Error::SampleRange(40_000))); + assert_eq!(enc.encode(&[&[-32_769], &[0]]), Err(Error::SampleRange(-32_769))); + // nothing encoded: no frames, and a header that still says "unknown" + assert!(enc.finish().is_empty()); + assert_eq!(&enc.header()[..4], b"fLaC"); +} + +/// 32-bit stereo at the extremes of i32: left minus right would need 33 bits, so the encoder must +/// not code a side channel (it overflowed `i32` and wrote garbage); every level decodes exactly. +#[test] +fn full_scale_32_bit_stereo_round_trips() { + let n = 9_000; + let left: Vec = (0..n).map(|i| if i % 3 == 0 { i32::MAX } else { i32::MIN }).collect(); + let right: Vec = left.iter().map(|&v| if v == i32::MAX { i32::MIN } else { i32::MAX }).collect(); + let planar = vec![left, right]; + for level in [0u8, 5, 8] { + let cfg = EncoderConfig { level, ..EncoderConfig::new(48_000, 2, 32) }; + if round_trip(&format!("extremes-32-level{level}"), &planar, cfg, 4096).is_none() { + return; + } + } + let mut enc = Encoder::new(EncoderConfig::new(48_000, 2, 32)).unwrap(); + assert!(enc.encode(&[&[i32::MIN, i32::MAX], &[0, -1]]).is_ok()); +} diff --git a/crates/audio/src/write/flac/predict.rs b/crates/audio/src/write/flac/predict.rs new file mode 100644 index 00000000..81646cce --- /dev/null +++ b/crates/audio/src/write/flac/predict.rs @@ -0,0 +1,327 @@ +//! Subframe prediction (RFC 9639 §9.2.5–9.2.6) and residual coding (§9.2.7): the fixed polynomial +//! predictors, linear prediction from the windowed autocorrelation (Levinson–Durbin), and +//! partitioned Rice codes. Everything here only measures and chooses; [`super::frame`] writes. + +/// Residuals larger than this are not Rice coded (a 5-bit parameter tops out at 30). +const MAX_RESIDUAL: i64 = (1 << 30) - 1; + +/// A residual coded with partitioned Rice codes. +#[derive(Clone, Debug, PartialEq)] +pub struct Residual { + /// Rice parameter per partition; `1 << order` partitions. + pub params: Vec, + pub partition_order: u8, + /// Total bits of the residual section, coding method included. + pub bits: u64, +} + +impl Residual { + /// The 5-bit parameter method is needed when any parameter passes 14 (15 is the escape code). + pub fn wide(&self) -> bool { + self.params.iter().any(|&k| k > 14) + } +} + +/// How one channel of one block is coded. +#[derive(Clone, Debug, PartialEq)] +pub enum Subframe { + Constant(i32), + Verbatim, + Fixed { order: u8, residual: Residual }, + Lpc { coefs: Vec, precision: u8, shift: u8, residual: Residual }, +} + +/// Folded residual: 0, -1, 1, -2, 2 … → 0, 1, 2, 3, 4 …. +fn fold(r: i64) -> u64 { + if r >= 0 { (r as u64) << 1 } else { ((-r - 1) as u64) << 1 | 1 } +} + +/// Bits of `n` folded values summing to `sum` with Rice parameter `k`: unary quotient, stop bit, +/// `k` low bits. The quotient total is approximated by `sum >> k`, exact up to the per-value +/// carries; [`rice_bits_exact`] gives the true cost for the chosen parameter. +fn rice_estimate(sum: u64, n: u64, k: u32) -> u64 { + (sum >> k) + n * (u64::from(k) + 1) +} + +fn rice_bits_exact(u: &[u64], k: u32) -> u64 { + u.iter().map(|&v| (v >> k) + u64::from(k) + 1).sum() +} + +/// The best parameter for a partition of folded values. +fn best_param(u: &[u64]) -> (u8, u64) { + let n = u.len() as u64; + let sum: u64 = u.iter().sum(); + if n == 0 { + return (0, 0); + } + let mean = sum / n; + let guess = if mean == 0 { 0 } else { 63 - mean.leading_zeros() }; + let mut best = (0u32, u64::MAX); + for k in guess.saturating_sub(1)..=(guess + 1).min(30) { + let b = rice_estimate(sum, n, k); + if b < best.1 { + best = (k, b); + } + } + (best.0 as u8, rice_bits_exact(u, best.0)) +} + +/// Choose the partition order and parameters for the residual of a block of `block` samples whose +/// first `order` samples are warm-up (`u` holds the folded residual after them). +fn code_residual(u: &[u64], block: usize, order: usize, max_partition_order: u8) -> Residual { + let mut best: Option = None; + for po in 0..=max_partition_order { + let parts = 1usize << po; + if !block.is_multiple_of(parts) { + break; + } + let len = block / parts; + if len <= order { + break; + } + let mut params = Vec::with_capacity(parts); + let mut bits = 0u64; + let mut start = 0usize; + for p in 0..parts { + let n = if p == 0 { len - order } else { len }; + let Some(chunk) = u.get(start..start + n) else { break }; + let (k, b) = best_param(chunk); + params.push(k); + bits += b; + start += n; + } + let wide = params.iter().any(|&k| k > 14); + bits += 2 + 4 + parts as u64 * if wide { 5 } else { 4 }; + if best.as_ref().is_none_or(|b| bits < b.bits) { + best = Some(Residual { params, partition_order: po, bits }); + } + } + best.unwrap_or(Residual { params: vec![0], partition_order: 0, bits: u64::MAX }) +} + +/// The residual of fixed predictor `order` (0–4), or None when it does not fit. +pub fn fixed_residual(x: &[i32], order: usize) -> Option> { + let x: Vec = x.iter().map(|&v| i64::from(v)).collect(); + let mut out = Vec::with_capacity(x.len().saturating_sub(order)); + for i in order..x.len() { + let s = |j: usize| x.get(i - j).copied().unwrap_or(0); + let r = match order { + 0 => s(0), + 1 => s(0) - s(1), + 2 => s(0) - 2 * s(1) + s(2), + 3 => s(0) - 3 * s(1) + 3 * s(2) - s(3), + _ => s(0) - 4 * s(1) + 6 * s(2) - 4 * s(3) + s(4), + }; + if r.abs() > MAX_RESIDUAL { + return None; + } + out.push(r); + } + Some(out) +} + +/// The residual of quantised linear prediction, or None when it does not fit. +pub fn lpc_residual(x: &[i32], coefs: &[i32], shift: u8) -> Option> { + let order = coefs.len(); + let mut out = Vec::with_capacity(x.len().saturating_sub(order)); + for i in order..x.len() { + let hist = x.get(i - order..i)?; + // coefs[0] weighs the previous sample + let pred: i64 = coefs.iter().zip(hist.iter().rev()).map(|(&c, &s)| i64::from(c) * i64::from(s)).sum(); + let r = i64::from(*x.get(i)?) - (pred >> shift); + if r.abs() > MAX_RESIDUAL { + return None; + } + out.push(r); + } + Some(out) +} + +/// Tukey(0.5) window: flat in the middle half, raised-cosine tapers. +fn window(n: usize) -> Vec { + let taper = (n / 4).max(1); + (0..n) + .map(|i| { + let edge = i.min(n - 1 - i); + if edge >= taper { 1.0 } else { 0.5 * (1.0 - (std::f64::consts::PI * edge as f64 / taper as f64).cos()) } + }) + .collect() +} + +/// Linear prediction coefficients of every order 1..=`max` (Levinson–Durbin on the windowed +/// autocorrelation); empty for a silent or degenerate block. +fn lpc_orders(x: &[i32], max: usize) -> Vec> { + let n = x.len(); + if n <= max { + return Vec::new(); + } + let w = window(n); + let s: Vec = x.iter().zip(&w).map(|(&v, &w)| f64::from(v) * w).collect(); + let r: Vec = (0..=max).map(|lag| s.iter().zip(s.iter().skip(lag)).map(|(a, b)| a * b).sum()).collect(); + let Some(&r0) = r.first() else { return Vec::new() }; + if r0 <= 0.0 { + return Vec::new(); + } + let mut err = r0 * (1.0 + 1e-9); + let mut a: Vec = Vec::new(); + let mut all = Vec::with_capacity(max); + for i in 0..max { + let ri = r.get(i + 1).copied().unwrap_or(0.0); + let acc: f64 = a.iter().enumerate().map(|(j, &aj)| aj * r.get(i - j).copied().unwrap_or(0.0)).sum(); + let k = (ri - acc) / err; + if !k.is_finite() { + break; + } + let prev = a.clone(); + a.push(k); + for j in 0..i { + if let (Some(aj), Some(&p)) = (a.get_mut(j), prev.get(i - 1 - j)) { + *aj -= k * p; + } + } + err *= 1.0 - k * k; + all.push(a.clone()); + if err <= 0.0 { + break; + } + } + all +} + +/// Quantise coefficients to `precision`-bit integers with a right shift (0–15), carrying the +/// rounding error forward so the quantised filter stays close to the real one. +fn quantise(coefs: &[f64], precision: u8) -> Option<(Vec, u8)> { + let cmax = coefs.iter().fold(0.0f64, |m, c| m.max(c.abs())); + if !(cmax.is_finite() && cmax > 0.0) { + return None; + } + let lim = (1i64 << (precision - 1)) - 1; + // the largest shift that keeps the biggest coefficient inside the precision + let log = cmax.log2().floor() as i32 + 1; + let shift = (i32::from(precision) - 1 - log).clamp(0, 15); + let scale = f64::from(1u32 << shift); + let mut carry = 0.0; + let mut q = Vec::with_capacity(coefs.len()); + for &c in coefs { + carry += c * scale; + let v = (carry.round() as i64).clamp(-lim - 1, lim); + carry -= v as f64; + q.push(v as i32); + } + Some((q, shift as u8)) +} + +/// Encoding effort, from the compression level. +#[derive(Clone, Copy, Debug)] +pub struct Effort { + pub max_lpc_order: usize, + pub max_partition_order: u8, + pub stereo: bool, +} + +impl Effort { + pub fn level(level: u8) -> Effort { + let (max_lpc_order, max_partition_order, stereo) = match level { + 0 => (0, 3, false), + 1 | 2 => (0, 3, true), + 3 => (6, 4, false), + 4 => (8, 4, true), + 5 => (8, 5, true), + 6 => (8, 6, true), + _ => (12, 6, true), + }; + Effort { max_lpc_order, max_partition_order, stereo } + } +} + +/// The cheapest coding of one channel of `bps`-bit samples, and its size in bits. +pub fn choose(x: &[i32], bps: u8, effort: Effort) -> (Subframe, u64) { + let n = x.len(); + let b = u64::from(bps); + let head = 8; // zero bit, type, wasted-bits flag + let first = x.first().copied().unwrap_or(0); + if x.iter().all(|&v| v == first) { + return (Subframe::Constant(first), head + b); + } + let mut best = (Subframe::Verbatim, head + n as u64 * b); + let mut consider = |sub: Subframe, bits: u64| { + if bits < best.1 { + best = (sub, bits); + } + }; + let folded = |r: &[i64]| r.iter().map(|&v| fold(v)).collect::>(); + for order in 0..=4usize.min(n.saturating_sub(1)) { + let Some(r) = fixed_residual(x, order) else { continue }; + let residual = code_residual(&folded(&r), n, order, effort.max_partition_order); + let bits = head + order as u64 * b + residual.bits; + consider(Subframe::Fixed { order: order as u8, residual }, bits); + } + // 15-bit coefficients, fewer for short blocks (their side information weighs more) + let precision: u8 = if n <= 1152 { 12 } else { 15 }; + for coefs in lpc_orders(x, effort.max_lpc_order) { + let Some((q, shift)) = quantise(&coefs, precision) else { continue }; + let order = q.len(); + let Some(r) = lpc_residual(x, &q, shift) else { continue }; + let residual = code_residual(&folded(&r), n, order, effort.max_partition_order); + let bits = head + order as u64 * (b + u64::from(precision)) + 4 + 5 + residual.bits; + consider(Subframe::Lpc { coefs: q, precision, shift, residual }, bits); + } + best +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn folding() { + assert_eq!([0, -1, 1, -2, 2].map(fold), [0, 1, 2, 3, 4]); + } + + #[test] + fn predictable_signals_beat_verbatim() { + // a ramp is exactly predicted by the order-2 fixed predictor + let ramp: Vec = (0..4096).map(|i| i * 3 - 6000).collect(); + let (sub, bits) = choose(&ramp, 16, Effort::level(5)); + assert!(matches!(sub, Subframe::Fixed { order: 2, .. } | Subframe::Lpc { .. }), "{sub:?}"); + assert!(bits < 4096 * 2, "{bits}"); + // a sine is well predicted by LPC + let sine: Vec = (0..4096).map(|i| ((i as f64 * 0.05).sin() * 20000.0) as i32).collect(); + let (_, bits) = choose(&sine, 16, Effort::level(5)); + assert!(bits < 4096 * 6, "{bits}"); + assert_eq!(choose(&[7; 100], 16, Effort::level(5)), (Subframe::Constant(7), 24)); + } + + #[test] + fn lpc_residual_inverts() { + let x: Vec = (0..500).map(|i| ((i as f64 * 0.1).sin() * 1000.0) as i32).collect(); + let coefs = lpc_orders(&x, 4).pop().unwrap(); + let (q, shift) = quantise(&coefs, 15).unwrap(); + let r = lpc_residual(&x, &q, shift).unwrap(); + // rebuild the signal from the warm-up and the residual, as a decoder would + let mut y: Vec = x[..q.len()].to_vec(); + for (i, &res) in r.iter().enumerate() { + let p: i64 = q.iter().zip(y[i..i + q.len()].iter().rev()).map(|(&c, &s)| i64::from(c) * i64::from(s)).sum(); + y.push((res + (p >> shift)) as i32); + } + assert_eq!(y, x); + } + + #[test] + fn noise_and_extremes_do_not_panic() { + let mut seed = 1u32; + let noise: Vec = (0..4096) + .map(|_| { + seed = seed.wrapping_mul(1_664_525).wrapping_add(1_013_904_223); + (seed >> 8) as i32 - (1 << 23) + }) + .collect(); + let (_, bits) = choose(&noise, 24, Effort::level(8)); + assert!(bits <= 8 + 4096 * 24); + let square: Vec = (0..4096).map(|i| if i % 2 == 0 { (1 << 23) - 1 } else { -(1 << 23) }).collect(); + choose(&square, 24, Effort::level(8)); + choose(&[], 16, Effort::level(5)); + choose(&[1], 16, Effort::level(5)); + choose(&[1, 2], 16, Effort::level(8)); + } +} diff --git a/crates/audio/src/write/mod.rs b/crates/audio/src/write/mod.rs index 8e845aa1..cb75f0d3 100644 --- a/crates/audio/src/write/mod.rs +++ b/crates/audio/src/write/mod.rs @@ -1,49 +1,83 @@ -//! Encoding: WAV/BWF/RF64 and AIFF/AIFF-C writers (`pcm-encode`), whole-buffer and streaming. +//! Encoding: WAV/BWF/RF64 and AIFF/AIFF-C writers (`pcm-encode`) and the FLAC encoder +//! (`flac-encode`), whole-buffer and streaming. +#[cfg(feature = "pcm-encode")] mod aiff; +#[cfg(feature = "flac-encode")] +mod flac; mod pcm; +#[cfg(feature = "pcm-encode")] mod wav; use crate::{AudioBuffer, AudioError, ChannelLayout, EncodeFormat, EncodeOptions, Result}; use std::io::{Seek, Write}; -/// Encode `buf` as WAV/BWF or AIFF/AIFF-C. Integer depths clamp samples to [-1, 1]; float +/// Encode `buf` as WAV/BWF, AIFF/AIFF-C or FLAC. Integer depths clamp samples to [-1, 1]; float /// keeps values above full scale. `opts.layout`, when set, must have `buf`'s channel count. /// /// ``` +/// # #[cfg(feature = "pcm-encode")] { /// use craft_audio::{AudioBuffer, BitDepth, EncodeFormat, EncodeOptions, encode}; /// /// let buf = AudioBuffer::new(48_000, 2, 48_000); // one second of stereo silence /// let opts = EncodeOptions::new(EncodeFormat::Wav).with_bit_depth(BitDepth::Int24).with_dither(true); /// let wav: Vec = encode(&buf, &opts).unwrap(); /// assert_eq!(&wav[..4], b"RIFF"); +/// # } /// ``` pub fn encode(buf: &AudioBuffer, opts: &EncodeOptions) -> Result> { // Checked for every format; only a WAV records it. + #[cfg_attr(not(feature = "pcm-encode"), allow(unused_variables))] let mask = opts.layout.map(|l| wav_mask(&l, buf.channels.len())).transpose()?; match opts.format { + #[cfg(feature = "pcm-encode")] EncodeFormat::Wav => wav::encode(buf, opts.bit_depth, opts.dither, opts.bwf.as_ref(), opts.force_rf64, mask), + #[cfg(feature = "pcm-encode")] EncodeFormat::Aiff => aiff::encode(buf, opts.bit_depth, opts.dither), + #[cfg(feature = "flac-encode")] + EncodeFormat::Flac => flac::encode(buf, opts.bit_depth, opts.dither), + #[allow(unreachable_patterns)] + f => Err(not_built(f)), } } -/// The file extensions of the formats [`encode`] can write. -pub const ENCODE_EXTENSIONS: &[&str] = &["wav", "wave", "bwf", "aif", "aiff", "aifc"]; +/// The error for a format whose writer is not built. +#[allow(dead_code)] +fn not_built(format: EncodeFormat) -> AudioError { + let feature = if format == EncodeFormat::Flac { "flac-encode" } else { "pcm-encode" }; + AudioError::Unsupported(format!("writing {format:?} needs the `{feature}` feature")) +} -/// The format [`encode`] writes for a file extension or format name (`wav`, `aiff`, case-insensitive, -/// with or without the dot), or `None` when it cannot write it. `aifc` is AIFF: float AIFF is -/// written as AIFF-C. +/// The file extensions of the formats [`encode`] can write (those whose features are on). +#[cfg(all(feature = "pcm-encode", feature = "flac-encode"))] +pub const ENCODE_EXTENSIONS: &[&str] = &["wav", "wave", "bwf", "aif", "aiff", "aifc", "flac"]; +/// The file extensions of the formats [`encode`] can write (those whose features are on). +#[cfg(all(feature = "pcm-encode", not(feature = "flac-encode")))] +pub const ENCODE_EXTENSIONS: &[&str] = &["wav", "wave", "bwf", "aif", "aiff", "aifc"]; +/// The file extensions of the formats [`encode`] can write (those whose features are on). +#[cfg(all(feature = "flac-encode", not(feature = "pcm-encode")))] +pub const ENCODE_EXTENSIONS: &[&str] = &["flac"]; + +/// The format [`encode`] writes for a file extension or format name (`wav`, `aiff`, `flac`, +/// case-insensitive, with or without the dot), or `None` when it cannot write it. `aifc` is +/// AIFF: float AIFF is written as AIFF-C. /// /// ``` +/// # #[cfg(feature = "pcm-encode")] { /// use craft_audio::{EncodeFormat, encode_format_for}; /// /// assert_eq!(encode_format_for(".AIFF"), Some(EncodeFormat::Aiff)); /// assert_eq!(encode_format_for("mp3"), None); +/// # } /// ``` pub fn encode_format_for(name: &str) -> Option { match name.trim_start_matches('.').to_ascii_lowercase().as_str() { + #[cfg(feature = "pcm-encode")] "wav" | "wave" | "bwf" => Some(EncodeFormat::Wav), + #[cfg(feature = "pcm-encode")] "aif" | "aiff" | "aifc" => Some(EncodeFormat::Aiff), + #[cfg(feature = "flac-encode")] + "flac" => Some(EncodeFormat::Flac), _ => None, } } @@ -64,24 +98,26 @@ fn failed_write_error() -> AudioError { AudioError::Encode("the stream encoder already failed on an earlier write; its output is incomplete".into()) } -/// Writes WAV/BWF (RF64 above 4 GiB) or AIFF / AIFF-C `fl32` block by block, producing +/// Writes WAV/BWF (RF64 above 4 GiB), AIFF / AIFF-C `fl32` or FLAC block by block, producing /// the same bytes [`encode`] would for the whole buffer: a file of any length never /// has to fit in memory. The file starts at the sink's position, and nothing may follow that /// position (a new file, or one truncated there): the encoder cannot truncate a `Write + Seek`, /// so the bytes of a longer file already there would stay after the new one. /// /// ``` +/// # #[cfg(feature = "flac-encode")] { /// use craft_audio::{EncodeFormat, EncodeOptions, FileEncoder}; /// use std::io::Cursor; /// -/// let opts = EncodeOptions::new(EncodeFormat::Aiff); +/// let opts = EncodeOptions::new(EncodeFormat::Flac); /// let mut enc = FileEncoder::new(Cursor::new(Vec::new()), &opts, 48_000, 2).unwrap(); /// let block = [vec![0.25f32; 4096], vec![-0.25f32; 4096]]; // one Vec (or slice) per channel /// for _ in 0..10 { /// enc.write(&block, 4096).unwrap(); /// } -/// let aiff = enc.finish().unwrap().into_inner(); -/// assert_eq!(&aiff[..4], b"FORM"); +/// let flac = enc.finish().unwrap().into_inner(); +/// assert_eq!(&flac[..4], b"fLaC"); +/// # } /// ``` pub struct FileEncoder { writer: Writer, @@ -91,16 +127,24 @@ pub struct FileEncoder { impl std::fmt::Debug for FileEncoder { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let format = match &self.writer { + #[cfg(feature = "pcm-encode")] Writer::Wav(_) => EncodeFormat::Wav, + #[cfg(feature = "pcm-encode")] Writer::Aiff(_) => EncodeFormat::Aiff, + #[cfg(feature = "flac-encode")] + Writer::Flac(_) => EncodeFormat::Flac, }; f.debug_struct("FileEncoder").field("format", &format).finish_non_exhaustive() } } enum Writer { + #[cfg(feature = "pcm-encode")] Wav(wav::Writer), + #[cfg(feature = "pcm-encode")] Aiff(aiff::Writer), + #[cfg(feature = "flac-encode")] + Flac(flac::Writer), } impl FileEncoder { @@ -108,8 +152,10 @@ impl FileEncoder { /// `opts.layout`, when set, must have `channels` channels. pub fn new(out: W, opts: &EncodeOptions, sample_rate: u32, channels: u16) -> Result { let (depth, dither, channels) = (opts.bit_depth, opts.dither, usize::from(channels)); + #[cfg_attr(not(feature = "pcm-encode"), allow(unused_variables))] let mask = opts.layout.map(|l| wav_mask(&l, channels)).transpose()?; let writer = match opts.format { + #[cfg(feature = "pcm-encode")] EncodeFormat::Wav => Writer::Wav(wav::Writer::new( out, channels, @@ -118,17 +164,26 @@ impl FileEncoder { dither, wav::HeaderExtras { bwf: opts.bwf.as_ref(), mask, force_rf64: opts.force_rf64 }, )?), + #[cfg(feature = "pcm-encode")] EncodeFormat::Aiff => Writer::Aiff(aiff::Writer::new(out, channels, sample_rate, depth, dither)?), + #[cfg(feature = "flac-encode")] + EncodeFormat::Flac => Writer::Flac(flac::Writer::new(out, channels, sample_rate, depth, dither)?), + #[allow(unreachable_patterns)] + f => return Err(not_built(f)), }; Ok(FileEncoder { writer, channels }) } /// Fails, without writing anything, when `frames` more frames would not fit the format (an - /// AIFF holds 4 GiB). + /// AIFF holds 4 GiB, a FLAC 2^36 frames). pub fn check_room(&self, frames: u64) -> Result<()> { match &self.writer { + #[cfg(feature = "pcm-encode")] Writer::Wav(w) => w.check_room(frames), + #[cfg(feature = "pcm-encode")] Writer::Aiff(w) => w.check_room(frames), + #[cfg(feature = "flac-encode")] + Writer::Flac(w) => w.check_room(frames), } .map(drop) } @@ -147,16 +202,24 @@ impl FileEncoder { return Err(AudioError::Encode(format!("{frames} frames asked for, but a channel holds {short}"))); } match &mut self.writer { + #[cfg(feature = "pcm-encode")] Writer::Wav(w) => w.write(block, frames), + #[cfg(feature = "pcm-encode")] Writer::Aiff(w) => w.write(block, frames), + #[cfg(feature = "flac-encode")] + Writer::Flac(w) => w.write(block, frames), } } /// Completes the headers and hands back `out`, positioned at the end of the file. pub fn finish(self) -> Result { match self.writer { + #[cfg(feature = "pcm-encode")] Writer::Wav(w) => w.finish(), + #[cfg(feature = "pcm-encode")] Writer::Aiff(w) => w.finish(), + #[cfg(feature = "flac-encode")] + Writer::Flac(w) => w.finish(), } } } @@ -175,9 +238,28 @@ impl FileEncoder { /// let header = wav_float_header(48_000, 2, 2).unwrap(); /// file[..header.len()].copy_from_slice(&header); /// ``` +#[cfg(feature = "pcm-encode")] pub fn wav_float_header(sample_rate: u32, channels: u16, frames: u64) -> Result> { if channels == 0 || sample_rate == 0 { return Err(AudioError::Encode("a WAV needs channels and a sample rate".into())); } wav::header(usize::from(channels), sample_rate, (32, true), None, None, frames, false) } + +// With only one of `pcm-encode` and `flac-encode`, the other's formats are refused, not silently +// written as something else. +#[cfg(all(test, not(all(feature = "pcm-encode", feature = "flac-encode"))))] +mod tests { + use super::*; + + #[test] + fn unbuilt_format_is_unsupported() { + let (format, name) = if cfg!(feature = "pcm-encode") { (EncodeFormat::Flac, "flac") } else { (EncodeFormat::Wav, "wav") }; + let opts = EncodeOptions::new(format); + let buf = AudioBuffer::new(48_000, 1, 16); + assert!(matches!(encode(&buf, &opts), Err(AudioError::Unsupported(_)))); + let enc = FileEncoder::new(std::io::Cursor::new(Vec::new()), &opts, 48_000, 1); + assert!(matches!(enc, Err(AudioError::Unsupported(_)))); + assert_eq!(encode_format_for(name), None); + } +} diff --git a/crates/audio/src/write/pcm.rs b/crates/audio/src/write/pcm.rs index 834cd687..8374f3a0 100644 --- a/crates/audio/src/write/pcm.rs +++ b/crates/audio/src/write/pcm.rs @@ -1,4 +1,4 @@ -//! PCM helpers shared by the WAV and AIFF writers: quantization (with optional TPDF +//! PCM helpers shared by the WAV, AIFF and FLAC writers: quantization (with optional TPDF //! dither), interleaving and layout checks. use crate::{AudioBuffer, AudioError, Result}; diff --git a/crates/audio/tests/audio/encode_format.rs b/crates/audio/tests/audio/encode_format.rs index 67c5da07..fb47652c 100644 --- a/crates/audio/tests/audio/encode_format.rs +++ b/crates/audio/tests/audio/encode_format.rs @@ -9,10 +9,11 @@ fn only_formats_the_encoder_writes_are_named() { assert_eq!(encode_format_for("wave"), Some(EncodeFormat::Wav)); assert_eq!(encode_format_for("BWF"), Some(EncodeFormat::Wav)); assert_eq!(encode_format_for(".aifc"), Some(EncodeFormat::Aiff)); + assert_eq!(encode_format_for("flac"), Some(EncodeFormat::Flac)); for name in craft_audio::ENCODE_EXTENSIONS { assert!(encode_format_for(name).is_some(), "{name}"); } - for other in ["flac", "mp3", "ogg", "m4a", "xyz", ""] { + for other in ["mp3", "ogg", "m4a", "xyz", ""] { assert_eq!(encode_format_for(other), None, "{other}"); } } diff --git a/crates/audio/tests/audio/float_headroom.rs b/crates/audio/tests/audio/float_headroom.rs index a16098b2..e0c62204 100644 --- a/crates/audio/tests/audio/float_headroom.rs +++ b/crates/audio/tests/audio/float_headroom.rs @@ -89,7 +89,7 @@ fn float32_keeps_non_finite_sample_handling() { fn integer_pcm_still_clips_at_full_scale() { let audio = AudioBuffer { sample_rate: 48_000, channels: vec![vec![-2.0, -1.25, -1.0, -0.5, 0.0, 0.5, 1.0, 1.25, 2.0]] }; for (depth, max) in [(BitDepth::Int16, 32767.0 / 32768.0), (BitDepth::Int24, 8_388_607.0 / 8_388_608.0), (BitDepth::Int32, 1.0)] { - for format in [EncodeFormat::Wav, EncodeFormat::Aiff] { + for format in [EncodeFormat::Wav, EncodeFormat::Aiff, EncodeFormat::Flac] { for streaming in [false, true] { let options = EncodeOptions::new(format).with_bit_depth(depth).with_dither(false); let bytes = encoded(&audio, &options, streaming); diff --git a/crates/audio/tests/audio/malformed.rs b/crates/audio/tests/audio/malformed.rs index 2486c232..c5deb532 100644 --- a/crates/audio/tests/audio/malformed.rs +++ b/crates/audio/tests/audio/malformed.rs @@ -62,6 +62,8 @@ fn every_truncation_is_handled() { small(EncodeFormat::Wav, BitDepth::Float32, 6), small(EncodeFormat::Aiff, BitDepth::Int24, 1), small(EncodeFormat::Aiff, BitDepth::Float32, 2), + small(EncodeFormat::Flac, BitDepth::Int16, 2), + small(EncodeFormat::Flac, BitDepth::Int24, 1), ]; for file in &files { for n in 0..file.len() { @@ -85,7 +87,9 @@ fn truncated_data_chunk_yields_partial_audio() { #[test] fn every_single_byte_corruption_is_handled() { - for file in [small(EncodeFormat::Wav, BitDepth::Int24, 2), small(EncodeFormat::Aiff, BitDepth::Int16, 2)] { + for file in + [small(EncodeFormat::Wav, BitDepth::Int24, 2), small(EncodeFormat::Aiff, BitDepth::Int16, 2), small(EncodeFormat::Flac, BitDepth::Int16, 1)] + { for i in 0..file.len().min(200) { for v in [0x00, 0xFF, 0x7F, 0x80] { let mut f = file.clone(); @@ -305,6 +309,29 @@ fn packets_longer_than_any_codec_uses_are_refused() { assert!(matches!(decode(&mp4(480, &alac(1 << 21), |_, a| a), None), Err(AudioError::Malformed(m)) if m.contains("declares packets"))); } +#[test] +fn flac_metadata_fields_beyond_their_block_are_refused() { + let with_block = |kind: u8, body: &[u8]| { + let f = small(EncodeFormat::Flac, BitDepth::Int16, 1); + let head = ((0x80 | u32::from(kind)) << 24 | body.len() as u32).to_be_bytes(); + [&f[..4], &[f[4] & 0x7f], &f[5..42], &head, body, &f[42..]].concat() + }; + let picture = |data: u32| [&[0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 0][..], &[0; 16], &data.to_be_bytes(), &[1, 2, 3]].concat(); + assert_eq!(decode(&with_block(6, &picture(3)), None).unwrap().1.frames(), 300); + let comment = [&0u32.to_le_bytes()[..], &1u32.to_le_bytes(), &u32::MAX.to_le_bytes()].concat(); + for f in [with_block(6, &picture(0x7fff_fff0)), with_block(4, &comment)] { + assert!(matches!(decode(&f, None), Err(AudioError::Malformed(m)) if m.contains("larger than its block"))); + } +} + +#[test] +fn an_id3_tag_before_the_audio_is_skipped_unread() { + let frame = [&b"TIT2"[..], &0x7fff_ffffu32.to_be_bytes(), &[0, 0, 3, b'h', b'i', 0]].concat(); + let tag = [&b"ID3\x03\0\0\0\0\0"[..], &[frame.len() as u8], &frame].concat(); + let flac = small(EncodeFormat::Flac, BitDepth::Int16, 1); + assert_eq!(decode(&[tag, flac].concat(), None).unwrap().1.frames(), 300); +} + /// Container magic inside an MP3 frame payload must not change detection or make probing reject the stream. #[test] fn a_valid_mp3_with_incidental_container_magic_still_probes() { @@ -366,6 +393,40 @@ fn an_exhausted_audio_score_budget_does_not_reject_the_mp3_behind_it() { assert_eq!(decoded.frames, buf.frames() as u64); } +/// Past the score budget the preflight does not run, so if the probe then selects a container +/// whose reader allocates from sizes in the file, the post-probe refusal still applies. +#[test] +fn a_container_selected_without_its_size_check_is_refused() { + let flac = small(EncodeFormat::Flac, BitDepth::Int16, 1); + // Exactly the cap of false candidates, then one more that the reader rejects (invalid + // bit-rate), so the preflight stops unchecked before reaching the FLAC marker behind it. + let mut false_syncs: Vec = (0..256).flat_map(|_| [0xFFu8, 0xE2]).collect(); + false_syncs.extend_from_slice(&[0xFF, 0xFB, 0xF0, 0x00]); + let f = [false_syncs, flac].concat(); + assert_eq!(detect_container(&f, None), Some(Container::MpegAudio)); + assert!(matches!(probe(&f, None), Err(AudioError::Malformed(m)) if m.contains("was not size-checked"))); + assert!(matches!(decode(&f, None), Err(AudioError::Malformed(m)) if m.contains("was not size-checked"))); +} + +/// A zero-frame FLAC after arbitrary bytes or ID3 tags is still detected and reported as an empty stream, not a +/// decode failure. +#[test] +fn zero_frame_flac_after_a_prefix_is_still_empty() { + let flac = + encode(&AudioBuffer::new(48_000, 1, 0), &EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(BitDepth::Int16).with_dither(false)).unwrap(); + let tag = id3_tag(b"tag"); + for (f, detected) in [ + ([vec![0x7A; 64], flac.clone()].concat(), None), + ([tag.clone(), vec![0x7A; 16], flac.clone()].concat(), Some(Container::MpegAudio)), + ([tag.clone(), tag, flac.clone()].concat(), Some(Container::MpegAudio)), + ] { + assert_eq!(detect_container(&f, None), detected); + let info = probe(&f, None).expect("empty FLAC after a prefix"); + assert_eq!((info.container, info.sample_rate, info.channels, info.frames), (Container::Flac, 48_000, 1, 0)); + assert_eq!(decode(&f, None).unwrap().1.frames(), 0); + } +} + /// Build one MPEG-4 AAC-LC ADTS frame at 48 kHz stereo around `payload`, with the 13-bit length covering the 7-byte /// header. fn adts_frame(payload: &[u8]) -> Vec { @@ -378,6 +439,19 @@ fn adts_frame(payload: &[u8]) -> Vec { [&h[..], payload].concat() } +/// False MPEG sync prefixes must not stop a following zero-frame FLAC from probing as empty. +#[test] +fn zero_frame_flac_after_a_false_mpeg_sync_is_still_empty() { + let flac = + encode(&AudioBuffer::new(48_000, 1, 0), &EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(BitDepth::Int16).with_dither(false)).unwrap(); + for junk in [vec![0xFF, 0xE0, 0x00], vec![0xFF, 0xF0, 0x00], vec![0xFF, 0xFB, 0xF0, 0x00]] { + let f = [junk, flac.clone()].concat(); + let info = probe(&f, None).expect("empty FLAC after a false sync"); + assert_eq!((info.container, info.sample_rate, info.channels, info.frames), (Container::Flac, 48_000, 1, 0)); + assert_eq!(decode(&f, None).unwrap().1.frames(), 0); + } +} + /// A false MPEG sync prefix must not let malformed CAF or MP4 size fields bypass the container preflight. #[test] fn a_false_sync_does_not_bypass_container_checks() { @@ -550,6 +624,7 @@ fn aiff_sowt_little_endian() { fn detect_container_by_magic_and_hint() { assert_eq!(detect_container(&small(EncodeFormat::Wav, BitDepth::Int16, 1), Some("mp3")), Some(Container::Wav)); assert_eq!(detect_container(&small(EncodeFormat::Aiff, BitDepth::Int16, 1), None), Some(Container::Aiff)); + assert_eq!(detect_container(&small(EncodeFormat::Flac, BitDepth::Int16, 1), None), Some(Container::Flac)); assert_eq!(detect_container(b"OggS\0\x02rest", None), Some(Container::Ogg)); assert_eq!(detect_container(b"caff\0\x01\0\0", None), Some(Container::Caf)); assert_eq!(detect_container(b"ID3\x04\0\0\0\0\0\x00", None), Some(Container::MpegAudio)); @@ -591,6 +666,20 @@ fn garbage_in_compressed_containers_errors() { assert!(decode(&[], None).is_err()); } +/// A marker that passes its container check must not let the probe select a different container +/// without its own checks: the reader is refused when the probed format disagrees. +#[test] +fn a_probed_container_that_disagrees_with_the_checked_marker_is_refused() { + // The junk byte hides both magics, so the hint picks the marker: the FLAC is checked, but the + // probe reads the WAVE before it. + let bytes = [vec![0], small(EncodeFormat::Wav, BitDepth::Int24, 2), small(EncodeFormat::Flac, BitDepth::Int24, 2)].concat(); + match decode(&bytes, Some("flac")) { + Err(AudioError::Malformed(message)) => assert!(message.contains("probed container"), "unexpected message: {message}"), + Err(other) => panic!("unexpected error kind: {other:?}"), + Ok(_) => panic!("the mismatched container must be refused"), + } +} + proptest! { #![proptest_config(ProptestConfig { cases: 256, ..ProptestConfig::default() })] @@ -614,6 +703,16 @@ proptest! { let _ = probe(&f, None); let _ = decode(&f, None); } + + #[test] + fn random_audio_roundtrips_through_flac(samples in proptest::collection::vec(-1.2f32..1.2, 1..3000), channels in 1usize..4) { + let buf = AudioBuffer::from_interleaved(48_000, channels as u16, &samples); + prop_assume!(buf.frames() > 0); + let opts = EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(BitDepth::Int24).with_dither(false); + let (_, flac) = decode(&encode(&buf, &opts).unwrap(), None).unwrap(); + let (_, wav) = decode(&encode(&buf, &opts.clone().with_format(EncodeFormat::Wav)).unwrap(), None).unwrap(); + prop_assert_eq!(flac, wav); + } } /// An extensible `fmt ` chunk: cbSize, valid bits, mask, then the 16-byte subtype GUID. diff --git a/crates/audio/tests/audio/roundtrip.rs b/crates/audio/tests/audio/roundtrip.rs index 51198048..67db9880 100644 --- a/crates/audio/tests/audio/roundtrip.rs +++ b/crates/audio/tests/audio/roundtrip.rs @@ -70,6 +70,66 @@ fn wav_and_aiff_roundtrip_all_depths_and_layouts() { } } +#[test] +fn flac_roundtrip_is_lossless_vs_wav() { + for depth in [BitDepth::Int16, BitDepth::Int24, BitDepth::Int32] { + for channels in [1, 2, 6] { + // Spans several blocks plus a short final block. + let src = sine(44_100, channels, 4096 * 2 + 17); + let wav = encode(&src, &EncodeOptions::new(EncodeFormat::Wav).with_bit_depth(depth).with_dither(false)).unwrap(); + let flac = encode(&src, &EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(depth).with_dither(false)).unwrap(); + assert_eq!(detect_container(&flac, None), Some(craft_audio::Container::Flac)); + let info = probe(&flac, None).unwrap(); + assert_eq!((info.container, info.codec), (craft_audio::Container::Flac, craft_audio::CodecId::Flac)); + assert_eq!((info.sample_rate, info.channels as usize, info.frames), (44_100, channels, src.frames() as u64)); + assert_eq!(info.sample_format, expected_sf(depth)); + let (_, from_wav) = decode(&wav, None).unwrap(); + let (finfo, from_flac) = decode(&flac, None).unwrap(); + assert_eq!(finfo.frames, src.frames() as u64); + assert_eq!(from_flac, from_wav, "{depth:?} {channels}ch"); + if depth != BitDepth::Int32 { + assert!(flac.len() < wav.len(), "{depth:?} {channels}ch: flac {} >= wav {}", flac.len(), wav.len()); + } + } + } +} + +#[test] +fn flac_handles_silence_noise_full_scale_and_odd_rates() { + let frames = 10_000; + let mut b = AudioBuffer::new(37_123, 3, frames); + let mut seed = 1u32; + for i in 0..frames { + seed = seed.wrapping_mul(1_664_525).wrapping_add(1_013_904_223); + b.channels[1][i] = (seed as f32 / u32::MAX as f32) * 2.0 - 1.0; // white noise + b.channels[2][i] = if i % 2 == 0 { 1.5 } else { -1.5 }; // clipped full-scale square + } + for depth in [BitDepth::Int16, BitDepth::Int24, BitDepth::Int32] { + for dither in [false, true] { + let opts = EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(depth).with_dither(dither); + let flac = encode(&b, &opts).unwrap(); + let (info, out) = decode(&flac, None).unwrap(); + assert_eq!(info.sample_rate, 37_123); + let wav = encode(&b, &opts.clone().with_format(EncodeFormat::Wav)).unwrap(); + let (_, expect) = decode(&wav, None).unwrap(); + assert_eq!(out, expect, "{depth:?} dither={dither}"); + } + } +} + +#[test] +fn flac_rejects_unsupported_layouts() { + let opts = EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(BitDepth::Float32).with_dither(false); + assert!(encode(&sine(48_000, 2, 10), &opts).is_err()); + let opts = opts.clone().with_bit_depth(BitDepth::Int16); + assert!(encode(&sine(48_000, 9, 10), &opts).is_err()); + assert!(encode(&sine(700_000, 1, 10), &opts).is_err()); + // Tiny buffers still encode and decode. + let one = sine(48_000, 2, 1); + let (_, out) = decode(&encode(&one, &opts).unwrap(), None).unwrap(); + assert_eq!(out.frames(), 1); +} + #[test] fn bwf_roundtrip() { let bwf = BwfInfo::default() @@ -80,8 +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 { let opts = EncodeOptions::new(EncodeFormat::Wav).with_bit_depth(depth).with_dither(false).with_bwf(bwf.clone()); let bytes = encode(&src, &opts).unwrap(); @@ -149,8 +209,11 @@ fn encode_rejects_bad_buffers_and_formats() { // Non-finite samples are sanitized; only integer output clips finite samples. let mut b = AudioBuffer::new(48_000, 1, 4); b.channels[0] = vec![f32::NAN, f32::INFINITY, -5.0, 0.5]; - for format in [EncodeFormat::Wav, EncodeFormat::Aiff] { + for format in [EncodeFormat::Wav, EncodeFormat::Aiff, EncodeFormat::Flac] { for depth in DEPTHS { + if format == EncodeFormat::Flac && depth == BitDepth::Float32 { + continue; + } let bytes = encode(&b, &EncodeOptions::new(format).with_bit_depth(depth)).unwrap(); let (_, out) = decode(&bytes, None).unwrap(); assert_eq!(out.channels[0][0], 0.0); @@ -214,6 +277,10 @@ fn multichannel_wav_speaker_masks() { let fmt = bytes.windows(4).position(|w| w == b"fmt ").unwrap(); assert_eq!(bytes.get(fmt + 8..fmt + 10), Some(&[1u8, 0][..])); assert_eq!(probe(&bytes, None).unwrap().layout, ChannelLayout::STEREO); + // FLAC (decoded by symphonia) reports its channel assignment: six channels are 5.1. + let flac = o16.clone().with_format(EncodeFormat::Flac); + let info = probe(&craft_audio::encode(&buf(6), &flac).unwrap(), None).unwrap(); + assert_eq!((info.container, info.codec, info.layout), (craft_audio::Container::Flac, craft_audio::CodecId::Flac, ChannelLayout::SURROUND_5_1)); // A layout for another channel count is refused. assert!(craft_audio::encode(&buf(2), &o16.clone().with_layout(ChannelLayout::SURROUND_5_1)).is_err()); } @@ -234,6 +301,31 @@ fn float_wav_header_is_rewritten_in_place_as_the_file_grows() { assert!(wav_float_header(48_000, 0, 0).is_err()); } +#[test] +fn zero_frame_flac_round_trips_like_wav_and_aiff() { + for format in [EncodeFormat::Wav, EncodeFormat::Aiff, EncodeFormat::Flac] { + for depth in [BitDepth::Int16, BitDepth::Int24] { + for channels in [1, 2] { + let opts = EncodeOptions::new(format).with_bit_depth(depth); + let bytes = encode(&AudioBuffer::new(48_000, channels, 0), &opts).unwrap(); + let info = probe(&bytes, None).unwrap(); + assert_eq!((info.container, info.sample_format, info.sample_rate, info.frames), (format.container(), expected_sf(depth), 48_000, 0)); + assert_eq!(info.channels, channels); + let (info, buf) = decode(&bytes, None).unwrap(); + assert_eq!((info.frames, buf.frames(), buf.num_channels()), (0, 0, channels)); + } + } + } +} + +#[test] +fn flac_claiming_frames_but_holding_none_still_errors() { + let opts = EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(BitDepth::Int16); + let mut bytes = encode(&AudioBuffer::new(48_000, 2, 0), &opts).unwrap(); + bytes[25] = 1; // STREAMINFO total samples (low byte of the 36-bit field) = 1 + assert!(decode(&bytes, None).is_err()); +} + #[test] fn float_wav_header_rejects_a_frame_count_whose_total_size_overflows() { // 4 bytes per frame: the data length fits in u64, but adding the header to it does not. diff --git a/crates/audio/tests/audio/stream.rs b/crates/audio/tests/audio/stream.rs index a948ccf7..b3271b40 100644 --- a/crates/audio/tests/audio/stream.rs +++ b/crates/audio/tests/audio/stream.rs @@ -33,6 +33,8 @@ fn streamed_files_match_whole_buffer_encodes() { (EncodeFormat::Wav, BitDepth::Int16, true, Some(bwf), Some(ChannelLayout::SURROUND_5_1_SIDE)), (EncodeFormat::Aiff, BitDepth::Int24, true, None, None), (EncodeFormat::Aiff, BitDepth::Float32, false, None, None), + (EncodeFormat::Flac, BitDepth::Int16, true, None, None), + (EncodeFormat::Flac, BitDepth::Int24, false, None, None), ]; for (format, bit_depth, dither, bwf, layout) in cases { let base = EncodeOptions::new(format).with_bit_depth(bit_depth).with_dither(dither); @@ -84,14 +86,14 @@ impl Seek for FailAfter { #[test] fn a_failed_write_poisons_the_encoder() { let buf = tone(2, 20_001); - for format in [EncodeFormat::Wav, EncodeFormat::Aiff] { + for format in [EncodeFormat::Wav, EncodeFormat::Aiff, EncodeFormat::Flac] { let opts = EncodeOptions::new(format).with_bit_depth(BitDepth::Int16).with_dither(false); let sink = FailAfter { inner: Cursor::new(Vec::new()), budget: 256 }; let mut enc = FileEncoder::new(sink, &opts, buf.sample_rate, 2).unwrap(); // The sink accepts a prefix of the first block, then fails. assert!(enc.write(&buf.channels, buf.frames()).is_err(), "{format:?}"); // Later calls must not claim success: the bytes on the sink no longer match the - // encoder's counters. + // encoder's counters (and for FLAC the failed frame's samples are already gone). assert!(matches!(enc.write(&buf.channels, 16), Err(AudioError::Encode(_))), "{format:?}"); assert!(matches!(enc.finish(), Err(AudioError::Encode(_))), "{format:?}"); } @@ -100,9 +102,7 @@ fn a_failed_write_poisons_the_encoder() { #[test] fn blocks_past_a_format_limit_are_refused_before_anything_is_written() { let buf = tone(2, 100); - // AIFF sizes are 32-bit: 4 GiB of 16-bit stereo. - { - let (format, limit) = (EncodeFormat::Aiff, 1u64 << 30); + for (format, limit) in [(EncodeFormat::Aiff, 1u64 << 30), (EncodeFormat::Flac, 1u64 << 36)] { let opts = EncodeOptions::new(format).with_bit_depth(BitDepth::Int16).with_dither(false); let mut enc = FileEncoder::new(Cursor::new(Vec::new()), &opts, buf.sample_rate, 2).unwrap(); enc.write(&buf.channels, 100).unwrap(); @@ -165,7 +165,7 @@ fn decoder_reads_any_read_seek_source_and_reports_source_failures() { fn seeking_to_or_past_the_end_ends_the_stream() { use craft_audio::{DecodeOptions, Decoder}; let buf = tone(2, 4800); - for format in [EncodeFormat::Wav, EncodeFormat::Aiff] { + for format in [EncodeFormat::Wav, EncodeFormat::Aiff, EncodeFormat::Flac] { let file = encode(&buf, &EncodeOptions::new(format).with_bit_depth(BitDepth::Int16)).unwrap(); for target in [4800, 4801, 1_000_000] { let mut dec = Decoder::from_bytes(&file, None, &DecodeOptions::default()).unwrap(); From 7c88c201a530f8e461f718cd1dc1d6d80f3d1b2f Mon Sep 17 00:00:00 2001 From: alehel Date: Tue, 3 Nov 2026 21:06:54 +0100 Subject: [PATCH 02/20] 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 --- crates/audio/README.md | 2 +- crates/audio/src/read/mod.rs | 2 +- crates/audio/src/write/flac/frame.rs | 31 +++++++++++++++++---- crates/audio/src/write/flac/mod.rs | 28 ++++++++++++------- crates/audio/src/write/flac/oracle_tests.rs | 6 +++- crates/audio/tests/audio/roundtrip.rs | 2 ++ crates/audio/tests/audio/stream.rs | 5 ++++ 7 files changed, 57 insertions(+), 19 deletions(-) diff --git a/crates/audio/README.md b/crates/audio/README.md index b6b9a9a5..3ba2fcde 100644 --- a/crates/audio/README.md +++ b/crates/audio/README.md @@ -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 diff --git a/crates/audio/src/read/mod.rs b/crates/audio/src/read/mod.rs index 2a62ef1e..f205e83b 100644 --- a/crates/audio/src/read/mod.rs +++ b/crates/audio/src/read/mod.rs @@ -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 = (0..4800).map(|i| (i as f32 * 0.05).sin() * 0.5).collect(); diff --git a/crates/audio/src/write/flac/frame.rs b/crates/audio/src/write/flac/frame.rs index 6b3d28e8..aa4bafaf 100644 --- a/crates/audio/src/write/flac/frame.rs +++ b/crates/audio/src/write/flac/frame.rs @@ -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 { 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)); + } } diff --git a/crates/audio/src/write/flac/mod.rs b/crates/audio/src/write/flac/mod.rs index b4069474..bbe46072 100644 --- a/crates/audio/src/write/flac/mod.rs +++ b/crates/audio/src/write/flac/mod.rs @@ -36,6 +36,9 @@ fn check(channels: usize, sample_rate: u32, depth: BitDepth) -> Result { } } +/// 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 Writer { 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(()) diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index 500365df..8fa63edd 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -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 { diff --git a/crates/audio/tests/audio/roundtrip.rs b/crates/audio/tests/audio/roundtrip.rs index 67db9880..a043a631 100644 --- a/crates/audio/tests/audio/roundtrip.rs +++ b/crates/audio/tests/audio/roundtrip.rs @@ -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 { diff --git a/crates/audio/tests/audio/stream.rs b/crates/audio/tests/audio/stream.rs index b3271b40..bfab2fbd 100644 --- a/crates/audio/tests/audio/stream.rs +++ b/crates/audio/tests/audio/stream.rs @@ -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::>(&[], frames), Err(AudioError::Encode(_))), "{format:?}"); From 76df23e86e08cd31c9ba4a467005fc106778216e Mon Sep 17 00:00:00 2001 From: alehel Date: Tue, 3 Nov 2026 21:14:20 +0100 Subject: [PATCH 03/20] craft-audio: review fixes, round 2 (#442) - check_room docs: FLAC holds at most 2^36 - 1 frames. - The FLAC round trip covers every channel count from 1 to 8. Co-Authored-By: Claude Opus 5.5 --- crates/audio/src/write/flac/mod.rs | 2 +- crates/audio/src/write/mod.rs | 2 +- crates/audio/tests/audio/roundtrip.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/audio/src/write/flac/mod.rs b/crates/audio/src/write/flac/mod.rs index bbe46072..5f25c25c 100644 --- a/crates/audio/src/write/flac/mod.rs +++ b/crates/audio/src/write/flac/mod.rs @@ -75,7 +75,7 @@ impl Writer { }) } - /// The frame count after `frames` more, or an error past what FLAC can count (2^36 frames). + /// The frame count after `frames` more, or an error past what FLAC can count (2^36 - 1 frames). pub(crate) fn check_room(&self, frames: u64) -> Result { let total = self.frames.saturating_add(frames); if total >= 1u64 << 36 { diff --git a/crates/audio/src/write/mod.rs b/crates/audio/src/write/mod.rs index cb75f0d3..13e42ea7 100644 --- a/crates/audio/src/write/mod.rs +++ b/crates/audio/src/write/mod.rs @@ -175,7 +175,7 @@ impl FileEncoder { } /// Fails, without writing anything, when `frames` more frames would not fit the format (an - /// AIFF holds 4 GiB, a FLAC 2^36 frames). + /// AIFF holds 4 GiB, a FLAC 2^36 - 1 frames). pub fn check_room(&self, frames: u64) -> Result<()> { match &self.writer { #[cfg(feature = "pcm-encode")] diff --git a/crates/audio/tests/audio/roundtrip.rs b/crates/audio/tests/audio/roundtrip.rs index a043a631..907a830d 100644 --- a/crates/audio/tests/audio/roundtrip.rs +++ b/crates/audio/tests/audio/roundtrip.rs @@ -73,7 +73,7 @@ fn wav_and_aiff_roundtrip_all_depths_and_layouts() { #[test] fn flac_roundtrip_is_lossless_vs_wav() { for depth in [BitDepth::Int16, BitDepth::Int24, BitDepth::Int32] { - for channels in [1, 2, 6] { + for channels in [1, 2, 3, 4, 5, 6, 7, 8] { // Spans several blocks plus a short final block. let src = sine(44_100, channels, 4096 * 2 + 17); let wav = encode(&src, &EncodeOptions::new(EncodeFormat::Wav).with_bit_depth(depth).with_dither(false)).unwrap(); From 1648f33e6a4ba4361926495968517332adf57728 Mon Sep 17 00:00:00 2001 From: alehel Date: Tue, 3 Nov 2026 21:23:52 +0100 Subject: [PATCH 04/20] craft-audio: review fixes, round 3 (#442) FLAC files record the speaker layout when FLAC's channel assignment does not already say it, as a WAVEFORMATEXTENSIBLE_CHANNEL_MASK comment. FLAC's 7-channel order is 6.1 with side surrounds, so a buffer in the crate's usual 7-channel layout (backs and a back centre) was decoded by ffmpeg and libFLAC with three channels on the wrong speakers; explicit layouts such as 5.1 with side surrounds were dropped. Co-Authored-By: Claude Opus 5.5 --- crates/audio/README.md | 2 +- crates/audio/src/lib.rs | 6 +++-- crates/audio/src/write/flac/encoder.rs | 12 ++++++--- crates/audio/src/write/flac/mod.rs | 28 ++++++++++++++++---- crates/audio/src/write/flac/oracle_tests.rs | 29 +++++++++++++++++++-- crates/audio/src/write/mod.rs | 6 ++--- 6 files changed, 65 insertions(+), 18 deletions(-) diff --git a/crates/audio/README.md b/crates/audio/README.md index 3ba2fcde..2000fe96 100644 --- a/crates/audio/README.md +++ b/crates/audio/README.md @@ -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, STREAMINFO MD5 checked against ffmpeg | +| FLAC encoder | RFC 9639 | 16, 24, 32 bits; 1–8 channels, layouts outside FLAC's channel assignment as a `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment; up to 655,350 Hz; LPC, stereo decorrelation, STREAMINFO MD5 | bit-exact through ffmpeg, STREAMINFO MD5 checked against ffmpeg | ## symphonia diff --git a/crates/audio/src/lib.rs b/crates/audio/src/lib.rs index cbdc3c07..70bf9a3f 100644 --- a/crates/audio/src/lib.rs +++ b/crates/audio/src/lib.rs @@ -268,8 +268,10 @@ pub struct EncodeOptions { pub bwf: Option, /// The speaker layout to record, for layouts a channel count alone cannot identify (5.1 with /// side vs back surrounds, 7.0 vs 6.1, Ambisonics as [`ChannelLayout::unordered`]). `None` is - /// the usual layout for the channel count. WAV stores it as `dwChannelMask`; AIFF and FLAC - /// have fixed channel orders and ignore it. It must have as many channels as the audio. + /// the usual layout for the channel count. WAV stores it as `dwChannelMask`, FLAC as a + /// `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment when it is not FLAC's fixed assignment for the + /// count (which for 7 channels is 6.1 with side surrounds, not the usual layout); AIFF has a + /// fixed channel order and ignores it. It must have as many channels as the audio. pub layout: Option, /// Write WAV in the RF64 layout even below 4 GiB (WAV only; ignored for other formats). pub force_rf64: bool, diff --git a/crates/audio/src/write/flac/encoder.rs b/crates/audio/src/write/flac/encoder.rs index 508bbf59..6ad7545d 100644 --- a/crates/audio/src/write/flac/encoder.rs +++ b/crates/audio/src/write/flac/encoder.rs @@ -88,19 +88,23 @@ impl Encoder { } /// `fLaC`, the STREAMINFO block (sizes and MD5 unknown yet: see [`Encoder::streaminfo`]) and a - /// VORBIS_COMMENT block with the vendor string. - pub fn header(&self) -> Vec { + /// VORBIS_COMMENT block with the vendor string and `comments` (`NAME=value`, each short). + pub fn header(&self, comments: &[&str]) -> Vec { let mut v = b"fLaC".to_vec(); v.extend_from_slice(&[0, 0, 0, 34]); v.extend_from_slice(&self.streaminfo_with(0, 0, 0, [0; 16])); let vendor = concat!("craft-audio ", env!("CARGO_PKG_VERSION")); - let len = 4 + vendor.len() + 4; + let len = 4 + vendor.len() + 4 + comments.iter().map(|c| 4 + c.len()).sum::(); // last metadata block, type 4 (VORBIS_COMMENT), little-endian lengths inside v.push(0x84); v.extend_from_slice(&(len as u32).to_be_bytes()[1..]); v.extend_from_slice(&(vendor.len() as u32).to_le_bytes()); v.extend_from_slice(vendor.as_bytes()); - v.extend_from_slice(&0u32.to_le_bytes()); + v.extend_from_slice(&(comments.len() as u32).to_le_bytes()); + for c in comments { + v.extend_from_slice(&(c.len() as u32).to_le_bytes()); + v.extend_from_slice(c.as_bytes()); + } v } diff --git a/crates/audio/src/write/flac/mod.rs b/crates/audio/src/write/flac/mod.rs index 5f25c25c..f8accca4 100644 --- a/crates/audio/src/write/flac/mod.rs +++ b/crates/audio/src/write/flac/mod.rs @@ -13,7 +13,7 @@ use craft_media_testkit as common; mod oracle_tests; use super::pcm::{Quantizer, validate_buffer, validate_layout}; -use crate::{AudioBuffer, AudioError, BitDepth, Result}; +use crate::{AudioBuffer, AudioError, BitDepth, ChannelLayout, Result}; use encoder::{Encoder, EncoderConfig, STREAMINFO_OFFSET}; use std::io::{Cursor, Seek, SeekFrom, Write}; @@ -39,6 +39,20 @@ fn check(channels: usize, sample_rate: u32, depth: BitDepth) -> Result { /// Frames quantized and handed to the encoder at a time: its block size. const CHUNK: usize = 4096; +/// FLAC's channel assignment for 1–8 channels (RFC 9639 §9.1.3) as a WAV mask: the usual layout +/// for the count, except that 7 channels are 6.1 with side surrounds (FL FR FC LFE BC SL SR). +fn flac_mask(channels: usize) -> u32 { + let layout = if channels == 7 { ChannelLayout::SURROUND_6_1 } else { ChannelLayout::default_for(channels as u16) }; + layout.mask().unwrap_or(0) +} + +/// The `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment (RFC 9639 §8.6.2) for a layout FLAC's channel +/// assignment does not already say: `mask` (the usual layout for the count when `None`). +fn mask_comment(channels: usize, mask: Option) -> Option { + let mask = mask.unwrap_or_else(|| ChannelLayout::default_for(channels as u16).mask().unwrap_or(0)); + (mask != flac_mask(channels)).then(|| format!("WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x{mask:04X}")) +} + fn encoder_error(e: encoder::Error) -> AudioError { AudioError::Encode(format!("FLAC: {e}")) } @@ -58,12 +72,16 @@ pub(crate) struct Writer { } impl Writer { - pub(crate) fn new(mut out: W, channels: usize, sample_rate: u32, depth: BitDepth, dither: bool) -> Result { + /// `mask` is the speaker layout as a WAV channel mask (the usual layout for the count when + /// `None`), recorded as a comment when FLAC's channel assignment says otherwise. + pub(crate) fn new(mut out: W, channels: usize, sample_rate: u32, depth: BitDepth, dither: bool, mask: Option) -> Result { let bps = check(channels, sample_rate, depth)?; // `check` bounds channels to 1..=8. let encoder = Encoder::new(EncoderConfig::new(sample_rate, channels as u8, bps)).map_err(encoder_error)?; let start = out.stream_position()?; - out.write_all(&encoder.header())?; + let comment = mask_comment(channels, mask); + let comments: Vec<&str> = comment.iter().map(String::as_str).collect(); + out.write_all(&encoder.header(&comments))?; Ok(Writer { out, start, @@ -125,14 +143,14 @@ impl Writer { } } -pub(crate) fn encode(buf: &AudioBuffer, depth: BitDepth, dither: bool) -> Result> { +pub(crate) fn encode(buf: &AudioBuffer, depth: BitDepth, dither: bool, mask: Option) -> Result> { validate_buffer(buf)?; let bps = check(buf.channels.len(), buf.sample_rate, depth)?; let frames = buf.frames(); let mut out = Vec::new(); out.try_reserve(frames.saturating_mul(buf.channels.len()).saturating_mul(usize::from(bps) / 8) / 2) .map_err(|_| AudioError::TooLarge("cannot allocate FLAC output".into()))?; - let mut w = Writer::new(Cursor::new(out), buf.channels.len(), buf.sample_rate, depth, dither)?; + let mut w = Writer::new(Cursor::new(out), buf.channels.len(), buf.sample_rate, depth, dither, mask)?; w.write(&buf.channels, frames)?; Ok(w.finish()?.into_inner()) } diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index 8fa63edd..772ea9ae 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -53,7 +53,7 @@ fn signal(kind: &str, n: usize, bps: u8, seed: u64) -> Vec { fn encode(planar: &[Vec], cfg: EncoderConfig, chunk: usize) -> Result, Error> { let mut enc = Encoder::new(cfg)?; - let mut file = enc.header(); + let mut file = enc.header(&[]); let n = planar.first().map_or(0, Vec::len); let mut at = 0; while at < n { @@ -188,7 +188,7 @@ fn bad_input_is_an_error_not_a_panic() { assert_eq!(enc.encode(&[&[-32_769], &[0]]), Err(Error::SampleRange(-32_769))); // nothing encoded: no frames, and a header that still says "unknown" assert!(enc.finish().is_empty()); - assert_eq!(&enc.header()[..4], b"fLaC"); + assert_eq!(&enc.header(&[])[..4], b"fLaC"); } /// 32-bit stereo at the extremes of i32: left minus right would need 33 bits, so the encoder must @@ -208,3 +208,28 @@ fn full_scale_32_bit_stereo_round_trips() { let mut enc = Encoder::new(EncoderConfig::new(48_000, 2, 32)).unwrap(); assert!(enc.encode(&[&[i32::MIN, i32::MAX], &[0, -1]]).is_ok()); } + +/// A layout FLAC's channel assignment does not name is recorded as a channel mask comment, which +/// ffmpeg reads: the usual 7-channel layout (backs and a back centre) differs from FLAC's 6.1. +#[test] +fn layouts_other_than_flacs_are_recorded() { + use crate::{AudioBuffer, ChannelLayout, EncodeFormat, EncodeOptions}; + let Some(ffprobe) = common::ffprobe() else { return }; + let cases = [ + ("layout-stereo", 2, None, "stereo"), + ("layout-7-default", 7, None, "6.1(back)"), + ("layout-6.1-side", 7, Some(ChannelLayout::SURROUND_6_1), "6.1"), + ("layout-5.1-side", 6, Some(ChannelLayout::SURROUND_5_1_SIDE), "5.1(side)"), + ]; + for (name, channels, layout, expected) in cases { + let buf = AudioBuffer::new(48_000, channels, 100); + let mut opts = EncodeOptions::new(EncodeFormat::Flac); + opts.layout = layout; + let file = crate::encode(&buf, &opts).unwrap(); + let path = common::fixture_subdir("flac").join(format!("{name}.flac")); + std::fs::write(&path, &file).unwrap(); + let out = + Command::new(&ffprobe).args(["-v", "error", "-show_entries", "stream=channel_layout", "-of", "csv=p=0"]).arg(&path).output().unwrap(); + assert_eq!(String::from_utf8_lossy(&out.stdout).trim(), expected, "{name}"); + } +} diff --git a/crates/audio/src/write/mod.rs b/crates/audio/src/write/mod.rs index 13e42ea7..852d9283 100644 --- a/crates/audio/src/write/mod.rs +++ b/crates/audio/src/write/mod.rs @@ -27,7 +27,6 @@ use std::io::{Seek, Write}; /// ``` pub fn encode(buf: &AudioBuffer, opts: &EncodeOptions) -> Result> { // Checked for every format; only a WAV records it. - #[cfg_attr(not(feature = "pcm-encode"), allow(unused_variables))] let mask = opts.layout.map(|l| wav_mask(&l, buf.channels.len())).transpose()?; match opts.format { #[cfg(feature = "pcm-encode")] @@ -35,7 +34,7 @@ pub fn encode(buf: &AudioBuffer, opts: &EncodeOptions) -> Result> { #[cfg(feature = "pcm-encode")] EncodeFormat::Aiff => aiff::encode(buf, opts.bit_depth, opts.dither), #[cfg(feature = "flac-encode")] - EncodeFormat::Flac => flac::encode(buf, opts.bit_depth, opts.dither), + EncodeFormat::Flac => flac::encode(buf, opts.bit_depth, opts.dither, mask), #[allow(unreachable_patterns)] f => Err(not_built(f)), } @@ -152,7 +151,6 @@ impl FileEncoder { /// `opts.layout`, when set, must have `channels` channels. pub fn new(out: W, opts: &EncodeOptions, sample_rate: u32, channels: u16) -> Result { let (depth, dither, channels) = (opts.bit_depth, opts.dither, usize::from(channels)); - #[cfg_attr(not(feature = "pcm-encode"), allow(unused_variables))] let mask = opts.layout.map(|l| wav_mask(&l, channels)).transpose()?; let writer = match opts.format { #[cfg(feature = "pcm-encode")] @@ -167,7 +165,7 @@ impl FileEncoder { #[cfg(feature = "pcm-encode")] EncodeFormat::Aiff => Writer::Aiff(aiff::Writer::new(out, channels, sample_rate, depth, dither)?), #[cfg(feature = "flac-encode")] - EncodeFormat::Flac => Writer::Flac(flac::Writer::new(out, channels, sample_rate, depth, dither)?), + EncodeFormat::Flac => Writer::Flac(flac::Writer::new(out, channels, sample_rate, depth, dither, mask)?), #[allow(unreachable_patterns)] f => return Err(not_built(f)), }; From 8ba7778f8cd3dc44968354daa17683ce5c607c52 Mon Sep 17 00:00:00 2001 From: alehel Date: Tue, 3 Nov 2026 21:32:23 +0100 Subject: [PATCH 05/20] craft-audio: review fixes, round 4 (#442) The PICTURE metadata test inserts its block without the last-block flag: the encoder's VORBIS_COMMENT follows it, and the flagged version was a broken metadata chain that only decoded by resyncing. Co-Authored-By: Claude Opus 5.5 --- crates/audio/tests/audio/malformed.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/audio/tests/audio/malformed.rs b/crates/audio/tests/audio/malformed.rs index c5deb532..0e712055 100644 --- a/crates/audio/tests/audio/malformed.rs +++ b/crates/audio/tests/audio/malformed.rs @@ -313,8 +313,9 @@ fn packets_longer_than_any_codec_uses_are_refused() { fn flac_metadata_fields_beyond_their_block_are_refused() { let with_block = |kind: u8, body: &[u8]| { let f = small(EncodeFormat::Flac, BitDepth::Int16, 1); - let head = ((0x80 | u32::from(kind)) << 24 | body.len() as u32).to_be_bytes(); - [&f[..4], &[f[4] & 0x7f], &f[5..42], &head, body, &f[42..]].concat() + // between STREAMINFO and the encoder's VORBIS_COMMENT, which stays the last block + let head = (u32::from(kind) << 24 | body.len() as u32).to_be_bytes(); + [&f[..42], &head, body, &f[42..]].concat() }; let picture = |data: u32| [&[0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 0][..], &[0; 16], &data.to_be_bytes(), &[1, 2, 3]].concat(); assert_eq!(decode(&with_block(6, &picture(3)), None).unwrap().1.frames(), 300); From 82424e59c3dfa31cbabf334591778293dbb0ac3e Mon Sep 17 00:00:00 2001 From: alehel Date: Tue, 3 Nov 2026 21:44:20 +0100 Subject: [PATCH 06/20] craft-audio: review fixes, round 5 (#442) - libFLAC and ffmpeg read a FLAC with 5 or 6 channels and no mask comment as having side surrounds, so the comment is now written for anything else, including the crate's usual back-surround layouts. The round 3 check treated back surrounds as FLAC's assignment, which lost them. - Tests: the mask comment rules, opposite-polarity stereo, STREAMINFO block and frame sizes. - A stale comment said only WAV records the layout. Co-Authored-By: Claude Opus 5.5 --- crates/audio/README.md | 2 +- crates/audio/src/lib.rs | 7 +-- crates/audio/src/write/flac/mod.rs | 23 +++++---- crates/audio/src/write/flac/oracle_tests.rs | 52 +++++++++++++++++++-- crates/audio/src/write/mod.rs | 2 +- 5 files changed, 69 insertions(+), 17 deletions(-) diff --git a/crates/audio/README.md b/crates/audio/README.md index 2000fe96..5cf6b9f9 100644 --- a/crates/audio/README.md +++ b/crates/audio/README.md @@ -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, layouts outside FLAC's channel assignment as a `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment; up to 655,350 Hz; LPC, stereo decorrelation, STREAMINFO MD5 | bit-exact through ffmpeg, STREAMINFO MD5 checked against ffmpeg | +| FLAC encoder | RFC 9639 | 16, 24, 32 bits; 1–8 channels, layouts decoders would not assume for the count as a `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment; up to 655,350 Hz; LPC, stereo decorrelation, STREAMINFO MD5 | bit-exact through ffmpeg, STREAMINFO MD5 checked against ffmpeg | ## symphonia diff --git a/crates/audio/src/lib.rs b/crates/audio/src/lib.rs index 70bf9a3f..f582c74f 100644 --- a/crates/audio/src/lib.rs +++ b/crates/audio/src/lib.rs @@ -269,9 +269,10 @@ pub struct EncodeOptions { /// The speaker layout to record, for layouts a channel count alone cannot identify (5.1 with /// side vs back surrounds, 7.0 vs 6.1, Ambisonics as [`ChannelLayout::unordered`]). `None` is /// the usual layout for the channel count. WAV stores it as `dwChannelMask`, FLAC as a - /// `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment when it is not FLAC's fixed assignment for the - /// count (which for 7 channels is 6.1 with side surrounds, not the usual layout); AIFF has a - /// fixed channel order and ignores it. It must have as many channels as the audio. + /// `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment when decoders would assume another layout for + /// the count (they read 5 and 6 channels with side surrounds and 7 as 6.1 with sides, so the + /// usual layouts for those counts get one); AIFF has a fixed channel order and ignores it. It + /// must have as many channels as the audio. pub layout: Option, /// Write WAV in the RF64 layout even below 4 GiB (WAV only; ignored for other formats). pub force_rf64: bool, diff --git a/crates/audio/src/write/flac/mod.rs b/crates/audio/src/write/flac/mod.rs index f8accca4..5ada9ad0 100644 --- a/crates/audio/src/write/flac/mod.rs +++ b/crates/audio/src/write/flac/mod.rs @@ -39,18 +39,23 @@ fn check(channels: usize, sample_rate: u32, depth: BitDepth) -> Result { /// Frames quantized and handed to the encoder at a time: its block size. const CHUNK: usize = 4096; -/// FLAC's channel assignment for 1–8 channels (RFC 9639 §9.1.3) as a WAV mask: the usual layout -/// for the count, except that 7 channels are 6.1 with side surrounds (FL FR FC LFE BC SL SR). -fn flac_mask(channels: usize) -> u32 { - let layout = if channels == 7 { ChannelLayout::SURROUND_6_1 } else { ChannelLayout::default_for(channels as u16) }; - layout.mask().unwrap_or(0) +/// The layout decoders assume for 1–8 channels with no channel mask comment, as a WAV mask. RFC +/// 9639 §9.1.3 leaves "back/surround" open for 5 and 6 channels; libFLAC and ffmpeg read side +/// surrounds. 7 channels are 6.1 with sides (FL FR FC LFE BC SL SR). +fn assumed_mask(channels: usize) -> u32 { + match channels { + 5 => 0x607, + 6 => 0x60F, + 7 => 0x70F, + n => ChannelLayout::default_for(n as u16).mask().unwrap_or(0), + } } -/// The `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment (RFC 9639 §8.6.2) for a layout FLAC's channel -/// assignment does not already say: `mask` (the usual layout for the count when `None`). +/// The `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment (RFC 9639 §8.6.2) for a layout decoders would +/// not assume: `mask` (the usual layout for the count when `None`). fn mask_comment(channels: usize, mask: Option) -> Option { let mask = mask.unwrap_or_else(|| ChannelLayout::default_for(channels as u16).mask().unwrap_or(0)); - (mask != flac_mask(channels)).then(|| format!("WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x{mask:04X}")) + (mask != assumed_mask(channels)).then(|| format!("WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x{mask:04X}")) } fn encoder_error(e: encoder::Error) -> AudioError { @@ -73,7 +78,7 @@ pub(crate) struct Writer { impl Writer { /// `mask` is the speaker layout as a WAV channel mask (the usual layout for the count when - /// `None`), recorded as a comment when FLAC's channel assignment says otherwise. + /// `None`), recorded as a comment when decoders would assume another one. pub(crate) fn new(mut out: W, channels: usize, sample_rate: u32, depth: BitDepth, dither: bool, mask: Option) -> Result { let bps = check(channels, sample_rate, depth)?; // `check` bounds channels to 1..=8. diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index 772ea9ae..997f75cc 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -209,10 +209,10 @@ fn full_scale_32_bit_stereo_round_trips() { assert!(enc.encode(&[&[i32::MIN, i32::MAX], &[0, -1]]).is_ok()); } -/// A layout FLAC's channel assignment does not name is recorded as a channel mask comment, which -/// ffmpeg reads: the usual 7-channel layout (backs and a back centre) differs from FLAC's 6.1. +/// A layout decoders would not assume for the channel count is recorded as a channel mask comment, +/// which ffmpeg reads: they read 6 channels with side surrounds, and 7 as 6.1 with sides. #[test] -fn layouts_other_than_flacs_are_recorded() { +fn layouts_decoders_would_not_assume_are_recorded() { use crate::{AudioBuffer, ChannelLayout, EncodeFormat, EncodeOptions}; let Some(ffprobe) = common::ffprobe() else { return }; let cases = [ @@ -220,6 +220,8 @@ fn layouts_other_than_flacs_are_recorded() { ("layout-7-default", 7, None, "6.1(back)"), ("layout-6.1-side", 7, Some(ChannelLayout::SURROUND_6_1), "6.1"), ("layout-5.1-side", 6, Some(ChannelLayout::SURROUND_5_1_SIDE), "5.1(side)"), + ("layout-5.1-back", 6, Some(ChannelLayout::SURROUND_5_1), "5.1"), + ("layout-6-default", 6, None, "5.1"), ]; for (name, channels, layout, expected) in cases { let buf = AudioBuffer::new(48_000, channels, 100); @@ -233,3 +235,47 @@ fn layouts_other_than_flacs_are_recorded() { assert_eq!(String::from_utf8_lossy(&out.stdout).trim(), expected, "{name}"); } } + +#[test] +fn the_mask_comment_is_written_only_when_decoders_would_assume_otherwise() { + use super::mask_comment; + let comment = |m: &str| Some(format!("WAVEFORMATEXTENSIBLE_CHANNEL_MASK={m}")); + for channels in [1, 2, 3, 4, 8] { + assert_eq!(mask_comment(channels, None), None, "{channels}"); + } + assert_eq!(mask_comment(5, None), comment("0x0037")); + assert_eq!(mask_comment(5, Some(0x607)), None); + assert_eq!(mask_comment(6, None), comment("0x003F")); + assert_eq!(mask_comment(6, Some(0x60F)), None); + assert_eq!(mask_comment(7, None), comment("0x013F")); + assert_eq!(mask_comment(7, Some(0x70F)), None); + // no speaker assignment + assert_eq!(mask_comment(2, Some(0)), comment("0x0000")); +} + +/// Left and right at opposite full scale: the side channel needs its extra bit at every depth. +#[test] +fn opposite_polarity_stereo_round_trips() { + for bps in [16u8, 24] { + let left = signal("square", 9_000, bps, 1); + let right: Vec = left.iter().map(|&v| !v).collect(); + let planar = vec![left, right]; + if round_trip(&format!("opposite-{bps}"), &planar, EncoderConfig::new(48_000, 2, bps), 4096).is_none() { + return; + } + } +} + +#[test] +fn streaminfo_states_the_block_and_frame_sizes() { + let planar = [signal("music", 10_000, 16, 21), signal("noise", 10_000, 16, 22)]; + let mut enc = Encoder::new(EncoderConfig::new(48_000, 2, 16)).unwrap(); + let parts: Vec<&[i32]> = planar.iter().map(Vec::as_slice).collect(); + let mut sizes: Vec = enc.encode_frames(&parts).unwrap().iter().map(Vec::len).collect(); + sizes.push(enc.finish().len()); + let si = enc.streaminfo(); + let u24 = |b: &[u8]| usize::from(b[0]) << 16 | usize::from(b[1]) << 8 | usize::from(b[2]); + assert_eq!(&si[..4], &[0x10, 0x00, 0x10, 0x00]); + assert_eq!(u24(&si[4..7]), sizes.iter().copied().min().unwrap()); + assert_eq!(u24(&si[7..10]), sizes.iter().copied().max().unwrap()); +} diff --git a/crates/audio/src/write/mod.rs b/crates/audio/src/write/mod.rs index 852d9283..9492d5e1 100644 --- a/crates/audio/src/write/mod.rs +++ b/crates/audio/src/write/mod.rs @@ -26,7 +26,7 @@ use std::io::{Seek, Write}; /// # } /// ``` pub fn encode(buf: &AudioBuffer, opts: &EncodeOptions) -> Result> { - // Checked for every format; only a WAV records it. + // Checked for every format; WAV and FLAC record it. let mask = opts.layout.map(|l| wav_mask(&l, buf.channels.len())).transpose()?; match opts.format { #[cfg(feature = "pcm-encode")] From 430ad15683bb5bbd7c906d78e9b6e7c532871242 Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 04:57:39 +0100 Subject: [PATCH 07/20] craft-audio: review fixes, round 6 (#442) Tests only, plus docs; the encoder does not change. Each new check was confirmed to fail on a mutated encoder. - Tests: frame numbers count up, LPC and Rice partitions and stereo decorrelation save the space they should, FileEncoder records a FLAC layout, the smallest frame anywhere reaches STREAMINFO, Writer::write applies the frame limit, finish leaves the sink at the end, the exact 655350 Hz cap, the sample size codes. - README limits: a FLAC layout decoders would not assume depends on its mask comment, and ffmpeg and flac ignore a zero mask. - The encode example's comment named no referent. Co-Authored-By: Claude Opus 5.5 --- crates/audio/README.md | 6 +++ crates/audio/examples/encode.rs | 2 +- crates/audio/src/lib.rs | 4 +- crates/audio/src/write/flac/frame.rs | 5 +++ crates/audio/src/write/flac/oracle_tests.rs | 42 +++++++++++++++++++-- crates/audio/src/write/flac/predict.rs | 32 ++++++++++++++++ crates/audio/tests/audio/roundtrip.rs | 3 +- crates/audio/tests/audio/stream.rs | 6 ++- 8 files changed, 92 insertions(+), 8 deletions(-) diff --git a/crates/audio/README.md b/crates/audio/README.md index 5cf6b9f9..5dd33f5f 100644 --- a/crates/audio/README.md +++ b/crates/audio/README.md @@ -260,6 +260,12 @@ match craft_audio::decode(&bytes, None) { ## Limits - 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. +- `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. - `TrackDecoder` returns `Unsupported` for every codec but PCM; `decode` and `Decoder` read the other codecs in the table above through symphonia. diff --git a/crates/audio/examples/encode.rs b/crates/audio/examples/encode.rs index 6d49d55d..9ed70e7a 100644 --- a/crates/audio/examples/encode.rs +++ b/crates/audio/examples/encode.rs @@ -20,7 +20,7 @@ fn main() -> Result<(), Box> { let wav = encode(&surround, &EncodeOptions::default().with_layout(ChannelLayout::SURROUND_5_1_SIDE))?; std::fs::write(dir.join("tone-5.1.wav"), wav)?; - // Streamed: the same bytes, block by block into a file. + // Streamed: the bytes `encode` would give, block by block into a file. let opts = EncodeOptions::new(EncodeFormat::Flac); let mut enc = FileEncoder::new(std::fs::File::create(dir.join("tone.flac"))?, &opts, rate as u32, 2)?; for chunk in tone.chunks(4096) { diff --git a/crates/audio/src/lib.rs b/crates/audio/src/lib.rs index f582c74f..b85a7611 100644 --- a/crates/audio/src/lib.rs +++ b/crates/audio/src/lib.rs @@ -271,8 +271,8 @@ pub struct EncodeOptions { /// the usual layout for the channel count. WAV stores it as `dwChannelMask`, FLAC as a /// `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment when decoders would assume another layout for /// the count (they read 5 and 6 channels with side surrounds and 7 as 6.1 with sides, so the - /// usual layouts for those counts get one); AIFF has a fixed channel order and ignores it. It - /// must have as many channels as the audio. + /// usual layouts for those counts get one; see the README's limits); AIFF has a fixed channel + /// order and ignores it. It must have as many channels as the audio. pub layout: Option, /// Write WAV in the RF64 layout even below 4 GiB (WAV only; ignored for other formats). pub force_rf64: bool, diff --git a/crates/audio/src/write/flac/frame.rs b/crates/audio/src/write/flac/frame.rs index aa4bafaf..cc644ddf 100644 --- a/crates/audio/src/write/flac/frame.rs +++ b/crates/audio/src/write/flac/frame.rs @@ -210,6 +210,11 @@ mod tests { assert_eq!(code(0xF_FFFF_FFFF), [0xFE, 0xBF, 0xBF, 0xBF, 0xBF, 0xBF, 0xBF]); } + #[test] + fn sample_sizes_are_named_in_the_header_except_32_bits() { + assert_eq!([8, 12, 16, 20, 24, 32].map(sample_size_code), [1, 2, 4, 5, 6, 0]); + } + #[test] fn sample_rates_outside_the_table_are_named_in_the_header() { assert_eq!(sample_rate_code(48_000), (10, None)); diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index 997f75cc..a258579f 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -164,8 +164,8 @@ fn levels_and_block_sizes_trade_time_for_size() { let Some(size) = round_trip(&format!("level{level}"), &planar, cfg, 48_000) else { return }; sizes.push(size); } - // the effortful levels are never larger than the fastest, and all beat PCM - assert!(sizes[8] <= sizes[0] && sizes[5] <= sizes[0], "{sizes:?}"); + // linear prediction beats the fastest level by a wide margin, and all beat PCM + assert!(sizes[8] <= sizes[5] && sizes[5] < sizes[0] * 4 / 5, "{sizes:?}"); assert!(sizes.iter().all(|&s| s < 48_000 * 4), "{sizes:?}"); for bs in [16u16, 192, 1152, 4608, 65_535] { let cfg = EncoderConfig { block_size: bs, ..EncoderConfig::new(48_000, 2, 16) }; @@ -268,7 +268,9 @@ fn opposite_polarity_stereo_round_trips() { #[test] fn streaminfo_states_the_block_and_frame_sizes() { - let planar = [signal("music", 10_000, 16, 21), signal("noise", 10_000, 16, 22)]; + // silence in the middle: the smallest frame is neither the first nor the last + let channel = |seed| [signal("noise", 4096, 16, seed), signal("silence", 4096, 16, seed), signal("noise", 1000, 16, seed)].concat(); + let planar = [channel(21), channel(22)]; let mut enc = Encoder::new(EncoderConfig::new(48_000, 2, 16)).unwrap(); let parts: Vec<&[i32]> = planar.iter().map(Vec::as_slice).collect(); let mut sizes: Vec = enc.encode_frames(&parts).unwrap().iter().map(Vec::len).collect(); @@ -279,3 +281,37 @@ fn streaminfo_states_the_block_and_frame_sizes() { assert_eq!(u24(&si[4..7]), sizes.iter().copied().min().unwrap()); assert_eq!(u24(&si[7..10]), sizes.iter().copied().max().unwrap()); } + +/// Frame numbers count up from 0; a decoder that seeks depends on them, though one that reads from +/// the start does not. +#[test] +fn frames_are_numbered_in_order() { + let planar = [signal("music", 4096 * 10, 16, 31)]; + let mut enc = Encoder::new(EncoderConfig::new(48_000, 1, 16)).unwrap(); + let frames = enc.encode_frames(&[&planar[0]]).unwrap(); + let numbers: Vec = frames.iter().map(|f| f[4]).collect(); + assert_eq!(numbers, (0..10).collect::>()); +} + +/// Two equal channels cost little more than one: the side channel is all zeros. +#[test] +fn stereo_decorrelation_removes_a_duplicated_channel() { + let x = signal("music", 20_000, 16, 41); + let mono = encode(std::slice::from_ref(&x), EncoderConfig::new(48_000, 1, 16), 4096).unwrap(); + let stereo = encode(&[x.clone(), x], EncoderConfig::new(48_000, 2, 16), 4096).unwrap(); + assert!(stereo.len() < mono.len() * 11 / 10, "stereo {} mono {}", stereo.len(), mono.len()); +} + +#[test] +fn the_frame_limit_stops_a_write_before_it_reaches_the_sink() { + use super::Writer; + use crate::{AudioError, BitDepth}; + let mut w = Writer::new(std::io::Cursor::new(Vec::new()), 1, 48_000, BitDepth::Int16, false, None).unwrap(); + let header = w.out.get_ref().len(); + w.frames = (1 << 36) - 2; + assert!(matches!(w.write(&[[0.0f32; 4096]], 4096), Err(AudioError::TooLarge(_)))); + assert_eq!(w.out.get_ref().len(), header); + // refused, not failed: what fits can still be written + assert!(w.write(&[[0.0f32; 1]], 1).is_ok()); + assert!(matches!(w.write(&[[0.0f32; 1]], 1), Err(AudioError::TooLarge(_)))); +} diff --git a/crates/audio/src/write/flac/predict.rs b/crates/audio/src/write/flac/predict.rs index 81646cce..bd4951b2 100644 --- a/crates/audio/src/write/flac/predict.rs +++ b/crates/audio/src/write/flac/predict.rs @@ -292,6 +292,38 @@ mod tests { assert_eq!(choose(&[7; 100], 16, Effort::level(5)), (Subframe::Constant(7), 24)); } + /// Two tones and a little noise, from a fixed seed. + fn tones(n: usize) -> Vec { + let mut seed = 1u32; + (0..n) + .map(|i| { + seed = seed.wrapping_mul(1_664_525).wrapping_add(1_013_904_223); + let t = i as f64; + let noise = f64::from(seed >> 24) - 128.0; + ((t * 0.31).sin() * 9000.0 + (t * 0.77).sin() * 6000.0 + noise * 0.2) as i32 + }) + .collect() + } + + #[test] + fn linear_prediction_beats_the_fixed_predictors() { + let x = tones(4096); + let effort = Effort::level(5); + let (_, lpc) = choose(&x, 16, effort); + let (_, fixed) = choose(&x, 16, Effort { max_lpc_order: 0, ..effort }); + assert!(lpc < fixed * 4 / 5, "lpc {lpc} fixed {fixed}"); + } + + #[test] + fn rice_partitions_follow_a_change_in_loudness() { + // quiet, then loud: one Rice parameter cannot suit both + let x: Vec = tones(4096).iter().enumerate().map(|(i, &s)| if i < 2048 { s / 256 } else { s }).collect(); + let effort = Effort::level(5); + let (_, partitioned) = choose(&x, 16, effort); + let (_, whole) = choose(&x, 16, Effort { max_partition_order: 0, ..effort }); + assert!(partitioned < whole * 19 / 20, "partitioned {partitioned} whole {whole}"); + } + #[test] fn lpc_residual_inverts() { let x: Vec = (0..500).map(|i| ((i as f64 * 0.1).sin() * 1000.0) as i32).collect(); diff --git a/crates/audio/tests/audio/roundtrip.rs b/crates/audio/tests/audio/roundtrip.rs index 907a830d..764a3220 100644 --- a/crates/audio/tests/audio/roundtrip.rs +++ b/crates/audio/tests/audio/roundtrip.rs @@ -123,7 +123,8 @@ fn flac_rejects_unsupported_layouts() { assert!(encode(&sine(48_000, 2, 10), &opts).is_err()); let opts = opts.clone().with_bit_depth(BitDepth::Int16); assert!(encode(&sine(48_000, 9, 10), &opts).is_err()); - assert!(encode(&sine(700_000, 1, 10), &opts).is_err()); + assert!(encode(&sine(655_351, 1, 10), &opts).is_err()); + assert!(encode(&sine(655_350, 1, 10), &opts).is_ok()); // Tiny buffers still encode and decode. let one = sine(48_000, 2, 1); let (_, out) = decode(&encode(&one, &opts).unwrap(), None).unwrap(); diff --git a/crates/audio/tests/audio/stream.rs b/crates/audio/tests/audio/stream.rs index bfab2fbd..fcd27def 100644 --- a/crates/audio/tests/audio/stream.rs +++ b/crates/audio/tests/audio/stream.rs @@ -21,7 +21,10 @@ fn streamed(buf: &AudioBuffer, opts: &EncodeOptions) -> Vec { enc.write(&block, len).unwrap(); start += len; } - enc.finish().unwrap().into_inner() + let out = enc.finish().unwrap(); + // the sink is left at the end of the file + assert_eq!(out.position(), out.get_ref().len() as u64); + out.into_inner() } #[test] @@ -35,6 +38,7 @@ fn streamed_files_match_whole_buffer_encodes() { (EncodeFormat::Aiff, BitDepth::Float32, false, None, None), (EncodeFormat::Flac, BitDepth::Int16, true, None, None), (EncodeFormat::Flac, BitDepth::Int24, false, None, None), + (EncodeFormat::Flac, BitDepth::Int16, false, None, Some(ChannelLayout::SURROUND_5_1_SIDE)), ]; for (format, bit_depth, dither, bwf, layout) in cases { let base = EncodeOptions::new(format).with_bit_depth(bit_depth).with_dither(dither); From c008e8ee07baa71ddc596160a3923cf039d96106 Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 05:24:55 +0100 Subject: [PATCH 08/20] 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 --- crates/audio/README.md | 9 ++++++--- crates/audio/src/write/flac/encoder.rs | 17 +++++++++++++++++ crates/audio/src/write/flac/frame.rs | 3 ++- crates/audio/src/write/flac/oracle_tests.rs | 2 +- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/crates/audio/README.md b/crates/audio/README.md index 5dd33f5f..dbca8c25 100644 --- a/crates/audio/README.md +++ b/crates/audio/README.md @@ -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. diff --git a/crates/audio/src/write/flac/encoder.rs b/crates/audio/src/write/flac/encoder.rs index 6ad7545d..c401c746 100644 --- a/crates/audio/src/write/flac/encoder.rs +++ b/crates/audio/src/write/flac/encoder.rs @@ -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); + } +} diff --git a/crates/audio/src/write/flac/frame.rs b/crates/audio/src/write/flac/frame.rs index cc644ddf..c8903836 100644 --- a/crates/audio/src/write/flac/frame.rs +++ b/crates/audio/src/write/flac/frame.rs @@ -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, } } diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index a258579f..f7ba676c 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -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. From 82f63f3a4f552ca39f84a5d4561114d27abac370 Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 05:37:11 +0100 Subject: [PATCH 09/20] craft-audio: review fixes, round 8 (#442) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Code wasted bits (RFC 9639 §9.2.2): low bits that are zero in every sample of a subframe were stored anyway, so 16-bit audio exported at 24 or 32 bits grew by the padding. - Allow residuals up to the 32-bit limit (§9.2.7.3). The limit was half that, so at 32 bits a transient forced its whole block to verbatim. - README: 32-bit FLAC codes stereo channels independently. Co-Authored-By: Claude Opus 5.5 --- crates/audio/README.md | 3 +- crates/audio/src/write/flac/frame.rs | 33 +++++++++---- crates/audio/src/write/flac/oracle_tests.rs | 27 +++++++++++ crates/audio/src/write/flac/predict.rs | 52 +++++++++++++++++++-- crates/audio/tests/audio/roundtrip.rs | 19 ++++++++ 5 files changed, 120 insertions(+), 14 deletions(-) diff --git a/crates/audio/README.md b/crates/audio/README.md index dbca8c25..c1f7dcef 100644 --- a/crates/audio/README.md +++ b/crates/audio/README.md @@ -265,6 +265,7 @@ match craft_audio::decode(&bytes, None) { 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. +- FLAC at 32 bits codes stereo channels independently, because a side channel would need 33 bits. - 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 @@ -280,7 +281,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, layouts decoders would not assume for the count as a `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment; up to 655,350 Hz; LPC, stereo decorrelation, STREAMINFO MD5 | bit-exact through ffmpeg, STREAMINFO MD5 checked against ffmpeg | +| FLAC encoder | RFC 9639 | 16, 24, 32 bits; 1–8 channels, layouts decoders would not assume for the count as a `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment; up to 655,350 Hz; LPC, stereo decorrelation, wasted bits, STREAMINFO MD5 | bit-exact through ffmpeg, STREAMINFO MD5 checked against ffmpeg | ## symphonia diff --git a/crates/audio/src/write/flac/frame.rs b/crates/audio/src/write/flac/frame.rs index c8903836..72528c5b 100644 --- a/crates/audio/src/write/flac/frame.rs +++ b/crates/audio/src/write/flac/frame.rs @@ -120,23 +120,37 @@ fn write_residual(w: &mut BitWriter, residual: &Residual, r: &[i64], block: usiz } fn write_subframe(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8) { + if let Subframe::Wasted { bits, coded } = sub { + let shifted: Vec = x.iter().map(|&v| v >> bits).collect(); + return write_coded(w, &shifted, coded, bps - bits, *bits); + } + write_coded(w, x, sub, bps, 0); +} + +/// Subframe header (§9.2.1) and body of samples already without their `wasted` low bits. +fn write_coded(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8, wasted: u8) { let b = u32::from(bps); let sample = |w: &mut BitWriter, v: i32| w.write_bits(v as u32, b); - w.write_bit(false); + // the flag, then the count minus 1 in unary + let header = |w: &mut BitWriter, kind: u32| { + w.write_bit(false); + w.write_bits(kind, 6); + w.write_bit(wasted > 0); + if wasted > 0 { + w.write_bits(1, u32::from(wasted)); + } + }; match sub { Subframe::Constant(v) => { - w.write_bits(0, 6); - w.write_bit(false); + header(w, 0); sample(w, *v); } Subframe::Verbatim => { - w.write_bits(1, 6); - w.write_bit(false); + header(w, 1); x.iter().for_each(|&v| sample(w, v)); } Subframe::Fixed { order, residual } => { - w.write_bits(0b001000 | u32::from(*order), 6); - w.write_bit(false); + header(w, 0b001000 | u32::from(*order)); let o = usize::from(*order); x.iter().take(o).for_each(|&v| sample(w, v)); let r = super::predict::fixed_residual(x, o).unwrap_or_default(); @@ -144,8 +158,7 @@ fn write_subframe(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8) { } Subframe::Lpc { coefs, precision, shift, residual } => { let o = coefs.len(); - w.write_bits(0b100000 | (o as u32 - 1), 6); - w.write_bit(false); + header(w, 0b100000 | (o as u32 - 1)); x.iter().take(o).for_each(|&v| sample(w, v)); w.write_bits(u32::from(*precision) - 1, 4); w.write_bits(u32::from(*shift), 5); @@ -153,6 +166,8 @@ fn write_subframe(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8) { let r = super::predict::lpc_residual(x, coefs, *shift).unwrap_or_default(); write_residual(w, residual, &r, x.len(), o); } + // `choose` does not nest them + Subframe::Wasted { coded, .. } => write_coded(w, x, coded, bps, wasted), } } diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index f7ba676c..6645b26f 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -315,3 +315,30 @@ fn the_frame_limit_stops_a_write_before_it_reaches_the_sink() { assert!(w.write(&[[0.0f32; 1]], 1).is_ok()); assert!(matches!(w.write(&[[0.0f32; 1]], 1), Err(AudioError::TooLarge(_)))); } + +/// Samples with unused low bits, on every kind of subframe and stereo mode: the decoder must put +/// the bits back before it undoes the stereo decorrelation. +#[test] +fn wasted_bits_round_trip() { + for (bps, shift) in [(16u8, 3u32), (24, 8), (32, 16), (32, 8)] { + let source = 16.min(bps - shift as u8); + let pad = |kind, seed| -> Vec { signal(kind, 9_000, source, seed).iter().map(|&v| v << shift).collect() }; + for (kind, other) in [("music", "music"), ("noise", "noise"), ("square", "music"), ("click", "sweep")] { + let planar = [pad(kind, 51), pad(other, 52)]; + if round_trip(&format!("wasted-{bps}-{shift}-{kind}"), &planar, EncoderConfig::new(48_000, 2, bps), 4096).is_none() { + return; + } + } + } + // only the top bit can vary: one bit per sample remains + let lowest: Vec = (0..9_000).map(|i| if i % 3 == 0 { -(1 << 23) } else { 0 }).collect(); + round_trip("wasted-to-one-bit", &[lowest], EncoderConfig::new(48_000, 1, 24), 4096); +} + +/// A click at full scale needs a residual near the limit; a block is not stored verbatim for it. +#[test] +fn a_full_scale_click_at_32_bits_is_still_predicted() { + let planar = [signal("click", 4_096, 32, 61)]; + let Some(size) = round_trip("click-32", &planar, EncoderConfig::new(48_000, 1, 32), 4096) else { return }; + assert!(size < 4_096 * 4 / 2, "{size}"); +} diff --git a/crates/audio/src/write/flac/predict.rs b/crates/audio/src/write/flac/predict.rs index bd4951b2..ccf66e76 100644 --- a/crates/audio/src/write/flac/predict.rs +++ b/crates/audio/src/write/flac/predict.rs @@ -2,8 +2,8 @@ //! predictors, linear prediction from the windowed autocorrelation (Levinson–Durbin), and //! partitioned Rice codes. Everything here only measures and chooses; [`super::frame`] writes. -/// Residuals larger than this are not Rice coded (a 5-bit parameter tops out at 30). -const MAX_RESIDUAL: i64 = (1 << 30) - 1; +/// Residuals must fit a 32-bit integer, leaving out its most negative value (§9.2.7.3). +const MAX_RESIDUAL: i64 = i32::MAX as i64; /// A residual coded with partitioned Rice codes. #[derive(Clone, Debug, PartialEq)] @@ -27,8 +27,21 @@ impl Residual { pub enum Subframe { Constant(i32), Verbatim, - Fixed { order: u8, residual: Residual }, - Lpc { coefs: Vec, precision: u8, shift: u8, residual: Residual }, + Fixed { + order: u8, + residual: Residual, + }, + Lpc { + coefs: Vec, + precision: u8, + shift: u8, + residual: Residual, + }, + /// `coded` is of the samples without their `bits` zero low bits, at that many bits fewer (§9.2.2). + Wasted { + bits: u8, + coded: Box, + }, } /// Folded residual: 0, -1, 1, -2, 2 … → 0, 1, 2, 3, 4 …. @@ -243,6 +256,13 @@ pub fn choose(x: &[i32], bps: u8, effort: Effort) -> (Subframe, u64) { if x.iter().all(|&v| v == first) { return (Subframe::Constant(first), head + b); } + // not constant, so not all zero: at least one bit per sample stays + let wasted = x.iter().fold(0, |bits, &v| bits | v).trailing_zeros(); + if wasted > 0 { + let shifted: Vec = x.iter().map(|&v| v >> wasted).collect(); + let (coded, bits) = choose(&shifted, bps - wasted as u8, effort); + return (Subframe::Wasted { bits: wasted as u8, coded: Box::new(coded) }, bits + u64::from(wasted)); + } let mut best = (Subframe::Verbatim, head + n as u64 * b); let mut consider = |sub: Subframe, bits: u64| { if bits < best.1 { @@ -292,6 +312,30 @@ mod tests { assert_eq!(choose(&[7; 100], 16, Effort::level(5)), (Subframe::Constant(7), 24)); } + #[test] + fn residuals_may_reach_the_limit_of_a_32_bit_integer() { + assert!(fixed_residual(&[i32::MAX, -i32::MAX], 0).is_some()); + assert!(fixed_residual(&[0, i32::MAX], 1).is_some()); + // the most negative value is out of range (§9.2.7.3) + assert!(fixed_residual(&[i32::MIN], 0).is_none()); + assert!(fixed_residual(&[0, 1 << 30, -(1 << 30)], 1).is_none()); + assert!(lpc_residual(&[0, i32::MAX], &[1], 0).is_some()); + } + + #[test] + fn wasted_bits_are_not_coded() { + let x = tones(4096); + let padded: Vec = x.iter().map(|&s| s << 8).collect(); + let effort = Effort::level(5); + let (sub, bits) = choose(&x, 16, effort); + // the same coding, plus the 8 bits that say how many are wasted + assert_eq!(choose(&padded, 24, effort), (Subframe::Wasted { bits: 8, coded: Box::new(sub) }, bits + 8)); + // at most what leaves one bit per sample, and nothing for silence, a constant keeps its bits + let lowest = [0, -(1 << 23)]; + assert!(matches!(choose(&lowest, 24, effort).0, Subframe::Wasted { bits: 23, .. })); + assert_eq!(choose(&[256; 10], 24, effort).0, Subframe::Constant(256)); + } + /// Two tones and a little noise, from a fixed seed. fn tones(n: usize) -> Vec { let mut seed = 1u32; diff --git a/crates/audio/tests/audio/roundtrip.rs b/crates/audio/tests/audio/roundtrip.rs index 764a3220..567dc2d7 100644 --- a/crates/audio/tests/audio/roundtrip.rs +++ b/crates/audio/tests/audio/roundtrip.rs @@ -117,6 +117,25 @@ fn flac_handles_silence_noise_full_scale_and_odd_rates() { } } +/// 16-bit audio exported at a larger depth costs no more: the padding is not coded. +#[test] +fn flac_does_not_store_padding() { + let mut src = sine(48_000, 2, 20_000); + for s in src.channels.iter_mut().flatten() { + *s = (*s * 32768.0).round() / 32768.0; + } + let size = |depth| { + let opts = EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(depth).with_dither(false); + let flac = encode(&src, &opts).unwrap(); + let (_, wav) = decode(&encode(&src, &opts.clone().with_format(EncodeFormat::Wav)).unwrap(), None).unwrap(); + assert_eq!(decode(&flac, None).unwrap().1, wav, "{depth:?}"); + flac.len() + }; + let base = size(BitDepth::Int16); + assert!(size(BitDepth::Int24) < base * 11 / 10); + assert!(size(BitDepth::Int32) < base * 11 / 10); +} + #[test] fn flac_rejects_unsupported_layouts() { let opts = EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(BitDepth::Float32).with_dither(false); From 5617261fdc5e6d60abcf82d714183ffa289f8cca Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 06:07:08 +0100 Subject: [PATCH 10/20] craft-audio: review fixes, round 9 (#442) Refuse a linear predictor whose prediction leaves the i32 range. Such a stream is valid, but symphonia adds the prediction to the residual as an i32, which panics in a debug build. The round 8 residual limit made this reachable at 24 bits; without the override, 13 of 240 clipped 32-bit test streams failed in a debug build and none do now, for 0.09% more bytes. Co-Authored-By: Claude Opus 5.5 --- crates/audio/src/write/flac/predict.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/audio/src/write/flac/predict.rs b/crates/audio/src/write/flac/predict.rs index ccf66e76..983dbd4a 100644 --- a/crates/audio/src/write/flac/predict.rs +++ b/crates/audio/src/write/flac/predict.rs @@ -141,7 +141,10 @@ pub fn lpc_residual(x: &[i32], coefs: &[i32], shift: u8) -> Option> { let hist = x.get(i - order..i)?; // coefs[0] weighs the previous sample let pred: i64 = coefs.iter().zip(hist.iter().rev()).map(|(&c, &s)| i64::from(c) * i64::from(s)).sum(); - let r = i64::from(*x.get(i)?) - (pred >> shift); + // Valid past the i32 range too, but symphonia adds it to the residual as an i32, which + // panics in a debug build: its overflow checks are on in apps without our profile override. + let pred = i32::try_from(pred >> shift).ok()?; + let r = i64::from(*x.get(i)?) - i64::from(pred); if r.abs() > MAX_RESIDUAL { return None; } @@ -322,6 +325,16 @@ mod tests { assert!(lpc_residual(&[0, i32::MAX], &[1], 0).is_some()); } + #[test] + fn linear_predictions_stay_in_the_range_of_a_32_bit_integer() { + let x = [1 << 30, 1 << 30]; + assert!(lpc_residual(&x, &[1], 0).is_some()); + // predicts 2^31, and the residual would be small + assert!(lpc_residual(&x, &[2], 0).is_none()); + assert!(lpc_residual(&x, &[4], 1).is_none()); + assert!(lpc_residual(&x, &[3], 1).is_some()); + } + #[test] fn wasted_bits_are_not_coded() { let x = tones(4096); From afdc045e94b5dd30fd7204c7f9b8e91bef5ff960 Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 06:30:29 +0100 Subject: [PATCH 11/20] craft-audio: review fixes, round 10 (#442) Tests only; each was confirmed to fail on a mutated encoder. - Every sample rate code in the frame header table. - Signals that reach the Rice parameter and LPC shift limits, where a value past the limit gives a file decoders refuse. - ENCODE_EXTENSIONS lists flac. Co-Authored-By: Claude Opus 5.5 --- crates/audio/src/write/flac/frame.rs | 8 ++++++++ crates/audio/src/write/flac/oracle_tests.rs | 20 ++++++++++++++++++++ crates/audio/tests/audio/encode_format.rs | 1 + 3 files changed, 29 insertions(+) diff --git a/crates/audio/src/write/flac/frame.rs b/crates/audio/src/write/flac/frame.rs index 72528c5b..593807ea 100644 --- a/crates/audio/src/write/flac/frame.rs +++ b/crates/audio/src/write/flac/frame.rs @@ -231,6 +231,14 @@ mod tests { assert_eq!([8, 12, 16, 20, 24, 32].map(sample_size_code), [1, 2, 4, 5, 6, 0]); } + #[test] + fn sample_rates_in_the_table_have_their_codes() { + let table = [88_200, 176_400, 192_000, 8_000, 16_000, 22_050, 24_000, 32_000, 44_100, 48_000, 96_000]; + for (rate, code) in table.into_iter().zip(1..) { + assert_eq!(sample_rate_code(rate), (code, None), "{rate}"); + } + } + #[test] fn sample_rates_outside_the_table_are_named_in_the_header() { assert_eq!(sample_rate_code(48_000), (10, None)); diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index 6645b26f..2ac0a853 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -342,3 +342,23 @@ fn a_full_scale_click_at_32_bits_is_still_predicted() { let Some(size) = round_trip("click-32", &planar, EncoderConfig::new(48_000, 1, 32), 4096) else { return }; assert!(size < 4_096 * 4 / 2, "{size}"); } + +/// Signals that reach the format's limits, which a decoder refuses when they are passed: a Rice +/// parameter of 31 is the escape code, and an LPC shift above 15 reads as negative. +#[test] +fn rice_parameter_and_lpc_shift_stay_in_range() { + let mut r = Rng(71); + let bursts: Vec = + (0..9_000).map(|i| if i % 1024 < 128 { if i % 2 == 0 { i32::MAX } else { -i32::MAX } } else { (r.next() * 100.0) as i32 }).collect(); + if round_trip("limit-rice", &[bursts], EncoderConfig::new(48_000, 1, 32), 4096).is_none() { + return; + } + let mut y = 0.0; + let correlated: Vec = (0..9_000) + .map(|_| { + y = 0.24 * y + r.next() * 8000.0; + y as i32 + }) + .collect(); + round_trip("limit-shift", &[correlated], EncoderConfig::new(48_000, 1, 16), 4096); +} diff --git a/crates/audio/tests/audio/encode_format.rs b/crates/audio/tests/audio/encode_format.rs index fb47652c..30b99268 100644 --- a/crates/audio/tests/audio/encode_format.rs +++ b/crates/audio/tests/audio/encode_format.rs @@ -10,6 +10,7 @@ fn only_formats_the_encoder_writes_are_named() { assert_eq!(encode_format_for("BWF"), Some(EncodeFormat::Wav)); assert_eq!(encode_format_for(".aifc"), Some(EncodeFormat::Aiff)); assert_eq!(encode_format_for("flac"), Some(EncodeFormat::Flac)); + assert!(craft_audio::ENCODE_EXTENSIONS.contains(&"flac")); for name in craft_audio::ENCODE_EXTENSIONS { assert!(encode_format_for(name).is_some(), "{name}"); } From 1a9f579a07b4a9fbb148a98f52f86bf4f0bfe9dc Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 06:50:56 +0100 Subject: [PATCH 12/20] craft-audio: review fixes, round 11 (#442) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No change in output: the 74 oracle test streams are byte-identical. - The Wasted case is handled in one function, without the unreachable arm and the wrapper round 8 added. - The bitstream writer uses predict::fold, the tested copy, not its own. - STREAMINFO's MD5 may be 0 (RFC 9639 §8.2), so the comment no longer says the RFC requires it. Co-Authored-By: Claude Opus 5.5 --- crates/audio/src/write/flac/encoder.rs | 2 +- crates/audio/src/write/flac/frame.rs | 20 +++++++------------- crates/audio/src/write/flac/predict.rs | 2 +- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/crates/audio/src/write/flac/encoder.rs b/crates/audio/src/write/flac/encoder.rs index c401c746..2e01c62d 100644 --- a/crates/audio/src/write/flac/encoder.rs +++ b/crates/audio/src/write/flac/encoder.rs @@ -81,7 +81,7 @@ impl Encoder { total_samples: 0, min_frame: 0, max_frame: 0, - // RFC 9639 requires an MD5 signature of the unencoded audio in STREAMINFO; this is an + // STREAMINFO holds an MD5 signature of the unencoded audio (RFC 9639 §8.2); this is an // integrity checksum, not a password hash or a security boundary. md5: super::md5::Md5::default(), // NOSONAR }) diff --git a/crates/audio/src/write/flac/frame.rs b/crates/audio/src/write/flac/frame.rs index 593807ea..34756ca0 100644 --- a/crates/audio/src/write/flac/frame.rs +++ b/crates/audio/src/write/flac/frame.rs @@ -106,7 +106,7 @@ fn write_residual(w: &mut BitWriter, residual: &Residual, r: &[i64], block: usiz w.write_bits(u32::from(k), if wide { 5 } else { 4 }); let n = if p == 0 { len.saturating_sub(order) } else { len }; for &v in r.get(start..start + n).unwrap_or_default() { - let u = if v >= 0 { (v as u64) << 1 } else { ((-v - 1) as u64) << 1 | 1 }; + let u = super::predict::fold(v); let mut q = u >> k; while q >= 32 { w.write_bits(0, 32); @@ -119,16 +119,8 @@ fn write_residual(w: &mut BitWriter, residual: &Residual, r: &[i64], block: usiz } } -fn write_subframe(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8) { - if let Subframe::Wasted { bits, coded } = sub { - let shifted: Vec = x.iter().map(|&v| v >> bits).collect(); - return write_coded(w, &shifted, coded, bps - bits, *bits); - } - write_coded(w, x, sub, bps, 0); -} - /// Subframe header (§9.2.1) and body of samples already without their `wasted` low bits. -fn write_coded(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8, wasted: u8) { +fn write_subframe(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8, wasted: u8) { let b = u32::from(bps); let sample = |w: &mut BitWriter, v: i32| w.write_bits(v as u32, b); // the flag, then the count minus 1 in unary @@ -166,8 +158,10 @@ fn write_coded(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8, wasted: u8 let r = super::predict::lpc_residual(x, coefs, *shift).unwrap_or_default(); write_residual(w, residual, &r, x.len(), o); } - // `choose` does not nest them - Subframe::Wasted { coded, .. } => write_coded(w, x, coded, bps, wasted), + Subframe::Wasted { bits, coded } => { + let shifted: Vec = x.iter().map(|&v| v >> bits).collect(); + write_subframe(w, &shifted, coded, bps - bits, *bits); + } } } @@ -196,7 +190,7 @@ pub fn write(spec: &FrameSpec) -> Vec { let mut w = BitWriter::new(); w.write_bytes(&h); for (x, sub, bps) in spec.channels { - write_subframe(&mut w, x, sub, *bps); + write_subframe(&mut w, x, sub, *bps, 0); } let mut out = w.finish(); let crc = crc16(&out); diff --git a/crates/audio/src/write/flac/predict.rs b/crates/audio/src/write/flac/predict.rs index 983dbd4a..7a99464e 100644 --- a/crates/audio/src/write/flac/predict.rs +++ b/crates/audio/src/write/flac/predict.rs @@ -45,7 +45,7 @@ pub enum Subframe { } /// Folded residual: 0, -1, 1, -2, 2 … → 0, 1, 2, 3, 4 …. -fn fold(r: i64) -> u64 { +pub(super) fn fold(r: i64) -> u64 { if r >= 0 { (r as u64) << 1 } else { ((-r - 1) as u64) << 1 | 1 } } From 57f5d063fe9155a30b3f68ed584f7b870d8905cc Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 13:51:18 +0100 Subject: [PATCH 13/20] craft-audio: review fixes, round 12 (#442) - FLAC files read their layout from the channel mask comment, or else the one decoders assume for the count, which is what the writer omits the comment for - an unbuilt format is Unsupported before the layout is checked - FLAC encode no longer reserves half the PCM size up front - flac-encode's lib tests stream a file and compare it with encode - the seek test compares samples after the seek Co-Authored-By: Claude Opus 5.5 --- crates/audio/README.md | 2 +- crates/audio/src/layout.rs | 13 ++++ crates/audio/src/read/flac.rs | 103 ++++++++++++++++++++++++-- crates/audio/src/read/symph.rs | 31 +++++--- crates/audio/src/write/flac/mod.rs | 21 +----- crates/audio/src/write/mod.rs | 40 ++++++++-- crates/audio/tests/audio/roundtrip.rs | 26 +++++++ crates/audio/tests/audio/stream.rs | 12 ++- 8 files changed, 201 insertions(+), 47 deletions(-) diff --git a/crates/audio/README.md b/crates/audio/README.md index c1f7dcef..7853f11b 100644 --- a/crates/audio/README.md +++ b/crates/audio/README.md @@ -264,7 +264,7 @@ match craft_audio::decode(&bytes, None) { `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. + centre, side left and side right. craft-audio reads the comment in FLAC files, not in FLAC in Ogg. - FLAC at 32 bits codes stereo channels independently, because a side channel would need 33 bits. - 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. diff --git a/crates/audio/src/layout.rs b/crates/audio/src/layout.rs index e4028f0e..5cd054ca 100644 --- a/crates/audio/src/layout.rs +++ b/crates/audio/src/layout.rs @@ -263,6 +263,19 @@ impl std::fmt::Display for ChannelLayout { } } +/// The layout decoders assume for 1–8 FLAC channels with no channel mask comment, as a WAV mask. +/// RFC 9639 §9.1.3 leaves "back/surround" open for 5 and 6 channels; libFLAC and ffmpeg read side +/// surrounds. 7 channels are 6.1 with sides (FL FR FC LFE BC SL SR). +#[cfg(any(feature = "symphonia", feature = "flac-encode"))] +pub(crate) fn flac_assumed_mask(channels: usize) -> u32 { + match channels { + 5 => 0x607, + 6 => 0x60F, + 7 => 0x70F, + n => ChannelLayout::default_for(n as u16).mask().unwrap_or(0), + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/audio/src/read/flac.rs b/crates/audio/src/read/flac.rs index 896150e2..ad4f4d88 100644 --- a/crates/audio/src/read/flac.rs +++ b/crates/audio/src/read/flac.rs @@ -67,16 +67,66 @@ pub(crate) fn empty_stream_info(b: &[u8], at: usize) -> Option { 17..=24 => SampleFormat::Int24, _ => SampleFormat::Int32, }; - Some(AudioInfo::new(Container::Flac, CodecId::Flac, sample_format, sample_rate, flac_layout(usize::from(channels)), 0)) + Some(AudioInfo::new(Container::Flac, CodecId::Flac, sample_format, sample_rate, layout_of(b, at, channels), 0)) } -/// The FLAC channel assignment for a channel count (RFC 9639 §9.1.3). -pub(crate) fn flac_layout(channels: usize) -> ChannelLayout { - match channels { - 7 => ChannelLayout::SURROUND_6_1, - 1..=8 => ChannelLayout::default_for(crate::layout::count(channels)), - n => ChannelLayout::unordered(crate::layout::count(n)), +/// The layout of the FLAC stream at `at`, from its STREAMINFO channel count and its channel mask +/// comment, or `None` when it does not start with a STREAMINFO block. +pub(crate) fn layout(b: &[u8], at: usize) -> Option { + let head = be_u32(b, at.checked_add(4)?)?; + if (head >> 24) & 0x7f != 0 { + return None; } + // In STREAMINFO: 3 bits of channels - 1 after the 20 bits of rate. + let channels = u16::from((b.get(at.checked_add(4 + 4 + 10 + 2)?)? >> 1) & 0x7) + 1; + Some(layout_of(b, at, channels)) +} + +/// `channels` channels described by the channel mask comment of the stream at `at`, or else in +/// the layout decoders assume for the count (RFC 9639 §8.6.2, §9.1.3). +fn layout_of(b: &[u8], at: usize, channels: u16) -> ChannelLayout { + let mask = channel_mask(b, at).unwrap_or_else(|| crate::layout::flac_assumed_mask(usize::from(channels))); + ChannelLayout::from_mask_and_channels(mask, channels) +} + +/// The value of the first `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment that holds a `0x` hexadecimal +/// number, in the stream at `at`. +fn channel_mask(b: &[u8], at: usize) -> Option { + let mut pos = at.checked_add(4)?; + loop { + let head = be_u32(b, pos)?; + let (body, len) = (pos.checked_add(4)?, (head & 0x00ff_ffff) as usize); + let end = body.checked_add(len)?.min(b.len()); + if (head >> 24) & 0x7f == 4 { + return comments_mask(b.get(body..end)?); + } + if head >> 31 == 1 { + return None; + } + pos = body.checked_add(len)?; + } +} + +/// The channel mask in a Vorbis comment block's body. +fn comments_mask(block: &[u8]) -> Option { + let vendor = le_u32(block, 0)? as usize; + let mut at = 4usize.checked_add(vendor)?; + let count = le_u32(block, at)?; + at += 4; + for _ in 0..count { + let len = le_u32(block, at)? as usize; + let comment = block.get(at + 4..(at + 4).checked_add(len)?)?; + at += 4 + len; + let Some(eq) = comment.iter().position(|&c| c == b'=') else { continue }; + if !comment[..eq].eq_ignore_ascii_case(b"WAVEFORMATEXTENSIBLE_CHANNEL_MASK") { + continue; + } + let hex = comment[eq + 1..].strip_prefix(b"0x").or_else(|| comment[eq + 1..].strip_prefix(b"0X")); + if let Some(mask) = hex.and_then(|h| std::str::from_utf8(h).ok()).and_then(|h| u32::from_str_radix(h, 16).ok()) { + return Some(mask); + } + } + None } /// Whether a comment in a (little-endian) Vorbis comment block claims more than the block holds. @@ -102,3 +152,42 @@ fn overruns(b: &[u8], mut at: usize, end: usize, read: fn(&[u8], usize) -> Optio } false } + +#[cfg(test)] +mod tests { + use super::comments_mask; + + fn block(comments: &[&str]) -> Vec { + let mut b = 3u32.to_le_bytes().to_vec(); + b.extend_from_slice(b"abc"); + b.extend_from_slice(&(comments.len() as u32).to_le_bytes()); + for c in comments { + b.extend_from_slice(&(c.len() as u32).to_le_bytes()); + b.extend_from_slice(c.as_bytes()); + } + b + } + + #[test] + fn the_channel_mask_comment_is_found_in_any_case() { + assert_eq!(comments_mask(&block(&["TITLE=x", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x0107"])), Some(0x107)); + assert_eq!(comments_mask(&block(&["waveformatextensible_channel_mask=0X3f"])), Some(0x3F)); + assert_eq!(comments_mask(&block(&["TITLE=x"])), None); + } + + #[test] + fn a_bad_channel_mask_comment_is_ignored() { + for value in ["63", "0x", "0xZZ", "0x1FFFFFFFF"] { + assert_eq!(comments_mask(&block(&[&format!("WAVEFORMATEXTENSIBLE_CHANNEL_MASK={value}")])), None, "{value}"); + } + assert_eq!(comments_mask(&block(&["WAVEFORMATEXTENSIBLE_CHANNEL_MASK=bad", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x3"])), Some(3)); + } + + #[test] + fn a_truncated_comment_block_has_no_mask() { + let whole = block(&["WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x3"]); + for cut in 0..whole.len() { + assert_eq!(comments_mask(&whole[..cut]), None, "{cut}"); + } + } +} diff --git a/crates/audio/src/read/symph.rs b/crates/audio/src/read/symph.rs index f53aca1b..a27d2308 100644 --- a/crates/audio/src/read/symph.rs +++ b/crates/audio/src/read/symph.rs @@ -81,9 +81,10 @@ fn guarded(f: impl FnOnce() -> Result) -> Result { catch_unwind(AssertUnwindSafe(f)).unwrap_or_else(|_| Err(AudioError::Malformed("decoder failed on malformed input".into()))) } -/// What [`open`] found: a reader, or a stream that holds no audio frames and has no reader. +/// What [`open`] found: a reader (with the layout of a FLAC stream), or a stream that holds no +/// audio frames and has no reader. enum Opened { - Reader(Box), + Reader(Box, Option), Empty(AudioInfo), } @@ -124,6 +125,8 @@ enum Precheck { Empty(AudioInfo), /// The size checks ran for the container the probe reports under this short name. Checked(&'static str), + /// As `Checked`, for a FLAC stream with this layout, which symphonia's reader does not parse. + Flac(&'static str, Option), /// No container size check was needed. Unchecked, } @@ -330,7 +333,7 @@ fn precheck(b: &[u8], start: usize, detected: Option) -> Result { super::caf::check(b, at)?; @@ -356,10 +359,11 @@ fn open(bytes: Cow<'_, [u8]>, detected: Option) -> Result { if start > 0 && start >= bytes.len() { return Err(AudioError::Malformed("ID3v2 tag extends past the end of the file".into())); } - let checked = precheck(&bytes, start, detected)?; - if let Precheck::Empty(info) = checked { - return Ok(Opened::Empty(info)); - } + let (checked, flac_layout) = match precheck(&bytes, start, detected)? { + Precheck::Empty(info) => return Ok(Opened::Empty(info)), + Precheck::Flac(expected, layout) => (Precheck::Checked(expected), layout), + checked => (checked, None), + }; // symphonia reads from an owned stream: a borrowed buffer is copied once, a buffer read from // a shared source is moved in. let mut mss = MediaSourceStream::new(Box::new(Cursor::new(bytes.into_owned())), MediaSourceStreamOptions::default()); @@ -381,7 +385,7 @@ fn open(bytes: Cow<'_, [u8]>, detected: Option) -> Result { } _ => {} } - Ok(Opened::Reader(reader)) + Ok(Opened::Reader(reader, flac_layout.filter(|_| probed == "flac"))) } /// The codec symphonia reports, as a [`CodecId`]. @@ -543,8 +547,8 @@ impl Symph { /// Open audio track `track` (the first when `None`) of the file in `bytes`. pub(crate) fn open(bytes: Cow<'_, [u8]>, detected: Option, track: Option) -> Result { guarded(move || { - let reader = match open(bytes, detected)? { - Opened::Reader(reader) => reader, + let (reader, flac_layout) = match open(bytes, detected)? { + Opened::Reader(reader, layout) => (reader, layout), Opened::Empty(info) => { // a stream with no audio still has one track, as WAV and AIFF do if let Some(n) = track.filter(|&n| n > 0) { @@ -566,7 +570,10 @@ impl Symph { }); } }; - let (track_id, info) = info_of(reader.as_ref(), detected, track)?; + let (track_id, mut info) = info_of(reader.as_ref(), detected, track)?; + if let Some(layout) = flac_layout.filter(|l| l.channels() == info.channels) { + info.layout = layout; + } let t = reader.tracks().iter().find(|t| t.id == track_id).ok_or_else(|| AudioError::Malformed("track vanished".into()))?; let params = audio_params(t).cloned().ok_or_else(|| AudioError::Malformed("track vanished".into()))?; let alac = params.extra_data.as_deref().filter(|_| params.codec == CODEC_ID_ALAC).and_then(|d| be_u32(d, 0)); @@ -759,7 +766,7 @@ impl Symph { pub(crate) fn tracks(bytes: Cow<'_, [u8]>, detected: Option) -> Result> { guarded(move || { let reader = match open(bytes, detected)? { - Opened::Reader(reader) => reader, + Opened::Reader(reader, _) => reader, Opened::Empty(info) => return Ok(vec![(info.codec, info.sample_rate, info.channels)]), }; Ok(audio_tracks(reader.as_ref()) diff --git a/crates/audio/src/write/flac/mod.rs b/crates/audio/src/write/flac/mod.rs index 5ada9ad0..720b59b4 100644 --- a/crates/audio/src/write/flac/mod.rs +++ b/crates/audio/src/write/flac/mod.rs @@ -39,23 +39,11 @@ fn check(channels: usize, sample_rate: u32, depth: BitDepth) -> Result { /// Frames quantized and handed to the encoder at a time: its block size. const CHUNK: usize = 4096; -/// The layout decoders assume for 1–8 channels with no channel mask comment, as a WAV mask. RFC -/// 9639 §9.1.3 leaves "back/surround" open for 5 and 6 channels; libFLAC and ffmpeg read side -/// surrounds. 7 channels are 6.1 with sides (FL FR FC LFE BC SL SR). -fn assumed_mask(channels: usize) -> u32 { - match channels { - 5 => 0x607, - 6 => 0x60F, - 7 => 0x70F, - n => ChannelLayout::default_for(n as u16).mask().unwrap_or(0), - } -} - /// The `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment (RFC 9639 §8.6.2) for a layout decoders would /// not assume: `mask` (the usual layout for the count when `None`). fn mask_comment(channels: usize, mask: Option) -> Option { let mask = mask.unwrap_or_else(|| ChannelLayout::default_for(channels as u16).mask().unwrap_or(0)); - (mask != assumed_mask(channels)).then(|| format!("WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x{mask:04X}")) + (mask != crate::layout::flac_assumed_mask(channels)).then(|| format!("WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x{mask:04X}")) } fn encoder_error(e: encoder::Error) -> AudioError { @@ -150,12 +138,9 @@ impl Writer { pub(crate) fn encode(buf: &AudioBuffer, depth: BitDepth, dither: bool, mask: Option) -> Result> { validate_buffer(buf)?; - let bps = check(buf.channels.len(), buf.sample_rate, depth)?; + check(buf.channels.len(), buf.sample_rate, depth)?; let frames = buf.frames(); - let mut out = Vec::new(); - out.try_reserve(frames.saturating_mul(buf.channels.len()).saturating_mul(usize::from(bps) / 8) / 2) - .map_err(|_| AudioError::TooLarge("cannot allocate FLAC output".into()))?; - let mut w = Writer::new(Cursor::new(out), buf.channels.len(), buf.sample_rate, depth, dither, mask)?; + let mut w = Writer::new(Cursor::new(Vec::new()), buf.channels.len(), buf.sample_rate, depth, dither, mask)?; w.write(&buf.channels, frames)?; Ok(w.finish()?.into_inner()) } diff --git a/crates/audio/src/write/mod.rs b/crates/audio/src/write/mod.rs index 9492d5e1..eaad8e96 100644 --- a/crates/audio/src/write/mod.rs +++ b/crates/audio/src/write/mod.rs @@ -26,6 +26,7 @@ use std::io::{Seek, Write}; /// # } /// ``` pub fn encode(buf: &AudioBuffer, opts: &EncodeOptions) -> Result> { + check_built(opts.format)?; // Checked for every format; WAV and FLAC record it. let mask = opts.layout.map(|l| wav_mask(&l, buf.channels.len())).transpose()?; match opts.format { @@ -36,15 +37,15 @@ pub fn encode(buf: &AudioBuffer, opts: &EncodeOptions) -> Result> { #[cfg(feature = "flac-encode")] EncodeFormat::Flac => flac::encode(buf, opts.bit_depth, opts.dither, mask), #[allow(unreachable_patterns)] - f => Err(not_built(f)), + f => Err(AudioError::Unsupported(format!("writing {f:?}"))), } } -/// The error for a format whose writer is not built. -#[allow(dead_code)] -fn not_built(format: EncodeFormat) -> AudioError { - let feature = if format == EncodeFormat::Flac { "flac-encode" } else { "pcm-encode" }; - AudioError::Unsupported(format!("writing {format:?} needs the `{feature}` feature")) +/// Refuses a format whose writer is not built, before anything else about the request is checked. +fn check_built(format: EncodeFormat) -> Result<()> { + let (built, feature) = + if format == EncodeFormat::Flac { (cfg!(feature = "flac-encode"), "flac-encode") } else { (cfg!(feature = "pcm-encode"), "pcm-encode") }; + if built { Ok(()) } else { Err(AudioError::Unsupported(format!("writing {format:?} needs the `{feature}` feature"))) } } /// The file extensions of the formats [`encode`] can write (those whose features are on). @@ -150,6 +151,7 @@ impl FileEncoder { /// Starts a file of `channels` channels at `sample_rate` at the current position of `out`. /// `opts.layout`, when set, must have `channels` channels. pub fn new(out: W, opts: &EncodeOptions, sample_rate: u32, channels: u16) -> Result { + check_built(opts.format)?; let (depth, dither, channels) = (opts.bit_depth, opts.dither, usize::from(channels)); let mask = opts.layout.map(|l| wav_mask(&l, channels)).transpose()?; let writer = match opts.format { @@ -167,7 +169,7 @@ impl FileEncoder { #[cfg(feature = "flac-encode")] EncodeFormat::Flac => Writer::Flac(flac::Writer::new(out, channels, sample_rate, depth, dither, mask)?), #[allow(unreachable_patterns)] - f => return Err(not_built(f)), + f => return Err(AudioError::Unsupported(format!("writing {f:?}"))), }; Ok(FileEncoder { writer, channels }) } @@ -259,5 +261,29 @@ mod tests { let enc = FileEncoder::new(std::io::Cursor::new(Vec::new()), &opts, 48_000, 1); assert!(matches!(enc, Err(AudioError::Unsupported(_)))); assert_eq!(encode_format_for(name), None); + // Also with a layout it could not use. + let opts = opts.with_layout(ChannelLayout::STEREO); + assert!(matches!(encode(&buf, &opts), Err(AudioError::Unsupported(_)))); + let enc = FileEncoder::new(std::io::Cursor::new(Vec::new()), &opts, 48_000, 1); + assert!(matches!(enc, Err(AudioError::Unsupported(_)))); + } + + /// Streaming in any block sizes writes the bytes `encode` writes. + #[cfg(feature = "flac-encode")] + #[test] + fn streamed_flac_matches_encode() { + let channels: Vec> = (0..2).map(|c| (0..20_000).map(|i| ((i * (c + 3)) as f32 * 0.01).sin() * 0.5).collect()).collect(); + let buf = AudioBuffer { sample_rate: 48_000, channels }; + let opts = EncodeOptions::new(EncodeFormat::Flac); + let whole = encode(&buf, &opts).unwrap(); + let mut enc = FileEncoder::new(std::io::Cursor::new(Vec::new()), &opts, 48_000, 2).unwrap(); + let mut at = 0; + for n in [1000, 4096, 1, 9000, 5903] { + let block: Vec<&[f32]> = buf.channels.iter().map(|c| &c[at..at + n]).collect(); + enc.write(&block, n).unwrap(); + at += n; + } + assert_eq!(at, buf.frames()); + assert_eq!(enc.finish().unwrap().into_inner(), whole); } } diff --git a/crates/audio/tests/audio/roundtrip.rs b/crates/audio/tests/audio/roundtrip.rs index 567dc2d7..35f1b156 100644 --- a/crates/audio/tests/audio/roundtrip.rs +++ b/crates/audio/tests/audio/roundtrip.rs @@ -386,3 +386,29 @@ fn a_byte_rate_past_32_bits_is_an_error() { assert!(matches!(wav_float_header(4_000_000_000, 2, 0), Err(craft_audio::AudioError::Encode(_)))); assert!(wav_float_header(4_000_000_000 / 8, 2, 0).is_ok()); } + +/// The channel mask comment of a FLAC file is its layout, for `probe`, `decode` and `Decoder`, and +/// for a file with no audio frames. +#[test] +fn flac_layouts_read_back_from_the_mask_comment() { + use craft_audio::{DecodeOptions, Decoder}; + let cases = [ + (6, Some(ChannelLayout::SURROUND_5_1_SIDE), ChannelLayout::SURROUND_5_1_SIDE), + (6, None, ChannelLayout::SURROUND_5_1), + (7, Some(ChannelLayout::SURROUND_6_1), ChannelLayout::SURROUND_6_1), + (7, None, ChannelLayout::default_for(7)), + (4, Some(ChannelLayout::from_mask(0x107)), ChannelLayout::from_mask(0x107)), + (2, Some(ChannelLayout::unordered(2)), ChannelLayout::unordered(2)), + ]; + for (channels, layout, expected) in cases { + for frames in [4096, 0] { + let mut opts = EncodeOptions::new(EncodeFormat::Flac); + opts.layout = layout; + let file = craft_audio::encode(&AudioBuffer::new(48_000, channels, frames), &opts).unwrap(); + assert_eq!(probe(&file, None).unwrap().layout, expected, "probe {channels} {layout:?} {frames}"); + assert_eq!(decode(&file, None).unwrap().0.layout, expected, "decode {channels} {layout:?} {frames}"); + let dec = Decoder::from_bytes(&file, None, &DecodeOptions::default()).unwrap(); + assert_eq!(dec.info().layout, expected, "Decoder {channels} {layout:?} {frames}"); + } + } +} diff --git a/crates/audio/tests/audio/stream.rs b/crates/audio/tests/audio/stream.rs index fcd27def..183bdab9 100644 --- a/crates/audio/tests/audio/stream.rs +++ b/crates/audio/tests/audio/stream.rs @@ -182,8 +182,16 @@ fn seeking_to_or_past_the_end_ends_the_stream() { assert!(dec.next_block().unwrap().is_none(), "{format:?} seek({target})"); // Seeking back still works. assert_eq!(dec.seek(4000).unwrap(), 4000, "{format:?}"); - let left: usize = std::iter::from_fn(|| dec.next_block().unwrap()).map(|b| b.frames()).sum(); - assert_eq!(left, 800, "{format:?}"); + let mut left = vec![Vec::new(); buf.channels.len()]; + while let Some(b) = dec.next_block().unwrap() { + for (l, c) in left.iter_mut().zip(b.channels) { + l.extend(c); + } + } + for (l, source) in left.iter().zip(&buf.channels) { + assert_eq!(l.len(), 800, "{format:?}"); + assert!(l.iter().zip(&source[4000..]).all(|(a, b)| (a - b.clamp(-1.0, 1.0)).abs() <= 2.0 / 32768.0), "{format:?}"); + } } } } From 99dcf517a4f475670f456a05d81b9a5cd1cce82e Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 14:49:49 +0100 Subject: [PATCH 14/20] craft-audio: review fixes, round 13 (#442) FLAC in Ogg reads its layout the way a FLAC file does: from the mask comment, or else the one decoders assume for the count. Without this, a 6-channel stream with no comment reported back surrounds in Ogg and side surrounds in a FLAC file. Co-Authored-By: Claude Opus 5.5 --- crates/audio/README.md | 2 +- crates/audio/src/read/flac.rs | 23 ++++++++++++----- crates/audio/src/read/symph.rs | 21 ++++++++++++++-- crates/audio/src/write/flac/oracle_tests.rs | 28 +++++++++++++++++++++ 4 files changed, 65 insertions(+), 9 deletions(-) diff --git a/crates/audio/README.md b/crates/audio/README.md index 7853f11b..c1f7dcef 100644 --- a/crates/audio/README.md +++ b/crates/audio/README.md @@ -264,7 +264,7 @@ match craft_audio::decode(&bytes, None) { `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. craft-audio reads the comment in FLAC files, not in FLAC in Ogg. + centre, side left and side right. - FLAC at 32 bits codes stereo channels independently, because a side channel would need 33 bits. - 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. diff --git a/crates/audio/src/read/flac.rs b/crates/audio/src/read/flac.rs index ad4f4d88..86090e45 100644 --- a/crates/audio/src/read/flac.rs +++ b/crates/audio/src/read/flac.rs @@ -85,10 +85,25 @@ pub(crate) fn layout(b: &[u8], at: usize) -> Option { /// `channels` channels described by the channel mask comment of the stream at `at`, or else in /// the layout decoders assume for the count (RFC 9639 §8.6.2, §9.1.3). fn layout_of(b: &[u8], at: usize, channels: u16) -> ChannelLayout { - let mask = channel_mask(b, at).unwrap_or_else(|| crate::layout::flac_assumed_mask(usize::from(channels))); + layout_from(channel_mask(b, at), channels) +} + +/// `channels` channels described by a channel mask comment's value, or else in the layout decoders +/// assume for the count. +pub(crate) fn layout_from(mask: Option, channels: u16) -> ChannelLayout { + let mask = mask.unwrap_or_else(|| crate::layout::flac_assumed_mask(usize::from(channels))); ChannelLayout::from_mask_and_channels(mask, channels) } +/// The mask in a comment, when its key is the channel mask's and its value a `0x` hexadecimal number. +pub(crate) fn comment_mask(key: &[u8], value: &[u8]) -> Option { + if !key.eq_ignore_ascii_case(b"WAVEFORMATEXTENSIBLE_CHANNEL_MASK") { + return None; + } + let hex = value.strip_prefix(b"0x").or_else(|| value.strip_prefix(b"0X"))?; + u32::from_str_radix(std::str::from_utf8(hex).ok()?, 16).ok() +} + /// The value of the first `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment that holds a `0x` hexadecimal /// number, in the stream at `at`. fn channel_mask(b: &[u8], at: usize) -> Option { @@ -118,11 +133,7 @@ fn comments_mask(block: &[u8]) -> Option { let comment = block.get(at + 4..(at + 4).checked_add(len)?)?; at += 4 + len; let Some(eq) = comment.iter().position(|&c| c == b'=') else { continue }; - if !comment[..eq].eq_ignore_ascii_case(b"WAVEFORMATEXTENSIBLE_CHANNEL_MASK") { - continue; - } - let hex = comment[eq + 1..].strip_prefix(b"0x").or_else(|| comment[eq + 1..].strip_prefix(b"0X")); - if let Some(mask) = hex.and_then(|h| std::str::from_utf8(h).ok()).and_then(|h| u32::from_str_radix(h, 16).ok()) { + if let Some(mask) = comment_mask(&comment[..eq], &comment[eq + 1..]) { return Some(mask); } } diff --git a/crates/audio/src/read/symph.rs b/crates/audio/src/read/symph.rs index a27d2308..8a6f3531 100644 --- a/crates/audio/src/read/symph.rs +++ b/crates/audio/src/read/symph.rs @@ -16,7 +16,7 @@ use symphonia::core::errors::Error as SymError; use symphonia::core::formats::probe::{Hint, ProbeOptions, ProbeableFormat, Score, Scoreable}; use symphonia::core::formats::{FormatOptions, FormatReader, SeekMode, SeekTo, Track}; use symphonia::core::io::{MediaSourceStream, MediaSourceStreamOptions, ScopedStream}; -use symphonia::core::meta::MetadataOptions; +use symphonia::core::meta::{MetadataOptions, RawValue}; use symphonia::core::units::{TimeBase, Timestamp}; use super::id3v2_len; @@ -446,6 +446,21 @@ fn layout_of(channels: Option<&Channels>) -> ChannelLayout { } } +/// The channel mask comment among the tags the reader has read. +fn metadata_channel_mask(reader: &mut dyn FormatReader) -> Option { + let mut metadata = reader.metadata(); + loop { + let mask = metadata.current()?.media.tags.iter().find_map(|t| match &t.raw.value { + RawValue::String(value) => super::flac::comment_mask(t.raw.key.as_bytes(), value.as_bytes()), + _ => None, + }); + if mask.is_some() { + return mask; + } + metadata.pop()?; + } +} + /// The stored sample format, using the codec and its declared bit depth. Lossy and unusual /// codecs decode to `Compressed`. fn sample_format_of(p: &AudioCodecParameters) -> SampleFormat { @@ -547,7 +562,7 @@ impl Symph { /// Open audio track `track` (the first when `None`) of the file in `bytes`. pub(crate) fn open(bytes: Cow<'_, [u8]>, detected: Option, track: Option) -> Result { guarded(move || { - let (reader, flac_layout) = match open(bytes, detected)? { + let (mut reader, flac_layout) = match open(bytes, detected)? { Opened::Reader(reader, layout) => (reader, layout), Opened::Empty(info) => { // a stream with no audio still has one track, as WAV and AIFF do @@ -573,6 +588,8 @@ impl Symph { let (track_id, mut info) = info_of(reader.as_ref(), detected, track)?; if let Some(layout) = flac_layout.filter(|l| l.channels() == info.channels) { info.layout = layout; + } else if info.codec == CodecId::Flac && info.container == Container::Ogg { + info.layout = super::flac::layout_from(metadata_channel_mask(reader.as_mut()), info.channels); } let t = reader.tracks().iter().find(|t| t.id == track_id).ok_or_else(|| AudioError::Malformed("track vanished".into()))?; let params = audio_params(t).cloned().ok_or_else(|| AudioError::Malformed("track vanished".into()))?; diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index 2ac0a853..6dd19f17 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -236,6 +236,34 @@ fn layouts_decoders_would_not_assume_are_recorded() { } } +/// FLAC in Ogg reads its layout as a FLAC file does: from the mask comment, or else the one +/// decoders assume for the count. +#[cfg(feature = "symphonia")] +#[test] +fn ogg_flac_reads_the_mask_comment() { + use crate::ChannelLayout; + let Some(ff) = ffmpeg() else { return }; + let cases: [(&str, &str, &[&str], u16, u32); 3] = [ + ("ogg-mask", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=48000", &["-metadata", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x0107"], 4, 0x107), + ("ogg-5.1", "aevalsrc=0|0|0|0|0|0:c=5.1:d=0.1:s=48000", &[], 6, 0x60F), + ("ogg-stereo", "aevalsrc=0|0:c=stereo:d=0.1:s=48000", &["-metadata", "TITLE=x"], 2, 0x3), + ]; + for (name, source, extra, channels, mask) in cases { + let path = common::fixture_subdir("flac").join(format!("{name}.oga")); + let status = Command::new(&ff) + .args(["-v", "error", "-y", "-f", "lavfi", "-i", source, "-c:a", "flac", "-f", "ogg"]) + .args(extra) + .arg(&path) + .status() + .unwrap(); + assert!(status.success(), "{name}"); + let bytes = std::fs::read(&path).unwrap(); + let expected = ChannelLayout::from_mask_and_channels(mask, channels); + assert_eq!(crate::probe(&bytes, None).unwrap().layout, expected, "probe {name}"); + assert_eq!(crate::decode(&bytes, None).unwrap().0.layout, expected, "decode {name}"); + } +} + #[test] fn the_mask_comment_is_written_only_when_decoders_would_assume_otherwise() { use super::mask_comment; From 2f4fb8bfe07b081c3a311ca9df1d793d74d8ca29 Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 15:09:27 +0100 Subject: [PATCH 15/20] craft-audio: review fixes, round 14 (#442) Each FLAC stream in an Ogg file reads its own channel mask comment. The tags symphonia's reader collects do not say which track they are from, so a track could get another track's mask. The masks now come from craft-media's Ogg demuxer, by serial, which is symphonia's track ID. The Ogg layout test also covers 7 channels with no comment and a zero mask. Co-Authored-By: Claude Opus 5.5 --- crates/audio/src/read/flac.rs | 31 +++++++---- crates/audio/src/read/symph.rs | 57 +++++++++++---------- crates/audio/src/write/flac/oracle_tests.rs | 32 +++++++++++- 3 files changed, 84 insertions(+), 36 deletions(-) diff --git a/crates/audio/src/read/flac.rs b/crates/audio/src/read/flac.rs index 86090e45..1dc4d0f3 100644 --- a/crates/audio/src/read/flac.rs +++ b/crates/audio/src/read/flac.rs @@ -1,5 +1,7 @@ //! FLAC metadata checks run before symphonia decodes a stream (RFC 9639 metadata blocks). +use craft_media::ogg; + use crate::pcm::{be_u32, le_u32}; use crate::{AudioError, AudioInfo, ChannelLayout, CodecId, Container, Result, SampleFormat}; @@ -95,15 +97,6 @@ pub(crate) fn layout_from(mask: Option, channels: u16) -> ChannelLayout { ChannelLayout::from_mask_and_channels(mask, channels) } -/// The mask in a comment, when its key is the channel mask's and its value a `0x` hexadecimal number. -pub(crate) fn comment_mask(key: &[u8], value: &[u8]) -> Option { - if !key.eq_ignore_ascii_case(b"WAVEFORMATEXTENSIBLE_CHANNEL_MASK") { - return None; - } - let hex = value.strip_prefix(b"0x").or_else(|| value.strip_prefix(b"0X"))?; - u32::from_str_radix(std::str::from_utf8(hex).ok()?, 16).ok() -} - /// The value of the first `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment that holds a `0x` hexadecimal /// number, in the stream at `at`. fn channel_mask(b: &[u8], at: usize) -> Option { @@ -122,6 +115,26 @@ fn channel_mask(b: &[u8], at: usize) -> Option { } } +/// The mask in a comment, when its key is the channel mask's and its value a `0x` hexadecimal number. +fn comment_mask(key: &[u8], value: &[u8]) -> Option { + if !key.eq_ignore_ascii_case(b"WAVEFORMATEXTENSIBLE_CHANNEL_MASK") { + return None; + } + let hex = value.strip_prefix(b"0x").or_else(|| value.strip_prefix(b"0X"))?; + u32::from_str_radix(std::str::from_utf8(hex).ok()?, 16).ok() +} + +/// The channel mask comment of each FLAC stream that has one in the Ogg file `b`, by serial. +pub(crate) fn ogg_channel_masks(b: &[u8]) -> Vec<(u32, u32)> { + let Ok(file) = ogg::open(b) else { return Vec::new() }; + let comments = |s: &ogg::Stream| { + // After the first packet, each header packet is a metadata block with its 4-byte header. + let block = s.headers.iter().skip(1).find(|h| h.first().is_some_and(|t| t & 0x7f == 4))?; + comments_mask(block.get(4..)?) + }; + file.streams.iter().filter(|s| s.codec == ogg::Codec::Flac).filter_map(|s| Some((s.serial, comments(s)?))).collect() +} + /// The channel mask in a Vorbis comment block's body. fn comments_mask(block: &[u8]) -> Option { let vendor = le_u32(block, 0)? as usize; diff --git a/crates/audio/src/read/symph.rs b/crates/audio/src/read/symph.rs index 8a6f3531..bf78a14e 100644 --- a/crates/audio/src/read/symph.rs +++ b/crates/audio/src/read/symph.rs @@ -16,7 +16,7 @@ use symphonia::core::errors::Error as SymError; use symphonia::core::formats::probe::{Hint, ProbeOptions, ProbeableFormat, Score, Scoreable}; use symphonia::core::formats::{FormatOptions, FormatReader, SeekMode, SeekTo, Track}; use symphonia::core::io::{MediaSourceStream, MediaSourceStreamOptions, ScopedStream}; -use symphonia::core::meta::{MetadataOptions, RawValue}; +use symphonia::core::meta::MetadataOptions; use symphonia::core::units::{TimeBase, Timestamp}; use super::id3v2_len; @@ -81,10 +81,20 @@ fn guarded(f: impl FnOnce() -> Result) -> Result { catch_unwind(AssertUnwindSafe(f)).unwrap_or_else(|_| Err(AudioError::Malformed("decoder failed on malformed input".into()))) } -/// What [`open`] found: a reader (with the layout of a FLAC stream), or a stream that holds no -/// audio frames and has no reader. +/// What symphonia's FLAC and Ogg readers leave out about a file's layout. +enum FlacLayouts { + /// Not a FLAC file or an Ogg file. + Unknown, + /// A FLAC file's layout. + File(ChannelLayout), + /// The channel masks of an Ogg file's FLAC streams, by serial (symphonia's track ID). The tags + /// its reader collects do not say which track they are from. + Ogg(Vec<(u32, u32)>), +} + +/// What [`open`] found: a reader, or a stream that holds no audio frames and has no reader. enum Opened { - Reader(Box, Option), + Reader(Box, FlacLayouts), Empty(AudioInfo), } @@ -364,6 +374,8 @@ fn open(bytes: Cow<'_, [u8]>, detected: Option) -> Result { Precheck::Flac(expected, layout) => (Precheck::Checked(expected), layout), checked => (checked, None), }; + let body = bytes.get(start..).unwrap_or_default(); + let ogg_masks = craft_media::ogg::sniff(body).then(|| super::flac::ogg_channel_masks(body)); // symphonia reads from an owned stream: a borrowed buffer is copied once, a buffer read from // a shared source is moved in. let mut mss = MediaSourceStream::new(Box::new(Cursor::new(bytes.into_owned())), MediaSourceStreamOptions::default()); @@ -385,7 +397,12 @@ fn open(bytes: Cow<'_, [u8]>, detected: Option) -> Result { } _ => {} } - Ok(Opened::Reader(reader, flac_layout.filter(|_| probed == "flac"))) + let layouts = match (flac_layout.filter(|_| probed == "flac"), ogg_masks) { + (Some(layout), _) => FlacLayouts::File(layout), + (None, Some(masks)) => FlacLayouts::Ogg(masks), + _ => FlacLayouts::Unknown, + }; + Ok(Opened::Reader(reader, layouts)) } /// The codec symphonia reports, as a [`CodecId`]. @@ -446,21 +463,6 @@ fn layout_of(channels: Option<&Channels>) -> ChannelLayout { } } -/// The channel mask comment among the tags the reader has read. -fn metadata_channel_mask(reader: &mut dyn FormatReader) -> Option { - let mut metadata = reader.metadata(); - loop { - let mask = metadata.current()?.media.tags.iter().find_map(|t| match &t.raw.value { - RawValue::String(value) => super::flac::comment_mask(t.raw.key.as_bytes(), value.as_bytes()), - _ => None, - }); - if mask.is_some() { - return mask; - } - metadata.pop()?; - } -} - /// The stored sample format, using the codec and its declared bit depth. Lossy and unusual /// codecs decode to `Compressed`. fn sample_format_of(p: &AudioCodecParameters) -> SampleFormat { @@ -562,8 +564,8 @@ impl Symph { /// Open audio track `track` (the first when `None`) of the file in `bytes`. pub(crate) fn open(bytes: Cow<'_, [u8]>, detected: Option, track: Option) -> Result { guarded(move || { - let (mut reader, flac_layout) = match open(bytes, detected)? { - Opened::Reader(reader, layout) => (reader, layout), + let (reader, flac_layouts) = match open(bytes, detected)? { + Opened::Reader(reader, layouts) => (reader, layouts), Opened::Empty(info) => { // a stream with no audio still has one track, as WAV and AIFF do if let Some(n) = track.filter(|&n| n > 0) { @@ -586,10 +588,13 @@ impl Symph { } }; let (track_id, mut info) = info_of(reader.as_ref(), detected, track)?; - if let Some(layout) = flac_layout.filter(|l| l.channels() == info.channels) { - info.layout = layout; - } else if info.codec == CodecId::Flac && info.container == Container::Ogg { - info.layout = super::flac::layout_from(metadata_channel_mask(reader.as_mut()), info.channels); + match flac_layouts { + FlacLayouts::File(layout) if layout.channels() == info.channels => info.layout = layout, + FlacLayouts::Ogg(masks) if info.codec == CodecId::Flac => { + let mask = masks.iter().find(|(serial, _)| *serial == track_id).map(|&(_, mask)| mask); + info.layout = super::flac::layout_from(mask, info.channels); + } + _ => {} } let t = reader.tracks().iter().find(|t| t.id == track_id).ok_or_else(|| AudioError::Malformed("track vanished".into()))?; let params = audio_params(t).cloned().ok_or_else(|| AudioError::Malformed("track vanished".into()))?; diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index 6dd19f17..7b5fc538 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -243,10 +243,12 @@ fn layouts_decoders_would_not_assume_are_recorded() { fn ogg_flac_reads_the_mask_comment() { use crate::ChannelLayout; let Some(ff) = ffmpeg() else { return }; - let cases: [(&str, &str, &[&str], u16, u32); 3] = [ + let cases: [(&str, &str, &[&str], u16, u32); 5] = [ ("ogg-mask", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=48000", &["-metadata", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x0107"], 4, 0x107), ("ogg-5.1", "aevalsrc=0|0|0|0|0|0:c=5.1:d=0.1:s=48000", &[], 6, 0x60F), ("ogg-stereo", "aevalsrc=0|0:c=stereo:d=0.1:s=48000", &["-metadata", "TITLE=x"], 2, 0x3), + ("ogg-6.1", "aevalsrc=0|0|0|0|0|0|0:c=6.1:d=0.1:s=48000", &[], 7, 0x70F), + ("ogg-zero-mask", "aevalsrc=0|0|0|0|0|0:c=5.1:d=0.1:s=48000", &["-metadata", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x0"], 6, 0), ]; for (name, source, extra, channels, mask) in cases { let path = common::fixture_subdir("flac").join(format!("{name}.oga")); @@ -264,6 +266,34 @@ fn ogg_flac_reads_the_mask_comment() { } } +/// Each track of an Ogg file has its own mask comment. The tracks are told apart by their audio, +/// because the track order follows the serial numbers ffmpeg picks at random. +#[cfg(feature = "symphonia")] +#[test] +fn ogg_flac_tracks_read_their_own_mask_comments() { + use crate::{ChannelLayout, DecodeOptions}; + let Some(ff) = ffmpeg() else { return }; + let path = common::fixture_subdir("flac").join("ogg-two-masks.oga"); + let status = Command::new(&ff) + .args(["-v", "error", "-y", "-f", "lavfi", "-i", "aevalsrc=0.5|0.5|0.5|0.5:c=4c:d=0.1:s=48000"]) + .args(["-f", "lavfi", "-i", "aevalsrc=0.25|0.25|0.25|0.25:c=4c:d=0.1:s=48000"]) + .args(["-map", "0", "-map", "1", "-c:a", "flac", "-f", "ogg"]) + .args(["-metadata:s:a:0", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x0107", "-metadata:s:a:1", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x000F"]) + .arg(&path) + .status() + .unwrap(); + assert!(status.success()); + let bytes = std::fs::read(&path).unwrap(); + for track in [0, 1] { + let opts = DecodeOptions::default().with_track(track); + let (info, buf) = crate::decode_with(&bytes, None, &opts).unwrap(); + let mask = if buf.channels[0][0] > 0.4 { 0x107 } else { 0xF }; + let expected = ChannelLayout::from_mask_and_channels(mask, 4); + assert_eq!(info.layout, expected, "decode track {track}"); + assert_eq!(crate::probe_with(&bytes, None, &opts).unwrap().layout, expected, "probe track {track}"); + } +} + #[test] fn the_mask_comment_is_written_only_when_decoders_would_assume_otherwise() { use super::mask_comment; From a4f02a385436f6390e344fb9d08f8ff7dc235fcd Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 15:32:10 +0100 Subject: [PATCH 16/20] craft-audio: review fixes, round 15 (#442) A track index in an Ogg file means the same stream everywhere: the file's order, as `tracks` lists them. Streams the native path does not read fell back to symphonia with the same index, but symphonia orders tracks by serial, which ffmpeg picks at random. A file could decode the wrong track, or fail on a FLAC stream listed before an Opus one. The tests write each file eight times, so both serial orders occur, and cover a track with no mask comment next to one with a mask. Co-Authored-By: Claude Opus 5.5 --- crates/audio/src/read/flac.rs | 9 +-- crates/audio/src/read/symph.rs | 62 +++++++++++-------- crates/audio/src/write/flac/oracle_tests.rs | 68 ++++++++++++++------- 3 files changed, 90 insertions(+), 49 deletions(-) diff --git a/crates/audio/src/read/flac.rs b/crates/audio/src/read/flac.rs index 1dc4d0f3..a16e7bdf 100644 --- a/crates/audio/src/read/flac.rs +++ b/crates/audio/src/read/flac.rs @@ -124,15 +124,16 @@ fn comment_mask(key: &[u8], value: &[u8]) -> Option { u32::from_str_radix(std::str::from_utf8(hex).ok()?, 16).ok() } -/// The channel mask comment of each FLAC stream that has one in the Ogg file `b`, by serial. -pub(crate) fn ogg_channel_masks(b: &[u8]) -> Vec<(u32, u32)> { +/// The audio streams of the Ogg file `b` in file order, as `tracks` lists them: each one's serial +/// and, for FLAC, its channel mask comment. Empty when the demuxer cannot read the file. +pub(crate) fn ogg_audio_streams(b: &[u8]) -> Vec<(u32, Option)> { let Ok(file) = ogg::open(b) else { return Vec::new() }; - let comments = |s: &ogg::Stream| { + let mask = |s: &ogg::Stream| { // After the first packet, each header packet is a metadata block with its 4-byte header. let block = s.headers.iter().skip(1).find(|h| h.first().is_some_and(|t| t & 0x7f == 4))?; comments_mask(block.get(4..)?) }; - file.streams.iter().filter(|s| s.codec == ogg::Codec::Flac).filter_map(|s| Some((s.serial, comments(s)?))).collect() + file.streams.iter().filter(|s| s.codec.codec_id().is_audio()).map(|s| (s.serial, mask(s).filter(|_| s.codec == ogg::Codec::Flac))).collect() } /// The channel mask in a Vorbis comment block's body. diff --git a/crates/audio/src/read/symph.rs b/crates/audio/src/read/symph.rs index bf78a14e..126bd5c8 100644 --- a/crates/audio/src/read/symph.rs +++ b/crates/audio/src/read/symph.rs @@ -81,20 +81,21 @@ fn guarded(f: impl FnOnce() -> Result) -> Result { catch_unwind(AssertUnwindSafe(f)).unwrap_or_else(|_| Err(AudioError::Malformed("decoder failed on malformed input".into()))) } -/// What symphonia's FLAC and Ogg readers leave out about a file's layout. -enum FlacLayouts { +/// What [`open`] read from the file for what symphonia's FLAC and Ogg readers leave out. +enum Preread { /// Not a FLAC file or an Ogg file. - Unknown, + Nothing, /// A FLAC file's layout. - File(ChannelLayout), - /// The channel masks of an Ogg file's FLAC streams, by serial (symphonia's track ID). The tags - /// its reader collects do not say which track they are from. - Ogg(Vec<(u32, u32)>), + Flac(ChannelLayout), + /// An Ogg file's audio streams in file order: each one's serial, which is symphonia's track ID, + /// and its FLAC channel mask. Symphonia orders the tracks by serial, and the tags its reader + /// collects do not say which track they are from. + Ogg(Vec<(u32, Option)>), } /// What [`open`] found: a reader, or a stream that holds no audio frames and has no reader. enum Opened { - Reader(Box, FlacLayouts), + Reader(Box, Preread), Empty(AudioInfo), } @@ -375,7 +376,7 @@ fn open(bytes: Cow<'_, [u8]>, detected: Option) -> Result { checked => (checked, None), }; let body = bytes.get(start..).unwrap_or_default(); - let ogg_masks = craft_media::ogg::sniff(body).then(|| super::flac::ogg_channel_masks(body)); + let ogg_streams = craft_media::ogg::sniff(body).then(|| super::flac::ogg_audio_streams(body)); // symphonia reads from an owned stream: a borrowed buffer is copied once, a buffer read from // a shared source is moved in. let mut mss = MediaSourceStream::new(Box::new(Cursor::new(bytes.into_owned())), MediaSourceStreamOptions::default()); @@ -397,12 +398,12 @@ fn open(bytes: Cow<'_, [u8]>, detected: Option) -> Result { } _ => {} } - let layouts = match (flac_layout.filter(|_| probed == "flac"), ogg_masks) { - (Some(layout), _) => FlacLayouts::File(layout), - (None, Some(masks)) => FlacLayouts::Ogg(masks), - _ => FlacLayouts::Unknown, + let preread = match (flac_layout.filter(|_| probed == "flac"), ogg_streams.filter(|_| probed == "ogg")) { + (Some(layout), _) => Preread::Flac(layout), + (None, Some(streams)) => Preread::Ogg(streams), + _ => Preread::Nothing, }; - Ok(Opened::Reader(reader, layouts)) + Ok(Opened::Reader(reader, preread)) } /// The codec symphonia reports, as a [`CodecId`]. @@ -509,10 +510,22 @@ fn audio_tracks(reader: &dyn FormatReader) -> impl Iterator { reader.tracks().iter().filter(|t| audio_params(t).is_some_and(|p| p.codec != CODEC_ID_NULL_AUDIO)) } -/// The `n`-th audio track (the first when `None`), with the stream information the UI shows. -fn info_of(reader: &dyn FormatReader, detected: Option, n: Option) -> Result<(u32, AudioInfo)> { +/// The `n`-th audio track (the first when `None`), counting an Ogg file's `streams` in file order +/// when they are known. With no `n`, the first stream symphonia has a track for: it has none for +/// some of the codecs `tracks` lists. +fn pick_track<'a>(reader: &'a dyn FormatReader, streams: &[(u32, Option)], n: Option) -> Option<&'a Track> { + let with_serial = |serial: u32| audio_tracks(reader).find(|t| t.id == serial); + match n { + _ if streams.is_empty() => audio_tracks(reader).nth(n.unwrap_or(0)), + Some(n) => with_serial(streams.get(n)?.0), + None => streams.iter().find_map(|s| with_serial(s.0)), + } +} + +/// The `n`-th audio track (see [`pick_track`]), with the stream information the UI shows. +fn info_of(reader: &dyn FormatReader, detected: Option, ogg_streams: &[(u32, Option)], n: Option) -> Result<(u32, AudioInfo)> { let container = container_of(reader.format_info().short_name, detected); - let track = audio_tracks(reader).nth(n.unwrap_or(0)).ok_or_else(|| AudioError::Unsupported("no decodable audio track".into()))?; + let track = pick_track(reader, ogg_streams, n).ok_or_else(|| AudioError::Unsupported("no decodable audio track".into()))?; let p = audio_params(track).ok_or_else(|| AudioError::Unsupported("no decodable audio track".into()))?; let sample_rate = p.sample_rate.filter(|&r| r > 0).ok_or_else(|| AudioError::Malformed("unknown sample rate".into()))?; let layout = layout_of(p.channels.as_ref()); @@ -564,8 +577,8 @@ impl Symph { /// Open audio track `track` (the first when `None`) of the file in `bytes`. pub(crate) fn open(bytes: Cow<'_, [u8]>, detected: Option, track: Option) -> Result { guarded(move || { - let (reader, flac_layouts) = match open(bytes, detected)? { - Opened::Reader(reader, layouts) => (reader, layouts), + let (reader, preread) = match open(bytes, detected)? { + Opened::Reader(reader, preread) => (reader, preread), Opened::Empty(info) => { // a stream with no audio still has one track, as WAV and AIFF do if let Some(n) = track.filter(|&n| n > 0) { @@ -587,11 +600,12 @@ impl Symph { }); } }; - let (track_id, mut info) = info_of(reader.as_ref(), detected, track)?; - match flac_layouts { - FlacLayouts::File(layout) if layout.channels() == info.channels => info.layout = layout, - FlacLayouts::Ogg(masks) if info.codec == CodecId::Flac => { - let mask = masks.iter().find(|(serial, _)| *serial == track_id).map(|&(_, mask)| mask); + let ogg_streams = if let Preread::Ogg(streams) = &preread { streams.as_slice() } else { &[] }; + let (track_id, mut info) = info_of(reader.as_ref(), detected, ogg_streams, track)?; + match &preread { + Preread::Flac(layout) if layout.channels() == info.channels => info.layout = *layout, + Preread::Ogg(streams) if info.codec == CodecId::Flac => { + let mask = streams.iter().find(|(serial, _)| *serial == track_id).and_then(|&(_, mask)| mask); info.layout = super::flac::layout_from(mask, info.channels); } _ => {} diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index 7b5fc538..75d02d63 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -266,31 +266,57 @@ fn ogg_flac_reads_the_mask_comment() { } } -/// Each track of an Ogg file has its own mask comment. The tracks are told apart by their audio, -/// because the track order follows the serial numbers ffmpeg picks at random. +/// Two streams in an Ogg file written by ffmpeg, which picks their serial numbers at random. The +/// tests below write each file a few times, so that both serial orders occur. #[cfg(feature = "symphonia")] -#[test] -fn ogg_flac_tracks_read_their_own_mask_comments() { - use crate::{ChannelLayout, DecodeOptions}; - let Some(ff) = ffmpeg() else { return }; - let path = common::fixture_subdir("flac").join("ogg-two-masks.oga"); - let status = Command::new(&ff) - .args(["-v", "error", "-y", "-f", "lavfi", "-i", "aevalsrc=0.5|0.5|0.5|0.5:c=4c:d=0.1:s=48000"]) - .args(["-f", "lavfi", "-i", "aevalsrc=0.25|0.25|0.25|0.25:c=4c:d=0.1:s=48000"]) - .args(["-map", "0", "-map", "1", "-c:a", "flac", "-f", "ogg"]) - .args(["-metadata:s:a:0", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x0107", "-metadata:s:a:1", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x000F"]) +fn two_ogg_streams(ff: &Path, name: &str, args: &[&str]) -> Option> { + let path = common::fixture_subdir("flac").join(format!("{name}.ogg")); + let status = Command::new(ff) + .args(["-v", "error", "-y", "-f", "lavfi", "-i", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=48000"]) + .args(["-f", "lavfi", "-i", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=44100"]) + .args(["-map", "0", "-map", "1"]) + .args(args) + .args(["-f", "ogg"]) .arg(&path) .status() .unwrap(); - assert!(status.success()); - let bytes = std::fs::read(&path).unwrap(); - for track in [0, 1] { - let opts = DecodeOptions::default().with_track(track); - let (info, buf) = crate::decode_with(&bytes, None, &opts).unwrap(); - let mask = if buf.channels[0][0] > 0.4 { 0x107 } else { 0xF }; - let expected = ChannelLayout::from_mask_and_channels(mask, 4); - assert_eq!(info.layout, expected, "decode track {track}"); - assert_eq!(crate::probe_with(&bytes, None, &opts).unwrap().layout, expected, "probe track {track}"); + status.success().then(|| std::fs::read(&path).unwrap()) +} + +/// A track index means the same stream in `tracks`, `probe` and `decode`: the file's order. Each +/// track has its own mask comment, or else the layout decoders assume for the count. +#[cfg(feature = "symphonia")] +#[test] +fn ogg_flac_tracks_are_in_file_order_with_their_own_mask_comments() { + use crate::{ChannelLayout, DecodeOptions}; + let Some(ff) = ffmpeg() else { return }; + let args = ["-c:a", "flac", "-metadata:s:a:0", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x0107"]; + for _ in 0..8 { + let bytes = two_ogg_streams(&ff, "ogg-one-mask", &args).unwrap(); + let rates: Vec<_> = crate::tracks(&bytes, None).unwrap().iter().map(|t| t.sample_rate).collect(); + assert_eq!(rates, [Some(48_000), Some(44_100)]); + for (track, rate, layout) in [(0, 48_000, ChannelLayout::from_mask(0x107)), (1, 44_100, ChannelLayout::default_for(4))] { + let opts = DecodeOptions::default().with_track(track); + let info = crate::probe_with(&bytes, None, &opts).unwrap(); + assert_eq!((info.sample_rate, info.layout), (rate, layout), "probe track {track}"); + let info = crate::decode_with(&bytes, None, &opts).unwrap().0; + assert_eq!((info.sample_rate, info.layout), (rate, layout), "decode track {track}"); + } + assert_eq!(crate::decode(&bytes, None).unwrap().0.sample_rate, 48_000, "the first track"); + } +} + +/// A FLAC stream before a stream symphonia cannot decode is still the first track. +#[cfg(feature = "symphonia")] +#[test] +fn ogg_flac_before_opus_is_the_first_track() { + use crate::CodecId; + let Some(ff) = ffmpeg() else { return }; + let args = ["-c:a:0", "flac", "-c:a:1", "libopus", "-ac:a:1", "2"]; + for _ in 0..8 { + let Some(bytes) = two_ogg_streams(&ff, "ogg-flac-opus", &args) else { return }; + assert_eq!(crate::tracks(&bytes, None).unwrap()[0].codec, CodecId::Flac); + assert_eq!(crate::decode(&bytes, None).unwrap().0.codec, CodecId::Flac); } } From e8b7c40ea0f7f03cb025744769ae4d7f795d16c5 Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 19:45:11 +0100 Subject: [PATCH 17/20] craft-audio: review fixes, round 16 (#442) The Ogg track tests no longer depend on chance. Each writes its file until the serial numbers are in the order it asks for, in both orders, so the reversed order that told the old code apart always runs. A file this ffmpeg cannot write goes through `skip`, which fails the test when oracles are required. Co-Authored-By: Claude Opus 5.5 --- crates/audio/src/write/flac/oracle_tests.rs | 48 +++++++++++++-------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index 75d02d63..13cb82dc 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -266,21 +266,35 @@ fn ogg_flac_reads_the_mask_comment() { } } -/// Two streams in an Ogg file written by ffmpeg, which picks their serial numbers at random. The -/// tests below write each file a few times, so that both serial orders occur. +/// Two streams in an Ogg file written by ffmpeg, whose serial numbers are in file order or in +/// `reversed` order. ffmpeg picks them at random, so this writes the file until the order occurs; +/// symphonia orders tracks by serial, so the reversed order is the one that tells them apart. +/// `None` after `skip` when this ffmpeg cannot write the file. #[cfg(feature = "symphonia")] -fn two_ogg_streams(ff: &Path, name: &str, args: &[&str]) -> Option> { +fn two_ogg_streams(ff: &Path, name: &str, args: &[&str], reversed: bool) -> Option> { let path = common::fixture_subdir("flac").join(format!("{name}.ogg")); - let status = Command::new(ff) - .args(["-v", "error", "-y", "-f", "lavfi", "-i", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=48000"]) - .args(["-f", "lavfi", "-i", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=44100"]) - .args(["-map", "0", "-map", "1"]) - .args(args) - .args(["-f", "ogg"]) - .arg(&path) - .status() - .unwrap(); - status.success().then(|| std::fs::read(&path).unwrap()) + for attempt in 0..64 { + let status = Command::new(ff) + .args(["-v", "error", "-y", "-f", "lavfi", "-i", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=48000"]) + .args(["-f", "lavfi", "-i", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=44100"]) + .args(["-map", "0", "-map", "1"]) + .args(args) + .args(["-f", "ogg"]) + .arg(&path) + .status() + .unwrap(); + if !status.success() { + return common::skip(&format!("this ffmpeg cannot write {name}")); + } + let bytes = std::fs::read(&path).unwrap(); + let file = craft_media::ogg::open(bytes.as_slice()).unwrap(); + assert_eq!(file.streams.len(), 2, "{name}"); + if (file.streams[0].serial > file.streams[1].serial) == reversed { + return Some(bytes); + } + assert!(attempt < 63, "{name}: 64 files with their serial numbers in one order"); + } + None } /// A track index means the same stream in `tracks`, `probe` and `decode`: the file's order. Each @@ -291,8 +305,8 @@ fn ogg_flac_tracks_are_in_file_order_with_their_own_mask_comments() { use crate::{ChannelLayout, DecodeOptions}; let Some(ff) = ffmpeg() else { return }; let args = ["-c:a", "flac", "-metadata:s:a:0", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x0107"]; - for _ in 0..8 { - let bytes = two_ogg_streams(&ff, "ogg-one-mask", &args).unwrap(); + for reversed in [false, true] { + let Some(bytes) = two_ogg_streams(&ff, "ogg-one-mask", &args, reversed) else { return }; let rates: Vec<_> = crate::tracks(&bytes, None).unwrap().iter().map(|t| t.sample_rate).collect(); assert_eq!(rates, [Some(48_000), Some(44_100)]); for (track, rate, layout) in [(0, 48_000, ChannelLayout::from_mask(0x107)), (1, 44_100, ChannelLayout::default_for(4))] { @@ -313,8 +327,8 @@ fn ogg_flac_before_opus_is_the_first_track() { use crate::CodecId; let Some(ff) = ffmpeg() else { return }; let args = ["-c:a:0", "flac", "-c:a:1", "libopus", "-ac:a:1", "2"]; - for _ in 0..8 { - let Some(bytes) = two_ogg_streams(&ff, "ogg-flac-opus", &args) else { return }; + for reversed in [false, true] { + let Some(bytes) = two_ogg_streams(&ff, "ogg-flac-opus", &args, reversed) else { return }; assert_eq!(crate::tracks(&bytes, None).unwrap()[0].codec, CodecId::Flac); assert_eq!(crate::decode(&bytes, None).unwrap().0.codec, CodecId::Flac); } From 04df866591d0b4dc694b962ea3ab8ee22442a76d Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 21:06:42 +0100 Subject: [PATCH 18/20] craft-audio: review fixes, round 17 (#442) The Ogg track tests set their files' serial numbers instead of writing the files until ffmpeg happens to pick the wanted order. They rewrite each page's serial and checksum, so both orders run with one ffmpeg call and do not depend on ffmpeg's serials being random. Co-Authored-By: Claude Opus 5.5 --- crates/audio/src/write/flac/oracle_tests.rs | 73 ++++++++++++++------- 1 file changed, 48 insertions(+), 25 deletions(-) diff --git a/crates/audio/src/write/flac/oracle_tests.rs b/crates/audio/src/write/flac/oracle_tests.rs index 13cb82dc..a9f80ece 100644 --- a/crates/audio/src/write/flac/oracle_tests.rs +++ b/crates/audio/src/write/flac/oracle_tests.rs @@ -266,35 +266,58 @@ fn ogg_flac_reads_the_mask_comment() { } } -/// Two streams in an Ogg file written by ffmpeg, whose serial numbers are in file order or in -/// `reversed` order. ffmpeg picks them at random, so this writes the file until the order occurs; -/// symphonia orders tracks by serial, so the reversed order is the one that tells them apart. -/// `None` after `skip` when this ffmpeg cannot write the file. +/// `file`, an Ogg file of two streams, with its streams' serial numbers replaced by `serials`, in +/// the order the streams start. +#[cfg(feature = "symphonia")] +fn set_ogg_serials(mut file: Vec, serials: [u32; 2]) -> Vec { + let mut seen = Vec::new(); + let mut at = 0; + while at < file.len() { + assert_eq!(&file[at..at + 4], b"OggS"); + let segments = usize::from(file[at + 26]); + let body: usize = file[at + 27..at + 27 + segments].iter().map(|&n| usize::from(n)).sum(); + let end = at + 27 + segments + body; + let old: [u8; 4] = file[at + 14..at + 18].try_into().unwrap(); + let stream = seen.iter().position(|s| *s == old).unwrap_or_else(|| { + seen.push(old); + seen.len() - 1 + }); + file[at + 14..at + 18].copy_from_slice(&serials[stream].to_le_bytes()); + // The page's checksum covers its serial number, and is computed with its own field zero. + file[at + 22..at + 26].fill(0); + let crc = craft_media::ogg::crc32(&file[at..end]); + file[at + 22..at + 26].copy_from_slice(&crc.to_le_bytes()); + at = end; + } + assert_eq!(seen.len(), 2); + file +} + +/// Two streams in an Ogg file written by ffmpeg, with serial numbers in file order or in `reversed` +/// order. ffmpeg picks them at random, and symphonia orders tracks by serial, so the reversed order +/// is the one that tells the two orders apart. `None` after `skip` when this ffmpeg cannot write the +/// file. #[cfg(feature = "symphonia")] fn two_ogg_streams(ff: &Path, name: &str, args: &[&str], reversed: bool) -> Option> { let path = common::fixture_subdir("flac").join(format!("{name}.ogg")); - for attempt in 0..64 { - let status = Command::new(ff) - .args(["-v", "error", "-y", "-f", "lavfi", "-i", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=48000"]) - .args(["-f", "lavfi", "-i", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=44100"]) - .args(["-map", "0", "-map", "1"]) - .args(args) - .args(["-f", "ogg"]) - .arg(&path) - .status() - .unwrap(); - if !status.success() { - return common::skip(&format!("this ffmpeg cannot write {name}")); - } - let bytes = std::fs::read(&path).unwrap(); - let file = craft_media::ogg::open(bytes.as_slice()).unwrap(); - assert_eq!(file.streams.len(), 2, "{name}"); - if (file.streams[0].serial > file.streams[1].serial) == reversed { - return Some(bytes); - } - assert!(attempt < 63, "{name}: 64 files with their serial numbers in one order"); + let status = Command::new(ff) + .args(["-v", "error", "-y", "-f", "lavfi", "-i", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=48000"]) + .args(["-f", "lavfi", "-i", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=44100"]) + .args(["-map", "0", "-map", "1"]) + .args(args) + .args(["-f", "ogg"]) + .arg(&path) + .status() + .unwrap(); + if !status.success() { + return common::skip(&format!("this ffmpeg cannot write {name}")); } - None + let serials = if reversed { [2, 1] } else { [1, 2] }; + let bytes = set_ogg_serials(std::fs::read(&path).unwrap(), serials); + let file = craft_media::ogg::open(bytes.as_slice()).unwrap(); + assert_eq!(file.streams.iter().map(|s| s.serial).collect::>(), serials, "{name}"); + assert!(file.warnings.is_empty(), "{name}: {:?}", file.warnings); + Some(bytes) } /// A track index means the same stream in `tracks`, `probe` and `decode`: the file's order. Each From d1d2564fd3c0b79fc2ed29e6a76624aa708199ed Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 21:52:03 +0100 Subject: [PATCH 19/20] craft-audio: review fixes, round 18 (#442) CI: time-limit and retry the ffmpeg install. A stalled apt mirror used up the oracle job's 20 minutes on #442, #443 and #447 before any test ran, and its log is only printed when the step fails. Each try now gets 3 minutes, up to three tries, and a timed-out install prints why. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/audio.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/audio.yml b/.github/workflows/audio.yml index f2d7d49e..fd7ec4a5 100644 --- a/.github/workflows/audio.yml +++ b/.github/workflows/audio.yml @@ -102,7 +102,16 @@ jobs: run: | # The man-db index rebuild after dpkg is slow on hosted runners. sudo rm -f /var/lib/man-db/auto-update - (sudo apt-get update -qq && sudo apt-get install -y -qq --no-install-recommends ffmpeg) > "$RUNNER_TEMP/apt.log" 2>&1 & + # A mirror can stall for most of the job's time limit, and the next try usually gets a good one. + install_ffmpeg() { + for attempt in 1 2 3; do + timeout -k 10 180 sudo sh -c 'apt-get update -qq && apt-get install -y -qq --no-install-recommends ffmpeg' && return 0 + echo "apt: attempt $attempt failed with exit code $?" + sudo dpkg --configure -a || true + done + return 1 + } + install_ffmpeg > "$RUNNER_TEMP/apt.log" 2>&1 & apt=$! cargo test -p craft-audio --all-features --locked --no-run wait "$apt" || { cat "$RUNNER_TEMP/apt.log"; exit 1; } From f31c932385d3e215fb7ec045e4113700efcb15e3 Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 4 Nov 2026 22:02:29 +0100 Subject: [PATCH 20/20] craft-audio: review fixes, round 19 (#442) CI: the package repair between ffmpeg install tries gets a time limit too, so a blocked configure script cannot use up the job's time. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/audio.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/audio.yml b/.github/workflows/audio.yml index fd7ec4a5..21692c77 100644 --- a/.github/workflows/audio.yml +++ b/.github/workflows/audio.yml @@ -107,7 +107,7 @@ jobs: for attempt in 1 2 3; do timeout -k 10 180 sudo sh -c 'apt-get update -qq && apt-get install -y -qq --no-install-recommends ffmpeg' && return 0 echo "apt: attempt $attempt failed with exit code $?" - sudo dpkg --configure -a || true + timeout -k 10 60 sudo dpkg --configure -a || true done return 1 }