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

add support to extractcd for GDROM .cue/.bin #7717

Closed
wants to merge 20 commits into from
Closed

add support to extractcd for GDROM .cue/.bin #7717

wants to merge 20 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 29, 2021

This adds support to chdman extractcd for Redump GDROM .cue/.bin format. This complements an earlier patch adding Redump GDROM .cue/.bin support to createcd.

This has been tested and works well for TYPE_I and TYPE_II discs. For those GDROM types the output from repeated createcd -> extractcd cycles is byte-identical.

Type I disc.

74477ca268dcfec219463d921cbed89a  type_i_disc.cue
be87b59616171c57d9fd252acdda806b  type_i_disc (Track 1).bin
824ff123fbce45b5883962eb3fa43cab  type_i_disc (Track 2).bin
a127c8645b365990d8ec74a24dfc97c2  type_i_disc (Track 3).bin
be87b59616171c57d9fd252acdda806b  output01.bin
824ff123fbce45b5883962eb3fa43cab  output02.raw
a127c8645b365990d8ec74a24dfc97c2  output03.bin
064ae98cc6ba4431293eba78496b19af  output.cue

Type II disc.

1fb5fdaad0a7eb9ded22aef0994d9b12  type_ii_disc.cue
ba46bf7c1538394ce47e07c6f55054fb  type_ii_disc (Track 1).bin
4e8310bf18cae2749551c22a978f7ad7  type_ii_disc (Track 2).bin
9ef9194c451f3119c75050743967387e  type_ii_disc (Track 3).bin
997e6cf460f0a50e3c9c2e68c58c9a9d  type_ii_disc (Track 4).bin
c17bb9b873f34b01370f51688f7e25f5  type_ii_disc (Track 5).bin
ba46bf7c1538394ce47e07c6f55054fb  output01.bin
4e8310bf18cae2749551c22a978f7ad7  output02.raw
9ef9194c451f3119c75050743967387e  output03.bin
997e6cf460f0a50e3c9c2e68c58c9a9d  output04.raw
c17bb9b873f34b01370f51688f7e25f5  output05.raw
3d456f88c8ba986a7a29c7176b979945  output.cue

For TYPE_III some of the Redump data is thrown away by createcd. For example frames 75 - 224 in the final track. These are zero frames and can be recreated by populating the sync bytes, the MSF, the mode, then calculating the EDC/ECC. The EDC routines have been copied mostly verbatim from cdrtools (licensed LGPL). For the TYPE_III titles tested so far, the output from repeated createcd -> extractcd cycles is byte-identical.

Type III disc.

9c6830264544845880e14fe64eb94193  type_iii_disc (Track 1).bin
8973940cea2f0c670cb0084f12710a21  type_iii_disc (Track 2).bin
3753e9d495afabab3ef68228a4864b88  type_iii_disc (Track 3).bin
92f4b008fb82c9cd5c8b10b02a540b96  type_iii_disc (Track 4).bin
886ef9f9532ba942ebb369df3784090c  type_iii_disc (Track 5).bin
d243c502fa1654f21f09d38fcc2ea9f3  type_iii_disc (Track 6).bin
9c6830264544845880e14fe64eb94193  output01.bin
8973940cea2f0c670cb0084f12710a21  output02.raw
3753e9d495afabab3ef68228a4864b88  output03.bin
92f4b008fb82c9cd5c8b10b02a540b96  output04.raw
886ef9f9532ba942ebb369df3784090c  output05.raw
d243c502fa1654f21f09d38fcc2ea9f3  output06.bin

This code does assume the zero frames don’t contain any purposefully inaccurate EDC or ECC data as for copy-protection. This should be true for every legitimate Dreamcast GDROM but if there is an exception please let me know.

Copy link
Member

@cuavas cuavas left a comment

Choose a reason for hiding this comment

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

The same issues apply as last time you opened this PR. Closing and reopening the PR strips the history.

src/lib/util/cdrom.cpp Outdated Show resolved Hide resolved
src/lib/util/cdrom.cpp Outdated Show resolved Hide resolved
Comment on lines 67 to 74
enum gdrom_pattern
{
GDROM_TYPE_UNKNOWN = 0,
GDROM_TYPE_I,
GDROM_TYPE_II,
GDROM_TYPE_III,
GDROM_TYPE_III_SPLIT
};
Copy link
Member

