[FFmpeg-devel] [PATCH] lavc/aarch64: h264qpel, add lowpass_8 based functions

[Martin Storsjö]

Hi,

On Mon, 4 Dec 2023, Mikhail Nitenko wrote:

> ---

I think the patch subject is missing to tell that this adds 10 bit 
functions?

> I remodeled the patch (as Martin once suggested), it doesn't
> go to 32bits in lowpass_8_10 and is also using the much
> faster lowpass_8_10_v.
>
> libavcodec/aarch64/h264qpel_init_aarch64.c |  91 +++-
> libavcodec/aarch64/h264qpel_neon.S         | 532 +++++++++++++++++++++
> 2 files changed, 621 insertions(+), 2 deletions(-)

I think the patch overall looks good to me, thanks! I can try to push it a 
bit later.

> diff --git a/libavcodec/aarch64/h264qpel_neon.S b/libavcodec/aarch64/h264qpel_neon.S
> index f4475d96f9..31130a57fd 100644
> --- a/libavcodec/aarch64/h264qpel_neon.S
> +++ b/libavcodec/aarch64/h264qpel_neon.S
> @@ -933,3 +933,535 @@ endfunc
>
>         h264_qpel16     put
>         h264_qpel16     avg
> +
> +//trashes v0-v5
> +.macro  lowpass_8_10    r0,  r1,  r2,  r3,  d0,  d1
> +        ext             v2.16B,     \r0\().16B,  \r1\().16B, #4

This patch uses upper case element specifiers like .16B here; we 
reformatted our assembly to consistenty use lower case element specifiers 
in 184103b3105f02f1189fa0047af4269e027dfbd6 (also see 
7f905f3672da4f1fa28d7cccf1fef7f9984e0480); I can apply this change 
mechanically on this patch before pushing it.

// Martin