Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP saturn cue/bin parsing #232

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

WIP saturn cue/bin parsing #232

wants to merge 2 commits into from

Conversation

sozud
Copy link
Collaborator

@sozud sozud commented May 22, 2023

The Saturn cue/bin I have looks like this:

FILE "Akumajou Dracula X - Gekka no Yasoukyoku (J)(Saturn).BIN" BINARY
  TRACK 01 MODE1/2352
    INDEX 01 00:00:00
  TRACK 02 AUDIO
    PREGAP 00:02:00
    INDEX 01 45:30:63

I attempted to add MODE1/2352 support. It's able to extract some of the files, which seem to be correct, but some are missing. I'm not sure why that's happening. Maybe the Saturn disk uses some filesystem features the PSX one doesn't?

I had to change the track adding code to add more than one track per file, otherwise track 1 gets replaced with the audio track.

@sozud
Copy link
Collaborator Author

sozud commented May 22, 2023

It's rejecting sectors that don't meet the offset+bufSafe > secSize requirement here. Maybe the missing files are there. Could you explain what the fix to this code would be?

		// horrible hack as it can read unnecessary bytes, but it works
		if offset+bufSafe > secSize {
			sec, err := readSector(file.reader, location(chloc), file.mode, false)
			if err != nil {
				return nil, err
			}
			data = []byte(sec)
			offset = 0
			chloc++
		} else {
			fmt.Print("rejected sector\n")
		}

@Xeeynamo
Copy link
Owner

// horrible hack as it can read unnecessary bytes, but it works

I do not remember it very well, but I think it was related to the fact in rare scenarios readSector needed to read either extra bytes or simply bytes overflowing to a new sector. That bufSafe is a buffer of safety to not crash the app in those rare scenario. I think I tweaked close to the minimum value before crashing. I am not very proud of that.

Xeeynamo pushed a commit that referenced this pull request May 14, 2024
In CUE files `FILE` is a stateful, global declaration that applies to
all following `TRACK`s. `sotn-disk` was treating `FILE` declarations as
`TRACK` delimiters which would result in incorrect parsing of CUE files
with a single `FILE`, but multiple tracks.

Now when reading a `FILE` declaration, the path is stored and processing
continues. A `TRACK` declaration will use the previously defined path,
and if a previous track had been started, append that previous track to
the list.

This also builds `sotn-disk` using the local repository instead of
pulling the latest commit from GitHub. The target depends on `sotn-disk`
sources and will rebuild as necessary (or with `make
~/go/bin/sotn-disk`).

As an aside,
[pull/232](#232) ran into
this same error, but fixes in a slightly different way that leaves side
effects that may make supporting things like `INDEX` (for extracting the
placeholder audio, for example) more error prone in the future.

Co-authored-by: Jonathan Hohle <[email protected]>
Xeeynamo pushed a commit that referenced this pull request Aug 30, 2024
In CUE files `FILE` is a stateful, global declaration that applies to
all following `TRACK`s. `sotn-disk` was treating `FILE` declarations as
`TRACK` delimiters which would result in incorrect parsing of CUE files
with a single `FILE`, but multiple tracks.

Now when reading a `FILE` declaration, the path is stored and processing
continues. A `TRACK` declaration will use the previously defined path,
and if a previous track had been started, append that previous track to
the list.

This also builds `sotn-disk` using the local repository instead of
pulling the latest commit from GitHub. The target depends on `sotn-disk`
sources and will rebuild as necessary (or with `make
~/go/bin/sotn-disk`).

As an aside,
[pull/232](#232) ran into
this same error, but fixes in a slightly different way that leaves side
effects that may make supporting things like `INDEX` (for extracting the
placeholder audio, for example) more error prone in the future.

Co-authored-by: Jonathan Hohle <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants