[FFmpeg-devel] [PATCH] Channel Layout Negotiation
Michael Niedermayer
michaelni at gmx.at
Sat Jun 18 01:58:23 CEST 2011
On Fri, Jun 17, 2011 at 11:21:04PM +0200, Stefano Sabatini wrote:
> On date Friday 2011-06-17 20:41:05 +0300, Mina Nagy Zaki encoded:
> > 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.
> [...]
>
> No more comments from me, I'll apply both patches in a few days if I
> see no comments from other devs, maybe Michael?
looks fine to me too
thx mina and stefano
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
DNS cache poisoning attacks, popular search engine, Google internet authority
dont be evil, please
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110618/3117aebb/attachment.asc>
More information about the ffmpeg-devel
mailing list