Add LZMA Decompression Support#11
Conversation
chyyran
left a comment
chyyran
left a comment
There was a problem hiding this comment.
Overall looks good, if you could write a test by generating an LZMA compressed diff via xdelta that would be great.
Also if you're going to pull in SharpCompress anyways, could you see if you could implement the Encoder side as well? That would let us check off the ❌ for xdelta with compression.
| private AddressCache addressCache; | ||
| private MemoryStream targetData; | ||
| private CustomCodeTableDecoder? customTable; | ||
| private readonly bool disableChecksums; |
There was a problem hiding this comment.
Is this actually useful? It seems like a big footgun...
There was a problem hiding this comment.
Sometimes ROMS of the same game are slightly different, but not in the way the delta cares about. With my test file, disabling checksums got me a playable rom hack.
src/VCDiff/Decoders/VcDecoderEx.cs
Outdated
| if (!Decode_Init(out bytesWritten, out var result, out var decodeAsync)) | ||
| return result; | ||
|
|
||
| var decompressors = new SharedDecompressors(); // Decompression streams are shared across windows |
There was a problem hiding this comment.
This shouldn't be created if the file is not compressed.
src/VCDiff/Decoders/WindowDecoder.cs
Outdated
| /// <param name="maxWindowSize">The maximum target window size in bytes</param> | ||
| public WindowDecoder(long dictionarySize, TByteBuffer buffer, int maxWindowSize = DefaultMaxTargetFileSize) | ||
| /// <param name="sharedDecompressors">Container for compression streams used across windows</param> | ||
| public WindowDecoder(long dictionarySize, TByteBuffer buffer, SharedDecompressors sharedDecompressors, int maxWindowSize = DefaultMaxTargetFileSize) |
There was a problem hiding this comment.
SharedDecompressors should be nullable; if the file is not compressed it should not be created.
src/VCDiff/Decoders/WindowDecoder.cs
Outdated
|
|
||
| private PinnedArrayRental Decompress(PinnedArrayRental pinnedArrayRental, byte secondaryCompressorId, ref XZStream? xzStream, ref MemoryStream? memoryStream) | ||
| { | ||
| if (secondaryCompressorId == 0) |
There was a problem hiding this comment.
This should be checked before the call to Decompress.
src/VCDiff/Decoders/VcDecoderEx.cs
Outdated
| if (!Decode_Init(out var bytesWritten, out var result, out var decodeAsync)) | ||
| return decodeAsync; | ||
|
|
||
| var decompressors = new SharedDecompressors(); // Decompression streams are shared across windows |
There was a problem hiding this comment.
Similarly here we should check if the diff requires compression and if not don't instantiate the decompressor.
|
I make no promises about being able to implement encoding, but I'll give it a try. |
|
It's fine if you don't implement encoding but it would be useful to generate test cases. For obvious reasons we can't use commercial ROMs to test. |
|
I did all the work for untested compression support: evandixon@be1e80b However, the critical blocker is that SharpCompress doesn't actually support XZ compression. XZStream is a read only stream. I could have saved myself almost 2 hours if I checked ahead of time. I pushed part of the refactoring I did to help the compression be more clean (confining compression to its own class), but I'm not including the encoder changes in this PR since they can't be tested yet. At the very least, the patch I used when I opened the PR still works. |
|
Could you write a test for the decompression support? It's fine to just create a compressed diff between |
|
Test added. I verified the patch is indeed compressed by checking these bytes in HxD. (Bit 1 of 0x04 means compressed, and 0x05 (only when compressed) means lzma/xz.) |
2 participants
I'm toying with the idea of making my own rom patcher using xdelta files. Doing so requires a library that can handle it, and yours is the best candidate I've found so far.
I encountered an xdelta file with secondary compression (found here, but requires a specific nds rom to test) that this library doesn't support, and I managed to make it work.
When VCD_DECOMPRESS is set and the compressor id is 2, xdelta uses LZMA compression. Decompressing it is fairly straightforward, as each section of a window has a variable int describing the uncompressed size, followed by XZ compressed data. The tricky part is that xdelta's decoder isn't reset after each window. I only got it working after ensuring the same XZStream is used for each section across all windows.
I only learned how vcdiff files are structured yesterday, so I'm open to feedback and suggestions on how to make these changes better fit into your project.