* [PATCH 1/2] v4l: add support for extended crop/compose API
2011-04-06 8:44 [PATCH 0/2] V4L: Extended crop/compose API, ver2 Tomasz Stanislawski
@ 2011-04-06 8:44 ` Tomasz Stanislawski
2011-04-06 8:44 ` [PATCH 2/2] v4l: simulate old crop API using extcrop/compose Tomasz Stanislawski
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Tomasz Stanislawski @ 2011-04-06 8:44 UTC (permalink / raw)
To: linux-media; +Cc: m.szyprowski, t.stanislaws
Four new ioctl for a precise control of cropping and composing:
VIDIOC_S_EXTCROP
VIDIOC_G_EXTCROP
VIDIOC_S_COMPOSE
VIDIOC_G_COMPOSE
Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
---
drivers/media/video/v4l2-compat-ioctl32.c | 4 ++
drivers/media/video/v4l2-ioctl.c | 56 +++++++++++++++++++++++++++++
include/linux/videodev2.h | 44 ++++++++++++++++++++++
include/media/v4l2-ioctl.h | 8 ++++
4 files changed, 112 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
index 7c26947..81481bc 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -891,6 +891,10 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
case VIDIOC_CROPCAP:
case VIDIOC_G_CROP:
case VIDIOC_S_CROP:
+ case VIDIOC_G_EXTCROP:
+ case VIDIOC_S_EXTCROP:
+ case VIDIOC_G_COMPOSE:
+ case VIDIOC_S_COMPOSE:
case VIDIOC_G_JPEGCOMP:
case VIDIOC_S_JPEGCOMP:
case VIDIOC_QUERYSTD:
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 7a72074..3f69218 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -223,6 +223,10 @@ static const char *v4l2_ioctls[] = {
[_IOC_NR(VIDIOC_CROPCAP)] = "VIDIOC_CROPCAP",
[_IOC_NR(VIDIOC_G_CROP)] = "VIDIOC_G_CROP",
[_IOC_NR(VIDIOC_S_CROP)] = "VIDIOC_S_CROP",
+ [_IOC_NR(VIDIOC_G_EXTCROP)] = "VIDIOC_G_EXTCROP",
+ [_IOC_NR(VIDIOC_S_EXTCROP)] = "VIDIOC_S_EXTCROP",
+ [_IOC_NR(VIDIOC_G_COMPOSE)] = "VIDIOC_G_COMPOSE",
+ [_IOC_NR(VIDIOC_S_COMPOSE)] = "VIDIOC_S_COMPOSE",
[_IOC_NR(VIDIOC_G_JPEGCOMP)] = "VIDIOC_G_JPEGCOMP",
[_IOC_NR(VIDIOC_S_JPEGCOMP)] = "VIDIOC_S_JPEGCOMP",
[_IOC_NR(VIDIOC_QUERYSTD)] = "VIDIOC_QUERYSTD",
@@ -1741,6 +1745,58 @@ static long __video_do_ioctl(struct file *file,
ret = ops->vidioc_s_crop(file, fh, p);
break;
}
+ case VIDIOC_G_EXTCROP:
+ {
+ struct v4l2_selection *p = arg;
+
+ if (!ops->vidioc_g_extcrop)
+ break;
+
+ dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
+
+ ret = ops->vidioc_g_extcrop(file, fh, p);
+ if (!ret)
+ dbgrect(vfd, "", &p->r);
+ break;
+ }
+ case VIDIOC_S_EXTCROP:
+ {
+ struct v4l2_selection *p = arg;
+
+ if (!ops->vidioc_s_extcrop)
+ break;
+ dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
+ dbgrect(vfd, "", &p->r);
+
+ ret = ops->vidioc_s_extcrop(file, fh, p);
+ break;
+ }
+ case VIDIOC_G_COMPOSE:
+ {
+ struct v4l2_selection *p = arg;
+
+ if (!ops->vidioc_g_compose)
+ break;
+
+ dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
+
+ ret = ops->vidioc_g_compose(file, fh, p);
+ if (!ret)
+ dbgrect(vfd, "", &p->r);
+ break;
+ }
+ case VIDIOC_S_COMPOSE:
+ {
+ struct v4l2_selection *p = arg;
+
+ if (!ops->vidioc_s_compose)
+ break;
+ dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
+ dbgrect(vfd, "", &p->r);
+
+ ret = ops->vidioc_s_compose(file, fh, p);
+ break;
+ }
case VIDIOC_CROPCAP:
{
struct v4l2_cropcap *p = arg;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index a94c4d5..50a9981 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -718,6 +718,44 @@ struct v4l2_crop {
struct v4l2_rect c;
};
+/* Hints for adjustments of selection rectangle */
+#define V4L2_SEL_WIDTH_GE 0x00000001
+#define V4L2_SEL_WIDTH_LE 0x00000002
+#define V4L2_SEL_HEIGHT_GE 0x00000004
+#define V4L2_SEL_HEIGHT_LE 0x00000008
+#define V4L2_SEL_LEFT_GE 0x00000010
+#define V4L2_SEL_LEFT_LE 0x00000020
+#define V4L2_SEL_TOP_GE 0x00000040
+#define V4L2_SEL_TOP_LE 0x00000080
+#define V4L2_SEL_RIGHT_GE 0x00000100
+#define V4L2_SEL_RIGHT_LE 0x00000200
+#define V4L2_SEL_BOTTOM_GE 0x00000400
+#define V4L2_SEL_BOTTOM_LE 0x00000800
+
+#define V4L2_SEL_WIDTH_FIXED 0x00000003
+#define V4L2_SEL_HEIGHT_FIXED 0x0000000c
+#define V4L2_SEL_LEFT_FIXED 0x00000030
+#define V4L2_SEL_TOP_FIXED 0x000000c0
+#define V4L2_SEL_RIGHT_FIXED 0x00000300
+#define V4L2_SEL_BOTTOM_FIXED 0x00000c00
+
+#define V4L2_SEL_FIXED 0x00000fff
+
+enum v4l2_sel_target {
+ V4L2_SEL_TARGET_ACTIVE = 0,
+ V4L2_SEL_TARGET_DEFAULT = 1,
+ V4L2_SEL_TARGET_BOUNDS = 2,
+};
+
+struct v4l2_selection {
+ enum v4l2_buf_type type;
+ enum v4l2_sel_target target;
+ struct v4l2_rect r;
+ __u32 flags;
+ __u32 reserved[9];
+};
+
+
/*
* A N A L O G V I D E O S T A N D A R D
*/
@@ -1932,6 +1970,12 @@ struct v4l2_dbg_chip_ident {
#define VIDIOC_SUBSCRIBE_EVENT _IOW('V', 90, struct v4l2_event_subscription)
#define VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91, struct v4l2_event_subscription)
+/* New cropping/compose API */
+#define VIDIOC_G_EXTCROP _IOWR('V', 92, struct v4l2_selection)
+#define VIDIOC_S_EXTCROP _IOWR('V', 93, struct v4l2_selection)
+#define VIDIOC_G_COMPOSE _IOWR('V', 94, struct v4l2_selection)
+#define VIDIOC_S_COMPOSE _IOWR('V', 95, struct v4l2_selection)
+
/* Reminder: when adding new ioctls please add support for them to
drivers/media/video/v4l2-compat-ioctl32.c as well! */
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index 1572c7f..3174b88 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -194,6 +194,14 @@ struct v4l2_ioctl_ops {
struct v4l2_crop *a);
int (*vidioc_s_crop) (struct file *file, void *fh,
struct v4l2_crop *a);
+ int (*vidioc_g_extcrop) (struct file *file, void *fh,
+ struct v4l2_selection *a);
+ int (*vidioc_s_extcrop) (struct file *file, void *fh,
+ struct v4l2_selection *a);
+ int (*vidioc_g_compose) (struct file *file, void *fh,
+ struct v4l2_selection *a);
+ int (*vidioc_s_compose) (struct file *file, void *fh,
+ struct v4l2_selection *a);
/* Compression ioctls */
int (*vidioc_g_jpegcomp) (struct file *file, void *fh,
struct v4l2_jpegcompression *a);
--
1.7.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/2] v4l: simulate old crop API using extcrop/compose
2011-04-06 8:44 [PATCH 0/2] V4L: Extended crop/compose API, ver2 Tomasz Stanislawski
2011-04-06 8:44 ` [PATCH 1/2] v4l: add support for extended crop/compose API Tomasz Stanislawski
@ 2011-04-06 8:44 ` Tomasz Stanislawski
2011-04-08 12:53 ` [PATCH 0/2] V4L: Extended crop/compose API, ver2 Hans Verkuil
[not found] ` <201104131507.55171.laurent.pinchart@ideasonboard.com>
3 siblings, 0 replies; 10+ messages in thread
From: Tomasz Stanislawski @ 2011-04-06 8:44 UTC (permalink / raw)
To: linux-media; +Cc: m.szyprowski, t.stanislaws
This patch allows new drivers to work correctly with
applications that use old-style crop API.
The old crop ioctl is simulated by using extcrop/compose.
Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
---
drivers/media/video/v4l2-ioctl.c | 94 +++++++++++++++++++++++++++++++++-----
1 files changed, 82 insertions(+), 12 deletions(-)
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 3f69218..1e7d18d 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -1725,11 +1725,30 @@ static long __video_do_ioctl(struct file *file,
{
struct v4l2_crop *p = arg;
- if (!ops->vidioc_g_crop)
- break;
-
dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
- ret = ops->vidioc_g_crop(file, fh, p);
+
+ if (ops->vidioc_g_crop) {
+ ret = ops->vidioc_g_crop(file, fh, p);
+ } else {
+ struct v4l2_selection s = {
+ .type = p->type,
+ .target = V4L2_SEL_TARGET_ACTIVE,
+ };
+ /* simulate capture crop using extcrop */
+ if (p->type == V4L2_BUF_TYPE_VIDEO_CAPTURE
+ && ops->vidioc_g_extcrop) {
+ ret = ops->vidioc_g_extcrop(file, fh, &s);
+ }
+ /* simulate output crop using compose */
+ if (p->type == V4L2_BUF_TYPE_VIDEO_OUTPUT
+ && ops->vidioc_g_compose) {
+ ret = ops->vidioc_g_compose(file, fh, &s);
+ }
+ /* copying results to old structure */
+ if (!ret)
+ p->c = s.r;
+ }
+
if (!ret)
dbgrect(vfd, "", &p->c);
break;
@@ -1738,11 +1757,28 @@ static long __video_do_ioctl(struct file *file,
{
struct v4l2_crop *p = arg;
- if (!ops->vidioc_s_crop)
- break;
dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
dbgrect(vfd, "", &p->c);
- ret = ops->vidioc_s_crop(file, fh, p);
+
+ if (ops->vidioc_s_crop) {
+ ret = ops->vidioc_s_crop(file, fh, p);
+ } else {
+ struct v4l2_selection s = {
+ .type = p->type,
+ .target = V4L2_SEL_TARGET_ACTIVE,
+ .r = p->c,
+ };
+ /* simulate capture crop using extcrop */
+ if (p->type == V4L2_BUF_TYPE_VIDEO_CAPTURE
+ && ops->vidioc_s_extcrop) {
+ ret = ops->vidioc_s_extcrop(file, fh, &s);
+ }
+ /* simulate output crop using compose */
+ if (p->type == V4L2_BUF_TYPE_VIDEO_OUTPUT
+ && ops->vidioc_s_compose) {
+ ret = ops->vidioc_s_compose(file, fh, &s);
+ }
+ }
break;
}
case VIDIOC_G_EXTCROP:
@@ -1801,12 +1837,46 @@ static long __video_do_ioctl(struct file *file,
{
struct v4l2_cropcap *p = arg;
- /*FIXME: Should also show v4l2_fract pixelaspect */
- if (!ops->vidioc_cropcap)
- break;
-
dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
- ret = ops->vidioc_cropcap(file, fh, p);
+ if (ops->vidioc_cropcap) {
+ ret = ops->vidioc_cropcap(file, fh, p);
+ } else {
+ struct v4l2_selection s = { .type = p->type };
+ int (*func)(struct file *file, void *fh,
+ struct v4l2_selection *a) = NULL;
+ struct v4l2_rect bounds;
+
+ /* simulate capture crop using extcrop */
+ if (p->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ func = ops->vidioc_g_extcrop;
+ /* simulate output crop using compose */
+ if (p->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ func = ops->vidioc_g_compose;
+ /* leave if cropcap can not be simulated */
+ if (func == NULL)
+ break;
+
+ /* obtaining bounds */
+ s.target = V4L2_SEL_TARGET_BOUNDS;
+ ret = func(file, fh, &s);
+ if (ret)
+ break;
+ bounds = s.r;
+
+ /* obtaining defrect */
+ s.target = V4L2_SEL_TARGET_DEFAULT;
+ ret = func(file, fh, &s);
+ if (ret)
+ break;
+
+ /* storing results */
+ p->bounds = bounds;
+ p->defrect = s.r;
+ p->pixelaspect.numerator = 1;
+ p->pixelaspect.denominator = 1;
+ }
+
+ /*FIXME: Should also show v4l2_fract pixelaspect */
if (!ret) {
dbgrect(vfd, "bounds ", &p->bounds);
dbgrect(vfd, "defrect ", &p->defrect);
--
1.7.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 0/2] V4L: Extended crop/compose API, ver2
2011-04-06 8:44 [PATCH 0/2] V4L: Extended crop/compose API, ver2 Tomasz Stanislawski
2011-04-06 8:44 ` [PATCH 1/2] v4l: add support for extended crop/compose API Tomasz Stanislawski
2011-04-06 8:44 ` [PATCH 2/2] v4l: simulate old crop API using extcrop/compose Tomasz Stanislawski
@ 2011-04-08 12:53 ` Hans Verkuil
2011-04-09 16:43 ` Sylwester Nawrocki
2011-04-11 10:42 ` Tomasz Stanislawski
[not found] ` <201104131507.55171.laurent.pinchart@ideasonboard.com>
3 siblings, 2 replies; 10+ messages in thread
From: Hans Verkuil @ 2011-04-08 12:53 UTC (permalink / raw)
To: Tomasz Stanislawski; +Cc: linux-media, m.szyprowski, Laurent Pinchart
Hi Tomasz!
Some comments below...
On Wednesday, April 06, 2011 10:44:17 Tomasz Stanislawski wrote:
> Hello everyone,
>
> This patch-set introduces new ioctls to V4L2 API. The new method for
> configuration of cropping and composition is presented.
>
> This is the second version of extcrop RFC. It was enriched with new features
> like additional hint flags, and a support for auxiliary crop/compose
> rectangles.
>
> There is some confusion in understanding of a cropping in current version of
> V4L2. For CAPTURE devices cropping refers to choosing only a part of input
> data stream and processing it and storing it in a memory buffer. The buffer is
> fully filled by data. It is not possible to choose only a part of a buffer for
> being updated by hardware.
>
> In case of OUTPUT devices, the whole content of a buffer is passed by hardware
> to output display. Cropping means selecting only a part of an output
> display/signal. It is not possible to choose only a part for a memory buffer
> to be processed.
>
> The overmentioned flaws in cropping API were discussed in post:
> http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/28945
>
> A solution was proposed during brainstorming session in Warsaw.
>
>
> 1. Data structures.
>
> The structure v4l2_crop used by current API lacks any place for further
> extensions. Therefore new structure is proposed.
>
> struct v4l2_selection {
> u32 type;
> u32 target;
> struct v4l2_rect r;
> u32 flags;
> u32 reserved[9];
> };
>
> Where,
> type - type of buffer queue: V4L2_BUF_TYPE_VIDEO_CAPTURE,
> V4L2_BUF_TYPE_VIDEO_OUTPUT, etc.
> target - choose one of cropping/composing rectangles
> r - selection rectangle
> flags - control over coordinates adjustments
> reserved - place for further extensions, adjust struct size to 64 bytes
>
> At first, the distinction between cropping and composing was stated. The
> cropping operation means choosing only part of input data bounding it by a
> cropping rectangle. All other data must be discarded. On the other hand,
> composing means pasting processed data into rectangular part of data sink. The
> sink may be output device, user buffer, etc.
>
>
> 2. Crop/Compose ioctl.
> Four new ioctls would be added to V4L2.
>
> Name
> VIDIOC_S_EXTCROP - set cropping rectangle on an input of a device
>
> Synopsis
> int ioctl(fd, VIDIOC_S_EXTCROP, struct v4l2_selection *s)
>
> Description:
> The ioctl is used to configure:
> - for input devices, a part of input data that is processed in hardware
> - for output devices, a part of a data buffer to be passed to hardware
> Drivers may adjust a cropping area. The adjustment can be controlled
> by v4l2_selection::flags. Please refer to Hints section.
> - an adjusted crop rectangle is returned in v4l2_selection::r
>
> Return value
> On success 0 is returned, on error -1 and the errno variable is set
> appropriately:
> ERANGE - failed to find a rectangle that satisfy all constraints
> EINVAL - incorrect buffer type, incorrect target, cropping not supported
>
> -----------------------------------------------------------------
>
> Name
> VIDIOC_G_EXTCROP - get cropping rectangle on an input of a device
>
> Synopsis
> int ioctl(fd, VIDIOC_G_EXTCROP, struct v4l2_selection *s)
>
> Description:
> The ioctl is used to query:
> - for input devices, a part of input data that is processed in hardware
> Other crop rectangles might be examined using this ioctl.
> Please refer to Targets section.
> - for output devices, a part of data buffer to be passed to hardware
>
> Return value
> On success 0 is returned, on error -1 and the errno variable is set
> appropriately:
> EINVAL - incorrect buffer type, incorrect target, cropping not supported
>
> -----------------------------------------------------------------
>
> Name
> VIDIOC_S_COMPOSE - set destination rectangle on an output of a device
>
> Synopsis
> int ioctl(fd, VIDIOC_S_COMPOSE, struct v4l2_selection *s)
>
> Description:
> The ioctl is used to configure:
> - for input devices, a part of a data buffer that is filled by hardware
> - for output devices, a area on output device where image is inserted
> Drivers may adjust a composing area. The adjustment can be controlled
> by v4l2_selection::flags. Please refer to Hints section.
> - an adjusted composing rectangle is returned in v4l2_selection::r
>
> Return value
> On success 0 is returned, on error -1 and the errno variable is set
> appropriately:
> ERANGE - failed to find a rectangle that satisfy all constraints
> EINVAL - incorrect buffer type, incorrect target, composing not supported
>
> -----------------------------------------------------------------
>
> Name
> VIDIOC_G_COMPOSE - get destination rectangle on an output of a device
>
> Synopsis
> int ioctl(fd, VIDIOC_G_COMPOSE, struct v4l2_selection *s)
>
> Description:
> The ioctl is used to query:
> - for input devices, a part of a data buffer that is filled by hardware
> Other compose rectangles might be examined using this ioctl.
> Please refer to Targets section.
> - for output devices, a area on output device where image is inserted
>
> Return value
> On success 0 is returned, on error -1 and the errno variable is set
> appropriately:
> EINVAL - incorrect buffer type, incorrect target, composing not supported
>
>
> 3. Hints
>
> The v4l2_selection::flags field is used to give a driver a hint about
> coordinate adjustments. Below one can find the proposition of adjustment
> flags. The syntax is V4L2_SEL_{name}_{LE/GE}, where {name} refer to a field in
> struct v4l2_rect. Two additional properties exist 'right' and 'bottom'. The
> refer to respectively: left + width, and top + height. The LE is abbreviation
> from "lesser or equal". It prevents the driver form increasing a parameter. In
> similar fashion GE means "greater or equal" and it disallows decreasing.
> Combining LE and GE flags prevents the driver from any adjustments of
> parameters. In such a manner, setting flags field to zero would give a driver
> a free hand in coordinate adjustment.
>
> #define V4L2_SEL_WIDTH_GE 0x00000001
> #define V4L2_SEL_WIDTH_LE 0x00000002
> #define V4L2_SEL_HEIGHT_GE 0x00000004
> #define V4L2_SEL_HEIGHT_LE 0x00000008
> #define V4L2_SEL_LEFT_GE 0x00000010
> #define V4L2_SEL_LEFT_LE 0x00000020
> #define V4L2_SEL_TOP_GE 0x00000040
> #define V4L2_SEL_TOP_LE 0x00000080
> #define V4L2_SEL_RIGHT_GE 0x00000100
> #define V4L2_SEL_RIGHT_LE 0x00000200
> #define V4L2_SEL_BOTTOM_GE 0x00000400
> #define V4L2_SEL_BOTTOM_LE 0x00000800
>
> #define V4L2_SEL_WIDTH_FIXED 0x00000003
> #define V4L2_SEL_HEIGHT_FIXED 0x0000000c
> #define V4L2_SEL_LEFT_FIXED 0x00000030
> #define V4L2_SEL_TOP_FIXED 0x000000c0
> #define V4L2_SEL_RIGHT_FIXED 0x00000300
> #define V4L2_SEL_BOTTOM_FIXED 0x00000c00
>
> #define V4L2_SEL_FIXED 0x00000fff
>
> The hint flags may be useful in a following scenario. There is a sensor with a
> face detection functionality. An application receives information about a
> position of a face on sensor array. Assume that the camera pipeline is capable
> of an image scaling. The application is capable of obtaining a location of a
> face using V4L2 controls. The task it to grab only part of image that contains
> a face, and store it to a framebuffer at a fixed window. Therefore following
> constrains have to be satisfied:
> - the rectangle that contains a face must lay inside cropping area
> - hardware is allowed only to access area inside window on the framebuffer
>
> Both constraints could be satisfied with two ioctl calls.
> - VIDIOC_EXTCROP with flags field equal to
> V4L2_SEL_TOP_LE | V4L2_SEL_LEFT_LE |
> V4L2_SEL_RIGHT_GE | V4L2_SEL_BOTTOM_GE.
> - VIDIOC_COMPOSE with flags field equal to
> V4L2_SEL_TOP_FIXED | V4L2_SEL_LEFT_FIXED |
> V4L2_SEL_WIDTH_LE | V4L2_SEL_HEIGHT_LE
>
> Feel free to add a new flag if necessary.
While this is very flexible, I am a bit concerned about the complexity this
would introduce in a driver. I think I would want to see this actually
implemented in a driver first. I suspect that some utility functions are
probably needed.
>
>
> 4. Targets
> The cropping/composing subsystem may use auxiliary rectangles other than a
> normal cropping rectangle. The field v4l2_selection::target is used to choose
> the rectangle. This functionality was added to simulate VIDIOC_CROPCAP ioctl.
> All cropcap fields except pixel aspect are supported. I noticed that there was
> discussion about pixel aspect and I am not convinced that it should be a part
> of the cropping API. Please refer to the post:
> http://lists-archives.org/video4linux/22837-need-vidioc_cropcap-clarification.html
>
> Proposed targets are:
> - active - numerical value 0, an area that is processed by hardware
> - default - is the suggested active rectangle that covers the "whole picture"
> - bounds - the limits that active rectangle cannot exceed
>
> V4L2_SEL_TARGET_ACTIVE = 0
> V4L2_SEL_TARGET_DEFAULT = 1
> V4L2_SEL_TARGET_BOUNDS = 2
>
> Feel free to add other targets.
>
> Only V4L2_SEL_TARGET_ACTIVE is accepted for VIDIOC_S_EXTCROP/VIDIOC_S_COMPOSE
> ioctls. Auxiliary target like DEFAULT and BOUNDS are supported only by 'get'
> interface.
Sorry, but I really don't like this idea of a target. It doesn't make sense to
add this when you can only choose a different target for a get.
I think a EXTCROPCAP/COMPOSECAP pair (or a single CROPCOMPOSECAP ioctl, see below)
makes more sense.
>
>
> 5. Possible improvements and extensions.
> - combine composing and cropping ioctl into a single ioctl
I think this could be very interesting. By doing this in a single ioctl you
should have all the information needed to setup a scaler. And with the hints
you can tell the driver how the input/output rectangles need to be adjusted.
This would make sense as well on the subdev level.
Laurent, wouldn't this solve the way the omap3 ISP sets up the scaler? By
fixing the output of the scaler and setting hints to allow changes to the
input crop rectangle we would fix the scaler setup issues we discussed in
Warsaw.
> - add subpixel resolution
> * hardware is often capable of subpixel processing. The ioctl triple
> S_EXTCROP, S_SCALE, S_COMPOSE can be converted to S_EXTCROP and S_COMPOSE
> pair if a subpixel resolution is supported
I'm not sure I understand this. Can you give an example?
> - merge v4l2_selection::target and v4l2_selection::flags into single field
> - allow using VIDIOC_S_EXTCROP with target type V4L2_SEL_TARGET_BOUNDS to
> choose a resolution of a sensor
Too obscure IMHO. That said, it would be nice to have a more explicit method
of selecting a sensor resolution. You can enumerate them, but you choose it
using VIDIOC_S_FMT, which I've always thought was very dubious. This prevents
any sensor-built-in scalers from being used. For video you have S_STD and
S_DV_PRESET that select a particular input resolution, but a similar ioctl is
missing for sensors. Laurent, what are your thoughts?
> - add TRY flag to ask a driver to adjust a rectangle without applying it
Don't use a flag, use TRY_EXTCROP and TRY_COMPOSE, just like the other try
ioctls. Otherwise I'm in favor of this.
Regards,
Hans
>
> What it your opinion about proposed solutions?
>
> Looking for a reply,
>
> Best regards,
> Tomasz Stanislawski
>
>
> Tomasz Stanislawski (2):
> v4l: add support for extended crop/compose API
> v4l: simulate old crop API using extcrop/compose
>
> drivers/media/video/v4l2-compat-ioctl32.c | 4 +
> drivers/media/video/v4l2-ioctl.c | 150 ++++++++++++++++++++++++++---
> include/linux/videodev2.h | 44 +++++++++
> include/media/v4l2-ioctl.h | 8 ++
> 4 files changed, 194 insertions(+), 12 deletions(-)
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/2] V4L: Extended crop/compose API, ver2
2011-04-08 12:53 ` [PATCH 0/2] V4L: Extended crop/compose API, ver2 Hans Verkuil
@ 2011-04-09 16:43 ` Sylwester Nawrocki
2011-04-10 7:59 ` Sylwester Nawrocki
2011-04-11 10:42 ` Tomasz Stanislawski
1 sibling, 1 reply; 10+ messages in thread
From: Sylwester Nawrocki @ 2011-04-09 16:43 UTC (permalink / raw)
To: Hans Verkuil
Cc: Tomasz Stanislawski, linux-media, Marek Szyprowski,
Laurent Pinchart
Hello,
On 04/08/2011 02:53 PM, Hans Verkuil wrote:
> Hi Tomasz!
>
> Some comments below...
>
> On Wednesday, April 06, 2011 10:44:17 Tomasz Stanislawski wrote:
>> Hello everyone,
>>
>> This patch-set introduces new ioctls to V4L2 API. The new method for
>> configuration of cropping and composition is presented.
>>
>> This is the second version of extcrop RFC. It was enriched with new features
>> like additional hint flags, and a support for auxiliary crop/compose
>> rectangles.
>>
>> There is some confusion in understanding of a cropping in current version of
>> V4L2. For CAPTURE devices cropping refers to choosing only a part of input
>> data stream and processing it and storing it in a memory buffer. The buffer is
>> fully filled by data. It is not possible to choose only a part of a buffer for
>> being updated by hardware.
>>
>> In case of OUTPUT devices, the whole content of a buffer is passed by hardware
>> to output display. Cropping means selecting only a part of an output
>> display/signal. It is not possible to choose only a part for a memory buffer
>> to be processed.
>>
>> The overmentioned flaws in cropping API were discussed in post:
>> http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/28945
>>
>> A solution was proposed during brainstorming session in Warsaw.
>>
...
>> - merge v4l2_selection::target and v4l2_selection::flags into single field
>> - allow using VIDIOC_S_EXTCROP with target type V4L2_SEL_TARGET_BOUNDS to
>> choose a resolution of a sensor
Assuming here that the "resolution of a sensor" refers to the output resolution
of a sensor with embedded ISP as seen by a bridge. Otherwise if it refers to
the active pixel matrix resolution VIDIOC_S_EXTCROP(V4L2_SEL_TARGET_BOUNDS)
would only make sense for sensors that support different pixel matrix layout,
e.g. portrait/landscape. Something that Laurent brought to our attention during
the Warsaw meeting, i.e. where actual sensor matrix contour is square but there
are square polygons of dark pixels at each corner of the contour.
>
> Too obscure IMHO. That said, it would be nice to have a more explicit method
> of selecting a sensor resolution. You can enumerate them, but you choose it
> using VIDIOC_S_FMT, which I've always thought was very dubious. This prevents
> any sensor-built-in scalers from being used. For video you have S_STD and
IMHO sensor-built-in scalers can be used by means of the VIDIOC_[S/G]_CROP
ioctls, which allows to select not only width/height of the part of a sensor
matrix to be fed to the sensor's scaler but also a position of a pixel crop
rectangle.
> S_DV_PRESET that select a particular input resolution, but a similar ioctl is
> missing for sensors. Laurent, what are your thoughts?
>
I suppose new ioctls like [G/S]_FRAMESIZE could be useful for selecting
sensor's output resolution, those could then call sensor's pad set_fmt/get_fmt
ops. Please note there are image sensors that support any resolution in their
nominal range with some alignment requirements. For a maximum resolution 1024x1280
and 2 pixels alignment those would yield 327680 different resolutions.
Does enum_framesizes make sense in such cases?
There is currently no way to configure a scaler built in in the bridge with
the regular V4L2 API though. Only the final output buffer resolution can be set
with VIDIOC_S_FMT. We can select an active pixel array area with S_CROP.
Depending where the cropping is actually performed - in an image sensor or in
a bridge we are able to use sensor-built-in scaler OR bridge-built-in scaler,
never both. Would setting sensor's output and bridge's input resolution with
new VIDIOC_S_FRAMESIZE ioctl make sense?
......................................... .....................................
. . . .
. G/S_CROP G/S_FRAMESIZE G/S_FMT .
. (x,y) w1 x h1 . w2 x h2 . w3 x h3 .
. +-------------+ +------------+ . . +------------+ +---------+ .
. | | | | . . | | | | .
. | Pixel | | ISP | . . | SCALER | | DMA | .
. | matrix |_______| (scaler) |_______________| |_______| eng. | .
. | | | | . . | (color | | | .
. | | | | . . | converter) | | | .
. | | | | . . | | | | .
. +-------------+ +------------+ . . +------------+ +---------+ .
. . . .
. SENSOR PAD S0 PAD S1. . PAD B0 BRIDGE .
........................................ .....................................
Also how could one enumerate what media bus formats are supported at bridge input pad
(PAD B0 in the ascii diagram above) if the bridge does not support the v4l2 subdev
user space API and the application needs to match formats at pads PAD S1 and PAD B0 ?
--
Regards,
Sylwester Nawrocki
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] V4L: Extended crop/compose API, ver2
2011-04-09 16:43 ` Sylwester Nawrocki
@ 2011-04-10 7:59 ` Sylwester Nawrocki
0 siblings, 0 replies; 10+ messages in thread
From: Sylwester Nawrocki @ 2011-04-10 7:59 UTC (permalink / raw)
To: Hans Verkuil
Cc: Tomasz Stanislawski, linux-media, Marek Szyprowski,
Laurent Pinchart
On 04/09/2011 06:43 PM, Sylwester Nawrocki wrote:
> Hello,
>
> On 04/08/2011 02:53 PM, Hans Verkuil wrote:
>> Hi Tomasz!
>>
>> Some comments below...
>>
>> On Wednesday, April 06, 2011 10:44:17 Tomasz Stanislawski wrote:
>>> Hello everyone,
>>>
>>> This patch-set introduces new ioctls to V4L2 API. The new method for
>>> configuration of cropping and composition is presented.
>>>
>>> This is the second version of extcrop RFC. It was enriched with new features
>>> like additional hint flags, and a support for auxiliary crop/compose
>>> rectangles.
>>>
>>> There is some confusion in understanding of a cropping in current version of
>>> V4L2. For CAPTURE devices cropping refers to choosing only a part of input
>>> data stream and processing it and storing it in a memory buffer. The buffer is
>>> fully filled by data. It is not possible to choose only a part of a buffer for
>>> being updated by hardware.
>>>
>>> In case of OUTPUT devices, the whole content of a buffer is passed by hardware
>>> to output display. Cropping means selecting only a part of an output
>>> display/signal. It is not possible to choose only a part for a memory buffer
>>> to be processed.
>>>
>>> The overmentioned flaws in cropping API were discussed in post:
>>> http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/28945
>>>
>>> A solution was proposed during brainstorming session in Warsaw.
>>>
> ...
>>> - merge v4l2_selection::target and v4l2_selection::flags into single field
>>> - allow using VIDIOC_S_EXTCROP with target type V4L2_SEL_TARGET_BOUNDS to
>>> choose a resolution of a sensor
>
> Assuming here that the "resolution of a sensor" refers to the output resolution
> of a sensor with embedded ISP as seen by a bridge. Otherwise if it refers to
> the active pixel matrix resolution VIDIOC_S_EXTCROP(V4L2_SEL_TARGET_BOUNDS)
> would only make sense for sensors that support different pixel matrix layout,
> e.g. portrait/landscape. Something that Laurent brought to our attention during
> the Warsaw meeting, i.e. where actual sensor matrix contour is square but there
> are square polygons of dark pixels at each corner of the contour.
>
>>
>> Too obscure IMHO. That said, it would be nice to have a more explicit method
>> of selecting a sensor resolution. You can enumerate them, but you choose it
>> using VIDIOC_S_FMT, which I've always thought was very dubious. This prevents
>> any sensor-built-in scalers from being used. For video you have S_STD and
>
> IMHO sensor-built-in scalers can be used by means of the VIDIOC_[S/G]_CROP
> ioctls, which allows to select not only width/height of the part of a sensor
> matrix to be fed to the sensor's scaler but also a position of a pixel crop
> rectangle.
>
>> S_DV_PRESET that select a particular input resolution, but a similar ioctl is
>> missing for sensors. Laurent, what are your thoughts?
>>
>
> I suppose new ioctls like [G/S]_FRAMESIZE could be useful for selecting
> sensor's output resolution, those could then call sensor's pad set_fmt/get_fmt
> ops. Please note there are image sensors that support any resolution in their
> nominal range with some alignment requirements. For a maximum resolution 1024x1280
> and 2 pixels alignment those would yield 327680 different resolutions.
> Does enum_framesizes make sense in such cases?
Oops, I should have read the documentation carefully before asking silly
questions.. :/ Of course the above case could be easily covered by
the step-wise frame size type.
>
> There is currently no way to configure a scaler built in in the bridge with
> the regular V4L2 API though. Only the final output buffer resolution can be set
> with VIDIOC_S_FMT. We can select an active pixel array area with S_CROP.
> Depending where the cropping is actually performed - in an image sensor or in
> a bridge we are able to use sensor-built-in scaler OR bridge-built-in scaler,
> never both. Would setting sensor's output and bridge's input resolution with
> new VIDIOC_S_FRAMESIZE ioctl make sense?
>
> ......................................... .....................................
> . . . .
> . G/S_CROP G/S_FRAMESIZE G/S_FMT .
> . (x,y) w1 x h1 . w2 x h2 . w3 x h3 .
> . +-------------+ +------------+ . . +------------+ +---------+ .
> . | | | | . . | | | | .
> . | Pixel | | ISP | . . | SCALER | | DMA | .
> . | matrix |_______| (scaler) |_______________| |_______| eng. | .
> . | | | | . . | (color | | | .
> . | | | | . . | converter) | | | .
> . | | | | . . | | | | .
> . +-------------+ +------------+ . . +------------+ +---------+ .
> . . . .
> . SENSOR PAD S0 PAD S1. . PAD B0 BRIDGE .
> ........................................ .....................................
>
>
> Also how could one enumerate what media bus formats are supported at bridge input pad
> (PAD B0 in the ascii diagram above) if the bridge does not support the v4l2 subdev
> user space API and the application needs to match formats at pads PAD S1 and PAD B0 ?
--
Regards,
Sylwester Nawrocki
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] V4L: Extended crop/compose API, ver2
2011-04-08 12:53 ` [PATCH 0/2] V4L: Extended crop/compose API, ver2 Hans Verkuil
2011-04-09 16:43 ` Sylwester Nawrocki
@ 2011-04-11 10:42 ` Tomasz Stanislawski
2011-04-12 9:40 ` Laurent Pinchart
1 sibling, 1 reply; 10+ messages in thread
From: Tomasz Stanislawski @ 2011-04-11 10:42 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
Hans Verkuil wrote:
> Hi Tomasz!
>
> Some comments below...
>
> On Wednesday, April 06, 2011 10:44:17 Tomasz Stanislawski wrote:
>> Hello everyone,
>>
>> This patch-set introduces new ioctls to V4L2 API. The new method for
>> configuration of cropping and composition is presented.
>>
>> This is the second version of extcrop RFC. It was enriched with new features
>> like additional hint flags, and a support for auxiliary crop/compose
>> rectangles.
>>
>> There is some confusion in understanding of a cropping in current version of
>> V4L2. For CAPTURE devices cropping refers to choosing only a part of input
>> data stream and processing it and storing it in a memory buffer. The buffer is
>> fully filled by data. It is not possible to choose only a part of a buffer for
>> being updated by hardware.
>>
>> In case of OUTPUT devices, the whole content of a buffer is passed by hardware
>> to output display. Cropping means selecting only a part of an output
>> display/signal. It is not possible to choose only a part for a memory buffer
>> to be processed.
>>
>> The overmentioned flaws in cropping API were discussed in post:
>> http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/28945
>>
>> A solution was proposed during brainstorming session in Warsaw.
>>
>>
>> 1. Data structures.
>>
>> The structure v4l2_crop used by current API lacks any place for further
>> extensions. Therefore new structure is proposed.
>>
>> struct v4l2_selection {
>> u32 type;
>> u32 target;
>> struct v4l2_rect r;
>> u32 flags;
>> u32 reserved[9];
>> };
>>
>> Where,
>> type - type of buffer queue: V4L2_BUF_TYPE_VIDEO_CAPTURE,
>> V4L2_BUF_TYPE_VIDEO_OUTPUT, etc.
>> target - choose one of cropping/composing rectangles
>> r - selection rectangle
>> flags - control over coordinates adjustments
>> reserved - place for further extensions, adjust struct size to 64 bytes
>>
>> At first, the distinction between cropping and composing was stated. The
>> cropping operation means choosing only part of input data bounding it by a
>> cropping rectangle. All other data must be discarded. On the other hand,
>> composing means pasting processed data into rectangular part of data sink. The
>> sink may be output device, user buffer, etc.
>>
>>
>> 2. Crop/Compose ioctl.
>> Four new ioctls would be added to V4L2.
>>
>> Name
>> VIDIOC_S_EXTCROP - set cropping rectangle on an input of a device
>>
>> Synopsis
>> int ioctl(fd, VIDIOC_S_EXTCROP, struct v4l2_selection *s)
>>
>> Description:
>> The ioctl is used to configure:
>> - for input devices, a part of input data that is processed in hardware
>> - for output devices, a part of a data buffer to be passed to hardware
>> Drivers may adjust a cropping area. The adjustment can be controlled
>> by v4l2_selection::flags. Please refer to Hints section.
>> - an adjusted crop rectangle is returned in v4l2_selection::r
>>
>> Return value
>> On success 0 is returned, on error -1 and the errno variable is set
>> appropriately:
>> ERANGE - failed to find a rectangle that satisfy all constraints
>> EINVAL - incorrect buffer type, incorrect target, cropping not supported
>>
>> -----------------------------------------------------------------
>>
>> Name
>> VIDIOC_G_EXTCROP - get cropping rectangle on an input of a device
>>
>> Synopsis
>> int ioctl(fd, VIDIOC_G_EXTCROP, struct v4l2_selection *s)
>>
>> Description:
>> The ioctl is used to query:
>> - for input devices, a part of input data that is processed in hardware
>> Other crop rectangles might be examined using this ioctl.
>> Please refer to Targets section.
>> - for output devices, a part of data buffer to be passed to hardware
>>
>> Return value
>> On success 0 is returned, on error -1 and the errno variable is set
>> appropriately:
>> EINVAL - incorrect buffer type, incorrect target, cropping not supported
>>
>> -----------------------------------------------------------------
>>
>> Name
>> VIDIOC_S_COMPOSE - set destination rectangle on an output of a device
>>
>> Synopsis
>> int ioctl(fd, VIDIOC_S_COMPOSE, struct v4l2_selection *s)
>>
>> Description:
>> The ioctl is used to configure:
>> - for input devices, a part of a data buffer that is filled by hardware
>> - for output devices, a area on output device where image is inserted
>> Drivers may adjust a composing area. The adjustment can be controlled
>> by v4l2_selection::flags. Please refer to Hints section.
>> - an adjusted composing rectangle is returned in v4l2_selection::r
>>
>> Return value
>> On success 0 is returned, on error -1 and the errno variable is set
>> appropriately:
>> ERANGE - failed to find a rectangle that satisfy all constraints
>> EINVAL - incorrect buffer type, incorrect target, composing not supported
>>
>> -----------------------------------------------------------------
>>
>> Name
>> VIDIOC_G_COMPOSE - get destination rectangle on an output of a device
>>
>> Synopsis
>> int ioctl(fd, VIDIOC_G_COMPOSE, struct v4l2_selection *s)
>>
>> Description:
>> The ioctl is used to query:
>> - for input devices, a part of a data buffer that is filled by hardware
>> Other compose rectangles might be examined using this ioctl.
>> Please refer to Targets section.
>> - for output devices, a area on output device where image is inserted
>>
>> Return value
>> On success 0 is returned, on error -1 and the errno variable is set
>> appropriately:
>> EINVAL - incorrect buffer type, incorrect target, composing not supported
>>
>>
>> 3. Hints
>>
>> The v4l2_selection::flags field is used to give a driver a hint about
>> coordinate adjustments. Below one can find the proposition of adjustment
>> flags. The syntax is V4L2_SEL_{name}_{LE/GE}, where {name} refer to a field in
>> struct v4l2_rect. Two additional properties exist 'right' and 'bottom'. The
>> refer to respectively: left + width, and top + height. The LE is abbreviation
>> from "lesser or equal". It prevents the driver form increasing a parameter. In
>> similar fashion GE means "greater or equal" and it disallows decreasing.
>> Combining LE and GE flags prevents the driver from any adjustments of
>> parameters. In such a manner, setting flags field to zero would give a driver
>> a free hand in coordinate adjustment.
>>
>> #define V4L2_SEL_WIDTH_GE 0x00000001
>> #define V4L2_SEL_WIDTH_LE 0x00000002
>> #define V4L2_SEL_HEIGHT_GE 0x00000004
>> #define V4L2_SEL_HEIGHT_LE 0x00000008
>> #define V4L2_SEL_LEFT_GE 0x00000010
>> #define V4L2_SEL_LEFT_LE 0x00000020
>> #define V4L2_SEL_TOP_GE 0x00000040
>> #define V4L2_SEL_TOP_LE 0x00000080
>> #define V4L2_SEL_RIGHT_GE 0x00000100
>> #define V4L2_SEL_RIGHT_LE 0x00000200
>> #define V4L2_SEL_BOTTOM_GE 0x00000400
>> #define V4L2_SEL_BOTTOM_LE 0x00000800
>>
>> #define V4L2_SEL_WIDTH_FIXED 0x00000003
>> #define V4L2_SEL_HEIGHT_FIXED 0x0000000c
>> #define V4L2_SEL_LEFT_FIXED 0x00000030
>> #define V4L2_SEL_TOP_FIXED 0x000000c0
>> #define V4L2_SEL_RIGHT_FIXED 0x00000300
>> #define V4L2_SEL_BOTTOM_FIXED 0x00000c00
>>
>> #define V4L2_SEL_FIXED 0x00000fff
>>
>> The hint flags may be useful in a following scenario. There is a sensor with a
>> face detection functionality. An application receives information about a
>> position of a face on sensor array. Assume that the camera pipeline is capable
>> of an image scaling. The application is capable of obtaining a location of a
>> face using V4L2 controls. The task it to grab only part of image that contains
>> a face, and store it to a framebuffer at a fixed window. Therefore following
>> constrains have to be satisfied:
>> - the rectangle that contains a face must lay inside cropping area
>> - hardware is allowed only to access area inside window on the framebuffer
>>
>> Both constraints could be satisfied with two ioctl calls.
>> - VIDIOC_EXTCROP with flags field equal to
>> V4L2_SEL_TOP_LE | V4L2_SEL_LEFT_LE |
>> V4L2_SEL_RIGHT_GE | V4L2_SEL_BOTTOM_GE.
>> - VIDIOC_COMPOSE with flags field equal to
>> V4L2_SEL_TOP_FIXED | V4L2_SEL_LEFT_FIXED |
>> V4L2_SEL_WIDTH_LE | V4L2_SEL_HEIGHT_LE
>>
>> Feel free to add a new flag if necessary.
>
> While this is very flexible, I am a bit concerned about the complexity this
> would introduce in a driver. I think I would want to see this actually
> implemented in a driver first. I suspect that some utility functions are
> probably needed.
>
>>
>> 4. Targets
>> The cropping/composing subsystem may use auxiliary rectangles other than a
>> normal cropping rectangle. The field v4l2_selection::target is used to choose
>> the rectangle. This functionality was added to simulate VIDIOC_CROPCAP ioctl.
>> All cropcap fields except pixel aspect are supported. I noticed that there was
>> discussion about pixel aspect and I am not convinced that it should be a part
>> of the cropping API. Please refer to the post:
>> http://lists-archives.org/video4linux/22837-need-vidioc_cropcap-clarification.html
>>
>> Proposed targets are:
>> - active - numerical value 0, an area that is processed by hardware
>> - default - is the suggested active rectangle that covers the "whole picture"
>> - bounds - the limits that active rectangle cannot exceed
>>
>> V4L2_SEL_TARGET_ACTIVE = 0
>> V4L2_SEL_TARGET_DEFAULT = 1
>> V4L2_SEL_TARGET_BOUNDS = 2
>>
>> Feel free to add other targets.
>>
>> Only V4L2_SEL_TARGET_ACTIVE is accepted for VIDIOC_S_EXTCROP/VIDIOC_S_COMPOSE
>> ioctls. Auxiliary target like DEFAULT and BOUNDS are supported only by 'get'
>> interface.
>
> Sorry, but I really don't like this idea of a target. It doesn't make sense to
> add this when you can only choose a different target for a get.
>
> I think a EXTCROPCAP/COMPOSECAP pair (or a single CROPCOMPOSECAP ioctl, see below)
> makes more sense.
>
I think that we should avoid adding new ioctl if they are not required.
Laurent suggested that it natural solution to use G_EXTCROP to get
different kinds of rectangles.
>>
>> 5. Possible improvements and extensions.
>> - combine composing and cropping ioctl into a single ioctl
>
> I think this could be very interesting. By doing this in a single ioctl you
> should have all the information needed to setup a scaler. And with the hints
> you can tell the driver how the input/output rectangles need to be adjusted.
>
> This would make sense as well on the subdev level.
>
> Laurent, wouldn't this solve the way the omap3 ISP sets up the scaler? By
> fixing the output of the scaler and setting hints to allow changes to the
> input crop rectangle we would fix the scaler setup issues we discussed in
> Warsaw.
>
What about merging EXTCROP and COMPOSE into single ioct, which argument
contains two rectangles.. similar to the one posted here:
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/28945
>> - add subpixel resolution
>> * hardware is often capable of subpixel processing. The ioctl triple
>> S_EXTCROP, S_SCALE, S_COMPOSE can be converted to S_EXTCROP and S_COMPOSE
>> pair if a subpixel resolution is supported
>
> I'm not sure I understand this. Can you give an example?
>
There is a problem with scaling. Look at the following (little non-life)
example:
Assume that we have sensor with 2x2 resolution.
The buffer has resolution 4x4.
One would like to crop area of size 1.5x1.5 and copy to to area 3x3 in
memory buffer.
Current Crop api does not allow it because first it does not support
composing, second there is no subpixel resolution in crop rectangles.
I know this could be solved in two ways:
a) use S_SCALING:
- set cropping 2x2 on sensor
- set scaling 2x using S_SCALING
- set compose in buffer as 3x3 area (it will work like clipping)
b) use subpixel cropping:
- set cropping area 1.5x1.5 using S_EXTCROP
- set composing in buffer to 3x3 area
The problem with solution a) is need of introduction new family of
ioctls {G/S/TRY}_SCALING, and making composing work as clipping for
scaled data.
I suppose that subpixel cropping could be realized in one of two ways:
1. Introducing v4l2_fract_rect structure. Its all fields type would be
v4l2_fract.
2. Use one of reserved fields in v4l2_selection as divisor for
coordinates in v4l2_selection::r.
What is your opinion?
>> - merge v4l2_selection::target and v4l2_selection::flags into single field
>> - allow using VIDIOC_S_EXTCROP with target type V4L2_SEL_TARGET_BOUNDS to
>> choose a resolution of a sensor
>
> Too obscure IMHO. That said, it would be nice to have a more explicit method
> of selecting a sensor resolution. You can enumerate them, but you choose it
> using VIDIOC_S_FMT, which I've always thought was very dubious. This prevents
> any sensor-built-in scalers from being used. For video you have S_STD and
> S_DV_PRESET that select a particular input resolution, but a similar ioctl is
> missing for sensors. Laurent, what are your thoughts?
>
>> - add TRY flag to ask a driver to adjust a rectangle without applying it
>
> Don't use a flag, use TRY_EXTCROP and TRY_COMPOSE, just like the other try
> ioctls. Otherwise I'm in favor of this.
>
> Regards,
>
> Hans
>
>> What it your opinion about proposed solutions?
>>
>> Looking for a reply,
>>
>> Best regards,
>> Tomasz Stanislawski
>>
>>
>> Tomasz Stanislawski (2):
>> v4l: add support for extended crop/compose API
>> v4l: simulate old crop API using extcrop/compose
>>
>> drivers/media/video/v4l2-compat-ioctl32.c | 4 +
>> drivers/media/video/v4l2-ioctl.c | 150 ++++++++++++++++++++++++++---
>> include/linux/videodev2.h | 44 +++++++++
>> include/media/v4l2-ioctl.h | 8 ++
>> 4 files changed, 194 insertions(+), 12 deletions(-)
>>
>>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/2] V4L: Extended crop/compose API, ver2
2011-04-11 10:42 ` Tomasz Stanislawski
@ 2011-04-12 9:40 ` Laurent Pinchart
2011-04-12 15:41 ` Tomasz Stanislawski
0 siblings, 1 reply; 10+ messages in thread
From: Laurent Pinchart @ 2011-04-12 9:40 UTC (permalink / raw)
To: Tomasz Stanislawski; +Cc: linux-media, Hans Verkuil
Hi Hans, Tomasz,
On Monday 11 April 2011 12:42:10 Tomasz Stanislawski wrote:
> Hans Verkuil wrote:
> > On Wednesday, April 06, 2011 10:44:17 Tomasz Stanislawski wrote:
> >> Hello everyone,
> >>
> >> This patch-set introduces new ioctls to V4L2 API. The new method for
> >> configuration of cropping and composition is presented.
> >>
> >> This is the second version of extcrop RFC. It was enriched with new
> >> features like additional hint flags, and a support for auxiliary
> >> crop/compose rectangles.
> >>
> >> There is some confusion in understanding of a cropping in current
> >> version of V4L2. For CAPTURE devices cropping refers to choosing only a
> >> part of input data stream and processing it and storing it in a memory
> >> buffer. The buffer is fully filled by data. It is not possible to
> >> choose only a part of a buffer for being updated by hardware.
> >>
> >> In case of OUTPUT devices, the whole content of a buffer is passed by
> >> hardware to output display. Cropping means selecting only a part of an
> >> output display/signal. It is not possible to choose only a part for a
> >> memory buffer to be processed.
> >>
> >> The overmentioned flaws in cropping API were discussed in post:
> >> http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/
> >> 28945
> >>
> >> A solution was proposed during brainstorming session in Warsaw.
> >>
> >>
> >> 1. Data structures.
> >>
> >> The structure v4l2_crop used by current API lacks any place for further
> >> extensions. Therefore new structure is proposed.
> >>
> >> struct v4l2_selection {
> >>
> >> u32 type;
> >> u32 target;
> >> struct v4l2_rect r;
> >> u32 flags;
> >> u32 reserved[9];
> >>
> >> };
> >>
> >> Where,
> >> type - type of buffer queue: V4L2_BUF_TYPE_VIDEO_CAPTURE,
> >>
> >> V4L2_BUF_TYPE_VIDEO_OUTPUT, etc.
> >>
> >> target - choose one of cropping/composing rectangles
> >> r - selection rectangle
> >> flags - control over coordinates adjustments
> >> reserved - place for further extensions, adjust struct size to 64 bytes
> >>
> >> At first, the distinction between cropping and composing was stated. The
> >> cropping operation means choosing only part of input data bounding it by
> >> a cropping rectangle. All other data must be discarded. On the other
> >> hand, composing means pasting processed data into rectangular part of
> >> data sink. The sink may be output device, user buffer, etc.
> >>
> >>
> >> 2. Crop/Compose ioctl.
> >> Four new ioctls would be added to V4L2.
> >>
> >> Name
> >>
> >> VIDIOC_S_EXTCROP - set cropping rectangle on an input of a device
> >>
> >> Synopsis
> >>
> >> int ioctl(fd, VIDIOC_S_EXTCROP, struct v4l2_selection *s)
> >>
> >> Description:
> >> The ioctl is used to configure:
> >> - for input devices, a part of input data that is processed in hardware
> >> - for output devices, a part of a data buffer to be passed to hardware
> >>
> >> Drivers may adjust a cropping area. The adjustment can be controlled
> >>
> >> by v4l2_selection::flags. Please refer to Hints section.
> >>
> >> - an adjusted crop rectangle is returned in v4l2_selection::r
> >>
> >> Return value
> >>
> >> On success 0 is returned, on error -1 and the errno variable is set
> >>
> >> appropriately:
> >> ERANGE - failed to find a rectangle that satisfy all constraints
> >> EINVAL - incorrect buffer type, incorrect target, cropping not
> >> supported
> >>
> >> -----------------------------------------------------------------
> >>
> >> Name
> >>
> >> VIDIOC_G_EXTCROP - get cropping rectangle on an input of a device
> >>
> >> Synopsis
> >>
> >> int ioctl(fd, VIDIOC_G_EXTCROP, struct v4l2_selection *s)
> >>
> >> Description:
> >> The ioctl is used to query:
> >> - for input devices, a part of input data that is processed in hardware
> >>
> >> Other crop rectangles might be examined using this ioctl.
> >> Please refer to Targets section.
> >>
> >> - for output devices, a part of data buffer to be passed to hardware
> >>
> >> Return value
> >>
> >> On success 0 is returned, on error -1 and the errno variable is set
> >>
> >> appropriately:
> >> EINVAL - incorrect buffer type, incorrect target, cropping not
> >> supported
> >>
> >> -----------------------------------------------------------------
> >>
> >> Name
> >>
> >> VIDIOC_S_COMPOSE - set destination rectangle on an output of a device
> >>
> >> Synopsis
> >>
> >> int ioctl(fd, VIDIOC_S_COMPOSE, struct v4l2_selection *s)
> >>
> >> Description:
> >> The ioctl is used to configure:
> >> - for input devices, a part of a data buffer that is filled by hardware
> >> - for output devices, a area on output device where image is inserted
> >> Drivers may adjust a composing area. The adjustment can be controlled
> >>
> >> by v4l2_selection::flags. Please refer to Hints section.
> >>
> >> - an adjusted composing rectangle is returned in v4l2_selection::r
> >>
> >> Return value
> >>
> >> On success 0 is returned, on error -1 and the errno variable is set
> >>
> >> appropriately:
> >> ERANGE - failed to find a rectangle that satisfy all constraints
> >> EINVAL - incorrect buffer type, incorrect target, composing not
> >> supported
> >>
> >> -----------------------------------------------------------------
> >>
> >> Name
> >>
> >> VIDIOC_G_COMPOSE - get destination rectangle on an output of a device
> >>
> >> Synopsis
> >>
> >> int ioctl(fd, VIDIOC_G_COMPOSE, struct v4l2_selection *s)
> >>
> >> Description:
> >> The ioctl is used to query:
> >> - for input devices, a part of a data buffer that is filled by hardware
> >>
> >> Other compose rectangles might be examined using this ioctl.
> >> Please refer to Targets section.
> >>
> >> - for output devices, a area on output device where image is inserted
> >>
> >> Return value
> >>
> >> On success 0 is returned, on error -1 and the errno variable is set
> >>
> >> appropriately:
> >> EINVAL - incorrect buffer type, incorrect target, composing not
> >> supported
> >>
> >> 3. Hints
> >>
> >> The v4l2_selection::flags field is used to give a driver a hint about
> >> coordinate adjustments. Below one can find the proposition of
> >> adjustment flags. The syntax is V4L2_SEL_{name}_{LE/GE}, where {name}
> >> refer to a field in struct v4l2_rect. Two additional properties exist
> >> 'right' and 'bottom'. The refer to respectively: left + width, and top
> >> + height. The LE is abbreviation from "lesser or equal". It prevents
> >> the driver form increasing a parameter. In similar fashion GE means
> >> "greater or equal" and it disallows decreasing. Combining LE and GE
> >> flags prevents the driver from any adjustments of parameters. In such
> >> a manner, setting flags field to zero would give a driver a free hand
> >> in coordinate adjustment.
> >>
> >> #define V4L2_SEL_WIDTH_GE 0x00000001
> >> #define V4L2_SEL_WIDTH_LE 0x00000002
> >> #define V4L2_SEL_HEIGHT_GE 0x00000004
> >> #define V4L2_SEL_HEIGHT_LE 0x00000008
> >> #define V4L2_SEL_LEFT_GE 0x00000010
> >> #define V4L2_SEL_LEFT_LE 0x00000020
> >> #define V4L2_SEL_TOP_GE 0x00000040
> >> #define V4L2_SEL_TOP_LE 0x00000080
> >> #define V4L2_SEL_RIGHT_GE 0x00000100
> >> #define V4L2_SEL_RIGHT_LE 0x00000200
> >> #define V4L2_SEL_BOTTOM_GE 0x00000400
> >> #define V4L2_SEL_BOTTOM_LE 0x00000800
> >>
> >> #define V4L2_SEL_WIDTH_FIXED 0x00000003
> >> #define V4L2_SEL_HEIGHT_FIXED 0x0000000c
> >> #define V4L2_SEL_LEFT_FIXED 0x00000030
> >> #define V4L2_SEL_TOP_FIXED 0x000000c0
> >> #define V4L2_SEL_RIGHT_FIXED 0x00000300
> >> #define V4L2_SEL_BOTTOM_FIXED 0x00000c00
> >>
> >> #define V4L2_SEL_FIXED 0x00000fff
> >>
> >> The hint flags may be useful in a following scenario. There is a sensor
> >> with a face detection functionality. An application receives
> >> information about a position of a face on sensor array. Assume that the
> >> camera pipeline is capable of an image scaling. The application is
> >> capable of obtaining a location of a face using V4L2 controls. The task
> >> it to grab only part of image that contains a face, and store it to a
> >> framebuffer at a fixed window. Therefore following constrains have to
> >> be satisfied:
> >> - the rectangle that contains a face must lay inside cropping area
> >> - hardware is allowed only to access area inside window on the
> >> framebuffer
> >>
> >> Both constraints could be satisfied with two ioctl calls.
> >> - VIDIOC_EXTCROP with flags field equal to
> >>
> >> V4L2_SEL_TOP_LE | V4L2_SEL_LEFT_LE |
> >> V4L2_SEL_RIGHT_GE | V4L2_SEL_BOTTOM_GE.
> >>
> >> - VIDIOC_COMPOSE with flags field equal to
> >>
> >> V4L2_SEL_TOP_FIXED | V4L2_SEL_LEFT_FIXED |
> >> V4L2_SEL_WIDTH_LE | V4L2_SEL_HEIGHT_LE
> >>
> >> Feel free to add a new flag if necessary.
> >
> > While this is very flexible, I am a bit concerned about the complexity
> > this would introduce in a driver. I think I would want to see this
> > actually implemented in a driver first. I suspect that some utility
> > functions are probably needed.
I'm very concerned about that as well. Without hints, computing the OMAP3 ISP
resizer configuration parameters in the driver is already very complex. With
hints it would become even worse, close to impossible. I know that I won't
have a month to spend on the implementation.
> >> 4. Targets
> >> The cropping/composing subsystem may use auxiliary rectangles other than
> >> a normal cropping rectangle. The field v4l2_selection::target is used
> >> to choose the rectangle. This functionality was added to simulate
> >> VIDIOC_CROPCAP ioctl. All cropcap fields except pixel aspect are
> >> supported. I noticed that there was discussion about pixel aspect and I
> >> am not convinced that it should be a part of the cropping API. Please
> >> refer to the post:
> >> http://lists-archives.org/video4linux/22837-need-vidioc_cropcap-clarific
> >> ation.html
> >>
> >> Proposed targets are:
> >> - active - numerical value 0, an area that is processed by hardware
> >> - default - is the suggested active rectangle that covers the "whole
> >> picture" - bounds - the limits that active rectangle cannot exceed
> >>
> >> V4L2_SEL_TARGET_ACTIVE = 0
> >> V4L2_SEL_TARGET_DEFAULT = 1
> >> V4L2_SEL_TARGET_BOUNDS = 2
> >>
> >> Feel free to add other targets.
> >>
> >> Only V4L2_SEL_TARGET_ACTIVE is accepted for
> >> VIDIOC_S_EXTCROP/VIDIOC_S_COMPOSE ioctls. Auxiliary target like
> >> DEFAULT and BOUNDS are supported only by 'get' interface.
> >
> > Sorry, but I really don't like this idea of a target. It doesn't make
> > sense to add this when you can only choose a different target for a get.
> >
> > I think a EXTCROPCAP/COMPOSECAP pair (or a single CROPCOMPOSECAP ioctl,
> > see below) makes more sense.
>
> I think that we should avoid adding new ioctl if they are not required.
> Laurent suggested that it natural solution to use G_EXTCROP to get
> different kinds of rectangles.
I totally agree here. We don't have G_FMT_CAPTURE and G_FMT_OUTPUT, we have a
single G_FMT ioctl with a buffer type argument. Let's not create a bunch of
ioctls just for the fun of it :-)
Using a target would also make the ioctls easier to extend later if we need to
add new targets.
> >> 5. Possible improvements and extensions.
> >> - combine composing and cropping ioctl into a single ioctl
> >
> > I think this could be very interesting. By doing this in a single ioctl
> > you should have all the information needed to setup a scaler. And with
> > the hints you can tell the driver how the input/output rectangles need
> > to be adjusted.
You would still need S_FMT to define the size of the captured (output) image
for capture (output) devices.
> > This would make sense as well on the subdev level.
> >
> > Laurent, wouldn't this solve the way the omap3 ISP sets up the scaler? By
> > fixing the output of the scaler and setting hints to allow changes to the
> > input crop rectangle we would fix the scaler setup issues we discussed in
> > Warsaw.
On subdevs you would need something even more generic, with the ability to set
parameters on multiple pads at the same time. For the OMAP3 ISP scaler,
cropping is done on the input, and we need to set format on the output pads.
There's no compose capability (that's not entirely true, compose could be
implemented by configuring offsets and line length in the DMA engine, but
that's unrelated).
> What about merging EXTCROP and COMPOSE into single ioct, which argument
> contains two rectangles.. similar to the one posted here:
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/2894
> 5
Would that allow new use cases that can't be supported by two separate ioctls
?
It would also double the number of hints. I would then create a separate hint
flags field, as we would already use 24 out of the 32 available flags.
> >> - add subpixel resolution
> >>
> >> * hardware is often capable of subpixel processing. The ioctl triple
> >>
> >> S_EXTCROP, S_SCALE, S_COMPOSE can be converted to S_EXTCROP and
> >> S_COMPOSE pair if a subpixel resolution is supported
> >
> > I'm not sure I understand this. Can you give an example?
>
> There is a problem with scaling. Look at the following (little non-life)
> example:
> Assume that we have sensor with 2x2 resolution.
> The buffer has resolution 4x4.
> One would like to crop area of size 1.5x1.5 and copy to to area 3x3 in
> memory buffer.
> Current Crop api does not allow it because first it does not support
> composing, second there is no subpixel resolution in crop rectangles.
>
> I know this could be solved in two ways:
> a) use S_SCALING:
> - set cropping 2x2 on sensor
> - set scaling 2x using S_SCALING
> - set compose in buffer as 3x3 area (it will work like clipping)
>
> b) use subpixel cropping:
> - set cropping area 1.5x1.5 using S_EXTCROP
> - set composing in buffer to 3x3 area
>
> The problem with solution a) is need of introduction new family of
> ioctls {G/S/TRY}_SCALING, and making composing work as clipping for
> scaled data.
>
> I suppose that subpixel cropping could be realized in one of two ways:
> 1. Introducing v4l2_fract_rect structure. Its all fields type would be
> v4l2_fract.
> 2. Use one of reserved fields in v4l2_selection as divisor for
> coordinates in v4l2_selection::r.
>
> What is your opinion?
>
> >> - merge v4l2_selection::target and v4l2_selection::flags into single
> >> field - allow using VIDIOC_S_EXTCROP with target type
> >> V4L2_SEL_TARGET_BOUNDS to
> >>
> >> choose a resolution of a sensor
> >
> > Too obscure IMHO. That said, it would be nice to have a more explicit
> > method of selecting a sensor resolution. You can enumerate them, but you
> > choose it using VIDIOC_S_FMT, which I've always thought was very
> > dubious. This prevents any sensor-built-in scalers from being used. For
> > video you have S_STD and S_DV_PRESET that select a particular input
> > resolution, but a similar ioctl is missing for sensors. Laurent, what
> > are your thoughts?
> >
> >> - add TRY flag to ask a driver to adjust a rectangle without applying it
> >
> > Don't use a flag, use TRY_EXTCROP and TRY_COMPOSE, just like the other
> > try ioctls. Otherwise I'm in favor of this.
I actually like the flag better :-) It avoid adding too many new ioctls and
it's in line with what we do on subdevs.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/2] V4L: Extended crop/compose API, ver2
2011-04-12 9:40 ` Laurent Pinchart
@ 2011-04-12 15:41 ` Tomasz Stanislawski
0 siblings, 0 replies; 10+ messages in thread
From: Tomasz Stanislawski @ 2011-04-12 15:41 UTC (permalink / raw)
To: linux-media
Hi Laurent,
Thank you for your comments.
> Hi Hans, Tomasz,
>
> On Monday 11 April 2011 12:42:10 Tomasz Stanislawski wrote:
>
>> Hans Verkuil wrote:
>>
>>> On Wednesday, April 06, 2011 10:44:17 Tomasz Stanislawski wrote:
>>>
>>>> Hello everyone,
>>>>
>>>> This patch-set introduces new ioctls to V4L2 API. The new method for
>>>> configuration of cropping and composition is presented.
>>>>
>>>>
>>>>
[snip]
>>>> 3. Hints
>>>>
>>>> The v4l2_selection::flags field is used to give a driver a hint about
>>>> coordinate adjustments. Below one can find the proposition of
>>>> adjustment flags. The syntax is V4L2_SEL_{name}_{LE/GE}, where {name}
>>>> refer to a field in struct v4l2_rect. Two additional properties exist
>>>> 'right' and 'bottom'. The refer to respectively: left + width, and top
>>>> + height. The LE is abbreviation from "lesser or equal". It prevents
>>>> the driver form increasing a parameter. In similar fashion GE means
>>>> "greater or equal" and it disallows decreasing. Combining LE and GE
>>>> flags prevents the driver from any adjustments of parameters. In such
>>>> a manner, setting flags field to zero would give a driver a free hand
>>>> in coordinate adjustment.
>>>>
>>>> #define V4L2_SEL_WIDTH_GE 0x00000001
>>>> #define V4L2_SEL_WIDTH_LE 0x00000002
>>>> #define V4L2_SEL_HEIGHT_GE 0x00000004
>>>> #define V4L2_SEL_HEIGHT_LE 0x00000008
>>>> #define V4L2_SEL_LEFT_GE 0x00000010
>>>> #define V4L2_SEL_LEFT_LE 0x00000020
>>>> #define V4L2_SEL_TOP_GE 0x00000040
>>>> #define V4L2_SEL_TOP_LE 0x00000080
>>>> #define V4L2_SEL_RIGHT_GE 0x00000100
>>>> #define V4L2_SEL_RIGHT_LE 0x00000200
>>>> #define V4L2_SEL_BOTTOM_GE 0x00000400
>>>> #define V4L2_SEL_BOTTOM_LE 0x00000800
>>>>
>>>> #define V4L2_SEL_WIDTH_FIXED 0x00000003
>>>> #define V4L2_SEL_HEIGHT_FIXED 0x0000000c
>>>> #define V4L2_SEL_LEFT_FIXED 0x00000030
>>>> #define V4L2_SEL_TOP_FIXED 0x000000c0
>>>> #define V4L2_SEL_RIGHT_FIXED 0x00000300
>>>> #define V4L2_SEL_BOTTOM_FIXED 0x00000c00
>>>>
>>>> #define V4L2_SEL_FIXED 0x00000fff
>>>>
>>>> The hint flags may be useful in a following scenario. There is a
sensor
>>>> with a face detection functionality. An application receives
>>>> information about a position of a face on sensor array. Assume
that the
>>>> camera pipeline is capable of an image scaling. The application is
>>>> capable of obtaining a location of a face using V4L2 controls. The
task
>>>> it to grab only part of image that contains a face, and store it to a
>>>> framebuffer at a fixed window. Therefore following constrains have to
>>>> be satisfied:
>>>> - the rectangle that contains a face must lay inside cropping area
>>>> - hardware is allowed only to access area inside window on the
>>>> framebuffer
>>>>
>>>> Both constraints could be satisfied with two ioctl calls.
>>>> - VIDIOC_EXTCROP with flags field equal to
>>>>
>>>> V4L2_SEL_TOP_LE | V4L2_SEL_LEFT_LE |
>>>> V4L2_SEL_RIGHT_GE | V4L2_SEL_BOTTOM_GE.
>>>>
>>>> - VIDIOC_COMPOSE with flags field equal to
>>>>
>>>> V4L2_SEL_TOP_FIXED | V4L2_SEL_LEFT_FIXED |
>>>> V4L2_SEL_WIDTH_LE | V4L2_SEL_HEIGHT_LE
>>>>
>>>> Feel free to add a new flag if necessary.
>>>>
>>> While this is very flexible, I am a bit concerned about the complexity
>>> this would introduce in a driver. I think I would want to see this
>>> actually implemented in a driver first. I suspect that some utility
>>> functions are probably needed.
>>>
>
> I'm very concerned about that as well. Without hints, computing the
OMAP3 ISP
> resizer configuration parameters in the driver is already very
complex. With
> hints it would become even worse, close to impossible. I know that I
won't
> have a month to spend on the implementation.
>
>
I think we will need a new helper function in V4L2 kernel API in order
to reduce complexity introduced by hints. This function would contain
typical business logic used to adjust cropping rectangle. It would use
structures similar
to struct v4l2_frmsizeenum to specify available ranges of rectangles'
sizes and offsets. The function would
also take hints flags and a rectangle provided by userspace. It would
return an adjusted rectangle.
Moreover, we will need a additional function in form:
u32 v4l2_rect_verify_constraints(struct v4l2_rect *desired, struct
v4l2_rect *proposed);
that checks which constraints are satisfied by 'proposed' rectangle
relative to 'desired' rectangle.
If returned hints have a zero bit that is set in user hints then EINVAL
is returned. Additionally, if
ioctl was called in TRY mode then v4l2_selection::r is substituted with
a rectangle proposed by a driver.
Other solution would be introduction of an ioctl similar to
VIDIOC_ENUM_FRAMESIZES but dedicated for cropping/composing. This way a
business logic could be partially exported to a userspace.
I will try to prepare an RFC about it.
Do you have any suggestions?
Please look below for comments about S_FMT stuff.
>>>> 4. Targets
>>>> The cropping/composing subsystem may use auxiliary rectangles
other than
>>>> a normal cropping rectangle. The field v4l2_selection::target is used
>>>> to choose the rectangle. This functionality was added to simulate
>>>> VIDIOC_CROPCAP ioctl. All cropcap fields except pixel aspect are
>>>> supported. I noticed that there was discussion about pixel aspect
and I
>>>> am not convinced that it should be a part of the cropping API. Please
>>>> refer to the post:
>>>>
http://lists-archives.org/video4linux/22837-need-vidioc_cropcap-clarific
>>>> ation.html
>>>>
>>>> Proposed targets are:
>>>> - active - numerical value 0, an area that is processed by hardware
>>>> - default - is the suggested active rectangle that covers the "whole
>>>> picture" - bounds - the limits that active rectangle cannot exceed
>>>>
>>>> V4L2_SEL_TARGET_ACTIVE = 0
>>>> V4L2_SEL_TARGET_DEFAULT = 1
>>>> V4L2_SEL_TARGET_BOUNDS = 2
>>>>
>>>> Feel free to add other targets.
>>>>
>>>> Only V4L2_SEL_TARGET_ACTIVE is accepted for
>>>> VIDIOC_S_EXTCROP/VIDIOC_S_COMPOSE ioctls. Auxiliary target like
>>>> DEFAULT and BOUNDS are supported only by 'get' interface.
>>>>
>>> Sorry, but I really don't like this idea of a target. It doesn't make
>>> sense to add this when you can only choose a different target for a
get.
>>>
>>> I think a EXTCROPCAP/COMPOSECAP pair (or a single CROPCOMPOSECAP ioctl,
>>> see below) makes more sense.
>>>
>> I think that we should avoid adding new ioctl if they are not required.
>> Laurent suggested that it natural solution to use G_EXTCROP to get
>> different kinds of rectangles.
>>
>
> I totally agree here. We don't have G_FMT_CAPTURE and G_FMT_OUTPUT,
we have a
> single G_FMT ioctl with a buffer type argument. Let's not create a
bunch of
> ioctls just for the fun of it :-)
>
> Using a target would also make the ioctls easier to extend later if
we need to
> add new targets.
>
Thanks for support on the idea :).
>
>>>> 5. Possible improvements and extensions.
>>>> - combine composing and cropping ioctl into a single ioctl
>>>>
>>> I think this could be very interesting. By doing this in a single ioctl
>>> you should have all the information needed to setup a scaler. And with
>>> the hints you can tell the driver how the input/output rectangles need
>>> to be adjusted.
>>>
>
> You would still need S_FMT to define the size of the captured
(output) image
> for capture (output) devices.
>
>
Frankly, I think that there is a general flaw in a purpose of S_FMT.
In V4l2, there are following entities and associated ioctl used for
configuration:
analog TV input/output - VIDIOC_S_STD
digital TV input/output - VIDIOC_S_DV_PRESET
audio input - VIDIOC_S_AUDIO
memory buffer - VIDIOC_S_FMT
Now I ask:
Why S_CROP can change a format in memory buffer (width and size) but it
is not allowed to change DV preset?
Why symmetry is broken between these entities?
In my opinion, a format should stay fixed after successful VIDIOC_S_FMT.
It would mean that width and height of an image must not be changed by
CROP/COMPOSE setup.
For input devices, if an image is too large for desired cropping
rectangle then a buffer's composing rectangle is adjusted. So data from
a sensor are blit on a part of an image. If HW did not support buffer
composing then it would return EINVAL or increase cropping rectangle if
hints allow this.
Using this treat CROP/COMPOSE ioctls could be merged. Driver could
adjust crop/compose rectangle simultaneously according to its scaling
capabilities. No adjustment of resolution of input data, output data.
Moreover no memory management would be involved because a buffer size
would not change. I think it may greatly simply driver's code.
BTW: I think that sensors need some dedicated ioctl for configuration
similar to ioctls available for other entities (like S_DV_PRESET or more
general S_DV_TIMINGS).
>>> This would make sense as well on the subdev level.
>>>
>>> Laurent, wouldn't this solve the way the omap3 ISP sets up the
scaler? By
>>> fixing the output of the scaler and setting hints to allow changes
to the
>>> input crop rectangle we would fix the scaler setup issues we
discussed in
>>> Warsaw.
>>>
>
> On subdevs you would need something even more generic, with the
ability to set
> parameters on multiple pads at the same time. For the OMAP3 ISP scaler,
> cropping is done on the input, and we need to set format on the
output pads.
> There's no compose capability (that's not entirely true, compose
could be
> implemented by configuring offsets and line length in the DMA engine,
but
> that's unrelated).
>
Additional target rectangles may help here.
>> What about merging EXTCROP and COMPOSE into single ioct, which argument
>> contains two rectangles.. similar to the one posted here:
>>
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/2894
>> 5
>>
>
> Would that allow new use cases that can't be supported by two
separate ioctls
> ?
>
If a device has very limited scaling abilities that not all combinations
of cropping/composing rectangles could be accepted. A problem with
configuration was described by you in a post:
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/27581
The driver could choose the best possible configuration because it has
compose and crop rectangles and is allowed to adjust them according to
hints flags.
What do you think about a following idea?
The cropping/compose configuration would introduce no scaling but only
setting active area assuming no scaling.
Scaling configuration would be done using S_SCALING ioctl.
However, some of pixels in destination buffer may be undefined for a
flawed configuration.
> It would also double the number of hints. I would then create a
separate hint
> flags field, as we would already use 24 out of the 32 available flags.
>
Hmm.. there are already to sets of hints flags, one passed by
VIDIOC_S_EXTCROP and another passed by VIDIOC_S_COMPOSE. Therefore a
merged structure would contains two hints fields.
Waiting for comment.
Best regards,
Tomasz Stanislawski
>
>>>> - add subpixel resolution
>>>>
>>>> * hardware is often capable of subpixel processing. The ioctl triple
>>>>
>>>> S_EXTCROP, S_SCALE, S_COMPOSE can be converted to S_EXTCROP and
>>>> S_COMPOSE pair if a subpixel resolution is supported
>>>>
>>> I'm not sure I understand this. Can you give an example?
>>>
>> There is a problem with scaling. Look at the following (little non-life)
>> example:
>> Assume that we have sensor with 2x2 resolution.
>> The buffer has resolution 4x4.
>> One would like to crop area of size 1.5x1.5 and copy to to area 3x3 in
>> memory buffer.
>> Current Crop api does not allow it because first it does not support
>> composing, second there is no subpixel resolution in crop rectangles.
>>
>> I know this could be solved in two ways:
>> a) use S_SCALING:
>> - set cropping 2x2 on sensor
>> - set scaling 2x using S_SCALING
>> - set compose in buffer as 3x3 area (it will work like clipping)
>>
>> b) use subpixel cropping:
>> - set cropping area 1.5x1.5 using S_EXTCROP
>> - set composing in buffer to 3x3 area
>>
>> The problem with solution a) is need of introduction new family of
>> ioctls {G/S/TRY}_SCALING, and making composing work as clipping for
>> scaled data.
>>
>> I suppose that subpixel cropping could be realized in one of two ways:
>> 1. Introducing v4l2_fract_rect structure. Its all fields type would be
>> v4l2_fract.
>> 2. Use one of reserved fields in v4l2_selection as divisor for
>> coordinates in v4l2_selection::r.
>>
>> What is your opinion?
>>
>>
>>>> - merge v4l2_selection::target and v4l2_selection::flags into single
>>>> field - allow using VIDIOC_S_EXTCROP with target type
>>>> V4L2_SEL_TARGET_BOUNDS to
>>>>
>>>> choose a resolution of a sensor
>>>>
>>> Too obscure IMHO. That said, it would be nice to have a more explicit
>>> method of selecting a sensor resolution. You can enumerate them,
but you
>>> choose it using VIDIOC_S_FMT, which I've always thought was very
>>> dubious. This prevents any sensor-built-in scalers from being used. For
>>> video you have S_STD and S_DV_PRESET that select a particular input
>>> resolution, but a similar ioctl is missing for sensors. Laurent, what
>>> are your thoughts?
>>>
>>>
>>>> - add TRY flag to ask a driver to adjust a rectangle without
applying it
>>>>
>>> Don't use a flag, use TRY_EXTCROP and TRY_COMPOSE, just like the other
>>> try ioctls. Otherwise I'm in favor of this.
>>>
>
> I actually like the flag better :-) It avoid adding too many new
ioctls and
> it's in line with what we do on subdevs.
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <201104131507.55171.laurent.pinchart@ideasonboard.com>]