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

Amend for brotli (de)compress (-mmt1), update README.md #352

Merged
merged 4 commits into from
Sep 11, 2023

Conversation

sebres
Copy link
Contributor

@sebres sebres commented Sep 7, 2023

The fix contains:

  • if single-threaded brotli, retain artificial number set in BrotliHandler (0) - always prefer it for .br format, otherwise -mmt1 causes that it'd use brotli-mt and the stream is incompatible; -mmt=n with n >= 2 forces multi-threaded compression/decompression (therefore incompatible with standard brotli)
  • translate threads to decoder (previously ignored): important for -mmt>=2 to use brotli-mt (the streams are incompatible)
  • update README.md - brotli (de)compress, new hash algorithms (amend to Add SHA3 #330).

Amend to #351: without the fix, compression with -mmt1 creates brotli-mt stream (incompatible with default brotli) and decompression always used single-threaded compression, so can't unpack the stream, created with -mmt1 at all.

Now the compression and decompression are consistent and:

  • without -mmt it is equivalent -mmt0 but also -mmt1 and uses single-threaded brotli for compress/decompress.
  • with -mmt=n where n >= 2, it uses multi-threaded brotli (would create or extract incompatible to standard brotli stream).

@sebres
Copy link
Contributor Author

sebres commented Sep 7, 2023

@mcmilk, by the way, new brotli version (v.1.1.0) got released last weak.
I was trying to update it, but Brotli-Adjust.sh seems to be outdated (needs to wrap more headers and renames), no time to fix at the moment.
May be instead of such mockups for brotli a static library build as it is (and linking it to 7z) would be better in the long term?

@sebres sebres marked this pull request as draft September 7, 2023 18:02
…ler (0) - always prefer it for .br format, otherwise `-mmt1` causes that it'd use brotli-mt and the stream is incompatible; `-mmt=n` with `n >= 2` forces multi-threaded compression/decompression (therefore incompatible with standard brotli)
…>=2 to use brotli-mt (the streams are incompatible)
@sebres sebres changed the title README.md - brotli (de)compress, new hash algorithms Amend for brotli (de)compress (-mmt1), update README.md Sep 7, 2023
@sebres sebres marked this pull request as ready for review September 7, 2023 18:41
…rotli codec, zip type gets missing because exceeded kNumArcsMax in registering structure g_Arcs)
@sebres
Copy link
Contributor Author

sebres commented Sep 11, 2023

I added to this PR still one simple but important fix - 09a6777.
Without that fix zip type was missing (for instance 7z i | grep zip doesn't contains zip handler anymore), because after adding of brotli code, RegisterArc for zip silently exceeds kNumArcsMax in registering structure g_Arcs.

BTW, I guess this expects some todo later - some error must be thrown for the case we want to register more archive handlers than allowed by kNumArcsMax.

@mcmilk mcmilk merged commit 62bfad5 into mcmilk:master Sep 11, 2023
5 checks passed
@sebres sebres deleted the patch-1 branch September 11, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants