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

[Kostya]

On Fri, Jun 20, 2008 at 05:08:43PM +0200, Michael Niedermayer wrote:
> 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?
 
Maybe I've misunderstood a bit, but I just defended <struct>.present flags.
Also tools are mostly applied after spectrum coefficients are read but
their data in bitstream occurs before spectrum coefficients.
If that is not what you meant, discard this as noise.
 
> >  
> > > > +    int max_sfb;
> > > 
> > > whats a sfb?
> > 
> > Scalefactor band. A group of coefficients with a single quantizer.
> 
> That makes a very good comment explaining it :)

I think it will be documented either by Robert or by me.

[...]
> -- 
> 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