[FFmpeg-devel] [PATCH] lavu/dict: add av_dict_set_from_string() function

Nicolas George nicolas.george at normalesup.org
Tue May 8 19:57:21 CEST 2012


Le primidi 11 floréal, an CCXX, Stefano Sabatini a écrit :
> Based on an idea by Andrei Utkin <andrey.krieger.utkin at gmail.com>.
> ---
>  libavutil/Makefile |    2 +-
>  libavutil/dict.c   |  114 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libavutil/dict.h   |   27 ++++++++++++
>  3 files changed, 142 insertions(+), 1 deletions(-)
> 
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 83e6c07..eb12414 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -81,7 +81,7 @@ OBJS = adler32.o                                                        \
>         tree.o                                                           \
>         utils.o                                                          \
>  
> -TESTPROGS = adler32 aes avstring base64 bprint cpu crc des eval file fifo \
> +TESTPROGS = adler32 aes avstring base64 bprint cpu crc dict des eval file fifo \
>              lfg lls md5 opt pca parseutils random_seed rational sha tree
>  TESTPROGS-$(HAVE_LZO1X_999_COMPRESS) += lzo
>  
> diff --git a/libavutil/dict.c b/libavutil/dict.c
> index 6177ddd..f8d1845 100644
> --- a/libavutil/dict.c
> +++ b/libavutil/dict.c
> @@ -118,3 +118,117 @@ void av_dict_copy(AVDictionary **dst, AVDictionary *src, int flags)
>      while ((t = av_dict_get(src, "", t, AV_DICT_IGNORE_SUFFIX)))
>          av_dict_set(dst, t->key, t->value, flags);
>  }
> +
> +typedef struct {
> +    const AVClass *class;
> +    int   log_offset;
> +    void *log_ctx;
> +} DictLogContext;
> +
> +static const AVClass dict_class = { "DICT", av_default_item_name, NULL, LIBAVUTIL_VERSION_INT, offsetof(DictLogContext, log_offset), offsetof(DictLogContext, log_ctx) };

This could be more readable.

> +
> +static int set_key_value_pair(AVDictionary **dict, const char **buf, int flags,
> +                              const char *key_val_sep, const char *pairs_sep,
> +                              int log_offset, void *log_ctx)
> +{
> +    DictLogContext dict_log_ctx = { &dict_class, log_offset, log_ctx };

Unless I am mistaken, the log messages will include the address of this
dict_log_ctx, which is not relevant whatsoever.

> +    char *key = NULL, *val = NULL;
> +    int ret;
> +
> +    ret = ff_parse_key_value_pair(&key, &val, buf, key_val_sep, pairs_sep);

What is this ff_parse_key_value_pair function? I can not find anywhere in
the code.

> +    if (ret < 0) {
> +        av_log(&dict_log_ctx, AV_LOG_ERROR,
> +               "Missing key or no key/value separator found after key in '%s'\n", key);
> +        goto end;
> +    }
> +
> +    av_log(&dict_log_ctx, AV_LOG_DEBUG,
> +           "Setting entry with key '%s' to value '%s'\n", key, val);
> +    ret = av_dict_set(dict, key, val, flags);
> +
> +end:
> +    av_free(key);
> +    av_free(val);
> +    return ret;
> +}
> +
> +int av_dict_set_from_string(AVDictionary **dict, const char *kv, int flags,
> +                            const char *key_val_sep, const char *pairs_sep,
> +                            int log_offset, void *log_ctx)
> +{
> +    int ret, count = 0;
> +
> +    if (!kv)
> +        return 0;
> +
> +    while (*kv) {
> +        ret = set_key_value_pair(dict, &kv, flags, key_val_sep, pairs_sep,
> +                                 log_offset, log_ctx);
> +        if (ret < 0)
> +            return ret;
> +        count++;
> +
> +        if (*kv)
> +            kv++;
> +    }
> +
> +    return count;
> +}
> +
> +#ifdef TEST

The test could be added in a separate patch, but that does not matter much.

