[FFmpeg-devel] [PATCH] Implement avfilter_make_format_list2(enum PixelFormat pix_fmt, ...)

Michael Niedermayer michaelni
Sun Oct 18 01:13:34 CEST 2009


On Sun, Oct 18, 2009 at 12:00:59AM +0200, Stefano Sabatini wrote:
> On date Saturday 2009-10-17 22:21:49 +0200, Michael Niedermayer encoded:
> > On Sat, Oct 17, 2009 at 01:55:39AM +0200, Stefano Sabatini wrote:
> > > Hi,
> > > 
> > > the interface of AVFilterFormats *avfilter_make_format_list(int len,
> > > ...) is quite brittle, since it requires the exact number of
> > > parameters to be passed, especially with long lists it is quite easy
> > > to get it wrong or get it de-synched for example if a new format is
> > > added to the list.
> > > 
> > > This new function provides a more robust / convenient interface, the
> > > list is terminated by PIX_FMT_NONE and so the user doesn't need to
> > > manually adjust the value of len.
> > > 
> > > No it doesn't look very convenient to factorize the code
> > > avfilter_make_format_list() with avfilter_make_format_list(), anyway
> > > my idea was to deprecate the old interface, or maybe simply to change
> > > the avfilter_make_format_list() interface as the API is declared
> > > non-stable yet.
> > > 
> > > Regards.
> > > -- 
> > > FFmpeg = Friendly & Friendly Monstrous Philosofic Enhancing Geisha
> > 
> > >  avfilter.h |    9 +++++++++
> > >  formats.c  |   27 +++++++++++++++++++++++++++
> > >  2 files changed, 36 insertions(+)
> > > 31802b3c59325cbbdab58c6e275b3e083293b604  lavfi-make-format-list2.patch
> > > Index: ffmpeg/libavfilter/avfilter.h
> > > ===================================================================
> > > --- ffmpeg.orig/libavfilter/avfilter.h	2009-10-17 01:38:34.000000000 +0200
> > > +++ ffmpeg/libavfilter/avfilter.h	2009-10-17 01:39:12.000000000 +0200
> > > @@ -180,6 +180,15 @@
> > >  AVFilterFormats *avfilter_make_format_list(int len, ...);
> > >  
> > >  /**
> > > + * Helper function to create a list of supported formats. This is intended
> > > + * for use in AVFilter->query_formats().
> > > + * @param pix_fmt the first pixel format of the provided list
> > > + * @param ... a list of the supported pixel formats, terminated by PIX_FMT_NONE.
> > > + * @return the format list, with no existing references
> > > + */
> > > +AVFilterFormats *avfilter_make_format_list2(enum PixelFormat pix_fmt, ...);
> > > +
> > > +/**
> > >   * Returns a list of all colorspaces supported by FFmpeg.
> > >   */
> > >  AVFilterFormats *avfilter_all_colorspaces(void);
> > > Index: ffmpeg/libavfilter/formats.c
> > > ===================================================================
> > > --- ffmpeg.orig/libavfilter/formats.c	2009-10-17 01:39:45.000000000 +0200
> > > +++ ffmpeg/libavfilter/formats.c	2009-10-17 01:51:21.000000000 +0200
> > > @@ -87,6 +87,33 @@
> > >      return ret;
> > >  }
> > >  
> > > +AVFilterFormats *avfilter_make_format_list2(enum PixelFormat pix_fmt, ...)
> > 
> > any reason why this is not
> > 
> > AVFilterFormats *avfilter_make_format_list2(const enum PixelFormat *pix_fmt)
> > ?
> > 
> > seems simpler to me, just a simple malloc+memcpy after finding the 0
> 
> Indeed it is, also I have no strong opinion on this, what about the
> new attached one?
> 
> Regards.
> -- 
> FFmpeg = Frenzy & Funny Merciless Powerful Extended Gangster

>  avfilter.h |    8 ++++++++
>  formats.c  |   17 +++++++++++++++++
>  2 files changed, 25 insertions(+)
> 7546b65b947c11febb58ff26fee5223fc4fb9d91  implement-make-format-list2.patch
> Index: ffmpeg-vfilters/ffmpeg/libavfilter/avfilter.h
> ===================================================================
> --- ffmpeg-vfilters.orig/ffmpeg/libavfilter/avfilter.h	2009-10-17 23:46:40.000000000 +0200
> +++ ffmpeg-vfilters/ffmpeg/libavfilter/avfilter.h	2009-10-17 23:58:32.000000000 +0200
> @@ -180,6 +180,14 @@
>  AVFilterFormats *avfilter_make_format_list(int len, ...);
>  
>  /**
> + * Helper function to create a list of supported formats. This is intended
> + * for use in AVFilter->query_formats().
> + * @param pix_fmt list of pixel formats, terminated by PIX_FMT_NONE
> + * @return the format list, with no existing references
> + */
> +AVFilterFormats *avfilter_make_format_list2(enum PixelFormat *pix_fmt);
> +
> +/**
>   * Returns a list of all colorspaces supported by FFmpeg.
>   */
>  AVFilterFormats *avfilter_all_colorspaces(void);
> Index: ffmpeg-vfilters/ffmpeg/libavfilter/formats.c
> ===================================================================
> --- ffmpeg-vfilters.orig/ffmpeg/libavfilter/formats.c	2009-10-17 23:46:40.000000000 +0200
> +++ ffmpeg-vfilters/ffmpeg/libavfilter/formats.c	2009-10-17 23:47:13.000000000 +0200
> @@ -87,6 +87,23 @@
>      return ret;
>  }
>  
> +AVFilterFormats *avfilter_make_format_list2(enum PixelFormat *pix_fmt)
> +{
> +    AVFilterFormats *formats;
> +    int count;

> +    enum PixelFormat *p;
> +
> +    for (count = 0, p = pix_fmt; *p != PIX_FMT_NONE; p++, count++)
> +        ;

too many ; and , on one line (unreadable)
i suggest:

for(i=0; pix_fmt[i]; i++)
    ;



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

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- 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/20091018/772f913e/attachment.pgp>



More information about the ffmpeg-devel mailing list