Test the origin probe with the pregap in the stream; fix EMU3's file-relative scans (#4) - #49
Merged
Merged
Conversation
…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>
This was referenced Aug 28, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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, andNrgImagehandsFlatImageexactly 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-zeroorigin.offset. This does that for the backends that lacked it.Audit of the three named files
test_nrg_finds_the_track_behind_the_pregapassertsimage.track.start == 150 * SECTOR_SIZEand reads at 0; it is the backend-agnostic container-path test and makes no false origin claim.test_origin_probe_finds_a_partition_behind_a_pregapis the container path (NRG strips the pregap, resolves at 0);test_origin_probe_finds_a_partition_offset_into_the_imageis 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.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 toorigin.offset == 150 * 2048and its samples list from that origin (Proteus1Presets→Piano 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 anE3S1directory. 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 assertsread_filereturns 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_2suffix) 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_headersand_eiv_scanscanned from absolute position 0 and returned absolute addresses, while every read that consumes them —_samples,_declared_run,_eiv_samples,read_file— is taken atoffset + address. Atoffset == 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
offsetand report addresses relative to it, so the whole backend speaks one coordinate system. It is self-consistent by construction:_placement/_bank_offsetsand_eiv_unitfitaddress == unit * start + bias, and thebiasterm absorbs the origin shift, so the located addresses come out origin-relative and match theoffset + addressreads. No format constant or byte meaning changed;docs/formats/emu3.mdis 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.pydeduplicates onsha256of 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 newtest_extract.pytest 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), anduv tool install --editable . && samplerdisc --versionall pass.