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

Serial Scanning Removal for disk CRC scanning. #3570

Closed
wants to merge 4 commits into from
Closed

Serial Scanning Removal for disk CRC scanning. #3570

wants to merge 4 commits into from

Conversation

ValiantBlade
Copy link

Removed the ISO and CUE serial scanning to prevent a conflict that will occur when the disk CRC scanning is merged.

Removed the ISO and CUE serial scanning in task_database.c in order to allow for the eventual Redump based verification.
@andres-asm
Copy link
Contributor

But serial scanner is faster why remove it?

On Sun, Sep 11, 2016, 6:44 PM ValiantBlade notifications@github.com wrote:

Removed the ISO and CUE serial scanning to prevent a conflict that will
occur when the disk CRC scanning

libretro/libretro-database#251 is merged.

You can view, comment on, or merge this pull request online at:

#3570
Commit Summary

  • Modified task_database.c

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#3570, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABpC0AYFvKom1Xbr2nDuBva9-6AR9d5qks5qpJJBgaJpZM4J6Iz2
.

@ValiantBlade
Copy link
Author

ValiantBlade commented Sep 12, 2016

Well, among other things, the serial scanner actually does not work with certain games, and the scans are still pretty quick regardless for the new CRC method. I tested the CRC scanning, I couldn't notice a major time difference, besides, this is mostly based on scanning the cues, and the cue files are all less than 500 bytes each, meaning the scans are only slow when you scan the whole directory and it tries to compare the bins. Also, having both in at the same time cause a huge conflict. Some games also automatically fail the serial scan due to having bad serials, as I discovered with Dragon Warrior VII. The largest problem is that older ROM collections usually won't work, but that's only a problem with large collections, and Redump ROMs are actually somewhat easier to find than No-Intro ROMs.

@orbea
Copy link
Contributor

orbea commented Sep 12, 2016

Serial scanning just works with fan translations, undubs and other hacks. CRC scanning doesn't (Without updating the database a lot).

@ghost
Copy link

ghost commented Sep 12, 2016

Another thing to keep in mind is that when we go to add database support for Dreamcast, the .bin magic string happens to be the same as Playstation, so the current scanning method is inadequate for that, and with CRC we would have the same issue... it doesn't make sense to have to scan both systems to find one or the other.

@ValiantBlade
Copy link
Author

@orbea, the CRC scanning is based on the cue file, which means that if only the bin is modified (as it should be, considering how sometimes you can find a ROM that's just a cueless bin, or with a really bad cue, like the SoTN cue I found that had a filepath to the dumper's desktop), the fan translations/hacks should work out of the box regardless (when hard patched, because soft patching is only supported with ips, ups, and bps formats). To make sure this was the case, I picked a random fan translation, Suikoden's Spanish Translation, and I looked through the readme, using the help of google translate, and I was able to discern that the translation patches the bin file, and not the cue file, meaning that it should work. I would firsthand test it, but I'm not really experienced in patching PS1 games, all the patching I've done in the past was on SNES and GBA.

@ValiantBlade
Copy link
Author

ValiantBlade commented Sep 12, 2016

@bparker06, Dreamcast is probably going to be handled through a separate method, using a file extension filter, probably in the same method used to single out cues and isos for serial scanning, because, Dreamcast ROMs are in the gdi and cdi formats instead of the bin/cue or iso formats (might be in ISO format actually, but a majority seems to be gdi and cdi). I'm not involved in the process of adding Dreamcast support, but that's one way it could work.

@ghost
Copy link

ghost commented Sep 12, 2016

@ValiantBlade My Dreamcast games are all bin/cue, so I would hate to lose the ability to play them easily or be included in playlists when that happens.

@ValiantBlade
Copy link
Author

Well, you can always add custom playlist entries. Besides, your current playlist should remain fine, the problems will come when adding new ROMs. Just make sure to keep an extra copy of the playlist file, and that when you finish editing the line count is a multiple of six, and that there are no blank lines. All you really need to add to there are the name, file path, and a few other things. Trust me, I know the feeling, a large amount of my PS1 ROMs are from before I knew what Redump was, and most of them are ccd/img/sub formatted.

@kivutar
Copy link
Member

kivutar commented Sep 12, 2016

@fr500 I'm not sure that finding the serial is faster than computing the CRC of a .cue

For a list of pro/con, see this libretro/libretro-database#251

@ghost
Copy link

ghost commented Sep 12, 2016

