Commit Graph

106 Commits

Author SHA1 Message Date
Jed Denlea c52aca1c76 image/gif: fix writeImageBlock with SubImages
If an image has been cropped horizontally, writeImageBlock detects that
its width and Stride differ and acts accordingly.

However, if an image has been cropped vertically, trimming from the
bottom, the whole original image will be written in place.  This results
in more data in the LZW stream than necessary, and many decoders
including image/gif's itself will fail to load.

Fixes #20692

Change-Id: Id332877e31bcf3729c89d8a50c1be0464028d82e
Reviewed-on: https://go-review.googlesource.com/45972
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Nigel Tao <nigeltao@golang.org>
2017-06-16 05:45:48 +00:00
Nigel Tao 6d9b900a6f image/gif: don't panic on large or nil-containing color.Palettes.
Fixes #20249

Change-Id: I5dceaef31de96345f8e6c155e12775dc4cc31bfb
Reviewed-on: https://go-review.googlesource.com/42790
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-05-06 00:37:43 +00:00
Nigel Tao 35cbc3b55b image/jpeg: fix extended sequential Huffman table selector (Th).
Previously, the package did not distinguish between baseline and
extended sequential images. Both are non-progressive images, but the Th
range differs between the two, as per Annex B of
https://www.w3.org/Graphics/JPEG/itu-t81.pdf

Extended sequential images are often emitted by the Guetzli encoder.

Fixes #19913

Change-Id: I3d0f9e16d5d374ee1c65e3a8fb87519de61cff94
Reviewed-on: https://go-review.googlesource.com/41831
Reviewed-by: David Symonds <dsymonds@golang.org>
2017-04-27 03:48:40 +00:00
Daniel Martí 516e6f6d5d all: remove some unused parameters in test code
Mostly unnecessary *testing.T arguments.

Found with github.com/mvdan/unparam.

Change-Id: Ifb955cb88f2ce8784ee4172f4f94d860fa36ae9a
Reviewed-on: https://go-review.googlesource.com/41691
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-04-25 14:38:10 +00:00
Nigel Tao 16663a85ba image/png: decode Gray8 transparent images.
Fixes #19553.

Change-Id: I414cb3b1c2dab20f41a7f4e7aba49c534ff19942
Reviewed-on: https://go-review.googlesource.com/38271
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-03-17 07:50:05 +00:00
Will Storey 4775b7feb1 image/gif: handle an extra data sub-block byte.
This changes the decoder's behaviour when there is stray/extra data
found after an image is decompressed (e.g., data sub-blocks after an LZW
End of Information Code). Instead of raising an error, we silently skip
over such data until we find the end of the image data marked by a Block
Terminator. We skip at most one byte as sample problem GIFs exhibit this
property.

GIFs should not have and do not need such stray data (though the
specification is arguably ambiguous). However GIFs with such properties
have been seen in the wild.

Fixes #16146

Change-Id: Ie7e69052bab5256b4834992304e6ca58e93c1879
Reviewed-on: https://go-review.googlesource.com/37258
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-03-02 23:49:32 +00:00
Cezar Sa Espinola 93a18acf1e image/png: reduce memory allocs encoding images by reusing buffers
This change allows greatly reducing memory allocations with a slightly
performance improvement as well.

Instances of (*png).Encoder can have a optional BufferPool attached to
them. This allows reusing temporary buffers used when encoding a new
image. This buffers include instances to zlib.Writer and bufio.Writer.

Also, buffers for current and previous rows are saved in the encoder
instance and reused as long as their cap() is enough to fit the current
image row.

A new benchmark was added to demonstrate the performance improvement
when setting a BufferPool to an Encoder instance:

$ go test -bench BenchmarkEncodeGray -benchmem
BenchmarkEncodeGray-4                 	    1000	   2349584 ns/op	 130.75 MB/s	  852230 B/op	      32 allocs/op
BenchmarkEncodeGrayWithBufferPool-4   	    1000	   2241650 ns/op	 137.04 MB/s	     900 B/op	       3 allocs/op

