Skip to content

Commit

Permalink
ccitt: style fixes
Browse files Browse the repository at this point in the history
This commit applies some style fixes raised in the
https://go-review.googlesource.com/c/image/+/194417 "ccitt: set
bitReader default order to MSB" review.

Having nextBit return uint64, not uint32, removes some type conversions.

Removed a redundant "& 1" computation.

Updated a comment with s/LSB/MSB/, missed in the previous commit.

Restored the semantics of the "nBits" variable. Throughout the package,
there are a number of times where we use paired local variables or
paired struct fields, named "bits" and "nBits". The semantics is that
"bits"' type is uint32 or uint64 that actually holds a variable number
of bits, and "nBits" is that variable number. The previous commit,
writing MSB-first instead of LSB-first, changed the semantics (without
changing the variable name) so that one of those "nBits" variables now
held the index of the next bit, not the number of bits. This commit
restores the semantics: the shift is (7 - nBits), not just nBits.

Change-Id: I9afada9becceeb5642ce2c60eaf7bc0ede0cdd12
Reviewed-on: https://go-review.googlesource.com/c/image/+/199957
Reviewed-by: Horst Rutter <hhrutter@gmail.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Run-TryBot: Benny Siegert <bsiegert@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
  • Loading branch information
nigeltao committed Oct 9, 2019
1 parent 69e4b85 commit e7c1f5e
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 11 deletions.
8 changes: 4 additions & 4 deletions ccitt/reader.go
Expand Up @@ -158,13 +158,13 @@ func (b *bitReader) alignToByteBoundary() {
// bitReader.nBits value above nextBitMaxNBits.
const nextBitMaxNBits = 31

func (b *bitReader) nextBit() (uint32, error) {
func (b *bitReader) nextBit() (uint64, error) {
for {
if b.nBits > 0 {
bit := (b.bits >> 63) & 1
bit := b.bits >> 63
b.bits <<= 1
b.nBits--
return uint32(bit), nil
return bit, nil
}

if available := b.bw - b.br; available >= 4 {
Expand Down Expand Up @@ -207,7 +207,7 @@ func decode(b *bitReader, decodeTable [][2]int16) (uint32, error) {
if err != nil {
return 0, err
}
bitsRead |= uint64(bit) << (63 - nBitsRead)
bitsRead |= bit << (63 - nBitsRead)
nBitsRead++
// The "&1" is redundant, but can eliminate a bounds check.
state = int32(decodeTable[state][bit&1])
Expand Down
14 changes: 7 additions & 7 deletions ccitt/reader_test.go
Expand Up @@ -161,27 +161,27 @@ func testDecodeTable(t *testing.T, decodeTable [][2]int16, codes []code, values
m[code.val] = code.str
}

// Build the encoded form of those values in LSB order.
// Build the encoded form of those values in MSB order.
enc := []byte(nil)
bits := uint8(0)
nBits := uint8(7)
nBits := uint32(0)
for _, v := range values {
code := m[v]
if code == "" {
panic("unmapped code")
}
for _, c := range code {
bits |= uint8(c&1) << nBits
if nBits == 0 {
bits |= uint8(c&1) << (7 - nBits)
nBits++
if nBits == 8 {
enc = append(enc, bits)
bits = 0
nBits = 7
nBits = 0
continue
}
nBits--
}
}
if nBits < 7 {
if nBits > 0 {
enc = append(enc, bits)
}

Expand Down

0 comments on commit e7c1f5e

Please sign in to comment.