mirror of
https://github.com/freemkv/libfreemkv.git
synced 2026-10-07 18:13:10 +00:00
mux: guarantee a BlockDuration on every subtitle block (#52)
Fix (B) — the reproducible bug behind ffmpeg "Timestamps are unset in a packet for stream N" on PGS subtitle tracks (worst on the sparse forced track). Subtitle tracks carry no DefaultDuration, so a frame emitted with duration_ns: None became a bare SimpleBlock with nothing to bound the cue. - codec/pgs.rs: synthesize a fallback dwell (DEFAULT_PGS_DURATION_NS, 5 s) for every emission that was duration_ns: None — the EOS trailing set and the no-PTS clear/display, truncated, and lone-pass-through arms. emit_pending keeps its real computed span but clamps a pathologically large one (missing intermediate PCS) past MAX_PGS_DURATION_NS back to the fallback. - mkv.rs: track_is_subtitle (track_type == 17), mirroring track_is_video. In write_frame_at a subtitle frame with no duration now substitutes a minimum fallback (MIN_SUBTITLE_BLOCK_DURATION_NS, 1 s) so it is emitted as a BlockGroup WITH BlockDuration, never a bare SimpleBlock (defense in depth). Fix (A) — defensive HEVC guard (the "Invalid NAL unit size" error is not reproducible in our clean disc->MKV path; this makes our output structurally incapable of originating it): - codec/hevc.rs: length_prefix_tiles() self-consistency check — the [u32-BE len][body] records must EXACTLY tile the frame; on mismatch the desynced access unit is dropped (logging src position) rather than emitted. - LENGTH_PREFIX_SIZE constant + regression test that hvcC byte-21 lengthSizeMinusOne+1 equals the prefix width push_length_prefixed writes. Tests: pgs.rs None-arm duration assertions updated to the fallback + a clamp/no-clamp pair; mkv.rs subtitle-BlockGroup and two-track (dense+sparse) "every block has a BlockDuration" cases; hevc.rs tiling accept/reject, byte-21 regression, and a positive parsed-AU-tiles-exactly test.
This commit is contained in:
@@ -362,6 +362,11 @@ fn reassert_active(prefix: &mut Vec<u8>, cur: &Option<Vec<u8>>, emitted: bool) {
|
||||
push_length_prefixed(prefix, active);
|
||||
}
|
||||
|
||||
// Width of the NAL length prefix this parser writes (4-byte BE, `u32::to_be_bytes`).
|
||||
// MUST equal hvcC `lengthSizeMinusOne + 1` (byte 21, low bits = 3 => 4); a mismatch
|
||||
// is the framing desync a demuxer reports as "Invalid NAL unit size".
|
||||
const LENGTH_PREFIX_SIZE: usize = 4;
|
||||
|
||||
// Appends `nal` as a 4-byte BE length prefix + body. Skipped (not
|
||||
// mis-framed) if `nal.len()` overflows u32 — unreachable in practice.
|
||||
fn push_length_prefixed(out: &mut Vec<u8>, nal: &[u8]) {
|
||||
@@ -372,6 +377,37 @@ fn push_length_prefixed(out: &mut Vec<u8>, nal: &[u8]) {
|
||||
out.extend_from_slice(nal);
|
||||
}
|
||||
|
||||
/// Walk a length-prefixed NAL buffer ([u32-BE len][body] records) and confirm the
|
||||
/// records EXACTLY tile it: every declared length fits, none is zero, and the last
|
||||
/// body ends precisely at the buffer end with no trailing bytes. Our `frame_data`
|
||||
/// is length-prefixed by construction, so this is a self-consistency guard — a
|
||||
/// `false` means a framing desync that a downstream demuxer would report as
|
||||
/// "Invalid NAL unit size (N>M)", and such an access unit must be dropped rather
|
||||
/// than emitted. See docs/hevc.md — length-prefix self-check (issue #52).
|
||||
fn length_prefix_tiles(data: &[u8]) -> bool {
|
||||
let mut pos = 0usize;
|
||||
while pos < data.len() {
|
||||
// The full length field must be present.
|
||||
if pos + LENGTH_PREFIX_SIZE > data.len() {
|
||||
return false;
|
||||
}
|
||||
let len =
|
||||
u32::from_be_bytes([data[pos], data[pos + 1], data[pos + 2], data[pos + 3]]) as usize;
|
||||
// A zero-length record is a structurally invalid (empty) NALU.
|
||||
if len == 0 {
|
||||
return false;
|
||||
}
|
||||
pos += LENGTH_PREFIX_SIZE;
|
||||
// The declared body must fit within the remaining buffer.
|
||||
match pos.checked_add(len) {
|
||||
Some(end) if end <= data.len() => pos = end,
|
||||
_ => return false,
|
||||
}
|
||||
}
|
||||
// Exact tiling: the walk must land precisely on the buffer end.
|
||||
pos == data.len()
|
||||
}
|
||||
|
||||
impl CodecParser for HevcParser {
|
||||
fn parse(&mut self, pes: &PesPacket) -> Vec<Frame> {
|
||||
if pes.data.is_empty() {
|
||||
@@ -563,6 +599,21 @@ impl CodecParser for HevcParser {
|
||||
}
|
||||
}
|
||||
|
||||
// Defense in depth (issue #52): frame_data is length-prefixed by
|
||||
// construction, but a desynced buffer would surface as "Invalid NAL unit
|
||||
// size". Drop it (log src) rather than emit a mis-framed access unit.
|
||||
if !length_prefix_tiles(&frame_data) {
|
||||
tracing::warn!(
|
||||
target: "freemkv::mux::hevc",
|
||||
src = ?pes.source,
|
||||
frame_len = frame_data.len(),
|
||||
keyframe,
|
||||
"HEVC length-prefix self-check failed; dropping desynced access unit \
|
||||
(would surface downstream as \"Invalid NAL unit size\")"
|
||||
);
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
// HDR10 static metadata is stamped onto every frame's PictureInfo once
|
||||
// both SEI messages are seen, riding the deferred-muxer path (reads it
|
||||
// from the first coded picture before the track header). `None` for SDR tracks.
|
||||
@@ -2689,6 +2740,93 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
// ── issue #52 (Fix A): length-prefix self-consistency ───────────────────
|
||||
|
||||
#[test]
|
||||
fn hvcc_length_size_matches_push_length_prefixed_width() {
|
||||
// Regression: hvcC byte-21 lengthSizeMinusOne+1 (low 2 bits) MUST equal the
|
||||
// prefix width push_length_prefixed writes (LENGTH_PREFIX_SIZE); a divergence
|
||||
// is the framing desync reported as "Invalid NAL unit size".
|
||||
let sps = make_sps_with_chroma(1, 2, 2);
|
||||
let cp = codec_private_from_sps(&sps);
|
||||
let length_size_minus_one = (cp[21] & 0x03) as usize;
|
||||
assert_eq!(
|
||||
length_size_minus_one + 1,
|
||||
LENGTH_PREFIX_SIZE,
|
||||
"hvcC lengthSizeMinusOne+1 must equal the NAL length-prefix width"
|
||||
);
|
||||
assert_eq!(LENGTH_PREFIX_SIZE, 4, "HEVC length prefix is 4-byte BE");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn length_prefix_tiles_accepts_well_formed_records() {
|
||||
// Two records: len 3 + len 2, back to back, tiling the buffer exactly.
|
||||
let mut buf = Vec::new();
|
||||
push_length_prefixed(&mut buf, &[0xAA, 0xBB, 0xCC]);
|
||||
push_length_prefixed(&mut buf, &[0x11, 0x22]);
|
||||
assert!(length_prefix_tiles(&buf));
|
||||
assert!(length_prefix_tiles(&[]), "an empty buffer trivially tiles");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn length_prefix_tiles_rejects_a_desynced_frame() {
|
||||
// A declared length that OVERRUNS the buffer — the shape that surfaces as
|
||||
// "Invalid NAL unit size (N>M)". Declared 0x1000_0001 bytes, only a few
|
||||
// present.
|
||||
let overrun = [0x10, 0x00, 0x00, 0x01, 0xAA, 0xBB, 0xCC];
|
||||
assert!(
|
||||
!length_prefix_tiles(&overrun),
|
||||
"a length that overruns the buffer must be rejected"
|
||||
);
|
||||
// Trailing garbage past the last complete record.
|
||||
let mut trailing = Vec::new();
|
||||
push_length_prefixed(&mut trailing, &[0xAA, 0xBB]);
|
||||
trailing.push(0x99); // one stray byte, not a full length field
|
||||
assert!(
|
||||
!length_prefix_tiles(&trailing),
|
||||
"trailing bytes that don't form a record must be rejected"
|
||||
);
|
||||
// A zero-length record (empty NALU) is structurally invalid.
|
||||
let zero_len = [0x00, 0x00, 0x00, 0x00];
|
||||
assert!(
|
||||
!length_prefix_tiles(&zero_len),
|
||||
"a zero-length record must be rejected"
|
||||
);
|
||||
// A truncated length prefix (fewer than 4 bytes at the tail).
|
||||
assert!(
|
||||
!length_prefix_tiles(&[0x00, 0x00]),
|
||||
"a truncated length prefix must be rejected"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parsed_annex_b_access_unit_tiles_exactly_and_is_emitted() {
|
||||
// Positive path: a normal Annex-B AU (VPS + SPS + PPS + IDR slice) parses
|
||||
// to a frame whose length-prefixed data tiles EXACTLY, so the guard emits
|
||||
// it (never dropped).
|
||||
let mut parser = HevcParser::new();
|
||||
let sps = make_sps_with_chroma(1, 2, 2);
|
||||
let mut data = Vec::new();
|
||||
data.extend_from_slice(&[0x00, 0x00, 0x01]);
|
||||
data.extend_from_slice(&hevc_nal_header(32)); // VPS
|
||||
data.extend_from_slice(&[0xAA, 0xBB]);
|
||||
data.extend_from_slice(&[0x00, 0x00, 0x01]);
|
||||
data.extend_from_slice(&sps); // SPS
|
||||
data.extend_from_slice(&[0x00, 0x00, 0x01]);
|
||||
data.extend_from_slice(&hevc_nal_header(34)); // PPS
|
||||
data.extend_from_slice(&[0xDD, 0xEE]);
|
||||
data.extend_from_slice(&[0x00, 0x00, 0x01]);
|
||||
data.extend_from_slice(&hevc_nal_header(19)); // IDR_W_RADL slice → keyframe
|
||||
data.extend_from_slice(&[0x10, 0x20, 0x30]);
|
||||
|
||||
let frames = parser.parse(&make_pes(data, Some(0)));
|
||||
assert_eq!(frames.len(), 1, "the access unit is emitted, not dropped");
|
||||
assert!(
|
||||
length_prefix_tiles(&frames[0].data),
|
||||
"an emitted HEVC frame's records must tile exactly"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hvcc_handles_emulation_prevention_in_sps() {
|
||||
// Insert emulation-prevention bytes (00 00 03) after the parsed fields
|
||||
|
||||
+77
-18
@@ -16,6 +16,14 @@ const SEGMENT_PCS: u8 = 0x16;
|
||||
// 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.
|
||||
const MAX_PGS_PENDING_BYTES: usize = 4 * 1024 * 1024;
|
||||
// Fallback on-screen dwell (5 s) for a display set whose real end is unknown (EOS
|
||||
// trailing set, no-PTS/truncated arms): a None duration on a DefaultDuration-less
|
||||
// subtitle track leaves ffmpeg "Timestamps are unset in a packet" (issue #52).
|
||||
const DEFAULT_PGS_DURATION_NS: u64 = 5_000_000_000;
|
||||
// Cap on a COMPUTED span (clear_pts - display_pts): a missing intermediate PCS can
|
||||
// inflate one set to minutes. Past this the value is untrusted and the fallback
|
||||
// dwell is used instead; set well above any real dwell so long cues aren't clipped.
|
||||
const MAX_PGS_DURATION_NS: u64 = 30_000_000_000;
|
||||
// Offset of number_of_composition_objects in a PCS: 3-byte segment header +
|
||||
// 10 bytes of PCS fields (video_w/h, frame_rate, comp_num, comp_state,
|
||||
// palette_update, palette_id_ref) = 13.
|
||||
@@ -209,7 +217,15 @@ impl PgsParser {
|
||||
fn emit_pending(&mut self, end_pts_ns: i64) -> Option<Frame> {
|
||||
let (facts, data) = self.pending.take()?;
|
||||
let start_pts = facts.presentation_ns().unwrap_or(0);
|
||||
let duration = end_pts_ns.saturating_sub(start_pts).max(0) as u64;
|
||||
let computed = end_pts_ns.saturating_sub(start_pts).max(0) as u64;
|
||||
// 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 {
|
||||
DEFAULT_PGS_DURATION_NS
|
||||
} else {
|
||||
computed
|
||||
};
|
||||
Some(Frame {
|
||||
discontinuity: false,
|
||||
coding: None,
|
||||
@@ -249,7 +265,7 @@ impl CodecParser for PgsParser {
|
||||
pts_ns: facts.presentation_ns().unwrap_or(0),
|
||||
keyframe: true,
|
||||
data,
|
||||
duration_ns: None,
|
||||
duration_ns: Some(DEFAULT_PGS_DURATION_NS),
|
||||
}]
|
||||
})
|
||||
.unwrap_or_default();
|
||||
@@ -276,7 +292,7 @@ impl CodecParser for PgsParser {
|
||||
pts_ns: facts.presentation_ns().unwrap_or(0),
|
||||
keyframe: true,
|
||||
data,
|
||||
duration_ns: None,
|
||||
duration_ns: Some(DEFAULT_PGS_DURATION_NS),
|
||||
}),
|
||||
};
|
||||
out.extend(frame);
|
||||
@@ -307,7 +323,7 @@ impl CodecParser for PgsParser {
|
||||
pts_ns: facts.presentation_ns().unwrap_or(0),
|
||||
keyframe: true,
|
||||
data,
|
||||
duration_ns: None,
|
||||
duration_ns: Some(DEFAULT_PGS_DURATION_NS),
|
||||
}));
|
||||
}
|
||||
},
|
||||
@@ -336,7 +352,7 @@ impl CodecParser for PgsParser {
|
||||
pts_ns: pts.unwrap_or(0),
|
||||
keyframe: true,
|
||||
data: pes.data.clone(),
|
||||
duration_ns: None,
|
||||
duration_ns: Some(DEFAULT_PGS_DURATION_NS),
|
||||
});
|
||||
}
|
||||
// No pending set AND no PTS: drop it. Emitting at pts_ns=0 would
|
||||
@@ -360,7 +376,7 @@ impl CodecParser for PgsParser {
|
||||
pts_ns: facts.presentation_ns().unwrap_or(0),
|
||||
keyframe: true,
|
||||
data,
|
||||
duration_ns: None,
|
||||
duration_ns: Some(DEFAULT_PGS_DURATION_NS),
|
||||
}],
|
||||
None => Vec::new(),
|
||||
}
|
||||
@@ -548,8 +564,10 @@ mod tests {
|
||||
assert_eq!(frames.len(), 1, "final pending subtitle must flush");
|
||||
assert_eq!(frames[0].pts_ns, 1_000_000_000);
|
||||
assert_eq!(frames[0].data, display);
|
||||
// Trailing block lingers to EOF — no duration per module doc.
|
||||
assert_eq!(frames[0].duration_ns, None);
|
||||
// Trailing block has no follower PCS, so its real end is unknown; it now
|
||||
// carries the synthesized fallback dwell (was `None`) so every subtitle
|
||||
// block gets a BlockDuration (issue #52).
|
||||
assert_eq!(frames[0].duration_ns, Some(DEFAULT_PGS_DURATION_NS));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -575,16 +593,20 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clear_pcs_without_pts_emits_pending_undurated() {
|
||||
// A clear PCS that lacks a PTS can't compute a duration; the pending
|
||||
// display is still emitted, but with no duration (lingers to EOF)
|
||||
// instead of a bogus absurd one.
|
||||
fn clear_pcs_without_pts_emits_fallback_duration() {
|
||||
// A clear PCS that lacks a PTS can't compute a real duration; the pending
|
||||
// display is still emitted, now with the synthesized fallback dwell (was
|
||||
// `None`) so the subtitle block carries a BlockDuration (issue #52).
|
||||
let mut parser = PgsParser::new();
|
||||
let _ = parser.parse(&make_pes(pcs_bytes(1), Some(90000)));
|
||||
let f = parser.parse(&make_pes(pcs_bytes(0), None));
|
||||
assert_eq!(f.len(), 1);
|
||||
assert_eq!(f[0].pts_ns, 1_000_000_000, "pending keeps its real start");
|
||||
assert_eq!(f[0].duration_ns, None, "no duration without a clear PTS");
|
||||
assert_eq!(
|
||||
f[0].duration_ns,
|
||||
Some(DEFAULT_PGS_DURATION_NS),
|
||||
"fallback duration when the clear PCS carries no PTS"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -605,7 +627,11 @@ mod tests {
|
||||
let frames = parser.parse(&make_pes(truncated, Some(180000)));
|
||||
assert_eq!(frames.len(), 1, "pending display flushed on truncated PCS");
|
||||
assert_eq!(frames[0].data, display, "pending bitmap not polluted");
|
||||
assert_eq!(frames[0].duration_ns, None, "flushed undurated");
|
||||
assert_eq!(
|
||||
frames[0].duration_ns,
|
||||
Some(DEFAULT_PGS_DURATION_NS),
|
||||
"flushed with fallback duration (issue #52)"
|
||||
);
|
||||
assert!(parser.pending.is_none(), "parser resynced");
|
||||
}
|
||||
|
||||
@@ -695,6 +721,37 @@ mod tests {
|
||||
assert_eq!(f[0].duration_ns, Some(0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pathologically_large_computed_duration_clamps_to_fallback() {
|
||||
// A missing intermediate clear PCS makes a set appear to linger until a much
|
||||
// later one; a computed span past MAX_PGS_DURATION_NS is untrusted and falls
|
||||
// back to the dwell instead of an absurd multi-minute cue (90000 ticks = 1s).
|
||||
let display_pts = 90_000_i64; // 1 s
|
||||
// 40 s later in 90 kHz ticks (> 30 s cap).
|
||||
let clear_pts = display_pts + 40 * 90_000;
|
||||
let mut parser = PgsParser::new();
|
||||
let _ = parser.parse(&make_pes(pcs_bytes(1), Some(display_pts)));
|
||||
let f = parser.parse(&make_pes(pcs_bytes(0), Some(clear_pts)));
|
||||
assert_eq!(f.len(), 1);
|
||||
assert_eq!(
|
||||
f[0].duration_ns,
|
||||
Some(DEFAULT_PGS_DURATION_NS),
|
||||
"a >30s computed span is clamped to the fallback dwell"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legitimate_long_duration_is_not_clamped() {
|
||||
// A real, in-range dwell (10 s, well under the 30 s cap) is preserved
|
||||
// exactly — the clamp must never clip a legitimately long subtitle.
|
||||
let display_pts = 90_000_i64; // 1 s
|
||||
let clear_pts = display_pts + 10 * 90_000; // +10 s
|
||||
let mut parser = PgsParser::new();
|
||||
let _ = parser.parse(&make_pes(pcs_bytes(1), Some(display_pts)));
|
||||
let f = parser.parse(&make_pes(pcs_bytes(0), Some(clear_pts)));
|
||||
assert_eq!(f[0].duration_ns, Some(10_000_000_000));
|
||||
}
|
||||
|
||||
// --- clear / replace edge cases ---
|
||||
|
||||
#[test]
|
||||
@@ -722,11 +779,11 @@ mod tests {
|
||||
assert_eq!(f2.len(), 1);
|
||||
assert_eq!(f2[0].pts_ns, 2_000_000_000);
|
||||
assert_eq!(f2[0].duration_ns, Some(1_000_000_000));
|
||||
// Third held; flush emits it undurated.
|
||||
// Third held; flush emits it with the fallback dwell (no follower PCS).
|
||||
let tail = parser.flush();
|
||||
assert_eq!(tail.len(), 1);
|
||||
assert_eq!(tail[0].pts_ns, 3_000_000_000);
|
||||
assert_eq!(tail[0].duration_ns, None);
|
||||
assert_eq!(tail[0].duration_ns, Some(DEFAULT_PGS_DURATION_NS));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -740,7 +797,7 @@ mod tests {
|
||||
let exactly_13 = vec![SEGMENT_PCS; PCS_NUM_OBJECTS_OFFSET]; // 13 bytes
|
||||
let f = parser.parse(&make_pes(exactly_13, Some(180000)));
|
||||
assert_eq!(f.len(), 1, "13-byte PCS is truncated → flush pending");
|
||||
assert_eq!(f[0].duration_ns, None);
|
||||
assert_eq!(f[0].duration_ns, Some(DEFAULT_PGS_DURATION_NS));
|
||||
assert!(parser.pending.is_none());
|
||||
}
|
||||
|
||||
@@ -768,7 +825,9 @@ mod tests {
|
||||
assert_eq!(f.len(), 1);
|
||||
assert!(f[0].keyframe);
|
||||
assert_eq!(f[0].pts_ns, 1_000_000_000);
|
||||
assert_eq!(f[0].duration_ns, None);
|
||||
// A pass-through segment has no known end; it now carries the fallback
|
||||
// dwell (was `None`) so it too gets a BlockDuration (issue #52).
|
||||
assert_eq!(f[0].duration_ns, Some(DEFAULT_PGS_DURATION_NS));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
+159
@@ -546,6 +546,14 @@ pub struct MkvMuxer<W: Write + Seek> {
|
||||
/// non-monotonic. Keying the exemption on track type (not index) keeps that
|
||||
/// EL's true PTS instead of clobbering it to prev+1ms.
|
||||
track_is_video: Vec<bool>,
|
||||
/// Per-track-index flag: true if the track is a subtitle track
|
||||
/// (`track_type == 17`). A subtitle block must NEVER be written as a bare
|
||||
/// `SimpleBlock`: subtitle tracks carry no `DefaultDuration`, so without a
|
||||
/// `BlockDuration` a demuxer has nothing to bound the cue and ffmpeg reports
|
||||
/// "Timestamps are unset in a packet for stream N" (issue #52). Keyed on
|
||||
/// track type (mirroring `track_is_video`) so the invariant holds for every
|
||||
/// subtitle track — including a sparse forced-narrative second track.
|
||||
track_is_subtitle: Vec<bool>,
|
||||
/// Index of the PRIMARY video track — the first track whose type is video.
|
||||
/// This (not the literal index 0) is the clip-boundary epoch driver: the
|
||||
/// M2TS/PMT path orders streams by PMT declaration order and may list an
|
||||
@@ -698,6 +706,11 @@ struct InterlacedRewrite {
|
||||
// video and TrueHD's 0.833 ms AUs. See docs/mkv-mux.md — TimestampScale choice.
|
||||
const TIMESTAMP_SCALE_NS: i64 = 100_000;
|
||||
|
||||
// Last-resort BlockDuration (1 s) for a SUBTITLE frame that reaches the muxer with
|
||||
// no duration. Parsers (PGS) already synthesize one; this only fires if another
|
||||
// subtitle path forgets, keeping every subtitle block durated (issue #52).
|
||||
const MIN_SUBTITLE_BLOCK_DURATION_NS: i64 = 1_000_000_000;
|
||||
|
||||
// Nominal new-cluster interval (2 s) in TimestampScale ticks; a keyframe
|
||||
// only opens a new cluster once this much has elapsed. See docs/mkv-mux.md
|
||||
// — cluster duration vs. i16 block-relative limit.
|
||||
@@ -1128,6 +1141,10 @@ impl<W: Write + Seek> MkvMuxer<W> {
|
||||
.iter()
|
||||
.map(|t| t.track_type == ebml::TRACK_TYPE_VIDEO)
|
||||
.collect(),
|
||||
track_is_subtitle: tracks
|
||||
.iter()
|
||||
.map(|t| t.track_type == ebml::TRACK_TYPE_SUBTITLE)
|
||||
.collect(),
|
||||
primary_video_track: tracks
|
||||
.iter()
|
||||
.position(|t| t.track_type == ebml::TRACK_TYPE_VIDEO),
|
||||
@@ -1350,6 +1367,21 @@ impl<W: Write + Seek> MkvMuxer<W> {
|
||||
let relative_ts = (pts_ticks - self.cluster_ts_ticks) as i16;
|
||||
let duration_ticks =
|
||||
duration_ns.map(|dur_ns| (dur_ns as i64 / TIMESTAMP_SCALE_NS).max(1) as u64);
|
||||
// Defense in depth (issue #52): a SUBTITLE block must never be a bare
|
||||
// SimpleBlock (no DefaultDuration => unbounded cue, ffmpeg "Timestamps are
|
||||
// unset"). Substitute a minimum fallback so it takes the BlockGroup arm.
|
||||
let is_subtitle = self
|
||||
.track_is_subtitle
|
||||
.get(track_idx)
|
||||
.copied()
|
||||
.unwrap_or(false);
|
||||
let duration_ticks = match duration_ticks {
|
||||
Some(dt) => Some(dt),
|
||||
None if is_subtitle => {
|
||||
Some((MIN_SUBTITLE_BLOCK_DURATION_NS / TIMESTAMP_SCALE_NS).max(1) as u64)
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
// A BlockAdditional with BlockAddID=2 conforms only when the track declared
|
||||
// the matching mvcC mapping; if not (dependent-view params never captured
|
||||
// before the header was written), drop it and emit a plain block instead.
|
||||
@@ -4790,6 +4822,133 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
// ── issue #52: every subtitle block carries a BlockDuration ──────────────
|
||||
|
||||
fn make_subtitle_track() -> MkvTrack {
|
||||
MkvTrack {
|
||||
track_type: ebml::TRACK_TYPE_SUBTITLE,
|
||||
codec_id: ebml::CODEC_PGS,
|
||||
language: "eng".into(),
|
||||
name: String::new(),
|
||||
codec_private: None,
|
||||
is_default: false,
|
||||
is_forced: false,
|
||||
pixel_width: 0,
|
||||
pixel_height: 0,
|
||||
default_duration_ns: 0,
|
||||
display_width: 0,
|
||||
display_height: 0,
|
||||
colour_matrix: 0,
|
||||
colour_transfer: 0,
|
||||
colour_primaries: 0,
|
||||
colour_range: 0,
|
||||
interlaced: false,
|
||||
field_order: ebml::FIELD_ORDER_UNDETERMINED,
|
||||
field_duration_ns: 0,
|
||||
sample_rate: 0.0,
|
||||
channels: 0,
|
||||
bit_depth: 0,
|
||||
dv_config: None,
|
||||
hdr10: None,
|
||||
mvc_params: None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Matroska TrackNumbers (1-based) of every SimpleBlock in the output, in
|
||||
/// emission order. The subtitle invariant is that NONE of these is a
|
||||
/// subtitle track.
|
||||
fn all_simple_block_tracks(data: &[u8]) -> Vec<u64> {
|
||||
let mut out = Vec::new();
|
||||
for (body_start, body_size, _ts) in find_clusters(data) {
|
||||
let body = &data[body_start..body_start + body_size as usize];
|
||||
let mut cursor = Cursor::new(body);
|
||||
let (tid, tsize, _) = ebml::read_element_header(&mut cursor).unwrap();
|
||||
assert_eq!(tid, ebml::CLUSTER_TIMESTAMP);
|
||||
cursor.seek(io::SeekFrom::Current(tsize as i64)).unwrap();
|
||||
while (cursor.position() as usize) < body.len() {
|
||||
let Ok((id, size, _)) = ebml::read_element_header(&mut cursor) else {
|
||||
break;
|
||||
};
|
||||
let start = cursor.position() as usize;
|
||||
if start + size as usize > body.len() {
|
||||
break;
|
||||
}
|
||||
if id == ebml::SIMPLE_BLOCK {
|
||||
let blk = &body[start..start + size as usize];
|
||||
let track = if blk[0] & 0x80 != 0 {
|
||||
(blk[0] & 0x7F) as u64
|
||||
} else {
|
||||
(((blk[0] & 0x3F) as u64) << 8) | blk[1] as u64
|
||||
};
|
||||
out.push(track);
|
||||
}
|
||||
cursor.seek(io::SeekFrom::Current(size as i64)).unwrap();
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn subtitle_frame_with_no_duration_becomes_block_group_with_block_duration() {
|
||||
// issue #52: a SUBTITLE frame with duration=None must never be a bare
|
||||
// SimpleBlock (no DefaultDuration => unbounded cue). The muxer substitutes a
|
||||
// minimum fallback and emits a BlockGroup WITH a BlockDuration.
|
||||
let tracks = [make_subtitle_track()];
|
||||
let data = mux_with_durations(&tracks, &[(0, 0, true, vec![0x16, 0xAA], None)]);
|
||||
|
||||
let groups = all_block_groups(&data);
|
||||
assert_eq!(
|
||||
groups.len(),
|
||||
1,
|
||||
"the subtitle frame must be a BlockGroup, not a SimpleBlock"
|
||||
);
|
||||
assert!(
|
||||
groups[0].duration > 0,
|
||||
"the BlockGroup must carry a BlockDuration (got {})",
|
||||
groups[0].duration
|
||||
);
|
||||
assert!(
|
||||
all_simple_block_tracks(&data).is_empty(),
|
||||
"no subtitle frame may be emitted as a bare SimpleBlock"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_block_on_both_subtitle_tracks_has_a_block_duration() {
|
||||
// Models a Blu-ray's two PGS tracks: a DENSE dialogue track and a SPARSE
|
||||
// forced-narrative one. EVERY block on BOTH must be a BlockGroup carrying a
|
||||
// BlockDuration, even those handed in with duration=None.
|
||||
let tracks = [make_subtitle_track(), make_subtitle_track()];
|
||||
let frames: Vec<DurFrame> = vec![
|
||||
// Track 0 (dense): opens the cluster, then two more, mixing a real
|
||||
// duration with two None durations.
|
||||
(0, 0, true, vec![0x16, 0x01], Some(2_000_000_000)),
|
||||
(0, 1_000_000_000, true, vec![0x16, 0x02], None),
|
||||
(0, 2_000_000_000, true, vec![0x16, 0x03], None),
|
||||
// Track 1 (sparse): a single forced cue, handed in with no duration.
|
||||
(1, 500_000_000, true, vec![0x16, 0xFF], None),
|
||||
];
|
||||
let data = mux_with_durations(&tracks, &frames);
|
||||
|
||||
assert!(
|
||||
all_simple_block_tracks(&data).is_empty(),
|
||||
"no subtitle frame on EITHER track may be a bare SimpleBlock"
|
||||
);
|
||||
let groups = all_block_groups(&data);
|
||||
assert_eq!(groups.len(), 4, "all four subtitle frames are BlockGroups");
|
||||
for g in &groups {
|
||||
assert!(
|
||||
g.duration > 0,
|
||||
"every subtitle block must carry a BlockDuration (track {}, dur {})",
|
||||
g.track,
|
||||
g.duration
|
||||
);
|
||||
}
|
||||
// Both subtitle tracks (Matroska TrackNumbers 1 and 2) are represented.
|
||||
assert!(groups.iter().any(|g| g.track == 1), "dense track present");
|
||||
assert!(groups.iter().any(|g| g.track == 2), "sparse track present");
|
||||
}
|
||||
|
||||
// Cluster boundary (CLUSTER_DURATION_TICKS): a new cluster opens on a video
|
||||
// keyframe once >= the cluster duration has elapsed since the open cluster's
|
||||
// timestamp; one just under the boundary stays in the current cluster.
|
||||
|
||||
Reference in New Issue
Block a user