fabi/Sylpheed
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

formats: drop the 1.5 MB cap that truncated 17 voice regions' first stream

Browse Source 
The cause, and the fix, with a disc-wide check.

resolve_movie_voice_region picks start = the predecessor cue's trailer, then
filtered it with 'end - s < 1_500_000' -- 'only within one bank'. ADV's
predecessor sits 3 618 816 B before end, so the filter rejected it and start fell
back to anchor, which is a TOC offset and not a stream boundary. That explains
the shape of the defect exactly: it strikes regions larger than 1.5 MB, which is
why the three-stream multichannel regions are hit and single-stream ones never
are. 17 of 95 resolving movies took the fallback.

ADV's predecessor trailer at 433 425 776 plus 17 040 B of descriptor and padding
is 433 442 816 -- the -238-packet start measured against the decoder, to the byte.

Dropping the cap: unchanged 78, fixed cleanly 17, changed in any other way ZERO.
In all 17 the only difference is a larger first chunk with every later chunk
byte-identical, which is what a corrected start looks like and what pulling in a
neighbouring asset does not.

Regression test pinned to the RUNNING DECODER's byte_sizes rather than to this
crate's own output. That is the point of it: every internal check passed happily
while a third of a stream was missing, so only an external number could have
caught this class of bug.

sylpheed-formats: 136 tests pass, 0 fail (the one still running at commit time is
an unrelated long mesh test).

Exact clips for the other 16 are not independently verified -- the sweep is
strong but ADV is the only one with a decoder measurement behind it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wuu56cE8vJGTBtn1ppsk8v
This commit is contained in:
sylph-decoder
2026-08-30 08:55:47 +00:00
parent a29532a1e0
commit c8d3a6a15d
8 changed files with 351 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
crates/sylpheed-formats/examples/voice_region_cap_sweep.rs Normal file
View File

@@ -0,0 +1,71 @@
//! Is raising the start filter's 1.5 MB cap safe, disc-wide?
//!
//! `voice_region_fix_test.rs` shows that for `ADV` the predecessor start recovers
//! the decoder's own three byte_sizes exactly. But the cap exists to protect a
//! case: the code says *"only within one bank (~1.5 MB), else this is the first cue
//! in its block and the audio starts at the anchor itself"*. Raising it blindly
//! could pull a **previous asset's** streams into the region.
//!
//! So compare, per movie: the chunk list the resolver gives today against the one
//! the predecessor start gives. A safe change makes the FIRST chunk bigger and
//! leaves the rest identical. An unsafe one adds leading chunks.
//!
//! cargo run -p sylpheed-formats --example voice_region_cap_sweep
use sylpheed_formats::hash::name_hash;
use sylpheed_formats::media::{DirectorySource, DiscSource};
use sylpheed_formats::pak::PakArchive;
use sylpheed_formats::slb::{self, VoiceLang};
use sylpheed_formats::{movie_manifest, movie_voice};
fn main() {
let disc = std::env::var("SYLPHEED_DISC").expect("SYLPHEED_DISC");
let src = DirectorySource::new(std::path::PathBuf::from(&disc));
let code = VoiceLang::English.code_pub();
let tpak = src.open_pak("dat/tables.pak").expect("tables.pak");
let manifest = tpak.entries().iter()
.find_map(|e| tpak.read(e).ok().filter(|b| movie_manifest::is_manifest(b)))
.expect("manifest");
let marker = format!("{code}\\Movie\\VOICE_ADV.slb");
let registry = tpak.entries().iter().find_map(|e| {
tpak.read(e).ok().filter(|b| b.windows(marker.len()).any(|w| w == marker.as_bytes()))
}).expect("registry");
let ids = movie_voice::registry_voice_ids(&registry);
let stoc = src.read_file("dat/sound.pak").expect("sound.pak");
let entries = PakArchive::parse_toc(&stoc).expect("toc");
let (mut same, mut grew, mut extra, mut skip) = (0, 0, 0, 0);
for m in movie_manifest::parse(&manifest) {
let movie = m.movie;
let Some(token) = movie_manifest::voice_token(&manifest, &movie) else { skip += 1; continue };
let Some(&id) = ids.get(&token) else { skip += 1; continue };
let Some(anchor) = ["Movie","etc","Voice"].iter().find_map(|dir| {
let h = name_hash(&format!("{code}\\{dir}\\{token}.slb"));
entries.binary_search_by_key(&h, |e| e.name_hash).ok().map(|i| entries[i].offset as u64)
}) else { skip += 1; continue };
let win_start = anchor.saturating_sub(2*1024*1024) & !3;
let Ok(window) = src.read_segment_range("dat/sound", win_start, 8*1024*1024) else { skip += 1; continue };
let Some(end_local) = movie_voice::find_descriptor(&window, id) else { skip += 1; continue };
let end = win_start + end_local as u64;
let cand = movie_voice::find_descriptor(&window, id.wrapping_sub(1))
.or_else(|| movie_voice::find_descriptor_before(&window, end_local))
.map(|o| win_start + o as u64)
.filter(|&s| s < end);
let today = cand.filter(|&s| end - s < 1_500_000).unwrap_or(anchor);
let Some(proposed) = cand else { skip += 1; continue };
if today == proposed { same += 1; continue }
let sizes = |s: u64| -> Vec<usize> {
src.read_segment_range("dat/sound", s, (end - s) as usize)
.map(|b| slb::to_xma_riffs(&b).iter().map(|r| r.len() - 60).collect())
.unwrap_or_default()
};
let (a, b) = (sizes(today), sizes(proposed));
let tail_same = a.len() == b.len() && a.iter().skip(1).eq(b.iter().skip(1));
let verdict = if a.len() == b.len() && tail_same && b[0] > a[0] {
grew += 1; "first chunk GREW, tail identical"
} else if b.len() > a.len() { extra += 1; "EXTRA leading chunks" }
else { extra += 1; "changed otherwise" };
println!("{movie:10} today {a:?}\n{:10} prop {b:?} {verdict}", "");
}
println!("\nunchanged {same} fixed-cleanly {grew} would-break {extra} skipped {skip}");
}

40
crates/sylpheed-formats/examples/voice_region_fix_test.rs Normal file
View File

@@ -0,0 +1,40 @@
//! Would keeping the predecessor (instead of falling back to `anchor`) recover
//! the streams the running decoder actually decodes?
//!
//! `voice_region_start_why.rs` shows the failing branch: the start filter
//! `end - s < 1_500_000` rejects `ADV`'s predecessor because its span is 3.6 MB,
//! so `start` falls back to `anchor` — a TOC offset, not a stream boundary.
//!
//! This does NOT patch the resolver. It asks the one question that decides whether
//! raising that cap is the fix: **from the predecessor, does `to_xma_riffs` return
//! the decoder's own byte_sizes?** For `ADV` those are known, so this is a test and
//! not a fit.
//!
//! cargo run -p sylpheed-formats --example voice_region_fix_test
use sylpheed_formats::media::{DirectorySource, DiscSource};
use sylpheed_formats::slb;
const ADV_PRED: u64 = 433_425_776;
const ADV_ANCHOR: u64 = 433_930_240;
const ADV_END: u64 = 437_044_592;
/// What the running decoder reported.
const WANT: [usize; 3] = [1_294_336, 1_118_208, 1_171_456];
fn main() {
let disc = std::env::var("SYLPHEED_DISC").expect("SYLPHEED_DISC");
let src = DirectorySource::new(std::path::PathBuf::from(&disc));
for (label, start) in [("anchor (today)", ADV_ANCHOR), ("predecessor (proposed)", ADV_PRED)] {
let bytes = src
.read_segment_range("dat/sound", start, (ADV_END - start) as usize)
.expect("region");
let sizes: Vec<usize> = slb::to_xma_riffs(&bytes).iter().map(|r| r.len() - 60).collect();
let hit = sizes.len() == 3 && sizes.iter().zip(WANT.iter()).all(|(a, b)| a == b);
println!(
"{label:24} start {start} span {:>9} -> {:?}{}",
ADV_END - start,
sizes,
if hit { " <== MATCHES THE DECODER" } else { "" }
);
}
}

70
crates/sylpheed-formats/examples/voice_region_start_why.rs Normal file
View File

@@ -0,0 +1,70 @@
//! WHY does `resolve_movie_voice_region` start inside the first stream?
//!
//! [`voice-region-starts-late.md`] establishes that it does — 238 packets late for
//! `ADV`, 8 of 10 multichannel regions disc-wide — but not why, and a fix guessed
//! from one movie would be worse than a documented defect. This reproduces the
//! resolver's own steps and prints each candidate, so the failing branch is visible
//! rather than inferred.
//!
//! The suspicion the code itself raises: the start is filtered by
//! `end - s < 1_500_000` — "only within one bank" — and `ADV`'s region has to span
//! **3.6 MB**. If that filter rejects the real predecessor, `start` silently falls
//! back to `anchor`, which is a TOC offset and not a stream boundary at all.
//!
//! cargo run -p sylpheed-formats --example voice_region_start_why
use sylpheed_formats::hash::name_hash;
use sylpheed_formats::media::{DirectorySource, DiscSource};
use sylpheed_formats::pak::PakArchive;
use sylpheed_formats::slb::VoiceLang;
use sylpheed_formats::{movie_manifest, movie_voice};
fn main() {
let disc = std::env::var("SYLPHEED_DISC").expect("SYLPHEED_DISC");
let src = DirectorySource::new(std::path::PathBuf::from(&disc));
let code = VoiceLang::English.code_pub();
let tpak = src.open_pak("dat/tables.pak").expect("tables.pak");
let manifest = tpak
.entries().iter()
.find_map(|e| tpak.read(e).ok().filter(|b| movie_manifest::is_manifest(b)))
.expect("manifest");
let marker = format!("{code}\\Movie\\VOICE_ADV.slb");
let registry = tpak.entries().iter().find_map(|e| {
tpak.read(e).ok().filter(|b| b.windows(marker.len()).any(|w| w == marker.as_bytes()))
}).expect("registry");
let ids = movie_voice::registry_voice_ids(&registry);
let stoc = src.read_file("dat/sound.pak").expect("sound.pak");
let entries = PakArchive::parse_toc(&stoc).expect("toc");
println!("{:10} {:>6} {:>12} {:>12} {:>12} {:>10} {:>9} {}",
"movie","id","anchor","pred(id-1)","pred(before)","span","chosen","note");
for m in movie_manifest::parse(&manifest) {
let movie = m.movie;
let Some(token) = movie_manifest::voice_token(&manifest, &movie) else { continue };
let Some(&id) = ids.get(&token) else { continue };
let Some(anchor) = ["Movie","etc","Voice"].iter().find_map(|dir| {
let h = name_hash(&format!("{code}\\{dir}\\{token}.slb"));
entries.binary_search_by_key(&h, |e| e.name_hash).ok().map(|i| entries[i].offset as u64)
}) else { continue };
let win_start = anchor.saturating_sub(2*1024*1024) & !3;
let Ok(window) = src.read_segment_range("dat/sound", win_start, 8*1024*1024) else { continue };
let Some(end_local) = movie_voice::find_descriptor(&window, id) else { continue };
let end = win_start + end_local as u64;
let p1 = movie_voice::find_descriptor(&window, id.wrapping_sub(1)).map(|o| win_start + o as u64);
let pb = movie_voice::find_descriptor_before(&window, end_local).map(|o| win_start + o as u64);
let cand = p1.or(pb);
// the resolver's own filter
let kept = cand.filter(|&s| s < end && end - s < 1_500_000);
let chosen = kept.unwrap_or(anchor);
let span = cand.map(|s| end.saturating_sub(s)).unwrap_or(0);
let note = match (cand, kept) {
(Some(_), None) => "REJECTED by the 1.5 MB filter -> fell back to anchor",
(Some(_), Some(_)) => "predecessor kept",
(None, _) => "no predecessor found -> anchor",
};
println!("{movie:10} {id:>6} {anchor:>12} {:>12} {:>12} {span:>10} {:>9} {note}",
p1.map(|v| v.to_string()).unwrap_or("-".into()),
pb.map(|v| v.to_string()).unwrap_or("-".into()),
if chosen == anchor { "anchor" } else { "pred" });
}
}

