[FFmpeg-devel] [PATCH 5/6] avfilter/formats: Simplify cleanup for ff_merge_* functions

[Andreas Rheinhardt]

Nicolas George:
> Andreas Rheinhardt (12020-08-08):
>> PS: Is the order of the formats/channel_layouts arrays actually important?
> 
> IIRC, there are parts of the code that selects best formats and put them
> in the front of the list. I need to dig back into all this to refresh my
> memory, it is complex and it will need to be simplified before it can be
> extended for subtitles or data packets.
> 

I was thinking about this part of the MERGE_FORMATS macro:

        for (i = 0; i < a->nb; i++)
            for (j = 0; j < b->nb; j++)
                if (a->fmts[i] == b->fmts[j]) {
                    if(k >= FFMIN(a->nb, b->nb)){
                        av_log(NULL, AV_LOG_ERROR, "Duplicate formats in
%s detected\n", __FUNCTION__);
                        av_free(ret->fmts);
                        av_free(ret);
                        return NULL;
                    }
                    ret->fmts[k++] = a->fmts[i];
                }

Said check is not sufficient to ensure that there are no duplicates in
ret->fmts, of course. It has been added in the merge commit 1cbf7fb4345
as an additional safety check to ensure that one does not write beyond
the end of ret->fmts (it has FFMIN(a->nb, b->nb) entries). Yet this goal
could also be achieved in a simpler way: If a->nb <= b->nb, one could
simply rewrite the above to

        for (i = 0; i < a->nb; i++)
            for (j = 0; j < b->nb; j++)
                if (a->fmts[i] == b->fmts[j]) {
                    ret->fmts[k++] = a->fmts[i];
                    break;
                }

This ensures that every entry of a will end up at most once in ret. And
if one were simply allowed to switch a and b, one could ensure a->nb <=
b->nb easily. ff_merge_channel_layouts() already does something similar.

As a next step one could then even reuse the a->fmts array for ret->fmts
(or simply reuse a altogether); this is possible because k <= i at the
beginning of every iteration of the outer loop.

Notice that if there is a total ordering of all the formats and if both
a->fmts and b->fmts are ordered according to this ordering, then
ret->fmts will also ordered in that way (regardless of whether one swaps
or not).

- Andreas