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

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

Open
Deleplace opened this issue Jun 8, 2017 · 6 comments
Open

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

Deleplace opened this issue Jun 8, 2017 · 6 comments
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@Deleplace
Copy link
Contributor

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

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 bradfitz changed the title Suggestion: not ignore PNG gAMA chunk image/png: don't ignore PNG gAMA chunk Jun 8, 2017
@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 8, 2017
@bradfitz bradfitz added this to the Go1.10 milestone Jun 8, 2017
@bradfitz
Copy link
Contributor

bradfitz commented Jun 8, 2017

/cc @nigeltao

@nigeltao
Copy link
Contributor

nigeltao commented Jun 9, 2017

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

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

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

MichaelTJones commented Oct 3, 2017

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants