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

PPSSPP warns about bad performant CHD while using ZSTD #18925

Closed
2 of 5 tasks
LodanZark opened this issue Mar 13, 2024 · 16 comments · Fixed by #18931
Closed
2 of 5 tasks

PPSSPP warns about bad performant CHD while using ZSTD #18925

LodanZark opened this issue Mar 13, 2024 · 16 comments · Fixed by #18931
Labels
Milestone

Comments

@LodanZark
Copy link

Game or games this happens in

None

What area of the game / PPSSPP

image

using the follow command:
chdman createdvd -c "zstd,zlib,huff,flac" -i "game.iso" -o "game.chd"

the emulator identifies as bad performant CHD, zstd should be more performant than lzma...

PS: using the MAME 0.263 chdman.

What should happen

It should identify the CHD as good one.

Logs

No response

Platform

Windows

Mobile device model or graphics card (GPU)

AMD 7900 XT

PPSSPP version affected

1.17.1 (stable)

Last working version

No response

Graphics backend (3D API)

Vulkan

Checklist

  • Test in the latest git build in case it's already fixed.
  • Search for other reports of the same issue.
  • Try resetting settings or older versions and include if the issue is related.
  • Try without any cheats and without loading any save states.
  • Include logs or screenshots of issue.
@hrydgard
Copy link
Owner

Hm, strange. I'll look into it.

@hrydgard hrydgard added this to the v1.18.0 milestone Mar 13, 2024
@LunaMoo
Copy link
Collaborator

LunaMoo commented Mar 13, 2024

Probably the issue is caused by what's mentioned here: #18803 (comment)
That chdman changed default hunk size in some recent version. As such it's not enough to use createdvd anymore, but also change hunk size to what it should be if there's an option for it.

Edit: yeah they changed it to 2 sectors per hunk, didn't tested, but I guess to create a valid chd now for PPSSPP, the commands should be changed to:
chdman createdvd --hunksize 2048 -i game.iso -o game.chd
chdman createdvd --hunksize 2048 -i game.iso -o game.chd -c zstd

@hrydgard
Copy link
Owner

My god, chdman finds ever more ways to make trouble for us..

I suppose we could add a bit of caching, which would improve performance with larger hunks, but would rather not add the complexity when a 2048 hunk size will perform just fine.

@LunaMoo
Copy link
Collaborator

LunaMoo commented Mar 13, 2024

Or just change the guide and warning message to include the specific hunk size and let people, who're in love with container format heavily linked to an arcade cabinet emulation, deal with it on their own.
This might change again in the future while the setting to specify it to what it should be, will probably always work.

Edit:
Just to note, I tested it right now and setting --hunksize 2048 does indeed fixes the problem, results of my very small game test shows that zstd with proper hunksize was smaller than default lzma with default 2 sector per hunk and not that much bigger from lzma with proper hunksize.

So seeing the format was requested and is loved by some purely based on the filesize, bothersome and complex workarounds in here would just make the format being missused and lose it's potential benefits.

~Just to vent I detest mame for one new reason, building it on windows was a chore, heh all just to get that new, broken chdman.

@cuavas
Copy link

cuavas commented Mar 16, 2024

I suppose we could add a bit of caching, which would improve performance with larger hunks, but would rather not add the complexity when a 2048 hunk size will perform just fine.

People have been complaining that chdman createdvd gives lower compression than chdman createcd because of the smaller default hunk size. I expect you’ll get more users complaining that they can’t use perfectly valid CHDs with your emulator when they increase the hunk size up to 8 sectors to get comparable compression.

Variable hunk size has always been a feature of the CHD format. The hunk size is always a whole multiple of the sector size.

~Just to vent I detest mame for one new reason, building it on windows was a chore, heh all just to get that new, broken chdman.

  • You could grab the Windows release binaries from GitHub or SourceForge, as linked on our web site, or grab an up-to-the-minute CI build from GitHub Actions.
  • Numerous chdman issues were fixed in 0.263 – it’s less broken than 0.262.
  • It’s not chdman that’s “broken” – it’s an emulator not supporting a baseline feature.

@hrydgard
Copy link
Owner

PPSSPP accesses ISOs in 2048-byte increments since it's the natural sector size, games often do things by sector.

It seems ridiculous to want to create CHDs with non-native sector sizes.

@cuavas
Copy link

cuavas commented Mar 16, 2024

PPSSPP accesses ISOs in 2048-byte increments since it's the natural sector size, games often do things by sector.

It seems ridiculous to want to create CHDs with non-native sector sizes.

People use CHD format because they want compression. That works better with larger hunk sizes.

Would you also complain that hard disk CHDs aren’t limited to 512-byte hunks to match the sector size? Compression isn’t very effective at that size because the compressor doesn’t have much to work with and won’t find much redundancy.

Every emulator for CD-based systems has been working fine with the default of 8 sectors/hunk for CD-ROM/GD-ROM, and every emulator for hard disk-based systems has been working fine with the default of 16 sectors/hunk for regular ATA/SCSI hard disks.

@hrydgard
Copy link
Owner

But before CHD, there was never any reason for us to implement sector caching. CSO is in the native sector size.

So supporting the CHD format adds additional hassle.

@cuavas
Copy link

cuavas commented Mar 16, 2024

Why are you guys taking it out on us?

My god, chdman finds ever more ways to make trouble for us..

For a feature of the format that’s been there forever and works with every other emulator.

It seems ridiculous to want to create CHDs with non-native sector sizes.

You know the users want CHD format for compression. Let’s face it, the majority of them probably aren’t asking for it for the SHA1 digests in the header or something. Giving the compressor more data gives better compression, at the cost of needing to decompress more if the software only wants a single sector. A lot of the time, software does linear reads anyway, so it’s a performance win with zlib or Zstandard due to less I/O (it’s less of a win with LZMA due to high decompression overhead).

~Just to vent I detest mame for one new reason, building it on windows was a chore, heh all just to get that new, broken chdman.

Going out of your way to do things the hard way, then blaming the developers for it, and “detest” is pretty strong language.

@hrydgard
Copy link
Owner

hrydgard commented Mar 16, 2024

Why are you guys taking it out on us?

Simply because PPSSPP's I/O code relies on a 2048-byte hunk size (it's a very deep assumption, and various PSP APIs are 2048-byte block-based), we never had to support anything else before, and it was working fine with the createdvd command until you guys changed it. It's a bit frustrating.

PPSSPP could probably support other hunk sizes too but it makes CHD support much less of a drop-in change than people advocating for supporting it had claimed.

@cuavas
Copy link

cuavas commented Mar 16, 2024

Simply because PPSSPP's I/O code relies on a 2048-byte hunk size, and it was working fine with the createdvd command until you guys changed it. It's a bit frustrating.

We had a bunch of PPSSPP users complaining on reddit about lower compression ratios with default chdman creatdvd options compared to default chdman createcd options. We obviously told them that it’s because createcd uses 8-sector hunks by default while createdvd used 1-sector hunks by default. One way or another, you were going to end up with people using CHD files with other hunk sizes.

PPSSPP could probably support other hunk sizes too but it makes CHD support much less of a drop-in change than people advocating for supporting it had claimed.

I fail to see how what these advocates did or didn’t say is our fault.

Anyway, my main points are:

  • Your users clearly weren’t happy with results when using 1 sector/hunk given the feedback on r/emulation over at reddit.
  • Variable hunk sizes has always been a baseline feature of the CHD format available for users to set at creation time. We didn’t add it recently. If you don’t support it, you don’t support CHD format.
  • Being accused of breaking something when I actually fixed a lot of issues in it by someone who apparently “detests” me and the rest of the hyperbole here leaves a pretty bad taste in my mouth.

@LunaMoo
Copy link
Collaborator

LunaMoo commented Mar 16, 2024

Just to note, CHD compression of PSP games is on average better with 2048 than 4096. It varies per game, but 2048 wins, so if CHD users care only about that, CHD should actually work like maxcso and test every sector size for every file to find the best compression. Good luck 😆

@cuavas
Copy link

cuavas commented Mar 16, 2024

Just to note, CHD compression of PSP games is on average better with 2048 than 4096.

Did you compare it to 16384? Because that’s equivalent to the 8-sector hunks used for CDs.

@hrydgard
Copy link
Owner

hrydgard commented Mar 16, 2024

@cuavas I concede the point that what the advocates said isn't your fault, and as for "detesting", that's not me.

I'll look into how hard it would be to at least make larger hunk sizes work faster in the common large-read cases. But the 2048-byte assumption permeates our I/O code (and the PSP's APIs) so it may be hard to avoid overlapping reads in some cases.

EDIT: I found a bug causing large reads to be unnecessarily suboptimal. Fixing that might be enough, actually.

@hrydgard
Copy link
Owner

hrydgard commented Mar 16, 2024

OK, turns out our already existing block read combination code is mostly sufficient, just with a bug that made it do redundant reads.

With #18931, it performs quite a bit better, so removing the warning. Still, for best performance, I'll keep recommending 2048.

@Anuskuss
Copy link
Contributor

Anuskuss commented Apr 28, 2024

Just wanna leave this here for future reference:
I initially converted my entire PSP collection from CHD-CD to CHD-DVD after I noticed some music stuttering and while a few files were bigger after the conversion, overall the size went down (don't remember the exact amount but definitely worth it).
Then I tried to push it further by increasing the hunk size. At first I could trigger the stutter at 4x but after #18931 I can't even trigger at it the max hunk size (512x). Anyway, most of the files were bigger going from 1x to 8x but I still went ahead and converted my entire collection in the hopes that it would be a win overall; unfortunately it wasn't and my collection grew by 1.1%.
So yeah, that was pretty unexpected. I don't know what went wrong but CHD seems to indeed have some problems (for another example look at how badly CHD handles data that can easily be regenerated). I'd hate to say it but I kinda have to agree with LunaMoo: Yes, CHD beats CSO by 5.8% but maybe CHD isn't as awesome as I initially thought it would be. Maybe Aaru Format will save us all?

Edit: More stats because why not:
⅓ of my games are smaller with 1x hunk size but they save 50GB.
The other ⅔ are smaller with 8x hunk size but they only save 26GB (increasing the size by 24GB over my entire collection).
Meaning the best solution would be to make that decision on a file-by-file basis but nobody has time for that so as a rule of thumb I recommend going for createdvd -hs 2048 until CHD gets fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants