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

Don't verify adler32 checksum by default #254

Closed
Shnatsel opened this issue Nov 1, 2020 · 3 comments · Fixed by #386
Closed

Don't verify adler32 checksum by default #254

Shnatsel opened this issue Nov 1, 2020 · 3 comments · Fixed by #386

Comments

@Shnatsel
Copy link
Contributor

Shnatsel commented Nov 1, 2020

For lightly compressed images adler32 checksum calculation accounts for 50% of zlib decompression time, and the efforts to optimize adler32 implementation itself are largely exhausted.

adler32 allows verification of the final uncompressed bitstream; however, the input file integrity is already verified by crc32 in the PNG format. This makes adler32 verification redundant, since the only things it can detect are:

  1. A bug in the zlib decompression implementation
  2. File corruption so subtle that the outer crc32 missed it

Both of these scenarios are so rare that they're not worth the extra runtime overhead in the default configuration.

Disclaimer: my understanding is based on the summary provided here, I'm not an expert on the PNG format.

@fintelia
Copy link
Contributor

fintelia commented Nov 1, 2020

Even if there weren't an outer checksum, I'd still be quite sympathetic to this idea. File corruption really doesn't seem like much of a concern these days given the reliability guarantees built into lower layers of the stack

@HeroicKatora
Copy link
Member

I very much like the idea of a builder-style struct for the decoder that would allow configuring these options, and also to turning that validation off by default. At most it should go into some warning output, if the user provides any.

I also never quite understood what you're supposed to do with a crc failure. Ignore the image? That doesn't seem reasonable, if there's a perfectly fine amount of pixel data. You have a CRC which is supposed to allow you to repair small failures anyways but that's far too expensive on large data blocks—although it would be interesting to offer some optional post processing that at least tries. Is there any of the decoding crates that implements this?

@Shnatsel
Copy link
Contributor Author

Shnatsel commented Nov 1, 2020

PNG was created in the 90s for network use cases specifically. Back then networks were less reliable and could flip bits at random far more frequently than they do now. I assume you're supposed to re-download the image if CRC doesn't match. But that's just my guess.

From what I've seen, image viewers just ignore CRC mismatches.

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 a pull request may close this issue.

3 participants