diff --git a/src/stream.c b/src/stream.c index c85ca31b54..3b61f9f39e 100644 --- a/src/stream.c +++ b/src/stream.c @@ -189,31 +189,24 @@ avifBool avifROStreamReadBits(avifROStream * stream, uint32_t * v, size_t bitCou return AVIF_TRUE; } -// Based on https://sqlite.org/src4/doc/trunk/www/varint.wiki. +static const int VARINT_DEPTH_0 = 7; // +1 bit to stop or continue. +static const int VARINT_DEPTH_1 = 3; // +1 bit to stop or continue. +static const int VARINT_DEPTH_2 = 18; + avifBool avifROStreamReadVarInt(avifROStream * stream, uint32_t * v) { - uint32_t a[5]; - AVIF_CHECK(avifROStreamReadBits(stream, &a[0], 8)); - if (a[0] <= 240) { - *v = a[0]; - } else { - AVIF_CHECK(avifROStreamReadBits(stream, &a[1], 8)); - if (a[0] <= 248) { - *v = 240 + 256 * (a[0] - 241) + a[1]; - } else { - AVIF_CHECK(avifROStreamReadBits(stream, &a[2], 8)); - if (a[0] == 249) { - *v = 2288 + 256 * a[1] + a[2]; - } else { - AVIF_CHECK(avifROStreamReadBits(stream, &a[3], 8)); - if (a[0] == 250) { - *v = (a[3] << 16) | (a[2] << 8) | a[1]; - } else { - // TODO(yguyon): Use values of a[0] in range [252-255] (avoid pessimization). - AVIF_CHECK(avifROStreamReadBits(stream, &a[4], 8)); - *v = (a[4] << 24) | (a[3] << 16) | (a[2] << 8) | a[1]; - } - } + AVIF_CHECK(avifROStreamReadBits(stream, v, VARINT_DEPTH_0)); + uint32_t extended, extension; + + AVIF_CHECK(avifROStreamReadBits(stream, &extended, 1)); + if (extended) { + AVIF_CHECK(avifROStreamReadBits(stream, &extension, VARINT_DEPTH_1)); + *v += (extension + 1) << VARINT_DEPTH_0; + + AVIF_CHECK(avifROStreamReadBits(stream, &extended, 1)); + if (extended) { + AVIF_CHECK(avifROStreamReadBits(stream, &extension, VARINT_DEPTH_2)); + *v += (extension + 1) << (VARINT_DEPTH_0 + VARINT_DEPTH_1); } } return AVIF_TRUE; @@ -495,29 +488,24 @@ avifResult avifRWStreamWriteBits(avifRWStream * stream, uint32_t v, size_t bitCo return AVIF_RESULT_OK; } -// Based on https://sqlite.org/src4/doc/trunk/www/varint.wiki. avifResult avifRWStreamWriteVarInt(avifRWStream * stream, uint32_t v) { - if (v <= 240) { - AVIF_CHECKRES(avifRWStreamWriteBits(stream, v, 8)); - } else if (v <= 2287) { - AVIF_CHECKRES(avifRWStreamWriteBits(stream, (v - 240) / 256 + 241, 8)); - AVIF_CHECKRES(avifRWStreamWriteBits(stream, (v - 240) % 256, 8)); - } else if (v <= 67823) { - AVIF_CHECKRES(avifRWStreamWriteBits(stream, 249, 8)); - AVIF_CHECKRES(avifRWStreamWriteBits(stream, (v - 2288) / 256, 8)); - AVIF_CHECKRES(avifRWStreamWriteBits(stream, (v - 2288) % 256, 8)); - } else if (v <= 16777215) { - AVIF_CHECKRES(avifRWStreamWriteBits(stream, 250, 8)); - AVIF_CHECKRES(avifRWStreamWriteBits(stream, (v >> 0) & 0xff, 8)); - AVIF_CHECKRES(avifRWStreamWriteBits(stream, (v >> 8) & 0xff, 8)); - AVIF_CHECKRES(avifRWStreamWriteBits(stream, (v >> 16) & 0xff, 8)); - } else { - AVIF_CHECKRES(avifRWStreamWriteBits(stream, 251, 8)); - AVIF_CHECKRES(avifRWStreamWriteBits(stream, (v >> 0) & 0xff, 8)); - AVIF_CHECKRES(avifRWStreamWriteBits(stream, (v >> 8) & 0xff, 8)); - AVIF_CHECKRES(avifRWStreamWriteBits(stream, (v >> 16) & 0xff, 8)); - AVIF_CHECKRES(avifRWStreamWriteBits(stream, (v >> 24) & 0xff, 8)); + AVIF_CHECKERR(v < (1u << (VARINT_DEPTH_0 + VARINT_DEPTH_1 + VARINT_DEPTH_2)) + (1u << (VARINT_DEPTH_0 + VARINT_DEPTH_1)) + + (1u << VARINT_DEPTH_0), + AVIF_RESULT_INVALID_ARGUMENT); + + AVIF_CHECKRES(avifRWStreamWriteBits(stream, v & ((1u << VARINT_DEPTH_0) - 1), VARINT_DEPTH_0)); // value + v >>= VARINT_DEPTH_0; + AVIF_CHECKRES(avifRWStreamWriteBits(stream, v > 0, 1)); // extended + if (v > 0) { + v -= 1; + AVIF_CHECKRES(avifRWStreamWriteBits(stream, v & ((1u << VARINT_DEPTH_1) - 1), VARINT_DEPTH_1)); // extension + v >>= VARINT_DEPTH_1; + AVIF_CHECKRES(avifRWStreamWriteBits(stream, v > 0, 1)); // extended + if (v > 0) { + v -= 1; + AVIF_CHECKRES(avifRWStreamWriteBits(stream, v, VARINT_DEPTH_2)); // extension + } } return AVIF_RESULT_OK; } diff --git a/tests/gtest/avifstreamtest.cc b/tests/gtest/avifstreamtest.cc index 79ff8b0ce9..565efe21aa 100644 --- a/tests/gtest/avifstreamtest.cc +++ b/tests/gtest/avifstreamtest.cc @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "avif/internal.h" @@ -13,11 +14,14 @@ namespace avif { namespace { +// Taken from stream.c. +static const int VARINT_DEPTH_0 = 7; +static const int VARINT_DEPTH_1 = 3; +static const int VARINT_DEPTH_2 = 18; + //------------------------------------------------------------------------------ TEST(StreamTest, Roundtrip) { - // TODO(yguyon): Check return values once pull request #1498 is merged. - // Write some fields. testutil::AvifRwData rw_data; avifRWStream rw_stream; @@ -61,17 +65,22 @@ TEST(StreamTest, Roundtrip) { EXPECT_EQ(avifRWStreamWriteU32(&rw_stream, rw_someu32), AVIF_RESULT_OK); size_t offset = avifRWStreamOffset(&rw_stream); - const uint32_t rw_somevarint_1byte = 240; + const uint32_t rw_somevarint_1byte = (1 << VARINT_DEPTH_0) - 1; EXPECT_EQ(avifRWStreamWriteVarInt(&rw_stream, rw_somevarint_1byte), AVIF_RESULT_OK); EXPECT_EQ(avifRWStreamOffset(&rw_stream), offset + 1); offset = avifRWStreamOffset(&rw_stream); - const uint32_t rw_somevarint_2bytes = 241; + const uint32_t rw_somevarint_2bytes = (1 << VARINT_DEPTH_0); EXPECT_EQ(avifRWStreamWriteVarInt(&rw_stream, rw_somevarint_2bytes), AVIF_RESULT_OK); EXPECT_EQ(avifRWStreamOffset(&rw_stream), offset + 2); + // Pad till byte alignment. + EXPECT_EQ(avifRWStreamWriteBits(&rw_stream, 0, + 8 - rw_stream.numUsedBitsInPartialByte), + AVIF_RESULT_OK); + const uint64_t rw_someu64 = 0xAABBCCDDEEFF0011; EXPECT_EQ(avifRWStreamWriteU64(&rw_stream, rw_someu64), AVIF_RESULT_OK); @@ -85,27 +94,19 @@ TEST(StreamTest, Roundtrip) { AVIF_RESULT_OK); offset = avifRWStreamOffset(&rw_stream); - const uint32_t rw_somevarint_3bytes = 2288; - EXPECT_EQ(avifRWStreamWriteVarInt(&rw_stream, rw_somevarint_3bytes), - AVIF_RESULT_OK); - const uint32_t rw_somevarint_4bytes = 67824; + const uint32_t rw_somevarint_4bytes = 1 << VARINT_DEPTH_2; EXPECT_EQ(avifRWStreamWriteVarInt(&rw_stream, rw_somevarint_4bytes), AVIF_RESULT_OK); - EXPECT_EQ(avifRWStreamOffset(&rw_stream), offset + 3 + 4); + EXPECT_EQ(avifRWStreamOffset(&rw_stream), offset + 4); const uint32_t rw_somebit = 1; EXPECT_EQ(avifRWStreamWriteBits(&rw_stream, rw_somebit, /*bitCount=*/1), AVIF_RESULT_OK); // Pad till byte alignment. - EXPECT_EQ(avifRWStreamWriteBits(&rw_stream, rw_somebit, /*bitCount=*/1), + EXPECT_EQ(avifRWStreamWriteBits(&rw_stream, 0, + 8 - rw_stream.numUsedBitsInPartialByte), AVIF_RESULT_OK); - offset = avifRWStreamOffset(&rw_stream); - const uint32_t rw_somevarint_5bytes = std::numeric_limits::max(); - EXPECT_EQ(avifRWStreamWriteVarInt(&rw_stream, rw_somevarint_5bytes), - AVIF_RESULT_OK); - EXPECT_EQ(avifRWStreamOffset(&rw_stream), offset + 5); - const size_t num_zeros = 10000; EXPECT_EQ(avifRWStreamWriteZeros(&rw_stream, /*byteCount=*/num_zeros), AVIF_RESULT_OK); @@ -171,6 +172,10 @@ TEST(StreamTest, Roundtrip) { EXPECT_TRUE(avifROStreamReadVarInt(&ro_stream, &ro_somevarint_2bytes)); EXPECT_EQ(rw_somevarint_2bytes, ro_somevarint_2bytes); + // Pad till byte alignment. + EXPECT_TRUE(avifROStreamReadBits8(&ro_stream, &ro_someu8, + 8 - ro_stream.numUsedBitsInPartialByte)); + uint64_t ro_someu64; EXPECT_TRUE(avifROStreamReadU64(&ro_stream, &ro_someu64)); EXPECT_EQ(rw_someu64, ro_someu64); @@ -182,10 +187,6 @@ TEST(StreamTest, Roundtrip) { EXPECT_TRUE(avifROStreamReadBits(&ro_stream, &ro_maxbits, rw_maxbitcount)); EXPECT_EQ(rw_maxbits, ro_maxbits); - uint32_t ro_somevarint_3bytes; - EXPECT_TRUE(avifROStreamReadVarInt(&ro_stream, &ro_somevarint_3bytes)); - EXPECT_EQ(rw_somevarint_3bytes, ro_somevarint_3bytes); - uint32_t ro_somevarint_4bytes; EXPECT_TRUE(avifROStreamReadVarInt(&ro_stream, &ro_somevarint_4bytes)); EXPECT_EQ(rw_somevarint_4bytes, ro_somevarint_4bytes); @@ -193,17 +194,58 @@ TEST(StreamTest, Roundtrip) { uint8_t ro_somebit; EXPECT_TRUE(avifROStreamReadBits8(&ro_stream, &ro_somebit, /*bitCount=*/1)); EXPECT_EQ(rw_somebit, ro_somebit); - EXPECT_TRUE(avifROStreamReadBits8(&ro_stream, &ro_somebit, /*bitCount=*/1)); - EXPECT_EQ(rw_somebit, ro_somebit); - uint32_t ro_somevarint_5bytes; - EXPECT_TRUE(avifROStreamReadVarInt(&ro_stream, &ro_somevarint_5bytes)); - EXPECT_EQ(rw_somevarint_5bytes, ro_somevarint_5bytes); + // Pad till byte alignment. + EXPECT_TRUE(avifROStreamReadBits8(&ro_stream, &ro_someu8, + 8 - ro_stream.numUsedBitsInPartialByte)); EXPECT_TRUE(avifROStreamSkip(&ro_stream, /*byteCount=*/num_zeros)); EXPECT_FALSE(avifROStreamSkip(&ro_stream, /*byteCount=*/1)); } +//------------------------------------------------------------------------------ +// Variable length integer implementation + +constexpr uint32_t kMaxVarint = + (1 << (VARINT_DEPTH_2 + VARINT_DEPTH_1 + VARINT_DEPTH_0)) + + (1 << (VARINT_DEPTH_1 + VARINT_DEPTH_0)) + (1 << VARINT_DEPTH_0) - 1; + +TEST(StreamTest, VarIntEncDecRoundtrip) { + std::vector values(1 << 12); + std::iota(values.begin(), values.end(), 0u); + for (int depth = 13; depth <= 28; ++depth) { + values.push_back(1 << depth); + } + values.push_back(kMaxVarint - 1); + values.push_back(kMaxVarint); + + testutil::AvifRwData rw_data; + avifRWStream rw_stream; + avifRWStreamStart(&rw_stream, &rw_data); + for (uint32_t rw_value : values) { + EXPECT_EQ(avifRWStreamWriteVarInt(&rw_stream, rw_value), AVIF_RESULT_OK); + } + avifRWStreamFinishWrite(&rw_stream); + + avifROData ro_data = {rw_data.data, rw_data.size}; + avifROStream ro_stream; + avifROStreamStart(&ro_stream, &ro_data, /*diag=*/nullptr, + /*diagContext=*/nullptr); + uint32_t ro_value; + for (uint32_t rw_value : values) { + EXPECT_TRUE(avifROStreamReadVarInt(&ro_stream, &ro_value)); + ASSERT_EQ(rw_value, ro_value); + } +} + +TEST(StreamTest, VarIntLimit) { + testutil::AvifRwData rw_data; + avifRWStream rw_stream; + avifRWStreamStart(&rw_stream, &rw_data); + EXPECT_EQ(avifRWStreamWriteVarInt(&rw_stream, kMaxVarint + 1), + AVIF_RESULT_INVALID_ARGUMENT); +} + //------------------------------------------------------------------------------ } // namespace