Hi, On Mon, May 30, 2011 at 2:09 PM, Anton Khirnov <[email protected]> wrote: > On Mon, 30 May 2011 22:11:48 +0200, Stefano Sabatini > <[email protected]> wrote: >> On date Monday 2011-05-30 21:15:19 +0200, Anton Khirnov encoded: >> > This way the caller can pass all the options in one nice package. >> > --- >> > libavutil/opt.c | 23 +++++++++++++++++++++++ >> > libavutil/opt.h | 13 +++++++++++++ >> > 2 files changed, 36 insertions(+), 0 deletions(-) >> > >> > diff --git a/libavutil/opt.c b/libavutil/opt.c >> > index 9e06b01..e7f7780 100644 >> > --- a/libavutil/opt.c >> > +++ b/libavutil/opt.c >> > @@ -29,6 +29,7 @@ >> > #include "avstring.h" >> > #include "opt.h" >> > #include "eval.h" >> > +#include "meta.h" >> > >> > //FIXME order them and do a bin search >> > const AVOption *av_find_opt(void *v, const char *name, const char *unit, >> > int mask, int flags) >> > @@ -518,6 +519,28 @@ int av_set_options_string(void *ctx, const char *opts, >> > return count; >> > } >> > >> > +int avopt_process(void *obj, AVMetadata **options) >> > +{ >> > + AVMetadataTag *t = NULL; >> > + AVMetadata *tmp = NULL; >> > + int ret = 0; >> > + >> > + avmeta_copy(&tmp, *options, 0); >> > + >> > + while ((t = avmeta_get(*options, "", t, AV_METADATA_IGNORE_SUFFIX))) { >> > + int ret = av_set_string3(obj, t->key, t->value, 1, NULL); >> > + if (ret >= 0) >> > + avmeta_set(&tmp, t->key, NULL, 0); >> > + else if (ret != AVERROR_OPTION_NOT_FOUND) { >> > + av_log(obj, AV_LOG_ERROR, "Error setting option %s to value >> > %s\n", t->key, t->value); >> > + break; >> >> In case of missing options I believe this should fail, or user the >> will have weird suprises (I'm sure I set "foo", why isn't it set?), >> having to check the not set options look awkward. > > Imagine a media player, which is configured to set a demuxer private > option for e.g. matroska files. There's no (clean) way for it to know in > advance, what format the file will be, so it can't know whether to set > the option for a given file or no. With this API, it sets the option > always, and just ignores it if it's returned as unprocessed. > I don't see this working with what you're proposing
I kind of agree with Anton here. In the end, it should be user-visible that options could not be set, and av_log() does that. But there's no reason to abort in most cases. There surely is API to do that, but again, that's likely not what most applications will do. If it turns out we were wrong, we can always add the more complex API later on. For now, this will do, IMO. Ronald _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
