Kill codec mutation survivors and unify H.264's duplicated escape stripper
The mux/codec parsers (startcode, h264, hevc, dts) had 300 surviving mutants between them, and it turned out to be for the reason you'd fear: the exp-Golomb readers and the AU-boundary bitstream scanners had essentially no direct unit coverage, only indirect exercise through full-frame parse() calls that never touched the actual edge cases. Direct fixes to test gaps: - The shared BitReader's read_ue truncation guard (`leading_zeros > 31`) and skip_start_code's 4-byte-vs-3-byte boundary check had no test at their exact boundary. Added tests that hit the boundary precisely; a `>=`/`==`/`<=` typo either rejects a legal 31-leading- zero code or reads one byte past the buffer. - H.264's private SpsReader duplicates the same read_bits/read_ue shapes with no tests of its own at all (only reached through multi-field SPS parsing, several fields deep). Added direct tests. - HEVC's per-AU trailing-zero strip after the last NAL (no start code following) walks `end` down to trim padding; a wrong-direction typo there walks off the end of the buffer instead of terminating - exactly the "loop must make positive progress on malformed input" class. Added a test with a zero-padded trailing NAL. - HEVC's SEI match guards (`sei_mastering.is_none()` / `sei_content_light.is_none()`) implement "first HDR10 value in the title wins" - untested, and a naive test using both-messages-per-AU can't even exercise the guards because the whole-scan early return above them already handles that case. Split into single-message- per-AU tests that actually reach the arms. - parse_mastering_display/parse_content_light_level's length guards were `< N` with no boundary test; one-byte-short input now confirmed to return None instead of indexing out of bounds. - DTS's drain_front collapses duplicate offset-0 PTS markers after rebasing; untested, and the visible effect (front_pts()) can't tell a working collapse from a broken one since it already returns the right marker either way - the actual defect is unbounded growth of pts_marks over a long recording, so the new test asserts the bound directly across repeated drains. - DTS's dts_core_samples/dts_core_sample_rate header-length guard and next_core_boundary's syncword-length guard got exact-boundary tests the same way; also caught a nblks `<<`/`>>` direction bug candidate in the mutant (confirmed the real code is correct, just untested). Real bug found and fixed, not just a test gap: H.264's parse_sps_high_profile_ext re-implemented emulation-prevention byte stripping inline (a window scan: match `00 00 03` at position i, advance 3, else advance 1) instead of calling the existing unescape_ebsp_prefix used by slice-header parsing. On a run of 3+ real zero bytes ahead of an 0x03 - non-conformant, but this is disc bytes, not a spec-clean encoder - the two disagreed: unescape_ebsp_prefix's cumulative zero counter (matching the H.264 reference decode process and libavcodec's RBSP extractor) treats it as an escape and drops the 0x03; the window scan treats it as real payload and keeps it, corrupting the SPS bits read after it. Extracted the shared rule into `unescape_ebsp` (parameterized on output length so both the 16-byte slice-header prefix and the unbounded SPS case can share it) and pointed both call sites at the one implementation. Added a regression test pinning the shared function's behaviour on the input that used to separate them. All new tests hand-verified against the actual mutation (operator flipped or guard replaced by hand, confirmed red, then restored) per the mutation-testing brief, not just written and trusted.
This commit is contained in:
+70
-19
@@ -141,10 +141,29 @@ thread_local! {
|
||||
/// off the per-frame allocation path for anything larger.
|
||||
fn unescape_ebsp_prefix(ebsp: &[u8]) -> Vec<u8> {
|
||||
const PREFIX_OCTETS: usize = 16;
|
||||
let mut out = Vec::with_capacity(PREFIX_OCTETS);
|
||||
unescape_ebsp(ebsp, PREFIX_OCTETS)
|
||||
}
|
||||
|
||||
/// Copy `ebsp` with emulation-prevention bytes removed, stopping after at
|
||||
/// most `max_octets` OUTPUT bytes (pass `ebsp.len()` for "no cap").
|
||||
///
|
||||
/// The zero run-length counter is cumulative across the whole scan and is
|
||||
/// reset only when an escape byte is actually dropped — matching the
|
||||
/// reference decoding process (ITU-T H.264 §7.3.1 / the libavcodec RBSP
|
||||
/// extractor), which discards a 0x03 following ANY run of two-or-more 0x00
|
||||
/// bytes, not just an exact `00 00 03` sliding window. A window-based scanner
|
||||
/// that advances by 3 on a match and by 1 otherwise disagrees with this on a
|
||||
/// run of 3+ real zero bytes ahead of an 0x03: `00 00 00 03` decodes here as
|
||||
/// `00 00 00` (the run is escaped, matching every other H.264 decoder), where
|
||||
/// a window scan starting fresh after each non-match would keep the 0x03 as
|
||||
/// real payload. Only one implementation of this rule may exist in this
|
||||
/// module — see `parse_sps_high_profile_ext`, which used to keep its own
|
||||
/// window-scan copy that disagreed with this one on exactly that input.
|
||||
fn unescape_ebsp(ebsp: &[u8], max_octets: usize) -> Vec<u8> {
|
||||
let mut out = Vec::with_capacity(max_octets.min(ebsp.len()));
|
||||
let mut zeros = 0usize;
|
||||
for &b in ebsp {
|
||||
if out.len() == PREFIX_OCTETS {
|
||||
if out.len() == max_octets {
|
||||
break;
|
||||
}
|
||||
// Drop the escape octet itself, but only in the 00 00 03 position.
|
||||
@@ -493,23 +512,12 @@ impl CodecParser for H264Parser {
|
||||
/// omits the extension in that case.
|
||||
fn parse_sps_high_profile_ext(sps: &[u8]) -> Option<(u8, u8, u8)> {
|
||||
// Strip emulation-prevention bytes: 00 00 03 xx → 00 00 xx (drop the 03).
|
||||
// We skip byte 0 (NAL header) and start the RBSP from byte 1.
|
||||
let rbsp: Vec<u8> = {
|
||||
let raw = &sps[1..]; // skip NAL header byte
|
||||
let mut out = Vec::with_capacity(raw.len());
|
||||
let mut i = 0;
|
||||
while i < raw.len() {
|
||||
if i + 2 < raw.len() && raw[i] == 0x00 && raw[i + 1] == 0x00 && raw[i + 2] == 0x03 {
|
||||
out.push(0x00);
|
||||
out.push(0x00);
|
||||
i += 3; // skip the 0x03 emulation-prevention byte
|
||||
} else {
|
||||
out.push(raw[i]);
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
out
|
||||
};
|
||||
// We skip byte 0 (NAL header) and start the RBSP from byte 1. Shares
|
||||
// `unescape_ebsp` with the slice-header prefix reader rather than
|
||||
// re-deriving the same rule — see that function's doc comment for why a
|
||||
// second, window-based copy of this rule used to disagree with it.
|
||||
let raw = &sps[1..]; // skip NAL header byte
|
||||
let rbsp: Vec<u8> = unescape_ebsp(raw, raw.len());
|
||||
|
||||
// RBSP layout after stripping the NAL header byte:
|
||||
// [0] profile_idc (already checked by caller)
|
||||
@@ -747,6 +755,26 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression: `parse_sps_high_profile_ext` used to re-derive the
|
||||
/// emulation-prevention rule with its own window scanner (match `00 00
|
||||
/// 03` at position i, advance by 3; else advance by 1) instead of calling
|
||||
/// the shared `unescape_ebsp`. On a run of 3+ real 0x00 bytes ahead of an
|
||||
/// 0x03 — non-conformant, but this is untrusted disc input, not a
|
||||
/// spec-clean encoder — the two disagreed: `unescape_ebsp`'s cumulative
|
||||
/// zero counter (the same rule libavcodec's RBSP extractor and the H.264
|
||||
/// reference decoding process use) drops the 0x03 as an escape, while a
|
||||
/// fresh 3-byte window starting right after the non-matching first byte
|
||||
/// kept it as real payload. Pin the shared function's behaviour here so a
|
||||
/// second hand-rolled copy doesn't quietly reappear.
|
||||
#[test]
|
||||
fn unescape_ebsp_drops_escape_after_a_run_of_three_zeros() {
|
||||
assert_eq!(
|
||||
super::unescape_ebsp(&[0x00, 0x00, 0x00, 0x03, 0x42], 5),
|
||||
vec![0x00, 0x00, 0x00, 0x42],
|
||||
"the 0x03 after a 3-zero run is an escape byte, not payload"
|
||||
);
|
||||
}
|
||||
|
||||
// --- keyframe parameter-set re-assert: exact bytes + no whole-frame copy ---
|
||||
|
||||
/// The keyframe SPS/PPS re-assert must produce EXACTLY these bytes: the
|
||||
@@ -2011,4 +2039,27 @@ mod tests {
|
||||
expected_len
|
||||
);
|
||||
}
|
||||
|
||||
/// `SpsReader::read_bits` shifts each new bit into the low end of the
|
||||
/// accumulator (`val << 1 | bit`). Pins the direction directly: a
|
||||
/// `<<` -> `>>` typo would leave every accumulated bit shifted out and
|
||||
/// the result would collapse towards 0 instead of building up the value.
|
||||
#[test]
|
||||
fn sps_reader_read_bits_builds_value_msb_first() {
|
||||
// 0b1011_0000 read 4 bits MSB-first -> 0b1011 = 11.
|
||||
let mut r = super::SpsReader::new(&[0b1011_0000]);
|
||||
assert_eq!(r.read_bits(4), Some(0b1011));
|
||||
}
|
||||
|
||||
/// `SpsReader::read_ue`'s truncation guard is `leading_zeros > 31`: 31
|
||||
/// leading zero bits is the longest legal code and must decode, not
|
||||
/// abort. Mirrors the equivalent guard in the shared `BitReader` in
|
||||
/// `startcode.rs`, kept here because `SpsReader` is a separate,
|
||||
/// unshared implementation used only for SPS parsing.
|
||||
#[test]
|
||||
fn sps_reader_read_ue_thirty_one_leading_zeros_is_still_valid() {
|
||||
let data = [0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00];
|
||||
let mut r = super::SpsReader::new(&data);
|
||||
assert_eq!(r.read_ue(), Some(u32::MAX >> 1));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user