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

Cancellation support #5

Closed
PiKeyAr opened this issue Jul 26, 2024 · 14 comments
Closed

Cancellation support #5

PiKeyAr opened this issue Jul 26, 2024 · 14 comments
Assignees

Comments

@PiKeyAr
Copy link

PiKeyAr commented Jul 26, 2024

Hi! Thank you for making this tool. I use it in this project that allows the user to add hacks to Dreamcast games.

I was wondering if you were interested in adding built-in support for cancelling an ongoing GDI build process via a separate method or a CancellationToken. I put together a crude hack that simply makes all data-heavy functions check the CancellationToken before reporting progress and return if cancellation is requested.

This is just a quick hack I did for my own purposes, so it would be nicer to have it implemented properly in the original version. What do you think?

@Sappharad
Copy link
Owner

Yes. I think this would be worth adding.
I agree with your general approach but I would need to look over the code again to see if that's the best place to add the checks. I'll check it out in the next week.

@Sappharad Sappharad self-assigned this Jul 27, 2024
@Sappharad
Copy link
Owner

Have you made any other changes to this?

The reason I'm asking is that one of the things I think would be useful is if I separate the DiscUtils fork into a separate project (still part of this repo) and retarget to .NET Standard 2.0. That way it can be consumed by other projects as a .dll from any version of .NET 4.8 and above. I'm going to retarget the UI and console builds to .NET 8, but I think there's value in targeting .NET Standard with a shared DLL because .NET 4.8 still ships preinstalled with Windows 10 & 11 and I'm sure some people still target that on purpose so their users don't need to install anything. I think the last .NET Core builds I posted were probably self contained, but some people might dislike the large binaries that result from that.

I'm not sure if anyone would benefit by consuming this as a DLL or if I should just leave the code as-is and people can just continue having complete copies of it. I believe FamilyGuy was just using the command line version, but I haven't looked at his stuff in years.

@PiKeyAr
Copy link
Author

PiKeyAr commented Jul 29, 2024

Hi, thanks for the response! I absolutely would use this as a separate library. It was one of the things I thought of when I first used GDI Builder in my project.

I haven't made any other changes besides the cancellation thing, but I do have some ideas to explore, and I'll be happy if you consider any of these if they fit the purpose and scale of the project.

  1. Making track data more accurate: I noticed that even if you make no changes to the data, the tracks built by GDI Builder are different from original tracks in TOSEC GDIs. I don't know if making them completely identical is feasible. It seems that the data is arranged in a different order? I saw that sizious' fork of an older version of this project sets application and data preparer identifiers to those of GD Workshop, so it might be a good idea to have options to set those identifiers.

  2. Adding Reader classes to extract data from GDIs: At the moment it seems that there are no open source C# libraries for comprehensive reading and writing the most popular Dreamcast image formats. The tools used by the community to hack and selfboot games rely on old command line tools. There is gditools by FamilyGuy but it's not perfect (fails to extract some GDIs) and is written in Python which makes it difficult to integrate into C# projects. So at the moment to extract GDIs, tools like Universal Dreamcast Patcher and my image builder have to rely on old gdi2data or gditools. Although I realize it wouldn't be just "builder" anymore, if GDI Builder could also read and extract GDIs, it would make a huge difference for these tools.

  3. Building CDIs: This might be out of scope for this project, but similar functionality for CDI where it takes a list of tracks and LBAs and builds a CDI out of it would be absolutely amazing, even better if it built the data tracks from a folder like it does with GDI. Of course binhack and other selfboot stuff would need to be done outside of this tool. There's a CDI maker in KallistiOS toolkit that is very close but it's in C++ and meant for homebrew.

I was going to look into addressing 2 and 3 myself by looking at the decompilation of japanese-cake's GDROM Explorer and rewriting some of the Python tools in C#, but if anything like this is implemented in GDI Builder I would love to use it!

@Sappharad
Copy link
Owner

Sappharad commented Jul 30, 2024

  1. The option to set all of the metadata fields already exists in the current app. It's all in the advanced dialog of the UI.
image

