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

Use a single Vec between Decoder and ZlibStream #452

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fintelia
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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 #447 but in exchange this PS enables getting rid of an additional buffer / memcpy hop, right?

@fintelia
Copy link
Contributor Author

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.

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.

None yet

3 participants