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 6EF0D45004 for ; Wed, 22 Mar 2023 08:06:38 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D054C68C19E; Wed, 22 Mar 2023 10:06:35 +0200 (EET) Received: from mail8.parnet.fi (mail8.parnet.fi [77.234.108.134]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id AC44A68C195 for ; Wed, 22 Mar 2023 10:06:29 +0200 (EET) Received: from mail9.parnet.fi (mail9.parnet.fi [77.234.108.21]) by mail8.parnet.fi with ESMTP id 32M86Sw1008977-32M86Sw2008977; Wed, 22 Mar 2023 10:06:28 +0200 Received: from foo.martin.st (host-97-187.parnet.fi [77.234.97.187]) by mail9.parnet.fi (Postfix) with ESMTPS id EF206A1407; Wed, 22 Mar 2023 10:06:28 +0200 (EET) Date: Wed, 22 Mar 2023 10:06:27 +0200 (EET) From: =?ISO-8859-15?Q?Martin_Storsj=F6?= To: FFmpeg development discussions and patches In-Reply-To: Message-ID: References: MIME-Version: 1.0 X-FE-Policy-ID: 3:14:2:SYSTEM Subject: Re: [FFmpeg-devel] [PATCH] avcodec/aarch64/hevc: add transform_luma_4x4_neon note: run_count=1000, CPU=Cortex A53 transform_4x4_luma_neon: 45 transform_4x4_luma_c: 103 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 Cc: xufuji456 <839789740@qq.com> Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Sat, 11 Mar 2023, xufuji456 wrote: > --- > libavcodec/aarch64/hevcdsp_idct_neon.S | 52 ++++++++++++++++++++++- > libavcodec/aarch64/hevcdsp_init_aarch64.c | 2 + > 2 files changed, 53 insertions(+), 1 deletion(-) Overall the code seems fine, but there's a couple more minor issues: - There's no checkasm test, so we don't have any continuous tracking that this function doesn't break ABI details subtly. From reading the code it seems fine, but we really want to have checkasm coverage for all new assembly. Can you please add one? It should hopefully not be very complicated given the existing tests for other idct parts. - The commit message is a bit garbled - not all that text belongs in the subject line. - It was hard to get the patch applied: > diff --git a/libavcodec/aarch64/hevcdsp_idct_neon.S b/libavcodec/aarch64/hevcdsp_idct_neon.S > index b11f56862a..00d9690466 100644 > --- a/libavcodec/aarch64/hevcdsp_idct_neon.S > +++ b/libavcodec/aarch64/hevcdsp_idct_neon.S > @@ -665,4 +667,52 @@ idct_dc 16, 8 > idct_dc 16, 10 > > idct_dc 32, 8 > -idct_dc 32, 10 > \ No newline at end of file > +idct_dc 32, 10 The file upstream actually did have the proper newline at the end of the file (some earlier patch of yours, I think, were missing this but I fixed it up when pushing it), but the patch expected a file without a trailing newline. It would be much less hassle for me to apply the patch if it was properly rebased on the actually pushed git master version. // 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-request@ffmpeg.org with subject "unsubscribe".