Improve LEB128 support and test coverage.

Also some minor cosmetic changes and style fixes.

BUG=aomedia:1125

Change-Id: Ide24057736376accd2f9907ef87b5a725ab3f3c2

aom/aom_integer.h

@@ -81,8 +81,9 @@ extern "C" {
 // Returns size of uint32_t when encoded using LEB128.
 size_t aom_uleb_size_in_bytes(uint64_t value);
 
-// Returns decoded LEB128 value.
-void aom_uleb_decode(const uint8_t *buffer, size_t available, uint64_t *value);
+// Returns -1 upon decode failure, or 0 and decoded LEB128 value via last
+// argument when decode is successful.
+int aom_uleb_decode(const uint8_t *buffer, size_t available, uint64_t *value);
 
 // Encodes LEB128 integer. Returns 0 when successful, and -1 upon failure.
 int aom_uleb_encode(uint64_t value, size_t available, uint8_t *coded_value,

aom/src/aom_integer.c

@@ -23,14 +23,21 @@ size_t aom_uleb_size_in_bytes(uint64_t value) {
   return size;
 }
 
-void aom_uleb_decode(const uint8_t *buffer, size_t available, uint64_t *value) {
+int aom_uleb_decode(const uint8_t *buffer, size_t available, uint64_t *value) {
+  int status = -1;
+
   if (buffer && value) {
     for (size_t i = 0; i < kMaximumLeb128Size && i < available; ++i) {
       const uint8_t decoded_byte = *(buffer + i) & kLeb128ByteMask;
       *value |= decoded_byte << (i * 7);
-      if ((*(buffer + i) >> 7) == 0) break;
+      if ((*(buffer + i) >> 7) == 0) {
+        status = 0;
+        break;
+      }
     }
   }
+
+  return status;
 }
 
 int aom_uleb_encode(uint64_t value, size_t available, uint8_t *coded_value,

av1/decoder/obu.c

@@ -282,7 +282,11 @@ void av1_decode_frame_from_obus(struct AV1Decoder *pbi, const uint8_t *data,
 #if CONFIG_OBU_SIZING
     // OBUs are preceded by an unsigned leb128 coded unsigned integer.
     uint64_t u_obu_size = 0;
-    aom_uleb_decode(data, bytes_available, &u_obu_size);
+    if (aom_uleb_decode(data, bytes_available, &u_obu_size) != 0) {
+      cm->error.error_code = AOM_CODEC_CORRUPT_FRAME;
+      return;
+    }
+
     const size_t obu_size = (size_t)u_obu_size;
     const size_t length_field_size = aom_uleb_size_in_bytes(u_obu_size);
 #else

av1/encoder/bitstream.c

@@ -4574,7 +4574,7 @@ uint32_t write_obu_header(OBU_TYPE obu_type, int obu_extension,
 int write_uleb_obu_size(uint32_t obu_size, uint8_t *dest) {
   size_t coded_obu_size = 0;
 
-  if (aom_uleb_encode(obu_size, sizeof(uint32_t), dest, &coded_obu_size) != 0)
+  if (aom_uleb_encode(obu_size, sizeof(obu_size), dest, &coded_obu_size) != 0)
     return AOM_CODEC_ERROR;
 
   return AOM_CODEC_OK;

test/aom_integer_test.cc

@@ -14,6 +14,9 @@
 #include "gtest/gtest.h"
 
 namespace {
+const uint64_t kMaximumLeb128CodedSize = 8;
+const uint8_t kLeb128PadByte = 0x80;                    // Binary: 10000000
+const uint64_t kMaximumLeb128Value = 0xFFFFFFFFFFFFFF;  // 2 ^ 56 - 1
 const uint32_t kSizeTestNumValues = 6;
 const uint32_t kSizeTestExpectedSizes[kSizeTestNumValues] = {
   1, 1, 2, 3, 4, 5
@@ -28,7 +31,7 @@ TEST(AomLeb128, DecodeTest) {
   const uint8_t leb128_bytes[num_leb128_bytes] = { 0xE5, 0x8E, 0x26 };
   const uint64_t expected_value = 0x98765;  // 624485
   uint64_t value = 0;
-  aom_uleb_decode(&leb128_bytes[0], num_leb128_bytes, &value);
+  ASSERT_EQ(aom_uleb_decode(&leb128_bytes[0], num_leb128_bytes, &value), 0);
   ASSERT_EQ(expected_value, value);
 
   // Make sure the decoder stops on the last marked LEB128 byte.
@@ -54,7 +57,7 @@ TEST(AomLeb128, EncodeTest) {
   ASSERT_EQ(expected_value, encoded_value);
 }
 
-TEST(AomLeb128, EncDecTest) {
+TEST(AomLeb128, EncodeDecodeTest) {
   const uint32_t value = 0x98765;  // 624485
   const size_t kWriteBufferSize = 4;
   uint8_t write_buffer[kWriteBufferSize] = { 0 };
@@ -86,7 +89,7 @@ TEST(AomLeb128, FixedSizeEncodeTest) {
   ASSERT_EQ(expected_value, encoded_value);
 }
 
-TEST(AomLeb128, FixedSizeEncDecTest) {
+TEST(AomLeb128, FixedSizeEncodeDecodeTest) {
   const uint32_t value = 0x1;
   const size_t kWriteBufferSize = 4;
   uint8_t write_buffer[kWriteBufferSize] = { 0 };
@@ -108,7 +111,29 @@ TEST(AomLeb128, SizeTest) {
   }
 }
 
-TEST(AomLeb128, FailTest) {
+TEST(AomLeb128, DecodeFailTest) {
+  // Input buffer containing what would be a valid 9 byte LEB128 encoded
+  // unsigned integer.
+  const uint8_t kAllPadBytesBuffer[kMaximumLeb128CodedSize + 1] = {
+    kLeb128PadByte, kLeb128PadByte, kLeb128PadByte,
+    kLeb128PadByte, kLeb128PadByte, kLeb128PadByte,
+    kLeb128PadByte, kLeb128PadByte, 0
+  };
+  uint64_t decoded_value = 0;
+
+  // Test that decode fails when result would be valid 9 byte integer.
+  ASSERT_EQ(aom_uleb_decode(&kAllPadBytesBuffer[0], kMaximumLeb128CodedSize + 1,
+                            &decoded_value),
+            -1);
+
+  // Test that encoded value missing terminator byte within available buffer
+  // range causes decode error.
+  ASSERT_EQ(aom_uleb_decode(&kAllPadBytesBuffer[0], kMaximumLeb128CodedSize,
+                            &decoded_value),
+            -1);
+}
+
+TEST(AomLeb128, EncodeFailTest) {
   const size_t kWriteBufferSize = 4;
   const uint32_t kValidTestValue = 1;
   uint8_t write_buffer[kWriteBufferSize] = { 0 };
@@ -120,7 +145,12 @@ TEST(AomLeb128, FailTest) {
                             NULL),
             -1);
 
-  const uint32_t kValueOutOfRange = 0xFFFFFFFF;
+  const uint32_t kValueOutOfRangeForBuffer = 0xFFFFFFFF;
+  ASSERT_EQ(aom_uleb_encode(kValueOutOfRangeForBuffer, kWriteBufferSize,
+                            &write_buffer[0], &coded_size),
+            -1);
+
+  const uint64_t kValueOutOfRange = kMaximumLeb128Value + 1;
   ASSERT_EQ(aom_uleb_encode(kValueOutOfRange, kWriteBufferSize,
                             &write_buffer[0], &coded_size),
             -1);

tools/obu_parser.cc

@@ -220,7 +220,7 @@ bool DumpObu(const uint8_t *data, int length, int *obu_overhead_bytes) {
     consumed += current_obu_length;
   }
 
-  if (obu_overhead_bytes) *obu_overhead_bytes = obu_overhead;
+  if (obu_overhead_bytes != nullptr) *obu_overhead_bytes = obu_overhead;
 
   return true;
 }