[FFmpeg-devel] [PATCH] avcodec/mips/aaccoder_mips: Remove MIPS-specific aaccoder

Lynne dev at lynne.ee
Fri Mar 15 05:41:34 EET 2024


Mar 15, 2024, 02:20 by andreas.rheinhardt at outlook.com:

> ff_aac_coder_init_mips() modifies a static const structure of
> function pointers. This will crash if the binary uses relro
> and is a data race in any case.
>
> Furthermore it points to a maintainability issue: The
> AACCoefficientsEncoder structures have been constified
> in commit fd9212f2edfe9b107c3c08ba2df5fd2cba5ab9e3,
> a Libav commit merged in 318778de9ebec276cb9dfc65509231ca56590d13.
> Libav did not have the MIPS-specific AAC code and so this was
> fine for them; yet FFmpeg had them, but this was not recognized.
>
> Commit 75a099fc734a4ee2b1347d0a3d8c53d883b95174 points to another
> maintainability issue: Contrary to ordinary DSP code, this code
> here is way more complex and needs to be constantly kept in sync
> with the ordinary code which it mimicks and replaces. Said commit
> is the only commit actually changing aaccoder.c in the last few
> years and the same change has not been performed for the MIPS
> clone; before that, it even happened several times that the mips
> code was broken due to changes of the generic code (see commits
> 97437bd17a8c5d4135b2f3b1b299bd7bb72ce02c and
> de262d018d7d7d9c967af1dfd1b861c4b9eb2a60 or
> 860dbe0275e57cbf4228f3f653f872ff66ca596b or
> 933309a6ca0f18bf1d40e917fff455221f57fb4b or
> b65ffa316e377213c29736929beba584d0d80d7c). This might even lead
> to scenarios where someone changing non-dsp aacenc code would
> have to modify mips inline asm in order to keep them in sync.
> This is obviously a significant burden (if the AAC encoder were
> actively developed).
>
> Finally, the code does not even compile here due to errors like
> "Error: float register should be even, was 1".
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
>  libavcodec/aacenc.c             |    4 -
>  libavcodec/aacenc.h             |    1 -
>  libavcodec/mips/Makefile        |    1 -
>  libavcodec/mips/aaccoder_mips.c | 2503 -------------------------------
>  4 files changed, 2509 deletions(-)
>  delete mode 100644 libavcodec/mips/aaccoder_mips.c
>

I didn't even know we had one.
Looks good to me.

Most MIPS code except Loongson is unmaintained
and largely untested, we should consider removing
it in a few years.


More information about the ffmpeg-devel mailing list