[FFmpeg-devel] [RFC] Channel layouts

[Michael Niedermayer]

On Sun, Oct 26, 2008 at 12:06:52PM +1100, Peter Ross wrote:
> On Sat, Oct 25, 2008 at 11:32:46AM +0200, Michael Niedermayer wrote:
> > On Sat, Oct 25, 2008 at 03:34:16PM +1100, Peter Ross wrote:
> > > On Tue, Sep 23, 2008 at 10:43:22PM +1000, Peter Ross wrote:
> > > > On Sun, Sep 07, 2008 at 08:58:28PM +1000, Peter Ross wrote:
> > > > > On Sat, Aug 30, 2008 at 11:05:43AM +1000, Peter Ross wrote:
> > > > > > On Fri, Aug 29, 2008 at 04:28:00PM +1000, Peter Ross wrote:
> > > > > > > Hi.
> > > > > > > 
> > > > > > > This patch adds the notion of channel layouts to libavcodec.
> > > > > > 
> > > > > > Patch updated. Thanks for the feedback.
> > > > > 
> > > > > Patch updated.
> > > > 
> > > > Patch updated.
> > > 
> > > Patch updated. (See the parent post for the RIFF/WAV patch.)
> 
> Patch updated.
> 
> > [...]
> > > +        snprintf(buf + strlen(buf), buf_size - strlen(buf), ", ");
> > 
> > av_strlcat()
> 
> Okay.
> 
> Note that libavcodec/utils.c could be further optimised with
> av_strlcat/strlcatf.  I'll think about doing that later.
> 
> -- Peter
> (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

> Index: libavcodec/utils.c
> ===================================================================
> --- libavcodec/utils.c	(revision 15697)
> +++ libavcodec/utils.c	(working copy)
> @@ -741,6 +741,7 @@
>  {"reservoir", "use bit reservoir", 0, FF_OPT_TYPE_CONST, CODEC_FLAG2_BIT_RESERVOIR, INT_MIN, INT_MAX, A|E, "flags2"},
>  {"bits_per_raw_sample", NULL, OFFSET(bits_per_raw_sample), FF_OPT_TYPE_INT, DEFAULT, INT_MIN, INT_MAX},
>  {NULL},
> +{"channel_layout", NULL, OFFSET(channel_layout), FF_OPT_TYPE_FLAGS, DEFAULT, 0, INT_MAX, A|E|D, "channel_layout"},
>  };
>  
>  #undef A

is it intended to be after the NULL?
and FF_OPT_TYPE_FLAGS means int not int64
and request_channel_layout is missing


[...]
> Index: libavcodec/audioconvert.c
> ===================================================================
> --- libavcodec/audioconvert.c	(revision 15697)
> +++ libavcodec/audioconvert.c	(working copy)
> @@ -27,6 +27,7 @@
>  
>  #include "avcodec.h"
>  #include "audioconvert.h"
> +#include <libavutil/avstring.h>
>  
>  typedef struct SampleFmtInfo {
>      const char *name;
> @@ -70,6 +71,100 @@
>      }
>  }
>  
> +static const char* const channel_names[]={
> +    "front-left",
> +    "front-right",
> +    "front-center",
> +    "low-frequency",
> +    "back-left",
> +    "back-right",
> +    "front-left-of-center",
> +    "front-right-of-center",
> +    "back-center",
> +    "side-left",
> +    "side-right",
> +    "top-center",
> +    "top-front-left",
> +    "top-front-center",
> +    "top-front-right",
> +    "top-back-left",
> +    "top-back-center",
> +    "top-back-right",
> +    [29] = "stereo-left",
> +    [30] = "stereo-right",
> +};

i think these become too long when ore than a single channel is printed
as they are in this patch.


[...]
> +    snprintf(buf, buf_size, "%d channels", nb_channels);
> +    if (channel_layout) {
> +        int i,ch;
> +        av_strlcat(buf, "(", buf_size);
> +        for(i=0,ch=0; i<64 && ch<nb_channels; i++) { 
> +            if ((channel_layout & (1<<i))) {
> +                const char *name = get_channel_name(i);
> +                if (name) {
> +                    if (ch>0) av_strlcat(buf, "|", buf_size);
> +                    av_strlcat(buf, name, buf_size);
> +                }
> +                ch++;
> +            }
> +        }

is the ch<nb_channels check needed? and there is trailing whitespace


[...]
> @@ -2202,8 +2237,11 @@
>       * Decoder should decode to this many channels if it can (0 for default)
>       * - encoding: unused
>       * - decoding: Set by user.
> +     * @deprecated Deprecated in favor of request_channel_layout.
>       */
> +#if LIBAVCODEC_VERSION_MAJOR < 53
>      int request_channels;
> +#endif

i think the if should also be over the corresponding doxy


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- 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/20081026/7c8bb3ea/attachment.pgp>