Change-Id: I4488201ae53cb2ad010c68c1e0118ee12beae14e
Reviewed-on: https://go-review.googlesource.com/34150
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-02-12 05:40:47 +00:00
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
Nigel Tao 3a20928157 image: fix the overlap check in Rectangle.Intersect.
This is a re-roll of a previous commit,
a855da29db, which was rolled back in
14347ee480.

It was rolled back because it broke a unit test in image/gif. The
image/gif code was fixed by 9ef65dbe06
"image/gif: fix frame-inside-image bounds checking".

The original commit message:

image: fix the overlap check in Rectangle.Intersect.

The doc comment for Rectangle.Intersect clearly states, "If the two
rectangles do not overlap then the zero rectangle will be returned."
Prior to this fix, calling Intersect on adjacent but non-overlapping
rectangles would return an empty but non-zero rectangle.

The fix essentially changes
if r.Min.X > r.Max.X || r.Min.Y > r.Max.Y { etc }
to
if r.Min.X >= r.Max.X || r.Min.Y >= r.Max.Y { etc }
(note that the > signs have become >= signs), but changing that line to:
if r.Empty() { etc }
seems clearer (and equivalent).

Change-Id: I2e3af1f1686064a573b2e513b39246fe60c03631
Reviewed-on: https://go-review.googlesource.com/36734
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-02-10 05:05:59 +00:00
Nigel Tao 9ef65dbe06 image/gif: fix frame-inside-image bounds checking.
The semantics of the Go image.Rectangle type is that the In and
Intersects methods treat empty rectangles specially. There are multiple
valid representations of an empty image.Rectangle. One of them is the
zero image.Rectangle but there are others. They're obviously not all
equal in the == sense, so we shouldn't use != to check GIF's semantics.

This change will allow us to re-roll
a855da29db "image: fix the overlap check
in Rectangle.Intersect" which was rolled back in
14347ee480.

Change-Id: Ie1a0d092510a7bb6170e61adbf334b21361ff9e6
Reviewed-on: https://go-review.googlesource.com/36639
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
2017-02-10 03:31:09 +00:00
Russ Cox 14347ee480 Revert "image: fix the overlap check in Rectangle.Intersect."
This reverts commit a855da29db.

Change-Id: I23c0351b0708877e0b3d1b44a2bc2799cee52cd1
Reviewed-on: https://go-review.googlesource.com/36426
Reviewed-by: Russ Cox <rsc@golang.org>
2017-02-07 15:21:56 +00:00
Nigel Tao a855da29db image: fix the overlap check in Rectangle.Intersect.
The doc comment for Rectangle.Intersect clearly states, "If the two
rectangles do not overlap then the zero rectangle will be returned."
Prior to this fix, calling Intersect on adjacent but non-overlapping
rectangles would return an empty but non-zero rectangle.

The fix essentially changes
if r.Min.X > r.Max.X || r.Min.Y > r.Max.Y { etc }
to
if r.Min.X >= r.Max.X || r.Min.Y >= r.Max.Y { etc }
(note that the > signs have become >= signs), but changing that line to:
if r.Empty() { etc }
seems clearer (and equivalent).

Change-Id: Ia654e4b9dc805978db3e94d7a9718b6366005360
Reviewed-on: https://go-review.googlesource.com/34853
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2017-02-07 14:07:02 +00:00
Thomas Bonfort 435450bf3c image/jpeg: improve performance when encoding *image.YCbCr
The existing implementation falls back to using image.At()
for each pixel when encoding an *image.YCbCr which is
inefficient and causes many memory allocations.

This change makes the jpeg encoder directly read Y, Cb, and Cr
pixel values.

benchmark                  old ns/op     new ns/op     delta
BenchmarkEncodeYCbCr-4     43990846      24201148      -44.99%

benchmark                  old MB/s     new MB/s     speedup
BenchmarkEncodeYCbCr-4     20.95        38.08        1.82x

Fixes #18487

