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 EF69044719 for ; Wed, 21 Sep 2022 16:48:16 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C447668BAE8; Wed, 21 Sep 2022 19:48:13 +0300 (EEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 36AE568BA5B for ; Wed, 21 Sep 2022 19:48:07 +0300 (EEST) Received: (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id 7FE0A240006 for ; Wed, 21 Sep 2022 16:48:06 +0000 (UTC) Date: Wed, 21 Sep 2022 18:48:05 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220921164805.GH6583@pb2> References: <20220921091954.GC6583@pb2> MIME-Version: 1.0 In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH 1/4] avcodec/snow: Move ff_snow_inner_add_yblock() to snow_dwt.c 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: multipart/mixed; boundary="===============6584986249652560089==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============6584986249652560089== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="MP5ln1Rcf9Bvi+ZW" Content-Disposition: inline --MP5ln1Rcf9Bvi+ZW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Sep 21, 2022 at 01:00:07PM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > On Mon, Sep 19, 2022 at 11:27:49PM +0200, Andreas Rheinhardt wrote: > >> Only used there and by x86 snow asm code as fallback. > >> > >> Signed-off-by: Andreas Rheinhardt > >> --- > >> libavcodec/snow.c | 33 --------------------------------- > >> libavcodec/snow.h | 3 --- > >> libavcodec/snow_dwt.c | 32 ++++++++++++++++++++++++++++++++ > >> libavcodec/snow_dwt.h | 3 +++ > >> 4 files changed, 35 insertions(+), 36 deletions(-) > >> > >> diff --git a/libavcodec/snow.c b/libavcodec/snow.c > >> index aa15fccc42..85ad6d10a1 100644 > >> --- a/libavcodec/snow.c > >> +++ b/libavcodec/snow.c > >> @@ -29,39 +29,6 @@ > >> #include "snowdata.h" > >> =20 > >> =20 > >> -void ff_snow_inner_add_yblock(const uint8_t *obmc, const int obmc_str= ide, uint8_t * * block, int b_w, int b_h, > >> - int src_x, int src_y, int src_stride, s= lice_buffer * sb, int add, uint8_t * dst8){ > >> - int y, x; > >> - IDWTELEM * dst; > >> - for(y=3D0; y >> - //FIXME ugly misuse of obmc_stride > >> - const uint8_t *obmc1=3D obmc + y*obmc_stride; > >> - const uint8_t *obmc2=3D obmc1+ (obmc_stride>>1); > >> - const uint8_t *obmc3=3D obmc1+ obmc_stride*(obmc_stride>>1); > >> - const uint8_t *obmc4=3D obmc3+ (obmc_stride>>1); > >> - dst =3D slice_buffer_get_line(sb, src_y + y); > >> - for(x=3D0; x >> - int v=3D obmc1[x] * block[3][x + y*src_stride] > >> - +obmc2[x] * block[2][x + y*src_stride] > >> - +obmc3[x] * block[1][x + y*src_stride] > >> - +obmc4[x] * block[0][x + y*src_stride]; > >> - > >> - v <<=3D 8 - LOG2_OBMC_MAX; > >> - if(FRAC_BITS !=3D 8){ > >> - v >>=3D 8 - FRAC_BITS; > >> - } > >> - if(add){ > >> - v +=3D dst[x + src_x]; > >> - v =3D (v + (1<<(FRAC_BITS-1))) >> FRAC_BITS; > >> - if(v&(~255)) v=3D ~(v>>31); > >> - dst8[x + y*src_stride] =3D v; > >> - }else{ > >> - dst[x + src_x] -=3D v; > >> - } > >> - } > >> - } > >> -} > >> - > >> int ff_snow_get_buffer(SnowContext *s, AVFrame *frame) > >> { > >> int ret, i; > >> diff --git a/libavcodec/snow.h b/libavcodec/snow.h > >> index ed0f9abb42..1c976b9ba7 100644 > >> --- a/libavcodec/snow.h > >> +++ b/libavcodec/snow.h > >> @@ -45,11 +45,8 @@ > >> #define QSHIFT 5 > >> #define QROOT (1< >> #define LOSSLESS_QLOG -128 > >> -#define FRAC_BITS 4 > >> #define MAX_REF_FRAMES 8 > >> =20 > >> -#define LOG2_OBMC_MAX 8 > >> -#define OBMC_MAX (1<<(LOG2_OBMC_MAX)) > >> typedef struct BlockNode{ > >> int16_t mx; ///< Motion vector component X, see m= v_scale > >> int16_t my; ///< Motion vector component Y, see m= v_scale > >> diff --git a/libavcodec/snow_dwt.c b/libavcodec/snow_dwt.c > >> index 18b315ef66..9401d119d0 100644 > >> --- a/libavcodec/snow_dwt.c > >> +++ b/libavcodec/snow_dwt.c > >> @@ -25,6 +25,38 @@ > >> #include "me_cmp.h" > >> #include "snow_dwt.h" > >> =20 > >> +void ff_snow_inner_add_yblock(const uint8_t *obmc, const int obmc_str= ide, > >> + uint8_t **block, int b_w, int b_h, > >> + int src_x, int src_y, int src_stride, > >> + slice_buffer * sb, int add, uint8_t * d= st8) > >> +{ > >> + for (int y =3D 0; y < b_h; y++) { > >> + //FIXME ugly misuse of obmc_stride > >> + const uint8_t *obmc1 =3D obmc + y * obmc_stride; > >> + const uint8_t *obmc2 =3D obmc1 + (obmc_stride >> 1); > >> + const uint8_t *obmc3 =3D obmc1 + obmc_stride * (obmc_stride >= > 1); > >> + const uint8_t *obmc4 =3D obmc3 + (obmc_stride >> 1); > >> + IDWTELEM *dst =3D slice_buffer_get_line(sb, src_y + y); > >> + for (int x =3D 0; x < b_w; x++) { > >> + int v =3D obmc1[x] * block[3][x + y*src_stride] > >> + + obmc2[x] * block[2][x + y*src_stride] > >> + + obmc3[x] * block[1][x + y*src_stride] > >> + + obmc4[x] * block[0][x + y*src_stride]; > >> + > >> + v <<=3D 8 - LOG2_OBMC_MAX; > >> + if (FRAC_BITS !=3D 8) > >> + v >>=3D 8 - FRAC_BITS; > >> + if (add) { > >> + v +=3D dst[x + src_x]; > >> + v =3D (v + (1 << (FRAC_BITS - 1))) >> FRAC_BITS; > >> + if (v & (~255)) v=3D ~(v>>31); > >> + dst8[x + y*src_stride] =3D v; > >> + } else > >> + dst[x + src_x] -=3D v; > >> + } > >> + } > >> +} > >> + > >=20 > > putting this in snow_dwt may be convenient but it is not part of the dw= t so > > this feels semantically wrong > >=20 >=20 > If it is not part of the dwt, then why does SnowDWTContext have an > inner_add_yblock function pointer whose C version is > ff_snow_inner_add_yblock? Iam not sure how this relates to what is part of a DWT you can check the definition of a DWT and then look at this function its not a DWT, its not part of some optimized DWT implementation either what it is, is part of snows OBMC+DWT related code. The callback is a optimization of that, for the purpose of optimization this is mixed together like that. a git grep inner_add_yblock origin also shows mostly snowdsp matches not snowdwt is it really a problem to leave dsp code in files with dsp in t the name ? if so iam not against making an exception here thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Nations do behave wisely once they have exhausted all other alternatives.= =20 -- Abba Eban --MP5ln1Rcf9Bvi+ZW Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYytAQQAKCRBhHseHBAsP q/4lAJ9vlFtfWUj/Qhicp65bK398GFutxwCeNwK5cyWB8oMYRCteHac7M+glSns= =VPph -----END PGP SIGNATURE----- --MP5ln1Rcf9Bvi+ZW-- --===============6584986249652560089== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============6584986249652560089==--