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 62599475C4 for ; Wed, 13 Sep 2023 06:46:58 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D971D68C844; Wed, 13 Sep 2023 09:46:55 +0300 (EEST) Received: from wout1-smtp.messagingengine.com (wout1-smtp.messagingengine.com [64.147.123.24]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B54B968C5B3 for ; Wed, 13 Sep 2023 09:46:49 +0300 (EEST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id 35B9032009F0 for ; Wed, 13 Sep 2023 02:46:46 -0400 (EDT) Received: from imap51 ([10.202.2.101]) by compute3.internal (MEProxy); Wed, 13 Sep 2023 02:46:46 -0400 X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedviedrudeijedguddufecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefofgggkfgjfhffhffvufgtsehttdertderredtnecuhfhrohhmpedfvfhh ohhmrghsucfiuhhilhhlvghmfdcuoehthhhomhgrshesghhllhhmrdhfrheqnecuggftrf grthhtvghrnhepheefgfegheettdefjeegieetheejheegjeegfeehjefhhfelgfeggfeh ieehudevnecuffhomhgrihhnpehffhhmphgvghdrohhrghenucevlhhushhtvghrufhiii gvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesghhllhhmrdhfrh X-ME-Proxy: Feedback-ID: i239440f1:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id 66097B60089; Wed, 13 Sep 2023 02:46:45 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.9.0-alpha0-745-g95dd7bea33-fm-20230905.001-g95dd7bea Mime-Version: 1.0 Message-Id: <5e952ec2-16c4-4141-b1bc-e0c84ccfc869@app.fastmail.com> In-Reply-To: References: <20230912114015.59937-1-thomas@gllm.fr> <20230912114015.59937-3-thomas@gllm.fr> Date: Wed, 13 Sep 2023 08:46:24 +0200 To: =?UTF-8?Q?Andreas_H=C3=A5kon_via_ffmpeg-devel?= Subject: Re: [FFmpeg-devel] [PATCH 3/3] h264: fix data-race with FF_DECODE_ERROR_DECODE_SLICES 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: , From: Thomas Guillem via ffmpeg-devel Reply-To: FFmpeg development discussions and patches Cc: Thomas Guillem Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Tue, Sep 12, 2023, at 15:11, Andreas Rheinhardt wrote: > Thomas Guillem via ffmpeg-devel: >> Same than the previous commit but with FF_DECODE_ERROR_DECODE_SLICES >> >> Fix the following data-race: >> >> WARNING: ThreadSanitizer: data race (pid=55935) >> Write of size 4 at 0x7b5000009378 by thread T1 (mutexes: write M608): >> #0 decode_nal_units src/libavcodec/h264dec.c:742 (ffmpeg+0xb19dd6) >> #1 h264_decode_frame src/libavcodec/h264dec.c:1016 (ffmpeg+0xb19dd6) >> #2 frame_worker_thread src/libavcodec/pthread_frame.c:228 (ffmpeg+0xdeea7e) >> >> Previous read of size 4 at 0x7b5000009378 by thread T14 (mutexes: write M610): >> #0 frame_copy_props src/libavutil/frame.c:321 (ffmpeg+0x1793759) >> #1 av_frame_replace src/libavutil/frame.c:530 (ffmpeg+0x1794714) >> #2 ff_thread_replace_frame src/libavcodec/utils.c:898 (ffmpeg+0xfb1d0f) >> #3 ff_h264_replace_picture src/libavcodec/h264_picture.c:159 (ffmpeg+0x149cd3d) >> #4 ff_h264_update_thread_context src/libavcodec/h264_slice.c:413 (ffmpeg+0x14abf04) >> #5 update_context_from_thread src/libavcodec/pthread_frame.c:355 (ffmpeg+0xdec39c) >> #6 submit_packet src/libavcodec/pthread_frame.c:494 (ffmpeg+0xdecee3) >> #7 ff_thread_decode_frame src/libavcodec/pthread_frame.c:545 (ffmpeg+0xdecee3) >> #8 decode_simple_internal src/libavcodec/decode.c:431 (ffmpeg+0x9e1e20) >> #9 decode_simple_receive_frame src/libavcodec/decode.c:607 (ffmpeg+0x9e1e20) >> #10 decode_receive_frame_internal src/libavcodec/decode.c:635 (ffmpeg+0x9e1e20) >> #11 avcodec_send_packet src/libavcodec/decode.c:732 (ffmpeg+0x9e28fa) >> #12 packet_decode src/fftools/ffmpeg_dec.c:555 (ffmpeg+0x229888) >> #13 decoder_thread src/fftools/ffmpeg_dec.c:702 (ffmpeg+0x229888) >> --- >> libavcodec/h264dec.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c >> index 24e849fc5b..b82ca8f14f 100644 >> --- a/libavcodec/h264dec.c >> +++ b/libavcodec/h264dec.c >> @@ -739,7 +739,7 @@ static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size) >> >> // set decode_error_flags to allow users to detect concealed decoding errors >> if ((ret < 0 || h->er.error_occurred) && h->cur_pic_ptr) { >> - h->cur_pic_ptr->f->decode_error_flags |= FF_DECODE_ERROR_DECODE_SLICES; >> + h->cur_pic_ptr->decode_error_flags |= FF_DECODE_ERROR_DECODE_SLICES; >> } >> >> ret = 0; > > IIRC this does not work: The thread that decodes a frame is typically > not the same thread that outputs said frame. The H264Picture srcp in > output_frame() points to one of the H264Picture in the H264Context.DBP > of the outputting thread, not the decoding thread. The outputting > threads decode_error_flags will therefore always be zero. > > My preferred way to fix this is to allocate the H264Pictures (or at > least the stuff needed by later decoding threads) separately and make > them shared between decoder threads (with the caveat that only the > actual decoding threads may modify them; and they may only do so in a > controlled manner (i.e. no changes after having signalled to finish the > picture etc.). But this would be a major rewrite which will probably > never happen. > > In the meantime i will send an alternative patch for this. Thanks for your patches and your clear explanations ! > > - Andreas > > _______________________________________________ > 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". _______________________________________________ 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".