Change-Id: Iaf2ebc646997e3e1fffa5335f1b0d642e15bd453
Reviewed-on: https://go-review.googlesource.com/34773
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Nigel Tao <nigeltao@golang.org>
2017-02-01 21:38:59 +00:00
Nigel Tao caba0bd189 image/png: implement grayscale transparency.
Change-Id: Ib9309ee499fc51be2662d778430ee30089822e57
Reviewed-on: https://go-review.googlesource.com/32143
Reviewed-by: Rob Pike <r@golang.org>
2016-10-28 02:10:13 +00:00
Nigel Tao 4951c79363 image/png: implement truecolor transparency.
Change-Id: I99b9a51db29d514ebaa9c1cfde65c0b5184c0f42
Reviewed-on: https://go-review.googlesource.com/32140
Reviewed-by: Rob Pike <r@golang.org>
2016-10-27 22:27:41 +00:00
Nigel Tao de4b065591 image/png: allow tRNS chunk without a PLTE chunk.
While https://www.w3.org/TR/PNG/#5ChunkOrdering says that tRNS's
ordering constraint is "After PLTE; before IDAT", it is legal for a tRNS
chunk to occur without a PLTE chunk at all, for greyscale and truecolor
transparency as opposed to palette-based transparency. See
https://www.w3.org/TR/PNG/#11transinfo

Fixes #17511.

Change-Id: I047b0b01d78a1cda65e00eeac229bb972cda431d
Reviewed-on: https://go-review.googlesource.com/32139
Reviewed-by: Rob Pike <r@golang.org>
2016-10-27 02:59:31 +00:00
Martin Möhrmann 2113c9ad0d image/color: improve speed of RGBA methods
Apply the optimizations added to color conversion functions in
https://go-review.googlesource.com/#/c/21910/ to the RGBA methods.

YCbCrToRGBA/0-4      6.32ns ± 3%  6.58ns ± 2%   +4.15%  (p=0.000 n=20+19)
YCbCrToRGBA/128-4    8.02ns ± 2%  5.89ns ± 2%  -26.57%  (p=0.000 n=20+19)
YCbCrToRGBA/255-4    8.06ns ± 2%  6.59ns ± 3%  -18.18%  (p=0.000 n=20+20)
NYCbCrAToRGBA/0-4    8.71ns ± 2%  8.78ns ± 2%   +0.86%  (p=0.036 n=19+20)
NYCbCrAToRGBA/128-4  10.3ns ± 4%   7.9ns ± 2%  -23.44%  (p=0.000 n=20+20)
NYCbCrAToRGBA/255-4  9.64ns ± 2%  8.79ns ± 3%   -8.80%  (p=0.000 n=20+20)

Fixes: #15260

Change-Id: I225efdf74603e8d2b4f063054f7baee7a5029de6
Reviewed-on: https://go-review.googlesource.com/31773
Run-TryBot: Martin Möhrmann <martisch@uos.de>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Nigel Tao <nigeltao@golang.org>
2016-10-25 07:51:17 +00:00
Nigel Tao 584e3ea258 image/color: tweak the formula for converting to gray.
This makes grayModel and gray16Model in color.go use the exact same
formula as RGBToYCbCr in ycbcr.go. They were the same formula in theory,
but in practice the color.go versions used a divide by 1000 and the
ycbcr.go versions used a (presumably faster) shift by 16.

This implies the nice property that converting an image.RGBA to an
image.YCbCr and then taking only the Y channel is equivalent to
converting an image.RGBA directly to an image.Gray.

The difference between the two formulae is non-zero, but small:
https://play.golang.org/p/qG7oe-eqHI

Updates #16251

Change-Id: I288ecb957fd6eceb9626410bd1a8084d2e4f8198
Reviewed-on: https://go-review.googlesource.com/31538
Reviewed-by: Rob Pike <r@golang.org>
2016-10-20 22:27:28 +00:00
Jeff R. Allen d1d798dd15 image/gif: check handling of truncated GIF files
All the prefixes of the testGIF produce errors today,
but they differ wildly in which errors: some are io.EOF,
others are io.ErrUnexpectedEOF, and others are gif-specific.
Make them all gif-specific to explain context, and make
any complaining about EOF be sure to mention the EOF
is unexpected.

Fixes #11390.

