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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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(-)
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux