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

[Michael Niedermayer]

On Sun, Dec 07, 2008 at 05:10:11PM +0100, Stefano Sabatini wrote:
> 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)

i think you can just use AVERROR(ERANGE) ...


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

i dont mind 1.

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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081207/31855a51/attachment.pgp>