image-rs / image-png

PNG decoding and encoding library in pure Rust
https://docs.rs/png
Apache License 2.0
357 stars 140 forks source link

Use a single Vec between Decoder and ZlibStream #452

Closed fintelia closed 2 months ago

fintelia commented 8 months ago

This simplifies buffer management and speeds up decoding. Still some lingering bugs/TODOs. Also need to figure out API considerations as right now this is a breaking change

Shnatsel commented 8 months ago

This Vec can also exceed allocation limits: #286 I was about to work on adding support for limits, but I see there are in-flight PRs touching this code, so I'll wait until this lands.

I'm excited to see this refactoring! Let me know if you want me to do a before/after test run on a large image corpus, or if there's any other way I can help test this.

anforowicz commented 8 months ago

Very interesting - thanks for looking into this! AFAIU fdeflate's requirement for retaining 32kB unmodified decompressed data is met by only calling unfilter on data that was decompressed earlier. Did I get that right? If so, this PR is to some extent reverting the changes from https://github.com/image-rs/image-png/pull/447 but in exchange this PS enables getting rid of an additional buffer / memcpy hop, right?

fintelia commented 8 months ago

Yes, but this enables another trick: for PNGs with less than 256 KB of uncompressed image data, it does all the decompression in one go. Which means that for those images we don't have to save any space for lookback because we'll be completely done processing the deflate stream by the time we start modifying the data using unfilter.