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

[Nicolas George]

Andreas Rheinhardt (12020-08-12):
> 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).

I had not thought of that optimization, good idea.

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

You are right, they are somewhat linked.

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

Since the lists are about to be merged anyway, we can use any part we
want: we can build the new list in a local variable, using the smallest
formats array and the larger refs array, and in the end overwrite one of
the lists and free the other.

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

I do not think memory allocation failures are worth worrying about too
much: if something that small fails, then everything else will fail. So
we have to handle them gracefully, but we do not need to be able to
recover from them. So I say: if memory allocation fails, let us free
everything and cleanly return an error.

But I have another idea worth considering, that could get us rid of the
refs array entirely; I would like your advice on it before trying to
implement it:

Let us use a linked list of references instead of an array.

That would look approximatively like this:

    struct AVFilterFormatsRef {
	AVFilterFormats *formats;
	AVFilterFormatsRef *next;
    };

And we put this structure directly into AVFilterLink. Then to merge two
formats lists, we just walk the list and update all the pointers, and
then connect the linked lists.

There is the small drawback that the next pointer stays in the link
structure forever, but if we worry about this, there is more gain to be
obtained by putting everything related to the initialization in a
separate structure.

> Then I'll prepare some patches for the low-hanging fruits here and
> proceed with other things not related to the formats API.

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

Ok to both.

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/83c80c6e/attachment.sig>