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

[Lukasz Marek]

On 29.03.2014 21:24, Michael Niedermayer wrote:
> On Sat, Mar 29, 2014 at 07:06:28PM +0100, Lukasz Marek wrote:
>>>>   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.
>
> this one should work, yes
>
>
>
>>
>>> 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.

Changed patch attached. It is very similar to previous, but stores all 
components in one array as you hinted.
I haven't reimplemented opengl to test it, but I suppose there should be 
no problems with it.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavu-opt-extend-AVOptionRange-by-extra-values.patch
Type: text/x-patch
Size: 7415 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140330/e324b079/attachment.bin>