Change-Id: I742c39c88591649276268327ea314e68d1de1845
Reviewed-on: https://go-review.googlesource.com/17493
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-10-05 04:28:45 +00:00
Oliver Tonnhofer 7de7d20e9f image/png: improve compression by skipping filter for paletted images
Compression of paletted images is more efficient if they are not filtered.
This patch skips filtering for cbP8 images.
The improvements are demonstrated at https://github.com/olt/compressbench

Fixes #16196

Change-Id: Ie973aad287cacf9057e394bb01cf0e4448a77618
Reviewed-on: https://go-review.googlesource.com/29872
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-09-27 14:01:20 +00:00
Suyash 445f51fb11 image/png: add Encode and Decode examples
partially addresses #16360

Change-Id: I8274825b9ca6aec46294c8513b4795b0eb3062a2
Reviewed-on: https://go-review.googlesource.com/28992
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-09-21 19:47:04 +00:00
Brad Fitzpatrick 16f81b617e image/draw: add FloydSteinberg Drawer example
Updates #16360

Change-Id: I80b981aa291a8e16d2986d4a2dfd84d3819bf488
Reviewed-on: https://go-review.googlesource.com/29443
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2016-09-21 00:08:58 +00:00
Nigel Tao f782a7e075 image/draw: optimize drawFillOver as drawFillSrc for opaque fills.
Benchmarks are much better for opaque fills and slightly worse on non
opaque fills. I think that on balance, this is still a win.

When the source is uniform(color.RGBA{0x11, 0x22, 0x33, 0xff}):
name        old time/op  new time/op  delta
FillOver-8   966µs ± 1%    32µs ± 1%  -96.67%  (p=0.000 n=10+10)
FillSrc-8   32.4µs ± 1%  32.2µs ± 1%     ~      (p=0.053 n=9+10)

When the source is uniform(color.RGBA{0x11, 0x22, 0x33, 0x44}):
name        old time/op  new time/op  delta
FillOver-8   962µs ± 0%  1018µs ± 0%  +5.85%   (p=0.000 n=9+10)
FillSrc-8   32.2µs ± 1%  32.1µs ± 0%    ~     (p=0.148 n=10+10)

Change-Id: I52ec6d5fcd0fbc6710cef0e973a21ee7827c0dd9
Reviewed-on: https://go-review.googlesource.com/28790
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2016-09-09 01:17:26 +00:00
Nigel Tao 4618dd8704 image/gif: accept an out-of-bounds transparent color index.
This is an error according to the spec, but Firefox and Google Chrome
seem OK with this.

Fixes #15059.

Change-Id: I841cf44e96655e91a2481555f38fbd7055a32202
Reviewed-on: https://go-review.googlesource.com/22546
Reviewed-by: Rob Pike <r@golang.org>
2016-04-29 00:01:22 +00:00
Nigel Tao ac0ee77d63 image/gif: be stricter on parsing graphic control extensions.
See Section 23. Graphic Control Extension of the spec:
https://www.w3.org/Graphics/GIF/spec-gif89a.txt

Change-Id: Ie78b4ff4aa97e1b332ade67ae4fa25f7c0770610
Reviewed-on: https://go-review.googlesource.com/22547
Reviewed-by: Rob Pike <r@golang.org>
2016-04-28 23:58:26 +00:00
Martin Möhrmann 102cf2ae03 image/color: optimize RGBToYCbCr
Apply optimizations used to speed up YCbCrToRGB from
https://go-review.googlesource.com/#/c/21910/
to RGBToYCbCr.

name             old time/op  new time/op  delta
RGBToYCbCr/0-2   6.81ns ± 0%  5.96ns ± 0%  -12.48%  (p=0.000 n=38+50)
RGBToYCbCr/Cb-2  7.68ns ± 0%  6.13ns ± 0%  -20.21%  (p=0.000 n=50+33)
RGBToYCbCr/Cr-2  6.84ns ± 0%  6.04ns ± 0%  -11.70%  (p=0.000 n=39+42)

Updates #15260

