Re: [FFmpeg-devel] [PATCH v3 0/5] avcodec/ac3: Add aarch64 NEON DSP

["Martin Storsjö" <martin@martin.st>]

On Tue, 2 Apr 2024, Geoff Hill wrote:

> Here's v3 to push the AC-3 ARMv8 NEON experiment a step further.
>
> This version implements 5 of the AC-3 encoder DSP functions,
> and adds checkasm tests where missing.
>
> I've tested that the checkasm tests pass on aarch64 and x86.

Thanks, I've tested that checkasm also passes on 32 bit arm (where we also 
do have an ac3dsp implementation).

Overall the patches look mostly fine.

Are these implementations based on the existing 32 bit arm ones? The code 
is quite similar (although there's not very many different ways to 
implement things, so this could be a coincidence)? If based on the 
existing code, it would be good to retain the copyright statement from 
that file.

These functions have a different indentation than the rest of 
essentially all our aarch64 assembly (the code you're adding is aligned in 
two different ways) - please check other files (e.g. vp8dsp_neon.S) for 
example. The instructions should be aligned to 8 leading spaces, and 
operands to 24 leading characters.

Other than those generic points, I have two comments on the patches 
themselves.

// Martin

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".