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

[Mikhail Nitenko <mnitenko@gmail.com>]

On Thu, 7 Dec 2023 at 18:03, Martin Storsjö <martin@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?
>
> > 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.

Oh, and /sligthly/ unrelated, macros names in libavcodec/aarch64/neon.S
use uppercase, should they also be lowercase then or are they fine?

>
> // Martin
>


-- 
Mikhail
_______________________________________________
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".