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

[Ganesh Ajjanagadde]

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.

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

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

> That you tried to cram three (mostly unrelated) changes in one
> patch definitely explains a good part of the problems.

That is a very fair assessment. Remarks dropped, patch tested and LGTM. Thanks.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel