[FFmpeg-devel] [PATCH] Enhance ffmpeg.c:opt_default()
Michael Niedermayer
michaelni
Fri May 16 18:24:11 CEST 2008
On Fri, May 16, 2008 at 01:57:09AM +0200, Stefano Sabatini wrote:
> On date Thursday 2008-05-15 01:26:58 +0200, Michael Niedermayer encoded:
> > On Wed, May 14, 2008 at 11:56:18PM +0200, Stefano Sabatini wrote:
> [...]
> > > I think that this patch implements the correct behaviour, which is:
> > >
> > > patch) exit immediately when you find in some context the option X,
> > > but its value is invalid, and tell explicitly that the value is
> > > invalid
> > >
> > > as opposed to the current behaviour:
> > >
> > > none) if a value is found in a context but it fails to set it, try
> > > again with the following context, finally if the value hasn't been
> > > set because it was invalid in one or more contexts or hasn't been
> > > found in any context then always say the option hasn't been found
> > >
> > > To me the actual behaviour looks wrong and confusing for the
> > > user.
> >
> > Well, true but i do not like the solution.
> > av_set_string() should make it clear to the caller if an error happened
> > and the code should then not continue blindly.
> > These wraper and exit() functions really are ugly.
> >
> > a "const char **error" argument instead of av_log() calls seems to be
> > an option.
>
> Michael, do you prefer the attached solution? (yes I'll split it if
> you approve the general idea.)
>
> I'm defining a new av_set_string2() with this signature:
> const AVOption *av_set_string2(void *obj, const char *name, const char *val, const char **error_ptr);
>
> which returns the pointer to the option if found even if the value
> wasn't valid, in this case sets error_ptr to a static string
> containing an error message: this function uses uses the new
> av_set_number2().
>
> In order to preserve the semantic of av_set_{double|q|int} I'm also
> keeping a function av_set_number() with the old semantic and
> signature.
>
> Comments are welcome.
>
> Regards.
> --
> FFmpeg = Frenzy and Foolish Multimedia Proud EnGraver
> Index: libavcodec/avcodec.h
> ===================================================================
> --- libavcodec/avcodec.h (revision 13158)
> +++ libavcodec/avcodec.h (working copy)
> @@ -30,7 +30,7 @@
> #include "libavutil/avutil.h"
>
> #define LIBAVCODEC_VERSION_MAJOR 51
> -#define LIBAVCODEC_VERSION_MINOR 56
> +#define LIBAVCODEC_VERSION_MINOR 57
> #define LIBAVCODEC_VERSION_MICRO 0
>
> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> Index: libavcodec/opt.h
> ===================================================================
> --- libavcodec/opt.h (revision 13158)
> +++ libavcodec/opt.h (working copy)
> @@ -73,6 +73,7 @@
>
> const AVOption *av_find_opt(void *obj, const char *name, const char *unit, int mask, int flags);
> const AVOption *av_set_string(void *obj, const char *name, const char *val);
> +const AVOption *av_set_string2(void *obj, const char *name, const char *val, const char **error_ptr);
> const AVOption *av_set_double(void *obj, const char *name, double n);
> const AVOption *av_set_q(void *obj, const char *name, AVRational n);
> const AVOption *av_set_int(void *obj, const char *name, int64_t n);
> Index: libavcodec/opt.c
> ===================================================================
> --- libavcodec/opt.c (revision 13158)
> +++ libavcodec/opt.c (working copy)
> @@ -25,6 +25,7 @@
> * @author Michael Niedermayer <michaelni at gmx.at>
> */
>
> +#include <libavutil/avstring.h>
> #include "avcodec.h"
> #include "opt.h"
> #include "eval.h"
> @@ -47,15 +48,19 @@
> else return (*(AVClass**)obj)->option;
> }
>
> -static const AVOption *av_set_number(void *obj, const char *name, double num, int den, int64_t intnum){
> +static const AVOption *av_set_number2(void *obj, const char *name, double num, int den, int64_t intnum, const char **error_ptr){
> const AVOption *o= av_find_opt(obj, name, NULL, 0, 0);
> + static char error[128];
> +
rejected
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- 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/20080516/18c3803c/attachment.pgp>
More information about the ffmpeg-devel
mailing list