[FFmpeg-devel] [PATCH 2/5] audioconvert: makes av_get_channel_layout accept composite names.

Stefano Sabatini stefasab at gmail.com
Sun Nov 6 12:10:28 CET 2011


On date Sunday 2011-11-06 11:34:09 +0100, Nicolas George encoded:
> Le sextidi 16 brumaire, an CCXX, Stefano Sabatini a écrit :
> > I wonder if this should be something like "2channels", while a bare
> > "2" may be interpreted as the channel layout with code 2.
> 
> I believe that writing 2 for stereo is more practical and obvious than
> writing 2 for front-right. I changed it to this:
> 
>     i = strtol(name, &end, 10);
>     if (end - name == name_len)
>         return av_get_default_channel_layout(i);
>     layout = strtoll(name, &end, 0); 
>     if (end - name == name_len)
>         return layout;
> 
> Thus, "2" (decimal) will mean stereo, while "0x2" (hexadecimal) will mean
> front-right.

I still wonder if 2channels (or "2c", or "2ch" or even "2 channels"
e.g. like in "2 channels+downmix) would make for more self-explicative
descriptions. This would also make the string returned by
av_get_channel_layout_string() usable as input to
av_get_channel_layout().

But this can be extended later (with no break syntax) if you don't
like/want to implement this.

> > Nit: please meaningful names, e.g. cl and cl1 (cl-single will do)
> 
> Done: layout and layout_single.
> 
> > Note: we may want to extend the function to take a log context +
> > offset, and avoid spamming log with unwanted warnings.
> 

> That makes an API change for a very dubious reason: I would rather like to
> drop the warning altogether. What do you think?

I tend to agree.

> 
> >			      Also please document the extension
> 
> AFAIK, there is currently no documentation for the names of the channel
> layouts, and the documentation of the filters is rather inconsistent (some
> mention libavutil/chlayout.h, which does not exist, some point to the source
> code).
> 

> Where do you think this information should go? general.texi seems the
> obvious place, since it can concern any part.
> 
> (No new patch per se as there are still open issues.)

What I mean is that the function doxy should mention the supported
syntax, even if not in a complete way (one sentence referring the
list of accepted channel names in audioconvert.c may do).

For what regards user documentation, maybe we could have a
libavutil.texi file documenting stuff specific to libavutil and which
doesn't have a specific section (e.g. eval stuff, parsing utilities,
etc.). Alternatively a dedicated channel_layout.texi section may do as
well. Having some way to expose the accepted names (e.g. through a
command line list command) would be nice, but missing that a reference
to the source code or even the function used is better than nothing.
 
> Regards,
-- 
FFmpeg = Fanning and Friendly Mystic Philosofic Enhancing Game


More information about the ffmpeg-devel mailing list