[PATCH 0/2] V4L: Extended crop/compose API

[PATCH 0/2] V4L: Extended crop/compose API

[Tomasz Stanislawski]

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 third version of extended crop/compose RFC. List of applied
changes:
- reduced number of hints and its semantics to be more practical and less
  restrictive
- combined EXTCROP and COMPOSE ioctls into VIDIOC_{S/G}_SELECTION
- introduced crop and compose targets
- introduced try flag that prevents passing configuration to a hardware
- added usage examples

----------------
      RFC
----------------

1. Introduction

There is some confusion in understanding of cropping in current version of
V4L2. In a case of 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. There is now generic API to choose only a part
of an image buffer for being updated by hardware.

In case of OUTPUT devices, the whole content of a buffer is passed to hardware
and to output display. Cropping means selecting only a part of an output
display/signal. It is not possible to choose only a part of the image 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.

At first, the distinction between cropping and composing was stated. The
cropping operation means choosing only part of input data by bounding it by a
cropping rectangle. All data outside cropping area 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.

Two concepts were introduced:

Cropping rectangle: a) for input devices, a part of input data selected by
hardware from input stream and pasted to an image buffer b) for output devices,
a part of image buffer to be passed by hardware to output stream

Composing rectangle: a) for input devices, a part of a image buffer that is
filled by hardware b) for output devices, an area on output device where image
is inserted

The configuration of composing/cropping areas is the subject of this document.

2. Data structures.

The structure v4l2_crop used by current API lacks any place for further
extensions. Therefore new and more generic structure is proposed.

struct v4l2_selection {
	u32 type;
	u32 target;
	u32 flags;
	struct v4l2_rect r;
	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
flags	 - control over coordinates adjustments
r	 - selection rectangle
reserved - place for further extensions, adjust struct size to 64 bytes

3. Crop/Compose ioctl.
New ioctls are added to V4L2.

Name
	VIDIOC_G_SELECTION - get crop/compose rectangles from a driver

Synopsis
	int ioctl(fd, VIDIOC_G_SELECTION, struct v4l2_selection *s)

Description:
	The ioctl is used to query selection rectangles. Currently, it involves
only cropping and composing ones. To query cropping rectangle application must
fill selection::type with respective stream type from V4L2_BUF_TYPE_VIDEO_*
family.  Next, v4l2_selection::target must be field with desired target type.
Please refer to section Target for details. On success the rectangle is
returned in v4l2_selection::r field. Field v4l2_selection::flags and
v4l2_selection::reserved are ignored and they must be filled with zeros.

Return value
	On success 0 is returned, on error -1 and the errno variable is set
        appropriately:
	EINVAL - incorrect buffer type, incorrect/not supported target

-----------------------------------------------------------------

Name
	VIDIOC_S_SELECTION - set cropping rectangle on an input of a device

Synopsis
	int ioctl(fd, VIDIOC_S_SELECTION, struct v4l2_selection *s)

Description:
	The ioctl is used to configure a selection rectangle.  An application
fills v4l2_selection::type field to specify an adequate stream type.  Next, the
v4l2_selection::field target is filled. Basically, an application choose
between cropping or composing rectangle. Please refer to section Targets for
more details. Finally, structure v4l2-selection::r is filled with suggested
coordinates. The coordinates are expressed in driver-dependant units. The only
exception are rectangles for images in raw formats, which coordinates are
expressed in pixels.

Drivers are free to adjust selection rectangles on their own. The suggested
behaviour is to choose a rectangle with the closest coordinates to desired ones
passed in v4l2_selection::r. However, drivers are allowed to ignore suggested
it completely. A driver may even return a fixed and immutable rectangle every
call. If there is an alternative between multiple, then a driver may use hint
flags. Please refer to section Hints. Hints are optional but it is strongly
encouraged to use them.

Applications set V4L2_SEL_TRY flag in v4l2_selection::flags to prevent a driver
from applying selection configuration to hardware.

On success field v4l2_selection::r is filled with adjusted rectangle.

Return value
	On success 0 is returned, on error -1 and the errno variable is set
        appropriately:
	EINVAL - incorrect buffer type, incorrect/not supported target


4. Flags
4.1. Hints

The field v4l2_selection::flags is used to give a driver a hint about
coordinate adjustments. The set of possible hint flags was reduced to two
entities for practical reasons. The flag V4L2_SEL_SIZE_LE is a suggestion for
the driver to decrease or keep size of a rectangle.  The flags V4L2_SEL_SIZE_GE
imply keeping or increasing a size of a rectangle.

By default, lack of any hint flags indicate that driver has to choose selection
coordinates as close as possible to the ones passed in v4l2_selection::r field.

Setting both flags implies that the driver is free to adjust the rectangle.  It
may return even random one however much more encouraged behaviour would be
adjusting coordinates in such a way that other stream parameters are left
intact. This means that the driver should avoid changing a format of an image
buffer and/or any other controls.

The hint flag V4L2_SEL_SIZE_GE may be useful in a following scenario. There is
a sensor with a face detection feature. An application receives information
about a position of a face via V4L2 controls. Assume that the camera's pipeline
is capable of scaling and cropping. The task it to grab only a part of an image
that contains a face. In such a case, the application would try to prevent the
driver from decreasing the rectangle, thus cutting off part of a face. It is
achieved by passing V4L2_SEL_SIZE_GE for a cropping target.

The hint V4L2_SEL_SIZE_LE is useful when data from a sensor are pasted directly
to an application window on a framebuffer. It is expected that the image would
not be inserted outside bounds of the window. Passing V4L2_SEL_SIZE_LE for a
composing target would inform driver not to exceed window's bounds.

Hints definitions:

#define V4L2_SEL_SIZE_GE	0x00000001
#define V4L2_SEL_SIZE_LE	0x00000002

Feel free to add a new flag if necessary.

4.2. Try flag.
A new flag was introduced in order to avoid adding to many new ioctl to V4L2
API.  The flag name if V4L2_SEL_TRY. It is or-ed info field
v4l2_selection::flags.  The flag can only be used with VIDIOC_S_SELECTION
ioctl. It implies that the driver should only adjust passed rectangle. The
rectangle must not be passed to hardware.

#define V4L2_SEL_TRY		0x80000000

Feel free to add a new flag if necessary.

5. Targets
The idea of targets was introduced for to reduce the number of new ioctls and
to increase their flexibility. Both compose and crop rectangles are types of
selection rectangles that describe a pipeline. They are very similar, the only
difference is that one touches data source, another touches data sink.
Therefore crop and compose rectangles are only distinguished by a target type.
The field v4l2_selection::target is used to choose a target.

Following targets are added:

V4L2_SEL_CROP_ACTIVE		= 0
V4L2_SEL_CROP_DEFAULT		= 1
V4L2_SEL_CROP_BOUNDS		= 2
V4L2_SEL_COMPOSE_ACTIVE		= 16 + 0
V4L2_SEL_COMPOSE_DEFAULT	= 16 + 1
V4L2_SEL_COMPOSE_BOUNDS		= 16 + 2

The gap between V4L2_SEL_CROP_BOUNDS and V4L2_SEL_COMPOSE_ACTIVE gives place
for further extensions.

The within cropping/composing target one may use auxiliary rectangles other
than a normal cropping/composing rectangle.  Proposed auxiliary targets are:
- active - 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

Auxiliary target default and bounds can be only used with VIDIOC_G_SELECTION.
This functionality was added to simulate VIDIOC_CROPCAP ioctl.

An application uses V4L2_SEL_CROP_ACTIVE to setup cropping rectangle.  Target
V4L2_SEL_COMPOSE_ACTIVE is used to setup composing rectangle.

All cropcap fields except pixel aspect are supported in new API. 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

Feel free to add new targets if necessary.

6. Usage examples.

6.1. Getting default rectangle of composing for video output.
	struct v4l2_selection sel = {
		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT,
		.target = V4L2_SEL_COMPOSE_DEFAULT,
	};
	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);

6.2. Obtain crop from capture device and use it as composing area for an image
buffer in order to avoid scaling.
	struct v4l2_selection sel = {
		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
		.target = V4L2_SEL_CROP_ACTIVE,
	};
	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);
	... /* some error checking */
	/* using input crop as a composing rectangle for the image */
	sel.target = V4L2_SEL_COMPOSE_ACTIVE;
	ioctl(fd, VIDIOC_S_SELECTION, &sel);

6.3. Setting a composing area on output of size of AT MOST half of limit placed
at a center of a display.
	struct v4l2_selection sel = {
		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT,
		.target = V4L2_SEL_COMPOSE_BOUNDS,
	};
	struct v4l2_rect r;
	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);
	/* setting smaller compose rectangle */
	r.width = sel.r.width / 2;
	r.height = sel.r.height / 2;
	r.left = sel.r.width / 4;
	r.top = sel.r.height / 4;
	sel.r = r;
	sel.flags = V4L2_SEL_SIZE_LE;
	ret = ioctl(fd, VIDIOC_S_SELECTION, &sel);

6.4. Trying to set crop 150 x 100 at coordinates (200,300) on a sensor array.
Inform the driver that the actual crop should contain desired rectangle. The
hardware configuration must not change.

	struct v4l2_selection sel = {
		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
		.target = V4L2_SEL_CROP_ACTIVE,
		.flags = V4L2_SEL_SIZE_GE | V4L2_SEL_TRY,
		.r = {
			.left = 200,
			.top = 300,
			.width = 150,
			.height = 100,
		},
	};
	ret = ioctl(fd, VIDIOC_S_SELECTION, &sel);

7. Possible improvements and extensions.
- add subpixel resolution
  * hardware is often capable of subpixel processing by passing denominator of
    rectangle's coordinates in one of v4l2_selection reserved fields
- introduce more hint
  * add {RIGHT/LEFT/TOP/BOTTOM/HEIGHT/WIDTH}_{LE/GE} flags to give more
    flexibility
  * split SIZE_GE to LEFT_LE | RIGHT_GE | TOP_LE | BOTTOM_GE
  * split SIZE_LE to LEFT_GE | RIGHT_LE | TOP_GE | BOTTOM_LE
- implement VIDIOC_S_MULTISELECTION
  * allow to configure multiple rectangles simultaneously
- add image size of new target
  * remove setup of width and height of image buffer from S_FMT, use S_SELECTION
    with targets:
    + V4L2_SEL_FORMAT_ACTIVE - active format
    + V4L2_SEL_FORMAT_BOUNDS - maximal resolution of an image
    + V4L2_SEL_FORMAT_DEFAULT - optimal suggestion about
  * resolution of an image combined with support for VIDIOC_S_MULTISELECTION
    allows to pass a triple format/crop/compose sizes in a single ioctl

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 extended crop/compose API

 drivers/media/video/v4l2-compat-ioctl32.c |    2 +
 drivers/media/video/v4l2-ioctl.c          |  113 ++++++++++++++++++++++++++---
 include/linux/videodev2.h                 |   26 +++++++
 include/media/v4l2-ioctl.h                |    4 +
 4 files changed, 135 insertions(+), 10 deletions(-)

-- 
1.7.5

[PATCH 1/2] v4l: add support for extended crop/compose API

[Tomasz Stanislawski]

New ioctl for a precise control of cropping and composing:
VIDIOC_S_SELECTION
VIDIOC_G_SELECTION

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
---
 drivers/media/video/v4l2-compat-ioctl32.c |    2 ++
 drivers/media/video/v4l2-ioctl.c          |   28 ++++++++++++++++++++++++++++
 include/linux/videodev2.h                 |   26 ++++++++++++++++++++++++++
 include/media/v4l2-ioctl.h                |    4 ++++
 4 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
index 7c26947..de108d4 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -891,6 +891,8 @@ 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_SELECTION:
+	case VIDIOC_S_SELECTION:
 	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..aeef966 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -223,6 +223,8 @@ 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_SELECTION)]      = "VIDIOC_G_SELECTION",
+	[_IOC_NR(VIDIOC_S_SELECTION)]      = "VIDIOC_S_SELECTION",
 	[_IOC_NR(VIDIOC_G_JPEGCOMP)]       = "VIDIOC_G_JPEGCOMP",
 	[_IOC_NR(VIDIOC_S_JPEGCOMP)]       = "VIDIOC_S_JPEGCOMP",
 	[_IOC_NR(VIDIOC_QUERYSTD)]         = "VIDIOC_QUERYSTD",
@@ -1741,6 +1743,32 @@ static long __video_do_ioctl(struct file *file,
 		ret = ops->vidioc_s_crop(file, fh, p);
 		break;
 	}
+	case VIDIOC_G_SELECTION:
+	{
+		struct v4l2_selection *p = arg;
+
+		if (!ops->vidioc_g_selection)
+			break;
+
+		dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
+
+		ret = ops->vidioc_g_selection(file, fh, p);
+		if (!ret)
+			dbgrect(vfd, "", &p->r);
+		break;
+	}
+	case VIDIOC_S_SELECTION:
+	{
+		struct v4l2_selection *p = arg;
+
+		if (!ops->vidioc_s_selection)
+			break;
+		dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
+		dbgrect(vfd, "", &p->r);
+
+		ret = ops->vidioc_s_selection(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..e044311 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -718,6 +718,28 @@ struct v4l2_crop {
 	struct v4l2_rect        c;
 };
 
+/* Hints for adjustments of selection rectangle */
+#define V4L2_SEL_SIZE_GE	0x00000001
+#define V4L2_SEL_SIZE_LE	0x00000002
+
+enum v4l2_sel_target {
+	V4L2_SEL_CROP_ACTIVE  = 0,
+	V4L2_SEL_CROP_DEFAULT = 1,
+	V4L2_SEL_CROP_BOUNDS  = 2,
+	V4L2_SEL_COMPOSE_ACTIVE  = 16 + 0,
+	V4L2_SEL_COMPOSE_DEFAULT = 16 + 1,
+	V4L2_SEL_COMPOSE_BOUNDS  = 16 + 2,
+};
+
+struct v4l2_selection {
+	enum v4l2_buf_type      type;
+	enum v4l2_sel_target	target;
+	__u32                   flags;
+	struct v4l2_rect        r;
+	__u32                   reserved[9];
+};
+
+
 /*
  *      A N A L O G   V I D E O   S T A N D A R D
  */
@@ -1932,6 +1954,10 @@ 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)
 
+/* Experimental crop/compose API */
+#define VIDIOC_G_SELECTION	_IOWR('V', 92, struct v4l2_selection)
+#define VIDIOC_S_SELECTION	_IOWR('V', 93, 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..e2ccef2 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -194,6 +194,10 @@ struct v4l2_ioctl_ops {
 					struct v4l2_crop *a);
 	int (*vidioc_s_crop)           (struct file *file, void *fh,
 					struct v4l2_crop *a);
