SabreTools / MPF

Redumper/Aaru/DiscImageCreator GUI in C#
GNU General Public License v3.0
415 stars 34 forks source link

Remove IMAPI2 as a dependency #563

Closed mnadareski closed 11 months ago

mnadareski commented 11 months ago

Problem

IMAPI2 is a Windows library that allows for media type detection and media burning in Windows applications. It is lightly maintained but is entirely locked to the Windows platform. IMAPI2 is a major blocker to making the entirety of MPF cross-platform, with the other major blocker being the UI framework. Removing this library would mean the end of very accurate media type detection.

Solution

Remove IMAPI2 from the application even if it means less accurate media type detection until another solution is found. This solution allows .NET Framework 4.8 builds to be built and packaged near-identically to the newer .NET 6 and .NET 7 builds and places those more modern .NET builds on equal footing.

Commentary

This is deliberately being made into a separate branch because this is a very divisive change. This removes the ability (on .NET Framework 4.8) to accurately determine the type of the media being detected, instead falling back on the same method that .NET 6 and .NET 7 use. This fallback method makes a best guess based on the media size and the filesystem.

The only dumping program supported by MPF right now that cares about the media type on dump is DiscImageCreator. It is still the default program and is the only one that benefits from this arrangement. If this is removed, there's more of a burden on the user to determine what type the media is before dumping.

This does put all 3 build versions (.NET Framework 4.8, .NET 6.0, and .NET 7.0) on equal footing for the first time, however. The build instructions become the same and no differences in functionality will be observed. For .NET Framework 4.8, this may also bring slightly more stability, given that IMAPI2 does not play well with all hardware.

Alternatives

TheRogueArchivist commented 11 months ago

Seeing as how it's only relevant for a program that support is gradually being moved away from, and how this wouldn't result in any bad dumps, I personally support this completely. Worst case scenario, a wrong media type is chosen and the dump errors out immediately.

FoxhackDN commented 11 months ago

Remove.

I don't consider media format autodetection to be useful, it's always given me more problems, and a user should already know what kind of media they're dumping.

Deterous commented 11 months ago

Selecting the media type is an extra step that is unlikely to cause issues for existing users, but may be another minor hurdle for new users. Therefore, I think it is okay to remove IMAPI2 and keep media detection as a feature request. If the feature was not already in MPF, then IMAPI2 would not be accepted today as a solution. I have had issues with IMAPI2 before that have caused Windows to become unresponsive, even requiring hard shutdowns. I will not miss it.

sadikyo commented 11 months ago

It's a bit difficult for me to give good feedback here as it applies to general users with varying levels of knowledge. I would expect most dumpers to know what type of media they are dumping - CD, DVD, BD, etc., but I imagine some dumpers starting out may not know and are relying on the software to detect that.

For me, I always know what media I'm dumping, and while the detection isn't currently absolutely necessary, it's quite helpful in saving me a step every single dump. I can live without it, but it's quite nice having it detect it automatically, in my experience.

If leaving this feature is causing all sorts of other problems or holding up progress, I don't think it's absolutely vital and could be removed if it really needs to be.

As for new dumpers - if they select the wrong one, the dump won't work anyway and they'll have to re-do it with the correct media setting. It is unlikely that we'll start seeing massive waves of issues here.

mnadareski commented 11 months ago

I should mention for those who didn't catch this in my rambling statements in the description: there is still going to be some media type detection, it's just going to be far less thorough. It uses a combination of size and filesystem to attempt to figure out what the type is.

mnadareski commented 11 months ago

It has been determined that this PR will be merged based on recent coverage changes.