Re: [REVIEWv2 PATCH 24/34] v4l2-ctrls/videodev2.h: add u8 and u16 types.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/12/14 13:11, Ricardo Ribalda Delgado wrote:
> Hi Hans
> 
> Thanks for your reply
> 
> On Wed, Feb 12, 2014 at 12:20 PM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>> Hi Ricardo,
>>
>> On 02/12/14 11:44, Ricardo Ribalda Delgado wrote:
>>> Hello Hans
>>>
>>> In the case of U8 and U16 data types. Why dont you fill the elem_size
>>> automatically in v4l2_ctrl and request the driver to fill the field?
>>
>> When you create the control the control framework has to know the element
>> size beforehand as it will use that to allocate the memory containing the
>> control's value. The control framework is aware of the 'old' control types
>> and will fill in the elem_size accordingly, but it cannot do that in the
>> general case for these complex types. I guess it could be filled in by the
>> framework for the more common types (U8, U16) but I felt it was more
>> consistent to just require drivers to fill it in manually, rather than have
>> it set for some types but not for others.
>>
>>>
>>> Other option would be not declaring the basic data types (U8, U16,
>>> U32...) and use elem_size. Ie. If type==V4L2_CTRL_COMPLEX_TYPES, then
>>> the type is basic and elem_size is the size of the type. If the type
>>>> V4L2_CTRL_COMPLEX_TYPES the type is not basic.
>>
>> You still need to know the type. Applications have to be able to check for
>> the type, the element size by itself doesn't tell you how to interpret the
>> data, you need the type identifier as well.
> 
> I think that the driver is setting twice the same info. I see no gain
> in declaring U8, U16 types etc if we still have to set the element
> size. This is why I believe that we should only declare the "structs".

Just to make sure I understand you: for simple types like U8/U16 you want
the control framework to fill in elem_size, for more complex types (structs)
you want the driver to fill in elem_size?
 
> what about something like: V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER +
> size, V4L2_CTRL_COMPLEX_TYPES_UNSIGNED_INTEGER + size.... instead of
> V4L2_CTRL_COMPLEX_TYPES_U8, V4L2_CTRL_COMPLEX_TYPES_U16,
> V4L2_CTRL_COMPLEX_TYPES_U32, V4L2_CTRL_COMPLEX_TYPES_S8 ....
> 
> Btw, I am trying to implement a dead pixel control on the top of you
> api. Shall I wait until you patchset is merged or shall I send the
> patches right away?

You're free to experiment, but I am not going to ask Mauro to pull additional
patches as long as this initial patch set isn't merged.

Regards,

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




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux