Avoid a bitread assert error The bit reading expects to read less than 32 bits, which is violated in the original macro. Define a new macro to correctly read it. BUG=aomedia:502133197 Change-Id: Ia617434a348f09267dd7547237f1d283e7cb4449
diff --git a/aom/exports_com b/aom/exports_com index f3dbeae..1b598a0 100644 --- a/aom/exports_com +++ b/aom/exports_com
@@ -27,6 +27,7 @@ text aom_rb_bytes_read text aom_rb_read_bit text aom_rb_read_literal +text aom_rb_read_unsigned_literal text aom_rb_read_uvlc text aom_uleb_decode text aom_uleb_encode
diff --git a/aom_dsp/bitreader_buffer.c b/aom_dsp/bitreader_buffer.c index df153df..191240b 100644 --- a/aom_dsp/bitreader_buffer.c +++ b/aom_dsp/bitreader_buffer.c
@@ -43,7 +43,6 @@ return value; } -#if CONFIG_AV1_DECODER uint32_t aom_rb_read_unsigned_literal(struct aom_read_bit_buffer *rb, int bits) { assert(bits <= 32); @@ -54,6 +53,7 @@ return value; } +#if CONFIG_AV1_DECODER int aom_rb_read_inv_signed_literal(struct aom_read_bit_buffer *rb, int bits) { const int nbits = sizeof(unsigned) * 8 - bits - 1; const unsigned value = (unsigned)aom_rb_read_literal(rb, bits + 1) << nbits;
diff --git a/aom_dsp/bitreader_buffer.h b/aom_dsp/bitreader_buffer.h index 0112fb4..8c217fd 100644 --- a/aom_dsp/bitreader_buffer.h +++ b/aom_dsp/bitreader_buffer.h
@@ -43,9 +43,9 @@ // bits). uint32_t aom_rb_read_uvlc(struct aom_read_bit_buffer *rb); -#if CONFIG_AV1_DECODER uint32_t aom_rb_read_unsigned_literal(struct aom_read_bit_buffer *rb, int bits); +#if CONFIG_AV1_DECODER int aom_rb_read_inv_signed_literal(struct aom_read_bit_buffer *rb, int bits); int16_t aom_rb_read_signed_primitive_refsubexpfin(
diff --git a/common/av1_config.c b/common/av1_config.c index b37d8dc..205f25f 100644 --- a/common/av1_config.c +++ b/common/av1_config.c
@@ -49,6 +49,19 @@ } \ } while (0) +#define AV1C_READ_UNSIGNED_OR_RETURN_ERROR(field, length) \ + int field = 0; \ + do { \ + field = aom_rb_read_unsigned_literal(reader, (length)); \ + if (result == -1) { \ + fprintf(stderr, \ + "av1c: Could not read bits for " #field \ + ", value=%d result=%d.\n", \ + field, result); \ + return -1; \ + } \ + } while (0) + // Helper macros for setting/restoring the error handler data in // aom_read_bit_buffer. #define AV1C_PUSH_ERROR_HANDLER_DATA(new_data) \ @@ -82,8 +95,8 @@ int result = 0; AV1C_PUSH_ERROR_HANDLER_DATA(result); - AV1C_READ_BITS_OR_RETURN_ERROR(num_units_in_display_tick, 32); - AV1C_READ_BITS_OR_RETURN_ERROR(time_scale, 32); + AV1C_READ_UNSIGNED_OR_RETURN_ERROR(num_units_in_display_tick, 32); + AV1C_READ_UNSIGNED_OR_RETURN_ERROR(time_scale, 32); AV1C_READ_BIT_OR_RETURN_ERROR(equal_picture_interval); if (equal_picture_interval) { @@ -120,7 +133,7 @@ AV1C_PUSH_ERROR_HANDLER_DATA(result); AV1C_READ_BITS_OR_RETURN_ERROR(buffer_delay_length_minus_1, 5); - AV1C_READ_BITS_OR_RETURN_ERROR(num_units_in_decoding_tick, 32); + AV1C_READ_UNSIGNED_OR_RETURN_ERROR(num_units_in_decoding_tick, 32); AV1C_READ_BITS_OR_RETURN_ERROR(buffer_removal_time_length_minus_1, 5); AV1C_READ_BITS_OR_RETURN_ERROR(frame_presentation_time_length_minus_1, 5);
diff --git a/test/av1_config_test.cc b/test/av1_config_test.cc index a198a56..2c6d9dd 100644 --- a/test/av1_config_test.cc +++ b/test/av1_config_test.cc
@@ -47,8 +47,7 @@ bool VerifyAv1c(const uint8_t *const obu_buffer, size_t obu_buffer_length, bool is_annexb) { - Av1Config av1_config; - memset(&av1_config, 0, sizeof(av1_config)); + Av1Config av1_config = {}; bool parse_ok = get_av1config_from_obu(obu_buffer, obu_buffer_length, is_annexb, &av1_config) == 0; if (parse_ok) { @@ -70,8 +69,7 @@ } TEST(Av1Config, ObuInvalidInputs) { - Av1Config av1_config; - memset(&av1_config, 0, sizeof(av1_config)); + Av1Config av1_config = {}; ASSERT_EQ(-1, get_av1config_from_obu(nullptr, 0, 0, nullptr)); ASSERT_EQ(-1, get_av1config_from_obu(&kLobfFullSequenceHeaderObu[0], 0, 0, nullptr)); @@ -84,9 +82,20 @@ &av1_config)); } +TEST(Av1Config, Buganizer502133197) { + Av1Config av1_config = {}; + + static constexpr uint8_t kBugReproObu[] = { 0x0a, 0x10, 0x04, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00 }; + + EXPECT_EQ(0, get_av1config_from_obu(kBugReproObu, sizeof(kBugReproObu), false, + &av1_config)); +} + TEST(Av1Config, ReadInvalidInputs) { - Av1Config av1_config; - memset(&av1_config, 0, sizeof(av1_config)); + Av1Config av1_config = {}; size_t bytes_read = 0; ASSERT_EQ(-1, read_av1config(nullptr, 0, nullptr, nullptr)); ASSERT_EQ(-1, read_av1config(nullptr, 4, nullptr, nullptr)); @@ -96,8 +105,7 @@ } TEST(Av1Config, WriteInvalidInputs) { - Av1Config av1_config; - memset(&av1_config, 0, sizeof(av1_config)); + Av1Config av1_config = {}; size_t bytes_written = 0; uint8_t av1c_buffer[4] = { 0 }; ASSERT_EQ(-1, write_av1config(nullptr, 0, nullptr, nullptr)); @@ -134,8 +142,7 @@ } TEST(Av1Config, ReadWriteConfig) { - Av1Config av1_config; - memset(&av1_config, 0, sizeof(av1_config)); + Av1Config av1_config = {}; // Test writing out the AV1 config. size_t bytes_written = 0;