[FFmpeg-devel] [PATCH] Move cbrt tables to a separate cbrt_data(_fixed).c files.

[Reimar Döffinger]

On Sun, Mar 13, 2016 at 12:24:25PM -0400, Ganesh Ajjanagadde wrote:
> On Sat, Mar 12, 2016 at 1:24 PM, Reimar Döffinger
> <Reimar.Doeffinger at gmx.de> wrote:
> >          for (i = 0; i < 1<<13; i++)
> > -            cbrt_tab[i] = CBRT(cbrt_tab_dbl[i]);
> > +            AAC_RENAME(ff_cbrt_tab)[i] = CBRT(cbrt_tab_dbl[i]);
> >      }
> >  }
> 
> Note that cbrt_tab_dbl is really intended to be shared by both the
> fixed/floating table inits. This was another thing my patch achieved:
> only doing the more expensive double table init once across
> float/fixed, and then doing the cheap conversion to uint32_t via
> av_float2int or lrint(x*8192). Please change; it could go into a
> separate patch if you prefer.

IMHO you really need to argue why that would be desirable.
The only case I can see this as useful is if someone
runs at the same time fixed-point AAC decode and AAC encode,
where it saves a bit of startup time.
In all other cases you waste time and memory initializing
a table that will never be used.
Also doing that you end up with 3 different things:
one that should only be compiled in when there is fixed-point
stuff, one you should only have for float, and one that is
needed if either is enabled.
As a result, you'd probably need a 3rd file (or some #ifdef
mess that duplicates stuff in the Makefile).
Though I admit once you have a single init function it becomes
easier to put it all in one file. It still will be quite ugly
ifdefs though.
That you tried to cram three (mostly unrelated) changes in one
patch definitely explains a good part of the problems.