[FFmpeg-soc] extension of ac3 parser

[Michael Niedermayer]

On Mon, Apr 21, 2008 at 12:49:41AM +0200, Bartlomiej Wolowiec wrote:
> On niedziela, 20 kwietnia 2008, Michael Niedermayer wrote:
> > > Index: libavcodec/ac3_parser.h
> > > ===================================================================
> > > --- libavcodec/ac3_parser.h	(wersja 12910)
> > > +++ libavcodec/ac3_parser.h	(kopia robocza)
> > > @@ -35,14 +35,15 @@
> > >
> > >  /**
> > >   * Parses AC-3 frame header.
> > > - * Parses the header up to the lfeon element, which is the first 52 or
> > > 54 bits - * depending on the audio coding mode.
> > > + * Parses the header up to the lfeon element, which is the first 52, 54,
> > > or 104 bits + * depending on the audio coding mode and read_channel_map.
> > >   * @param buf[in] Array containing the first 7 bytes of the frame.
> >
> >                                                 ^^^^^^^
> >
> > 7 bytes and 104 bits ?
> >
> > >   * @param hdr[out] Pointer to struct where header info is written.
> > >
> > > + * @param read_channel_map[in] Determines whether channel_map is to be
> > > set
> >
> > why is this conditional and not always done?
> >
> > [...]
> 
> Because  e.g. while parsing in buffer there are just 8 bytes, which won't be 
> enough to read full information. According to solution proposed by you, 
> parser firstly reads 7-byte headers, then, when full set of frames is in 
> buffer, it sets number of channels etc.

Well in that case i think it might be better to have these in seperate
functions.

That is we need the following patches (if you agree and theres no issue iam
missing)
* change ff_ac3_parse_header() to take a GetBitContext instead of const char *
* add a ff_ac3_parse_header_full() which calls ff_ac3_parse_header() and then
  reads the channel_map stuff.



[...]
> Index: libavcodec/ac3tab.c
> ===================================================================
> --- libavcodec/ac3tab.c	(wersja 12910)
> +++ libavcodec/ac3tab.c	(kopia robocza)
> @@ -247,3 +247,17 @@
>      1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 3, 3, 3, 3, 3, 3,
>      3, 6, 6, 6, 6, 6, 6, 12, 12, 12, 12, 24, 24, 24, 24, 24
>  };
> +
> +/**
> + * Default channel map for a dependent substream defined by acmod
> + */
> +const uint16_t ff_eac3_default_chmap[8] = {
> +    AC3_CHMAP_L | AC3_CHMAP_R, // FIXME Ch1+Ch2
> +    AC3_CHMAP_C,
> +    AC3_CHMAP_L | AC3_CHMAP_R,
> +    AC3_CHMAP_L | AC3_CHMAP_C | AC3_CHMAP_R,
> +    AC3_CHMAP_L | AC3_CHMAP_R | AC3_CHMAP_C_SUR,
> +    AC3_CHMAP_L | AC3_CHMAP_C | AC3_CHMAP_R     | AC3_CHMAP_C_SUR,
> +    AC3_CHMAP_L | AC3_CHMAP_R | AC3_CHMAP_L_SUR | AC3_CHMAP_R_SUR,
> +    AC3_CHMAP_L | AC3_CHMAP_C | AC3_CHMAP_R     | AC3_CHMAP_L_SUR | AC3_CHMAP_R_SUR

please align equal ones above each other
              AC3_CHMAP_C,
AC3_CHMAP_L |               AC3_CHMAP_R,
AC3_CHMAP_L | AC3_CHMAP_C | AC3_CHMAP_R,


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- 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-soc/attachments/20080421/a6d65afe/attachment.pgp>