May 17, 2023, 09:13 by arnie.ch...@sifive.com:
> Optimize the put and avg filtering for 8x8 chroma blocks
>
> Signed-off-by: Arnie Chang <arnie.ch...@sifive.com>
> ---
> libavcodec/h264chroma.c | 2 +
> libavcodec/h264chroma.h | 1 +
> libavcodec/riscv/Makefile | 3 +
> libavcodec/riscv/h264_chroma_init_riscv.c | 39 ++
> libavcodec/riscv/h264_mc_chroma.S | 492 ++++++++++++++++++++++
> libavcodec/riscv/h264_mc_chroma.h | 34 ++
> 6 files changed, 571 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
>
> +#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
> + const int high_bit_depth = bit_depth > 8;
>
You don't need this constant.
> +
> + if (!high_bit_depth) {
> + 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
>
You have to check if RVV is supported:
> int flags = av_get_cpu_flags();
>
> if (flags & AV_CPU_FLAG_RVV_F32) {
> if (bit_depth > 8) {
> + .text
> +
> + .globl h264_put_chroma_mc8_rvv
> + .p2align 1
> + .type h264_put_chroma_mc8_rvv,@function
> +h264_put_chroma_mc8_rvv:
>
You don't need any of this. We already have macros to
handle this - take a look at libavcodec/riscv/opusdsp_rvv.S:
> func ff_opus_postfilter_rvv_256, zve32f
> lvtypei a5, e32, m1, ta, ma // function instructions start here
Make sure to change zve32f to whatever instruction extension you use
to initialize the assembler to handle it.
> + slliw t2, a5, 3
> + mulw t1, a5, a4
> + sh3add a5, a4, t2
> + slliw a4, a4, 3
> + subw a5, t1, a5
> + subw a7, a4, t1
> + addiw a6, a5, 64
> + subw t0, t2, t1
>
Coding style issue - we style our RISC-V assembly the same way
we style our AArch64 assembly:
<8 spaces><instruction><spaces until the 24th character on the
line><arguments,registers .etc>
For example:
> vsetvl zero, a4, a5
> lw t2, 20(a1)
> vfmul.vv v8, v24, v16
> addi a0, a0, 4
> vslide1down.vx v16, v16, t2
> MACRO arg1, arg2
> +.LBB0_8: # if ((x8 - xy) == 0 && (y8 -xy) !=
> 0)
> + add a5, a1, a4
> + vsetvli zero, zero, e8, m1, ta, ma
> + addiw t1, t1, 4
> + vle8.v v8, (a5)
> + add a5, a5, a2
> + add t2, a5, a2
> + vwmulu.vx v10, v8, a6
>
This branch looks very similar to
> .LBB1_16: # the final else, none of the above
> conditions are met
> add a5, a1, a4
> vsetvli zero, zero, e8, m1, ta, ma
> addiw t0, t0, 4
> vle8.v v8, (a5)
> add a5, a5, a2
> add t1, a5, a2
> vwmulu.vx v10, v8, a6
Consider using a macro.
In fact, a lot of the branches look similar to each other. Looking at other
implementations, they only consider 3 possible variants, the same ones
that the C function has.
> + .size h264_avg_chroma_mc8_rvv, .Lfunc_end1-h264_avg_chroma_mc8_rvv
> diff --git a/libavcodec/riscv/h264_mc_chroma.h
> b/libavcodec/riscv/h264_mc_chroma.h
> new file mode 100644
> index 0000000000..cb350d0e4a
> --- /dev/null
> +++ b/libavcodec/riscv/h264_mc_chroma.h
> @@ -0,0 +1,34 @@
> +/*
> + * 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 <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <stddef.h>
> +#include "config.h"
>
You don't need all of these includes. Just config.h and stdint.h would be
enough.
> +#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);
> +#endif
> +#endif
> \ No newline at end of file
>
You need your file to end in a newline. Git already warns you if you don't.
Finally, run:
make checkasm && ./tests/checkasm/checkasm --bench
and report on the timings for both the C and assembly versions.
If you've made a mistake somewhere, (forgot to restore stack, or a callee-saved
register,
or your function produces an incorrect result), checkasm will fail.
_______________________________________________
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".
