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 7447443EA8 for ; Tue, 16 Aug 2022 19:48:50 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9ECB568B944; Tue, 16 Aug 2022 22:48:46 +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 3455068B8F0 for ; Tue, 16 Aug 2022 22:48:40 +0300 (EEST) Received: (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id 0E4BF240005 for ; Tue, 16 Aug 2022 19:48:38 +0000 (UTC) Date: Tue, 16 Aug 2022 21:48:37 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220816194837.GA2088045@pb2> References: MIME-Version: 1.0 In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH 4/6] avcodec/mpegpicture: Don't copy unnecessarily, fix race 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="===============6289230905772839235==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============6289230905772839235== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="J8ZfMsDg90ddfAv1" Content-Disposition: inline --J8ZfMsDg90ddfAv1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Aug 13, 2022 at 05:03:04PM +0200, Andreas Rheinhardt wrote: > mpegvideo uses an array of Pictures and when it is done with using > them, it only unreferences them incompletely: Some buffers are kept > so that they can be reused lateron if the same slot in the Picture > array is reused, making this a sort of a bufferpool. > (Basically, a Picture is considered used if the AVFrame's buf is set.) > Yet given that other pieces of the decoder may have a reference to > these buffers, they need not be writable and are made writable using > av_buffer_make_writable() when preparing a new Picture. This involves > reading the buffer's data, although the old content of the buffer > need not be retained. >=20 > Worse, this read can be racy, because the buffer can be used by another > thread at the same time. This happens for Real Video 3 and 4. >=20 > This commit fixes this race by no longer copying the data; > instead the old buffer is replaced by a new, zero-allocated buffer. >=20 > (Here are the details of what happens with three or more decoding threads > when decoding rv30.rm from the FATE-suite as happens in the rv30 test: > The first decoding thread uses the first slot of its picture array > to store its current pic; update_thread_context copies this for the > second thread that decodes a P-frame. It uses the second slot in its > Picture array to store its P-frame. This arrangement is then copied > to the third decode thread, which decodes a B-frame. It uses the third > slot in its Picture array for its current frame. > update_thread_context copies this to the next thread. It unreferences > the third slot containing the other B-frame and then it reuses this > slot for its current frame. Because the pic array slots are only > incompletely unreferenced, the buffers of the previous B-frame are > still in there and they are not writable; in fact the previous > thread is concurrently writing to them, causing races when making > the buffer writable.) >=20 > Signed-off-by: Andreas Rheinhardt > --- > libavcodec/mpegpicture.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) This causes a difference in some frames of: (i have not investigates why just reporting as i noticed) quite possibly thats just showing your bugfix in action =2E/ffmpeg -y -bitexact -i fate/svq3/Vertical400kbit.sorenson3.mov -ps 50 -= bf 1 -bitexact -an -qscale 5 -ss 40 -error_rate 4 -threads 1 /tmp/out4.avi= =20 =2E/ffmpeg -y -bitexact -v -1 -loglevel 0 -i /tmp/out4.avi -bitexact -vsync= drop -f framecrc - @@ -141,7 +141,7 @@ 0, 22, 22, 1, 115200, 0x4cc933e9 0, 23, 23, 1, 115200, 0x693a40a9 0, 24, 24, 1, 115200, 0x956e3b15 -0, 25, 25, 1, 115200, 0x61763ff4 +0, 25, 25, 1, 115200, 0xc9e53d1c 0, 26, 26, 1, 115200, 0x5c5c3dfc 0, 27, 27, 1, 115200, 0x7de641ea 0, 28, 28, 1, 115200, 0xe6cc4136 @@ -187,7 +187,7 @@ 0, 68, 68, 1, 115200, 0x49dcbf4e 0, 69, 69, 1, 115200, 0x1ea1c7d1 0, 70, 70, 1, 115200, 0xdf77c67b -0, 71, 71, 1, 115200, 0x7f6bd16d +0, 71, 71, 1, 115200, 0x33d9d206 0, 72, 72, 1, 115200, 0x5e37cb3a 0, 73, 73, 1, 115200, 0x15abcda3 0, 74, 74, 1, 115200, 0xbf4dcbd4 @@ -199,7 +199,7 @@ 0, 80, 80, 1, 115200, 0x17d1d667 0, 81, 81, 1, 115200, 0x0c1fdf9c 0, 82, 82, 1, 115200, 0x7eabde6b -0, 83, 83, 1, 115200, 0xe623e7af +0, 83, 83, 1, 115200, 0x3bf6e873 0, 84, 84, 1, 115200, 0xf480dc82 0, 85, 85, 1, 115200, 0x5fd6e098 0, 86, 86, 1, 115200, 0xf520de95 @@ -211,7 +211,7 @@ 0, 92, 92, 1, 115200, 0x34cfe1c2 0, 93, 93, 1, 115200, 0x1d94e1c3 0, 94, 94, 1, 115200, 0x6d32e147 -0, 95, 95, 1, 115200, 0x7e40ee91 +0, 95, 95, 1, 115200, 0x09fbefd0 0, 96, 96, 1, 115200, 0xa5f5eb43 0, 97, 97, 1, 115200, 0x39b9ec3d 0, 98, 98, 1, 115200, 0x3256ec18 [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt. --J8ZfMsDg90ddfAv1 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYvv0kQAKCRBhHseHBAsP q+GGAJ9FYxaTmtPZxkFXS7ZkIM2ZLSAODwCfefxzjIZWBmz8MoiHPxz5hxLDDrg= =y+Cn -----END PGP SIGNATURE----- --J8ZfMsDg90ddfAv1-- --===============6289230905772839235== 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". --===============6289230905772839235==--