Accumulate leb128 value using uint64_t as intermediate type

The shift-amount can be up to 56, and left-shifting 32-bit integers by values >=32 is undefined behaviour. Therefore, use 64-bit integers instead. Also slightly rewrite so we only call dav1d_get_bits() once for the combined more|bits value, and mask the relevant portions out instead of reading twice. Lastly, move the overflow check out of the loop (as suggested by @wtc)

Fixes #341.

CC @wtc

It requires them to be 8 bytes at most, yes, but also constrains all such values to 2^32-1, so instead of casting to uint64_t, we could ensure the loop doesn't go beyond i == 4 as Gramner mentioned.

We can't. Having the loop go more iterations is conformant as long as all higher bits are zero.

The usefulness of that feature is perhaps questionable, but as long as the spec allows it we need to handle it.

Maybe use a separate loop for i > 4, where as long as all its bits are zero for up to four iterations we return the actual value safely stored in val?

Although i don't know if it's worth the extra complexity. leb128() is not used in a lot of places, and a cast will only negatively affect 32bits targets.

added 1 commit

• fd007af4 - Cast value to uint64_t before left-shifting

Compare with previous version

added 1 commit

• 29bbed1a - Accumulate leb128 value using uint64_t as intermediate type

Compare with previous version

changed title from Cast value to uint64_t before left-shifting to Accumulate leb128 value using uint64_t as intermediate type

changed the description

added 1 commit

• 47daa4df - Accumulate leb128 value using uint64_t as intermediate type

Compare with previous version

merged

changed milestone to %0.8.0