23
crates/sylpheed-formats/src/media.rs
View File

@@ -288,12 +288,29 @@ pub fn resolve_movie_voice_region<S: DiscSource + ?Sized>(
// Start = the predecessor trailer. Prefer the exact `id-1`; where the id
// sequence has a gap (VOICE_D_453 → 454) fall back to the nearest trailer
// below — but only within one bank (~1.5 MB), else this is the first cue in
// its block and the audio starts at the anchor itself.
// below.
//
// 🔴 There used to be a second condition here — `end - s < 1_500_000`, "only
// within one bank, else this is the first cue in its block and the audio
// starts at the anchor itself". **It was wrong, and it silently truncated the
// first stream of every region larger than 1.5 MB.** `anchor` is a TOC offset,
// not a stream boundary, so the fallback started mid-packet-run: `ADV` began
// **238 packets (487 424 B) into its own first stream**, and a consumer then
// saw a leading chunk that "matched nothing" and dropped 62 % of a real stream.
//
// Ground truth is the running decoder, which reports `ADV`'s three contexts as
// 1 294 336 / 1 118 208 / 1 171 456 (`--xma_param_probe`). With the cap gone the
// region reproduces all three exactly; with it, the first is 806 912.
//
// Disc-wide over the 95 manifest movies that resolve: **17 regions fixed, 78
// unchanged, 0 changed in any other way** — in every one of the 17 the first
// chunk grows and the remaining chunks are byte-identical, which is what a
// corrected start looks like and what pulling in a neighbouring asset does not.
// `docs/re/structures/voice-region-starts-late.md`.
let start = movie_voice::find_descriptor(&window, id.wrapping_sub(1))
.or_else(|| movie_voice::find_descriptor_before(&window, end_local))
.map(|o| win_start + o as u64)
.filter(|&s| s < end && end - s < 1_500_000)
.filter(|&s| s < end)
.unwrap_or(anchor);
Some((start, end))
}

36
crates/sylpheed-formats/tests/media_disc.rs
View File

@@ -86,3 +86,39 @@ fn manifest_binding_is_the_only_route() {
None
);
}
/// The resolved `ADV` voice region must contain **all three** streams the running
/// decoder decodes — not a truncated first one.
///
/// Ground truth is the emulator, not this crate: booting with `--xma_param_probe`
/// reports three XMA contexts with `byte_size` 1 294 336 / 1 118 208 / 1 171 456
/// (`docs/re/structures/voice-three-streams-are-concurrent.md`). Until 2026-08-30
/// the resolver's start filter capped a region at 1.5 MB, `ADV`'s span is 3.6 MB,
/// so the start fell back to `anchor` — a TOC offset, 238 packets into the first
/// stream — and this returned 806 912 for the first chunk.
///
/// This is a regression test against an EXTERNAL measurement, which is the only
/// kind that can catch the class of bug it was written for: every internal check
/// passed happily while a third of a stream was missing.
#[test]
fn adv_voice_region_holds_all_three_decoded_streams() {
let Some(src) = disc() else {
eprintln!("SKIP: set SYLPHEED_DISC");
return;
};
let (start, end) = media::resolve_movie_voice_region(&src, "ADV", VoiceLang::English)
.expect("ADV voice region");
let bytes = src
.read_segment_range("dat/sound", start, (end - start) as usize)
.expect("region bytes");
let sizes: Vec<usize> = sylpheed_formats::slb::to_xma_riffs(&bytes)
.iter()
.map(|r| r.len() - 60)
.collect();
assert_eq!(
sizes,
vec![1_294_336, 1_118_208, 1_171_456],
"the region must reproduce the RUNNING DECODER's byte_sizes; \
a first chunk of 806912 means the start filter has come back"
);
}