| Message ID | 20200103142521.37264-1-quinkblack@foxmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,v2] avfilter/formats: optimize ff_all_formats | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | fail | Times out |
quinkblack@foxmail.com: > From: Zhao Zhili <zhilizhao@tencent.com> > > This is a micro-optimization. Saving almost 200 reallocations makes it > worth a try. > --- > libavfilter/formats.c | 43 +++++++++++++++++++++++++++++++++++-------- > 1 file changed, 35 insertions(+), 8 deletions(-) > > diff --git a/libavfilter/formats.c b/libavfilter/formats.c > index 33c64668a0..c04253d898 100644 > --- a/libavfilter/formats.c > +++ b/libavfilter/formats.c > @@ -348,21 +348,48 @@ int ff_add_channel_layout(AVFilterChannelLayouts **l, uint64_t channel_layout) > > AVFilterFormats *ff_all_formats(enum AVMediaType type) > { > - AVFilterFormats *ret = NULL; > + int count, i; > + AVFilterFormats *ret = av_mallocz(sizeof(*ret)); > + > + if (!ret) > + return NULL; > > 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; > + > + count = 0; > + while ((desc = av_pix_fmt_desc_next(desc))) > + count++; > + > + ret->formats = av_malloc_array(count, sizeof(*ret->formats)); > + if (!ret->formats) { > + av_free(ret); > + return NULL; > + } > + ret->nb_formats = count; > + > + for (i = 0, desc = NULL; i < count; i++) { > + desc = av_pix_fmt_desc_next(desc); > + ret->formats[i] = av_pix_fmt_desc_get_id(desc); > } > } 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++; > + > + count = 0; > + while (av_get_sample_fmt_name(fmt)) > + count++; This looks like an infinite loop. (The code you removed incremented fmt in the loop.) - Andreas > + > + ret->formats = av_malloc_array(count, sizeof(*ret->formats)); > + if (!ret->formats) { > + av_free(ret); > + return NULL; > } > + ret->nb_formats = count; > + > + for (fmt = 0; fmt < count; fmt++) > + ret->formats[fmt] = fmt; > + } else { > + av_freep(&ret); > } > > return ret; >
On Fri, 03. Jan 22:25, quinkblack@foxmail.com wrote: > From: Zhao Zhili <zhilizhao@tencent.com> > > This is a micro-optimization. Saving almost 200 reallocations makes it > worth a try. > --- > libavfilter/formats.c | 43 +++++++++++++++++++++++++++++++++++-------- > 1 file changed, 35 insertions(+), 8 deletions(-) > > diff --git a/libavfilter/formats.c b/libavfilter/formats.c > index 33c64668a0..c04253d898 100644 > --- a/libavfilter/formats.c > +++ b/libavfilter/formats.c > @@ -348,21 +348,48 @@ int ff_add_channel_layout(AVFilterChannelLayouts **l, uint64_t channel_layout) > > AVFilterFormats *ff_all_formats(enum AVMediaType type) > { > - AVFilterFormats *ret = NULL; > + int count, i; > + AVFilterFormats *ret = av_mallocz(sizeof(*ret)); > + > + if (!ret) > + return NULL; > > 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; > + > + count = 0; > + while ((desc = av_pix_fmt_desc_next(desc))) > + count++; > + > + ret->formats = av_malloc_array(count, sizeof(*ret->formats)); > + if (!ret->formats) { > + av_free(ret); > + return NULL; > + } > + ret->nb_formats = count; > + > + for (i = 0, desc = NULL; i < count; i++) { > + desc = av_pix_fmt_desc_next(desc); > + ret->formats[i] = av_pix_fmt_desc_get_id(desc); > } > } 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++; > + > + count = 0; > + while (av_get_sample_fmt_name(fmt)) > + count++; > + > + ret->formats = av_malloc_array(count, sizeof(*ret->formats)); > + if (!ret->formats) { > + av_free(ret); > + return NULL; > } > + ret->nb_formats = count; > + > + for (fmt = 0; fmt < count; fmt++) > + ret->formats[fmt] = fmt; > + } else { > + av_freep(&ret); > } > > return ret; > -- > 2.22.0 > There is somekind of infinite loop here. Gets stuck in fate during: TEST lavf-mxf
diff --git a/libavfilter/formats.c b/libavfilter/formats.c index 33c64668a0..c04253d898 100644 --- a/libavfilter/formats.c +++ b/libavfilter/formats.c @@ -348,21 +348,48 @@ int ff_add_channel_layout(AVFilterChannelLayouts **l, uint64_t channel_layout) AVFilterFormats *ff_all_formats(enum AVMediaType type) { - AVFilterFormats *ret = NULL; + int count, i; + AVFilterFormats *ret = av_mallocz(sizeof(*ret)); + + if (!ret) + return NULL; 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; + + count = 0; + while ((desc = av_pix_fmt_desc_next(desc))) + count++; + + ret->formats = av_malloc_array(count, sizeof(*ret->formats)); + if (!ret->formats) { + av_free(ret); + return NULL; + } + ret->nb_formats = count; + + for (i = 0, desc = NULL; i < count; i++) { + desc = av_pix_fmt_desc_next(desc); + ret->formats[i] = av_pix_fmt_desc_get_id(desc); } } 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++; + + count = 0; + while (av_get_sample_fmt_name(fmt)) + count++; + + ret->formats = av_malloc_array(count, sizeof(*ret->formats)); + if (!ret->formats) { + av_free(ret); + return NULL; } + ret->nb_formats = count; + + for (fmt = 0; fmt < count; fmt++) + ret->formats[fmt] = fmt; + } else { + av_freep(&ret); } return ret;
From: Zhao Zhili <zhilizhao@tencent.com> This is a micro-optimization. Saving almost 200 reallocations makes it worth a try. --- libavfilter/formats.c | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-)