[FFmpeg-devel] [PATCH] Channel Layout Negotiation

[Mina Nagy Zaki]

On Friday 17 June 2011 18:45:36 Stefano Sabatini wrote:
> On date Friday 2011-06-17 18:07:39 +0300, MNZ encoded:
> > Another iteration over this.
> > 
> > This time an avfilter_make_format64_list() was added and the original was
> > retained. Also instead of avfilter_set_common_formats() there are now separate
> > functions for pixel_formats, sample_formats and channel_layouts. The names are a
> > bit too long though.
> > 
> > --
> > Mina
> 
> > From 8de1ad310758db13fea3e2f5fc093b52de178c75 Mon Sep 17 00:00:00 2001
> > From: Mina Nagy Zaki <mnzaki at gmail.com>
> > Date: Tue, 7 Jun 2011 21:17:23 +0300
> > Subject: [PATCH 1/2] lavfi: Use int64_t lists in AVFilteFormats.
> > 
> > The list type was changed to int64_t to be able to hold
> > channel layouts.
> > 
> > avfilter_make_format_list() still takes a int32_t array and converts
> > it to int64_t. A new function, avfilter_make_format64_list, that
> > takes int64_t arrays has been added.
> > ---
> >  libavfilter/avfilter.h |    5 +++--
> >  libavfilter/formats.c  |   36 ++++++++++++++++++++++++------------
> >  2 files changed, 27 insertions(+), 14 deletions(-)
> > 
> > diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> > index 7628cd5..9651d86 100644
> > --- a/libavfilter/avfilter.h
> > +++ b/libavfilter/avfilter.h
> > @@ -223,7 +223,7 @@ void avfilter_unref_buffer(AVFilterBufferRef *ref);
> >   */
> >  typedef struct AVFilterFormats {
> >      unsigned format_count;      ///< number of formats
> > -    int *formats;               ///< list of media formats
> > +    int64_t *formats;           ///< list of media formats
> >  
> >      unsigned refcount;          ///< number of references to this list
> >      struct AVFilterFormats ***refs; ///< references to this list
> > @@ -238,6 +238,7 @@ typedef struct AVFilterFormats {
> >   * @return the format list, with no existing references
> >   */
> >  AVFilterFormats *avfilter_make_format_list(const int *fmts);
> > +AVFilterFormats *avfilter_make_format64_list(const int64_t *fmts);
> >  
> >  /**
> >   * Add fmt to the list of media formats contained in *avff.
> > @@ -247,7 +248,7 @@ AVFilterFormats *avfilter_make_format_list(const int *fmts);
> >   * @return a non negative value in case of success, or a negative
> >   * value corresponding to an AVERROR code in case of error
> >   */
> > -int avfilter_add_format(AVFilterFormats **avff, int fmt);
> > +int avfilter_add_format(AVFilterFormats **avff, int64_t fmt);
> >  
> >  /**
> >   * Return a list of all formats supported by FFmpeg for the given media type.
> > diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> > index 58593fc..40254a0 100644
> > --- a/libavfilter/formats.c
> > +++ b/libavfilter/formats.c
> > @@ -72,28 +72,40 @@ AVFilterFormats *avfilter_merge_formats(AVFilterFormats *a, AVFilterFormats *b)
> >      return ret;
> >  }
> >  
> 
> > +#define MAKE_FORMAT_LIST()                                              \
> > +    AVFilterFormats *formats;                                           \
> > +    int count = 0;                                                      \
> > +do {                                                                    \
> > +    if (fmts)                                                           \
> > +        for (count = 0; fmts[count] != -1; count++)                     \
> > +            ;                                                           \
> > +    formats = av_mallocz(sizeof(AVFilterFormats));                      \
> > +    formats->format_count = count;                                      \
> > +    if (count)                                                          \
> > +        formats->formats  = av_malloc(sizeof(*formats->formats)*count); \
> > +} while (0)
> 
> Missing NULL check => return NULL

Added NULL checks.

> Alternatively, you could create a function
> static AVFilterFormats *make_format_list(int count);
> which returns NULL in case of failure.
> 

I think the macro is more fitting since a static function leads to unnecessary
duplication of code. Also using a macro highlights the main difference between
the two functions: the list copying code.

I have also removed the do..while(0) wrapper  since it is useless in this case,
as pointed out by ubitux on IRC. 

Other nits taken care of.

-- 
Mina
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavfi-Use-int64_t-lists-in-AVFilteFormats.patch
Type: text/x-patch
Size: 4349 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110617/de04a2e1/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-lavfi-add-layout-negotiation-fields-and-helper-funct.patch
Type: text/x-patch
Size: 20550 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110617/de04a2e1/attachment-0001.bin>