[FFmpeg-devel] [PACTH] Change av_get_channel_layout() syntax

[Stefano Sabatini]

On date Tuesday 2013-10-15 09:54:15 +0200, Michael Niedermayer encoded:
> On Tue, Oct 15, 2013 at 01:33:19AM +0200, Stefano Sabatini wrote:
> > On date Sunday 2013-10-13 17:57:48 +0200, Michael Niedermayer encoded:
> > > On Fri, Oct 04, 2013 at 05:05:38PM +0200, Stefano Sabatini wrote:
> > [...] 
> > > >  channel_layout.c |   21 ++++++++++++++++++---
> > > >  channel_layout.h |    9 +++++++--
> > > >  internal.h       |    5 +++++
> > > >  version.h        |    3 +++
> > > >  4 files changed, 33 insertions(+), 5 deletions(-)
> > > > 43b5009e71e91a5be2e253a05123d294f888ccfd  0004-lavu-channel_layout-change-av_get_channel_layout-beh.patch
> > > > From 1743fb2c469f71fc2a37a50a15e0bbcd9174c441 Mon Sep 17 00:00:00 2001
> > > > From: Stefano Sabatini <stefasab at gmail.com>
> > > > Date: Fri, 4 Oct 2013 15:20:50 +0200
> > > > Subject: [PATCH] lavu/channel_layout: change av_get_channel_layout() behavior
> > > >  at the next bump
> > > > 
> > > > This is done introducing a compatibility option. The new syntax is
> > > > preferred since it allows backward syntax compatibility with libswr when
> > > > switching to the new option handling code with
> > > > AV_OPT_TYPE_CHANNEL_LAYOUT.
> > > > 
> > > > With the new parser the string:
> > > > 1234
> > > > 
> > > > is interpreted as a channel layout mask, rather than as a number of
> > > > channels, and thus it's compatible with the current way to set a channel
> > > > layout as an integer (for the icl and ocl options) making use of integer
> > > > option values.
> > > > 
> > > > ff_get_channel_layout() with compat=0 will be used in the
> > > > AV_OPT_TYPE_CHANNEL handler code.
> > > > ---
> > > >  libavutil/channel_layout.c | 21 ++++++++++++++++++---
> > > >  libavutil/channel_layout.h |  9 +++++++--
> > > >  libavutil/internal.h       |  5 +++++
> > > >  libavutil/version.h        |  3 +++
> > > >  4 files changed, 33 insertions(+), 5 deletions(-)
> > [...]
> > > > diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
> > > > index 2906098..0f3eb4e 100644
> > > > --- a/libavutil/channel_layout.h
> > > > +++ b/libavutil/channel_layout.h
> > > > @@ -132,11 +132,16 @@ enum AVMatrixEncoding {
> > > >   *   SL, SR, TC, TFL, TFC, TFR, TBL, TBC, TBR, DL, DR);
> > > >   * - a number of channels, in decimal, optionally followed by 'c', yielding
> > > >   *   the default channel layout for that number of channels (@see
> > > > - *   av_get_default_channel_layout);
> > > > + *   av_get_default_channel_layout());
> > > > + *   Note: the use of the following character 'c' to specify a number
> > > > + *   of channels will be required since the next major bump to make
> > > > + *   the syntax not ambiguous.
> > > >   * - a channel layout mask, in hexadecimal starting with "0x" (see the
> > > >   *   AV_CH_* macros).
> > > 
> > 
> > > > + *   Note: since the next major bump a channel layout mask can also be
> > > > + *   specified with a decimal number (iff not followed by "c").
> > > 
> > > maybe leave this "undefined" or deprecated or something
> > > as its rather ugly but iam fine with it as is too
> > 
> > Can't really understand what this sentence is referring to.
> 

> i just meant that we could deprecate the use of decimal numbers for
> channel masks as its somewhat odd and allows typos (lost c letter)
> that is
> 0x123 -> channel mask
> 12c   -> channel count
> 55    -> interpreted as mask for compatibility but deprecated

decimal syntax as channel mask is currently not supported, but it will
be at the next major bump (in order to emulate the way we set channel
layouts in lswr). So I can't see how we can deprecate something which
is not yet supported.
-- 
FFmpeg = Friendly Fostering Meaningful Plastic Exciting Governor