Change-Id: If3ea5393ae371a955ddf18ab226aae20b48f9692
Reviewed-on: https://go-review.googlesource.com/22411
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ralph Corderoy <ralph@inputplus.co.uk>
2016-04-27 06:19:37 +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
Matthew Dempsky 0da4dbe232 all: remove unnecessary type conversions
cmd and runtime were handled separately, and I'm intentionally skipped
syscall. This is the rest of the standard library.

CL generated mechanically with github.com/mdempsky/unconvert.

Change-Id: I9e0eff886974dedc37adb93f602064b83e469122
Reviewed-on: https://go-review.googlesource.com/22104
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-04-15 07:31:45 +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
Josh Bleecher Snyder 7166dfe0c1 image/color: add YCbCrToRGB benchmark
Change-Id: I9ba76d5b0861a901415fdceccaf2f5caa2facb7f
Reviewed-on: https://go-review.googlesource.com/21837
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-04-11 19:15:58 +00:00
Nigel Tao 225b223e47 image/jpeg: reconstruct progressive images even if incomplete.
Fixes #14522.

As I said on that issue:

----
This is a progressive JPEG image. There are two dimensions of
progressivity: spectral selection (variables zs and ze in scan.go,
ranging in [0, 63]) and successive approximation (variables ah and al in
scan.go, ranging in [0, 8), from LSB to MSB, although ah=0 implicitly
means ah=8).

For this particular image, there are three components, and the SOS
markers contain this progression:

zs, ze, ah, al:  0  0 0 0	components: 0, 1, 2
zs, ze, ah, al:  1 63 0 0	components: 1
zs, ze, ah, al:  1 63 0 0	components: 2
zs, ze, ah, al:  1 63 0 2	components: 0
zs, ze, ah, al:  1 10 2 1	components: 0
zs, ze, ah, al: 11 63 2 1	components: 0
zs, ze, ah, al:  1 10 1 0	components: 0

The combination of all of these is complete (i.e. spectra 0 to 63 and
bits 8 exclusive to 0) for components 1 and 2, but it is incomplete for
component 0 (the luma component). In particular, there is no data for
component 0, spectra 11 to 63 and bits 1 exclusive to 0.

The image/jpeg code, as of Go 1.6, waits until both dimensions are
complete before performing the de-quantization, IDCT and copy to an
*image.YCbCr. This is the "if zigEnd != blockSize-1 || al != 0 { ...
continue }" code and associated commentary in scan.go.

Almost all progressive JPEG images end up complete in both dimensions
for all components, but this particular image is incomplete for
component 0, so the Go code never writes anything to the Y values of the
resultant *image.YCbCr, which is why the broken output is so dark (but
still looks recognizable in terms of red and blue hues).

My reading of the ITU T.81 JPEG specification (Annex G) doesn't
explicitly say that this is a valid image, but it also doesn't rule it
out.

In any case, the fix is, for progressive JPEG images, to always
reconstruct the decoded blocks (by performing the de-quantization, IDCT
and copy to an *image.YCbCr), regardless of whether or not they end up
complete. Note that, in Go, the jpeg.Decode function does not return
until the entire image is decoded, so we still only want to reconstruct
each block once, not once per SOS (Start Of Scan) marker.
----

A test image was also added, based on video-001.progressive.jpeg. When
decoding that image, inserting a

println("nComp, zs, ze, ah, al:", nComp, zigStart, zigEnd, ah, al)

into decoder.processSOS in scan.go prints:

nComp, zs, ze, ah, al: 3 0 0 0 1
nComp, zs, ze, ah, al: 1 1 5 0 2
nComp, zs, ze, ah, al: 1 1 63 0 1
nComp, zs, ze, ah, al: 1 1 63 0 1
nComp, zs, ze, ah, al: 1 6 63 0 2
nComp, zs, ze, ah, al: 1 1 63 2 1
nComp, zs, ze, ah, al: 3 0 0 1 0
nComp, zs, ze, ah, al: 1 1 63 1 0
nComp, zs, ze, ah, al: 1 1 63 1 0
nComp, zs, ze, ah, al: 1 1 63 1 0

