From d0c9df91ef79d3aab980b972c9f052476295f8b4 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Sat, 26 Sep 2026 10:47:57 -0700 Subject: [PATCH] Fix PGS clear events and add FFmpeg QA interoperability gate --- .github/workflows/ffmpeg-validation.yml | 56 +++ .github/workflows/qa.yml | 7 + docs/README.md | 1 + docs/adts.md | 48 +- docs/demux-sink.md | 9 +- docs/ffmpeg-validation.md | 68 +++ docs/mpegaudio.md | 39 +- docs/pgs.md | 72 ++- src/mux/codec/adts.rs | 177 ++++--- src/mux/codec/audio_frames.rs | 138 ++++++ src/mux/codec/mod.rs | 1 + src/mux/codec/mpegaudio.rs | 188 ++++--- src/mux/codec/pgs.rs | 151 +++--- src/mux/codec/pgs_tests.rs | 632 ++++++++++++++++++++++++ src/mux/demux_sink.rs | 228 ++++++++- src/mux/disc.rs | 2 + src/mux/driver.rs | 8 +- src/mux/fvi_sink.rs | 1 + src/mux/interop_tests.rs | 536 ++++++++++++++++++++ src/mux/m2ts.rs | 3 + src/mux/mkv.rs | 135 ++++- src/mux/mkvstream.rs | 174 ++++++- src/mux/mod.rs | 3 + src/mux/mp4/mod.rs | 1 + src/mux/mp4/read.rs | 8 + src/mux/network.rs | 3 + src/mux/null.rs | 2 + src/mux/pipelined_stream.rs | 2 + src/mux/stdio.rs | 1 + src/mux/ts.rs | 27 +- src/mux/videomap.rs | 1 + src/pes.rs | 47 ++ tests/fvi_pipeline.rs | 1 + tests/streams.rs | 6 + 34 files changed, 2445 insertions(+), 331 deletions(-) create mode 100644 .github/workflows/ffmpeg-validation.yml create mode 100644 docs/ffmpeg-validation.md create mode 100644 src/mux/codec/audio_frames.rs create mode 100644 src/mux/codec/pgs_tests.rs create mode 100644 src/mux/interop_tests.rs diff --git a/.github/workflows/ffmpeg-validation.yml b/.github/workflows/ffmpeg-validation.yml new file mode 100644 index 00000000..87f393ef --- /dev/null +++ b/.github/workflows/ffmpeg-validation.yml @@ -0,0 +1,56 @@ +name: FFmpeg validation + +# Called by qa.yml on pushes to qa, or manually for a development branch. +# Pure Rust tests remain in dev CI; these tests require external codecs. +on: + workflow_call: + workflow_dispatch: + +permissions: + contents: read + +jobs: + validate: + runs-on: ubuntu-24.04 + timeout-minutes: 20 + env: + FREEMKV_FFMPEG_ARTIFACT_DIR: ${{ github.workspace }}/ffmpeg-artifacts + steps: + - uses: actions/checkout@v5 + with: + path: libfreemkv + - uses: freemkv/.github/.github/actions/checkout-siblings@v1 + with: + checkout: freemkv-unlock + - name: Read the crate's MSRV + id: msrv + working-directory: libfreemkv + run: echo "v=$(sed -n 's/^rust-version = "\(.*\)"/\1/p' Cargo.toml)" >> "$GITHUB_OUTPUT" + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ steps.msrv.outputs.v }} + - uses: Swatinem/rust-cache@v2 + with: + workspaces: libfreemkv + - name: Install software encoders and decoders + run: sudo apt-get update && sudo apt-get install -y ffmpeg + - name: Record tool versions + run: | + mkdir -p "$FREEMKV_FFMPEG_ARTIFACT_DIR" + ffmpeg -version > "$FREEMKV_FFMPEG_ARTIFACT_DIR/ffmpeg-version.txt" + ffprobe -version > "$FREEMKV_FFMPEG_ARTIFACT_DIR/ffprobe-version.txt" + rustc --version > "$FREEMKV_FFMPEG_ARTIFACT_DIR/rustc-version.txt" + - name: Generate fixtures and validate libfreemkv output + working-directory: libfreemkv + shell: bash + run: | + set -o pipefail + cargo test --release --lib ffmpeg_ -- --ignored --nocapture 2>&1 | tee "$FREEMKV_FFMPEG_ARTIFACT_DIR/tests.log" + - name: Upload fixtures, probes, hashes and logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: ffmpeg-validation-${{ github.run_id }}-${{ github.run_attempt }} + path: ${{ env.FREEMKV_FFMPEG_ARTIFACT_DIR }} + if-no-files-found: warn + retention-days: 14 diff --git a/.github/workflows/qa.yml b/.github/workflows/qa.yml index b9620d2d..8a44df7b 100644 --- a/.github/workflows/qa.yml +++ b/.github/workflows/qa.yml @@ -104,6 +104,13 @@ jobs: - run: cargo test --release --tests working-directory: libfreemkv + # Independent decoder/remux verification uses generated media and belongs + # on qa, alongside the release suite. Also runnable manually on a branch. + ffmpeg-validation: + uses: ./.github/workflows/ffmpeg-validation.yml + permissions: + contents: read + # clippy's output is target-dependent: cfg-gated code only gets linted on # the target it compiles for. Linting solely on the dev machine's host # target is how a lint that CI rejects reaches a push. diff --git a/docs/README.md b/docs/README.md index 0adb5d06..6e355c97 100644 --- a/docs/README.md +++ b/docs/README.md @@ -10,6 +10,7 @@ Technical documentation for [libfreemkv](https://github.com/freemkv/libfreemkv), | Document | What it covers | |----------|---------------| +| [FFmpeg QA validation](ffmpeg-validation.md) | Synthetic decoder interoperability gate, regressions, licensing | | [Architecture](architecture.md) | Module map, design principles, error codes, platform support | | [Drive Access](drive-access.md) | Drive, SCSI transport, profiles, unlock, why raw mode is needed | | [Rip Recovery](rip-recovery.md) | What this crate owns of the recovery model: single-shot Drive::read, SenseFamily, DiscStream batch halving (the strategy itself moved to freemkv-engine in 1.6.0) | diff --git a/docs/adts.md b/docs/adts.md index 888798e2..cc99ee2b 100644 --- a/docs/adts.md +++ b/docs/adts.md @@ -1,37 +1,17 @@ -# ADTS decodability gate +# AAC ADTS framing and validation -## Module rationale (`src/mux/codec/adts.rs`) +ADTS syncframes are buffered across PES boundaries and split when one PES +contains multiple frames. Each output is raw AAC: the ADTS header and optional +CRC are removed, and AudioSpecificConfig is exposed for the container header. +A PES timestamp anchors its first frame; subsequent frames advance by the +header's sample count and sample rate. The first byte's provenance is retained. -Per the ADTS framing defined in ISO/IEC 13818-7 / ISO/IEC 14496-3, a header -is structurally invalid in exactly three ways this gate treats as hard -rejects: syncword != 0xFFF, a reserved `sampling_frequency_index` (the sample -rate table has 13 valid entries, so index >= 13 is reserved), and -`aac_frame_length < 7` (shorter than the fixed+variable header itself). The -optional 16-bit ADTS CRC is not verified here — it is simply skipped. So the -gate enforces those three rejects: a packet that begins with the ADTS sync -but is otherwise malformed is dropped; a packet with no ADTS sync is raw AAC -(e.g. from an MP4 container, which carries no ADTS header) or a continuation -and passes through unchanged — never false-dropped. Raw AAC has no per-frame -integrity data, so like LPCM it cannot be gated. +Reserved sample rates and lengths shorter than the header are rejected. CRC +contents are not verified. Before ADTS sync is seen, nonsync payloads retain +the raw-AAC passthrough behavior; after sync, nonsync PES payloads are treated +as continuations. A partial EOF frame is not emitted. Discontinuities discard +a partial frame and restart timestamp anchoring. Buffering is capped at 1 MiB. -## Test: `a_crc_present_header_shorter_than_its_own_crc_is_invalid` - -A header that CLAIMS a CRC (protection_absent = 0) but declares a frame -length too short to contain one. - -`aac_frame_length` counts the header and the CRC, not just the payload, -so with a CRC present the smallest structurally possible frame is 9 -bytes: the 7-byte fixed+variable header plus the 16-bit crc_check. -The gate compared against a flat 7 and never read protection_absent at -all, so a frame whose own header says it is impossible was classified -Valid and forwarded to the muxer. - -## Test: `dropped_frames_are_counted_but_their_duration_is_not_invented` - -A dropped ADTS frame is dropped BECAUSE its header failed validation, so -the very fields a duration would come from (sampling_frequency_index, and -the 1024-samples-per-AAC-frame constant applied to it) are the ones known -to be untrustworthy. This gate therefore reports the drop's duration as -zero rather than deriving a number from a header it has just rejected — -the honest answer, and the one the count alongside it must be read with. -A nonzero constant here would report silence that was never measured. +Rust tests exercise every split point, multiple frames per PES, CRC removal, +AudioSpecificConfig, discontinuities, timestamps and malformed headers. The +FFmpeg QA gate compares decoded PCM after deliberately fragmented input. diff --git a/docs/demux-sink.md b/docs/demux-sink.md index ab8a2d8a..e9dbe349 100644 --- a/docs/demux-sink.md +++ b/docs/demux-sink.md @@ -27,9 +27,12 @@ The parser hands the demux sink the concatenated PGS segments of a display set in `frame.data` (segment_type + segment_size + payload, repeated), with no `PG` magic and no PTS/DTS. A `.sup` prefixes each segment with a 13-byte header: `0x50 0x47` ("PG") | PTS u32 BE | DTS u32 BE | (segment_type|size -already present in the payload). When the parser folded a trailing clear -(`duration_ns` set), the writer re-emits it as an empty composition at -`pts + duration` so players time the subtitle out. +already present in the payload). Original clear sets are emitted verbatim. +For duration-only input (older MKVs, or an EOF fallback), the writer holds +a synthetic clear for `pts + duration`. A real clear or replacement PCS at +or before that deadline cancels the synthetic one. Otherwise it is emitted +before the next later frame, or at finish. Clear and continuation frames do +not themselves schedule further clears. ## `synthetic_clear_display_set`: synthesizing the clear PCS diff --git a/docs/ffmpeg-validation.md b/docs/ffmpeg-validation.md new file mode 100644 index 00000000..4a9f2638 --- /dev/null +++ b/docs/ffmpeg-validation.md @@ -0,0 +1,68 @@ +# FFmpeg QA validation + +`.github/workflows/ffmpeg-validation.yml` is a reusable workflow called by +`qa.yml` on pushes to `qa`. It also supports manual runs on a side branch. +Normal development CI runs the Rust tests without installing FFmpeg. + +The QA job installs Ubuntu's FFmpeg package, reads the crate's MSRV, and runs: + +```sh +cargo test --release --lib ffmpeg_ -- --ignored --nocapture +``` + +All fixtures are synthetic and generated during the run: small test patterns, +sine waves, and a tiny PGS bitmap constructed in Rust. No ripped films or +external media corpus are downloaded. Set `FREEMKV_FFMPEG_ARTIFACT_DIR` locally +to retain fixtures. CI uploads generated media, commands, decoder/probe output, +hashes, tool versions and the test log for 14 days, including on failure. + +## Coverage + +- PGS visible/empty compositions at 180/183 and 3780/3783 seconds; decoded + clears and warning-free FFmpeg stream-copy remuxing. +- H.264/AC-3, HEVC/E-AC-3 and MPEG-2 transport-to-MKV through the public mux + driver, including reordered video pictures and delayed audio starts. +- AAC, MP2 and E-AC-3 transport audio discovery and remuxing. +- Fragmented AAC, MP2, AC-3 and E-AC-3 elementary streams: 137-byte input + chunks deliberately split headers and audio frames. +- MKV roundtrips with variable frame cadence, multiple audio tracks, + CodecDelay and DiscardPadding. + +Audio/video tests compare stream metadata, decoded frame counts, all decoded +presentation timestamps with a shared origin shift (2 ms tolerance), and exact +SHA-256 hashes of decoded pixels/PCM. Generated output must decode without +warnings or errors. Hashes compare input/output in the same FFmpeg run rather +than a fixed encoder-version-dependent golden file. + +The gate exposed and now guards fixes for lost PGS clears (also reproducing +issue #52's timestamp warnings), PES-granular AAC/MP2 parsing, missing generic +transport audio types, audio lost before a buffered MPEG-2 opening picture, +and dropped MKV decoder-delay/end-padding metadata. Pure Rust regressions run +in development CI; external decoder checks run explicitly in QA. + +The gate is bounded, synthetic interoperability coverage. It does not replace +real-disc QA, player testing, or cover every codec/profile. Free-format MPEG +audio retains PES-granular handling. Container timing/padding preservation is +currently local MKV-to-MKV; existing network/stdio PES serialization does not +carry the added metadata. Rust callers constructing `PesFrame` literals must +initialize the new `discard_padding_ns` field to zero unless preserving trimming. + +## FFmpeg licensing + +Reviewed against [FFmpeg's legal guidance](https://ffmpeg.org/legal.html) and +[the GPLv2 text distributed by FFmpeg](https://raw.githubusercontent.com/FFmpeg/FFmpeg/master/COPYING.GPLv2), +section 0. FFmpeg is LGPL-2.1-or-later, with optional components that make a +build GPL; Ubuntu's software encoders can include those components. GPLv2 +permits running the executable. Its output is not automatically GPL-covered +merely because FFmpeg produced it. + +This workflow invokes separately installed `ffmpeg`/`ffprobe` executables. +It does not link FFmpeg libraries into libfreemkv, copy FFmpeg implementation +code, modify FFmpeg, bundle its binaries, or upload those binaries as artifacts. +Our generated media and diagnostic logs do not embed FFmpeg implementation +code. On that basis, this CI use does not trigger FFmpeg redistribution or +library-linking obligations for libfreemkv. Reassess if packaging/linking changes. + +This is a software-license assessment, not a guarantee of patent clearance. +Codec patents are separate from FFmpeg's copyright licenses and depend on +jurisdiction and use; FFmpeg's legal page explicitly distinguishes them. diff --git a/docs/mpegaudio.md b/docs/mpegaudio.md index bfdf9291..caa07431 100644 --- a/docs/mpegaudio.md +++ b/docs/mpegaudio.md @@ -1,28 +1,17 @@ -# MPEG audio (MP1/MP2/MP3) decodability gate +# MPEG audio framing and validation -## Module rationale (`src/mux/codec/mpegaudio.rs`) +MPEG-1/2/2.5 audio frames are assembled across PES boundaries and split when +one PES contains multiple frames. Header version, layer, bitrate, sample rate +and padding determine the frame size and sample duration. Timestamp anchors +and source provenance survive fragmentation. Invalid reserved header fields +are rejected; audio payload CRCs are not checked. -Per ISO/IEC 11172-3 / ISO/IEC 13818-3, an MPEG-audio frame is validated by -header sanity + framing resync, not a payload CRC (the optional 16-bit CRC in -the header protects only the side-information and is absent unless the -protection bit says otherwise). The gate mirrors that header-only check and -ACCEPTS free-format (`bitrate_index == 0`) as a legal decodable mode — it -deliberately does NOT apply the stricter free-format reject that a full -decoder would (see the note at the `bitrate_index` check in `mpa_verdict`). -So the gate rejects only the truly invalid headers: a packet that begins with -the 11-bit MPEG-audio sync but whose version / layer / sample-rate fields (or -the reserved bitrate index 15) are reserved/invalid is undecodable → drop it -(a silence gap; each packet keeps its own PTS). A packet with no leading sync -is not a frame we can validate (raw payload / continuation), so it passes -through unchanged — never false-dropped. +Free-format bitrate index zero remains a legal passthrough case at PES +granularity because its header does not signal a frame length. Before sync is +seen, nonsync payloads retain passthrough behavior; after sync, later PES +payloads are continuations. Partial EOF frames are discarded, not manufactured. +Buffering is capped at 1 MiB. -## Test: `flush_adds_no_phantom_frame_after_the_last_real_packet` - -This parser is self-framing at PES granularity: `parse` emits (or drops) -every packet immediately and buffers nothing, so end-of-stream has nothing -left to hand over. A `flush` that manufactured a frame would append a -zero-length block at PTS 0 AFTER a track that has already run to its real -end — a Matroska Block whose timestamp jumps backwards past every cluster -before it (RFC 9559 §5.1.3.2 Blocks are relative to their cluster's -timestamp; a phantom 0 lands in the wrong cluster entirely) and an empty -audio frame no decoder can consume. +Rust tests cover every split point, multiple frames per PES, version/layer +frame-size tables, timestamps and invalid headers. The FFmpeg QA gate compares +decoded PCM after deliberately fragmented MP2 input. diff --git a/docs/pgs.md b/docs/pgs.md index 723fdf2c..5de66c10 100644 --- a/docs/pgs.md +++ b/docs/pgs.md @@ -1,12 +1,72 @@ # HDMV PGS subtitle parser (`src/mux/codec/pgs.rs`) -## Module overview: why display sets need a synthesized duration +## Display and clear are separate decoder events -For Matroska output the parser collapses a display PCS / clear PCS pair into -one block with `BlockDuration` set to `clear_pts - display_pts`. Without a -duration, hardware players linger on the last bitmap until the next subtitle -replaces it — which can be many seconds, and on a disc where the final -subtitle has no follower, until end of file. +A visible composition (PCS with objects) starts a subtitle. An empty +composition (PCS with zero objects, followed by its remaining segments and +END) clears it. Both sets must survive ripping. The parser also computes +`BlockDuration = clear_pts - display_pts` for the visible set, retaining the +existing fallback duration when the actual end is unavailable. + +Previously, the parser discarded the empty PCS and relied solely on +`BlockDuration`. FFmpeg's PGS decoder sets `end_display_time` to `UINT32_MAX` +and expects another composition to replace or clear it. The generic decoder +only uses packet duration when `end_display_time` is zero. A player using +that decoded lifetime can therefore leave a subtitle visible until the next +one, however large the gap. See the [PGS decoder][pgsdec] and +[generic subtitle decoder][decode]. + +Empty compositions are now accumulated with their WDS/END continuation +packets, retaining the opening PCS's timestamp and source position. A +complete clear is emitted at END without waiting for the next subtitle. +Its duration is zero (the MKV writer rounds this to one timestamp tick); +it changes decoder state immediately and has no visible bitmap to expire. +Clear sets do not count toward forced-track classification. + +### Relationship to freemkv/freemkv#52 + +[#52] reports missing timestamps during FFmpeg transcoding with subtitle +stream copy. Adding a duration to every block does not, by itself, guarantee +valid packet timestamps after bitstream filtering. FFmpeg's Matroska muxer +automatically applies [`pgs_frame_merge`][merge]. When it merges separate +WDS and END packets, it copies packet properties from the PCS-containing +packet. Dropping that PCS leaves an orphaned set whose merged packet can +have no timestamp even though each input MKV block had one. + +Running the new FFmpeg tests against the old parser reproduces both reported +warnings (unset timestamps and fabricated PTS), and decodes the supposed +clear events as visible compositions. Both tests pass with the fixed parser. +Preserving the complete clear set addresses that mechanism as well as the +lingering bitmap. This does not establish that every playback/transcode +problem in #52 has the same cause; verification against the reporter's +source is still needed. + +### Regression coverage + +`src/mux/codec/pgs_tests.rs` builds decoder-valid synthetic PGS sets with a +2x2 bitmap; no movie/disc assets are needed. Coverage includes an hour-long +gap between forced subtitles, split/timestamp-less continuations, leading +and repeated clears, byte preservation, provenance, incomplete sets, +forced-track classification, and a parser → MKV → reader round trip. +The `.sup` writer tests also cover original clear precedence and fallback +clears for older MKVs carrying only durations. + +Two additional Rust tests invoke `ffprobe`/`ffmpeg`: one checks decoded +empty compositions at the actual clear times, the other stream-copies the +MKV and checks warnings, timestamps, corruption flags and decoded clears. +These are explicitly ignored in normal local runs because they require +external binaries. The QA workflow installs FFmpeg on Linux and runs them +against the release build; dev CI runs the pure Rust coverage: + +```sh +cargo test --lib pgs +cargo test --release --lib mux::codec::pgs::lifecycle_tests::ffmpeg_ -- --ignored +``` + +[pgsdec]: https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/pgssubdec.c +[decode]: https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/decode.c +[merge]: https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/bsf/pgs_frame_merge.c +[#52]: https://github.com/freemkv/freemkv/issues/52 ## `display_set_is_forced` diff --git a/src/mux/codec/adts.rs b/src/mux/codec/adts.rs index fc5725d6..8e5d9ac7 100644 --- a/src/mux/codec/adts.rs +++ b/src/mux/codec/adts.rs @@ -1,15 +1,11 @@ -//! AAC ADTS decodability gate. +//! AAC ADTS framing and validation. //! -//! Per ISO/IEC 13818-7 / ISO/IEC 14496-3, a header is structurally invalid -//! in exactly three ways treated as hard rejects: syncword != 0xFFF, a -//! reserved `sampling_frequency_index`, and `aac_frame_length` shorter than -//! the header itself. The ADTS CRC is not verified, only skipped. -//! A packet with no ADTS sync is raw AAC (e.g. from an MP4 container) or a -//! continuation and passes through unchanged — never false-dropped. -//! See docs/adts.md for the full rationale. +//! See docs/adts.md for framing, timestamp and validation rules. -use super::dropgate::DropTally; -use super::{CodecParser, Frame, PesPacket, pts_to_ns}; +use super::audio_frames::{AudioFrames, Header}; +#[cfg(test)] +use super::pts_to_ns; +use super::{CodecParser, Frame, PesPacket}; /// ADTS `sampling_frequency_index` table (ISO/IEC 14496-3) — 13 valid entries; /// indices 13/14/15 are 0 (reserved) and constitute a hard reject. @@ -57,11 +53,8 @@ fn adts_verdict(data: &[u8]) -> AdtsVerdict { } pub struct AdtsParser { - tally: DropTally, - /// Last emitted PTS (ns). A PES with no PTS (legal for audio, e.g. a - /// post-discontinuity continuation) carries this forward rather than resetting - /// the timeline to 0 — matching the AC-3/DTS parsers and preserving A/V sync. - last_pts_ns: i64, + frames: AudioFrames, + config: Option>, } impl Default for AdtsParser { @@ -73,67 +66,52 @@ impl Default for AdtsParser { impl AdtsParser { pub fn new() -> Self { Self { - tally: DropTally::new("aac"), - last_pts_ns: 0, + frames: AudioFrames::new("aac"), + config: None, } } - pub fn dropped_frames(&self) -> u64 { - self.tally.dropped_frames() + self.frames.dropped_frames() } - pub fn dropped_duration_ns(&self) -> u64 { - self.tally.dropped_duration_ns() + self.frames.dropped_duration_ns() } } impl CodecParser for AdtsParser { fn parse(&mut self, pes: &PesPacket) -> Vec { - if pes.data.is_empty() { - return Vec::new(); - } - let pts_ns = pes - .pts - .or(pes.dts) - .map(pts_to_ns) - .unwrap_or(self.last_pts_ns); - self.last_pts_ns = pts_ns; - - let drop = - self.tally.is_poisoned() || matches!(adts_verdict(&pes.data), AdtsVerdict::Invalid); - if drop { - let reason = if self.tally.is_poisoned() { - "track-poisoned" - } else { - "header" - }; - self.tally.record_drop(pts_ns, 0, pes.data.len(), reason); - return Vec::new(); - } - - self.tally.record_kept(); - // One PES is one unit here, so the unit's first byte is in THIS packet - // and its facts are this packet's -- the same rule the buffering - // parsers apply through `PesBuf::front`, with nothing carried over. - let facts = super::pesbuf::PesFacts::of(pes); - vec![Frame { - discontinuity: facts.discontinuity, - coding: None, - source: facts.source, - pts_ns, - keyframe: true, - data: pes.data.clone(), - duration_ns: None, - }] + let config = &mut self.config; + self.frames.parse(pes, 7, |data| { + if !matches!(adts_verdict(data), AdtsVerdict::Valid) { + return None; + } + let rate_index = (data[2] >> 2) & 15; + let object_type = (data[2] >> 6) + 1; + let channels = ((data[2] & 1) << 2) | (data[3] >> 6); + // MPEG-4 AudioSpecificConfig replaces the ADTS transport header in + // Matroska. The raw AAC payload must not retain that header or CRC. + config.get_or_insert_with(|| { + vec![ + (object_type << 3) | (rate_index >> 1), + (rate_index << 7) | (channels << 3), + ] + }); + Some(Header { + bytes: (usize::from(data[3] & 3) << 11) + | (usize::from(data[4]) << 3) + | usize::from(data[5] >> 5), + skip: if data[1] & 1 == 0 { 9 } else { 7 }, + samples: 1024 * (u32::from(data[6] & 3) + 1), + rate: ADTS_SAMPLE_RATE_VALID[usize::from(rate_index)], + }) + }) } fn flush(&mut self) -> Vec { - self.tally.log_summary(); - Vec::new() + self.frames.flush() } - fn codec_private(&self) -> Option> { - None + self.config.clone() } } @@ -217,9 +195,9 @@ mod tests { } #[test] - fn pes_without_pts_carries_last_timestamp_not_zero() { + fn pes_without_pts_advances_by_sample_count() { // A PES with no PTS (legal for audio, e.g. after a discontinuity) must - // carry the last known timestamp forward — resetting to 0 would corrupt + // advance from the last timestamp by sample count — resetting to 0 would corrupt // A/V sync. let mut p = AdtsParser::new(); p.parse(&make_pes(adts_frame(400), Some(90000))); @@ -227,8 +205,8 @@ mod tests { assert_eq!(f.len(), 1); assert_eq!( f[0].pts_ns, - pts_to_ns(90000), - "carried forward, not reset to 0" + pts_to_ns(90000) + 1024 * 1_000_000_000 / 44100, + "continuation advances by one AAC frame" ); } @@ -304,10 +282,10 @@ mod tests { } #[test] - fn short_packet_passes_through() { + fn short_sync_header_waits_for_continuation() { let mut p = AdtsParser::new(); let f = p.parse(&make_pes(vec![0xFF, 0xF1, 0x50], Some(0))); - assert_eq!(f.len(), 1, "too short to validate → kept"); + assert!(f.is_empty(), "partial ADTS header must be buffered"); } /// One PES is one unit here, so the frame carries that packet's offset. @@ -320,4 +298,69 @@ mod tests { assert!(!frames.is_empty(), "a valid ADTS frame is emitted"); assert_eq!(frames[0].source.map(|s| s.byte), Some(4_242)); } + #[test] + fn every_pes_split_reassembles_and_strips_adts() { + let data = adts_frame(40); + for split in 1..data.len() { + let mut p = AdtsParser::new(); + assert!( + p.parse(&make_pes(data[..split].to_vec(), Some(90000))) + .is_empty() + ); + let frames = p.parse(&make_pes(data[split..].to_vec(), None)); + assert_eq!(frames.len(), 1, "split {split}"); + assert_eq!(frames[0].data, data[7..]); + assert_eq!(frames[0].pts_ns, 1_000_000_000); + assert!(p.flush().is_empty()); + assert_eq!(p.dropped_frames(), 0); + } + } + + #[test] + fn multiple_adts_frames_in_one_pes_have_sample_timestamps() { + let data = adts_frame(20); + let mut p = AdtsParser::new(); + let frames = p.parse(&make_pes(data.repeat(3), Some(90000))); + assert_eq!(frames.len(), 3); + for (i, f) in frames.iter().enumerate() { + assert_eq!( + f.pts_ns, + 1_000_000_000 + i as i64 * (1024 * 1_000_000_000i64 / 44100) + ); + assert_eq!(f.data, data[7..]); + } + assert_eq!(p.codec_private(), Some(vec![0x12, 0x10])); + } + + #[test] + fn crc_bytes_are_removed_with_transport_header() { + let mut data = adts_frame(20); + data[1] &= !1; + let mut p = AdtsParser::new(); + let frames = p.parse(&make_pes(data.clone(), Some(0))); + assert_eq!(frames[0].data, data[9..]); + } + + #[test] + fn partial_tail_is_not_emitted_at_eof() { + let data = adts_frame(20); + let mut p = AdtsParser::new(); + assert!(p.parse(&make_pes(data[..10].to_vec(), Some(0))).is_empty()); + assert!(p.flush().is_empty()); + } + + #[test] + fn discontinuity_drops_partial_frame_and_reanchors() { + let data = adts_frame(20); + let mut p = AdtsParser::new(); + p.parse(&make_pes(data[..10].to_vec(), Some(0))); + let mut fresh = make_pes(data.clone(), Some(180000)); + fresh.discontinuity = true; + fresh.source = Some(crate::pes::SourcePos::at_byte(4096)); + let frames = p.parse(&fresh); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].pts_ns, 2_000_000_000); + assert_eq!(frames[0].source, fresh.source); + assert!(frames[0].discontinuity); + } } diff --git a/src/mux/codec/audio_frames.rs b/src/mux/codec/audio_frames.rs new file mode 100644 index 00000000..096c43de --- /dev/null +++ b/src/mux/codec/audio_frames.rs @@ -0,0 +1,138 @@ +//! Bounded framing and timestamp accounting shared by ADTS and MPEG audio. + +use super::dropgate::DropTally; +use super::pesbuf::{PesBuf, PesFacts}; +use super::{Frame, PesPacket}; + +pub(super) struct Header { + pub bytes: usize, + pub skip: usize, + pub samples: u32, + pub rate: u32, +} + +pub(super) struct AudioFrames { + buf: PesBuf, + framed: bool, + anchor: Option, + next_pts: i64, + tally: DropTally, +} + +impl AudioFrames { + pub fn new(codec: &'static str) -> Self { + Self { + buf: PesBuf::with_capacity(8192), + framed: false, + anchor: None, + next_pts: 0, + tally: DropTally::new(codec), + } + } + + pub fn dropped_frames(&self) -> u64 { + self.tally.dropped_frames() + } + pub fn dropped_duration_ns(&self) -> u64 { + self.tally.dropped_duration_ns() + } + + pub fn parse( + &mut self, + pes: &PesPacket, + min_header: usize, + mut header: impl FnMut(&[u8]) -> Option
, + ) -> Vec { + if pes.data.is_empty() { + return Vec::new(); + } + let facts = PesFacts::of(pes); + if pes.discontinuity { + self.buf.clear(); + self.anchor = None; + } + if self.tally.is_poisoned() { + self.tally.record_drop( + facts.presentation_ns().unwrap_or(self.next_pts), + 0, + pes.data.len(), + "track-poisoned", + ); + return Vec::new(); + } + // Preserve the existing raw-AAC/nonframed passthrough contract. Once + // sync has been seen, later nonsync bytes are continuations, not units. + if !self.framed && self.buf.is_empty() && pes.data[0] != 0xff { + self.tally.record_kept(); + let pts_ns = facts.presentation_ns().unwrap_or(self.next_pts); + self.next_pts = pts_ns; + return vec![Frame { + pts_ns, + keyframe: true, + data: pes.data.clone(), + source: facts.source, + discontinuity: facts.discontinuity, + ..Frame::default() + }]; + } + self.framed = true; + // Audio PES packets are small. Refuse pathological accumulation before + // copying; a corrupt size must not grow memory without bound. + if self.buf.len().saturating_add(pes.data.len()) > 1024 * 1024 { + self.buf.clear(); + self.tally.record_drop( + facts.presentation_ns().unwrap_or(self.next_pts), + 0, + pes.data.len(), + "buffer-limit", + ); + return Vec::new(); + } + self.buf.push(pes); + let mut frames = Vec::new(); + let mut consumed = 0; + while self.buf.len() - consumed >= min_header { + let data = &self.buf.as_slice()[consumed..]; + let Some(h) = header(data) else { + if data[0] == 0xff && data[1] & 0xe0 == 0xe0 { + self.tally + .record_drop(self.next_pts, 0, min_header, "header"); + } + consumed += 1; + continue; + }; + if data.len() < h.bytes { + break; + } + let facts = self.buf.facts_at(consumed); + if let Some(pts) = facts.presentation_ns() + && self.anchor != Some(pts) + { + self.anchor = Some(pts); + self.next_pts = pts; + } + let duration = u64::from(h.samples) * 1_000_000_000 / u64::from(h.rate); + frames.push(Frame { + pts_ns: self.next_pts, + keyframe: true, + data: data[h.skip..h.bytes].to_vec(), + duration_ns: Some(duration), + source: facts.source, + discontinuity: facts.discontinuity, + coding: None, + }); + self.next_pts = self.next_pts.saturating_add(duration as i64); + self.tally.record_kept(); + consumed += h.bytes; + } + self.buf.drain(consumed); + frames + } + + pub fn flush(&mut self) -> Vec { + // A trailing partial syncframe is not decodable; never manufacture one. + self.buf.clear(); + self.tally.log_summary(); + Vec::new() + } +} diff --git a/src/mux/codec/mod.rs b/src/mux/codec/mod.rs index 0a851150..8802086a 100644 --- a/src/mux/codec/mod.rs +++ b/src/mux/codec/mod.rs @@ -11,6 +11,7 @@ pub mod ac3; pub mod adts; +mod audio_frames; /// Codec-agnostic per-picture coding carrier (`PictureInfo` + accessors). pub mod coding; /// DTS / DTS-HD elementary-stream parser. diff --git a/src/mux/codec/mpegaudio.rs b/src/mux/codec/mpegaudio.rs index bc2e3def..55a2d34a 100644 --- a/src/mux/codec/mpegaudio.rs +++ b/src/mux/codec/mpegaudio.rs @@ -1,16 +1,11 @@ -//! MPEG-1/2/2.5 audio (MP1/MP2/MP3) decodability gate. +//! MPEG audio framing and validation. //! -//! Validates header sanity + framing resync per ISO/IEC 11172-3 / ISO/IEC -//! 13818-3, not a payload CRC. ACCEPTS free-format (`bitrate_index == 0`) as a -//! legal decodable mode (see `mpa_verdict`'s doc). Rejects only truly invalid -//! headers (bad version / layer / sample-rate / reserved bitrate index 15) → -//! dropped as a silence gap, each packet keeping its own PTS. A packet with no -//! leading sync passes through unchanged — never false-dropped. -//! -//! See docs/mpegaudio.md for the CRC and free-format-reject rationale. +//! See docs/mpegaudio.md for framing, timestamp and validation rules. -use super::dropgate::DropTally; -use super::{CodecParser, Frame, PesPacket, pts_to_ns}; +use super::audio_frames::{AudioFrames, Header}; +#[cfg(test)] +use super::pts_to_ns; +use super::{CodecParser, Frame, PesPacket}; /// Decoded validity of a candidate MPEG-audio header. enum MpaVerdict { @@ -50,77 +45,91 @@ fn mpa_verdict(data: &[u8]) -> MpaVerdict { MpaVerdict::Valid } -pub struct MpegAudioParser { - tally: DropTally, - /// Last emitted PTS (ns), carried forward across a PES with no PTS rather than - /// resetting the timeline to 0 (see the AC-3/DTS parsers) — preserves A/V sync. - last_pts_ns: i64, +// ISO/IEC 11172-3 and 13818-3 header bitrate tables, in kbit/s. +const MPEG1_L1: [u32; 15] = [ + 0, 32, 64, 96, 128, 160, 192, 224, 256, 288, 320, 352, 384, 416, 448, +]; +const MPEG1_L2: [u32; 15] = [ + 0, 32, 48, 56, 64, 80, 96, 112, 128, 160, 192, 224, 256, 320, 384, +]; +const MPEG1_L3: [u32; 15] = [ + 0, 32, 40, 48, 56, 64, 80, 96, 112, 128, 160, 192, 224, 256, 320, +]; +const MPEG2_L1: [u32; 15] = [ + 0, 32, 48, 56, 64, 80, 96, 112, 128, 144, 160, 176, 192, 224, 256, +]; +const MPEG2_L23: [u32; 15] = [0, 8, 16, 24, 32, 40, 48, 56, 64, 80, 96, 112, 128, 144, 160]; + +fn frame_header(data: &[u8]) -> Option
{ + if !matches!(mpa_verdict(data), MpaVerdict::Valid) { + return None; + } + let version = (data[1] >> 3) & 3; + let layer = (data[1] >> 1) & 3; + let rate = [44100, 48000, 32000][usize::from((data[2] >> 2) & 3)] + >> match version { + 3 => 0, + 2 => 1, + _ => 2, + }; + let rates = match (version == 3, layer) { + (true, 3) => MPEG1_L1, + (true, 2) => MPEG1_L2, + (true, _) => MPEG1_L3, + (false, 3) => MPEG2_L1, + (false, _) => MPEG2_L23, + }; + let bitrate = rates[usize::from(data[2] >> 4)] * 1000; + let padding = u32::from((data[2] >> 1) & 1); + let samples = match layer { + 3 => 384, + 1 if version != 3 => 576, + _ => 1152, + }; + let bytes = if bitrate == 0 { + // Free-format has no signalled size; retain PES-granular passthrough. + data.len() + } else if layer == 3 { + ((12 * bitrate / rate + padding) * 4) as usize + } else { + ((samples / 8) * bitrate / rate + padding) as usize + }; + Some(Header { + bytes, + skip: 0, + samples, + rate, + }) } +pub struct MpegAudioParser { + frames: AudioFrames, +} impl Default for MpegAudioParser { fn default() -> Self { Self::new() } } - impl MpegAudioParser { pub fn new() -> Self { Self { - tally: DropTally::new("mpegaudio"), - last_pts_ns: 0, + frames: AudioFrames::new("mpegaudio"), } } - pub fn dropped_frames(&self) -> u64 { - self.tally.dropped_frames() + self.frames.dropped_frames() } - pub fn dropped_duration_ns(&self) -> u64 { - self.tally.dropped_duration_ns() + self.frames.dropped_duration_ns() } } - impl CodecParser for MpegAudioParser { fn parse(&mut self, pes: &PesPacket) -> Vec { - if pes.data.is_empty() { - return Vec::new(); - } - let pts_ns = pes - .pts - .or(pes.dts) - .map(pts_to_ns) - .unwrap_or(self.last_pts_ns); - self.last_pts_ns = pts_ns; - - let drop = - self.tally.is_poisoned() || matches!(mpa_verdict(&pes.data), MpaVerdict::Invalid); - if drop { - let reason = if self.tally.is_poisoned() { - "track-poisoned" - } else { - "header" - }; - self.tally.record_drop(pts_ns, 0, pes.data.len(), reason); - return Vec::new(); - } - - self.tally.record_kept(); - vec![Frame { - discontinuity: pes.discontinuity, - coding: None, - source: super::pesbuf::PesFacts::of(pes).source, - pts_ns, - keyframe: true, - data: pes.data.clone(), - duration_ns: None, - }] + self.frames.parse(pes, 4, frame_header) } - fn flush(&mut self) -> Vec { - self.tally.log_summary(); - Vec::new() + self.frames.flush() } - fn codec_private(&self) -> Option> { None } @@ -144,9 +153,9 @@ mod tests { /// A valid MPEG-1 Layer III header: sync 0xFFF, version MPEG-1 (11), layer /// III (01), bitrate_index 9, sample-rate 0 (44.1 kHz), no CRC. Bytes: /// 0xFF 0xFB 0x90 0x00 — the canonical MP3 frame header. - fn mp3_frame(payload: usize) -> Vec { + fn mp3_frame(_payload: usize) -> Vec { let mut f = vec![0xFF, 0xFB, 0x90, 0x00]; - f.extend(std::iter::repeat_n(0xAA, payload)); + f.extend(std::iter::repeat_n(0xAA, 413)); f } @@ -160,9 +169,9 @@ mod tests { } #[test] - fn pes_without_pts_carries_last_timestamp_not_zero() { + fn pes_without_pts_advances_by_sample_count() { // A PES with no PTS (legal for audio, e.g. after a discontinuity) must - // carry the last known timestamp forward — resetting to 0 would corrupt + // advance from the last timestamp by sample count — resetting to 0 would corrupt // A/V sync. Mirrors the adts.rs guard test. let mut p = MpegAudioParser::new(); p.parse(&make_pes(mp3_frame(400), Some(90000))); @@ -170,8 +179,8 @@ mod tests { assert_eq!(f.len(), 1); assert_eq!( f[0].pts_ns, - pts_to_ns(90000), - "carried forward, not reset to 0" + pts_to_ns(90000) + 1152 * 1_000_000_000 / 44100, + "continuation advances by one MPEG audio frame" ); } @@ -278,8 +287,8 @@ mod tests { ); } - // Self-framing at PES granularity: parse emits/drops immediately, buffers - // nothing, so end-of-stream has nothing left to hand over. + // Complete frames have already been emitted; EOF must not turn a trailing + // invalid header or partial frame into another access unit. // See docs/mpegaudio.md — flush-no-phantom-frame rationale. #[test] fn flush_adds_no_phantom_frame_after_the_last_real_packet() { @@ -317,4 +326,47 @@ mod tests { assert!(!f.is_empty(), "the frame is emitted"); assert_eq!(f[0].source.map(|s| s.byte), Some(7_777)); } + #[test] + fn every_pes_split_reassembles_a_complete_mpeg_frame() { + let data = mp3_frame(413); + for split in 1..data.len() { + let mut p = MpegAudioParser::new(); + assert!( + p.parse(&make_pes(data[..split].to_vec(), Some(90000))) + .is_empty() + ); + let frames = p.parse(&make_pes(data[split..].to_vec(), None)); + assert_eq!(frames.len(), 1, "split {split}"); + assert_eq!(frames[0].data, data); + assert_eq!(frames[0].pts_ns, 1_000_000_000); + assert_eq!(p.dropped_frames(), 0); + } + } + + #[test] + fn multiple_mpeg_frames_in_one_pes_get_distinct_timestamps() { + let data = mp3_frame(413); + let mut p = MpegAudioParser::new(); + let frames = p.parse(&make_pes(data.repeat(3), Some(0))); + assert_eq!(frames.len(), 3); + for (i, frame) in frames.iter().enumerate() { + assert_eq!(frame.data, data); + assert_eq!(frame.pts_ns, i as i64 * (1152 * 1_000_000_000i64 / 44100)); + } + } + + #[test] + fn frame_sizes_cover_versions_layers_and_padding() { + for (header, bytes, samples, rate) in [ + ([0xff, 0xfb, 0x90, 0], 417, 1152, 44100), + ([0xff, 0xfb, 0x92, 0], 418, 1152, 44100), + ([0xff, 0xfd, 0xa4, 0], 576, 1152, 48000), + ([0xff, 0xff, 0x90, 0], 312, 384, 44100), + ([0xff, 0xf3, 0x80, 0], 208, 576, 22050), + ([0xff, 0xe3, 0x80, 0], 417, 576, 11025), + ] { + let h = frame_header(&header).unwrap(); + assert_eq!((h.bytes, h.samples, h.rate), (bytes, samples, rate)); + } + } } diff --git a/src/mux/codec/pgs.rs b/src/mux/codec/pgs.rs index 15f002e1..a121ced6 100644 --- a/src/mux/codec/pgs.rs +++ b/src/mux/codec/pgs.rs @@ -5,13 +5,15 @@ //! //! Subtitle display lifecycle (BD spec): a "display" PCS //! (number_of_composition_objects > 0) starts a visible subtitle, and a -//! later "empty" PCS (== 0) clears it. For Matroska output we collapse that -//! pair into one block with `BlockDuration` set to (clear_pts - display_pts). -// See docs/pgs.md — why a missing BlockDuration makes hardware players linger. +//! later "empty" PCS (== 0) clears it. Preserve BOTH display sets: some +//! decoders rely on the empty PCS + END even when Matroska carries a duration. +//! Also set the visible block's `BlockDuration` to (clear_pts - display_pts). +// See docs/pgs.md for decoder and remux interoperability. use super::{CodecParser, Frame, PesPacket, pts_to_ns}; const SEGMENT_PCS: u8 = 0x16; +const SEGMENT_END: u8 = 0x80; // Upper bound on a pending display set's bytes (real sets are well under 1 MB). // Caps a malformed stream that appends non-PCS segments forever without a PCS, // dropping further appends until the next PCS resyncs — mirrors DTS/AC-3 caps. @@ -188,12 +190,12 @@ impl ForcedTracker { } } -/// Stateful parser that collapses PGS display/clear PCS pairs into -/// duration-bearing Matroska frames. Implements [`CodecParser`]. +/// Stateful parser that preserves PGS display and clear sets, adding durations +/// to visible subtitles. Implements [`CodecParser`]. pub struct PgsParser { /// The display set being accumulated, with the facts of the PES that /// STARTED it. A set spans PES packets — it opens on a display PCS and - /// closes on the next one — so its timestamp and its source offset are the + /// closes on the next one (or END for a clear) — so its timestamp and source are the /// opening packet's, never the closing packet's. Same rule the other /// buffering parsers get from `PesBuf::front`. pending: Option<(super::pesbuf::PesFacts, Vec)>, @@ -211,17 +213,48 @@ impl PgsParser { Self { pending: None } } - // Emit the pending display set as a Frame from its start PTS to - // `end_pts_ns`, clamped to >= 0. Shared by the clear-PCS and replace-PCS - // arms so the Frame shape stays in one place. - fn emit_pending(&mut self, end_pts_ns: i64) -> Option { + fn is_clear(data: &[u8]) -> bool { + data.first() == Some(&SEGMENT_PCS) && data.get(PCS_NUM_OBJECTS_OFFSET) == Some(&0) + } + + // Clear sets must include END: it is what makes a decoder apply the empty + // composition. Walk segment lengths, never search bitmap/palette bytes for + // 0x80. END can arrive in a later PES without its own timestamp. + fn complete_clear_pts(&self) -> Option { + let (facts, data) = self.pending.as_ref()?; + if !Self::is_clear(data) { + return None; + } + let mut rest = data.as_slice(); + while rest.len() >= 3 { + let size = 3 + usize::from(u16::from_be_bytes([rest[1], rest[2]])); + if size > rest.len() { + return None; + } + if rest[0] == SEGMENT_END && size == 3 && rest.len() == 3 { + return facts.presentation_ns(); + } + rest = &rest[size..]; + } + None + } + + // One emission path for clear, replacement, malformed input and EOF. + // Missing end times use the existing fallback only for visible sets. + fn emit_pending(&mut self, end_pts_ns: Option) -> Option { let (facts, data) = self.pending.take()?; let start_pts = facts.presentation_ns().unwrap_or(0); - let computed = end_pts_ns.saturating_sub(start_pts).max(0) as u64; + let computed = end_pts_ns + .map(|end| end.saturating_sub(start_pts).max(0) as u64) + .unwrap_or(DEFAULT_PGS_DURATION_NS); // Trust the real computed span, but clamp a pathologically large one (a // missing intermediate PCS makes one set look like it lingered for // minutes) back to the fallback dwell rather than emit an absurd cue. - let duration = if computed > MAX_PGS_DURATION_NS { + let duration = if Self::is_clear(&data) { + // A clear is an instantaneous state change, not a visible cue. + // The MKV writer rounds this up to its minimum duration tick. + 0 + } else if computed > MAX_PGS_DURATION_NS { DEFAULT_PGS_DURATION_NS } else { computed @@ -252,23 +285,9 @@ impl CodecParser for PgsParser { // A PCS too short for number_of_composition_objects is malformed. Don't // let it fall to the non-PCS arm (would pollute the pending set): close - // any pending set undurated and drop the header to resync on next PCS. + // any pending set with a fallback and drop the header to resync on next PCS. if is_pcs && pes.data.len() <= PCS_NUM_OBJECTS_OFFSET { - return self - .pending - .take() - .map(|(facts, data)| { - vec![Frame { - discontinuity: false, - coding: None, - source: facts.source, - pts_ns: facts.presentation_ns().unwrap_or(0), - keyframe: true, - data, - duration_ns: Some(DEFAULT_PGS_DURATION_NS), - }] - }) - .unwrap_or_default(); + return self.emit_pending(None).into_iter().collect(); } let pcs_num_objects = if is_pcs { @@ -279,30 +298,12 @@ impl CodecParser for PgsParser { let mut out = Vec::new(); match pcs_num_objects { - // Clear/empty PCS closes any pending display; drop the segment - // itself (BlockDuration covers the wipe). No PTS means no - // duration, so the pending set is emitted lingering to EOF. - Some(0) => { - let frame = match pts { - Some(end) => self.emit_pending(end), - None => self.pending.take().map(|(facts, data)| Frame { - discontinuity: false, - coding: None, - source: facts.source, - pts_ns: facts.presentation_ns().unwrap_or(0), - keyframe: true, - data, - duration_ns: Some(DEFAULT_PGS_DURATION_NS), - }), - }; - out.extend(frame); - } - // Display PCS — start a new pending. If a prior display - // was never explicitly cleared (replace-without-clear), - // emit it with the new PCS's PTS as its end. + // Every PCS starts a new set, including empty compositions. Keep + // the clear's following WDS/END with it, rather than emitting orphan + // packets that lose timing in FFmpeg's pgs_frame_merge filter. Some(_) => match pts { Some(start) => { - out.extend(self.emit_pending(start)); + out.extend(self.emit_pending(Some(start))); // The set's facts are THIS packet's — the one that opened // it. `start` is that packet's PTS by construction. self.pending = Some((super::pesbuf::PesFacts::of(pes), pes.data.clone())); @@ -312,19 +313,11 @@ impl CodecParser for PgsParser { "the opening packet's PTS is the set's start" ); } - // A display PCS with no PTS has an unknown start time. Don't + // A PCS with no PTS has an unknown start time. Don't // store it with a 0 sentinel (wrong start, absurd duration). - // Flush any prior pending undurated and skip storing this one. + // Flush any prior pending with a fallback and skip this one. None => { - out.extend(self.pending.take().map(|(facts, data)| Frame { - discontinuity: false, - coding: None, - source: facts.source, - pts_ns: facts.presentation_ns().unwrap_or(0), - keyframe: true, - data, - duration_ns: Some(DEFAULT_PGS_DURATION_NS), - })); + out.extend(self.emit_pending(None)); } }, // Non-PCS first segment — either a continuation of the @@ -361,25 +354,19 @@ impl CodecParser for PgsParser { } } + // No need to wait for the next (possibly hour-distant) subtitle to + // release a completed clear. Its timestamp is the opening PCS's PTS. + if let Some(pts) = self.complete_clear_pts() { + out.extend(self.emit_pending(Some(pts))); + } out } fn flush(&mut self) -> Vec { // A display set is only emitted when the next PCS arrives; at EOS there // is no follower, so without this the last subtitle would be silently - // dropped. Emit it with no duration — it lingers to EOF (see module doc). - match self.pending.take() { - Some((facts, data)) => vec![Frame { - discontinuity: false, - coding: None, - source: facts.source, - pts_ns: facts.presentation_ns().unwrap_or(0), - keyframe: true, - data, - duration_ns: Some(DEFAULT_PGS_DURATION_NS), - }], - None => Vec::new(), - } + // dropped. Keep the fallback duration for a display with no known end. + self.emit_pending(None).into_iter().collect() } fn codec_private(&self) -> Option> { @@ -755,12 +742,16 @@ mod tests { // --- clear / replace edge cases --- #[test] - fn clear_with_no_pending_emits_nothing() { - // An empty PCS arriving with no pending display is a no-op. + fn clear_with_no_pending_is_preserved_until_end_or_flush() { + // Preserve a leading clear too: decoder state can outlive a seek. let mut parser = PgsParser::new(); let f = parser.parse(&make_pes(pcs_bytes(0), Some(90000))); - assert!(f.is_empty(), "clear with nothing pending → no frame"); - assert!(parser.pending.is_none()); + assert!(f.is_empty(), "wait for the clear's END"); + let tail = parser.flush(); + assert_eq!(tail.len(), 1); + assert_eq!(tail[0].data, pcs_bytes(0)); + assert_eq!(tail[0].pts_ns, 1_000_000_000); + assert_eq!(tail[0].duration_ns, Some(0)); } #[test] @@ -948,3 +939,7 @@ mod tests { ); } } + +#[cfg(test)] +#[path = "pgs_tests.rs"] +mod lifecycle_tests; diff --git a/src/mux/codec/pgs_tests.rs b/src/mux/codec/pgs_tests.rs new file mode 100644 index 00000000..21aaaf27 --- /dev/null +++ b/src/mux/codec/pgs_tests.rs @@ -0,0 +1,632 @@ +//! Decoder-valid synthetic PGS fixtures: no disc data or external assets. +//! Unit tests run without FFmpeg; the explicitly ignored interoperability tests +//! require ffmpeg/ffprobe and are also run by CI. + +use super::*; +use crate::disc::{Codec, SubtitleStream}; +use crate::mux::mkv::{MkvMuxer, MkvTrack}; +use crate::mux::mkvstream::MkvStream; +use crate::pes::{PesFrame, SourcePos, Stream}; +use std::io::Cursor; +use std::path::Path; +use std::process::Command; + +fn segment(kind: u8, payload: &[u8]) -> Vec { + let mut data = vec![kind]; + data.extend_from_slice(&u16::try_from(payload.len()).unwrap().to_be_bytes()); + data.extend_from_slice(payload); + data +} + +fn pcs(objects: u8, forced: bool, number: u16) -> Vec { + let mut data = vec![0x07, 0x80, 0x04, 0x38, 0x10]; // 1920x1080 + data.extend_from_slice(&number.to_be_bytes()); + data.extend_from_slice(&[if objects == 0 { 0 } else { 0x80 }, 0, 0, objects]); + for object in 0..objects { + data.extend_from_slice(&[0, object, 0, if forced { 0x40 } else { 0 }, 0, 0, 0, 0]); + } + segment(0x16, &data) +} + +fn window() -> Vec { + segment(0x17, &[1, 0, 0, 0, 0, 0, 0, 2, 0, 2]) +} + +fn end() -> Vec { + segment(0x80, &[]) +} + +fn display(forced: bool, number: u16) -> Vec { + [ + pcs(1, forced, number), + window(), + // Palette 0, version 0, colour 1: opaque white. + segment(0x14, &[0, 0, 1, 235, 128, 128, 255]), + // Object 0, first+last, 2x2, RLE: two white pixels + EOL per row. + segment( + 0x15, + &[0, 0, 0, 0xc0, 0, 0, 12, 0, 2, 0, 2, 1, 1, 0, 0, 1, 1, 0, 0], + ), + end(), + ] + .concat() +} + +fn clear(number: u16) -> Vec { + [pcs(0, false, number), window(), end()].concat() +} + +fn packet(data: Vec, pts: Option) -> PesPacket { + PesPacket { + pid: 0x1200, + pts, + dts: None, + data, + source: None, + discontinuity: false, + } +} + +fn subtitle_track() -> MkvTrack { + MkvTrack::subtitle(&SubtitleStream { + pid: 0x1200, + codec: Codec::Pgs, + language: "eng".into(), + forced: false, + qualifier: crate::labels::LabelQualifier::None, + codec_data: None, + }) +} + +fn mux(frames: &[Frame]) -> Vec { + let mut bytes = Cursor::new(Vec::new()); + let mut muxer = MkvMuxer::new(&mut bytes, &[subtitle_track()], None, 0.0, &[]).unwrap(); + for frame in frames { + muxer + .write_frame( + 0, + frame.pts_ns, + frame.keyframe, + &frame.data, + frame.duration_ns, + None, + ) + .unwrap(); + } + muxer.finish().unwrap(); + bytes.into_inner() +} + +fn read_mkv(bytes: Vec) -> Vec { + let mut reader = MkvStream::open(Cursor::new(bytes)).unwrap(); + let mut frames = Vec::new(); + while let Some(frame) = reader.read().unwrap() { + frames.push(frame); + } + frames +} + +fn sparse_sequence() -> Vec { + let mut parser = PgsParser::new(); + let mut frames = Vec::new(); + for (start, number) in [(180, 0), (3780, 2)] { + frames.extend(parser.parse(&packet(display(true, number), Some(start * 90_000)))); + frames.extend(parser.parse(&packet( + pcs(0, false, number + 1), + Some((start + 3) * 90_000), + ))); + // Separate WDS and END PES packets reproduce the old orphaned segments. + frames.extend(parser.parse(&packet(window(), Some((start + 3) * 90_000)))); + frames.extend(parser.parse(&packet(end(), Some((start + 3) * 90_000)))); + } + frames.extend(parser.flush()); + frames +} + +#[test] +fn forced_subtitles_clear_before_an_hour_long_gap() { + let frames = sparse_sequence(); + assert_eq!(frames.len(), 4); + assert_eq!( + frames.iter().map(|f| f.pts_ns).collect::>(), + vec![ + 180_000_000_000, + 183_000_000_000, + 3_780_000_000_000, + 3_783_000_000_000 + ] + ); + for (index, frame) in frames.iter().enumerate() { + assert_eq!( + frame.data, + if index % 2 == 0 { + display(true, index as u16) + } else { + clear(index as u16) + } + ); + assert_eq!( + frame.duration_ns, + Some(if index % 2 == 0 { 3_000_000_000 } else { 0 }) + ); + } +} + +#[test] +fn clear_emits_at_end_without_waiting_for_another_subtitle() { + let mut parser = PgsParser::new(); + assert!( + parser + .parse(&packet(display(true, 0), Some(90_000))) + .is_empty() + ); + let visible = parser.parse(&packet(pcs(0, false, 1), Some(360_000))); + assert_eq!(visible.len(), 1); + assert_eq!(visible[0].duration_ns, Some(3_000_000_000)); + assert!(parser.parse(&packet(window(), None)).is_empty()); + let wipe = parser.parse(&packet(end(), None)); + assert_eq!(wipe.len(), 1); + assert_eq!(wipe[0].data, clear(1)); + assert_eq!(wipe[0].pts_ns, 4_000_000_000); + assert!(parser.flush().is_empty()); +} + +#[test] +fn complete_clear_in_one_pes_emits_display_and_clear_separately() { + let mut parser = PgsParser::new(); + parser.parse(&packet(display(false, 0), Some(90_000))); + let frames = parser.parse(&packet(clear(1), Some(270_000))); + assert_eq!(frames.len(), 2); + assert_eq!(frames[0].data, display(false, 0)); + assert_eq!(frames[1].data, clear(1)); + assert_eq!(frames[1].pts_ns, 3_000_000_000); +} + +#[test] +fn every_clear_continuation_split_preserves_bytes_and_opening_timestamp() { + let tail = [window(), end()].concat(); + for split in 0..=tail.len() { + let mut parser = PgsParser::new(); + let mut opening = packet(pcs(0, false, 7), Some(900_000)); + opening.source = Some(SourcePos::at_byte(777)); + let mut frames = parser.parse(&opening); + frames.extend(parser.parse(&packet(tail[..split].to_vec(), None))); + frames.extend(parser.parse(&packet(tail[split..].to_vec(), Some(999_000)))); + frames.extend(parser.flush()); + assert_eq!(frames.len(), 1, "split {split}"); + assert_eq!(frames[0].data, clear(7), "split {split}"); + assert_eq!(frames[0].pts_ns, 10_000_000_000, "split {split}"); + assert_eq!(frames[0].source.unwrap().byte, 777, "split {split}"); + } +} + +#[test] +fn leading_and_consecutive_clears_are_preserved() { + let mut parser = PgsParser::new(); + for number in 0..3 { + let frames = parser.parse(&packet(clear(number), Some(i64::from(number) * 90_000))); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].data, clear(number)); + assert_eq!(frames[0].duration_ns, Some(0)); + } + assert!(parser.flush().is_empty()); +} + +#[test] +fn incomplete_clear_is_flushed_once_at_eof() { + let mut parser = PgsParser::new(); + parser.parse(&packet(pcs(0, false, 1), Some(90_000))); + parser.parse(&packet(window(), None)); + let frames = parser.flush(); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].data, [pcs(0, false, 1), window()].concat()); + assert_eq!(frames[0].duration_ns, Some(0)); + assert!(parser.flush().is_empty()); +} + +#[test] +fn incomplete_clear_does_not_absorb_the_next_display() { + let mut parser = PgsParser::new(); + parser.parse(&packet(pcs(0, false, 0), Some(90_000))); + let frames = parser.parse(&packet(display(true, 1), Some(180_000))); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].data, pcs(0, false, 0)); + assert_eq!(frames[0].duration_ns, Some(0)); + assert_eq!(parser.flush()[0].data, display(true, 1)); +} + +#[test] +fn end_marker_inside_segment_payload_does_not_complete_clear() { + let mut parser = PgsParser::new(); + let data = [pcs(0, false, 0), segment(0x17, &[0x80, 0, 0])].concat(); + assert!(parser.parse(&packet(data.clone(), Some(90_000))).is_empty()); + let frames = parser.parse(&packet(end(), None)); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].data, [data, end()].concat()); +} + +#[test] +fn truncated_segment_length_cannot_fabricate_a_complete_clear() { + let mut parser = PgsParser::new(); + let bad = [pcs(0, false, 0), vec![0x17, 0xff, 0xff], end()].concat(); + assert!(parser.parse(&packet(bad, Some(90_000))).is_empty()); + // Resync at the next PCS and preserve the new valid clear. + let frames = parser.parse(&packet(clear(1), Some(180_000))); + assert_eq!(frames.len(), 2); + assert_eq!(frames[1].data, clear(1)); +} + +#[test] +fn clear_without_pts_never_uses_zero_as_its_timestamp() { + let mut parser = PgsParser::new(); + parser.parse(&packet(display(true, 0), Some(90_000))); + let frames = parser.parse(&packet(clear(1), None)); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].data, display(true, 0)); + assert_eq!(frames[0].duration_ns, Some(DEFAULT_PGS_DURATION_NS)); + assert!(parser.flush().is_empty()); +} + +#[test] +fn clear_timestamp_and_source_come_from_clear_not_display() { + let mut parser = PgsParser::new(); + let mut visible = packet(display(true, 0), Some(90_000)); + visible.source = Some(SourcePos::at_byte(111)); + parser.parse(&visible); + let mut wipe = packet(clear(1), Some(270_000)); + wipe.source = Some(SourcePos::at_byte(222)); + let frames = parser.parse(&wipe); + assert_eq!(frames.len(), 2); + assert_eq!(frames[0].source.unwrap().byte, 111); + assert_eq!(frames[1].source.unwrap().byte, 222); + assert_eq!(frames[1].pts_ns, 3_000_000_000); +} + +#[test] +fn clear_sets_do_not_demote_a_forced_track() { + let mut tracker = ForcedTracker::new(); + for frame in sparse_sequence() { + tracker.observe(&frame.data); + } + assert!(tracker.is_forced()); + assert_eq!( + tracker.facts(), + ForcedFacts { + displays: 2, + forced_displays: 2 + } + ); +} + +#[test] +fn clear_only_track_is_not_mistaken_for_a_forced_track() { + let mut tracker = ForcedTracker::new(); + for number in 0..10 { + tracker.observe(&clear(number)); + } + assert!(!tracker.observed()); + assert!(!tracker.is_forced()); + assert_eq!(tracker.facts(), ForcedFacts::default()); +} + +#[test] +fn ordinary_and_forced_subtitles_share_the_same_clear_lifecycle() { + for forced in [false, true] { + let mut parser = PgsParser::new(); + parser.parse(&packet(display(forced, 0), Some(90_000))); + let frames = parser.parse(&packet(clear(1), Some(270_000))); + assert_eq!(frames.len(), 2); + assert_eq!(display_set_is_forced(&frames[0].data), Some(forced)); + assert_eq!(display_set_is_forced(&frames[1].data), None); + assert_eq!(frames[0].duration_ns, Some(2_000_000_000)); + } +} + +#[test] +fn mkv_roundtrip_preserves_display_clear_bytes_and_timestamps() { + let original = sparse_sequence(); + let bytes = mux(&original); + let decoded = read_mkv(bytes.clone()); + assert_eq!(decoded.len(), 4); + for (before, after) in original.iter().zip(decoded) { + // A subtitle-only MKV is rebased to its first input timestamp. + assert_eq!(before.pts_ns - original[0].pts_ns, after.pts); + assert_eq!(before.data, after.data); + assert_eq!( + after.duration_ns, + Some(before.duration_ns.unwrap().max(100_000)) + ); + } + let reader = MkvStream::open(Cursor::new(bytes)).unwrap(); + assert!(reader.info().subtitle_streams().next().unwrap().forced); +} + +#[test] +fn display_segment_pes_boundaries_do_not_change_the_bitmap_or_duration() { + let data = display(true, 0); + let mut parser = PgsParser::new(); + let mut pos = 0; + while pos < data.len() { + let size = 3 + usize::from(u16::from_be_bytes([data[pos + 1], data[pos + 2]])); + let pts = if pos == 0 { Some(90_000) } else { None }; + assert!( + parser + .parse(&packet(data[pos..pos + size].to_vec(), pts)) + .is_empty() + ); + pos += size; + } + let frames = parser.parse(&packet(clear(1), Some(270_000))); + assert_eq!(frames.len(), 2); + assert_eq!(frames[0].data, data); + assert_eq!(frames[0].duration_ns, Some(2_000_000_000)); +} + +#[test] +fn empty_pes_does_not_flush_or_retime_a_pending_clear() { + let mut parser = PgsParser::new(); + parser.parse(&packet(pcs(0, false, 0), Some(90_000))); + assert!( + parser + .parse(&packet(Vec::new(), Some(9_000_000))) + .is_empty() + ); + let frames = parser.parse(&packet(end(), None)); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].pts_ns, 1_000_000_000); +} + +#[test] +fn malformed_next_pcs_does_not_give_an_incomplete_clear_a_visible_duration() { + for bad in [ + packet(vec![0x16, 0], Some(180_000)), + packet(display(true, 1), None), + ] { + let mut parser = PgsParser::new(); + parser.parse(&packet(pcs(0, false, 0), Some(90_000))); + let frames = parser.parse(&bad); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].data, pcs(0, false, 0)); + assert_eq!(frames[0].duration_ns, Some(0)); + assert!(parser.flush().is_empty()); + } +} + +#[test] +fn pending_clear_memory_is_bounded_and_next_pcs_resynchronizes() { + let mut parser = PgsParser::new(); + parser.parse(&packet(pcs(0, false, 0), Some(90_000))); + // A corrupt clear that keeps appending WDS-like bytes must remain bounded. + for _ in 0..20 { + parser.parse(&packet(vec![0x17; 256 * 1024], None)); + } + assert!(parser.pending.as_ref().unwrap().1.len() <= MAX_PGS_PENDING_BYTES); + let frames = parser.parse(&packet(clear(1), Some(180_000))); + assert_eq!(frames.len(), 2); + assert_eq!(frames[1].data, clear(1)); + assert!(parser.pending.is_none()); +} + +#[test] +fn duration_fallback_does_not_move_the_original_clear_timestamp() { + let mut parser = PgsParser::new(); + parser.parse(&packet(display(true, 0), Some(90_000))); + let frames = parser.parse(&packet(clear(1), Some(41 * 90_000))); + assert_eq!(frames.len(), 2); + assert_eq!(frames[0].duration_ns, Some(DEFAULT_PGS_DURATION_NS)); + assert_eq!(frames[1].pts_ns, 41_000_000_000); + assert_eq!(frames[1].data, clear(1)); +} + +#[test] +fn negative_source_origin_keeps_relative_subtitle_timing() { + let mut parser = PgsParser::new(); + parser.parse(&packet(display(true, 0), Some(-90_000))); + let frames = parser.parse(&packet(clear(1), Some(180_000))); + // pts_to_ns has sub-tick integer rounding for negative source timestamps. + assert_eq!(frames[0].pts_ns, pts_to_ns(-90_000)); + assert_eq!( + frames[0].duration_ns, + Some((2_000_000_000 - pts_to_ns(-90_000)) as u64) + ); + assert_eq!(frames[1].pts_ns, 2_000_000_000); +} + +#[test] +fn dense_dialogue_keeps_every_clear_and_does_not_accumulate_between_cues() { + let mut parser = PgsParser::new(); + let mut tracker = ForcedTracker::new(); + for cue in 0..100u16 { + assert!( + parser + .parse(&packet( + display(false, cue * 2), + Some(i64::from(cue) * 270_000) + )) + .is_empty() + ); + let frames = parser.parse(&packet( + clear(cue * 2 + 1), + Some(i64::from(cue) * 270_000 + 180_000), + )); + assert_eq!(frames.len(), 2); + assert_eq!(frames[0].duration_ns, Some(2_000_000_000)); + assert_eq!(frames[1].data, clear(cue * 2 + 1)); + assert!(parser.pending.is_none()); + for frame in frames { + tracker.observe(&frame.data); + } + } + assert_eq!( + tracker.facts(), + ForcedFacts { + displays: 100, + forced_displays: 0 + } + ); + assert!(!tracker.is_forced()); +} + +#[test] +fn two_subtitle_tracks_keep_independent_clear_times_and_forced_flags_in_mkv() { + let mut parsers = [PgsParser::new(), PgsParser::new()]; + let mut frames = Vec::new(); + for (track, start, stop, forced) in [(0, 0, 270_000, true), (1, 90_000, 450_000, false)] { + parsers[track].parse(&packet(display(forced, 0), Some(start))); + for frame in parsers[track].parse(&packet(clear(1), Some(stop))) { + frames.push((track, frame)); + } + } + frames.sort_by_key(|(_, f)| f.pts_ns); + let mut bytes = Cursor::new(Vec::new()); + let mut muxer = MkvMuxer::new( + &mut bytes, + &[subtitle_track(), subtitle_track()], + None, + 0.0, + &[], + ) + .unwrap(); + for (track, frame) in &frames { + muxer + .write_frame( + *track, + frame.pts_ns, + true, + &frame.data, + frame.duration_ns, + None, + ) + .unwrap(); + } + muxer.finish().unwrap(); + let mut reader = MkvStream::open(Cursor::new(bytes.into_inner())).unwrap(); + assert_eq!( + reader + .info() + .subtitle_streams() + .map(|s| s.forced) + .collect::>(), + [true, false] + ); + for (track, expected) in frames { + let actual = reader.read().unwrap().unwrap(); + assert_eq!(actual.track, track); + assert_eq!(actual.pts, expected.pts_ns); + assert_eq!(actual.data, expected.data); + } + assert!(reader.read().unwrap().is_none()); +} + +#[test] +fn pes_wire_roundtrip_preserves_clear_payload_and_zero_duration() { + let original = sparse_sequence(); + for frame in original { + let pes = PesFrame { + discard_padding_ns: 0, + track: 2, + pts: frame.pts_ns, + data: frame.data.clone(), + keyframe: true, + duration_ns: frame.duration_ns, + source: None, + coding: None, + }; + let mut wire = Vec::new(); + pes.serialize(&mut wire).unwrap(); + let result = PesFrame::deserialize(&mut wire.as_slice()) + .unwrap() + .unwrap(); + assert_eq!(result.track, 2); + assert_eq!(result.pts, frame.pts_ns); + assert_eq!(result.data, frame.data); + assert_eq!(result.duration_ns, frame.duration_ns); + } +} + +fn run(command: &mut Command) -> std::process::Output { + let output = command + .output() + .expect("install ffmpeg and ffprobe to run this test"); + assert!( + output.status.success(), + "{command:?}: {}", + String::from_utf8_lossy(&output.stderr) + ); + output +} + +fn probe(path: &Path, frames: bool) -> serde_json::Value { + let output = run(Command::new("ffprobe") + .args(["-v", "warning", "-of", "json"]) + .arg(if frames { + "-show_frames" + } else { + "-show_packets" + }) + .arg(path)); + assert!( + output.stderr.is_empty(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + serde_json::from_slice(&output.stdout).unwrap() +} + +#[test] +#[ignore = "requires ffmpeg and ffprobe; run explicitly in CI"] +fn ffmpeg_decodes_timed_empty_compositions() { + let dir = crate::mux::interop_tests::fixture_dir("pgs-decode"); + let input = dir.path().join("forced.mkv"); + std::fs::write(&input, mux(&sparse_sequence())).unwrap(); + let json = probe(&input, true); + let subtitles = json["frames"].as_array().unwrap(); + assert_eq!(subtitles.len(), 4, "{json}"); + for (index, (pts, rects)) in [(0.0, 1), (3.0, 0), (3600.0, 1), (3603.0, 0)] + .iter() + .enumerate() + { + assert_eq!(subtitles[index]["num_rects"], *rects, "{json}"); + assert_eq!( + subtitles[index]["pts_time"] + .as_str() + .unwrap() + .parse::() + .unwrap(), + *pts + ); + } +} + +#[test] +#[ignore = "requires ffmpeg and ffprobe; run explicitly in CI"] +fn ffmpeg_remux_keeps_all_subtitle_timestamps_without_warnings() { + let dir = crate::mux::interop_tests::fixture_dir("pgs-remux"); + let input = dir.path().join("input.mkv"); + let output = dir.path().join("remux.mkv"); + std::fs::write(&input, mux(&sparse_sequence())).unwrap(); + let result = run(Command::new("ffmpeg") + .args(["-nostdin", "-y", "-v", "warning", "-copyts", "-i"]) + .arg(&input) + .args(["-map", "0", "-c", "copy"]) + .arg(&output)); + assert!( + result.stderr.is_empty(), + "{}", + String::from_utf8_lossy(&result.stderr) + ); + let json = probe(&output, false); + let packets = json["packets"].as_array().unwrap(); + assert_eq!(packets.len(), 4, "{json}"); + for (packet, pts) in packets.iter().zip([0.0, 3.0, 3600.0, 3603.0]) { + assert_eq!( + packet["pts_time"].as_str().unwrap().parse::().unwrap(), + pts + ); + assert!(!packet["flags"].as_str().unwrap().contains('C'), "{packet}"); + } + // The clear commands must survive FFmpeg's automatic pgs_frame_merge BSF. + let json = probe(&output, true); + assert_eq!(json["frames"][1]["num_rects"], 0, "{json}"); + assert_eq!(json["frames"][3]["num_rects"], 0, "{json}"); +} diff --git a/src/mux/demux_sink.rs b/src/mux/demux_sink.rs index 0815fd00..0d07ed31 100644 --- a/src/mux/demux_sink.rs +++ b/src/mux/demux_sink.rs @@ -284,7 +284,12 @@ fn annexb_param_sets(codec: Codec, record: &[u8]) -> Vec { // PGS `.sup` writer: rebuilds the HDMV segment framing the parser stripped, // prefixing each segment with a 13-byte `PG` header (PTS/DTS). // See docs/demux-sink.md — PgsSupWriter. -struct PgsSupWriter; +#[derive(Default)] +struct PgsSupWriter { + // Delay a duration-derived clear until the next frame, so an original + // clear (or replacement PCS) at that timestamp takes precedence. + pending_clear: Option<(i64, u16, u16)>, +} // ── PGS / HDMV segment framing constants ───────────────────────────────────── // HDMV Presentation Graphics Stream, per BD-ROM Part 3 graphics-stream spec @@ -338,7 +343,7 @@ impl PgsSupWriter { w.write_all(&pts90k.to_be_bytes())?; w.write_all(&dts90k.to_be_bytes())?; w.write_all(&data[pos..seg_end])?; - written += SUP_HEADER_LEN + size; + written += SUP_HEADER_LEN + PGS_SEG_HEADER_LEN + size; pos = seg_end; } Ok(written) @@ -395,17 +400,39 @@ impl PgsSupWriter { impl EsWriter for PgsSupWriter { fn write_frame(&mut self, w: &mut dyn Write, f: &PesFrame, pts_ns: i64) -> io::Result { let pts90 = ns_to_90k(pts_ns); - let mut written = Self::emit_segments(&f.data, pts90, pts90, w)?; - // The parser folds the wipe time into `duration_ns` and drops the clear - // bytes; re-emit a synthetic clear at `pts + duration` so it times out. - if let Some(dur) = f.duration_ns { - let clear_pts = ns_to_90k(pts_ns.saturating_add(dur as i64)); + let is_pcs = f.data.first() == Some(&SEG_PCS) && f.data.len() > 13; + let mut written = 0; + if let Some((end, width, height)) = self.pending_clear { + if end < pts_ns || (end == pts_ns && !is_pcs) { + let clear = Self::synthetic_clear_display_set(width, height); + let pts = ns_to_90k(end); + written += Self::emit_segments(&clear, pts, pts, w)?; + self.pending_clear = None; + } else if is_pcs { + // A real clear/replacement at or before the computed end wins. + self.pending_clear = None; + } + } + written += Self::emit_segments(&f.data, pts90, pts90, w)?; + // Retain fallback support for old MKVs that only carry durations, and + // for a final display without a clear. Never synthesize a clear OF a + // clear (nor of a standalone WDS/END segment). + if let Some(dur) = f.duration_ns.filter(|_| is_pcs && f.data[13] > 0) { + let end = pts_ns.saturating_add(i64::try_from(dur).unwrap_or(i64::MAX)); let (w_px, h_px) = Self::pcs_dimensions(&f.data); - let clear = Self::synthetic_clear_display_set(w_px, h_px); - written += Self::emit_segments(&clear, clear_pts, clear_pts, w)?; + self.pending_clear = Some((end, w_px, h_px)); } Ok(written) } + + fn finish(&mut self, w: &mut dyn Write) -> io::Result<()> { + if let Some((end, width, height)) = self.pending_clear.take() { + let clear = Self::synthetic_clear_display_set(width, height); + let pts = ns_to_90k(end); + Self::emit_segments(&clear, pts, pts, w)?; + } + Ok(()) + } } /// VobSub writer: appends raw SPUs to the `.sub` and records `(pts, filepos)` @@ -496,7 +523,7 @@ fn es_writer_for( ) -> Box { match codec { Codec::Hevc | Codec::H264 => Box::new(AnnexBWriter::new(codec, codec_private)), - Codec::Pgs => Box::new(PgsSupWriter), + Codec::Pgs => Box::new(PgsSupWriter::default()), Codec::DvdSub => Box::new(VobSubWriter::new( idx_path.unwrap_or_else(|| PathBuf::from("subtitle.idx")), codec_private, @@ -1086,6 +1113,7 @@ mod tests { let mut sink = DemuxSink::create(&dir, &title, &opts).unwrap(); // Video starts at 500ms, audio at 600ms → true delay is +100ms. sink.write(&PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1096,6 +1124,7 @@ mod tests { }) .unwrap(); sink.write(&PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 1, @@ -1133,6 +1162,7 @@ mod tests { }; let mut sink = DemuxSink::create(&dir, &title, &opts).unwrap(); sink.write(&PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1167,6 +1197,7 @@ mod tests { let mut w = AnnexBWriter::new(Codec::H264, None); let mut out = Vec::new(); let f = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1202,6 +1233,7 @@ mod tests { let mut w = AnnexBWriter::new(Codec::H264, Some(&rec)); let mut out = Vec::new(); let f = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1232,6 +1264,7 @@ mod tests { let mut w = AnnexBWriter::new(Codec::H264, Some(&rec)); let mut out = Vec::new(); let f1 = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1253,6 +1286,7 @@ mod tests { // Second frame: NO param re-prepend. let mut out2 = Vec::new(); let f2 = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1323,7 +1357,7 @@ mod tests { assert_eq!(&out[2..6], &0x10u32.to_be_bytes()); // PTS assert_eq!(&out[6..10], &0x10u32.to_be_bytes()); // DTS assert_eq!(&out[SUP_HEADER_LEN..], &payload); // segment body verbatim - assert_eq!(written, SUP_HEADER_LEN + 2); + assert_eq!(written, out.len()); } #[test] @@ -1335,6 +1369,7 @@ mod tests { pcs.extend_from_slice(&[0x07, 0x80, 0x04, 0x38]); // 1920x1080 pcs.extend_from_slice(&[0x10, 0x00, 0x00, 0x80, 0x00, 0x00, 0x01]); // 1 object let f = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1344,8 +1379,9 @@ mod tests { duration_ns: Some(2_000_000_000), // 2s display → clear at 3s }; let mut out = Vec::new(); - let mut w = PgsSupWriter; + let mut w = PgsSupWriter::default(); w.write_frame(&mut out, &f, f.pts).unwrap(); + w.finish(&mut out).unwrap(); // Parse out every PG-framed segment: PG(2) PTS(4) DTS(4) type(1) size(2). let mut segs: Vec<(u8, u32)> = Vec::new(); @@ -1380,6 +1416,7 @@ mod tests { fn pgs_frame_without_duration_emits_no_clear() { // No duration → no synthetic clear (the subtitle's wipe time is unknown). let f = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1389,7 +1426,7 @@ mod tests { duration_ns: None, }; let mut out = Vec::new(); - let mut w = PgsSupWriter; + let mut w = PgsSupWriter::default(); w.write_frame(&mut out, &f, 0).unwrap(); // Exactly one PG-framed segment (the display), no clear appended. // Output = `.sup` header (10) + the on-wire segment (type+size 3 + 2 @@ -1402,6 +1439,159 @@ mod tests { ); } + fn sup_frame(pts: i64, duration_ns: Option, visible: bool) -> PesFrame { + let mut data = PgsSupWriter::synthetic_clear_display_set(1920, 1080); + if visible { + data[13] = 1; + data[2] += 8; + data.splice(14..14, [0, 0, 0, 0x40, 0, 0, 0, 0]); + } + PesFrame { + discard_padding_ns: 0, + track: 0, + pts, + duration_ns, + data, + keyframe: true, + coding: None, + source: None, + } + } + + fn sup_compositions(bytes: &[u8]) -> Vec<(u32, u8)> { + let mut compositions = Vec::new(); + let mut pos = 0; + while pos < bytes.len() { + assert_eq!(&bytes[pos..pos + 2], &SUP_MAGIC); + let pts = u32::from_be_bytes(bytes[pos + 2..pos + 6].try_into().unwrap()); + let data = &bytes[pos + SUP_HEADER_LEN..]; + let size = usize::from(u16::from_be_bytes([data[1], data[2]])); + if data[0] == SEG_PCS { + compositions.push((pts, data[13])); + } + pos += SUP_HEADER_LEN + PGS_SEG_HEADER_LEN + size; + } + assert_eq!(pos, bytes.len()); + compositions + } + + #[test] + fn pgs_sup_preserves_original_clear_without_a_duplicate_or_later_clear() { + let mut writer = PgsSupWriter::default(); + let mut bytes = Vec::new(); + for frame in [ + sup_frame(1_000_000_000, Some(2_000_000_000), true), + sup_frame(3_000_000_000, Some(100_000), false), + ] { + let before = bytes.len(); + let written = writer.write_frame(&mut bytes, &frame, frame.pts).unwrap(); + assert_eq!(written, bytes.len() - before); + } + writer.finish(&mut bytes).unwrap(); + assert_eq!(sup_compositions(&bytes), [(90_000, 1), (270_000, 0)]); + } + + #[test] + fn pgs_sup_replacement_cancels_old_clear_before_or_at_its_deadline() { + for replace_ns in [2_000_000_000, 3_000_000_000] { + let mut writer = PgsSupWriter::default(); + let mut bytes = Vec::new(); + for frame in [ + sup_frame(1_000_000_000, Some(2_000_000_000), true), + sup_frame(replace_ns, Some(4_000_000_000), true), + ] { + writer.write_frame(&mut bytes, &frame, frame.pts).unwrap(); + } + writer.finish(&mut bytes).unwrap(); + assert_eq!( + sup_compositions(&bytes), + [ + (90_000, 1), + (ns_to_90k(replace_ns), 1), + (ns_to_90k(replace_ns + 4_000_000_000), 0) + ] + ); + } + } + + #[test] + fn pgs_sup_legacy_duration_only_input_clears_before_a_later_display() { + let mut writer = PgsSupWriter::default(); + let mut bytes = Vec::new(); + for frame in [ + sup_frame(0, Some(2_000_000_000), true), + sup_frame(3_600_000_000_000, Some(3_000_000_000), true), + ] { + writer.write_frame(&mut bytes, &frame, frame.pts).unwrap(); + } + writer.finish(&mut bytes).unwrap(); + assert_eq!( + sup_compositions(&bytes), + [(0, 1), (180_000, 0), (324_000_000, 1), (324_270_000, 0)] + ); + } + + #[test] + fn pgs_sup_clear_only_input_never_synthesizes_another_clear() { + for duration in [None, Some(0), Some(100_000), Some(5_000_000_000)] { + let mut writer = PgsSupWriter::default(); + let mut bytes = Vec::new(); + let frame = sup_frame(1_000_000_000, duration, false); + writer.write_frame(&mut bytes, &frame, frame.pts).unwrap(); + writer.finish(&mut bytes).unwrap(); + assert_eq!(sup_compositions(&bytes), [(90_000, 0)]); + } + } + + #[test] + fn pgs_sup_continuation_does_not_cancel_or_create_a_pending_clear() { + let mut writer = PgsSupWriter::default(); + let mut bytes = Vec::new(); + let frame = sup_frame(1_000_000_000, Some(2_000_000_000), true); + writer.write_frame(&mut bytes, &frame, frame.pts).unwrap(); + let mut continuation = sup_frame(2_000_000_000, Some(5_000_000_000), false); + continuation.data = vec![SEG_END, 0, 0]; + writer + .write_frame(&mut bytes, &continuation, continuation.pts) + .unwrap(); + writer.finish(&mut bytes).unwrap(); + assert_eq!(sup_compositions(&bytes), [(90_000, 1), (270_000, 0)]); + } + + #[test] + fn pgs_sup_finish_is_idempotent() { + let mut writer = PgsSupWriter::default(); + let mut bytes = Vec::new(); + let frame = sup_frame(0, Some(2_000_000_000), true); + writer.write_frame(&mut bytes, &frame, 0).unwrap(); + writer.finish(&mut bytes).unwrap(); + let once = bytes.clone(); + writer.finish(&mut bytes).unwrap(); + assert_eq!(bytes, once); + } + + #[test] + fn pgs_sup_huge_duration_saturates_instead_of_wrapping_into_the_past() { + let mut writer = PgsSupWriter::default(); + let mut bytes = Vec::new(); + let frame = sup_frame(1_000_000_000, Some(u64::MAX), true); + writer.write_frame(&mut bytes, &frame, frame.pts).unwrap(); + writer.finish(&mut bytes).unwrap(); + assert_eq!(sup_compositions(&bytes), [(90_000, 1), (u32::MAX, 0)]); + } + + #[test] + fn pgs_sup_truncated_segment_is_not_partially_written() { + for data in [&[SEG_PCS][..], &[SEG_PCS, 0], &[SEG_PCS, 0, 2, 0xff]] { + let mut bytes = Vec::new(); + assert_eq!( + PgsSupWriter::emit_segments(data, 0, 0, &mut bytes).unwrap(), + 0 + ); + assert!(bytes.is_empty()); + } + } + #[test] fn ns_to_90k_conversion() { assert_eq!(ns_to_90k(0), 0); @@ -1419,6 +1609,7 @@ mod tests { let mut w = VobSubWriter::new(idx.clone(), Some(b"palette: 000000, ffffff"), "eng"); let mut sub = Vec::new(); let f1 = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1428,6 +1619,7 @@ mod tests { duration_ns: None, }; let f2 = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1556,6 +1748,7 @@ mod tests { ); let vid = |pts: i64, data: u8| PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 1, // VIDEO is track 1 here @@ -1598,6 +1791,7 @@ mod tests { // Video frame (track 0) and audio frame (track 1). sink.write(&PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1608,6 +1802,7 @@ mod tests { }) .unwrap(); sink.write(&PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 1, @@ -1642,6 +1837,7 @@ mod tests { }; let mut sink = DemuxSink::create(&dir, &title, &opts).unwrap(); sink.write(&PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 1, @@ -1695,6 +1891,7 @@ mod tests { // Frames far before the first IN mark: the plan places none of them. for (i, pts) in [0i64, 1_000_000_000].iter().enumerate() { let f = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1747,6 +1944,7 @@ mod tests { // Two frames before the first IN mark (dropped) … for (i, pts) in [0i64, 1_000_000_000].iter().enumerate() { let f = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1760,6 +1958,7 @@ mod tests { // … and one frame inside the first clip (150 s, in ns) that DOES persist, // so frames_mapped > 0 while seam_dropped (2) still exceeds it (1). let inside = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1828,6 +2027,7 @@ mod tests { // video track has no file — these drops are not this export's shortfall. for (i, pts) in [0i64, 1_000_000_000].iter().enumerate() { let f = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -1842,6 +2042,7 @@ mod tests { Stream::write( &mut sink, &PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 1, @@ -2016,6 +2217,7 @@ mod tests { }; let mut sink = DemuxSink::create(&dir, &title, &opts).unwrap(); sink.write(&PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, diff --git a/src/mux/disc.rs b/src/mux/disc.rs index 1ec3206d..e2d931f9 100644 --- a/src/mux/disc.rs +++ b/src/mux/disc.rs @@ -851,6 +851,7 @@ impl crate::pes::Stream for DiscStream { // pass raw PES bytes through as one PesFrame, so we // can attribute time to demux+framing vs codec parse. self.pending_frames.push_back(crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track, @@ -2175,6 +2176,7 @@ mod tests { fn write_refuses_with_the_read_only_code() { let mut s = short_read_stream(false); let frame = crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, diff --git a/src/mux/driver.rs b/src/mux/driver.rs index 82cc6efd..a2b5082b 100644 --- a/src/mux/driver.rs +++ b/src/mux/driver.rs @@ -658,7 +658,11 @@ fn drive_mux( let num_streams = info.streams.len(); // ── Open the sink, wrap in a byte counter, hand it to the write pipeline ── - let output_stream = CountingStream::new(output(dest_url, &out_title, source)?); + let mut output_stream = output(dest_url, &out_title, source)?; + for track in 0..num_streams { + output_stream.set_track_timing(track, stream.track_timing(track))?; + } + let output_stream = CountingStream::new(output_stream); events.on_output_opened(&out_title); // The write consumer runs on its own thread so the latency-bound sink write @@ -888,6 +892,7 @@ mod tests { fn with_frames(mut self, n: usize) -> Self { for i in 0..n { self.frames.push_back(PesFrame { + discard_padding_ns: 0, track: 0, pts: i as i64, keyframe: true, @@ -1958,6 +1963,7 @@ mod tests { fs.read_observer = Some(reads_seen.clone()); for i in 0..MANY { fs.frames.push_back(PesFrame { + discard_padding_ns: 0, track: 0, pts: i as i64, keyframe: true, diff --git a/src/mux/fvi_sink.rs b/src/mux/fvi_sink.rs index 5fd40330..c555bf91 100644 --- a/src/mux/fvi_sink.rs +++ b/src/mux/fvi_sink.rs @@ -293,6 +293,7 @@ mod tests { source: Option, ) -> PesFrame { PesFrame { + discard_padding_ns: 0, track, pts: 0, keyframe, diff --git a/src/mux/interop_tests.rs b/src/mux/interop_tests.rs new file mode 100644 index 00000000..3c0906f1 --- /dev/null +++ b/src/mux/interop_tests.rs @@ -0,0 +1,536 @@ +//! External interoperability gate. Pure Rust tests remain in normal dev CI; +//! QA explicitly runs ignored `ffmpeg_` tests with software codecs installed. +//! Fixtures are generated locally and never downloaded from a media corpus. + +use super::driver::{MuxInput, MuxOptions, NoopEvents, mux_stream}; +use crate::halt::Halt; +use serde_json::Value; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::sync::Arc; + +pub(crate) struct FixtureDir { + path: PathBuf, + _temporary: Option, +} + +impl FixtureDir { + pub(crate) fn path(&self) -> &Path { + &self.path + } +} + +// QA retains generated input/output media and probes as downloadable artifacts, +// including on failure. Local runs clean up unless the caller opts in. +pub(crate) fn fixture_dir(name: &str) -> FixtureDir { + if let Some(root) = std::env::var_os("FREEMKV_FFMPEG_ARTIFACT_DIR") { + let path = PathBuf::from(root).join(name); + std::fs::create_dir_all(&path).unwrap(); + FixtureDir { + path, + _temporary: None, + } + } else { + let temporary = tempfile::tempdir().unwrap(); + FixtureDir { + path: temporary.path().to_owned(), + _temporary: Some(temporary), + } + } +} + +fn run(dir: &Path, label: &str, command: &mut Command) -> Output { + let output = command + .output() + .expect("install ffmpeg and ffprobe to run the interoperability tests"); + std::fs::write( + dir.join(format!("{label}.command.txt")), + format!("{command:?}\n"), + ) + .unwrap(); + std::fs::write(dir.join(format!("{label}.stdout")), &output.stdout).unwrap(); + std::fs::write(dir.join(format!("{label}.stderr")), &output.stderr).unwrap(); + assert!( + output.status.success(), + "{command:?}: {}", + String::from_utf8_lossy(&output.stderr) + ); + output +} + +fn generate( + dir: &Path, + video: &str, + audio: Option<&str>, + transport: bool, + vfr: bool, + bframes: bool, + multi_audio: bool, +) -> PathBuf { + let path = dir.join(if transport { "input.m2ts" } else { "input.mkv" }); + let mut command = Command::new("ffmpeg"); + command.args([ + "-nostdin", + "-y", + "-v", + "error", + "-f", + "lavfi", + "-i", + "testsrc2=size=128x96:rate=25:duration=2", + ]); + if audio.is_some() { + command.args([ + "-f", + "lavfi", + "-i", + "sine=frequency=440:sample_rate=48000:duration=2", + ]); + } + if multi_audio { + command.args([ + "-f", + "lavfi", + "-i", + "sine=frequency=880:sample_rate=48000:duration=2", + ]); + } + command.args(["-map", "0:v:0"]); + if audio.is_some() { + command.args(["-map", "1:a:0"]); + } + if multi_audio { + command.args(["-map", "2:a:0"]); + } + // A deliberate positive audio offset tests A/V alignment without conflating + // encoder preroll clamped at zero with drift introduced by the remuxer. + command.args([ + "-c:v", + video, + "-threads:v", + "1", + "-g", + "12", + "-bf", + if bframes { "2" } else { "0" }, + ]); + if let Some(audio) = audio { + command.args([ + "-c:a", + audio, + "-b:a", + "192k", + "-threads:a", + "1", + "-af", + "asetpts=PTS+0.1/TB", + ]); + } + if video == "libx264" { + command.args(["-preset", "ultrafast"]); + } + if video == "libx265" { + command.args([ + "-preset", + "ultrafast", + "-x265-params", + "pools=none:frame-threads=1:log-level=error", + ]); + } + if vfr { + command.args([ + "-vf", + "select=lt(n\\,12)+not(mod(n\\,3))", + "-fps_mode", + "vfr", + ]); + } + if transport { + command.args(["-f", "mpegts", "-mpegts_m2ts_mode", "0"]); + } + run(dir, "generate", command.arg(&path)); + if transport { + let ts = std::fs::read(&path).unwrap(); + assert_eq!(ts.len() % 188, 0); + let mut m2ts = Vec::with_capacity(ts.len() / 188 * 192); + for packet in ts.as_chunks::<188>().0 { + m2ts.extend_from_slice(&[0; 4]); + m2ts.extend_from_slice(packet); + } + std::fs::write(&path, m2ts).unwrap(); + } + path +} + +fn remux(source: &Path, output: &Path, transport: bool) { + let scheme = if transport { "m2ts" } else { "mkv" }; + let url = format!("{scheme}://{}", source.display()); + let result = mux_stream( + MuxInput::Url { + url: &url, + opts: Default::default(), + }, + &format!("mkv://{}", output.display()), + &MuxOptions::default(), + &Halt::new(), + Arc::new(NoopEvents), + ) + .unwrap(); + assert!(result.completed && result.output_opened && result.bytes_written > 0); + assert_eq!(result.errors, 0); + assert_eq!(result.lost_bytes, 0); + assert!(result.undelivered_streams.is_empty()); +} + +fn probe(dir: &Path, label: &str, path: &Path) -> Value { + // Raw AAC/MP2 has no container duration; FFmpeg legitimately warns that it + // estimates it from bitrate. Inputs must be error-free; output MKVs must + // be warning-free. Compare decoded frames, never the estimated duration. + let verbosity = if label.starts_with("input") { + "error" + } else { + "warning" + }; + let output = run(dir, label, Command::new("ffprobe").args([ + "-v", verbosity, "-threads", "1", "-of", "json", "-show_streams", "-show_frames", + "-show_entries", "stream=index,codec_type,codec_name,width,height,sample_rate,channels:frame=stream_index,best_effort_timestamp_time,pts_time", + ]).arg(path)); + assert!( + output.stderr.is_empty(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + serde_json::from_slice(&output.stdout).unwrap() +} + +fn decoded_hash(dir: &Path, label: &str, path: &Path, stream: usize, video: bool) -> Vec { + let mut command = Command::new("ffmpeg"); + let verbosity = if label.starts_with("input") { + "error" + } else { + "warning" + }; + command + .args([ + "-nostdin", "-v", verbosity, "-xerror", "-threads", "1", "-i", + ]) + .arg(path) + .args(["-map", &format!("0:{stream}"), "-threads", "1"]); + if video { + command.args(["-c:v", "rawvideo", "-fps_mode", "passthrough"]); + } else { + command.args(["-c:a", "pcm_s16le"]); + } + command.args(["-f", "hash", "-hash", "sha256", "-"]); + let output = run(dir, label, &mut command); + assert!( + output.stderr.is_empty(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(output.stdout.starts_with(b"SHA256=")); + output.stdout +} + +fn timestamps(probe: &Value, stream: usize) -> Vec { + probe["frames"] + .as_array() + .unwrap() + .iter() + .filter(|frame| frame["stream_index"].as_u64() == Some(stream as u64)) + .map(|frame| { + frame["best_effort_timestamp_time"] + .as_str() + .or_else(|| frame["pts_time"].as_str()) + .expect("every decoded frame must have a timestamp") + .parse() + .unwrap() + }) + .collect() +} + +fn validate(dir: &Path, source: &Path, output: &Path, expected_streams: usize, vfr: bool) { + let before = probe(dir, "input-probe", source); + let after = probe(dir, "output-probe", output); + let streams = before["streams"].as_array().unwrap(); + assert_eq!(streams.len(), expected_streams); + assert_eq!(after["streams"].as_array().unwrap().len(), expected_streams); + // Remuxing may rebase all tracks by one constant. It may not drift tracks + // relative to each other or replace a VFR timeline with a guessed cadence. + let shift = timestamps(&after, 0)[0] - timestamps(&before, 0)[0]; + for (index, stream) in streams.iter().enumerate() { + for field in [ + "codec_type", + "codec_name", + "width", + "height", + "sample_rate", + "channels", + ] { + assert_eq!( + stream[field], after["streams"][index][field], + "stream {index} {field}" + ); + } + let input_pts = timestamps(&before, index); + let output_pts = timestamps(&after, index); + assert!( + input_pts.len() >= 10, + "fixture must exercise multiple frames" + ); + assert_eq!( + input_pts.len(), + output_pts.len(), + "frame loss on stream {index}" + ); + for (input, output) in input_pts.iter().zip(&output_pts) { + assert!( + (output - input - shift).abs() < 0.002, + "stream {index}: input {input}, output {output}, common shift {shift}" + ); + } + assert!( + output_pts.windows(2).all(|p| p[1] > p[0]), + "decoded stream {index} must advance" + ); + let video = stream["codec_type"] == "video"; + if vfr && video { + let gaps: Vec<_> = input_pts + .windows(2) + .map(|p| ((p[1] - p[0]) * 1000.0).round() as i64) + .collect(); + assert!( + gaps.iter().any(|g| *g != gaps[0]), + "fixture must actually be VFR" + ); + } + assert_eq!( + decoded_hash(dir, &format!("input-hash-{index}"), source, index, video), + decoded_hash(dir, &format!("output-hash-{index}"), output, index, video), + "decoded content changed on stream {index}" + ); + } +} + +#[test] +#[ignore = "requires FFmpeg software codecs; QA interoperability workflow"] +fn ffmpeg_h264_ac3_transport_to_mkv_preserves_decoded_content_and_timing() { + let dir = fixture_dir("h264-ac3-transport"); + let input = generate(dir.path(), "libx264", Some("ac3"), true, false, true, false); + let output = dir.path().join("output.mkv"); + remux(&input, &output, true); + validate(dir.path(), &input, &output, 2, false); +} + +#[test] +#[ignore = "requires FFmpeg software codecs; QA interoperability workflow"] +fn ffmpeg_hevc_eac3_transport_to_mkv_preserves_decoded_content_and_timing() { + let dir = fixture_dir("hevc-eac3-transport"); + let input = generate( + dir.path(), + "libx265", + Some("eac3"), + true, + false, + true, + false, + ); + let output = dir.path().join("output.mkv"); + remux(&input, &output, true); + validate(dir.path(), &input, &output, 2, false); +} + +#[test] +#[ignore = "requires FFmpeg software codecs; QA interoperability workflow"] +fn ffmpeg_mpeg2_transport_to_mkv_preserves_decoded_video_and_timing() { + let dir = fixture_dir("mpeg2-video-transport"); + let input = generate(dir.path(), "mpeg2video", None, true, false, true, false); + let output = dir.path().join("output.mkv"); + remux(&input, &output, true); + validate(dir.path(), &input, &output, 1, false); +} + +#[test] +#[ignore = "requires FFmpeg software codecs; QA interoperability workflow"] +fn ffmpeg_ac3_parser_preserves_audio_packet_boundaries() { + validate_audio_parser("ac3", "ac3", crate::disc::Codec::Ac3); +} + +#[test] +#[ignore = "requires FFmpeg software codecs; QA interoperability workflow"] +fn ffmpeg_eac3_parser_preserves_audio_packet_boundaries() { + validate_audio_parser("eac3", "eac3", crate::disc::Codec::Ac3Plus); +} + +fn validate_audio_parser(encoder: &str, format: &str, codec: crate::disc::Codec) { + use super::codec::{CodecParser, ac3::Ac3Parser, adts::AdtsParser, mpegaudio::MpegAudioParser}; + use super::mkv::{MkvMuxer, MkvTrack}; + use super::ts::PesPacket; + use crate::disc::{AudioChannels, AudioStream, LabelPurpose, SampleRate}; + let dir = fixture_dir(&format!("{encoder}-parser")); + let input = dir.path().join(format!("input.{format}")); + run( + dir.path(), + "generate", + Command::new("ffmpeg") + .args([ + "-nostdin", + "-y", + "-v", + "error", + "-f", + "lavfi", + "-i", + "sine=frequency=440:sample_rate=48000:duration=2", + "-c:a", + encoder, + "-b:a", + "192k", + "-threads", + "1", + "-f", + format, + ]) + .arg(&input), + ); + let mut parser: Box = match codec { + crate::disc::Codec::Aac => Box::new(AdtsParser::new()), + crate::disc::Codec::Mp2 => Box::new(MpegAudioParser::new()), + _ => Box::new(Ac3Parser::new()), + }; + let mut frames = Vec::new(); + // Deliberately split headers and bodies across PES boundaries. Only the + // first chunk has PTS; subsequent frame times must come from codec cadence. + for (index, data) in std::fs::read(&input).unwrap().chunks(137).enumerate() { + frames.extend(parser.parse(&PesPacket { + pid: 0x1100, + pts: (index == 0).then_some(0), + dts: None, + data: data.to_vec(), + source: None, + discontinuity: false, + })); + } + frames.extend(parser.flush()); + let mut track = MkvTrack::audio(&AudioStream { + pid: 0x1100, + codec, + channels: AudioChannels::Mono, + language: "eng".into(), + sample_rate: SampleRate::S48, + secondary: false, + purpose: LabelPurpose::Normal, + label: String::new(), + }); + track.codec_private = parser.codec_private(); + let output = dir.path().join("output.mkv"); + let mut muxer = MkvMuxer::new( + std::fs::File::create(&output).unwrap(), + &[track], + None, + 0.0, + &[], + ) + .unwrap(); + for frame in frames { + muxer + .write_frame( + 0, + frame.pts_ns, + frame.keyframe, + &frame.data, + frame.duration_ns, + None, + ) + .unwrap(); + } + muxer.finish().unwrap(); + validate(dir.path(), &input, &output, 1, false); +} + +#[test] +#[ignore = "requires FFmpeg software codecs; QA interoperability workflow"] +fn ffmpeg_mkv_roundtrip_preserves_vfr_and_multiple_audio_tracks() { + let dir = fixture_dir("vfr-multiple-audio"); + // Stream-copy transport packets to create a disc-shaped MKV reference. + // Direct encoding into MKV adds CodecDelay/SkipSamples, a separate + // unsupported preservation path documented in the QA guide. + let transport = generate(dir.path(), "libx264", Some("ac3"), true, true, true, true); + let input = dir.path().join("input.mkv"); + run( + dir.path(), + "reference-remux", + Command::new("ffmpeg") + .args(["-nostdin", "-y", "-v", "error", "-i"]) + .arg(&transport) + .args(["-map", "0", "-c", "copy"]) + .arg(&input), + ); + let output = dir.path().join("output.mkv"); + remux(&input, &output, false); + validate(dir.path(), &input, &output, 3, true); +} + +// Discovered while building the gate; retain executable, strict reproducers. +// These deliberately do NOT have the ffmpeg_ prefix: run ffmpeg_ explicitly +// to investigate them. Promote each into the QA gate when its fix lands. + +#[test] +#[ignore = "requires ffmpeg and ffprobe"] +fn ffmpeg_aac_frame_assembly() { + validate_audio_parser("aac", "adts", crate::disc::Codec::Aac); +} + +#[test] +#[ignore = "requires ffmpeg and ffprobe"] +fn ffmpeg_mp2_frame_assembly() { + validate_audio_parser("mp2", "mp2", crate::disc::Codec::Mp2); +} + +#[test] +#[ignore = "requires ffmpeg and ffprobe"] +fn ffmpeg_mpeg2_transport_opening_audio() { + let dir = fixture_dir("mpeg2-audio"); + let input = generate( + dir.path(), + "mpeg2video", + Some("ac3"), + true, + false, + true, + false, + ); + let output = dir.path().join("output.mkv"); + remux(&input, &output, true); + validate(dir.path(), &input, &output, 2, false); +} + +#[test] +#[ignore = "requires ffmpeg and ffprobe"] +fn ffmpeg_mkv_encoder_delay() { + let dir = fixture_dir("codec-delay"); + let input = generate(dir.path(), "libx264", Some("ac3"), false, true, true, true); + let output = dir.path().join("output.mkv"); + remux(&input, &output, false); + validate(dir.path(), &input, &output, 3, true); +} + +#[test] +#[ignore = "requires ffmpeg and ffprobe"] +fn ffmpeg_generic_transport_audio_discovery() { + for audio in ["aac", "mp2"] { + let dir = fixture_dir(&format!("transport-{audio}")); + let input = generate( + dir.path(), + "libx264", + Some(audio), + true, + false, + false, + false, + ); + let output = dir.path().join("output.mkv"); + remux(&input, &output, true); + validate(dir.path(), &input, &output, 2, false); + } +} diff --git a/src/mux/m2ts.rs b/src/mux/m2ts.rs index 8f76fd0c..2afe0dad 100644 --- a/src/mux/m2ts.rs +++ b/src/mux/m2ts.rs @@ -198,6 +198,7 @@ mod tests { let mut stream = M2tsStream::create(sink, &title).unwrap(); stream .write(&PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -241,6 +242,7 @@ mod tests { let mut stream = M2tsStream::create(sink, &title).unwrap(); stream .write(&PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -267,6 +269,7 @@ mod tests { let sink = SharedSink(shared.clone()); let mut stream = M2tsStream::create(sink, &title).unwrap(); let frame = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, diff --git a/src/mux/mkv.rs b/src/mux/mkv.rs index 67b17619..bf1e056e 100644 --- a/src/mux/mkv.rs +++ b/src/mux/mkv.rs @@ -770,9 +770,21 @@ fn track_vint(track_num: usize) -> ([u8; 3], usize) { impl MkvMuxer { /// Create a new MKV muxer: writes EBML header, Segment start, Info, Tracks, Chapters. + #[cfg(test)] pub fn new( + writer: W, + tracks: &[MkvTrack], + title: Option<&str>, + duration_secs: f64, + chapters: &[Chapter], + ) -> io::Result { + Self::new_with_timing(writer, tracks, &[], title, duration_secs, chapters) + } + + pub(crate) fn new_with_timing( mut writer: W, tracks: &[MkvTrack], + timings: &[crate::pes::TrackTiming], title: Option<&str>, duration_secs: f64, chapters: &[Chapter], @@ -888,6 +900,15 @@ impl MkvMuxer { ebml::write_uint(&mut writer, ebml::TRACK_NUMBER, (i + 1) as u64)?; ebml::write_uint(&mut writer, ebml::TRACK_UID, track_uid)?; ebml::write_uint(&mut writer, ebml::TRACK_TYPE, track.track_type)?; + if let Some(timing) = timings.get(i) { + if timing.codec_delay_ns > 0 { + ebml::write_uint(&mut writer, 0x56AA, timing.codec_delay_ns)?; + } + if timing.seek_preroll_ns > 0 { + ebml::write_uint(&mut writer, 0x56BB, timing.seek_preroll_ns)?; + } + } + ebml::write_uint(&mut writer, ebml::FLAG_LACING, 0)?; ebml::write_string(&mut writer, ebml::CODEC_ID, track.codec_id)?; ebml::write_string(&mut writer, ebml::LANGUAGE, &track.language)?; @@ -1197,7 +1218,7 @@ impl MkvMuxer { /// removed at the right time); otherwise a plain `SimpleBlock`. /// /// TEST-ONLY wrapper over [`MkvMuxer::write_frame_at`] — production - /// always calls `write_frame_at` directly. + /// calls `write_frame_at_with_padding` to preserve container trimming. #[cfg(test)] pub fn write_frame( &mut self, @@ -1230,6 +1251,7 @@ impl MkvMuxer { /// keyframe. `None` for non-3D. See docs/mkv-mux.md — write_frame_at /// seam/MVC BlockAdditional rationale. #[allow(clippy::too_many_arguments)] + #[cfg(test)] pub fn write_frame_at( &mut self, track_idx: usize, @@ -1240,6 +1262,32 @@ impl MkvMuxer { block_additional: Option<&[u8]>, src_byte: Option, scan_progressive: Option, + ) -> io::Result<()> { + self.write_frame_at_with_padding( + track_idx, + pts_ns, + keyframe, + data, + duration_ns, + block_additional, + src_byte, + scan_progressive, + 0, + ) + } + + #[allow(clippy::too_many_arguments)] + pub(crate) fn write_frame_at_with_padding( + &mut self, + track_idx: usize, + pts_ns: i64, + keyframe: bool, + data: &[u8], + duration_ns: Option, + block_additional: Option<&[u8]>, + src_byte: Option, + scan_progressive: Option, + discard_padding_ns: i64, ) -> io::Result<()> { // --log-level 3: capture the first ~100 coded frames per track to the side file // BEFORE any timeline mangling, with the parser's own PTS, so an opening-GOP @@ -1413,30 +1461,48 @@ impl MkvMuxer { .unwrap_or(0), ) }; - match block_additional { - // MVC: base view Block + dependent-view BlockAdditional, always a BlockGroup; - // non-keyframe base frames get a ReferenceBlock so a player never treats - // a P/B frame as a seek point. - Some(additional) => { - self.write_block_group_mvc( - track_idx + 1, - relative_ts, - data, - additional, - reference, - duration_ticks, - )?; + if discard_padding_ns != 0 { + let mut buf = std::mem::take(&mut self.block_group_buf); + buf.clear(); + let result = Self::build_block_group( + &mut buf, + track_idx + 1, + relative_ts, + data, + reference, + duration_ticks, + block_additional, + discard_padding_ns, + ) + .and_then(|()| self.writer.write_all(&buf)); + self.block_group_buf = buf; + result?; + } else { + match block_additional { + // MVC: base view Block + dependent-view BlockAdditional, always a BlockGroup; + // non-keyframe base frames get a ReferenceBlock so a player never treats + // a P/B frame as a seek point. + Some(additional) => { + self.write_block_group_mvc( + track_idx + 1, + relative_ts, + data, + additional, + reference, + duration_ticks, + )?; + } + None => match duration_ticks { + // BlockDuration present (PGS subtitles, AC-3 audio, and EVERY + // MPEG-2 video frame) → BlockGroup. + Some(dt) => { + self.write_block_group(track_idx + 1, relative_ts, data, reference, dt)?; + } + None => { + self.write_simple_block(track_idx + 1, relative_ts, keyframe, data)?; + } + }, } - None => match duration_ticks { - // BlockDuration present (PGS subtitles, AC-3 audio, and EVERY - // MPEG-2 video frame) → BlockGroup. - Some(dt) => { - self.write_block_group(track_idx + 1, relative_ts, data, reference, dt)?; - } - None => { - self.write_simple_block(track_idx + 1, relative_ts, keyframe, data)?; - } - }, } // Recorded per track (not a single global slot) so a later non-keyframe // references a keyframe on its OWN track — a shared slot produced cross-track @@ -1852,6 +1918,7 @@ impl MkvMuxer { reference, Some(duration_ticks), None, + 0, ) .and_then(|()| self.writer.write_all(&buf)); // Hand the (now grown) scratch buffer back so the next frame reuses the @@ -1863,6 +1930,7 @@ impl MkvMuxer { // Assemble a complete BlockGroup into `buf` (one write_all at the call // site, no seek). `additional` = Some(dependent AU) for MVC, appending // BlockAdditions > BlockMore { BlockAddID=2, BlockAdditional }. + #[allow(clippy::too_many_arguments)] fn build_block_group( buf: &mut Vec, track_num: usize, @@ -1871,6 +1939,7 @@ impl MkvMuxer { reference: Option, duration_ticks: Option, additional: Option<&[u8]>, + discard_padding_ns: i64, ) -> io::Result<()> { let (tv, tv_len) = track_vint(track_num); let track_vint = &tv[..tv_len]; @@ -1887,6 +1956,9 @@ impl MkvMuxer { buf.extend_from_slice(&relative_ts.to_be_bytes()); buf.push(flags); buf.extend_from_slice(data); + if discard_padding_ns != 0 { + ebml::write_int(buf, 0x75A2, discard_padding_ns)?; + } if let Some(dt) = duration_ticks { ebml::write_uint(buf, ebml::BLOCK_DURATION, dt)?; } @@ -1927,6 +1999,7 @@ impl MkvMuxer { reference, duration_ticks, Some(additional), + 0, ) .and_then(|()| self.writer.write_all(&buf)); self.block_group_buf = buf; @@ -2116,6 +2189,7 @@ mod tests { // 5 audio frames arrive first (realistic pre-video buffering). for i in 0..5u32 { let f = crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 1, @@ -2129,6 +2203,7 @@ mod tests { // Then the true video keyframe — duration-bearing, as MPEG-2 always is, // so it is written as a BlockGroup (where the keyframe bit is reserved). let vf = crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -2149,6 +2224,18 @@ mod tests { f0.keyframe, "video keyframe written with 5 audio frames ahead of it must read back keyframe=true" ); + for i in 0..5 { + let audio = crate::pes::Stream::read(&mut r) + .unwrap() + .expect("buffered audio must survive"); + assert_eq!(audio.track, 1); + assert_eq!(audio.pts, i * 32_000_000); + assert_eq!(audio.data, vec![0xCC; 768]); + } + assert!( + crate::pes::Stream::read(&mut r).unwrap().is_none(), + "trigger frame must not be duplicated" + ); std::fs::remove_dir_all(&dir).ok(); } diff --git a/src/mux/mkvstream.rs b/src/mux/mkvstream.rs index 6fb70aa4..83fc52cb 100644 --- a/src/mux/mkvstream.rs +++ b/src/mux/mkvstream.rs @@ -134,6 +134,7 @@ enum WriteMode { struct PendingMux { writer: Box, tracks: Vec, + timings: Vec, /// Index of the primary (first) video track, if any — the track whose /// `FieldOrder` is set from the first coded picture's measured coding. video_track: Option, @@ -294,7 +295,7 @@ fn emit_to_muxer( frame: &crate::pes::PesFrame, additional: Option<&[u8]>, ) -> io::Result<()> { - m.write_frame_at( + m.write_frame_at_with_padding( frame.track, frame.pts, frame.keyframe, @@ -305,6 +306,7 @@ fn emit_to_muxer( frame.source.map(|s| s.byte), // This picture's measured scan type, tallied for the FlagInterlaced majority. frame.coding.as_ref().and_then(|c| c.progressive()), + frame.discard_padding_ns, ) } @@ -434,6 +436,7 @@ impl MkvStream { disc_title: title.clone(), mvc, mode: Mode::Write(WriteMode::Pending(Box::new(PendingMux { + timings: vec![crate::pes::TrackTiming::default(); tracks.len()], writer, tracks, video_track, @@ -484,9 +487,10 @@ impl MkvStream { for (i, track) in pending.tracks.iter().enumerate() { crate::diag::dump_mkv_track((i + 1) as u64, track); } - let mut muxer = MkvMuxer::new( + let mut muxer = MkvMuxer::new_with_timing( pending.writer, &pending.tracks, + &pending.timings, Some(&self.disc_title.playlist), self.disc_title.duration_secs, &self.disc_title.chapters, @@ -500,7 +504,7 @@ impl MkvStream { // Provenance must survive the replay: these pre-muxer frames still carry // the byte offset they were read from — dropping it here would fall back // to the timestamp heuristic this change set exists to stop relying on. - muxer.write_frame_at( + muxer.write_frame_at_with_padding( f.track, f.pts, f.keyframe, @@ -509,6 +513,7 @@ impl MkvStream { additional.as_deref(), f.source.map(|s| s.byte), f.coding.as_ref().and_then(|c| c.progressive()), + f.discard_padding_ns, )?; } self.mode = Mode::Write(WriteMode::Active(Box::new(muxer))); @@ -544,6 +549,16 @@ impl MkvStream { _ => unreachable!("guarded above"), }; if activate_now { + // The first video keyframe must open a cluster BEFORE replaying + // audio already buffered while its parser assembled the opening GOP. + // Replaying the audio first made the muxer drop that entire prefix. + if use_coding && frame.keyframe { + if let Mode::Write(WriteMode::Pending(p)) = &mut self.mode { + p.buffered + .insert(0, (frame.clone(), additional.map(<[u8]>::to_vec))); + } + return self.activate(frame.coding, true); + } // Pass the trigger frame's coding only when it IS the video frame; a // cap-triggered build never saw the video frame, so nothing measured // is passed (apply_coding_to_track then logs + leaves UNDETERMINED). @@ -707,6 +722,7 @@ impl crate::pes::Stream for MkvStream { // presence — SimpleBlock's 0x80 bit is reserved (always 0) here, // so reading it broke every MPEG-2 frame (always this path). let mut has_reference = false; + let mut discard_padding_ns = 0i64; while remaining > 0 { let (cid, cs, hlen) = ebml::read_element_header(&mut rs.reader)?; if cs == u64::MAX { @@ -726,6 +742,14 @@ impl crate::pes::Stream for MkvStream { checked_size(cs, MAX_BLOCK_SIZE)?, )?); } + 0x75A2 => { + if cs == 0 || cs > 8 { + return Err(crate::error::Error::MkvSourceInvalid.into()); + } + let raw = read_uint_bounded(&mut rs.reader, cs)?; + let shift = 64 - cs * 8; + discard_padding_ns = ((raw << shift) as i64) >> shift; + } ebml::BLOCK_DURATION => { duration_ms = Some(read_uint_bounded(&mut rs.reader, cs)?); } @@ -774,10 +798,17 @@ impl crate::pes::Stream for MkvStream { // Override the flag-bit guess from `parse_block` // (meaningful for SimpleBlock only) with the // BlockGroup's authoritative signal. - rs.pending.extend(frames.into_iter().map(|mut f| { - f.keyframe = !has_reference; - f - })); + let last = frames.len().saturating_sub(1); + rs.pending + .extend(frames.into_iter().enumerate().map(|(i, mut f)| { + if (discard_padding_ns > 0 && i == last) + || (discard_padding_ns < 0 && i == 0) + { + f.discard_padding_ns = discard_padding_ns; + } + f.keyframe = !has_reference; + f + })); if let Some(frame) = rs.pending.pop_front() { return Ok(Some(frame)); } @@ -852,6 +883,38 @@ impl crate::pes::Stream for MkvStream { &self.disc_title } + fn track_timing(&self, track: usize) -> crate::pes::TrackTiming { + match &self.mode { + Mode::Read(rs) => rs.tracks.timings.get(track).copied().unwrap_or_default(), + _ => Default::default(), + } + } + + fn set_track_timing( + &mut self, + track: usize, + timing: crate::pes::TrackTiming, + ) -> io::Result<()> { + if let Mode::Write(WriteMode::Pending(p)) = &mut self.mode { + let mapped = if let Some(m) = &self.mvc { + let Some(mapped) = m.stream_to_track.get(track).copied().flatten() else { + return Ok(()); + }; + mapped + } else { + track + }; + if let Some(t) = p.timings.get_mut(mapped) { + *t = timing; + } + Ok(()) + } else { + Err(io::Error::other( + "track timing must be set before writing frames", + )) + } + } + fn codec_private(&self, track: usize) -> Option> { if let Mode::Read(ref rs) = self.mode { // `track` is a stream index but `codec_privates` is keyed by Matroska @@ -985,13 +1048,14 @@ fn parse_mkv_header(r: &mut impl Read) -> MkvHeaderResult { } remaining -= consumed; if cid == ebml::TRACK_ENTRY { - let (stream, tnum, cp, default_dur) = parse_track(r, cs)?; + let (stream, tnum, cp, default_dur, timing) = parse_track(r, cs)?; if let Some(s) = stream { // Record the TrackNumber alongside the stream it maps // to, in the SAME order, so block routing never has to // guess that TrackNumbers are 1..=N. streams.push(s); tracks.push(tnum, default_dur); + *tracks.timings.last_mut().unwrap() = timing; } if let Some(cp) = cp { codec_privates.push((tnum, cp)); @@ -1057,6 +1121,7 @@ type ParsedTrack = ( u16, Option>, Option, + crate::pes::TrackTiming, ); /// Returns (stream, track_number, codec_private_bytes, default_duration_ns) @@ -1067,6 +1132,7 @@ fn parse_track(r: &mut impl Read, size: u64) -> io::Result { /// would only skew laced-frame spacing, so treat it as absent. const MAX_DEFAULT_DURATION_NS: u64 = 60 * 1_000_000_000; let mut default_dur: Option = None; + let mut timing = crate::pes::TrackTiming::default(); let (mut codec_id, mut lang, mut name) = (String::new(), String::from("und"), String::new()); let (mut ph, mut sr, mut ch, mut forced) = (0u32, 0.0f64, 0u8, false); let mut codec_priv: Option> = None; @@ -1100,6 +1166,8 @@ fn parse_track(r: &mut impl Read, size: u64) -> io::Result { let ns = read_uint_bounded(r, cs)?; default_dur = (ns > 0 && ns <= MAX_DEFAULT_DURATION_NS).then_some(ns); } + 0x56AA => timing.codec_delay_ns = read_uint_bounded(r, cs)?, + 0x56BB => timing.seek_preroll_ns = read_uint_bounded(r, cs)?, ebml::CODEC_ID => codec_id = read_string_bounded(r, cs)?, ebml::CODEC_PRIVATE => { codec_priv = Some(ebml::read_binary_val( @@ -1255,7 +1323,7 @@ fn parse_track(r: &mut impl Read, size: u64) -> io::Result { })), _ => None, }; - Ok((stream, tnum, codec_priv, default_dur)) + Ok((stream, tnum, codec_priv, default_dur, timing)) } // Read-side map from Matroska TrackNumber to the index of the corresponding @@ -1270,11 +1338,13 @@ struct TrackTable { /// Used to space the frames of a LACED Block, whose second and later frames /// carry an "underdetermined" timestamp per RFC 9559 §10.3.5. default_durations: Vec>, + timings: Vec, } impl TrackTable { fn push(&mut self, num: u16, default_duration_ns: Option) { self.nums.push(num); + self.timings.push(Default::default()); self.default_durations.push(default_duration_ns); } @@ -1300,6 +1370,7 @@ impl TrackTable { Self { nums: (1..=n as u16).collect(), default_durations: vec![None; n], + timings: vec![Default::default(); n], } } } @@ -1469,6 +1540,7 @@ fn parse_block( let lacing = (flags & LACING_MASK) >> 1; if lacing == LACING_NONE { return Ok(vec![crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: track_idx, @@ -1523,6 +1595,7 @@ fn parse_block( .saturating_mul(i as u64) .min(i64::MAX as u64) as i64; out.push(crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: track_idx, @@ -1604,6 +1677,7 @@ mod tests { fn mvc_frame(track: usize, pts: i64, keyframe: bool, data: Vec) -> crate::pes::PesFrame { crate::pes::PesFrame { + discard_padding_ns: 0, track, pts, keyframe, @@ -2501,6 +2575,7 @@ mod tests { TrackTable { nums: vec![65535], default_durations: vec![None], + timings: vec![Default::default()], } .index_of(65535), Some(0), @@ -3605,6 +3680,7 @@ mod tests { ]; for (pts, keyframe, data) in &frames { s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -3846,6 +3922,7 @@ mod tests { let title = three_track_title(); let mut s = MkvStream::create(Box::new(out.clone()), &title, None).unwrap(); s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -3910,6 +3987,7 @@ mod tests { let out2 = SharedOut::new(); let mut s = MkvStream::create(Box::new(out2.clone()), &relaxed, None).unwrap(); s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -4093,6 +4171,7 @@ mod tests { let out = SharedOut::new(); let mut s = MkvStream::create(Box::new(out.clone()), &title, None).unwrap(); s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: Some(PictureInfo::mpeg2( CodingType::I, Mpeg2Coding { @@ -4171,6 +4250,7 @@ mod tests { let mut s = MkvStream::create(Box::new(out.clone()), &title, None).unwrap(); // First picture: a lone progressive leader. s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: pic(true, CodingType::I), source: None, track: 0, @@ -4183,6 +4263,7 @@ mod tests { // The feature itself: genuinely interlaced pictures dominate. for i in 1..6 { s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: pic(false, CodingType::P), source: None, track: 0, @@ -4242,6 +4323,7 @@ mod tests { let mut s = MkvStream::create(Box::new(out.clone()), &title, None).unwrap(); // First picture: interlaced (TFF) → provisional interlaced + FieldOrder written. s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: pic(false, CodingType::I), source: None, track: 0, @@ -4254,6 +4336,7 @@ mod tests { // The feature itself: progressive pictures dominate. for i in 1..6 { s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: pic(true, CodingType::P), source: None, track: 0, @@ -4321,6 +4404,7 @@ mod tests { let scans = [true, true, false, false]; for (i, prog) in scans.iter().enumerate() { s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: pic(*prog, if i == 0 { CodingType::I } else { CodingType::P }), source: None, track: 0, @@ -4381,6 +4465,7 @@ mod tests { // Audio arrives first and carries NO coding — it must be buffered, not // used to build the header. s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -4392,6 +4477,7 @@ mod tests { .unwrap(); // The first coded picture on the VIDEO track measures top-field-first. s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: Some(PictureInfo::mpeg2( CodingType::I, Mpeg2Coding { @@ -4806,6 +4892,7 @@ mod tests { let title = h264_title(); let mut s = MkvStream::create(Box::new(out.clone()), &title, None).unwrap(); s.write(&crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -4828,4 +4915,73 @@ mod tests { before any frame has been read" ); } + #[test] + fn container_timing_and_signed_padding_survive_roundtrip() { + for padding in [-10_666_667, 0, 10_666_667] { + let out = SharedOut::new(); + let title = h264_title(); + let timing = crate::pes::TrackTiming { + codec_delay_ns: 5_333_333, + seek_preroll_ns: 80_000_000, + }; + let mut writer = MkvStream::create(Box::new(out.clone()), &title, None).unwrap(); + writer.set_track_timing(0, timing).unwrap(); + let mut frame = mvc_frame(0, 0, true, vec![1, 2, 3]); + frame.discard_padding_ns = padding; + writer.write(&frame).unwrap(); + assert!(writer.set_track_timing(0, timing).is_err()); + writer.finish().unwrap(); + let mut reader = MkvStream::open(Cursor::new(out.bytes())).unwrap(); + assert_eq!(reader.track_timing(0), timing); + assert_eq!(reader.track_timing(1), Default::default()); + let back = reader.read().unwrap().unwrap(); + assert_eq!(back.discard_padding_ns, padding); + assert_eq!(back.data, frame.data); + assert!(reader.read().unwrap().is_none()); + } + } + + #[test] + fn laced_padding_applies_only_to_the_appropriate_edge_frame() { + for padding in [-1_000_000i64, 1_000_000] { + let mut cluster = Vec::new(); + ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); + ebml::write_unknown_size(&mut cluster).unwrap(); + ebml::write_uint(&mut cluster, ebml::CLUSTER_TIMESTAMP, 0).unwrap(); + let mut group = Vec::new(); + // Fixed lace: two one-byte frames. + ebml::write_binary(&mut group, ebml::BLOCK, &[0x81, 0, 0, 4, 1, 0xaa, 0xbb]).unwrap(); + ebml::write_int(&mut group, 0x75A2, padding).unwrap(); + ebml::write_binary(&mut cluster, ebml::BLOCK_GROUP, &group).unwrap(); + let bytes = mkv_with_tracks_and_cluster( + &[TrackSpec::new(1, 2).with_default_duration(32_000_000)], + &cluster, + ); + let frames = drain(&mut MkvStream::open(Cursor::new(bytes)).unwrap()); + assert_eq!(frames.len(), 2); + assert_eq!( + frames[0].discard_padding_ns, + if padding < 0 { padding } else { 0 } + ); + assert_eq!( + frames[1].discard_padding_ns, + if padding > 0 { padding } else { 0 } + ); + } + } + + #[test] + fn malformed_discard_padding_size_is_rejected() { + for size in [0, 9] { + let mut cluster = Vec::new(); + ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); + ebml::write_unknown_size(&mut cluster).unwrap(); + let mut group = Vec::new(); + ebml::write_binary(&mut group, ebml::BLOCK, &[0x81, 0, 0, 0, 0xaa]).unwrap(); + ebml::write_binary(&mut group, 0x75A2, &vec![0; size]).unwrap(); + ebml::write_binary(&mut cluster, ebml::BLOCK_GROUP, &group).unwrap(); + let bytes = mkv_with_tracks_and_cluster(&[TrackSpec::new(1, 2)], &cluster); + assert!(MkvStream::open(Cursor::new(bytes)).unwrap().read().is_err()); + } + } } diff --git a/src/mux/mod.rs b/src/mux/mod.rs index 0b62b44c..92c3fd0a 100644 --- a/src/mux/mod.rs +++ b/src/mux/mod.rs @@ -110,6 +110,9 @@ use std::io::{Seek, Write}; pub trait WriteSeek: Write + Seek {} impl WriteSeek for T {} +#[cfg(test)] +pub(crate) mod interop_tests; + #[cfg(test)] mod tests { use super::resolve::{StreamUrl, parse_url}; diff --git a/src/mux/mp4/mod.rs b/src/mux/mp4/mod.rs index 9c25e8ec..d73435b8 100644 --- a/src/mux/mp4/mod.rs +++ b/src/mux/mp4/mod.rs @@ -1143,6 +1143,7 @@ mod tests { fn frame(track: usize, pts_ns: i64, key: bool, data: Vec) -> PesFrame { PesFrame { + discard_padding_ns: 0, track, pts: pts_ns, keyframe: key, diff --git a/src/mux/mp4/read.rs b/src/mux/mp4/read.rs index 6a33d40e..323f0748 100644 --- a/src/mux/mp4/read.rs +++ b/src/mux/mp4/read.rs @@ -344,6 +344,7 @@ impl Stream for Mp4Reader { let mut data = vec![0u8; s.size as usize]; self.file.read_exact(&mut data)?; Ok(Some(PesFrame { + discard_padding_ns: 0, track: s.track, pts: s.pts_ns, keyframe: s.keyframe, @@ -1183,6 +1184,7 @@ mod tests { { let mut sink = Mp4Sink::create(Cursor::new(&mut buf), &t).unwrap(); sink.write(&PesFrame { + discard_padding_ns: 0, track: 0, pts: 0, keyframe: true, @@ -1291,6 +1293,7 @@ mod tests { cursor: 0, }; let frame = PesFrame { + discard_padding_ns: 0, track: 0, pts: 0, keyframe: true, @@ -1367,6 +1370,7 @@ mod tests { let mut buf = Vec::new(); { let mk = |track, pts, key, data: Vec| PesFrame { + discard_padding_ns: 0, track, pts, keyframe: key, @@ -1517,6 +1521,7 @@ mod tests { let mut sink = Mp4Sink::create(Cursor::new(&mut buf), &t).unwrap(); for (i, &(pts, fill)) in plan.iter().enumerate() { sink.write(&PesFrame { + discard_padding_ns: 0, track: 0, pts, keyframe: i == 0, @@ -1597,6 +1602,7 @@ mod tests { let mut sink = Mp4Sink::create(Cursor::new(&mut buf), &t).unwrap(); for i in 0..FRAMES { sink.write(&PesFrame { + discard_padding_ns: 0, track: 0, pts: i as i64 * FRAME_NS, keyframe: i == 0, @@ -1706,6 +1712,7 @@ mod tests { let mut sink = Mp4Sink::create(Cursor::new(&mut buf), &t).unwrap(); for i in 0..4i64 { sink.write(&PesFrame { + discard_padding_ns: 0, track: 0, pts: i * 40_000_000, keyframe: i == 0, @@ -1716,6 +1723,7 @@ mod tests { }) .unwrap(); sink.write(&PesFrame { + discard_padding_ns: 0, track: 1, pts: i * 32_000_000, keyframe: true, diff --git a/src/mux/network.rs b/src/mux/network.rs index cbc9b343..ba735524 100644 --- a/src/mux/network.rs +++ b/src/mux/network.rs @@ -486,6 +486,7 @@ mod tests { .unwrap() .meta(&dt); let frame = pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -598,6 +599,7 @@ mod tests { let h = std::thread::spawn(move || { let mut ns = NetworkStream::accept_from(listener).unwrap(); let frame = pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -651,6 +653,7 @@ mod tests { .meta(&dt); for i in 0..5u8 { let frame = pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: (i % 2) as usize, diff --git a/src/mux/null.rs b/src/mux/null.rs index 1a2d1d1c..58b6e8f3 100644 --- a/src/mux/null.rs +++ b/src/mux/null.rs @@ -46,6 +46,7 @@ mod tests { let mut sink: Box = Box::new(NullStream::new(&title)); let frame = crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -79,6 +80,7 @@ mod tests { sink.finish().unwrap(); sink.finish().unwrap(); let frame = crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 3, diff --git a/src/mux/pipelined_stream.rs b/src/mux/pipelined_stream.rs index fb178150..6503a724 100644 --- a/src/mux/pipelined_stream.rs +++ b/src/mux/pipelined_stream.rs @@ -146,6 +146,7 @@ impl PipelinedPesStream { if skip_parse { // Profiling escape hatch — bypass codec parser. self.pending_frames.push_back(PesFrame { + discard_padding_ns: 0, coding: None, source: None, track, @@ -916,6 +917,7 @@ mod tests { fn write_is_read_only_error() { let (mut stream, _tx) = make_stream(DiscTitle::empty(), vec![], vec![]); let frame = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, diff --git a/src/mux/stdio.rs b/src/mux/stdio.rs index 051a5ca0..d73e261c 100644 --- a/src/mux/stdio.rs +++ b/src/mux/stdio.rs @@ -163,6 +163,7 @@ mod tests { fn write_on_input_stream_is_read_only_error() { let mut s = StdioStream::input(); let frame = crate::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, diff --git a/src/mux/ts.rs b/src/mux/ts.rs index 9f16eb38..0837dd8b 100644 --- a/src/mux/ts.rs +++ b/src/mux/ts.rs @@ -766,9 +766,14 @@ pub fn scan_streams(data: &[u8]) -> Option> { let es_pid = (((pmt[pos + 1] & 0x1F) as u16) << 8) | pmt[pos + 2] as u16; let es_info_len = (((pmt[pos + 3] & 0x0F) as usize) << 8) | pmt[pos + 4] as usize; - // Single source of truth for stream_type → Codec: reuse `Codec::from_coding_type` - // (same table the BD STN/disc scanner uses) so the two mappings can't drift. - let codec = Codec::from_coding_type(stream_type); + // PMTs may also carry ISO/IEC 13818-1 audio stream types that are + // absent from Blu-ray's STN table. Keep that distinction local to TS. + let codec = match stream_type { + 0x03 | 0x04 => Codec::Mp2, + 0x0f => Codec::Aac, + 0x87 => Codec::Ac3Plus, + _ => Codec::from_coding_type(stream_type), + }; let stream = match codec.kind() { CodecKind::Video => { // Default resolution by codec generation (HEVC → @@ -1397,6 +1402,22 @@ mod tests { ); } + #[test] + fn scan_streams_maps_standard_transport_audio_types() { + use crate::disc::{Codec, Stream}; + for (kind, expected) in [ + (0x03, Codec::Mp2), + (0x04, Codec::Mp2), + (0x0f, Codec::Aac), + (0x87, Codec::Ac3Plus), + ] { + let mut data = pat_packet(0x100); + data.extend(pmt_packet(0x100, &[(0x1b, 0x1011), (kind, 0x1100)])); + let streams = scan_streams(&data).unwrap(); + assert!(matches!(&streams[1], Stream::Audio(a) if a.codec == expected)); + } + } + #[test] fn scan_streams_maps_lpcm_via_from_coding_type() { use crate::disc::{Codec, Stream}; diff --git a/src/mux/videomap.rs b/src/mux/videomap.rs index 68505370..25ad2b07 100644 --- a/src/mux/videomap.rs +++ b/src/mux/videomap.rs @@ -423,6 +423,7 @@ mod tests { fn vframe(coding: Option, pts: i64, source: Option) -> PesFrame { let keyframe = coding.map(|c| c.keyframe()).unwrap_or(false); PesFrame { + discard_padding_ns: 0, track: 0, pts, keyframe, diff --git a/src/pes.rs b/src/pes.rs index ef1a4583..b0d43ad4 100644 --- a/src/pes.rs +++ b/src/pes.rs @@ -56,6 +56,9 @@ pub struct PesFrame { /// the PGS parser so the MKV muxer can emit `BlockDuration`; also /// preserved across network:// and stdio:// hops. pub duration_ns: Option, + /// Matroska DiscardPadding in nanoseconds: positive trims the end, negative + /// trims the beginning. Local container remux metadata; not on the PES wire. + pub discard_padding_ns: i64, /// Byte-exact source provenance of this frame's first byte, stamped at the /// demux seam. `None` for synthetic sources / the `skip_parse` path and for /// the `deserialize` (network/stdio) hop — NOT serialized on the wire. @@ -154,6 +157,7 @@ impl PesFrame { filled += want; } Ok(Some(Self { + discard_padding_ns: 0, track, pts, keyframe, @@ -171,6 +175,7 @@ impl PesFrame { // the internal `mux::codec::Frame` type, so it can't be public API. pub(crate) fn from_codec_frame(track: usize, frame: crate::mux::codec::Frame) -> Self { Self { + discard_padding_ns: 0, track, pts: frame.pts_ns, keyframe: frame.keyframe, @@ -182,6 +187,15 @@ impl PesFrame { } } +/// Decoder delay and seek preroll retained across container remuxing. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct TrackTiming { + /// Nanoseconds of decoder delay, applied by the container's consumer. + pub codec_delay_ns: u64, + /// Nanoseconds of decoding required before a seek target. + pub seek_preroll_ns: u64, +} + /// A PES frame stream. One trait per format — same type opens for read /// (`open()` / `listen()` / `input()`) or write (`create()` / `connect()` / /// `output()`). Calling the wrong-direction method returns a typed @@ -207,6 +221,17 @@ pub trait Stream: Send { /// consistent reference for the lifetime of the stream. fn info(&self) -> &crate::disc::DiscTitle; + /// Container timing metadata, indexed in the same order as `info().streams`. + fn track_timing(&self, _track: usize) -> TrackTiming { + TrackTiming::default() + } + + /// Set timing before the first frame is written. Sinks without this metadata + /// retain their existing behavior. + fn set_track_timing(&mut self, _track: usize, _timing: TrackTiming) -> std::io::Result<()> { + Ok(()) + } + /// Codec initialization data for a track (SPS/PPS, AC-3 fscod, etc.). /// `None` for tracks that don't need codec_private (raw passthrough). fn codec_private(&self, _track: usize) -> Option> { @@ -304,6 +329,13 @@ impl Stream for CountingStream { self.inner.info() } + fn track_timing(&self, track: usize) -> TrackTiming { + self.inner.track_timing(track) + } + fn set_track_timing(&mut self, track: usize, timing: TrackTiming) -> std::io::Result<()> { + self.inner.set_track_timing(track, timing) + } + fn codec_private(&self, track: usize) -> Option> { self.inner.codec_private(track) } @@ -332,6 +364,7 @@ mod tests { fn make_frame(track: usize, pts: i64) -> PesFrame { PesFrame { + discard_padding_ns: 0, coding: None, source: None, track, @@ -512,6 +545,7 @@ mod tests { fn serialize_wire_format_matches_spec() { // Wire format: [track(1)][pts_le(8)][keyframe(1)][duration_le(8)][len_le(4)][data...] let frame = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 2, @@ -560,6 +594,7 @@ mod tests { #[test] fn serialize_keyframe_false_encodes_as_zero() { let frame = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -583,6 +618,7 @@ mod tests { #[test] fn serialize_track_255_is_ok_track_256_is_err() { let ok_frame = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 255, @@ -596,6 +632,7 @@ mod tests { assert_eq!(buf[0], 255, "track 255 must serialize to 0xFF"); let too_large = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 256, @@ -617,6 +654,7 @@ mod tests { fn deserialize_round_trips_pts_boundaries() { for pts in [0_i64, i64::MAX, i64::MIN] { let frame = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -639,6 +677,7 @@ mod tests { #[test] fn deserialize_accepts_zero_length_data() { let frame = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 3, @@ -665,6 +704,7 @@ mod tests { fn deserialize_duration_ns_roundtrips() { // None encodes as u64::MAX sentinel and decodes back to None. let frame_none = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -684,6 +724,7 @@ mod tests { // Some(0) must survive — 0 is a valid zero-length duration, not the sentinel. let frame_zero = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 1, @@ -704,6 +745,7 @@ mod tests { // Some(N) for a typical PGS duration (~3 seconds). let frame_n = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 2, @@ -729,6 +771,7 @@ mod tests { #[test] fn deserialize_two_sequential_frames() { let f1 = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -738,6 +781,7 @@ mod tests { duration_ns: None, }; let f2 = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 1, @@ -768,6 +812,7 @@ mod tests { #[test] fn counting_stream_accumulates_across_multiple_writes() { let f1 = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -777,6 +822,7 @@ mod tests { duration_ns: None, }; let f2 = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -801,6 +847,7 @@ mod tests { let size = 2 * 1024 * 1024 + 12_345; let payload: Vec = (0..size).map(|i| (i % 251) as u8).collect(); let frame = PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 7, diff --git a/tests/fvi_pipeline.rs b/tests/fvi_pipeline.rs index 9702eb55..a1eedeaa 100644 --- a/tests/fvi_pipeline.rs +++ b/tests/fvi_pipeline.rs @@ -325,6 +325,7 @@ fn fvi_sink_indexes_non_mpeg2_frames_codec_agnostically() { let title = mpeg2_dvd_title(0); let mk = |pts: i64, keyframe: bool, source: Option| PesFrame { + discard_padding_ns: 0, track: 0, pts, keyframe, diff --git a/tests/streams.rs b/tests/streams.rs index 38ced83c..b82b68e8 100644 --- a/tests/streams.rs +++ b/tests/streams.rs @@ -299,6 +299,7 @@ fn m2ts_stream_write_read() { // Write some PES frames for i in 0..5u8 { let frame = libfreemkv::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -323,6 +324,7 @@ fn m2ts_stream_write_read() { fn m2ts_pes_frame_roundtrip() { // PesFrame serialize/deserialize roundtrip let frame = libfreemkv::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 2, @@ -597,6 +599,7 @@ fn mkvstream_write_finish() { // since there is no real codec data, but it should not panic) for i in 0..20u8 { let frame = libfreemkv::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -660,6 +663,7 @@ fn mkvstream_roundtrip_bdts() { // Write PES frames targeting the audio track for i in 0..10u8 { let frame = libfreemkv::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -859,6 +863,7 @@ fn mkvstream_e2e_h264_produces_valid_mkv() { // Write the ES data (SPS+PPS+IDR) as a keyframe PES frame. let frame1 = libfreemkv::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0, @@ -871,6 +876,7 @@ fn mkvstream_e2e_h264_produces_valid_mkv() { // Write a second non-IDR frame let frame2 = libfreemkv::pes::PesFrame { + discard_padding_ns: 0, coding: None, source: None, track: 0,