That stuff was shoved into advanced and not exposed in the command like build because the goal was to build working GD-ROM images, not to build fake copies of official ones. It's also why there's no additional control over the data sorting although that one does have legit reasons to exist if someone was trying to make a real disc.

  1. I have a decompiled version of GD-ROM Explorer hiding in the FAQ section of the GDIBuilder page to fix the bug where it ignored the time zone data on file timestamps. I had even updated my local copy of the code to .NET 6 a year or so ago so I could compile it for modern versions of macOS. I never actually got around to releasing that update though. Nor have I posted the code anywhere because that would be rude. I tried 3 times over the span of 2 years to get in touch with japanese-cake but they never replied to my emails. I might've even tweeted at them a couple years ago, I can't recall if I did. A few weeks after no response to my first email is when I released the patched build. Technically it's another advanced thing that could be tweaked if someone was trying to 100% clone an existing disc - official GD-ROMs are all written with the time zone set to the local time zone where the disc was authored. But GDIbuilder writes the time in UTC. It's always going to display correctly in Windows / MacOS because the OS will display local time, but original discs were stored in the source time zone.

Having said all that, I believe DiscUtils can actually read images but it would just need to be adapted to work with GD-ROM. I don't have plans for that since this already meets my needs, but I'll double check and see how much work would be required to use what's already there.

  1. I had actually done this by hand many years ago, before GDIBuilder:
image

Those were MIL-CD, too large to burn on a physical disc. I don't think GDI existed yet? Since part of the protection on GD-ROMs was the location of 1ST_READ.BIN needed to be after 100 mins into the disc, a few years ago I did an experiment to see if the DC would boot a normal 2 session CD image with 1ST_READ.BIN far enough into the disc to be a real one. The experiment was a failure, I think it actually crashed DEmul, and so I never proceeded to try and add CDI support. My goal was to try and generate CDI images that didn't need to be patched to run. Obviously not what you're asking for here, but again it's another thing that I don't have plans for. If I had unlimited time I'd absolutely want to do this, and maybe someday I'll actually need the functionality for something, but right now there's no benefit for me to do it. I'd welcome PR's though. :-)

Summary:
I'll split off the GD-ROM stuff into a separate .NET Standard library, add the cancellation token stuff, and review what the reading functionality in DiskUtils can do - maybe it just needs minor tweaks to account for the session 2 LBA.

@PiKeyAr
Copy link
Author

PiKeyAr commented Jul 30, 2024

Completely understandable. Looking forward to the update!

@Sappharad
Copy link
Owner

This is what I get for not looking at the code before I reply. 😕 DiscUtils is already its own separate .NET Standard 2.0 library.

Other changes still pending.

@Sappharad
Copy link
Owner

Sappharad commented Aug 3, 2024

I've pushed a new branch called Cancellation which generally follows a similar approach to what you did.
https://github.com/Sappharad/GDIbuilder/tree/Cancellation

Some differences:

  • The cancellation token is an optional parameter to BuildGDROMI(). This is to avoid breaking any existing code that doesn't provide one.
  • The cancellation token is only provided as input on BuildGDROM, it is not passed along with progress updates like you sample. I'm not sure what the intent of that was, but if you're calling BuildGDROM() and providing the token you should already have it. Similarly, usage of tokens that I've seen involves passing the token to the long running operation, not having your token passed back to you later.
  • Added sanity checks to the UI because new users keep adding track02.raw to their images and reporting errors, not understand that the tool only builds the high density part of the disc.

I may release this update as-is, or I might do more with it. I'm not sure yet. When implementing cancellation tokens I thought "why not make an async version of BuildGDROM()?" since most apps with background activity use async instead nowadays. But that would have to go all the way down into DiscUtils. There's a fork of DiskUtils that implements async and other modern .NET features, but I'd have to reimplement the GD-ROM specific changes to use that. It's probably not worth the effort for such a tiny performance boost right now.

I still need to check on the reading stuff. Please let me know if you have any feedback on the changes so far in case there's something else I should change before finishing this up.

@PiKeyAr
Copy link
Author

PiKeyAr commented Aug 3, 2024

I think the way you did it is better, I didn't really think it through in my fork because I needed a quick solution.

@Sappharad
Copy link
Owner

Just a quick update on why nothing has been posted yet. Apparently I forked DiskUtils before it had ISO reading support, so I migrated my changes to a version from August 2021. There's an even newer fork with some additional optimizations, but that fork contained some modern .NET features that aren't available in .NET Standard 2.0 and I kind of wanted to leave the option for .NET 4.8 compatibility in there even though I'm targeting .NET 8 now. I would have considered forcing consumers of the library to update if it had await, but it actually didn't have that for ISO building even though it introduced it to other parts of the library.

Out of the box it won't read GDI tracks because they're raw mode instead of pure ISO. I wrote an adapter class that basically reads raw as if it were ISO, which allows the PC Track01 to read fine but I still need to do additional work for the high density tracks to be readable. I didn't start digging into the details of the reader code yet, but it's not finding any files on the disc and I assume that's because it's expecting to be at the beginning of a disc instead of somewhere in the middle.

