Reference keyframes per track, size the DTS reserve, correct two claims
The ReferenceBlock offset was computed for ANY video track, but the keyframe tick it measures against was recorded in a single global slot gated to the PRIMARY video track. On a title with two video tracks — an MVC base plus secondary view, or a multi-angle disc — a secondary track's non-keyframe therefore referenced a keyframe on a different track, or 0 (a self-reference) when the primary had not produced one yet. The tick is now recorded per track, so a non-keyframe can only reference a keyframe on its own track. The faststart moov-hole estimate modelled every audio track as (E-)AC-3 at 1536 samples per frame. 1.6.0 added DTS to the writer's carried set, and a DTS core AU is commonly 512 samples — a third of that — so a DTS track's sample table was under-reserved threefold and the mux fell back to moov-at-end, losing faststart on exactly the files 1.6.0 newly supports. mvc_frame_emits_blockgroup_additional_and_reference asserted only that the non-keyframe's ReferenceBlock was Some(_). Its non-MVC sibling, added in the same commit, pins the exact offset; this one now does too, so a mutant emitting a constant or wrong-signed offset no longer passes. The comment above the mp4 sample budget claimed file_len stops a crafted file inflating allocations "past the file's own size". Each indexed sample costs ~52 bytes, so the real ceiling is ~52x file_len (still capped by MAX_SAMPLE_COUNT). The bound is real; the comment overstated how tight it is.
This commit is contained in:
+33
-14
@@ -679,10 +679,15 @@ pub struct MkvMuxer<W: Write + Seek> {
|
||||
/// the muxed runtime, used to back-patch the DURATION placeholder.
|
||||
max_block_ticks: i64,
|
||||
/// Timestamp (TimestampScale ticks) of the last video keyframe written on the
|
||||
/// primary video track. A non-keyframe MVC base frame lives in a BlockGroup
|
||||
/// (to carry its dependent-view BlockAdditional) and needs a `ReferenceBlock`
|
||||
/// so players don't mistake it for a keyframe; it references this keyframe.
|
||||
last_video_keyframe_ticks: Option<i64>,
|
||||
/// video track. A non-keyframe frame written as a BlockGroup needs a
|
||||
/// `ReferenceBlock` so players don't mistake it for a keyframe, and it must
|
||||
/// reference a keyframe on its OWN track.
|
||||
///
|
||||
/// Per track, not global: the ReferenceBlock is emitted for ANY video track,
|
||||
/// so a single global value made a secondary video track's non-keyframe point
|
||||
/// at a keyframe on a different track (or at 0, a self-reference, when the
|
||||
/// primary had not produced one yet). Indexed by `track_idx`.
|
||||
last_video_keyframe_ticks: Vec<Option<i64>>,
|
||||
/// Per-AC-3-audio-track channel-correction state. The DVD IFO audio nibble
|
||||
/// is unreliable, so the channel count written in the track header is
|
||||
/// corrected from the AC-3 bitstream `acmod` of the first frame on the
|
||||
@@ -1257,7 +1262,7 @@ impl<W: Write + Seek> MkvMuxer<W> {
|
||||
duration_secs,
|
||||
duration_patch_pos,
|
||||
max_block_ticks: 0,
|
||||
last_video_keyframe_ticks: None,
|
||||
last_video_keyframe_ticks: vec![None; tracks.len()],
|
||||
ac3_channel_fixups,
|
||||
pgs_forced_fixups,
|
||||
opening_capture: None,
|
||||
@@ -1506,6 +1511,9 @@ impl<W: Write + Seek> MkvMuxer<W> {
|
||||
} else {
|
||||
Some(
|
||||
self.last_video_keyframe_ticks
|
||||
.get(track_idx)
|
||||
.copied()
|
||||
.flatten()
|
||||
.map(|kf| kf - pts_ticks)
|
||||
.unwrap_or(0),
|
||||
)
|
||||
@@ -1536,12 +1544,15 @@ impl<W: Write + Seek> MkvMuxer<W> {
|
||||
}
|
||||
},
|
||||
}
|
||||
// Remember the last PRIMARY-video keyframe's tick so a later non-keyframe
|
||||
// MVC base frame references a keyframe on its OWN track (see the
|
||||
// block_additional path above). Gating to the primary video track avoids a
|
||||
// secondary video track's keyframe becoming a cross-track reference target.
|
||||
if keyframe && Some(track_idx) == self.primary_video_track {
|
||||
self.last_video_keyframe_ticks = Some(pts_ticks);
|
||||
// Remember this VIDEO track's last keyframe tick, so a later non-keyframe
|
||||
// on the same track references a keyframe on its own track. Recorded per
|
||||
// track: the ReferenceBlock above is emitted for any video track, so a
|
||||
// single global slot produced cross-track references on a multi-video-track
|
||||
// title (MVC base + secondary view, or a disc with two angles).
|
||||
if keyframe && is_video {
|
||||
if let Some(slot) = self.last_video_keyframe_ticks.get_mut(track_idx) {
|
||||
*slot = Some(pts_ticks);
|
||||
}
|
||||
}
|
||||
self.frame_count += 1;
|
||||
|
||||
@@ -5220,9 +5231,17 @@ mod tests {
|
||||
groups[0].reference, None,
|
||||
"the MVC keyframe must carry NO ReferenceBlock — that absence IS the keyframe signal"
|
||||
);
|
||||
assert!(
|
||||
groups[1].reference.is_some(),
|
||||
"the non-keyframe MVC base frame must carry a ReferenceBlock"
|
||||
// Assert the OFFSET, not just presence — the sibling non-MVC BlockGroup
|
||||
// test pins the exact value, and a mutant emitting a constant or a
|
||||
// wrong-signed offset would pass a presence-only check. The keyframe sits
|
||||
// at tick 0, so the offset is the negated block-relative timestamp.
|
||||
let off = groups[1]
|
||||
.reference
|
||||
.expect("the non-keyframe MVC base frame must carry a ReferenceBlock");
|
||||
assert_eq!(
|
||||
off,
|
||||
-(groups[1].rel_ts as i64),
|
||||
"the MVC ReferenceBlock must point back to the keyframe at tick 0"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
+11
-2
@@ -82,8 +82,17 @@ fn estimate_reserve(title: &DiscTitle, included: &[usize]) -> u64 {
|
||||
est_samples += dur * fps;
|
||||
}
|
||||
DiscStream::Audio(a) => {
|
||||
// ~1536 samples per (E-)AC-3 frame.
|
||||
est_samples += dur * (a.sample_rate.hz() / 1536.0);
|
||||
// Samples per frame differs sharply by codec, and 1.6.0 added DTS
|
||||
// to the carried set (audio_fits now admits Dts | DtsHdMa |
|
||||
// DtsHdHr). A DTS core AU is (nblks+1)*32 — commonly 512 samples,
|
||||
// a third of an (E-)AC-3 frame's 1536 — so modelling every audio
|
||||
// track as AC-3 under-reserved a DTS track's sample table 3x and
|
||||
// pushed the mux onto the moov-at-end fallback.
|
||||
let samples_per_frame = match a.codec {
|
||||
Codec::Dts | Codec::DtsHdMa | Codec::DtsHdHr => 512.0,
|
||||
_ => 1536.0,
|
||||
};
|
||||
est_samples += dur * (a.sample_rate.hz() / samples_per_frame);
|
||||
}
|
||||
DiscStream::Subtitle(_) => {}
|
||||
}
|
||||
|
||||
@@ -98,6 +98,12 @@ impl<R: Read + Seek> Mp4Reader<R> {
|
||||
// crafted file with a fixed-size `stsz` claiming count=0xFFFFFFFF can't
|
||||
// inflate the `sizes`/`Vec<SampleRef>` allocations past the file's own size
|
||||
// (a genuine large title has file_len ≫ sample count, so it is unaffected).
|
||||
// NOTE on the bound this actually gives: each indexed sample costs about
|
||||
// 52 bytes of RAM (SampleRef 40 + u32 size 4 + u64 offset 8, plus 4 each
|
||||
// for the expanded stts/ctts), so the ceiling is ~52x file_len, not 1x —
|
||||
// capped by MAX_SAMPLE_COUNT. That is still a real bound (a 1 MiB crafted
|
||||
// file cannot reach the 16M-sample ceiling), just not the "past the file's
|
||||
// own size" the previous comment implied.
|
||||
let mut sample_budget = MAX_SAMPLE_COUNT.min(file_len.min(usize::MAX as u64) as usize);
|
||||
|
||||
// Bound the scan at MAX_TRACKS *matches* so a crafted moov packed with tiny
|
||||
|
||||
Reference in New Issue
Block a user