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/color: Go 1.5 unexpected change in color.YCbCr.RGBA() #11691

Closed
pierrre opened this issue Jul 13, 2015 · 13 comments
Closed

image/color: Go 1.5 unexpected change in color.YCbCr.RGBA() #11691

pierrre opened this issue Jul 13, 2015 · 13 comments
Milestone

Comments

@pierrre
Copy link

pierrre commented Jul 13, 2015

My code: https://play.golang.org/p/AslT5NMUiU

Go 1.4: 0 0 0 65535
Go 1.5: 128 128 128 65535

@pierrre
Copy link
Author

pierrre commented Jul 13, 2015

color.YCbCr.RGBA() uses:

  • in Go 1.4 : color.YCbCrToRGB() with 8 bits color
  • in Go 1.5: custom code with 16 bits color

@pierrre pierrre changed the title Go 1.5, unexpected change in color.YCbCr.RGBA() image/color: Go 1.5 unexpected change in color.YCbCr.RGBA() Jul 13, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.5 milestone Jul 13, 2015
@ianlancetaylor
Copy link
Contributor

Leaving to Nigel to decide whether this needs to be documented or fixed.

@nigeltao
Copy link
Contributor

Yes, this was a deliberate change: https://go-review.googlesource.com/8073

As for whether this needs calling out in the Go 1.5 release notes, I'll leave that decision up to @robpike.

@robpike
Copy link
Contributor

robpike commented Jul 14, 2015

Someone was bitten so an item in the release notes would be good. Words please @nigeltao?

@nigeltao
Copy link
Contributor

The conversion of a color.YCbCr to RGBA now has finer detail. The RGBA method has always returned 16-bit color, but prior to Go 1.5, the low 8 bits equalled the high 8 bits. The following code:

var c color.YCbCr = etc
r, g, b, a := c.RGBA()
fmt.Println("Red (in the range [0-255]):", uint8(r))

has always been incorrect, but coincidentally printed the right number prior to Go 1.5. The correct code replaces "uint8(r)" with "uint8(r >> 8)". Even so, the number printed can differ slightly between Go 1.5 and previous versions.

@nigeltao
Copy link
Contributor

Also, if you are converting an image.YCbCr full of many YCbCr colors, then consider using the image/draw package (http://blog.golang.org/go-imagedraw-package) instead of explicitly calling the RGBA method on each pixel. Once again, the resultant RGBA image can differ slightly between Go 1.5 and previous versions.

@pierrre
Copy link
Author

pierrre commented Jul 14, 2015

color.YCbCr{Y: 0, Cb: 128, Cr: 128} is black, right?

This code:

func main() {
    c := color.YCbCr{Y: 0, Cb: 128, Cr: 128}
    r, g, b, a := c.RGBA()
    fmt.Println(r, g, b, a)
}

Prints 0 0 0 65535 with Go 1.4 and 128 128 128 65535 with Go 1.5.
color.RGBA64{128, 128, 128, 65535} is not black!
It's between color.YCbCr{Y: 0, Cb: 128, Cr: 128} and color.YCbCr{Y: 1, Cb: 128, Cr: 128}
(it's black if you convert it to RGBA with uint8(i >> 8))

@pierrre
Copy link
Author

pierrre commented Jul 14, 2015

I understand the "finer detail" improvement with 16-bit color.

But I don't understand:

but prior to Go 1.5, the low 8 bits equalled the high 8 bits. The following code:

var c color.YCbCr = etc
r, g, b, a := c.RGBA()
fmt.Println("Red (in the range [0-255]):", uint8(r))

has always been incorrect, but coincidentally printed the right number prior to Go 1.5. The correct code replaces "uint8(r)" with "uint8(r >> 8)".

It was done correctly in Go 1.4.
See:
https://github.com/golang/go/blob/release-branch.go1.4/src/image/color/ycbcr.go#L97
https://github.com/golang/go/blob/release-branch.go1.4/src/image/color/ycbcr.go#L86
https://github.com/golang/go/blob/release-branch.go1.4/src/image/color/color.go#L172

@pierrre
Copy link
Author

pierrre commented Jul 14, 2015

I just saw the release note:

Previously, the low 8 bits were just an echo of the high 8 bits;
now they contain more accurate information.

Does 128 128 128 65535 contains more accurate information?
I honestly don't understand.

@nigeltao
Copy link
Contributor

Yes, it is more accurate.

Those numbers (0 vs 128) are integers. Here's a floating point analogy: you can think of red, green, etc. values being between 0 (no red) and 1 (fully saturated red). In the equivalent of Go 1.4, converting a particular YCbCr color to RGBA would yield something like '0.1 red'. In the equivalent of Go 1.5, it would yield something like '0.102 red'. The actual number is different; it is more accurate.

@nigeltao
Copy link
Contributor

Separately, you're right that color.YCbCr{Y: 0, Cb: 128, Cr: 128} should be black. Go 1.5 returns 128 instead of 0 because it adds one half (of 256) to round to nearest instead of round down. Perhaps it shouldn't. I'll think about this.

@nigeltao nigeltao reopened this Jul 15, 2015
@gopherbot
Copy link

CL https://golang.org/cl/12220 mentions this issue.

@pierrre
Copy link
Author

pierrre commented Jul 15, 2015

Thank you! :)

@golang golang locked and limited conversation to collaborators Jul 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants