From ff83918572d2950818cc465c2b962ae439fa635e Mon Sep 17 00:00:00 2001 From: Sylpheed RE agent Date: Wed, 26 Aug 2026 02:55:26 +0000 Subject: [PATCH] =?UTF-8?q?re:=20wire=20the=20IXUD=20record=20table=20into?= =?UTF-8?q?=20the=20crate=20=E2=80=94=20captions=20go=20537=20to=208800=20?= =?UTF-8?q?of=208800?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ixud.rs now has an IdxdObject-shaped reader, IxudObject, and build_caption_text reads captions as FIELDS instead of pairing them with whatever token follows in the pool. build_demo_text token adjacency 134 ids 537 lines build_caption_text token adjacency 3721 8074 build_caption_text record fields 4085 8800 = all of them Verified over the whole disc by tests/ixud_records_disc.rs: 1104/1104 objects parse, 1476/1476 records and 628165/628165 named fields reproduce their ixud_hash, 48 positional, zero failures. The header word at 0x08 is record 0's hash, asserted per object -- there is no schema field, exactly as for IDXD. The module doc described a 12-byte record directory and a "schema/type hash"; both were wrong and are corrected. I also have to correct my own number from the previous commit. "1.3% of the game's text" counted OCCURRENCES: each family lives in 24-45 IXUD blocks and the same key repeats across them. Distinct text-bearing MSG_* keys number 8800, not 44579, and every one has the __ shape. So the real coverage was 537/8800 = 6.1%, and I overstated the gap about fivefold. Direction right, magnitude wrong. The DEMO control is the sharpest evidence for the change: token adjacency finds 537 lines there, the field reader 541. It was dropping lines even in the one family it was written for -- which is why the test now asserts "must not lose lines" rather than "must be identical". Same lesson twice in one session: pool adjacency is a consequence of how records are written, not a rule of the format. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PMRJjbxLqZtsb5Vb7KunPE --- crates/sylpheed-formats/src/ixud.rs | 170 +++++++++++++++++- crates/sylpheed-formats/src/lib.rs | 2 +- crates/sylpheed-formats/src/movie_subtitle.rs | 29 +-- .../tests/caption_families_disc.rs | 24 ++- .../tests/ixud_records_disc.rs | 95 ++++++++++ docs/re/structures/movie-subtitles.md | 51 +++--- 6 files changed, 320 insertions(+), 51 deletions(-) create mode 100644 crates/sylpheed-formats/tests/ixud_records_disc.rs diff --git a/crates/sylpheed-formats/src/ixud.rs b/crates/sylpheed-formats/src/ixud.rs index 8564b402..6589705a 100644 --- a/crates/sylpheed-formats/src/ixud.rs +++ b/crates/sylpheed-formats/src/ixud.rs @@ -15,16 +15,35 @@ //! start timecode instead of inline text; the actual string then lives in a //! separate global table (not resolved here). //! -//! ## Binary layout (as far as needed) +//! ## Binary layout — ✅ decoded and verified disc-wide +//! //! ```text -//! 0x00 4 Magic "IXUD" -//! 0x04 4 version (1) -//! 0x08 4 schema/type hash (constant 0x6CC83E70) -//! .. .. record directory { key_hash u32, offset u32, len u32 } × n -//! .. .. UTF-16BE string pool, NUL-terminated entries +//! 0x00 4 Magic "IXUD" +//! 0x04 4 record_count n +//! 0x08 16*n records { u32 name_hash, u32 name_off, u32 field_begin, u32 field_end } +//! .. 4 field_count m +//! .. 12*m fields { u32 key, u32 name_off, u32 value_off } +//! .. 4 pool_size (in CHARS) +//! .. .. UTF-16BE string pool //! ``` -//! We don't need the directory to *present* the track — decoding the pool and -//! pairing tokens after the `SUBTITLE` header is enough and robust. +//! +//! **Every offset is in 16-bit chars, not bytes** — `pool_base + 2*off`. That +//! includes record and field *names*, not only values, and it is what makes the +//! `STR + 2*strsize == filesize` identity hold. +//! +//! `name_hash` is [`crate::hash::ixud_hash`] of the record's own name, and a +//! field's `key` is the same hash of its name; `name_off == 0xFFFF_FFFF` marks a +//! positional field whose key is a literal index. Verified over every IXUD object +//! on the disc: **1104/1104** objects parse, **1476/1476** records and +//! **628 165/628 165** named fields reproduce their stored hash. +//! +//! ❌ The layout this comment used to describe — a 12-byte record directory and a +//! "schema/type hash" at `0x08` — was wrong in the same way IDXD's was. There is +//! no schema field: the word at `0x08` is record 0's `name_hash`. See +//! `docs/re/structures/idxd-container.md`. +//! +//! The cue reader below still works on the string pool directly, which is fine +//! for presenting a track; [`IxudObject`] is the addressable route. /// Magic at the start of every IXUD entry. pub const IXUD_MAGIC: [u8; 4] = *b"IXUD"; @@ -185,3 +204,138 @@ mod tests { assert!(parse(b"IDXD\0\0\0\0").is_none()); } } + + +/// A parsed IXUD object: its records and their fields, addressable by name. +/// +/// The counterpart of [`crate::IdxdObject`] for wide strings. Prefer this to the +/// token-pairing readers when you need a *specific* field: pool adjacency is a +/// consequence of how records are written, not a rule of the format. +#[derive(Debug, Clone)] +pub struct IxudObject { + /// The word at `0x08` — record 0's `name_hash`, not a schema id. + pub first_record_hash: u32, + records: Vec, +} + +/// One named record of an [`IxudObject`]. +#[derive(Debug, Clone)] +pub struct IxudRecord { + /// The record's name, decoded from UTF-16BE. + pub name: String, + /// `ixud_hash(name)`, as stored. + pub name_hash: u32, + /// This record's fields, in on-disc order. + pub fields: Vec, +} + +/// One field of an [`IxudRecord`]. +#[derive(Debug, Clone)] +pub struct IxudField { + /// `ixud_hash(name)` when named, else a literal positional index. + pub key: u32, + /// The field's own name, when it has one. + pub name: Option, + /// The field's value. + pub value: String, +} + +impl IxudRecord { + /// The field named `name` (matched by `ixud_hash`), if present. + pub fn field(&self, name: &str) -> Option<&IxudField> { + let key = crate::hash::ixud_hash_str(name); + self.fields.iter().find(|f| f.key == key) + } + + /// The value of the field named `name`. + pub fn get(&self, name: &str) -> Option<&str> { + self.field(name).map(|f| f.value.as_str()) + } +} + +impl IxudObject { + /// Parse an IXUD object. `None` when the layout does not check out — the + /// trailing `pool_size` identity makes that self-verifying. + pub fn parse(b: &[u8]) -> Option { + if b.len() < 12 || b[0..4] != IXUD_MAGIC { + return None; + } + let be32 = |o: usize| u32::from_be_bytes([b[o], b[o + 1], b[o + 2], b[o + 3]]); + let n = be32(4) as usize; + if n == 0 || n > b.len() / 16 { + return None; + } + let field_count_at = 8usize.checked_add(n.checked_mul(16)?)?; + if field_count_at + 4 > b.len() { + return None; + } + let m = be32(field_count_at) as usize; + let fields_at = field_count_at + 4; + let pool_size_at = fields_at.checked_add(m.checked_mul(12)?)?; + let pool = pool_size_at.checked_add(4)?; + if pool > b.len() { + return None; + } + // pool_size counts CHARS, so this identity also proves the char scaling. + if (be32(pool_size_at) as usize).checked_mul(2)? != b.len() - pool { + return None; + } + let wstr = |off: u32| -> Option { + if off == NO_NAME { + return None; + } + let start = pool.checked_add((off as usize).checked_mul(2)?)?; + let mut units = Vec::new(); + let mut i = start; + while i + 1 < b.len() { + let u = u16::from_be_bytes([b[i], b[i + 1]]); + if u == 0 { + return Some(String::from_utf16_lossy(&units)); + } + units.push(u); + i += 2; + } + None + }; + let mut records = Vec::with_capacity(n); + for i in 0..n { + let r = 8 + 16 * i; + let (begin, end) = (be32(r + 8) as usize, be32(r + 12) as usize); + if begin > end || end > m { + return None; + } + let mut fields = Vec::with_capacity(end - begin); + for j in begin..end { + let f = fields_at + 12 * j; + fields.push(IxudField { + key: be32(f), + name: wstr(be32(f + 4)), + value: wstr(be32(f + 8))?, + }); + } + records.push(IxudRecord { + name: wstr(be32(r + 4))?, + name_hash: be32(r), + fields, + }); + } + Some(Self { + first_record_hash: be32(8), + records, + }) + } + + /// Every record, in on-disc order (ascending `name_hash`). + pub fn records(&self) -> &[IxudRecord] { + &self.records + } + + /// The record named `name` (matched by `ixud_hash`). + pub fn record(&self, name: &str) -> Option<&IxudRecord> { + let key = crate::hash::ixud_hash_str(name); + self.records.iter().find(|r| r.name_hash == key) + } +} + +/// A field `name_off` of `0xFFFF_FFFF` marks a positional field. +const NO_NAME: u32 = 0xFFFF_FFFF; diff --git a/crates/sylpheed-formats/src/lib.rs b/crates/sylpheed-formats/src/lib.rs index 79eeddbf..65bbab3c 100644 --- a/crates/sylpheed-formats/src/lib.rs +++ b/crates/sylpheed-formats/src/lib.rs @@ -85,7 +85,7 @@ pub mod ship_capture; pub use audio::{AudioCodec, AudioInfo, GameAudio}; pub use font::FontInfo; pub use idxd::{IdxdError, IdxdObject}; -pub use ixud::{Cue, Subtitle}; +pub use ixud::{Cue, IxudField, IxudObject, IxudRecord, Subtitle}; pub use movie_subtitle::{SubCue, SubLang}; pub use mesh::{GameMesh, Xbg7Model}; pub use ratc::RatcChild; diff --git a/crates/sylpheed-formats/src/movie_subtitle.rs b/crates/sylpheed-formats/src/movie_subtitle.rs index 1fada570..dcc03b36 100644 --- a/crates/sylpheed-formats/src/movie_subtitle.rs +++ b/crates/sylpheed-formats/src/movie_subtitle.rs @@ -265,19 +265,24 @@ pub fn build_caption_text(text_pak: &PakArchive) -> BTreeMap let Ok(bytes) = text_pak.read(entry) else { continue; }; - if !is_ixud(&bytes) { + let Some(obj) = crate::IxudObject::parse(&bytes) else { continue; - } - let toks = utf16le_tokens(&bytes); - for w in toks.windows(2) { - let Some((id, page, line)) = caption_key(&w[1]) else { - continue; - }; - // Same pairing rule as build_demo_text: the value is the token - // immediately before the key, and only when it is real text rather - // than another key (bare keys are serialized consecutively too). - if caption_key(&w[0]).is_none() && !w[0].trim().is_empty() { - by_id.entry(id).or_default().insert((page, line), clean(&w[0])); + }; + // Read the caption as a FIELD -- its key is the `MSG_*` name and its + // value is the text. The token-adjacency version of this recovered 8074 + // lines against the 44579 fields that actually carry text, because pool + // adjacency is a consequence of how records are written rather than a + // rule of the format. Same mistake the IDXD reader made. + for rec in obj.records() { + for f in &rec.fields { + let Some(key) = f.name.as_deref() else { continue }; + let Some((id, page, line)) = caption_key(key) else { + continue; + }; + if f.value.trim().is_empty() { + continue; + } + by_id.entry(id).or_default().insert((page, line), clean(&f.value)); } } } diff --git a/crates/sylpheed-formats/tests/caption_families_disc.rs b/crates/sylpheed-formats/tests/caption_families_disc.rs index d87b8829..1b779c12 100644 --- a/crates/sylpheed-formats/tests/caption_families_disc.rs +++ b/crates/sylpheed-formats/tests/caption_families_disc.rs @@ -48,17 +48,19 @@ fn all_eight_caption_families_are_read() { ); let total: usize = all.values().map(|v| v.len()).sum(); - assert_eq!(total, 8074, "recovered caption lines"); - assert_eq!(all.len(), 3721, "recovered caption ids"); + // 8800 is ALL of them: every distinct text-bearing MSG_* key on the disc has + // the __ shape, and the field reader recovers 8800 of 8800. + assert_eq!(total, 8800, "recovered caption lines"); + assert_eq!(all.len(), 4085, "recovered caption ids"); // `VOICE` is the only family with a letter before the id; its ids must keep it. assert!(all.contains_key("VOICE_A_150"), "VOICE ids keep their family letter"); } -/// The control: the DEMO family must come out identical through the new reader, -/// so generalising cannot have changed what already worked. +/// The control: generalising must not lose anything the DEMO-only reader had. +/// It does not — it gains, because token adjacency was dropping lines there too. #[test] -fn demo_family_is_unchanged_by_generalising() { +fn demo_family_is_not_lost_by_generalising() { skip_without_disc!(root); let pak = PakArchive::open(root.join("dat/GP_MAIN_GAME_E.pak")).expect("pak"); let demo = movie_subtitle::build_demo_text(&pak); @@ -70,10 +72,14 @@ fn demo_family_is_unchanged_by_generalising() { .filter(|(k, _)| k.starts_with("DEMO_")) .map(|(_, v)| v.len()) .sum(); - assert_eq!(old, 537); - assert_eq!(new, old, "DEMO must be identical through both readers"); + // The token-adjacency reader misses 4 DEMO lines that the field reader gets, + // so the record route is strictly better even on the family it was written + // for. It must never be WORSE. + assert_eq!(old, 537, "build_demo_text, token adjacency"); + assert_eq!(new, 541, "build_caption_text, record fields"); + assert!(new >= old, "the record route must not lose lines"); - // …and 15x more text overall than the DEMO-only path saw. + // …and 16x more text overall than the DEMO-only path saw. let total: usize = all.values().map(|v| v.len()).sum(); - assert!(total > old * 14, "expected a large gain, got {total} vs {old}"); + assert!(total > old * 16, "expected a large gain, got {total} vs {old}"); } diff --git a/crates/sylpheed-formats/tests/ixud_records_disc.rs b/crates/sylpheed-formats/tests/ixud_records_disc.rs new file mode 100644 index 00000000..b84eb78e --- /dev/null +++ b/crates/sylpheed-formats/tests/ixud_records_disc.rs @@ -0,0 +1,95 @@ +//! The IXUD record/field table, checked against every IXUD object on the disc. +//! +//! The decode was verified with a standalone parser hours before it existed in +//! the crate; this is the same check through `IxudObject`. + +use std::path::{Path, PathBuf}; + +use sylpheed_formats::hash::ixud_hash_str; +use sylpheed_formats::{IxudObject, PakArchive}; + +fn disc_root() -> Option { + if let Ok(p) = std::env::var("SYLPHEED_DISC") { + let p = PathBuf::from(p); + if p.join("dat").is_dir() { + return Some(p); + } + } + let d = Path::new( + "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", + ); + d.join("dat").is_dir().then(|| d.to_path_buf()) +} + +macro_rules! skip_without_disc { + ($root:ident) => { + let Some($root) = disc_root() else { + eprintln!("SKIP: set SYLPHEED_DISC"); + return; + }; + }; +} + +fn all_paks(root: &Path) -> Vec { + let mut out = Vec::new(); + let mut stack = vec![root.to_path_buf()]; + while let Some(dir) = stack.pop() { + let Ok(rd) = std::fs::read_dir(&dir) else { continue }; + for e in rd.flatten() { + let p = e.path(); + if p.is_dir() { + stack.push(p); + } else if p.extension().is_some_and(|x| x == "pak") { + out.push(p); + } + } + } + out.sort(); + out +} + +#[test] +fn ixud_records_roundtrip_disc() { + skip_without_disc!(root); + let (mut objects, mut records, mut named, mut positional, mut bad) = (0, 0, 0, 0, 0); + for pak in all_paks(&root) { + let Ok(ar) = PakArchive::open(&pak) else { continue }; + for entry in ar.entries() { + let Ok(bytes) = ar.read(entry) else { continue }; + if bytes.len() < 4 || bytes[0..4] != *b"IXUD" { + continue; + } + let Some(obj) = IxudObject::parse(&bytes) else { + bad += 1; + continue; + }; + objects += 1; + // The header word is record 0's hash, not a schema id. + assert_eq!( + obj.first_record_hash, + obj.records()[0].name_hash, + "{}: header word is record 0's hash", + pak.display() + ); + for r in obj.records() { + records += 1; + assert_eq!(r.name_hash, ixud_hash_str(&r.name), "{}", pak.display()); + for f in &r.fields { + match &f.name { + Some(n) => { + named += 1; + assert_eq!(f.key, ixud_hash_str(n), "{}", pak.display()); + } + None => positional += 1, + } + } + } + } + } + eprintln!("objects {objects}, records {records}, named {named}, positional {positional}"); + assert_eq!(bad, 0, "every IXUD object must parse"); + assert_eq!(objects, 1104); + assert_eq!(records, 1476); + assert_eq!(named, 628_165); + assert_eq!(positional, 48); +} diff --git a/docs/re/structures/movie-subtitles.md b/docs/re/structures/movie-subtitles.md index 56aff5d8..72e28736 100644 --- a/docs/re/structures/movie-subtitles.md +++ b/docs/re/structures/movie-subtitles.md @@ -311,7 +311,7 @@ Message_044: voice = VOICE_E_012B `VOICE_E_012B` ↔ `MSG_VOICE_E_044`. Same family letter, **different index space**. Deriving one from the other will silently mis-pair audio and text. -## ❔ The crate reads 1.3 % of the game's text +## ✅ The crate now reads ALL of the game's caption text (was 6 %) `movie_subtitle` handles the `MSG_DEMO_*` family — the cutscene captions. That is the **smallest of eight** caption families in the English pak, and the rest have @@ -331,14 +331,23 @@ Counted over every IXUD block in `GP_MAIN_GAME_E.pak`: | **`MSG_DEMO`** | **1 252** | **560** | **cutscene captions — the only one read** | | **total** | **107 372** | **44 579** | | -**560 of 44 579 = 1.3 %.** (41.5 % of keys carry text; the rest are the empty -line slots this container uses for padding, so the "with text" column is the -honest denominator.) +❌ **CORRECTED — that column counts OCCURRENCES, not lines.** Each family lives in +24–45 IXUD blocks and the same key repeats across them. The distinct figures: + +``` +44 579 text-bearing MSG_* field occurrences + 8 800 DISTINCT keys <- the honest denominator + 8 800 of those have the __ shape (100 %) +``` + +So the real coverage was **537 of 8800 = 6.1 %**, not 1.3 %. My earlier number +used occurrences and overstated the gap about fivefold. The direction was right; +the magnitude was not. `MSG_VOICE_*` is the family the message tables reference — the dialogue whose voice bindings are analysed above — and nothing in `crates/` parses it. -### 🟡 Generalised — 15× more text, but the gap is NOT closed +### ✅ Closed — 8800 of 8800, by reading fields instead of adjacent tokens `movie_subtitle::build_caption_text` now reads all eight families. Key shapes are uniform and each family is 100 % consistent with its own: @@ -350,25 +359,25 @@ Recovered (`tests/caption_families_disc.rs`, `examples/caption_coverage.rs`): | | ids | lines | |---|---|---| -| `build_demo_text` (before) | 134 | **537** | -| `build_caption_text` (now) | **3721** | **8074** | +| `build_demo_text` — token adjacency | 134 | **537** | +| `build_caption_text` — token adjacency | 3721 | 8074 | +| `build_caption_text` — **record fields** | **4085** | **8800 = all of them** | -The `DEMO` family comes out **identical** through both readers — 537 lines either -way — which is the control that generalising changed nothing that worked. +Two steps, and the second is the one that mattered. Generalising the *key parser* +took 537 → 8074; switching from **token adjacency to the record/field table** +took 8074 → **8800, which is 8800 of 8800 distinct keys**. -❌ **But 8074 is still far short of the 44 579 text-bearing fields** the -record-level scan counts. The new reader recovers about **18 %** of them. +`ixud.rs` now has an `IdxdObject`-shaped reader — [`IxudObject`] — verified over +the whole disc by `tests/ixud_records_disc.rs`: **1104/1104** objects parse, +**1476/1476** records and **628 165/628 165** named fields reproduce their +`ixud_hash`, 48 positional. The decode had been verified with a standalone +parser hours earlier and was simply never wired in. -🔑 **Why, and it is the same lesson twice.** `ixud.rs` has **no record/field -reader** — `build_caption_text` pairs a value with the key that happens to follow -it in the raw UTF-16 token stream, exactly the adjacency heuristic that was wrong -for IDXD. The IXUD record table *is* decoded and verified disc-wide (1104/1104 -objects, 628 165/628 165 fields reproducing their key) in -[idxd-container](idxd-container.md) — it was simply never wired into the crate. - -▶️ **Next:** give `ixud.rs` an `IdxdObject`-shaped record/field reader and read -captions as *fields*, not adjacent tokens. The decode already exists; only the -plumbing is missing. +🔑 **The same lesson, twice in one session.** Pool adjacency is a *consequence* of +how records are written, not a rule of the format — true for IDXD, true here. The +`DEMO` control shows it plainly: the token reader finds **537** lines, the field +reader **541**. Token adjacency was quietly dropping lines even in the one family +it was written for. ▶️ Superseded first step: `movie_subtitle::build_demo_text` already pairs a text value with the `MSG_DEMO___` key that follows it; the other seven