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

[Stefano Sabatini]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: implement-make-format-list2.patch
Type: text/x-diff
Size: 1754 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091018/e0659215/attachment.patch>