code review 91700044: go.image/vp8: implement the loop filter.

go.image/vp8: implement the loop filter.

The testdata files were created by:
cwebp video-001.png -o video-001.webp
dwebp -pgm video-001.webp -o tmp.pgm
convert tmp.pgm video-001.webp.ycbcr.png
rm tmp.pgm

[nigeltao, 2014-05-26 06:55:19]

Hello r@golang.org (cc: chaishushan@gmail.com,
golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.image

[r, 2014-05-26 21:42:23]

LGTM for quality. i didn't compare to the spec - do you want me to do that?

https://codereview.appspot.com/91700044/diff/40001/vp8/decode.go
File vp8/decode.go (right):

https://codereview.appspot.com/91700044/diff/40001/vp8/decode.go#newcode139
vp8/decode.go:139: usePredY16 bool // The C code calls this !is_i4x4_.
what C code? given the trailing underscore i suspect it's C++ anyway

https://codereview.appspot.com/91700044/diff/40001/vp8/filter.go
File vp8/filter.go (right):

https://codereview.appspot.com/91700044/diff/40001/vp8/filter.go#newcode81
vp8/filter.go:81: // The C code has a "TODO: only CURRENT is handled for now."
which C code? see other comment

https://codereview.appspot.com/91700044/diff/40001/webp/decode_test.go
File webp/decode_test.go (right):

https://codereview.appspot.com/91700044/diff/40001/webp/decode_test.go#newcode27
webp/decode_test.go:27: fmt.Fprintf(buf, " . % x", y)
this seems clearer to me. it might even be correct.

n := len(x)
if n > 16 {
  n = 16
}
fmt.Fprintf(buf, " . % x", x[:n])
x = x[n:]

you choose

https://codereview.appspot.com/91700044/diff/40001/webp/decode_test.go#newcode46
webp/decode_test.go:46: }
should you check w and h?

[nigeltao, 2014-05-27 00:47:53]

LGTM for quality is all I want.

https://codereview.appspot.com/91700044/diff/40001/vp8/decode.go
File vp8/decode.go (right):

https://codereview.appspot.com/91700044/diff/40001/vp8/decode.go#newcode139
vp8/decode.go:139: usePredY16 bool // The C code calls this !is_i4x4_.
On 2014/05/26 21:42:22, r wrote:
> what C code? given the trailing underscore i suspect it's C++ anyway

Done.

https://codereview.appspot.com/91700044/diff/40001/vp8/filter.go
File vp8/filter.go (right):

https://codereview.appspot.com/91700044/diff/40001/vp8/filter.go#newcode81
vp8/filter.go:81: // The C code has a "TODO: only CURRENT is handled for now."
On 2014/05/26 21:42:22, r wrote:
> which C code? see other comment

Done.

https://codereview.appspot.com/91700044/diff/40001/webp/decode_test.go
File webp/decode_test.go (right):

https://codereview.appspot.com/91700044/diff/40001/webp/decode_test.go#newcode27
webp/decode_test.go:27: fmt.Fprintf(buf, " . % x", y)
On 2014/05/26 21:42:23, r wrote:
> this seems clearer to me. it might even be correct.
> 
> n := len(x)
> if n > 16 {
>   n = 16
> }
> fmt.Fprintf(buf, " . % x", x[:n])
> x = x[n:]
> 
> you choose

Done.

https://codereview.appspot.com/91700044/diff/40001/webp/decode_test.go#newcode46
webp/decode_test.go:46: }
On 2014/05/26 21:42:22, r wrote:
> should you check w and h?

img0 is not wxh, it's (2*w2)x(h+h2), which is the next thing I check (after the
comment). img1 is wxh, and I check for that too.

[nigeltao, 2014-05-27 00:53:30]

*** Submitted as
https://code.google.com/p/go/source/detail?r=1bd33de47ec4&repo=image ***

go.image/vp8: implement the loop filter.

The testdata files were created by:
cwebp video-001.png -o video-001.webp
dwebp -pgm video-001.webp -o tmp.pgm
convert tmp.pgm video-001.webp.ycbcr.png
rm tmp.pgm

LGTM=r
R=r
CC=chaishushan, golang-codereviews
https://codereview.appspot.com/91700044