[FFmpeg-devel] [PATCH 4/6] avfilter/formats: Leave lists' ownership unchanged upon merge failure

[Andreas Rheinhardt]

Nicolas George:
> Andreas Rheinhardt (12020-08-11):
>> Do you remember
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2020-August/267549.html? If I
>> were allowed to reorder a and b so that a->nb <= b->nb, one would not
>> have to allocate a new list to return at all: One could reuse a and
>> would only have to reallocate a->refs to also include b->refs and update
>> the refs coming from b (the ones from a are already fine!).
> 
> I remember, but we are not talking about the same thing.
> 
> You are talking about the intersection algorithm, applied to the list of
> supported formats. For this, we know that the output is at most as large
> as the smaller list, and we could keep it here instead of allocating a
> new list. I will reply to this.
> 
> But but here I was talking about the list of references, which are just
> concatenated: instead of creating an empty array and copying both lists
> into it, we could grow one of the list and copy the other.
> 
My intention was to eventually reuse the whole AVFilterFormats with the
smaller nb_formats; only the refs need to be reallocated and only the
refs from the other AVFilterFormats will need to be added and updated
(the ones from the AVFilterFormats that is reused are already fine after
all). This will of course not always ensure that the bigger refs list is
retained, but if one is willing to live with this (as I was when I wrote
what you answered to), then reusing the list of supported formats and
reusing a refs list goes nicely together. Therefore I thought of these
two issues as linked and intended to address them together.

If one always wants to reuse the larger refs array, one should also
reuse the AVFilterFormats struct belonging to said refs array (so that
these refs needn't be updated). This would involve a minor complication
if one also wants to reuse the formats array.

Anyway there is another complication for reusing the formats array: If
reallocating the refs array fails lateron, one has already changed one
formats array; and if one reallocates the refs array before looking
through the formats arrays, it might be that said reallocating was in
vain (namely if there are no common formats). The only solution to this
conundrum is that reusing the formats list will have to wait until the
can_merge part is factored out, because when one is able to return an
int < 0 for failure, one can just document that in this case the formats
lists might have been modified.

> For references, there is no doubt that the order does not matter.
> 
> Bot both are micro-optimizations, probably better left for a bit later
> when other things have been made more clean and robust.
> 
>> I have also pondered a second possible way of improvement: There are two
>> types of callers (all in avfiltergraph.c) of the ff_merge_ functions:
>> can_merge_formats() actually only wants to know whether two formats are
>> mergeable and to do so it currently creates short-lived throwaway copies
>> of the lists. Very wasteful. can_merge_formats() could be removed
>> altogether if one would implement check-only functions (which should
>> return an int, not an AVFilterFormats *).
>> The other callers also don't really inspect the current return value at
>> all except from checking whether it is NULL or not (that's because in
>> this case the returned AVFilterFormats has owners, so one doesn't need
>> to care about leaks). These could be satisfied by a function returning
>> an int which also allows to actually distinguish between inability to
>> merge and (re)allocation failures (< 0 would be errors, 0 inability to
>> merge and 1 successfull merge).
> 
> You are right, this part is not very satisfactory; it was added as an
> afterthought 
> 
> If we are to support subtitles or data frames, and partial graph
> reconfiguration, we will need to include all these afterthoughts into a
> clean whole. In particular, we probably need to negotiate the media type
> to avoid having n filters with the same function just for different
> media types.
> 
> I suspect it would be better to have a system of types with callbacks
> rather than a labyrinth of macros. But my ideas about this are not yet
> mature.
> 
Then I'll prepare some patches for the low-hanging fruits here and
proceed with other things not related to the formats API.

- Andreas

PS: I'll apply patches 1-6 this evening or so if there are no more
objections.