[FFmpeg-devel] [PATCH] Move error codes definitions from lavc to lavu

[Stefano Sabatini]

On date Thursday 2009-07-30 01:27:54 +0200, Stefano Sabatini encoded:
> On date Tuesday 2009-07-21 04:04:32 +0200, Michael Niedermayer encoded:
> > On Tue, Jul 21, 2009 at 01:54:39AM +0200, Stefano Sabatini wrote:
> > > On date Monday 2009-07-20 17:24:45 +0200, Michael Niedermayer encoded:
> > > > On Sun, Jul 19, 2009 at 12:09:16PM +0200, Stefano Sabatini wrote:
> > > > > Hi, as in subject,
> > > > > 
> > > > > this is required if we want for example move the error handling code
> > > > > to lavu, as it uses it. Also reduces the clutter in avcodec.h.
> > > > 
> > > > i am fine with the move in priciple but didnt we want to deprecate the
> > > > AVERROR_* macros in favor of AVERROR(E*) ?
> > > 
> > > What about AVERROR_PATCHWELCOME?
> > 
> > that has no E* so it can stay
> 
> Fine.
>  
> > > 
> > > Also how AVERROR(XXX) is better than AVERRROR_XXX?
> > 
> > it needs less code
> > 
> > 
> > > 
> > > Also are we sure mapping A/V multimedia specific errors to POSIX
> > > errors is a good idea?
> > 
> > Without an alternative suggestion its hard to say ...
> 
> My idea is to use AVERROR(EXXX) for each error which can be specified
> using POSIX error codes, *then* having a minimal but meaningful list
> of AVERROR_XXX FFmpeg error codes.
> 
> For example:
> AVERROR_PATCHWELCOME
> AVERROR_NO_OPTION_FOUND
> AVERROR_UNSUPPORTED_FORMAT
> AVERROR_VBV_FAILURE
> 
> and a corresponding av_errstring() func.
> 
> > > Also how can we say that e.g. -MKTAG('P','A','W','E') doesn't clutter
> > > with a POSIX error code?
> > 
> > we cannot but it works with all systems for which ive seen E* values
> > and a clash is exceedingly unlikely (1 in many millions) if a clash
> > happens we just change it and bump major ver, given the low likeliness
> > of conflict this seems acceptable to me but i guess some will disagree
> 
> Not very fine but acceptable (we may change idea in a distant? future).
> 
> > > Also the use of the POSIX error codes is arguable (ENOENT is used for
> > > example when we don't find an option in a context, doxy is "No such
> > > file or directory"), EILSEQ -> "unknown format", EDOM -> "Number
> > > syntax expected in filename".
> >
> > > Also how can we map an error code to an error message?
> > 
> > by writing a function that does that
> >
> > 
> > > 
> > > All these questions raise the old problem of how to deal more sanely
> > > with errors in libav*... and since I don't want to get stucked there,
> > 
> > what old problem exactly do you refer to here?
> > which issue number on roundup is it?
> 
> No roundup issue, but the issue has been discussed sometimes in the
> past (mainly proposed by Takis or me):
> 
> http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/54380
> http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/68265/focus=78180

It seems we agree to choose when possible error codes borrowed from
POSIX, otherwise use the -MKTAG trick for creating FFmpeg specific
error codes.

This also means that we should drop all the AVERROR_* macros which
have a corresponding code in POSIX.

Then we have to deal with the FFmpeg specific error codes. Actually
we have just one of them (AVERROR_PATCHWELCOME).

Possible idea:

#define AVERROR2(a, b, c, d) -MKTAG(a, b, c, d)
then we could use it like:
        return AVERROR2('P','A','W','E');

*or* we could keep:

#define AVERROR_PATCHWELCOME    -MKTAG('P','A','W','E')
and simply use:
        return AVERROR_PATCHWELCOME.

I really have no idea what would be better.

As for the error string something like the following should fit:

int av_strerror(int errcode)
{
   switch (errcode) {
       AVERROR_PATCHWELCOME:
           return "Not yet implemented in FFmpeg. Patches welcome.";
       AVERROR_BIKESHED:
           return "Invalid color or argument or flavor selected, mine is better.";
       AVERROR_NOTENOUGHMONEY:
           return "You have to pay more buck$ for enabling this feature.";
       ...
       default:
          return strerror(errcode); 
   }
}

As for the retrocompatibility issue, we could just ifdef the old
AVERROR_* definitions and drop them at the next major bump.

See attached patch.

Regards.
-- 
FFmpeg = Fabulous and Faithful Mastering Peaceless Enigmatic Geisha