Skip to content

Test the origin probe with the pregap in the stream; fix EMU3's file-relative scans (#4) - #49

Merged
bmxcode merged 1 commit into
mainfrom
fix/pregap-origin-tests
Aug 28, 2026
Merged

Test the origin probe with the pregap in the stream; fix EMU3's file-relative scans (#4)#49
bmxcode merged 1 commit into
mainfrom
fix/pregap-origin-tests

Conversation

@bmxcode

@bmxcode bmxcode commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Closes #4.

The NRG pregap tests did not exercise the ADR-0005 origin probe. make_nrg() writes the DAOX track start at the end of the 150-sector pregap, and NrgImage hands FlatImage exactly that range, so the container strips the pregap before the probe runs. The header is found at offset 0 of the cooked stream and a test named for the pregap passed for a reason unrelated to a non-zero origin. The model D13 established for Roland is to keep the NRG test (it covers the container path) and add a flat-image variant with the zeros genuinely in the stream, asserting a non-zero origin.offset. This does that for the backends that lacked it.

Audit of the three named files

  • test_containers.py — clean. test_nrg_finds_the_track_behind_the_pregap asserts image.track.start == 150 * SECTOR_SIZE and reads at 0; it is the backend-agnostic container-path test and makes no false origin claim.
  • test_akai_fs.py — already covered on both halves. test_origin_probe_finds_a_partition_behind_a_pregap is the container path (NRG strips the pregap, resolves at 0); test_origin_probe_finds_a_partition_offset_into_the_image is already a genuine flat-image variant — zeros in the stream, origin.offset == len(padding) (non-zero). Only the two docstrings changed, to name which half each covers.
  • test_emu3.py — the real hole. Its only origin test asserted origin.offset == 0, so it never exercised a non-zero origin at all.

What each added test now proves

test_emu3.py::test_origin_resolves_when_the_pregap_is_inside_the_cooked_stream — an EIII disc behind 150 zeroed sectors in the stream resolves to origin.offset == 150 * 2048 and its samples list from that origin (Proteus1PresetsPiano E0, Piano A0). Before the fix below, the bank listing came back empty.

test_emu3.py::test_eiv_samples_resolve_when_the_pregap_is_inside_the_cooked_stream — the same for the E-IV path, which locates records by a whole-image scan and an E3S1 directory. E-IV records are read from an in-memory scan window, so a bank still lists under the old scan; the tell is the PCM. The test asserts read_file returns the sample's audio, which under the bug was 1 024 zero bytes read out of the pregap — a silent empty sample of the right length.

test_extract.py::test_names_differing_only_in_case_extract_to_distinct_files_by_content — two Roland samples whose names differ only in case (C_6E / C_6e) both survive extraction as distinct files, verified by a multiset of the PCM read back from the written WAVs against the two source clusters — not by name. Holds on a case-insensitive filesystem (the second lands under a _2 suffix) and a case-sensitive one alike.

Both EMU3 tests were confirmed to fail against the pre-fix scans and pass against the fixed ones; the case-collision test fails if extraction is made to key its writes on the sanitised name.

The EMU3 bug this uncovered

_bank_headers and _eiv_scan scanned from absolute position 0 and returned absolute addresses, while every read that consumes them — _samples, _declared_run, _eiv_samples, read_file — is taken at offset + address. At offset == 0 (every real EMU3 disc, and NRG discs after the container strips the pregap) the two coincide, so nothing shipped was wrong. At a genuine non-zero origin (a hybrid disc, or a raw rip with the pregap left in) they double-count: the bank listing comes back empty on EIII, and on E-IV the records list but their audio reads as zeros. That is exactly the silent empty-disc failure ADR-0005 exists to prevent, reached this time from inside the filesystem rather than at the container seam — and it was invisible precisely because no test drove the probe to a non-zero origin.

The fix makes both scans begin at offset and report addresses relative to it, so the whole backend speaks one coordinate system. It is self-consistent by construction: _placement/_bank_offsets and _eiv_unit fit address == unit * start + bias, and the bias term absorbs the origin shift, so the located addresses come out origin-relative and match the offset + address reads. No format constant or byte meaning changed; docs/formats/emu3.md is untouched. The opt-in disc-backed suite (which resolves every EMU3 reference disc at offset 0) is unaffected, and all 339 synthetic tests pass.

The filename-keyed verification gap (#4, second item)

Audited and found nothing to fix in the repo: the committed code already verifies by content, not by sanitised path. extract.py deduplicates on sha256 of PCM and the stereo joiner deliberately uses a list rather than a name-keyed dict (issue #11); the synthetic extraction tests compare content-sets ({p.read_bytes() for p in written} == set(...)); and the disc-backed suite pins per-disc SHA-256 digests over payloads, the same "multisets of hashes over PCM" the 0.2.0 notes describe. The byte-identity sweep that reported 6 false mismatches on the Roland discs (two names differing only in case folding onto one file on macOS) was an external measurement pass, not committed here. The new test_extract.py test is what now keeps that guarantee from regressing.

Verify

uv run ruff check ., uv run ruff format --check ., uv run pytest -q (339 passed, 112 skipped), and uv tool install --editable . && samplerdisc --version all pass.

…relative scans (#4)

The NRG pregap tests did not exercise the ADR-0005 origin probe: make_nrg()
writes the DAOX track start at the end of the pregap, so NrgImage strips it
before the probe runs and the header is found at offset 0 of the cooked stream.
A test named for the pregap passed for a reason unrelated to a non-zero origin.

Adds the flat-image variant (zeros genuinely in the stream, offset asserted)
alongside the NRG one where a backend's ADR-0005 coverage lacked it, modelled on
test_roland_s7xx.py's test_origin_resolves_when_the_pregap_is_inside_the_cooked_stream.

Fixing EMU3's variant uncovered a real backend bug the missing coverage hid.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@bmxcode
bmxcode merged commit 662f5ab into main Aug 28, 2026
1 check passed
@bmxcode
bmxcode deleted the fix/pregap-origin-tests branch August 28, 2026 14:00
bmxcode added a commit that referenced this pull request Aug 28, 2026
Finishes the ADR-0005 per-backend origin coverage. ISO 9660 was the one
filesystem backend without a synthetic non-zero-origin test, and it is the
exact hybrid-disc case the ADR names.

The test prepends a 150-sector zeroed pregap to a make_iso9660 image, resolves
with find_origin, and asserts the origin, the volume/listing, and -- the part
that matters -- that read_file returns the payload byte for byte from the
resolved origin. A listing alone is not enough: an extent-vs-origin bug can
list every file and still read pregap zeros, which is how the EMU3 bug in #49
hid behind a whole-looking listing.

The ISO 9660 path was already offset-clean: probe, the descriptor scan, the
directory walk and read_file all read origin + extent * SECTOR_SIZE, so no fix
was needed -- the test proves it. Confirmed the test discriminates by
temporarily dropping the origin in read_file (listing passed, read-back
failed), then reverting.

docs/formats/iso9660.md now records the LBAs-relative-to-origin assumption and
flags the untreated case: a real hybrid disc whose ISO LBAs count from the
physical disc start rather than the track would double-count and need its own
treatment.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The NRG pregap tests do not exercise the origin probe — the container strips the pregap first

1 participant