fix(mp4): match copied MP3 bitrate metadata

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,

crates/coreav-cli/src/transcode.rs

@@ -103,8 +103,9 @@ pub struct Request {
     pub aac: Option<AacRequest>,
     /// 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<u32>,
+    /// 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<coreav_isobmff_write::CopiedAudioBitRate>,
     /// Optional actual AAC-encoder stdin capture.
     #[cfg(feature = "input-capture")]
     pub input_capture: Option<coreav_transcode::capture::Target>,

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<u8> {
+    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("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",
+                ];
+                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");

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" }

crates/coreav-isobmff-write/src/audio.rs

@@ -15,7 +15,7 @@ pub(crate) fn set_bit_rate(
     room: u64,
     box_limit: u64,
 ) -> Option<u64> {
-    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<u8> = [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<u8> {