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

[Reimar Döffinger]

On Sun, Mar 13, 2016 at 01:12:57PM -0400, Ganesh Ajjanagadde wrote:
> On Sun, Mar 13, 2016 at 12:49 PM, Reimar Döffinger
> <Reimar.Doeffinger at gmx.de> wrote:
> > 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.
> 
> I don't understand the waste; the double init anyway needs to be done,
> all tables are derived from it. This dates to an approach I did in
> commit:  07a11ebcab9b31e9fc784029e5d24e6fbf486ff3.

The waste is in writing to the table that will never be used.
As long as it is left as 0, it will not use memory, as soon
as you touch it it uses up memory.
But see my patch, if people think it's better it can be added.

> I had no fancy ifdefs for this, just some Makefile magic.

Because you also always included both tables in the binary,
whether used or not (and due to the shared init function
the linker could not remove the unused one).
I can see that one could argue either way concerning
ifdef mess vs. a few kB wasted though.