[FFmpeg-devel] [PATCH] Enhance ffmpeg.c:opt_default()

[Stefano Sabatini]

On date Sunday 2008-11-30 23:27:50 +0100, Michael Niedermayer encoded:
> On Sun, Nov 30, 2008 at 10:27:01PM +0100, Stefano Sabatini wrote:
> > On date Sunday 2008-11-30 21:02:43 +0100, Michael Niedermayer encoded:
> > > On Sun, Nov 30, 2008 at 08:22:35PM +0100, Stefano Sabatini wrote:
> > [...]
> > > > The current semantics of av_set_string2() is to return NULL if the
> > > > option hasn't been found *or* if the value wasn't valid, so there is
> > > > currently no way to distinguish the two cases (which is the reason of
> > > > the patchset).
> > > > 
> > > > I think that if the function *finds* the option but can't set the
> > > > value because the value isn't valid, it is still a good idea to return
> > > > the option found, at least it seems better because this way we're
> > > > providing more information to the invoker.
> > > 
> > > the problem iam having with this is when the "user" doesnt care about the
> > > error message and thus passes NULL.
> > > At that point he cant find out if the call succeeded
> > 
> > He can always do:
> > AVOption *o = av_set_string3(..., error, error_size);
> > if (o && !error)
> >    ...
> > 
> > (yes this way he's forced to pass a non-NULL error...).
> > 
> > On the other hand if he wants to know for whatever reason if the
> > option was found and doesn't care to know if the value has been set or
> > not he can do it in just one step.
> > 
> > But I won't insist on this if you prefer the other way.
> 
> what about just returning an index into the
> AVOption array and a negative value to indicate exactly which error happened?
> (thats just an idea, i dont know how this would look in code so maybe it
>  ends up ugly ...)

The main problem with this is that as I see it, you need to map each
index to an error string, *and* you can't provide specifics
information (e.g. "unvalid value: x must be > min and < max").

What about something like:
int av_set_string3(void *obj, const char *name, const char *val, const AVOption **o_out);
?

We could use it like:

const AVOption *o;
if (av_set_string3(..,  &o) < 0) {
   if (!o)
      fprintf(stderr, "Option not found\n");
   else            
      fprintf(stderr, "Option found but invalid value\n");
}

logging the error message with av_log(), which is also consistent with
the rest of libav*.

Regards.
-- 
FFmpeg = Freak Furious MultiPurpose Embarassing Geek