[FFmpeg-devel] [PATCH 001/114] avcodec/bitstream: Add second function to create VLCs

[Andreas Rheinhardt]

Derek Buitenhuis:
> On 12/11/2020 20:51, Andreas Rheinhardt wrote:
>> The ff_init_vlc_... functions are inherently thread-safe: Everything is
>> modified only once and directly set to its final value; so it's no
>> problem if two threads are initializing the same static VLC at the same
>> time.
> 
> 
> 
> Hmm, indeed. At the very least it is doing repeated work and allocations on every init
> that needn't happen. Wrapping in ff_thread_once is still preferable IMO...
> 
> Unrelatedly: It looks like buf may not be freed when ff_init_vlc_from_lengths is called
> with INIT_VLC_USE_NEW_STATIC with more than LOCALBUF_ELEMS (which while currently
> set to something higher than the max used, could accidentally be called with more than
> 1500 in the future.) There should be fixed, or maybe be an assert added so that nobody
> ends up doing that in the first place by accident.
> 
It is currently asserted that this case doesn't happen. My patch doesn't
change that. (on2avc could use static tables, yet it would need more
than 3000 elements in the localbuf, therefore I did not switch it to
static tables (one would either have to make localbuf bigger or allow
for static table initialization to fail (due to allocation error). The
latter one would mean that the caller would need to check the return
value which is currently not done and unnecessary for all other static
tables; a middle way would be to define a constant and document that
initialization of static tables with less nb_codes than this constant
can't fail. Said constant would then of course be set equal to the size
of the local buffer.)

- Andreas