code review 6497083: image/jpeg: fix quantization tables to be in zig-zag or...

image/jpeg: fix quantization tables to be in zig-zag order, not natural
order.

JPEG images generated prior to this CL are still valid JPEGs, as the
quantization tables used are encoded in the wire format. Such JPEGs just
don't use the recommended quantization tables.

[nigeltao, 2012-09-05 04:09:19]

Hello r@golang.org (cc: golang-dev@googlegroups.com, raph@google.com,
tuom.larsen@gmail.com),

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

[nigeltao, 2012-09-05 04:15:22]

Attached is the before/after JPEG encoding of
$GOROOT/src/pkg/image/testdata/video-001.png at quality 5 out of 100
(i.e. extreme quantization). Before (left) is 1255 bytes, after
(right) is 1286 bytes. In my opinion, the after looks better.

At more typical quality levels (75+), the difference isn't really
noticable unless you're looking for it.

[dsymonds, 2012-09-05 04:19:16]

On Wed, Sep 5, 2012 at 2:15 PM, Nigel Tao <nigeltao@golang.org> wrote:

> Before (left) is 1255 bytes, after (right) is 1286 bytes. In my opinion, the
after looks better.

I dunno. Russ looks a lot angrier in the after picture.

[nigeltao, 2012-09-05 04:36:38]

On 5 September 2012 14:19, David Symonds <dsymonds@golang.org> wrote:
> I dunno. Russ looks a lot angrier in the after picture.

PTAL.

[raph, 2012-09-05 05:03:06]

On 2012/09/05 04:36:38, nigeltao wrote:
> On 5 September 2012 14:19, David Symonds <mailto:dsymonds@golang.org> wrote:
> > I dunno. Russ looks a lot angrier in the after picture.
> 
> PTAL.

LGTM.  Note though that this puts an increased burden on future changes to allow
customized quantization tables, to make sure those are transformed into zig-zag
order.

Also, I noticed some tab characters - not sure what the policy is around those.

[dsymonds, 2012-09-05 05:06:14]

On Wed, Sep 5, 2012 at 3:03 PM,  <raph@google.com> wrote:

> Also, I noticed some tab characters - not sure what the policy is around
those.

They're fine. It's Go style.

[adg, 2012-09-05 05:13:06]

On 5 September 2012 14:36, Nigel Tao <nigeltao@golang.org> wrote:
> On 5 September 2012 14:19, David Symonds <dsymonds@golang.org> wrote:
>> I dunno. Russ looks a lot angrier in the after picture.
>
> PTAL.

In the final picture we see Russ' hidden robot form revealed.

[r, 2012-09-05 12:13:20]

LGTM but there are a couple of cases where i'd rename k.

http://codereview.appspot.com/6497083/diff/9002/src/pkg/image/jpeg/reader.go
File src/pkg/image/jpeg/reader.go (right):

http://codereview.appspot.com/6497083/diff/9002/src/pkg/image/jpeg/reader.go#...
src/pkg/image/jpeg/reader.go:290: for k := 1; k < blockSize; k++ { // k is in
zig-zag order.
you could s/k/zig/ so the loop body makes this clear

[nigeltao, 2012-09-06 01:10:59]

*** Submitted as http://code.google.com/p/go/source/detail?r=292816148e44 ***

image/jpeg: fix quantization tables to be in zig-zag order, not natural
order.

JPEG images generated prior to this CL are still valid JPEGs, as the
quantization tables used are encoded in the wire format. Such JPEGs just
don't use the recommended quantization tables.

R=r, dsymonds, raph, adg
CC=golang-dev, tuom.larsen
http://codereview.appspot.com/6497083