I have no estimate on that, but I would be disappointed if I haven't looked into it further in the next couple weeks.

@Sappharad
Copy link
Owner

@PiKeyAr If you check out my new commits to the Cancellation branch, I did a few things:

  1. I forked a newer version of DiscUtils with reading support
  2. Moved the DiscUtils fork to the root of the repo, in a separate project called DiscUtilsGD that builds as DiscUtilsGD.dll. You can reference the binary if you just want that.
  3. Added the GDReader class and the other changes necessary to read a GD-ROM like specifying the LBA and loading multiple tracks at once. You can also read track01 with the stock reader just loading the track directly, but GDReader has a .FromGDI() factory to load the data tracks needed to read the high density area of the disc.

Example:

using (var myDisk = GDReader.FromGDI(openPanel.Url.Path))
{
    Console.WriteLine(myDisk.VolumeLabel);
    var allFiles = myDisk.GetFileSystemEntries("");
    foreach (var file in allFiles)
    {
        Console.WriteLine(file);
    }

    using (var firstFile = myDisk.OpenFile(allFiles[0], FileMode.Open, FileAccess.Read))
    {
        byte[] firstFour = new byte[4];
        firstFile.Read(firstFour, 0, 4);
        Console.WriteLine(System.Text.Encoding.ASCII.GetString(firstFour));
    }
}

I assume your end goal is to be able to write a new disc without having to extract the original disc first. If that's the case I still have some more work to do, namely adding additional methods to GDromBuilder to let you add files from a stream instead of from a folder. That is probably all I have left to do before a new release, unless something else needs to be fixed.

@PiKeyAr
Copy link
Author

PiKeyAr commented Aug 23, 2024

Hi! Sorry, I was away for two weeks, and now I need to catch up with work stuff, so I can barely do anything right now.

I managed to extract files from the high density track of SA1 GDI using the new branch, haven't tested multi-track games yet.

I ran into a problem with cancellation during the extraction phase in my program: the program hangs after the Task is cancelled, but it's not related to GDIBuilder since it hangs without it too. It's just my lack of understanding how async and Tasks work, so it'll take some time for me to resolve. When I figure it out, I'll be able to do more testing.

Writing a new GDI without having to extract original files first sounds like a great option, so I'm happy if it gets implemented. At the moment though, my tool is built around having to extract all files first, and it has an option to let the user make final changes to the files manually before building the GDI.

@Sappharad
Copy link
Owner

My commit yesterday has most of the changes needed to build a new GD without extracting files first. I still have a bit more work and testing to go; I hope to finish it up in the next week. I changed the entire build process so you now add the files yourself and the source for a file can both be an actual path or a stream. Plus, the order that you call AddFile() determines the order of files on the disc in case someone wants to control that. The biggest thing missing from reading is a ReadIpBin() method because the current commit has no way to access that but I have that method locally already.

My next commit of buildgdi (the command line tool) should have a -rebuild argument which I'm using to try out "patching" an image. -rebuild in combination with -data should let a user provide a folder with only changed files and just copy the rest from the original disc. I'm also going to add an -extract argument because I might as well. Not sure yet how much of this I'll add to the GUI versions. If I had a lot of time I'd probably want to build a GD-ROM navigator to browse / extract, but for now I'll just add to the command line tool.

@Sappharad
Copy link
Owner

I committed some more breaking changes to the Cancellation branch today, things that I wanted to improve while adding the -extract and -rebuild arguments to the buildgdi command line app. Both features appear to be working on my end, I can extract discs and IP.BIN and also rebuild them successfully while patching some files. The rebuild process used under 30MB of memory for an entire 1GB image and was nice and fast.

I think the API changes are solid now, I have no more breaking changes to the methods & arguments available unless I get feedback that results in them needing to change. I'd like to publish DiskUtilsGD to Nuget once this is done so that developers can use this without having to pull down the code or a DLL. Program.cs in the buildgdi folder can function as an example of how to use the new API.

I released a beta build of buildgdi here:
https://github.com/Sappharad/GDIbuilder/releases/tag/v2.0b1
It's compiled with .NET Native AOT, I'm not sure if that means it won't work on a lower OS version than the Windows 11 that I used to compile it, but I'll find out as other people test it.

I'd like to add these features to the GUI version too, but I think most people are using the command line version with a bunch of scripts right now. Releasing this beta should give me some time to rework the GUI's.

@Sappharad
Copy link
Owner

This has been released

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

No branches or pull requests

2 participants