Re: [FFmpeg-devel] [PATCH v3] lavc/h264chroma: RISC-V V add motion compensation for 8x8 chroma blocks

[Lynne <dev@lynne.ee>]

May 22, 2023, 16:48 by arnie.chang@sifive.com:

> Optimize the put and avg filtering for 8x8 chroma blocks
>
> Signed-off-by: Arnie Chang <arnie.chang@sifive.com>
> ---
> V3:
> 1. Use a macro to extract repetitive segments
> 2. Fix coding style issues
> 3. Use macros in riscv/asm.S to handle function declarations
> 4. Replace vslidedown with vslide1down
> checkasm: using random seed 2379273251
> RVVi32:
>  - h264dsp.chroma_mc         [OK]
> checkasm: all 2 tests passed
> avg_h264_chroma_mc1_8_c: 1821.5
> avg_h264_chroma_mc1_8_rvv_i32: 482.5
> put_h264_chroma_mc1_8_c: 1436.5
> put_h264_chroma_mc1_8_rvv_i32: 390.5
>

Pretty nice. You'd struggle to get this speedup with NEON.
Though, it's still only an FPGA.
The checkasm patch was merged with a better and more extensive form,
you should check again that it passes, just in case.


>  libavcodec/h264chroma.c                   |   2 +
>  libavcodec/h264chroma.h                   |   1 +
>  libavcodec/riscv/Makefile                 |   2 +
>  libavcodec/riscv/h264_chroma_init_riscv.c |  40 +++
>  libavcodec/riscv/h264_mc_chroma.S         | 306 ++++++++++++++++++++++
>  libavcodec/riscv/h264_mc_chroma.h         |  30 +++
>  6 files changed, 381 insertions(+)
>  create mode 100644 libavcodec/riscv/h264_chroma_init_riscv.c
>  create mode 100644 libavcodec/riscv/h264_mc_chroma.S
>  create mode 100644 libavcodec/riscv/h264_mc_chroma.h
>
> diff --git a/libavcodec/h264chroma.c b/libavcodec/h264chroma.c
> index 60b86b6fba..1eeab7bc40 100644
> --- a/libavcodec/h264chroma.c
> +++ b/libavcodec/h264chroma.c
> @@ -58,5 +58,7 @@ av_cold void ff_h264chroma_init(H264ChromaContext *c, int bit_depth)
>  ff_h264chroma_init_mips(c, bit_depth);
>  #elif ARCH_LOONGARCH64
>  ff_h264chroma_init_loongarch(c, bit_depth);
> +#elif ARCH_RISCV
> +    ff_h264chroma_init_riscv(c, bit_depth);
>  #endif
>  }
> diff --git a/libavcodec/h264chroma.h b/libavcodec/h264chroma.h
> index b8f9c8f4fc..9c81c18a76 100644
> --- a/libavcodec/h264chroma.h
> +++ b/libavcodec/h264chroma.h
> @@ -37,5 +37,6 @@ void ff_h264chroma_init_ppc(H264ChromaContext *c, int bit_depth);
>  void ff_h264chroma_init_x86(H264ChromaContext *c, int bit_depth);
>  void ff_h264chroma_init_mips(H264ChromaContext *c, int bit_depth);
>  void ff_h264chroma_init_loongarch(H264ChromaContext *c, int bit_depth);
> +void ff_h264chroma_init_riscv(H264ChromaContext *c, int bit_depth);
>  
>  #endif /* AVCODEC_H264CHROMA_H */
> diff --git a/libavcodec/riscv/Makefile b/libavcodec/riscv/Makefile
> index 965942f4df..ee17a521fd 100644
> --- a/libavcodec/riscv/Makefile
> +++ b/libavcodec/riscv/Makefile
> @@ -10,6 +10,8 @@ OBJS-$(CONFIG_BSWAPDSP) += riscv/bswapdsp_init.o \
>  RVV-OBJS-$(CONFIG_BSWAPDSP) += riscv/bswapdsp_rvv.o
>  OBJS-$(CONFIG_FMTCONVERT) += riscv/fmtconvert_init.o
>  RVV-OBJS-$(CONFIG_FMTCONVERT) += riscv/fmtconvert_rvv.o
> +OBJS-$(CONFIG_H264CHROMA) += riscv/h264_chroma_init_riscv.o
> +RVV-OBJS-$(CONFIG_H264CHROMA) += riscv/h264_mc_chroma.o
>  OBJS-$(CONFIG_IDCTDSP) += riscv/idctdsp_init.o
>  RVV-OBJS-$(CONFIG_IDCTDSP) += riscv/idctdsp_rvv.o
>  OBJS-$(CONFIG_OPUS_DECODER) += riscv/opusdsp_init.o
> diff --git a/libavcodec/riscv/h264_chroma_init_riscv.c b/libavcodec/riscv/h264_chroma_init_riscv.c
> new file mode 100644
> index 0000000000..2e47f1365e
> --- /dev/null
> +++ b/libavcodec/riscv/h264_chroma_init_riscv.c
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright (c) 2023 SiFive, Inc. All rights reserved.
> + *
> + * 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 <stdint.h>
> +
> +#include "libavutil/attributes.h"
> +#include "libavutil/cpu.h"
> +#include "libavcodec/h264chroma.h"
> +#include "config.h"
> +#include "h264_mc_chroma.h"
> +
> +av_cold void ff_h264chroma_init_riscv(H264ChromaContext *c, int bit_depth)
> +{
> +#if HAVE_RVV
> +    int flags = av_get_cpu_flags();
> +
> +    if (bit_depth == 8 && (flags & AV_CPU_FLAG_RVV_I32)) {
> +        c->put_h264_chroma_pixels_tab[0] = h264_put_chroma_mc8_rvv;
> +        c->avg_h264_chroma_pixels_tab[0] = h264_avg_chroma_mc8_rvv;
> +    }
> +#endif
> +}
> index 0000000000..027f2ee053
> --- /dev/null
> +++ b/libavcodec/riscv/h264_mc_chroma.h
> @@ -0,0 +1,30 @@
> +/*
> + * Copyright (c) 2023 SiFive, Inc. All rights reserved.
> + *
> + * 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_RISCV_H264_MC_CHROMA_H
> +#define AVCODEC_RISCV_H264_MC_CHROMA_H
> +#include "config.h"
> +
> +#if HAVE_RVV
> +void h264_put_chroma_mc8_rvv(uint8_t *p_dst, const uint8_t *p_src, ptrdiff_t stride, int h, int x, int y);
> +void h264_avg_chroma_mc8_rvv(uint8_t *p_dst, const uint8_t *p_src, ptrdiff_t stride, int h, int x, int y);
>

You should remove the entire h264_mc_chroma.h file, and instead just
put the function definitions at the top of libavcodec/riscv/h264_chroma_init_riscv.c
It's how everything else does this.
With that change, the non-asm portion of the patch looks good.
No comment on the assembly.
_______________________________________________
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".