oxipng
oxipng copied to clipboard
shssoichiro
Use customizable or user-supplied BufWriter for Zopfli
I have a program that creates large PNGs (up to 48 MiB of raw pixel data, but usually 12 or 16 MiB) and uses Zopfli as the deflater for PNG encoding. At these sizes, compression ratios are usually between 99.4% and 99.9%. Compression is currently slow because of the splits at every ZOPFLI_MASTER_BLOCK_SIZE (currently 1 million) bytes. When I raised this issue at https://github.com/zopfli-rs/zopfli/issues/18, they suggested to instead use a large BufWriter wrapping a DeflateEncoder. Could we please have the option to do that? The best way would be to have the user supply a mutably borrowed BufWriter, so that we could reuse the buffer for multiple images.
Putting on my OxiPNG contributor hat, I think this might be part of the explanation for #414. I'm busy with other things at the moment, but I feel it's worth taking a look at how performance compares with different master block sizes here.
See https://github.com/Pr0methean/oxipng/commit/97248f799f3895e808a9720e296b2717927a468a.
I'll experiment with this in a fork.
@Pr0methean How are the results from this fork/commit?
@AlexTMjugador Afaik, zopflipng doesn't alter the ZOPFLI_MASTER_BLOCK_SIZE - it's a compile time constant which defaults to 1MB. So surely that wouldn't explain the other issue?
So surely that wouldn't explain the other issue?
That constant is indeed unchanged, but it might be the case (I'm not sure right now, I may be wrong) that ZopfliPNG exercises a different code path that does not use that constant for segmenting the input data. After all, practical PNG images are not that big, and that master block size was mostly meant to deal with big datasets such as the enwiki corpus.
@TPS So far, the speed seems to be about the same, but it has the advantage that Zopfli can choose strategically where to split the file. I currently set it to a maximum of 64 splits for a 4096x4096 32-bit RGBA image, but in practice it chooses 24-36 blocks for the first attempt and up to 54 for the second attempt.
@Pr0methean That seems encouraging. Why not make this an official PR here (draft, if you're not satisfied &/or done w/ tweaking it), referring this & the above issue, so, whenever the rest are merged by @shssoichiro in preparation for the next version & binary update, your work can benefit all of us? 🙇🏾♂️
- As noted in the comments, part of the code is "forked" from zopfli-rs. I'm not sure what's different, but surely it would be better to get the required changes submitted to the zopfli library? It doesn't seem like a great idea to keep partially forked code here.
@Pr0methean Per andrews05's comment, did you happen to PR to zopli-rs? I'm curious to find out how it'd do.
Yes; my PR was remade as https://github.com/shssoichiro/oxipng/pull/530 and merged.
@Pr0methean Since this is merged @ https://github.com/zopfli-rs/zopfli/commit/21d552107a003ac312c49f7346bba3f7722458a8, close FTW?