[FFmpeg-devel] [PATCH] avcodec/aac_tablegen: speed up table initialization

Ganesh Ajjanagadde gajjanag at mit.edu
Fri Nov 27 13:48:41 CET 2015


On Fri, Nov 27, 2015 at 7:05 AM, wm4 <nfxjfg at googlemail.com> wrote:
> On Fri, 27 Nov 2015 06:42:21 -0500
> Ganesh Ajjanagadde <gajjanagadde at gmail.com> wrote:
>
>> On Fri, Nov 27, 2015 at 5:35 AM, Rostislav Pehlivanov
>> <atomnuker at 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 at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list