[FFmpeg-devel] [PATCH 1/2] opt: Add support to query ranges

Stefano Sabatini stefasab at gmail.com
Tue Dec 4 23:31:40 CET 2012


On date Tuesday 2012-12-04 20:46:40 +0100, Michael Niedermayer encoded:
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libavutil/log.h |    6 +++++
>  libavutil/opt.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libavutil/opt.h |   50 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 133 insertions(+)
> 
> diff --git a/libavutil/log.h b/libavutil/log.h
> index ba7315f..b5205c5 100644
> --- a/libavutil/log.h
> +++ b/libavutil/log.h
> @@ -114,6 +114,12 @@ typedef struct AVClass {
>       * available since version (51 << 16 | 59 << 8 | 100)
>       */
>      AVClassCategory (*get_category)(void* ctx);
> +
> +    /**
> +     * Callback to return the supported/allowed ranges.
> +     * available since version (???)
> +     */
> +    struct AVOptionRanges *(*query_ranges)(void *obj, const char *key, int flags);
>  } AVClass;
>  
>  /* av_log API */
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 0ff45c8..cd26682 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -1161,6 +1161,83 @@ void *av_opt_ptr(const AVClass *class, void *obj, const char *name)
>      return (uint8_t*)obj + opt->offset;
>  }
>  
> +AVOptionRanges *av_opt_query_ranges(void *obj, const char *key, int flags) {
> +    const AVClass *c = *(AVClass**)obj;
> +    AVOptionRanges *(*callback)(void *obj, const char *key, int flags) = NULL;
> +

> +    if(c->version > (52 << 16 | 8 << 9))

8 << 9 => 9 << 8?

also the minor number should be updated.

> +        callback = c->query_ranges;
> +
> +    if(!callback)
> +        callback = av_opt_query_ranges_default;
> +
> +    return callback(obj, key, flags);
> +}
> +
> +AVOptionRanges *av_opt_query_ranges_default(void *obj, const char *key, int flags) {
> +    AVOptionRanges *ranges = av_mallocz(sizeof(*ranges));
> +    AVOptionRange **range_array = av_mallocz(sizeof(void*));
> +    AVOptionRange *range = av_mallocz(sizeof(*range));
> +    const AVOption *field = av_opt_find(obj, key, NULL, 0, flags);
> +
> +    if(!ranges || !range || !range_array || !field)
> +        goto fail;
> +
> +    ranges->range = range_array;
> +    ranges->range[0] = range;
> +    ranges->nb_ranges = 1;
> +    range->is_range = 1;
> +    range->value_min = field->min;
> +    range->value_max = field->max;
> +
> +    switch(field->type){
> +    case AV_OPT_TYPE_INT:
> +    case AV_OPT_TYPE_INT64:
> +    case AV_OPT_TYPE_PIXEL_FMT:
> +    case AV_OPT_TYPE_SAMPLE_FMT:
> +    case AV_OPT_TYPE_FLOAT:
> +    case AV_OPT_TYPE_DOUBLE:
> +        break;
> +    case AV_OPT_TYPE_STRING:
> +        range->component_min = 0;

> +        range->component_max = 0x10FFFF;

just curious, what this value represents? An inline comment may also help.

> +        range->value_min = -1;
> +        range->value_max = INT_MAX;
> +        break;
> +    case AV_OPT_TYPE_RATIONAL:
> +        range->component_min = INT_MIN;
> +        range->component_max = INT_MAX;
> +        break;
> +    case AV_OPT_TYPE_IMAGE_SIZE:
> +        range->component_min = 0;
> +        range->component_max = INT_MAX/128/8;
> +        range->value_min = 0;
> +        range->value_max = INT_MAX/8;
> +        break;
> +    default:
> +        goto fail;
> +    }
> +
> +    return ranges;
> +fail:
> +    av_free(ranges);
> +    av_free(range);
> +    return NULL;
> +}
> +
> +void av_opt_freep_ranges(AVOptionRanges **rangesp) {
> +    int i;
> +    AVOptionRanges *ranges = *rangesp;
> +
> +    for(i=0; i<ranges->nb_ranges; i++){
> +        AVOptionRange *range = ranges->range[i];
> +        av_freep(&range->str);
> +        av_freep(&ranges->range[i]);
> +    }
> +    av_freep(&ranges->range);
> +    av_freep(rangesp);
> +}
> +
>  #ifdef TEST
>  
>  #undef printf
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index bf84a9b..742aaec 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -293,6 +293,25 @@ typedef struct AVOption {
>      const char *unit;
>  } AVOption;
>  
> +/**
> + * A single allowed range of values, or a single allowed value.
> + */
> +typedef struct AVOptionRange {
> +    const char *str;

> +    double value_min, value_max;     ///< For string ranges this represents the min/max length, for dimensions this represents the min/max pixel count
> +    double component_min, component_max;     ///< For string this represents the unicode range for chars, 0-127 limits to ASCII

I'm not yet convinced that this is actually useful in case of strings,
checks are usually performed in the setting code so the exposed
constraints are doomed to stay inconsistent.

> +    int is_range;       ///< 1->range, 0->single value

Less compressed (saves some decoding on the user side):
///< if set to 1 the struct encodes a range, if set to 0 a single value

> +} AVOptionRange;
> +
> +/**
> + * List of AVOptionRange structs
> + */
> +typedef struct AVOptionRanges {
> +    AVOptionRange **range;
> +    int nb_ranges;
> +} AVOptionRanges;
> +
> +
>  #if FF_API_FIND_OPT
>  /**
>   * Look for an option in obj. Look only for the options which
> @@ -672,6 +691,37 @@ int av_opt_get_sample_fmt(void *obj, const char *name, int search_flags, enum AV
>   *          or written to.
>   */
>  void *av_opt_ptr(const AVClass *avclass, void *obj, const char *name);
> +
> +/**

> + * Free a AVOptionRanges struct and set it to NULL.

Nit++: Free an AVOptionRanges?

> + */
> +void av_opt_freep_ranges(AVOptionRanges **ranges);
> +
> +/**
> + * Get a list of allowed ranges for the given option.
> + *
> + * The returned list may depend on other fields in obj like for example profile.
> + *
> + * @param flags is a bitmask of flags, undefined flags should not be set and should be ignored
> + *              AV_OPT_SEARCH_FAKE_OBJ indicates that the obj is a double pointer to a AVClass instead of a full instance
> + *
> + * The result must be freed with av_opt_freep_ranges

Nit: missing final dot.

> + */
> +AVOptionRanges *av_opt_query_ranges(void *obj, const char *key, int flags);

Regarding the return value: I prefer to handle an int, rather than
picking and propagating an arbitrary error code, also should improve
coding consistency.

> +
> +/**
> + * Get a default list of allowed ranges for the given option.
> + *

> + * This list is constructed without using the callback and can be used as
> + * fallback from within the callback.

This may be clarified, what callback is referenced?

> + *
> + * @param flags is a bitmask of flags, undefined flags should not be set and should be ignored
> + *              AV_OPT_SEARCH_FAKE_OBJ indicates that the obj is a double pointer to a AVClass instead of a full instance
> + *
> + * The result must be freed with av_opt_free_ranges

Nit+: missing final dot.
-- 
FFmpeg = Fanciful and Fabulous Merciful Patchable Enlightening Geisha


More information about the ffmpeg-devel mailing list