+	int (*vidioc_g_selection)      (struct file *file, void *fh,
+					struct v4l2_selection *a);
+	int (*vidioc_s_selection)      (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.5

[PATCH 2/2] v4l: simulate old crop API using extended crop/compose API

[Tomasz Stanislawski]

This patch allows new drivers to work correctly with
applications that use old-style crop API.
The old crop ioctl is simulated by using selection ioctls.

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
---
 drivers/media/video/v4l2-ioctl.c |   85 +++++++++++++++++++++++++++++++++----
 1 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index aeef966..d0a4073 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -1723,11 +1723,31 @@ static long __video_do_ioctl(struct file *file,
 	{
 		struct v4l2_crop *p = arg;
 
-		if (!ops->vidioc_g_crop)
+		dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
+
+		if (ops->vidioc_g_crop) {
+			ret = ops->vidioc_g_crop(file, fh, p);
+		} else
+		if (ops->vidioc_g_selection) {
+			/* simulate capture crop using selection api */
+			struct v4l2_selection s = {
+				.type = p->type,
+				.target = V4L2_SEL_CROP_ACTIVE,
+			};
+
+			/* crop means compose for output devices */
+			if (p->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+				s.target = V4L2_SEL_COMPOSE_ACTIVE;
+
+			ret = ops->vidioc_g_selection(file, fh, &s);
+
+			/* copying results to old structure on success */
+			if (!ret)
+				p->c = s.r;
+		} else {
 			break;
+		}
 
-		dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
-		ret = ops->vidioc_g_crop(file, fh, p);
 		if (!ret)
 			dbgrect(vfd, "", &p->c);
 		break;
@@ -1736,11 +1756,25 @@ 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 {
+			/* simulate capture crop using selection api */
+			struct v4l2_selection s = {
+				.type = p->type,
+				.target = V4L2_SEL_CROP_ACTIVE,
+				.r = p->c,
+			};
+
+			/* crop means compose for output devices */
+			if (p->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+				s.target = V4L2_SEL_COMPOSE_ACTIVE;
+
+			ret = ops->vidioc_g_selection(file, fh, &s);
+		}
 		break;
 	}
 	case VIDIOC_G_SELECTION:
@@ -1773,12 +1807,43 @@ static long __video_do_ioctl(struct file *file,
 	{
 		struct v4l2_cropcap *p = arg;
 
-		/*FIXME: Should also show v4l2_fract pixelaspect */
-		if (!ops->vidioc_cropcap)
+		dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
+		if (ops->vidioc_cropcap) {
+			ret = ops->vidioc_cropcap(file, fh, p);
+		} else
+		if (ops->vidioc_g_selection) {
+			struct v4l2_selection s = { .type = p->type };
+			struct v4l2_rect bounds;
+
+			/* obtaining bounds */
+			if (p->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+				s.target = V4L2_SEL_COMPOSE_BOUNDS;
+			else
+				s.target = V4L2_SEL_CROP_BOUNDS;
+			ret = ops->vidioc_g_selection(file, fh, &s);
+			if (ret)
+				break;
+			bounds = s.r;
+
+			/* obtaining defrect */
+			if (p->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+				s.target = V4L2_SEL_COMPOSE_DEFAULT;
+			else
+				s.target = V4L2_SEL_CROP_DEFAULT;
+			ret = ops->vidioc_g_selection(file, fh, &s);
+			if (ret)
+				break;
+
+			/* storing results */
+			p->bounds = bounds;
+			p->defrect = s.r;
+			p->pixelaspect.numerator = 1;
+			p->pixelaspect.denominator = 1;
+		} else {
 			break;
+		}
 
-		dbgarg(cmd, "type=%s\n", prt_names(p->type, v4l2_type_names));
-		ret = ops->vidioc_cropcap(file, fh, p);
+		/*FIXME: Should also show v4l2_fract pixelaspect */
 		if (!ret) {
 			dbgrect(vfd, "bounds ", &p->bounds);
 			dbgrect(vfd, "defrect ", &p->defrect);
-- 
1.7.5

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Hans Verkuil]

On Thursday, May 05, 2011 11:39:54 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 third version of extended crop/compose RFC. List of applied
> changes:
> - reduced number of hints and its semantics to be more practical and less
>   restrictive
> - combined EXTCROP and COMPOSE ioctls into VIDIOC_{S/G}_SELECTION
> - introduced crop and compose targets
> - introduced try flag that prevents passing configuration to a hardware
> - added usage examples
> 
> ----------------
>       RFC
> ----------------
> 
> 1. Introduction
> 
> There is some confusion in understanding of cropping in current version of
> V4L2. In a case of 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. There is now generic API to choose only a part
> of an image buffer for being updated by hardware.
> 
> In case of OUTPUT devices, the whole content of a buffer is passed to hardware
> and to output display. Cropping means selecting only a part of an output
> display/signal. It is not possible to choose only a part of the image 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.
> 
> At first, the distinction between cropping and composing was stated. The
> cropping operation means choosing only part of input data by bounding it by a
> cropping rectangle. All data outside cropping area 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.
> 
> Two concepts were introduced:
> 
> Cropping rectangle: a) for input devices, a part of input data selected by
> hardware from input stream and pasted to an image buffer b) for output devices,
> a part of image buffer to be passed by hardware to output stream
> 
> Composing rectangle: a) for input devices, a part of a image buffer that is
> filled by hardware b) for output devices, an area on output device where image
> is inserted
> 
> The configuration of composing/cropping areas is the subject of this document.
> 
> 2. Data structures.
> 
> The structure v4l2_crop used by current API lacks any place for further
> extensions. Therefore new and more generic structure is proposed.
> 
> struct v4l2_selection {
> 	u32 type;
> 	u32 target;
> 	u32 flags;
> 	struct v4l2_rect r;
> 	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
> flags	 - control over coordinates adjustments
> r	 - selection rectangle
> reserved - place for further extensions, adjust struct size to 64 bytes
> 
> 3. Crop/Compose ioctl.
> New ioctls are added to V4L2.
> 
> Name
> 	VIDIOC_G_SELECTION - get crop/compose rectangles from a driver
> 
> Synopsis
> 	int ioctl(fd, VIDIOC_G_SELECTION, struct v4l2_selection *s)
> 
> Description:
> 	The ioctl is used to query selection rectangles. Currently, it involves
> only cropping and composing ones. To query cropping rectangle application must
> fill selection::type with respective stream type from V4L2_BUF_TYPE_VIDEO_*
> family.  Next, v4l2_selection::target must be field with desired target type.
> Please refer to section Target for details. On success the rectangle is
> returned in v4l2_selection::r field. Field v4l2_selection::flags and
> v4l2_selection::reserved are ignored and they must be filled with zeros.
> 
> Return value
> 	On success 0 is returned, on error -1 and the errno variable is set
>         appropriately:
> 	EINVAL - incorrect buffer type, incorrect/not supported target

Looks fine.

> 
> -----------------------------------------------------------------
> 
> Name
> 	VIDIOC_S_SELECTION - set cropping rectangle on an input of a device
> 
> Synopsis
> 	int ioctl(fd, VIDIOC_S_SELECTION, struct v4l2_selection *s)
> 
> Description:
> 	The ioctl is used to configure a selection rectangle.  An application
> fills v4l2_selection::type field to specify an adequate stream type.  Next, the
> v4l2_selection::field target is filled. Basically, an application choose
> between cropping or composing rectangle. Please refer to section Targets for
> more details. Finally, structure v4l2-selection::r is filled with suggested
> coordinates. The coordinates are expressed in driver-dependant units. The only
> exception are rectangles for images in raw formats, which coordinates are
> expressed in pixels.
> 
> Drivers are free to adjust selection rectangles on their own. The suggested
> behaviour is to choose a rectangle with the closest coordinates to desired ones
> passed in v4l2_selection::r. However, drivers are allowed to ignore suggested
> it completely. A driver may even return a fixed and immutable rectangle every
> call. If there is an alternative between multiple, then a driver may use hint
> flags. Please refer to section Hints. Hints are optional but it is strongly
> encouraged to use them.
> 
> Applications set V4L2_SEL_TRY flag in v4l2_selection::flags to prevent a driver
> from applying selection configuration to hardware.

I mentioned this before but I am very much opposed to this flag. It is
inconsistent with the TRY_FMT and TRY_EXT_CTRLS ioctls. Note that in
video_ioctl2 it should be just one function with 'try' boolean argument. It
has always been a mistake that the try and set functions were separated in
the driver code IMHO.

I know that the subdev ioctls do not have a try version, but it is not quite
the same in that those try functions actually store the try information.

This is something we need to look into more carefully. I am slowly becoming
convinced that we need some sort of transaction-based configuration for
pipelines.

Regardless of how such a scheme should work, one thing that I believe is
missing in the format ioctls (both on the video and subdev level) is a similar
concept like the flags in this proposal. It would be quite useful if you
could indicate that the desired WxH has to be exact or can be larger or
smaller. It would certainly simplify the interaction between the selection
and scaling.

> 
> On success field v4l2_selection::r is filled with adjusted rectangle.
> 
> Return value
> 	On success 0 is returned, on error -1 and the errno variable is set
>         appropriately:
> 	EINVAL - incorrect buffer type, incorrect/not supported target
> 
> 
> 4. Flags
> 4.1. Hints
> 
> The field v4l2_selection::flags is used to give a driver a hint about
> coordinate adjustments. The set of possible hint flags was reduced to two
> entities for practical reasons. The flag V4L2_SEL_SIZE_LE is a suggestion for
> the driver to decrease or keep size of a rectangle.  The flags V4L2_SEL_SIZE_GE
> imply keeping or increasing a size of a rectangle.
> 
> By default, lack of any hint flags indicate that driver has to choose selection
> coordinates as close as possible to the ones passed in v4l2_selection::r field.
> 
> Setting both flags implies that the driver is free to adjust the rectangle.  It
> may return even random one however much more encouraged behaviour would be
> adjusting coordinates in such a way that other stream parameters are left
> intact. This means that the driver should avoid changing a format of an image
> buffer and/or any other controls.

This makes no sense to me. It sounds like this is what flags == 0 should do.

I would expect that if both flags are specified that that would equal SEL_SIZE_EQ.
I.E., the rectangle has to be exact size-wise, and should be as close as possible 
to the original position.

Otherwise I like these flags.

> The hint flag V4L2_SEL_SIZE_GE may be useful in a following scenario. There is
> a sensor with a face detection feature. An application receives information
> about a position of a face via V4L2 controls. Assume that the camera's pipeline
> is capable of scaling and cropping. The task it to grab only a part of an image
> that contains a face. In such a case, the application would try to prevent the
> driver from decreasing the rectangle, thus cutting off part of a face. It is
> achieved by passing V4L2_SEL_SIZE_GE for a cropping target.
> 
> The hint V4L2_SEL_SIZE_LE is useful when data from a sensor are pasted directly
> to an application window on a framebuffer. It is expected that the image would
> not be inserted outside bounds of the window. Passing V4L2_SEL_SIZE_LE for a
> composing target would inform driver not to exceed window's bounds.
> 
> Hints definitions:
> 
> #define V4L2_SEL_SIZE_GE	0x00000001
> #define V4L2_SEL_SIZE_LE	0x00000002
> 
> Feel free to add a new flag if necessary.
> 
> 4.2. Try flag.
> A new flag was introduced in order to avoid adding to many new ioctl to V4L2
> API.  The flag name if V4L2_SEL_TRY. It is or-ed info field
> v4l2_selection::flags.  The flag can only be used with VIDIOC_S_SELECTION
> ioctl. It implies that the driver should only adjust passed rectangle. The
> rectangle must not be passed to hardware.
> 
> #define V4L2_SEL_TRY		0x80000000
> 
> Feel free to add a new flag if necessary.

As mentioned above, I still do not like this at all.
 
> 5. Targets
> The idea of targets was introduced for to reduce the number of new ioctls and
> to increase their flexibility. Both compose and crop rectangles are types of
> selection rectangles that describe a pipeline. They are very similar, the only
> difference is that one touches data source, another touches data sink.
> Therefore crop and compose rectangles are only distinguished by a target type.
> The field v4l2_selection::target is used to choose a target.
> 
> Following targets are added:
> 
> V4L2_SEL_CROP_ACTIVE		= 0
> V4L2_SEL_CROP_DEFAULT		= 1
> V4L2_SEL_CROP_BOUNDS		= 2
> V4L2_SEL_COMPOSE_ACTIVE		= 16 + 0
> V4L2_SEL_COMPOSE_DEFAULT	= 16 + 1
> V4L2_SEL_COMPOSE_BOUNDS		= 16 + 2
> 
> The gap between V4L2_SEL_CROP_BOUNDS and V4L2_SEL_COMPOSE_ACTIVE gives place
> for further extensions.

Just 16? I'd go with 256. We got 32 bits to play with here :-)
 
> The within cropping/composing target one may use auxiliary rectangles other
> than a normal cropping/composing rectangle.  Proposed auxiliary targets are:
> - active - 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
> 
> Auxiliary target default and bounds can be only used with VIDIOC_G_SELECTION.
> This functionality was added to simulate VIDIOC_CROPCAP ioctl.
> 
> An application uses V4L2_SEL_CROP_ACTIVE to setup cropping rectangle.  Target
> V4L2_SEL_COMPOSE_ACTIVE is used to setup composing rectangle.

OK, I'm giving up my resistance to having default and bounds targets. I have to
admit that it looks clean.

One thing I think would be helpful though, is if the target name would tell you
whether it is a read-only or a read-write target. It might also be helpful if
the IDs of the read-only targets would set some read-only bit. That would make
it easy for video_ioctl2 to test for invalid targets for S_SELECTION.

Not sure about the naming though:

V4L2_SEL_RO_CROP_DEFAULT
V4L2_SEL_CROP_RO_DEFAULT
V4L2_SEL_CROP_DEFAULT_RO

None looks right. A read-only bit might be sufficient as it would clearly
indicate in the header that that target is a read-only target.
 
> All cropcap fields except pixel aspect are supported in new API. 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

Yeah, what are we going to do about that? I agree that it does not belong here.
But we can't sweep it under the carpet either.

The pixel aspect ratio is typically a property of the current input or output
video format (either a DV preset or a PAL/NTSC STD). For DV presets we could
add it to struct v4l2_dv_enum_presets and we could do the same for STD formats
by adding it to struct v4l2_standard.

This would fail for sensors, though, since there the chosen sensor framesize
is set through S_FMT. (This never quite made sense to me, though, from a V4L2
API perspective). I'm not sure whether we can always assume 1:1 pixel ratio
for sensors. Does anyone know?
 
> Feel free to add new targets if necessary.
> 
> 6. Usage examples.
> 
> 6.1. Getting default rectangle of composing for video output.
> 	struct v4l2_selection sel = {
> 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT,
> 		.target = V4L2_SEL_COMPOSE_DEFAULT,
> 	};
> 	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);
> 
> 6.2. Obtain crop from capture device and use it as composing area for an image
> buffer in order to avoid scaling.
> 	struct v4l2_selection sel = {
> 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
> 		.target = V4L2_SEL_CROP_ACTIVE,
> 	};
> 	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);
> 	... /* some error checking */
> 	/* using input crop as a composing rectangle for the image */
> 	sel.target = V4L2_SEL_COMPOSE_ACTIVE;
> 	ioctl(fd, VIDIOC_S_SELECTION, &sel);
> 
> 6.3. Setting a composing area on output of size of AT MOST half of limit placed
> at a center of a display.
> 	struct v4l2_selection sel = {
> 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT,
> 		.target = V4L2_SEL_COMPOSE_BOUNDS,
> 	};
> 	struct v4l2_rect r;
> 	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);
> 	/* setting smaller compose rectangle */
> 	r.width = sel.r.width / 2;
> 	r.height = sel.r.height / 2;
> 	r.left = sel.r.width / 4;
> 	r.top = sel.r.height / 4;
> 	sel.r = r;
> 	sel.flags = V4L2_SEL_SIZE_LE;
> 	ret = ioctl(fd, VIDIOC_S_SELECTION, &sel);
> 
> 6.4. Trying to set crop 150 x 100 at coordinates (200,300) on a sensor array.
> Inform the driver that the actual crop should contain desired rectangle. The
> hardware configuration must not change.
> 
> 	struct v4l2_selection sel = {
> 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
> 		.target = V4L2_SEL_CROP_ACTIVE,
> 		.flags = V4L2_SEL_SIZE_GE | V4L2_SEL_TRY,
> 		.r = {
> 			.left = 200,
> 			.top = 300,
> 			.width = 150,
> 			.height = 100,
> 		},
> 	};
> 	ret = ioctl(fd, VIDIOC_S_SELECTION, &sel);

It still doesn't look right to me, not to have a TRY_SELECTION. You really
have to look carefully at the code to notice that you are not actually
changing the selection but just trying it. I'd see that immediately if the
ioctl would read 'VIDIOC_TRY_SELECTION'.

> 
> 7. Possible improvements and extensions.
> - add subpixel resolution
>   * hardware is often capable of subpixel processing by passing denominator of
>     rectangle's coordinates in one of v4l2_selection reserved fields
> - introduce more hint
>   * add {RIGHT/LEFT/TOP/BOTTOM/HEIGHT/WIDTH}_{LE/GE} flags to give more
>     flexibility
>   * split SIZE_GE to LEFT_LE | RIGHT_GE | TOP_LE | BOTTOM_GE
>   * split SIZE_LE to LEFT_GE | RIGHT_LE | TOP_GE | BOTTOM_LE
> - implement VIDIOC_S_MULTISELECTION
>   * allow to configure multiple rectangles simultaneously
> - add image size of new target
>   * remove setup of width and height of image buffer from S_FMT, use S_SELECTION
>     with targets:
>     + V4L2_SEL_FORMAT_ACTIVE - active format
>     + V4L2_SEL_FORMAT_BOUNDS - maximal resolution of an image
>     + V4L2_SEL_FORMAT_DEFAULT - optimal suggestion about

Interesting idea. Not sure what the implications are, though.

>   * resolution of an image combined with support for VIDIOC_S_MULTISELECTION
>     allows to pass a triple format/crop/compose sizes in a single ioctl

I don't believe S_MULTISELECTION will solve anything. Specific use-cases perhaps,
but not the general problem of setting up a pipeline. I feel another brainstorm
session coming to solve that. We never came to a solution for it in Warsaw.

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 extended crop/compose API
> 
>  drivers/media/video/v4l2-compat-ioctl32.c |    2 +
>  drivers/media/video/v4l2-ioctl.c          |  113 ++++++++++++++++++++++++++---
>  include/linux/videodev2.h                 |   26 +++++++
>  include/media/v4l2-ioctl.h                |    4 +
>  4 files changed, 135 insertions(+), 10 deletions(-)
> 
> 

RE: [PATCH 2/2] v4l: simulate old crop API using extended crop/compose API

[Jonghun Han]

Hi Tomasz Stanislawski,

On Thursday, May 05, 2011 6:40 PM Tomasz Stanislawski wrote:
> This patch allows new drivers to work correctly with applications that use
> old-style crop API.
> The old crop ioctl is simulated by using selection ioctls.
> 
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> ---
>  drivers/media/video/v4l2-ioctl.c |   85
+++++++++++++++++++++++++++++++++----
>  1 files changed, 75 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-
> ioctl.c
> index aeef966..d0a4073 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -1723,11 +1723,31 @@ static long __video_do_ioctl(struct file *file,
>  	{
>  		struct v4l2_crop *p = arg;
> 
> -		if (!ops->vidioc_g_crop)
> +		dbgarg(cmd, "type=%s\n", prt_names(p->type,
v4l2_type_names));
> +
> +		if (ops->vidioc_g_crop) {
> +			ret = ops->vidioc_g_crop(file, fh, p);
> +		} else
> +		if (ops->vidioc_g_selection) {
> +			/* simulate capture crop using selection api */
> +			struct v4l2_selection s = {
> +				.type = p->type,
> +				.target = V4L2_SEL_CROP_ACTIVE,
> +			};
> +
> +			/* crop means compose for output devices */
> +			if (p->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> +				s.target = V4L2_SEL_COMPOSE_ACTIVE;
> +

If it also supports V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
how about using Macro like V4L2_TYPE_IS_OUTPUT(type) ?

[snip]

Best regards,
Jonghun Han

Re: [PATCH 2/2] v4l: simulate old crop API using extended crop/compose API

[Tomasz Stanislawski]

Jonghun Han wrote:
> Hi Tomasz Stanislawski,
> 
> On Thursday, May 05, 2011 6:40 PM Tomasz Stanislawski wrote:
>> This patch allows new drivers to work correctly with applications that use
>> old-style crop API.
>> The old crop ioctl is simulated by using selection ioctls.
>>
>> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
>> ---
>>  drivers/media/video/v4l2-ioctl.c |   85
> +++++++++++++++++++++++++++++++++----
>>  1 files changed, 75 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-
>> ioctl.c
>> index aeef966..d0a4073 100644
>> --- a/drivers/media/video/v4l2-ioctl.c
>> +++ b/drivers/media/video/v4l2-ioctl.c
>> @@ -1723,11 +1723,31 @@ static long __video_do_ioctl(struct file *file,
>>  	{
>>  		struct v4l2_crop *p = arg;
>>
>> -		if (!ops->vidioc_g_crop)
>> +		dbgarg(cmd, "type=%s\n", prt_names(p->type,
> v4l2_type_names));
>> +
>> +		if (ops->vidioc_g_crop) {
>> +			ret = ops->vidioc_g_crop(file, fh, p);
>> +		} else
>> +		if (ops->vidioc_g_selection) {
>> +			/* simulate capture crop using selection api */
>> +			struct v4l2_selection s = {
>> +				.type = p->type,
>> +				.target = V4L2_SEL_CROP_ACTIVE,
>> +			};
>> +
>> +			/* crop means compose for output devices */
>> +			if (p->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
>> +				s.target = V4L2_SEL_COMPOSE_ACTIVE;
>> +
> 
> If it also supports V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> how about using Macro like V4L2_TYPE_IS_OUTPUT(type) ?
> 
> [snip]
> 
> Best regards,
> Jonghun Han
> 
> 
Hi Jonghun,
Thank you for noticing MPLANE bug. I will fix it in next version.
There is some version of V4L2 with automatic conversion of buffer type.
However, the main purpose of this RFC is discussion over extended crop API.
Patches are a less relevant part at the moment.
I hope that the final consensus over API will emerge soon.
Do you have any comment or suggestions?

Best regards,
Tomasz Stanislawski

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Laurent Pinchart]

Hi Tomasz and Hans,

First of all, apologies for the late reply.

On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
> On Thursday, May 05, 2011 11:39:54 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 third version of extended crop/compose RFC. List of applied
> > changes:
> > - reduced number of hints and its semantics to be more practical and less
> >   restrictive
> > - combined EXTCROP and COMPOSE ioctls into VIDIOC_{S/G}_SELECTION
> > - introduced crop and compose targets
> > - introduced try flag that prevents passing configuration to a hardware
> > - added usage examples
> > 
> > ----------------
> >       RFC
> > ----------------
> > 
> > 1. Introduction
> > 
> > There is some confusion in understanding of cropping in current version
> > of V4L2. In a case of 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. There is now generic
> > API to choose only a part of an image buffer for being updated by
> > hardware.
> > 
> > In case of OUTPUT devices, the whole content of a buffer is passed to
> > hardware and to output display. Cropping means selecting only a part of
> > an output display/signal. It is not possible to choose only a part of
> > the image buffer to be processed.
> > 
> > The overmentioned flaws in cropping API were discussed in post:
> > http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/2
> > 8945
> > 
> > A solution was proposed during brainstorming session in Warsaw.
> > 
> > At first, the distinction between cropping and composing was stated. The
> > cropping operation means choosing only part of input data by bounding it
> > by a cropping rectangle. All data outside cropping area 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.
> > 
> > Two concepts were introduced:
> > 
> > Cropping rectangle: a) for input devices, a part of input data selected
> > by hardware from input stream and pasted to an image buffer b) for
> > output devices, a part of image buffer to be passed by hardware to
> > output stream
> > 
> > Composing rectangle: a) for input devices, a part of a image buffer that
> > is filled by hardware b) for output devices, an area on output device
> > where image is inserted
> > 
> > The configuration of composing/cropping areas is the subject of this
> > document.
> > 
> > 2. Data structures.
> > 
> > The structure v4l2_crop used by current API lacks any place for further
> > extensions. Therefore new and more generic structure is proposed.
> > 
> > struct v4l2_selection {
> > 
> > 	u32 type;
> > 	u32 target;
> > 	u32 flags;
> > 	struct v4l2_rect r;
> > 	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
> > flags	 - control over coordinates adjustments
> > r	 - selection rectangle
> > reserved - place for further extensions, adjust struct size to 64 bytes
> > 
> > 3. Crop/Compose ioctl.
> > New ioctls are added to V4L2.
> > 
> > Name
> > 
> > 	VIDIOC_G_SELECTION - get crop/compose rectangles from a driver
> > 
> > Synopsis
> > 
> > 	int ioctl(fd, VIDIOC_G_SELECTION, struct v4l2_selection *s)
> > 
> > Description:
> > 	The ioctl is used to query selection rectangles. Currently, it involves
> > 
> > only cropping and composing ones. To query cropping rectangle application
> > must fill selection::type with respective stream type from
> > V4L2_BUF_TYPE_VIDEO_* family.  Next, v4l2_selection::target must be
> > field with desired target type. Please refer to section Target for
> > details. On success the rectangle is returned in v4l2_selection::r
> > field. Field v4l2_selection::flags and v4l2_selection::reserved are
> > ignored and they must be filled with zeros.
> > 
> > Return value
> > 
> > 	On success 0 is returned, on error -1 and the errno variable is set
> > 	
> >         appropriately:
> > 	EINVAL - incorrect buffer type, incorrect/not supported target
> 
> Looks fine.
> 
> > -----------------------------------------------------------------
> > 
> > Name
> > 
> > 	VIDIOC_S_SELECTION - set cropping rectangle on an input of a device
> > 
> > Synopsis
> > 
> > 	int ioctl(fd, VIDIOC_S_SELECTION, struct v4l2_selection *s)
> > 
> > Description:
> > 	The ioctl is used to configure a selection rectangle.  An application
> > 
> > fills v4l2_selection::type field to specify an adequate stream type. 
> > Next, the v4l2_selection::field target is filled. Basically, an
> > application choose between cropping or composing rectangle. Please refer
> > to section Targets for more details. Finally, structure
> > v4l2-selection::r is filled with suggested coordinates. The coordinates
> > are expressed in driver-dependant units.

What kind of driver-dependant units do you think will be used in practice ?

> > The only exception are rectangles for images in raw formats, which
> > coordinates are expressed in pixels.
> > 
> > Drivers are free to adjust selection rectangles on their own. The
> > suggested behaviour is to choose a rectangle with the closest
> > coordinates to desired ones passed in v4l2_selection::r. However,
> > drivers are allowed to ignore suggested it completely. A driver may even
> > return a fixed and immutable rectangle every call. If there is an
> > alternative between multiple, then a driver may use hint flags. Please
> > refer to section Hints. Hints are optional but it is strongly encouraged
> > to use them.
> > 
> > Applications set V4L2_SEL_TRY flag in v4l2_selection::flags to prevent a
> > driver from applying selection configuration to hardware.
> 
> I mentioned this before but I am very much opposed to this flag. It is
> inconsistent with the TRY_FMT and TRY_EXT_CTRLS ioctls. Note that in
> video_ioctl2 it should be just one function with 'try' boolean argument. It
> has always been a mistake that the try and set functions were separated in
> the driver code IMHO.
> 
> I know that the subdev ioctls do not have a try version, but it is not
> quite the same in that those try functions actually store the try
> information.

That's exactly why the subdevs pad-level API has a try flag instead of a try 
operation, and that's why g/s_selection on subdevs will be done with a try 
flag.

As for the video device node API, I won't oppose a TRY ioctl, as long as we 
can guarantee there will never be dependencies between the selection 
rectangles and other parameters (or between the different selection 
rectangles). If the crop rectangle depends on the compose rectangle for 
instance, how can you implement a TRY ioctl to try a crop rectangle for a 
specific compose rectangle, without modifying the active compose rectangle 
first ?

> This is something we need to look into more carefully. I am slowly becoming
> convinced that we need some sort of transaction-based configuration for
> pipelines.

This RFC is about video device node configuration, not pipelines. For 
pipelines I think we'll need a transaction-based API. For video device nodes, 
I'm still unsure. As stated above, if we have multiple parameters that depend 
on each other, how do we let the user try them without changing the active 
configuration ?

> Regardless of how such a scheme should work, one thing that I believe is
> missing in the format ioctls (both on the video and subdev level) is a
> similar concept like the flags in this proposal. It would be quite useful
> if you could indicate that the desired WxH has to be exact or can be
> larger or smaller. It would certainly simplify the interaction between the
> selection and scaling.
> 
> > On success field v4l2_selection::r is filled with adjusted rectangle.
> > 
> > Return value
> > 
> > 	On success 0 is returned, on error -1 and the errno variable is set
> > 	
> >         appropriately:
> > 	EINVAL - incorrect buffer type, incorrect/not supported target
> > 
> > 4. Flags
> > 4.1. Hints
> > 
> > The field v4l2_selection::flags is used to give a driver a hint about
> > coordinate adjustments. The set of possible hint flags was reduced to two
> > entities for practical reasons. The flag V4L2_SEL_SIZE_LE is a suggestion
> > for the driver to decrease or keep size of a rectangle.  The flags
> > V4L2_SEL_SIZE_GE imply keeping or increasing a size of a rectangle.
> > 
> > By default, lack of any hint flags indicate that driver has to choose
> > selection coordinates as close as possible to the ones passed in
> > v4l2_selection::r field.
> > 
> > Setting both flags implies that the driver is free to adjust the
> > rectangle.  It may return even random one however much more encouraged
> > behaviour would be adjusting coordinates in such a way that other stream
> > parameters are left intact. This means that the driver should avoid
> > changing a format of an image buffer and/or any other controls.
> 
> This makes no sense to me. It sounds like this is what flags == 0 should
> do.
> 
> I would expect that if both flags are specified that that would equal
> SEL_SIZE_EQ. I.E., the rectangle has to be exact size-wise, and should be
> as close as possible to the original position.

What happens if that's not possible ? The ioctl should never return an error, 
it should modify the rectangle instead. Setting boths flags would be then be 
identical to setting none. I don't think we should allow the user to set both 
hint flags.

> Otherwise I like these flags.

So do I :-)

> > The hint flag V4L2_SEL_SIZE_GE may be useful in a following scenario.
> > There is a sensor with a face detection feature. An application receives
> > information about a position of a face via V4L2 controls. Assume that
> > the camera's pipeline is capable of scaling and cropping. The task it to
> > grab only a part of an image that contains a face. In such a case, the
> > application would try to prevent the driver from decreasing the
> > rectangle, thus cutting off part of a face. It is achieved by passing
> > V4L2_SEL_SIZE_GE for a cropping target.
> > 
> > The hint V4L2_SEL_SIZE_LE is useful when data from a sensor are pasted
> > directly to an application window on a framebuffer. It is expected that
> > the image would not be inserted outside bounds of the window. Passing
> > V4L2_SEL_SIZE_LE for a composing target would inform driver not to
> > exceed window's bounds.
> > 
> > Hints definitions:
> > 
> > #define V4L2_SEL_SIZE_GE	0x00000001
> > #define V4L2_SEL_SIZE_LE	0x00000002
> > 
> > Feel free to add a new flag if necessary.

You should make it clear that drivers must try to keep the rectangle center 
position as close to the requested one, regardless of hint flags.

> > 4.2. Try flag.
> > A new flag was introduced in order to avoid adding to many new ioctl to
> > V4L2 API.  The flag name if V4L2_SEL_TRY. It is or-ed info field
> > v4l2_selection::flags.  The flag can only be used with VIDIOC_S_SELECTION
> > ioctl. It implies that the driver should only adjust passed rectangle.
> > The rectangle must not be passed to hardware.
> > 
> > #define V4L2_SEL_TRY		0x80000000
> > 
> > Feel free to add a new flag if necessary.
> 
> As mentioned above, I still do not like this at all.
> 
> > 5. Targets
> > The idea of targets was introduced for to reduce the number of new ioctls
> > and to increase their flexibility. Both compose and crop rectangles are
> > types of selection rectangles that describe a pipeline. They are very
> > similar, the only difference is that one touches data source, another
> > touches data sink. Therefore crop and compose rectangles are only
> > distinguished by a target type. The field v4l2_selection::target is used
> > to choose a target.
> > 
> > Following targets are added:
> > 
> > V4L2_SEL_CROP_ACTIVE		= 0
> > V4L2_SEL_CROP_DEFAULT		= 1
> > V4L2_SEL_CROP_BOUNDS		= 2
> > V4L2_SEL_COMPOSE_ACTIVE		= 16 + 0
> > V4L2_SEL_COMPOSE_DEFAULT	= 16 + 1
> > V4L2_SEL_COMPOSE_BOUNDS		= 16 + 2
> > 
> > The gap between V4L2_SEL_CROP_BOUNDS and V4L2_SEL_COMPOSE_ACTIVE gives
> > place for further extensions.
> 
> Just 16? I'd go with 256. We got 32 bits to play with here :-)
> 
> > The within cropping/composing target one may use auxiliary rectangles
> > other than a normal cropping/composing rectangle.  Proposed auxiliary
> > targets are: - active - 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
> > 
> > Auxiliary target default and bounds can be only used with
> > VIDIOC_G_SELECTION. This functionality was added to simulate
> > VIDIOC_CROPCAP ioctl.
> > 
> > An application uses V4L2_SEL_CROP_ACTIVE to setup cropping rectangle. 
> > Target V4L2_SEL_COMPOSE_ACTIVE is used to setup composing rectangle.
> 
> OK, I'm giving up my resistance to having default and bounds targets. I
> have to admit that it looks clean.
> 
> One thing I think would be helpful though, is if the target name would tell
> you whether it is a read-only or a read-write target. It might also be
> helpful if the IDs of the read-only targets would set some read-only bit.
> That would make it easy for video_ioctl2 to test for invalid targets for
> S_SELECTION.
> 
> Not sure about the naming though:
> 
> V4L2_SEL_RO_CROP_DEFAULT
> V4L2_SEL_CROP_RO_DEFAULT
> V4L2_SEL_CROP_DEFAULT_RO
> 
> None looks right. A read-only bit might be sufficient as it would clearly
> indicate in the header that that target is a read-only target.

What if some future hardware have setable default or bounds rectangles ? I 
don't know what that would be used for, it's just in case :-)

> > All cropcap fields except pixel aspect are supported in new API. 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-clarifica
> > tion.html
> 
> Yeah, what are we going to do about that? I agree that it does not belong
> here. But we can't sweep it under the carpet either.
> 
> The pixel aspect ratio is typically a property of the current input or
> output video format (either a DV preset or a PAL/NTSC STD). For DV presets
> we could add it to struct v4l2_dv_enum_presets and we could do the same
> for STD formats by adding it to struct v4l2_standard.

Cropping and composing doesn't modify the pixel aspect ratio, so I agree it 
doesn't belong there.

> This would fail for sensors, though, since there the chosen sensor
> framesize is set through S_FMT. (This never quite made sense to me,
> though, from a V4L2 API perspective). I'm not sure whether we can always
> assume 1:1 pixel ratio for sensors. Does anyone know?

Most of the time I suppose so, but I wouldn't be surprise if some exotic 
sensors had non-square pixel aspect ratios.

> > Feel free to add new targets if necessary.
> > 
> > 6. Usage examples.
> > 
> > 6.1. Getting default rectangle of composing for video output.
> > 
> > 	struct v4l2_selection sel = {
> > 	
> > 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT,
> > 		.target = V4L2_SEL_COMPOSE_DEFAULT,
> > 	
> > 	};
> > 	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);
> > 
> > 6.2. Obtain crop from capture device and use it as composing area for an
> > image buffer in order to avoid scaling.
> > 
> > 	struct v4l2_selection sel = {
> > 	
> > 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
> > 		.target = V4L2_SEL_CROP_ACTIVE,
> > 	
> > 	};
> > 	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);
> > 	... /* some error checking */
> > 	/* using input crop as a composing rectangle for the image */
> > 	sel.target = V4L2_SEL_COMPOSE_ACTIVE;
> > 	ioctl(fd, VIDIOC_S_SELECTION, &sel);
> > 
> > 6.3. Setting a composing area on output of size of AT MOST half of limit
> > placed at a center of a display.
> > 
> > 	struct v4l2_selection sel = {
> > 	
> > 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT,
> > 		.target = V4L2_SEL_COMPOSE_BOUNDS,
> > 	
> > 	};
> > 	struct v4l2_rect r;
> > 	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);
> > 	/* setting smaller compose rectangle */
> > 	r.width = sel.r.width / 2;
> > 	r.height = sel.r.height / 2;
> > 	r.left = sel.r.width / 4;
> > 	r.top = sel.r.height / 4;
> > 	sel.r = r;
> > 	sel.flags = V4L2_SEL_SIZE_LE;
> > 	ret = ioctl(fd, VIDIOC_S_SELECTION, &sel);
> > 
> > 6.4. Trying to set crop 150 x 100 at coordinates (200,300) on a sensor
> > array. Inform the driver that the actual crop should contain desired
> > rectangle. The hardware configuration must not change.
> > 
> > 	struct v4l2_selection sel = {
> > 	
> > 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
> > 		.target = V4L2_SEL_CROP_ACTIVE,
> > 		.flags = V4L2_SEL_SIZE_GE | V4L2_SEL_TRY,
> > 		.r = {
> > 		
> > 			.left = 200,
> > 			.top = 300,
> > 			.width = 150,
> > 			.height = 100,
> > 		
> > 		},
> > 	
> > 	};
> > 	ret = ioctl(fd, VIDIOC_S_SELECTION, &sel);
> 
> It still doesn't look right to me, not to have a TRY_SELECTION. You really
> have to look carefully at the code to notice that you are not actually
> changing the selection but just trying it. I'd see that immediately if the
> ioctl would read 'VIDIOC_TRY_SELECTION'.
> 
> > 7. Possible improvements and extensions.
> > - add subpixel resolution
> > 
> >   * hardware is often capable of subpixel processing by passing
> >   denominator of
> >   
> >     rectangle's coordinates in one of v4l2_selection reserved fields
> > 
> > - introduce more hint
> > 
> >   * add {RIGHT/LEFT/TOP/BOTTOM/HEIGHT/WIDTH}_{LE/GE} flags to give more
> >   
> >     flexibility
> >   
> >   * split SIZE_GE to LEFT_LE | RIGHT_GE | TOP_LE | BOTTOM_GE
> >   * split SIZE_LE to LEFT_GE | RIGHT_LE | TOP_GE | BOTTOM_LE
> > 
> > - implement VIDIOC_S_MULTISELECTION
> > 
> >   * allow to configure multiple rectangles simultaneously
> > 
> > - add image size of new target
> > 
> >   * remove setup of width and height of image buffer from S_FMT, use
> >   S_SELECTION
> >   
> >     with targets:
> >     + V4L2_SEL_FORMAT_ACTIVE - active format
> >     + V4L2_SEL_FORMAT_BOUNDS - maximal resolution of an image
> >     + V4L2_SEL_FORMAT_DEFAULT - optimal suggestion about

In that case we might need to rename selection to something else :-)

> Interesting idea. Not sure what the implications are, though.

Breaking API/ABI ?

> >   * resolution of an image combined with support for
> >   VIDIOC_S_MULTISELECTION
> >   
> >     allows to pass a triple format/crop/compose sizes in a single ioctl
> 
> I don't believe S_MULTISELECTION will solve anything. Specific use-cases
> perhaps, but not the general problem of setting up a pipeline. I feel
> another brainstorm session coming to solve that. We never came to a
> solution for it in Warsaw.

Pipelines are configured on subdev nodes, not on video nodes, so I'm also 
unsure whether multiselection support would really be useful.

If we decide to implement multiselection support, we might as well use that 
only. We would need a multiselection target bitmask, so selection targets 
should all be < 32.

Thinking some more about it, does it make sense to set both crop and compose 
on a single video device node (not talking about mem-to-mem, where you use the 
type to multiplex input/output devices on the same node) ? If so, what would 
the use cases be ?

Should all devices support the selection API, or only the simple ones that 
don't expose a user-configurable pipeline to userspace through the MC API ? 
The proposed API looks good to me, but before acking it I'd like to clarify 
how (if at all) this will interact with subdev pad-level configuration on 
devices that support that. My current feeling is that video device nodes for 
such devices should only be used for video streaming. All parameters should be 
configured directly on the subdevs. Application might still need to call S_FMT 
in order to be able to allocate buffers though.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Hans Verkuil]

On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
> Hi Tomasz and Hans,
> 
> First of all, apologies for the late reply.
> 
> On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
> > On Thursday, May 05, 2011 11:39:54 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 third version of extended crop/compose RFC. List of applied
> > > changes:
> > > - reduced number of hints and its semantics to be more practical and less
> > >   restrictive
> > > - combined EXTCROP and COMPOSE ioctls into VIDIOC_{S/G}_SELECTION
> > > - introduced crop and compose targets
> > > - introduced try flag that prevents passing configuration to a hardware
> > > - added usage examples
> > > 
> > > ----------------
> > >       RFC
> > > ----------------
> > > 
> > > 1. Introduction
> > > 
> > > There is some confusion in understanding of cropping in current version
> > > of V4L2. In a case of 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. There is now generic
> > > API to choose only a part of an image buffer for being updated by
> > > hardware.
> > > 
> > > In case of OUTPUT devices, the whole content of a buffer is passed to
> > > hardware and to output display. Cropping means selecting only a part of
> > > an output display/signal. It is not possible to choose only a part of
> > > the image buffer to be processed.
> > > 
> > > The overmentioned flaws in cropping API were discussed in post:
> > > http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/2
> > > 8945
> > > 
> > > A solution was proposed during brainstorming session in Warsaw.
> > > 
> > > At first, the distinction between cropping and composing was stated. The
> > > cropping operation means choosing only part of input data by bounding it
> > > by a cropping rectangle. All data outside cropping area 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.
> > > 
> > > Two concepts were introduced:
> > > 
> > > Cropping rectangle: a) for input devices, a part of input data selected
> > > by hardware from input stream and pasted to an image buffer b) for
> > > output devices, a part of image buffer to be passed by hardware to
> > > output stream
> > > 
> > > Composing rectangle: a) for input devices, a part of a image buffer that
> > > is filled by hardware b) for output devices, an area on output device
> > > where image is inserted
> > > 
> > > The configuration of composing/cropping areas is the subject of this
> > > document.
> > > 
> > > 2. Data structures.
> > > 
> > > The structure v4l2_crop used by current API lacks any place for further
> > > extensions. Therefore new and more generic structure is proposed.
> > > 
> > > struct v4l2_selection {
> > > 
> > > 	u32 type;
> > > 	u32 target;
> > > 	u32 flags;
> > > 	struct v4l2_rect r;
> > > 	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
> > > flags	 - control over coordinates adjustments
> > > r	 - selection rectangle
> > > reserved - place for further extensions, adjust struct size to 64 bytes
> > > 
> > > 3. Crop/Compose ioctl.
> > > New ioctls are added to V4L2.
> > > 
> > > Name
> > > 
> > > 	VIDIOC_G_SELECTION - get crop/compose rectangles from a driver
> > > 
> > > Synopsis
> > > 
> > > 	int ioctl(fd, VIDIOC_G_SELECTION, struct v4l2_selection *s)
> > > 
> > > Description:
> > > 	The ioctl is used to query selection rectangles. Currently, it involves
> > > 
> > > only cropping and composing ones. To query cropping rectangle application
> > > must fill selection::type with respective stream type from
> > > V4L2_BUF_TYPE_VIDEO_* family.  Next, v4l2_selection::target must be
> > > field with desired target type. Please refer to section Target for
> > > details. On success the rectangle is returned in v4l2_selection::r
> > > field. Field v4l2_selection::flags and v4l2_selection::reserved are
> > > ignored and they must be filled with zeros.
> > > 
> > > Return value
> > > 
> > > 	On success 0 is returned, on error -1 and the errno variable is set
> > > 	
> > >         appropriately:
> > > 	EINVAL - incorrect buffer type, incorrect/not supported target
> > 
> > Looks fine.
> > 
> > > -----------------------------------------------------------------
> > > 
> > > Name
> > > 
> > > 	VIDIOC_S_SELECTION - set cropping rectangle on an input of a device
> > > 
> > > Synopsis
> > > 
> > > 	int ioctl(fd, VIDIOC_S_SELECTION, struct v4l2_selection *s)
> > > 
> > > Description:
> > > 	The ioctl is used to configure a selection rectangle.  An application
> > > 
> > > fills v4l2_selection::type field to specify an adequate stream type. 
> > > Next, the v4l2_selection::field target is filled. Basically, an
> > > application choose between cropping or composing rectangle. Please refer
> > > to section Targets for more details. Finally, structure
> > > v4l2-selection::r is filled with suggested coordinates. The coordinates
> > > are expressed in driver-dependant units.
> 
> What kind of driver-dependant units do you think will be used in practice ?
> 
> > > The only exception are rectangles for images in raw formats, which
> > > coordinates are expressed in pixels.
> > > 
> > > Drivers are free to adjust selection rectangles on their own. The
> > > suggested behaviour is to choose a rectangle with the closest
> > > coordinates to desired ones passed in v4l2_selection::r. However,
> > > drivers are allowed to ignore suggested it completely. A driver may even
> > > return a fixed and immutable rectangle every call. If there is an
> > > alternative between multiple, then a driver may use hint flags. Please
> > > refer to section Hints. Hints are optional but it is strongly encouraged
> > > to use them.
> > > 
> > > Applications set V4L2_SEL_TRY flag in v4l2_selection::flags to prevent a
> > > driver from applying selection configuration to hardware.
> > 
> > I mentioned this before but I am very much opposed to this flag. It is
> > inconsistent with the TRY_FMT and TRY_EXT_CTRLS ioctls. Note that in
> > video_ioctl2 it should be just one function with 'try' boolean argument. It
> > has always been a mistake that the try and set functions were separated in
> > the driver code IMHO.
> > 
> > I know that the subdev ioctls do not have a try version, but it is not
> > quite the same in that those try functions actually store the try
> > information.
> 
> That's exactly why the subdevs pad-level API has a try flag instead of a try 
> operation, and that's why g/s_selection on subdevs will be done with a try 
> flag.
> 
> As for the video device node API, I won't oppose a TRY ioctl, as long as we 
> can guarantee there will never be dependencies between the selection 
> rectangles and other parameters (or between the different selection 
> rectangles). If the crop rectangle depends on the compose rectangle for 
> instance, how can you implement a TRY ioctl to try a crop rectangle for a 
> specific compose rectangle, without modifying the active compose rectangle 
> first ?

In that case the TRY would adjust the crop so that it works with the current
compose rectangle.

But this is just one more example of our lack of proper support for pipeline
setup. It doesn't really matter whether this is at the subdev level or at the
driver level, both have the same problems.

This requires a brainstorm session to work out.

> > This is something we need to look into more carefully. I am slowly becoming
> > convinced that we need some sort of transaction-based configuration for
> > pipelines.
> 
> This RFC is about video device node configuration, not pipelines. For 
> pipelines I think we'll need a transaction-based API. For video device nodes, 
> I'm still unsure. As stated above, if we have multiple parameters that depend 
> on each other, how do we let the user try them without changing the active 
> configuration ?

Cropping/scaling/composing IS a pipeline. Until recently the V4L2 device node
API was sufficient to setup the trivial pipelines that most V4L2 consumer
devices have. But with the more modern devices it starts to show its limitations.

The whole 'try' concept we had for a long time needs to be re-examined.

As you remember, I was never satisfied with the subdev 'try' approach either,
but I never could come up with a better alternative.

> > Regardless of how such a scheme should work, one thing that I believe is
> > missing in the format ioctls (both on the video and subdev level) is a
> > similar concept like the flags in this proposal. It would be quite useful
> > if you could indicate that the desired WxH has to be exact or can be
> > larger or smaller. It would certainly simplify the interaction between the
> > selection and scaling.
> > 
> > > On success field v4l2_selection::r is filled with adjusted rectangle.
> > > 
> > > Return value
> > > 
> > > 	On success 0 is returned, on error -1 and the errno variable is set
> > > 	
> > >         appropriately:
> > > 	EINVAL - incorrect buffer type, incorrect/not supported target
> > > 
> > > 4. Flags
> > > 4.1. Hints
> > > 
> > > The field v4l2_selection::flags is used to give a driver a hint about
> > > coordinate adjustments. The set of possible hint flags was reduced to two
> > > entities for practical reasons. The flag V4L2_SEL_SIZE_LE is a suggestion
> > > for the driver to decrease or keep size of a rectangle.  The flags
> > > V4L2_SEL_SIZE_GE imply keeping or increasing a size of a rectangle.
> > > 
> > > By default, lack of any hint flags indicate that driver has to choose
> > > selection coordinates as close as possible to the ones passed in
> > > v4l2_selection::r field.
> > > 
> > > Setting both flags implies that the driver is free to adjust the
> > > rectangle.  It may return even random one however much more encouraged
> > > behaviour would be adjusting coordinates in such a way that other stream
> > > parameters are left intact. This means that the driver should avoid
> > > changing a format of an image buffer and/or any other controls.
> > 
> > This makes no sense to me. It sounds like this is what flags == 0 should
> > do.
> > 
> > I would expect that if both flags are specified that that would equal
> > SEL_SIZE_EQ. I.E., the rectangle has to be exact size-wise, and should be
> > as close as possible to the original position.
> 
> What happens if that's not possible ? The ioctl should never return an error, 

Why not? If I tell the driver that I want exactly WxH, then I see no reason
why it can't return an error. An application might use that result to switch
to a different resolution, for example. E.g., the application tries 640x480
first, that fails, then it tries 320x240 (or whatever).

> it should modify the rectangle instead. Setting boths flags would be then be 
> identical to setting none. I don't think we should allow the user to set both 
> hint flags.
> 
> > Otherwise I like these flags.
> 
> So do I :-)
> 
> > > The hint flag V4L2_SEL_SIZE_GE may be useful in a following scenario.
> > > There is a sensor with a face detection feature. An application receives
> > > information about a position of a face via V4L2 controls. Assume that
> > > the camera's pipeline is capable of scaling and cropping. The task it to
> > > grab only a part of an image that contains a face. In such a case, the
> > > application would try to prevent the driver from decreasing the
> > > rectangle, thus cutting off part of a face. It is achieved by passing
> > > V4L2_SEL_SIZE_GE for a cropping target.
> > > 
> > > The hint V4L2_SEL_SIZE_LE is useful when data from a sensor are pasted
> > > directly to an application window on a framebuffer. It is expected that
> > > the image would not be inserted outside bounds of the window. Passing
> > > V4L2_SEL_SIZE_LE for a composing target would inform driver not to
> > > exceed window's bounds.
> > > 
> > > Hints definitions:
> > > 
> > > #define V4L2_SEL_SIZE_GE	0x00000001
> > > #define V4L2_SEL_SIZE_LE	0x00000002
> > > 
> > > Feel free to add a new flag if necessary.
> 
> You should make it clear that drivers must try to keep the rectangle center 
> position as close to the requested one, regardless of hint flags.

Indeed.
 
> > > 4.2. Try flag.
> > > A new flag was introduced in order to avoid adding to many new ioctl to
> > > V4L2 API.  The flag name if V4L2_SEL_TRY. It is or-ed info field
> > > v4l2_selection::flags.  The flag can only be used with VIDIOC_S_SELECTION
> > > ioctl. It implies that the driver should only adjust passed rectangle.
> > > The rectangle must not be passed to hardware.
> > > 
> > > #define V4L2_SEL_TRY		0x80000000
> > > 
> > > Feel free to add a new flag if necessary.
> > 
> > As mentioned above, I still do not like this at all.
> > 
> > > 5. Targets
> > > The idea of targets was introduced for to reduce the number of new ioctls
> > > and to increase their flexibility. Both compose and crop rectangles are
> > > types of selection rectangles that describe a pipeline. They are very
> > > similar, the only difference is that one touches data source, another
> > > touches data sink. Therefore crop and compose rectangles are only
> > > distinguished by a target type. The field v4l2_selection::target is used
> > > to choose a target.
> > > 
> > > Following targets are added:
> > > 
> > > V4L2_SEL_CROP_ACTIVE		= 0
> > > V4L2_SEL_CROP_DEFAULT		= 1
> > > V4L2_SEL_CROP_BOUNDS		= 2
> > > V4L2_SEL_COMPOSE_ACTIVE		= 16 + 0
> > > V4L2_SEL_COMPOSE_DEFAULT	= 16 + 1
> > > V4L2_SEL_COMPOSE_BOUNDS		= 16 + 2
> > > 
> > > The gap between V4L2_SEL_CROP_BOUNDS and V4L2_SEL_COMPOSE_ACTIVE gives
> > > place for further extensions.
> > 
> > Just 16? I'd go with 256. We got 32 bits to play with here :-)
> > 
> > > The within cropping/composing target one may use auxiliary rectangles
> > > other than a normal cropping/composing rectangle.  Proposed auxiliary
> > > targets are: - active - 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
> > > 
> > > Auxiliary target default and bounds can be only used with
> > > VIDIOC_G_SELECTION. This functionality was added to simulate
> > > VIDIOC_CROPCAP ioctl.
> > > 
> > > An application uses V4L2_SEL_CROP_ACTIVE to setup cropping rectangle. 
> > > Target V4L2_SEL_COMPOSE_ACTIVE is used to setup composing rectangle.
> > 
> > OK, I'm giving up my resistance to having default and bounds targets. I
> > have to admit that it looks clean.
> > 
> > One thing I think would be helpful though, is if the target name would tell
> > you whether it is a read-only or a read-write target. It might also be
> > helpful if the IDs of the read-only targets would set some read-only bit.
> > That would make it easy for video_ioctl2 to test for invalid targets for
> > S_SELECTION.
> > 
> > Not sure about the naming though:
> > 
> > V4L2_SEL_RO_CROP_DEFAULT
> > V4L2_SEL_CROP_RO_DEFAULT
> > V4L2_SEL_CROP_DEFAULT_RO
> > 
> > None looks right. A read-only bit might be sufficient as it would clearly
> > indicate in the header that that target is a read-only target.
> 
> What if some future hardware have setable default or bounds rectangles ? I 
> don't know what that would be used for, it's just in case :-)

If it is settable, then it is no longer a default or bounds rectangle but
some other rectangle :-)
 
> > > All cropcap fields except pixel aspect are supported in new API. 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-clarifica
> > > tion.html
> > 
> > Yeah, what are we going to do about that? I agree that it does not belong
> > here. But we can't sweep it under the carpet either.
> > 
> > The pixel aspect ratio is typically a property of the current input or
> > output video format (either a DV preset or a PAL/NTSC STD). For DV presets
> > we could add it to struct v4l2_dv_enum_presets and we could do the same
> > for STD formats by adding it to struct v4l2_standard.
> 
> Cropping and composing doesn't modify the pixel aspect ratio, so I agree it 
> doesn't belong there.
> 
> > This would fail for sensors, though, since there the chosen sensor
> > framesize is set through S_FMT. (This never quite made sense to me,
> > though, from a V4L2 API perspective). I'm not sure whether we can always
> > assume 1:1 pixel ratio for sensors. Does anyone know?
> 
> Most of the time I suppose so, but I wouldn't be surprise if some exotic 
> sensors had non-square pixel aspect ratios.

Would it make sense to add the pixel ratio to struct v4l2_frmsizeenum?

And what about adding a VIDIOC_G/S_FRAMESIZE to select a sensor resolution?

This would typically be one of the discrete framesizes supported by a sensor
through binning/skipping. If there is also a scaler on the sensor, then that
is controlled through S_FMT. For video it is S_FMT that controls the scaler
(together with S_CROP at the moment), but the source resolution is set through
S_STD/S_DV_PRESET/S_DV_TIMINGS. It always felt very inconsistent to me that
there is no equivalent for sensors, even though you can enumerate all the
available framesizes (just as you can with ENUMSTD and ENUM_DV_PRESETS).

> > > Feel free to add new targets if necessary.
> > > 
> > > 6. Usage examples.
> > > 
> > > 6.1. Getting default rectangle of composing for video output.
> > > 
> > > 	struct v4l2_selection sel = {
> > > 	
> > > 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT,
> > > 		.target = V4L2_SEL_COMPOSE_DEFAULT,
> > > 	
> > > 	};
> > > 	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);
> > > 
> > > 6.2. Obtain crop from capture device and use it as composing area for an
> > > image buffer in order to avoid scaling.
> > > 
> > > 	struct v4l2_selection sel = {
> > > 	
> > > 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
> > > 		.target = V4L2_SEL_CROP_ACTIVE,
> > > 	
> > > 	};
> > > 	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);
> > > 	... /* some error checking */
> > > 	/* using input crop as a composing rectangle for the image */
> > > 	sel.target = V4L2_SEL_COMPOSE_ACTIVE;
> > > 	ioctl(fd, VIDIOC_S_SELECTION, &sel);
> > > 
> > > 6.3. Setting a composing area on output of size of AT MOST half of limit
> > > placed at a center of a display.
> > > 
> > > 	struct v4l2_selection sel = {
> > > 	
> > > 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT,
> > > 		.target = V4L2_SEL_COMPOSE_BOUNDS,
> > > 	
> > > 	};
> > > 	struct v4l2_rect r;
> > > 	ret = ioctl(fd, VIDIOC_G_SELECTION, &sel);
> > > 	/* setting smaller compose rectangle */
> > > 	r.width = sel.r.width / 2;
> > > 	r.height = sel.r.height / 2;
> > > 	r.left = sel.r.width / 4;
> > > 	r.top = sel.r.height / 4;
> > > 	sel.r = r;
> > > 	sel.flags = V4L2_SEL_SIZE_LE;
> > > 	ret = ioctl(fd, VIDIOC_S_SELECTION, &sel);
> > > 
> > > 6.4. Trying to set crop 150 x 100 at coordinates (200,300) on a sensor
> > > array. Inform the driver that the actual crop should contain desired
> > > rectangle. The hardware configuration must not change.
> > > 
> > > 	struct v4l2_selection sel = {
> > > 	
> > > 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
> > > 		.target = V4L2_SEL_CROP_ACTIVE,
> > > 		.flags = V4L2_SEL_SIZE_GE | V4L2_SEL_TRY,
> > > 		.r = {
> > > 		
> > > 			.left = 200,
> > > 			.top = 300,
> > > 			.width = 150,
> > > 			.height = 100,
> > > 		
> > > 		},
> > > 	
> > > 	};
> > > 	ret = ioctl(fd, VIDIOC_S_SELECTION, &sel);
> > 
> > It still doesn't look right to me, not to have a TRY_SELECTION. You really
> > have to look carefully at the code to notice that you are not actually
> > changing the selection but just trying it. I'd see that immediately if the
> > ioctl would read 'VIDIOC_TRY_SELECTION'.
> > 
> > > 7. Possible improvements and extensions.
> > > - add subpixel resolution
> > > 
> > >   * hardware is often capable of subpixel processing by passing
> > >   denominator of
> > >   
> > >     rectangle's coordinates in one of v4l2_selection reserved fields
> > > 
> > > - introduce more hint
> > > 
> > >   * add {RIGHT/LEFT/TOP/BOTTOM/HEIGHT/WIDTH}_{LE/GE} flags to give more
> > >   
> > >     flexibility
> > >   
> > >   * split SIZE_GE to LEFT_LE | RIGHT_GE | TOP_LE | BOTTOM_GE
> > >   * split SIZE_LE to LEFT_GE | RIGHT_LE | TOP_GE | BOTTOM_LE
> > > 
> > > - implement VIDIOC_S_MULTISELECTION
> > > 
> > >   * allow to configure multiple rectangles simultaneously
> > > 
> > > - add image size of new target
> > > 
> > >   * remove setup of width and height of image buffer from S_FMT, use
> > >   S_SELECTION
> > >   
> > >     with targets:
> > >     + V4L2_SEL_FORMAT_ACTIVE - active format
> > >     + V4L2_SEL_FORMAT_BOUNDS - maximal resolution of an image
> > >     + V4L2_SEL_FORMAT_DEFAULT - optimal suggestion about
> 
> In that case we might need to rename selection to something else :-)
> 
> > Interesting idea. Not sure what the implications are, though.
> 
> Breaking API/ABI ?
> 
> > >   * resolution of an image combined with support for
> > >   VIDIOC_S_MULTISELECTION
> > >   
> > >     allows to pass a triple format/crop/compose sizes in a single ioctl
> > 
> > I don't believe S_MULTISELECTION will solve anything. Specific use-cases
> > perhaps, but not the general problem of setting up a pipeline. I feel
> > another brainstorm session coming to solve that. We never came to a
> > solution for it in Warsaw.
> 
> Pipelines are configured on subdev nodes, not on video nodes, so I'm also 
> unsure whether multiselection support would really be useful.
> 
> If we decide to implement multiselection support, we might as well use that 
> only. We would need a multiselection target bitmask, so selection targets 
> should all be < 32.
> 
> Thinking some more about it, does it make sense to set both crop and compose 
> on a single video device node (not talking about mem-to-mem, where you use the 
> type to multiplex input/output devices on the same node) ? If so, what would 
> the use cases be ?
> 
> Should all devices support the selection API, or only the simple ones that 
> don't expose a user-configurable pipeline to userspace through the MC API ? 
> The proposed API looks good to me, but before acking it I'd like to clarify 
> how (if at all) this will interact with subdev pad-level configuration on 
> devices that support that. My current feeling is that video device nodes for 
> such devices should only be used for video streaming. All parameters should be 
> configured directly on the subdevs. Application might still need to call S_FMT 
> in order to be able to allocate buffers though.

This comes back to how we want to implement backwards compatibility for existing
applications. There must be a way for 'standard' apps to work with complex drivers
for specific video nodes (the mc would probably mark those as a 'DEFAULT' node).

I'd say that there are roughly two options: either implement the selection etc.
APIs for those video nodes only in the driver, letting the driver set up the subdev
pipeline, or do it via libv4l SoC-specific plugins.

In my opinion we need to finish the pipeline configuration topic we started in
Warsaw before we can finalize this RFC. This RFC clearly demonstrates that we
have inconsistencies and deficiencies in our API that need to be solved first.
When we have done that, then I expect that this selection API will be easy to
finalize.

Regards,

	Hans

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Laurent Pinchart]

On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
> > On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
> > > On Thursday, May 05, 2011 11:39:54 Tomasz Stanislawski wrote:

[snip]

> > > > Applications set V4L2_SEL_TRY flag in v4l2_selection::flags to
> > > > prevent a driver from applying selection configuration to hardware.
> > > 
> > > I mentioned this before but I am very much opposed to this flag. It is
> > > inconsistent with the TRY_FMT and TRY_EXT_CTRLS ioctls. Note that in
> > > video_ioctl2 it should be just one function with 'try' boolean
> > > argument. It has always been a mistake that the try and set functions
> > > were separated in the driver code IMHO.
> > > 
> > > I know that the subdev ioctls do not have a try version, but it is not
> > > quite the same in that those try functions actually store the try
> > > information.
> > 
> > That's exactly why the subdevs pad-level API has a try flag instead of a
> > try operation, and that's why g/s_selection on subdevs will be done with
> > a try flag.
> > 
> > As for the video device node API, I won't oppose a TRY ioctl, as long as
> > we can guarantee there will never be dependencies between the selection
> > rectangles and other parameters (or between the different selection
> > rectangles). If the crop rectangle depends on the compose rectangle for
> > instance, how can you implement a TRY ioctl to try a crop rectangle for
> > a specific compose rectangle, without modifying the active compose
> > rectangle first ?
> 
> In that case the TRY would adjust the crop so that it works with the
> current compose rectangle.

And how do you try both crop and compose settings without modifying the active 
configuration ? That's not possible, and I think it's a bad API limitation.

> But this is just one more example of our lack of proper support for pipeline
> setup. It doesn't really matter whether this is at the subdev level or at
> the driver level, both have the same problems.
> 
> This requires a brainstorm session to work out.
> 
> > > This is something we need to look into more carefully. I am slowly
> > > becoming convinced that we need some sort of transaction-based
> > > configuration for pipelines.
> > 
> > This RFC is about video device node configuration, not pipelines. For
> > pipelines I think we'll need a transaction-based API. For video device
> > nodes, I'm still unsure. As stated above, if we have multiple parameters
> > that depend on each other, how do we let the user try them without
> > changing the active configuration ?
> 
> Cropping/scaling/composing IS a pipeline. Until recently the V4L2 device
> node API was sufficient to setup the trivial pipelines that most V4L2
> consumer devices have. But with the more modern devices it starts to show
> its limitations.

It's still a simple pipeline, and I think we should aim at making the V4L2 
device node API useful to configure this kind of pipeline. The selection API 
is a superset of the crop API, applications should be able to use it to 
replace the crop API in the long term.

> The whole 'try' concept we had for a long time needs to be re-examined.

I agree.

