From 2506ad3005d241fe34de26d40b9178f7553ed659 Mon Sep 17 00:00:00 2001 From: alehel Date: Sun, 8 Nov 2026 05:42:48 +0100 Subject: [PATCH] craft-audio: review fixes, round 10 (#443) ADTS-framed AAC in MP4: durations short of the frames in the samples read ahead no longer end the track. With HE-AAC samples of two ADTS frames each, timed as two core frames, the durations matched one output frame per sample, so the undercount check passed and the track stopped at half its length, which probe reported as known. The frames each read-ahead sample holds are now counted, and durations short of them leave the length unknown until decoded, as durations short of one frame per sample already did. Correctly timed AAC-LC samples of several frames keep their end trim. Reported by FredApps. Co-Authored-By: Claude Opus 5.5 --- crates/audio/src/read/native.rs | 59 ++++++++++++++++++++++++++++++--- crates/audio/src/track.rs | 8 ++--- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/crates/audio/src/read/native.rs b/crates/audio/src/read/native.rs index 0441486b..e005778e 100644 --- a/crates/audio/src/read/native.rs +++ b/crates/audio/src/read/native.rs @@ -11,7 +11,7 @@ use super::aiff::{self, AiffParse}; use super::id3v2_len; use super::source::{Src, io_error}; use super::wav::{self, WavParse}; -use crate::track::{TrackDecoder, adts_format_of, adts_framed, adts_stream_header}; +use crate::track::{TrackDecoder, adts_format_of, adts_frame_count, adts_stream_header}; use crate::{AudioBuffer, AudioError, AudioInfo, CodecId, CodecParams, Container, PcmFormat, Result, aac}; use craft_media::{isobmff, matroska, ogg}; @@ -665,16 +665,24 @@ fn open_mp4<'a>(src: &Src<'a>, container: Container, track: Option) -> Op // HE-AAC packet's core frames) and can't cut audio off. // ADTS-framed samples (not standard) may hold several frames each, and finding out would mean // reading every sample: their sample table's timing is kept instead, unless it falls short of - // one frame per sample (durations counting HE-AAC's core frames). Then it can't end the track, - // and its length is unknown until decoded. - let adts = lookahead.iter().any(|p| adts_framed(p)); + // the frames the samples hold (durations counting HE-AAC's core frames): short of one frame + // per sample, or of the frames in the samples already read. Then it can't end the track, and + // its length is unknown until decoded. + let adts_counts: Vec = lookahead.iter().map(|p| adts_frame_count(p) as u64).collect(); + let adts = adts_counts.iter().any(|&c| c > 0); let per_packet = probe.frames_per_packet().map(|n| n as u64).filter(|_| !adts); let mut length_known = true; if let Some(n) = probe.frames_per_packet().map(|n| n as u64) { let one_each = (t.samples.len() as u64).saturating_mul(n).saturating_sub(skip); + // The samples read ahead are the first ones unless one could not be read. + let read_short = adts_counts.len() == t.samples.len().min(crate::track::LOOKAHEAD_PACKETS) && { + let held = adts_counts.iter().sum::().saturating_mul(n); + let timed: u64 = t.samples[..adts_counts.len()].iter().map(|s| u64::from(s.duration)).sum(); + held.saturating_sub(rescale(timed, u64::from(timescale), u64::from(rate))) >= n + }; if !adts && (stored > one_each || one_each - stored >= n) { stored = one_each; - } else if adts && one_each.saturating_sub(stored) >= n { + } else if adts && (one_each.saturating_sub(stored) >= n || read_short) { stored = 0; length_known = limit.is_some(); } @@ -1040,6 +1048,47 @@ mod adts_tests { assert_eq!(crate::probe(&file, None).unwrap().frames, 12 * 2048); } + /// Samples of two HE-AAC ADTS frames each, timed as two core frames: the durations match one + /// output frame per sample, and the frames in the samples read ahead show they fall short. + #[test] + fn adts_framed_he_aac_in_mp4_with_several_frames_per_sample_keeps_its_length() { + use craft_media::bitstream::BitWriter; + use craft_media::isobmff::{Brand, Mp4Writer, SampleEntry, TrackConfig, WriteSample, WriterOptions}; + let mut bw = BitWriter::new(); + for (value, bits) in [(0, 7), (0, 22), (6, 3), (1, 4), (13, 4), (0, 4), (7, 3)] { + bw.write_bits(value, bits); + } + let au = bw.finish(); + let frame = [&crate::aac::AdtsHeader::write(2, 7, 1, au.len(), 0x7FF).unwrap()[..], &au].concat().repeat(2); + let asc = crate::aac::AudioSpecificConfig::lc(7, 1).to_bytes().unwrap(); + let mut w = Mp4Writer::new(std::io::Cursor::new(Vec::new()), WriterOptions::new(Brand::Mp4)).unwrap(); + let track = w.add_track(TrackConfig::new(SampleEntry::aac(asc, 1, 44_100), 44_100)).unwrap(); + for _ in 0..12 { + w.write_sample(track, WriteSample { data: &frame, duration: 2048, composition_offset: 0, is_sync: true }).unwrap(); + } + let file = w.finish().unwrap().into_inner(); + let (info, buf) = decode(&file, None).unwrap(); + assert_eq!((info.codec, info.sample_rate, buf.frames()), (crate::CodecId::HeAac, 44_100, 12 * 4096)); + assert_eq!(crate::probe(&file, None).unwrap().frames, 12 * 4096); + } + + /// AAC-LC samples of two ADTS frames each, timed correctly, still end at their durations. + #[test] + fn adts_framed_lc_in_mp4_with_several_frames_per_sample_keeps_its_trim() { + use craft_media::isobmff::{Brand, Mp4Writer, SampleEntry, TrackConfig, WriteSample, WriterOptions}; + let asc = crate::aac::AudioSpecificConfig::lc(3, 1).to_bytes().unwrap(); + let mut w = Mp4Writer::new(std::io::Cursor::new(Vec::new()), WriterOptions::new(Brand::Mp4)).unwrap(); + let track = w.add_track(TrackConfig::new(SampleEntry::aac(asc, 1, 48_000), 48_000)).unwrap(); + for i in 0..12 { + let duration = if i == 11 { 2048 - 500 } else { 2048 }; + w.write_sample(track, WriteSample { data: &frame(3).repeat(2), duration, composition_offset: 0, is_sync: true }).unwrap(); + } + let file = w.finish().unwrap().into_inner(); + let (_, buf) = decode(&file, None).unwrap(); + assert_eq!(buf.frames(), 12 * 2048 - 500); + assert_eq!(crate::probe(&file, None).unwrap().frames, 12 * 2048 - 500); + } + /// A sample of several ADTS frames past the lookahead still counts them all. #[test] fn a_later_mp4_sample_of_several_adts_frames_keeps_its_length() { diff --git a/crates/audio/src/track.rs b/crates/audio/src/track.rs index 39418357..5816e099 100644 --- a/crates/audio/src/track.rs +++ b/crates/audio/src/track.rs @@ -323,10 +323,10 @@ pub(crate) fn adts_stream_header(headers: &[aac::AdtsHeader]) -> Option bool { - adts_frames(packet).is_some() +pub(crate) fn adts_frame_count(packet: &[u8]) -> usize { + adts_frames(packet).map_or(0, |f| f.len()) } /// The configuration of an AAC stream from its ADTS `header`. Channel configuration 0 leaves @@ -587,7 +587,7 @@ mod tests { let tied: [&[u8]; 2] = [&good, &damaged]; assert_eq!(TrackDecoder::new(&aac_params(), &tied).unwrap().sample_rate(), 48_000); #[cfg(feature = "decode-native")] - assert!(adts_framed(&good) && adts_framed(&good.repeat(2)) && !adts_framed(&[0; 8])); + assert_eq!((adts_frame_count(&good), adts_frame_count(&good.repeat(2)), adts_frame_count(&[0; 8])), (1, 2, 0)); } /// `au` as a stereo ADTS frame at 48 kHz (channel configuration `channel_config`).