Re: [RFCv2 PATCH 07/11] v4l2-ctrls: add control events.

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

 



Hi Hans,

On Wed, May 25, 2011 at 03:33:51PM +0200, Hans Verkuil wrote:
> @@ -1800,21 +1801,45 @@ struct v4l2_event_vsync {
>  	__u8 field;
>  } __attribute__ ((packed));
>  
> +/* Payload for V4L2_EVENT_CTRL */
> +#define V4L2_EVENT_CTRL_CH_VALUE		(1 << 0)
> +#define V4L2_EVENT_CTRL_CH_FLAGS		(1 << 1)
> +
> +struct v4l2_event_ctrl {
> +	__u32 changes;
> +	__u32 type;
> +	union {
> +		__s32 value;
> +		__s64 value64;
> +	};
> +	__u32 flags;
> +	__s32 minimum;
> +	__s32 maximum;
> +	__s32 step;
> +	__s32 default_value;
> +} __attribute__ ((packed));
> +

One more comment.

Do we really need type and default_value in the event? They are static, and
on the other hand, the type should be already defined by the control so
that's static, as I'd expect default_value would be.

It just looks like this attempts to reimplement what QUERYCTRL does. :-)
Step, min and max values may change, so they are good.

More fields can be added later on. User space libraries / applications using
this structure might have different views of its size, though, depending
which definition they used at compile time. So in principle also this
structure should have reserved fields, although not having such and still
changing it might not have any adverse effects at all.

Btw. why __attribute__ ((packed))?

Regards,

-- 
Sakari Ailus
sakari dot ailus at iki dot fi
--
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