[FFmpeg-devel] [PATCH 4/8] lavu/opt: extend AVOptionRange by second value

[Lukasz Marek]

>>   opt.c |   33 ++++++++++++++++++++++++---------
>>   opt.h |   13 +++++++++++--
>>   2 files changed, 35 insertions(+), 11 deletions(-)
>> e560f163c6f61b5819e9f382615877e0bdfe12fe  0001-lavu-opt-extend-AVOptionRange-by-extra-values.patch
>>  From 702508c0c97812ed72deaa133140291fe3637196 Mon Sep 17 00:00:00 2001
>> From: Lukasz Marek <lukasz.m.luki at gmail.com>
>> Date: Sat, 22 Feb 2014 23:32:57 +0100
>> Subject: [PATCH] lavu/opt: extend AVOptionRange by extra values
>>
>> TODO: micro bump
>>
>> AVOptionRange is not flexible enough to store AV_OPT_TYPE_IMAGE_SIZE
>> ranges. Current implementation can only store pixel count.
>> This patch aims to keep backward compatibility and extend
>> AVOptionRange with possibility to store width/height ranges.
>>
>> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
>> ---
>>   libavutil/opt.c | 33 ++++++++++++++++++++++++---------
>>   libavutil/opt.h | 13 +++++++++++--
>>   2 files changed, 35 insertions(+), 11 deletions(-)
>>
>> diff --git a/libavutil/opt.c b/libavutil/opt.c
>> index 652a2dd..77d20b9 100644
>> --- a/libavutil/opt.c
>> +++ b/libavutil/opt.c
>> @@ -1511,6 +1511,7 @@ void *av_opt_ptr(const AVClass *class, void *obj, const char *name)
>>
>>   int av_opt_query_ranges(AVOptionRanges **ranges_arg, void *obj, const char *key, int flags)
>>   {
>> +    int ret, i;
>>       const AVClass *c = *(AVClass**)obj;
>>       int (*callback)(AVOptionRanges **, void *obj, const char *key, int flags) = NULL;
>>
>> @@ -1520,7 +1521,14 @@ int av_opt_query_ranges(AVOptionRanges **ranges_arg, void *obj, const char *key,
>>       if (!callback)
>>           callback = av_opt_query_ranges_default;
>>
>> -    return callback(ranges_arg, obj, key, flags);
>> +    ret = callback(ranges_arg, obj, key, flags);
>> +    if (ret >= 0) {
>> +        if (!(flags & AV_OPT_MULTI_COMPINENT_RANGE))
>> +            ret = 1;
>
>> +        for (i = 0; i < ret; i++)
>> +            (*ranges_arg)[i].nb_components = ret;
>
> doesnt this  depends on sizeof(AVOptionRanges) ?
> sizeof(AVOptionRanges) is not and cannot be part of the public ABI
> so iam not sure how this can be accessed by a user

Yes, it does depend on sizeof(AVOptionRanges)

>>   /**
>> + *  Allows av_opt_query_ranges and av_opt_query_ranges_default to return more than
>> + *  one instance of AVOptionRanges.
>> + */
>> +#define AV_OPT_MULTI_COMPINENT_RANGE 0x0004
>
> what is a compInent ?
> also how and where were the ranges defined for multi component options
> before?
> If there where not defined at all then a change is possibly not a
> API/ABI break and this can be simplified away

It is not documented yet.

> also what do multiple component ranges mean exactly?
> range1: 320,640
> range2: 240,480
>
> means 320x240 || 640x480
> or
> means 320x240 || 320x480 || 640x240 || 640x480
> (this should be documented, and one of these looks insufficient)

I have first option in mind. Each AVOptionRanges must have the same 
value on nb_ranges. So valid ranges are:

{ AVOptionRanges[0].range[0], AVOptionRanges[1].range[0], ... },
{ AVOptionRanges[0].range[1], AVOptionRanges[1].range[1], ... },
...

Please elaborate, because it don't know what case you have in mind this 
time.

> also you can put any N-Dimensional array in a 1d array like:
> ranges[component*nb_ranges + range] or
> ranges[componen + range*nb_components]

This may help to get rid of sizeof(AVOptionRanges) dependency, thanks 
for hint.

I will fix it somehow, but please answer for question above.