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 53B4543D85 for ; Thu, 8 Sep 2022 20:54:25 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 200BA68BA9B; Thu, 8 Sep 2022 23:54:24 +0300 (EEST) Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 02BBD68B545 for ; Thu, 8 Sep 2022 23:54:17 +0300 (EEST) Received: (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id 46A3A60004 for ; Thu, 8 Sep 2022 20:54:16 +0000 (UTC) Date: Thu, 8 Sep 2022 22:54:16 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220908205416.GD2088045@pb2> References: <20220611145528.GD396728@pb2> <165511144493.13099.15326407212118049404@lain> <165511367625.5088.3673342022457056713@lain.khirnov.net> <20220613191319.GG396728@pb2> <20220902163156.GS2088045@pb2> <20220904214249.GU2088045@pb2> MIME-Version: 1.0 In-Reply-To: <20220904214249.GU2088045@pb2> Subject: Re: [FFmpeg-devel] [PATCH 2/3] avcodec/fmvc: buffer size is stride based not 4*width 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="===============4901040342765824683==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============4901040342765824683== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="XFiqAmqSa4Sjr/mi" Content-Disposition: inline --XFiqAmqSa4Sjr/mi Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Sep 04, 2022 at 11:42:49PM +0200, Michael Niedermayer wrote: > On Fri, Sep 02, 2022 at 06:48:57PM +0200, Paul B Mahol wrote: > > On Fri, Sep 2, 2022 at 6:32 PM Michael Niedermayer > > wrote: > >=20 > > > On Mon, Jun 13, 2022 at 09:13:19PM +0200, Michael Niedermayer wrote: > > > > On Mon, Jun 13, 2022 at 12:10:44PM +0200, Paul B Mahol wrote: > > > > > On Mon, Jun 13, 2022 at 11:48 AM Anton Khirnov > > > wrote: > > > > > > > > > > > Quoting Paul B Mahol (2022-06-13 11:34:44) > > > > > > > On Mon, Jun 13, 2022 at 11:10 AM Anton Khirnov > > > > > > wrote: > > > > > > > > > > > > > > > Quoting Paul B Mahol (2022-06-13 10:04:04) > > > > > > > > > On Sat, Jun 11, 2022 at 4:55 PM Michael Niedermayer < > > > > > > > > michael@niedermayer.cc> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > On Sat, Jun 11, 2022 at 10:47:57AM +0200, Paul B Mahol = wrote: > > > > > > > > > > > Have you actually tested this "change" ? > > > > > > > > > > > > > > > > > > > > On every file i found > > > > > > > > > > 6-methyl-5-hepten-2-one-CC-db_small.avi > > > > > > > > > > fmvcVirtualDub_small.avi > > > > > > > > > > skrzyzowanie4.avi > > > > > > > > > > fmvc-poc.avi > > > > > > > > > > > > > > > > > > > > are there any other files i should test it on ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, the ones where stride !=3D width. > > > > > > > > > > > > > > > > Give examples of such files then. And add more tests. > > > > > > > > > > > > > > > > You really should try to be more helpful if you care about = this > > > code > > > > > > > > working. > > > > > > > > > > > > > > > > > > > > > Code works perfectly from start. There are always attempts to > > > break it. > > > > > > > Your attempts to belittle my work are futile. > > > > > > > > > > > > Perfect code should live in an external repository that is lock= ed > > > > > > against modification. > > > > > > > > > > > > The ffmpeg repository is only for imperfect code that evolves w= ith > > > time, > > > > > > and so requires changes. > > > > > > > > > > > > > > > > > I dunno what Michael attempts to fix. Decoder works fine with val= id > > > files. > > > > > I doubt that encoder would encode random bytes or padding into va= lid > > > file > > > > > bitstream. > > > > > > > > the stride*4 / width*4 change was because of 2 things. > > > > first with AV_PIX_FMT_BGR24 the data stored is not width*4 > > > > > > > > stride is in units of 4 bytes for some reason, so stride*4 > > > > fixes this > > > > The 2nd issue is that the code addresses it by "s->stride * 4" > > > > so the buffer allocation should be stride*4 if we belive the > > > > other code is correct > > > > > > > > src =3D s->buffer; > > > > ... > > > > for (y =3D 0; y < avctx->height; y++) { > > > > ... > > > > src +=3D s->stride * 4; > > > > > > > > width*4 works because its bigger than stride*4 for BGR24 which is w= hat > > > all > > > > samples i have use. > > > > > > > > also > > > > ssrc =3D s->buffer; > > > > ... > > > > for (y =3D 0; y < avctx->height; y++) { > > > > ... > > > > ssrc +=3D s->stride * 4; > > > > and > > > > dst =3D (uint32_t *)s->buffer; > > > > > > > > for (block =3D 0, y =3D 0; y < s->yb; y++) { > > > > int block_h =3D s->blocks[block].h; > > > > uint32_t *rect =3D dst; > > > > > > > > for (x =3D 0; x < s->xb; x++) { > > > > int block_w =3D s->blocks[block].w; > > > > uint32_t *row =3D dst; > > > > > > > > block_h =3D s->blocks[block].h; > > > > if (s->blocks[block].xor) { > > > > for (k =3D 0; k < block_h; k++) { > > > > uint32_t *column =3D dst; > > > > for (l =3D 0; l < block_w; l++) > > > > *dst++ ^=3D *src++; > > > > dst =3D &column[s->stride]; > > > > } > > > > } > > > > dst =3D &row[block_w]; > > > > ++block; > > > > } > > > > dst =3D &rect[block_h * s->stride]; > > > > } > > > > > > > > Again, if you have fmvc files with more odd widths or other pixel f= ormats > > > > these would be very welcome. I can just say the code as is in git is > > > wrong > > > > and the buffer size as is in git is wrong. I noticed this when i ad= ded > > > > a check to see if the buffer is only partly filled and realized its > > > > always partly filled even when the whole image is actually touched > > > > > > If there are no objections aka noone sees a bug in this then id like > > > to apply this > > > > >=20 > > Since when are partially filled buffers are bad thing? >=20 > - waste of memory > - breaks subsequent patch > - width and stride relate this way:=20 > s->stride =3D (avctx->width * avctx->bits_per_coded_sample + 31) / 32; > is width always bigger or equal ? > If not we might be accessing outside the array because access uses > stride, allocation width >=20 > Or one line awnser > Since when is it a good thing to mismatch allocation and access? any objections to this ? i think this should be applied thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who would give up essential Liberty, to purchase a little temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin --XFiqAmqSa4Sjr/mi Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYxpWeAAKCRBhHseHBAsP q2tvAJ9qV1qibdcurzEYDH3jXHLa+7rYVACfe1iH2AtWpLig/+wGFJglsRpBs/8= =qdQc -----END PGP SIGNATURE----- --XFiqAmqSa4Sjr/mi-- --===============4901040342765824683== 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". --===============4901040342765824683==--