[FFmpeg-devel] [PATCH] opt: implement av_opt_set_from_string().

Stefano Sabatini stefasab at gmail.com
Tue Aug 14 13:48:23 CEST 2012


On date Monday 2012-08-13 17:33:32 +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.
> 

> It also does not allow to specify the separators, which was
> a feature never used anyway.

Uh? Can you elaborate?

> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  libavutil/opt.c     |   58 +++++++++++++++++++++++++++++++++++++++++++--------
>  libavutil/opt.h     |   21 +++++++++++++++++++
>  libavutil/version.h |    2 +-
>  3 files changed, 71 insertions(+), 10 deletions(-)
> 
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 0adbddd..b65dea1 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -728,6 +728,11 @@ void av_opt_set_defaults2(void *s, int mask, int flags)
>   * separate key from value
>   * @param pairs_sep a 0-terminated list of characters used to separate
>   * two pairs from each other

> + * @param key_short_sep a 0-terminated list of characters that can terminate
> + * keys: either identical to key_val_sep or to the union of key_val_sep and
> + * pairs_sep

Why don't you simply assume pairs_sep? Also this requirement is not
very robust.

> + * @param shorthand a pointer to a pointer to a NULL terminated array of
> + * option names, for shorthand syntax
>   * @return 0 if the key/value pair has been successfully parsed and
>   * set, or a negative value corresponding to an AVERROR code in case
>   * of error:
> @@ -736,28 +741,40 @@ void av_opt_set_defaults2(void *s, int mask, int flags)
>   * cannot be set
>   */
>  static int parse_key_value_pair(void *ctx, const char **buf,
> -                                const char *key_val_sep, const char *pairs_sep)
> +                                const char *key_val_sep, const char *pairs_sep,
> +                                const char *key_short_sep,
> +                                const char *const **shorthand)
>  {
> -    char *key = av_get_token(buf, key_val_sep);
> +    char *first_token = av_get_token(buf, key_short_sep);

I'd choose a different strategy, I'd get the token until the option
separator, and then I'd test for the presence of the key/val
separator. Seems simpler to me.

> +    const char *key;
>      char *val;
>      int ret;
>  
> -    if (*key && strspn(*buf, key_val_sep)) {
> +    if (!first_token)
> +        return AVERROR(ENOMEM);
> +    if (*first_token && strspn(*buf, key_val_sep)) {
>          (*buf)++;
> +        key = first_token;
>          val = av_get_token(buf, pairs_sep);
> +        while (**shorthand) /* named option -> no more shorthand */
> +            (*shorthand)++;
>      } else {
> -        av_log(ctx, AV_LOG_ERROR, "Missing key or no key/value separator found after key '%s'\n", key);
> -        av_free(key);
> -        return AVERROR(EINVAL);
> +        if (!**shorthand) {
> +            av_log(ctx, AV_LOG_ERROR, "Missing key or no key/value separator found after key '%s'\n", first_token);
> +            av_free(first_token);
> +            return AVERROR(EINVAL);
> +        }
> +        key = *((*shorthand)++);
> +        val = first_token;
> +        first_token = NULL;
>      }
> -
>      av_log(ctx, AV_LOG_DEBUG, "Setting entry with key '%s' to value '%s'\n", key, val);
>  
>      ret = av_opt_set(ctx, key, val, 0);
>      if (ret == AVERROR_OPTION_NOT_FOUND)
>          av_log(ctx, AV_LOG_ERROR, "Key '%s' not found.\n", key);
>  
> -    av_free(key);
> +    av_free(first_token);
>      av_free(val);
>      return ret;
>  }
> @@ -766,12 +783,13 @@ int av_set_options_string(void *ctx, const char *opts,
>                            const char *key_val_sep, const char *pairs_sep)
>  {
>      int ret, count = 0;
> +    const char *dummy_shorthand = NULL, *const *shorthand = &dummy_shorthand;
>  
>      if (!opts)
>          return 0;
>  
>      while (*opts) {
> -        if ((ret = parse_key_value_pair(ctx, &opts, key_val_sep, pairs_sep)) < 0)
> +        if ((ret = parse_key_value_pair(ctx, &opts, key_val_sep, pairs_sep, key_val_sep, &shorthand)) < 0)
>              return ret;
>          count++;
>  
> @@ -782,6 +800,27 @@ int av_set_options_string(void *ctx, const char *opts,
>      return count;
>  }
>  
> +int av_opt_set_from_string(void *ctx, const char *opts,
> +                           const char *const *shorthand)
> +{
> +    int ret, count = 0;
> +    const char *dummy_shorthand = NULL;
> +
> +    if (!opts)
> +        return 0;
> +    if (!shorthand)
> +        shorthand = &dummy_shorthand;
> +
> +    while (*opts) {
> +        if ((ret = parse_key_value_pair(ctx, &opts, "=", ":", "=:", &shorthand)) < 0)
> +            return ret;
> +        count++;
> +        if (*opts)
> +            opts++;
> +    }
> +    return count;
> +}
> +
>  void av_opt_free(void *obj)
>  {
>      const AVOption *o = NULL;
> @@ -932,6 +971,7 @@ int main(void)
>  {
>      int i;
>  
> +

spurious

>      printf("\nTesting av_set_options_string()\n");
>      {
>          TestContext test_ctx = { 0 };
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index 3bf30a5..cef820d 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -394,6 +394,27 @@ 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,
> + * set the value of the corresponding option in ctx.

Needs to be updated with mention to short options.

> + *
> + * @param ctx        the AVClass object to set options
> + * @param opts       the options string, "key=value" pairs separated by ':'
> + * @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

> + * @return  the number of successfully set key=value pairs, or a negative

the number of successfully set options, or a negative ...

> + *          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
> + */

[...]

Note: an usage example in opt-test could be useful.
-- 
FFmpeg = Frenzy & Fabulous Multimedia Political Energized Gymnast


More information about the ffmpeg-devel mailing list