cuebin: add multi-BIN cue support - #242
Merged
Merged
Conversation
A redump-style rip stores one .bin per track and names each in its own FILE line. CUEParser already carried what was needed to place them -- filename and file_index per track, and a next_track() overload taking the previous file's size -- but nothing supplied the sizes, so every caller used the no-argument form and passed zero. With zero, each FILE after the first starts where the previous track's INDEX said rather than where its bytes ran out, and the tracks pile up on top of each other. Rather than thread a size through all 18 next_track() call sites, CUEParser now carries the table itself. set_file_sizes() is handed the device's sizes once, where SetDevice() adopts the image, and the no-argument next_track() selects the ending file's size when it crosses a FILE line. The explicit overload stays for cueutil, and a device that reports no sizes -- CHD, MDS, every single-file cue -- gets a zero and behaves exactly as before. The sheet itself is untouched: REM SESSION, REM LEAD-OUT, PREGAP and both INDEX forms are still read from the original text. Seek() space becomes the cue's data files concatenated in order, so the SCSI layer above keeps addressing one flat image and only the mapping underneath changes. Read() no longer stops at a .bin boundary; it reads from each file in turn until the caller's byte count is satisfied, because a short read is a medium error to the gadget and a failed transfer to the host. Reading at exactly GetSize() returns 0 without logging, past it is still an error, and a cache window still never spans two files. Every file is opened at mount and held open, since opening on demand would put an f_open in the path of an audio read, which is the one place that cannot afford it. The loader uses the FILE names only when there is more than one, resolving them against the cue's own directory. A single-FILE cue still takes its data file from the cue's own name: rips with a stale FILE line next to a correctly named .bin are common and work today. A multi-FILE sheet is refused unless every file it names is present, openable and larger than zero, and the error says which one is at fault. Zero bytes was the failure that looked like success -- David's Alien Trilogy rip had tracks 3 and 7 uploaded empty, and they opened without error, contributed nothing, and slid every later file's base down on top of them. CueResolveLBA likewise refuses a multi-file sheet with a short size table rather than inventing a plausible layout. A FILE name is also checked before anything opens it. FatFs runs with FF_FS_RPATH 2 and takes both slashes as separators, so an absolute name or a ".." component would resolve outside the directory holding the cue; both are refused, naming the entry at fault. "." components and nested relative paths are left alone because "./track.bin" and "tracks/track01.bin" are ordinary. For the same reason GetByteOffsetForLBA no longer falls back to the single-file offset helper when a split rip's LBA cannot be resolved. That answer assumes one flat .bin, so it addresses the wrong file; it now returns an invalid offset and lets Seek() fail instead of serving wrong bytes. The single-file path is unchanged. Two defects this exposed. GetLeadoutLBA measured the last track against the whole image, which is right for one file and wrong for a concatenation, so a split rip's leadout landed at 738 instead of 438; it now measures against the file the last track lives in. And READ TRACK INFORMATION kept a CUETrackInfo pointer across a second next_track(), which overwrites the struct it points into, so trackLength came out as next minus itself and every track on every image reported 0 sectors. That one was never split-rip specific. The tests mount through loadCueBinIsoFileDevice rather than a helper that copied its logic, which is what makes the enumeration and the cue-relative path resolution covered at all. That meant linking util.cpp into the suite: it supplies the real FatFsOptimizer, so the harness stub is gone, and its CHD branch is compiled out in the no-libchdr build. The mixed-mode fixture is the tracked ISO wrapped into real MODE1/2352 sectors, so the test reads LBA 16 and checks a genuine ISO9660 Primary Volume Descriptor, and pins medium type 0x03. Each data file gets its own FatFs fast-seek link map. Only the file the device was constructed with used to get one, so every seek into an adopted file walked the FAT chain -- and a split rip keeps its audio tracks in exactly those files. On a USB 1.1 host there is no slack to absorb that: the audio queue drained and playback broke up. A CLMT describes one file and is never shared; it stays best effort, so a file whose map cannot be built is still accepted and reads normally. The host FatFs seam counts CREATE_LINKMAP requests so the tests can pin one map per .bin. Verified the empty-file check is load-bearing: with it disabled the new test mounts the broken image instead of refusing it, and with per-file fast seek removed the link-map count drops to 1 and its test fails. Verified on hardware with a 17-file image and a separate 13-file image across three machines -- data track and all audio tracks present, the previously silent tracks 3 and 7 play, and a 440LX USB 1.1 host that crackled on a split rip now plays clean. 184 tests pass, 187 with WITH_CHD=1, 184 under ASan/UBSan. Refs danifunker#241
David's split rip came up with tracks missing. An unstored PREGAP costs disc addresses but no file bytes, so the parser carries it in cumulative_offset and adds it when it turns an INDEX time into an LBA. At a FILE line the next file's start was computed from the previous track's data_start, which already had cumulative_offset folded in -- so the gap was charged again, and every track after the first PREGAP moved that far too late. A 150-frame gap put track 3 at LBA 600 instead of 450, and the tracks walked off the end of the disc the leadout described. Single-file cues never saw it because file_start stays 0 for them, and the split-rip tests that existed used stored pregaps (INDEX 00), which cost file bytes and leave cumulative_offset at zero. The fix subtracts the offset back out where file_start is built, so the INDEX lines add it exactly once. Refs danifunker#241
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
1 participant
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.
Fixes #241.
Adds support for cue sheets with multiple FILE entries, the layout redump-style rips use when each track is its own .bin.
How it works. Actual data-file sizes are threaded into the cue track calculations, without which the parser cannot place any file after the first. The referenced bins are treated as one concatenated logical seek space, so the SCSI layer above still addresses a single flat image; each LBA resolves to a source file and a local offset within it, and a read that reaches a file boundary continues into the next file until the request is satisfied. Every referenced file gets its own FatFs fast-seek link map, without which seeks into the later files walk the FAT chain.
Validation. A multi-FILE cue is refused, naming the entry at fault, if a referenced file is missing, is zero bytes, or uses an unsafe absolute or parent-directory path. Empty files were the real-world failure: they open successfully, contribute nothing, and collapse every later file's base into an invalid TOC. Names are checked before anything is opened.
Single-FILE cues are unchanged, including the same-stem .bin fallback for sheets with a stale FILE line.
Also corrects two defects this exposed: an unstored PREGAP was charged twice when it crossed a FILE boundary, and READ TRACK INFORMATION retained a parser pointer across a second next_track() call, which made it report every track as zero sectors long on any image.
Tests. Split audio, mixed-mode data with a genuine ISO9660 PVD read at LBA 16, cross-boundary reads, EOF, missing files, empty files, unsafe paths, stale FILE names, one link map per .bin, PREGAP, TOC and lead-out. 184 pass normally, 187 with WITH_CHD=1, 184 under ASan/UBSan.
Hardware. Verified with 17-file and 13-file images across three systems. Per-file FatFs fast-seek maps eliminate split-image audio underruns on an Intel 440LX USB 1.1 host; data and all audio tracks remain available.
FTP upload reliability is outside this PR and is tracked in #243.