vp10: skip unreachable cat6 token extrabits.

We have historically added new bits to cat6 whenever we added a new
transform size (or bitdepth, for that matter). However, we have
always coded these new bits regardless of the actual transform size,
which means that for smaller transforms, we code bits that cannot
possibly be set. The coding (quality) impact of this is negligible,
but the bigger issue is that this allows creating bitstreams with
coefficient values that are nonsensible and can cause int overflows,
which then de facto become part of the bitstream spec. By not coding
these bits, we remove this possibility.

See issue 1065.

Change-Id: Ib3186eca2df6a7a15ddc60c8b55af182aadd964d

diff --git a/vp10/decoder/detokenize.c b/vp10/decoder/detokenize.c
index 2902ece7c2fd045f82e867ef45171f94571f2ca1..d39e3dc06c22bdafb94437b14e5ba08fb1318218 100644 (file)
--- a/vp10/decoder/detokenize.c
+++ b/vp10/decoder/detokenize.c
@@ -163,26 +163,33 @@ static int decode_coefs(const MACROBLOCKD *xd,
         case CATEGORY5_TOKEN:
           val = CAT5_MIN_VAL + read_coeff(cat5_prob, 5, r);
           break;
-        case CATEGORY6_TOKEN:
+        case CATEGORY6_TOKEN: {
+#if CONFIG_MISC_FIXES
+          const int skip_bits = TX_SIZES - 1 - tx_size;
+#else
+          const int skip_bits = 0;
+#endif
+          const uint8_t *cat6p = cat6_prob + skip_bits;
 #if CONFIG_VP9_HIGHBITDEPTH
           switch (xd->bd) {
             case VPX_BITS_8:
-              val = CAT6_MIN_VAL + read_coeff(cat6_prob, 14, r);
+              val = CAT6_MIN_VAL + read_coeff(cat6p, 14 - skip_bits, r);
               break;
             case VPX_BITS_10:
-              val = CAT6_MIN_VAL + read_coeff(cat6_prob, 16, r);
+              val = CAT6_MIN_VAL + read_coeff(cat6p, 16 - skip_bits, r);
               break;
             case VPX_BITS_12:
-              val = CAT6_MIN_VAL + read_coeff(cat6_prob, 18, r);
+              val = CAT6_MIN_VAL + read_coeff(cat6p, 18 - skip_bits, r);
               break;
             default:
               assert(0);
               return -1;
           }
 #else
-          val = CAT6_MIN_VAL + read_coeff(cat6_prob, 14, r);
+          val = CAT6_MIN_VAL + read_coeff(cat6p, 14 - skip_bits, r);
 #endif
           break;
+        }
       }
     }
     v = (val * dqv) >> dq_shift;

diff --git a/vp10/encoder/bitstream.c b/vp10/encoder/bitstream.c
index 940ae8832ee86ca50d66949d712d67f5a82d9d61..1661fbd5d897556d880942effec4a7dfd3cdbf6a 100644 (file)
--- a/vp10/encoder/bitstream.c
+++ b/vp10/encoder/bitstream.c
@@ -122,8 +122,11 @@ static void update_switchable_interp_probs(VP10_COMMON *cm, vpx_writer *w,
 
 static void pack_mb_tokens(vpx_writer *w,
                            TOKENEXTRA **tp, const TOKENEXTRA *const stop,
-                           vpx_bit_depth_t bit_depth) {
+                           vpx_bit_depth_t bit_depth, const TX_SIZE tx) {
   TOKENEXTRA *p = *tp;
+#if !CONFIG_MISC_FIXES
+  (void) tx;
+#endif
 
   while (p < stop && p->token != EOSB_TOKEN) {
     const int t = p->token;
@@ -171,6 +174,12 @@ static void pack_mb_tokens(vpx_writer *w,
 
     if (b->base_val) {
       const int e = p->extra, l = b->len;
+#if CONFIG_MISC_FIXES
+      int skip_bits =
+          (b->base_val == CAT6_MIN_VAL) ? TX_SIZES - 1 - tx : 0;
+#else
+      int skip_bits = 0;
+#endif
 
       if (l) {
         const unsigned char *pb = b->prob;
@@ -180,7 +189,12 @@ static void pack_mb_tokens(vpx_writer *w,
 
         do {
           const int bb = (v >> --n) & 1;
-          vpx_write(w, bb, pb[i >> 1]);
+          if (skip_bits) {
+            skip_bits--;
+            assert(!bb);
+          } else {
+            vpx_write(w, bb, pb[i >> 1]);
+          }
           i = b->tree[i + bb];
         } while (n);
       }
@@ -190,7 +204,7 @@ static void pack_mb_tokens(vpx_writer *w,
     ++p;
   }
 
-  *tp = p + (p->token == EOSB_TOKEN);
+  *tp = p;
 }
 
 static void write_segment_id(vpx_writer *w, const struct segmentation *seg,
@@ -382,6 +396,7 @@ static void write_modes_b(VP10_COMP *cpi, const TileInfo *const tile,
   const VP10_COMMON *const cm = &cpi->common;
   MACROBLOCKD *const xd = &cpi->td.mb.e_mbd;
   MODE_INFO *m;
+  int plane;
 
   xd->mi = cm->mi_grid_visible + (mi_row * cm->mi_stride + mi_col);
   m = xd->mi[0];
@@ -398,8 +413,16 @@ static void write_modes_b(VP10_COMP *cpi, const TileInfo *const tile,
     pack_inter_mode_mvs(cpi, m, w);
   }
 
-  assert(*tok < tok_end);
-  pack_mb_tokens(w, tok, tok_end, cm->bit_depth);
+  if (!m->mbmi.skip) {
+    assert(*tok < tok_end);
+    for (plane = 0; plane < MAX_MB_PLANE; ++plane) {
+      TX_SIZE tx = plane ? get_uv_tx_size(&m->mbmi, &xd->plane[plane])
+                         : m->mbmi.tx_size;
+      pack_mb_tokens(w, tok, tok_end, cm->bit_depth, tx);
+      assert(*tok < tok_end && (*tok)->token == EOSB_TOKEN);
+      (*tok)++;
+    }
+  }
 }
 
 static void write_partition(const VP10_COMMON *const cm,

diff --git a/vp10/encoder/encodeframe.c b/vp10/encoder/encodeframe.c
index 019e5b1e9bd9959dc1296e009900c3d1c9869b51..ce1530c635f550759fd66fa96575887b19e067c4 100644 (file)
--- a/vp10/encoder/encodeframe.c
+++ b/vp10/encoder/encodeframe.c
@@ -1356,9 +1356,6 @@ static void encode_b(VP10_COMP *cpi, const TileInfo *const tile,
 
   if (output_enabled) {
     update_stats(&cpi->common, td);
-
-    (*tp)->token = EOSB_TOKEN;
-    (*tp)++;
   }
 }
 

diff --git a/vp10/encoder/tokenize.c b/vp10/encoder/tokenize.c
index af915feeda49453c683fc53793f6339de3fd4e4d..cbebd5aa84c19110d756051541346d0ef7377fa5 100644 (file)
--- a/vp10/encoder/tokenize.c
+++ b/vp10/encoder/tokenize.c
@@ -628,8 +628,16 @@ void vp10_tokenize_sb(VP10_COMP *cpi, ThreadData *td, TOKENEXTRA **t,
   }
 
   if (!dry_run) {
+    int plane;
+
     td->counts->skip[ctx][0] += skip_inc;
-    vp10_foreach_transformed_block(xd, bsize, tokenize_b, &arg);
+
+    for (plane = 0; plane < MAX_MB_PLANE; ++plane) {
+      vp10_foreach_transformed_block_in_plane(xd, bsize, plane, tokenize_b,
+                                              &arg);
+      (*t)->token = EOSB_TOKEN;
+      (*t)++;
+    }
   } else {
     vp10_foreach_transformed_block(xd, bsize, set_entropy_context_b, &arg);
   }