code review 4435051: image/jpeg: add an encoder.

src/pkg/image/jpeg/writer.go

 // Copyright 2011 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 package jpeg
 
 import (
         "bufio"
         "image"
         "image/ycbcr"
@@ skipping 153 matching lines @@
                         0xc4, 0xc5, 0xc6, 0xc7, 0xc8, 0xc9, 0xca, 0xd2,
                         0xd3, 0xd4, 0xd5, 0xd6, 0xd7, 0xd8, 0xd9, 0xda,
                         0xe2, 0xe3, 0xe4, 0xe5, 0xe6, 0xe7, 0xe8, 0xe9,
                         0xea, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, 0xf8,
                         0xf9, 0xfa,
                 },
         },
 }
 
 // huffmanLUT is a compiled look-up table representation of a huffmanSpec.
-// Each value maps to an uint32 of which the 8 most significant bits hold the

[r, 2011/04/18 19:30:42]

s/an/a/

[nigeltao, 2011/04/19 00:47:06]

Done.

+// Each value maps to a uint32 of which the 8 most significant bits hold the
 // codeword size in bits and the 24 least significant bits hold the codeword.
 // The maximum codeword size is 16 bits.
 type huffmanLUT []uint32
 
 func (h *huffmanLUT) init(s huffmanSpec) {
         maxValue := 0
         for _, v := range s.value {
                 if int(v) > maxValue {
                         maxValue = int(v)
                 }
@@ skipping 18 matching lines @@
         for i, s := range theHuffmanSpec {
                 theHuffmanLUT[i].init(s)
         }
 }
 
 // writer is a buffered writer.
 type writer interface {
         Flush() os.Error
         Write([]byte) (int, os.Error)
         WriteByte(byte) os.Error
-        WriteString(string) (int, os.Error)
-}
-
-// An encoder wraps a writer's methods to save the first returned error as
-// e.err and return void.

[r, 2011/04/18 19:30:42]

void?

[nigeltao, 2011/04/19 00:47:06]

Rewritten.

-
-func (e *encoder) flush() {
-        if e.err != nil {
-                return
-        }
-        e.err = e.w.Flush()
-}
-
-func (e *encoder) write(p []byte) {
-        if e.err != nil {
-                return
-        }
-        _, e.err = e.w.Write(p)
-}
-
-func (e *encoder) writeByte(b byte) {
-        if e.err != nil {
-                return
-        }
-        e.err = e.w.WriteByte(b)
-}
-
-func (e *encoder) writeString(s string) {
-        if e.err != nil {
-                return
-        }
-        _, e.err = e.w.WriteString(s)
 }
 
 // encoder encodes an image to the JPEG format.

[r, 2011/04/18 19:30:42]

move this above its methods and combine the comments

[nigeltao, 2011/04/19 00:47:06]

Done.

 type encoder struct {
-        // w is the writer to write to. err is the first error encountered
-        // during writing.
+        // w is the writer to write to. err is the first error encountered during
+        // writing. All attempted writes after the first error become no-ops.
         w   writer
         err os.Error
+        // buf is a scratch buffer.
+        buf [16]byte
         // bits and nBits are accumulated bits to write to w.
         bits  uint32
         nBits uint8
         // quant is the scaled quantization tables.
         quant [nQuantIndex][blockSize]byte
+}
+
+func (e *encoder) flush() {
+        if e.err != nil {
+                return
+        }
+        e.err = e.w.Flush()
+}
+
+func (e *encoder) write(p []byte) {
+        if e.err != nil {
+                return
+        }
+        _, e.err = e.w.Write(p)
+}
+
+func (e *encoder) writeByte(b byte) {
+        if e.err != nil {
+                return
+        }
+        e.err = e.w.WriteByte(b)
 }
 
 // emit emits the least significant nBits bits of bits to the bitstream.

[r, 2011/04/18 19:30:42]

nBits bits of bits to the bits. wow.

 // The precondition is bits < 1<<nBits && nBits <= 16.
 func (e *encoder) emit(bits uint32, nBits uint8) {
         nBits += e.nBits
         bits <<= 32 - nBits
         bits |= e.bits
         for nBits >= 8 {
                 b := uint8(bits >> 24)
                 e.writeByte(b)
-                if b == 255 {
+                if b == 0xff {
                         e.writeByte(0x00)
                 }
                 bits <<= 8
                 nBits -= 8
         }
         e.bits, e.nBits = bits, nBits
 }
 
 // emitHuff emits the given value with the given Huffman encoder.
 func (e *encoder) emitHuff(h huffIndex, value int) {
@@ skipping 13 matching lines @@
                 nBits = bitCount[a]
         } else {
                 nBits = 8 + bitCount[a>>8]
         }
         e.emitHuff(h, runLength<<4|int(nBits))
         if nBits > 0 {
                 e.emit(uint32(b)&(1<<nBits-1), nBits)
         }
 }
 
-// writeMarkerHeader writes the header for a maker with the given length.

[r, 2011/04/18 19:30:42]

s/maker/marker/

[nigeltao, 2011/04/19 00:47:06]

Done.

+// writeMarkerHeader writes the header for a marker with the given length.
 func (e *encoder) writeMarkerHeader(marker uint8, markerlen int) {
-        e.writeByte(0xff)
-        e.writeByte(marker)
-        e.writeByte(uint8(markerlen >> 8))
-        e.writeByte(uint8(markerlen & 0xff))

[r, 2011/04/18 19:30:42]

this is a fair bit of overhead. we could put a [4]byte into encoder and do a
single Write call.

[nigeltao, 2011/04/19 00:47:06]

Done.

+        e.buf[0] = 0xff
+        e.buf[1] = marker
+        e.buf[2] = uint8(markerlen >> 8)
+        e.buf[3] = uint8(markerlen & 0xff)
+        e.write(e.buf[:4])
 }
 
 // writeDQT writes the Define Quantization Table marker.
 func (e *encoder) writeDQT() {
         markerlen := 2
         for _, q := range e.quant {
                 markerlen += 1 + len(q)
         }
         e.writeMarkerHeader(dqtMarker, markerlen)
         for i, q := range e.quant {
                 e.writeByte(uint8(i))
                 e.write(q[:])
         }
 }
 
 // writeSOF0 writes the Start Of Frame (Baseline) marker.
 func (e *encoder) writeSOF0(size image.Point) {
         markerlen := 8 + 3*nComponent
         e.writeMarkerHeader(sof0Marker, markerlen)
-        e.writeByte(8) // 8-bit color.
-        e.writeByte(uint8(size.Y >> 8))
-        e.writeByte(uint8(size.Y & 0xff))
-        e.writeByte(uint8(size.X >> 8))
-        e.writeByte(uint8(size.X & 0xff))

[r, 2011/04/18 19:30:42]

ditto.

[nigeltao, 2011/04/19 00:47:06]

Done.

-        e.writeByte(nComponent)
+        e.buf[0] = 8 // 8-bit color.
+        e.buf[1] = uint8(size.Y >> 8)
+        e.buf[2] = uint8(size.Y & 0xff)
+        e.buf[3] = uint8(size.X >> 8)
+        e.buf[4] = uint8(size.X & 0xff)
+        e.buf[5] = nComponent
         for i := 0; i < nComponent; i++ {
-                e.writeByte(uint8(i + 1))
+                e.buf[3*i+6] = uint8(i + 1)
                 // We use 4:2:0 chroma subsampling.
-                e.writeByte("\x22\x11\x11"[i])
-                e.writeByte("\x00\x01\x01"[i])

[r, 2011/04/18 19:30:42]

ditto

[nigeltao, 2011/04/19 00:47:06]

Done.

-        }
+                e.buf[3*i+7] = "\x22\x11\x11"[i]
+                e.buf[3*i+8] = "\x00\x01\x01"[i]
+        }
+        e.write(e.buf[:3*(nComponent-1)+9])
 }
 
 // writeDHT writes the Define Huffman Table marker.
 func (e *encoder) writeDHT() {
         markerlen := 2
         for _, s := range theHuffmanSpec {
                 markerlen += 1 + 16 + len(s.value)
         }
         e.writeMarkerHeader(dhtMarker, markerlen)
         for i, s := range theHuffmanSpec {
                 e.writeByte("\x00\x10\x01\x11"[i])
                 e.write(s.count[:])
                 e.write(s.value)

[r, 2011/04/18 19:30:42]

ditto

[nigeltao, 2011/04/19 00:47:06]

I don't think this applies here; s.value is a slice. Besides, the underlying
writer e.w is presumably buffered (since it implements a WriteByte method), so
I'm not sure if adding additional buffering would be a significant win.

         }
 }
 
 // writeBlock writes a block of pixel data using the given quantization table,
 // returning the post-quantized DC value of the DCT-transformed block.
 func (e *encoder) writeBlock(b *block, q quantIndex, prevDC int) int {
         fdct(b)
         // Emit the DC delta.
         dc := div(b[0], (8 * int(e.quant[q][0])))
         e.emitHuffRLE(huffIndex(2*q+0), 0, dc-prevDC)
@@ skipping 43 matching lines @@
                 for y := 0; y < 4; y++ {
                         for x := 0; x < 4; x++ {
                                 j := 16*y + 2*x
                                 sum := src[i][j] + src[i][j+1] + src[i][j+8] + src[i][j+9]
                                 dst[8*y+x+dstOff] = (sum + 2) >> 2
                         }
                 }
         }
 }
 
+// sosHeader is the SOS marker "\xff\xda" followed by 12 bytes:
+//      - the marker length "\x00\x0c",
+//      - the number of components "\x03",
+//      - component 1 uses DC table 0 and AC table 0 "\x01\x00",
+//      - component 2 uses DC table 1 and AC table 1 "\x02\x11",
+//      - component 3 uses DC table 1 and AC table 1 "\x03\x11",
+//      - padding "\x00\x00\x00".
+var sosHeader = []byte{
+        0xff, 0xda, 0x00, 0x0c, 0x03, 0x01, 0x00, 0x02,
+        0x11, 0x03, 0x11, 0x00, 0x00, 0x00,
+}
+
 // writeSOS writes the StartOfScan marker.
 func (e *encoder) writeSOS(m image.Image) {
-        // Write the SOS marker "\xff\xda" followed by 12 bytes:
-        //      - the marker length "\x00\x0c",
-        //      - the number of components "\x03",
-        //      - component 1 uses DC table 0 and AC table 0 "\x01\x00",
-        //      - component 2 uses DC table 1 and AC table 1 "\x02\x11",
-        //      - component 3 uses DC table 1 and AC table 1 "\x03\x11",
-        //      - padding "\x00\x00\x00".
-        e.writeString("\xff\xda\x00\x0c\x03\x01\x00\x02\x11\x03\x11\x00\x00\x00")

[r, 2011/04/18 19:30:42]

depending on the implementation of the writer, writeString may copy.  better
than this approach is to have a global
var sosHeader = []byte("....")

also i'm not sure you're getting much readability from turning these bytes into
strings.

[nigeltao, 2011/04/19 00:47:06]

Done.

+        e.write(sosHeader)
         var (
                 // Scratch buffers to hold the YCbCr values.
                 yBlock  block
                 cbBlock [4]block
                 crBlock [4]block
                 cBlock  block

[r, 2011/04/18 19:30:42]

ok for now, but these are more unnecessary allocations.
probably not important, because we only do one SOS per image (right?)

[nigeltao, 2011/04/19 00:47:06]

There is only one SOS per image. Also, eventually, escape analysis should be
able to discover that these blocks are stack-allocatable.

                 // DC components are delta-encoded.
-                lastY, lastCb, lastCr int
+                prevDCY, prevDCCb, prevDCCr int
         )
         bounds := m.Bounds()
         for y := bounds.Min.Y; y < bounds.Max.Y; y += 16 {
                 for x := bounds.Min.X; x < bounds.Max.X; x += 16 {
                         for i := 0; i < 4; i++ {
                                 xOff := (i & 1) * 8
                                 yOff := (i & 2) * 4
                                 p := image.Point{x + xOff, y + yOff}
                                 toYCbCr(m, p, &yBlock, &cbBlock[i], &crBlock[i])
-                                lastY = e.writeBlock(&yBlock, 0, lastY)
+                                prevDCY = e.writeBlock(&yBlock, 0, prevDCY)
                         }
                         scale(&cBlock, &cbBlock)
-                        lastCb = e.writeBlock(&cBlock, 1, lastCb)
+                        prevDCCb = e.writeBlock(&cBlock, 1, prevDCCb)
                         scale(&cBlock, &crBlock)
-                        lastCr = e.writeBlock(&cBlock, 1, lastCr)
+                        prevDCCr = e.writeBlock(&cBlock, 1, prevDCCr)
                 }
         }
         // Pad the last byte with 1's.
         e.emit(0x7f, 7)
 }
 
 // DefaultQuality is the default quality encoding parameter.
 const DefaultQuality = 75
 
 // Options are the encoding parameters.
@@ skipping 39 matching lines @@
                         x = (x*scale + 50) / 100
                         if x < 1 {
                                 x = 1
                         } else if x > 255 {
                                 x = 255
                         }
                         e.quant[i][j] = uint8(x)
                 }
         }
         // Write the Start Of Image marker.
-        e.writeString("\xff\xd8")
+        e.buf[0] = 0xff
+        e.buf[1] = 0xd8
+        e.write(e.buf[:2])
         // Write the quantization tables.
         e.writeDQT()
         // Write the image dimensions.
         e.writeSOF0(b.Size())
         // Write the Huffman tables.
         e.writeDHT()
         // Write the image data.
         e.writeSOS(m)
         // Write the End Of Image marker.
-        e.writeString("\xff\xd9")
+        e.buf[0] = 0xff
+        e.buf[1] = 0xd9
+        e.write(e.buf[:2])
         e.flush()
         return e.err
 }