dvd: warn about titles split across PGCs once per disc, not per title per scan

62d56df warned from parse_pgcit for every split title on every scan; a CLI rip scans its disc
three times, so a DVD with four split titles logged the same warning twelve times. The scan
now logs one line per disc naming every split title, and a rescan of that disc logs it at
debug.
This commit is contained in:
Matthew Jackson
2026-10-01 22:03:30 -07:00
parent 8e317afc16
commit 07a7ea8aad
2 changed files with 112 additions and 8 deletions
+102
View File
@@ -256,10 +256,51 @@ impl Disc {
if halt.is_some_and(|h| h.is_cancelled()) { if halt.is_some_and(|h| h.is_cancelled()) {
return Err(Error::Halted); return Err(Error::Halted);
} }
warn_multi_pgc_titles(&vmg_bytes, &dvd_info);
Ok((titles, nav_feature)) Ok((titles, nav_feature))
} }
} }
// One warning per disc per process for titles split across PGCs (only the first is read):
// a rip scans its disc more than once, and each scan would repeat it. Later scans log at
// debug. The disc is told apart by its VIDEO_TS.IFO.
fn warn_multi_pgc_titles(vmg: &[u8], info: &ifo::DvdInfo) {
use std::hash::{Hash, Hasher};
static WARNED: std::sync::Mutex<Vec<u64>> = std::sync::Mutex::new(Vec::new());
let split: Vec<String> = info
.title_sets
.iter()
.flat_map(|ts| {
ts.titles.iter().filter(|t| t.pgcs > 1).map(move |t| {
format!(
"VTS {} title {} ({} PGCs)",
ts.vts_number, t.vts_title_num, t.pgcs
)
})
})
.collect();
if split.is_empty() {
return;
}
let mut h = std::hash::DefaultHasher::new();
vmg.hash(&mut h);
let key = h.finish();
let first = {
let mut warned = WARNED.lock().unwrap_or_else(|e| e.into_inner());
let first = !warned.contains(&key);
if first {
warned.push(key);
}
first
};
let titles = split.join(", ");
if first {
tracing::warn!(target: "freemkv::scan", titles = %titles, "titles span several PGCs; only the first PGC of each is read, the rest of those titles is missing");
} else {
tracing::debug!(target: "freemkv::scan", titles = %titles, "titles span several PGCs; only the first PGC of each is read");
}
}
// This title's audio tracks: logical stream i plays the physical stream its PGC_AST_CTL // This title's audio tracks: logical stream i plays the physical stream its PGC_AST_CTL
// names (libdvdnav semantics), absent streams get no track, a repeated physical id keeps // names (libdvdnav semantics), absent streams get no track, a repeated physical id keeps
// the first. A PGC marking nothing present keeps the positional ids rather than go silent. // the first. A PGC marking nothing present keeps the positional ids rather than go silent.
@@ -2184,4 +2225,65 @@ mod tests {
fn scan_dvd_titles_bad_ifo_and_bup_is_empty() { fn scan_dvd_titles_bad_ifo_and_bup_is_empty() {
assert_eq!(scan_ifo_bup(false, false).expect("no error").len(), 0); assert_eq!(scan_ifo_bup(false, false).expect("no error").len(), 0);
} }
fn title(vts_title_num: u8, pgcs: usize) -> ifo::DvdTitle {
ifo::DvdTitle {
chapters: 1,
duration_secs: 1.0,
cells: Vec::new(),
chapter_times: Vec::new(),
palette: None,
ast_ctl: [0; 8],
spst_ctl: [0; 32],
vts_title_num,
pgcs,
}
}
// A rip scans its disc more than once: the split-title warning is one line per disc,
// naming every split title, and a rescan of the same disc logs it at debug only.
#[test]
fn multi_pgc_titles_warn_once_per_disc() {
let set = |n: u8, titles: Vec<ifo::DvdTitle>| ifo::DvdTitleSet {
vts_number: n,
vob_start_sector: 0,
video: ifo::DvdVideoAttr {
codec: Codec::Mpeg2,
resolution: Resolution::R480i,
aspect: ifo::DvdAspect::R16x9,
standard: ifo::TvSystem::Ntsc,
},
audio_streams: Vec::new(),
subtitle_streams: Vec::new(),
titles,
};
let info = ifo::DvdInfo {
title_sets: vec![
set(3, vec![title(1, 8)]),
set(4, vec![title(1, 5), title(2, 1)]),
set(11, vec![title(1, 1)]),
],
};
let vmg = b"multi_pgc_titles_warn_once_per_disc".to_vec();
let ((), ev) = crate::testlog::capture(|| {
warn_multi_pgc_titles(&vmg, &info);
warn_multi_pgc_titles(&vmg, &info);
warn_multi_pgc_titles(&vmg, &info);
});
let warns: Vec<_> = ev
.iter()
.filter(|e| e.level == tracing::Level::WARN)
.collect();
assert_eq!(warns.len(), 1, "{ev:?}");
assert_eq!(
warns[0].field("titles"),
Some("VTS 3 title 1 (8 PGCs), VTS 4 title 1 (5 PGCs)")
);
// A disc with no split title logs nothing.
let clear = ifo::DvdInfo {
title_sets: vec![set(1, vec![title(1, 1)])],
};
let ((), ev) = crate::testlog::capture(|| warn_multi_pgc_titles(b"no split", &clear));
assert!(ev.is_empty(), "{ev:?}");
}
} }
+10 -8
View File
@@ -60,6 +60,9 @@ pub struct DvdTitle {
/// resolution joins on the REAL title number rather than the position in the /// resolution joins on the REAL title number rather than the position in the
/// titles vec, which desyncs when a sibling PGC is dropped as unparseable. /// titles vec, which desyncs when a sibling PGC is dropped as unparseable.
pub vts_title_num: u8, pub vts_title_num: u8,
/// Distinct PGCs this title's part-of-title entries point at. Only the first is read,
/// so a title with more is missing the rest.
pub pgcs: usize,
} }
/// A cell — contiguous sector range within a VOB. /// A cell — contiguous sector range within a VOB.
@@ -904,15 +907,8 @@ pub(crate) fn parse_pgcit(
continue; continue;
} }
// Warned once per disc by the DVD scan, which sees every title set.
let pgcs = ptt_srpt_pgc_count(data, ptt_offset, vts_title_num); let pgcs = ptt_srpt_pgc_count(data, ptt_offset, vts_title_num);
if pgcs > 1 {
tracing::warn!(
target: "freemkv::scan",
vts_title_num,
pgcs,
"title spans several PGCs; only its first PGC is read, the rest of the title is missing"
);
}
let entry_offset = entries_start + pgc_index * 8; let entry_offset = entries_start + pgc_index * 8;
if entry_offset + 8 > data.len() { if entry_offset + 8 > data.len() {
@@ -938,6 +934,7 @@ pub(crate) fn parse_pgcit(
// Stamp the REAL title number so downstream nav joins survive a // Stamp the REAL title number so downstream nav joins survive a
// dropped sibling PGC (position in `titles` is not vts_title_num). // dropped sibling PGC (position in `titles` is not vts_title_num).
title.vts_title_num = vts_title_num; title.vts_title_num = vts_title_num;
title.pgcs = pgcs;
titles.push(title); titles.push(title);
} }
Err(e) => { Err(e) => {
@@ -1118,6 +1115,7 @@ pub(crate) fn parse_pgc(data: &[u8], pgc_offset: usize, chapters: u16) -> Result
spst_ctl, spst_ctl,
// Set by the caller (parse_pgcit) which knows the TT_SRPT title number. // Set by the caller (parse_pgcit) which knows the TT_SRPT title number.
vts_title_num: 0, vts_title_num: 0,
pgcs: 0,
}) })
} }
@@ -2071,6 +2069,7 @@ mod tests {
ast_ctl: [0; 8], ast_ctl: [0; 8],
spst_ctl: [0; 32], spst_ctl: [0; 32],
vts_title_num: 0, vts_title_num: 0,
pgcs: 1,
}; };
assert_eq!(t.feature_start_cell(), 0); assert_eq!(t.feature_start_cell(), 0);
assert_eq!(t.feature_cells().len(), 3); assert_eq!(t.feature_cells().len(), 3);
@@ -2095,6 +2094,7 @@ mod tests {
ast_ctl: [0; 8], ast_ctl: [0; 8],
spst_ctl: [0; 32], spst_ctl: [0; 32],
vts_title_num: 0, vts_title_num: 0,
pgcs: 1,
}; };
assert_eq!(t.feature_start_cell(), 2); assert_eq!(t.feature_start_cell(), 2);
let fc = t.feature_cells(); let fc = t.feature_cells();
@@ -2115,6 +2115,7 @@ mod tests {
ast_ctl: [0; 8], ast_ctl: [0; 8],
spst_ctl: [0; 32], spst_ctl: [0; 32],
vts_title_num: 0, vts_title_num: 0,
pgcs: 1,
}; };
assert_eq!(t.feature_start_cell(), 0); assert_eq!(t.feature_start_cell(), 0);
assert_eq!(t.feature_cells().len(), 2); assert_eq!(t.feature_cells().len(), 2);
@@ -2132,6 +2133,7 @@ mod tests {
ast_ctl: [0; 8], ast_ctl: [0; 8],
spst_ctl: [0; 32], spst_ctl: [0; 32],
vts_title_num: 0, vts_title_num: 0,
pgcs: 1,
}; };
assert_eq!(t.feature_start_cell(), 0); assert_eq!(t.feature_start_cell(), 0);
assert!(t.feature_cells().is_empty()); assert!(t.feature_cells().is_empty());