> As you remember, I was never satisfied with the subdev 'try' approach
> either, but I never could come up with a better alternative.
> 
> > > Regardless of how such a scheme should work, one thing that I believe
> > > is missing in the format ioctls (both on the video and subdev level)
> > > is a similar concept like the flags in this proposal. It would be
> > > quite useful if you could indicate that the desired WxH has to be
> > > exact or can be larger or smaller. It would certainly simplify the
> > > interaction between the selection and scaling.
> > > 
> > > > On success field v4l2_selection::r is filled with adjusted rectangle.
> > > > 
> > > > Return value
> > > > 
> > > > 	On success 0 is returned, on error -1 and the errno variable is set
> > > > 	
> > > >         appropriately:
> > > > 	EINVAL - incorrect buffer type, incorrect/not supported target
> > > > 
> > > > 4. Flags
> > > > 4.1. Hints
> > > > 
> > > > The field v4l2_selection::flags is used to give a driver a hint about
> > > > coordinate adjustments. The set of possible hint flags was reduced to
> > > > two entities for practical reasons. The flag V4L2_SEL_SIZE_LE is a
> > > > suggestion for the driver to decrease or keep size of a rectangle. 
> > > > The flags V4L2_SEL_SIZE_GE imply keeping or increasing a size of a
> > > > rectangle.
> > > > 
> > > > By default, lack of any hint flags indicate that driver has to choose
> > > > selection coordinates as close as possible to the ones passed in
> > > > v4l2_selection::r field.
> > > > 
> > > > Setting both flags implies that the driver is free to adjust the
> > > > rectangle.  It may return even random one however much more
> > > > encouraged behaviour would be adjusting coordinates in such a way
> > > > that other stream parameters are left intact. This means that the
> > > > driver should avoid changing a format of an image buffer and/or any
> > > > other controls.
> > > 
> > > This makes no sense to me. It sounds like this is what flags == 0
> > > should do.
> > > 
> > > I would expect that if both flags are specified that that would equal
> > > SEL_SIZE_EQ. I.E., the rectangle has to be exact size-wise, and should
> > > be as close as possible to the original position.
> > 
> > What happens if that's not possible ? The ioctl should never return an
> > error,
> 
> Why not? If I tell the driver that I want exactly WxH, then I see no reason
> why it can't return an error. An application might use that result to
> switch to a different resolution, for example. E.g., the application tries
> 640x480 first, that fails, then it tries 320x240 (or whatever).

To make the API more consistent. Applications ask drivers for specific 
settings (including optional hints), and drivers return what they've been able 
to configure. It's then the application's responsibility to check the return 
values and act upon it. Drivers shouldn't return an error when setting 
formats/selections, except if the given arguments can't be understood.

[snip]

> > > > All cropcap fields except pixel aspect are supported in new API. 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-clari
> > > > fica tion.html
> > > 
> > > Yeah, what are we going to do about that? I agree that it does not
> > > belong here. But we can't sweep it under the carpet either.
> > > 
> > > The pixel aspect ratio is typically a property of the current input or
> > > output video format (either a DV preset or a PAL/NTSC STD). For DV
> > > presets we could add it to struct v4l2_dv_enum_presets and we could do
> > > the same for STD formats by adding it to struct v4l2_standard.
> > 
> > Cropping and composing doesn't modify the pixel aspect ratio, so I agree
> > it doesn't belong there.
> > 
> > > This would fail for sensors, though, since there the chosen sensor
> > > framesize is set through S_FMT. (This never quite made sense to me,
> > > though, from a V4L2 API perspective). I'm not sure whether we can
> > > always assume 1:1 pixel ratio for sensors. Does anyone know?
> > 
> > Most of the time I suppose so, but I wouldn't be surprise if some exotic
> > sensors had non-square pixel aspect ratios.
> 
> Would it make sense to add the pixel ratio to struct v4l2_frmsizeenum?
> 
> And what about adding a VIDIOC_G/S_FRAMESIZE to select a sensor resolution?
> 
> This would typically be one of the discrete framesizes supported by a
> sensor through binning/skipping. If there is also a scaler on the sensor,
> then that is controlled through S_FMT. For video it is S_FMT that controls
> the scaler (together with S_CROP at the moment), but the source resolution
> is set through S_STD/S_DV_PRESET/S_DV_TIMINGS. It always felt very
> inconsistent to me that there is no equivalent for sensors, even though
> you can enumerate all the available framesizes (just as you can with
> ENUMSTD and ENUM_DV_PRESETS).

Let's take one step back here.

We started with the V4L2 device node API to control (more or less) simple 
devices. Device became more complex, and we created a new MC API (along with 
the subdev pad-level API) to configure complex pipelines. The V4L2 device node 
API still lives, and we want to enhance it to configure medium complexity 
devices.

Before going much further, I think we need to define what a medium complexity 
device is and where we put the boundary between devices that can be configured 
with the V4L2 device node API, and devices that require the MC API.

I believe this shouldn't be too difficult. What we need to do is create a 
simple virtual pipeline that supports cropping, scaling and composing, and map 
the V4L2 device node API to that pipeline configuration. Devices that map to 
that pipeline could then use the V4L2 device node API only, with clearly 
defined semantics.

[snip]

> > > >   * resolution of an image combined with support for
> > > >   VIDIOC_S_MULTISELECTION
> > > >   
> > > >     allows to pass a triple format/crop/compose sizes in a single
> > > >     ioctl
> > > 
> > > I don't believe S_MULTISELECTION will solve anything. Specific
> > > use-cases perhaps, but not the general problem of setting up a
> > > pipeline. I feel another brainstorm session coming to solve that. We
> > > never came to a solution for it in Warsaw.
> > 
> > Pipelines are configured on subdev nodes, not on video nodes, so I'm also
> > unsure whether multiselection support would really be useful.
> >
> > If we decide to implement multiselection support, we might as well use
> > that only. We would need a multiselection target bitmask, so selection
> > targets should all be < 32.
> > 
> > Thinking some more about it, does it make sense to set both crop and
> > compose on a single video device node (not talking about mem-to-mem,
> > where you use the type to multiplex input/output devices on the same
> > node) ? If so, what would the use cases be ?
> > 
> > Should all devices support the selection API, or only the simple ones
> > that don't expose a user-configurable pipeline to userspace through the
> > MC API ? The proposed API looks good to me, but before acking it I'd
> > like to clarify how (if at all) this will interact with subdev pad-level
> > configuration on devices that support that. My current feeling is that
> > video device nodes for such devices should only be used for video
> > streaming. All parameters should be configured directly on the subdevs.
> > Application might still need to call S_FMT in order to be able to
> > allocate buffers though.
> 
> This comes back to how we want to implement backwards compatibility for
> existing applications. There must be a way for 'standard' apps to work
> with complex drivers for specific video nodes (the mc would probably mark
> those as a 'DEFAULT' node).
> 
> I'd say that there are roughly two options: either implement the selection
> etc. APIs for those video nodes only in the driver, letting the driver set
> up the subdev pipeline, or do it via libv4l SoC-specific plugins.
> 
> In my opinion we need to finish the pipeline configuration topic we started
> in Warsaw before we can finalize this RFC. This RFC clearly demonstrates
> that we have inconsistencies and deficiencies in our API that need to be
> solved first. When we have done that, then I expect that this selection
> API will be easy to finalize.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Sylwester Nawrocki]

Hi Laurent, Hans,

On 05/16/2011 09:21 AM, Laurent Pinchart wrote:
> On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
>> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
>>> On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
>>>> On Thursday, May 05, 2011 11:39:54 Tomasz Stanislawski wrote:
> 
> [snip]

...
 
>>>>> All cropcap fields except pixel aspect are supported in new API. 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-clari
>>>>> fica tion.html
>>>>
>>>> Yeah, what are we going to do about that? I agree that it does not
>>>> belong here. But we can't sweep it under the carpet either.
>>>>
>>>> The pixel aspect ratio is typically a property of the current input or
>>>> output video format (either a DV preset or a PAL/NTSC STD). For DV
>>>> presets we could add it to struct v4l2_dv_enum_presets and we could do
>>>> the same for STD formats by adding it to struct v4l2_standard.
>>>
>>> Cropping and composing doesn't modify the pixel aspect ratio, so I agree
>>> it doesn't belong there.
>>>
>>>> This would fail for sensors, though, since there the chosen sensor
>>>> framesize is set through S_FMT. (This never quite made sense to me,
>>>> though, from a V4L2 API perspective). I'm not sure whether we can
>>>> always assume 1:1 pixel ratio for sensors. Does anyone know?
>>>
>>> Most of the time I suppose so, but I wouldn't be surprise if some exotic
>>> sensors had non-square pixel aspect ratios.
>>
>> Would it make sense to add the pixel ratio to struct v4l2_frmsizeenum?
>>
>> And what about adding a VIDIOC_G/S_FRAMESIZE to select a sensor resolution?
>>
>> This would typically be one of the discrete framesizes supported by a
>> sensor through binning/skipping. If there is also a scaler on the sensor,
>> then that is controlled through S_FMT. For video it is S_FMT that controls
>> the scaler (together with S_CROP at the moment), but the source resolution
>> is set through S_STD/S_DV_PRESET/S_DV_TIMINGS. It always felt very
>> inconsistent to me that there is no equivalent for sensors, even though
>> you can enumerate all the available framesizes (just as you can with
>> ENUMSTD and ENUM_DV_PRESETS).
> 
> Let's take one step back here.
> 
> We started with the V4L2 device node API to control (more or less) simple 
> devices. Device became more complex, and we created a new MC API (along with 
> the subdev pad-level API) to configure complex pipelines. The V4L2 device node 
> API still lives, and we want to enhance it to configure medium complexity 
> devices.
> 
> Before going much further, I think we need to define what a medium complexity 
> device is and where we put the boundary between devices that can be configured 
> with the V4L2 device node API, and devices that require the MC API.
> 
> I believe this shouldn't be too difficult. What we need to do is create a 
> simple virtual pipeline that supports cropping, scaling and composing, and map 
> the V4L2 device node API to that pipeline configuration. Devices that map to 
> that pipeline could then use the V4L2 device node API only, with clearly 
> defined semantics.

The need to define clearly what's possible to cover with the V4L2 device node
API sounds reasonable. Nevertheless I like Hans' proposal of new 
VIDIOC_G/S_FRAMESIZE ioctls unifying the video capture and tv/dv APIs.
It extends V4L2 API with capability to handle scaling feature which well might
be available in video pipelines that might not want to use MC API at all.

> [snip]
> 
>>>>>   * resolution of an image combined with support for
>>>>>   VIDIOC_S_MULTISELECTION
>>>>>   
>>>>>     allows to pass a triple format/crop/compose sizes in a single
>>>>>     ioctl
>>>>
>>>> I don't believe S_MULTISELECTION will solve anything. Specific
>>>> use-cases perhaps, but not the general problem of setting up a
>>>> pipeline. I feel another brainstorm session coming to solve that. We
>>>> never came to a solution for it in Warsaw.
>>>
>>> Pipelines are configured on subdev nodes, not on video nodes, so I'm also
>>> unsure whether multiselection support would really be useful.
>>>
>>> If we decide to implement multiselection support, we might as well use
>>> that only. We would need a multiselection target bitmask, so selection
>>> targets should all be < 32.
>>>
>>> Thinking some more about it, does it make sense to set both crop and
>>> compose on a single video device node (not talking about mem-to-mem,
>>> where you use the type to multiplex input/output devices on the same
>>> node) ? If so, what would the use cases be ?

I can't think of any, one either use crop or compose.

>>> Should all devices support the selection API, or only the simple ones
>>> that don't expose a user-configurable pipeline to userspace through the
>>> MC API ? The proposed API looks good to me, but before acking it I'd
>>> like to clarify how (if at all) this will interact with subdev pad-level

>>> configuration on devices that support that. My current feeling is that
>>> video device nodes for such devices should only be used for video
>>> streaming. All parameters should be configured directly on the subdevs.
>>> Application might still need to call S_FMT in order to be able to
>>> allocate buffers though.

I'm not sure whether moving all configuration to subdev API for medium
complexity devices which well might have exposed core functionality through
standard V4L2 device node API and still use MC API for pipeline reconfiguration
(in terms of linking entities with each other) is the way to go.

In order to support existing applications SoC-specific libraries would
have to be used in addition to a MC driver ?
Leaving ourselves without support for default configuration that would work
with "old" V4L2 device node API (in connection with common libv4l library) ?

I thought that weren't the prerequisites when designing the MC API.
I'm afraid we'll end up with two distinct APIs in Video4Linux and thus
two sets of drivers and applications that will not work with one another.

>>
>> This comes back to how we want to implement backwards compatibility for
>> existing applications. There must be a way for 'standard' apps to work
>> with complex drivers for specific video nodes (the mc would probably mark
>> those as a 'DEFAULT' node).
>>
>> I'd say that there are roughly two options: either implement the selection
>> etc. APIs for those video nodes only in the driver, letting the driver set
>> up the subdev pipeline, or do it via libv4l SoC-specific plugins.


>>
>> In my opinion we need to finish the pipeline configuration topic we started
>> in Warsaw before we can finalize this RFC. This RFC clearly demonstrates
>> that we have inconsistencies and deficiencies in our API that need to be
>> solved first. When we have done that, then I expect that this selection
>> API will be easy to finalize.
> 

Regards,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Sylwester Nawrocki]

On 05/18/2011 02:31 PM, Hans Verkuil wrote:
> On Wednesday, May 18, 2011 14:06:21 Sylwester Nawrocki wrote:
>> On 05/16/2011 09:21 AM, Laurent Pinchart wrote:
>> > On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
>> >> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
>> >>> On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
>> >>>> On Thursday, May 05, 2011 11:39:54 Tomasz Stanislawski wrote:
>> >
> 
>> > [snip]
>> ...
> 
>> >>>>> * resolution of an image combined with support for
> 
>> >>>>> VIDIOC_S_MULTISELECTION
> 
>> >>>>>
> 
>> >>>>> allows to pass a triple format/crop/compose sizes in a single
> 
>> >>>>> ioctl
> 
>> >>>>
> 
>> >>>> I don't believe S_MULTISELECTION will solve anything. Specific
> 
>> >>>> use-cases perhaps, but not the general problem of setting up a
> 
>> >>>> pipeline. I feel another brainstorm session coming to solve that. We
> 
>> >>>> never came to a solution for it in Warsaw.
> 
>> >>>
> 
>> >>> Pipelines are configured on subdev nodes, not on video nodes, so I'm also
> 
>> >>> unsure whether multiselection support would really be useful.
> 
>> >>>
> 
>> >>> If we decide to implement multiselection support, we might as well use
> 
>> >>> that only. We would need a multiselection target bitmask, so selection
> 
>> >>> targets should all be < 32.
> 
>> >>>
> 
>> >>> Thinking some more about it, does it make sense to set both crop and
> 
>> >>> compose on a single video device node (not talking about mem-to-mem,
> 
>> >>> where you use the type to multiplex input/output devices on the same
> 
>> >>> node) ? If so, what would the use cases be ?
> 
>>
> 
>> I can't think of any, one either use crop or compose.
> 
> 
> I can: you crop in the video receiver and compose it into a larger buffer.
> 
> Actually quite a desirable feature.

Yes, right. Don't know why I imagined something different.
And we need it in Samsung capture capture interfaces as well. The H/W
is capable of cropping and composing with camera interface as a data source
similarly as it is done with memory buffers.


Regards,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Tomasz Stanislawski]

Laurent Pinchart wrote:
> On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
>> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
>>> On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
>>>> On Thursday, May 05, 2011 11:39:54 Tomasz Stanislawski wrote:
Hi Laurent and Hans,
I am very sorry for replying so lately. I was really busy during last 
week. Thank you very much for your interest and comments :).
> 
> [snip]
> 
 >> -----------------------------------------------------------------
 >>>>>
 >>>>> Name
 >>>>>
 >>>>> 	VIDIOC_S_SELECTION - set cropping rectangle on an input of a device

[snip]

 >>>>> v4l2-selection::r is filled with suggested coordinates. The 
coordinates
 >>>>> are expressed in driver-dependant units.
 >
 > What kind of driver-dependant units do you think will be used in 
practice ?

In a case of sensors, units could be subpixels. For analog TV inputs, 
units may be milliseconds. For printers/scanner, units may be centimeters.
I think that there are many other examples out there.

>>>>> Applications set V4L2_SEL_TRY flag in v4l2_selection::flags to
>>>>> prevent a driver from applying selection configuration to hardware.
>>>> I mentioned this before but I am very much opposed to this flag. It is
>>>> inconsistent with the TRY_FMT and TRY_EXT_CTRLS ioctls. Note that in
>>>> video_ioctl2 it should be just one function with 'try' boolean
>>>> argument. It has always been a mistake that the try and set functions
>>>> were separated in the driver code IMHO.
>>>>
>>>> I know that the subdev ioctls do not have a try version, but it is not
>>>> quite the same in that those try functions actually store the try
>>>> information.
>>> That's exactly why the subdevs pad-level API has a try flag instead of a
>>> try operation, and that's why g/s_selection on subdevs will be done with
>>> a try flag.
>>>
>>> As for the video device node API, I won't oppose a TRY ioctl, as long as
>>> we can guarantee there will never be dependencies between the selection
>>> rectangles and other parameters (or between the different selection
>>> rectangles). If the crop rectangle depends on the compose rectangle for
>>> instance, how can you implement a TRY ioctl to try a crop rectangle for
>>> a specific compose rectangle, without modifying the active compose
>>> rectangle first ?
>> In that case the TRY would adjust the crop so that it works with the
>> current compose rectangle.
> 
> And how do you try both crop and compose settings without modifying the active 
> configuration ? That's not possible, and I think it's a bad API limitation.

VIDIOC_TRY_MULTISELECTION ?

> 
>> But this is just one more example of our lack of proper support for pipeline
>> setup. It doesn't really matter whether this is at the subdev level or at
>> the driver level, both have the same problems.
>>
>> This requires a brainstorm session to work out.
>>
>>>> This is something we need to look into more carefully. I am slowly
>>>> becoming convinced that we need some sort of transaction-based
>>>> configuration for pipelines.
>>> This RFC is about video device node configuration, not pipelines. For
>>> pipelines I think we'll need a transaction-based API. For video device
>>> nodes, I'm still unsure. As stated above, if we have multiple parameters
>>> that depend on each other, how do we let the user try them without
>>> changing the active configuration ?
>> Cropping/scaling/composing IS a pipeline. Until recently the V4L2 device
>> node API was sufficient to setup the trivial pipelines that most V4L2
>> consumer devices have. But with the more modern devices it starts to show
>> its limitations.
> 
> It's still a simple pipeline, and I think we should aim at making the V4L2 
> device node API useful to configure this kind of pipeline. The selection API 
> is a superset of the crop API, applications should be able to use it to 
> replace the crop API in the long term.
> 
>> The whole 'try' concept we had for a long time needs to be re-examined.
> 
> I agree.
> 
>> As you remember, I was never satisfied with the subdev 'try' approach
>> either, but I never could come up with a better alternative.
>>
I've noticed that there are two different meaning of TRY flag
a) checking if a proposed configuration is applicable for a driver
b) storing proposed configuration in some form of temporary buffer

Ad. a. This is a real TRY command. The state of both hardware and driver 
does not change after TRY call.

Ad. b. It should be called not TRY flag because the internal state of a 
driver changes. It should be named as something like SHADOW flag. It 
would indicate that the change is applied only to shadow configuration.

I propose to change name of TRY flag for subdev to SHADOW flag. I think 
it would much clear to express the difference of TRY meaning in video 
node and subdev contexts.

Therefore ioctl VIDIOC_TRY_SELECTION is probably better and more 
convenient way of testing if configuration is applicable.

>>>> Regardless of how such a scheme should work, one thing that I believe
>>>> is missing in the format ioctls (both on the video and subdev level)
>>>> is a similar concept like the flags in this proposal. It would be
>>>> quite useful if you could indicate that the desired WxH has to be
>>>> exact or can be larger or smaller. It would certainly simplify the
>>>> interaction between the selection and scaling.
>>>>
>>>>> On success field v4l2_selection::r is filled with adjusted rectangle.
>>>>>
>>>>> Return value
>>>>>
>>>>> 	On success 0 is returned, on error -1 and the errno variable is set
>>>>> 	
>>>>>         appropriately:
>>>>> 	EINVAL - incorrect buffer type, incorrect/not supported target
>>>>>
>>>>> 4. Flags
>>>>> 4.1. Hints
>>>>>
>>>>> The field v4l2_selection::flags is used to give a driver a hint about
>>>>> coordinate adjustments. The set of possible hint flags was reduced to
>>>>> two entities for practical reasons. The flag V4L2_SEL_SIZE_LE is a
>>>>> suggestion for the driver to decrease or keep size of a rectangle. 
>>>>> The flags V4L2_SEL_SIZE_GE imply keeping or increasing a size of a
>>>>> rectangle.
>>>>>
>>>>> By default, lack of any hint flags indicate that driver has to choose
>>>>> selection coordinates as close as possible to the ones passed in
>>>>> v4l2_selection::r field.
>>>>>
>>>>> Setting both flags implies that the driver is free to adjust the
>>>>> rectangle.  It may return even random one however much more
>>>>> encouraged behaviour would be adjusting coordinates in such a way
>>>>> that other stream parameters are left intact. This means that the
>>>>> driver should avoid changing a format of an image buffer and/or any
>>>>> other controls.
>>>> This makes no sense to me. It sounds like this is what flags == 0
>>>> should do.
>>>>
>>>> I would expect that if both flags are specified that that would equal
>>>> SEL_SIZE_EQ. I.E., the rectangle has to be exact size-wise, and should
>>>> be as close as possible to the original position.
>>> What happens if that's not possible ? The ioctl should never return an
>>> error,
>> Why not? If I tell the driver that I want exactly WxH, then I see no reason
>> why it can't return an error. An application might use that result to
>> switch to a different resolution, for example. E.g., the application tries
>> 640x480 first, that fails, then it tries 320x240 (or whatever).
> 
> To make the API more consistent. Applications ask drivers for specific 
> settings (including optional hints), and drivers return what they've been able 
> to configure. It's then the application's responsibility to check the return 
> values and act upon it. Drivers shouldn't return an error when setting 
> formats/selections, except if the given arguments can't be understood.
Hmm.. I see two solutions:

Solution I (more restrictive):
0 - driver is free to adjust size, it is recommended to choose the 
crop/compose rectangle as close as possible to desired one

SEL_SIZE_GE - drive is not allowed to shrink the rectangle. If no such a 
rectangle exists ERANGE is returned (EINVAL is used for 
not-understandable configuration)

SEL_SIZE_LE - drive is not allowed to grow the rectangle. If no such a 
rectangle exists ERANGE is returned (EINVAL is used for 
not-understandable configuration)

SEL_SIZE_EQ = SEL_SIZE_GE | SEL_SIZE_LE - choose size exactly the same 
as in desired rectangle. Return ERANGE if such a configuration is not 
possible.

-----------------------------------------

Solution II (less restrictive). Proposed in this RFC.

0 - apply rectangle as close as possible to desired one like the default 
behavior of  VIDIOC_S_CROP.

SEL_SIZE_GE - suggestion to increase or keep size of both coordinates

SEL_SIZE_LE - suggestion to decrease or keep size of both coordinates

SEL_SIZE_GE | SEL_SIZE_LE - technically suggestion to "increase or keep 
or decrease" sizes. Basically, it means that driver is completely free 
to choose coordinates. It works like saying "give me a crop similar to 
this one" to the driver. I agree that it is not "a very useful" 
combination of flags.

In both solutions, the driver is recommended to keep the center of the 
rectangle in the same place.

Personally, I prefer 'solution I' because it is more logical one.
One day, the SEL_SIZE_GE could be expanded to LEFT_LE | RIGHT_GE | 
TOP_LE | BOTTOM_GE flags if drivers could support it.

[snip]

 >>> One thing I think would be helpful though, is if the target name 
would tell
 >>> you whether it is a read-only or a read-write target. It might also be
 >>> helpful if the IDs of the read-only targets would set some 
read-only bit.
 >>> That would make it easy for video_ioctl2 to test for invalid 
targets for
 >>> S_SELECTION.
 >>>
 >>> Not sure about the naming though:
 >>>
 >>> V4L2_SEL_RO_CROP_DEFAULT
 >>> V4L2_SEL_CROP_RO_DEFAULT
 >>> V4L2_SEL_CROP_DEFAULT_RO
 >>>
 >>> None looks right. A read-only bit might be sufficient as it would 
clearly
 >>> indicate in the header that that target is a read-only target.
 >> What if some future hardware have setable default or bounds 
rectangles ? I
 >> don't know what that would be used for, it's just in case :-)
 >
 > If it is settable, then it is no longer a default or bounds rectangle but
 > some other rectangle :-)
 >

I agree with Laurent. Both bounds and defrect are not exactly read-only.
They could be modified by S_FMT, S_STD/S_DV_PRESET ioctl.
The DEFRECT can change after switching aspect ratio on TV output.

If cropping is not supported setting ACTIVE target returns EINVAL, 
because this target is read-only in this context.
Change of ACTIVE target might be also impossible because hardware is in 
streaming state.

Basically, ant target could be Read-only because of some reason.

Therefore I see no reason to break symmetry between 
active/bound/defrect/? by adding RO {pre/suff}ix.

[snip]

>>>>> All cropcap fields except pixel aspect are supported in new API. 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-clari
>>>>> fica tion.html
>>>> Yeah, what are we going to do about that? I agree that it does not
>>>> belong here. But we can't sweep it under the carpet either.
>>>>
>>>> The pixel aspect ratio is typically a property of the current input or
>>>> output video format (either a DV preset or a PAL/NTSC STD). For DV
>>>> presets we could add it to struct v4l2_dv_enum_presets and we could do
>>>> the same for STD formats by adding it to struct v4l2_standard.
>>> Cropping and composing doesn't modify the pixel aspect ratio, so I agree
>>> it doesn't belong there.
>>>
>>>> This would fail for sensors, though, since there the chosen sensor
>>>> framesize is set through S_FMT. (This never quite made sense to me,
>>>> though, from a V4L2 API perspective). I'm not sure whether we can
>>>> always assume 1:1 pixel ratio for sensors. Does anyone know?
>>> Most of the time I suppose so, but I wouldn't be surprise if some exotic
>>> sensors had non-square pixel aspect ratios.
>> Would it make sense to add the pixel ratio to struct v4l2_frmsizeenum?
>>
>> And what about adding a VIDIOC_G/S_FRAMESIZE to select a sensor resolution?
>>
>> This would typically be one of the discrete framesizes supported by a
>> sensor through binning/skipping. If there is also a scaler on the sensor,
>> then that is controlled through S_FMT. For video it is S_FMT that controls
>> the scaler (together with S_CROP at the moment), but the source resolution
>> is set through S_STD/S_DV_PRESET/S_DV_TIMINGS. It always felt very
>> inconsistent to me that there is no equivalent for sensors, even though
>> you can enumerate all the available framesizes (just as you can with
>> ENUMSTD and ENUM_DV_PRESETS).

