From e5bdd51ad0ae96ce9481de1b41c6cec252e5192f Mon Sep 17 00:00:00 2001 From: Gunwoo Kim Date: Sun, 8 Nov 2026 01:34:33 +0900 Subject: [PATCH 1/3] docs(mp4): plan copied MP3 bitrate metadata Signed-off-by: Gunwoo Kim --- docs/design/copied-mp3-bitrate.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 docs/design/copied-mp3-bitrate.md diff --git a/docs/design/copied-mp3-bitrate.md b/docs/design/copied-mp3-bitrate.md new file mode 100644 index 000000000..c58121464 --- /dev/null +++ b/docs/design/copied-mp3-bitrate.md @@ -0,0 +1,21 @@ +# Copied MP3 bitrate metadata + +MP4-sourced MP3 copied beside encoded video retains source bitrate fields today. A generated 48 kHz control carries 67,511 bits/s, while pinned FFmpeg 9.0.2 writes 64,000. The difference comes from a shortened final stored sample: the complete compressed frames represent more samples than the container table clock. + +## Clock and ownership + +The writer reuses `coreav_audio::mp3::Header` rather than adding a second MPEG audio header parser. A metadata-only packet pass requires one complete Layer III frame per MP4 sample, consistent MPEG version and sample rate, and the same sample count as the completed output tables. It sums 1,152 samples per MPEG-1 frame or 576 per MPEG-2 frame. The existing parser also supports the non-ISO MPEG-2.5 extension. The average is `packet_bytes * 8 * sample_rate / decoded_frame_samples`, rounded down. It uses actual frame bytes, including padding: a generated 44.1 kHz constant-rate control averages 63,978 rather than its nominal header rate of 64,000. + +ISO/IEC 11172-3 and 13818-3 establish the frame sample counts. The choice between complete-frame and shortened stored clocks for bitrate hints is an oracle compatibility policy, not a claim that the specifications forbid the stored durations. The maximum remains the larger of the average and the requested rate. Buffer size is zero. Neither packets nor sample tables change. + +The extra packet pass uses only work left after the existing listing and copy passes. Insufficient work, malformed headers, partial or compound samples, free format, inconsistent clocks and source errors leave the complete entry unchanged. They never add a refusal. The writer depends on the existing audio crate, which the CLI and both bindings already use; no decoder implementation changes. + +## Default and excluded paths + +For the ffmpeg adapter's already-native video-encode route, no requested bitrate is represented by zero: the maximum equals the average. Missing, malformed and out-of-range `-b:a` values all use that default. This also normalizes copied AAC and Opus metadata when `-b:a` is absent, as the pinned oracle does for stale or missing fields. Native transcode and ordinary remux retain `None` and their existing bytes. + +Matroska input and output remain unchanged. MPEG-2 AAC object types and QuickTime version-1/version-2 sound entries remain explicit, tested skips rather than new admission checks. Exact Matroska copy-clock parity is separate work. + +## Verification + +Generated CBR and VBR MP3 controls at 48, 44.1 and 22.05 kHz compare `esds` and `btrt` with the pinned oracle, with and without `-b:a`. Full-file witnesses preserve every non-metadata byte, including the shortened stored sample duration. AAC and Opus controls include absent requests with stale or missing metadata. Synthetic MP3 controls prove byte-identical fallback for unproved clocks and an exhausted optional-work allowance. Native transcode and Matroska before/after files remain byte-identical. New dependency notices are checked without changing distribution licenses. From d8977726c60b20e939dd2f487f06485c5eb125b6 Mon Sep 17 00:00:00 2001 From: Gunwoo Kim Date: Sun, 8 Nov 2026 02:06:00 +0900 Subject: [PATCH 2/3] fix(mp4): match copied MP3 bitrate metadata Signed-off-by: Gunwoo Kim --- Cargo.lock | 1 + crates/coreav-cli/fuzz/Cargo.lock | 20 ++ crates/coreav-cli/src/ffmpeg/encode.rs | 19 +- crates/coreav-cli/src/transcode.rs | 5 +- .../coreav-cli/tests/ffmpeg_audio_options.rs | 106 ++++++- crates/coreav-isobmff-write/Cargo.toml | 1 + crates/coreav-isobmff-write/fuzz/Cargo.lock | 22 ++ crates/coreav-isobmff-write/src/audio.rs | 49 ++- crates/coreav-isobmff-write/src/lib.rs | 15 +- crates/coreav-isobmff-write/src/mux.rs | 70 ++++- crates/coreav-isobmff-write/src/remux.rs | 75 ++++- .../tests/copied_bitrate.rs | 288 ++++++++++++++++++ crates/coreav-isobmff/src/codec.rs | 25 +- crates/coreav-isobmff/src/lib.rs | 8 +- crates/coreav-policy/fuzz/Cargo.lock | 20 ++ crates/coreav-transcode/fuzz/Cargo.lock | 2 + crates/coreav-transcode/src/pipeline.rs | 5 +- crates/coreav-wasm/fuzz/Cargo.lock | 20 ++ docs/agents/containers.md | 1 + docs/design/copied-audio-bitrate.md | 4 +- docs/design/copied-mp3-bitrate.md | 6 +- 21 files changed, 686 insertions(+), 76 deletions(-) create mode 100644 crates/coreav-isobmff-write/tests/copied_bitrate.rs diff --git a/Cargo.lock b/Cargo.lock index 1b60107af..125cebf3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -451,6 +451,7 @@ dependencies = [ name = "coreav-isobmff-write" version = "0.0.1" dependencies = [ + "coreav-audio", "coreav-counting-alloc", "coreav-isobmff", "coreav-mkv", diff --git a/crates/coreav-cli/fuzz/Cargo.lock b/crates/coreav-cli/fuzz/Cargo.lock index 3545c5159..f615c3f71 100644 --- a/crates/coreav-cli/fuzz/Cargo.lock +++ b/crates/coreav-cli/fuzz/Cargo.lock @@ -65,6 +65,7 @@ dependencies = [ "coreav-mpegts", "coreav-policy", "coreav-text", + "mimalloc", "rustix", "serde_json", ] @@ -100,6 +101,7 @@ dependencies = [ name = "coreav-isobmff-write" version = "0.0.1" dependencies = [ + "coreav-audio", "coreav-isobmff", "coreav-mkv", "coreav-mpegts", @@ -204,6 +206,15 @@ dependencies = [ "cc", ] +[[package]] +name = "libmimalloc-sys" +version = "0.1.49" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a45a52f43e1c16f667ccfe4dd8c85b7f7c204fd5e3bf46c5b0db9a5c3c0b8e9" +dependencies = [ + "cc", +] + [[package]] name = "linux-raw-sys" version = "0.12.1" @@ -216,6 +227,15 @@ version = "2.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" +[[package]] +name = "mimalloc" +version = "0.1.52" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d4139bb28d14ad1facf21d5eb8825051b326e172d216b39f6d31df53cc97862" +dependencies = [ + "libmimalloc-sys", +] + [[package]] name = "miniz_oxide" version = "0.8.9" diff --git a/crates/coreav-cli/src/ffmpeg/encode.rs b/crates/coreav-cli/src/ffmpeg/encode.rs index 5dc19f81c..330b8ab32 100644 --- a/crates/coreav-cli/src/ffmpeg/encode.rs +++ b/crates/coreav-cli/src/ffmpeg/encode.rs @@ -313,15 +313,16 @@ pub fn request( opus, aac, aac_mp4_tail: true, - copied_audio_bit_rate: audio_encoder - .is_none() - .then(|| { - audio_bitrate - .as_deref() - .and_then(bits) - .and_then(|b| u32::try_from(b).ok()) - }) - .flatten(), + copied_audio_bit_rate: audio_encoder.is_none().then(|| { + audio_bitrate + .as_deref() + .and_then(bits) + .and_then(|b| u32::try_from(b).ok()) + .map_or( + coreav_isobmff_write::CopiedAudioBitRate::FromSource, + coreav_isobmff_write::CopiedAudioBitRate::Requested, + ) + }), #[cfg(feature = "input-capture")] input_capture: None, pixel_format, diff --git a/crates/coreav-cli/src/transcode.rs b/crates/coreav-cli/src/transcode.rs index 9547302a1..e02dc592d 100644 --- a/crates/coreav-cli/src/transcode.rs +++ b/crates/coreav-cli/src/transcode.rs @@ -103,8 +103,9 @@ pub struct Request { pub aac: Option, /// Match the FFmpeg adapter's terminal AAC sample timing in MP4. pub aac_mp4_tail: bool, - /// Optional metadata bitrate for copied AAC and Opus in MP4. - pub copied_audio_bit_rate: Option, + /// Optional metadata bitrate for MP4-sourced copied AAC, MP3 and Opus. + /// `None` preserves metadata rather than normalizing from the source clock. + pub copied_audio_bit_rate: Option, /// Optional actual AAC-encoder stdin capture. #[cfg(feature = "input-capture")] pub input_capture: Option, diff --git a/crates/coreav-cli/tests/ffmpeg_audio_options.rs b/crates/coreav-cli/tests/ffmpeg_audio_options.rs index 6d85c7281..ef1519550 100644 --- a/crates/coreav-cli/tests/ffmpeg_audio_options.rs +++ b/crates/coreav-cli/tests/ffmpeg_audio_options.rs @@ -105,6 +105,22 @@ fn oracle(dir: &Path, output: &Path, options: &[&str]) { ); } +fn native_transcode_control(dir: &Path) -> Vec { + let output = dir.join("native-control.mp4"); + success( + Command::new(env!("CARGO_BIN_EXE_coreav")) + .arg("transcode") + .arg(dir.join("source.mp4")) + .arg(&output) + .args(["--threads", "1", "--encoder", "auto"]) + .env("COREAV_FFMPEG", test_oracle::require().ffmpeg()) + .env_remove("COREAV_STRICT_SAFE") + .output() + .unwrap(), + ); + std::fs::read(output).unwrap() +} + fn audio_properties(path: &Path) -> serde_json::Value { let bytes = success( Command::new(test_oracle::require().ffprobe()) @@ -176,7 +192,7 @@ fn esds_rates(payload: &[u8]) -> usize { at += 3; } } - assert_eq!(payload[at], 0x40); + assert!(matches!(payload[at], 0x40 | 0x69 | 0x6B)); at + 2 } @@ -569,7 +585,7 @@ fn identity_audio_options_and_default_aac_bitrate_run_natively() { } #[test] -fn default_bitrate_admission_preserves_legacy_native_commands() { +fn copied_bitrate_metadata_preserves_command_admission() { let dir = fixture("bitrate"); let source = dir.0.join("aac-source.mp4"); success( @@ -709,13 +725,13 @@ fn copied_aac_and_opus_match_oracle_metadata_without_other_file_changes() { "-c:a", "copy", ]; - let control = dir.0.join("control.mp4"); - success(native(&dir.0, &control, &options, false)); - let control = std::fs::read(control).unwrap(); + let control = native_transcode_control(&dir.0); assert_eq!(audio_entries(&control).len(), 2); - for rate in ["0", "16k", "96k", "128k"] { + for rate in [None, Some("0"), Some("16k"), Some("96k"), Some("128k")] { let mut args = options.to_vec(); - args.extend(["-b:a", rate]); + if let Some(rate) = rate { + args.extend(["-b:a", rate]); + } let actual = dir.0.join("actual.mp4"); let reference = dir.0.join("reference.mp4"); success(native(&dir.0, &actual, &args, false)); @@ -725,11 +741,11 @@ fn copied_aac_and_opus_match_oracle_metadata_without_other_file_changes() { assert_eq!( actual.len(), expected.len(), - "{encoder} {container} {variant} {rate}" + "{encoder} {container} {variant} {rate:?}" ); assert!( actual == expected, - "complete file differs: {encoder} {container} {variant} {rate}; first mismatch {:?}", + "complete file differs: {encoder} {container} {variant} {rate:?}; first mismatch {:?}", actual.iter().zip(&expected).position(|(a, b)| a != b) ); } @@ -793,7 +809,7 @@ fn matroska_copy_bitrate_keeps_complete_native_files() { } #[test] -fn ignored_copy_bitrates_and_copied_mp3_keep_complete_native_files() { +fn ignored_copy_bitrates_match_the_native_default() { for encoder in ["aac", "libmp3lame"] { let dir = fixture(&format!("copied-preserved-{encoder}")); let encoded = dir.0.join("encoded.mp4"); @@ -812,11 +828,7 @@ fn ignored_copy_bitrates_and_copied_mp3_keep_complete_native_files() { let control = dir.0.join("control.mp4"); success(native(&dir.0, &control, &options, false)); let control = std::fs::read(control).unwrap(); - let mut rates = vec!["garbage", "4294967296", "1e6", "-1"]; - if encoder == "libmp3lame" { - rates.extend(["16k", "96k", "128k"]); - } - for rate in rates { + for rate in ["garbage", "4294967296", "1e6", "-1"] { let mut args = options.to_vec(); args.extend(["-b:a", rate]); let actual = dir.0.join("actual.mp4"); @@ -826,6 +838,70 @@ fn ignored_copy_bitrates_and_copied_mp3_keep_complete_native_files() { } } +#[test] +fn copied_mp3_matches_oracle_metadata_with_and_without_bitrate() { + for sample_rate in ["48000", "44100", "22050", "11025"] { + for mode in ["cbr", "vbr"] { + let dir = fixture(&format!("copied-mp3-{sample_rate}-{mode}")); + let encoded = dir.0.join("encoded.mp4"); + let mut encode = vec![ + "-map", + "0:v", + "-map", + "0:a", + "-c:v", + "copy", + "-c:a", + "libmp3lame", + "-ar", + sample_rate, + "-disposition:a", + "0", + ]; + encode.extend(if mode == "cbr" { + ["-b:a", "64k"] + } else { + ["-q:a", "4"] + }); + if sample_rate == "11025" { + encode.extend(["-strict", "-1"]); + } + oracle(&dir.0, &encoded, &encode); + std::fs::rename(encoded, dir.0.join("source.mp4")).unwrap(); + let control = native_transcode_control(&dir.0); + assert_eq!(audio_entries(&control).len(), 2); + for rate in [None, Some("16k"), Some("96k"), Some("128k")] { + let mut args = vec![ + "-map", "0:v", "-map", "0:a", "-c:v", "libx264", "-threads", "1", "-c:a", + "aac", "-c:a", "copy", + ]; + if let Some(rate) = rate { + args.extend(["-b:a", rate]); + } + let actual = dir.0.join("actual.mp4"); + success(native(&dir.0, &actual, &args, false)); + if sample_rate == "11025" { + args.extend(["-strict", "-1"]); + } + let reference = dir.0.join("reference.mp4"); + oracle(&dir.0, &reference, &args); + let expected = bitrate_reference(&control, &std::fs::read(reference).unwrap()); + let actual = std::fs::read(actual).unwrap(); + assert_eq!( + actual.len(), + expected.len(), + "{sample_rate} {mode} {rate:?}" + ); + assert!( + actual == expected, + "complete MP3 file differs: {sample_rate} {mode} {rate:?}; first mismatch {:?}", + actual.iter().zip(&expected).position(|(a, b)| a != b) + ); + } + } + } +} + #[test] fn nonidentity_and_copy_audio_options_preserve_destinations() { let dir = fixture("refuse"); diff --git a/crates/coreav-isobmff-write/Cargo.toml b/crates/coreav-isobmff-write/Cargo.toml index 79f9171d2..a44edb537 100644 --- a/crates/coreav-isobmff-write/Cargo.toml +++ b/crates/coreav-isobmff-write/Cargo.toml @@ -9,6 +9,7 @@ repository.workspace = true publish = false [dependencies] +coreav-audio = { path = "../coreav-audio" } coreav-isobmff = { path = "../coreav-isobmff" } coreav-mkv = { path = "../coreav-mkv" } coreav-mpegts = { path = "../coreav-mpegts" } diff --git a/crates/coreav-isobmff-write/fuzz/Cargo.lock b/crates/coreav-isobmff-write/fuzz/Cargo.lock index 40f26e3d5..dab6087c1 100644 --- a/crates/coreav-isobmff-write/fuzz/Cargo.lock +++ b/crates/coreav-isobmff-write/fuzz/Cargo.lock @@ -32,6 +32,16 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4e7648175b45a9a48536d676f68d918270699102aa8dab5496df06904c914600" +[[package]] +name = "coreav-audio" +version = "0.0.1" +dependencies = [ + "coreav-isobmff", + "coreav-kernels-audio", + "coreav-mkv", + "coreav-opus", +] + [[package]] name = "coreav-isobmff" version = "0.0.1" @@ -43,6 +53,7 @@ dependencies = [ name = "coreav-isobmff-write" version = "0.0.1" dependencies = [ + "coreav-audio", "coreav-isobmff", "coreav-mkv", "coreav-mpegts", @@ -58,6 +69,10 @@ dependencies = [ "libfuzzer-sys", ] +[[package]] +name = "coreav-kernels-audio" +version = "0.0.1" + [[package]] name = "coreav-mkv" version = "0.0.1" @@ -73,6 +88,13 @@ dependencies = [ "coreav-isobmff", ] +[[package]] +name = "coreav-opus" +version = "0.0.1" +dependencies = [ + "coreav-kernels-audio", +] + [[package]] name = "find-msvc-tools" version = "0.1.14" diff --git a/crates/coreav-isobmff-write/src/audio.rs b/crates/coreav-isobmff-write/src/audio.rs index 26cd4691c..3f8a5908f 100644 --- a/crates/coreav-isobmff-write/src/audio.rs +++ b/crates/coreav-isobmff-write/src/audio.rs @@ -15,7 +15,7 @@ pub(crate) fn set_bit_rate( room: u64, box_limit: u64, ) -> Option { - let aac = match entry.get(4..8) { + let mpeg_audio = match entry.get(4..8) { Some(b"mp4a") => true, Some(b"Opus") => false, _ => return None, @@ -43,11 +43,11 @@ pub(crate) fn set_bit_rate( let start = at + usize::from(header.header_len); let body = entry.get(start..end)?; match &header.kind.0 { - b"esds" if aac => { + b"esds" if mpeg_audio => { if esds.is_some() { return None; } - let offset = coreav_isobmff::aac_esds_bit_rate_offset(body)?; + let offset = coreav_isobmff::esds_bit_rate_offset(body)?; esds = Some(start + offset); } b"btrt" => { @@ -60,7 +60,7 @@ pub(crate) fn set_bit_rate( } at = end; } - if aac && esds.is_none() { + if mpeg_audio && esds.is_none() { return None; } let growth = if btrt.is_none() { 20 } else { 0 }; @@ -764,14 +764,39 @@ mod tests { ); assert_eq!(actual, duplicate); } - let mut mp3 = bitrate_entry(*b"mp4a", true); - mp3[55] = 0x6B; - let mut actual = mp3.clone(); - assert_eq!( - set_bit_rate(&mut actual, 72_000, 96_000, 20, u64::MAX), - None - ); - assert_eq!(actual, mp3); + for kind in [0x69, 0x6B] { + let mut mp3 = bitrate_entry(*b"mp4a", true); + mp3[55] = kind; + assert_eq!(set_bit_rate(&mut mp3, 72_000, 96_000, 0, u64::MAX), Some(0)); + let rates: Vec = [0u32, 96_000, 72_000] + .into_iter() + .flat_map(u32::to_be_bytes) + .collect(); + assert_eq!(&mp3[57..68], &rates[1..]); + assert_eq!(&mp3[mp3.len() - 12..], &rates); + } + } + + #[test] + fn unhandled_audio_descriptions_explicitly_preserve_bitrate_metadata() { + for kind in 0x66..=0x68 { + let mut entry = bitrate_entry(*b"mp4a", true); + entry[55] = kind; + let before = entry.clone(); + assert_eq!(set_bit_rate(&mut entry, 72_000, 96_000, 20, u64::MAX), None); + assert_eq!(entry, before); + } + for version in [1u16, 2] { + let mut entry = bitrate_entry(*b"mp4a", true); + entry[16..18].copy_from_slice(&version.to_be_bytes()); + let before = entry.clone(); + assert_eq!(set_bit_rate(&mut entry, 72_000, 96_000, 20, u64::MAX), None); + assert_eq!(entry, before); + } + let mut entry = bitrate_entry(*b".mp3", true); + let before = entry.clone(); + assert_eq!(set_bit_rate(&mut entry, 72_000, 96_000, 20, u64::MAX), None); + assert_eq!(entry, before); } fn pack(fields: &[(u32, u32)]) -> Vec { diff --git a/crates/coreav-isobmff-write/src/lib.rs b/crates/coreav-isobmff-write/src/lib.rs index 38478fa16..d5d042a88 100644 --- a/crates/coreav-isobmff-write/src/lib.rs +++ b/crates/coreav-isobmff-write/src/lib.rs @@ -268,12 +268,21 @@ pub enum OnUnsupported { Fail, } +/// The maximum-bitrate hint used when normalizing copied audio metadata. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum CopiedAudioBitRate { + /// Inherit the average from the source's stored sample clock. + FromSource, + /// Use an explicit request in bits per second, including zero. + Requested(u32), +} + /// What [`remux_mp4`] copies. #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct RemuxOptions { - /// Recompute copied AAC and Opus bitrate metadata, with this requested - /// maximum in bits per second. Other codecs and replacements are unchanged. - pub copied_audio_bit_rate: Option, + /// Normalize copied AAC, MP3 and Opus bitrate metadata with this maximum + /// hint. `None` preserves metadata; other codecs and replacements are unchanged. + pub copied_audio_bit_rate: Option, /// Store copied packets in edited decode-time order, with output track /// order breaking ties. Uses one sample per chunk under the existing /// table budget. Unsupported edit clocks fail before writing; native diff --git a/crates/coreav-isobmff-write/src/mux.rs b/crates/coreav-isobmff-write/src/mux.rs index 7241cb738..db8fff18a 100644 --- a/crates/coreav-isobmff-write/src/mux.rs +++ b/crates/coreav-isobmff-write/src/mux.rs @@ -21,7 +21,6 @@ //! [`Limits::max_table_bytes`] across all tracks, plus what the caller //! copies with; never the size of the media. -use crate::ExtractError; use crate::boxes::Node; use crate::copy::{COPY_BUFFER, Window}; use crate::moov::{ @@ -29,6 +28,7 @@ use crate::moov::{ rescale, write_head, }; use crate::tables::Tables; +use crate::{CopiedAudioBitRate, ExtractError}; use coreav_isobmff::{Error, FourCc, LimitKind, Limits, Source}; use std::borrow::Cow; use std::io::Write; @@ -420,24 +420,36 @@ impl Muxer { pub(crate) fn set_audio_bit_rate( &mut self, track: usize, - requested: u32, + policy: CopiedAudioBitRate, + clock: Option<(u64, u32)>, room: u64, limits: &Limits, ) -> Result { self.open()?; let t = self.tracks.get_mut(track).ok_or(Error::NoSuchTrack)?; - if t.tables.duration == 0 { + let (duration, timescale) = clock.unwrap_or((t.tables.duration, t.header.timescale)); + if duration == 0 { return Ok(0); } - let average = u128::from(t.tables.data_bytes) * 8 * u128::from(t.header.timescale) - / u128::from(t.tables.duration); - let Ok(average) = u32::try_from(average) else { + let bits = u128::from(t.tables.data_bytes) * 8; + let stored_average = if t.tables.duration == 0 { + 0 + } else { + bits * u128::from(t.header.timescale) / u128::from(t.tables.duration) + }; + let average = bits * u128::from(timescale) / u128::from(duration); + let hint = match policy { + CopiedAudioBitRate::FromSource => stored_average, + CopiedAudioBitRate::Requested(rate) => u128::from(rate), + }; + let maximum = average.max(hint); + let (Ok(average), Ok(maximum)) = (u32::try_from(average), u32::try_from(maximum)) else { return Ok(0); }; Ok(crate::audio::set_bit_rate( &mut t.header.sample_entry, average, - average.max(requested), + maximum, room, limits.max_box_bytes, ) @@ -932,7 +944,13 @@ mod tests { let index = muxer.add_track(h).unwrap(); assert_eq!( muxer - .set_audio_bit_rate(index, 96_000, 20, &limits) + .set_audio_bit_rate( + index, + CopiedAudioBitRate::Requested(96_000), + None, + 20, + &limits + ) .unwrap(), 0 ); @@ -961,7 +979,13 @@ mod tests { .unwrap(); assert_eq!( muxer - .set_audio_bit_rate(index, 96_000, 20, &limits) + .set_audio_bit_rate( + index, + CopiedAudioBitRate::Requested(96_000), + None, + 20, + &limits + ) .unwrap(), 20 ); @@ -971,12 +995,36 @@ mod tests { .flat_map(u32::to_be_bytes) .collect(); assert_eq!(&actual[actual.len() - 12..], &expected); - let before = actual.clone(); + for (policy, samples, maximum, average) in [ + (CopiedAudioBitRate::FromSource, 1200, 120_000, 96_000), + (CopiedAudioBitRate::Requested(0), 1200, 96_000, 96_000), + (CopiedAudioBitRate::FromSource, 480, 240_000, 240_000), + ] { + assert_eq!( + muxer + .set_audio_bit_rate(index, policy, Some((samples, 48_000)), 0, &limits) + .unwrap(), + 0 + ); + let entry = &muxer.tracks[index].header.sample_entry; + let expected: Vec = [0u32, maximum, average] + .into_iter() + .flat_map(u32::to_be_bytes) + .collect(); + assert_eq!(&entry[entry.len() - 12..], &expected); + } + let before = muxer.tracks[index].header.sample_entry.clone(); muxer.tracks[index].tables.data_bytes = u64::MAX; muxer.tracks[index].tables.duration = 1; assert_eq!( muxer - .set_audio_bit_rate(index, 96_000, 20, &limits) + .set_audio_bit_rate( + index, + CopiedAudioBitRate::Requested(96_000), + None, + 20, + &limits + ) .unwrap(), 0 ); diff --git a/crates/coreav-isobmff-write/src/remux.rs b/crates/coreav-isobmff-write/src/remux.rs index aee7b3bb2..d52dd576f 100644 --- a/crates/coreav-isobmff-write/src/remux.rs +++ b/crates/coreav-isobmff-write/src/remux.rs @@ -896,13 +896,35 @@ fn remux( } .into()); } - if let Some(rate) = options.copied_audio_bit_rate { + if let Some(policy) = options.copied_audio_bit_rate { + let mut spare = limits + .max_sample_work + .saturating_sub(budget.used()) + .saturating_sub(copy_work); for (index, k) in kept.iter().enumerate() { - if k.replaced.is_none() && matches!(k.track.description.codec, Codec::Aac | Codec::Opus) - { - let growth = muxer.set_audio_bit_rate(index, rate, retained.left, limits)?; - retained.charge(growth, 0)?; + if k.replaced.is_some() { + continue; } + let clock = match k.track.description.codec { + Codec::Aac | Codec::Opus => None, + Codec::Mp3 if k.input == 0 && k.work <= spare => { + spare -= k.work; + let Some(clock) = mp3_clock( + &movie, + k.track, + &mut source, + limits, + k.work, + muxer.samples(index).unwrap_or(0), + ) else { + continue; + }; + Some(clock) + } + _ => continue, + }; + let growth = muxer.set_audio_bit_rate(index, policy, clock, retained.left, limits)?; + retained.charge(growth, 0)?; } } let (dropped_boxes, not_listed) = report(&file, &movie_gone, &kept, &retained); @@ -1050,6 +1072,49 @@ pub fn sanitize_mp4( remux_mp4(source, limits, allowlist, &options, out) } +fn mp3_clock( + movie: &Movie, + track: &Track, + source: &mut S, + limits: &Limits, + work: u64, + expected: u32, +) -> Option<(u64, u32)> { + let limits = Limits { + max_sample_work: work, + ..*limits + }; + let mut budget = Budget::new(&limits); + let mut packets = movie + .packets(track.index, source, &limits, &mut budget) + .ok()?; + let (mut clock, mut samples, mut count) = (None, 0u64, 0u32); + while let Some(packet) = packets.next() { + let packet = packet.ok()?; + if packet.size < 4 || count >= expected { + return None; + } + let mut bytes = [0; 4]; + packets + .source_mut() + .read_exact_at(packet.offset, &mut bytes) + .ok()?; + let header = coreav_audio::mp3::Header::parse(&bytes).ok()?; + if header.frame_bytes()? != usize::try_from(packet.size).ok()? { + return None; + } + let current = (header.version, header.sample_rate); + if clock.is_some_and(|previous| previous != current) { + return None; + } + clock = Some(current); + samples = samples.checked_add(u64::try_from(header.samples()).ok()?)?; + count = count.checked_add(1)?; + } + let (_, rate) = clock?; + (count == expected && samples != 0).then_some((samples, rate)) +} + fn dropped(track: &Track, reason: TrackDropReason) -> DroppedTrack { DroppedTrack { stream_index: track.stream_index, diff --git a/crates/coreav-isobmff-write/tests/copied_bitrate.rs b/crates/coreav-isobmff-write/tests/copied_bitrate.rs new file mode 100644 index 000000000..b30cf3831 --- /dev/null +++ b/crates/coreav-isobmff-write/tests/copied_bitrate.rs @@ -0,0 +1,288 @@ +//! Copied MP3 bitrate normalization and byte-identical unproved-clock fallbacks. +#![allow(clippy::unwrap_used, clippy::panic, clippy::indexing_slicing)] + +use coreav_isobmff::FourCc; +use coreav_isobmff::{Allowlist, Error, Limits, SliceSource, Source}; +use coreav_isobmff_write::mux::{ + FileType, MovieHeader, Muxer, RawBox, Sample, TrackHeader, TrackMedia, audio_sample_entry, +}; +use coreav_isobmff_write::{CopiedAudioBitRate, RemuxOptions, remux_mp4}; + +fn frame(version: u32, index: u32, rate_index: u32) -> Vec { + let (rate, bitrate, factor) = if version == 3 { + ( + [44_100u32, 48_000, 32_000][rate_index as usize], + if index == 5 { 64_000 } else { 128_000 }, + 144, + ) + } else { + (if version == 2 { 22_050 } else { 11_025 }, 64_000, 72) + }; + let word = (0x7FFu32 << 21) + | (version << 19) + | (1 << 17) + | (1 << 16) + | (index << 12) + | (rate_index << 10) + | (3 << 6); + let mut bytes = vec![0; (factor * bitrate / rate) as usize]; + bytes[..4].copy_from_slice(&word.to_be_bytes()); + bytes +} + +fn source(frames: &[Vec], rate: u32, frame_samples: u32, object: u8) -> Vec { + let mut esds = vec![0, 0, 0, 0, 3, 21, 0, 1, 0, 4, 13, object, 0x15]; + let rates: Vec = [1234u32, 220_000, 1000] + .into_iter() + .flat_map(u32::to_be_bytes) + .collect(); + esds.extend_from_slice(&rates[1..]); + esds.extend_from_slice(&[6, 1, 2]); + let entry = audio_sample_entry( + FourCc(*b"mp4a"), + 1, + 16, + rate, + &[ + RawBox { + kind: FourCc(*b"esds"), + payload: esds, + }, + RawBox { + kind: FourCc(*b"btrt"), + payload: rates, + }, + ], + ) + .unwrap(); + let mut muxer = Muxer::new( + FileType { + major: FourCc(*b"isom"), + minor: 0, + compatible: vec![FourCc(*b"isom")], + }, + MovieHeader { + timescale: 1000, + creation_time: 0, + boxes: Vec::new(), + }, + &Limits::default(), + ) + .unwrap(); + let track = muxer + .add_track(TrackHeader { + media: TrackMedia::Audio { volume: 0x100 }, + timescale: rate, + language: *b"und", + creation_time: 0, + media_creation_time: 0, + flags: 3, + layer: 0, + alternate_group: 0, + edits: Vec::new(), + duration: None, + media_duration: None, + sample_entry: entry, + sample_table_boxes: Vec::new(), + boxes: Vec::new(), + }) + .unwrap(); + for (index, bytes) in frames.iter().enumerate() { + muxer + .push_sample( + track, + Sample { + size: bytes.len() as u32, + duration: if index + 1 == frames.len() { + 433 + } else { + frame_samples + }, + sync: true, + composition_offset: 0, + }, + ) + .unwrap(); + } + let mut output = Vec::new(); + muxer.write_head(&mut output).unwrap(); + for bytes in frames { + output.extend_from_slice(bytes); + } + output +} + +fn remux(input: &[u8], rate: Option, limits: &Limits) -> Option> { + let mut output = Vec::new(); + remux_mp4( + SliceSource::new(input), + limits, + &Allowlist::default(), + &RemuxOptions { + copied_audio_bit_rate: rate.map(CopiedAudioBitRate::Requested), + ..RemuxOptions::default() + }, + &mut output, + ) + .ok()?; + Some(output) +} + +fn expected(control: &[u8], average: u32, maximum: u32) -> Vec { + let before: Vec = [1234u32, 220_000, 1000] + .into_iter() + .flat_map(u32::to_be_bytes) + .collect(); + let after: Vec = [0u32, maximum, average] + .into_iter() + .flat_map(u32::to_be_bytes) + .collect(); + let positions: Vec<_> = control + .windows(11) + .enumerate() + .filter_map(|(at, bytes)| (bytes == &before[1..]).then_some(at)) + .collect(); + assert_eq!(positions.len(), 2); + let mut output = control.to_vec(); + for at in positions { + output[at..at + 11].copy_from_slice(&after[1..]); + } + output +} + +#[test] +fn complete_mp3_frame_clocks_change_only_bitrate_fields() { + for (version, rate, frame_samples, object) in [ + (3, 48_000, 1152, 0x6B), + (2, 22_050, 576, 0x69), + (0, 11_025, 576, 0x69), + ] { + let frames: Vec<_> = (0..12) + .map(|index| { + frame( + version, + if version == 3 { + if index % 2 == 0 { 5 } else { 9 } + } else { + 8 + }, + u32::from(version == 3), + ) + }) + .collect(); + let input = source(&frames, rate, frame_samples, object); + let control = remux(&input, None, &Limits::default()).unwrap(); + let average = (frames.iter().map(Vec::len).sum::() as u64 * 8 * u64::from(rate) + / (12 * u64::from(frame_samples))) as u32; + for requested in [0, 16_000, 128_000] { + let actual = remux(&input, Some(requested), &Limits::default()).unwrap(); + assert_eq!(actual, expected(&control, average, average.max(requested))); + } + } +} + +#[test] +fn unproved_mp3_clocks_preserve_complete_files_without_refusal() { + let good = frame(3, 5, 1); + let compound = [good.clone(), good.clone()].concat(); + let mut free = good.clone(); + free[2] &= 0x0F; + let mut layer2 = good.clone(); + layer2[1] = (layer2[1] & !6) | 4; + for frames in [ + vec![good.clone(), compound], + vec![free], + vec![good[..good.len() - 1].to_vec()], + vec![good.clone(), frame(3, 5, 0)], + vec![good.clone(), frame(2, 8, 0)], + vec![layer2], + vec![vec![0; 192]], + vec![vec![0; 3]], + ] { + let input = source(&frames, 48_000, 1152, 0x6B); + let control = remux(&input, None, &Limits::default()).unwrap(); + assert_eq!( + remux(&input, Some(96_000), &Limits::default()).unwrap(), + control + ); + } +} + +#[test] +fn absent_request_inherits_the_stored_clock_not_declared_bitrate_fields() { + let input = source(&[frame(3, 5, 1), frame(3, 5, 1)], 48_000, 1152, 0x6B); + let control = remux(&input, None, &Limits::default()).unwrap(); + let mut output = Vec::new(); + remux_mp4( + SliceSource::new(&input), + &Limits::default(), + &Allowlist::default(), + &RemuxOptions { + copied_audio_bit_rate: Some(CopiedAudioBitRate::FromSource), + ..RemuxOptions::default() + }, + &mut output, + ) + .unwrap(); + let stored = 384 * 8 * 48_000 / (1152 + 433); + assert_eq!(output, expected(&control, 64_000, stored)); +} + +#[test] +fn a_clock_pass_without_spare_work_preserves_complete_files() { + let input = source(&[frame(3, 5, 1), frame(3, 5, 1)], 48_000, 1152, 0x6B); + for work in 0..1024 { + let limits = Limits { + max_sample_work: work, + ..Limits::default() + }; + if let Some(control) = remux(&input, None, &limits) { + assert_eq!(remux(&input, Some(96_000), &limits).unwrap(), control); + return; + } + } + panic!("no native two-pass work limit was found"); +} + +#[test] +fn an_optional_header_read_failure_preserves_the_native_output() { + struct Failing<'a> { + source: SliceSource<'a>, + offset: u64, + tripped: bool, + } + impl Source for Failing<'_> { + fn len(&mut self) -> Result { + self.source.len() + } + fn read_exact_at(&mut self, offset: u64, bytes: &mut [u8]) -> Result<(), Error> { + if offset == self.offset && bytes.len() == 4 { + self.tripped = true; + return Err(Error::Truncated { offset }); + } + self.source.read_exact_at(offset, bytes) + } + } + let frame = frame(3, 5, 1); + let input = source(&[frame.clone(), frame.clone()], 48_000, 1152, 0x6B); + let control = remux(&input, None, &Limits::default()).unwrap(); + let mut source = Failing { + source: SliceSource::new(&input), + offset: (input.len() - frame.len() * 2) as u64, + tripped: false, + }; + let mut actual = Vec::new(); + remux_mp4( + &mut source, + &Limits::default(), + &Allowlist::default(), + &RemuxOptions { + copied_audio_bit_rate: Some(CopiedAudioBitRate::Requested(96_000)), + ..RemuxOptions::default() + }, + &mut actual, + ) + .unwrap(); + assert!(source.tripped); + assert_eq!(actual, control); +} diff --git a/crates/coreav-isobmff/src/codec.rs b/crates/coreav-isobmff/src/codec.rs index 0a26c78f3..22fc1a6e6 100644 --- a/crates/coreav-isobmff/src/codec.rs +++ b/crates/coreav-isobmff/src/codec.rs @@ -1742,10 +1742,11 @@ pub fn esds_decoder_specific_info(esds: &[u8]) -> Option<(u8, Option<&[u8]>)> { } /// Offset of the complete eleven-byte buffer-size and bitrate fields in an -/// AAC `esds` payload. Other object types and incomplete fields return `None`. -pub fn aac_esds_bit_rate_offset(esds: &[u8]) -> Option { +/// MPEG-4 AAC or MPEG audio `esds` payload. Other object types and incomplete +/// fields return `None`. +pub fn esds_bit_rate_offset(esds: &[u8]) -> Option { let parts = esds_parts(esds)?; - (parts.object_type == 0x40) + matches!(parts.object_type, 0x40 | 0x69 | 0x6B) .then_some(parts.rate_offset) .flatten() } @@ -1860,27 +1861,33 @@ mod tests { let mut payload = vec![0, 0, 0, 0, 3, 25, 0, 1, 0, 4, 17, 0x40, 0x15]; payload.extend_from_slice(&[0; 11]); payload.extend_from_slice(&[5, 2, 0x11, 0x90, 6, 1, 2]); - assert_eq!(aac_esds_bit_rate_offset(&payload), Some(13)); + assert_eq!(esds_bit_rate_offset(&payload), Some(13)); assert_eq!( esds_decoder_specific_info(&payload), Some((0x40, Some(&[0x11, 0x90][..]))) ); for end in 0..payload.len() { - assert_eq!(aac_esds_bit_rate_offset(&payload[..end]), None, "{end}"); + assert_eq!(esds_bit_rate_offset(&payload[..end]), None, "{end}"); } let mut flags = payload.clone(); flags[5] += 8; flags[8] = 0xE0; flags.splice(9..9, [0, 2, 3, b'u', b'r', b'l', 0, 3]); - assert_eq!(aac_esds_bit_rate_offset(&flags), Some(21)); - payload[11] = 0x6B; - assert_eq!(aac_esds_bit_rate_offset(&payload), None); + assert_eq!(esds_bit_rate_offset(&flags), Some(21)); + for kind in [0x69, 0x6B] { + payload[11] = kind; + assert_eq!(esds_bit_rate_offset(&payload), Some(13)); + } + for kind in 0x66..=0x68 { + payload[11] = kind; + assert_eq!(esds_bit_rate_offset(&payload), None); + } let mut expanded = vec![ 0, 0, 0, 0, 3, 0x80, 0x80, 0x80, 28, 0, 1, 0, 4, 0x80, 0x80, 0x80, 17, 0x40, 0x15, ]; expanded.extend_from_slice(&[0; 11]); expanded.extend_from_slice(&[5, 2, 0x11, 0x90, 6, 1, 2]); - assert_eq!(aac_esds_bit_rate_offset(&expanded), Some(19)); + assert_eq!(esds_bit_rate_offset(&expanded), Some(19)); } #[test] diff --git a/crates/coreav-isobmff/src/lib.rs b/crates/coreav-isobmff/src/lib.rs index cee8c223f..c3b5425f2 100644 --- a/crates/coreav-isobmff/src/lib.rs +++ b/crates/coreav-isobmff/src/lib.rs @@ -39,10 +39,10 @@ pub use bitstream::{AvcCopyFacts, avc_copy_facts}; pub use codec::{ AacFrameConfig, Allowlist, Codec, CodecConfig, Color, ConfigRecord, DeclaredBitRate, DolbyVision, LosslessConfig, MAX_OTHER_BOXES, PcmFormat, ProResProfile, Profile, - SampleDescription, aac_esds_bit_rate_offset, aac_frame_config, aac_has_explicit_no_sbr, - avc_max_num_ref_frames, avc_max_num_ref_frames_with_planning, avc_parameter_sets, - avc_parameter_sets_with_planning, avc_timing, describe_audio_specific_config, - describe_config_record, describe_config_record_with_planning, esds_decoder_specific_info, + SampleDescription, aac_frame_config, aac_has_explicit_no_sbr, avc_max_num_ref_frames, + avc_max_num_ref_frames_with_planning, avc_parameter_sets, avc_parameter_sets_with_planning, + avc_timing, describe_audio_specific_config, describe_config_record, + describe_config_record_with_planning, esds_bit_rate_offset, esds_decoder_specific_info, hevc_window_empty, lossless_config, parameter_set_color, parameter_set_color_with_planning, }; pub use error::{Error, LimitKind, Warning}; diff --git a/crates/coreav-policy/fuzz/Cargo.lock b/crates/coreav-policy/fuzz/Cargo.lock index 7eb97fbd2..04d5cfe60 100644 --- a/crates/coreav-policy/fuzz/Cargo.lock +++ b/crates/coreav-policy/fuzz/Cargo.lock @@ -65,6 +65,7 @@ dependencies = [ "coreav-mpegts", "coreav-policy", "coreav-text", + "mimalloc", "rustix", "serde_json", ] @@ -91,6 +92,7 @@ dependencies = [ name = "coreav-isobmff-write" version = "0.0.1" dependencies = [ + "coreav-audio", "coreav-isobmff", "coreav-mkv", "coreav-mpegts", @@ -207,6 +209,15 @@ dependencies = [ "cc", ] +[[package]] +name = "libmimalloc-sys" +version = "0.1.49" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a45a52f43e1c16f667ccfe4dd8c85b7f7c204fd5e3bf46c5b0db9a5c3c0b8e9" +dependencies = [ + "cc", +] + [[package]] name = "linux-raw-sys" version = "0.12.1" @@ -219,6 +230,15 @@ version = "2.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" +[[package]] +name = "mimalloc" +version = "0.1.52" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d4139bb28d14ad1facf21d5eb8825051b326e172d216b39f6d31df53cc97862" +dependencies = [ + "libmimalloc-sys", +] + [[package]] name = "miniz_oxide" version = "0.8.9" diff --git a/crates/coreav-transcode/fuzz/Cargo.lock b/crates/coreav-transcode/fuzz/Cargo.lock index 4cdfd1000..2671fb973 100644 --- a/crates/coreav-transcode/fuzz/Cargo.lock +++ b/crates/coreav-transcode/fuzz/Cargo.lock @@ -83,6 +83,7 @@ dependencies = [ name = "coreav-isobmff-write" version = "0.0.1" dependencies = [ + "coreav-audio", "coreav-isobmff", "coreav-mkv", "coreav-mpegts", @@ -157,6 +158,7 @@ version = "0.0.1" dependencies = [ "coreav-decode", "coreav-frame", + "libc", "memmap2", "rustix", ] diff --git a/crates/coreav-transcode/src/pipeline.rs b/crates/coreav-transcode/src/pipeline.rs index a646924b7..8bfdca05d 100644 --- a/crates/coreav-transcode/src/pipeline.rs +++ b/crates/coreav-transcode/src/pipeline.rs @@ -142,8 +142,9 @@ pub struct TranscodeOptions { /// Use the FFmpeg-compatible terminal AAC delta in MP4 output. /// The default retains complete frame durations and native transcode bytes. pub aac_mp4_tail: bool, - /// Optional metadata bitrate for copied AAC and Opus in MP4. - pub copied_audio_bit_rate: Option, + /// Optional metadata bitrate for MP4-sourced copied AAC, MP3 and Opus. + /// `None` preserves metadata rather than normalizing from the source clock. + pub copied_audio_bit_rate: Option, /// The cover pictures of an MP4 source to leave out, as /// `RemuxOptions::left_out_covers` names them; the others are copied /// with the movie's metadata. diff --git a/crates/coreav-wasm/fuzz/Cargo.lock b/crates/coreav-wasm/fuzz/Cargo.lock index 72054cf10..ff515d3a5 100644 --- a/crates/coreav-wasm/fuzz/Cargo.lock +++ b/crates/coreav-wasm/fuzz/Cargo.lock @@ -71,6 +71,7 @@ dependencies = [ "coreav-mpegts", "coreav-policy", "coreav-text", + "mimalloc", "rustix", "serde_json", ] @@ -97,6 +98,7 @@ dependencies = [ name = "coreav-isobmff-write" version = "0.0.1" dependencies = [ + "coreav-audio", "coreav-isobmff", "coreav-mkv", "coreav-mpegts", @@ -226,6 +228,15 @@ dependencies = [ "cc", ] +[[package]] +name = "libmimalloc-sys" +version = "0.1.49" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a45a52f43e1c16f667ccfe4dd8c85b7f7c204fd5e3bf46c5b0db9a5c3c0b8e9" +dependencies = [ + "cc", +] + [[package]] name = "linux-raw-sys" version = "0.12.1" @@ -238,6 +249,15 @@ version = "2.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" +[[package]] +name = "mimalloc" +version = "0.1.52" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d4139bb28d14ad1facf21d5eb8825051b326e172d216b39f6d31df53cc97862" +dependencies = [ + "libmimalloc-sys", +] + [[package]] name = "miniz_oxide" version = "0.8.9" diff --git a/docs/agents/containers.md b/docs/agents/containers.md index 9bd678248..d3ed37fff 100644 --- a/docs/agents/containers.md +++ b/docs/agents/containers.md @@ -4,6 +4,7 @@ ISO BMFF, Matroska, MPEG-TS, and AVI. | Rule | Enforced by | |---|---| +| Copied-audio bitrate normalization distinguishes absent and explicit-zero requests and preserves unproved descriptions or MP3 clocks without new refusals | The shared policy type separates source-clock hints from explicit requests; the writer owns a spare-work-only MP3 header pass. Complete-file oracle controls cover default, CBR and VBR paths; synthetic fallback and descriptor/version tests pin unchanged output for unproved cases. Codec labels alone cannot prove a packet's complete frame clock | | An audio-bearing input-seek summary reports the edit's presentation origin, not the earlier decode access point | `clip::input_seek` uses the requested start for both the edit origin and `snapped_start`; `audio_bearing_input_seek_plans_complete_samples_before_writing` checks that summary alongside the retained sample ranges and serialized edits. A rational type cannot distinguish these two source-clock positions | | A joined sample-entry child is reported as omitted only when the writer did not retain it | `join_mp4` derives `fiel` omission reporting from the same optional two-byte declaration used by the visual-entry builder. The complete native-video regression checks both serialized output and CLI summary, retains absent/short declaration reporting, and `h264_field_metadata_still_reports_its_omission` preserves the excluded codec boundary. A source-only list cannot establish what the output copied | | Native AV1, VP9 and HEVC joins retain the selected first input's visual declarations without changing admission or edit presentation | The existing bounded scan retains one qualified `fiel` header independently of its summary-list cap, and `visual_sample_entry` owns compressor-name and two-byte field serialization. `scan::tests` covers missing/short metadata and box limits; `native_video_metadata_compares_complete_records_and_all_packet_bytes` checks all three codecs, differing/absent declarations, native execution with unavailable fallback, complete records and mutation rejection. External source metadata and oracle packets require runtime checks rather than a codec enum | diff --git a/docs/design/copied-audio-bitrate.md b/docs/design/copied-audio-bitrate.md index f4d7d9c54..023ee23ff 100644 --- a/docs/design/copied-audio-bitrate.md +++ b/docs/design/copied-audio-bitrate.md @@ -6,7 +6,7 @@ The writer derives the average from complete copied sample tables as `data_bytes ## Ownership -The ffmpeg adapter parses the existing unsigned integer and decimal SI-suffix grammar without introducing a new error. Text that does not parse or fit the 32-bit bitrate fields retains its previous ignored behavior. Native `coreav transcode` and the ordinary remux entry points default to no override. The request passes the optional rate to the shared transcode options and then only to the MP4-source remux options. Matroska-source copies keep their existing output byte for byte. +The ffmpeg adapter parses the existing unsigned integer and decimal SI-suffix grammar without introducing a new error. Missing, malformed and out-of-range requests use the absent-request policy. Native `coreav transcode` and the ordinary remux entry points default to no override. The request passes the shared policy to the transcode options and then only to the MP4-source remux options. Matroska-source copies keep their existing output byte for byte. The [MP3 extension](copied-mp3-bitrate.md) distinguishes the source-clock hint from an explicit zero request. After all samples of a copied AAC or Opus track have been listed, a single `Muxer` operation updates its sample entry from the completed tables. Encoded replacements, video and copied MP3 never enter this operation. The `esds` parser remains owned by `coreav-isobmff`; the writer obtains validated rate-field offsets from that owner rather than adding another descriptor parser. Existing `btrt` fields are updated in place, or one bounded `btrt` is appended when absent. Added bytes stay within the writer's existing box and retained-metadata allowances. An incomplete or malformed entry, insufficient allowance or an unrepresentable average leaves the complete entry unchanged rather than adding a refusal. @@ -18,4 +18,4 @@ Generated pinned-oracle references cover MP4-sourced AAC and Opus, low and high Matroska copy-clock parity is a known gap, not a passing oracle case. A generated AAC witness has 13,312 stored ticks in CoreAV and 13,000 in FFmpeg, yielding different averages for the same packets. Opus also differs. Replacing only the metadata would not reconcile those clocks, so both the Matroska tables and its existing metadata remain unchanged in this change. Exact Matroska copy-clock parity is a separate follow-up. -The copied MP3 bitrate clock is a separate follow-up. In a generated control, its stored durations imply 67,511 bits/s, while the pinned oracle's complete frame clock implies 64,000. Applying the AAC/Opus formula to it would replace one incorrect value with another; this change does not guess frame timing or add packet-header admission. +Copied MP3 uses the separately verified [complete-frame clock](copied-mp3-bitrate.md), not the AAC/Opus table formula. Unproved frame clocks keep their metadata without changing command admission. diff --git a/docs/design/copied-mp3-bitrate.md b/docs/design/copied-mp3-bitrate.md index c58121464..6c60617ef 100644 --- a/docs/design/copied-mp3-bitrate.md +++ b/docs/design/copied-mp3-bitrate.md @@ -6,13 +6,15 @@ MP4-sourced MP3 copied beside encoded video retains source bitrate fields today. The writer reuses `coreav_audio::mp3::Header` rather than adding a second MPEG audio header parser. A metadata-only packet pass requires one complete Layer III frame per MP4 sample, consistent MPEG version and sample rate, and the same sample count as the completed output tables. It sums 1,152 samples per MPEG-1 frame or 576 per MPEG-2 frame. The existing parser also supports the non-ISO MPEG-2.5 extension. The average is `packet_bytes * 8 * sample_rate / decoded_frame_samples`, rounded down. It uses actual frame bytes, including padding: a generated 44.1 kHz constant-rate control averages 63,978 rather than its nominal header rate of 64,000. -ISO/IEC 11172-3 and 13818-3 establish the frame sample counts. The choice between complete-frame and shortened stored clocks for bitrate hints is an oracle compatibility policy, not a claim that the specifications forbid the stored durations. The maximum remains the larger of the average and the requested rate. Buffer size is zero. Neither packets nor sample tables change. +ISO/IEC 11172-3 and 13818-3 establish the frame sample counts. The choice between complete-frame and shortened stored clocks for bitrate hints is an oracle compatibility policy, not a claim that the specifications forbid the stored durations. The maximum is at least the average. An explicit request supplies its other hint; otherwise the source's stored-clock average supplies it. Buffer size is zero. Neither packets nor sample tables change. The extra packet pass uses only work left after the existing listing and copy passes. Insufficient work, malformed headers, partial or compound samples, free format, inconsistent clocks and source errors leave the complete entry unchanged. They never add a refusal. The writer depends on the existing audio crate, which the CLI and both bindings already use; no decoder implementation changes. ## Default and excluded paths -For the ffmpeg adapter's already-native video-encode route, no requested bitrate is represented by zero: the maximum equals the average. Missing, malformed and out-of-range `-b:a` values all use that default. This also normalizes copied AAC and Opus metadata when `-b:a` is absent, as the pinned oracle does for stale or missing fields. Native transcode and ordinary remux retain `None` and their existing bytes. +The ffmpeg adapter's already-native video-encode route distinguishes no request from an explicit zero. The shared `CopiedAudioBitRate` type has `FromSource` and `Requested(u32)` states; outer `None` preserves native metadata. Missing, malformed and out-of-range `-b:a` values use `FromSource`. A generated MP3 control without `-b:a` writes maximum 67,511 and average 64,000; an explicit zero writes 64,000 for both. This also normalizes copied AAC and Opus metadata when `-b:a` is absent, as the pinned oracle does for stale or missing fields. Native transcode and ordinary remux retain `None` and their existing bytes. + +Ignored values are not an oracle-parity claim. In a generated AAC control, FFmpeg wraps a request of 4,294,967,296 into maximum zero, while CoreAV retains its absent-request policy. Previously ignored values still introduce no refusal. Matroska input and output remain unchanged. MPEG-2 AAC object types and QuickTime version-1/version-2 sound entries remain explicit, tested skips rather than new admission checks. Exact Matroska copy-clock parity is separate work. From aa1b7fdadc4831f218111c93135704fc9a831ec1 Mon Sep 17 00:00:00 2001 From: Gunwoo Kim Date: Sun, 8 Nov 2026 02:28:21 +0900 Subject: [PATCH 3/3] test(cli): sync copied audio oracle selection Signed-off-by: Gunwoo Kim --- crates/coreav-cli/tests/ffmpeg_audio_options.rs | 2 +- docs/agents/ci.md | 2 +- docs/agents/ffmpeg-cli.md | 2 +- docs/design/copied-audio-bitrate.md | 4 ++-- scripts/tests/test_oracle_free.py | 10 ++++++++++ test-support/oracle-dependent.json | 7 ++++++- 6 files changed, 21 insertions(+), 6 deletions(-) diff --git a/crates/coreav-cli/tests/ffmpeg_audio_options.rs b/crates/coreav-cli/tests/ffmpeg_audio_options.rs index ef1519550..24723a2f2 100644 --- a/crates/coreav-cli/tests/ffmpeg_audio_options.rs +++ b/crates/coreav-cli/tests/ffmpeg_audio_options.rs @@ -870,7 +870,7 @@ fn copied_mp3_matches_oracle_metadata_with_and_without_bitrate() { std::fs::rename(encoded, dir.0.join("source.mp4")).unwrap(); let control = native_transcode_control(&dir.0); assert_eq!(audio_entries(&control).len(), 2); - for rate in [None, Some("16k"), Some("96k"), Some("128k")] { + for rate in [None, Some("0"), Some("16k"), Some("96k"), Some("128k")] { let mut args = vec![ "-map", "0:v", "-map", "0:a", "-c:v", "libx264", "-threads", "1", "-c:a", "aac", "-c:a", "copy", diff --git a/docs/agents/ci.md b/docs/agents/ci.md index 86bea1acf..5e695520c 100644 --- a/docs/agents/ci.md +++ b/docs/agents/ci.md @@ -55,7 +55,7 @@ List development caches with `docker volume ls --filter name=coreav-dev-`. After | Manual-only HEVC C3 differential fuzz remains outside push/schedule C1 and C2 coverage without losing its dispatch failure gate | `test_manual_only_jobs_never_run_on_automatic_events` closes the exception to named jobs and tests both crons, all automatic events and scalar/mode inputs. The full-evidence test requires exact C1/C2 membership plus exactly two automatic-mode dispatch C3 jobs; reporter/verifier tests retain covered-skip handling and reject a failed differential workflow. Separate event scopes cannot share one unconditional proof roster | | The macOS crop case executes real media with qualified tools rather than passing with zero selected tests | The explicit `check-macos` step requires the exact crop test success line after oracle and x264 provisioning. `test_macos_crop_runs_real_media_with_qualified_tools` checks its command and rejects zero-test and failed-test logs; Rust types cannot select workflow features or prove execution | | The macOS SAR media case actually runs with its qualified tools rather than disappearing in an oracle-free workspace build | The explicit step in `check-macos` installs pinned x264 after oracle provisioning, selects the native-H.264 media test and requires its success line, so zero tests fail. `test_macos_runs_the_declared_sar_media_case_with_qualified_tools` pins the command, success witness, required-oracle environment and provisioning order; Rust types cannot select Cargo features or prevent a zero-test workflow invocation | -| Every test in the input-seek oracle fixture target is excluded on unregistered oracle platforms | The shared `test-support/oracle-dependent.json` roster owns the exact selection; `test_input_seek_oracle_tests_are_excluded_on_unregistered_platforms` derives the target's tests and checks each skip. Rust types cannot select test functions in a platform's Cargo invocation | +| Every test in the fixture-only input-seek and audio-option targets is excluded on unregistered oracle platforms | The shared `test-support/oracle-dependent.json` roster owns exact selection. The input-seek and audio-option coverage tests in `test_oracle_free.py` derive each target's real tests and require every skip; the identity test rejects stale renamed entries. Rust types cannot select test functions in a platform's external Cargo invocation | | Conformance archive retries recognize dropped connections even when the exception has no errno | `fetch` classifies `ConnectionError` alongside transient errno values; `test_transient_errors_recover_with_backoff_and_source_logs` covers raw and URL-wrapped `RemoteDisconnected`. External exceptions require runtime classification, not only an errno list | | Unix socket fixtures fit macOS's 104-byte socket-address path even under long checkout or temporary-directory paths | `wav_non_regular_destinations_refuse_without_staging_or_replacement` and `output::tests::special_destinations_are_never_staged_or_replaced` each create one exclusive mode-0700 directory under a short Unix root and assert the encoded socket path is shorter than 104 bytes before binding. The bounded construction removes both external prefixes; Rust's `Path` type cannot constrain encoded address length | | AAC qualification keeps its complete JSON report even when the comparison fails | The existing always-run artifact upload owns retention; `test_aac_qualification_report_survives_failure` checks its condition and report path. Rust types cannot govern workflow artifact paths | diff --git a/docs/agents/ffmpeg-cli.md b/docs/agents/ffmpeg-cli.md index 69b061ca4..6358e1661 100644 --- a/docs/agents/ffmpeg-cli.md +++ b/docs/agents/ffmpeg-cli.md @@ -6,7 +6,7 @@ CLI and ffprobe behavior, the FFmpeg pin, and fallback names. |---|---| | Newly admitted default-video options match the pinned oracle's decoded frames and aspect, not merely the existing explicit encoder path | The MP4 parser keeps implicit scale and bitrate on fallback; `transcode_job` admits implicit output size only when source dimensions match. `ffmpeg_default_video_encoder_applies_options` compares complete frame hashes and geometry/aspect with the oracle and checks unchanged destinations plus byte-exact fallback for excluded paths. Runtime source dimensions and external output cannot be established by argument types | | Generated default-video option coverage treats x264 as optional only outside required verification, without requiring unrelated fixtures | `ffmpeg_default_video_encoder_applies_options` checks the shared `required()` policy when x264 is absent. Its explicit macOS CI step sets `COREAV_REQUIRE_FFMPEG=1` and requires the exact test-success witness. Filesystem availability is a runtime fact, so a type cannot decide whether the selected executable exists | -| New default AAC bitrate admission requires effective selected AAC encoding without removing pre-existing explicit-encoder command admission | `non_pcm_audio_options` checks the resolved audio codec only for the newly admitted implicit-encoder case. `default_bitrate_admission_preserves_legacy_native_commands` covers copied and absent audio, ordered and unmatched overrides, and destination preservation. A syntax type cannot establish selected source streams | +| New default AAC bitrate admission requires effective selected AAC encoding without removing pre-existing explicit-encoder command admission | `non_pcm_audio_options` checks the resolved audio codec only for the newly admitted implicit-encoder case. `copied_bitrate_metadata_preserves_command_admission` covers copied and absent audio, ordered and unmatched overrides, and destination preservation. A syntax type cannot establish selected source streams | | A strict-safe default AAC bitrate refusal checks its resolved external-encoder policy rather than an obsolete parser diagnostic | `ffmpeg_libopus_lines_encode_the_audio` retains the same command and exit 4 assertion and requires the complete AAC stderr, so an earlier x264 refusal with the shared FFmpeg prefix cannot pass. Unsupported hosts retain the exact memory-limit refusal. Syntax types cannot establish which runtime policy refusal a command reaches | | Shell oracle consumers retain the complete pair and reject qualification or response failures before creating output, including under macOS's Bash 3.2 | The shared owner's two-line CLI and plain assignment own qualification; guarded array expansions and read loops avoid newer Bash builtins and nounset behavior. Shell regressions exercise malformed responses, retained paths, explicit overrides and audio arguments without `mapfile`; they default to `/bin/bash` and accept `COREAV_TEST_BASH` for legacy-interpreter checks. A path type cannot propagate shell exit status or establish an external interpreter's behavior | | Invocation evidence is exact-site, source-bound and dependency-bound, including import-candidate absence and same-file fold severity | The classifier's single binding owner normalizes checkout roots and binds reviewed inward links to their literal text and tracked regular targets. Its schema validator and freshness controls reject changed targets, retargeting, outward or indirect links, stale files, import shadows, missing relay dependencies, duplicate or dead entries and weaker fold origins. Observer-copy status prevents a development source from inheriting the candidate's registered-source exclusion. The closure, source-span and proposal regressions exercise these external-file facts; a schema type alone cannot establish current checkout bytes | diff --git a/docs/design/copied-audio-bitrate.md b/docs/design/copied-audio-bitrate.md index 023ee23ff..02e087787 100644 --- a/docs/design/copied-audio-bitrate.md +++ b/docs/design/copied-audio-bitrate.md @@ -8,13 +8,13 @@ The writer derives the average from complete copied sample tables as `data_bytes The ffmpeg adapter parses the existing unsigned integer and decimal SI-suffix grammar without introducing a new error. Missing, malformed and out-of-range requests use the absent-request policy. Native `coreav transcode` and the ordinary remux entry points default to no override. The request passes the shared policy to the transcode options and then only to the MP4-source remux options. Matroska-source copies keep their existing output byte for byte. The [MP3 extension](copied-mp3-bitrate.md) distinguishes the source-clock hint from an explicit zero request. -After all samples of a copied AAC or Opus track have been listed, a single `Muxer` operation updates its sample entry from the completed tables. Encoded replacements, video and copied MP3 never enter this operation. The `esds` parser remains owned by `coreav-isobmff`; the writer obtains validated rate-field offsets from that owner rather than adding another descriptor parser. Existing `btrt` fields are updated in place, or one bounded `btrt` is appended when absent. Added bytes stay within the writer's existing box and retained-metadata allowances. An incomplete or malformed entry, insufficient allowance or an unrepresentable average leaves the complete entry unchanged rather than adding a refusal. +After all copied samples have been listed, a single `Muxer` operation updates AAC and Opus sample entries from the completed tables, or MP3 entries from a separately proved frame clock. Encoded replacements and video never enter this operation. The `esds` parser remains owned by `coreav-isobmff`; the writer obtains validated rate-field offsets from that owner rather than adding another descriptor parser. Existing `btrt` fields are updated in place, or one bounded `btrt` is appended when absent. Added bytes stay within the writer's existing box and retained-metadata allowances. An incomplete or malformed entry, insufficient allowance or an unrepresentable average leaves the complete entry unchanged rather than adding a refusal. No packet, sample table, edit, priming field, tag or top-level layout policy changes. Matroska output ignores the MP4 metadata option. The complete legacy native-transcode comparison remains unchanged. ## Verification -Generated pinned-oracle references cover MP4-sourced AAC and Opus, low and high rates, multiple copied tracks, stale fields, missing `btrt`, and overridden codec selections. Tests compare each complete output file with a native control that has only the independently observed oracle metadata applied, including dependent box sizes and chunk offsets. This preserves packet bytes, timing, tags and layout rather than hiding their differences through normalization. Raw CoreAV and FFmpeg files still differ in the deliberately unchanged tags and layout. Default, invalid, out-of-range and absent-audio controls preserve native output and command admission. Copied MP3 and Matroska-sourced audio output remain byte-identical. +Generated pinned-oracle references cover MP4-sourced AAC and Opus, low and high rates, multiple copied tracks, stale fields, missing `btrt`, and overridden codec selections. Tests compare each complete output file with a native control that has only the independently observed oracle metadata applied, including dependent box sizes and chunk offsets. This preserves packet bytes, timing, tags and layout rather than hiding their differences through normalization. Raw CoreAV and FFmpeg files still differ in the deliberately unchanged tags and layout. Absent and ignored bitrate requests normalize copied metadata without changing admission; proved copied MP3 is normalized too. Native transcode, ordinary remux and Matroska-sourced audio keep their existing bytes. Matroska copy-clock parity is a known gap, not a passing oracle case. A generated AAC witness has 13,312 stored ticks in CoreAV and 13,000 in FFmpeg, yielding different averages for the same packets. Opus also differs. Replacing only the metadata would not reconcile those clocks, so both the Matroska tables and its existing metadata remain unchanged in this change. Exact Matroska copy-clock parity is a separate follow-up. diff --git a/scripts/tests/test_oracle_free.py b/scripts/tests/test_oracle_free.py index cef7877d2..45e7f7e24 100644 --- a/scripts/tests/test_oracle_free.py +++ b/scripts/tests/test_oracle_free.py @@ -39,6 +39,16 @@ def test_input_seek_oracle_tests_are_excluded_on_unregistered_platforms(self): for name in names: self.assertIn(name, skipped, 'Register the oracle test in test-support/oracle-dependent.json') + def test_audio_options_fixture_tests_are_excluded_on_unregistered_platforms(self): + source = (ROOT / 'crates/coreav-cli/tests/ffmpeg_audio_options.rs').read_text() + names = re.findall(r'#\[test\]\s*fn\s+(\w+)', source) + self.assertTrue(names) + args = oracle_free.command(['cargo', 'test', '-p', 'coreav-cli', + '--test', 'ffmpeg_audio_options']) + skipped = {args[i + 1] for i, arg in enumerate(args[:-1]) if arg == '--skip'} + for name in names: + self.assertIn(name, skipped, 'Register the oracle test in test-support/oracle-dependent.json') + if __name__ == '__main__': unittest.main() diff --git a/test-support/oracle-dependent.json b/test-support/oracle-dependent.json index 523a2c3f1..48689dce7 100644 --- a/test-support/oracle-dependent.json +++ b/test-support/oracle-dependent.json @@ -74,8 +74,13 @@ "aac_filter_is_an_exact_concat_identity_or_preserved_fallback" ], "crates/coreav-cli/tests/ffmpeg_audio_options.rs": [ + "aac_terminal_timing_matches_the_complete_oracle_boxes", + "copied_aac_and_opus_match_oracle_metadata_without_other_file_changes", + "copied_bitrate_metadata_preserves_command_admission", + "copied_mp3_matches_oracle_metadata_with_and_without_bitrate", "identity_audio_options_and_default_aac_bitrate_run_natively", - "default_bitrate_admission_preserves_legacy_native_commands", + "ignored_copy_bitrates_match_the_native_default", + "matroska_copy_bitrate_keeps_complete_native_files", "nonidentity_and_copy_audio_options_preserve_destinations" ], "crates/coreav-cli/tests/ffmpeg_concat.rs": [