Guard the invariant that provenance is universal
Every emitted frame must carry the source byte offset of the packet it came from. That held only for video for as long as it existed, and nothing asserted it, so nothing caught it: ten parsers built frames with `source: None` and a multi-clip title could not place audio or subtitles by byte at all. Finding them took a brace-balanced scan of the tree by hand — a regex cannot do it, because a Frame literal contains nested braces and a non-greedy match stops at the first `}`, which is how five sites survived the first pass. This is that scan, as a test. A second test fails if a codec module is added and not listed, since an unchecked parser is exactly how the gap persists. Modules that emit no Frame are named explicitly rather than skipped silently. The guard tripped on itself twice while being written — first on its own doc comment, then on its own string literals — so it strips line comments and excludes its own module. A check that matches prose about the defect rather than the defect is the same mistake in a different place.
This commit is contained in:
@@ -359,3 +359,153 @@ mod tests {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod provenance_guard {
|
||||
//! Every emitted frame must carry the source byte offset of the packet it
|
||||
//! came from. That invariant held only for video for as long as it existed:
|
||||
//! `dts`, `ac3`, `adts`, `truehd`, `pgs`, `dvdsub`, `flac`, `lpcm`,
|
||||
//! `mpegaudio` and the passthrough parser all built frames with
|
||||
//! `source: None`, so a multi-clip title could not place audio or subtitles
|
||||
//! by byte and fell back to inferring the clip from timestamps — which is
|
||||
//! what made branched titles run minutes long.
|
||||
//!
|
||||
//! Nothing asserted it, so nothing caught it. Finding it took a
|
||||
//! brace-balanced scan of the tree by hand, which also turned up five sites
|
||||
//! a regex had missed. This is that scan, as a test.
|
||||
|
||||
/// Every parser module's source, checked for a `Frame` built without a
|
||||
/// source. Paired with `every_codec_module_is_covered` below, which fails
|
||||
/// if a module is added and not listed here.
|
||||
const PARSER_SOURCES: &[(&str, &str)] = &[
|
||||
("mod.rs", include_str!("mod.rs")),
|
||||
("ac3.rs", include_str!("ac3.rs")),
|
||||
("adts.rs", include_str!("adts.rs")),
|
||||
("dts.rs", include_str!("dts.rs")),
|
||||
("dvdsub.rs", include_str!("dvdsub.rs")),
|
||||
("flac.rs", include_str!("flac.rs")),
|
||||
("h264.rs", include_str!("h264.rs")),
|
||||
("hevc.rs", include_str!("hevc.rs")),
|
||||
("lpcm.rs", include_str!("lpcm.rs")),
|
||||
("mpeg2.rs", include_str!("mpeg2.rs")),
|
||||
("mpegaudio.rs", include_str!("mpegaudio.rs")),
|
||||
("pgs.rs", include_str!("pgs.rs")),
|
||||
("truehd.rs", include_str!("truehd.rs")),
|
||||
("vc1.rs", include_str!("vc1.rs")),
|
||||
];
|
||||
|
||||
/// Modules that declare no `Frame` and so need no entry above.
|
||||
const NON_PARSER_MODULES: &[&str] = &[
|
||||
"coding",
|
||||
"crc",
|
||||
"dropgate",
|
||||
"pesbuf",
|
||||
"reorder",
|
||||
"startcode",
|
||||
];
|
||||
|
||||
/// Walk `Frame { .. }` literals with balanced braces. A regex cannot do
|
||||
/// this — `Frame` blocks contain nested braces (`coding: Some(..)`, closures)
|
||||
/// and a non-greedy match stops at the first `}`, which is how five sites
|
||||
/// survived the first pass.
|
||||
/// Strip line comments before scanning. A comment that MENTIONS
|
||||
/// `source: None` is prose, not a construction — this guard's own doc
|
||||
/// comment tripped it on the first run, which is the same mistake as
|
||||
/// grepping a file and matching its commentary.
|
||||
fn code_only(src: &str) -> String {
|
||||
// The guard itself constructs no Frame; it only talks about them, in
|
||||
// prose and in string literals. Scanning its own body matches both.
|
||||
let src = match src.find("mod provenance_guard") {
|
||||
Some(i) => &src[..i],
|
||||
None => src,
|
||||
};
|
||||
src.lines()
|
||||
.map(|l| match l.find("//") {
|
||||
// Not inside a string literal: no quote before the slashes.
|
||||
Some(i) if !l[..i].contains('"') => &l[..i],
|
||||
_ => l,
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
}
|
||||
|
||||
fn frame_literals(src: &str) -> Vec<&str> {
|
||||
let mut out = Vec::new();
|
||||
let b = src.as_bytes();
|
||||
let mut i = 0;
|
||||
while let Some(p) = src[i..].find("Frame {") {
|
||||
let start = i + p;
|
||||
let open = start + src[start..].find('{').unwrap();
|
||||
let (mut depth, mut k) = (0usize, open);
|
||||
while k < b.len() {
|
||||
match b[k] {
|
||||
b'{' => depth += 1,
|
||||
b'}' => {
|
||||
depth -= 1;
|
||||
if depth == 0 {
|
||||
break;
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
k += 1;
|
||||
}
|
||||
out.push(&src[start..(k + 1).min(src.len())]);
|
||||
i = k + 1;
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_parser_emits_a_frame_without_provenance() {
|
||||
let mut offenders = Vec::new();
|
||||
for (name, src) in PARSER_SOURCES {
|
||||
let src = &code_only(src);
|
||||
for blk in frame_literals(src) {
|
||||
// `PesFrame` in the tests of other modules is not ours; only
|
||||
// codec `Frame` literals are scanned, and a test fixture that
|
||||
// builds a PesPacket with `source: None` is legitimate.
|
||||
if blk.contains("source: None") {
|
||||
let line = src[..src.find(blk).unwrap_or(0)].lines().count() + 1;
|
||||
offenders.push(format!("{name}:{line}"));
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
offenders.is_empty(),
|
||||
"these parsers build a Frame with no source byte offset, so a \
|
||||
multi-clip title cannot place their tracks by provenance and \
|
||||
falls back to inferring the clip from timestamps: {offenders:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// A parser added without an entry in `PARSER_SOURCES` would be silently
|
||||
/// unchecked, which is exactly how this class of gap persists.
|
||||
#[test]
|
||||
fn every_codec_module_is_covered() {
|
||||
let modules: Vec<&str> = include_str!("mod.rs")
|
||||
.lines()
|
||||
.filter_map(|l| {
|
||||
let l = l.trim();
|
||||
let rest = l
|
||||
.strip_prefix("pub mod ")
|
||||
.or_else(|| l.strip_prefix("pub(crate) mod "))?;
|
||||
rest.strip_suffix(';')
|
||||
})
|
||||
.collect();
|
||||
let listed: Vec<&str> = PARSER_SOURCES
|
||||
.iter()
|
||||
.map(|(n, _)| n.trim_end_matches(".rs"))
|
||||
.collect();
|
||||
let unchecked: Vec<&&str> = modules
|
||||
.iter()
|
||||
.filter(|m| !listed.contains(m) && !NON_PARSER_MODULES.contains(m))
|
||||
.collect();
|
||||
assert!(
|
||||
unchecked.is_empty(),
|
||||
"codec module(s) not covered by the provenance guard — add them to \
|
||||
PARSER_SOURCES (or to NON_PARSER_MODULES if they emit no Frame): \
|
||||
{unchecked:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user