[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190925083043.7dt6f7hahmoap3vt@uno.localdomain>
Date: Wed, 25 Sep 2019 10:30:43 +0200
From: Jacopo Mondi <jacopo@...ndi.org>
To: Ricardo Ribalda Delgado <ricardo@...alda.com>
Cc: Philipp Zabel <p.zabel@...gutronix.de>,
Hans Verkuil <hverkuil-cisco@...all.nl>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 3/7] media: add V4L2_CTRL_TYPE_AREA control type
Hi Ricardo,
On Fri, Sep 20, 2019 at 03:51:33PM +0200, Ricardo Ribalda Delgado wrote:
> This type contains the width and the height of a rectangular area.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo@...alda.com>
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 21 ++++++++++++++
> include/media/v4l2-ctrls.h | 41 ++++++++++++++++++++++++++++
> include/uapi/linux/videodev2.h | 6 ++++
> 3 files changed, 68 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 219d8aeefa20..b9a46f536406 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1678,6 +1678,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> struct v4l2_ctrl_mpeg2_slice_params *p_mpeg2_slice_params;
> struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
> void *p = ptr.p + idx * ctrl->elem_size;
> + struct v4l2_area *area;
>
> switch ((u32)ctrl->type) {
> case V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS:
> @@ -1753,6 +1754,11 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> zero_padding(p_vp8_frame_header->entropy_header);
> zero_padding(p_vp8_frame_header->coder_state);
> break;
> + case V4L2_CTRL_TYPE_AREA:
> + area = p;
> + if (!area->width || !area->height)
> + return -EINVAL;
> + break;
> default:
> return -EINVAL;
> }
> @@ -2427,6 +2433,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
> case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
> elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
> break;
> + case V4L2_CTRL_TYPE_AREA:
> + elem_size = sizeof(struct v4l2_area);
> + break;
> default:
> if (type < V4L2_CTRL_COMPOUND_TYPES)
> elem_size = sizeof(s32);
> @@ -4116,6 +4125,18 @@ int __v4l2_ctrl_s_ctrl_string(struct v4l2_ctrl *ctrl, const char *s)
> }
> EXPORT_SYMBOL(__v4l2_ctrl_s_ctrl_string);
>
> +int __v4l2_ctrl_s_ctrl_area(struct v4l2_ctrl *ctrl,
> + const struct v4l2_area *area)
> +{
> + lockdep_assert_held(ctrl->handler->lock);
> +
> + /* It's a driver bug if this happens. */
> + WARN_ON(ctrl->type != V4L2_CTRL_TYPE_AREA);
> + memcpy(ctrl->p_new.p_area, area, sizeof(*area));
> + return set_ctrl(NULL, ctrl, 0);
> +}
> +EXPORT_SYMBOL(__v4l2_ctrl_s_ctrl_area);
> +
> void v4l2_ctrl_request_complete(struct media_request *req,
> struct v4l2_ctrl_handler *main_hdl)
> {
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index 4b356df850a1..746969559ef3 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -50,6 +50,7 @@ struct poll_table_struct;
> * @p_h264_slice_params: Pointer to a struct v4l2_ctrl_h264_slice_params.
> * @p_h264_decode_params: Pointer to a struct v4l2_ctrl_h264_decode_params.
> * @p_vp8_frame_header: Pointer to a VP8 frame header structure.
> + * @p_area: Pointer to an area.
> * @p: Pointer to a compound value.
> */
> union v4l2_ctrl_ptr {
> @@ -68,6 +69,7 @@ union v4l2_ctrl_ptr {
> struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
> struct v4l2_ctrl_h264_decode_params *p_h264_decode_params;
> struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
> + struct v4l2_area *p_area;
> void *p;
> };
>
> @@ -1085,6 +1087,45 @@ static inline int v4l2_ctrl_s_ctrl_string(struct v4l2_ctrl *ctrl, const char *s)
> return rval;
> }
>
> +/**
> + * __v4l2_ctrl_s_ctrl_area() - Unlocked variant of v4l2_ctrl_s_ctrl_area().
> + *
> + * @ctrl: The control.
> + * @area: The new area.
> + *
> + * This sets the control's new area safely by going through the control
> + * framework. This function assumes the control's handler is already locked,
> + * allowing it to be used from within the &v4l2_ctrl_ops functions.
> + *
> + * This function is for area type controls only.
> + */
> +int __v4l2_ctrl_s_ctrl_area(struct v4l2_ctrl *ctrl,
> + const struct v4l2_area *area);
> +
> +/**
> + * v4l2_ctrl_s_ctrl_area() - Helper function to set a control's area value
> + * from within a driver.
> + *
> + * @ctrl: The control.
> + * @s: The new area.
> + *
> + * This sets the control's new area safely by going through the control
> + * framework. This function will lock the control's handler, so it cannot be
> + * used from within the &v4l2_ctrl_ops functions.
> + *
> + * This function is for area type controls only.
> + */
> +static inline int v4l2_ctrl_s_ctrl_area(struct v4l2_ctrl *ctrl,
> + const struct v4l2_area *area)
> +{
> + int rval;
> +
> + v4l2_ctrl_lock(ctrl);
> + rval = __v4l2_ctrl_s_ctrl_area(ctrl, area);
> + v4l2_ctrl_unlock(ctrl);
> +
> + return rval;
> +}
Newline maybe?
> /* Internal helper functions that deal with control events. */
> extern const struct v4l2_subscribed_event_ops v4l2_ctrl_sub_ev_ops;
>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 530638dffd93..b3c0961b62a0 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -422,6 +422,11 @@ struct v4l2_fract {
> __u32 denominator;
> };
>
> +struct v4l2_area {
> + __u32 width;
> + __u32 height;
> +};
> +
> /**
> * struct v4l2_capability - Describes V4L2 device caps returned by VIDIOC_QUERYCAP
> *
> @@ -1720,6 +1725,7 @@ enum v4l2_ctrl_type {
> V4L2_CTRL_TYPE_U8 = 0x0100,
> V4L2_CTRL_TYPE_U16 = 0x0101,
> V4L2_CTRL_TYPE_U32 = 0x0102,
> + V4L2_CTRL_TYPE_AREA = 0x0106,
I'll let Hans comment on these two bits as I don't know if that's
adding a new control type is right or not.
Have my R-b for the rest though:
Reviewed-by: Jacopo Mondi <jacopo@...ndi.org>
Thanks
j
> };
>
> /* Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
> --
> 2.23.0
>
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists