Every AV1 session on an AMD host died after ~287 frames and silently fell
back to H.265. The client log named it on the first access unit — "AV1
parse: more than 31 (32) bits were requested" — and then "No sequence
header parsed yet" for every AU after, because the sequence header never
parsed and each new keyframe re-hit the same wall.
The vendored cros-codecs BitReader refused any read wider than 31 bits,
"because that would break the read_bits_signed() function". True of the
signed path's i32 accumulator, and misplaced: AV1 needs 32 bits in five
places — timing_info's num_units_in_display_tick and time_scale,
decoder_model_info's num_units_in_decoding_tick, and the variable-width
buffer-delay and buffer_removal_time fields, whose lengths come from the
stream and reach 32. AMF sets timing_info_present_flag; NVENC does not,
which is why the rung's own evidence string ("one vendor, no soak")
described a codec that had never once decoded on AMD. Upstream's
BitWriter already accepted 32 bits, so the crate could emit a header it
could not read back.
Relaxing the guard alone would have been worse than the bug — three edits
are required together:
- the trailing mask is u32::MAX at 32. `1u32 << 32` overflows: a debug
panic, and in release a mask of zero, i.e. a silent 0 return;
- the byte cursor is advanced before the accumulation loop when it sits
at zero remaining bits, which otherwise shifts by the full width and
ORs the spent byte in. At <=31 bits the mask discarded those bits, so
it was invisible; at 32 the mask is all-ones and cannot;
- read_bits_signed carries its own > 31 guard, so widening the unsigned
path does not silently widen the signed one into an overflow. This is
the limit the original comment was actually protecting.
That last guard made a latent panic reachable by test: the sign extension
`-1 ^ ((1 << num_bits) - 1)` overflows at num_bits == 31, where 1i32 << 31
is i32::MIN and subtracting one from it panics in debug — a width the
guard admits and upstream considered safe. Rewritten as `-1i32 <<
num_bits`, equal for every accepted width.
Blast radius is provably AV1-only: neither H.264 nor H.265 has a read
wider than 31 bits, literal or variable — every dynamic-width call site in
the vendored tree is in the AV1 parser. The 52 upstream conformance tests
(H.264/H.265/AV1/VP9) still pass unchanged.
Tests: 32-bit reads byte-aligned, mid-byte, and entered on a spent cursor;
33 bits still refused; the signed path stops at 31 and still sign-extends;
every width 1..=31 checked against an independent extraction across a
spent-byte boundary; zero-width reads still consume nothing. End to end,
an AV1 sequence header carrying timing_info now survives a synthesize/parse
round trip — and reproduces the field error string exactly when the guard
is reverted.
Recorded as PROVENANCE deviation 8; owed upstream as a cros-codecs issue.
enricobuehler
a602e7cf91