The problem is that S_FMT is used to configure two entities:
- memory buffer
- sensor
The selection API help to configure crop/compose/scaling. Therefore if 
selection would be accepted in V4L, then it may be worth to consider 
separation of memory and sensors configuration.

I agree that sensors need a dedicated ioctl of {G,S,TRY,ENUM}_SENSOR 
family. I do not feel competent in this matter but I think that the 
ioctl should support feature typical for sensors like:
- array size (before scaling)
- array shape
- (sub)pixel ratio or equivalent
- binning/skipping

The V4L is designed for simple pipelines like one below:

Input ---->  Processing ----> Output

In sensor case we have:

Sensor array  ----->  Processing --------> Memory
- resolution          - compose            - format
- binning             - crop               - resolution bounds
- skipping            - scaling            - size in bytes (!)
                                            - alignment/bytesperline

For TV output the pipeline is following:

Memory       ------> Processing --------> TV output
- format             - compose            - tv standard
- bounds             - crop               - timing
- size               - scaling            - DAC encoding
- alignment

I think that memory(buffers) should be configured using S_FMT.
Sensors should be configured with new S_SENSOR ioctl.

I do not see how this approach could break backward compatibility?
The only problem might be that the sensor may not work optimally is its 
default resolution is too big in comparison to buffer resolution.
Choosing optimal configuration of sensor/scaler is done in MC.
Applying overmentioned approach, it could be also configured in video node.

What is your opinion about this idea?

> 
> Let's take one step back here.
> 
> We started with the V4L2 device node API to control (more or less) simple 
> devices. Device became more complex, and we created a new MC API (along with 
> the subdev pad-level API) to configure complex pipelines. The V4L2 device node 
> API still lives, and we want to enhance it to configure medium complexity 
> devices.
> 
> Before going much further, I think we need to define what a medium complexity 
> device is and where we put the boundary between devices that can be configured 
> with the V4L2 device node API, and devices that require the MC API.
> 
> I believe this shouldn't be too difficult. What we need to do is create a 
> simple virtual pipeline that supports cropping, scaling and composing, and map 
> the V4L2 device node API to that pipeline configuration. Devices that map to 
> that pipeline could then use the V4L2 device node API only, with clearly 
> defined semantics.
> 
> [snip]
> 
>>>>>   * resolution of an image combined with support for
>>>>>   VIDIOC_S_MULTISELECTION
>>>>>   
>>>>>     allows to pass a triple format/crop/compose sizes in a single
>>>>>     ioctl
>>>> I don't believe S_MULTISELECTION will solve anything. Specific
>>>> use-cases perhaps, but not the general problem of setting up a
>>>> pipeline. I feel another brainstorm session coming to solve that. We
>>>> never came to a solution for it in Warsaw.
>>> Pipelines are configured on subdev nodes, not on video nodes, so I'm also
>>> unsure whether multiselection support would really be useful.
>>>

Passing compose and crop rectangle in single ioctl might help in some 
cases, like the one described here:
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/27581

>>> If we decide to implement multiselection support, we might as well use
>>> that only. We would need a multiselection target bitmask, so selection
>>> targets should all be < 32.
I was considering something like this:

struct v4l2_multiselection {
	int n; /* number of targets */
	struct v4l2_selection s[0]; /* 0-length array */
};

There is no need for a bitmask.

>>>
>>> Thinking some more about it, does it make sense to set both crop and
>>> compose on a single video device node (not talking about mem-to-mem,
>>> where you use the type to multiplex input/output devices on the same
>>> node) ? If so, what would the use cases be ?

Configuration of multi input/output devices could be realised by adding 
extra targets and passing them all using MULTISELECTION.

To sum up, the multiselection is only an brainstorm idea. I think that 
transaction-based API is simpler and more robust.
Multiselection would be realized by passing multiple VIDIOC_S_SELECTION
inside single transaction window.

Best regards,
Tomasz Stanislawski

>>>
>>> Should all devices support the selection API, or only the simple ones
>>> that don't expose a user-configurable pipeline to userspace through the
>>> MC API ? The proposed API looks good to me, but before acking it I'd
>>> like to clarify how (if at all) this will interact with subdev pad-level
>>> configuration on devices that support that. My current feeling is that
>>> video device nodes for such devices should only be used for video
>>> streaming. All parameters should be configured directly on the subdevs.
>>> Application might still need to call S_FMT in order to be able to
>>> allocate buffers though.
>> This comes back to how we want to implement backwards compatibility for
>> existing applications. There must be a way for 'standard' apps to work
>> with complex drivers for specific video nodes (the mc would probably mark
>> those as a 'DEFAULT' node).
>>
>> I'd say that there are roughly two options: either implement the selection
>> etc. APIs for those video nodes only in the driver, letting the driver set
>> up the subdev pipeline, or do it via libv4l SoC-specific plugins.
>>
>> In my opinion we need to finish the pipeline configuration topic we started
>> in Warsaw before we can finalize this RFC. This RFC clearly demonstrates
>> that we have inconsistencies and deficiencies in our API that need to be
>> solved first. When we have done that, then I expect that this selection
>> API will be easy to finalize.
> 

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Laurent Pinchart]

Hi Hans,

On Wednesday 18 May 2011 14:31:59 Hans Verkuil wrote:
> On Wednesday, May 18, 2011 14:06:21 Sylwester Nawrocki wrote:
> > On 05/16/2011 09:21 AM, Laurent Pinchart wrote:
> > > On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
> > >> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
> > >>> On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
> > >>>> On Thursday, May 05, 2011 11:39:54 Tomasz Stanislawski wrote:
> > > [snip]
> > 
> > ...
> > 
> > >>>>>   * resolution of an image combined with support for
> > >>>>>   VIDIOC_S_MULTISELECTION
> > >>>>>   
> > >>>>>     allows to pass a triple format/crop/compose sizes in a single
> > >>>>>     ioctl
> > >>>> 
> > >>>> I don't believe S_MULTISELECTION will solve anything. Specific
> > >>>> use-cases perhaps, but not the general problem of setting up a
> > >>>> pipeline. I feel another brainstorm session coming to solve that. We
> > >>>> never came to a solution for it in Warsaw.
> > >>> 
> > >>> Pipelines are configured on subdev nodes, not on video nodes, so I'm
> > >>> also unsure whether multiselection support would really be useful.
> > >>> 
> > >>> If we decide to implement multiselection support, we might as well
> > >>> use that only. We would need a multiselection target bitmask, so
> > >>> selection targets should all be < 32.
> > >>> 
> > >>> Thinking some more about it, does it make sense to set both crop and
> > >>> compose on a single video device node (not talking about mem-to-mem,
> > >>> where you use the type to multiplex input/output devices on the same
> > >>> node) ? If so, what would the use cases be ?
> > 
> > I can't think of any, one either use crop or compose.
> 
> I can: you crop in the video receiver and compose it into a larger buffer.
> Actually quite a desirable feature.

Composing into a buffer can be achieved by 

> > >>> Should all devices support the selection API, or only the simple ones
> > >>> that don't expose a user-configurable pipeline to userspace through
> > >>> the MC API ? The proposed API looks good to me, but before acking it
> > >>> I'd like to clarify how (if at all) this will interact with subdev
> > >>> pad-level
> > >>> 
> > >>> configuration on devices that support that. My current feeling is
> > >>> that video device nodes for such devices should only be used for
> > >>> video streaming. All parameters should be configured directly on the
> > >>> subdevs. Application might still need to call S_FMT in order to be
> > >>> able to allocate buffers though.
> > 
> > I'm not sure whether moving all configuration to subdev API for medium
> > complexity devices which well might have exposed core functionality
> > through standard V4L2 device node API and still use MC API for pipeline
> > reconfiguration (in terms of linking entities with each other) is the way
> > to go.

One thing we haven't defined is what a "medium complexity device" is. I 
believe it should be defined in terms of the hardware pipeline, or in terms of 
a virtual pipeline that the hardware can map to. When that will be done it 
shouldn't be too difficult to unambiguously define how the V4L2 ioctls map to 
actions on that pipeline.

> > In order to support existing applications SoC-specific libraries would
> > have to be used in addition to a MC driver ?
> > Leaving ourselves without support for default configuration that would
> > work with "old" V4L2 device node API (in connection with common libv4l
> > library) ?
> > 
> > I thought that weren't the prerequisites when designing the MC API.
> > I'm afraid we'll end up with two distinct APIs in Video4Linux and thus
> > two sets of drivers and applications that will not work with one another.
> 
> Indeed. The original requirement was (and is!) that the default video node
> must be usable by standard apps, either through the driver or through
> libv4l.

And I'm still happy with that requirement :-) The OMAP3 ISP libv4l plugin is 
not finished yet, but we're working on it.

> It is a missing piece in the MC how to mark default nodes. We also need to
> clearly define which ioctls are optional for non-default nodes.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Laurent Pinchart]

On Wednesday 18 May 2011 15:03:13 Sylwester Nawrocki wrote:
> On 05/18/2011 02:31 PM, Hans Verkuil wrote:
> > On Wednesday, May 18, 2011 14:06:21 Sylwester Nawrocki wrote:
> >> On 05/16/2011 09:21 AM, Laurent Pinchart wrote:
> >> > On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
> >> >> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
> >> >>>
> >> >>> Thinking some more about it, does it make sense to set both crop and
> >> >>> compose on a single video device node (not talking about mem-to-mem,
> >> >>> where you use the type to multiplex input/output devices on the same
> >> >>> node) ? If so, what would the use cases be ?
> >> 
> >> I can't think of any, one either use crop or compose.
> > 
> > I can: you crop in the video receiver and compose it into a larger
> > buffer.
> > 
> > Actually quite a desirable feature.
> 
> Yes, right. Don't know why I imagined something different.
> And we need it in Samsung capture capture interfaces as well. The H/W
> is capable of cropping and composing with camera interface as a data source
> similarly as it is done with memory buffers.

The same result could be achieved by adding an offset to the buffer address 
and setting the bytesperline field accordingly, but that would only work with 
userptr buffers. As we're working on an API to share buffers between 
subsystems, I agree that composing into a larger buffer is desirable and 
shouldn't be implemented using offset/stride.

-- 
Regards,

Laurent Pinchart

RE: [PATCH 0/2] V4L: Extended crop/compose API

[Marek Szyprowski]

Hello,

On Thursday, May 19, 2011 3:48 PM Laurent Pinchart wrote:

> On Wednesday 18 May 2011 15:03:13 Sylwester Nawrocki wrote:
> > On 05/18/2011 02:31 PM, Hans Verkuil wrote:
> > > On Wednesday, May 18, 2011 14:06:21 Sylwester Nawrocki wrote:
> > >> On 05/16/2011 09:21 AM, Laurent Pinchart wrote:
> > >> > On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
> > >> >> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
> > >> >>>
> > >> >>> Thinking some more about it, does it make sense to set both crop
> and
> > >> >>> compose on a single video device node (not talking about mem-to-
> mem,
> > >> >>> where you use the type to multiplex input/output devices on the
> same
> > >> >>> node) ? If so, what would the use cases be ?
> > >>
> > >> I can't think of any, one either use crop or compose.
> > >
> > > I can: you crop in the video receiver and compose it into a larger
> > > buffer.
> > >
> > > Actually quite a desirable feature.
> >
> > Yes, right. Don't know why I imagined something different.
> > And we need it in Samsung capture capture interfaces as well. The H/W
> > is capable of cropping and composing with camera interface as a data
> > source similarly as it is done with memory buffers.
> 
> The same result could be achieved by adding an offset to the buffer address
> and setting the bytesperline field accordingly, but that would only work
> with userptr buffers.

Playing with offset and bytesperline to achieve composing effect should be
considered only as a huge hack. Please note that there is a bunch of pixel
formats that have no clear definition of bytesperline at all - like macro
block formats or some (multi)planar ones. We would really like to have a
generic solution for the composing problem.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Tomasz Stanislawski]

Laurent Pinchart wrote:
> On Wednesday 18 May 2011 15:03:13 Sylwester Nawrocki wrote:
>> On 05/18/2011 02:31 PM, Hans Verkuil wrote:
>>> On Wednesday, May 18, 2011 14:06:21 Sylwester Nawrocki wrote:
>>>> On 05/16/2011 09:21 AM, Laurent Pinchart wrote:
>>>>> On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
>>>>>> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
>>>>>>> Thinking some more about it, does it make sense to set both crop and
>>>>>>> compose on a single video device node (not talking about mem-to-mem,
>>>>>>> where you use the type to multiplex input/output devices on the same
>>>>>>> node) ? If so, what would the use cases be ?
>>>> I can't think of any, one either use crop or compose.
>>> I can: you crop in the video receiver and compose it into a larger
>>> buffer.
>>>
>>> Actually quite a desirable feature.
>> Yes, right. Don't know why I imagined something different.
>> And we need it in Samsung capture capture interfaces as well. The H/W
>> is capable of cropping and composing with camera interface as a data source
>> similarly as it is done with memory buffers.
> 
> The same result could be achieved by adding an offset to the buffer address 
> and setting the bytesperline field accordingly, but that would only work with 
> userptr buffers. As we're working on an API to share buffers between 
> subsystems, I agree that composing into a larger buffer is desirable and 
> shouldn't be implemented using offset/stride.
> 
Hi,
Simulation of cropping on a data source using offset/bytesperline is not 
possible for compressed formats like JPEG. I could not find any good 
definition of bytesperline for macroblock and planar formats.
These problems were the reason of proposing extcrop (aka selection) API.

Bye
TS

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Laurent Pinchart]

On Thursday 19 May 2011 16:06:12 Tomasz Stanislawski wrote:
> Laurent Pinchart wrote:
> > On Wednesday 18 May 2011 15:03:13 Sylwester Nawrocki wrote:
> >> On 05/18/2011 02:31 PM, Hans Verkuil wrote:
> >>> On Wednesday, May 18, 2011 14:06:21 Sylwester Nawrocki wrote:
> >>>> On 05/16/2011 09:21 AM, Laurent Pinchart wrote:
> >>>>> On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
> >>>>>> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
> >>>>>>> Thinking some more about it, does it make sense to set both crop
> >>>>>>> and compose on a single video device node (not talking about
> >>>>>>> mem-to-mem, where you use the type to multiplex input/output
> >>>>>>> devices on the same node) ? If so, what would the use cases be ?
> >>>> 
> >>>> I can't think of any, one either use crop or compose.
> >>> 
> >>> I can: you crop in the video receiver and compose it into a larger
> >>> buffer.
> >>> 
> >>> Actually quite a desirable feature.
> >> 
> >> Yes, right. Don't know why I imagined something different.
> >> And we need it in Samsung capture capture interfaces as well. The H/W
> >> is capable of cropping and composing with camera interface as a data
> >> source similarly as it is done with memory buffers.
> > 
> > The same result could be achieved by adding an offset to the buffer
> > address and setting the bytesperline field accordingly, but that would
> > only work with userptr buffers. As we're working on an API to share
> > buffers between subsystems, I agree that composing into a larger buffer
> > is desirable and shouldn't be implemented using offset/stride.
> 
> Hi,
> Simulation of cropping on a data source using offset/bytesperline is not
> possible for compressed formats like JPEG.

I agree with you, but for composing I wonder how you're going to compose an 
image into a JPEG buffer :-)

> I could not find any good definition of bytesperline for macroblock and
> planar formats. These problems were the reason of proposing extcrop (aka
> selection) API.

As I said, I agree that composing shouldn't be implemented using 
offset/stride, so there's no disagreement.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Tomasz Stanislawski]

Tomasz Stanislawski wrote:
> Laurent Pinchart wrote:
>> On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
>>> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
>>>> On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
>>>>> On Thursday, May 05, 2011 11:39:54 Tomasz Stanislawski wrote:
> Hi Laurent and Hans,
> I am very sorry for replying so lately. I was really busy during last 
> week. Thank you very much for your interest and comments :).
>>
>> [snip]
>>
> >> -----------------------------------------------------------------
> >>>>>
> >>>>> Name
> >>>>>
> >>>>>     VIDIOC_S_SELECTION - set cropping rectangle on an input of a 
> device
>
> [snip]
>
> >>>>> v4l2-selection::r is filled with suggested coordinates. The 
> coordinates
> >>>>> are expressed in driver-dependant units.
> >
> > What kind of driver-dependant units do you think will be used in 
> practice ?
>
> In a case of sensors, units could be subpixels. For analog TV inputs, 
> units may be milliseconds. For printers/scanner, units may be 
> centimeters.
> I think that there are many other examples out there.
>
>>>>>> Applications set V4L2_SEL_TRY flag in v4l2_selection::flags to
>>>>>> prevent a driver from applying selection configuration to hardware.
>>>>> I mentioned this before but I am very much opposed to this flag. 
>>>>> It is
>>>>> inconsistent with the TRY_FMT and TRY_EXT_CTRLS ioctls. Note that in
>>>>> video_ioctl2 it should be just one function with 'try' boolean
>>>>> argument. It has always been a mistake that the try and set functions
>>>>> were separated in the driver code IMHO.
>>>>>
>>>>> I know that the subdev ioctls do not have a try version, but it is 
>>>>> not
>>>>> quite the same in that those try functions actually store the try
>>>>> information.
>>>> That's exactly why the subdevs pad-level API has a try flag instead 
>>>> of a
>>>> try operation, and that's why g/s_selection on subdevs will be done 
>>>> with
>>>> a try flag.
>>>>
>>>> As for the video device node API, I won't oppose a TRY ioctl, as 
>>>> long as
>>>> we can guarantee there will never be dependencies between the 
>>>> selection
>>>> rectangles and other parameters (or between the different selection
>>>> rectangles). If the crop rectangle depends on the compose rectangle 
>>>> for
>>>> instance, how can you implement a TRY ioctl to try a crop rectangle 
>>>> for
>>>> a specific compose rectangle, without modifying the active compose
>>>> rectangle first ?
>>> In that case the TRY would adjust the crop so that it works with the
>>> current compose rectangle.
>>
>> And how do you try both crop and compose settings without modifying 
>> the active configuration ? That's not possible, and I think it's a 
>> bad API limitation.
>
> VIDIOC_TRY_MULTISELECTION ?
>
>>
>>> But this is just one more example of our lack of proper support for 
>>> pipeline
>>> setup. It doesn't really matter whether this is at the subdev level 
>>> or at
>>> the driver level, both have the same problems.
>>>
>>> This requires a brainstorm session to work out.
>>>
>>>>> This is something we need to look into more carefully. I am slowly
>>>>> becoming convinced that we need some sort of transaction-based
>>>>> configuration for pipelines.
>>>> This RFC is about video device node configuration, not pipelines. For
>>>> pipelines I think we'll need a transaction-based API. For video device
>>>> nodes, I'm still unsure. As stated above, if we have multiple 
>>>> parameters
>>>> that depend on each other, how do we let the user try them without
>>>> changing the active configuration ?
>>> Cropping/scaling/composing IS a pipeline. Until recently the V4L2 
>>> device
>>> node API was sufficient to setup the trivial pipelines that most V4L2
>>> consumer devices have. But with the more modern devices it starts to 
>>> show
>>> its limitations.
>>
>> It's still a simple pipeline, and I think we should aim at making the 
>> V4L2 device node API useful to configure this kind of pipeline. The 
>> selection API is a superset of the crop API, applications should be 
>> able to use it to replace the crop API in the long term.
>>
>>> The whole 'try' concept we had for a long time needs to be re-examined.
>>
>> I agree.
>>
>>> As you remember, I was never satisfied with the subdev 'try' approach
>>> either, but I never could come up with a better alternative.
>>>
> I've noticed that there are two different meaning of TRY flag
> a) checking if a proposed configuration is applicable for a driver
> b) storing proposed configuration in some form of temporary buffer
>
> Ad. a. This is a real TRY command. The state of both hardware and 
> driver does not change after TRY call.
>
> Ad. b. It should be called not TRY flag because the internal state of 
> a driver changes. It should be named as something like SHADOW flag. It 
> would indicate that the change is applied only to shadow configuration.
>
> I propose to change name of TRY flag for subdev to SHADOW flag. I 
> think it would much clear to express the difference of TRY meaning in 
> video node and subdev contexts.
>
> Therefore ioctl VIDIOC_TRY_SELECTION is probably better and more 
> convenient way of testing if configuration is applicable.
>
>>>>> Regardless of how such a scheme should work, one thing that I believe
>>>>> is missing in the format ioctls (both on the video and subdev level)
>>>>> is a similar concept like the flags in this proposal. It would be
>>>>> quite useful if you could indicate that the desired WxH has to be
>>>>> exact or can be larger or smaller. It would certainly simplify the
>>>>> interaction between the selection and scaling.
>>>>>
>>>>>> On success field v4l2_selection::r is filled with adjusted 
>>>>>> rectangle.
>>>>>>
>>>>>> Return value
>>>>>>
>>>>>>     On success 0 is returned, on error -1 and the errno variable 
>>>>>> is set
>>>>>>     
>>>>>>         appropriately:
>>>>>>     EINVAL - incorrect buffer type, incorrect/not supported target
>>>>>>
>>>>>> 4. Flags
>>>>>> 4.1. Hints
>>>>>>
>>>>>> The field v4l2_selection::flags is used to give a driver a hint 
>>>>>> about
>>>>>> coordinate adjustments. The set of possible hint flags was 
>>>>>> reduced to
>>>>>> two entities for practical reasons. The flag V4L2_SEL_SIZE_LE is a
>>>>>> suggestion for the driver to decrease or keep size of a 
>>>>>> rectangle. The flags V4L2_SEL_SIZE_GE imply keeping or increasing 
>>>>>> a size of a
>>>>>> rectangle.
>>>>>>
>>>>>> By default, lack of any hint flags indicate that driver has to 
>>>>>> choose
>>>>>> selection coordinates as close as possible to the ones passed in
>>>>>> v4l2_selection::r field.
>>>>>>
>>>>>> Setting both flags implies that the driver is free to adjust the
>>>>>> rectangle.  It may return even random one however much more
>>>>>> encouraged behaviour would be adjusting coordinates in such a way
>>>>>> that other stream parameters are left intact. This means that the
>>>>>> driver should avoid changing a format of an image buffer and/or any
>>>>>> other controls.
>>>>> This makes no sense to me. It sounds like this is what flags == 0
>>>>> should do.
>>>>>
>>>>> I would expect that if both flags are specified that that would equal
>>>>> SEL_SIZE_EQ. I.E., the rectangle has to be exact size-wise, and 
>>>>> should
>>>>> be as close as possible to the original position.
>>>> What happens if that's not possible ? The ioctl should never return an
>>>> error,
>>> Why not? If I tell the driver that I want exactly WxH, then I see no 
>>> reason
>>> why it can't return an error. An application might use that result to
>>> switch to a different resolution, for example. E.g., the application 
>>> tries
>>> 640x480 first, that fails, then it tries 320x240 (or whatever).
>>
>> To make the API more consistent. Applications ask drivers for 
>> specific settings (including optional hints), and drivers return what 
>> they've been able to configure. It's then the application's 
>> responsibility to check the return values and act upon it. Drivers 
>> shouldn't return an error when setting formats/selections, except if 
>> the given arguments can't be understood.
> Hmm.. I see two solutions:
>
> Solution I (more restrictive):
> 0 - driver is free to adjust size, it is recommended to choose the 
> crop/compose rectangle as close as possible to desired one
>
> SEL_SIZE_GE - drive is not allowed to shrink the rectangle. If no such 
> a rectangle exists ERANGE is returned (EINVAL is used for 
> not-understandable configuration)
>
> SEL_SIZE_LE - drive is not allowed to grow the rectangle. If no such a 
> rectangle exists ERANGE is returned (EINVAL is used for 
> not-understandable configuration)
>
> SEL_SIZE_EQ = SEL_SIZE_GE | SEL_SIZE_LE - choose size exactly the same 
> as in desired rectangle. Return ERANGE if such a configuration is not 
> possible.
>
> -----------------------------------------
>
> Solution II (less restrictive). Proposed in this RFC.
>
> 0 - apply rectangle as close as possible to desired one like the 
> default behavior of  VIDIOC_S_CROP.
>
> SEL_SIZE_GE - suggestion to increase or keep size of both coordinates
>
> SEL_SIZE_LE - suggestion to decrease or keep size of both coordinates
>
> SEL_SIZE_GE | SEL_SIZE_LE - technically suggestion to "increase or 
> keep or decrease" sizes. Basically, it means that driver is completely 
> free to choose coordinates. It works like saying "give me a crop 
> similar to this one" to the driver. I agree that it is not "a very 
> useful" combination of flags.
>
> In both solutions, the driver is recommended to keep the center of the 
> rectangle in the same place.
>
> Personally, I prefer 'solution I' because it is more logical one.
> One day, the SEL_SIZE_GE could be expanded to LEFT_LE | RIGHT_GE | 
> TOP_LE | BOTTOM_GE flags if drivers could support it.
On a second thought, the 'solution I' could be used in subdevs because 
gives a better control over hardware.
'Solution II' suits better to video nodes. I mean applications that use 
V4L for very simple purposes.
The 'solution II' is less restrictive therefore it is easier to 
implement in a driver.

However, I made a little social experiment and I asked few developers in 
my team which API they would like to use.
They preferred 'solution I' because it is logical and deterministic.

Moreover, an application that use these flags is aware that introduction 
of constraints may cause a failure.
Anyway, the application can always withdraw explicitly to using 
S_SELECTION without any flags.

BTW.. what do you think about changing the flags names from 
SE_SIZE_LE/SEL_SIZE_GE to SEL_INSIDE/SEL_OUTSIDE?
New name better describe the flags' purpose.
For the first solution the 'hint flags' should be renamed to 'constraint 
flags', too.

