[FFmpeg-devel] [PATCH] Fix opt_default()

[Stefano Sabatini]

On date Sunday 2008-12-07 14:35:58 +0100, Michael Niedermayer encoded:
> On Sun, Dec 07, 2008 at 02:45:16AM +0100, Stefano Sabatini wrote:
> > On date Saturday 2008-12-06 17:06:57 +0100, Michael Niedermayer encoded:
[...]
> > > id like to repeat that when an integer is returned that should be an error
> > > code according to AVERROR(). i am not ok with this kind of ptr argument
> > > checking to guess which of 2 cases out of 10 apply. Next we would have
> > > to add a sscanf() of the inercepted av_log() output with this design ...
> > 
> > I just see the cases "value set", "option found but invalid value",
> > "option not found", also I'm not sure how to map these errors
> > (AVERRR_INVALIDDATA, AVERROR_NOENT?).
> 
> i see plenty more, like value ok but outside allowed range ...
> anyway, just see the descriptions from POSIX iam sure you will find
> something that matches approximately

I agree with the principle, in practice it is not that easy to
implement, also because the matches are not really accurate.

Anyway I'll try:
* No option found -> ENOENT (the problem is that it is normally used
  for files or dir)

* Invalid data (e.g. an unparsable value) -> EINVAL

* Out of range value -> ERANGE (not currently implemented in
  avcodec.h, check the patch)

(BTW the error management in avcodec.h is quite a mess, many mappings
are rather arbitrary to my eyes, for example AVERROR_NUMEXPECTED ->
EDOM, AVERROR_NOFMT -> EILSEQ).

And of course we could define new FFmpeg specifics error codes, for
example AVERROR_NOOPTIONFOUND.

> > As for the index in the options array, I found its use quite awkward,
> > but anyway if you like it I can implement this:
> > 
> > int av_set_string3(void *obj, const char *name, const char *val);
> 
> iam not insisting on this, what i insist on is that we will not have
> 20 return -1 and parse an error message (or check if its NULL)  to find
> out which kind of error it was.

I continue to prefer:

1) int av_set_string3(void *obj, const char *name, const char *val, const AVOption **o_out);
(return an error code or 0 if the option has been set, put in o_out
the option eventually found)

over:

2) int av_set_string3(void *obj, const char *name, const char *val);
(return an error code or the index in the obj->options array if the
option is found and the value is valid)

but I won't insist if you prefer the other one, please just say which
you prefer.

Advantage of 1) over 2) is that you know the option which has been
found even in case of error, so no need to iterate through options to
find it again, if you don't care about the option found you can simply
set o_out to NULL.

[...]

Regards.
-- 
FFmpeg = Free and Fostering Minimalistic Pitiless Easy Guru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add-averror-range.patch
Type: text/x-diff
Size: 952 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081207/334cf656/attachment.patch>