diff --git a/CHANGELOG.md b/CHANGELOG.md index 4709313..869eb32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,26 @@ Notable changes to `samplerdisc`. Format-level findings live in [docs/formats/]( ## Unreleased +### Fixed + +- **S3000 AKAI samples had 42 bytes of header at the front of their audio.** The S3000 family writes a **192-byte** header where the S1000 family writes 150, and every sample was read at 150. That does not fail — the frame count comes out right, the WAV opens, the length is within 0.1 % — so what shipped was a file beginning with a burst of roughly ±20 000 lasting 0.24 ms in place of the attack, missing the last 21 frames of the sound, with every loop point 21 frames out of alignment. **13 451 of the collection's 56 490 AKAI samples**, on nine discs: `AKAI.S3000.Sound.Library.1`–`7` (4 455, 3 086, 1 990, 1 010, 601, 168 and 218), `East Connexion Piano` (730) and `AMG - Now CD-Rom for (AKAI)` (1 193). **Anyone who extracted those discs should do it again.** ([docs/formats/akai-fs.md](docs/formats/akai-fs.md), [ADR-0027](docs/adr/0027-a-payload-must-be-the-file-its-entry-placed.md)) + + **Which length applies is declared, not sniffed.** The directory entry's type byte carries the generation in its high bit — the same bit that already named a kept original `.s3s` rather than `.s1s` — and it splits the 44 discs' samples perfectly, 13 451 at 192 and 42 989 at 150, with no disc mixing the two rules. The payload confirms it from the other side: the directory's declared size is `words × 2 + header_len` on **56 430 of 56 430** readable payloads. The format doc's old advice — that S3000 discs *may* use a 192-byte variant, and to branch on the id and valid bytes — was wrong twice: the variant is not conditional, and `0x80` appears on 42 989 samples at 150 and 13 410 at 192. + + **The suite had no AKAI payload check and now does.** The E-mu and Roland paths have been asserting their audio against the disc for two deliverables; AKAI's was verified against the same wrong offset it was written from, which is how a 42-byte slip survived four releases in a green suite. Every accepted payload is now checked byte for byte against the disc from the chosen offset, and the word-count identity is asserted over every AKAI disc rather than the pinned ones. + +- **31 healthy S3000 samples were being thrown away as damage.** Their valid byte is `0x81` (29 on `Library.2`) or `0x9c` (2 on `Library.1`) rather than exactly `0x80`, with a correct id, a name matching their directory entry, a normal rate and a word count the declared size agrees with. `0x80` is a flag inside the byte and was being tested as the whole byte. The collection goes from 89 125 samples to **89 156**, AKAI from 56 394 to **56 425**, and AKAI stereo joins from 14 449 to **14 461** — the extra pairs completing because both halves are now present. ([ADR-0027](docs/adr/0027-a-payload-must-be-the-file-its-entry-placed.md)) + ### Added +- **An AKAI payload is checked against the directory entry that placed it.** The payload repeats the file's id, valid flag and name, and nothing compared them ([#23](https://github.com/bmxcode/samplerdisc/issues/23)). Where they disagree the file is refused rather than written under that entry's name, and the reason names **every** field that disagrees and the entry involved — `payload header carries id 224 not 3, a name that does not decode in the AKAI charset, placed by an entry named 'PADY CHORD06'` — instead of the old `payload does not start with an AKAI sample header`, which was true of a payload that is mid-audio and of one that is a perfectly good sample under the wrong name alike. Counted apart from damage in the summary and the manifest, as duplicates already were. ([docs/formats/akai-fs.md](docs/formats/akai-fs.md), [ADR-0027](docs/adr/0027-a-payload-must-be-the-file-its-entry-placed.md)) + + **#23's bug did not reproduce, and saying so is the finding.** All 96 payloads that disagreed with their entry were already being refused — the nine named files on `Kickin' Lunatic Beats 2 CD1` included — and they were exactly the 96 skips the README already reported, not a disjoint set. Three and a half of the four proposed tests had been there since D3. What was missing was that the refusal said nothing useful, so nobody checked whether it existed. Ruling out the false-positive case the issue itself pointed at — the S3000 header — is what found the defect above. + + **The name comparison ships with zero positives on real data**, and that is stated rather than buried. Across all 44 discs it fires 60 times and never once without the id and valid tests firing too, because on these images the displacement lands mid-audio and mid-audio does not look like a header. It is kept because it is the only test that asks whether a payload is *this* sample rather than *a* sample — the case [#25](https://github.com/bmxcode/samplerdisc/issues/25) will raise — and it is exercised synthetically alone. + + **The 65 remaining refusals cluster, which is what says the check is right.** 60 of them are a run to the end of one volume, on ten discs; the other 34 AKAI discs have none. Five are single files — four with a corrupt rate byte and an otherwise perfect header, which are counted as damage rather than as mismatches because they *are* the files their entries placed. `Best Service - Alpha Dance II` is the one to note: it declares six partitions and holds all six, and still loses 21 of one volume's 22 samples, so a rip can drop a run of blocks inside a partition without the partition table registering anything. + - **E-mu stereo samples come out as stereo.** The sample record's pointer block declares a channel count, and where it declares two the payload is a **block** split — all of the left channel, then all of the right. Read as one mono stream that is a file twice as long as the sound, with the right channel playing after the left instead of alongside it, and that is what this project wrote for **2 656 of the 14 738** E-mu samples: 28, 601, 592, 8, 279, 320 and 828 across the seven reference discs. `Miroslav Vitous String Ensembles` is all of it — 828 of 828. They are now interleaved into one stereo WAV each. ([docs/formats/emu3.md](docs/formats/emu3.md), [ADR-0026](docs/adr/0026-the-record-declares-the-channel-count.md), [#32](https://github.com/bmxcode/samplerdisc/issues/32)) **A channel count is not enough on its own.** 2 721 records declare `start_R` half a payload on, and **65 of them close `end_L` somewhere else** — 19 on `protozoa`, 40 on `eiiix-1`, 6 on `eiiix-2`. Those 65 are not stereo: their halves correlate at 0.01 on fine structure and 0.01 at best lag, which is the score of two unrelated records, against 0.40 and 0.53 for the known-true stereo pairs on the same shelf. On `protozoa` six of them can be identified exactly: the first half of each is, byte for byte, the whole of a one-channel record of the same name in another bank, nothing matches the second half, and `end_L` closes 8 bytes past the halfway point rather than on it. Requiring the left block to close exactly where the right one opens is what separates them, and those 65 stay mono. diff --git a/README.md b/README.md index ce648fe..9537475 100644 --- a/README.md +++ b/README.md @@ -78,31 +78,32 @@ Compressed `.mdx` is the piece no other open-source tool reads today. The format | | | |---|---| | Discs converted | 72 of 79 | -| Samples | 89 125 | -| Stereo pairs rejoined | 15 796 | +| Samples | 89 156 | +| Stereo pairs rejoined | 15 808 | | Audio CD tracks | 161 | | Duplicate audio suppressed | 5 719 | -| Entries skipped (damage) | 97 | -| Time | 46 s | +| Entries not the file their entry placed | 61 | +| Entries skipped (damage) | 5 | +| Time | 48 s | By filesystem: | | Discs | Samples | Stereo pairs | Skipped | |---|---:|---:|---:|---:| -| AKAI | 44 | 56 394 | 14 449 | 97 | +| AKAI | 44 | 56 425 | 14 461 | 66 | | E-mu `EMU3` | 7 | 14 738 | 6 | 0 | | ISO 9660 | 15 | 11 601 | — | 0 | | Roland `S770 MR25A` | 5 | 6 392 | 1 341 | 0 | "Stereo pairs" counts files joined from an `-L`/`-R` pair by name. E-mu's six are the only ones on those discs, and they are a different and much rarer thing than the 2 656 samples whose record declares two channels. -Every WAV was checked against the disc it came from — **70 of 70 discs match exactly**, comparing multisets of SHA-256 over the PCM per disc rather than going via filenames, so duplicate names cannot mask a mismatch and no path is guessed. 89 125 payloads, zero mismatches. The E-mu stereo samples are compared with their channels put back the way the disc stored them, since their WAV holds the same bytes interleaved; `tests/test_discs.py` asserts that de-interleaving reproduces the disc's two blocks exactly, per sample, on all seven discs. The two audio CDs are not in that count: their tracks are cut from a stream by a cue, so there is no run of bytes on the disc to compare a track against. +Every WAV was checked against the disc it came from — **70 of 70 discs match exactly**, comparing multisets of SHA-256 over the PCM per disc rather than going via filenames, so duplicate names cannot mask a mismatch and no path is guessed. 89 156 payloads, zero mismatches. The E-mu stereo samples are compared with their channels put back the way the disc stored them, since their WAV holds the same bytes interleaved; `tests/test_discs.py` asserts that de-interleaving reproduces the disc's two blocks exactly, per sample, on all seven discs. The two audio CDs are not in that count: their tracks are cut from a stream by a cue, so there is no run of bytes on the disc to compare a track against. -105 082 WAV files were written in all — the samples, the stereo joins and the audio CD tracks. None is unreadable and none is zero-length. **275 are silent for their whole length, and every one of them matches the disc exactly**: 267 are the blank `15G-KIT…Z` slots on `ProSamples vol.15`, six are on a Proteus library that ships `Dead Air` as a sample, and two are on a Roland disc. That is what the discs hold, not something the decoder did. +105 125 WAV files were written in all — the samples, the stereo joins and the audio CD tracks. None is unreadable and none is zero-length. **275 are silent for their whole length, and every one of them matches the disc exactly**: 267 are the blank `15G-KIT…Z` slots on `ProSamples vol.15`, six are on a Proteus library that ships `Dead Air` as a sample, and two are on a Roland disc. That is what the discs hold, not something the decoder did. Sample rates run from 6 000 to 49 999 Hz across 1 047 distinct values. The odd ones are real — E-mu writes rates like 24 444 and 27 778, and AKAI uses 33 075 (¾ of 44 100) and 29 400 (⅔) to trade bandwidth for memory. They are carried through exactly as the disc states them and never rounded. -The 97 damage skips are almost all one thing: 92 payloads that do not begin with an AKAI sample header, concentrated on seven `.mdx` images. The rest are four implausible sample rates and one stereo pair whose halves declare 44 033 and 44 100, so the joiner refuses to fuse them and writes both mono halves instead. +The 66 AKAI entries not written are two different faults. **61 are payloads that are not the file the directory placed there** — their header carries another file's id, valid flag or name — and 60 of those 61 are a run to the end of one volume, on ten discs; the other 34 AKAI discs have none. That is what a rip losing a run of blocks looks like from inside a directory, and it does not need a partition to go missing: `Best Service - Alpha Dance II` declares six partitions and holds all six, and still loses 21 of `AC.DRUMLOOPS`'s 22 samples. Each is refused with a line naming every field that disagrees and the entry that placed it, rather than being written out under a name that is not its own ([ADR-0027](docs/adr/0027-a-payload-must-be-the-file-its-entry-placed.md)). The remaining five are damage of a different kind: four samples whose header is otherwise perfect and whose rate field reads 0, 519, 519 or 1280, and one stereo pair whose halves declare 44 033 and 44 100, so the joiner refuses to fuse them and writes both mono halves instead. The seven that do not convert are accounted for: one S-550 disc present as both `.iso` and `.nrg` — a different format from the S-7xx and not yet read ([ADR-0014](docs/adr/0014-one-backend-per-on-disc-format.md)) — two Digidesign SampleCell discs, one audio CD with no cue sheet present as both `.mdx` and `.cdr`, and one ISO 9660 disc holding E-mu `.EBL` banks rather than audio. diff --git a/docs/README.md b/docs/README.md index 227b964..ad008aa 100644 --- a/docs/README.md +++ b/docs/README.md @@ -58,11 +58,16 @@ All of that vanishes into a working parser. Six months on, the code says *what* | D16 | AIFF payloads converted, deduplicated against their WAV twin, and EXS24/HALion instruments kept ([ADR-0024](adr/0024-the-aiff-twin-is-converted-and-deduplicated.md)) | done | | D17 | E-mu loop points in the WAV `smpl` chunk ([ADR-0025](adr/0025-the-loop-is-decoded-the-root-key-is-not.md)) | done | | D18 | E-mu stereo samples decoded from the record's channel count ([ADR-0026](adr/0026-the-record-declares-the-channel-count.md)) | done | +| D19 | An AKAI payload must be the file its entry placed, and the S3000 header is 192 bytes ([ADR-0027](adr/0027-a-payload-must-be-the-file-its-entry-placed.md)) | done | Across the local collection, by listing: 71 of 79 images claimed, 2 578 volumes, 110 989 files, 77 620 of them samples. The AKAI discs are 44 of those images and 68 997 of those files, read across 275 partitions — before D15 they were 14 670 files, because only the partition at the origin was read. ## What is not done +- **25 AKAI files are displaced inside a partition the table calls complete.** `Best Service - Alpha Dance II` declares six partitions and holds all six, and 21 of `AC.DRUMLOOPS`'s 22 samples are somebody else's audio; `AKAI.S3000.Sound.Library.1` and `.3` lose three and one the same way. The rip dropped a run of blocks *inside* a partition rather than the blocks a header sat on, so nothing in the partition table registers a gap and the declared-against-present count reads clean. They are refused and named rather than written ([ADR-0027](adr/0027-a-payload-must-be-the-file-its-entry-placed.md)); recovering them would mean locating a volume's blocks by something other than the chain the allocation map declares, which is the search [ADR-0022](adr/0022-a-volume-is-explained-by-the-allocation-map.md) and [ADR-0023](adr/0023-partitions-come-from-the-table-the-disc-declares.md) both refused. Sibling of [#25](https://github.com/bmxcode/samplerdisc/issues/25) and invisible to it. +- **The AKAI payload name check has no positives on real data.** It is the test that separates *"this payload is a sample"* from *"this payload is **this** sample"*, and across all 44 discs it fires 60 times and never once without the id and valid tests having fired too — because on the short images the displacement lands mid-audio, which does not look like a header. It is exercised only synthetically. A displacement landing exactly on another sample's header is the case it exists for and the collection has none, which is worth knowing before [#25](https://github.com/bmxcode/samplerdisc/issues/25) recovers those partitions ([ADR-0027](adr/0027-a-payload-must-be-the-file-its-entry-placed.md)). +- **What the AKAI valid byte's low bits mean is unestablished.** `0x81` on 29 samples and `0x9c` on two, against `0x80` on 56 397. The `0x80` flag is the sample-is-valid bit and the rest is unread; three combinations on two discs is not enough to read them from. + - **Roland S-550.** `Roland LCD1.iso`/`.nrg` opens `* ROLAND S-550 *` and is a different format from the S-7xx entirely ([ADR-0014](adr/0014-one-backend-per-on-disc-format.md)). Neither archive holds a second specimen, so it stays deferred rather than being reverse-engineered from one disc. - **Ensoniq and Kurzweil backends.** The archives are full of these discs and the containers already open them; each needs a module in `fs/` and nothing else ([ADR-0003](adr/0003-brand-neutral-pluggable-backends.md)). - **65 records declare two channels and are not stereo.** They put `start_R` half a payload on and then close `end_L` somewhere else, and their halves measure as unrelated audio. Six of `protozoa`'s are explained — their first half is byte for byte a whole one-channel record of the same name in another bank, so the payload is twice the sound — and the rest are not. They come out mono, which is right, but *what they hold* is unestablished on `eiiix-1`'s 40, `eiiix-2`'s 6 and thirteen of `protozoa`'s ([formats/emu3.md](formats/emu3.md), [ADR-0026](adr/0026-the-record-declares-the-channel-count.md)). diff --git a/docs/adr/0027-a-payload-must-be-the-file-its-entry-placed.md b/docs/adr/0027-a-payload-must-be-the-file-its-entry-placed.md new file mode 100644 index 0000000..69ca890 --- /dev/null +++ b/docs/adr/0027-a-payload-must-be-the-file-its-entry-placed.md @@ -0,0 +1,121 @@ +# ADR-0027 · An AKAI payload must be the file its entry placed, and its header length is the generation the entry declares + +**Status:** accepted · 2026-08-22 + +## Context + +[Issue #23](https://github.com/bmxcode/samplerdisc/issues/23), raised while settling #17, says an AKAI sample payload whose header disagrees with its directory entry is extracted silently — as a WAV that opens, plays, and is somebody else's audio. It names nine files on `AMG - Kickin' Lunatic Beats 2 AKAI CD1`, proposes four tests, and asks for a sweep across the collection before deciding what a failure should do. + +**The sweep says the bug does not reproduce.** Across all 44 AKAI discs and 56 490 sample entries, 96 payloads disagree with their entry and **all 96 are already refused**. The nine named files are among them. Nothing is extracted silently, and nothing ever was: `_looks_like_header` has been testing the id, the valid byte and the name's decodability since D3, and `parse` has been testing the rate. Three and a half of #23's four tests were already there and nobody had noticed, including the person who wrote the issue. + +The 96 are also, exactly, the 96 skips the README already reported — 92 "do not begin with an AKAI sample header" plus four "implausible sample rates". Not a disjoint set, not a near-miss: the same files. So the deliverable as briefed had nothing to add. + +What it did have was the false-positive case to rule out first, which the issue's own wording pointed at: *the S3000 192-byte header variant exists, and a check keyed to 150 that fires on every S3000 disc is what to rule out first.* Ruling it out found the defect underneath. + +### The S3000 header is not a variant to be aware of; it is a quarter of the collection, read wrong + +**13 451 of the 56 490 AKAI samples carry a 192-byte header and were read as though it were 150.** Three independent structures agree on it and none of them knows about the others: + +- **The directory says so.** Every one of the 13 451 is a file whose type byte has the high bit set — the S3000-family flag that already names a kept original `.s3s` rather than `.s1s`. The split is perfect: 13 451 high-bit files at 192, 42 989 low-bit files at 150, and no disc mixing the two rules. +- **The payload says so.** The directory's declared size equals `words × 2 + header_len` on **56 430 of 56 430** payloads readable at all — 13 441 at 192 and 42 989 at 150. The 60 that fail that identity are the damaged ones, and every one of them fails an identity test too. +- **The bytes say so.** At 150 on `AKAI.S3000.Sound.Library.2`'s `NPF E0` sit 21 zero bytes and then a repeating `00 0a ff ff 22 a8 00 aa ff ff …` — structurally identical across samples, which audio is not. Real waveform starts at 192. + +The consequence is not an error, which is why it survived four deliverables. `frames` came out right, the WAV opened, its length was within 0.1 %, and the payload-versus-output check the README ran compared the output against the same wrong offset. What actually shipped was a WAV **beginning with 42 bytes of header read as PCM** — a burst of roughly ±20 000 lasting 0.24 ms, an audible click on the attack — **missing the last 21 frames** of the sound, and carrying every loop point 21 frames out of alignment. On nine discs: `AKAI.S3000.Sound.Library.1`–`7`, `East Connexion Piano` and `AMG - Now CD-Rom for (AKAI)`. + +`HEADER_LEN_S3000 = 192` was declared in `sample/akai.py` and never read by anything. The format doc said "S3000 discs **may** use a 192-byte header variant. Branch on the id and valid bytes rather than assuming 150" — and that advice is wrong twice over: the variant is not conditional, and those two bytes do not carry the answer. `0x80` appears on 42 989 samples at 150 and 13 410 at 192. + +### 31 healthy samples were being thrown away for a bit + +Of the 96 refusals, **31 fail on the valid byte alone**: `0x81` on 29 samples of `Library.2` and `0x9c` on two of `Library.1`. Their id is 3, their name matches the directory exactly, their rate is a normal 44 100 or 22 050, and their word count agrees with the declared size at 192. `0x80` is a flag inside a byte and was being tested as the whole byte. + +### What the four tests are actually worth + +Over the 65 that remain once the flag is read as a flag: + +| Test | Fires | Fires **alone** | +|---|---:|---:| +| id `!= 3` | 61 | 1 | +| valid, no `0x80` bit | 60 | 0 | +| name `!=` the entry's | 60 | **0** | +| rate outside 4000–50000 | 58 | 4 | + +**The name comparison — the one thing #23 was actually about — detects nothing the others miss.** Every payload whose name disagrees also has a wrong id and a cleared valid flag, because on these images the displacement lands mid-audio, and mid-audio does not look like a header. + +### The failures cluster, which is what says the check is right + +**60 of the 65 are a run to the end of one volume.** Not a scatter, and not spread thinly across healthy discs. + +| Disc | Mismatches | Where | Partitions declared / present | +|---|---:|---|---| +| `Best Service - Alpha Dance II` | 21 | `AC.DRUMLOOPS`, last 21 of 22 | **6 / 6** | +| `Best Service - Alpha Dance I` | 15 | `ATTACK BANK2`, last 15 of 18 | 5 / 4 | +| `Kickin' Lunatic Beats 2 CD1` | 9 | `13-TRACK 06`, last 9 of 20 | 11 / 1 | +| `AKAI.S3000.Sound.Library.5` | 7 | `SURDO`, last 7 of 13 | 9 / 3 | +| `AKAI.S3000.Sound.Library.1` | 4 | `3084 B.BEAT6` last 3 of 8; one rate | 13 / 13 | +| `AMG - Global Trance Mission 2` | 3 | `AMBIENT PAD2`, last 3 of 6 | 9 / 4 | +| `AKAI.S3000.Sound.Library.2` | 3 | three isolated rate bytes — 0, 519, 519 | 13 / 13 | +| `AKAI.S3000.Sound.Library.3` | 1 | `VOLUME 001`, its only file | 13 / 13 | +| `Audio Factory - Classical Wild Takes` | 1 | `VOLUME 002`, last of 2 | 11 / 6 | +| `AMG - Loop Soup` | 1 | `SOUP 101-103` #27 — the format doc's known one | 9 / 9 | + +The other 34 AKAI discs have none at all, `Advance Orchestra`'s 2 236 samples included. + +The falsifying case the sweep was meant to find turns out to be a finding instead. `Alpha Dance II` declares six partitions and holds all six, and still loses 21 of one volume's 22 samples as a tail run; `Library.1` and `Library.3` are the same shape. That is **a run of blocks lost inside a partition**, which the partition table cannot see because no header goes missing — 25 files on three `.mdx` images, a sibling of [#25](https://github.com/bmxcode/samplerdisc/issues/25) and invisible to it. + +## Decision + +**A payload must be the file its directory entry placed, and the header length in front of it is the generation that entry declares.** + +Five parts. + +**The identity is checked, against the entry, at parse time.** The id must be 3, the valid byte must carry `0x80`, the name must decode, and where the entry supplied a name the payload's must equal it. All four are kept. + +**The header length comes from the directory's type-byte high bit.** 192 on the S3000 family, 150 on the S1000 one. Placed by one structure and confirmed by another, the shape [ADR-0020](0020-read-e-iv-through-its-sample-directory.md), [ADR-0021](0021-a-bank-owns-the-run-its-header-declares.md) and [ADR-0023](0023-partitions-come-from-the-table-the-disc-declares.md) already use — here the confirmation is the payload's own word count against the declared size, and it is asserted per disc in `tests/test_discs.py` rather than re-derived per sample at runtime. + +**`valid` is read as a flag, `& 0x80`, not as a byte.** Measured safe on its own: no payload anywhere in the collection has the `0x80` bit set, an id of 3 and a plausible rate except the 31. + +**A refusal is a `Skipped` carrying a `mismatch` flag, counted apart in the summary and the manifest.** [ADR-0024](0024-the-aiff-twin-is-converted-and-deduplicated.md)'s precedent one step on: a duplicate is not damage, and a payload that is a good sample under the wrong name is not the same news as a payload that is mid-audio. The reason names **every** field that disagrees rather than the first, because what they disagree about together is the diagnosis. The four rate refusals keep `mismatch=False`: on all four the id, the flag and the name agree with the directory, so those *are* the files their entries placed, with one field unusable. + +**The declared name and the generation reach the parser through `AkaiBackend.parse_sample`.** Both are the directory's knowledge, not the payload's, and the shared extract path may not learn what an AKAI type byte means ([ADR-0003](0003-brand-neutral-pluggable-backends.md)). `Emu3Backend` and `RolandS7xxBackend` already carry parameters across this way for the same reason. + +## Alternatives rejected + +**Close #23 as not reproducing and ship nothing.** Defensible on the counts — the check adds no behaviour on this collection, and the S3000 header could have been a separate deliverable. Rejected because the two are one piece of work: the header length was found by ruling out the check's false-positive case, and the 31 samples the flag relaxation admits are only safe to admit because the identity check is there. Shipping a check that certifies payloads agree with their entries while 13 451 of them are read 42 bytes early would have been the worse outcome of the two. + +**Drop the name comparison, since it catches nothing the other three miss.** Numerically it is dead weight: 60 fires, 0 unique. Rejected on what it asks rather than on what it caught. The other three ask whether the payload is *a* sample; only this one asks whether it is *this* sample, which is the entire failure class #23 named and the one [#25](https://github.com/bmxcode/samplerdisc/issues/25) will raise when a short image's partitions are recovered and a displacement lands on a real header rather than mid-audio. It costs one string comparison. The honest form of this is not to remove it but to say plainly that it has no positives on real data and is exercised only synthetically — which the test docstring and the format doc both do. + +**Choose the header length by solving `size == words × 2 + H` at runtime.** The same identity that verifies the rule, used to apply it, and it would need no directory knowledge in the parser. Rejected for turning a field the disc *states* into arithmetic between two structures — the register [ADR-0021](0021-a-bank-owns-the-run-its-header-declares.md) settled — and because it answers nothing exactly when it is needed: on the 60 damaged payloads neither length solves it, so the fallback would be the generation bit regardless. A rule that consults the declared field only when it cannot be checked is worse than one that consults it always. + +**Branch on the id and valid bytes, as the format doc advised.** Rejected on measurement: `0x80` appears on 42 989 samples at 150 and 13 410 at 192, and the id is 3 on both. The advice predates anyone having counted, and it is corrected in the format doc rather than left standing beside the new text. + +**Sniff bytes 150–191 for the header's own pattern.** It is distinctive and constant across samples. Rejected for the reason [ADR-0023](0023-partitions-come-from-the-table-the-disc-declares.md) refused scanning for the partition header: a pattern that sample data can reproduce is not evidence, and here there is a declared field that is right 13 451 times out of 13 451. + +**Add `0x81` and `0x9c` to a set of accepted valid bytes.** One line, and it admits exactly the 31. Rejected as a table of two observed values standing in for what the field is. A third disc with a third low-bit combination would be discarded silently, and the discarding would look like damage. + +**Refuse only, and never relax the flag.** The conservative reading, and it keeps throwing away 31 samples whose every other field is correct — including 29 consecutive `1015 E.PF`, `1051 VL+PIZ` and `1054CHO HARP` entries that are plainly a library's own multisamples. Rejected: "the byte is not exactly 0x80" was never a finding, it was an assumption from three reference discs. + +**Write a mismatched payload out under the payload's own name instead of the entry's.** Recovers audio rather than refusing it, and on a displaced image the payload's name is arguably the truer one. Rejected on the data: the name does not decode at all on 54 of the 60, and decodes to `0C0B0B0B0A0`-style noise on the other six. There is no name there to prefer. + +**A `--force` flag to extract mismatches anyway.** Rejected as an option nothing on the shelf would use: all 65 are unusable, and an escape hatch whose only effect is to write 65 known-bad files is a maintenance cost with no user. + +**A third result shape beside `Extracted` and `Skipped`.** The strongest signal, and it was seriously considered because [ADR-0012](0012-a-probe-must-confirm-a-file.md)'s lesson is that an unexplained absence is itself a failure signature. Rejected because a mismatch *is* a subset of not-written: every consumer — `cli`, `batch`, the manifest — would grow a branch to say something the existing shape says with one boolean, and `Skipped.duplicate` established that boolean's precedent one deliverable ago. + +**Fix `Alpha Dance II` and the other two discs' mid-partition damage here.** 25 files, and the shape is now understood. Rejected as a different deliverable: recovering them means locating a volume's blocks by something other than the chain the map declares, which is the search [ADR-0022](0022-a-volume-is-explained-by-the-allocation-map.md) and [ADR-0023](0023-partitions-come-from-the-table-the-disc-declares.md) both refused. It gets an issue. + +## Consequences + +**Good, and the headline.** 13 451 samples on nine discs stop carrying 42 bytes of header at the front of their audio, stop losing their last 21 frames, and get their loop points back in alignment. Every WAV from `AKAI.S3000.Sound.Library.1`–`7`, `East Connexion Piano` and `AMG - Now CD-Rom for (AKAI)` changes. Anyone who extracted those discs should do it again. + +**Good.** 31 samples that were being discarded as damage now extract. The collection goes from 89 125 samples to **89 156**, AKAI from 56 394 to **56 425**, and AKAI stereo joins from 14 449 to 14 461 — the extra pairs completing because both halves are now present. + +**Good.** The audio is verified against an independent structure per sample: `entry.size == words × 2 + header_len` holds on **56 425 of 56 425** accepted payloads, and `tests/test_discs.py` asserts it, along with each WAV's PCM being the disc's own bytes from that offset. The suite had nothing of the kind for AKAI — the E-mu and Roland paths both did — which is why a 42-byte slip survived four deliverables in a green suite. + +**Good.** The whole-collection payload check is re-established rather than inherited: 70 of 70 discs, **89 156 WAVs, zero whose audio is not on the disc it came from**. That claim was true before this change against the wrong offset, which is precisely what made it worthless. + +**Bad, and stated plainly.** #23 was open for a day and describes a bug that was not there. The nine files it names were being refused the whole time, with a message — "payload does not start with an AKAI sample header" — that was true and told nobody which of four things was wrong or that a directory entry was involved at all. That is the part worth keeping from the issue: the refusal existed and said nothing useful, so nobody checked whether it existed. + +**Bad.** The name comparison ships with zero positives on real data. It is the reason the deliverable was requested and it is the least load-bearing line in it, and no amount of measurement can change that until a disc arrives whose displacement lands on a header. It is exercised synthetically and the test says so. + +**Watch for.** A disc whose type byte's high bit does not mean the generation. The bit is doing more work now than it was: before this it named a file `.s3s` rather than `.s1s`, which is cosmetic, and now it decides where the audio starts. A disc that sets it wrongly would produce samples 42 bytes off in one direction or the other — and the word-count assertion in the suite is what would catch it, per disc, which is why that assertion is over every AKAI disc rather than over the pinned eight. + +**Watch for.** The 60 payloads on complete-looking images. `Alpha Dance II`, `Library.1` and `Library.3` are damaged inside a partition the table calls whole, so "declared equals present" is not a clean bill of health and must not start being read as one. diff --git a/docs/adr/README.md b/docs/adr/README.md index 45e0e18..b907ce1 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -32,3 +32,4 @@ If you find yourself writing an ADR with no rejected alternative, you are writin | [0024](0024-the-aiff-twin-is-converted-and-deduplicated.md) | Convert AIFF, and drop the twin only when it says nothing new | Writing both trees; never converting; deduplicating by name; preferring the AIFF; merging its metadata into the copied WAV; calling the byte swap a conversion | | [0025](0025-the-loop-is-decoded-the-root-key-is-not.md) | The E-mu loop is decoded from the record; the root key is not there to decode | Deriving the root key from the sample name; no `smpl` chunk without a root key; clamping the loop end as AKAI and Roland do; emitting the second channel's loop as well; trusting the declared extent over the record length | | [0026](0026-the-record-declares-the-channel-count.md) | The E-mu record declares the channel count, and its own extents confirm it | Splitting on the channel count alone; writing the mono halves alongside; filing it under `stereo/`; deciding stereo from the audio; taking high-correlation one-channel records as stereo; splitting at `end_L`; writing two mono files | +| [0027](0027-a-payload-must-be-the-file-its-entry-placed.md) | An AKAI payload must be the file its entry placed, and its header length is the generation the entry declares | Closing #23 as not reproducing; dropping the name comparison; solving for the header length at runtime; branching on the id and valid bytes; sniffing the header pattern; an allow-list of valid bytes; never relaxing the flag; writing a mismatch under the payload's name; a `--force` flag; a third result shape; fixing the mid-partition damage here | diff --git a/docs/formats/akai-fs.md b/docs/formats/akai-fs.md index 74a6744..eac2bf0 100644 --- a/docs/formats/akai-fs.md +++ b/docs/formats/akai-fs.md @@ -152,7 +152,9 @@ A missing partition is not always damage, and the two are distinguishable: on th A short image also shows up in the table's arithmetic alone: `Kickin' Lunatic Beats 2 CD1` declares eleven partitions where the image holds one, and `ProSamples vol.54` declares nine of a 63 488-block disk on a CD of 30 720 blocks — the second of those is not damage, it is a CD carrying only the front of a larger disk. Declared against present is worth printing for that reason: it names the gap without diagnosing it. -The visible consequence inside partition 1 is not only the four empty volumes. **Nine files in `13-TRACK 06` extract audio that is not theirs**: their payload header no longer matches the name the directory gives them, because everything past the first gap has slid. A payload whose header disagrees with its directory entry is a cheap check that would catch this and is not made anywhere yet — [issue #23](https://github.com/bmxcode/samplerdisc/issues/23). +The visible consequence inside partition 1 is not only the four empty volumes. **Nine files in `13-TRACK 06` no longer hold their own audio**: everything past the first gap has slid, so their payload is mid-PCM rather than a header. They are refused and named, one line each saying which fields disagree and which entry placed them ([ADR-0027](../adr/0027-a-payload-must-be-the-file-its-entry-placed.md)). [Issue #23](https://github.com/bmxcode/samplerdisc/issues/23) proposed that check believing they were being written out; they were already being refused, with a message that said neither which test failed nor that a directory entry was involved. + +The same damage appears **without a partition going missing**, which is worth stating separately because the table's arithmetic cannot see it. `Alpha Dance II` declares six partitions and holds all six, and 21 of `AC.DRUMLOOPS`'s 22 samples are displaced; `AKAI.S3000.Sound.Library.1` and `.3` are the same shape, 25 files across the three. The rip lost a run of blocks inside a partition rather than the blocks a header sat on. Observed volume names, useful as a smoke test that charset and offsets are both right: @@ -214,7 +216,7 @@ Programs hold key ranges and envelopes, not audio. They are listed and skipped b The generation is readable from the same byte: the high bit is set on S3000-family discs and clear on S1000 ones, which is what names a kept original `.s3p`/`.s3s` rather than `.s1p`/`.s1s`. `s3000-lib1` sets it; `black2black` and `loopsoup` do not. -## Sample file — 150-byte header, then PCM +## Sample file — a header, then PCM | Offset | Size | Meaning | |---|---|---| @@ -222,17 +224,67 @@ The generation is readable from the same byte: the high bit is set on S3000-fami | 1 | 1 | bandwidth | | 2 | 1 | original pitch, MIDI note | | 3 | 12 | name, AKAI charset | -| 15 | 1 | valid — `0x80` | +| 15 | 1 | valid — a **flag byte**, `0x80` set | | 26 | 4 | u32 LE number of sample **words** | | 132 | 4 | u32 LE SLOCAT | | **138** | **2** | **u16 LE sample rate** | -| 150 | … | sample data | +| **150 or 192** | … | sample data | The name sits at offset **3**, not 4, and the valid byte at **15**, not 16. Both are off-by-one traps that produce names shifted by one character — readable enough to look like success. Sample data is **signed 16-bit little-endian mono PCM**. That is already exactly what a WAV data chunk holds, so writing a WAV is a copy with a header in front of it, not a conversion. There is no resampling, no bit-depth change and no dithering anywhere in this project. -S3000 discs may use a 192-byte header variant. Branch on the id and valid bytes rather than assuming 150. +### The header is 150 bytes or 192, and the directory says which + +**The S3000 family writes 192 and the S1000 family 150**, and the two are not distinguishable from the bytes in front of the audio — every field above sits at the same offset in both. What separates them is the **high bit of the directory entry's type byte**, the same bit that already names a kept original `.s3s` rather than `.s1s`. + +Three structures agree, none of them aware of the others: + +- **The type byte.** 13 451 of the 44 discs' 56 490 samples have it set and every one of them is 192; the other 42 989 are 150. No disc mixes the two rules and there is not one exception. +- **The word count.** The directory's declared size is `words × 2 + header_len` on **56 430 of 56 430** payloads readable at all — 13 441 at 192, 42 989 at 150. The 60 that fail it are the damaged ones, and every one of those also fails an identity test below. +- **The bytes.** On `AKAI.S3000.Sound.Library.2`'s `NPF E0`, offsets 150–170 are zero and 171–191 are `0a ff ff 22 a8 00 aa ff ff 00 8c ff ff 00 aa ff ff 00 88 ff ff` — the same shape on every sample of the disc, which audio is not. Waveform starts at 192. + +Nine of the 44 discs are affected: `AKAI.S3000.Sound.Library.1`–`7` (4 455, 3 086, 1 990, 1 010, 601, 168 and 218 samples), `East Connexion Piano` (730) and `AMG - Now CD-Rom for (AKAI)` (1 193). + +**Getting this wrong does not fail, which is why it lasted four deliverables.** Reading a 192 at 150 yields the right frame count, a WAV that opens, and a length within 0.1 % — with 42 bytes of header in place of the attack (a burst of roughly ±20 000 lasting 0.24 ms, an audible click), the last 21 frames of the sound gone, and every loop point 21 frames out. An earlier note here said S3000 discs *may* use a 192-byte variant and advised branching on the id and valid bytes. Both halves were wrong: the variant is not conditional, and those two bytes do not carry the answer — `0x80` appears on 42 989 samples at 150 and 13 410 at 192, and the id is `3` on both. See [ADR-0027](../adr/0027-a-payload-must-be-the-file-its-entry-placed.md). + +### The valid byte is a flag, not a value + +`0x80` is a bit within the byte and not the byte. 29 samples on `AKAI.S3000.Sound.Library.2` carry `0x81` and two on `Library.1` carry `0x9c`, with a correct id, a name matching their directory entry exactly, a rate of 44 100 or 22 050 and a word count the declared size agrees with. What the low bits mean is not established, and there is no third combination in the collection to check a reading against. + +### The payload repeats what the directory said, and the two must agree + +Every sample payload restates the file's id, its valid flag and its name, and the directory entry states the name and the size independently. Where they disagree, the payload is not the file the entry placed and it is refused rather than written under that entry's name — a WAV that opens, plays, and is somebody else's audio is the worst failure this format offers ([ADR-0027](../adr/0027-a-payload-must-be-the-file-its-entry-placed.md), [issue #23](https://github.com/bmxcode/samplerdisc/issues/23)). + +**65 of the 56 490 samples disagree**, on ten discs; the other 34 have none. What each test catches: + +| Test | Fires | Fires alone | +|---|---:|---:| +| id is not `3` | 61 | 1 | +| valid byte has no `0x80` | 60 | 0 | +| name is not the entry's | 60 | **0** | +| rate outside 4 000–50 000 | 58 | 4 | + +**The name comparison has no unique catch anywhere in the collection.** Every payload whose name disagrees also has a wrong id and a cleared valid flag, because on these images the displacement lands mid-audio and mid-audio does not look like a header. It is kept anyway: the other three ask whether the payload is *a* sample, and only this one asks whether it is *this* sample. + +Where they are: + +| Disc | Mismatches | Where | Partitions declared / present | +|---|---:|---|---| +| `Best Service - Alpha Dance II` | 21 | `AC.DRUMLOOPS`, last 21 of 22 | **6 / 6** | +| `Best Service - Alpha Dance I` | 15 | `ATTACK BANK2`, last 15 of 18 | 5 / 4 | +| `Kickin' Lunatic Beats 2 CD1` | 9 | `13-TRACK 06`, last 9 of 20 | 11 / 1 | +| `AKAI.S3000.Sound.Library.5` | 7 | `SURDO`, last 7 of 13 | 9 / 3 | +| `AKAI.S3000.Sound.Library.1` | 4 | `3084 B.BEAT6`, last 3 of 8; one rate | 13 / 13 | +| `AMG - Global Trance Mission 2` | 3 | `AMBIENT PAD2`, last 3 of 6 | 9 / 4 | +| `AKAI.S3000.Sound.Library.2` | 3 | three isolated rate bytes — 0, 519, 519 | 13 / 13 | +| `AKAI.S3000.Sound.Library.3` | 1 | `VOLUME 001`, its only file | 13 / 13 | +| `Audio Factory - Classical Wild Takes` | 1 | `VOLUME 002`, last of 2 | 11 / 6 | +| `AMG - Loop Soup` | 1 | `SOUP 101-103`, entry 27 of 39 | 9 / 9 | + +**60 of the 65 are a run to the end of one volume**, which is what a lost run of blocks looks like from inside a directory. The remaining five are single files: four with a corrupt rate byte and an otherwise perfect header — `EG 2MUTE` at 0 Hz, `M.VOICE A1` and `SYN 1` at 519, `HOUSE BASS` at 1280 — and `Loop Soup`'s one directory record whose start block lands mid-sample. Those four *are* the files their entries placed, with one field unusable, which is a different fault from the other 61 and is counted apart. + +**A tail run does not require a missing partition.** `Alpha Dance II` declares six partitions and holds all six; `Library.1` and `Library.3` likewise. Their damage is a run of blocks lost *inside* a partition, so no header goes missing and the table's declared-against-present arithmetic sees nothing. Declared equalling present is not a clean bill of health. ## Where a directory ends diff --git a/src/samplerdisc/batch.py b/src/samplerdisc/batch.py index 70d6a61..649a096 100644 --- a/src/samplerdisc/batch.py +++ b/src/samplerdisc/batch.py @@ -47,6 +47,11 @@ class DiscReport: #: audio was already written from another file on the same disc. Counted #: apart from ``skipped`` so a clean disc does not read as a damaged one. duplicates: int = 0 + #: Entries whose payload is not the file the directory placed there. + #: Counted apart from ``skipped`` because it is a different fault: the + #: filesystem and the data have come apart, which on these discs means the + #: image is short of the disc it was made from (ADR-0027). + mismatches: int = 0 audio_tracks: int = 0 skipped: list[dict[str, object]] = field(default_factory=list) error: str | None = None @@ -121,6 +126,8 @@ def convert_disc( elif isinstance(result, Skipped): if result.duplicate: report.duplicates += 1 + if result.mismatch: + report.mismatches += 1 report.skipped.append( { "volume": result.volume, @@ -128,6 +135,7 @@ def convert_disc( "name": result.name, "reason": result.reason, "duplicate": result.duplicate, + "mismatch": result.mismatch, } ) report.volumes = list(volumes.values()) @@ -158,6 +166,7 @@ def write_manifest(path: str, reports: list[DiscReport]) -> None: "originals": sum(r.originals for r in reports), "skipped": sum(len(r.skipped) for r in reports), "duplicates": sum(r.duplicates for r in reports), + "mismatches": sum(r.mismatches for r in reports), }, } directory = os.path.dirname(os.path.abspath(path)) diff --git a/src/samplerdisc/cli.py b/src/samplerdisc/cli.py index c1ea188..d3627a6 100644 --- a/src/samplerdisc/cli.py +++ b/src/samplerdisc/cli.py @@ -107,6 +107,7 @@ def cmd_extract(args: argparse.Namespace) -> int: kept = 0 skipped = 0 duplicates = 0 + mismatches = 0 results = extract_disc( image, origin.backend, @@ -138,6 +139,8 @@ def cmd_extract(args: argparse.Namespace) -> int: else: if result.duplicate: duplicates += 1 + elif result.mismatch: + mismatches += 1 else: skipped += 1 print(f" skipped {result.volume}/{result.name}: {result.reason}", file=sys.stderr) @@ -156,6 +159,12 @@ def cmd_extract(args: argparse.Namespace) -> int: # Not damage, and saying so matters: a disc that lists 423 skips reads # as a bad rip when every one of them is a sound already written. print(f"skipped {duplicates} duplicates of audio already written") + if mismatches: + # The loudest line on a short image, and the one ADR-0012's argument + # applies to: a payload that is not the file the directory placed is + # not "damage" in the sense the next line means, and lumping the two + # together is how nine wrong samples went unremarked on for a release. + print(f"skipped {mismatches} payloads that are not the file the directory placed") if skipped: # A disc that yields most of its samples is a good outcome; say so # plainly rather than burying it. diff --git a/src/samplerdisc/extract.py b/src/samplerdisc/extract.py index e9b8af7..bbb8627 100644 --- a/src/samplerdisc/extract.py +++ b/src/samplerdisc/extract.py @@ -19,7 +19,7 @@ from typing import TYPE_CHECKING, Protocol from samplerdisc.fs.base import original_suffix -from samplerdisc.sample import NotASample, aiff +from samplerdisc.sample import NotASample, PayloadMismatch, aiff from samplerdisc.sample.akai import parse from samplerdisc.stereo import find_pairs, interleave from samplerdisc.wav import LOOP_FORWARD, Loop, read_header, write_wav @@ -110,6 +110,13 @@ class Skipped: #: duplicate are both "skipped" and they are not the same news, so the #: summary must be able to tell them apart (ADR-0024). duplicate: bool = False + #: True where the payload is not the file this entry placed -- the + #: filesystem repeats a file's identity in its payload and the two + #: disagree. Counted apart for ADR-0024's reason one step on: "damaged or + #: unreadable" is true of a payload that is mid-audio and of one that is a + #: perfectly good sample under the wrong name, and only the second says the + #: directory and the data have come apart (ADR-0027). + mismatch: bool = False @dataclass @@ -208,6 +215,13 @@ def extract_volume( continue try: sample = _parse_sample(backend, entry, payload) + except PayloadMismatch as exc: + # Caught ahead of NotASample, which it subclasses: the payload may + # be usable audio and is still refused, because it is not the file + # the directory placed here and writing it puts one sample's sound + # under another's name (ADR-0027). + yield Skipped(volume.name, entry.name, str(exc), volume.partition, mismatch=True) + continue except NotASample as exc: yield Skipped(volume.name, entry.name, str(exc), volume.partition) continue diff --git a/src/samplerdisc/fs/akai.py b/src/samplerdisc/fs/akai.py index 30afebc..552b33c 100644 --- a/src/samplerdisc/fs/akai.py +++ b/src/samplerdisc/fs/akai.py @@ -68,7 +68,9 @@ "m": "multi", } -#: Sample payload header (docs/formats/akai-fs.md). +#: Sample payload header (docs/formats/akai-fs.md). The S1000 length; the S3000 +#: family writes 192, chosen by the type byte's high bit -- see +#: ``AkaiBackend.parse_sample`` and ``sample/akai.py``. SAMPLE_HEADER_LEN = 150 SAMPLE_ID = 3 PROGRAM_ID = 1 @@ -542,6 +544,40 @@ def layout(self, image: SectorImage, offset: int) -> str: return "1 partition" return f"{len(declared)} partitions declared, {present} present in this image" + def parse_sample(self, entry: File, payload: bytes): + """Parse one sample, telling the parser what the directory declared. + + Two things travelled on the ``File`` and are gone from the payload's + point of view, and both have to come back across here rather than being + sniffed out of the bytes: + + **The name**, so the payload can be required to be the file this entry + placed. The payload repeats it and nothing compared the two, which is + issue #23. + + **The generation**, because it chooses the header length. The type + byte's high bit is set on the S3000 family and clear on the S1000 one, + and it is already what names a kept original ``.s3s`` rather than + ``.s1s``. S3000 samples put 192 bytes in front of the audio and S1000 + ones 150; reading a 192 at 150 puts 42 bytes of header into the WAV as + PCM, drops the last 21 frames and leaves every loop point 21 frames + out. That was happening to 13 451 of the collection's 56 490 AKAI + samples. See docs/formats/akai-fs.md and ADR-0027. + + Imported here rather than at module scope: ``sample.akai`` imports the + charset and the name helpers from this module, so a top-level import + would be circular. ``Emu3Backend`` and ``RolandS7xxBackend`` do the + same for the same reason. + """ + from samplerdisc.sample import akai as sample_akai + + return sample_akai.parse( + payload, + fallback_name=entry.name, + declared_name=entry.name, + s3000=bool(entry.raw_type & 0x80), + ) + def original_suffix(self, entry: File) -> str: """Name an original after the machine that wrote it. diff --git a/src/samplerdisc/sample/__init__.py b/src/samplerdisc/sample/__init__.py index f584fea..7a85685 100644 --- a/src/samplerdisc/sample/__init__.py +++ b/src/samplerdisc/sample/__init__.py @@ -22,3 +22,14 @@ class NotASample(ValueError): Each format raises its own subclass, so a caller can catch this one thing rather than a tuple that grows by one every time a backend is added. """ + + +class PayloadMismatch(NotASample): + """The payload is not the file the directory placed here. + + Narrower than ``NotASample`` and a different fact: the bytes may be a + perfectly good sample, they are just not *this* sample. A filesystem that + repeats a file's identity in its payload can say so; the extract path + counts these apart from damage without knowing which filesystem it was + (ADR-0003, ADR-0024, ADR-0027). + """ diff --git a/src/samplerdisc/sample/akai.py b/src/samplerdisc/sample/akai.py index 0bdce7b..239e1f7 100644 --- a/src/samplerdisc/sample/akai.py +++ b/src/samplerdisc/sample/akai.py @@ -12,12 +12,30 @@ from samplerdisc.fs.akai import NAME_LEN, decode_name, is_plausible_name from samplerdisc.sample import NotASample as _NotASample - -#: S1000 header. S3000 discs may use a 192-byte variant. +from samplerdisc.sample import PayloadMismatch as _PayloadMismatch + +#: Two header lengths, and **which one applies is declared, not sniffed**: the +#: S3000 family writes 192 bytes and the S1000 family 150. The directory's type +#: byte carries the generation in its high bit -- the same bit that already +#: names a kept original `.s3s` rather than `.s1s` -- and it splits the 56 490 +#: samples of the 44 discs perfectly, 13 451 at 192 and 42 989 at 150, with no +#: disc mixing the two rules. +#: +#: The payload confirms it from the other side: the directory's declared size +#: is ``words * 2 + header_len`` on **56 430 of 56 430** readable payloads, and +#: the 60 that fail that identity are the damaged ones. Placed by one +#: structure, confirmed by another, the shape ADR-0020, ADR-0021 and ADR-0023 +#: already use. See docs/formats/akai-fs.md and ADR-0027. HEADER_LEN_S1000 = 150 HEADER_LEN_S3000 = 192 SAMPLE_ID = 3 + +#: The valid **flag**, tested as a bit and not as a whole byte. 29 samples on +#: `AKAI.S3000.Sound.Library.2` carry 0x81 and two on `Library.1` carry 0x9c, +#: with a correct id, a correct name, a plausible rate and a word count the +#: directory's size agrees with. Requiring the byte to equal 0x80 threw all 31 +#: away as damage (ADR-0027). VALID_FLAG = 0x80 #: Field offsets, all verified in docs/formats/akai-fs.md. @@ -55,6 +73,10 @@ class NotASample(_NotASample): """The payload does not begin with a sample header.""" +class PayloadMismatch(_PayloadMismatch, NotASample): + """The payload is not the file the directory entry placed here.""" + + @dataclass(frozen=True) class SampleLoop: """One loop, in frames. ``end`` is exclusive here; the WAV writer makes it @@ -80,34 +102,88 @@ def duration(self) -> float: return self.frames / self.rate if self.rate else 0.0 -def _looks_like_header(payload: bytes) -> bool: - return ( - len(payload) > OFF_RATE + 2 - and payload[OFF_ID] == SAMPLE_ID - and payload[OFF_VALID] == VALID_FLAG - and is_plausible_name(payload[OFF_NAME : OFF_NAME + NAME_LEN]) - ) +def _check_identity(payload: bytes, declared_name: str | None) -> str: + """Is this payload the file the directory placed here? Returns its name. + + Every AKAI sample payload repeats what the directory already said -- an id, + a valid flag and the name -- and until D19 nothing compared the two. The + tests run in the order a failure is most informative in, and each names the + field and both values, because "does not start with an AKAI sample header" + is true of a mid-PCM payload and of a wrong-but-valid one alike and they + are not the same news. + + **What the name test is for.** Across the 44 AKAI discs it fires 60 times + and never once on its own: every payload whose name disagrees also has a + wrong id and a cleared valid flag, because on these images the displacement + lands mid-audio rather than on another header. It is kept regardless. The + other three ask whether the payload is *a* sample; only this one asks + whether it is *this* sample, which is the failure issue #23 named and the + one a short image's recovered partitions would raise (#25). It is also what + makes the valid flag safe to read as a bit rather than as a byte. + See ADR-0027. + """ + if len(payload) <= OFF_RATE + 2: + raise NotASample(f"payload is {len(payload)} bytes, too short for a sample header") + + # Every disagreement is collected rather than the first one raised. On the + # 61 real mismatches the id is wrong on all of them, so stopping at the + # first would report "id is 179" every time and never once mention the name + # -- which is the test that says the payload belongs to a different file + # rather than to none. What the fields disagree about *together* is also + # the diagnosis: id, valid and name all wrong is a payload that is + # mid-audio, while a name alone is one sample's header under another's + # entry, and those want different answers from whoever reads the log. + wrong: list[str] = [] + if payload[OFF_ID] != SAMPLE_ID: + wrong.append(f"id {payload[OFF_ID]} not {SAMPLE_ID}") + if not payload[OFF_VALID] & VALID_FLAG: + wrong.append(f"valid byte 0x{payload[OFF_VALID]:02x} without the 0x80 flag") + raw = payload[OFF_NAME : OFF_NAME + NAME_LEN] + name = decode_name(raw) if is_plausible_name(raw) else "" + if not is_plausible_name(raw): + wrong.append("a name that does not decode in the AKAI charset") + elif declared_name is not None and name != declared_name: + wrong.append(f"the name {name!r}") + if wrong: + placed = f", placed by an entry named {declared_name!r}" if declared_name else "" + raise PayloadMismatch(f"payload header carries {', '.join(wrong)}{placed}") + return name + + +def parse( + payload: bytes, + fallback_name: str = "", + *, + declared_name: str | None = None, + s3000: bool = False, +) -> AkaiSample: + """Parse a sample file. Raises NotASample if the payload is not one. + ``declared_name`` is the name the directory entry gave this file, and where + one is supplied the payload's own name must match it. ``s3000`` says the + entry's type byte carries the S3000 generation bit, which is what chooses + between the two header lengths. -def parse(payload: bytes, fallback_name: str = "") -> AkaiSample: - """Parse a sample file. Raises NotASample if the payload is not one. + Both come from the directory rather than from these bytes, so they arrive + through ``AkaiBackend.parse_sample``; called bare, this parses a payload on + its own terms as it always did. Lengths are clamped to what is actually present: a truncated tail is common in these rips and yields a short sample rather than an exception. """ - if not _looks_like_header(payload): - raise NotASample("payload does not start with an AKAI sample header") - - name = decode_name(payload[OFF_NAME : OFF_NAME + NAME_LEN]) or fallback_name + name = _check_identity(payload, declared_name) or fallback_name pitch = payload[OFF_PITCH] (words,) = struct.unpack_from(" bytes: - """A 150-byte S1000 sample header followed by signed 16-bit LE PCM. + """A sample header followed by signed 16-bit LE PCM. ``loop`` is (start, end) in frames; the header stores the end and the length, not the start. + + ``header_len`` is 150 by default, the S1000 length. Pass 192 for the S3000 + variant: the fields are the same and the audio starts 42 bytes later, which + is what the type byte's high bit selects on a real disc. ``valid`` and + ``sample_id`` are overridable so a payload can be made *not* the file its + directory entry names, which no real disc in the collection provides. """ from samplerdisc.fs.akai import NAME_LEN, SAMPLE_HEADER_LEN, SAMPLE_VALID from samplerdisc.sample.akai import OFF_LOOP_RECORDS, OFF_LOOPS, OFF_TUNE_CENTS - header = bytearray(SAMPLE_HEADER_LEN) - header[0] = 3 + header = bytearray(SAMPLE_HEADER_LEN if header_len is None else header_len) + header[0] = sample_id header[1] = 1 header[2] = pitch header[3 : 3 + NAME_LEN] = akai_name(name) - header[15] = SAMPLE_VALID + header[15] = SAMPLE_VALID if valid is None else valid struct.pack_into(" FlatImage: + path = tmp_path / name + path.write_bytes(fixtures.akai_partition(volumes)) + return FlatImage(path) + + +def results(tmp_path, volumes, **kwargs): + image = disc(tmp_path, volumes) + return list(extract_disc(image, BACKEND, 0, str(tmp_path / "out"), **kwargs)) + + +def one_volume(entries): + return [("SOUP 101-103", entries)] + + +# --- the header length -------------------------------------------------- + + +def test_an_s3000_entry_reads_its_payload_at_192(tmp_path): + """The audio starts where the S3000 header ends, not where the S1000 one does. + + The assertion is on the *bytes*, against the payload the fixture built, so + it fails if the parser takes the audio from anywhere else at all -- not on + a frame count, which a 42-byte slip leaves very nearly right. + """ + payload = fixtures.akai_sample("PIANO C3", words=64, header_len=HEADER_LEN_S3000) + assert len(payload) == HEADER_LEN_S3000 + 128 + out = results(tmp_path, one_volume([("PIANO C3", S3000_SAMPLE, len(payload), payload)])) + written = [r for r in out if isinstance(r, Extracted)] + assert len(written) == 1 + sample = _parse(payload, s3000=True) + assert sample.header_len == HEADER_LEN_S3000 + assert sample.frames == 64 + assert sample.pcm == payload[HEADER_LEN_S3000:] + + +def test_an_s1000_entry_still_reads_its_payload_at_150(tmp_path): + payload = fixtures.akai_sample("KICK 1", words=64) + assert len(payload) == HEADER_LEN_S1000 + 128 + sample = _parse(payload, s3000=False) + assert (sample.header_len, sample.frames) == (HEADER_LEN_S1000, 64) + assert sample.pcm == payload[HEADER_LEN_S1000:] + out = results(tmp_path, one_volume([("KICK 1", S1000_SAMPLE, len(payload), payload)])) + assert len([r for r in out if isinstance(r, Extracted)]) == 1 + + +def test_reading_a_192_header_at_150_puts_header_bytes_in_the_audio(tmp_path): + """What the bug sounded like, asserted rather than described. + + The first 42 bytes of the WAV were the tail of the header and the last 21 + frames of the sound were gone. Both halves are checked, because a fix that + corrected the start and left the length short would pass the first alone. + """ + payload = bytearray(fixtures.akai_sample("PIANO C3", words=64, header_len=HEADER_LEN_S3000)) + # Something recognisable in the 42 bytes the S1000 length would swallow. + payload[HEADER_LEN_S1000:HEADER_LEN_S3000] = b"\xff\x7f" * 21 + wrong = _parse(bytes(payload), s3000=False) + right = _parse(bytes(payload), s3000=True) + # The head: 21 frames of header where the attack should be. + assert wrong.pcm[:42] == b"\xff\x7f" * 21 + assert right.pcm == bytes(payload[HEADER_LEN_S3000:]) + # The tail: the same 42-byte slip drops the last 21 frames of the sound. + assert wrong.pcm[42:] == right.pcm[: len(right.pcm) - 42] + assert right.pcm[len(right.pcm) - 42 :] not in wrong.pcm + + +# --- the identity checks ------------------------------------------------ + + +def test_a_payload_naming_another_file_is_refused(tmp_path): + """The check this deliverable exists for, and the only place it is exercised. + + ``ALPHA``'s directory entry points at a payload that is a complete, valid, + perfectly extractable sample -- of ``BETA``. Every other test in the suite + would pass on it: the id is 3, the valid flag is set, the name decodes, the + rate is 44 100. Only the comparison with the entry catches it, and without + that this writes BETA's audio into ALPHA.wav and reports success. + """ + payload = fixtures.akai_sample("BETA", words=64) + out = results(tmp_path, one_volume([("ALPHA", S1000_SAMPLE, len(payload), payload)])) + assert not [r for r in out if isinstance(r, Extracted)] + (skipped,) = [r for r in out if isinstance(r, Skipped)] + assert skipped.mismatch + assert "the name 'BETA'" in skipped.reason + assert "entry named 'ALPHA'" in skipped.reason + + +def test_a_payload_whose_id_is_not_a_sample_is_refused(tmp_path): + payload = fixtures.akai_sample("KICK 1", sample_id=179) + (skipped,) = _skips(tmp_path, [("KICK 1", S1000_SAMPLE, len(payload), payload)]) + assert skipped.mismatch + assert "id 179 not 3" in skipped.reason + + +def test_a_payload_without_the_valid_flag_is_refused(tmp_path): + payload = fixtures.akai_sample("KICK 1", valid=0x03) + (skipped,) = _skips(tmp_path, [("KICK 1", S1000_SAMPLE, len(payload), payload)]) + assert skipped.mismatch + assert "valid byte 0x03" in skipped.reason + + +def test_the_valid_byte_is_a_flag_not_a_value(tmp_path): + """0x81 is 29 real samples on `AKAI.S3000.Sound.Library.2`, and 0x9c is two + more on `Library.1`. + + Their id, name, rate and word count are all correct; requiring the byte to + *equal* 0x80 discarded all 31 as damage. This is what makes the name check + load-bearing rather than decorative: relaxing the flag is only safe because + something else still asks whether the payload is this file. + """ + for flag in (0x80, 0x81, 0x9C): + payload = fixtures.akai_sample("CHOIR", words=64, valid=flag) + assert _parse(payload, s3000=False).name == "CHOIR" + + +def test_an_implausible_rate_is_refused_but_is_not_a_mismatch(tmp_path): + """Four files on the shelf, and they are a different fact from the other 61. + + `EG 2MUTE` declares 0 Hz, `M.VOICE A1` and `SYN 1` declare 519 and + `HOUSE BASS` 1280 -- and on all four the id, the valid flag and the name + agree with the directory. These *are* the files their entries placed, with + one field unusable, so they are refused without being counted as the + directory and the data having come apart (ADR-0024's principle, ADR-0027). + """ + payload = fixtures.akai_sample("HOUSE BASS", rate=1280) + (skipped,) = _skips(tmp_path, [("HOUSE BASS", S1000_SAMPLE, len(payload), payload)]) + assert not skipped.mismatch + assert "implausible sample rate 1280" in skipped.reason + + +def test_a_program_is_never_condemned_by_the_sample_check(tmp_path): + """A program's payload id is 1, not 3, and it must survive that. + + Programs hold the key ranges and envelopes a WAV cannot carry and the disc + is the only copy, so `--keep-originals` writes them out verbatim. They + never reach the sample parser -- if the id test ever started reading them + it would condemn every program on every disc, and the symptom would be + files quietly missing from ``original/``. + """ + program = fixtures.akai_sample("BASS PRG", sample_id=1) + sample = fixtures.akai_sample("BASS SMP", words=64) + out = results( + tmp_path, + one_volume( + [ + ("BASS PRG", S1000_PROGRAM, len(program), program), + ("BASS SMP", S1000_SAMPLE, len(sample), sample), + ] + ), + keep_originals=True, + ) + kept = [r for r in out if type(r).__name__ == "Kept"] + assert sorted(k.name for k in kept) == ["BASS PRG", "BASS SMP"] + assert not [r for r in out if isinstance(r, Skipped)] + + +def test_parsing_bare_still_works_without_a_directory_entry(tmp_path): + """``parse`` called with no declared name skips only the name test. + + The backend always supplies one; this is the floor for a caller that has a + payload and nothing else, and it must not silently become a no-op check. + """ + from samplerdisc.sample.akai import parse + + assert parse(fixtures.akai_sample("KICK 1")).name == "KICK 1" + with pytest.raises(PayloadMismatch): + parse(fixtures.akai_sample("KICK 1", sample_id=179)) + + +# --- helpers ------------------------------------------------------------ + + +def _parse(payload: bytes, *, s3000: bool): + """Parse through the backend hook, which is where the two declared values + -- the entry's name and its generation bit -- are handed across.""" + from samplerdisc.fs.akai import NAME_LEN, decode_name + from samplerdisc.fs.base import File + + entry = File( + name=decode_name(payload[3 : 3 + NAME_LEN]), + kind="sample", + size=len(payload), + start_block=1, + raw_type=S3000_SAMPLE if s3000 else S1000_SAMPLE, + ) + return BACKEND.parse_sample(entry, payload) + + +def _skips(tmp_path, entries): + out = results(tmp_path, one_volume(entries)) + assert not [r for r in out if isinstance(r, Extracted)] + return [r for r in out if isinstance(r, Skipped)] diff --git a/tests/test_batch.py b/tests/test_batch.py index ade0ea9..db9e940 100644 --- a/tests/test_batch.py +++ b/tests/test_batch.py @@ -95,6 +95,7 @@ def test_manifest_records_totals_and_failures(tmp_path): "audio_tracks": 0, "skipped": 0, "duplicates": 0, + "mismatches": 0, } failed = [d for d in payload["discs"] if d["error"]] assert len(failed) == 1 diff --git a/tests/test_discs.py b/tests/test_discs.py index b9c0ca7..72782bb 100644 --- a/tests/test_discs.py +++ b/tests/test_discs.py @@ -108,6 +108,7 @@ def _pinned_sizes() -> set[int]: *(size for size, _, _ in _ISO9660.values()), *(size for size, _, _, _, _, _ in _EMU3.values()), *(size for size, _, _, _, _, _ in _AKAI.values()), + *(size for size, _, _, _, _ in _AKAI_PAYLOAD.values()), } @@ -911,6 +912,142 @@ def test_akai_keeps_the_files_of_a_volume_the_allocation_map_calls_free() -> Non assert total == 63 +#: AKAI discs pinned by what their **payloads** say, as opposed to what their +#: directories say above. ``label: (size, samples, s3000 headers, mismatches, +#: damaged)``, where the last two account for every sample not written: +#: a *mismatch* is a payload that is not the file its entry placed, and +#: *damaged* is one that is that file with a field unusable -- four corrupt +#: rate bytes across the collection, and nothing else (ADR-0027). +#: +#: The eight are chosen to cover every case the collection offers. Four whole +#: S3000 discs, because the 192-byte header was read as 150 on every sample of +#: them and a regression would be silent -- the WAVs would still open. Three +#: discs carrying mismatches, one of which (`Alpha Dance II`) declares six +#: partitions and holds all six, so its 21 refusals are damage the partition +#: table cannot see. And `Advance Orchestra`, which is 2 236 samples with +#: nothing wrong anywhere: the control that says these numbers measure the +#: discs and not the checks. +_AKAI_PAYLOAD = { + "AKAI.S3000.Sound.Library.1": (264_088_447, 4455, 4451, 3, 1), + "AKAI.S3000.Sound.Library.2": (298_155_354, 3086, 3083, 0, 3), + "East Connexion Piano": (277_092_352, 730, 730, 0, 0), + "AMG - Now CD-Rom for (AKAI)": (521_322_496, 1193, 1193, 0, 0), + "Best Service - Alpha Dance II AKAI": (309_865_547, 1740, 0, 21, 0), + "AMG - Kickin' Lunatic Beats 2 AKAI CD1": (378_443_564, 624, 0, 9, 0), + "AMG - Loop Soup AKAI": (542_419_100, 3434, 0, 1, 0), + "AKAI Advance Orchestra Upgrade 97 Vol.1": (545_720_320, 2236, 0, 0, 0), +} + + +@pytest.mark.parametrize("label", sorted(_AKAI_PAYLOAD)) +def test_akai_payloads_are_the_files_their_directory_entries_placed(label: str) -> None: + """Every payload accepted must be the file its entry named, at the right length. + + Two findings pinned together, because ruling out the false positive for one + is what found the other (ADR-0027). + + **The header length.** S3000-family samples put 192 bytes in front of the + audio and S1000 ones 150, and the directory's type byte says which in its + high bit. Reading a 192 at 150 does not fail -- it writes a WAV that opens, + holding 42 bytes of header as PCM at the front, 21 frames short at the end, + with every loop point 21 frames out. That was happening to **13 451 of the + collection's 56 490** AKAI samples, on nine discs, and four of them are + pinned here whole. + + **The identity.** 65 payloads across the 44 discs are not the file their + entry placed or are that file with a field unusable, and all 65 were + already being refused -- as "does not begin with an AKAI sample header", + which is true of a payload that is mid-audio and of one that is a perfectly + good sample under the wrong name alike. The mismatch count is pinned as + tightly as the sample count for ADR-0012's reason: a refusal appearing + where none was measured is a check that has started condemning real audio, + and one disappearing is a check that has stopped looking. + """ + from samplerdisc.sample import NotASample, PayloadMismatch + from samplerdisc.sample.akai import HEADER_LEN_S1000, HEADER_LEN_S3000 + + size, samples, s3000_expected, mismatch_expected, damaged_expected = _AKAI_PAYLOAD[label] + with open_image(_pinned_disc(label, size)) as image: + origin = find_origin(image) + assert origin is not None and origin.backend.name == "akai" + seen = s3000 = mismatched = damaged = 0 + for volume in origin.backend.volumes(image, origin.offset): + for entry in volume.samples(): + seen += 1 + payload = origin.backend.read_file(image, origin.offset, entry) + try: + sample = origin.backend.parse_sample(entry, payload) + except PayloadMismatch: + mismatched += 1 + continue + except NotASample: + damaged += 1 + continue + assert sample.header_len in (HEADER_LEN_S1000, HEADER_LEN_S3000) + if sample.header_len == HEADER_LEN_S3000: + s3000 += 1 + # The generation bit chose the length; the payload's own word + # count against the directory's declared size is what confirms + # it, and the two are written by different structures. Across + # the collection this holds for 56 425 of 56 425 accepted + # payloads -- which is what makes the rule a finding rather + # than a reading that happens to fit (ADR-0020, ADR-0027). + (words,) = struct.unpack_from(" None: + """The general statement, over whatever AKAI discs a contributor has. + + The tables above are this collection's; this is the invariant, and it is + the one issue #23 asked for: **no AKAI sample is written whose payload + header names a different file**. It holds trivially now, since such a + payload is refused -- which is the point. If a future change relaxes the + name test, or reads the header at an offset where another file's name + happens to land, this fails on any shelf rather than on ours. + """ + from samplerdisc.fs.akai import NAME_LEN, decode_name + from samplerdisc.sample import NotASample + from samplerdisc.sample.akai import OFF_NAME + + with open_image(path) as image: + origin = find_origin(image) + if origin is None or origin.backend.name != "akai": + return + wrong = [] + for volume in origin.backend.volumes(image, origin.offset): + for entry in volume.samples(): + payload = origin.backend.read_file(image, origin.offset, entry) + try: + sample = origin.backend.parse_sample(entry, payload) + except NotASample: + continue + header_name = decode_name(payload[OFF_NAME : OFF_NAME + NAME_LEN]) + if header_name != entry.name or sample.name != entry.name: + wrong.append( + f"partition {volume.partition} {volume.name}/{entry.name}: " + f"payload header says {header_name!r}" + ) + assert not wrong, ( + f"{path.name}: {len(wrong)} samples written under a name their payload " + f"header does not carry, first: {wrong[:3]}" + ) + + @pytest.mark.parametrize("path", _discs(), ids=_ids(_discs())) def test_an_akai_file_chain_is_as_long_as_its_declared_size(path: Path) -> None: """The allocation map has to agree with the directory about every file.