In other words, video-001.progressive.jpeg contains 10 different scans.
This little program below drops half of them (remembering to keep the
"\xff\xd9" End of Image marker):

----
package main

import (
	"bytes"
	"io/ioutil"
	"log"
)

func main() {
	sos := []byte{0xff, 0xda}
	eoi := []byte{0xff, 0xd9}

	src, err := ioutil.ReadFile("video-001.progressive.jpeg")
	if err != nil {
		log.Fatal(err)
	}
	b := bytes.Split(src, sos)
	println(len(b)) // Prints 11.
	dst := bytes.Join(b[:5], sos)
	dst = append(dst, eoi...)
	if err := ioutil.WriteFile("video-001.progressive.truncated.jpeg", dst, 0666); err != nil {
		log.Fatal(err)
	}
}
----

The video-001.progressive.truncated.jpeg was converted to png via
libjpeg and ImageMagick:

djpeg -nosmooth video-001.progressive.truncated.jpeg > tmp.tga
convert tmp.tga video-001.progressive.truncated.png
rm tmp.tga

Change-Id: I72b20cd4fb6746d36d8d4d587f891fb3bc641f84
Reviewed-on: https://go-review.googlesource.com/21062
Reviewed-by: Rob Pike <r@golang.org>
2016-03-31 00:33:24 +00:00
Tilman Dilo 633e41432c image/png: ignore trailing IDAT chunks
Ignore superfluous trailing IDAT chunks which were not consumed when decoding
the image. This change fixes decoding of valid images in which a zero-length
IDAT chunk appears after the actual image data. It also prevents decoding of
trailing garbage IDAT chunks or maliciously embedded additional images.

Fixes #14936

Change-Id: I8c76cfa9a03496d9576f72bed2db109271f97c5e
Reviewed-on: https://go-review.googlesource.com/21045
Reviewed-by: Nigel Tao <nigeltao@golang.org>
2016-03-24 10:57:01 +00:00
Muhammed Uluyol 1d40e2b14b Rewrite leftover references to plan9.bell-labs.com to 9p.io.
Change-Id: Iadb4aa016a7b361d01827787dbc59164d5d147f2
Reviewed-on: https://go-review.googlesource.com/20291
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-03-06 07:07:01 +00:00
Brad Fitzpatrick 5fea2ccc77 all: single space after period.
The tree's pretty inconsistent about single space vs double space
after a period in documentation. Make it consistently a single space,
per earlier decisions. This means contributors won't be confused by
misleading precedence.

This CL doesn't use go/doc to parse. It only addresses // comments.
It was generated with:

$ perl -i -npe 's,^(\s*// .+[a-z]\.)  +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.)  +([A-Z])')
$ go test go/doc -update

Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7
Reviewed-on: https://go-review.googlesource.com/20022
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-03-02 00:13:47 +00:00
Brad Fitzpatrick 519474451a all: make copyright headers consistent with one space after period
This is a subset of https://golang.org/cl/20022 with only the copyright
header lines, so the next CL will be smaller and more reviewable.

Go policy has been single space after periods in comments for some time.

The copyright header template at:

    https://golang.org/doc/contribute.html#copyright

also uses a single space.

Make them all consistent.

Change-Id: Icc26c6b8495c3820da6b171ca96a74701b4a01b0
Reviewed-on: https://go-review.googlesource.com/20111
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-03-01 23:34:33 +00:00
Martin Möhrmann fdd0179bb1 all: fix typos and spelling
Change-Id: Icd06d99c42b8299fd931c7da821e1f418684d913
Reviewed-on: https://go-review.googlesource.com/19829
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-02-24 18:42:29 +00:00
Nigel Tao e4dcf5c8c2 image/color: have NYCbCrA.RGBA work in 16-bit color.
This makes NYCbCrA consistent with YCbCr.

Fixes #13706.

Change-Id: Ifced84372e4865925fa6efef9ca2f1de43da70e0
Reviewed-on: https://go-review.googlesource.com/18115
Reviewed-by: Rob Pike <r@golang.org>
2015-12-24 06:55:33 +00:00
Nigel Tao c478c48597 image: add NYCbCrA types.
Fixes #12722

