-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Import] Scanner cannot real dual-layer Wii discs #6544
Comments
Could you provide the CRC of the four tests you provided? |
|
It's actually not only dual layer discs. And has nothing to do with CRCs (my example is a redump file anyway), because for the Wii it's scanning by serial. It's not reading:
in spite of the data being on the Wii dat and redump dat. It finds the serial and bails out:
So i think there is something wrong with |
It actually fails at It's detecting at least 'Another Code R' (and several others in my game list) erroneously as gamecube games because Now, i'm wondering if there isn't a better way to detect Wii discs than 'didn't detect'. |
Ok, i have a idea. In
(ps1, scd and gc start with \00) The function searching for the signatures uses this code: Now imagine that the offset of the disc (that was read into magic) started with \00 string \00 is equal to \00 right? Basically, don't use string_is_equal to compare fixed size file offsets? Let's see the definition of it on
And a test:
Prints WRONG! because the 'string' ie: the characters until the first inequality or \00, are indeed equal and their subtraction == to 0. Needless to say, this should check the size of the magic bytes... the only reason it's not saying it's a 'scd' game instead of gamecube is that offset zero of the iso starts with the serial. The only reason not all wii isos aren't 'gc' is if you get lucky and the ofset of the gc magic starts on a non-null. There isn't actually a I'd also check other uses of string_is_equal to see if someone screwed up on its use on other places. |
BTW segacd 'search' is fragile too, because the offset is only taking into account 'MODE1/2352' cds. MODE1/2048 (ie: iso) is also a perfectly viable format even on a cue/iso and it's often used for translations or hacks of segacd or saturn or dreamcast (with the tracks separated so needs a cue). You should find a better way to figure out cds for scd because of this, something like this maybe:
ps1 doesn't need this because to convert to iso as bchunk says:
ie: mode2/2352 cds always keep the first 24 bytes when converting to iso, it's only MODE1/2352 (which is the redump format for scd, dreamcast and saturn) that have to worry about getting the first 16 bytes cut off I'm also surprised you don't try to search for serials from saturn or dreamcast. |
Replacing that if by |
Here's a question: How hard is it to isolate the database task into a separate program for the purposes of unit testing? Normally I'm not a TDD guy but if there's a way for me to just load up a filename and send it straight to the task without loading up all of RA I'd totally be up for stepping through this line-by-line. (Gotta do something while my REDACTED devkit is in limbo...) (Also argh, that strcmp - if nothing else comes out of this, let's at least turn that into a memcmp!) |
I'm totally a TDD guy, and while I haven't quite been able to set up an easy testing environment for RetroArch's database tasks, there is libretro-db/c_converter.c and the other libretro-db tools, which allow interacting with the .rdb files. In the RetroArch code, I've found good ol'
Haha, yes... I miss |
Guess we'd need to make a standalone program then that still implements the task system. This could actually be feasible since I think the task-based code is fairly well compartmentalized from the rest of the RetroArch code. |
Definitely! I did notice that the whole task was isolated in its own world in the source, but I wasn't sure that it could build as a lazy
Maybe it's all the FNA experience, but any time I can replace "string" classes with plain ol' C operations is welcome to me >_< |
I will try to take some stabs at making the database scanning a standalone program as well using just the plain libretro-common/RetroArch code parts. You will need to be running at least a shell in order to use it though, it will be a commandline program only. |
That sounds great! If this works out just ping me when the commits are in and I'll tear through this ASAP. |
BTW, import of wii dual layer discs fails at a filesize check when searching for serial here: https://github.com/libretro/RetroArch/blob/master/tasks/task_database.c#L226 at a guess it would also fail in the exact same check here (for crc): https://github.com/libretro/RetroArch/blob/master/tasks/task_database.c#L378 it seems that 'intfstream_tell' return result has to accommodate really large results. And the rest of the interface probably. Unless you feel like making a specific stream just for dual layer dvds. |
OK, I successfully converted this into a standalone program @flibitijibibo - go to samples/tasks/database, and run make. Basic usage - $ ./database_task.exe
database dir - path to the RDB database directory Here is a test invocation of the program - $ ./database_task.exe "C:/Games/RetroArch/database/rdb" "C:/Games/RetroArch/cores" "C:/Games/RetroArch/info" "C:/Users/libre/Downloads/Games/Saturn" "C:/Games/RetroArch/playlists" I turned verbose logging on so that is why it might be slower than usual. I hope that it will compile and work out of the box on both Windows and Linux/OSX. So far I tested it on Windows and it worked. And it compiled, linked and ran at least on OSX, so fingers crossed. |
@i30817 Can you try this patch? http://p.0bl.net/182560 |
Well, good news and bad news. Good news is that it made the Last Story (one of the two 'dual layer' discs i have) scan. Bad news is that it didn't work for Metroid Prime Trilogy or the 'Another Code R' (this one isn't dual layer). Seems like there are other bugs interfering. I'll add the memcmp fix back and see if that's it. In master none of the 3 scan (and it isn't that other problem of the the dots or they not being redump either because their filenames or paths have no dots and they're verified redump). So there is at least one more bug in addition to the dot, the memcmp and this 64 bit file offset thing. |
I should have mentioned that I edited the post earlier with an updated patch, maybe just double check that you're using the same one that's currently in the post. Otherwise, not sure what else is wrong at the moment. |
Your patch should still be applied @bparker06, since it fixes the scan of at least one game, you should open a PR. Also could you please add a commit for the memcmp fix too? edit: you could add my open commit about the scanner missing directories with '.' in the name too. |
edit: groaaan. Try to apply the patch, delete your database/rdb directory and update the database again, because there was bug about that @flibitijibibo That fixed the other problems for me (along with the dot PR because one of the games has '(v1.01)' in the name of its directory). I simply had a outdated database. At least two bug fixes came from this wild goose chase. |
Stepped through and can confirm @bparker06's patch resolves this issue, the file was indeed too large! |
I'm going to close this now if the current nightly indeed resolves this. |
Do you still happen to have the patch from http://p.0bl.net/182560 ? Unsure if it was pushed up in a PR. |
Filesize is int64_t in my link above now so i'd expect so (though maybe not a PR and just pushed directly). In fact, git blame from github interface makes it easy to see the commit: 1751f4a |
Oh great! Glad it got pushed. Thanks a lot, Twin, i3 + flibitijibibo. |
Description
When attempting to add dual layer Wii discs to the playlist via the Scan Directory/Scan File function, the process will fail to add them to the list due to an unknown bug in the file detection system.
Expected behavior
The serial number would get read from the top of the ISO, recognized by the database, and added to the Wii playlist.
Actual behavior
From what I can tell, we don't get as far as the serial detection. The serial is valid and exists in the database, but typically when you read in a Wii disc you get something like this in the log...
... but for DL discs that never happens. So presumably this function never trips...
RetroArch/tasks/task_database.c
Lines 166 to 172 in 0ee1245
... which means this doesn't happen...
RetroArch/tasks/task_database.c
Line 603 in 0ee1245
... so for some reason these files in particular don't get iterated on by the database task. The only significant difference between SL and DL for ISO files is the size; SL discs are 4699979776 bytes while DL discs are 8511160320 bytes. Perhaps the file is just too big...? Unfortunately since I'm using the Flatpak version I can't step through and see exactly where things go wrong.
Steps to reproduce the bug
Tested files are Metroid Prime Trilogy, Super Smash Bros. Brawl, Metroid: Other M, and Xenoblade Chronicles.
Version/Commit
Environment information
The text was updated successfully, but these errors were encountered: