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 C0B964305C for ; Sat, 18 Jun 2022 16:30:56 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 599B668B5B7; Sat, 18 Jun 2022 19:30:53 +0300 (EEST) Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9515A68B137 for ; Sat, 18 Jun 2022 19:30:46 +0300 (EEST) Received: from localhost (213-47-68-29.cable.dynamic.surfer.at [213.47.68.29]) (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id ADF4F1C0006 for ; Sat, 18 Jun 2022 16:30:45 +0000 (UTC) Date: Sat, 18 Jun 2022 18:30:44 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220618163044.GZ396728@pb2> References: <20220208202353.19554-1-michael@niedermayer.cc> <83406d0-fb76-b7ba-c19a-194f6bc6a69e@passwd.hu> MIME-Version: 1.0 In-Reply-To: <83406d0-fb76-b7ba-c19a-194f6bc6a69e@passwd.hu> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avformat/demux: Make read_frame_internal() return AVERREOR(EAGAIN) on stuck empty input parser 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="===============3280996960854927829==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============3280996960854927829== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="mT/EkForZZEIFsY4" Content-Disposition: inline --mT/EkForZZEIFsY4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jun 17, 2022 at 09:53:13PM +0200, Marton Balint wrote: >=20 >=20 > On Tue, 8 Feb 2022, Michael Niedermayer wrote: >=20 > > Fixes: read_frame_internal() which does not return even though both dem= uxer and parser do return > > Fixes: 43717/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-52= 06008287330304 > >=20 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz= /tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer > > --- > > libavformat/demux.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > >=20 > > diff --git a/libavformat/demux.c b/libavformat/demux.c > > index ec34b65288..dd42d32710 100644 > > --- a/libavformat/demux.c > > +++ b/libavformat/demux.c > > @@ -1222,11 +1222,15 @@ static int read_frame_internal(AVFormatContext = *s, AVPacket *pkt) > > FFFormatContext *const si =3D ffformatcontext(s); > > int ret, got_packet =3D 0; > > AVDictionary *metadata =3D NULL; > > + int empty =3D 0; > >=20 > > while (!got_packet && !si->parse_queue.head) { > > AVStream *st; > > FFStream *sti; > >=20 > > + if (empty > 1) > > + return AVERROR(EAGAIN); > > + > > /* read next packet */ > > ret =3D ff_read_packet(s, pkt); > > if (ret < 0) { > > @@ -1317,6 +1321,8 @@ static int read_frame_internal(AVFormatContext *s= , AVPacket *pkt) > > } > > got_packet =3D 1; > > } else if (st->discard < AVDISCARD_ALL) { > > + if (pkt->size =3D=3D 0) > > + empty ++; > > if ((ret =3D parse_packet(s, pkt, pkt->stream_index, 0)) < = 0) > > return ret; > > st->codecpar->sample_rate =3D sti->avctx->sample_rate; > > --=20 > > 2.17.1 >=20 > Sorry, just noticed this patchset, and it is very hackish. yes thats why i waited so many month before i applied it some patchsets i forget but this i kept pushing away >=20 > For starters the meaning of AVERROR(EAGAIN) for > av_read_frame()/read_frame_internal() is not very well defined. Should the > user retry immediately? Should the user sleep() sometime and the retry? C= an > the user expect that a loop of av_read_frame() will eventually return > something different than AVERROR(EAGAIN)? In the context of this problem the idea is to give the user an opertunity to do something else in a single threaded environment or error out if its taking too long not produingh anything >=20 > I am not sure I understand the original issue entirely, but it looks that > instead of fixing the component which returns infinite amount of zero siz= ed > packets you implemented a check that makes the user get an EAGAIN() on the > second zero-sized packet. IIRC the problem was that the demuxer produced 0 sized packets and alot of = them for a file only a few hundread bytes large. now the zero sized packets go into the parser which has no output because theres no bytes input so no packet is ever produced by the parser and nothi= ng comes out of the read_frame_internal() code, its just stuck in a loop =46rom the point of view of the parser, the parser seems behaving correct =46rom the point of view of the demuxer, the demuxer seems behaving correct if for example it produces a billion empty packets but from the point of view of the user the code is stuck and hangs >=20 > And this helps the user how? Instead of a busy CPU loop in the library he > either gets a busy CPU loop in its application or a non-busy CPU loop in = its > application (if he sleeps() on EAGAIN). The advantage for the user is, she can cleanly error out instead of killing the thread / process >=20 > Is there a file which causes this? Can't the underlying components be fix= ed > instead? i have 2 files it seems, i will mail them to you privatly As said, its not clear if this is fundamentally any component misbehaving maybe something does break some rule somewhere i dont know thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Let us carefully observe those good qualities wherein our enemies excel us and endeavor to excel them, by avoiding what is faulty, and imitating what is excellent in them. -- Plutarch --mT/EkForZZEIFsY4 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYq39sAAKCRBhHseHBAsP q0VhAJ9epFQCGBCNwvA6eZASYTwc/8/XXwCfRMBcphw2sZZnMgF94Cc0KX7KuH4= =CCJ2 -----END PGP SIGNATURE----- --mT/EkForZZEIFsY4-- --===============3280996960854927829== 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". --===============3280996960854927829==--