Em Tue, 11 Mar 2014 21:23:26 +0100
Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
> Hi Mauro,
>
> On 03/11/2014 08:34 PM, Mauro Carvalho Chehab wrote:
> > Em Mon, 17 Feb 2014 10:57:19 +0100
> > Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
> >
> >> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >>
> >> Complex controls are controls that can be used for compound and array
> >> types. This allows for more complex data structures to be used with the
> >> control framework.
> >>
> >> Such controls always have the V4L2_CTRL_FLAG_HIDDEN flag set. Note that
> >> 'simple' controls can also set that flag.
> >>
> >> The existing V4L2_CTRL_FLAG_NEXT_CTRL flag will only enumerate controls
> >> that do not have the HIDDEN flag, so a new V4L2_CTRL_FLAG_NEXT_HIDDEN flag
> >> is added to enumerate hidden controls. Set both flags to enumerate any
> >> controls (hidden or not).
> >>
> >> Complex control types will start at V4L2_CTRL_COMPLEX_TYPES. In addition, any
> >> control that uses the new 'p' field or the existing 'string' field will have
> >> flag V4L2_CTRL_FLAG_IS_PTR set.
> >>
> >> While not strictly necessary, adding that flag makes life for applications
> >> a lot simpler. If the flag is not set, then the control value is set
> >> through the value or value64 fields of struct v4l2_ext_control, otherwise
> >> a pointer points to the value.
> >>
> >> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >> Reviewed-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> >> ---
> >> include/uapi/linux/videodev2.h | 11 +++++++++--
> >> 1 file changed, 9 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> >> index 6ae7bbe..4d7782a 100644
> >> --- a/include/uapi/linux/videodev2.h
> >> +++ b/include/uapi/linux/videodev2.h
> >> @@ -1228,6 +1228,7 @@ struct v4l2_ext_control {
> >> __s32 value;
> >> __s64 value64;
> >> char *string;
> >> + void *p;
> >
> > Hmm... don't we have already "string" for pointers? Also, calling it
> > as "p" inside an userspace api doesn't seem to nice ("ptr" would be
> > better).
> >
> > Btw, you likely already noticed the mess, as, when you added
> > this email's comment, you said that complex controls could
> > either use "p" or "string".
> >
> > Nack. It should just use "string". Let's not add even more complexity
> > to this "complex" controls.
>
> It is really, really weird to refer to a matrix type or struct type
> through a 'string'. Also, the type of 'string' is a char pointer
> which is totally wrong unless the type is really a string.
>
> Finally there is a slight difference in handling strings: the size of
> the memory 'string' points to just has to be large enough to store the
> 0-terminated string instead of being able to store the maximum possible
> size of the string (max + 1). This is IMHO a design mistake regarding
> strings on my part. I should always have required that the memory is
> always sizes for the worst-case.
I see. Ok, given that string means a NUL-terminated sequence, but we
should let it very clear at DocBook that the difference between
a STRING type and a "COMPLEX" type is that on the latter, the size
is determined by a field, and not by a NUL character.
>
> I might take another look if I can change this behavior without
> breaking existing applications.
>
> I don't mind changing 'p' to 'ptr', no problem.
>
> >
> >> };
> >> } __attribute__ ((packed));
> >>
> >> @@ -1252,7 +1253,10 @@ enum v4l2_ctrl_type {
> >> V4L2_CTRL_TYPE_CTRL_CLASS = 6,
> >> V4L2_CTRL_TYPE_STRING = 7,
> >> V4L2_CTRL_TYPE_BITMASK = 8,
> >> - V4L2_CTRL_TYPE_INTEGER_MENU = 9,
> >> + V4L2_CTRL_TYPE_INTEGER_MENU = 9,
> >> +
> >> + /* Complex types are >= 0x0100 */
> >> + V4L2_CTRL_COMPLEX_TYPES = 0x0100,
> >> };
> >
> > Not sure if I got why you're calling it as "TYPES" and saying that
> > everything >= 0x100 is complex. What's your idea here?
>
> Types divide into 'simple' types (the ones we have today) and the
> new complex (compound?) types. I need an easy way to determine
> which it is, so any types with values >= 0x100 fall into the latter
> category. Later in the patch series when such types are actually
> added the meaning becomes more obvious.
Ok.
> > Also, at least for me with my engineering formation, "complex"
> > means a number with an imaginary component.
> >
> > And yes, we do have complex numbers on some usecases for
> > V4L (for example, SDR in-phase/quadrature, e. g. I/Q
> > representation can be seen as an array of complex numbers).
> >
> > So, I won't doubt that someone might propose some day to add a
> > way to set a complex number via a V4L2 control.
> >
> > So, please use a better naming here to avoid troubles.
>
> COMPOUND_TYPES? Not a bad name, I think.
Sounds a better name for me.
> Regards,
>
> Hans
>
> >
> >> /* Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
> >> @@ -1288,9 +1292,12 @@ struct v4l2_querymenu {
> >> #define V4L2_CTRL_FLAG_SLIDER 0x0020
> >> #define V4L2_CTRL_FLAG_WRITE_ONLY 0x0040
> >> #define V4L2_CTRL_FLAG_VOLATILE 0x0080
> >> +#define V4L2_CTRL_FLAG_HIDDEN 0x0100
> >> +#define V4L2_CTRL_FLAG_IS_PTR 0x0200
> >>
> >> -/* Query flag, to be ORed with the control ID */
> >> +/* Query flags, to be ORed with the control ID */
> >> #define V4L2_CTRL_FLAG_NEXT_CTRL 0x80000000
> >> +#define V4L2_CTRL_FLAG_NEXT_HIDDEN 0x40000000
> >>
> >> /* User-class control IDs defined by V4L2 */
> >> #define V4L2_CID_MAX_CTRLS 1024
> >
> >
>
--
Regards,
Mauro
--
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
