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

[Michael Niedermayer]

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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100107/82da96ef/attachment.pgp>