craft-audio: review fixes, round 4 (#443)

- TrackDecoder takes an ADTS stream's format from the one most of the lookahead
  shares, as the file path did, so one damaged first header does not fail every
  packet of an MPEG-TS stream. open_adts now just asks it.
- MP4 samples of several ADTS frames keep the sample table's timing; a packet
  count of one frame each truncated them silently.
- Sample durations past what the packets decode to no longer over-report the
  length.
- split_adts_upto is not behind aac-decode, as split_adts is not: an aac-encode
  build in the next PR did not compile.

Removes an unreachable ID_END check and open_adts's unused core rate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/audio/src/aac/config.rs

@@ -385,7 +385,6 @@ pub fn split_adts(data: &[u8]) -> Result<Vec<(AdtsHeader, &[u8])>> {
 }
 
 /// [`split_adts`], stopping after `max` frames.
-#[cfg(feature = "aac-decode")]
 pub(crate) fn split_adts_upto(mut data: &[u8], max: usize) -> Result<Vec<(AdtsHeader, &[u8])>> {
     let mut out = Vec::new();
     while data.len() >= 7 && out.len() < max {

crates/audio/src/aac/decoder.rs

@@ -211,9 +211,6 @@ impl Decoder {
                 }
                 _ => break, // ID_END
             }
-            if id == ID_END {
-                break;
-            }
         }
         if nch == 0 && !saw_element {
             return Err(Error::InvalidConfig("channel configuration 0 without a program config element"));

crates/audio/src/aac/mod.rs

@@ -26,7 +26,6 @@ mod mdct;
 mod tables;
 mod tns;
 
-#[cfg(feature = "aac-decode")]
 pub(crate) use config::split_adts_upto;
 pub use config::{AdtsHeader, AudioSpecificConfig, ElementType, ProgramConfig, config_layout, split_adts};
 pub use decoder::Decoder;

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;
+use crate::track::{TrackDecoder, adts_format_of, adts_stream_header, several_adts_frames};
 use crate::{AudioBuffer, AudioError, AudioInfo, CodecId, CodecParams, Container, PcmFormat, Result, aac};
 use craft_media::{isobmff, matroska, ogg};
 
@@ -573,27 +573,19 @@ fn open_adts<'a>(src: &Src<'a>, at: u64) -> Option<Result<Native<'a>>> {
             Ok(frames) => frames?,
             Err(e) => return Some(Err(e)),
         };
-    let format = |h: &aac::AdtsHeader| (h.object_type, h.sf_index, h.channel_config);
     let first = &frames[..frames.len().min(crate::track::LOOKAHEAD_PACKETS)];
