fix(mkv): trim copied Opus by its edit, not PreSkip

crates/coreav-cli/tests/ffmpeg_matroska.rs

@@ -460,6 +460,62 @@ fn iso_opus_keeps_the_declared_edit_end_instead_of_decoding_the_padded_tail() {
     );
 }
 
+#[test]
+fn iso_opus_edit_starting_before_preskip_keeps_its_leading_samples() {
+    let Some((ffmpeg, _)) = tools() else {
+        return;
+    };
+    let dir = scratch("iso-opus-media-time");
+    let source = dir.join("source.mp4");
+    run(Command::new(&ffmpeg)
+        .args([
+            "-v",
+            "error",
+            "-f",
+            "lavfi",
+            "-i",
+            "sine=frequency=440:sample_rate=48000:duration=0.24",
+            "-c:a",
+            "libopus",
+            "-y",
+        ])
+        .arg(&source));
+    let original = std::fs::read(&source).unwrap();
+    let body = |name: &[u8]| original.windows(4).position(|w| w == name).unwrap() + 4;
+    let (elst, dops) = (body(b"elst"), body(b"dOps"));
+    assert_eq!(original[elst..elst + 8], [0, 0, 0, 0, 0, 0, 0, 1]);
+    assert_eq!(original[elst + 12..elst + 16], 312u32.to_be_bytes());
+    assert_eq!(original[dops + 2..dops + 4], 312u16.to_be_bytes());
+    let variant = |name: &str, edit: &dyn Fn(&mut Vec<u8>)| {
+        let mut bytes = original.clone();
+        edit(&mut bytes);
+        let path = dir.join(name);
+        std::fs::write(&path, bytes).unwrap();
+        path
+    };
+    let edited = variant("edited.mp4", &|b| b[elst + 12..elst + 16].fill(0));
+    let complete = variant("complete.mp4", &|b| {
+        b[elst - 12..elst - 8].copy_from_slice(b"free");
+        b[dops + 2..dops + 4].fill(0);
+    });
+    let complete = decoded(&ffmpeg, &complete, true);
+    let copy = |input: &Path, name: &str| {
+        let output = dir.join(name);
+        success(core(input, &output, &["-c", "copy"], &ffmpeg, true));
+        decoded(&ffmpeg, &output, true)
+    };
+    assert_eq!(copy(&source, "control.mkv"), complete[312 * 2..11_832 * 2]);
+    let actual = copy(&edited, "actual.mkv");
+    assert_eq!(actual, complete[..11_520 * 2]);
+    let copied = dir.join("oracle.mkv");
+    run(Command::new(&ffmpeg)
+        .args(["-v", "error", "-i"])
+        .arg(&edited)
+        .args(["-c", "copy"])
+        .arg(&copied));
+    assert_eq!(decoded(&ffmpeg, &copied, true), actual[312 * 2..]);
+}
+
 #[cfg(feature = "native-h264")]
 #[test]
 fn h264_transcode_writes_matroska_with_copied_audio() {

crates/coreav-isobmff-write/src/mkv_output/source.rs

@@ -130,7 +130,11 @@ pub(super) fn isobmff<S: Source>(
         };
         let mut trim =
             u64::try_from(media).map_err(|_| unsupported("a negative media edit time"))?;
-        if replacement.is_none() && d.codec == coreav_isobmff::Codec::Opus && !preserve_opus_edit {
+        if replacement.is_none()
+            && d.codec == coreav_isobmff::Codec::Opus
+            && !preserve_opus_edit
+            && t.edit_list == EditList::None
+        {
             let skip = header
                 .private
                 .get(10..12)
@@ -143,14 +147,28 @@ pub(super) fn isobmff<S: Source>(
             if ticks % 48_000 != 0 {
                 return Err(unsupported("Opus pre-skip is not a whole media tick"));
             }
-            trim = trim.max(ticks / 48_000);
+            trim = ticks / 48_000;
         }
+        let mut shift = Rational::from_int(-i128::from(trim));
         if replacement.is_none() && t.kind == TrackKind::Audio {
-            config::set_delay(&mut header, trim, t.timescale)?;
+            let scale = u64::from(t.timescale);
+            if d.codec == coreav_isobmff::Codec::Opus && t.edit_list != EditList::None && scale != 0
+            {
+                let samples = trim
+                    .checked_mul(96_000)
+                    .and_then(|v| v.checked_add(scale))
+                    .ok_or_else(overflow)?
+                    / (2 * scale);
+                config::set_delay(&mut header, samples, 48_000)?;
+                shift = Rational::new(-i128::from(samples) * i128::from(scale), 48_000)
+                    .ok_or_else(overflow)?;
+            } else {
+                config::set_delay(&mut header, trim, t.timescale)?;
+            }
         }
         let offset = t
             .presentation_offset(movie.timescale)
-            .and_then(|v| v.checked_add(Rational::from_int(-i128::from(trim))))
+            .and_then(|v| v.checked_add(shift))
             .ok_or_else(overflow)?;
         let end = t
             .presentation(movie.timescale)

crates/coreav-isobmff-write/src/mkv_output/tests.rs

@@ -439,7 +439,7 @@ fn opus_edit_fixture(media_time: u32) -> Fixture {
 }
 
 #[test]
-fn native_video_cadence_preserves_iso_opus_edit_presentation_without_changing_ordinary_trim() {
+fn native_video_cadence_and_ordinary_copy_preserve_iso_opus_edit_presentation() {
     for media_time in [0, 960] {
         let fixture = opus_edit_fixture(media_time);
         let options = OutputOptions {
@@ -476,19 +476,10 @@ fn native_video_cadence_preserves_iso_opus_edit_presentation_without_changing_or
                 &mut native,
             )
             .unwrap();
-        if media_time >= 312 {
-            assert_eq!(ordinary, native);
-        } else {
-            assert_ne!(ordinary, native);
-        }
-        for (bytes, trim, presented_samples) in [
-            (
-                &ordinary,
-                media_time.max(312),
-                if media_time == 0 { 7680 } else { 7992 },
-            ),
-            (&native, media_time, 7992),
-        ] {
+        assert_eq!(ordinary, native);
+        for (bytes, trim, presented_samples) in
+            [(&ordinary, media_time, 7992), (&native, media_time, 7992)]
+        {
             let segment = probe(bytes);
             let header = &segment.tracks[1];
             let delay = u64::from(trim) * 1_000_000_000 / 48_000;

crates/coreav-isobmff-write/tests/mkv_output.rs

@@ -391,12 +391,18 @@ fn duplicate_selections_share_the_total_packet_limit() {
     ));
 }
 
-fn opus_movie(media_time: i32) -> Vec<u8> {
-    let mut spec = movies::aac_audio(5, vec![(40, media_time)]);
+/// Opus packets of 960 samples on a media clock of `timescale`, dOps
+/// PreSkip 312, and edits of 48 kHz movie ticks.
+fn opus_movie(timescale: u32, packets: usize, edits: Vec<(u32, i32)>) -> Vec<u8> {
+    let mut spec = movies::aac_audio(packets, edits);
+    spec.timescale = timescale;
     for sample in &mut spec.samples {
-        sample.duration = 960;
+        sample.duration = 960 * timescale / 48_000;
     }
     let mut input = movies::movie(&[spec]);
+    let mvhd = input.windows(4).position(|v| v == b"mvhd").unwrap();
+    input[mvhd + 16..mvhd + 20].copy_from_slice(&48_000u32.to_be_bytes());
+    input[mvhd + 20..mvhd + 24].copy_from_slice(&(packets as u32 * 960).to_be_bytes());
     let entry = input.windows(4).position(|v| v == b"mp4a").unwrap();
     input[entry..entry + 4].copy_from_slice(b"Opus");
     let child = input.windows(4).position(|v| v == b"esds").unwrap() - 4;
@@ -427,29 +433,41 @@ fn opus_movie(media_time: i32) -> Vec<u8> {
 }
 
 #[test]
-fn iso_opus_trim_owns_both_preskip_and_codec_delay() {
-    for (media, trim, count, padding) in [(96, 312, 3, 18_000_000), (960, 960, 3, 0)] {
-        let output = copy(&opus_movie(media)).unwrap();
+fn opus_copy_presents_the_source_edit_rather_than_preskip() {
+    let cases = [
+        (48_000, vec![(7992, 0)], 0, 9, 7992),
+        (48_000, vec![], 312, 9, 8328),
+        (48_000, vec![(4000, 100)], 100, 5, 4000),
+        (48_000, vec![(240, 0)], 0, 1, 240),
+        (48_000, vec![(240, 100)], 100, 1, 240),
+        (48_000, vec![(4000, 312)], 312, 5, 4000),
+        (48_000, vec![(4000, 960)], 960, 6, 4000),
+        (32_000, vec![(4000, 101)], 152, 5, 4000),
+        (32_000, vec![(4000, 100)], 150, 5, 4000),
+    ];
+    for (timescale, edits, trim, count, presented) in cases {
+        let name = format!("{timescale} {edits:?}");
+        let output = copy(&opus_movie(timescale, 9, edits)).unwrap();
         let segment = probe(&output);
         let track = &segment.tracks[0];
-        assert_eq!(track.codec_delay, (trim as u64) * 1_000_000_000 / 48_000);
         assert_eq!(
-            u16::from_le_bytes(
-                track.codec_private.as_ref().unwrap()[10..12]
-                    .try_into()
-                    .unwrap()
-            ),
-            trim
+            track.codec_delay,
+            (trim * 1_000_000_000 + 24_000) / 48_000,
+            "{name}"
         );
+        let pre_skip = track.codec_private.as_ref().unwrap()[10..12]
+            .try_into()
+            .map(u16::from_le_bytes)
+            .unwrap();
+        assert_eq!(u64::from(pre_skip), trim, "{name}");
         let packets = segment
             .packets(SliceSource::new(&output), &coreav_mkv::Limits::default())
             .collect::<Result<Vec<_>, _>>()
             .unwrap();
-        assert_eq!(packets.len(), count);
-        assert_eq!(
-            packets.last().unwrap().discard_padding.unwrap_or(0),
-            padding
-        );
+        assert_eq!(packets.len(), count, "{name}");
+        let padding = packets.last().unwrap().discard_padding.unwrap_or(0);
+        let padding = (u64::try_from(padding).unwrap() * 48_000 + 500_000_000) / 1_000_000_000;
+        assert_eq!(count as u64 * 960 - trim - padding, presented, "{name}");
     }
 }
 

docs/agents/containers.md

@@ -31,7 +31,8 @@ ISO BMFF, Matroska, MPEG-TS, and AVI.
 | Missing FLAC or ALAC configurations, invalid FLAC block-size bounds, and unsupported ALAC cookie versions produce a malformed-configuration warning, not configured audio values | `a_malformed_lossless_configuration_is_refused` in `coreav-isobmff/tests/audio_layouts.rs`; the library parses these bytes before exposing configured values, and `coreav_isobmff::lossless_config` is the one parser of them (the writers read the sample description's values or call it) |
 | M4A extraction with copied metadata preserves each header's distinct creation and modification times, widening the header when either requires 64 bits | `source_creation_and_modification_times_stay_distinct` in `coreav-isobmff-write/tests/extract.rs` checks stored values in both header versions and the metadata-clearing modes |
 | A Matroska writer omits Duration when no positive presentation end is known, including negative-only or fully trimmed packets | The writer's Info serializer owns the positive-value guard; `nonpositive_presentation_ends_omit_duration_without_losing_packets` checks both supported timelines and the absence of the Duration element in the actual bytes, because the reader can silently omit an invalid zero |
-| Matroska source output preserves ISO edit windows, binds Opus pre-skip to its effective leading trim and drops wholly padded replacement tails | `mkv_output::config::set_delay` owns OpusHead and CodecDelay together. The public-probe regressions in `coreav-isobmff-write/tests/mkv_output.rs` cover empty edits, finite audio/video windows, both Opus trim orderings and trailing replacement packets. External sample tables require runtime checks rather than types |
+| Matroska source output preserves ISO edit windows, binds Opus pre-skip to its effective leading trim and drops wholly padded replacement tails | `mkv_output::config::set_delay` owns OpusHead and CodecDelay together. The public-probe regressions in `coreav-isobmff-write/tests/mkv_output.rs` cover empty edits, finite audio/video windows and trailing replacement packets. External sample tables require runtime checks rather than types |
+| Copied Opus takes its leading trim from the ISO edit's media time, never `max(media time, PreSkip)`, rounded to the nearest 48 kHz sample as `decode-audio` rounds it; informative dOps PreSkip trims only a track without an edit list | `mkv_output::source::isobmff` reads PreSkip only for `EditList::None`. `opus_copy_presents_the_source_edit_rather_than_preskip` covers edits starting before, at and after PreSkip, edits shorter than PreSkip, an off-grid start on a 32 kHz media clock and the absent edit; `iso_opus_edit_starting_before_preskip_keeps_its_leading_samples` checks decoded output and the pinned oracle's divergence. The edit enum cannot say which field a caller trims by |
 | Replaced tracks need no source copy mapping, and copied QuickTime sound-packet units are not frames | `mkv_output::source` accepts prepared replacement headers before copy validation and refuses `sound_packet` descriptions; the unmapped-replacement and sound-packet regressions in `coreav-isobmff-write/tests/mkv_output.rs` cover both boundaries |
 | An ISO edit's nanosecond end-to-preroll distance fits unsigned width even when it exceeds signed timestamp width | `i64::abs_diff` owns the full-width distance in the Matroska adapter; `extreme_edit_boundaries_refuse_without_overflowing_the_preroll_distance` exercises a parsed version-1 edit with timestamps at opposite signed extremes and rejects the empty output without a panic. The unsigned result type prevents signed subtraction overflow |
 | Matroska adapter metadata is charged before duplication, including repeated stream selections, and transfer to the packet writer moves its ownership | `mkv_output::MetadataBudget` owns pre-allocation charges in both source adapters and replacement preparation. The exact-limit unit test, pre-packet-traversal refusal, ISO duplicate-header regression and moved header collection prevent the late-bound-copy mistake; input-dependent byte counts need runtime checks rather than types |

docs/design/cli-native-h264-matroska.md

@@ -20,14 +20,14 @@ New native Matroska admission requires a nominal period of at least one millisec
 
 Add a checked video-cadence value and an explicit writer entrypoint for one H.264 video replacement. Existing writer calls retain their current behavior. The new entrypoint validates that the named stream is the actual video replacement, supplies its nominal default duration, and rejects rounded timestamp collisions before publication. Audio payloads, stream selection, display dimensions and cluster policy remain unchanged.
 
-For copied Opus from ISO base media, this new entrypoint follows the original source edit list, not the informative dOps PreSkip field (Opus ISO mapping Sections 4.3.2 and 4.4). It carries that edit's trim into CodecDelay and OpusHead and retains its declared terminal presentation. The ordinary writer still takes the greater of the edit trim and PreSkip; correcting that existing path is separate work. In particular, media time zero with PreSkip of 312 and a declared 7,992-sample presentation retains all 7,992 samples in native Matroska, instead of the ordinary writer's 7,680 samples. Native MP4 remains byte-identical to its actual base.
+For copied Opus from ISO base media, this new entrypoint follows the original source edit list, not the informative dOps PreSkip field (Opus ISO mapping Sections 4.3.2 and 4.4). It carries that edit's trim into CodecDelay and OpusHead and retains its declared terminal presentation. The ordinary writer now follows the edit the same way ([Matroska writer](mkv-write.md)), so media time zero with PreSkip of 312 and a declared 7,992-sample presentation retains all 7,992 samples in both. The two differ only for a track without an edit list, where the ordinary writer still trims PreSkip and this entrypoint trims nothing. Native MP4 remains byte-identical to its actual base.
 
 The transcode pipeline uses that entrypoint only for a completed native H.264 Matroska encode, with the admitted encoder frame rate. MP4 never constructs the cadence override. Unsupported native cadence retries the unchanged external command under the original deadline; I/O, malformed-input and timeout errors remain terminal.
 
 ## Verification
 
 - Build and retain a separate native binary from the actual pre-change main revision. Compare complete MP4 bytes for the existing metadata, quality, thread, GOP, legacy duration, input, audio-copy and selector axes, with external programs unavailable. Record named cases and counts, not just candidate self-comparisons.
-- Preserve complete default, feature-enabled unselected and external Matroska output bytes against that revision. Retain unsupported-option, WebM, VFR, high-rate and timestamp-collision fallback controls and transactional deadline tests.
+- Preserve complete default, feature-enabled unselected and external Matroska output bytes against that revision. The one exception is the edited Opus source copied beside an external H.264 encode in the feature-enabled unselected build: its Matroska output must instead decode to the source edit's declared presentation, because the ordinary writer now follows that edit too. The default build refuses that encode and runs the oracle fallback, so its bytes stay compared with the revision. Retain unsupported-option, WebM, VFR, high-rate and timestamp-collision fallback controls and transactional deadline tests.
 - Check new Matroska output with an independent EBML parser: DefaultDuration, TimestampScale, block PTS, absence of conflicting BlockDuration, display ratio, codec private data and keyframe flags. Independently parse VUI timing and aspect.
 - Cover integer, NTSC and unusual rational rates, absent and explicit aspects, single pictures, legacy zero durations, source origins, the 250-picture GOP boundary, thread counts and copied audio.
 - Compare complete encoded access units and pinned-oracle reconstruction between same-head MP4 and Matroska. Verify frame counts and timestamp error against the declared quantization.