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

[Lukasz Marek]

On 26.02.2014 17:22, Michael Niedermayer wrote:
> On Wed, Feb 26, 2014 at 01:55:53PM +0100, Lukasz M wrote:
>> On 26 February 2014 12:41, Nicolas George <george at nsup.org> wrote:
>>
>>> L'octidi 8 ventôse, an CCXXII, Lukasz M a écrit :
>>>> Hmm, I treated it a bit different, but I think your proposition is
>>> correct.
>>>> So for example for size opt value would be number of pixels. components
>>>> would define ranges of width and height?
>>>
>>> Maybe I missed something, but I suspect you should completely ignore the
>>> component_min/max part.
>>>
>>
>> I'm not sure how it is supposed to be extended.
>> It was a question to Michael, because it is not clear and both options make
>> sense - depending how you interpret them.
>> But my first guess was also to ignore component part
>>
>>
>>>> I share your opinion. But technically this is api break. At major bump
>>>> extra_value_min can be removed and value_min/max can be array as in my
>>>> first patch.
>>>> On the other hand I believe in practice it would be better to change it
>>>> now. I doubt query_ranges is commonly used so far outside ffmpeg. When
>>> dev
>>>> cap api starts relying on it, users may start to use it more.
>>>
>>> Is there really a benefit in having a real array, i.e. being able to handle
>>> both values at once? If not, you could just add value2_min/max and be done
>>> with it. That has the advantage of not polluting the normal code with
>>> "[0]".
>>>
>>
>> OK, I can change it this way.
>>
>>
>
>>> Also, is it worth the complexity? IIRC, you did not respond to the option
>>> of
>>> leaving width and height separate, and just set width to get the
>>> corresponding heights.
>>>
>>
>> No, I haven't. Old version would work as you described (query width, set
>> width, query height),
>> Michael gave a hints about extending AVOptionRange struct and I think it is
>> better than previous solution.
>>  From client point of view it seems to have less complexity.
>
> yes, i think keeping the client complexity in mind is important as
> one developer had complained about the API
>
> the alternative to changing AVOptionRange would possibly be to add
> a function which can take 2 or more AVOptions and returns a list of
> their allowed values or value ranges
> This function would then querry the "One component" style API
> to create a list of allowed pairs/pair ranges
>
> That could then also be used to create widthxheight fps triplets
> if these depend on each other

I think if you went a bit further it would go back to the situation 
which we wanted to avoid. To have one big structure represented as 
massive amount of combinations of AVOptionRanges. Combining width with 
height is ok, as they are in fact strongly linked (you cannot create 
window with just width or height), fps can be queried separately.

Could you come back to the statement where you suggested width/height 
should be stored in "component", not "value". Me and Nicolas had first 
thought it should be in "values". I will work on it tommorow probably 
and prefer to have an answer in front in this matter.

> also i just realized that AVOptionRange probably should have a
> "multiple_of" field so that for example even width can be specified
> without having to specify each individual value in a list

OK, this may be useful. It is very probable video devices may get lost 
with odd width/heights

-- 
Best Regards,
Lukasz Marek

I may be drunk, Miss, but in the morning I will be sober and you will 
still be ugly. - Winston Churchill