Hello Hans Thanks for you promptly response On Wed, Feb 12, 2014 at 1:40 PM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > 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? I dont like that the type contains the size of the element, and then I have to provide the size again. (Hungarian notation) Instead, I think it is better: Defines ONLY this two types for simple types: V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER and V4L2_CTRL_COMPLEX_TYPE_UNSIGNED_INTEGER and use elem_size to determine the size. And then one define per "structured types" ie: V4L2_CTRL_COMPLEX_TYPE_POINT V4L2_CTRL_COMPLEX_TYPE_IRRATIONAL.. with elem_size determining the size. But if you dont like that idea, as second preference then I think elem_size should be filled by the subsystem for simple types. Thanks! > >> 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 -- Ricardo Ribalda -- 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
