[FFmpeg-devel] [PATCH 13/21] avcodec/smacker: Remove redundant checks when reading VLC codes

[Andreas Rheinhardt]

Paul B Mahol:
> On 8/1/20, Andreas Rheinhardt <andreas.rheinhardt at gmail.com> wrote:
>> The VLC codes in question originate from a Huffmann tree and so every
>> sequence of bits that is longer than the longest code contains an
>> initial sequence that is a valid code. Given that it has been checked
>> during reading said tree (and once again in ff_init_vlc_sparse()) that
>> the length of each code is <= 3 * the number of bits read at once when
>> reading codes, get_vlc2() will always find a matching entry.
>>
>> These checks have been added in 71d3c25a7ef442ac2dd7b6fbf7c489ebc0b58e9b
>> at a time when the length of the codes had not been checked when parsing
>> the tree.
>>
>> For GCC 9 and the sample from ticket #2425 this led to a slight
>> performance regression: The time for one call to smka_decode_frame()
>> increased from 2053671 to 2064529 decicycles; for Clang 9, performance
>> improved from 1521288 to 1508459 decicycles.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>> ---
>>  libavcodec/smacker.c | 32 --------------------------------
>>  1 file changed, 32 deletions(-)
>>
> 
> Sure this does not continue under bitstream errors?
> 
> Try some smart fuzzers.
> 
Every long enough sequence of bits (namely every sequence longer than
the longest Huffman code) contains an initial sequence that is a valid
Huffman code, so the only detectable bitstream error there could be is
end of bitstream (there is of course also the possibility of outputting
garbage, but that is not detectable in the decoder). This statement
follows directly from the fact that every non-leaf node in a Huffman
tree has exactly two children that correspond to 0 and 1.

- Andreas

PS: Thanks for taking a look at this patchset.