golang / go

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

image/png: don't ignore PNG gAMA chunk #20613

Open Deleplace opened 7 years ago

Deleplace commented 7 years ago

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.8.1 linux/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64" GOBIN="" GOEXE="" GOHOSTARCH="amd64" GOHOSTOS="linux" GOOS="linux" GOPATH="/home/valentin/go" GORACE="" GOROOT="/usr/local/go" GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64" GCCGO="gccgo" CC="gcc" GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build951360311=/tmp/go-build -gno-record-gcc-switches" CXX="g++" CGO_ENABLED="1" PKG_CONFIG="pkg-config" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2"

What did you do?

Decoded fruit.png (which contains a strong gamma value in a gAMA chunk).

Encoded back to result.png.

Opened result.png in a gAMA-aware viewer (Chromium)

What did you expect to see?

A pear.

What did you see instead?

An apple.

I understand (after reading file reader_test.go and part of the PNG spec) that handling ancillary chunks (e.g. gAMA) is not a requirement for a PNG decoder. I also understand that implementing it would require some (unknown to me) amount of work. So, the question boils down to "would it be a good idea or not to take the gAMA chunk into account?".

fruit

result

Deleplace commented 7 years ago
package main

import (
    "bytes"
    "image"
    _ "image/jpeg"
    "image/png"
    "io/ioutil"
)

func main() {
    data, err1 := ioutil.ReadFile("fruit.png")
    checkerr(err1)
    buf := bytes.NewBuffer(data)
    img, _, err2 := image.Decode(buf)
    checkerr(err2)

    var temoin bytes.Buffer
    err3 := png.Encode(&temoin, img)
    checkerr(err3)
    err4 := ioutil.WriteFile("result.png", temoin.Bytes(), 0666)
    checkerr(err4)
}

func checkerr(err error) {
    if err != nil {
        panic(err)
    }
}
bradfitz commented 7 years ago

/cc @nigeltao

nigeltao commented 7 years ago

It would be a good idea, but it's also non-trivial. The right solution would probably be to add some concept to the image/color package to represent color spaces (e.g. sRGB vs Adobe RGB), including the concept of gamma. Designing that new API is probably a substantial amount of work, and not something I have time for any time soon. Sorry.

See also #11420 "x/image/draw: color space-correct interpolation".

Deleplace commented 7 years ago

Hello Nigel, thanks for the reply.

Though my example involved Decode and Encode, I was more concerned about what happens in Decode, when the gAMA is encountered and ignored. How about "modifying the color values of each pixel at the end of Decode", i.e. "applying" the custom gamma, and then forget about gamma concepts?

Acknowledged, this is not the same thing as (and would be less thorough than) introducing color spaces concepts to the std image and color packages.

nigeltao commented 7 years ago

I'm wary of making a short-term workaround like that. If, for example, we shipped that in Go 1.10, then maintaining backwards compatibility might constrain us in doing the right solution for a later Go 1.x version.

MichaelTJones commented 6 years ago

I have implemented the gAMA, cHRM, and sRGB chunks in PNG's writer.go. I added constants for rendering intent, and parameters in Encoder.

// sRGB rendering intent: (https://www.w3.org/TR/PNG/#11cHRM 11.3.3.5)
type RenderingIntent byte

const (
    // Perceptual: images preferring good adaptation to the output device
    // gamut at the expense of colorimetric accuracy, such as photographs.
    Perceptual RenderingIntent = 0

    // Relative colorimetric: images requiring colour appearance matching
    // (relative to the output device white point), such as logos.
    Relative RenderingIntent = 1

    // Saturation: images preferring preservation of saturation at the expense
    // of hue and lightness, such as charts and graphs.
    Saturation RenderingIntent = 2

    // Absolute: images requiring preservation of absolute colorimetry, such
    // as previews of images destined for a different output device (proofs).
    Absolute RenderingIntent = 3
)

However, this is write-only and does not change any pixel values, it simply allows the user to have the output PNG represent the nature of the data; any gamma-encoding must have already been done before the image was supplied to the encoder. I could add the read side if desired.

It is dangerous to to the gamma encode in that commonly 8-bit components commonly need to be companded to a larger size and there is no natural place for that here. That said, I do have the code handy...