[FFmpeg-devel] [PATCH v2] avfilter/formats: optimize ff_all_formats

[Tomas Härdin]

tis 2019-12-10 klockan 22:14 +0800 skrev zhilizhao:
> > On Dec 10, 2019, at 4:20 AM, Tomas Härdin <tjoppen at acc.umu.se> wrote:
> > 
> > lör 2019-12-07 klockan 01:06 +0800 skrev Zhao Zhili:
> > > This is a micro-optimization. Saving almost 200 reallocations makes
> > > it
> > > worth a try.
> > > ---
> > > fix commit message typo: relocations -> reallocations
> > > 
> > > libavfilter/formats.c | 35 +++++++++++++++++++++--------------
> > > 1 file changed, 21 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> > > index 33c64668a0..1af7a1cedd 100644
> > > --- a/libavfilter/formats.c
> > > +++ b/libavfilter/formats.c
> > > @@ -348,23 +348,30 @@ int
> > > ff_add_channel_layout(AVFilterChannelLayouts **l, uint64_t
> > > channel_layout)
> > > 
> > > AVFilterFormats *ff_all_formats(enum AVMediaType type)
> > > {
> > > -    AVFilterFormats *ret = NULL;
> > > +    AVFilterFormats *ret;
> > > +    int i, count;
> > > 
> > > -    if (type == AVMEDIA_TYPE_VIDEO) {
> > > -        const AVPixFmtDescriptor *desc = NULL;
> > > -        while ((desc = av_pix_fmt_desc_next(desc))) {
> > > -            if (ff_add_format(&ret, av_pix_fmt_desc_get_id(desc)) <
> > > 0)
> > > -                return NULL;
> > > -        }
> > > -    } else if (type == AVMEDIA_TYPE_AUDIO) {
> > > -        enum AVSampleFormat fmt = 0;
> > > -        while (av_get_sample_fmt_name(fmt)) {
> > > -            if (ff_add_format(&ret, fmt) < 0)
> > > -                return NULL;
> > > -            fmt++;
> > > -        }
> > > +    if (type == AVMEDIA_TYPE_VIDEO)
> > > +        count = AV_PIX_FMT_NB;
> > > +    else if (type == AVMEDIA_TYPE_AUDIO)
> > > +        count = AV_SAMPLE_FMT_NB;
> > > +    else
> > > +        return NULL;
> > > +
> > > +    ret = av_mallocz(sizeof(*ret));
> > > +    if (!ret)
> > > +        return NULL;
> > > +
> > > +    ret->nb_formats = count;
> > > +    ret->formats = av_malloc_array(count, sizeof(*ret->formats));
> > > +    if (!ret->formats) {
> > > +        av_free(ret);
> > > +        return NULL;
> > >     }
> > > 
> > > +    for (i = 0; i < count; i++)
> > > +        ret->formats[i] = i;
> > 
> > As far as I can tell this is OK, and it passes FATE. But it just looks
> > very very wrong. Why does this function even exist if all it
> > effectively does is return the integers from 0..count-1?
> 
> The function is there since the first libavfilter commit. I guess it’s for forward compatibility.
> 
> I don’t know whether a ‘all_formats’ flag works or not.

Perhaps someone with more insight into lavfi wants to comment?

/Tomas