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

[Mikhail Nitenko]

On Thu, 7 Dec 2023 at 18:03, Martin Storsjö <martin at martin.st> wrote:
>
> Hi,
>
> On Mon, 4 Dec 2023, Mikhail Nitenko wrote:
>
> > ---
>
> I think the patch subject is missing to tell that this adds 10 bit
> functions?

Yes, you are right. Will you add it when you push it? Or should I
resend the patch?
(I would image it would go like "lavc/aarch64: h264qpel, add 10-bit
lowpass_8_10 based 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.

Thank you.

>
> > 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.

Sure, sounds good to me!

>
> // Martin
>


-- 
Mikhail