Bye
Tomasz Stanislawski
>
> [snip]
>
> >>> One thing I think would be helpful though, is if the target name 
> would tell
> >>> you whether it is a read-only or a read-write target. It might 
> also be
> >>> helpful if the IDs of the read-only targets would set some 
> read-only bit.
> >>> That would make it easy for video_ioctl2 to test for invalid 
> targets for
> >>> S_SELECTION.
> >>>
> >>> Not sure about the naming though:
> >>>
> >>> V4L2_SEL_RO_CROP_DEFAULT
> >>> V4L2_SEL_CROP_RO_DEFAULT
> >>> V4L2_SEL_CROP_DEFAULT_RO
> >>>
> >>> None looks right. A read-only bit might be sufficient as it would 
> clearly
> >>> indicate in the header that that target is a read-only target.
> >> What if some future hardware have setable default or bounds 
> rectangles ? I
> >> don't know what that would be used for, it's just in case :-)
> >
> > If it is settable, then it is no longer a default or bounds 
> rectangle but
> > some other rectangle :-)
> >
>
> I agree with Laurent. Both bounds and defrect are not exactly read-only.
> They could be modified by S_FMT, S_STD/S_DV_PRESET ioctl.
> The DEFRECT can change after switching aspect ratio on TV output.
>
> If cropping is not supported setting ACTIVE target returns EINVAL, 
> because this target is read-only in this context.
> Change of ACTIVE target might be also impossible because hardware is 
> in streaming state.
>
> Basically, ant target could be Read-only because of some reason.
>
> Therefore I see no reason to break symmetry between 
> active/bound/defrect/? by adding RO {pre/suff}ix.
>
> [snip]
>
>>>>>> All cropcap fields except pixel aspect are supported in new API. 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-clari 
>>>>>>
>>>>>> fica tion.html
>>>>> Yeah, what are we going to do about that? I agree that it does not
>>>>> belong here. But we can't sweep it under the carpet either.
>>>>>
>>>>> The pixel aspect ratio is typically a property of the current 
>>>>> input or
>>>>> output video format (either a DV preset or a PAL/NTSC STD). For DV
>>>>> presets we could add it to struct v4l2_dv_enum_presets and we 
>>>>> could do
>>>>> the same for STD formats by adding it to struct v4l2_standard.
>>>> Cropping and composing doesn't modify the pixel aspect ratio, so I 
>>>> agree
>>>> it doesn't belong there.
>>>>
>>>>> This would fail for sensors, though, since there the chosen sensor
>>>>> framesize is set through S_FMT. (This never quite made sense to me,
>>>>> though, from a V4L2 API perspective). I'm not sure whether we can
>>>>> always assume 1:1 pixel ratio for sensors. Does anyone know?
>>>> Most of the time I suppose so, but I wouldn't be surprise if some 
>>>> exotic
>>>> sensors had non-square pixel aspect ratios.
>>> Would it make sense to add the pixel ratio to struct v4l2_frmsizeenum?
>>>
>>> And what about adding a VIDIOC_G/S_FRAMESIZE to select a sensor 
>>> resolution?
>>>
>>> This would typically be one of the discrete framesizes supported by a
>>> sensor through binning/skipping. If there is also a scaler on the 
>>> sensor,
>>> then that is controlled through S_FMT. For video it is S_FMT that 
>>> controls
>>> the scaler (together with S_CROP at the moment), but the source 
>>> resolution
>>> is set through S_STD/S_DV_PRESET/S_DV_TIMINGS. It always felt very
>>> inconsistent to me that there is no equivalent for sensors, even though
>>> you can enumerate all the available framesizes (just as you can with
>>> ENUMSTD and ENUM_DV_PRESETS).
>
> The problem is that S_FMT is used to configure two entities:
> - memory buffer
> - sensor
> The selection API help to configure crop/compose/scaling. Therefore if 
> selection would be accepted in V4L, then it may be worth to consider 
> separation of memory and sensors configuration.
>
> I agree that sensors need a dedicated ioctl of {G,S,TRY,ENUM}_SENSOR 
> family. I do not feel competent in this matter but I think that the 
> ioctl should support feature typical for sensors like:
> - array size (before scaling)
> - array shape
> - (sub)pixel ratio or equivalent
> - binning/skipping
>
> The V4L is designed for simple pipelines like one below:
>
> Input ---->  Processing ----> Output
>
> In sensor case we have:
>
> Sensor array  ----->  Processing --------> Memory
> - resolution          - compose            - format
> - binning             - crop               - resolution bounds
> - skipping            - scaling            - size in bytes (!)
>                                            - alignment/bytesperline
>
> For TV output the pipeline is following:
>
> Memory       ------> Processing --------> TV output
> - format             - compose            - tv standard
> - bounds             - crop               - timing
> - size               - scaling            - DAC encoding
> - alignment
>
> I think that memory(buffers) should be configured using S_FMT.
> Sensors should be configured with new S_SENSOR ioctl.
>
> I do not see how this approach could break backward compatibility?
> The only problem might be that the sensor may not work optimally is 
> its default resolution is too big in comparison to buffer resolution.
> Choosing optimal configuration of sensor/scaler is done in MC.
> Applying overmentioned approach, it could be also configured in video 
> node.
>
> What is your opinion about this idea?
>
>>
>> Let's take one step back here.
>>
>> We started with the V4L2 device node API to control (more or less) 
>> simple devices. Device became more complex, and we created a new MC 
>> API (along with the subdev pad-level API) to configure complex 
>> pipelines. The V4L2 device node API still lives, and we want to 
>> enhance it to configure medium complexity devices.
>>
>> Before going much further, I think we need to define what a medium 
>> complexity device is and where we put the boundary between devices 
>> that can be configured with the V4L2 device node API, and devices 
>> that require the MC API.
>>
>> I believe this shouldn't be too difficult. What we need to do is 
>> create a simple virtual pipeline that supports cropping, scaling and 
>> composing, and map the V4L2 device node API to that pipeline 
>> configuration. Devices that map to that pipeline could then use the 
>> V4L2 device node API only, with clearly defined semantics.
>>
>> [snip]
>>
>>>>>>   * resolution of an image combined with support for
>>>>>>   VIDIOC_S_MULTISELECTION
>>>>>>       allows to pass a triple format/crop/compose sizes in a single
>>>>>>     ioctl
>>>>> I don't believe S_MULTISELECTION will solve anything. Specific
>>>>> use-cases perhaps, but not the general problem of setting up a
>>>>> pipeline. I feel another brainstorm session coming to solve that. We
>>>>> never came to a solution for it in Warsaw.
>>>> Pipelines are configured on subdev nodes, not on video nodes, so 
>>>> I'm also
>>>> unsure whether multiselection support would really be useful.
>>>>
>
> Passing compose and crop rectangle in single ioctl might help in some 
> cases, like the one described here:
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/27581 
>
>
>>>> If we decide to implement multiselection support, we might as well use
>>>> that only. We would need a multiselection target bitmask, so selection
>>>> targets should all be < 32.
> I was considering something like this:
>
> struct v4l2_multiselection {
>     int n; /* number of targets */
>     struct v4l2_selection s[0]; /* 0-length array */
> };
>
> There is no need for a bitmask.
>
>>>>
>>>> Thinking some more about it, does it make sense to set both crop and
>>>> compose on a single video device node (not talking about mem-to-mem,
>>>> where you use the type to multiplex input/output devices on the same
>>>> node) ? If so, what would the use cases be ?
>
> Configuration of multi input/output devices could be realised by 
> adding extra targets and passing them all using MULTISELECTION.
>
> To sum up, the multiselection is only an brainstorm idea. I think that 
> transaction-based API is simpler and more robust.
> Multiselection would be realized by passing multiple VIDIOC_S_SELECTION
> inside single transaction window.
>
> Best regards,
> Tomasz Stanislawski
>
>>>>
>>>> Should all devices support the selection API, or only the simple ones
>>>> that don't expose a user-configurable pipeline to userspace through 
>>>> the
>>>> MC API ? The proposed API looks good to me, but before acking it I'd
>>>> like to clarify how (if at all) this will interact with subdev 
>>>> pad-level
>>>> configuration on devices that support that. My current feeling is that
>>>> video device nodes for such devices should only be used for video
>>>> streaming. All parameters should be configured directly on the 
>>>> subdevs.
>>>> Application might still need to call S_FMT in order to be able to
>>>> allocate buffers though.
>>> This comes back to how we want to implement backwards compatibility for
>>> existing applications. There must be a way for 'standard' apps to work
>>> with complex drivers for specific video nodes (the mc would probably 
>>> mark
>>> those as a 'DEFAULT' node).
>>>
>>> I'd say that there are roughly two options: either implement the 
>>> selection
>>> etc. APIs for those video nodes only in the driver, letting the 
>>> driver set
>>> up the subdev pipeline, or do it via libv4l SoC-specific plugins.
>>>
>>> In my opinion we need to finish the pipeline configuration topic we 
>>> started
>>> in Warsaw before we can finalize this RFC. This RFC clearly 
>>> demonstrates
>>> that we have inconsistencies and deficiencies in our API that need 
>>> to be
>>> solved first. When we have done that, then I expect that this selection
>>> API will be easy to finalize.
>>
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Laurent Pinchart]

Hi Tomasz,

On Wednesday 18 May 2011 18:55:51 Tomasz Stanislawski wrote:
> Laurent Pinchart wrote:
> > On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
> >> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
> >>> On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
> >>>> On Thursday, May 05, 2011 11:39:54 Tomasz Stanislawski wrote:
> Hi Laurent and Hans,
> I am very sorry for replying so lately. I was really busy during last week.
> Thank you very much for your interest and comments :).

No worries. I get more time to rest when you don't reply on the spot, so I 
don't mind :-)

[snip]

> >>>>> Applications set V4L2_SEL_TRY flag in v4l2_selection::flags to
> >>>>> prevent a driver from applying selection configuration to hardware.
> >>>> 
> >>>> I mentioned this before but I am very much opposed to this flag. It is
> >>>> inconsistent with the TRY_FMT and TRY_EXT_CTRLS ioctls. Note that in
> >>>> video_ioctl2 it should be just one function with 'try' boolean
> >>>> argument. It has always been a mistake that the try and set functions
> >>>> were separated in the driver code IMHO.
> >>>> 
> >>>> I know that the subdev ioctls do not have a try version, but it is not
> >>>> quite the same in that those try functions actually store the try
> >>>> information.
> >>> 
> >>> That's exactly why the subdevs pad-level API has a try flag instead of
> >>> a try operation, and that's why g/s_selection on subdevs will be done
> >>> with a try flag.
> >>> 
> >>> As for the video device node API, I won't oppose a TRY ioctl, as long
> >>> as we can guarantee there will never be dependencies between the
> >>> selection rectangles and other parameters (or between the different
> >>> selection rectangles). If the crop rectangle depends on the compose
> >>> rectangle for instance, how can you implement a TRY ioctl to try a
> >>> crop rectangle for a specific compose rectangle, without modifying the
> >>> active compose rectangle first ?
> >> 
> >> In that case the TRY would adjust the crop so that it works with the
> >> current compose rectangle.
> > 
> > And how do you try both crop and compose settings without modifying the
> > active configuration ? That's not possible, and I think it's a bad API
> > limitation.
> 
> VIDIOC_TRY_MULTISELECTION ?
>
> >> But this is just one more example of our lack of proper support for
> >> pipeline setup. It doesn't really matter whether this is at the subdev
> >> level or at the driver level, both have the same problems.
> >> 
> >> This requires a brainstorm session to work out.
> >> 
> >>>> This is something we need to look into more carefully. I am slowly
> >>>> becoming convinced that we need some sort of transaction-based
> >>>> configuration for pipelines.
> >>> 
> >>> This RFC is about video device node configuration, not pipelines. For
> >>> pipelines I think we'll need a transaction-based API. For video device
> >>> nodes, I'm still unsure. As stated above, if we have multiple
> >>> parameters that depend on each other, how do we let the user try them
> >>> without changing the active configuration ?
> >> 
> >> Cropping/scaling/composing IS a pipeline. Until recently the V4L2 device
> >> node API was sufficient to setup the trivial pipelines that most V4L2
> >> consumer devices have. But with the more modern devices it starts to
> >> show its limitations.
> > 
> > It's still a simple pipeline, and I think we should aim at making the
> > V4L2 device node API useful to configure this kind of pipeline. The
> > selection API is a superset of the crop API, applications should be able
> > to use it to replace the crop API in the long term.
> > 
> >> The whole 'try' concept we had for a long time needs to be re-examined.
> > 
> > I agree.
> > 
> >> As you remember, I was never satisfied with the subdev 'try' approach
> >> either, but I never could come up with a better alternative.
> 
> I've noticed that there are two different meaning of TRY flag
> a) checking if a proposed configuration is applicable for a driver
> b) storing proposed configuration in some form of temporary buffer
> 
> Ad. a. This is a real TRY command. The state of both hardware and driver
> does not change after TRY call.
> 
> Ad. b. It should be called not TRY flag because the internal state of a
> driver changes. It should be named as something like SHADOW flag. It
> would indicate that the change is applied only to shadow configuration.
> 
> I propose to change name of TRY flag for subdev to SHADOW flag. I think
> it would much clear to express the difference of TRY meaning in video
> node and subdev contexts.

It's not a shadow configuration, it can't get applied to the active 
configuration (although that might open interesting opportunities). The TRY 
settings on subdevs are really TRY settings. They're local to the file handle, 
so you can have any number of unrelated TRY settings (limited by system 
resources limits of course). They're used to try settings, not to set a shadow 
configuration.

> Therefore ioctl VIDIOC_TRY_SELECTION is probably better and more
> convenient way of testing if configuration is applicable.

Only if you make that a multi-selection, and if it can handle format and 
scaling parameters as well. Now that devices have lots of interdependent 
parameters, we need to try combinations, not individual parameters.

> >>>> Regardless of how such a scheme should work, one thing that I believe
> >>>> is missing in the format ioctls (both on the video and subdev level)
> >>>> is a similar concept like the flags in this proposal. It would be
> >>>> quite useful if you could indicate that the desired WxH has to be
> >>>> exact or can be larger or smaller. It would certainly simplify the
> >>>> interaction between the selection and scaling.
> >>>> 
> >>>>> On success field v4l2_selection::r is filled with adjusted rectangle.
> >>>>> 
> >>>>> Return value
> >>>>> 
> >>>>> 	On success 0 is returned, on error -1 and the errno variable is set
> >>>>> 	
> >>>>>         appropriately:
> >>>>> 	EINVAL - incorrect buffer type, incorrect/not supported target
> >>>>> 
> >>>>> 4. Flags
> >>>>> 4.1. Hints
> >>>>> 
> >>>>> The field v4l2_selection::flags is used to give a driver a hint about
> >>>>> coordinate adjustments. The set of possible hint flags was reduced to
> >>>>> two entities for practical reasons. The flag V4L2_SEL_SIZE_LE is a
> >>>>> suggestion for the driver to decrease or keep size of a rectangle.
> >>>>> The flags V4L2_SEL_SIZE_GE imply keeping or increasing a size of a
> >>>>> rectangle.
> >>>>> 
> >>>>> By default, lack of any hint flags indicate that driver has to choose
> >>>>> selection coordinates as close as possible to the ones passed in
> >>>>> v4l2_selection::r field.
> >>>>> 
> >>>>> Setting both flags implies that the driver is free to adjust the
> >>>>> rectangle.  It may return even random one however much more
> >>>>> encouraged behaviour would be adjusting coordinates in such a way
> >>>>> that other stream parameters are left intact. This means that the
> >>>>> driver should avoid changing a format of an image buffer and/or any
> >>>>> other controls.
> >>>> 
> >>>> This makes no sense to me. It sounds like this is what flags == 0
> >>>> should do.
> >>>> 
> >>>> I would expect that if both flags are specified that that would equal
> >>>> SEL_SIZE_EQ. I.E., the rectangle has to be exact size-wise, and should
> >>>> be as close as possible to the original position.
> >>> 
> >>> What happens if that's not possible ? The ioctl should never return an
> >>> error,
> >> 
> >> Why not? If I tell the driver that I want exactly WxH, then I see no
> >> reason why it can't return an error. An application might use that
> >> result to switch to a different resolution, for example. E.g., the
> >> application tries 640x480 first, that fails, then it tries 320x240 (or
> >> whatever).
> > 
> > To make the API more consistent. Applications ask drivers for specific
> > settings (including optional hints), and drivers return what they've been
> > able to configure. It's then the application's responsibility to check
> > the return values and act upon it. Drivers shouldn't return an error
> > when setting formats/selections, except if the given arguments can't be
> > understood.
> 
> Hmm.. I see two solutions:
> 
> Solution I (more restrictive):
> 0 - driver is free to adjust size, it is recommended to choose the
> crop/compose rectangle as close as possible to desired one
> 
> SEL_SIZE_GE - drive is not allowed to shrink the rectangle. If no such a
> rectangle exists ERANGE is returned (EINVAL is used for
> not-understandable configuration)
> 
> SEL_SIZE_LE - drive is not allowed to grow the rectangle. If no such a
> rectangle exists ERANGE is returned (EINVAL is used for
> not-understandable configuration)
> 
> SEL_SIZE_EQ = SEL_SIZE_GE | SEL_SIZE_LE - choose size exactly the same
> as in desired rectangle. Return ERANGE if such a configuration is not
> possible.

So SEL_SIZE_EQ would be identical to 0, except that ERANGE would be returned 
if the resulting configuration is not equal to the requested configuration.

> -----------------------------------------
> 
> Solution II (less restrictive). Proposed in this RFC.
> 
> 0 - apply rectangle as close as possible to desired one like the default
> behavior of  VIDIOC_S_CROP.
> 
> SEL_SIZE_GE - suggestion to increase or keep size of both coordinates
> 
> SEL_SIZE_LE - suggestion to decrease or keep size of both coordinates
> 
> SEL_SIZE_GE | SEL_SIZE_LE - technically suggestion to "increase or keep
> or decrease" sizes. Basically, it means that driver is completely free
> to choose coordinates. It works like saying "give me a crop similar to
> this one" to the driver. I agree that it is not "a very useful"
> combination of flags.

I don't see any difference between that and 0. Drivers will implement both the 
same way.

> In both solutions, the driver is recommended to keep the center of the
> rectangle in the same place.
> 
> Personally, I prefer 'solution I' because it is more logical one.
> One day, the SEL_SIZE_GE could be expanded to LEFT_LE | RIGHT_GE |
> TOP_LE | BOTTOM_GE flags if drivers could support it.

But why return ERANGE ? That's one extra check in the driver that could easily 
be done in userspace. And it won't be very useful to applications, knowing 
that the driver doesn't support one exact configuration won't help the 
application finding out how to use the hardware. Applications will likely use 
0 instead of SEL_SIZE_EQ. If we got for solution I, I think we should disallow 
SEL_SIZE_LE | SEL_SIZE_GE. It's just not useful.

> [snip]
> 
>  >>> One thing I think would be helpful though, is if the target name would
>  >>> tell you whether it is a read-only or a read-write target. It might
>  >>> also be helpful if the IDs of the read-only targets would set some
>  >>> read-only bit. That would make it easy for video_ioctl2 to test for
>  >>> invalid targets for S_SELECTION.
>  >>> 
>  >>> Not sure about the naming though:
>  >>> 
>  >>> V4L2_SEL_RO_CROP_DEFAULT
>  >>> V4L2_SEL_CROP_RO_DEFAULT
>  >>> V4L2_SEL_CROP_DEFAULT_RO
>  >>> 
>  >>> None looks right. A read-only bit might be sufficient as it would
>  >>> clearly indicate in the header that that target is a read-only target.
>  >> 
>  >> What if some future hardware have setable default or bounds rectangles ?
>  >> I don't know what that would be used for, it's just in case :-)
>  > 
>  > If it is settable, then it is no longer a default or bounds rectangle
>  > but some other rectangle :-)
> 
> I agree with Laurent. Both bounds and defrect are not exactly read-only.
> They could be modified by S_FMT, S_STD/S_DV_PRESET ioctl.
> The DEFRECT can change after switching aspect ratio on TV output.
> 
> If cropping is not supported setting ACTIVE target returns EINVAL,
> because this target is read-only in this context.
> Change of ACTIVE target might be also impossible because hardware is in
> streaming state.
> 
> Basically, ant target could be Read-only because of some reason.
> 
> Therefore I see no reason to break symmetry between
> active/bound/defrect/? by adding RO {pre/suff}ix.
> 
> [snip]
> 
> >>>>> All cropcap fields except pixel aspect are supported in new API. 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-clari
> >>>>> fica tion.html
> >>>> 
> >>>> Yeah, what are we going to do about that? I agree that it does not
> >>>> belong here. But we can't sweep it under the carpet either.
> >>>> 
> >>>> The pixel aspect ratio is typically a property of the current input or
> >>>> output video format (either a DV preset or a PAL/NTSC STD). For DV
> >>>> presets we could add it to struct v4l2_dv_enum_presets and we could do
> >>>> the same for STD formats by adding it to struct v4l2_standard.
> >>> 
> >>> Cropping and composing doesn't modify the pixel aspect ratio, so I
> >>> agree it doesn't belong there.
> >>> 
> >>>> This would fail for sensors, though, since there the chosen sensor
> >>>> framesize is set through S_FMT. (This never quite made sense to me,
> >>>> though, from a V4L2 API perspective). I'm not sure whether we can
> >>>> always assume 1:1 pixel ratio for sensors. Does anyone know?
> >>> 
> >>> Most of the time I suppose so, but I wouldn't be surprise if some
> >>> exotic sensors had non-square pixel aspect ratios.
> >> 
> >> Would it make sense to add the pixel ratio to struct v4l2_frmsizeenum?
> >> 
> >> And what about adding a VIDIOC_G/S_FRAMESIZE to select a sensor
> >> resolution?
> >> 
> >> This would typically be one of the discrete framesizes supported by a
> >> sensor through binning/skipping. If there is also a scaler on the
> >> sensor, then that is controlled through S_FMT. For video it is S_FMT
> >> that controls the scaler (together with S_CROP at the moment), but the
> >> source resolution is set through S_STD/S_DV_PRESET/S_DV_TIMINGS. It
> >> always felt very inconsistent to me that there is no equivalent for
> >> sensors, even though you can enumerate all the available framesizes
> >> (just as you can with ENUMSTD and ENUM_DV_PRESETS).
> 
> The problem is that S_FMT is used to configure two entities:
> - memory buffer
> - sensor
> The selection API help to configure crop/compose/scaling. Therefore if
> selection would be accepted in V4L, then it may be worth to consider
> separation of memory and sensors configuration.

I agree about this. S_FMT served its purpose, but with modern hardware the API 
is becoming too limited. We need to configure more than just a format and a 
crop rectangle.

The S_CROP/S_FMT API works well for past and current simple hardware. With the 
increasing device complexity, "simple" gets redefined all the time. If we keep 
the V4L2 definition of "simple", most future devices will need to be 
configured at the subdev level, which is probably not desirable. We should 
thus rethink what "simple" means, and define what level of hardware complexity 
can be configured through video device nodes only. This will require 
additional ioctls (otherwise this whole discussion wouldn't have been 
started).

I believe the definition of "simple" should take the form of a hardware 
pipeline with several operations (cropping, binning, scaling, composing, ..., 
possibly at different levels) and a description of how the V4L2 ioctls 
(current and new) map to that pipeline. Hardware that can fit that 
descriptions will be controlable through video device nodes, hardware that 
doesn't will require the media controller API. Of course the real hardware 
topology can always be reported to userspace through the MC API, regarless of 
whether the hardware fits our "simple" pipeline or not.

> I agree that sensors need a dedicated ioctl of {G,S,TRY,ENUM}_SENSOR family.
> I do not feel competent in this matter but I think that the ioctl should
> support feature typical for sensors like:
> - array size (before scaling)
> - array shape
> - (sub)pixel ratio or equivalent
> - binning/skipping

I'm not sure if we need an ioctl specific to sensors. Selecting the input 
format and resolution might be enough.

> The V4L is designed for simple pipelines like one below:
> 
> Input ---->  Processing ----> Output
> 
> In sensor case we have:
> 
> Sensor array  ----->  Processing --------> Memory
> - resolution          - compose            - format
> - binning             - crop               - resolution bounds
> - skipping            - scaling            - size in bytes (!)
>                                             - alignment/bytesperline
>
> For TV output the pipeline is following:
> 
> Memory       ------> Processing --------> TV output
> - format             - compose            - tv standard
> - bounds             - crop               - timing
> - size               - scaling            - DAC encoding
> - alignment

The memory format is the result of the processing block output (or the 
expectations of the processing block input), so I'm not sure if it needs to be 
configured explictly.
 
> I think that memory(buffers) should be configured using S_FMT. Sensors
> should be configured with new S_SENSOR ioctl.
>
> I do not see how this approach could break backward compatibility?

I'm not sure if it does, I just think we need to carefully make sure that it 
won't break the V4L2 API semantics. Applications must not get a different 
behaviour from the same calls all of a sudden.

> The only problem might be that the sensor may not work optimally is its
> default resolution is too big in comparison to buffer resolution.
> Choosing optimal configuration of sensor/scaler is done in MC.
> Applying overmentioned approach, it could be also configured in video node.
> 
> What is your opinion about this idea?

As explained above, my belief is that we need to define the word "simple" (or 
any other word that we can use for the same purpose) in the V4L2 context.

> > Let's take one step back here.
> > 
> > We started with the V4L2 device node API to control (more or less) simple
> > devices. Device became more complex, and we created a new MC API (along
> > with the subdev pad-level API) to configure complex pipelines. The V4L2
> > device node API still lives, and we want to enhance it to configure
> > medium complexity devices.
> > 
> > Before going much further, I think we need to define what a medium
> > complexity device is and where we put the boundary between devices that
> > can be configured with the V4L2 device node API, and devices that
> > require the MC API.
> > 
> > I believe this shouldn't be too difficult. What we need to do is create a
> > simple virtual pipeline that supports cropping, scaling and composing,
> > and map the V4L2 device node API to that pipeline configuration. Devices
> > that map to that pipeline could then use the V4L2 device node API only,
> > with clearly defined semantics.
> > 
> > [snip]
> > 
> >>>>>   * resolution of an image combined with support for
> >>>>>   VIDIOC_S_MULTISELECTION
> >>>>>   
> >>>>>     allows to pass a triple format/crop/compose sizes in a single
> >>>>>     ioctl
> >>>> 
> >>>> I don't believe S_MULTISELECTION will solve anything. Specific
> >>>> use-cases perhaps, but not the general problem of setting up a
> >>>> pipeline. I feel another brainstorm session coming to solve that. We
> >>>> never came to a solution for it in Warsaw.
> >>> 
> >>> Pipelines are configured on subdev nodes, not on video nodes, so I'm
> >>> also unsure whether multiselection support would really be useful.
> 
> Passing compose and crop rectangle in single ioctl might help in some
> cases, like the one described here:
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/2758
> 1
> 
> >>> If we decide to implement multiselection support, we might as well use
> >>> that only. We would need a multiselection target bitmask, so selection
> >>> targets should all be < 32.
> 
> I was considering something like this:
> 
> struct v4l2_multiselection {
> 	int n; /* number of targets */
> 	struct v4l2_selection s[0]; /* 0-length array */
> };
> 
> There is no need for a bitmask.
> 
> >>> Thinking some more about it, does it make sense to set both crop and
> >>> compose on a single video device node (not talking about mem-to-mem,
> >>> where you use the type to multiplex input/output devices on the same
> >>> node) ? If so, what would the use cases be ?
> 
> Configuration of multi input/output devices could be realised by adding
> extra targets and passing them all using MULTISELECTION.
> 
> To sum up, the multiselection is only an brainstorm idea. I think that
> transaction-based API is simpler and more robust.
> Multiselection would be realized by passing multiple VIDIOC_S_SELECTION
> inside single transaction window.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Tomasz Stanislawski]

