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

Michael Niedermayer michaelni
Mon Dec 27 17:29:50 CET 2010

On Tue, Dec 21, 2010 at 03:49:52PM +0100, Stefano Sabatini wrote:
> 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.

nothing stops you from logging a message before return <0

> 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.)

I dont see what is inconventient to write a 10 line av_log_callcack() that
passes its strings to thread local storage (if the GUI has a log window
per thread) or global storeage through a mutex if theres one window
if what you have in mind is a kind of
getting this string can be implemented in log.c/h

it might look like this then:
ret = av_foo(..., log_offset, log_ctx);
if (ret < 0){
    show_lasterror_in_window_to_user( get_log_buffer(log_ctx) );
    return ret;

this also can easily show all prior warnings or debug or just errors depending
on log level

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

and if errbuf_size is too small ...

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

and each level may want to add to it
1. mmap failed
2. opening direct rendering window failed
3. playing foobar.avi failed
4. playing playlist entry line 125 of bar.list failed

> 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).

Its not hard to do with av_log() but it involves some complexity that iam not
completely happy about. Also as said elsewhere there are some low level
functions like mmaping a file or a generic AVL tree where a av_log dependancy
is simply annoying.

Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101227/a970384b/attachment.pgp>

More information about the ffmpeg-devel mailing list