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 for loading CSO-compressed ISOs #504

Merged
merged 6 commits into from Apr 15, 2015

Conversation

Projects
None yet
5 participants
@unknownbrackets
Contributor

unknownbrackets commented Apr 14, 2015

This adds support for loading files compressed using CSO, a format that is often used for PSP ISOs. It just compresses blocks of the file, so it can work with many different file types.

I've written a CSO compressor which supports >4GB CSO files. Not all existing CSO tools support > 4GB files unfortunately, but some others do.

This patch also works fine with such ISOs, e.g. Wild Arms 1: Alter Code F.

This implementation is similar to PPSSPP's, although for simplicity it lacks a few optimizations. If there's interest, I can probably implement async reading and the multi-block read optimization. But, I think this can/should be merged beforehand, as it already works fairly well.

This isn't intended to replace or be better than the existing gz feature. But supporting more file types is not a bad thing, and there are more tools available which support random-access in CSO files (for hacking or etc.)

-[Unknown]

@Dokman

This comment has been minimized.

Contributor

Dokman commented Apr 14, 2015

like you said i think it's more useful a gz not a CSO because have minor space

@unknownbrackets

This comment has been minimized.

Contributor

unknownbrackets commented Apr 14, 2015

Actually, that's not always the case. In the only size comparison I did, the size of a 7-zip .gz (max compression) file + the pcsx2 index file was 1.8% larger than the cso file (using --block=16384.) Plus, a cso file is only one file, so it's a bit more organized.

Again, that's not to say cso files are better than gz files, but I don't think it's fair to say that gz files are definitely better than cso files either.

-[Unknown]

@avih

This comment has been minimized.

Member

avih commented Apr 14, 2015

Well, both of them use zlib, so it'd be a fair assumption that their compression ratios would be similar (and probably configurable within the same parameters and performance ranges), up to the facilities each use to speed up seek (blocks and indexed with the gzip implementation, blocks with the CSO decompression).

Which open source tools can handle this format (compress/decompress/modify/etc)?

@unknownbrackets, the patch looks reasonable, though obviously I'm not able to tell if it actually works or not after just reading the code, so:

  1. Is there some specification for this file format? if yes, please add a link to it and add some implementation notes if/where this patch deviates from the reference implementation, and any other notes on constants etc which could help others who look at the code, tools which could be used to handle this format, etc.
  2. I think it'd be nicer if DetectCompressed / GetNewReader don't duplicate code.
  3. How much did you test it? did you try non-english file names, how's the performance of it in general, e.g. compared to uncompressed/ntfs-compressed/gzipped iso? are there known (to you) issues with this implementation (performance/compliance/etc)? etc.
  4. There's a cache facility which you can use practically for free and it could improve performance meaningfully. I'd consider trying to use it.
  5. CompressedFileReader.cpp became a bit big with this patch, and with two implementations which are mostly unrelated. I think some refactoring would be useful here, probably with CsoFileReader and GzippedFileReader each in its own cpp/h file, and common stuff in.. a common place :)

What do you think?

@unknownbrackets

This comment has been minimized.

Contributor

unknownbrackets commented Apr 14, 2015

There's a listing of tools here that can handle the format:
https://github.com/unknownbrackets/maxcso#other-tools

Of course, maxcso itself (as previously mentioned) is the tool I'd recommend (I may be biased.) It supports using multiple threads, uses multiple compression method trials based on options, and can generate the CRC of a CSO file's uncompressed data.

Booster's original version is arguably the original "specification", but I've created a document here:
https://github.com/unknownbrackets/maxcso/blob/master/README_CSO.md

I'll add a link to that in the code.

2 - They don't really. Unless you want DetectCompressed() to include a delete but that seems silly to me. I was intending not to change the current interface, but my slight preference would be to replace both of those functions with a GetNewReader() that simply returns nullptr when it can't handle it.

3 -
Testing: as mentioned, this is based on the implementation in PPSSPP; it's worked well there for quite some time.

I've tested this patch in pcsx2 with several ISOs mostly of different sizes (e.g. ~6+GB, 1GB, etc.) It works with non-English filenames, but it uses the same methods of file access as the gz compression type, so it will have whatever known bugs that has.

Performance: I have not carefully measured performance. This is a newly supported format and I've primarily ensured it does not impact existing performance. However, based on performance tests (and improvements, some of which incorporated here) made with PPSSPP, I'm fairly sure it won't perform poorly. That is, this is already based on an optimized implementation.

It could (as already mentioned in the description of this pull):

  • Read asynchronously (which gz does not either.)
  • Read multiple frames (when reading > 1 frame worth of blocks) in a single disk read.

The first is not even implemented with gz file support. The second did not give a very big improvement on PPSSPP, but did provide a measurable one, and so as I mentioned, I can implement it after this is merged.

Also, I suppose it could:

  • Use the block cache thing the gz implementation uses.
  • Use the async prefetch thing that the gz implementation uses.

Compliance: The only feature of the format that this patch lacks is alternative versions:

  • "ZSO" is a modification the procfw team created, which uses lz4 and different magic bytes.
  • "CSOv2" is a modification I created, which uses lz4 and zlib, and a different version identifier.

Neither have seen any significant adoption. This patch will reject files of either type as invalid.

