On Wed, 4 Jun 2014 14:33:01 -0400, Andrew Stone <and...@clovar.com> wrote:
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index b90feaa..3a712fe 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -223,7 +223,8 @@ enum AVOptionType{
> AV_OPT_TYPE_FLOAT,
> AV_OPT_TYPE_STRING,
> AV_OPT_TYPE_RATIONAL,
> - AV_OPT_TYPE_BINARY, ///< offset must point to a pointer immediately
> followed by an int for the length
> + AV_OPT_TYPE_BINARY,
> + AV_OPT_TYPE_DICT, ///< offset must point to a pointer immediately
> followed by an int for the length
> AV_OPT_TYPE_CONST = 128,
> };
>
> @@ -253,6 +254,7 @@ typedef struct AVOption {
> int64_t i64;
> double dbl;
> const char *str;
> + AVDictionary *dict;
This change is not needed, since (as the comment above says) we do not allow
default values for dicts and this union is only for holding the defaults.
> /* TODO those are unused now */
> AVRational q;
> } default_val;
> @@ -325,7 +327,7 @@ int av_set_options_string(void *ctx, const char *opts,
> const char *key_val_sep, const char *pairs_sep);
>
> /**
> - * Free all string and binary options in obj.
> + * Free all allocated objects in obj.
> */
> void av_opt_free(void *obj);
>
> @@ -491,11 +493,12 @@ const AVClass *av_opt_child_class_next(const AVClass
> *parent, const AVClass *pre
> * AVERROR(ERANGE) if the value is out of range
> * AVERROR(EINVAL) if the value is not valid
> */
> -int av_opt_set (void *obj, const char *name, const char *val, int
> search_flags);
> -int av_opt_set_int (void *obj, const char *name, int64_t val, int
> search_flags);
> -int av_opt_set_double(void *obj, const char *name, double val, int
> search_flags);
> -int av_opt_set_q (void *obj, const char *name, AVRational val, int
> search_flags);
> -int av_opt_set_bin (void *obj, const char *name, const uint8_t *val, int
> size, int search_flags);
> +int av_opt_set (void *obj, const char *name, const char *val, int
> search_flags);
> +int av_opt_set_int (void *obj, const char *name, int64_t val, int
> search_flags);
> +int av_opt_set_double (void *obj, const char *name, double val, int
> search_flags);
> +int av_opt_set_q (void *obj, const char *name, AVRational val, int
> search_flags);
> +int av_opt_set_bin (void *obj, const char *name, const uint8_t *val, int
> size, int search_flags);
> +int av_opt_set_dict_val(void *obj, const char *name, const AVDictionary
> *val, int search_flags);
The semantics of this functin needs a bit more documentation, since dicts are
more complex objects than the other option types.
Specifically, it should be explicitly mentioned that the old dict is completely
discarded (as opposed to overwritten with new values) and replaced with a _copy_
of the supplied dict (and not the supplied dict itself).
> /**
> * @}
> */
> @@ -515,10 +518,11 @@ int av_opt_set_bin (void *obj, const char *name,
> const uint8_t *val, int size,
> /**
> * @note the returned string will av_malloc()ed and must be av_free()ed by
> the caller
> */
> -int av_opt_get (void *obj, const char *name, int search_flags, uint8_t
> **out_val);
> -int av_opt_get_int (void *obj, const char *name, int search_flags, int64_t
> *out_val);
> -int av_opt_get_double(void *obj, const char *name, int search_flags, double
> *out_val);
> -int av_opt_get_q (void *obj, const char *name, int search_flags,
> AVRational *out_val);
> +int av_opt_get (void *obj, const char *name, int search_flags,
> uint8_t **out_val);
> +int av_opt_get_int (void *obj, const char *name, int search_flags,
> int64_t *out_val);
> +int av_opt_get_double (void *obj, const char *name, int search_flags,
> double *out_val);
> +int av_opt_get_q (void *obj, const char *name, int search_flags,
> AVRational *out_val);
> +int av_opt_get_dict_val(void *obj, const char *name, int search_flags,
> AVDictionary **out_val);
Same here, you should explicitly say that the caller receives a copy of the
dict that he's responsible for freeing, and not a pointer to the internal dict.
Also, why the _val postfix? Why not call it just av_opt_get/set_dict?
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
