diff --git a/crates/coreav-cli/tests/ffmpeg_matroska.rs b/crates/coreav-cli/tests/ffmpeg_matroska.rs index 4cd054961..cbdbeac3e 100644 --- a/crates/coreav-cli/tests/ffmpeg_matroska.rs +++ b/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)| { + 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() { diff --git a/crates/coreav-isobmff-write/src/mkv_output/source.rs b/crates/coreav-isobmff-write/src/mkv_output/source.rs index d13a05cb8..0204f8269 100644 --- a/crates/coreav-isobmff-write/src/mkv_output/source.rs +++ b/crates/coreav-isobmff-write/src/mkv_output/source.rs @@ -130,7 +130,11 @@ pub(super) fn isobmff( }; 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( 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) diff --git a/crates/coreav-isobmff-write/src/mkv_output/tests.rs b/crates/coreav-isobmff-write/src/mkv_output/tests.rs index 05f5ce6c4..5f117517d 100644 --- a/crates/coreav-isobmff-write/src/mkv_output/tests.rs +++ b/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; diff --git a/crates/coreav-isobmff-write/tests/mkv_output.rs b/crates/coreav-isobmff-write/tests/mkv_output.rs index 095a80b8b..568b8d5ec 100644 --- a/crates/coreav-isobmff-write/tests/mkv_output.rs +++ b/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 { - 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 { + 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 { } #[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::, _>>() .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}"); } } diff --git a/docs/agents/containers.md b/docs/agents/containers.md index bc025c21c..2ad2dc3b0 100644 --- a/docs/agents/containers.md +++ b/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 | diff --git a/docs/design/cli-native-h264-matroska.md b/docs/design/cli-native-h264-matroska.md index cc8ab3ac3..6d97d20eb 100644 --- a/docs/design/cli-native-h264-matroska.md +++ b/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. diff --git a/docs/design/mkv-write.md b/docs/design/mkv-write.md index 5f7ff99b9..438e71334 100644 --- a/docs/design/mkv-write.md +++ b/docs/design/mkv-write.md @@ -34,7 +34,7 @@ Video descriptions carry explicit container colour primaries, transfer, matrix a The source adapter accepts ISO base media or Matroska and keeps payloads in their source or encoder spool. It charges every selected packet against one allocation budget before retaining metadata, including repeated stream selections. A separate retained-metadata budget is charged before codec-private, prefix or string copies, including source metadata and replacement preparation; headers move into the packet writer rather than being cloned again. Replaced tracks retain their source identity and clock but do not require a copy mapping or list discarded source packets. QuickTime sound-packet tables, complex ISO edits, transformed video and unsupported copied codecs are refused. Fractional sampling rates and a distinct OutputSamplingFrequency are refused because the output model cannot represent them. Unsupported automatically selected subtitles require `-sn` or an explicit audio/video-only map rather than being silently omitted. The implicit text default leaves bitmap subtitles unselected; `-c:s copy` activates their selection and is refused, matching the pinned oracle's stream-selection distinction. -ISO edits map media ticks through one rational offset before nanosecond conversion. Audio CodecDelay owns leading trim; OpusHead pre-skip is rewritten to that same value. Finite windows retain decoder preroll but mark hidden video frames invisible, discard the trailing hidden suffix and attach positive DiscardPadding to straddling audio frames. Whole trailing replacement packets are omitted. ISO and replacement timestamps are rounded to milliseconds before adding the rounded CodecDelay, so quantization is not performed twice. Matroska copies preserve their stored timing and split laced padding only at its original boundary frame. +ISO edits map media ticks through one rational offset before nanosecond conversion. Audio CodecDelay owns leading trim; OpusHead pre-skip is rewritten to that same value. For copied Opus the leading trim is the edit's media time, even below dOps PreSkip, which [Opus in ISOBMFF section 4.3.2](https://opus-codec.org/docs/opus_in_isobmff.html#4.3.2) makes informative; PreSkip is the trim only for a track without an edit list, as in the [MP4 decoder](opus-mp4-presentation.md). An edit start between two 48 kHz samples, on a media clock that is not a multiple of 48 kHz, is rounded to the nearest sample, halves up, as `decode-audio` rounds it, and the copied packets move with it. Finite windows retain decoder preroll but mark hidden video frames invisible, discard the trailing hidden suffix and attach positive DiscardPadding to straddling audio frames. Whole trailing replacement packets are omitted. ISO and replacement timestamps are rounded to milliseconds before adding the rounded CodecDelay, so quantization is not performed twice. Matroska copies preserve their stored timing and split laced padding only at its original boundary frame. AAC frame sample counts come from AudioSpecificConfig (ISO/IEC 14496-3 section 4.5.1.1), not each ISO decoding-time delta. A shortened interior delta changes the next timestamp without making the current frame's decoded samples padding. Only the terminal sample duration or a finite presentation end shortens the encoded frame. The round-trip AAC regression checks the interior 1,016-tick delta and all 11,520 decoded samples, including the declared terminal trim. diff --git a/docs/known-divergences-audio-output.md b/docs/known-divergences-audio-output.md index 78839a673..4aa65aad5 100644 --- a/docs/known-divergences-audio-output.md +++ b/docs/known-divergences-audio-output.md @@ -91,6 +91,14 @@ CoreAV's decoded audio is byte-identical to the corresponding prefix of the orac `iso_opus_keeps_the_declared_edit_end_instead_of_decoding_the_padded_tail` in [`ffmpeg_matroska.rs`](../crates/coreav-cli/tests/ffmpeg_matroska.rs) binds the source's parsed presentation end to 11,520 samples, requires the exact decoded counts and prefix equality, and checks the final packet's `discard_padding=648`. Oracle agreement or a different extra count fails the assertion. The [Matroska writer design](design/mkv-write.md) describes the mapping; this fixture assertion is not a blanket exception for all Matroska copies. +## Opus MP4 to Matroska copy keeps the samples an edit presents before PreSkip + +For a generated 0.24-second mono Opus MP4 whose single rate-1 edit is changed to start at media time 0, with `dOps` PreSkip 312 and an edit duration of 11,520 samples at 48 kHz, compare `coreav ffmpeg -i input.mp4 -c copy output.mkv` with `ffmpeg -i input.mp4 -c copy reference.mkv`. CoreAV writes `CodecDelay` 0 and decodes to the edit's 11,520 samples, starting at the first decoded sample. FFmpeg's copy decodes to 11,208 samples: the same window without its first 312 samples. + +[Opus in ISOBMFF section 4.3.2](https://opus-codec.org/docs/opus_in_isobmff.html#4.3.2) makes PreSkip informative and leaves whole-playback priming removal to the edit list, and section 4.4 gives the edit's media time as the number of samples to trim. Applying PreSkip on top of the edit trims the start twice. CoreAV uses PreSkip only when the track has no edit list, as its MP4 decoder does ([Opus MP4 presentation](design/opus-mp4-presentation.md)). + +`iso_opus_edit_starting_before_preskip_keeps_its_leading_samples` in [`ffmpeg_matroska.rs`](../crates/coreav-cli/tests/ffmpeg_matroska.rs) checks the source's edit and PreSkip before the change, requires the native output to equal the first 11,520 samples of the complete decode, keeps the unchanged media-time-312 control, and requires the oracle copy to equal the native output without its first 312 samples. Oracle agreement or a different offset fails the assertion. + ## MP3 output keeps the original boundary length For a supported lossless source containing `N` samples per channel, compare `coreav ffmpeg -i input.mp4 output.mp3` with `ffmpeg -i input.mp4 reference.mp3`. Explicit `-c:a libmp3lame` selects the same encoder path. Let `F` be the codec frame length: 1,152 samples at 32, 44.1, and 48 kHz, and 576 at the lower supported rates. When `r = N mod F` is from 1 through 46, the pinned FFmpeg clamps final discard padding and presents `N + 47 - r` samples. CoreAV presents exactly `N`. diff --git a/scripts/native_h264_cli.py b/scripts/native_h264_cli.py index 10e6bda97..6394507e7 100644 --- a/scripts/native_h264_cli.py +++ b/scripts/native_h264_cli.py @@ -79,7 +79,8 @@ def oracle_rates(stream, rate, common): oracle_avg_frame_rate=stream['avg_frame_rate'], oracle_rate_interpretation=interpretation) -def unchanged_outputs(args, root, source_path, environment, fallback_environment, *, commands=None): +def unchanged_outputs(args, root, source_path, environment, fallback_environment, *, commands=None, + skip=()): if commands is None: commands = [('default-copy', ['-c', 'copy']), ('external', ['-an', '-c:v', 'libx264', '-crf', '23'])] @@ -90,6 +91,8 @@ def unchanged_outputs(args, root, source_path, environment, fallback_environment ('unselected', args.candidate, args.baseline_external), ('default', args.default_candidate, args.baseline_default), ): + if (container, mode) in skip: + continue previous = root / f'{label}-{mode}-base.{container}' current = root / f'{label}-{mode}.{container}' selected_environment = fallback_environment if container == 'mkv' else environment @@ -101,6 +104,19 @@ def unchanged_outputs(args, root, source_path, environment, fallback_environment return groups +def edited_opus_matroska(args, ffmpeg, ffprobe, root, source_path, command, environment): + expected, facts = opus_source_pcm(source_path, root / 'opus-source-edit-reference', ffmpeg, ffprobe) + output = root / 'opus-source-edit-unselected.mkv' + run([args.candidate, 'ffmpeg', '-i', source_path, *command, output], env=environment) + try: + matroska.compare_opus_pcm(oracle_audio_pcm(ffmpeg, output, output.with_suffix('.opus.f32le')), + expected, facts) + except ValueError as error: + raise ValueError(f'ordinary-matroska/opus-source-edit/unselected: {error}') from error + return dict(mode='unselected', declared_samples=facts['declared_samples'], + start_samples=facts['start_samples'], pre_skip=facts['pre_skip']) + + def copied_audio_sources(root, ffmpeg, video30, video25): sources = [] for label, container, encoder, rate, offset in [ @@ -698,11 +714,15 @@ def compare(source_path, extra, label, container='mp4'): matroska_rows = matroska_matrix(args, tools, root, source, native, native_only, source_path, legacy_sources, gop_source) + opus_edit_source = root / 'audio-mp4-opus-source.mp4' + opus_edit_command = ['-c:v', 'libx264', '-c:a', 'copy', '-fps_mode', 'passthrough'] for group, labels in unchanged_outputs( - args, root, root / 'audio-mp4-opus-source.mp4', environment, fallback_environment, - commands=[('opus-source-edit', ['-c:v', 'libx264', '-c:a', 'copy', '-fps_mode', 'passthrough'])], + args, root, opus_edit_source, environment, fallback_environment, + commands=[('opus-source-edit', opus_edit_command)], skip={('mkv', 'unselected')}, ).items(): unchanged_groups[group].extend(labels) + ordinary_opus_edit = edited_opus_matroska(args, tools.ffmpeg, tools.ffprobe, root, opus_edit_source, + opus_edit_command, fallback_environment) summary = dict(native=rows, metadata=metadata_rows, legacy_native_cases=len(legacy_sources), legacy_native_shapes=[label for label, _ in legacy_sources], gop_frames=251, fallback_cases=fallback_cases, default_bytes_unchanged=True, @@ -716,7 +736,8 @@ def compare(source_path, extra, label, container='mp4'): source_opus_pcm=dict(cases=sum('source_opus_pcm' in row for row in matroska_rows), direct_decodes=sum(row.get('source_opus_pcm', {}).get('verification') == 'direct-decode' for row in matroska_rows)), - native_matroska=matroska_rows) + native_matroska=matroska_rows, + ordinary_matroska_opus_edit=ordinary_opus_edit) (root / 'summary.json').write_text(json.dumps(summary, indent=2) + '\n') print(f'native CLI: {sum(map(len, baseline_groups.values()))} actual-base MP4 byte cases, ' f'{sum(map(len, unchanged_groups.values()))} default/unselected/external byte cases, ' diff --git a/scripts/tests/test_native_h264_cli.py b/scripts/tests/test_native_h264_cli.py index 4a96a3dcb..6e5c80f9a 100644 --- a/scripts/tests/test_native_h264_cli.py +++ b/scripts/tests/test_native_h264_cli.py @@ -140,21 +140,50 @@ def executable(name, mode): baseline_default=executable('baseline-default', 'default-decoder')) command = ['-c:v', 'libx264', '-c:a', 'copy', '-fps_mode', 'passthrough'] source = root / 'audio-mp4-opus-source.mp4' + skip = {('mkv', 'unselected')} groups = cli.unchanged_outputs(args, root, source, None, None, - commands=[('opus-source-edit', command)]) + commands=[('opus-source-edit', command)], skip=skip) + compared = [('mp4', 'unselected'), ('mp4', 'default'), ('mkv', 'default')] self.assertEqual(cli.byte_groups(groups), { - f'{container}-{mode}': {'count': 1, 'cases': ['opus-source-edit']} - for container in ('mp4', 'mkv') for mode in ('unselected', 'default')}) - for container in ('mp4', 'mkv'): - for mode, marker in (('unselected', 'native-decoder'), ('default', 'default-decoder')): - for suffix in ('', '-base'): - output = root / f'opus-source-edit-{mode}{suffix}.{container}' - self.assertEqual(cli.json.loads(output.read_text()), - [marker, ['ffmpeg', '-i', str(source), *command]]) + f'{container}-{mode}': {'count': 1, 'cases': ['opus-source-edit']} for container, mode in compared}) + for container, mode in compared: + marker = 'native-decoder' if mode == 'unselected' else 'default-decoder' + for suffix in ('', '-base'): + output = root / f'opus-source-edit-{mode}{suffix}.{container}' + self.assertEqual(cli.json.loads(output.read_text()), + [marker, ['ffmpeg', '-i', str(source), *command]]) + self.assertFalse((root / 'opus-source-edit-unselected.mkv').exists()) args.baseline_external = args.baseline_default with self.assertRaisesRegex(ValueError, 'actual-base/mp4/opus-source-edit/unselected/whole bytes differs'): cli.unchanged_outputs(args, root, source, None, None, - commands=[('opus-source-edit', command)]) + commands=[('opus-source-edit', command)], skip=skip) + + def test_ordinary_matroska_opus_copy_must_present_the_source_edit(self): + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + source = root / 'audio-mp4-opus-source.mp4' + command = ['-c:v', 'libx264', '-c:a', 'copy', '-fps_mode', 'passthrough'] + args = SimpleNamespace(candidate='candidate') + facts = {'declared_samples': 2, 'channels': 1, 'start_samples': 0, 'pre_skip': 312} + expected = bytes(8) + runs = [] + + def decode(_, output, __): + return outputs[output.name] + + outputs = {'opus-source-edit-unselected.mkv': expected} + patches = (mock.patch.object(cli, 'opus_source_pcm', return_value=(expected, facts)), + mock.patch.object(cli, 'run', side_effect=lambda command, env: runs.append((command, env))), + mock.patch.object(cli, 'oracle_audio_pcm', side_effect=decode)) + with patches[0], patches[1], patches[2]: + result = cli.edited_opus_matroska(args, 'ffmpeg', 'ffprobe', root, source, command, 'env') + self.assertEqual(result, dict(mode='unselected', declared_samples=2, start_samples=0, pre_skip=312)) + self.assertEqual(runs, [(['candidate', 'ffmpeg', '-i', source, *command, + root / 'opus-source-edit-unselected.mkv'], 'env')]) + outputs['opus-source-edit-unselected.mkv'] = expected[4:] + with self.assertRaisesRegex(ValueError, 'ordinary-matroska/opus-source-edit/unselected: ' + 'copied Opus: original declared sample count differs'): + cli.edited_opus_matroska(args, 'ffmpeg', 'ffprobe', root, source, command, 'env') def test_opus_source_oracle_uses_original_edit_and_only_zeroes_clone_informative_preskip(self): with tempfile.TemporaryDirectory() as directory: diff --git a/test-support/oracle-dependent.json b/test-support/oracle-dependent.json index 87b2a8642..d4417ea2c 100644 --- a/test-support/oracle-dependent.json +++ b/test-support/oracle-dependent.json @@ -127,6 +127,7 @@ "copy_to_matroska_and_webm_matches_the_oracle", "explicit_formats_and_audio_only_extension_select_the_container", "h264_transcode_writes_matroska_with_copied_audio", + "iso_opus_edit_starting_before_preskip_keeps_its_leading_samples", "iso_opus_keeps_the_declared_edit_end_instead_of_decoding_the_padded_tail", "matroska_container_colour_survives_copy_without_changing_packets", "matroska_default_video_encoder_matches_the_explicit_selection",