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

[Hendrik Leppkes]

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.

- Hendrik