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

Lynne dev at lynne.ee
Thu Jan 7 23:47:55 EET 2021


Jan 7, 2021, 21:26 by h.leppkes at gmail.com:

> On Thu, Jan 7, 2021 at 5:32 PM Lynne <dev at lynne.ee> wrote:
>
>>
>> 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.
>>
>
> The patch seems to remove more ifdeffery then it adds, at worst it
> moves a bit of it due to changing the previous hardcoded behavior.
>

To an extent.
I'm still objecting to hardcoding those tables though.


More information about the ffmpeg-devel mailing list