Cover the non-NAL video path, including the wiring that selects it

set_nal_video had exactly one production caller and zero test callers. The
branch it gates decides whether a video track's ES goes through
length_prefixed_to_annex_b, and MPEG-2 and VC-1 must not: they are already
start-code ES. Getting it wrong is silent — frame_count still increments,
so the mux reports success while emitting a video-less file.

Five tests, each verified against a real mutant:

  * non-NAL ES passes through byte-for-byte, and the default path still
    converts. Both use a deliberately length-prefix-SHAPED payload so a
    wrongly-applied conversion rewrites the leading four bytes into a start
    code — a payload the converter happened to leave alone would let a
    mutant pass.
  * a non-NAL keyframe arms params_written, so following non-keyframes are
    not dropped by the pre-keyframe guard.
  * set_nal_video rejects an out-of-range track instead of panicking.
  * M2tsStream::create wires a VC-1 track to the non-NAL path.

That last one matters more than it looks. The four TsMuxer-level tests set
the flag themselves, so deleting the set_nal_video loop from
M2tsStream::create left all 2366 tests passing — the exact mutant the
finding named went undetected until a test drove the real wiring. It now
also catches the subtler mutant of widening the matches! arm to include
Vc1.
This commit is contained in:
Matthew Jackson
2026-07-29 17:54:27 -07:00
parent d09ed76e07
commit bf2d15f39c
2 changed files with 145 additions and 0 deletions
+46
View File
@@ -170,6 +170,52 @@ mod tests {
} }
} }
/// `M2tsStream::create` must opt a VC-1 video track OUT of Annex-B conversion.
///
/// This pins the WIRING in `create`, not just `TsMuxer`'s flag: deleting the
/// `set_nal_video` loop leaves every TsMuxer-level test passing, because those
/// drive the muxer directly and set the flag themselves. Only a test that goes
/// through `create` catches it — and mangling MPEG-2/VC-1 video is silent, since
/// frame_count still increments and the mux reports success.
///
/// Mutation: remove the `set_nal_video` loop from `create`, or widen its
/// `matches!` to include Vc1 -> the ES gains a start code and this fails.
#[test]
fn vc1_video_is_wired_to_the_non_nal_path() {
let mut title = make_title();
if let DiscStream::Video(v) = &mut title.streams[0] {
v.codec = Codec::Vc1;
}
title.codec_privates = vec![None];
// Length-prefix SHAPED ES: if the conversion is wrongly applied it rewrites
// these leading four bytes into a 00 00 00 01 start code.
let es: Vec<u8> = vec![0x00, 0x00, 0x00, 0x06, 0x0F, 0x12, 0x34, 0x56, 0x78, 0x9A];
let shared = std::sync::Arc::new(std::sync::Mutex::new(Vec::<u8>::new()));
let sink = SharedSink(shared.clone());
let mut stream = M2tsStream::create(sink, &title).unwrap();
stream
.write(&PesFrame {
coding: None,
source: None,
track: 0,
pts: 0,
keyframe: true,
data: es.clone(),
duration_ns: None,
})
.unwrap();
stream.finish().unwrap();
drop(stream);
let buf = shared.lock().unwrap().clone();
assert!(
buf.windows(es.len()).any(|w| w == &es[..]),
"VC-1 ES must reach the output verbatim, not converted to Annex-B"
);
}
#[test] #[test]
fn m2ts_stream_forwards_keyframe_to_rai() { fn m2ts_stream_forwards_keyframe_to_rai() {
let title = make_title(); let title = make_title();
+99
View File
@@ -786,6 +786,105 @@ mod tests {
out out
} }
/// `set_nal_video(_, false)` must pass the ES through byte-for-byte: MPEG-2 and
/// VC-1 are not NAL-based, so their ES already IS the wire format and
/// `length_prefixed_to_annex_b` would mangle it.
///
/// The payload is deliberately length-prefix SHAPED (a big-endian length
/// followed by that many bytes) so the conversion, if wrongly applied, rewrites
/// the leading four bytes into a `00 00 00 01` start code. That makes the two
/// paths produce visibly different bytes; a payload the converter happened to
/// leave alone would let a mutant pass.
///
/// Mutation: delete the `set_nal_video` call, or flip the `nal_video` default,
/// and the emitted ES gains a start code -> this fails.
#[test]
fn non_nal_video_es_passes_through_unconverted() {
// 4-byte BE length (6) + 6 payload bytes: exactly what the Annex-B
// converter looks for, so a wrongly-applied conversion is unmissable.
let es: Vec<u8> = vec![0x00, 0x00, 0x00, 0x06, 0xB3, 0x12, 0x34, 0x56, 0x78, 0x9A];
let mut sink: Vec<u8> = Vec::new();
{
let mut mux = TsMuxer::new(&mut sink, &[VIDEO_PID]);
mux.set_nal_video(0, false).unwrap();
mux.write_frame(0, 0, true, &es).unwrap();
mux.finish().unwrap();
}
let packets = parse_bd_ts(&sink);
let out = reassemble_es(&packets, VIDEO_PID);
assert_eq!(
&out[..es.len()],
&es[..],
"non-NAL video ES must be emitted verbatim, start-code-free"
);
}
/// The default (`nal_video` = true) still converts, so the test above is
/// pinning the flag rather than a no-op. Same input, opposite expectation.
#[test]
fn nal_video_es_is_converted_to_annex_b_by_default() {
let es: Vec<u8> = vec![0x00, 0x00, 0x00, 0x06, 0xB3, 0x12, 0x34, 0x56, 0x78, 0x9A];
let mut sink: Vec<u8> = Vec::new();
{
let mut mux = TsMuxer::new(&mut sink, &[VIDEO_PID]);
// No set_nal_video call — the default must be the converting path.
mux.write_frame(0, 0, true, &es).unwrap();
mux.finish().unwrap();
}
let packets = parse_bd_ts(&sink);
let out = reassemble_es(&packets, VIDEO_PID);
assert_eq!(
&out[..4],
&[0x00, 0x00, 0x00, 0x01],
"the default path replaces the length prefix with an Annex-B start code"
);
assert_eq!(
&out[4..10],
&es[4..10],
"the NAL body itself is carried unchanged"
);
}
/// A non-NAL video track must still arm `params_written`, or every later
/// non-keyframe would fail the drop guard and silently vanish — the same class
/// of bug `empty_data_keyframe_arms_params_so_later_frames_survive` guards on
/// the NAL path.
#[test]
fn non_nal_video_keyframe_arms_params_so_later_frames_survive() {
let key: Vec<u8> = vec![0x00, 0x00, 0x01, 0xB3, 0xAA, 0xBB];
let non_key: Vec<u8> = vec![0x00, 0x00, 0x01, 0xB6, 0xCC, 0xDD];
let mut sink: Vec<u8> = Vec::new();
{
let mut mux = TsMuxer::new(&mut sink, &[VIDEO_PID]);
mux.set_nal_video(0, false).unwrap();
mux.write_frame(0, 0, true, &key).unwrap();
mux.write_frame(0, 41_000_000, false, &non_key).unwrap();
mux.finish().unwrap();
}
let packets = parse_bd_ts(&sink);
let out = reassemble_es(&packets, VIDEO_PID);
assert!(
out.windows(non_key.len()).any(|w| w == &non_key[..]),
"the non-keyframe following a non-NAL keyframe must not be dropped"
);
}
/// `set_nal_video` rejects an out-of-range track rather than panicking on the
/// index — this is library API and the crate must not panic from it.
#[test]
fn set_nal_video_out_of_range_track_errors() {
let mut sink: Vec<u8> = Vec::new();
let mut mux = TsMuxer::new(&mut sink, &[VIDEO_PID]);
assert!(
mux.set_nal_video(1, false).is_err(),
"track 1 does not exist on a one-track muxer"
);
assert!(mux.set_nal_video(0, false).is_ok(), "track 0 does exist");
}
#[test] #[test]
fn every_packet_is_exactly_192_bytes() { fn every_packet_is_exactly_192_bytes() {
// BD-TS packets are 192 bytes (4 TP_extra + 188 TS). The muxer must // BD-TS packets are 192 bytes (4 TP_extra + 188 TS). The muxer must