DiscIO: Rework the implementation of TGC reading #8644
Merged
Conversation
This file contains 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
JosJuice
added a commit
to JosJuice/dolphin
that referenced
this pull request
Apr 28, 2020
This should not be exposed to users before the merge of PR dolphin-emu#8644. (PR dolphin-emu#8738 was unintentionally merged before PR dolphin-emu#8644.)
dreamsyntax
pushed a commit
to dreamsyntax/dolphin
that referenced
this pull request
May 11, 2020
This should not be exposed to users before the merge of PR dolphin-emu#8644. (PR dolphin-emu#8738 was unintentionally merged before PR dolphin-emu#8644.)
Fixes https://bugs.dolphin-emu.org/issues/10654. To quote the documenation file included with the program tgctogcm: "TGC's are miniaturized .gcm images with a 32kB header. The embedded gcm contains some bogus data, namely: -FST Location (0x424 in gcm) -DOL Location (0x420 in gcm) -FST File offsets (all files are offset/spoofed by a certain amount)" Dolphin has been handling the values at 0x420 and 0x424 by simply overwriting them with a working value (just like tgctogcm does), but it has used a different approach for the file offsets in the FST. Instead of changing the offsets that are stored in the FST, Dolphin changed where the files actually are placed on the virtual disc. My hope was that this would make the loading times more accurate to how they are when running a TGC file as part of a larger disc. However, there are TGC files where we would need to move files backwards on the disc in order to do this (this is what issue 10654 is about), so the approach we have been using is flawed. This change makes Dolphin overwrite offsets in the FST instead, like tgctogcm does. Other than making Dolphin handle the affected TGC files correctly, this change also makes it so that unnecessary padding data isn't written if you use Dolphin to convert a TGC file to an ISO file. This feature is not actually implemented in Dolphin as of now, but I'm planning to add it in the near future as part of a larger feature.
This reverts commit c236d89.
This reverts commit d006a8b.
|
Has this been tested by anyone? If not, what testing does it need? |
|
I can confirm this fixes playing a demo tgc that crashed unless converted to gcm. Same for extracting files that otherwise got corrupted. |
|
I did a similar amount of testing to PPLToast when I created the PR. I'm not aware of anything additional that needs to be tested, unless you have something in particular. |
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.
Fixes https://bugs.dolphin-emu.org/issues/10654.
To quote the documenation file included with the program tgctogcm:
Dolphin has been handling the values at 0x420 and 0x424 by simply overwriting them with a working value (just like tgctogcm does), but it has used a different approach for the file offsets in the FST. Instead of changing the offsets that are stored in the FST, Dolphin changed where the files actually are placed on the virtual disc. My hope was that this would make the loading times more accurate to how they are when running a TGC file as part of a larger disc. However, there are TGC files where we would need to move files backwards on the disc in order to do this (this is what issue 10654 is about), so the approach we have been using is flawed.
This change makes Dolphin overwrite offsets in the FST instead, like tgctogcm does. Other than making Dolphin handle the affected TGC files correctly, this change also makes it so that unnecessary padding data isn't written if you use Dolphin to convert a TGC file to an ISO file.
This feature is not actually implemented in Dolphin as of now, but I'm planning to add it in the near future as part of a larger feature.