> +
> +#undef printf
> +
> +static void log_dict(AVDictionary *dict)
> +{
> +    AVDictionaryEntry *e = NULL;
> +    av_log(NULL, AV_LOG_INFO, "%p ", dict);
> +    while (e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX))

Doesn't this produce a warning about possible = / == mistake?

> +        av_log(NULL, AV_LOG_INFO, "%s->%s ", e->key, e->value);
> +    av_log(NULL, AV_LOG_INFO, "\n");
> +}

IMHO, tests should print their output to stdout (hence the #undef printf)
instead of stderr through the logging system. That way, their output can be
used in FATE directly.

> +
> +int main(void)
> +{
> +    int i;
> +
> +    av_log(NULL, AV_LOG_INFO, "\nTesting av_dict_set_from_string()\n");
> +    {
> +        const char *entries[] = {
> +            "",
> +            ":",
> +            "=",
> +            "foo=:",
> +            ":=foo",
> +            "=foo",
> +            "foo=",
> +            "foo",
> +            "foo=val",
> +            "foo==val",
> +            "toggle=:",
> +            "string=:",
> +            "toggle=1 : foo",
> +            "toggle=100",
> +            "toggle==1",
> +            "flags=+mu-lame : num=42: toggle=0",
> +            "num=42 : string=blahblah",
> +            "rational=0 : rational=1/2 : rational=1/-1",
> +            "rational=-1/0",
> +        };
> +
> +        av_log_set_level(AV_LOG_DEBUG);
> +
> +        for (i = 0; i < FF_ARRAY_ELEMS(entries); i++) {
> +            AVDictionary *dict = NULL;
> +            av_log(NULL, AV_LOG_DEBUG, "Setting entries string '%s'\n", entries[i]);
> +            if (av_dict_set_from_string(&dict, entries[i], 0, "=", ":", 0, NULL) < 0)
> +                av_log(NULL, AV_LOG_ERROR, "Error setting string: '%s'\n", entries[i]);
> +            log_dict(dict);
> +            av_log(NULL, AV_LOG_INFO, "\n");
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +#endif
> diff --git a/libavutil/dict.h b/libavutil/dict.h
> index 5a57224..d34b0d3 100644
> --- a/libavutil/dict.h
> +++ b/libavutil/dict.h
> @@ -121,6 +121,33 @@ void av_dict_copy(AVDictionary **dst, AVDictionary *src, int flags);
>  void av_dict_free(AVDictionary **m);
>  
>  /**
> + * Set an AVDictionary from a string containing a sequence of
> + * key/value pairs.
> + *
> + * @param[in][out] dict pointer to an AVDictionary. If *dict is NULL a new
> + * dictionary is created in put in the pointer, otherwise the
> + * dictionary is filled with the new entry.
> + * @param[in] kv string containing a key-value sequence to parse, may be NULL
> + * @param flags flags to use when setting entries in *dict
> + * @param[in] key_val_sep a 0-terminated list of characters used to
> + * separate key from value
> + * @param[in] pairs_sep a 0-terminated list of characters used to separate
> + * two key/value pairs each other
> + * @param[in] log_offset log level offset which is applied to the log
> + * level of log_ctx
> + * @param[in] log_ctx parent logging context
> + * @return the number of options set on success, a negative AVERROR code
> + * otherwise
> + */

Could be more readable with some alignment.

> +int av_dict_set_from_string(AVDictionary **dict, const char *kv, int flags,
> +                            const char *key_val_sep, const char *pairs_sep,
> +                            int log_offset, void *log_ctx);
> +
> +#define av_dict_set_from_string_quiet(dict, kv, flags, key_val_sep, pairs_sep) \
> +    av_dict_set_from_string(dict, kv, flags, key_val_sep, pairs_sep, \
> +                            AV_LOG_MAX_OFFSET, NULL)
> +
> +/**
>   * @}
>   */

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120508/1496f539/attachment.asc>


More information about the ffmpeg-devel mailing list