[FFmpeg-devel] [PATCH] AAC Decoder round 4

[Michael Niedermayer]

On Wed, Jul 30, 2008 at 08:55:34PM +0100, Robert Swain wrote:
> 2008/7/30 Michael Niedermayer <michaelni at gmx.at>:
> > On Wed, Jul 30, 2008 at 07:24:44PM +0100, Robert Swain wrote:
> 
> >> Again, I thought you mean line-breaks in the code, not in how the
> >> patch was coming through in the e-mail. Again, see attached
> >> (20080728-1344-refactor_excluded_channels.diff).
> 
> > [...]
> 
> >> Index: aac.c
> >> ===================================================================
> >> --- aac.c     (revision 2918)
> >> +++ aac.c     (working copy)
> >> @@ -279,7 +279,7 @@
> >>      int dyn_rng_sgn[17];                            ///< DRC sign information; 0 - positive, 1 - negative
> >>      int dyn_rng_ctl[17];                            ///< DRC magnitude information
> >>      int exclude_mask[MAX_CHANNELS];                 ///< Channels to be excluded from DRC processing.
> >> -    int additional_excluded_chns[MAX_CHANNELS];     /**< The exclude_mask bits are
> >> +    int additional_excluded_chns[MAX_CHANNELS / 7]; /**< The exclude_mask bits are
> >>                                                          coded in groups of 7 with 1 bit preceeding each group (except the first)
> >>                                                          indicating that 7 more mask bits are coded. */
> >>      int band_incr;                                  ///< Number of DRC bands greater than 1 having DRC info.
> >> @@ -1607,21 +1607,13 @@
> >>   * @return  Returns number of bytes consumed.
> >>   */
> >>  static int decode_drc_channel_exclusions(AACContext * ac, GetBitContext * gb) {
> >> -    int i;
> >> -    int n = 1;
> >> -    int num_excl_chan = 7;
> >> +    int i, n;
> >>
> >> -    for (i = 0; i < 7; i++)
> >> -         ac->che_drc.exclude_mask[i] = get_bits1(gb);
> >> +    for (i=0, n=0; i < MAX_CHANNELS && (((i+1)%7) || (ac->che_drc.additional_excluded_chns[n++] = get_bits1(gb))); i++)
> >> +        ac->che_drc.exclude_mask[i] = get_bits1(gb);
> >
> > i think this is a little obfuscated
> >
> > what i meant was more along the lines of:
> >
> > do{
> >    for (i = 0; i < 7; i++)
> >        ac->che_drc.exclude_mask[i+X] = get_bits1(gb);
> >    X+= 7;
> > }while(get_bits1(gb));
> >
> > (note this likely is not correct, its just to show what i meant)
> 
> See attached. I agree it's much more readable.
> 
> Rob

> Index: aac.c
> ===================================================================
> --- aac.c	(revision 2938)
> +++ aac.c	(working copy)
> @@ -279,7 +279,7 @@
>      int dyn_rng_sgn[17];                            ///< DRC sign information; 0 - positive, 1 - negative
>      int dyn_rng_ctl[17];                            ///< DRC magnitude information
>      int exclude_mask[MAX_CHANNELS];                 ///< Channels to be excluded from DRC processing.
> -    int additional_excluded_chns[MAX_CHANNELS];     /**< The exclude_mask bits are
> +    int additional_excluded_chns[MAX_CHANNELS / 7]; /**< The exclude_mask bits are
>                                                          coded in groups of 7 with 1 bit preceeding each group (except the first)
>                                                          indicating that 7 more mask bits are coded. */
>      int band_incr;                                  ///< Number of DRC bands greater than 1 having DRC info.
> @@ -1616,19 +1616,16 @@
>   */
>  static int decode_drc_channel_exclusions(AACContext * ac, GetBitContext * gb) {
>      int i;
> -    int n = 1;
> -    int num_excl_chan = 7;
> +    int n = 0;
> +    int num_excl_chan = 0;
>  
> -    for (i = 0; i < 7; i++)
> -         ac->che_drc.exclude_mask[i] = get_bits1(gb);
> -
> -    while (n <= MAX_CHANNELS && num_excl_chan < MAX_CHANNELS - 7 && get_bits1(gb)) {
> -        ac->che_drc.additional_excluded_chns[n-1]=1;
> -        for (i = num_excl_chan; i < num_excl_chan+7; i++)
> -            ac->che_drc.exclude_mask[i] = get_bits1(gb);
> +    do {
> +        for (i = 0; i < 7; i++)
> +            ac->che_drc.exclude_mask[num_excl_chan + i] = get_bits1(gb);
>          n++;
>          num_excl_chan += 7;
> -    }
> +    } while (num_excl_chan < MAX_CHANNELS - 7 && (ac->che_drc.additional_excluded_chns[n-1] = get_bits1(gb)));

i wonder if settig ac->che_drc.additional_excluded_chns[n-1] is really
needed. simply setting the array size after the loop might be better
anyway patch ok

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- 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/20080730/55b7383a/attachment.pgp>