[FFmpeg-devel] [RFC] AAC Encoder

[Kostya]

On Sun, Aug 24, 2008 at 05:51:19PM +0200, Michael Niedermayer wrote:
> On Sun, Aug 24, 2008 at 07:33:56AM +0300, Kostya wrote:
> > On Sat, Aug 23, 2008 at 08:39:19PM +0200, Michael Niedermayer wrote:
> > > > the code was
> > > > for(i = 1; i < info->num_windows; i++)
> > > >     put_bits(&s->pb, 1, info->group_len[i]);
> > > > 
> > > > why can it not be
> > > > for(i = 1; i < info->num_windows; i++)
> > > >     put_bits(&s->pb, 1, !!info->group_len[i]);
> > > > 
> > > > ?
> > > > 
> > > > group_len here can be the actual lens
> > > 
> > > ping, this is currently blocking quite a few hunks in your patch.
> > > I wont forget about it, nor will i skip going over all previous reviews
> > > at the end to ensure nothing has been forgotten.
> > 
> > There could be 1-8 window groups (now it's 3 or 4 but I'll implement a
> > better grouping eventually), and the standards says encoder
> > should always write 7 bits indicating whether window n+1 starts a new
> > window group or not (since window 0 is always a start window), so each
> > window group length is represented by a variable number of bits,
> > hence the loop in the form you see in my code.
> > 
> > And I did representing group lengths as in bitstream (if(!group_len[]) continue),
> > you didn't like it either.
> 
> let me try again, my code apparently didnt reflect what i wanted to suggest
> 
> for(i = 1; i < 8; i++)
>     put_bits(&s->pb, 1, info->group_len[i] > 1);

Hmm, an example:
num of groups: 3
group lens   : 3 2 3
grouping bits to write: 1101011 

so you suggest making group_len array sparse too? like:
num of groups: 8 (does not matter anymore)
group lens   : 3 1 1 2 1 3 1 1

> Anyway, let me elaborate on what i am trying to achive with this.
> 
> currently you use 2 variables all over the place like
>         w = 0;
>         for(wg = 0; wg < num_window_groups; wg++){
>             for(i = 0; i < max_sfb; i++)
>                 put_bits(pb, 1, ms_mask[w + i]);
>             w +=group_len[wg]*16;
>         }
> 
> with what i suggest, i think this would just be:
>         for(w = 0; w < C; w += group_len[w])
>             for(i = 0; i < max_sfb; i++)
>                 put_bits(pb, 1, ms_mask[w][i]);
 
changing flat array into two-dimensional would require either adding more
variables to AAC structures or changing decoder as well, so I'm not eager
to do that
 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Asymptotically faster algorithms should always be preferred if you have
> asymptotical amounts of data