[FFmpeg-devel] [PATCH 3/5] avcodec/fft_fixed: Hardcode cosine tables to save space

[Lynne]

Jan 7, 2021, 17:05 by andreas.rheinhardt at gmail.com:

> Lynne:
>
>> Jan 7, 2021, 00:13 by andreas.rheinhardt at gmail.com:
>>
>>> The tables that are used take 256B; the code to initialize them uses
>>> 281B here (GCC 9.3, x64, -O3, but in av_cold functions). On top of that,
>>> removing this code also allows to remove the array of AVOnce used to
>>> guard the cosine tables against multiple initializations; this also
>>> removes relocations.
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>> ---
>>>  libavcodec/Makefile       |  2 +-
>>>  libavcodec/fft.h          | 19 +++++++++---------
>>>  libavcodec/fft_fixed.c    | 42 +++++++++++++++++++++++++++++++++++++++
>>>  libavcodec/fft_template.c | 27 +++++++++----------------
>>>  4 files changed, 62 insertions(+), 28 deletions(-)
>>>
>>
>> I'm not a big fan of this one. It saves minor amounts of space, introduces
>> more hard/soft table init splits, and is for code due to be replaced by
>> libavutil/tx anyway.
>>
>
> It actually removes a hard/soft table init split: Before this patch the
> cos tables for the 16-bit fixed-point FFT were sometimes hardcoded and
> sometimes not; now the latter option doesn't exist any more, reducing
> complexity.
> (I actually pondered removing the big #if at the beginning of
> fft_template.c by moving the code for FFT_FIXED_32 to fft_fixed_32.c and
> the code for FFT_FLOAT to fft_float.c.)
>

My opinion still stands for this patch. This does introduce ifdeffery.
And the size gains are marginal at best, 256 bytes aren't worth it,
and can be likely saved in an easier and cleaner way elsewhere.