search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.71k stars 17.5k forks source link

proposal: image/png: Add HuffmanOnly compression levels. #59023

Open HyeockJinKim opened 1 year ago

HyeockJinKim commented 1 year ago

Problem:

The HuffmanOnly option, which only performs Huffman encoding, is supported by the zlib library, but not by png using zlib.

Solution

Add a HuffmanOnly const to image/png.

Golang developer can pass the HuffmanOnly option with the code below and get a PNG with only huffman encoding.

png.Encoder{
    CompressionLevel: png.HuffmanOnly,
}
gopherbot commented 1 year ago

Change https://go.dev/cl/476017 mentions this issue: image/png: Add huffmanOnly compression level to png

cherrymui commented 1 year ago

Are you proposing adding a new API to the image/png package? (Adding an exported constant is a new API.) If so, you'll need to follow the proposal process, see https://golang.org/s/proposal . Thanks.

HyeockJinKim commented 1 year ago

Is it better to edit this issue (#59023) now as a template for a proposal? Or should I follow the link below to create a new issue as a template for a proposal? https://github.com/golang/proposal/blob/master/go2-language-changes.md

Thanks for your help.

ianlancetaylor commented 1 year ago

@HyeockJinKim Either way is fine. Thanks.

HyeockJinKim commented 1 year ago

I've created it according to the proposal template. Can you review it? @ianlancetaylor

HyeockJinKim commented 1 year ago

It would also be nice to work on supporting all zlib compression levels from 1-9, would you mind if I put that in the proposal as well?

ianlancetaylor commented 1 year ago

Sure, you can update the proposal. But you used the wrong template. You used the one for a language or incompatible library change (Go 2) not the one for a simple additional proposal. May be easier to start a new issue at this point. Thanks.

HyeockJinKim commented 1 year ago

I've rewritten it, please review it again. :cry:

ianlancetaylor commented 1 year ago

CC @nigeltao

nigeltao commented 1 year ago

Adding const HuffmanOnly CompressionLevel = -4 to package png LGTM.

Optionally, if you also want to implement this (below) in image/png/writer.go, then that LGTM too.

    // Positive CompressionLevel values are reserved to mean a numeric zlib
    // compression level, although that is not implemented yet.

In hindsight, it would have been nice if the numeric values were consistent (and, perhaps, that 0 meant "default"). But it's too late to change existing constants.

HyeockJinKim commented 1 year ago

Shall I add to CL to receive compression levels of 1-9?

nigeltao commented 1 year ago

I'm OK with that, but I'd also wait until there's a formal response to the proposal.