]> Cypherpunks repositories - gostls13.git/commitdiff
image/jpeg: reconstruct progressive images even if incomplete.
authorNigel Tao <nigeltao@golang.org>
Thu, 24 Mar 2016 04:34:45 +0000 (15:34 +1100)
committerNigel Tao <nigeltao@golang.org>
Thu, 31 Mar 2016 00:33:24 +0000 (00:33 +0000)
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
src/image/jpeg/reader.go
src/image/jpeg/scan.go
src/image/testdata/video-001.progressive.truncated.jpeg [new file with mode: 0644]
src/image/testdata/video-001.progressive.truncated.png [new file with mode: 0644]

index d16ef8a1a4ddcbdf093ed68bfe5788ef06baab67..85e235e729b7cc02cc75b0379ca1bfa33c9fb887 100644 (file)
@@ -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},
index adf97abbd1d35030ccc349c46269daa781c1ce40..c5834219a3e45363fc3944fbcf518a2fa96e43d3 100644 (file)
@@ -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
        }
index 99734c01af01a97c5f7767c9f4255eed9ef63a5c..e1104d27c2342fa46a8fb89bbf42e66b7b7ab026 100644 (file)
@@ -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
-                                               }
-                                       }
-
-                                       // Dequantize, perform the inverse DCT and store the block to the image.
-                                       for zig := 0; zig < blockSize; zig++ {
-                                               b[unzig[zig]] *= qt[zig]
+                                               // 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
                                        }
-                                       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
+}
diff --git a/src/image/testdata/video-001.progressive.truncated.jpeg b/src/image/testdata/video-001.progressive.truncated.jpeg
new file mode 100644 (file)
index 0000000..b5be8bc
Binary files /dev/null and b/src/image/testdata/video-001.progressive.truncated.jpeg differ
diff --git a/src/image/testdata/video-001.progressive.truncated.png b/src/image/testdata/video-001.progressive.truncated.png
new file mode 100644 (file)
index 0000000..baf1981
Binary files /dev/null and b/src/image/testdata/video-001.progressive.truncated.png differ