Change-Id: I6a630d8b072ef2b1c63de941743148f8c96b8e5f
Reviewed-on: https://go-review.googlesource.com/15671
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2015-10-11 11:07:25 +00:00
Nathan Otterness 66c25fa9be image/png: integer underflow when decoding
This change addresses an integer underflow appearing only on systems
using a 32-bit int type. The patch addresses the problem by limiting the
length of unknown chunks to 0x7fffffff. This value appears to already be
checked for when parsing other chunk types, so the bug shouldn't appear
elsewhere in the package. The PNG spec recommends the maximum size for
any chunk to remain under 2^31, so this shouldn't cause errors with
valid images.

Fixes #12687

Change-Id: I17f0e1683515532c661cf2b0b2bc65309d1b7bb7
Reviewed-on: https://go-review.googlesource.com/14766
Reviewed-by: Nigel Tao <nigeltao@golang.org>
2015-09-21 23:09:22 +00:00
Nigel Tao 0cf7331391 image/png: reject zero-width and zero-height images.
http://www.w3.org/TR/PNG/#11IHDR says that "Zero is an invalid value".

This change only affects the decoder. The encoder already checks
non-positive instead of negative.

Fixes #12545.

Change-Id: Iba40e1a2f4e0eec8b2fbcd3bbdae886311434da7
Reviewed-on: https://go-review.googlesource.com/14411
Reviewed-by: Rob Pike <r@golang.org>
2015-09-09 02:53:58 +00:00
Didier Spezia 220b5f7b54 image/gif: map/slice literals janitoring
Simplify slice/map literal expressions.
Caught with gofmt -d -s, fixed with gofmt -w -s

Change-Id: Iefd5f263c4f89a81da9427a7b9d97f13c35ab64f
Reviewed-on: https://go-review.googlesource.com/13838
Reviewed-by: Andrew Gerrand <adg@golang.org>
2015-09-04 11:58:15 +00:00
Nigel Tao e424d59680 image/draw: optimize out some bounds checks.
We could undoubtedly squeeze even more out of these loops, and in the
long term, a better compiler would be smarter with bounds checks, but in
the short term, this small change is an easy win.

benchmark                      old ns/op     new ns/op     delta
BenchmarkFillOver-8            1619470       1323192       -18.29%
BenchmarkCopyOver-8            1129369       1062787       -5.90%
BenchmarkGlyphOver-8           420070        378608        -9.87%

On github.com/golang/freetype/truetype's BenchmarkDrawString:
benchmark                 old ns/op     new ns/op     delta
BenchmarkDrawString-8     9561435       8807019       -7.89%

Change-Id: Ib1c6271ac18bced85e0fb5ebf250dd57d7747e75
Reviewed-on: https://go-review.googlesource.com/14093
Reviewed-by: Rob Pike <r@golang.org>
2015-09-01 00:34:26 +00:00
Tarmigan Casebolt f7dc4eb921 image/gif: avoid unused assignment
Change-Id: Iaaecd8be9268c923f40cf0e5153cbf79f7015b8d
Reviewed-on: https://go-review.googlesource.com/13892
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2015-08-24 16:17:42 +00:00
Nigel Tao 816222d10f image/color: fix format typo in the tests.
Change-Id: I6f79d201aa4e8c0e3be8d965f14ed36518536036
Reviewed-on: https://go-review.googlesource.com/12281
Reviewed-by: Rob Pike <r@golang.org>
2015-07-16 01:41:34 +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 60b7d27c82 image/jpeg: don't unread a byte if we've already taken bits from it.
This rolls back most of golang.org/cl/8841, aka 2f98bac310, and makes a
different fix. It keeps the TestTruncatedSOSDataDoesntPanic test
introduced by that other CL, which obviously still passes after this CL.

Fixes #11650, a regression (introduced by cl/8841) from Go 1.4.

The original cl/8841 changed the image/jpeg not to panic on an input
given in #10387. We still do not panic on that input, after this CL.

