[FFmpeg-devel] [PATCH] opt: implement av_opt_set_from_string().
Stefano Sabatini
stefasab at gmail.com
Tue Sep 18 09:50:51 CEST 2012
On date Sunday 2012-09-16 11:18:19 +0200, Nicolas George encoded:
> It is similar to av_set_options_string() but accepts a list
> of options that can be in shorthand: if the key is omitted
> on the first fields, the keys from the shorthand list are
> assumed, in order.
>
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
> libavutil/opt.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> libavutil/opt.h | 30 ++++++++++++++
> 2 files changed, 146 insertions(+)
>
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 0957281..d826b4b 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -790,6 +790,91 @@ int av_set_options_string(void *ctx, const char *opts,
> return count;
> }
>
> +#define WHITESPACES " \n\t"
> +
> +static int is_key_char(char c)
> +{
> + return (unsigned)((c | 32) - 'a') < 26 ||
> + (unsigned)(c - '0') < 10 ||
> + c == '-' || c == '_' || c == '/' || c == '.';
> +}
> +
> +/**
> + * Read a key from a string. The key consists of is_key_char characters and
> + * must be terminated by a character from the delim string; spaces are
> + * ignored. The key buffer must be 4 bytes larger than the longest
> + * acceptable key. If the key is too long, an ellipsis will be written at
> + * the end. @return 0 for success (even with ellipsis), <0 for failure
Nit: @return on a separate line
> + */
> +static int get_key(const char **ropts, const char *delim, char *key, unsigned key_size)
> +{
> + unsigned key_pos = 0;
> + const char *opts = *ropts;
> +
> + opts += strspn(opts, WHITESPACES);
> + while (is_key_char(*opts)) {
> + key[key_pos++] = *opts;
> + if (key_pos == key_size)
> + key_pos--;
> + (opts)++;
> + }
> + opts += strspn(opts, WHITESPACES);
> + if (!*opts || !strchr(delim, *opts))
> + return AVERROR(EINVAL);
> + opts++;
> + key[key_pos++] = 0;
> + if (key_pos == key_size)
> + key[key_pos - 4] = key[key_pos - 3] = key[key_pos - 2] = '.';
> + *ropts = opts;
> + return 0;
> +}
> +
> +int av_opt_set_from_string(void *ctx, const char *opts,
> + const char *const *shorthand,
> + const char *key_val_sep, const char *pairs_sep)
> +{
> + int ret, count = 0;
> + const char *dummy_shorthand = NULL;
> + char key_buf[68], *value;
> + const char *key;
> +
> + if (!opts)
> + return 0;
> + if (!shorthand)
> + shorthand = &dummy_shorthand;
> +
> + while (*opts) {
> + if ((ret = get_key(&opts, key_val_sep, key_buf, sizeof(key_buf))) < 0) {
> + if (*shorthand) {
> + key = *(shorthand++);
> + } else {
> + av_log(ctx, AV_LOG_ERROR, "No option name near '%s'\n", opts);
Nit: "No option name *found* near..."
> + return AVERROR(EINVAL);
> + }
> + } else {
> + key = key_buf;
> + while (*shorthand) /* discard all remaining shorthand */
> + shorthand++;
> + }
> +
> + if (!(value = av_get_token(&opts, pairs_sep)))
> + return AVERROR(ENOMEM);
> + if (*opts && strchr(pairs_sep, *opts))
> + opts++;
> +
> + av_log(ctx, AV_LOG_DEBUG, "Setting '%s' to value '%s'\n", key, value);
> + if ((ret = av_opt_set(ctx, key, value, 0)) < 0) {
> + if (ret == AVERROR_OPTION_NOT_FOUND)
> + av_log(ctx, AV_LOG_ERROR, "Option '%s' not found\n", key);
> + return ret;
> + }
> +
> + av_free(value);
> + count++;
> + }
> + return count;
> +}
> +
> void av_opt_free(void *obj)
> {
> const AVOption *o = NULL;
> @@ -986,6 +1071,37 @@ int main(void)
> av_freep(&test_ctx.string);
> }
>
> + printf("\nTesting av_opt_set_from_string()\n");
> + {
> + TestContext test_ctx = { 0 };
> + const char *options[] = {
> + "",
> + "5",
> + "5:hello",
> + "5:hello:size=pal",
> + "5:size=pal:hello",
> + ":",
> + "=",
> + " 5 : hello : size = pal ",
> + "a_very_long_option_name_that_will_need_to_be_ellipsized_around_here=42"
> + };
> + const char *shorthand[] = { "num", "string", NULL };
> +
> + test_ctx.class = &test_class;
> + av_opt_set_defaults(&test_ctx);
> + test_ctx.string = av_strdup("default");
> +
> + av_log_set_level(AV_LOG_DEBUG);
> +
> + for (i=0; i < FF_ARRAY_ELEMS(options); i++) {
> + av_log(&test_ctx, AV_LOG_DEBUG, "Setting options string '%s'\n", options[i]);
> + if (av_opt_set_from_string(&test_ctx, options[i], shorthand, "=", ":") < 0)
> + av_log(&test_ctx, AV_LOG_ERROR, "Error setting options string: '%s'\n", options[i]);
> + printf("\n");
> + }
> + av_freep(&test_ctx.string);
> + }
> +
> return 0;
> }
>
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index 285d854..3aa912a 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -394,6 +394,36 @@ int av_set_options_string(void *ctx, const char *opts,
> const char *key_val_sep, const char *pairs_sep);
>
> /**
> + * Parse the key=value pairs list in opts. For each key=value pair found,
Nit+: key-value pairs
Rationale: the user may think that she can only specify "=", "-" is
more plain English so no assumption will be done on the key-value
separator.
> + * set the value of the corresponding option in ctx.
> + *
> + * @param ctx the AVClass object to set options on
> + * @param opts the options string, key-value pairs separated by a
> + * delimiter
> + * @param shorthand a NULL-terminated array of options names for shorthand
> + * notation: if the first field in opts has no key part,
> + * the key is taken from the first element of shorthand;
> + * then again for the second, etc., until either opts is
> + * finished, shorthand is finished or a named option is
> + * found; after that, all options must be named
> + * @param key_val_sep a 0-terminated list of characters used to separate
> + * key from value, for example '='
> + * @param pairs_sep a 0-terminated list of characters used to separate
> + * two pairs from each other, for example ':' or ','
> + * @return the number of successfully set key=value pairs, or a negative
Nit+: "key-value", as before
> + * value corresponding to an AVERROR code in case of error:
> + * AVERROR(EINVAL) if opts cannot be parsed,
> + * the error code issued by av_set_string3() if a key/value pair
> + * cannot be set
> + *
> + * Options names must use only the following characters: a-z A-Z 0-9 - . / _
> + * Separators must use characters distinct from from options and from each
Nit: "options" => "option names"
> + * other.
I still believe this should be a general requirement for option names,
so that using this function for parsing options will be painless,
since the user was already aware of the limitations for option names
when she creates the options in the first place (rather than having to
tweak them later).
Also we may set a limitation on the maximum option name length,
controlled by a macro. Alternatively we may remove the lenght
limitation entirely, and handle a dynamically allocated buffer in
get_key().
> + */
> +int av_opt_set_from_string(void *ctx, const char *opts,
> + const char *const *shorthand,
> + const char *key_val_sep, const char *pairs_sep);
> +/**
> * Free all string and binary options in obj.
> */
> void av_opt_free(void *obj);
Looks fine otherwise.
--
FFmpeg = Furious and Furious Magnificient Puristic Evanescent Gospel
More information about the ffmpeg-devel
mailing list