3D MVC mux: audit round 2 (converged)

Second audit round converged (severity collapsed 6 HIGH -> 1; the one
HIGH was a bounded 32-element scan, not a defect; the sole spec MEDIUM
was the same false-positive re-raised — 0xBF matches ISO/IEC 14496-15
§7.6.2 verbatim). One genuine robustness fix plus coverage:

- extract_mvc_params: skip a zero-length NAL instead of abandoning the
  scan, so a stray length prefix before the subset SPS/PPS no longer
  silently drops 3D signalling. Test proves params after a zero-length
  NAL are still found.
- Tests: parser_for_mvc_dependent routes H.264 to a passthrough parser;
  passthrough with an IDR does not re-assert param sets (the keyframe &&
  !mvc branch).
- Document the per-playlist (not per-clip) is_3d latching as a known
  limitation (real main-feature playlists are uniformly 3D).
This commit is contained in:
Matthew Jackson
2026-07-13 11:39:07 -07:00
parent d4021114cd
commit 422f2b6bcf
3 changed files with 72 additions and 2 deletions
+9
View File
@@ -257,6 +257,15 @@ impl Disc {
// dependent view (it lives in the MPLS STN_table_SS), so we use the
// BD-3D PID convention: dependent = base-view video PID + 1
// (e.g. 0x1011 -> 0x1012). Reading the SSIF (above) provides its packets.
//
// Limitation: `is_3d` latches per PLAYLIST, not per clip. A playlist that
// mixed a 3D clip (has an SSIF) with a 2D clip (no SSIF) would tag the
// whole title 3D; the 2D clip's frames then mux as plain Blocks (no
// dependent PID → no BlockAdditional) under a track that still advertises
// the mvcC mapping. That output is valid (per-frame BlockAdditional is
// optional) but over-claims 3D for those frames. Real 3D main-feature
// playlists are single-clip or uniformly 3D, so this is not exercised;
// per-clip 3D would need per-clip stream sets (a larger change).
if is_3d {
if let Some(base) = streams.iter().find_map(|s| match s {
Stream::Video(v) => Some(v.clone()),
+48
View File
@@ -714,6 +714,54 @@ mod tests {
assert!(types.contains(&20), "slice kept: {types:?}");
}
#[test]
fn parser_for_mvc_dependent_h264_is_passthrough() {
// The dependent-view stream must get a passthrough parser: a PPS in a
// non-keyframe AU is kept in-band, not stripped like the base parser.
let mut p = crate::mux::codec::parser_for_mvc_dependent(crate::disc::Codec::H264, false);
let mut d = Vec::new();
d.extend_from_slice(&h264_nal(0x68, &[0xCE, 0x01])); // PPS (8)
d.extend_from_slice(&h264_nal(0x74, &[0x11, 0x22])); // slice-ext (20)
let f = p.parse(&make_pes(d, Some(90000)));
assert_eq!(f.len(), 1);
let types: Vec<u8> = h264_nals_in(&f[0].data)
.iter()
.map(|n| n[0] & 0x1F)
.collect();
assert!(
types.contains(&8),
"dependent parser keeps PPS in-band (passthrough): {types:?}"
);
}
#[test]
fn mvc_passthrough_with_idr_does_not_reassert_param_sets() {
// With an IDR present (keyframe=true), passthrough must NOT re-assert the
// param sets (the `keyframe && !mvc` guard), so SPS/PPS appear exactly
// once — a duplicate would corrupt the dependent BlockAdditional.
let mut p = H264Parser::new().with_mvc_passthrough(true);
let mut d = Vec::new();
d.extend_from_slice(&h264_nal(0x67, &[0x42, 0x00, 0x1E, 0x01])); // SPS (7)
d.extend_from_slice(&h264_nal(0x68, &[0xCE, 0x01])); // PPS (8)
d.extend_from_slice(&h264_nal(0x65, &[0x88, 0x00])); // IDR slice (5)
let f = p.parse(&make_pes(d, Some(90000)));
assert_eq!(f.len(), 1);
let types: Vec<u8> = h264_nals_in(&f[0].data)
.iter()
.map(|n| n[0] & 0x1F)
.collect();
assert_eq!(
types.iter().filter(|&&t| t == 7).count(),
1,
"exactly one SPS, no keyframe re-assert under passthrough: {types:?}"
);
assert_eq!(
types.iter().filter(|&&t| t == 8).count(),
1,
"exactly one PPS, no keyframe re-assert under passthrough: {types:?}"
);
}
#[test]
fn h264_populates_measured_coding_type_and_source() {
use super::super::coding::CodingType;
+15 -2
View File
@@ -299,7 +299,13 @@ fn extract_mvc_params(data: &[u8]) -> Option<(Vec<u8>, Vec<u8>)> {
while i + 4 <= data.len() {
let len = u32::from_be_bytes([data[i], data[i + 1], data[i + 2], data[i + 3]]) as usize;
i += 4;
if len == 0 || i + len > data.len() {
// A zero-length NAL (a stray length prefix) is skipped, not fatal — the
// subset SPS / PPS may still follow. A length that runs past the buffer
// end IS unrecoverable (the NAL can't be read), so stop there.
if len == 0 {
continue;
}
if i + len > data.len() {
break;
}
let nal = &data[i..i + len];
@@ -1315,12 +1321,19 @@ mod tests {
assert!(extract_mvc_params(&[0, 0, 0]).is_none());
assert!(
extract_mvc_params(&[0, 0, 0, 0]).is_none(),
"zero-length NAL breaks"
"lone zero-length NAL yields no params"
);
assert!(
extract_mvc_params(&[0, 0, 0, 10, 0x6F]).is_none(),
"length prefix past end breaks, no slice panic"
);
// A zero-length NAL is SKIPPED, not fatal: valid param sets that follow
// are still found (a stray length prefix must not abandon the whole AU).
let mut d = vec![0, 0, 0, 0];
d.extend_from_slice(&lp(&[&SUBSET_SPS, &DEP_PPS]));
let (s, p) = extract_mvc_params(&d).expect("params found past the zero-length NAL");
assert_eq!(s, SUBSET_SPS);
assert_eq!(p, DEP_PPS);
}
#[test]