[FFmpeg-devel] [PATCH] Add av_file_get_size() and av_file_read(), replace cmdutils.h:read_file().

Stefano Sabatini stefano.sabatini-lala
Tue Dec 21 15:49:52 CET 2010


On date Tuesday 2010-12-21 13:02:52 +0100, Nicolas George encoded:
> Le decadi 30 frimaire, an CCXIX, Stefano Sabatini a ?crit?:
> > The problem is that the external function doesn't know the context of
> > where the error occurred, and an error code is not enough, for example
> > to distinguish if the error occurred in open(), lstat(), mmap() or if
> > the file was too big (in this case an AVERROR(EINVAL) error code
> > doesn't give a clue about the exact failure reason).
> > 
> > I spent unnamerable hours figuring out the meaning of lame error
> > messages which provided no hint about where the failure occurred, so
> > as a software user I tend to give *much* importance to these usability
> > issues.
> 
> I have already met problem you are discussing here, as well as probably most
> programmers.
> 
> The core of the problem resides in the fact that "error reporting" covers
> two similar but fundamentally distinct tasks:
> 
> 1. reporting to the calling context a simple cause so it can hopefully be
>    recovered (EAGAIN/EINTR, EEXIST vs. EACCESS, etc.);
> 
> 2. reporting to the used a detailed cause for the error so he can understand
>    the problem and fix it.
> 
> The errno system works because libc functions are low-level and perform only
> a simple task each: for example if opening a file fails, the calling context
> knows which file was being opened.
> 
> Anything higher-level that wants to use integer error codes imitating the
> errno system will stumble on the problem sooner or later.
> 
> Gtk+ has a nice way of solving the problem. Error causes are reported
> through an additional "GError **" argument to each function that can
> possibly fail. The GError structure has both an integer error code
> (actually, a pair of integer error codes, allowing different parts of the
> library to define new error codes independently) for automatic handling and
> a human-readable string.
> 
> Libav* could possibly imitate the solution. Of course, adding an extra
> argument to all existing functions is not an option, but new functions could
> benefit from it. Something like that maybe:
> 
> - Add an AVError structure in avutil.
> 
> - New functions that can have something subtle to report take an additional
>   pointer to an AVError. If it is null, the human-readable messages
>   automatically goes to av_log.
> 
> - AVFoobarContext and similar structures gain an AVError field, so that
>   whenever there is a context available, there is a place to store the
>   error.
> 
> What do you think about it?

Currently we do:
ret = av_foo(..., log_offset, log_ctx);
if (ret < 0)
   return ret;

with this approch is difficult to understand when the called will
actually log something in case of error, which basically depends on
the called function implementation (for example no error is logged in
most cases when there is a memory allocation problem), also in some
cases makes sense to log another more high-level message.

Also this system is quite inflexible, for example what if you want to
show the error message in a GUI? (Well I suppose you may implement a
per-thread stack of error messages, and fill it with an ad-hoc
implementation of the av_log callback, but this is not very
convenient.)

A possible approach would be to make the functions provide an
additional errbuf/errbuf_size where the error message is put, when you
have a context you could use the string provided by the log_context
(this should be more or less equivalent to the GError approach, since
we already return the error code). Main problem is the complexity
overhead:

char errbuf[128] = "";
ret = av_foo(..., errbuf, errbuf_size);
if (ret < 0) {
   // do something with the error string
   av_log(log_ctx, "%s\n", errbuf);
   return ret;
}

when using the errbuf in the log_ctx:
ret = av_foo(..., log_ctx->errbuf, log_ctx->errbuf_size);
...

then we may find some way to automatically fill errbuf with the
default error string for ret if no specific message is given
(BTW extending av_log this way:
av_log("Error occurred with code %e: %E"\n, errcode, errcode)
may be useful).

Another problem with this approach is when you have a deep stack of
function calls, and each one needs to report the error message to the
callee. Also how to manage when we need to deal with messages with
different log levels (not all messages pertain to error conditions)?

In general to design a sane error reporting system is difficult (that
explains why most software sucks with it).
-- 
FFmpeg = Faithful Funny Mysterious Perennial Entertaining God



More information about the ffmpeg-devel mailing list