Re: [FFmpeg-devel] [PATCH] lavc/aarch64/fdct: add neon-optimized fdct for aarch64
Hi, On Wed, Feb 14, 2024 at 10:42 AM Martin Storsjö wrote: > On Sun, 4 Feb 2024, Ramiro Polla wrote: > > > The code is imported from libjpeg-turbo-3.0.1. The neon registers used > > have been changed to avoid modifying v8-v15. > > --- > > I don't remember if we have any extra routines we need to do if importing > foreign code with a differing license. The license here seems fine in any > case though. I think the license should be ok (based on the "Patches/Committing" section in developer.texi). > This seems to work fine in all my test environments. And thanks for making > sure it doesn't use v8-v15! > > I'm not so familiar with these DSP functions, whether it is norm to add a > new constant like FF_DCT_NEON, but I guess it seems to match the pattern > of the existing code. I don't know either, so I just tried to match the existing code :) > I presume the main case that tests this is "make fate-dct8x8", which > builds and executes libavcodec/tests/dct? How much work would it be to > integrate testing of these routines into checkasm? That way we could rest > assured that the assembly passes all such ABI checks that we do there, > including what registers must not be clobbered. I added checkasm for fdct. It's especially useful to make sure there is no overflow in the DC coefficient. > The assembly uses a different indentation width than the rest of our > assembly. I recently spent some effort on cleaning that up so that our > code is mostly consistent, so I'd prefer not to add new code that deviates > from it. It primarily looks like you'd need to add 4 spaces at the start > of each line. > > I've used a script for mostly automatically reindenting our arm assembly, > you can grab it at https://martin.st/temp/ffmpeg-asm-indent.pl, run it as > "cat file.S | ./ffmpeg-asm-indent.pl > tmp; mv tmp file.S". It's not 100% > accurate, but mostly gets you there, but it's good to manually check it > afterwards as well. I fixed the indentation and tweaked a few more cosmetics in the comments. Thank you for the review and the help on IRC! I'll send v2 shortly. Ramiro ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/aarch64/fdct: add neon-optimized fdct for aarch64
On Wed, 6 Mar 2024, Ramiro Polla wrote: ping Did you miss my response here? https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/321448.html // 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/aarch64/fdct: add neon-optimized fdct for aarch64
ping
On Sun, Feb 4, 2024 at 3:42 PM Ramiro Polla wrote:
>
> The code is imported from libjpeg-turbo-3.0.1. The neon registers used
> have been changed to avoid modifying v8-v15.
> ---
> libavcodec/aarch64/Makefile | 2 +
> libavcodec/aarch64/fdct.h | 26 ++
> libavcodec/aarch64/fdctdsp_init_aarch64.c | 39 +++
> libavcodec/aarch64/fdctdsp_neon.S | 369 ++
> libavcodec/avcodec.h | 1 +
> libavcodec/fdctdsp.c | 4 +-
> libavcodec/fdctdsp.h | 2 +
> libavcodec/options_table.h| 1 +
> libavcodec/tests/aarch64/dct.c| 2 +
> 9 files changed, 445 insertions(+), 1 deletion(-)
> create mode 100644 libavcodec/aarch64/fdct.h
> create mode 100644 libavcodec/aarch64/fdctdsp_init_aarch64.c
> create mode 100644 libavcodec/aarch64/fdctdsp_neon.S
>
> diff --git a/libavcodec/aarch64/Makefile b/libavcodec/aarch64/Makefile
> index beb6a02f5f..eebccbe4a5 100644
> --- a/libavcodec/aarch64/Makefile
> +++ b/libavcodec/aarch64/Makefile
> @@ -1,4 +1,5 @@
> # subsystems
> +OBJS-$(CONFIG_FDCTDSP) += aarch64/fdctdsp_init_aarch64.o
> OBJS-$(CONFIG_FMTCONVERT) += aarch64/fmtconvert_init.o
> OBJS-$(CONFIG_H264CHROMA) += aarch64/h264chroma_init_aarch64.o
> OBJS-$(CONFIG_H264DSP) += aarch64/h264dsp_init_aarch64.o
> @@ -35,6 +36,7 @@ ARMV8-OBJS-$(CONFIG_VIDEODSP) +=
> aarch64/videodsp.o
>
> # subsystems
> NEON-OBJS-$(CONFIG_AAC_DECODER) += aarch64/sbrdsp_neon.o
> +NEON-OBJS-$(CONFIG_FDCTDSP) += aarch64/fdctdsp_neon.o
> NEON-OBJS-$(CONFIG_FMTCONVERT) += aarch64/fmtconvert_neon.o
> NEON-OBJS-$(CONFIG_H264CHROMA) += aarch64/h264cmc_neon.o
> NEON-OBJS-$(CONFIG_H264DSP) += aarch64/h264dsp_neon.o
> \
> diff --git a/libavcodec/aarch64/fdct.h b/libavcodec/aarch64/fdct.h
> new file mode 100644
> index 00..0901b53a83
> --- /dev/null
> +++ b/libavcodec/aarch64/fdct.h
> @@ -0,0 +1,26 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +#ifndef AVCODEC_AARCH64_FDCT_H
> +#define AVCODEC_AARCH64_FDCT_H
> +
> +#include
> +
> +void ff_fdct_neon(int16_t *block);
> +
> +#endif /* AVCODEC_AARCH64_FDCT_H */
> diff --git a/libavcodec/aarch64/fdctdsp_init_aarch64.c
> b/libavcodec/aarch64/fdctdsp_init_aarch64.c
> new file mode 100644
> index 00..59d91bc8fc
> --- /dev/null
> +++ b/libavcodec/aarch64/fdctdsp_init_aarch64.c
> @@ -0,0 +1,39 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +#include "libavutil/attributes.h"
> +#include "libavutil/cpu.h"
> +#include "libavutil/aarch64/cpu.h"
> +#include "libavcodec/avcodec.h"
> +#include "libavcodec/fdctdsp.h"
> +#include "fdct.h"
> +
> +av_cold void ff_fdctdsp_init_aarch64(FDCTDSPContext *c, AVCodecContext
> *avctx,
> + unsigned high_bit_depth)
> +{
> +int cpu_flags = av_get_cpu_flags();
> +
> +if (have_neon(cpu_flags)) {
> +if (!high_bit_depth) {
> +if (avctx->dct_algo == FF_DCT_AUTO ||
> +avctx->dct_algo == FF_DCT_NEON) {
> +c->fdct = ff_fdct_neon;
> +}
> +}
> +}
> +}
> diff --git a/libavcodec/aarch64/fdctdsp_neon.S
> b/libavcodec/aarch64/fdctdsp_neon.S
> new file mode 100644
> index 00..978c8d3002
> --- /dev/null
> +++
Re: [FFmpeg-devel] [PATCH] lavc/aarch64/fdct: add neon-optimized fdct for aarch64
Hi, On Sun, 4 Feb 2024, Ramiro Polla wrote: The code is imported from libjpeg-turbo-3.0.1. The neon registers used have been changed to avoid modifying v8-v15. --- I don't remember if we have any extra routines we need to do if importing foreign code with a differing license. The license here seems fine in any case though. This seems to work fine in all my test environments. And thanks for making sure it doesn't use v8-v15! I'm not so familiar with these DSP functions, whether it is norm to add a new constant like FF_DCT_NEON, but I guess it seems to match the pattern of the existing code. I presume the main case that tests this is "make fate-dct8x8", which builds and executes libavcodec/tests/dct? How much work would it be to integrate testing of these routines into checkasm? That way we could rest assured that the assembly passes all such ABI checks that we do there, including what registers must not be clobbered. The assembly uses a different indentation width than the rest of our assembly. I recently spent some effort on cleaning that up so that our code is mostly consistent, so I'd prefer not to add new code that deviates from it. It primarily looks like you'd need to add 4 spaces at the start of each line. I've used a script for mostly automatically reindenting our arm assembly, you can grab it at https://martin.st/temp/ffmpeg-asm-indent.pl, run it as "cat file.S | ./ffmpeg-asm-indent.pl > tmp; mv tmp file.S". It's not 100% accurate, but mostly gets you there, but it's good to manually check it afterwards as well. // 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/aarch64/fdct: add neon-optimized fdct for aarch64
The code is imported from libjpeg-turbo-3.0.1. The neon registers used
have been changed to avoid modifying v8-v15.
---
libavcodec/aarch64/Makefile | 2 +
libavcodec/aarch64/fdct.h | 26 ++
libavcodec/aarch64/fdctdsp_init_aarch64.c | 39 +++
libavcodec/aarch64/fdctdsp_neon.S | 369 ++
libavcodec/avcodec.h | 1 +
libavcodec/fdctdsp.c | 4 +-
libavcodec/fdctdsp.h | 2 +
libavcodec/options_table.h| 1 +
libavcodec/tests/aarch64/dct.c| 2 +
9 files changed, 445 insertions(+), 1 deletion(-)
create mode 100644 libavcodec/aarch64/fdct.h
create mode 100644 libavcodec/aarch64/fdctdsp_init_aarch64.c
create mode 100644 libavcodec/aarch64/fdctdsp_neon.S
diff --git a/libavcodec/aarch64/Makefile b/libavcodec/aarch64/Makefile
index beb6a02f5f..eebccbe4a5 100644
--- a/libavcodec/aarch64/Makefile
+++ b/libavcodec/aarch64/Makefile
@@ -1,4 +1,5 @@
# subsystems
+OBJS-$(CONFIG_FDCTDSP) += aarch64/fdctdsp_init_aarch64.o
OBJS-$(CONFIG_FMTCONVERT) += aarch64/fmtconvert_init.o
OBJS-$(CONFIG_H264CHROMA) += aarch64/h264chroma_init_aarch64.o
OBJS-$(CONFIG_H264DSP) += aarch64/h264dsp_init_aarch64.o
@@ -35,6 +36,7 @@ ARMV8-OBJS-$(CONFIG_VIDEODSP) += aarch64/videodsp.o
# subsystems
NEON-OBJS-$(CONFIG_AAC_DECODER) += aarch64/sbrdsp_neon.o
+NEON-OBJS-$(CONFIG_FDCTDSP) += aarch64/fdctdsp_neon.o
NEON-OBJS-$(CONFIG_FMTCONVERT) += aarch64/fmtconvert_neon.o
NEON-OBJS-$(CONFIG_H264CHROMA) += aarch64/h264cmc_neon.o
NEON-OBJS-$(CONFIG_H264DSP) += aarch64/h264dsp_neon.o
\
diff --git a/libavcodec/aarch64/fdct.h b/libavcodec/aarch64/fdct.h
new file mode 100644
index 00..0901b53a83
--- /dev/null
+++ b/libavcodec/aarch64/fdct.h
@@ -0,0 +1,26 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVCODEC_AARCH64_FDCT_H
+#define AVCODEC_AARCH64_FDCT_H
+
+#include
+
+void ff_fdct_neon(int16_t *block);
+
+#endif /* AVCODEC_AARCH64_FDCT_H */
diff --git a/libavcodec/aarch64/fdctdsp_init_aarch64.c
b/libavcodec/aarch64/fdctdsp_init_aarch64.c
new file mode 100644
index 00..59d91bc8fc
--- /dev/null
+++ b/libavcodec/aarch64/fdctdsp_init_aarch64.c
@@ -0,0 +1,39 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/attributes.h"
+#include "libavutil/cpu.h"
+#include "libavutil/aarch64/cpu.h"
+#include "libavcodec/avcodec.h"
+#include "libavcodec/fdctdsp.h"
+#include "fdct.h"
+
+av_cold void ff_fdctdsp_init_aarch64(FDCTDSPContext *c, AVCodecContext *avctx,
+ unsigned high_bit_depth)
+{
+int cpu_flags = av_get_cpu_flags();
+
+if (have_neon(cpu_flags)) {
+if (!high_bit_depth) {
+if (avctx->dct_algo == FF_DCT_AUTO ||
+avctx->dct_algo == FF_DCT_NEON) {
+c->fdct = ff_fdct_neon;
+}
+}
+}
+}
diff --git a/libavcodec/aarch64/fdctdsp_neon.S
b/libavcodec/aarch64/fdctdsp_neon.S
new file mode 100644
index 00..978c8d3002
--- /dev/null
+++ b/libavcodec/aarch64/fdctdsp_neon.S
@@ -0,0 +1,369 @@
+/*
+ * Armv8 Neon optimizations for libjpeg-turbo
+ *
+ * Copyright (C) 2009-2011, Nokia Corporation and/or its subsidiary(-ies).
+ * All Rights Reserved.
+ * Author: Siarhei Siamashka
+ * Copyright (C) 2013-2014, Linaro Limited. All Rights