I have a corpus of over 160,000 JPEG images, a sample of a web crawl.
The image/jpeg code ran happily over that whole corpus both before and
after this CL, although that corpus clearly didn't catch the regression
in the first place.

This code was otherwise tested manually. I don't think that it's trivial
to synthesize a JPEG input that happens to run out of Huffman data at
just the right place. The test image attached to #11650 obviously has
that property, but I don't think we can simply add that test image to
the repository: it's 227KiB, and I don't know its copyright status.

I also looked back over the issue tracker for problematic JPEGs that
people have filed. The Go code, after this CL, is still happy on these
files in my directory:
issue2362a.jpeg
issue3916.jpeg
issue3976.jpeg
issue4084.jpeg
issue4259.jpeg
issue4291.jpeg
issue4337.jpeg
issue4500.jpeg
issue4705.jpeg
issue4975.jpeg
issue5112.jpeg
issue6767.jpeg
issue9888.jpeg
issue10133.jpeg
issue10357.jpeg
issue10447.jpeg
issue11648.jpeg
issue11650.jpeg

There were other images attached in the issue tracker that aren't
actually valid JPEGs. They failed both before and after this CL:
broken-issue2362b.jpeg
broken-issue6450.jpeg
broken-issue8693.jpeg
broken-issue10154.jpeg
broken-issue10387.jpeg
broken-issue10388.jpeg
broken-issue10389.jpeg
broken-issue10413.jpeg

In summary, this CL fixes #11650 and, after some automated and manual
testing, I don't think introduces new regressions.

Change-Id: I30b67036e9b087f3051d57dac7ea05fb4fa36f66
Reviewed-on: https://go-review.googlesource.com/12163
Reviewed-by: Rob Pike <r@golang.org>
2015-07-14 06:21:57 +00:00
Nigel Tao ca6ba49269 image/png: don't read filter bytes for empty interlace passes.
Fixes #11604

The gray-gradient.png image was created by a Go program:

----
package main

import (
	"image"
	"image/color"
	"image/png"
	"log"
	"os"
)

func main() {
	f, err := os.Create("a.png")
	if err != nil {
		log.Fatal(err)
	}
	defer f.Close()
	m := image.NewGray(image.Rect(0, 0, 1, 16))
	for i := 0; i < 16; i++ {
		m.SetGray(0, i, color.Gray{uint8(i * 0x11)})
	}
	err = png.Encode(f, m)
	if err != nil {
		log.Fatal(err)
	}
}
----

The equivalent gray-gradient.interlaced.png image was created via ImageMagick:
$ convert -interlace PNG gray-gradient.png gray-gradient.interlaced.png

As a sanity check:
$ file gray-gradient.*
gray-gradient.interlaced.png: PNG image data, 1 x 16, 4-bit grayscale, interlaced
gray-gradient.png:            PNG image data, 1 x 16, 8-bit grayscale, non-interlaced

Change-Id: I7700284f74d1ea30073aede3bce4d7651787bdbc
Reviewed-on: https://go-review.googlesource.com/12064
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2015-07-13 06:40:00 +00:00
Brad Fitzpatrick 2ae77376f7 all: link to https instead of http
The one in misc/makerelease/makerelease.go is particularly bad and
probably warrants rotating our keys.

I didn't update old weekly notes, and reverted some changes involving
test code for now, since we're late in the Go 1.5 freeze. Otherwise,
the rest are all auto-generated changes, and all manually reviewed.

Change-Id: Ia2753576ab5d64826a167d259f48a2f50508792d
Reviewed-on: https://go-review.googlesource.com/12048
Reviewed-by: Rob Pike <r@golang.org>
2015-07-11 14:36:33 +00:00
Nigel Tao 40a1516a09 image/draw: fix double-draw when the dst is paletted.
The second (fallback) draw is a no-op, but it's a non-trivial amount of work.

Fixes #11550.

benchmark               old ns/op     new ns/op     delta
BenchmarkPaletted-4     16301219      7309568       -55.16%

Change-Id: Ic88c537b2b0c710cf517888f3dd15cb702dd142f
Reviewed-on: https://go-review.googlesource.com/11858
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2015-07-03 03:08:40 +00:00