From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 4FBF44C558 for ; Sun, 3 Aug 2025 21:32:46 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 4E13A68C154; Mon, 4 Aug 2025 00:32:42 +0300 (EEST) Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id 952B668B249 for ; Mon, 4 Aug 2025 00:32:35 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 04B5841C7B for ; Sun, 3 Aug 2025 21:32:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1754256754; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=l+JNsJFQ+f3monINS1B+zUXyt3wOjhnsyW70dmZBkLE=; b=IGqrZOceZiJc5liqEg3eEYkeptgZGp1aovNNS5JJaszABemWsCx7Cp0IOF6HAF8s8fzBD0 r6T27KpOlSIvI6U+orJvlOGRxH4F6z34V5SiRIuD2Z/3T5aE/j0tsa84oZrTBvL/5DwGhC 1ENsSdXozMNgKjhOWwfdSNqrvujJUdDYz4RqfFKk/dKDROO0VR3KxsprUQSowFOcvCi5Ab a2POZfNdYExGsvU50tvHGMkczv3DuD4lawfcL8WczqDUowFMa3p+GV0uw8Tv3lMhQsIEOh P23rQWthCvZzyvEnkaWQDPLtZP/KLZ7oPzw7a1brmw64BjVoNWiw2s6xhbk7eA== Date: Sun, 3 Aug 2025 23:32:32 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20250803213232.GH29660@pb2> References: <20250702144355.GE29660@pb2> <20250702171641.GF29660@pb2> <20250703155205.GK29660@pb2> <20250706143354.GA29660@pb2> MIME-Version: 1.0 In-Reply-To: X-GND-State: clean X-GND-Score: -70 X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdefgdduuddtheegucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuifetpfffkfdpucggtfgfnhhsuhgsshgtrhhisggvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenfghrlhcuvffnffculdeftddmnecujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefoihgthhgrvghlucfpihgvuggvrhhmrgihvghruceomhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtqeenucggtffrrghtthgvrhhnpeeigeektdejudffjefhteegjedtgeettefggedthfejgfevhfetgeekjedtvdfhveenucfkphepgedurdeiiedrieehrddujeeinecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepgedurdeiiedrieehrddujeeipdhhvghloheplhhotggrlhhhohhsthdpmhgrihhlfhhrohhmpehmihgthhgrvghlsehnihgvuggvrhhmrgihvghrrdgttgdpnhgspghrtghpthhtohepuddprhgtphhtthhopehffhhmphgvghdquggvvhgvlhesfhhfmhhpvghgrdhorhhg X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [RFC] Advanced Error Codes 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="===============2518664720774235858==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============2518664720774235858== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="FGj0qTPk+4CI8WgJ" Content-Disposition: inline --FGj0qTPk+4CI8WgJ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi On Tue, Jul 29, 2025 at 12:01:50AM +0200, Nicolas George wrote: > Michael Niedermayer (HE12025-07-06): > > Hi Nicolas >=20 > Hi. Sorry I procrastinated to reply to that too. >=20 > > theres a fixed length array of AdvancedError structs. >=20 > More global state is not a good idea. >=20 [...] > > * The contexts do not match the lifetime of the errors >=20 > How so? The typical code looks like: >=20 > 42 ret =3D av_do_something(ctx, params); > 43 if (ret < 0) { > 44 report_error(ctx, ret); > 45 return some_code; > 46 } >=20 > ctx does not disappear between line 42 and line 44. well, you may have deep call stacks user_app->libavfilter->libavformat->libavcodec->decoder->jpeg_decoder->... and the user of the user_app needs to know what file had what failure so the error details must pass through all this. Some of the API calls failing here can be final calls that are expected to leave nothing allocated on a failure return. also other subsequent and prior errors may have occured in some contexts I thiink we want to be able to distingish what caused teh current error from the previous or next [...] > > * There can be multiple relevant errors for a single context >=20 > Again: how so? If multiple errors appear in succession, the proper way > to deal with it is to report the first error as it has happened. Or > possibly copy it. Then processing continues and the next error happens, > it can reuse the storage. >=20 > Or do you mean cascading errors: error A caused error B caused error C? it can be both lets say we have 4 slices that can be independantly decoded. We try to decode the first and hit an error. Now we continue trying the 2nd and it too has an error, and so on. only once we tried all can we detrmine that the whole frame is a loss but now we have 4 independant error= s. We need to report these 4 errors together as the cause of the frame failing to decode. None of them has "higher" priority than the others. We would have been able to decode the frame somewhat had one remained. so its a failure to decode file because failure to decode the only keyframe which failed because all 4 slices had errors (error1, error2, error3, erro= r4) further you could wrap this in a failure to stream this because the input failed to decode and so on ... of course there has to be some limit on how much we can represent > Then sure, we must handle that, that was also my plan. We can include > room for a few errors in the context, not just one. Or better, design > the structure to be able to store a few cascading errors all using the > same buffer: that way, if the first error is terse, the second error has > more room. >=20 > > "square root of negative argument" in mydecoder.c at line 123 is more > > informative than EINVAL, and having no clue where or why that happened > >=20 > > for example decode() returning > > AVERROR(EINVAL) > > with 5000 lines of cryptic parser errors in messages > >=20 > > vs. > >=20 > > "square root of negative argument" in mydecoder.c at line 123, called by > > vector_length_compute() line 511 called by > > read_nnet() line 732 free_form_description=3D"training_set_maybe_corrup= ted.dat" >=20 > I will stop you right here: the first one is terrible, but yours is > terrible too. >=20 > Errors messages are for users. What you suggest is not an error message, > it is a debug message meant for developers. >=20 > In particular, the error message presented to users should not by > default include source file names and lines, because it confuses users. > (We might want to enable them in the fftools, because the fftools are > meant for power users. But remember that this is also for GUI > applications.) people copy and paste error messages into bugreports and such, developer details can be helpfull in fixing bugs [...] > > Note by the time this is returned decode failed and many contexts have > > been destroyed. >=20 > You mean: >=20 > 42 ret =3D av_do_something(ctx, params); > 43 if (ret < 0) { > 44 av_free_this_context(&ctx); > 45 return some_code; > 46 } >=20 > ? Well, it should be: >=20 > 42 ret =3D av_do_something(ctx, params); > 43 if (ret < 0) { > 44 av_forward_error(caller_ctx, ctx); > 45 av_free_this_context(&ctx); > 46 return some_code; > 47 } >=20 > I.e.: when freeing a context on error, the error messages must be > forwarded to the caller. I see what you suggest. The problem is simply that we likely dont have the manpower to implement such explicit forwarding of errors throughout the codebase My suggestion doesnt need it, but yes globals are ugly, yes >=20 > > > - We allocate the memory for error messages contexts. That way, we av= oid > > > the issue of using locks or thread-local storage but do not have a > > > hard limit on simultaneous errors. > > malloc() can fail and it needs to be freed exactly once. And not freed > > before the caller is finished doing anything it likes to do with the er= ror > > information. And it very possibly wants to pass it on to its caller >=20 > Sorry, there was two words missing in my sentence, and that made it > nonsensical. It should have been: >=20 > - We allocate the memory for error messages IN THE contexts. >=20 > i.e. we add the room for the error structure(s) to the memory we reserve > for the contexts when creating them, and we free all at the same time. > Possibly, we even allocate it in same malloc() as the context. thats possible, but the same problem with manpower exists. If every structure needs to be changed, I have to point out that what i suggested needs no such change. What i suggested can simply be done with very little change to existing code That simplicity is directly tied to the use of globals. If you want to implement a system with context based error details, instead of th global system i suggested iam perfectly fine with that, iam just saying it looks like quite a bit more work and not something i can add to my TODO list thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No great genius has ever existed without some touch of madness. -- Aristotle --FGj0qTPk+4CI8WgJ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCaI/VaAAKCRBhHseHBAsP q0bcAJwIXJTAY3J9tFi4FPcttt0aMOVo4ACdGZtC5K32Wt7GS92M2Cf/NSrkgGo= =aLkm -----END PGP SIGNATURE----- --FGj0qTPk+4CI8WgJ-- --===============2518664720774235858== 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". --===============2518664720774235858==--