On Sun, Mar 13, 2016 at 12:49 PM, Reimar Döffinger <reimar.doeffin...@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.doeffin...@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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
