From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id B0379465E6 for ; Mon, 22 May 2023 16:44:26 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D399468BFCB; Mon, 22 May 2023 19:44:24 +0300 (EEST) Received: from w4.tutanota.de (w4.tutanota.de [81.3.6.165]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id F06C668BB52 for ; Mon, 22 May 2023 19:44:17 +0300 (EEST) Received: from tutadb.w10.tutanota.de (unknown [192.168.1.10]) by w4.tutanota.de (Postfix) with ESMTP id AEE861060394 for ; Mon, 22 May 2023 16:44:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1684773857; s=s1; d=lynne.ee; h=From:From:To:To:Subject:Subject:Content-Description:Content-ID:Content-Type:Content-Type:Content-Transfer-Encoding:Content-Transfer-Encoding:Cc:Date:Date:In-Reply-To:In-Reply-To:MIME-Version:MIME-Version:Message-ID:Message-ID:Reply-To:References:References:Sender; bh=9MhoopzD8QmDYayeh087IpKCtRvNrwxJGjYE9ssVOf0=; b=GNMSS01uR21JHUbpn4jm1c3SECuDZvgXZmllE23KULmJj98Y4JkwvuGKktzpI5DM JzGqtXjdRJPwlWKLwK2vIrOSCpzd8jfqRVkDOda9fYPTaNZUF/rMvlCBwtong/nAGWg okyhbFuDnfVOHESe/7EgUXVwgRhDLDfzMxKw8LiZJkFtmNp/FIinGsDGfRqCIXUZD5S zLnXC7rTgB7UVHnNcIh2RKeUd/KyV3IvNVr51DrG5/DJdcShUlHQ3W4cD4iruUzEoYi 6/c1/9ZJSKsP0dWEbpXq5F/z/vyXW+S6yJb35NUuVSYReWACLTON455SvcSlcXWfSqI RQPuQu2Xqw== Date: Mon, 22 May 2023 18:44:17 +0200 (CEST) From: Lynne To: FFmpeg development discussions and patches Message-ID: In-Reply-To: <20230522144840.19483-1-arnie.chang@sifive.com> References: <20230522144840.19483-1-arnie.chang@sifive.com> MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH v3] lavc/h264chroma: RISC-V V add motion compensation for 8x8 chroma blocks X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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 > --- > 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 > + > +#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".