Checking the CRC of a .cue also adds the restriction of having exact matching filenames, whereas I believe with the serial scanning, the files can be named anything (but thumbnails won't match?).

@ValiantBlade
Copy link
Author

ValiantBlade commented Sep 12, 2016

Thumbnails also require exact filenames. Technically, you could use a older bin if you rename it to the same name as a Redump cue, which are all availible on the Redump website, as posted in the pull request linked in the original post and Kivutar's post.

@cudencuden
Copy link
Contributor

well, i think most should be okay with CRC of .cue because most have scene released sets..meaning name of bins/cues are from the scene dumps

@ValiantBlade
Copy link
Author

ValiantBlade commented Sep 12, 2016

@cudencuden, this is mostly a problem for people with older ROMs, or those who were unaware of ROMsets when they originally searched for them. It takes a LONG time to replace every non-working ROM. It's also a problem for PSP ROMs, but I think we're switching to the No-Intro PSP set along with the PS1 changes, and if not I can revert the changes on ISOs, and even then lr-PPSSPP has SO many problems and needs to be majorly redone. Either way, this pull request is 100% conflict removal and deleting functions that will become unused.

@kivutar
Copy link
Member

kivutar commented Sep 13, 2016

PSP shouldn't be concerned by this pull request, because PSP use ISO without CUE.

But the PR should be updated: only this case https://github.com/libretro/RetroArch/pull/3570/files#diff-349b6a34634a6cf2ab5ab9a790d4a2adL205 should be removed, the rest of the code should be kept.

@orbea
Copy link
Contributor

orbea commented Sep 13, 2016

Checking the crc of the cue files does not seem like a good idea. Cue files in the wild are inconsistent and extremely easy to edit.

@kivutar
Copy link
Member

kivutar commented Sep 13, 2016

@orbea we were talking about cue files from the scene, like REDUMP.

In fact, it's either we merge that, either we have to implement serial scanning for all the system that supports CD. (This could be done by reading how mednafen does it)

@orbea
Copy link
Contributor

orbea commented Sep 13, 2016

I realized, just that not everyone has those or will get them. This seems like replacing an imperfect solution with an inherently flawed one.

@kivutar
Copy link
Member

kivutar commented Sep 13, 2016

Yeah... I agree that asking the users to edit their filenames and cue files is as complex as asking them to edit the playlist manually...

Cartridge romsets are easy to find. But I wasn't able to find a redump romset myself.. so asking users to find it is a dead end.

@tony971
Copy link

tony971 commented Sep 13, 2016

Cartridge romsets are easy to find. But I wasn't able to find a redump romset myself.. so asking users to find it is a dead end.

There's a set linked in the emulation wiki. Not a dead end

Edit: There are sets on a system by system basis. I haven't checked Dreamcast or PSP.

@cudencuden
Copy link
Contributor

cudencuden commented Sep 13, 2016

yes, dreamcast or psp sets are dumped by one of these groups: redump, trurip, no-intro, and ADVANsCEne

@kivutar I think redump only do disk based sets....no cartridge

@RobLoach
Copy link
Member

Two new issues in https://github.com/robloach/libretro-dats/ for adding Advancescene and Darkwater DATs RobLoach/libretro-dats#3 RobLoach/libretro-dats#2

@ValiantBlade
Copy link
Author

@kivutar The reason iso files were changed there is because I've seen and even have an iso ROM. But, I'm going to go change that back, because looking back, iso ROMs are definately rarer than ccd/img/sub formatted ROMs. Also, there is an unused function warning caused by the case removal for the cue_get_serial function, so that's why the function was removed. Using the Redump cues all you really need to do is rename the bin to match the cue (which can be done in a few seconds using a copy and paste, still tedious with large collections, but maintaining those is always a chore), unless the game has multiple tracks, such as No One Can Stop Mr Domino, in which case the Redump is odd because the game is split into two bins. Either way, no matter which method we use for verification, there are going to be flaws no matter what. It's really just my opinon that CRC scanning is a better option, simply because I feel basing off of known ROMsets is a better option than just checking serials, which can be on broken ROMs, and can be lacking from working ROMs.

db->type = DATABASE_TYPE_SERIAL_LOOKUP;
break;
case FILE_TYPE_ISO:
case FILE_TYPE_ISO:
db_state->serial[0] = '\0';
iso_get_serial(db_state, db, name, db_state->serial);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like AdvanceSCENE provides CRCs for ISOs. What is the remaining system that uses Serial checking?

Copy link
Member

@RobLoach RobLoach Sep 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Sony - PlayStation
  2. Sony - PlayStation Portable
  3. Sony - PlayStation Portable Vita

As long as the DATs we provide have CRCs, then we could switch over.

@ValiantBlade
Copy link
Author

ValiantBlade commented Sep 13, 2016

I guess I'll remove iso serials again? I'm going to wait until some discussion happens. I'll just reverse the commit when we're absolutely sure it won't be needed. I don't really want to risk anything here.

@orbea
Copy link
Contributor

orbea commented Sep 14, 2016

My understanding is that with serials a few games will not work and it also fails to distinguish between different versions of the same game that share serials. While with scanning for the crc of the cue file unless it comes from a specific source that may be difficult to find or potentially illegal it will not work. I can understand why some may like that, but it does seem like a good standard and breaks several use cases.

For example what if someone has a specific naming conventions they painstakingly implemented for their files? Or what if someone wants to rename the .bin and .cue to reflect its a fan translation or a hack?

I would argue that asking users to edit playlists manually for an occasional game is a lot less than it is to ask them to edit or replace all their files.

@ValiantBlade
Copy link
Author

ValiantBlade commented Sep 14, 2016

@orbea, I notice most of these problems are ones shared with using CRC on cartridge format, yet everyone's happy with those just fine, so why is the PS1 any different? Also, there really isn't a need for complex naming conventions outside of being a borderline mental illness level perfectionist, when all of these ROMset ROMs already come in a proper naming convention that should work well for most people (and the Retroarch names are stored in the playlist file so anyone who wants a different naming convention will probably look there first). Going back to your point on using a custom playlist entry for a specific game, you could easily do exactly that if you want specific names for ROMhacked games. As for obtaining the CRCs of said cues, well, there are databases full of this information, and also the serial information is actually harder to obtain than the CRC information, as mentioned by Kivutar in his pros and cons list. You can download all the cues from Redump en masse, and each cue does not contain any information on the game aside from the loading instructions, which are simply "load this track here, and this one here", and are the generic format used by all CDs that use cue formated loading sheets, aside from the data type loaded. Therefore, the cues pose no legal trouble. All of your points are very good points, because not every one thinks of the same solutions to the same problems. But, no matter what, we can't see into the minds of every user, so who knows what the public reaction will be?

According to RobLoach, there's only 3 consoles using serial scanning and apparently he's working out CRC scanning for those. Probably should have branched this but I'm short on time.
@ValiantBlade
Copy link
Author

ValiantBlade commented Sep 15, 2016

I've been a little bit on the busy side with life lately, but I reremoved the ISO serial scanning because apparently @RobLoach is doing something about finding DATs for the last two consoles that use serial scanning aside from the PS1. I'll probably have to test PSP when that's done in order to see the detection rate. EDIT: Also, to be 100% honest, I actually wrote my last post at about 10PM when I was really tired.

@RobLoach
Copy link
Member

@ValiantBlade Also note that there's a whole bunch of cue-checking stuff in task_database_cue.c. May be able to remove it, possibly entirely. Will need more testing though.

@ValiantBlade
Copy link
Author

ValiantBlade commented Sep 15, 2016

Let me know what you find. Just so I can make the required edits.

@orbea
Copy link
Contributor

orbea commented Sep 19, 2016

"I notice most of these problems are ones shared with using CRC on cartridge format, yet everyone's happy with those just fine, so why is the PS1 any different?"

I'm not happy with them, lot of my files do not scan or do so with weird names, tracking down why can be a lot of annoyance. The difference is that those were implemented long ago. Ps1 and psp are the only ones that consistently scan correctly most of the time.

@kivutar
Copy link
Member

kivutar commented Sep 19, 2016

I have no problems with cartridge scanning. I just use no-intro 2016.

@ValiantBlade
Copy link
Author

ValiantBlade commented Sep 20, 2016

I actually have a cold storage folder full of the complete No-Intro ROMsets for various consoles, the only reason I haven't scanned all of them is because I don't want my playlists to be multiple megabytes (actually probably gigabytes) in size, and for it to take 8 mins to find the Shin Megami Tensei II ROM or 10 mins just SCROLLING to find Zombies Ate My Neighbors.

@RobLoach
Copy link
Member

Much of task_database.c has changed, we'll need to update this PR to adapt to those changes.

I redid the edits on a new version of task_database.c
@ValiantBlade
Copy link
Author

ValiantBlade commented Sep 29, 2016

I'm preparing a test for scanning a large number of Redump ROMs. I'll report back on the results, also some games may only need to be renamed if their checksums match the MD5s you can download from the Redump website, as I'm testing with Digimon World 2003.

EDIT: This is going to take longer than I thought, considering I had to reaquire all of these ROMs. I'm planning on seeing how many of these fail serial scanning, and if all of these work using this experimental scanning.

@ValiantBlade
Copy link
Author

ValiantBlade commented Oct 2, 2016

So I scanned a collection of Redump ROMs with the current algorithim, and it seems that the only game I've been able to find that doesn't scan, is Dragon Warrior VII. A rather underwheming result, but a result nonetheless. Here are the list of all the ROMs scanned (I just used an ls command and redirected to a file, all of these were scanned on my Android tablet):
ROMsTested.txt.

EDIT: Add Valkyrie Profile and Brave Fencer Musashi to the list.

@inactive123
Copy link
Contributor

Any consensus on this PR yet? There's already a merge conflict. So what to do with it?

@kivutar
Copy link
Member

kivutar commented Nov 29, 2016

No consensus yet

@meepingsnesroms
Copy link
Contributor

Could we switch to crc scanning only and if the .bin file matches check for a same named .cue and put that in the playlist if it exists and put the .bin in the playlist if no valid .cue is present?

@inactive123
Copy link
Contributor

I don't want serial scanning removed. This is very bad.

If anything, I want to add more serial scanning, like for Nintendo DS and 3DS now.

@RobLoach
Copy link
Member

Suggest closing this. Rather than removing it, better solution may be to support both. Check serial first, fallback to other methods.

@inactive123
Copy link
Contributor

OK, I'll close it then.

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.

None yet

9 participants