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

[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.

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.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200812/ea934b79/attachment.sig>