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

[Stefano Sabatini]

On date Thursday 2010-01-07 00:07:50 +0100, Michael Niedermayer encoded:
> On Wed, Jan 06, 2010 at 09:56:30PM +0100, Stefano Sabatini wrote:
> > 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:
> > [...]
> > > > +        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
> > 
> > I agree (I didn't understood what you meant here with "calling code").
> > 
> > Note that currently all the filter formats in case of query_formats
> > failure got freed when calling avfilter_graph_destroy().
> > 
> > Also maybe is also a good idea to extend the documentation of
> > query_formats() like in the attached patch.
> > 
> > Regards.
> > -- 
> > FFmpeg = Fundamentalist and Fantastic Minimalistic Puritan Exxagerate Gem
> 
> >  avfilter.h |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 10a2cd10a15a46fe552a40dd79710f3491d8486f  extend-query-formats-doxy.patch
> > Index: libavfilter-soc/ffmpeg/libavfilter/avfilter.h
> > ===================================================================
> > --- libavfilter-soc.orig/ffmpeg/libavfilter/avfilter.h	2010-01-06 21:13:21.000000000 +0100
> > +++ libavfilter-soc/ffmpeg/libavfilter/avfilter.h	2010-01-06 21:15:26.000000000 +0100
> > @@ -423,6 +423,10 @@
> >       * in_formats for links connected to its output pads, and out_formats
> >       * for links connected to its input pads.
> >       *
> > +     * In case of unsuccess the calling code should free all the
> > +     * in_formats and out_formats eventually allocated by the
> > +     * function.
> > +     *
> >       * @return zero on success, a negative value corresponding to an
> >       * AVERROR code otherwise.
> >       */
> 
> ok, also we maybe should clarify that AVFilter.*() should not be called by
> applications using lavfi but rather use the *() functions which then do
> in this case contain the cleanup in a central place

Please check again the patch, it should be more clear at least in this
case.

Regards.
-- 
FFmpeg = Furious and Fantastic Mastering Problematic Enigmatic Gargoyle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: extend-query-formats-doxy.patch
Type: text/x-diff
Size: 821 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100108/784e384c/attachment.patch>