Re: [FFmpeg-devel] [RFC] Advanced Error Codes

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 3952 bytes --]

Hi Nicolas

On Fri, Jul 04, 2025 at 03:29:19PM +0200, Nicolas George wrote:
> Michael Niedermayer (HE12025-07-03):
> > return av_adv_err_new(AVERROR_INVALIDDATA, "Garbled foobar data", "Foo triangle quantum decoder"
> >                       __FILE__, __LINE__, NULL, "Whaetver you like %s", favorite_food);
> > 
> > teh return type is int64_t here
> 
> So we need to change all our return types?

no

we can do it with int too, if thats preferred.


> 
> > this also cannot fail as it allocates nothing
> 
> Where does the memory storing “favorite_food” come from?

theres a fixed length array of AdvancedError structs.
AdvancedError has a fixed length char field.
Thats where a single custom text string can be stored.
Which would be maybe "Whaetver you like pizza" if favorite_food was "pizza"
its very very simple

typdef struct AdvancedError {
    const char *error_name;          ///< like "Timeout"       or  "Invalid Bitstream"
    const char *operation_name;      ///< like "Network Read"  or  "H.264 block parsing"
    const char *source_filename;     ///< like ffmpeg.c
    int source_line_number;          ///< like 123 for line 123 in ffmpeg.c
    char free_form_description[FREE_FORM_LEN];  ///< anything extra that doesnt fit in pointers to static const strings
    int64_t this_error;
    int64_t previous_error;          ///< A previous related error, if any
    int64_t timestamp;               ///< Timestamp at which the error occured
    int thread_id;
} AdvancedError;

struct AdvancedError   advanced_error[MAX_CONCURRENT_ERRORS];


> 
> > it also needs no context but would use a mutex or thread local storage
> > 
> > the message length would be bound by a maximum,
> 
> Of course.
> 
> > I am not sure if passing a context around is going to find the volunteers
> > to implement and maintain. Also it has a performance impact for small and
> > lightweight functions.
> 
> We already pass contexts around everywhere.

* Many places but not everywhere,
* The contexts do not match the lifetime of the errors
* The contexts can be accessed from multiple threads
* There can be multiple relevant errors for a single context


> 
> I would argue that the small lightweight functions that do not already
> have a context for the error are too low level to be able to provide a
> meaningful error message.

"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

for example decode() returning
AVERROR(EINVAL)
with 5000 lines of cryptic parser errors in messages

vs.

"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="training_set_maybe_corrupted.dat"

Note by the time this is returned decode failed and many contexts have
been destroyed.


> 
> So, my proposal is similar to yours, except for the following
> differences:
> 
> - We allocate the memory for error messages contexts. That way, we avoid
>   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 error
information. And it very possibly wants to pass it on to its caller


> 
> - We do not change the return type of all our API, we still return “a
>   negative AVERROR code” to keep source compatibility, and use the best
>   code we can find.

Then you no longer know which error relates to which message

So the problem that I see, isnt solved.
Maybe it helps with other problems, i dont know.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".