[FFmpeg-devel] [PATCH] AAC Decoder - Round 2.

[Michael Niedermayer]

On Fri, Jun 20, 2008 at 04:16:39PM +0300, Kostya wrote:
> On Fri, Jun 20, 2008 at 01:09:29PM +0200, Michael Niedermayer wrote:
> > On Thu, Jun 19, 2008 at 04:22:57PM +0100, Robert Swain wrote:
> [...]
> > > +
> > > +/**
> > > + * Individual Channel Stream
> > > + */
> > > +typedef struct {
> > 
> > > +    int intensity_present;
> > 
> > This seems to be used to prevent intensity_tool() from being run if its not
> > used anywhere, I do not think this is efficient.
> > instead of
> > 
> > if(a.present)
> >     for(i){
> >         if(tab[i] == a)
> >             do something
> >     }
> > 
> > if(b.present)
> >     for(i){
> >         if(tab[i] == b)
> >             do something
> >     }
> > 
> > the following looks simpler
> > 
> >     for(i){
> >         if(tab[i] == a)
> >             do something
> >         else if((tab[i] == b)
> >             do something
> >     }
> > 
> > or switch/case instead of if/else
> > 
> > Of course above is just a suggestion, i want the fastest and simplest code!
> > But i think quite a bit of these specific "tools" could be merged into a
> > single loop. the do_something itself could of course be a sperate function
> > for clarity ...
>  
> I'm all for leaving it. It's quite useful in encoder.

I dont see how merging some of these tools would negativly affect the
endoder side?


>  
> > > +    int max_sfb;
> > 
> > whats a sfb?
> 
> Scalefactor band. A group of coefficients with a single quantizer.

That makes a very good comment explaining it :)


[...]
> [...]
> > > +    float * saved = sce->saved;
> > 
> > > +    const float * lwindow      = ics->window_shape      ? kbd_long_1024 : sine_long_1024;
> > > +    const float * swindow      = ics->window_shape      ? kbd_short_128 : sine_short_128;
> > > +    const float * lwindow_prev = ics->window_shape_prev ? kbd_long_1024 : sine_long_1024;
> > > +    const float * swindow_prev = ics->window_shape_prev ? kbd_short_128 : sine_short_128;
> > 
> > duplicate 
>  
> no, just two possible cases - 128 and 1024 point windows,
> each can be in one of two possible shapes

i meant these 4 lines are at the top of several functions thus they
are duplicated


[...]
-- 
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/20080620/04ce3b58/attachment.pgp>