`slb_leading_segment_disc` was SIGKILLed by the OOM killer in `docker/ci/run`
at its 7 GB cap, so the documented 45/377 baseline did not reproduce on a 15 GB
box.
`PakArchive` holds the whole concatenated payload in memory and `sound.pak` is
1.01 GB (sound.p00-.p04). `bank()` and `bank_named()` opened it on every call --
inside loops -- and five tests opened their own besides, ~26 opens in all. With
cargo's default thread count that is ~6.1 GB of archive in flight against a
7 GB cap with `--memory-swap` equal to `--memory`, so there is no swap to
absorb it.
The archive is immutable once open and every accessor takes `&self`, so one
`OnceLock` instance is equivalent to N private ones at 1/N the memory.
⚠️ The failure mode is worth knowing: a SIGKILLed suite prints no
`test result:` line at all, so it disappears from a scraped tally instead of
failing visibly. The run still reported "0 failed" -- true, and useless. Check
cargo's exit code (101), not the tally.
Measured in the capped container, 7 GB, default threads:
* before: SIGKILL (signal 9), 0 of 10 tests reported
* after : 10 passed in 3.17s
* (single-threaded before the fix: 10 passed in 22.64s -- the fix is also
~7x faster, because it no longer re-reads 1 GB from disc 26 times)
* cargo fmt --check clean; cargo clippy --tests -D warnings clean
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`mesh_consistency_disc.rs` had the only conflict: this branch added a
`Sightings` type alias where #22 replaced the file's private `disc_root()`
with the shared `common::disc_root`. Both kept — they are unrelated edits
that happened to land in the same lines.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
80 findings, not the 14 the first run showed -- clippy stops at the first
failing compilation unit, so `--keep-going` is what makes the list complete.
60 were machine-applicable (`cargo clippy --fix`). The rest by hand:
* five descending `sort_by` -> `sort_by_key(Reverse(..))`
* `chunks_exact(4)` on both sides of four zips, so the compared items stay
`[u8; 4]` rather than one array against one slice
* three `type` aliases for the census maps and the captured-quad tuple
* `&PathBuf` -> `&Path` in two disc tests
* two range loops; one of them keeps `#[allow(needless_range_loop)]` with the
reason -- the index is into a map's value, which changes each iteration
* the module doc list in `invert_capture` re-indented to markdown's rules
* `blit`'s eight arguments get `#[allow(too_many_arguments)]`, not a struct
One dead `let off = b.len();` in a `ratc` test is dropped rather than renamed.
The sibling test at :162 is the one that asserts an offset; if this one was
meant to as well, that is a test change and not a lint fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Before this commit, `unset SYLPHEED_DISC` did not disable the disc-backed
suites on the machine that has the disc: every `disc_root()` fell back to a
hardcoded absolute path that exists on this box. The env var looked like a
control and was not one. Same for $SYLPHEED_RES3D and $SYLPHEED_ISO.
Replace the duplicated resolvers with one `tests/common/mod.rs`:
- 17 local `disc_root()` definitions -> 1
- 7 copies of the skip macro -> 1 (`skip_without_disc!` and siblings)
- 16 hardcoded absolute paths -> 0 executable ones
(3 of those were inline in `mesh_disc.rs`, in no resolver at all,
and 2 were in `examples/`)
- `corpus_report.rs` now reports on the SAME resolver the suites use,
instead of a second copy of the logic its own comments flagged as a
drift risk.
The 17 copies had already drifted into FIVE variants, and they were not all
the same function. `movie_manifest_disc`, `movie_subtitle_disc` and `slb_disc`
honoured $SYLPHEED_DISC and nothing else, while the other 14 fell back. So one
name already meant two things -- a third instance of the shape #16 is about.
The shared helper adopts the env-only behaviour those three already had, rather
than inventing a sixth variant.
Two module docs still described the fallback after it was deleted, which is the
same defect in prose: `texture_disc` claimed "or the default dev path exists"
and `pak_idxd_disc` said "or drop it at the default dev path below". Both now
say what the code does.
Verified both ways on the machine that HAS the corpus, which is the only place
this refactor can be falsified:
A env unset -> "ABSENT -- $SYLPHEED_DISC unset; its suites self-skip"
suites=31 passed=209 failed=0 ignored=14, slowest 0.12s
B env set -> "PRESENT via $SYLPHEED_DISC" (all three corpora)
suites=31 passed=209 failed=0 ignored=14,
slowest 1235.53s (mesh_consistency_disc)
Identical tallies, opposite corpus states, ~10000x apart in wall clock. (A) is
new behaviour -- it was previously unreachable here. (B) proves nothing broke.
`just test-disc` sources `.env` (already gitignored) for the set case. Note the
quoting trap documented there: the corpus paths contain spaces, and an unquoted
`VAR=a b c` parses as "run command `b`", failing silently into ABSENT -- which
looks exactly like a working skip.
Remedy (1) (`#[ignore]` + `--ignored`) is deliberately NOT done here: (3) already
moves the mode from the filesystem into the environment, and `#[ignore]` already
carries three meanings in this directory (corpus-absent, known-failing, bare).
Overloading it a fourth time would re-create the defect.
`cargo fmt --all --check` clean; no new compiler warnings.
Refs #16
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`cargo fmt --all -- --check` has failed on every run in this repository's
history, identically on `main` and on every branch. This is #12.
Mechanical: `cargo fmt --all`, nothing else. 154 files, all `.rs`, no other
extension touched. `cargo check --workspace` exits 0 afterwards, so nothing
changed semantically.
ON THE ORDERING, WHICH WAS THE REAL QUESTION.
HANDOFF-2026-09-06 section 7 warns this is the expensive fix: a whole-tree
reformat before #7 and #8 return "would put a conflict in every file of 861
commits and make the reviews those items exist to enable unreadable".
That is measurably too pessimistic, and it had been reasoned rather than
tested. Measured here by three-way merging a rustfmt'd `main` against both
unmerged branches, file by file:
file/branch pairs tested 32
merges CLEAN 28
merges CONFLICTING 4 (8 conflict hunks total)
sylpheed-cli/src/main.rs 1 hunk
sylpheed-export/src/check.rs 1
sylpheed-export/src/screen.rs 4
sylpheed-export/src/video.rs 2
All four are against `auto/frame-blend-draw-path` only;
`auto/port-p6-audio` does not conflict anywhere. The earlier framing --
154 dirty files, 133 that cannot collide, 21 that can, the collision set
carrying 147 of 774 hunks (19%) -- reproduces exactly. What it did not say
is that most of the 21 still merge cleanly, because rustfmt's edits and the
branches' edits rarely land on the same lines.
So the cost of sweeping now is 4 files and 8 hunks for one branch, against
a check that is otherwise red forever. Deliberately NOT folded into the
WASM PR: 154 reformatted files would make that one unreviewable.
Closes#12
The port hit `sound_bank_riffs("BGM_103.slb")` returning three against a census
that says two, and refused to guess which to drop. It was our reader.
`to_xma_riffs`'s hybrid branch derives a leading packet stream's start as
`first_riff % XMA1_PACKET`. That is right only when the bank header is smaller
than one 2048-byte packet -- true of the voice banks the branch was written for
(1392/1468/1600/1728), false of a music bank, whose header is exactly five
packets. The modulus returned 0 and the whole 10 240-byte header was emitted as
sub-wave 0.
The header states its own length, so the guard needs no threshold: BE u32 0x800
at +0x18 with the bank id repeated at +0x00 and +0x20, header length in blocks at
+0x24. Disc-wide over sound.pak's 9 519 entries, 28 match at offset 0 -- every
music bank, ids 1001-1023 and 1101-1105 -- and on 28/28 the declared header ends
EXACTLY at the first RIFF. Zero have a gap, so a header and a leading packet
stream never coexist here; zero false positives among the other 9 491.
Controlled rather than argued: decoding the emitted region through the same
chain, on the same bank, in the same run gives 0.009 s of PCM where the bank's
real wave 0 gives 87.744 s against a declared 87.75. The region is also 99.1%
zero bytes. And the oracle had already said two -- the XMA probe at the main menu
saw exactly two streams, at BGM_103's two declared wave sizes.
BGM_106-109 are deliberately NOT in the 28: their entries start mid-bank, so they
have no header at offset 0 and their leading region is real audio. The
VOICE_D_453 recovery is untouched and its tests still pass, 10/10 green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014voBspJ6kFncNErZJuZcLw
A wave runs to data_at + declared_size, and there is seek magic exactly there
whose little-endian packet count at +12 times 2048 equals the declared size --
7620/7620 disc-wide. That is the decoder-independent boundary and the thing that
proves the declared sizes honest, so it should not be able to regress silently.
The boundary routinely lies outside the entry's own comp_size window, so reading
it needs the flat segment stream rather than the entry slice; PakArchive gains a
small data_at(offset, len) accessor for that.
Test walks a bounded slice to stay fast -- 792 banks in this run, all holding.
8 disc tests pass.
1495 banks carry no RIFF and take a separate path that hardcoded both the
offset and stereo. Across a random 48-bank sample there was NOT ONE where the
old stereo-at-1392 pair beat the best mono offset; median gain 184x, individual
banks going from 0-4816 decoded bytes to 180000-380000. Stereo shows the same
stop-after-one-frame signature already recorded for the leading segment.
With no RIFF the offset cannot be derived, so scan_data_offset picks among the
four disc offsets by XMA1 packet-header plausibility. Validated on the LABELLED
set -- all 7358 banks that do have a RIFF, where the answer is forced: 7330
correct (99.62%), and all 28 misses are ties on the top score, never a wrong
unique winner. Ties fall back to 1392.
The winning offsets also reproduce, by directory, the distribution measured
independently from the RIFF-bearing banks. jpn\etc splits 1468/1600, so path
alone is not sufficient -- which is why this is a scan and not a lookup table.
7 disc tests pass (build-reborn test -p sylpheed-formats --test
slb_leading_segment_disc, SYLPHEED_DISC wired up).
HEADERLESS_DATA_OFFSET is the value the offset takes in <lang>\etc\, not a
property of the format. The leading stream is a whole number of 2048-byte XMA1
packets ending at the first RIFF, so its start is first_riff % XMA1_PACKET.
Disc-wide that takes four values -- 1392, 1468, 1600, 1728 -- varying by
language and subdirectory.
Verified by decoding, not by arithmetic: on a random 140-bank sample with a
non-empty leading region, the derived offset yields more audio in 85, identical
in 54 (the eng\etc controls, where it must and does reproduce the old
behaviour) and less in 1. Median gain among the improved is 70x --
eng\Voice\VOICE_TCAF_592 goes 1506 -> 97152 bytes, jpn 2910 -> 127178.
This withdraws my own claim from earlier today that the Japanese banks were a
different undecoded layout. They are the same format with a different offset;
I had treated a constant derived from one subdirectory as a property of the
format. The same error was hiding the identical defect in 1873 eng\Voice banks.
to_xma_riffs now emits the leading headerless segment when it sits at a whole
number of XMA1 packets and carries a non-zero byte. VOICE_D_453 goes from a
0.14 s trailing fragment to a 45116-byte leading sub-wave that dominates it.
I withdrew this exact change earlier for two reasons. Both are now answered
rather than argued away:
* "It recovers no audio" -- it used the STEREO format. At two channels every
bank yields exactly 1792 bytes, one frame, whatever its size. Mono yields up
to 113x more.
* "It matches 1524 of 8021 RIFF-bearing entries" -- the byte-level reach is
still 1524, but the audible reach is not. Across the 84 movie-bound banks
the segment adds >1 s to exactly 7, the hokyu_*_H tankers on D_453/D_454 --
precisely the broken ones -- and <=0.25 s to 66 of the rest. The largest
non-resupply addition is S04A at +0.66 s on a 256 s movie.
The safety oracle is recorded with its limits: 8 of the 84 banks ALREADY
exceed their movie's duration before the change, by hundredths of a second,
so it cannot resolve differences at that scale. It establishes scoping, not
correctness. Callers clamp to the movie length regardless.
VOICE_D_451's all-zero leading region is skipped by the non-zero guard, so
the rule cannot prepend silence to a bank that does not need it. Pinned, as
is the packet arithmetic (n = 8, 1, 7, 22, 29) which has no tunable.
slb_disc, movie_subtitle_disc and movie_manifest_disc all still pass.
NOT verified by ear -- that needs a human.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PMRJjbxLqZtsb5Vb7KunPE
