On Fri, Nov 27, 2015 at 7:05 AM, wm4 <nfx...@googlemail.com> wrote: > On Fri, 27 Nov 2015 06:42:21 -0500 > Ganesh Ajjanagadde <gajjanaga...@gmail.com> wrote: > >> On Fri, Nov 27, 2015 at 5:35 AM, Rostislav Pehlivanov >> <atomnu...@gmail.com> wrote: >> > LGTM, but could you leave (just comment it out) the old code in there >> > so it's a little easier to follow? >> >> //ff_aac_pow2sf_tab[i] = pow(2, (i - POW_SF2_ZERO) / 4.0); >> >> //ff_aac_pow34sf_tab[i] = pow(ff_aac_pow2sf_tab[i], 3.0/4.0); >> > >> > The accuracy increase is always nice. >> >> Done and pushed. Thanks. >> BTW, do you or others think that the new performance figures are >> sufficient to justify getting rid of config_hardcoded_tables and >> associated ifdefry and C file here? > > I think this would be a very good goal. I never understood why there > has to be both, and the complication caused by it is terrible.
I definitely agree that the complication is terrible, experiencing it first hand with some patches currently under review. I was initially skeptical of the need for both myself, but am now neutral about it, see https://ffmpeg.org/pipermail/ffmpeg-devel/2015-November/183932.html for some links about the rationale for this thing and my own thoughts on it - I am sure a computer systems person can explain it far better. However, for this to be valid, at least some basic tests need to be done, e.g what is the library size before and after --enable-hardcoded-tables? The trouble is that at the moment packagers and other clients (vlc, mpv, chromium?) have generally followed the defaults, so if we move to "hardcoded by default", clients can and perhaps should accuse us of library bloat. I hence personally prefer doing it all at runtime, but it will likely lead to bikeshedding. Maybe we can actually use our new "developer committee" for a poll on this. A simple library size test should be revealing in this respect. > > I'm not sure if dynamic generation was already the default, but in any > case the init call should be protected with ff_thread_once. Dynamic is in the sense that --enable-hardcoded-tables is not done by default. I got the numbers by running ffplay on some test file. As for the ff_thread_once, did not know that. I am not comfortable with threading, but if you send a patch for this, I can understand it better for future. > _______________________________________________ > 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
