[FFmpeg-devel] [PATCH 2/5] avcodec/fft_template: Remove unused fixed-point cosine tables

[Lynne]

Jan 7, 2021, 00:13 by andreas.rheinhardt at gmail.com:

> There are three types of FFTs: floating-point, 32-bit fixed-point and
> 16-bit fixed-point. The latter has exactly one user: The fixed-point
> AC-3-encoder; the cosine tables used by it use up to seven bits. The
> tables corresponding to eight to seventeen bits are unused, as are the
> FFT functions for these bits.
>
> Therefore this commit removes these tables and functions. This is
> especially beneficial when using hardcoded tables as they take up more
> than 255 KiB. But even without it one saves said unused functions as
> well as entries in corresponding tables (this also saves relocations).
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> Thee changes to ARM assembly are honstely untested. I hope someone can
> test them. Btw: It seems that the ARM assembly code wouldn't be able to
> deal with an FFT with more than 16 bits (no function for this has been
> defined), which only worked because no one ever used that many bits with
> the fixed-point FFT.
>
>  libavcodec/arm/fft_fixed_neon.S | 18 ------------------
>  libavcodec/cos_tablegen.c       |  4 ++--
>  libavcodec/fft.h                |  4 +++-
>  libavcodec/fft_fixed.c          |  1 +
>  libavcodec/fft_template.c       | 31 +++++++++++++++++++++++--------
>  tests/fate/fft.mak              |  8 ++++++--
>  6 files changed, 35 insertions(+), 31 deletions(-)
>

The whole comment was hard to make sense of, since you keep mixing
fixed-point FFT precision bits (16 and 32) and FFT length (confusingly also in bits).
I'd rather have a blank comment with just the code or just no references to
the 16-bit fixed-point FFT.

LGTM. Thankfully nothing of the eldritchian fixed-point FFT monstrosity
is exposed to API users, so as long as FATE passes on ARM, should
be okay.