Choose a reason for hiding this comment

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

As I said before, this is an artificial classification based on observation of pressed discs, not something inherent to the format. It doesn’t belong here.

Copy link
Author

Choose a reason for hiding this comment

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

I've split the GDROM routines out to gdrom.h and gdrom.cpp and updated build files. 38a7fd7

Copy link
Member

Choose a reason for hiding this comment

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

No, it doesn’t belong in a public header at all. It should be internal to the CHD conversion code. It’s an artificial classification and not something inherent to the disk format.

Copy link
Author

Choose a reason for hiding this comment

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

It's used in two source files chdcd.cpp and chdman.cpp. It could go in chdcd.h although that's also a public header. If no header is acceptable what's the alternative?

Copy link
Author

@ghost ghost Jan 30, 2021

Choose a reason for hiding this comment

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

Moved to chdcd.cpp and chdcd.h in 0d4bb49. The function is used in chdman.cpp:do_extract_cd() so the prototype and enums need to be in chdcd.h. If that's wrong there is a more disruptive option that involves moving the bulk of do_extract_cd from chdman.cpp to chdcd.cpp. However that will affect existing code that's unrelated to this pull request so the least-disruptive option is on the table first.

@ghost ghost requested a review from cuavas January 29, 2021 22:10
src/lib/util/cdrom.cpp Outdated Show resolved Hide resolved
@ghost ghost requested a review from cuavas January 30, 2021 07:26
Comment on lines 175 to 183
static inline uint32_t reverse32(uint32_t x)
{
x = ((x & 0x55555555) << 1) | ((x & 0xAAAAAAAA) >> 1);
x = ((x & 0x33333333) << 2) | ((x & 0xCCCCCCCC) >> 2);
x = ((x & 0x0F0F0F0F) << 4) | ((x & 0xF0F0F0F0) >> 4);
x = ((x & 0x00FF00FF) << 8) | ((x & 0xFF00FF00) >> 8);
x = ((x & 0x0000FFFF) << 16) | ((x & 0xFFFF0000) >> 16);
return x;
}
Copy link
Member

Choose a reason for hiding this comment

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

There are helpers for doing this stuff in one of the headers in src/osd already.

Copy link
Author

Choose a reason for hiding this comment

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

You’ll have to be more specific. I don’t know the helper you’re thinking of.

src/lib/util/cdrom.cpp Outdated Show resolved Hide resolved
src/lib/util/cdrom.cpp Outdated Show resolved Hide resolved
src/tools/chdman.cpp Show resolved Hide resolved
src/tools/chdman.cpp Show resolved Hide resolved
@ghost ghost requested a review from cuavas February 2, 2021 20:23
@ghost ghost closed this Oct 7, 2021
@KingKebab
Copy link

Hello @nhand42, I see you closed this request.

Does it means we will be able to rollback .chd files into Redump .bin/cue files with the new chdman 0.237 release ?
Or this functionality will never be available ?

@Sanaki
Copy link

Sanaki commented Oct 9, 2021

The functionality was not merged. At some point it will need to be handled, since as it stands now you can't round trip Redump cue based chds. That said, I don't know the exact situation as it stands now. Seems like changes were requested, with one side waiting on changes and the other waiting on feedback that wasn't provided/available. Presumably the closure was based on frustration with that standstill, rather than any actual statement on viability or readiness.

@KingKebab
Copy link

KingKebab commented Oct 9, 2021

Hello @Sanaki and thank you for your explanation. I find this way is too bad. I know some people are looking for this functionality too.

I understand Dreamcast dumps support is not essential here because MAME doesn't emulate this hardware. But if Nhand42 can provide a forked version and/or a Windows binarie of the chdman tool with its own code to rollback .chd files into ReDump's .bin/.cue files, it could be very helpfull.

@Anuskussssss
Copy link

Anuskussssss commented Oct 14, 2021

It's sad to see this closed instead of being part of MAME. Redump CHD is the better format and it would be great if chdman supported it (completely). No idea why we ended up in a dead end to begin with (@cuavas could've merged this ages ago since it's basically ready) but it is what it is.

@KingKebab I compiled this back in February: chdman.zip
It also includes my fix which was unfortunately ignored by the OP. I haven't been following the project much since then, and I don't know if there have been any important changes made to chdman and if it's worth the effort to rebase and recompile, but my EXE works and I have already converted my entire collection with it.

Edit: The files this PR touches aren't actually much different compared to February, so a rebase was pretty easy. I applied my fix and had to change a line (see diff) but it compiled and a quick test confirmed that it still works.
Updated version (diff included): chdman_v0.236#7717.zip

@KingKebab
Copy link

Hello @Anuskussssss, Thank you very much for your personal compiled release. I performed a try with the 4 Wheel Thunder (USA) dump and I was able to rollback my .chd into it's original .bin/.cue format. I did a binarie comparison for each file and it's absolutely perfect. Now I will try with the Dreamcast fullset (it will probably take a long time) and then I will let here a feedback.

@Anuskussssss
Copy link

@KingKebab I've updated chdman to v0.236 just for the hell of it. Please test.

@KingKebab
Copy link

@Anuskussssss I performed a rollback test using the Sega - Dreamcast - Datfile (1364) (2021-01-13 15-00-42).dat file from ReDump. I did a binary compare of all files. All .bin and .raw files rollbacked are excatly the same as their original file except for 3 disc dumps only :

. Shenmue II (Japan) (Disc 2)
Shenmue II (Japan) (Disc 2) (Track 3).bin
Shenmue II (Japan) (Disc 2) (Track 4).bin

. Shenmue II (Japan) (Disc 3)
Shenmue II (Japan) (Disc 3) (Track 3).bin
Shenmue II (Japan) (Disc 3) (Track 4).bin

. Shenmue II (Japan) (Disc 4)
Shenmue II (Japan) (Disc 4) (Track 3).bin
Shenmue II (Japan) (Disc 4) (Track 4).bin

For this 3 disc dumps, the Track 3 files are missing data at the end of the file in the rollbacked file.
And the Track 4 files show in the compare, their headers have many diffences.

I tried a full process (.bin/.cue => chd => .bin/.cue) with your two chdman version from 0.228 and 0.236.
I get the same binaries files.

I find this result is strange. Because there is no data trouble with other Shenmue II european disc dumps.
Why these differences happen with the japan version only ?!!

Now I'm looking to update my Dreamcast set with the latest 2021-10-06 .dat file from ReDump. Perhaphs my original dumps for Shenmue II (Japan) were corrupt and their .chd wrongly compressed.

@Anuskussssss
Copy link

@KingKebab Thanks for testing, I can reproduce this. Will investigate once I find some time.

@ghost ghost deleted the extract_gdrom_cue branch October 19, 2021 00:32
@KingKebab
Copy link

KingKebab commented Oct 25, 2021

Hello @Anuskussssss @nhand42

I updated my Dreamcast romset and I was able to rebuild the fullset with the latest ReDump .dat file (2021-10-06). 0 missing. I did a full and long process to convert/rollback all GD-Rom dumps. And I found another one with the same issue than Shenmu II's discs :

Virtua Figher History & VF4 (Japan)
. Virtua Figher History & VF4 (Japan) (Track 3).bin
. Virtua Figher History & VF4 (Japan) (Track 4).bin

So this game has only one disc and it's easier to test in an emulator than using Shenmue II's additional discs.

In Flycast, I can boot and start the original .bin/cue dump of Virtua Figher History & VF4 (Japan) porperly. But the .chd converted file can only boot the disc, but not start the game. I suspect something broke while the chdman tool convert the original dump into CHD format. Because all other converted .chd can boot and start properly. I think, this is the reason why the rollback into .bin/.cue is wrong too.

I performed the same test with the official chdman tool release and I get the same unworking .chd file for this game.

In any cases, thanks a lot you two for your help. This chdman hacked version works with most GD-Rom dumps. It's an incredible great job. Now I will be able to free many disc space on my hard drives and keep them as CHD files for storage. Except Shenmue II, Virtua Fighter History and all MIL-CD dumps (for them I use .7z).

@SamRohod
Copy link

SamRohod commented Oct 28, 2021

Hi @Anuskussssss

Thank you very much for the modified chdman.

It would be helpful to a lot of people if either you or @nhand42 put an up-to-date modified chdman (fork?) on your github page since that the MAME team seem to be uninterested in implementing this in the official chdman.

