[FFmpeg-devel] [PATCH 2/4] parse_key_value_pair: Support keys without values with AV_DICT_ALLOW_NULLVAL

Stefano Sabatini stefasab at gmail.com
Sun Jul 21 13:27:31 CEST 2013


"parse_key_value_pair" is not useful as module name, especially given
we have different implementations with different levels of NIH-ness.

Also any reason this is not merged with the previous patch?

On date Saturday 2013-07-20 18:55:03 +0200, Michael Niedermayer encoded:
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libavutil/dict.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/libavutil/dict.c b/libavutil/dict.c
> index f965492..40434b6 100644
> --- a/libavutil/dict.c
> +++ b/libavutil/dict.c
> @@ -116,6 +116,7 @@ static int parse_key_value_pair(AVDictionary **pm, const char **buf,
>                                  const char *key_val_sep, const char *pairs_sep,
>                                  int flags)
>  {

> +    char *buf2= *buf;

Nit: *buf2 = *buf;

Who gives a bit about vertical align anyway.

>      char *key = av_get_token(buf, key_val_sep);
>      char *val = NULL;
>      int ret;
> @@ -125,8 +126,19 @@ static int parse_key_value_pair(AVDictionary **pm, const char **buf,
>          val = av_get_token(buf, pairs_sep);
>      }
>  

> +    if (flags & AV_DICT_ALLOW_NULLVAL) {
> +        char *key2 = av_get_token(&buf2, pairs_sep);
> +        if (!key || (key2 && strlen(key2) < strlen(key))) {
> +            FFSWAP(char *, *buf, buf2);
> +            FFSWAP(char *, key, key2);
> +            av_freep(&val);
> +        }
> +        av_freep(&key2);
> +    }

What's the final outcome of this hack?

>      if (key && *key && val && *val)
>          ret = av_dict_set(pm, key, val, flags);
> +    else if (key && *key && !val && (flags & AV_DICT_ALLOW_NULLVAL))
> +        ret = av_dict_set(pm, key, val, flags);
>      else
>          ret = AVERROR(EINVAL);

The result is that now we have several option list functions with
different behaviours (av_dict_parse_string(), av_opt_get_key_value(),
av_opt_set_from_string()). Ideally we should try to unify them, rather
than adding more arbitrary differences.
-- 
FFmpeg = Fancy and Furious Marvellous Programmable Enlightening God


More information about the ffmpeg-devel mailing list