Commit Graph

9 Commits

Author SHA1 Message Date
Nigel Tao 5157039fbd image/color: tweak the YCbCr to RGBA conversion formula again.
The 0x10101 magic constant is a little more principled than 0x10100, as
the rounding adjustment now spans the complete range [0, 0xffff] instead
of [0, 0xff00].

Consider this round-tripping code:

y, cb, cr := color.RGBToYCbCr(r0, g0, b0)
r1, g1, b1 := color.YCbCrToRGB(y, cb, cr)

Due to rounding errors both ways, we often but not always get a perfect
round trip (where r0 == r1 && g0 == g1 && b0 == b1). This is true both
before and after this commit. In some cases we got luckier, in others we
got unluckier.

For example, before this commit, (180, 135, 164) doesn't round trip
perfectly (it's off by 1) but (180, 135, 165) does. After this commit,
both cases are reversed: the former does and the latter doesn't (again
off by 1). Over all possible (r, g, b) triples, there doesn't seem to be
a big change for better or worse.

There is some history in these CLs:

image/color: tweak the YCbCr to RGBA conversion formula.
https://go-review.googlesource.com/#/c/12220/2/src/image/color/ycbcr.go

image/color: have YCbCr.RGBA work in 16-bit color, per the Color
interface.
https://go-review.googlesource.com/#/c/8073/2/src/image/color/ycbcr.go

Change-Id: Ib25ba7039f49feab2a9d1a4141b86db17db7b3e1
Reviewed-on: https://go-review.googlesource.com/36732
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
2017-02-10 22:57:50 +00:00
Josh Bleecher Snyder 31da093c1e image/draw: remove some bounds checks from DrawYCbCr
It’d be nicer to write just

_ = dpix[x+3]

but the compiler isn’t able to reason about offsets
from symbols (yet).

image/draw benchmark:

YCbCr-8   722µs ± 3%   682µs ± 3%  -5.54%  (p=0.000 n=50+50)

Change-Id: Ia1e399496ed87c282bf0f9ca56c0b2d4948a0df9
Reviewed-on: https://go-review.googlesource.com/22146
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-04-17 06:25:28 +00:00
Martin Möhrmann 8955745bfb image/color: order color computation to match rgb
The order of computation was switched unintentionally
in https://go-review.googlesource.com/21910.

Revert the order to first compute g then b.

Change-Id: I8cedb5e45fbad2679246839f609bcac4f9052403
Reviewed-on: https://go-review.googlesource.com/22016
Reviewed-by: Nigel Tao <nigeltao@golang.org>
2016-04-15 07:34:37 +00:00
Martin Möhrmann f0c5b8b9c9 image/color: optimize YCbCrToRGB
Use one comparison to detect underflow and overflow simultaneously.
Use a shift, bitwise complement and uint8 type conversion to handle
clamping to upper and lower bound without additional branching.

Overall the new code is faster for a mix of
common case, underflow and overflow.

name     old time/op  new time/op  delta
YCbCr-2  1.12ms ± 0%  0.64ms ± 0%  -43.01%  (p=0.000 n=48+47)

name              old time/op  new time/op  delta
YCbCrToRGB/0-2    5.52ns ± 0%  5.77ns ± 0%  +4.48%  (p=0.000 n=50+49)
YCbCrToRGB/128-2  6.05ns ± 0%  5.52ns ± 0%  -8.69%  (p=0.000 n=39+50)
YCbCrToRGB/255-2  5.80ns ± 0%  5.77ns ± 0%  -0.58%  (p=0.000 n=50+49)

Found in collaboration with Josh Bleecher Snyder and Ralph Corderoy.

Change-Id: Ic5020320f704966f545fdc1ae6bc24ddb5d3d09a
Reviewed-on: https://go-review.googlesource.com/21910
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-04-12 23:04:26 +00:00
Nigel Tao c2023a0791 image/color: tweak the YCbCr to RGBA conversion formula.
Before, calling the RGBA method of YCbCr color would return red values
in the range [0x0080, 0xff80]. After, the range is [0x0000, 0xffff] and
is consistent with what Gray colors' RGBA method returns. In particular,
pure black, pure white and every Gray color in between are now exactly
representable as a YCbCr color.

This fixes a regression from Go 1.4 (where YCbCr{0x00, 0x80, 0x80} was
no longer equivalent to pure black), introduced by golang.org/cl/8073 in
the Go 1.5 development cycle. In Go 1.4, the +0x80 rounding was not
noticable when Cb == 0x80 && Cr == 0x80, because the YCbCr to RGBA
conversion truncated to 8 bits before multiplying by 0x101, so the
output range was [0x0000, 0xffff].

The TestYCbCrRoundtrip fuzzy-match tolerance grows from 1 to 2 because
the YCbCr to RGB conversion now maps to an ever-so-slightly larger
range, along with the usual imprecision of accumulating rounding errors.

Also s/int/int32/ in ycbcr.go. The conversion shouldn't overflow either
way, as int is always at least 32 bits, but it does make it clearer that
the computation doesn't depend on sizeof(int).

Fixes #11691

Change-Id: I538ca0adf7e040fa96c5bc8b3aef4454535126b9
Reviewed-on: https://go-review.googlesource.com/12220
Reviewed-by: Rob Pike <r@golang.org>
2015-07-15 05:29:00 +00:00
Nigel Tao f5b5e41814 image: spell coordinate consistently, without the hyphen.
Change-Id: I211c0d33dc292c6a703d788f6d4d286107bcb6b0
Reviewed-on: https://go-review.googlesource.com/8906
Reviewed-by: Rob Pike <r@golang.org>
2015-04-16 01:21:31 +00:00
Nigel Tao a3a193c018 image/internal/imageutil: inline the color.YCbCrToRGB calls.
The image/draw benchmark:
benchmark          old ns/op     new ns/op     delta
BenchmarkYCbCr     1198605       978647        -18.35%

Change-Id: Iacfc21e6f641ecb05adc00b3aec0048f1f43d265
Reviewed-on: https://go-review.googlesource.com/7952
Reviewed-by: Rob Pike <r@golang.org>
2015-03-25 21:01:57 +00:00
Nigel Tao 7180cfa864 image/internal/imageutil: generate subsample-ratio-specific code.
This is in preparation for inlining the color.YCbCrToRGB calls in a
follow-up change.

Change-Id: I30750ace11a8ef6016b3c1e0b4bfdbcc8151f9a5
Reviewed-on: https://go-review.googlesource.com/7951
Reviewed-by: Rob Pike <r@golang.org>
2015-03-24 01:01:20 +00:00
Nigel Tao b2f29511dd image/internal/imageutil: new package, used by image/draw and image/jpeg.
The imageutil.DrawYCbCr function lives in an internal package because it
is needed by both the image/draw and image/jpeg packages, but it doesn't
seem right for one of those two to depend on the other.

It could eventually go into the image package, but that would require
committing to an API for the rest of Go 1.x.

Change-Id: I7b12555c970d86409365e99eef9360702aaffa30
Reviewed-on: https://go-review.googlesource.com/7925
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
2015-03-23 23:22:58 +00:00