4 - Caching: I can test that. I remember trying the CachingFileLoader we have in PPSSPP and it not improving things much, but maybe the semantics of PSP game access are significantly different (PPSSPP's caching includes read-ahead and is tuned to play video/etc smoothly when reading over the network, though, so its semantics are surely different from the cache here.)

This implementation does use a small cache, which is the last-decompressed frame (important when games read small numbers of bytes sequentially.) At least in PPSSPP, I found that this was by far the most common case of a block being read multiple times. It's definitely good for performance, although deflate is fast even without it, of course.

5 - Well, I noticed that CompressedFileReader itself was in a completely different file:

// Factory - creates an AsyncFileReader derived instance which can read a compressed file

So I assumed this was intentional. If that's not the case, sure, I'd be more than happy to split them out. I also would prefer the two classes be in separate files with their own names. Is CDVD/ fine?

-[Unknown]

@cyclonmaster

This comment has been minimized.

cyclonmaster commented Apr 14, 2015

I always use YACC: http://yacc.pspgen.com/
This is for PSP, will it work for PS2 iso too?

@unknownbrackets

This comment has been minimized.

Contributor

unknownbrackets commented Apr 14, 2015

YACC just uses CisoPlus internally (it's just a frontend.) I don't know if it has large-file support, but at least for < 2GB ISOs I'm fairly sure it'll work.

Otherwise, I would recommend the following to answer your question (since CisoPlus is not open source):

  1. Compress a DVD-9 ISO with CisoPlus/YACC.
  2. Download maxcso from its Releases page.
  3. Run maxcso --crc result_from_cisoplus.cso.
  4. Verify the crc against the original via redump or maxcso --crc original.iso.

Note that PSP UMDs could only be 1.8GB. It won't be shocking if it does not support larger files (since YACC and CisoPlus are largely PSP-centric tools.)

-[Unknown]

@unknownbrackets

This comment has been minimized.

Contributor

unknownbrackets commented Apr 14, 2015

FWIW, I tried briefly, CisoPlus at least seems to fail immediately at trying to get the filesize of a 7.71 GB ISO. So it won't even appear to work, at least.

-[Unknown]

@cyclonmaster

This comment has been minimized.

cyclonmaster commented Apr 15, 2015

Trying maxcso 1.4.6.
There is two files inside which is maxcso.exe & maxcso32.exe
What is the different between both of it?
Which one should I use?

@unknownbrackets

This comment has been minimized.

Contributor

unknownbrackets commented Apr 15, 2015

Use maxcso.exe. maxcso32.exe is the 32-bit version for use on 32-bit operating systems.

-[Unknown]

@unknownbrackets

This comment has been minimized.

Contributor

unknownbrackets commented Apr 15, 2015

I've updated this with some refactoring as mentioned. Note that the code in Gzipped and etc. are just moved existing lines.

-[Unknown]

@avih

This comment has been minimized.

avih commented on pcsx2/CDVD/GzippedFileReader.h in 334f648 Apr 15, 2015

Since these three constants are now at an h file, they should probably be prefixed, e.g. GZI_SPAN_DEFAULT etc. You can do that in another commit or another PR or I can do it after we merge. Just let me know.

This comment has been minimized.

Owner

unknownbrackets replied Apr 15, 2015

Right, done.

-[Unknown]

#include "CompressedFileReaderUtils.h"
#include "CsoFileReader.h"
#include "Pcsx2Types.h"
#include "zlib_indexed.h"

This comment has been minimized.

@avih

avih Apr 15, 2015

Member

Why do you need zlib_indexed.h?

This comment has been minimized.

@unknownbrackets

unknownbrackets Apr 15, 2015

Contributor

For PX_fseeko. I see FileStream, but it doesn't seem to be unicode-safe.

Anyway, I've just moved the define into CompressedFileReaderUtils.h and included zlib directly.

-[Unknown]

@avih

This comment has been minimized.

Member

avih commented Apr 15, 2015

Looks very good overall, thanks! Just address my two code comments (constants prefixed at the h file and if you really need zlib_indexed.h) and we can merge.

avih added a commit that referenced this pull request Apr 15, 2015

Merge pull request #504 from unknownbrackets/cso
Add support for loading CSO-compressed ISOs

@avih avih merged commit 080459e into PCSX2:master Apr 15, 2015

@avih

This comment has been minimized.

Member

avih commented Apr 15, 2015

Much appreciated :)

@unknownbrackets

This comment has been minimized.

Contributor

unknownbrackets commented Apr 15, 2015

So, I tried using the chunk cache thing.

Here's what I found:

  1. Cache hit rate was something like 25% or so (sometimes higher.) This proved out in multiple games. That's great, seems much higher than I expected.
  2. However, on my ivy core i7, the overhead of looking up and copying into the cache was pretty high.

What this means to me is that the overhead may not be worth it, considering how fast deflate is.

So, that led me to do the read from zlib for cache hits or misses, and time that vs. the overhead of cache lookup + write on miss.

Here's a case where I let some music loop to get a ~25% cache hit:
CSO: cache: 6695 / 25925, wasted: 0.656114, reading: 1.093790

In this case, 1.09 represents the total time it would take to read without the cache. 0.656 is the extra time taken by using caching.

It proves out with other tests, but 1.093790 * (1 - 0.25) + 0.656114 (time spent with cache) is worse than 1.093790 (time spent without cache.) The difference in this case is +35%.

It would probably help more if you are in an area and music is looping a lot, the game is re-reading from the disc when you enter battles, etc. So, it can most likely win. However, it doesn't look like the win will be huge, and it will most likely increase load times to get there.

Therefore, I don't think it makes sense to add ChunksCache to the CSO implementation.

Edit: oops, I was actually counting cache lookups in the read time as well by mistake, so it's worse than that (although probably not much, I think the malloc/memcpy on write is slower.)

-[Unknown]

@VIRGINKLM

This comment has been minimized.

VIRGINKLM commented Nov 10, 2015

Very nice, I've been wanting cso support to be spread over more platforms/consoles/emulators for quite some time now. It's more or less a dream come true.

@cascardian cascardian referenced this pull request Sep 10, 2016

Closed

GZIP & CSO support #1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment