unknownbrackets / maxcso

Fast cso compressor
ISC License
390 stars 23 forks source link

Issues with ZSO created by maxcso #67

Closed malvarenga123 closed 2 years ago

malvarenga123 commented 2 years ago

I can't seem to replicate the issues people are having, but there are some reported errors with the ZSO files created by maxcso: https://github.com/ps2homebrew/Open-PS2-Loader/issues/225

unknownbrackets commented 2 years ago

Can you link to the actual decompression code? I can take a quick look.

That said, my guesses are one of:

There's absolutely no detail in that issue about actual issues, just statements that assert issues occur. I've seen no indication that maxcso ever loses the first 16 sectors (something that likely would happen with CSO as well, if it was a bug in maxcso, and someone would've reported by now surely.)

That said, maxcso is named "max" for a reason. Even with LZ4, maxcso brute forces to find the best compression: https://github.com/unknownbrackets/maxcso/blob/56ccff539c94de44f9745d9f1fbaac6f65a58b26/src/sector.cpp#L258-L260

Maybe the decompressor doesn't support LZ4HC or something.

-[Unknown]

unknownbrackets commented 2 years ago

If it's this: https://github.com/JoseAaronLopezGarcia/Open-PS2-Loader-ZSO/blob/master/modules/isofs/lz4.h

My first reaction is that it's using a version of LZ4 from 8 years ago, while maxcso is using one from only 2 years ago: https://github.com/lz4/lz4/commit/69dc85b8abe78246bea91a5ba1205e4c07b96a97 https://github.com/lz4/lz4/commit/293713a4fa085d73f396200d2387631b045c118e

I don't know if there's some compatibility issue between the versions, or any bugs in the older version. In theory, they're the same major version so the format ought to be the same. Although, supposedly there are performance improvements in some of those versions, at least...

Also, maxcso would need explicit --block=2048 because it assumes any decompressor capable of handling > 2GB files is also capable of handling larger block sizes. This is true of PS2 emulators. I guess this is probably the issue, but you could compare the header of a working zso with a not-working zso to be sure. (that's not really a bug in maxcso, but rather a limitation in that OPL code.)

-[Unknown]

JoseAaronLopezGarcia commented 2 years ago

No it must be an issue with maxcso since it affects other formats too (CSO, CSOv2 and DAX). As per my tests, DAX and CSO files generated by their original compressors work fine on ARK and ME (the only known CFW to support DAX), but CSO and DAX generated by maxcso are a hit and miss. Sometimes even maxcso itself can't recognize or uncompress its own generated files (happened to me with DAX specially). Same happens with CSOv2, some files work fine, others boot but eventually crash.

unknownbrackets commented 2 years ago

As they say, extraordinary claims require extraordinary evidence. maxcso has been used for years, with PSP and PS2 games, by probably at least thousands of people. It's on some Linux package managers and several people have used it to convert their entire library to CSO and verified each one. No one has reported issues with PPSSPP, PCSX2, Play!, or etc. for some time now.

A new client is written and has problems with it. Your assertion is that all the people who have used it before successfully simply haven't noticed that it's completely broken for multiple widespread use cases. This only took a handful of people trying 10s of games to discover and is somehow widespread, because everyone before couldn't even see their hands in front of their faces.

I'm not saying maxcso can't have a bug here, but I hope you can understand why I'm looking for more detail. Even if there is a bug in maxcso, I and others haven't been able to reproduce it - that's why I'm asking for more detail. It could definitely be there's an issue with ZSO, because that's generally unpopular, but let's focus on CSO since you're saying it's just completely broken trivially.

I've tried maxcso on about 74 different PS2 games and they've all been compressed without issue and decompress without issue. Emulators such as PCSX2 can load them without issue. Examples include: Dark Cloud 1 (US), Wild Arms: Alter Code F (US), Mana Khemia 2 (US), Xenosaga 1 (US). Xenosaga 1, for example, used a block size of 16384, and an index shift of 2. Its CRC, as calculated by maxcso --crc, is 3d9843e9 which is exactly correct. Everything in the header and data is consistent. This was using maxcso v1.13.0.

Now, can you please provide specific examples?

-[Unknown]

malvarenga123 commented 2 years ago

I also have compressed a few dozen games (PSP and PS2) with maxcso's default settings and decompressed them to make sure all hashes matched. There wasn't any issues.

JoseAaronLopezGarcia commented 2 years ago

As they say, extraordinary claims require extraordinary evidence. maxcso has been used for years, with PSP and PS2 games, by probably at least thousands of people. It's on some Linux package managers and several people have used it to convert their entire library to CSO and verified each one. No one has reported issues with PPSSPP, PCSX2, Play!, or etc. for some time now.

A new client is written and has problems with it. Your assertion is that all the people who have used it before successfully simply haven't noticed that it's completely broken for multiple widespread use cases. This only took a handful of people trying 10s of games to discover and is somehow widespread, because everyone before couldn't even see their hands in front of their faces.

I'm not saying maxcso can't have a bug here, but I hope you can understand why I'm looking for more detail. Even if there is a bug in maxcso, I and others haven't been able to reproduce it - that's why I'm asking for more detail. It could definitely be there's an issue with ZSO, because that's generally unpopular, but let's focus on CSO since you're saying it's just completely broken trivially.

I've tried maxcso on about 74 different PS2 games and they've all been compressed without issue and decompress without issue. Emulators such as PCSX2 can load them without issue. Examples include: Dark Cloud 1 (US), Wild Arms: Alter Code F (US), Mana Khemia 2 (US), Xenosaga 1 (US). Xenosaga 1, for example, used a block size of 16384, and an index shift of 2. Its CRC, as calculated by maxcso --crc, is 3d9843e9 which is exactly correct. Everything in the header and data is consistent. This was using maxcso v1.13.0.

Now, can you please provide specific examples?

-[Unknown]

My (wild) guess would be the compression libraries. I know that the LZ4 used for ZSO is not compatible with LZ4-HC, and perhaps the deflate algorithm (official one on PSP firmware) is old and outdated. You shouldn't use PC software (emulators) as the base of work: they have updated libraries and more RAM available. Try a few games on an actual PSP and PS2 and see if maybe you can reproduce the issue users are having. If it works on real hardware, it will work on an emulator.

Also please don't take this as an attack on maxcso. We all know and appreciate this all-in-one tool and all its features, but if users can't get their games working with it, they will move onto the other compressor that does generate valid files for them. I don't wanna have 5 different compressors around and having to remember their commands, I prefer using maxcso for all formats, but so far I keep having to go back to 20 year old compressors that aren't even open source to be able to generate working files.

Also offtopic: any plans to implement JSO support?

unknownbrackets commented 2 years ago

My (wild) guess would be the compression libraries. I know that the LZ4 used for ZSO is not compatible with LZ4-HC, and perhaps the deflate algorithm (official one on PSP firmware) is old and outdated.

Is there a particular reason that your recently written new code is using such an old version of LZ4? Is it codesize or toolchain compatibility?

If you're talking about CSOs not working specifically on some PSP CFW, that is #64 and you can use the latest git or --no-libdeflate. It's true that there's something that implementation of deflate isn't supporting which libdeflate does.

You shouldn't use PC software (emulators) as the base of work: they have updated libraries and more RAM available.

Yep, I know something about testing things on hardware and accurate emulation. I've probably created more console accurate algorithms than you have, so no need to explain that to me. But software is software. We're not attempting to support the specific values of a hardware produced ADSR curve here, the PS2 doesn't have hardware or BIOS LZ4 support.

Try a few games on an actual PSP and PS2 and see if maybe you can reproduce the issue users are having. If it works on real hardware, it will work on an emulator.

I've never used your custom software PS2 implementation that you wrote recently, even though you're somehow confident it cannot have bugs. Obviously I've used an actual PSP extensively. Sometimes I feel like it's only me and gid15 actually testing things on real PSP hardware these days.

Also offtopic: any plans to implement JSO support?

Not really. I'd probably accept a proper pull request, though.

-[Unknown]

JoseAaronLopezGarcia commented 2 years ago

Is there a particular reason that your recently written new code is using such an old version of LZ4? Is it codesize or toolchain compatibility?

On devices like the PSP and PS2 you don't really have a lot of RAM to spare, surely someone with so much experience like yourself should know this very basic fact.

I've probably created more console accurate algorithms than you have, so no need to explain that to me.

Is there any reason to be this toxic to your users? You'll end up driving them away and trust me maxcso isn't exactly popular in the PSP or PS2 scene.

BTW when you write a full CFW for PSP with all the features ARK-4 has then maybe you can lecture me about what works and what doesn't, and maxcso doesn't work, unlike 20 year old compressors that work flawlessly.

I've never used your custom software PS2 implementation that you wrote recently, even though you're somehow confident it cannot have bugs.

You've never used my code and you are sure it's full of bugs? Despite the fact that all other compressors, including decades old ones, work flawlessly in it.

You can keep being in denial and keep your toxicity to yourself. Close this issue and just let users go to alternative tools like they already are doing.

Not really. I'd probably accept a proper pull request, though.

No thanks, I've read the source code of maxcso and is a bloated mess of spaguetti code with no maintenance capabilities at all.

unknownbrackets commented 2 years ago

On devices like the PSP and PS2 you don't really have a lot of RAM to spare, surely someone with so much experience like yourself should know this very basic fact.

But no one is asking you to compress LZ4HC, just decompress. As far as I know, that has about the same memory requirements as LZ4. Neither is really allocating buffers. LZ4HC just works harder to find a more optimal match.

Either way you already have the entire block in RAM so it's hard to imagine it using significantly more RAM, it can't backreference outside the RAM you've already got allocated.

Is there any reason to be this toxic to your users? You'll end up driving them away and trust me maxcso isn't exactly popular in the PSP or PS2 scene.

It's strange to me that you're calling asking for specific details toxic. I'm just asking questions and saying that there's no reason to have a pointless contest or arguments about other things. I've also acknowledged that maxcso may have a bug, and that your PS2 loader may have a bug. I'm not sure why you consider assuming there's a possibility your more recently written code may have a bug to be an attack. I still don't understand why you've been unwilling to name any specific game with any specific problem. I outlined an example of this for you to make it easy in my comment referencing Xenosaga.

You said "Sometimes even maxcso itself can't recognize or uncompress its own generated files (happened to me with DAX specially)." If this is true, obviously I would like to fix it. Could you please just answer the question of further detail? What game? What error messages do you get?

-[Unknown]

JoseAaronLopezGarcia commented 2 years ago

I'm not even goint to bother reading what you have said.

Truth be told it is extremely strange, and dare I say stupid, of you to behave so toxicly and so aggressive against the only developer who has ever bothered to support your CSOv2 format and even give you proper credits for it.

That said, I am glad that absolutely nobody on the PSP community has ever even mentioned or acknowledged CSOv2, let alone use it. Even supporting the old, obsolete and barely known JSO format had more traction and acceptance, with many users calling it the best of them all.

Also I'm yet to see what exactly you have contributed to the PSP and PS2 community, I can't see your name on anything (except for ARK-4 and that's thanks to me).

unknownbrackets commented 2 years ago

@malvarenga123 you mentioned not being able to reproduce any issues - was that with ZSO as well? I'm not really sure what's going on here, but without a specific example or anyone able to replicate, it's hard to do anything.

I'm thinking of closing this since it has insufficient info. That's not to say there can't be an issue, just a lack of any actionable detail. We could try disabling LZ4HC compression - but without details on replication, it's unclear if anyone could verify that helps.

-[Unknown]

malvarenga123 commented 2 years ago

@unknownbrackets Yes, I used ZSO (--format=zso) for testing. All files decompressed without issues. Even God of War, which is a dual-layer disc, decompressed to the exact same original iso (md5 matched). Not once maxcso refused to read the files it had created. Granted I'm only using "good" iso files that can be verified in redump's database. Maybe the people having issues are using malformed or corrupt isos.

@JoseAaronLopezGarcia are you actually reading the same texts as I am? I don't see any toxicity or offense in unknownbrackets' responses. And I do agree that neither you or any of the users in the ZSO thread have provided useful information to debug this, which is ironic considering you are a developer yourself.

swosho commented 2 years ago

From what I've read so far on psx-place forums and Discord, the "mysterious" issue seems to be nothing but a user error. Gotta make ZSOs simply with --format=zso and no other arguments. All the clean ISOs I have on hand, a DVD9 game (Midnight Club 3 Remix), and also modded ISOs – zero compatibility problems on my end as of yet.

AKuHAK commented 2 years ago

@unknownbrackets I found the root of the problem: https://github.com/unknownbrackets/maxcso/blob/56ccff539c94de44f9745d9f1fbaac6f65a58b26/src/compress.cpp#L16-L20

Playstation 2 doesn't support LARGE_BLOCK_SIZE due to super low free memory. PS2 supports only SMALL_BLOCK_SIZE After reaching threshold LARGE_BLOCK_SIZE_THRESH the compression code instead increases Alignment. So for 0-2Gb alignment = 1, for 2-4 Gb alignment = 2, for 4-6 Gb alignment =3, for 6-8 Gb alignment = 4, etc.

Is it possible to make this behaviour either via some option flag, r by default?

unknownbrackets commented 2 years ago

Yes, you can just specify the block size as an argument as noted in the README:

   --block=N        Specify a block size (default depends on iso size)
                    Many readers only support the 2048 size

At the time of writing, some older tools for the PSP (which can only have UMDs up to 1.8 GB) didn't properly read the header and that's why the default is 2048 for < 2 GB. Many other softwares since properly read that header.

Use this to generate ZSO files with a 2048 byte block size.

maxcso --block=2048 --format=zso input.iso

The correct thing for a reader of this format to do would be to check the header, and reject the file with a clearer message if it's unsupported. That way people know what's going on, even if the user explicitly uses a higher block size (which gives better compression) without realizing it's unsupported. A very reasonable way this could happen is that someone compresses it for their emulator, and then weeks/months later decides to play it on a real PS2, not remembering anything about block sizes.

-[Unknown]

malvarenga123 commented 2 years ago

Welp, it seems you were right from the beginning :)

unknownbrackets commented 2 years ago

I'm not really against the idea of lowering the default block size for all ZSO files, since the likely case for ZSO is a weaker device with less available RAM for buffers.

I did also include an example Windows batch file for reference: https://github.com/unknownbrackets/maxcso/blob/2a26a15347767c8f343cc8d03b74028e1be25799/examples/format-zso.cmd

Hope that helps anyone having any trouble.

-[Unknown]