[FFmpeg-devel] [PATCH 20/39] avcodec/h261dec: Don't initialize unused part of RLTable

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2021-01-21 21:20:52)
>> Anton Khirnov:
>>> Quoting Andreas Rheinhardt (2020-12-10 12:16:38)
>>>> The H.261 decoder only uses an RLTable's VLC table, yet it also
>>>> initializes its index_run, max_level and max_run. This commit stops
>>>> doing so; it will also simplify making this decoder init-threadsafe,
>>>> as the H.261 decoder and encoder now initialize disjoint parts of their
>>>> common RLTable.
>>>
>>> Does it then make sense to keep this RLTable common?
>>>
>> I presume you want to know whether the RLTable structure should be split
>> into smaller structures?
> 
> No, what I meant was whether we shouldn't use different RLTable
> instances for encoder and decoder, since their use is disjoint. That
> would make the code easier to reason about.

I actually didn't think that these RLTables were difficult to reason
about: ff_rl_init and ff_rl_init_vlc/ff_init_2d_vlc_rl initialize
different parts of an RLTable and both only use the static parts of an
RLTable, so that these two can be called independently. In particular
there is no clash in the case of H.261 after the unnecessary call to
ff_rl_init by the decoder is gone. And in case the decoder needs
ff_rl_init, too, one just needs to make sure that it is only initialized
once and that is really not onerous.

So, my answer to your original question is that it makes sense to keep
these RLTables common.

>>
>> PS: It seems you overlooked patches 7-13.
> 
> It's more that I have next to zero experience with/insight into that
> code. I don't think I can review on more than the most superficial level
> without major effort.
> 
Ok, then I'll just ping these patches.

- Andreas