Re: [RFC PATCH 03/11] v4l2-ioctl: add QUIRK_INVERTED_CROP
On Fri, 5 Oct 2018 at 09:49, Hans Verkuil wrote: > > From: Hans Verkuil > > Some old Samsung drivers use the legacy crop API incorrectly: > the crop and compose targets are swapped. Normally VIDIOC_G_CROP > will return the CROP rectangle of a CAPTURE stream and the COMPOSE > rectangle of an OUTPUT stream. > > The Samsung drivers do the opposite. Note that these drivers predate > the selection API. > > If this 'QUIRK' flag is set, then the v4l2-ioctl core will swap > the CROP and COMPOSE targets as well. > > That way backwards compatibility is ensured and we can convert the > Samsung drivers to the selection API. > > Signed-off-by: Hans Verkuil Reviewed-by: Sylwester Nawrocki
Re: [RFC PATCH 03/11] v4l2-ioctl: add QUIRK_INVERTED_CROP
Hi Hans,
Thanks for your patch.
On 2018-10-05 09:49:03 +0200, Hans Verkuil wrote:
> From: Hans Verkuil
>
> Some old Samsung drivers use the legacy crop API incorrectly:
> the crop and compose targets are swapped. Normally VIDIOC_G_CROP
> will return the CROP rectangle of a CAPTURE stream and the COMPOSE
> rectangle of an OUTPUT stream.
>
> The Samsung drivers do the opposite. Note that these drivers predate
> the selection API.
>
> If this 'QUIRK' flag is set, then the v4l2-ioctl core will swap
> the CROP and COMPOSE targets as well.
>
> That way backwards compatibility is ensured and we can convert the
> Samsung drivers to the selection API.
>
> Signed-off-by: Hans Verkuil
I understand the need for this quirk but it make my head hurt :-)
Reviewed-by: Niklas Söderlund
> ---
> drivers/media/v4l2-core/v4l2-ioctl.c | 17 -
> include/media/v4l2-dev.h | 13 +++--
> 2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 9c2370e4d05c..63a92285de39 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2200,6 +2200,7 @@ static int v4l_s_selection(const struct v4l2_ioctl_ops
> *ops,
> static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
> struct file *file, void *fh, void *arg)
> {
> + struct video_device *vfd = video_devdata(file);
> struct v4l2_crop *p = arg;
> struct v4l2_selection s = {
> .type = p->type,
> @@ -2216,6 +2217,10 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
> else
> s.target = V4L2_SEL_TGT_CROP;
>
> + if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, >flags))
> + s.target = s.target == V4L2_SEL_TGT_COMPOSE ?
> + V4L2_SEL_TGT_CROP : V4L2_SEL_TGT_COMPOSE;
> +
> ret = v4l_g_selection(ops, file, fh, );
>
> /* copying results to old structure on success */
> @@ -2227,6 +2232,7 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
> static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
> struct file *file, void *fh, void *arg)
> {
> + struct video_device *vfd = video_devdata(file);
> struct v4l2_crop *p = arg;
> struct v4l2_selection s = {
> .type = p->type,
> @@ -2243,12 +2249,17 @@ static int v4l_s_crop(const struct v4l2_ioctl_ops
> *ops,
> else
> s.target = V4L2_SEL_TGT_CROP;
>
> + if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, >flags))
> + s.target = s.target == V4L2_SEL_TGT_COMPOSE ?
> + V4L2_SEL_TGT_CROP : V4L2_SEL_TGT_COMPOSE;
> +
> return v4l_s_selection(ops, file, fh, );
> }
>
> static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
> struct file *file, void *fh, void *arg)
> {
> + struct video_device *vfd = video_devdata(file);
> struct v4l2_cropcap *p = arg;
> struct v4l2_selection s = { .type = p->type };
> int ret = 0;
> @@ -2285,13 +2296,17 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops
> *ops,
> else
> s.target = V4L2_SEL_TGT_CROP_BOUNDS;
>
> + if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, >flags))
> + s.target = s.target == V4L2_SEL_TGT_COMPOSE_BOUNDS ?
> + V4L2_SEL_TGT_CROP_BOUNDS : V4L2_SEL_TGT_COMPOSE_BOUNDS;
> +
> ret = v4l_g_selection(ops, file, fh, );
> if (ret)
> return ret;
> p->bounds = s.r;
>
> /* obtaining defrect */
> - if (V4L2_TYPE_IS_OUTPUT(p->type))
> + if (s.target == V4L2_SEL_TGT_COMPOSE_BOUNDS)
> s.target = V4L2_SEL_TGT_COMPOSE_DEFAULT;
> else
> s.target = V4L2_SEL_TGT_CROP_DEFAULT;
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index 456ac13eca1d..48531e57cc5a 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -74,10 +74,19 @@ struct v4l2_ctrl_handler;
> * indicates that file->private_data points to v4l2_fh.
> * This flag is set by the core when v4l2_fh_init() is called.
> * All new drivers should use it.
> + * @V4L2_FL_QUIRK_INVERTED_CROP:
> + * some old M2M drivers use g/s_crop/cropcap incorrectly: crop and
> + * compose are swapped. If this flag is set, then the selection
> + * targets are swapped in the g/s_crop/cropcap functions in v4l2-ioctl.c.
> + * This allows those drivers to correctly implement the selection API,
> + * but the old crop API will still work as expected in order to preserve
> + * backwards compatibility.
> + * Never set this flag for new drivers.
> */
> enum v4l2_video_device_flags {
> - V4L2_FL_REGISTERED = 0,
> - V4L2_FL_USES_V4L2_FH= 1,
> + V4L2_FL_REGISTERED = 0,
> + V4L2_FL_USES_V4L2_FH= 1,
> + V4L2_FL_QUIRK_INVERTED_CROP = 2,
> };
>
> /*
[RFC PATCH 03/11] v4l2-ioctl: add QUIRK_INVERTED_CROP
From: Hans Verkuil
Some old Samsung drivers use the legacy crop API incorrectly:
the crop and compose targets are swapped. Normally VIDIOC_G_CROP
will return the CROP rectangle of a CAPTURE stream and the COMPOSE
rectangle of an OUTPUT stream.
The Samsung drivers do the opposite. Note that these drivers predate
the selection API.
If this 'QUIRK' flag is set, then the v4l2-ioctl core will swap
the CROP and COMPOSE targets as well.
That way backwards compatibility is ensured and we can convert the
Samsung drivers to the selection API.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-ioctl.c | 17 -
include/media/v4l2-dev.h | 13 +++--
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 9c2370e4d05c..63a92285de39 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2200,6 +2200,7 @@ static int v4l_s_selection(const struct v4l2_ioctl_ops
*ops,
static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
struct file *file, void *fh, void *arg)
{
+ struct video_device *vfd = video_devdata(file);
struct v4l2_crop *p = arg;
struct v4l2_selection s = {
.type = p->type,
@@ -2216,6 +2217,10 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
else
s.target = V4L2_SEL_TGT_CROP;
+ if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, >flags))
+ s.target = s.target == V4L2_SEL_TGT_COMPOSE ?
+ V4L2_SEL_TGT_CROP : V4L2_SEL_TGT_COMPOSE;
+
ret = v4l_g_selection(ops, file, fh, );
/* copying results to old structure on success */
@@ -2227,6 +2232,7 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
struct file *file, void *fh, void *arg)
{
+ struct video_device *vfd = video_devdata(file);
struct v4l2_crop *p = arg;
struct v4l2_selection s = {
.type = p->type,
@@ -2243,12 +2249,17 @@ static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
else
s.target = V4L2_SEL_TGT_CROP;
+ if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, >flags))
+ s.target = s.target == V4L2_SEL_TGT_COMPOSE ?
+ V4L2_SEL_TGT_CROP : V4L2_SEL_TGT_COMPOSE;
+
return v4l_s_selection(ops, file, fh, );
}
static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
struct file *file, void *fh, void *arg)
{
+ struct video_device *vfd = video_devdata(file);
struct v4l2_cropcap *p = arg;
struct v4l2_selection s = { .type = p->type };
int ret = 0;
@@ -2285,13 +2296,17 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
else
s.target = V4L2_SEL_TGT_CROP_BOUNDS;
+ if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, >flags))
+ s.target = s.target == V4L2_SEL_TGT_COMPOSE_BOUNDS ?
+ V4L2_SEL_TGT_CROP_BOUNDS : V4L2_SEL_TGT_COMPOSE_BOUNDS;
+
ret = v4l_g_selection(ops, file, fh, );
if (ret)
return ret;
p->bounds = s.r;
/* obtaining defrect */
- if (V4L2_TYPE_IS_OUTPUT(p->type))
+ if (s.target == V4L2_SEL_TGT_COMPOSE_BOUNDS)
s.target = V4L2_SEL_TGT_COMPOSE_DEFAULT;
else
s.target = V4L2_SEL_TGT_CROP_DEFAULT;
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 456ac13eca1d..48531e57cc5a 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -74,10 +74,19 @@ struct v4l2_ctrl_handler;
* indicates that file->private_data points to v4l2_fh.
* This flag is set by the core when v4l2_fh_init() is called.
* All new drivers should use it.
+ * @V4L2_FL_QUIRK_INVERTED_CROP:
+ * some old M2M drivers use g/s_crop/cropcap incorrectly: crop and
+ * compose are swapped. If this flag is set, then the selection
+ * targets are swapped in the g/s_crop/cropcap functions in v4l2-ioctl.c.
+ * This allows those drivers to correctly implement the selection API,
+ * but the old crop API will still work as expected in order to preserve
+ * backwards compatibility.
+ * Never set this flag for new drivers.
*/
enum v4l2_video_device_flags {
- V4L2_FL_REGISTERED = 0,
- V4L2_FL_USES_V4L2_FH= 1,
+ V4L2_FL_REGISTERED = 0,
+ V4L2_FL_USES_V4L2_FH= 1,
+ V4L2_FL_QUIRK_INVERTED_CROP = 2,
};
/* Priority helper functions */
--
2.18.0
