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 96AED442A0 for ; Sun, 4 Sep 2022 21:43:01 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DA4D768B88B; Mon, 5 Sep 2022 00:42:57 +0300 (EEST) Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 8CD7A68B291 for ; Mon, 5 Sep 2022 00:42:51 +0300 (EEST) Received: (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id 72294C0002 for ; Sun, 4 Sep 2022 21:42:50 +0000 (UTC) Date: Sun, 4 Sep 2022 23:42:49 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220904214249.GU2088045@pb2> References: <20220611145528.GD396728@pb2> <165511144493.13099.15326407212118049404@lain> <165511367625.5088.3673342022457056713@lain.khirnov.net> <20220613191319.GG396728@pb2> <20220902163156.GS2088045@pb2> MIME-Version: 1.0 In-Reply-To: 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="===============2811561618038338202==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============2811561618038338202== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="8OOZPntID6Q9niAh" Content-Disposition: inline --8OOZPntID6Q9niAh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 wr= ote: > > > > > > > > > > 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 th= is > > 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 locked > > > > > against modification. > > > > > > > > > > The ffmpeg repository is only for imperfect code that evolves with > > time, > > > > > and so requires changes. > > > > > > > > > > > > > > I dunno what Michael attempts to fix. Decoder works fine with valid > > files. > > > > I doubt that encoder would encode random bytes or padding into valid > > 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 what > > 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 for= mats > > > 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 added > > > 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? - 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 Or one line awnser Since when is it a good thing to mismatch allocation and access? thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Nations do behave wisely once they have exhausted all other alternatives.= =20 -- Abba Eban --8OOZPntID6Q9niAh Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYxUb1QAKCRBhHseHBAsP q6d9AJ9xkFdgzhAjXXln5wdkWI3UA55/5gCdGz6BKpxjxs0wV2DVi4LKZURNDuo= =lfV0 -----END PGP SIGNATURE----- --8OOZPntID6Q9niAh-- --===============2811561618038338202== 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". --===============2811561618038338202==--