Hi Ricardo,
On 09/11/2013 10:30 AM, Ricardo Ribalda Delgado wrote:
> A new id field is added to the struct selection. On devices that
> supports multiple sections this id indicate which of the selection to
> modify.
>
> A new control V4L2_CID_SELECTION_BITMASK selects which of the selections
> are used, if the control is set to zero the default rectangles are used.
>
> This is needed in cases where the user has to change multiple selections
> at the same time to get a valid combination.
>
> On devices where the control V4L2_CID_SELECTION_BITMASK does not exist,
> the id field is ignored
This feels like a hack to me. A big problem I have with using a control here
is that with a control you can't specify for which selection target it is.
If you want to set multiple rectangles, why not just pass them directly? E.g.:
struct v4l2_selection {
__u32 type;
__u32 target;
__u32 flags;
union {
struct v4l2_rect r;
struct v4l2_rect *pr;
};
__u32 rectangles;
__u32 reserved[8];
};
If rectangles > 1, then pr is used.
It's a bit more work to add this to the core code (VIDIOC_SUBDEV_G/S_SELECTION
should probably be changed at the same time and you have to fix existing drivers
to check/set the new rectangles field), but it scales much better.
Regards,
Hans
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@xxxxxxxxx>
> ---
> Documentation/DocBook/media/v4l/controls.xml | 6 ++++++
> drivers/media/v4l2-core/v4l2-ctrls.c | 2 ++
> drivers/media/v4l2-core/v4l2-ioctl.c | 4 ++--
> include/uapi/linux/v4l2-controls.h | 4 +++-
> include/uapi/linux/videodev2.h | 5 ++++-
> 5 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
> index c2fc9ec..a8c84bb 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -338,6 +338,12 @@ minimum value disables backlight compensation.</entry>
> coefficients determined by <constant>V4L2_CID_COLORFX_CBCR</constant>
> control.</entry>
> </row>
> + <row>
> + <entry><constant>V4L2_CID_SELECTION_BITMASK</constant> </entry>
> + <entry>For drivers that support multiple selections. This control
> + determine which selections are enabled. If it is set to zero the default
> + crop and compose rectangle are used.</entry>
> + </row>
> </tbody>
> </entrytbl>
> </row>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index fccd08b..fa3bfc2 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -599,6 +599,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT: return "Min Number of Output Buffers";
> case V4L2_CID_ALPHA_COMPONENT: return "Alpha Component";
> case V4L2_CID_COLORFX_CBCR: return "Color Effects, CbCr";
> + case V4L2_CID_SELECTION_BITMASK: return "Selection Bitmask";
>
> /* MPEG controls */
> /* Keep the order of the 'case's the same as in videodev2.h! */
> @@ -957,6 +958,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_DV_TX_RXSENSE:
> case V4L2_CID_DV_TX_EDID_PRESENT:
> case V4L2_CID_DV_RX_POWER_PRESENT:
> + case V4L2_CID_SELECTION_BITMASK:
> *type = V4L2_CTRL_TYPE_BITMASK;
> break;
> case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 68e6b5e..ca177bb 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -573,9 +573,9 @@ static void v4l_print_selection(const void *arg, bool write_only)
> {
> const struct v4l2_selection *p = arg;
>
> - pr_cont("type=%s, target=%d, flags=0x%x, wxh=%dx%d, x,y=%d,%d\n",
> + pr_cont("type=%s, target=%d, flags=0x%x, id=%d,wxh=%dx%d, x,y=%d,%d\n",
> prt_names(p->type, v4l2_type_names),
> - p->target, p->flags,
> + p->target, p->flags, p->id,
> p->r.width, p->r.height, p->r.left, p->r.top);
> }
>
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index e90a88a..ca44338 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -138,8 +138,10 @@ enum v4l2_colorfx {
> #define V4L2_CID_ALPHA_COMPONENT (V4L2_CID_BASE+41)
> #define V4L2_CID_COLORFX_CBCR (V4L2_CID_BASE+42)
>
> +#define V4L2_CID_SELECTION_BITMASK (V4L2_CID_BASE+43)
> +
> /* last CID + 1 */
> -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+43)
> +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+44)
>
> /* USER-class private control IDs */
>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 0e80472..4f68196 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -806,6 +806,8 @@ struct v4l2_crop {
> * @target: Selection target, used to choose one of possible rectangles;
> * defined in v4l2-common.h; V4L2_SEL_TGT_* .
> * @flags: constraints flags, defined in v4l2-common.h; V4L2_SEL_FLAG_*.
> + * @id: Selection to change, for devices that supports multiple,
> + * set to zero otherwise.
> * @r: coordinates of selection window
> * @reserved: for future use, rounds structure size to 64 bytes, set to zero
> *
> @@ -818,7 +820,8 @@ struct v4l2_selection {
> __u32 target;
> __u32 flags;
> struct v4l2_rect r;
> - __u32 reserved[9];
> + __u32 id;
> + __u32 reserved[8];
> };
>
>
>
--
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