-    let &(_, _, stream) = first.iter().max_by_key(|(_, _, h)| first.iter().filter(|(_, _, o)| format(o) == format(h)).count())?;
-    let usable = |h: &aac::AdtsHeader| format(h) == format(&stream) && h.raw_data_blocks == 1;
-    if first.iter().filter(|(_, _, h)| usable(h)).count() * 2 <= first.len() {
+    let headers: Vec<aac::AdtsHeader> = first.iter().map(|&(_, _, h)| h).collect();
+    let stream = adts_format_of(&adts_stream_header(&headers)?);
+    if headers.iter().filter(|h| adts_format_of(h) == stream && h.raw_data_blocks == 1).count() * 2 <= first.len() {
         return None;
     }
-    // The stream's own frames first: the first of them gives the configuration. The others can
-    // still hold the stream's only program config element.
-    let (mut ordered, others): (Vec<_>, Vec<_>) = first.iter().copied().partition(|(_, _, h)| usable(h));
-    ordered.extend(others);
-    let core = stream.to_asc().sample_rate;
-    // Without extradata the configuration comes from the first header (and, for channel
-    // configuration 0, a program config element in the stream).
-    let params = CodecParams::new(CodecId::Aac).with_sample_rate(core);
-    let lookahead = raw_lookahead(src, &ordered, crate::track::LOOKAHEAD_PACKETS);
+    // Without extradata the configuration comes from the frames themselves.
+    let params = CodecParams::new(CodecId::Aac);
+    let lookahead = raw_lookahead(src, &frames, crate::track::LOOKAHEAD_PACKETS);
     // Decoded frames are set below, when the decoder has said what a frame decodes to.
     let packets: Vec<(u64, u32, u32)> = frames.iter().map(|&(p, n, h)| (p, n, u32::from(h.raw_data_blocks))).collect();
     let mut n = native(Container::Adts, Packets::Raw { frames: packets, src: src.clone(), next: 0 }, &params, &lookahead, 0, None, None)?;
-    // HE-AAC decodes twice the core frame length.
-    let per_block = rescale(aac::FRAME_LEN as u64, u64::from(core), u64::from(n.info.sample_rate)) as u32;
+    let per_block = n.dec.frames_per_packet()? as u32;
     if let Packets::Raw { frames, .. } = &mut n.packets {
         for f in frames.iter_mut() {
             f.2 *= per_block;
@@ -660,10 +652,11 @@ fn open_mp4<'a>(src: &Src<'a>, container: Container, track: Option<usize>) -> Op
     let mut stored = rescale(media, u64::from(timescale), u64::from(rate)).saturating_sub(skip);
     // Durations only trim the last packet: ones that end anywhere else are wrong (some count a
     // HE-AAC packet's core frames) and can't cut audio off.
-    let per_packet = probe.frames_per_packet().map(|n| n as u64);
+    // Samples of several ADTS frames (not standard, but they decode) have no one length.
+    let per_packet = probe.frames_per_packet().map(|n| n as u64).filter(|_| !lookahead.iter().any(|p| several_adts_frames(p)));
     if let Some(n) = per_packet {
         let decoded = (t.samples.len() as u64).saturating_mul(n).saturating_sub(skip);
-        if stored.abs_diff(decoded) >= n {
+        if stored > decoded || decoded - stored >= n {
             stored = decoded;
         }
     }

crates/audio/src/track.rs

@@ -78,8 +78,8 @@ impl TrackDecoder {
     /// inside the stream is found in them.
     ///
     /// Streams without codec configuration are accepted where the packets carry it: AAC with
-    /// empty `extradata` takes its configuration from the first packet's ADTS header (MPEG-TS,
-    /// raw `.aac`), and ADTS headers are stripped from every packet. Other missing configuration is
+    /// empty `extradata` takes its configuration from the ADTS headers of the lookahead (MPEG-TS,
+    /// raw `.aac`), the one most of them share, and ADTS headers are stripped from every packet. Other missing configuration is
     /// [`AudioError::Malformed`].
     ///
     /// Codecs without a native packet decoder yet are [`AudioError::Unsupported`];
@@ -97,7 +97,8 @@ impl TrackDecoder {
                     lookahead.iter().map(|p| adts_frames(p).and_then(|f| f.first().map(|(_, unit)| *unit)).unwrap_or(p)).collect();
                 let mut adts_format = None;
                 let asc = if params.extradata.is_empty() {
-                    let h = lookahead.first().and_then(|p| aac::AdtsHeader::parse(p).ok()).ok_or_else(|| {
+                    let headers: Vec<aac::AdtsHeader> = lookahead.iter().filter_map(|p| aac::AdtsHeader::parse(p).ok()).collect();
+                    let h = adts_stream_header(&headers).ok_or_else(|| {
                         AudioError::Malformed("AAC track without an AudioSpecificConfig (extradata), and its packets are not ADTS".into())
                     })?;
                     adts_format = Some(adts_format_of(&h));
@@ -293,7 +294,20 @@ fn append(out: &mut Vec<Vec<f32>>, more: Vec<Vec<f32>>) {
 #[cfg(feature = "aac-decode")]
 const MAX_ADTS_FRAMES_PER_PACKET: usize = 1024;
 
-/// The configuration of an AAC stream from its first ADTS header. Channel configuration 0 leaves
+/// The header of the format most of `headers` share, the earliest on a tie, so one damaged header
+/// does not decide a stream's format.
+#[cfg(feature = "aac-decode")]
+pub(crate) fn adts_stream_header(headers: &[aac::AdtsHeader]) -> Option<aac::AdtsHeader> {
+    headers.iter().rev().max_by_key(|h| headers.iter().filter(|o| adts_format_of(o) == adts_format_of(h)).count()).copied()
+}
+
+/// Whether `packet` is several ADTS frames, which decode to more than one packet's length.
+#[cfg(feature = "decode-native")]
+pub(crate) fn several_adts_frames(packet: &[u8]) -> bool {
+    adts_frames(packet).is_some_and(|frames| frames.len() > 1)
+}
+
+/// The configuration of an AAC stream from its ADTS `header`. Channel configuration 0 leaves
 /// the layout to a program config element in the stream, looked for in `first_units`.
 #[cfg(feature = "aac-decode")]
 fn adts_config(header: &aac::AdtsHeader, first_units: &[&[u8]]) -> Result<Vec<u8>> {
@@ -326,10 +340,10 @@ fn adts_frames(packet: &[u8]) -> Option<Vec<(aac::AdtsHeader, &[u8])>> {
 /// What an ADTS frame says the stream's format is: profile, sampling frequency index and channel
 /// configuration.
 #[cfg(feature = "aac-decode")]
-type AdtsFormat = (u8, u8, u8);
+pub(crate) type AdtsFormat = (u8, u8, u8);
 
 #[cfg(feature = "aac-decode")]
-fn adts_format_of(h: &aac::AdtsHeader) -> AdtsFormat {
+pub(crate) fn adts_format_of(h: &aac::AdtsHeader) -> AdtsFormat {
     (h.object_type, h.sf_index, h.channel_config)
 }
 
@@ -535,6 +549,25 @@ mod tests {
         assert!(dec.decode(&frame).is_err(), "elements the configuration does not have");
     }
 
+    /// One damaged header at the start of a stream does not decide its format.
+    #[cfg(feature = "aac-decode")]
+    #[test]
+    fn the_adts_format_is_the_one_most_of_the_lookahead_shares() {
+        let good = adts_frame(2);
+        let mut damaged = good.clone();
+        damaged[2] = (damaged[2] & 0xC3) | (4 << 2); // sampling frequency index 4 where the rest have 3
+        let lookahead: [&[u8]; 4] = [&damaged, &good, &good, &good];
+        let mut dec = TrackDecoder::new(&aac_params(), &lookahead).unwrap();
+        assert_eq!(dec.sample_rate(), 48_000);
+        assert!(dec.decode(&damaged).is_err());
+        assert_eq!(dec.decode(&good).unwrap().frames(), 1024);
+        // on a tie, the earlier one
+        let tied: [&[u8]; 2] = [&good, &damaged];
+        assert_eq!(TrackDecoder::new(&aac_params(), &tied).unwrap().sample_rate(), 48_000);
+        #[cfg(feature = "decode-native")]
+        assert!(several_adts_frames(&good.repeat(2)) && !several_adts_frames(&good) && !several_adts_frames(&[0; 8]));
+    }
+
     #[cfg(feature = "aac-decode")]
     #[test]
     fn a_packet_of_too_many_adts_frames_is_refused() {

crates/audio/tests/codecs/routing_oracle.rs

@@ -531,6 +531,7 @@ fn mp4_with_wrong_durations_or_edits_presents_its_audio() {
         ("durations of one", patched(&bytes, b"stts", &[stts_delta], |_| 1)),
         ("durations doubled", patched(&bytes, b"stts", &[stts_delta], |v| v * 2)),
         ("durations of zero", patched(&bytes, b"stts", &[stts_delta], |_| 0)),
+        ("last delta longer than a packet", patched(&bytes, b"stts", &[stts_delta + 8], |_| 1500)),
     ] {
         assert_ne!(file, bytes, "{name}");
         let (info, buf) = decode(&file, None).unwrap();