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>

src/image/decode_test.go

@@ -36,6 +36,7 @@ var imageTests = []imageTest{
 	{"testdata/video-001.221212.png", "testdata/video-001.221212.jpeg", 8 << 8},
 	{"testdata/video-001.cmyk.png", "testdata/video-001.cmyk.jpeg", 8 << 8},
 	{"testdata/video-001.rgb.png", "testdata/video-001.rgb.jpeg", 8 << 8},
+	{"testdata/video-001.progressive.truncated.png", "testdata/video-001.progressive.truncated.jpeg", 8 << 8},
 	// Grayscale images.
 	{"testdata/video-005.gray.png", "testdata/video-005.gray.jpeg", 8 << 8},
 	{"testdata/video-005.gray.png", "testdata/video-005.gray.png", 0},

src/image/jpeg/reader.go

@@ -641,6 +641,12 @@ func (d *decoder) decode(r io.Reader, configOnly bool) (image.Image, error) {
 			return nil, err
 		}
 	}
+
+	if d.progressive {
+		if err := d.reconstructProgressiveImage(); err != nil {
+			return nil, err
+		}
+	}
 	if d.img1 != nil {
 		return d.img1, nil
 	}

src/image/jpeg/scan.go

@@ -173,7 +173,6 @@ func (d *decoder) processSOS(n int) error {
 				compIndex := scan[i].compIndex
 				hi := d.comp[compIndex].h
 				vi := d.comp[compIndex].v
-				qt := &d.quant[d.comp[compIndex].tq]
 				for j := 0; j < hi*vi; j++ {
 					// The blocks are traversed one MCU at a time. For 4:2:0 chroma
 					// subsampling, there are four Y 8x8 blocks in every 16x16 MCU.
@@ -286,55 +285,19 @@ func (d *decoder) processSOS(n int) error {
 					}
 
 					if d.progressive {
-						if zigEnd != blockSize-1 || al != 0 {
-							// We haven't completely decoded this 8x8 block. Save the coefficients.
-							d.progCoeffs[compIndex][by*mxx*hi+bx] = b
-							// At this point, we could execute the rest of the loop body to dequantize and
-							// perform the inverse DCT, to save early stages of a progressive image to the
-							// *image.YCbCr buffers (the whole point of progressive encoding), but in Go,
-							// the jpeg.Decode function does not return until the entire image is decoded,
-							// so we "continue" here to avoid wasted computation.
-							continue
-						}
+						// Save the coefficients.
+						d.progCoeffs[compIndex][by*mxx*hi+bx] = b
+						// At this point, we could call reconstructBlock to dequantize and perform the
+						// inverse DCT, to save early stages of a progressive image to the *image.YCbCr
+						// buffers (the whole point of progressive encoding), but in Go, the jpeg.Decode
+						// function does not return until the entire image is decoded, so we "continue"
+						// here to avoid wasted computation. Instead, reconstructBlock is called on each
+						// accumulated block by the reconstructProgressiveImage method after all of the
+						// SOS markers are processed.
+						continue
 					}
-
-					// Dequantize, perform the inverse DCT and store the block to the image.
-					for zig := 0; zig < blockSize; zig++ {
-						b[unzig[zig]] *= qt[zig]
-					}
-					idct(&b)
-					dst, stride := []byte(nil), 0
-					if d.nComp == 1 {
-						dst, stride = d.img1.Pix[8*(by*d.img1.Stride+bx):], d.img1.Stride
-					} else {
-						switch compIndex {
-						case 0:
-							dst, stride = d.img3.Y[8*(by*d.img3.YStride+bx):], d.img3.YStride
-						case 1:
-							dst, stride = d.img3.Cb[8*(by*d.img3.CStride+bx):], d.img3.CStride
-						case 2:
-							dst, stride = d.img3.Cr[8*(by*d.img3.CStride+bx):], d.img3.CStride
-						case 3:
-							dst, stride = d.blackPix[8*(by*d.blackStride+bx):], d.blackStride
-						default:
-							return UnsupportedError("too many components")
-						}
-					}
-					// Level shift by +128, clip to [0, 255], and write to dst.
-					for y := 0; y < 8; y++ {
-						y8 := y * 8
-						yStride := y * stride
-						for x := 0; x < 8; x++ {
-							c := b[y8+x]
-							if c < -128 {
-								c = 0
-							} else if c > 127 {
-								c = 255
-							} else {
-								c += 128
-							}
-							dst[yStride+x] = uint8(c)
-						}
+					if err := d.reconstructBlock(&b, bx, by, int(compIndex)); err != nil {
+						return err
 					}
 				} // for j
 			} // for i
@@ -470,3 +433,70 @@ func (d *decoder) refineNonZeroes(b *block, zig, zigEnd, nz, delta int32) (int32
 	}
 	return zig, nil
 }
+
+func (d *decoder) reconstructProgressiveImage() error {
+	// The h0, mxx, by and bx variables have the same meaning as in the
+	// processSOS method.
+	h0 := d.comp[0].h
+	mxx := (d.width + 8*h0 - 1) / (8 * h0)
+	for i := 0; i < d.nComp; i++ {
+		if d.progCoeffs[i] == nil {
+			continue
+		}
+		v := 8 * d.comp[0].v / d.comp[i].v
+		h := 8 * d.comp[0].h / d.comp[i].h
+		stride := mxx * d.comp[i].h
+		for by := 0; by*v < d.height; by++ {
+			for bx := 0; bx*h < d.width; bx++ {
+				if err := d.reconstructBlock(&d.progCoeffs[i][by*stride+bx], bx, by, i); err != nil {
+					return err
+				}
+			}
+		}
+	}
+	return nil
+}
+
+// reconstructBlock dequantizes, performs the inverse DCT and stores the block
+// to the image.
+func (d *decoder) reconstructBlock(b *block, bx, by, compIndex int) error {
+	qt := &d.quant[d.comp[compIndex].tq]
+	for zig := 0; zig < blockSize; zig++ {
+		b[unzig[zig]] *= qt[zig]
+	}
+	idct(b)
+	dst, stride := []byte(nil), 0
+	if d.nComp == 1 {
+		dst, stride = d.img1.Pix[8*(by*d.img1.Stride+bx):], d.img1.Stride
+	} else {
+		switch compIndex {
+		case 0:
+			dst, stride = d.img3.Y[8*(by*d.img3.YStride+bx):], d.img3.YStride
+		case 1:
+			dst, stride = d.img3.Cb[8*(by*d.img3.CStride+bx):], d.img3.CStride
+		case 2:
+			dst, stride = d.img3.Cr[8*(by*d.img3.CStride+bx):], d.img3.CStride
+		case 3:
+			dst, stride = d.blackPix[8*(by*d.blackStride+bx):], d.blackStride
+		default:
+			return UnsupportedError("too many components")
+		}
+	}
+	// Level shift by +128, clip to [0, 255], and write to dst.
+	for y := 0; y < 8; y++ {
+		y8 := y * 8
+		yStride := y * stride
+		for x := 0; x < 8; x++ {
+			c := b[y8+x]
+			if c < -128 {
+				c = 0
+			} else if c > 127 {
+				c = 255
+			} else {
+				c += 128
+			}
+			dst[yStride+x] = uint8(c)
+		}
+	}
+	return nil
+}