[FFmpeg-devel] [PATCH] Make the scale filter only declare the supported formats

Stefano Sabatini stefano.sabatini-lala
Tue Jan 5 01:44:18 CET 2010


On date Tuesday 2010-01-05 00:18:50 +0100, Michael Niedermayer encoded:
> On Mon, Jan 04, 2010 at 01:54:24AM +0100, Stefano Sabatini wrote:
> > Hi, as in subject.
> > 
> > Depends on the already posted patch:
> > "Make lsws expose supported formats".
> > 
> > Regards.
> > -- 
> > FFmpeg = Freak & Frenzy Meaningful Problematic Exuberant Gadget
> 
> >  vf_scale.c |   36 +++++++++++++++++++++++++++++++-----
> >  1 file changed, 31 insertions(+), 5 deletions(-)
> > 0394116cc822d6289f45e4e8cef28d7aa9dc6d43  make-scale-filter-declare-managed-fmts.patch
> > Index: libavfilter-soc/ffmpeg/libavfilter/vf_scale.c
> > ===================================================================
> > --- libavfilter-soc.orig/ffmpeg/libavfilter/vf_scale.c	2010-01-04 01:32:21.000000000 +0100
> > +++ libavfilter-soc/ffmpeg/libavfilter/vf_scale.c	2010-01-04 01:50:42.000000000 +0100
> > @@ -68,18 +68,44 @@
> >  
> >  static int query_formats(AVFilterContext *ctx)
> >  {
> > -    AVFilterFormats *formats;
> > +    AVFilterFormats *in_formats  = NULL;
> > +    AVFilterFormats *out_formats = NULL;
> > +    enum PixelFormat pix_fmt;
> > +    int ret;
> >  
> 
> >      if (ctx->inputs[0]) {
> 
> why do we need this check?

I wonder the same.

query_format() is supposed to be called when all the links have been
connected to the filter, in this case the check shouldn't be necessary
otherwise the application behavior is supposed to be undefined (->
crash), anyway this needs to be documented better.

> > -        formats = avfilter_all_colorspaces();
> > -        avfilter_formats_ref(formats, &ctx->inputs[0]->out_formats);
> 
> 
> > +        if (!(in_formats  = av_mallocz(sizeof(AVFilterFormats)))) {
> > +            ret = AVERROR(ENOMEM);
> > +            goto fail;
> > +        }
> > +
> > +        for (pix_fmt = 0; pix_fmt < PIX_FMT_NB; pix_fmt++)
> > +            if (sws_isSupportedInput(pix_fmt))
> > +                if ((ret = avfilter_add_colorspace(in_formats, pix_fmt)) < 0)
> > +                    goto fail;
> 
> this design is inconvenient, the first avfilter_add_colorspace() could
> allocate in_formats. Well heres your ** back :)

Yes I had the very same idea and was already working on a patch for
that :).

> > +        avfilter_formats_ref(in_formats, &ctx->inputs[0]->out_formats);
> >      }
> >      if (ctx->outputs[0]) {
> > -        formats = avfilter_all_colorspaces();
> > -        avfilter_formats_ref(formats, &ctx->outputs[0]->in_formats);
> > +        if (!(out_formats  = av_mallocz(sizeof(AVFilterFormats)))) {
> > +            ret = AVERROR(ENOMEM);
> > +            goto fail;
> > +        }
> > +
> > +        for (pix_fmt = 0; pix_fmt < PIX_FMT_NB; pix_fmt++)
> > +            if (sws_isSupportedOutput(pix_fmt))
> > +                if ((ret = avfilter_add_colorspace(out_formats, pix_fmt)) < 0)
> > +                    goto fail;
> > +        avfilter_formats_ref(out_formats, &ctx->outputs[0]->in_formats);
> >      }
> >  
> >      return 0;
> > +
> > +fail:
> 
> > +    if (in_formats)
> > +        avfilter_formats_unref(&in_formats);
> > +    if (out_formats)
> > +        avfilter_formats_unref(&out_formats);
> 
> do we need this freeing here?
> it seems more logic to let the common calling code do the cleanup

Mhh no strong opinion but I find slightly more readable to have common
cleanup code being the line count almost the same.

Check attached patches.

BTW, many functions in formats.c miss av_malloc checks, should we add
them?

Regards.
-- 
FFmpeg = Free and Forgiving Magnificient Patchable Extensive Goblin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: change-add-colorspace.patch
Type: text/x-diff
Size: 2590 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100105/508d9fa2/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: make-scale-filter-declare-managed-fmts.patch
Type: text/x-diff
Size: 1657 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100105/508d9fa2/attachment-0001.patch>



More information about the ffmpeg-devel mailing list