On 23/01/14 15:23, Hans Verkuil wrote:
> On 01/23/2014 12:02 AM, Sylwester Nawrocki wrote:
>> On 01/20/2014 01:45 PM, Hans Verkuil wrote:
>>> From: Hans Verkuil<hans.verk...@cisco.com>
>>>
>>> Add a new struct and ioctl to extend the amount of information you can
>>> get for a control.
>>>
>>> It gives back a unit string, the range is now a s64 type, and the matrix
>>> and element size can be reported through cols/rows/elem_size.
>>>
>>> Signed-off-by: Hans Verkuil<hans.verk...@cisco.com>
>>> ---
>>>   include/uapi/linux/videodev2.h | 30 ++++++++++++++++++++++++++++++
>>>   1 file changed, 30 insertions(+)
>>>
>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>> index 4d7782a..9e5b7d4 100644
>>> --- a/include/uapi/linux/videodev2.h
>>> +++ b/include/uapi/linux/videodev2.h
>>> @@ -1272,6 +1272,34 @@ struct v4l2_queryctrl {
>>>     __u32                reserved[2];
>>>   };
>>>
>>> +/*  Used in the VIDIOC_QUERY_EXT_CTRL ioctl for querying extended controls 
>>> */
>>> +struct v4l2_query_ext_ctrl {
>>> +   __u32                id;
>>> +   __u32                type;
>>> +   char                 name[32];
>>> +   char                 unit[32];
>>
>>> +   union {
>>> +           __s64 val;
>>> +           __u32 reserved[4];
>>> +   } min;
>>> +   union {
>>> +           __s64 val;
>>> +           __u32 reserved[4];
>>> +   } max;
>>> +   union {
>>> +           __u64 val;
>>> +           __u32 reserved[4];
>>> +   } step;
>>> +   union {
>>> +           __s64 val;
>>> +           __u32 reserved[4];
>>> +   } def;
>>
>> Are these reserved[] arrays of any use ?
> 
> Excellent question. I'd like to know as well :-)
> 
> The idea is that if the type of the control is complex, then for certain types
> it might still make sense to have a range. E.g. say that the type is 
> v4l2_rect,
> then you can define min/max/step/def v4l2_rect entries in the unions. Ditto
> for a v4l2_fract (it would be nice to be able to specify the min/max allowed
> scaling factors, for example).

Huh, sorry, I misread the patch. Please ignore this comment.

Certainly we need an ability to query other compound control types as well.
16 bytes seems a reasonable size, I guess it is going to be sufficient for
most cases. If not we could add a pointer member to the union...?

> The question is, am I over-engineering or is this the best idea since sliced
> bread? Without the 'reserved' part this idea will be impossible to implement,
> and I don't think it hurts to have it in.

Yes, that's how I imagined it as well, I didn't mean questioning the union
idea at all.

>>> +   __u32                flags;
>>> +   __u32                cols, rows;
>>
>> nit: I would put them on separate lines and use full words.
> 
> Separate lines: no problem, but do I really have to write 'columns' in full? 
> :-(

Yes, sorry, one shall abide by the rules! :-)

Really, it's up to you - as the author, I think you're entitled to decide
about such details. ;) The short version looks probably neater anyway.

--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to