Grokbase › Groups › Go › golang-dev › September 2012
FAQ
Reviewers: r,

Message:
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


Description:
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.

Please review this at http://codereview.appspot.com/6497083/

Affected files:
M src/pkg/image/jpeg/reader.go
M src/pkg/image/jpeg/writer.go
M src/pkg/image/jpeg/writer_test.go

Search Discussions

  • Nigel Tao at Sep 5, 2012 at 4:15 am ⇧
    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.
  • David Symonds at Sep 5, 2012 at 4:19 am ⇧

    On Wed, Sep 5, 2012 at 2:15 PM, Nigel Tao 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.
  • Nigel Tao at Sep 5, 2012 at 4:36 am ⇧

    On 5 September 2012 14:19, David Symonds wrote:
    I dunno. Russ looks a lot angrier in the after picture.
    PTAL.
  • Andrew Gerrand at Sep 5, 2012 at 5:13 am ⇧

    On 5 September 2012 14:36, Nigel Tao wrote:
    On 5 September 2012 14:19, David Symonds wrote:
    I dunno. Russ looks a lot angrier in the after picture.
    PTAL.
    In the final picture we see Russ' hidden robot form revealed.
  • Raph at Sep 5, 2012 at 5:06 am ⇧

    On 2012/09/05 04:36:38, nigeltao wrote:
    On 5 September 2012 14:19, David Symonds
    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.

    http://codereview.appspot.com/6497083/
  • David Symonds at Sep 5, 2012 at 5:06 am ⇧

    On Wed, Sep 5, 2012 at 3:03 PM, wrote:

    Also, I noticed some tab characters - not sure what the policy is around those.
    They're fine. It's Go style.
  • R at Sep 5, 2012 at 12:13 pm ⇧
    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#newcode290
    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

    http://codereview.appspot.com/6497083/
  • Nigeltao at Sep 6, 2012 at 1:11 am ⇧
    *** 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


    http://codereview.appspot.com/6497083/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedSep 5, '12 at 4:09a
activeSep 6, '12 at 1:11a
posts9
users5
websitegolang.org

5 users in discussion

Nigeltao: 4 posts David Symonds: 2 posts Raph: 1 post R: 1 post Andrew Gerrand: 1 post

Content

People

Support

Translate

site design / logo © 2022 Grokbase