Laurent Pinchart wrote:
> Hi Tomasz,
> 
> On Wednesday 18 May 2011 18:55:51 Tomasz Stanislawski wrote:
>> Laurent Pinchart wrote:
>>> On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
>>>> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
>>>>> On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
>>>>>> On Thursday, May 05, 2011 11:39:54 Tomasz Stanislawski wrote:
>> Hi Laurent and Hans,
>> I am very sorry for replying so lately. I was really busy during last week.
>> Thank you very much for your interest and comments :).
> 
> No worries. I get more time to rest when you don't reply on the spot, so I 
> don't mind :-)
> 
> [snip]
> 
>>>>>>> Applications set V4L2_SEL_TRY flag in v4l2_selection::flags to
>>>>>>> prevent a driver from applying selection configuration to hardware.
>>>>>> I mentioned this before but I am very much opposed to this flag. It is
>>>>>> inconsistent with the TRY_FMT and TRY_EXT_CTRLS ioctls. Note that in
>>>>>> video_ioctl2 it should be just one function with 'try' boolean
>>>>>> argument. It has always been a mistake that the try and set functions
>>>>>> were separated in the driver code IMHO.
>>>>>>
>>>>>> I know that the subdev ioctls do not have a try version, but it is not
>>>>>> quite the same in that those try functions actually store the try
>>>>>> information.
>>>>> That's exactly why the subdevs pad-level API has a try flag instead of
>>>>> a try operation, and that's why g/s_selection on subdevs will be done
>>>>> with a try flag.
>>>>>
>>>>> As for the video device node API, I won't oppose a TRY ioctl, as long
>>>>> as we can guarantee there will never be dependencies between the
>>>>> selection rectangles and other parameters (or between the different
>>>>> selection rectangles). If the crop rectangle depends on the compose
>>>>> rectangle for instance, how can you implement a TRY ioctl to try a
>>>>> crop rectangle for a specific compose rectangle, without modifying the
>>>>> active compose rectangle first ?
>>>> In that case the TRY would adjust the crop so that it works with the
>>>> current compose rectangle.
>>> And how do you try both crop and compose settings without modifying the
>>> active configuration ? That's not possible, and I think it's a bad API
>>> limitation.
>> VIDIOC_TRY_MULTISELECTION ?
>>
>>>> But this is just one more example of our lack of proper support for
>>>> pipeline setup. It doesn't really matter whether this is at the subdev
>>>> level or at the driver level, both have the same problems.
>>>>
>>>> This requires a brainstorm session to work out.
>>>>
>>>>>> This is something we need to look into more carefully. I am slowly
>>>>>> becoming convinced that we need some sort of transaction-based
>>>>>> configuration for pipelines.
>>>>> This RFC is about video device node configuration, not pipelines. For
>>>>> pipelines I think we'll need a transaction-based API. For video device
>>>>> nodes, I'm still unsure. As stated above, if we have multiple
>>>>> parameters that depend on each other, how do we let the user try them
>>>>> without changing the active configuration ?
>>>> Cropping/scaling/composing IS a pipeline. Until recently the V4L2 device
>>>> node API was sufficient to setup the trivial pipelines that most V4L2
>>>> consumer devices have. But with the more modern devices it starts to
>>>> show its limitations.
>>> It's still a simple pipeline, and I think we should aim at making the
>>> V4L2 device node API useful to configure this kind of pipeline. The
>>> selection API is a superset of the crop API, applications should be able
>>> to use it to replace the crop API in the long term.
>>>
>>>> The whole 'try' concept we had for a long time needs to be re-examined.
>>> I agree.
>>>
>>>> As you remember, I was never satisfied with the subdev 'try' approach
>>>> either, but I never could come up with a better alternative.
>> I've noticed that there are two different meaning of TRY flag
>> a) checking if a proposed configuration is applicable for a driver
>> b) storing proposed configuration in some form of temporary buffer
>>
>> Ad. a. This is a real TRY command. The state of both hardware and driver
>> does not change after TRY call.
>>
>> Ad. b. It should be called not TRY flag because the internal state of a
>> driver changes. It should be named as something like SHADOW flag. It
>> would indicate that the change is applied only to shadow configuration.
>>
>> I propose to change name of TRY flag for subdev to SHADOW flag. I think
>> it would much clear to express the difference of TRY meaning in video
>> node and subdev contexts.
> 
> It's not a shadow configuration, it can't get applied to the active 
> configuration (although that might open interesting opportunities). The TRY 
> settings on subdevs are really TRY settings. They're local to the file handle, 
> so you can have any number of unrelated TRY settings (limited by system 
> resources limits of course). They're used to try settings, not to set a shadow 
> configuration.
> 
>> Therefore ioctl VIDIOC_TRY_SELECTION is probably better and more
>> convenient way of testing if configuration is applicable.
> 
> Only if you make that a multi-selection, and if it can handle format and 
> scaling parameters as well. Now that devices have lots of interdependent 
> parameters, we need to try combinations, not individual parameters.
> 
>>>>>> Regardless of how such a scheme should work, one thing that I believe
>>>>>> is missing in the format ioctls (both on the video and subdev level)
>>>>>> is a similar concept like the flags in this proposal. It would be
>>>>>> quite useful if you could indicate that the desired WxH has to be
>>>>>> exact or can be larger or smaller. It would certainly simplify the
>>>>>> interaction between the selection and scaling.
>>>>>>
>>>>>>> On success field v4l2_selection::r is filled with adjusted rectangle.
>>>>>>>
>>>>>>> Return value
>>>>>>>
>>>>>>> 	On success 0 is returned, on error -1 and the errno variable is set
>>>>>>> 	
>>>>>>>         appropriately:
>>>>>>> 	EINVAL - incorrect buffer type, incorrect/not supported target
>>>>>>>
>>>>>>> 4. Flags
>>>>>>> 4.1. Hints
>>>>>>>
>>>>>>> The field v4l2_selection::flags is used to give a driver a hint about
>>>>>>> coordinate adjustments. The set of possible hint flags was reduced to
>>>>>>> two entities for practical reasons. The flag V4L2_SEL_SIZE_LE is a
>>>>>>> suggestion for the driver to decrease or keep size of a rectangle.
>>>>>>> The flags V4L2_SEL_SIZE_GE imply keeping or increasing a size of a
>>>>>>> rectangle.
>>>>>>>
>>>>>>> By default, lack of any hint flags indicate that driver has to choose
>>>>>>> selection coordinates as close as possible to the ones passed in
>>>>>>> v4l2_selection::r field.
>>>>>>>
>>>>>>> Setting both flags implies that the driver is free to adjust the
>>>>>>> rectangle.  It may return even random one however much more
>>>>>>> encouraged behaviour would be adjusting coordinates in such a way
>>>>>>> that other stream parameters are left intact. This means that the
>>>>>>> driver should avoid changing a format of an image buffer and/or any
>>>>>>> other controls.
>>>>>> This makes no sense to me. It sounds like this is what flags == 0
>>>>>> should do.
>>>>>>
>>>>>> I would expect that if both flags are specified that that would equal
>>>>>> SEL_SIZE_EQ. I.E., the rectangle has to be exact size-wise, and should
>>>>>> be as close as possible to the original position.
>>>>> What happens if that's not possible ? The ioctl should never return an
>>>>> error,
>>>> Why not? If I tell the driver that I want exactly WxH, then I see no
>>>> reason why it can't return an error. An application might use that
>>>> result to switch to a different resolution, for example. E.g., the
>>>> application tries 640x480 first, that fails, then it tries 320x240 (or
>>>> whatever).
>>> To make the API more consistent. Applications ask drivers for specific
>>> settings (including optional hints), and drivers return what they've been
>>> able to configure. It's then the application's responsibility to check
>>> the return values and act upon it. Drivers shouldn't return an error
>>> when setting formats/selections, except if the given arguments can't be
>>> understood.
>> Hmm.. I see two solutions:
>>
>> Solution I (more restrictive):
>> 0 - driver is free to adjust size, it is recommended to choose the
>> crop/compose rectangle as close as possible to desired one
>>
>> SEL_SIZE_GE - drive is not allowed to shrink the rectangle. If no such a
>> rectangle exists ERANGE is returned (EINVAL is used for
>> not-understandable configuration)
>>
>> SEL_SIZE_LE - drive is not allowed to grow the rectangle. If no such a
>> rectangle exists ERANGE is returned (EINVAL is used for
>> not-understandable configuration)
>>
>> SEL_SIZE_EQ = SEL_SIZE_GE | SEL_SIZE_LE - choose size exactly the same
>> as in desired rectangle. Return ERANGE if such a configuration is not
>> possible.
> 
> So SEL_SIZE_EQ would be identical to 0, except that ERANGE would be returned 
> if the resulting configuration is not equal to the requested configuration.
> 
>> -----------------------------------------
>>
>> Solution II (less restrictive). Proposed in this RFC.
>>
>> 0 - apply rectangle as close as possible to desired one like the default
>> behavior of  VIDIOC_S_CROP.
>>
>> SEL_SIZE_GE - suggestion to increase or keep size of both coordinates
>>
>> SEL_SIZE_LE - suggestion to decrease or keep size of both coordinates
>>
>> SEL_SIZE_GE | SEL_SIZE_LE - technically suggestion to "increase or keep
>> or decrease" sizes. Basically, it means that driver is completely free
>> to choose coordinates. It works like saying "give me a crop similar to
>> this one" to the driver. I agree that it is not "a very useful"
>> combination of flags.
> 
> I don't see any difference between that and 0. Drivers will implement both the 
> same way.
> 
>> In both solutions, the driver is recommended to keep the center of the
>> rectangle in the same place.
>>
>> Personally, I prefer 'solution I' because it is more logical one.
>> One day, the SEL_SIZE_GE could be expanded to LEFT_LE | RIGHT_GE |
>> TOP_LE | BOTTOM_GE flags if drivers could support it.
> 
> But why return ERANGE ? That's one extra check in the driver that could easily 
> be done in userspace. And it won't be very useful to applications, knowing 
> that the driver doesn't support one exact configuration won't help the 
> application finding out how to use the hardware. Applications will likely use 
> 0 instead of SEL_SIZE_EQ. If we got for solution I, I think we should disallow 
> SEL_SIZE_LE | SEL_SIZE_GE. It's just not useful.
> 

Hi Laurent,
You are right that the check could be done in the userspace.
However I think it is better to do it in driver or V4L2 framework
because of following reasons:

1. Checking by an application is a redundant work:
- application specifies constraint flags
- application checks if returned coordinates suit to the flags,
   so demands are implemented twice by passing flags and making checks,
   it may lead to error prone code and difficult to detect bugs.
- the code for checking of coordinates would be duplicated in every 
application that would use SELECTION

2. Coordinate checking could be done by v4l2 framework. I mean adding a 
function like one below:
int v4l2_selection_check_rect(const struct v4l2_rect *adjusted, const 
struct v4l2_rect *desired, int flags)

The function whould be called by driver after initial adjustments.
The function returns -ERANGE if coordinates of adjusted rectangle do not 
suit to desired one basing on constraint flags.

3. It is easier to add new flags if checking is controlled by 
driver/v4l2 framework (including libv4l2).

4. Successful S_SELECTION may change format set by S_FMT
- if adjusted rectangle does not suit to application's demands then 
falling back to other crop resolution requires to reconfigure the 
pipeline (calling S_FMT again).
- therefore S_SELECTION should fail if it not possible to satisfy 
applications constraints and leave the hardware configuration intact

5. Some application may want to have a fixed crop resolution, others may 
allow adjustment. I think that API should let applications explicitly 
decide which treatment they prefer and using SIZE_EQ is an intuitive way 
to force fixed coordinates. If the application if forced to use a fixed 
crop resolution. Without SIZE_EQ the application has to to a lot of 
checks only to detect that the resolution is not applicable.
The application that use SIZE_* flags knows failure may happen.

Looking for reply,
Best regards,
Tomasz Stanislawski

>> [snip]
>>
>>  >>> One thing I think would be helpful though, is if the target name would
>>  >>> tell you whether it is a read-only or a read-write target. It might
>>  >>> also be helpful if the IDs of the read-only targets would set some
>>  >>> read-only bit. That would make it easy for video_ioctl2 to test for
>>  >>> invalid targets for S_SELECTION.
>>  >>> 
>>  >>> Not sure about the naming though:
>>  >>> 
>>  >>> V4L2_SEL_RO_CROP_DEFAULT
>>  >>> V4L2_SEL_CROP_RO_DEFAULT
>>  >>> V4L2_SEL_CROP_DEFAULT_RO
>>  >>> 
>>  >>> None looks right. A read-only bit might be sufficient as it would
>>  >>> clearly indicate in the header that that target is a read-only target.
>>  >> 
>>  >> What if some future hardware have setable default or bounds rectangles ?
>>  >> I don't know what that would be used for, it's just in case :-)
>>  > 
>>  > If it is settable, then it is no longer a default or bounds rectangle
>>  > but some other rectangle :-)
>>
>> I agree with Laurent. Both bounds and defrect are not exactly read-only.
>> They could be modified by S_FMT, S_STD/S_DV_PRESET ioctl.
>> The DEFRECT can change after switching aspect ratio on TV output.
>>
>> If cropping is not supported setting ACTIVE target returns EINVAL,
>> because this target is read-only in this context.
>> Change of ACTIVE target might be also impossible because hardware is in
>> streaming state.
>>
>> Basically, ant target could be Read-only because of some reason.
>>
>> Therefore I see no reason to break symmetry between
>> active/bound/defrect/? by adding RO {pre/suff}ix.
>>
>> [snip]
>>
>>>>>>> All cropcap fields except pixel aspect are supported in new API. 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-clari
>>>>>>> fica tion.html
>>>>>> Yeah, what are we going to do about that? I agree that it does not
>>>>>> belong here. But we can't sweep it under the carpet either.
>>>>>>
>>>>>> The pixel aspect ratio is typically a property of the current input or
>>>>>> output video format (either a DV preset or a PAL/NTSC STD). For DV
>>>>>> presets we could add it to struct v4l2_dv_enum_presets and we could do
>>>>>> the same for STD formats by adding it to struct v4l2_standard.
>>>>> Cropping and composing doesn't modify the pixel aspect ratio, so I
>>>>> agree it doesn't belong there.
>>>>>
>>>>>> This would fail for sensors, though, since there the chosen sensor
>>>>>> framesize is set through S_FMT. (This never quite made sense to me,
>>>>>> though, from a V4L2 API perspective). I'm not sure whether we can
>>>>>> always assume 1:1 pixel ratio for sensors. Does anyone know?
>>>>> Most of the time I suppose so, but I wouldn't be surprise if some
>>>>> exotic sensors had non-square pixel aspect ratios.
>>>> Would it make sense to add the pixel ratio to struct v4l2_frmsizeenum?
>>>>
>>>> And what about adding a VIDIOC_G/S_FRAMESIZE to select a sensor
>>>> resolution?
>>>>
>>>> This would typically be one of the discrete framesizes supported by a
>>>> sensor through binning/skipping. If there is also a scaler on the
>>>> sensor, then that is controlled through S_FMT. For video it is S_FMT
>>>> that controls the scaler (together with S_CROP at the moment), but the
>>>> source resolution is set through S_STD/S_DV_PRESET/S_DV_TIMINGS. It
>>>> always felt very inconsistent to me that there is no equivalent for
>>>> sensors, even though you can enumerate all the available framesizes
>>>> (just as you can with ENUMSTD and ENUM_DV_PRESETS).
>> The problem is that S_FMT is used to configure two entities:
>> - memory buffer
>> - sensor
>> The selection API help to configure crop/compose/scaling. Therefore if
>> selection would be accepted in V4L, then it may be worth to consider
>> separation of memory and sensors configuration.
> 
> I agree about this. S_FMT served its purpose, but with modern hardware the API 
> is becoming too limited. We need to configure more than just a format and a 
> crop rectangle.
> 
> The S_CROP/S_FMT API works well for past and current simple hardware. With the 
> increasing device complexity, "simple" gets redefined all the time. If we keep 
> the V4L2 definition of "simple", most future devices will need to be 
> configured at the subdev level, which is probably not desirable. We should 
> thus rethink what "simple" means, and define what level of hardware complexity 
> can be configured through video device nodes only. This will require 
> additional ioctls (otherwise this whole discussion wouldn't have been 
> started).
> 
> I believe the definition of "simple" should take the form of a hardware 
> pipeline with several operations (cropping, binning, scaling, composing, ..., 
> possibly at different levels) and a description of how the V4L2 ioctls 
> (current and new) map to that pipeline. Hardware that can fit that 
> descriptions will be controlable through video device nodes, hardware that 
> doesn't will require the media controller API. Of course the real hardware 
> topology can always be reported to userspace through the MC API, regarless of 
> whether the hardware fits our "simple" pipeline or not.
> 
>> I agree that sensors need a dedicated ioctl of {G,S,TRY,ENUM}_SENSOR family.
>> I do not feel competent in this matter but I think that the ioctl should
>> support feature typical for sensors like:
>> - array size (before scaling)
>> - array shape
>> - (sub)pixel ratio or equivalent
>> - binning/skipping
> 
> I'm not sure if we need an ioctl specific to sensors. Selecting the input 
> format and resolution might be enough.
> 
>> The V4L is designed for simple pipelines like one below:
>>
>> Input ---->  Processing ----> Output
>>
>> In sensor case we have:
>>
>> Sensor array  ----->  Processing --------> Memory
>> - resolution          - compose            - format
>> - binning             - crop               - resolution bounds
>> - skipping            - scaling            - size in bytes (!)
>>                                             - alignment/bytesperline
>>
>> For TV output the pipeline is following:
>>
>> Memory       ------> Processing --------> TV output
>> - format             - compose            - tv standard
>> - bounds             - crop               - timing
>> - size               - scaling            - DAC encoding
>> - alignment
> 
> The memory format is the result of the processing block output (or the 
> expectations of the processing block input), so I'm not sure if it needs to be 
> configured explictly.
>  
>> I think that memory(buffers) should be configured using S_FMT. Sensors
>> should be configured with new S_SENSOR ioctl.
>>
>> I do not see how this approach could break backward compatibility?
> 
> I'm not sure if it does, I just think we need to carefully make sure that it 
> won't break the V4L2 API semantics. Applications must not get a different 
> behaviour from the same calls all of a sudden.
> 
>> The only problem might be that the sensor may not work optimally is its
>> default resolution is too big in comparison to buffer resolution.
>> Choosing optimal configuration of sensor/scaler is done in MC.
>> Applying overmentioned approach, it could be also configured in video node.
>>
>> What is your opinion about this idea?
> 
> As explained above, my belief is that we need to define the word "simple" (or 
> any other word that we can use for the same purpose) in the V4L2 context.
> 
>>> Let's take one step back here.
>>>
>>> We started with the V4L2 device node API to control (more or less) simple
>>> devices. Device became more complex, and we created a new MC API (along
>>> with the subdev pad-level API) to configure complex pipelines. The V4L2
>>> device node API still lives, and we want to enhance it to configure
>>> medium complexity devices.
>>>
>>> Before going much further, I think we need to define what a medium
>>> complexity device is and where we put the boundary between devices that
>>> can be configured with the V4L2 device node API, and devices that
>>> require the MC API.
>>>
>>> I believe this shouldn't be too difficult. What we need to do is create a
>>> simple virtual pipeline that supports cropping, scaling and composing,
>>> and map the V4L2 device node API to that pipeline configuration. Devices
>>> that map to that pipeline could then use the V4L2 device node API only,
>>> with clearly defined semantics.
>>>
>>> [snip]
>>>
>>>>>>>   * resolution of an image combined with support for
>>>>>>>   VIDIOC_S_MULTISELECTION
>>>>>>>   
>>>>>>>     allows to pass a triple format/crop/compose sizes in a single
>>>>>>>     ioctl
>>>>>> I don't believe S_MULTISELECTION will solve anything. Specific
>>>>>> use-cases perhaps, but not the general problem of setting up a
>>>>>> pipeline. I feel another brainstorm session coming to solve that. We
>>>>>> never came to a solution for it in Warsaw.
>>>>> Pipelines are configured on subdev nodes, not on video nodes, so I'm
>>>>> also unsure whether multiselection support would really be useful.
>> Passing compose and crop rectangle in single ioctl might help in some
>> cases, like the one described here:
>> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/2758
>> 1
>>
>>>>> If we decide to implement multiselection support, we might as well use
>>>>> that only. We would need a multiselection target bitmask, so selection
>>>>> targets should all be < 32.
>> I was considering something like this:
>>
>> struct v4l2_multiselection {
>> 	int n; /* number of targets */
>> 	struct v4l2_selection s[0]; /* 0-length array */
>> };
>>
>> There is no need for a bitmask.
>>
>>>>> Thinking some more about it, does it make sense to set both crop and
>>>>> compose on a single video device node (not talking about mem-to-mem,
>>>>> where you use the type to multiplex input/output devices on the same
>>>>> node) ? If so, what would the use cases be ?
>> Configuration of multi input/output devices could be realised by adding
>> extra targets and passing them all using MULTISELECTION.
>>
>> To sum up, the multiselection is only an brainstorm idea. I think that
>> transaction-based API is simpler and more robust.
>> Multiselection would be realized by passing multiple VIDIOC_S_SELECTION
>> inside single transaction window.
> 

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Laurent Pinchart]

Hi Tomasz,

On Tuesday 24 May 2011 14:28:49 Tomasz Stanislawski wrote:
> Laurent Pinchart wrote:
> > On Wednesday 18 May 2011 18:55:51 Tomasz Stanislawski wrote:
> >> Laurent Pinchart wrote:
> >>> On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
> >>>> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
> >>>>> On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
> >>>>>> On Thursday, May 05, 2011 11:39:54 Tomasz Stanislawski wrote:

[snip]

> >> Solution I (more restrictive):
> >> 0 - driver is free to adjust size, it is recommended to choose the
> >> crop/compose rectangle as close as possible to desired one
> >> 
> >> SEL_SIZE_GE - drive is not allowed to shrink the rectangle. If no such a
> >> rectangle exists ERANGE is returned (EINVAL is used for
> >> not-understandable configuration)
> >> 
> >> SEL_SIZE_LE - drive is not allowed to grow the rectangle. If no such a
> >> rectangle exists ERANGE is returned (EINVAL is used for
> >> not-understandable configuration)
> >> 
> >> SEL_SIZE_EQ = SEL_SIZE_GE | SEL_SIZE_LE - choose size exactly the same
> >> as in desired rectangle. Return ERANGE if such a configuration is not
> >> possible.
> > 
> > So SEL_SIZE_EQ would be identical to 0, except that ERANGE would be
> > returned if the resulting configuration is not equal to the requested
> > configuration.
> > 
> >> -----------------------------------------
> >> 
> >> Solution II (less restrictive). Proposed in this RFC.
> >> 
> >> 0 - apply rectangle as close as possible to desired one like the default
> >> behavior of  VIDIOC_S_CROP.
> >> 
> >> SEL_SIZE_GE - suggestion to increase or keep size of both coordinates
> >> 
> >> SEL_SIZE_LE - suggestion to decrease or keep size of both coordinates
> >> 
> >> SEL_SIZE_GE | SEL_SIZE_LE - technically suggestion to "increase or keep
> >> or decrease" sizes. Basically, it means that driver is completely free
> >> to choose coordinates. It works like saying "give me a crop similar to
> >> this one" to the driver. I agree that it is not "a very useful"
> >> combination of flags.
> > 
> > I don't see any difference between that and 0. Drivers will implement
> > both the same way.
> > 
> >> In both solutions, the driver is recommended to keep the center of the
> >> rectangle in the same place.
> >> 
> >> Personally, I prefer 'solution I' because it is more logical one.
> >> One day, the SEL_SIZE_GE could be expanded to LEFT_LE | RIGHT_GE |
> >> TOP_LE | BOTTOM_GE flags if drivers could support it.
> > 
> > But why return ERANGE ? That's one extra check in the driver that could
> > easily be done in userspace. And it won't be very useful to
> > applications, knowing that the driver doesn't support one exact
> > configuration won't help the application finding out how to use the
> > hardware. Applications will likely use 0 instead of SEL_SIZE_EQ. If we
> > got for solution I, I think we should disallow SEL_SIZE_LE |
> > SEL_SIZE_GE. It's just not useful.
> 
> Hi Laurent,
> You are right that the check could be done in the userspace.
> However I think it is better to do it in driver or V4L2 framework
> because of following reasons:
> 
> 1. Checking by an application is a redundant work:
> - application specifies constraint flags
> - application checks if returned coordinates suit to the flags,
>    so demands are implemented twice by passing flags and making checks,
>    it may lead to error prone code and difficult to detect bugs.
> - the code for checking of coordinates would be duplicated in every
> application that would use SELECTION
> 
> 2. Coordinate checking could be done by v4l2 framework. I mean adding a
> function like one below:
> int v4l2_selection_check_rect(const struct v4l2_rect *adjusted, const
> struct v4l2_rect *desired, int flags)
> 
> The function whould be called by driver after initial adjustments.
> The function returns -ERANGE if coordinates of adjusted rectangle do not
> suit to desired one basing on constraint flags.
> 
> 3. It is easier to add new flags if checking is controlled by
> driver/v4l2 framework (including libv4l2).
> 
> 4. Successful S_SELECTION may change format set by S_FMT
> - if adjusted rectangle does not suit to application's demands then
> falling back to other crop resolution requires to reconfigure the
> pipeline (calling S_FMT again).
> - therefore S_SELECTION should fail if it not possible to satisfy
> applications constraints and leave the hardware configuration intact

We need to define how S_SELECTION and S_FMT will interact before I can answer 
this :-) The LE | GE behaviour is a detail though, so I think we can postpone 
the decision and work on S_SELECTION/S_FMT/S_WHATEVER interaction first.

> 5. Some application may want to have a fixed crop resolution, others may
> allow adjustment. I think that API should let applications explicitly
> decide which treatment they prefer and using SIZE_EQ is an intuitive way
> to force fixed coordinates. If the application if forced to use a fixed
> crop resolution. Without SIZE_EQ the application has to to a lot of
> checks only to detect that the resolution is not applicable.
> The application that use SIZE_* flags knows failure may happen.

(In response to 1-3 and 5) Our current policy with most format-related ioctls 
is that the driver has to respond to a user request in a "best effort" way. 
This means the driver tries to honor the request as much as possible, and 
returns what it can achieve. It's then up to the application to use that 
information to implement application-specific policies. I think that a hint 
that would require the driver to return an error would not be consistent with 
the API, and that inconsistency wouldn't bring any added value.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Tomasz Stanislawski]

