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

[Stefano Sabatini]

On date Sunday 2012-11-25 17:39:36 +0100, Michael Niedermayer encoded:
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libavutil/log.h |    6 +++++
>  libavutil/opt.c |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libavutil/opt.h |   48 ++++++++++++++++++++++++++++++++++
>  3 files changed, 132 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 (???)

I wonder if this is a good idea. I'd prefer to update APIchanges, or
we end with lots of versioning annotation cruft in doxy.

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

What about:
int *(*query_ranges)(void *obj, struct AVOptionRanges **r, const char *key, int flags);

so it would be possible to distinguish amongst several failure reasons
(NOMEM, missing option, others?).

>  } AVClass;
>  
>  /* av_log API */
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 7a7f893..d1825fd 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -1027,6 +1027,84 @@ 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))
> +        callback = c->query_ranges;
> +
> +    if(!callback)
> +        callback = av_opt_query_ranges_default;
> +
> +    return callback(obj, key, flags);
> +}

Note that this can't fix the list_devices problem, since that requires
an already inited *instance* of a device (what we usually do, we
initialize the device given the params, query the device, log the
result).  This interface allows to query the class, but not the single
instance (which usually requires to provide at least a few params -
e.g. the device name).

> +
> +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, 0);
> +
> +    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[0].dbl = field->min;
> +    range->value[1].dbl = 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[0].dbl = 0;
> +        range->component[1].dbl = 0x10FFFF;
> +        range->value[0].dbl = -1;
> +        range->value[1].dbl = INT_MAX;
> +        break;
> +    case AV_OPT_TYPE_RATIONAL:
> +        range->component[0].dbl = INT_MIN;
> +        range->component[1].dbl = INT_MAX;
> +        break;

> +    case AV_OPT_TYPE_IMAGE_SIZE:
> +        range->component[0].dbl = 0;
> +        range->component[1].dbl = INT_MAX/128/8;
> +        range->value[0].dbl = 0;
> +        range->value[1].dbl = INT_MAX/8;
> +        break;

Uh? What's the meaning of "range" in case of an image?

> +    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];
> +        if (!range->is_range)
> +            av_freep(&range->value[0].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 c168f59..934d723 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -291,6 +291,29 @@ typedef struct AVOption {
>      const char *unit;
>  } AVOption;
>  

> +/**
> + * A single allowed range of values, or a single allowed value.
> + */
> +typedef struct AVOptionRange {
> +    union {
> +        double dbl;     ///< For string ranges this represents the min/max length
> +        const char *str;
> +    } value[2];
> +    union {
> +        double dbl;     ///< For string this represents the unicode range for chars, 0-127 limits to ASCII
> +    } component[2];
> +    int is_range;       ///< 1->range, 0->single value
> +} AVOptionRange;

Honestly I find this struct definition a bit confusing (certainly more
doxy may help. Also mix/max looks much more readable than
component[0]/component[1], and I'm unsure that the range of the single
char is really so much important (and even if it is, I'd rather have a
dedicated field min/max_char_value rather than abusing the dbl field).

> +
> +/**
> + * 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
> @@ -664,6 +687,31 @@ int av_opt_get_q     (void *obj, const char *name, int search_flags, AVRational
>   *          or written to.
>   */
>  void *av_opt_ptr(const AVClass *avclass, void *obj, const char *name);
> +

> +/**
> + * Frees a AVOptionRanges struct and sets it to NULL
> + */
> +void av_opt_freep_ranges(AVOptionRanges **ranges);

Nit: Free ... set, missing dot, here and below.

> +
> +/**
> + * Gets a list of allowed ranges for the given option.
> + *
> + * The returned list may depend on other fields in obj like for example profile.
> + *
> + * The result must be freed with av_opt_freep_ranges
> + */
> +AVOptionRanges *av_opt_query_ranges(void *obj, const char *key, int flags);

flags is undocumented, here and below.

> +
> +/**
> + * Gets 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.
> + *
> + * The result must be freed with av_opt_free_ranges
> + */
> +AVOptionRanges *av_opt_query_ranges_default(void *obj, const char *key, int flags);
-- 
FFmpeg = Fancy Faithless Minimal Puritan Enhancing Ghost