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

[Michael Niedermayer]

On Mon, Nov 26, 2012 at 12:32:32AM +0100, Stefano Sabatini wrote:
> On date Sunday 2012-11-25 17:39:36 +0100, Michael Niedermayer encoded:
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
[...]
> > +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).

obj is a instance, maybe i misunderstand you ?


> 
> > +
> > +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?

the component range is the range of the component of the vector
the value range is the range in count of pixels


[...]

> > +/**
> > + * 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).

that would make it quite inconsistent. A string is a vector of symbols
why should the component range be stored elsewhere for this case ?

ill look into fixing the other issues you mentioned ASAP

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- 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/20121126/5f02ba56/attachment.asc>