[FFmpeg-devel] [PATCH 2/4] lavd: add device capabilities API

[Lukasz Marek]

On 17.02.2014 18:55, Michael Niedermayer wrote:
> On Mon, Feb 17, 2014 at 06:45:58PM +0100, Michael Niedermayer wrote:
>> On Mon, Feb 17, 2014 at 05:14:32PM +0100, Lukasz M wrote:
>>> On 16 February 2014 18:45, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>
>>>> On Sun, Feb 16, 2014 at 01:28:03PM +0100, Lukasz Marek wrote:
>>>>>
>>>>>>> This version still has one open issue: how does it express a limited
>>>> set of
>>>>>>> frame sizes. For example, if a webcam supports exactly 640×480 and
>>>> 320×240.
>>>>>>> Does it return { 320, 640 } for the width and { 240, 480 } for the
>>>> height?
>>>>>>> Is the application supposed to try each width in turn and query the
>>>>>>> corresponding range for height? It works, although it is a bit
>>>> cumbersome,
>>>>>>> but it is not intuitive: maybe add a comment, or do it like that in the
>>>>>>> examples.
>>>>>>
>>>>>> maybe AV_OPT_TYPE_IMAGE_SIZE, should be used
>>>>>
>>>>> Can AV_OPT_TYPE_IMAGE_SIZE be represented in AVOptionRange?
>>>>> I'm asking because I'm willing to change that, but not sure it is
>>>>> doable without changing AVOptionRange struct.
>>>>
>>>> it was certainly intended to be representable
>>>> see for example opt_list()
>>>>
>>>> and of course things can be done differently if needed
>>>
>>>
>>> I did check it quickly and it seems image size is represented as string
>>> ("640x480" for example)
>>> I haven't found how range of image sizes should be represented.
>>>
>>> If there is no format defined then I can propose something like that
>>> "100x100-640x480"
>>> or
>>> "100-640x100-480"
>>
>> typedef struct AVOptionRange {
>>      const char *str;
>>      double value_min, value_max;             ///< For string ranges this represents the min/max length, for dimensions this represents the min/max pixel count
>>      double component_min, component_max;     ///< For string this represents the unicode range for chars, 0-127 limits to ASCII
>>
>> so the current API would allow limiting the pixel count or both w,h
>> against the same nummeric value. Thats possibly not flexible enough
>
> the obvious fix for this would be to make component_min/max an array
> of 2 or add a 2nd component_max/min

It is not used in many places I think and that change would be quite 
easy. I'm not convinced second component_min/max is really required.
And other option is to add component2_min/max witch would be even more 
easy change.

-- 
Best Regards,
Lukasz Marek

Insanity: doing the same thing over and over again and expecting 
different results. - Albert Einstein