[FFmpeg-devel] [PATCH 1/2] lavfi: regroup formats lists in a single structure.

[Andreas Rheinhardt]

Nicolas George:
> Andreas Rheinhardt (12020-08-18):
>> 1. Adding this struct has also another benefit besides the ones in your
>> two other patches: One can simplify the freeing process by adding a new
>> ff_formatscfg_unref() (I'm not sure on the name). This could also be
>> used in the uninit function of the aformat filter. If I am not mistaken,
>> the only place outside of formats.c where one needs to unref an
>> AVFilterContext/AVFilterChannelLayout is in hwdownload where one only
>> needs ff_formats_unref() -- ff_channel_layouts_unref() can therefore be
>> made static. (This presumes that redundant ff_*_unref() have been
>> removed as is done in [1].)
> 
> Actually, I find the use of ff_formats_unref() in filter somewhat
> suspicious. The functions are designed to free their arguments if
> something fails, so as not to leak anything.
> 

This only works if one only works with one list without owner at a time;
if one has several such lists at the same time and an error happens when
working with one of these lists, one has to free the other lists.

This scenario can of course usually be avoided by making sure that one
only has one list without owner at any given time. I used this method in
my patches a lot. I have also rewritten the hwdownload patch [1] to
avoid explicit ff_formats_unref() in it. This means that
ff_formats_unref() can become static, too.

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-August/268429.html

>> 2. For the ease of backportability the double-frees/memleaks ([2] and
>> the other patches from that patchset) should be applied before this one.
> 
> What is missing before applying this?
> 

What does "this" refer to here? If it refers to my patches, then nothing
is missing (except a review). They can be applied at once and then they
can also be directly backported and you can then apply your big patch on
top of it; of course, there will be trivial conflicts in the few files
that I touch.

If "this" refers to your patches: Applying them before fixing the
leaks/double-frees will make backporting the fixes much more troublesome.

- Andreas