Laurent Pinchart wrote:
> Hi Tomasz,
> 
> On Tuesday 24 May 2011 14:28:49 Tomasz Stanislawski wrote:
>> Laurent Pinchart wrote:
>>> On Wednesday 18 May 2011 18:55:51 Tomasz Stanislawski wrote:
>>>> Laurent Pinchart wrote:
>>>>> On Saturday 14 May 2011 12:50:32 Hans Verkuil wrote:
>>>>>> On Friday, May 13, 2011 14:43:08 Laurent Pinchart wrote:
>>>>>>> On Saturday 07 May 2011 13:52:25 Hans Verkuil wrote:
>>>>>>>> On Thursday, May 05, 2011 11:39:54 Tomasz Stanislawski wrote:
> 
> [snip]
> 
>>>> Solution I (more restrictive):
>>>> 0 - driver is free to adjust size, it is recommended to choose the
>>>> crop/compose rectangle as close as possible to desired one
>>>>
>>>> SEL_SIZE_GE - drive is not allowed to shrink the rectangle. If no such a
>>>> rectangle exists ERANGE is returned (EINVAL is used for
>>>> not-understandable configuration)
>>>>
>>>> SEL_SIZE_LE - drive is not allowed to grow the rectangle. If no such a
>>>> rectangle exists ERANGE is returned (EINVAL is used for
>>>> not-understandable configuration)
>>>>
>>>> SEL_SIZE_EQ = SEL_SIZE_GE | SEL_SIZE_LE - choose size exactly the same
>>>> as in desired rectangle. Return ERANGE if such a configuration is not
>>>> possible.
>>> So SEL_SIZE_EQ would be identical to 0, except that ERANGE would be
>>> returned if the resulting configuration is not equal to the requested
>>> configuration.
>>>
>>>> -----------------------------------------
>>>>
>>>> Solution II (less restrictive). Proposed in this RFC.
>>>>
>>>> 0 - apply rectangle as close as possible to desired one like the default
>>>> behavior of  VIDIOC_S_CROP.
>>>>
>>>> SEL_SIZE_GE - suggestion to increase or keep size of both coordinates
>>>>
>>>> SEL_SIZE_LE - suggestion to decrease or keep size of both coordinates
>>>>
>>>> SEL_SIZE_GE | SEL_SIZE_LE - technically suggestion to "increase or keep
>>>> or decrease" sizes. Basically, it means that driver is completely free
>>>> to choose coordinates. It works like saying "give me a crop similar to
>>>> this one" to the driver. I agree that it is not "a very useful"
>>>> combination of flags.
>>> I don't see any difference between that and 0. Drivers will implement
>>> both the same way.
>>>
>>>> In both solutions, the driver is recommended to keep the center of the
>>>> rectangle in the same place.
>>>>
>>>> Personally, I prefer 'solution I' because it is more logical one.
>>>> One day, the SEL_SIZE_GE could be expanded to LEFT_LE | RIGHT_GE |
>>>> TOP_LE | BOTTOM_GE flags if drivers could support it.
>>> But why return ERANGE ? That's one extra check in the driver that could
>>> easily be done in userspace. And it won't be very useful to
>>> applications, knowing that the driver doesn't support one exact
>>> configuration won't help the application finding out how to use the
>>> hardware. Applications will likely use 0 instead of SEL_SIZE_EQ. If we
>>> got for solution I, I think we should disallow SEL_SIZE_LE |
>>> SEL_SIZE_GE. It's just not useful.
>> Hi Laurent,
>> You are right that the check could be done in the userspace.
>> However I think it is better to do it in driver or V4L2 framework
>> because of following reasons:
>>
>> 1. Checking by an application is a redundant work:
>> - application specifies constraint flags
>> - application checks if returned coordinates suit to the flags,
>>    so demands are implemented twice by passing flags and making checks,
>>    it may lead to error prone code and difficult to detect bugs.
>> - the code for checking of coordinates would be duplicated in every
>> application that would use SELECTION
>>
>> 2. Coordinate checking could be done by v4l2 framework. I mean adding a
>> function like one below:
>> int v4l2_selection_check_rect(const struct v4l2_rect *adjusted, const
>> struct v4l2_rect *desired, int flags)
>>
>> The function whould be called by driver after initial adjustments.
>> The function returns -ERANGE if coordinates of adjusted rectangle do not
>> suit to desired one basing on constraint flags.
>>
>> 3. It is easier to add new flags if checking is controlled by
>> driver/v4l2 framework (including libv4l2).
>>
>> 4. Successful S_SELECTION may change format set by S_FMT
>> - if adjusted rectangle does not suit to application's demands then
>> falling back to other crop resolution requires to reconfigure the
>> pipeline (calling S_FMT again).
>> - therefore S_SELECTION should fail if it not possible to satisfy
>> applications constraints and leave the hardware configuration intact
> 
> We need to define how S_SELECTION and S_FMT will interact before I can answer 
> this :-) The LE | GE behaviour is a detail though, so I think we can postpone 
> the decision and work on S_SELECTION/S_FMT/S_WHATEVER interaction first.
> 
>> 5. Some application may want to have a fixed crop resolution, others may
>> allow adjustment. I think that API should let applications explicitly
>> decide which treatment they prefer and using SIZE_EQ is an intuitive way
>> to force fixed coordinates. If the application if forced to use a fixed
>> crop resolution. Without SIZE_EQ the application has to to a lot of
>> checks only to detect that the resolution is not applicable.
>> The application that use SIZE_* flags knows failure may happen.

6. In order to compare an original and adjusted rectangle, an 
application must keep a copy of the original rectangle. It introduces a 
small memory overhead.

> 
> (In response to 1-3 and 5) Our current policy with most format-related ioctls 
> is that the driver has to respond to a user request in a "best effort" way. 
> This means the driver tries to honor the request as much as possible, and 
> returns what it can achieve. It's then up to the application to use that 
> information to implement application-specific policies. I think that a hint 
> that would require the driver to return an error would not be consistent with 
> the API, and that inconsistency wouldn't bring any added value.
> 

Hi Laurent,
Thank you for your comments.

Keeping flags 0 is a request for a driver to behave in the "best effort" 
way. Using constraint flags would be used to control "best effort" 
policy. Moreover, this control is done explicitly. What do you think 
about returning the best achievable rectangle even after ERANGE error? 
The application would decide what to do next. Drawback to other 
configuration or simply to panic :).

I agree that "best effort" policy is adopted by S_FMT. I think it should 
be adopted by all configuration-related ioctls. All exceptions from this 
rule should be done explicitly by an application (like constraint flags).

Which are format-related ioctls? Is S_CROP one of them?

I noticed that there is some inconsistence in the current V4L2 API:

VIDIOC_S_CROP: EINVAL: Cropping is not supported.
- cropcap.bounds rectangle should be returned because it is the best 
crop rectangle which a driver can achieve. VIDIOC_CROPCAP is a mandatory 
ioctl, therefore a bounds rectangle always exists. Therefore driver can 
always draw back to bounds rectangle while executing S_CROP.

VIDIOC_S_DV_TIMINGS: EINVAL: This ioctl is not supported, or the 
VIDIOC_S_DV_TIMINGS parameter was unsuitable.
- a driver should adjust timings to reasonable ones, or a least return 
timings that suit to a supported preset.

VIDIOC_S_EXT_CTRLS: ERANGE: The struct v4l2_ext_control value is out of 
bounds, "When the value is out of bounds drivers can choose to take the 
closest valid value or return an ERANGE error code, whatever seems more 
appropriate"
- a driver should always saturate control's value according to bounds

I think that we need a discussion about configuration policies. Some of 
ideas might be useful for MC. As you said that the main difficulty is 
interaction between S_FMT and other ioctls and controls. The problem 
still is present in for MC framework. There you can find complex 
interaction between video node format, mbus formats/crops on pads. 
Someone has to "the dirty-job", it means a configuration of a pipeline. 
The business logic has to preset somewhere:
a) in kernel space (driver)
b) middleware (libv4l, by MC plugins)
c) application (directly using MC ioctls)

Solutions b/c are somehow forms of user a space driver.

Best regards,
Tomasz Stanislawski

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Tomasz Stanislawski]

Laurent Pinchart wrote:
> Hi Tomasz,
> 
> On Tuesday 29 March 2011 12:38:50 Tomasz Stanislawski wrote:
>> Hans Verkuil wrote:
>>> On Tuesday, March 29, 2011 11:22:17 Tomasz Stanislawski wrote:
>>>> Hans Verkuil wrote:
>>>>> On Monday, March 28, 2011 17:19:54 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.
>>>>>>
>>>>>> 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.
>>>> Hello. Thank you for a quick comment.
>>>>
>>>>> I don't have time right now to review this RFC in-depth, but one thing
>>>>> that needs more attention is the relationship between these new ioctls
>>>>> and CROPCAP.
>>>>> And also how this relates to analog inputs (I don't think analog
>>>>> outputs make any sense). And would a COMPOSECAP ioctl make sense?
>>>> Maybe two new ioctl COMPOSECAP and EXTCROPCAP should be added.
>>>> For input CROPCAP maps to EXTCROPCAP, for output it maps to COMPOSECAP.
>>>> The output EXTCROPCAP would return dimentions of a buffer.
>>>> But in my opinion field v4l2_selection::bounds should be added to
>>>> structure below. In such a case G_EXTCROP could be used to obtain
>>>> cropping bounds.
> 
> Using flags to tell G_EXTCROP and G_COMPOSE whether we want to retrieve the 
> bounds, default rectangle or current rectangle would be a sensible option in 
> my opinion.
> 
I am preparing second version of this RFC that added support for 
obtaining bounds and defrect using G_{EXTCROP / COMPOSE}. It will be 
posted soon.
>>> There is more in CROPCAP than just the bounds. I'd have to think about
>>> this myself.
> 
> [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. 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
>>>>> Wouldn't you also need similar flags for RIGHT and BOTTOM?
>>>>>
>>>>> Regards,
>>>>>
>>>>> 	Hans
>>>> Proposed flags refer to fields in v4l2_rect structure. These are left,
>>>> top, width and height. Fields bottom and right and not present in
>>>> v4l2_rect structure.
>>> But what if I want to keep the bottom right corner fixed, and allow other
>>> parameters to change? I don't think you can do that with the current set
>>> of flags.
>>>
>>> Regards,
>>>
>>> 	Hans
>> You are right. New flags should be added:
>>
>> #define V4L2_SEL_RIGHT_GE	0x00000100
>> #define V4L2_SEL_RIGHT_LE	0x00000200
>> #define V4L2_SEL_BOTTOM_GE	0x00000400
>> #define V4L2_SEL_BOTTOM_LE	0x00000800
>>
>> They  would be used to inform a driver about adjusting a bottom-right
>> corner. Right and bottom would be defined as:
>>
>> right = v4l2_rect::left + v4l2_rect::width
>> bottom = v4l2_rect::top + v4l2_rect::height
> 
> What if you want to allow the rectangle to be slightly enlarged or reduced, 
> but want to keep it centered ? I feel like this would get out of hands quite 
> fast. Hints are not a bad idea, but they will become very complex to implement 
> in drivers, especially when the hardware forces all kind of weird constraints 
> (and we all know that hardware designers get extremely creative in that area 
> :-)).
Yes.. it may be complex. Unfortunately, currently there is no way to 
control a crop adjustment. The proposed solution is not perfect but I 
think it is sufficient to deal with the most common situations.
Quite probably some simple framework or helper functions have to be 
added to V4L2 kernel API to help driver to deal with a business logic 
hidden behind crop adjustments.

Regards,
Tomasz Stanislawski

> 
>>>>>> #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_FIXED		0x000000ff
>>>>>>
>>>>>> 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_FIXED | V4L2_SEL_LEFT_FIXED |
>>>>>>   V4L2_SEL_WIDTH_GE | V4L2_SEL_HEIGHT_GE.
>>>>>>
>>>>>> - VIDIOC_COMPOSE with flags field equal to
>>>>>>
>>>>>>   V4L2_SEL_TOP_FIXED | V4L2_SEL_LEFT_FIXED |
>>>>>>   V4L2_SEL_WIDTH_LE | V4L2_SEL_HEIGHT_LE
> 

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Laurent Pinchart]

Hi Tomasz,

On Tuesday 29 March 2011 12:38:50 Tomasz Stanislawski wrote:
> Hans Verkuil wrote:
> > On Tuesday, March 29, 2011 11:22:17 Tomasz Stanislawski wrote:
> >> Hans Verkuil wrote:
> >>> On Monday, March 28, 2011 17:19:54 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.
> >>>> 
> >>>> 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.
> >> 
> >> Hello. Thank you for a quick comment.
> >> 
> >>> I don't have time right now to review this RFC in-depth, but one thing
> >>> that needs more attention is the relationship between these new ioctls
> >>> and CROPCAP.
> > 
> >>> And also how this relates to analog inputs (I don't think analog
> >>> outputs make any sense). And would a COMPOSECAP ioctl make sense?
> >> 
> >> Maybe two new ioctl COMPOSECAP and EXTCROPCAP should be added.
> >> For input CROPCAP maps to EXTCROPCAP, for output it maps to COMPOSECAP.
> >> The output EXTCROPCAP would return dimentions of a buffer.
> >> But in my opinion field v4l2_selection::bounds should be added to
> >> structure below. In such a case G_EXTCROP could be used to obtain
> >> cropping bounds.

Using flags to tell G_EXTCROP and G_COMPOSE whether we want to retrieve the 
bounds, default rectangle or current rectangle would be a sensible option in 
my opinion.

> > There is more in CROPCAP than just the bounds. I'd have to think about
> > this myself.

[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. 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
> >>> 
> >>> Wouldn't you also need similar flags for RIGHT and BOTTOM?
> >>> 
> >>> Regards,
> >>> 
> >>> 	Hans
> >> 
> >> Proposed flags refer to fields in v4l2_rect structure. These are left,
> >> top, width and height. Fields bottom and right and not present in
> >> v4l2_rect structure.
> > 
> > But what if I want to keep the bottom right corner fixed, and allow other
> > parameters to change? I don't think you can do that with the current set
> > of flags.
> > 
> > Regards,
> > 
> > 	Hans
> 
> You are right. New flags should be added:
> 
> #define V4L2_SEL_RIGHT_GE	0x00000100
> #define V4L2_SEL_RIGHT_LE	0x00000200
> #define V4L2_SEL_BOTTOM_GE	0x00000400
> #define V4L2_SEL_BOTTOM_LE	0x00000800
> 
> They  would be used to inform a driver about adjusting a bottom-right
> corner. Right and bottom would be defined as:
> 
> right = v4l2_rect::left + v4l2_rect::width
> bottom = v4l2_rect::top + v4l2_rect::height

What if you want to allow the rectangle to be slightly enlarged or reduced, 
but want to keep it centered ? I feel like this would get out of hands quite 
fast. Hints are not a bad idea, but they will become very complex to implement 
in drivers, especially when the hardware forces all kind of weird constraints 
(and we all know that hardware designers get extremely creative in that area 
:-)).

> >>>> #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_FIXED		0x000000ff
> >>>> 
> >>>> 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_FIXED | V4L2_SEL_LEFT_FIXED |
> >>>>   V4L2_SEL_WIDTH_GE | V4L2_SEL_HEIGHT_GE.
> >>>> 
> >>>> - VIDIOC_COMPOSE with flags field equal to
> >>>> 
> >>>>   V4L2_SEL_TOP_FIXED | V4L2_SEL_LEFT_FIXED |
> >>>>   V4L2_SEL_WIDTH_LE | V4L2_SEL_HEIGHT_LE

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Tomasz Stanislawski]

Hans Verkuil wrote:
> On Tuesday, March 29, 2011 11:22:17 Tomasz Stanislawski wrote:
>> Hans Verkuil wrote:
>>> On Monday, March 28, 2011 17:19:54 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.
>>>>
>>>> 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.
>> Hello. Thank you for a quick comment.
>>
>>> I don't have time right now to review this RFC in-depth, but one thing 
> that
>>> needs more attention is the relationship between these new ioctls and 
> CROPCAP.
>>> And also how this relates to analog inputs (I don't think analog outputs 
> make
>>> any sense). And would a COMPOSECAP ioctl make sense?
>>>
>> Maybe two new ioctl COMPOSECAP and EXTCROPCAP should be added.
>> For input CROPCAP maps to EXTCROPCAP, for output it maps to COMPOSECAP.
>> The output EXTCROPCAP would return dimentions of a buffer.
>> But in my opinion field v4l2_selection::bounds should be added to 
>> structure below. In such a case G_EXTCROP could be used to obtain 
>> cropping bounds.
> 
> There is more in CROPCAP than just the bounds. I'd have to think about this 
> myself.
> 
>>>> 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;
>>>> 	struct v4l2_rect r;
>>>> 	u32 flags;
>>>> 	u32 reserved[10];
>>>> };
>>>>
>>>> Where,
>>>> type	 - type of buffer queue: V4L2_BUF_TYPE_VIDEO_CAPTURE,
>>>>            V4L2_BUF_TYPE_VIDEO_OUTPUT, etc.
>>>> 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, 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
>>>> 	- 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, 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, 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
>>>> 	- 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, 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. 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
>>> Wouldn't you also need similar flags for RIGHT and BOTTOM?
>>>
>>> Regards,
>>>
>>> 	Hans
>>>
>> Proposed flags refer to fields in v4l2_rect structure. These are left, 
>> top, width and height. Fields bottom and right and not present in 
>> v4l2_rect structure.
> 
> But what if I want to keep the bottom right corner fixed, and allow other 
> parameters to change? I don't think you can do that with the current set of 
> flags.
> 
> Regards,
> 
> 	Hans

You are right. New flags should be added:

#define V4L2_SEL_RIGHT_GE	0x00000100
#define V4L2_SEL_RIGHT_LE	0x00000200
#define V4L2_SEL_BOTTOM_GE	0x00000400
#define V4L2_SEL_BOTTOM_LE	0x00000800

They  would be used to inform a driver about adjusting a bottom-right 
corner. Right and bottom would be defined as:

right = v4l2_rect::left + v4l2_rect::width
bottom = v4l2_rect::top + v4l2_rect::height

Best regards,
Tomasz Stanislawski

> 
>> Best regards,
>> Tomasz Stanislawski
>>
>>>> #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_FIXED		0x000000ff
>>>>
>>>> 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_FIXED | V4L2_SEL_LEFT_FIXED |
>>>>   V4L2_SEL_WIDTH_GE | V4L2_SEL_HEIGHT_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.
>>>>
>>>> 4. Possible improvements and extensions.
>>>> - combine composing and cropping ioctl into a single ioctl
>>>> - 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
>>>>
>>>>
>>>> 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          |  102 
> +++++++++++++++++++++++++++--
>>>>  include/linux/videodev2.h                 |   30 +++++++++
>>>>  include/media/v4l2-ioctl.h                |    8 ++
>>>>  4 files changed, 137 insertions(+), 7 deletions(-)
>>>>
>>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Hans Verkuil]

On Tuesday, March 29, 2011 11:22:17 Tomasz Stanislawski wrote:
> Hans Verkuil wrote:
> > On Monday, March 28, 2011 17:19:54 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.
> >>
> >> 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.
> 
> Hello. Thank you for a quick comment.
> 
> > 
> > I don't have time right now to review this RFC in-depth, but one thing 
that
> > needs more attention is the relationship between these new ioctls and 
CROPCAP.
> > 
> > And also how this relates to analog inputs (I don't think analog outputs 
make
> > any sense). And would a COMPOSECAP ioctl make sense?
> > 
> 
> Maybe two new ioctl COMPOSECAP and EXTCROPCAP should be added.
> For input CROPCAP maps to EXTCROPCAP, for output it maps to COMPOSECAP.
> The output EXTCROPCAP would return dimentions of a buffer.
> But in my opinion field v4l2_selection::bounds should be added to 
> structure below. In such a case G_EXTCROP could be used to obtain 
> cropping bounds.

There is more in CROPCAP than just the bounds. I'd have to think about this 
myself.

> 
> >> 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;
> >> 	struct v4l2_rect r;
> >> 	u32 flags;
> >> 	u32 reserved[10];
> >> };
> >>
> >> Where,
> >> type	 - type of buffer queue: V4L2_BUF_TYPE_VIDEO_CAPTURE,
> >>            V4L2_BUF_TYPE_VIDEO_OUTPUT, etc.
> >> 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, 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
> >> 	- 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, 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, 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
> >> 	- 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, 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. 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
> > 
> > Wouldn't you also need similar flags for RIGHT and BOTTOM?
> > 
> > Regards,
> > 
> > 	Hans
> > 
> Proposed flags refer to fields in v4l2_rect structure. These are left, 
> top, width and height. Fields bottom and right and not present in 
> v4l2_rect structure.

But what if I want to keep the bottom right corner fixed, and allow other 
parameters to change? I don't think you can do that with the current set of 
flags.

Regards,

	Hans

> 
> Best regards,
> Tomasz Stanislawski
> 
> >> #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_FIXED		0x000000ff
> >>
> >> 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_FIXED | V4L2_SEL_LEFT_FIXED |
> >>   V4L2_SEL_WIDTH_GE | V4L2_SEL_HEIGHT_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.
> >>
> >> 4. Possible improvements and extensions.
> >> - combine composing and cropping ioctl into a single ioctl
> >> - 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
> >>
> >>
> >> 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          |  102 
+++++++++++++++++++++++++++--
> >>  include/linux/videodev2.h                 |   30 +++++++++
> >>  include/media/v4l2-ioctl.h                |    8 ++
> >>  4 files changed, 137 insertions(+), 7 deletions(-)
> >>
> >>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Tomasz Stanislawski]

Hans Verkuil wrote:
> On Monday, March 28, 2011 17:19:54 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.
>>
>> 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.

Hello. Thank you for a quick comment.

> 
> I don't have time right now to review this RFC in-depth, but one thing that
> needs more attention is the relationship between these new ioctls and CROPCAP.
> 
> And also how this relates to analog inputs (I don't think analog outputs make
> any sense). And would a COMPOSECAP ioctl make sense?
> 

Maybe two new ioctl COMPOSECAP and EXTCROPCAP should be added.
For input CROPCAP maps to EXTCROPCAP, for output it maps to COMPOSECAP.
The output EXTCROPCAP would return dimentions of a buffer.
But in my opinion field v4l2_selection::bounds should be added to 
structure below. In such a case G_EXTCROP could be used to obtain 
cropping bounds.

>> 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;
>> 	struct v4l2_rect r;
>> 	u32 flags;
>> 	u32 reserved[10];
>> };
>>
>> Where,
>> type	 - type of buffer queue: V4L2_BUF_TYPE_VIDEO_CAPTURE,
>>            V4L2_BUF_TYPE_VIDEO_OUTPUT, etc.
>> 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, 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
>> 	- 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, 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, 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
>> 	- 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, 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. 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
> 
> Wouldn't you also need similar flags for RIGHT and BOTTOM?
> 
> Regards,
> 
> 	Hans
> 
Proposed flags refer to fields in v4l2_rect structure. These are left, 
top, width and height. Fields bottom and right and not present in 
v4l2_rect structure.

Best regards,
Tomasz Stanislawski

>> #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_FIXED		0x000000ff
>>
>> 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_FIXED | V4L2_SEL_LEFT_FIXED |
>>   V4L2_SEL_WIDTH_GE | V4L2_SEL_HEIGHT_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.
>>
>> 4. Possible improvements and extensions.
>> - combine composing and cropping ioctl into a single ioctl
>> - 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
>>
>>
>> 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          |  102 +++++++++++++++++++++++++++--
>>  include/linux/videodev2.h                 |   30 +++++++++
>>  include/media/v4l2-ioctl.h                |    8 ++
>>  4 files changed, 137 insertions(+), 7 deletions(-)
>>
>>

Re: [PATCH 0/2] V4L: Extended crop/compose API

[Hans Verkuil]

On Monday, March 28, 2011 17:19:54 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.
> 
> 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.

I don't have time right now to review this RFC in-depth, but one thing that
needs more attention is the relationship between these new ioctls and CROPCAP.

And also how this relates to analog inputs (I don't think analog outputs make
any sense). And would a COMPOSECAP ioctl make sense?

> 
> 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;
> 	struct v4l2_rect r;
> 	u32 flags;
> 	u32 reserved[10];
> };
> 
> Where,
> type	 - type of buffer queue: V4L2_BUF_TYPE_VIDEO_CAPTURE,
>            V4L2_BUF_TYPE_VIDEO_OUTPUT, etc.
> 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, 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
> 	- 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, 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, 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
> 	- 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, 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. 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

Wouldn't you also need similar flags for RIGHT and BOTTOM?

Regards,

	Hans

> 
> #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_FIXED		0x000000ff
> 
> 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_FIXED | V4L2_SEL_LEFT_FIXED |
>   V4L2_SEL_WIDTH_GE | V4L2_SEL_HEIGHT_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.
> 
> 4. Possible improvements and extensions.
> - combine composing and cropping ioctl into a single ioctl
> - 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
> 
> 
> 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          |  102 +++++++++++++++++++++++++++--
>  include/linux/videodev2.h                 |   30 +++++++++
>  include/media/v4l2-ioctl.h                |    8 ++
>  4 files changed, 137 insertions(+), 7 deletions(-)
> 
> 

[PATCH 0/2] V4L: Extended crop/compose API

[Tomasz Stanislawski]

Hello everyone,

This patch-set introduces new ioctls to V4L2 API. The new method for
configuration of cropping and composition is presented.

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;
	struct v4l2_rect r;
	u32 flags;
	u32 reserved[10];
};

Where,
type	 - type of buffer queue: V4L2_BUF_TYPE_VIDEO_CAPTURE,
           V4L2_BUF_TYPE_VIDEO_OUTPUT, etc.
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, 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
	- 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, 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, 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
	- 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, 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. 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_WIDTH_FIXED	0x00000003
#define V4L2_SEL_HEIGHT_FIXED	0x0000000c
#define V4L2_SEL_LEFT_FIXED	0x00000030
#define V4L2_SEL_TOP_FIXED	0x000000c0

#define V4L2_SEL_FIXED		0x000000ff

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_FIXED | V4L2_SEL_LEFT_FIXED |
  V4L2_SEL_WIDTH_GE | V4L2_SEL_HEIGHT_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.

4. Possible improvements and extensions.
- combine composing and cropping ioctl into a single ioctl
- 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


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          |  102 +++++++++++++++++++++++++++--
 include/linux/videodev2.h                 |   30 +++++++++
 include/media/v4l2-ioctl.h                |    8 ++
 4 files changed, 137 insertions(+), 7 deletions(-)

-- 
1.7.4.1