From 3dadce4d7c8840386acd9e1a455c56034b4daecd Mon Sep 17 00:00:00 2001 From: alehel Date: Sat, 14 Nov 2026 02:12:54 +0100 Subject: [PATCH] craft-audio: review round 1 for the coded formats - craft-media: TrackConfig::roll_distance writes a `roll` sample group (sgpd + sbgp) over every sample. The AAC M4A track sets -1: Apple's decoders took 2112 frames of priming and dropped the start of the audio without it (afinfo now reports the frames written and 1024 priming). - EncodeFormat::is_built and feature: which formats this build writes. - Clippy: the writer enum's variants differ in size with flac-encode and a codec feature but no pcm-encode. - Tests for the finished sample entry (AAC bit rate, ALAC cookie), ALAC dither and afinfo's reading of the priming. - README: ffmpeg's reading of 8-channel AAC, and files with no audio. Refs #1230 --- crates/audio/README.md | 9 +++- crates/audio/src/lib.rs | 29 ++++++++++++- crates/audio/src/write/coded.rs | 3 ++ crates/audio/src/write/mod.rs | 13 ++---- crates/audio/tests/codecs/file_formats.rs | 51 ++++++++++++++++++++++- crates/media/src/isobmff/mux.rs | 51 ++++++++++++++++++++++- 6 files changed, 141 insertions(+), 15 deletions(-) diff --git a/crates/audio/README.md b/crates/audio/README.md index 79f0efd46..5be0a3b23 100644 --- a/crates/audio/README.md +++ b/crates/audio/README.md @@ -240,8 +240,8 @@ let alac = encode(&buf, &EncodeOptions::new(EncodeFormat::Alac).with_bit_depth(B `EncodeOptions::default()` is 24-bit WAV. `with_bwf` adds a BWF `bext` chunk and `with_force_rf64` writes RF64 below 4 GiB; both apply to WAV only. `encode_format_for("aiff")` maps an extension to an `EncodeFormat`, and `ENCODE_EXTENSIONS` lists the extensions the enabled -features can write. `EncodeFormat::ALL` lists the formats; `extension()` is the extension a new -file gets and `mime_type()` its media type. `write` refuses a block without one slice per channel, or with a slice +features can write. `EncodeFormat::ALL` lists the formats and `is_built()` says whether this build +writes one; `extension()` is the extension a new file gets and `mime_type()` its media type. `write` refuses a block without one slice per channel, or with a slice shorter than `frames`, and writes nothing. After a write fails for another reason, every later `write` and `finish` returns an error. @@ -462,6 +462,11 @@ match craft_audio::decode(&bytes, None) { metadata. Raw ADTS AAC is not a file format of `encode`; `aac::EncoderConfig::with_adts` gives the frames. - This crate's file reader plays a raw E-AC-3 7.1 stream as its 5.1 independent substream. +- `EncodeFormat::Aac` with 8 channels is AAC channel configuration 7 (C, Lc Rc, L R, Ls Rs, LFE), + which Apple's decoders read as written. ffmpeg assumes such a stream is a mislabelled 7.1 with + backs and sides and moves the channels accordingly, unless run with `-strict 1`. +- A file with no audio: MP3, AC-3 and E-AC-3 are then zero bytes, which nothing can probe, and an + empty AAC M4A (a track with no samples) is read by ffmpeg but not by this crate's decoder. - ALAC 32-bit streams: ffmpeg does not decode their uncompressed (escaped) stereo frames, which Apple's encoder and this one write for audio that does not compress; Apple's decoder and this one do. diff --git a/crates/audio/src/lib.rs b/crates/audio/src/lib.rs index f6be431dc..326426b2a 100644 --- a/crates/audio/src/lib.rs +++ b/crates/audio/src/lib.rs @@ -260,7 +260,7 @@ pub enum EncodeFormat { impl EncodeFormat { /// Every format, in the order menus list them. Whether [`encode`] writes one depends on its - /// feature. + /// feature ([`EncodeFormat::is_built`]). pub const ALL: &[EncodeFormat] = &[ EncodeFormat::Wav, EncodeFormat::Aiff, @@ -325,6 +325,33 @@ impl EncodeFormat { } } + /// The feature that builds this format's writer. + pub fn feature(self) -> &'static str { + match self { + EncodeFormat::Wav | EncodeFormat::Aiff => "pcm-encode", + EncodeFormat::Flac => "flac-encode", + EncodeFormat::Aac => "aac-encode", + EncodeFormat::Alac => "alac-encode", + EncodeFormat::Mp3 => "mp3-encode", + EncodeFormat::Ac3 => "ac3-encode", + EncodeFormat::Eac3 => "eac3-encode", + } + } + + /// Whether [`encode`] and [`FileEncoder`] write this format in this build (its + /// [`feature`](Self::feature) is on): what to filter [`EncodeFormat::ALL`] by for a menu. + pub fn is_built(self) -> bool { + match self { + EncodeFormat::Wav | EncodeFormat::Aiff => cfg!(feature = "pcm-encode"), + EncodeFormat::Flac => cfg!(feature = "flac-encode"), + EncodeFormat::Aac => cfg!(feature = "aac-encode"), + EncodeFormat::Alac => cfg!(feature = "alac-encode"), + EncodeFormat::Mp3 => cfg!(feature = "mp3-encode"), + EncodeFormat::Ac3 => cfg!(feature = "ac3-encode"), + EncodeFormat::Eac3 => cfg!(feature = "eac3-encode"), + } + } + /// Whether the format loses audio: AAC, MP3, AC-3 and E-AC-3. These take /// [`EncodeOptions::rate_control`] and ignore the bit depth and dither. pub fn is_lossy(self) -> bool { diff --git a/crates/audio/src/write/coded.rs b/crates/audio/src/write/coded.rs index 415d309fc..84507bb44 100644 --- a/crates/audio/src/write/coded.rs +++ b/crates/audio/src/write/coded.rs @@ -408,6 +408,9 @@ impl M4a { let mut config = craft_media::isobmff::TrackConfig::new(self.entry.clone(), self.sample_rate); if has_audio && self.priming > 0 { config.media_start = Some(i64::from(self.priming)); + // AAC's overlap: a packet decodes right only after the one before it. Apple's + // decoders take the priming from the edit only when the track says so. + config.roll_distance = Some(-1); } let track = self.mux.add_track(config).map_err(mux_error)?; self.track = Some(track); diff --git a/crates/audio/src/write/mod.rs b/crates/audio/src/write/mod.rs index c743965cb..d7dcbd5f2 100644 --- a/crates/audio/src/write/mod.rs +++ b/crates/audio/src/write/mod.rs @@ -57,16 +57,7 @@ pub fn encode(buf: &AudioBuffer, opts: &EncodeOptions) -> Result> { /// 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) = match format { - EncodeFormat::Wav | EncodeFormat::Aiff => (cfg!(feature = "pcm-encode"), "pcm-encode"), - EncodeFormat::Flac => (cfg!(feature = "flac-encode"), "flac-encode"), - EncodeFormat::Aac => (cfg!(feature = "aac-encode"), "aac-encode"), - EncodeFormat::Alac => (cfg!(feature = "alac-encode"), "alac-encode"), - EncodeFormat::Mp3 => (cfg!(feature = "mp3-encode"), "mp3-encode"), - EncodeFormat::Ac3 => (cfg!(feature = "ac3-encode"), "ac3-encode"), - EncodeFormat::Eac3 => (cfg!(feature = "eac3-encode"), "eac3-encode"), - }; - if built { Ok(()) } else { Err(AudioError::Unsupported(format!("writing {format:?} needs the `{feature}` feature"))) } + if format.is_built() { Ok(()) } else { Err(AudioError::Unsupported(format!("writing {format:?} needs the `{}` feature", format.feature()))) } } /// Every extension [`ENCODE_EXTENSIONS`] can list, and how many of them the features built give. @@ -182,6 +173,8 @@ impl std::fmt::Debug for FileEncoder { } } +// The writers differ in size with the features built; the coded one is boxed for its encoder. +#[allow(clippy::large_enum_variant)] enum Writer { #[cfg(feature = "pcm-encode")] Wav(wav::Writer), diff --git a/crates/audio/tests/codecs/file_formats.rs b/crates/audio/tests/codecs/file_formats.rs index 9affab350..49caea243 100644 --- a/crates/audio/tests/codecs/file_formats.rs +++ b/crates/audio/tests/codecs/file_formats.rs @@ -62,6 +62,41 @@ fn aac_timing_is_in_the_sample_table_and_the_edit_list() { assert_eq!(track.samples.iter().map(|s| u64::from(s.duration)).sum::(), 1024 + 3000); assert_eq!(track.edits.len(), 1); assert_eq!((track.edits[0].media_time, track.edits[0].segment_duration), (1024, 3000)); + // Every packet is in a `roll` group of distance -1, which Apple's decoders need to apply the edit. + let groups = &track.sample_groups; + assert_eq!((groups.len(), groups[0].grouping_type), (1, isobmff::FourCc(*b"roll"))); + assert_eq!((groups[0].runs.as_slice(), &groups[0].descriptions), ([(track.samples.len() as u32, 1)].as_slice(), &vec![vec![0xFF, 0xFF]])); +} + +/// The sample entry is completed when the file is: AAC's average bit rate, and the largest packet +/// and average bit rate in ALAC's cookie. +#[test] +fn the_sample_entry_describes_the_finished_stream() { + use craft_audio::craft_media::isobmff::{self, CodecConfig}; + let buf = tones(48_000, 2, 48_000); + let file = encode(&buf, &EncodeOptions::new(EncodeFormat::Aac).with_bitrate(96_000)).unwrap(); + let mp4 = isobmff::open(file.as_slice()).unwrap(); + let CodecConfig::Aac(aac) = &mp4.tracks[0].entries[0].codec else { panic!("not AAC") }; + assert!((90_000..100_000).contains(&aac.avg_bitrate), "{} bit/s", aac.avg_bitrate); + + let file = encode(&buf, &EncodeOptions::new(EncodeFormat::Alac).with_bit_depth(BitDepth::Int16)).unwrap(); + let mp4 = isobmff::open(file.as_slice()).unwrap(); + let CodecConfig::Alac { cookie } = &mp4.tracks[0].entries[0].codec else { panic!("not ALAC") }; + let config = craft_audio::alac::SpecificConfig::parse(cookie).unwrap(); + let largest = mp4.tracks[0].samples.iter().map(|s| s.size).max().unwrap(); + assert_eq!((config.max_frame_bytes, config.bit_depth, config.frame_length), (largest, 16, 4096)); + assert!(config.avg_bit_rate > 0); +} + +/// Dither reaches ALAC's quantizer: a level below one 16-bit step is silence without it. +#[test] +fn alac_dithers_when_asked() { + let buf = AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.3 / 32768.0; 4096]] }; + let opts = EncodeOptions::new(EncodeFormat::Alac).with_bit_depth(BitDepth::Int16); + let (_, plain) = decode(&encode(&buf, &opts).unwrap(), Some("m4a")).unwrap(); + let (_, dithered) = decode(&encode(&buf, &opts.with_dither(true)).unwrap(), Some("m4a")).unwrap(); + assert!(plain.channels[0].iter().all(|&s| s == 0.0)); + assert!(dithered.channels[0].iter().any(|&s| s != 0.0)); } /// ALAC returns the quantized samples exactly, for every channel count, with each channel where @@ -356,7 +391,7 @@ fn the_formats_are_named() { } assert_eq!(ENCODE_EXTENSIONS.len(), 12); for format in CODED { - assert!(EncodeFormat::ALL.contains(&format)); + assert!(EncodeFormat::ALL.contains(&format) && format.is_built() && format.feature().ends_with("-encode")); } } @@ -418,3 +453,17 @@ fn ffmpeg_reads_the_files() { assert!(snr(&buf.channels[c], &out[c][256..]) > 12.0, "E-AC-3 7.1, channel {c}: {} dB", snr(&buf.channels[c], &out[c][256..])); } } + +/// Apple's decoder (macOS `afinfo`; skipped elsewhere) takes the priming and the length of an AAC +/// file from the edit list, which it does only for a track with a `roll` sample group. +#[test] +fn apple_reads_the_aac_priming_and_length() { + let Some(afinfo) = crate::testkit::oracle::find_tool("afinfo") else { return }; + for (frames, remainder) in [(20_000, 480), (1, 1023), (4096, 0)] { + let path = crate::testkit::fixtures_dir("file_formats").join(format!("apple_{frames}.m4a")); + std::fs::write(&path, encode(&tones(48_000, 2, frames), &EncodeOptions::new(EncodeFormat::Aac)).unwrap()).unwrap(); + let out = Command::new(&afinfo).arg(&path).output().unwrap(); + let text = String::from_utf8_lossy(&out.stdout); + assert!(text.contains(&format!("{frames} valid frames + 1024 priming + {remainder} remainder")), "{frames} frames: {text}"); + } +} diff --git a/crates/media/src/isobmff/mux.rs b/crates/media/src/isobmff/mux.rs index e38186e9f..730675db1 100644 --- a/crates/media/src/isobmff/mux.rs +++ b/crates/media/src/isobmff/mux.rs @@ -51,11 +51,17 @@ pub struct TrackConfig { pub media_start: Option, /// `hdlr` name; a sensible default is used when `None`. pub handler_name: Option, + /// Audio pre-roll: puts every sample in a `roll` sample group (`sgpd` + `sbgp`, ISO/IEC + /// 14496-12 ยง10.1) with this roll distance, the number of samples a decoder must decode before + /// (negative) a sample for its output to be correct; -1 for AAC. Apple's decoders take an AAC + /// track's priming from the edit list only when the track has this group. Written by + /// [`Mp4Writer`]; [`FragmentedWriter`] ignores it. + pub roll_distance: Option, } impl TrackConfig { pub fn new(entry: SampleEntry, timescale: u32) -> Self { - TrackConfig { entry, timescale, language: "und".into(), edits: Vec::new(), media_start: None, handler_name: None } + TrackConfig { entry, timescale, language: "und".into(), edits: Vec::new(), media_start: None, handler_name: None, roll_distance: None } } } @@ -543,6 +549,23 @@ fn write_stbl(b: &mut BoxBuf, t: &WTrack, qt: bool, shift: u64, force_co64: bool } } b.end(m); + if let Some(distance) = t.cfg.roll_distance + && sample_count > 0 + { + // One description, which every sample maps to. + let m = b.start_full(b"sgpd", 1, 0); + b.bytes(b"roll"); + b.u32(2); + b.u32(1); + b.i16(distance); + b.end(m); + let m = b.start_full(b"sbgp", 0, 0); + b.bytes(b"roll"); + b.u32(1); + b.u32(sample_count); + b.u32(1); + b.end(m); + } b.end(stbl); Ok(()) } @@ -1341,4 +1364,30 @@ mod tests { } assert_eq!(t.sync_sample_before(5), 3); } + + /// `roll_distance` puts every sample in one `roll` group; without it, or without samples, + /// the track has no sample group. + #[test] + fn roll_distance_is_a_sample_group_over_every_sample() { + let file = |roll: Option, samples: u32| { + let mut w = Mp4Writer::new(Cursor::new(Vec::new()), WriterOptions::new(Brand::Mp4)).unwrap(); + let mut cfg = TrackConfig::new(SampleEntry::aac(vec![0x11, 0x90], 2, 48_000), 48_000); + cfg.roll_distance = roll; + let t = w.add_track(cfg).unwrap(); + for _ in 0..samples { + w.write_sample(t, WriteSample { data: &[0; 8], duration: 1024, composition_offset: 0, is_sync: true }).unwrap(); + } + w.finish().unwrap().into_inner() + }; + let out = file(Some(-1), 5); + let f = crate::isobmff::open(out.as_slice()).unwrap(); + let groups = &f.tracks[0].sample_groups; + assert_eq!(groups.len(), 1); + assert_eq!((groups[0].grouping_type, groups[0].runs.as_slice()), (crate::isobmff::FourCc(*b"roll"), [(5, 1)].as_slice())); + assert_eq!(groups[0].descriptions, [(-1i16).to_be_bytes().to_vec()]); + assert_eq!(f.tracks[0].samples.len(), 5); + for out in [file(None, 5), file(Some(-1), 0)] { + assert!(crate::isobmff::open(out.as_slice()).unwrap().tracks[0].sample_groups.is_empty()); + } + } }