@Sanaki
Copy link

Sanaki commented Oct 28, 2021

The MAME team is not uninterested, as you can see in earlier comments. They simply weren't content with the quality of the PR as it remained, and none of them have expressed interest in dropping their other priorities to pick this up themselves. If someone addresses the concerns mentioned in review earlier and submits a new PR, they'd likely be happy to merge it.

@SamRohod
Copy link

Sorry for misunderstanding

@rb6502
Copy link
Contributor

rb6502 commented Oct 28, 2021

I'm absolutely interested in having the support in official MAME, but the PR had issues that were never resolved by the submitter. If @Anuskussssss has a better version of the patch and is willing to correct anything found on review, they can submit it themselves.

@ghost
Copy link
Author

ghost commented Oct 28, 2021

I'm absolutely interested in having the support in official MAME, but the PR had issues that were never resolved by the submitter. If @Anuskussssss has a better version of the patch and is willing to correct anything found on review, they can submit it themselves.

@Anuskussssss contributed nothing to this patch and has been blocked for abusive language. He keeps evading the blocks by creating alt accounts.

I asked for more info and was ignored for around 9 months. I have no idea what the concerns are and the reception was ice-cold, so the PR is withdrawn.

@Anuskusssssss
Copy link

Anuskusssssss commented Oct 28, 2021

@rb6502 I fixed the code, but the changes are garbage because I'm not a C++ developer. I personally don't care how it looks, as long as it works. Feel free to do whatever you want with the code, but it's probably not up to MAME's standard. Guess you guys will be stuck with a broken chdman until somebody writes better code then.

@KingKebab I only tested Shenmue 2, but all broken (Pattern III without audio) ROMs should be fine now. My changes don't affect any other game besides the broken ones. Do you actually have a full set? I'm actually relieved knowing that we have 100% compatiblity now (minus the MIL-CDs of course). Hopefully the guy on the Internet Archive maintaining the Redump CHD set will update those broken CHDs (and finally remove the bad Redump GDI CHDs).

chdman#7717.zip

If it's not too much work, I'll rebase and recompile an EXE once in a while until either somebody creates an actual PR or this thread gets locked.

Edit: I didn't realize that v0.237 has already been released. See below for an updated build (#7717 (comment)). @KingKebab No need to retest this version, as no functional changes have been made.

@happppp
Copy link
Member

happppp commented Oct 28, 2021

@Anuskussss and @Anuskusssss and @Anuskussssss and @Anuskusssssss keep it civil!
And if it turns out you were really stalking nhand42, you're not welcome here.

@KingKebab
Copy link

@Anuskusssssss Congratulations. I just did a quick test with your new chdman release for these dumps :
. Ikaruga (Japan)
. Jet Grind Radio (USA)
. Jet Set Radio (Europe) (En,Fr,De,Es)
. Jet Set Radio (Japan)
. Shenmue II (Japan) (Disc 1)
. Shenmue II (Japan) (Disc 2)
. Shenmue II (Japan) (Disc 3)
. Shenmue II (Japan) (Disc 4)
. Virtua Fighter History & VF4 (Japan)

And everything is all right. Your fix doesn't affect previous working dumps like Ikaruga or Jet Set Radio. But it fixes Shenmue II (Disc 2, 3 and 4) and Virtua Fighter History dump issue. Their .chd can now boot and start in Flycast emulator and I can rollback their .chd into .bin/.cue. All audio and data track have the same binary sum.

I own the fullset Sega - Dreamcast - Datfile (1405) (2021-10-06 20-36-04).dat from ReDump and I will make a new full test with your latest release to be absolutely sure everything is perfect. I wrote some scripts to did it. It will take few days on my computer then I will let a feedback to confirm. I'm optimistic about the result.

@Anuskuss
Copy link

