vp8: skip filtering for all-zero-DC macroblock residuals.
This makes the Go code match the libwebp C code's output on
blue-purple-pink-large.*-filter.lossy.webp
Also make the various WEBP benchmarks all decode a similar image,
the image at http://blog.golang.org/gophercon/image01.jpg, to make
it more meaningful to e.g. compare the simple filter's numbers with
the normal filter's numbers.
Also fix a "go vet" warning in webp/decode.go.
The test data was generated by:
wget http://blog.golang.org/gophercon/image01.jpg -O blue-purple-pink-large.jpeg
convert blue-purple-pink-large.jpeg blue-purple-pink-large.png
cwebp -lossless blue-purple-pink-large.png -o blue-purple-pink-large.lossless.webp
cwebp -q 80 -f 0 blue-purple-pink-large.png -o blue-purple-pink-large.no-filter.lossy.webp
cwebp -q 80 -strong blue-purple-pink-large.png -o blue-purple-pink-large.normal-filter.lossy.webp
cwebp -q 80 -nostrong blue-purple-pink-large.png -o blue-purple-pink-large.simple-filter.lossy.webp
dwebp -pgm blue-purple-pink-large.no-filter.lossy.webp -o tmp.pgm && convert tmp.pgm blue-purple-pink-large.no-filter.lossy.webp.ycbcr.png && rm tmp.pgm
dwebp -pgm blue-purple-pink-large.normal-filter.lossy.webp -o tmp.pgm && convert tmp.pgm blue-purple-pink-large.normal-filter.lossy.webp.ycbcr.png && rm tmp.pgm
dwebp -pgm blue-purple-pink-large.simple-filter.lossy.webp -o tmp.pgm && convert tmp.pgm blue-purple-pink-large.simple-filter.lossy.webp.ycbcr.png && rm tmp.pgm
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/106230044
diff --git a/testdata/blue-purple-pink-large.lossless.webp b/testdata/blue-purple-pink-large.lossless.webp
new file mode 100644
index 0000000..d00c81f
--- /dev/null
+++ b/testdata/blue-purple-pink-large.lossless.webp
Binary files differ
diff --git a/testdata/blue-purple-pink-large.no-filter.lossy.webp b/testdata/blue-purple-pink-large.no-filter.lossy.webp
new file mode 100644
index 0000000..9067f4d
--- /dev/null
+++ b/testdata/blue-purple-pink-large.no-filter.lossy.webp
Binary files differ
diff --git a/testdata/blue-purple-pink-large.no-filter.lossy.webp.ycbcr.png b/testdata/blue-purple-pink-large.no-filter.lossy.webp.ycbcr.png
new file mode 100644
index 0000000..2e32c28
--- /dev/null
+++ b/testdata/blue-purple-pink-large.no-filter.lossy.webp.ycbcr.png
Binary files differ
diff --git a/testdata/blue-purple-pink-large.normal-filter.lossy.webp b/testdata/blue-purple-pink-large.normal-filter.lossy.webp
new file mode 100644
index 0000000..a4ccc1a
--- /dev/null
+++ b/testdata/blue-purple-pink-large.normal-filter.lossy.webp
Binary files differ
diff --git a/testdata/blue-purple-pink-large.normal-filter.lossy.webp.ycbcr.png b/testdata/blue-purple-pink-large.normal-filter.lossy.webp.ycbcr.png
new file mode 100644
index 0000000..5f7ec42
--- /dev/null
+++ b/testdata/blue-purple-pink-large.normal-filter.lossy.webp.ycbcr.png
Binary files differ
diff --git a/testdata/blue-purple-pink-large.png b/testdata/blue-purple-pink-large.png
new file mode 100644
index 0000000..9755505
--- /dev/null
+++ b/testdata/blue-purple-pink-large.png
Binary files differ
diff --git a/testdata/blue-purple-pink-large.simple-filter.lossy.webp b/testdata/blue-purple-pink-large.simple-filter.lossy.webp
new file mode 100644
index 0000000..09fdb94
--- /dev/null
+++ b/testdata/blue-purple-pink-large.simple-filter.lossy.webp
Binary files differ
diff --git a/testdata/blue-purple-pink-large.simple-filter.lossy.webp.ycbcr.png b/testdata/blue-purple-pink-large.simple-filter.lossy.webp.ycbcr.png
new file mode 100644
index 0000000..946b3af
--- /dev/null
+++ b/testdata/blue-purple-pink-large.simple-filter.lossy.webp.ycbcr.png
Binary files differ
diff --git a/vp8/reconstruct.go b/vp8/reconstruct.go
index 309d1c1..c1cc4b5 100644
--- a/vp8/reconstruct.go
+++ b/vp8/reconstruct.go
@@ -266,8 +266,9 @@
return 1
}
-// parseResiduals parses the residuals.
-func (d *Decoder) parseResiduals(mbx, mby int) {
+// parseResiduals parses the residuals and returns whether inner loop filtering
+// should be skipped for this macroblock.
+func (d *Decoder) parseResiduals(mbx, mby int) (skip bool) {
partition := &d.op[mby&(d.nOP-1)]
plane := planeY1SansY2
quant := &d.quant[d.segment]
@@ -332,6 +333,11 @@
d.upMB[mbx].nzMask = uint8(unzMask)
d.nzDCMask = nzDCMask
d.nzACMask = nzACMask
+
+ // Section 15.1 of the spec says that "Steps 2 and 4 [of the loop filter]
+ // are skipped... [if] there is no DCT coefficient coded for the whole
+ // macroblock."
+ return nzDCMask == 0 && nzACMask == 0
}
// reconstructMacroblock applies the predictor functions and adds the inverse-
@@ -384,7 +390,8 @@
}
}
-// reconstruct reconstructs one macroblock.
+// reconstruct reconstructs one macroblock and returns whether inner loop
+// filtering should be skipped for it.
func (d *Decoder) reconstruct(mbx, mby int) (skip bool) {
if d.segmentHeader.updateMap {
if !d.fp.readBit(d.segmentHeader.prob[0]) {
@@ -411,9 +418,7 @@
d.parsePredModeC8()
// Parse the residuals.
if !skip {
- // TODO(nigeltao): make d.parseResiduals return a bool, and change this to
- // skip = d.parseResiduals(mbx, mby)
- d.parseResiduals(mbx, mby)
+ skip = d.parseResiduals(mbx, mby)
} else {
if d.usePredY16 {
d.leftMB.nzY16 = 0
diff --git a/webp/decode.go b/webp/decode.go
index 2ed7f36..453ccbd 100644
--- a/webp/decode.go
+++ b/webp/decode.go
@@ -65,7 +65,7 @@
return m, image.Config{}, nil
}
- r = &io.LimitedReader{r, int64(dataLen)}
+ r = &io.LimitedReader{R: r, N: int64(dataLen)}
if configOnly {
c, err := vp8l.DecodeConfig(r)
return nil, c, err
diff --git a/webp/decode_test.go b/webp/decode_test.go
index 635c5d0..a86e32c 100644
--- a/webp/decode_test.go
+++ b/webp/decode_test.go
@@ -33,6 +33,9 @@
func TestDecodeVP8(t *testing.T) {
testCases := []string{
"blue-purple-pink",
+ "blue-purple-pink-large.no-filter",
+ "blue-purple-pink-large.simple-filter",
+ "blue-purple-pink-large.normal-filter",
"video-001",
"yellow_rose",
}
@@ -40,17 +43,20 @@
for _, tc := range testCases {
f0, err := os.Open("../testdata/" + tc + ".lossy.webp")
if err != nil {
- t.Fatal(err)
+ t.Errorf("%s: Open WEBP: %v", tc, err)
+ continue
}
defer f0.Close()
img0, err := Decode(f0)
if err != nil {
- t.Fatal(err)
+ t.Errorf("%s: Decode WEBP: %v", tc, err)
+ continue
}
m0, ok := img0.(*image.YCbCr)
if !ok || m0.SubsampleRatio != image.YCbCrSubsampleRatio420 {
- t.Fatal("decoded WEBP image is not a 4:2:0 YCbCr")
+ t.Errorf("%s: decoded WEBP image is not a 4:2:0 YCbCr", tc)
+ continue
}
// w2 and h2 are the half-width and half-height, rounded up.
w, h := m0.Bounds().Dx(), m0.Bounds().Dy()
@@ -58,12 +64,14 @@
f1, err := os.Open("../testdata/" + tc + ".lossy.webp.ycbcr.png")
if err != nil {
- t.Fatal(err)
+ t.Errorf("%s: Open PNG: %v", tc, err)
+ continue
}
defer f1.Close()
img1, err := png.Decode(f1)
if err != nil {
- t.Fatal(err)
+ t.Errorf("%s: Open PNG: %v", tc, err)
+ continue
}
// The split-into-YCbCr-planes golden image is a 2*w2 wide and h+h2 high
@@ -73,11 +81,13 @@
// BBRR
// See http://www.fourcc.org/yuv.php#IMC4
if got, want := img1.Bounds(), image.Rect(0, 0, 2*w2, h+h2); got != want {
- t.Fatalf("bounds0: got %v, want %v", got, want)
+ t.Errorf("%s: bounds0: got %v, want %v", tc, got, want)
+ continue
}
m1, ok := img1.(*image.Gray)
if !ok {
- t.Fatal("decoded PNG image is not a Gray")
+ t.Errorf("%s: decoded PNG image is not a Gray", tc)
+ continue
}
planes := []struct {
@@ -101,14 +111,14 @@
}
nDiff++
if nDiff > 10 {
- t.Errorf("%s plane: more rows differ", plane.name)
+ t.Errorf("%s: %s plane: more rows differ", tc, plane.name)
break
}
for i := range got {
diff[i] = got[i] - want[i]
}
- t.Errorf("%s plane: m0 row %d, m1 row %d\ngot %s\nwant%s\ndiff%s",
- plane.name, j, y, hex(got), hex(want), hex(diff))
+ t.Errorf("%s: %s plane: m0 row %d, m1 row %d\ngot %s\nwant%s\ndiff%s",
+ tc, plane.name, j, y, hex(got), hex(want), hex(diff))
}
}
}
@@ -117,6 +127,7 @@
func TestDecodeVP8L(t *testing.T) {
testCases := []string{
"blue-purple-pink",
+ "blue-purple-pink-large",
"gopher-doc.1bpp",
"gopher-doc.2bpp",
"gopher-doc.4bpp",
@@ -197,7 +208,7 @@
}
func benchmarkDecode(b *testing.B, filename string) {
- data, err := ioutil.ReadFile("../testdata/" + filename + ".webp")
+ data, err := ioutil.ReadFile("../testdata/blue-purple-pink-large." + filename + ".webp")
if err != nil {
b.Fatal(err)
}
@@ -213,6 +224,7 @@
}
}
-func BenchmarkDecodeVP8SimpleFilter(b *testing.B) { benchmarkDecode(b, "blue-purple-pink.lossy") }
-func BenchmarkDecodeVP8NormalFilter(b *testing.B) { benchmarkDecode(b, "yellow_rose.lossy") }
-func BenchmarkDecodeVP8L(b *testing.B) { benchmarkDecode(b, "yellow_rose.lossless") }
+func BenchmarkDecodeVP8NoFilter(b *testing.B) { benchmarkDecode(b, "no-filter.lossy") }
+func BenchmarkDecodeVP8SimpleFilter(b *testing.B) { benchmarkDecode(b, "simple-filter.lossy") }
+func BenchmarkDecodeVP8NormalFilter(b *testing.B) { benchmarkDecode(b, "normal-filter.lossy") }
+func BenchmarkDecodeVP8L(b *testing.B) { benchmarkDecode(b, "lossless") }
