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 546CC44398 for ; Wed, 9 Nov 2022 12:42:33 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 19DA968BB84; Wed, 9 Nov 2022 14:42:31 +0200 (EET) Received: from nef.ens.fr (nef2.ens.fr [129.199.96.40]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6116068B01C for ; Wed, 9 Nov 2022 14:42:25 +0200 (EET) X-ENS-nef-client: 129.199.129.80 ( name = phare.normalesup.org ) Received: from phare.normalesup.org (phare.normalesup.org [129.199.129.80]) by nef.ens.fr (8.14.4/1.01.28121999) with ESMTP id 2A9CgOPu031042 for ; Wed, 9 Nov 2022 13:42:24 +0100 Received: by phare.normalesup.org (Postfix, from userid 1001) id 7983CEB5BF; Wed, 9 Nov 2022 13:42:24 +0100 (CET) Date: Wed, 9 Nov 2022 13:42:24 +0100 From: Nicolas George To: FFmpeg development discussions and patches Message-ID: References: <166791645128.20155.3271115273476016820@lain.khirnov.net> <166792556582.1198.13952919773053108092@lain.khirnov.net> <166798627583.1198.13595963875436534964@lain.khirnov.net> <166798905198.1198.15926476440641965970@lain.khirnov.net> <166799481927.1198.11220834405656083251@lain.khirnov.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <166799481927.1198.11220834405656083251@lain.khirnov.net> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (nef.ens.fr [129.199.96.32]); Wed, 09 Nov 2022 13:42:24 +0100 (CET) Subject: Re: [FFmpeg-devel] [PATCH 3/5] lavf: replace FFERROR_REDO with AVERROR(EAGAIN) 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: 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: Anton Khirnov (12022-11-09): > I have no idea what you mean by this. What is being broken and how? EAGAIN and REDO are different. If you make them the same, you break one of them. Or possibly both if you are being really stupid, but at least one. > Maybe more context will clarify the motivation here. What became this > patchset started as an attempt to make all uses of EAGAIN/REDO in > lavf/lavd consistent. And as I explained, it is not possible. > While doing that I gradually came to the > conclusion that the distinction is arbitrary - e.g. multiple demuxers > deliberately return EAGAIN as context switch method. Now we could > declare this practice invalid, but as this has been done in multiple > demuxers independently, and has been in the tree for over 10 years, so > this should not be done lightly. I introduced REDO precisely to fix demuxers who abused EAGAIN like that. > The question then is - what should be the effective rule for deciding > whether a demuxer returns EAGAIN or REDO in any given case? > Opinions on this are very much welcome. I do not have an opinion, I have knowledge. The semantic of EAGAIN has always been, since it stabilized, that the operation cannot be completed immediately, it requires an external event, like a network packet or an IRQ on a device. Therefore, the only correct reaction to EAGAIN is to delay retrying. Ideally delay until an event is received; if that cannot be achieved a small arbitrary delay is still infinitely better than busy wait. This is standard Unix system and network programming, knowing this in and out is IMO an absolute prerequisite for tinkering with FFmpeg's network code. The semantic of AVERROR(EAGAIN) is ours to decide, but it would be really really stupid from us to decide for something that is not aligned with the wider semantic of EAGAIN. And aligning with EAGAIN is what we did, with the caveat that most of it only works with devices and elementary protocols. Except for the bugs at hand. Now, as you observed, some demuxers have abused AVERROR(EAGAIN) to let the framework re-call them and continue their work, for example looking for a resync marker. This is an abuse, this is invalid. If AVERROR(EAGAIN) is handled correctly, using it to look for a resync marker causes a delay to be inserted. The delay is very small, negligible in most cases, but it should not be there. And if the resync marker is hard to find, the delay will add up. If I remember correctly, this is precisely the issue AVERROR_REDO fixes: a damaged sample that would be very slow to demux without consuming CPU because of the delays introduced by the correct handling of AVERROR(EAGAIN). There was another way to fix the issue: have the demuxers loop themselves looking for a resync. But it would have required more reworking of the code. Last detail: we do not have the API to return to the user in the middle of the search for a resync marker. If this is something you want for some reason, you will need to introduce something new. But if so, please first explain your use case. -- Nicolas George _______________________________________________ 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".