@happppp Don't try to stir up drama when there's none. I don't care about @nhand42, or anyone else in this thread for that matter. I would like to use this functionality myself, saw that somebody created a PR and tried to help where I could.
Because my suggestion was ignored, I was waiting for this PR to get merged so I could open a follow-up PR myself. Since @cuavas stalled progress for whatever reason, I compiled it myself, converted my entire collection with it and called it a day. I had my solution and I don't care what you guys merge (or don't).
9 months later OP returns and closes this PR, being frustrated by the lack of feedback from the MAMEDEV team (understandably so). Still, I had my own chdman so whatever.
Only after @KingKebab reported that the compatibility is in fact not 100%, I started to get interested again. Now I did actually fix it, and provided a rebase of OP's commits together with a diff of my own fixes, completely free of charge and as a community service (mind you, I could've just kept it to myself). I don't have the nerves to open my own PR and wait 9+ months for nit-picky feedback (not that my code is good to begin with), so I opted to instead comment my diffs and an EXE in here for people who might find this thread and need this functionality.
I don't have any personal feelings (positive or negative) about anyone here. Quite frankly, you're all just usernames to me.


Latest version (v0.237): chdman.zip

@happppp
Copy link
Member

happppp commented Oct 29, 2021

No drama? To me it looks like it's more than enough to put in a soap opera arc,
It starts over here: #7605 where nhand42 doesn't like your feedback and you create an alt account(Anuskusss) to have the last word.
Then this PR, you made 4 more alt accounts to keep commenting after nhand42 blocks you each time. In the end he finally gives up and removes his github account after you insulted him (I moderated that part in your post).

I'm not blaming you for him closing this PR. That's on us for ignoring his code review requests for 9 months.

@happppp
Copy link
Member

happppp commented Oct 29, 2021

Anyway, this PR is dead after the author removed his branch and left github. It looks like RB is pretty lenient on you so no matter how much I'm disgusted with your lack of etiquette, I don't expect to see a ban coming.
If you(or anyone) want to contribute an update to chdman, make a new PR.

@KingKebab
Copy link

@Anuskusssssss I restarted my conversion script process with your new chdman 0.237 release... so now, wait and see for the result. If we don't get new issue to rollback .chd files into .bin/.cue, any user can easily keep your fixed chdman release as a standalone binarie tool. But only for use with Dreamcast .bin/.cue dumps. In my memories, some MAME members were disliking ReDump's dumps and they prefer Tosec's dumps instead (cf: #6466). This means, you'll not need to provide a next release. It's a finalized tool. And no matter about how code is writted inside, nobody knows^^

@Sanaki
Copy link

Sanaki commented Oct 29, 2021

It isn't finalized. It's a kludge solution that's good enough for one use case on one operating system. Support will still need to be added in another PR at some point.

@Anuskuss
Copy link

@happppp Again with the gaslighting. Just drop it. If you wanna talk about your feelings, send me an email. This is not the place for that.

If you(or anyone) want to contribute an update to chdman, make a new PR.

It's always "or anyone". Why don't you just take my code and merge it then? Or even better, improve it? Otherwise, please refrain from creating any more useless comments. You know that everyone in here gets a notification everytime you speak. If you find a game that doesn't work, and isn't a MIL-CD, you may comment again.

@KingKebab Well, I know that it's technically finished, but I'd still like to inherit any new changes/improvements made to the codebase. I also dislike the idea of having to keep multiple copies of chdman, so I will keep supporting it as much as I can. I don't mind spending less than an hour a month rebasing and recompiling this. Only when it takes longer than that (possibly when CHDv6 releases), will I probably stop uploading new builds.

@Sanaki I know you have no idea what you're talking about, but I'll entertain the idea that you're really that oblivious. If you would've read my comments, or you know, downloaded my ZIP, you would've found out that I included diffs. If you need help applying my patches and compiling MAME, google it. This is not a support forum.

@Sanaki
Copy link

Sanaki commented Oct 30, 2021

I included diffs

Cool, I stand corrected on the one operating system part. Still needs to be fixed up and submitted properly at some point. I compile MAME myself, I just have no need of this feature until it's properly merged, because my use case involves more widespread deployment where "grab a file someone uploaded" isn't viable. As such, I didn't bother looking at it directly, since it would serve no purpose in my hands at this time.

To be clear, I'm not disparaging the work done. It's great for people who have a use for it now. It just isn't "done" until it's merged. And had I the knowledge to fix it up to MAME's standards, I'd be more than eager to do so, but unfortunately that's beyond me.

@happppp
Copy link
Member

happppp commented Oct 30, 2021

If you want to maintain it with diffs and attachments, do it at https://github.com/Anuskuss/chdman
PR will be locked.

@mamedev mamedev locked and limited conversation to collaborators Oct 30, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet