faster decoding (3%-6%)

. revamped the boolean decoder to use less shifts
. added some description and ASCII art as explanations too.
. clarified the types further (bit_t, lbit_t, range_t, etc.)
. changed the negative field 'missing_' into positive 'bits_'

Some stats, decoding some randomly encoded WebP files:

with USE_RIGHT_JUSTIFY:
  BITS=32 => 133 files, 50 loops => 7.3s (1.097 ms/file/iterations)
  BITS=24 => 133 files, 50 loops => 7.3s (1.097 ms/file/iterations)
  BITS=16 => 133 files, 50 loops => 7.4s (1.120 ms/file/iterations)
  BITS=8 => 133 files, 50 loops => 7.5s (1.128 ms/file/iterations)

without USE_RIGHT_JUSTIFY:
  BITS=32 => 133 files, 50 loops => 7.5s (1.131 ms/file/iterations)
  BITS=24 => 133 files, 50 loops => 7.6s (1.142 ms/file/iterations)
  BITS=16 => 133 files, 50 loops => 7.6s (1.143 ms/file/iterations)
  BITS=8 => 133 files, 50 loops => 7.6s (1.149 ms/file/iterations)

Change-Id: I9277fb051676c05582e9c7ea3cb5a4b2a3ffb12e

src/utils/bit_reader.c

@@ -15,7 +15,11 @@
 extern "C" {
 #endif
 
-#define MK(X) (((bit_t)(X) << (BITS)) | (MASK))
+#ifndef USE_RIGHT_JUSTIFY
+#define MK(X) (((range_t)(X) << (BITS)) | (MASK))
+#else
+#define MK(X) ((range_t)(X))
+#endif
 
 //------------------------------------------------------------------------------
 // VP8BitReader
@@ -29,7 +33,7 @@ void VP8InitBitReader(VP8BitReader* const br,
   br->buf_     = start;
   br->buf_end_ = end;
   br->value_   = 0;
-  br->missing_ = 8;   // to load the very first 8bits
+  br->bits_    = -8;   // to load the very first 8bits
   br->eof_     = 0;
 }
 
@@ -46,7 +50,7 @@ const uint8_t kVP8Log2Range[128] = {
 };
 
 // range = (range << kVP8Log2Range[range]) + trailing 1's
-const bit_t kVP8NewRange[128] = {
+const range_t kVP8NewRange[128] = {
   MK(127), MK(127), MK(191), MK(127), MK(159), MK(191), MK(223), MK(127),
   MK(143), MK(159), MK(175), MK(191), MK(207), MK(223), MK(239), MK(127),
   MK(135), MK(143), MK(151), MK(159), MK(167), MK(175), MK(183), MK(191),
@@ -71,9 +75,19 @@ void VP8LoadFinalBytes(VP8BitReader* const br) {
   assert(br != NULL && br->buf_ != NULL);
   // Only read 8bits at a time
   if (br->buf_ < br->buf_end_) {
-    br->value_ |= (bit_t)(*br->buf_++) << ((BITS) - 8 + br->missing_);
+#ifndef USE_RIGHT_JUSTIFY
-    br->missing_ -= 8;
+    br->value_ |= (bit_t)(*br->buf_++) << ((BITS) - 8 - br->bits_);
-  } else {
+#else
+    br->value_ = (bit_t)(*br->buf_++) | (br->value_ << 8);
+#endif
+    br->bits_ += 8;
+  } else if (!br->eof_) {
+#ifdef USE_RIGHT_JUSTIFY
+    // These are not strictly needed, but it makes the behaviour
+    // consistent for both USE_RIGHT_JUSTIFY and !USE_RIGHT_JUSTIFY.
+    br->value_ <<= 8;
+    br->bits_ += 8;
+#endif
     br->eof_ = 1;
   }
 }

src/utils/bit_reader.h

@@ -24,6 +24,42 @@
 extern "C" {
 #endif
 
+// The Boolean decoder needs to maintain infinite precision on the value_ field.
+// However, since range_ is only 8bit, we only need an active window of 8 bits
+// for value_. Left bits (MSB) gets zeroed and shifted away when value_ falls
+// below 128, range_ is updated, and fresh bits read from the bitstream are
+// brought in as LSB.
+// To avoid reading the fresh bits one by one (slow), we cache a few of them
+// ahead (actually, we cache BITS of them ahead. See below). There's two
+// strategies regarding how to shift these looked-ahead fresh bits into the
+// 8bit window of value_: either we shift them in, while keeping the position of
+// the window fixed. Or we slide the window to the right while keeping the cache
+// bits at a fixed, right-justified, position.
+//
+//  Example, for BITS=16: here is the content of value_ for both strategies:
+//
+//          !USE_RIGHT_JUSTIFY            ||        USE_RIGHT_JUSTIFY
+//                                        ||
+//   <- 8b -><- 8b -><- BITS bits  ->     ||  <- 8b+3b -><- 8b -><- 13 bits ->
+//   [unused][value_][cached bits][0]     ||  [unused...][value_][cached bits]
+//  [........00vvvvvvBBBBBBBBBBBBB000]LSB || [...........00vvvvvvBBBBBBBBBBBBB]
+//                                        ||
+// After calling VP8Shift(), where we need to shift away two zeros:
+//  [........vvvvvvvvBBBBBBBBBBB00000]LSB || [.............vvvvvvvvBBBBBBBBBBB]
+//                                        ||
+// Just before we need to call VP8LoadNewBytes(), the situation is:
+//  [........vvvvvv000000000000000000]LSB || [..........................vvvvvv]
+//                                        ||
+// And just after calling VP8LoadNewBytes():
+//  [........vvvvvvvvBBBBBBBBBBBBBBBB]LSB || [........vvvvvvvvBBBBBBBBBBBBBBBB]
+//
+// -> we're back to height active 'value_' bits (marked 'v') and BITS cached
+// bits (marked 'B')
+//
+// The right-justify strategy tends to use less shifts, so let's use it:
+
+#define USE_RIGHT_JUSTIFY
+
 //------------------------------------------------------------------------------
 // BITS can be either 32, 24, 16 or 8.
 // Pick values that fit natural register size.
@@ -39,7 +75,6 @@ extern "C" {
 //------------------------------------------------------------------------------
 // Derived types and constants
 
-#define MASK ((((bit_t)1) << (BITS)) - 1)
 #if (BITS == 32)
 typedef uint64_t bit_t;   // natural register type
 typedef uint32_t lbit_t;  // natural type for memory I/O
@@ -54,6 +89,13 @@ typedef uint32_t bit_t;
 typedef uint8_t lbit_t;
 #endif
 
+#ifndef USE_RIGHT_JUSTIFY
+typedef bit_t range_t;     // type for storing range_
+#define MASK ((((bit_t)1) << (BITS)) - 1)
+#else
+typedef uint32_t range_t;  // range_ only uses 8bits here. No need for bit_t.
+#endif
+
 //------------------------------------------------------------------------------
 // Bitreader
 
@@ -64,9 +106,9 @@ struct VP8BitReader {
   int eof_;                   // true if input is exhausted
 
   // boolean decoder
-  bit_t range_;            // current range minus 1. In [127, 254] interval.
+  range_t range_;            // current range minus 1. In [127, 254] interval.
   bit_t value_;              // current value
-  int missing_;            // number of missing bits in value_ (8bit)
+  int bits_;                 // number of valid bits left
 };
 
 // Initialize the bit reader and the boolean decoder.
@@ -84,12 +126,12 @@ int32_t VP8GetSignedValue(VP8BitReader* const br, int num_bits);
 
 // Read a bit with proba 'prob'. Speed-critical function!
 extern const uint8_t kVP8Log2Range[128];
-extern const bit_t kVP8NewRange[128];
+extern const range_t kVP8NewRange[128];
 
 void VP8LoadFinalBytes(VP8BitReader* const br);    // special case for the tail
 
 static WEBP_INLINE void VP8LoadNewBytes(VP8BitReader* const br) {
-  assert(br && br->buf_);
+  assert(br != NULL && br->buf_ != NULL);
   // Read 'BITS' bits at a time if possible.
   if (br->buf_ + sizeof(lbit_t) <= br->buf_end_) {
     // convert memory type to register type (with some zero'ing!)
@@ -119,47 +161,81 @@ static WEBP_INLINE void VP8LoadNewBytes(VP8BitReader* const br) {
 #else    // BIG_ENDIAN
     bits = (bit_t)in_bits;
 #endif
-    br->value_ |= bits << br->missing_;
+#ifndef USE_RIGHT_JUSTIFY
-    br->missing_ -= (BITS);
+    br->value_ |= bits << (-br->bits_);
+#else
+    br->value_ = bits | (br->value_ << (BITS));
+#endif
+    br->bits_ += (BITS);
   } else {
     VP8LoadFinalBytes(br);    // no need to be inlined
   }
 }
 
-static WEBP_INLINE int VP8BitUpdate(VP8BitReader* const br, bit_t split) {
+static WEBP_INLINE int VP8BitUpdate(VP8BitReader* const br, range_t split) {
-  const bit_t value_split = split | (MASK);
+  if (br->bits_ < 0) {  // Make sure we have a least BITS bits in 'value_'
-  if (br->missing_ > 0) {  // Make sure we have a least BITS bits in 'value_'
     VP8LoadNewBytes(br);
   }
-  if (br->value_ > value_split) {
+#ifndef USE_RIGHT_JUSTIFY
-    br->range_ -= value_split + 1;
+  split |= (MASK);
-    br->value_ -= value_split + 1;
+  if (br->value_ > split) {
+    br->range_ -= split + 1;
+    br->value_ -= split + 1;
     return 1;
   } else {
-    br->range_ = value_split;
+    br->range_ = split;
+    return 0;
+  }
+#else
+  {
+    const int pos = br->bits_;
+    const range_t value = (range_t)(br->value_ >> pos);
+    if (value > split) {
+      br->range_ -= split + 1;
+      br->value_ -= (bit_t)(split + 1) << pos;
+      return 1;
+    } else {
+      br->range_ = split;
       return 0;
     }
   }
+#endif
+}
 
 static WEBP_INLINE void VP8Shift(VP8BitReader* const br) {
+#ifndef USE_RIGHT_JUSTIFY
   // range_ is in [0..127] interval here.
   const bit_t idx = br->range_ >> (BITS);
   const int shift = kVP8Log2Range[idx];
   br->range_ = kVP8NewRange[idx];
   br->value_ <<= shift;
-  br->missing_ += shift;
+  br->bits_ -= shift;
+#else
+  const int shift = kVP8Log2Range[br->range_];
+  assert(br->range_ < (range_t)128);
+  br->range_ = kVP8NewRange[br->range_];
+  br->bits_ -= shift;
+#endif
 }
-
 static WEBP_INLINE int VP8GetBit(VP8BitReader* const br, int prob) {
+#ifndef USE_RIGHT_JUSTIFY
   // It's important to avoid generating a 64bit x 64bit multiply here.
   // We just need an 8b x 8b after all.
-  const bit_t split =
+  const range_t split =
-      (bit_t)((uint32_t)(br->range_ >> (BITS)) * prob) << ((BITS) - 8);
+      (range_t)((uint32_t)(br->range_ >> (BITS)) * prob) << ((BITS) - 8);
   const int bit = VP8BitUpdate(br, split);
-  if (br->range_ <= (((bit_t)0x7e << (BITS)) | (MASK))) {
+  if (br->range_ <= (((range_t)0x7e << (BITS)) | (MASK))) {
     VP8Shift(br);
   }
   return bit;
+#else
+  const range_t split = (br->range_ * prob) >> 8;
+  const int bit = VP8BitUpdate(br, split);
+  if (br->range_ <= (range_t)0x7e) {
+    VP8Shift(br);
+  }
+  return bit;
+#endif
 }
 
 static WEBP_INLINE int VP8GetSigned(VP8BitReader* const br, int v) {