From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp.nokia.com ([147.243.1.48]:35265 "EHLO mgw-sa02.nokia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755796Ab2AISQZ (ORCPT ); Mon, 9 Jan 2012 13:16:25 -0500 Message-ID: <4F0B2EF0.5080203@maxwell.research.nokia.com> Date: Mon, 09 Jan 2012 20:16:16 +0200 From: Sakari Ailus MIME-Version: 1.0 To: Laurent Pinchart CC: linux-media@vger.kernel.org, dacohen@gmail.com, snjw23@gmail.com, Tomasz Stanislawski Subject: Re: [RFC 06/17] v4l: Add selections documentation. References: <4EF0EFC9.6080501@maxwell.research.nokia.com> <1324412889-17961-6-git-send-email-sakari.ailus@maxwell.research.nokia.com> <201201061243.56158.laurent.pinchart@ideasonboard.com> In-Reply-To: <201201061243.56158.laurent.pinchart@ideasonboard.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: Hi Laurent, Thanks for the comments! Laurent Pinchart wrote: > On Tuesday 20 December 2011 21:27:58 Sakari Ailus wrote: > > [snip] > >> diff --git a/Documentation/DocBook/media/v4l/dev-subdev.xml >> b/Documentation/DocBook/media/v4l/dev-subdev.xml index 0916a73..722db60 >> 100644 >> --- a/Documentation/DocBook/media/v4l/dev-subdev.xml >> +++ b/Documentation/DocBook/media/v4l/dev-subdev.xml > > [snip] > >> @@ -288,26 +288,81 @@ > > [snip] > >> + Scaling operation changes the size of the image by scaling >> + it to new dimensions. Some sub-devices support it. The scaled >> + size (width and height) is represented by &v4l2-rect;. In the >> + case of scaling, top and left will always be zero. Scaling is >> + configured using &sub-subdev-g-selection; and >> + V4L2_SUBDEV_SEL_COMPOSE_ACTIVE selection >> + target on the sink pad of the subdev. The scaling is performed >> + related to the width and height of the crop rectangle on the >> + subdev's sink pad. >> + >> + As for pad formats, drivers store try and active >> + rectangles for the selection targets of ACTIVE type > + linkend="v4l2-subdev-selection-targets">. >> + >> + On sink pads, cropping is applied relatively to the >> + current pad format. The pad format represents the image size as >> + received by the sub-device from the previous block in the >> + pipeline, and the crop rectangle represents the sub-image that >> + will be transmitted further inside the sub-device for >> + processing. >> + >> + On source pads, cropping is similar to sink pads, with the >> + exception that the source size from which the cropping is >> + performed, is the COMPOSE rectangle on the sink pad. In both >> + sink and source pads, the crop rectangle must be entirely >> + containted inside the source image size for the crop >> + operation. >> + >> + The drivers should always use the closest possible >> + rectangle the user requests on all selection targets, unless >> + specificly told otherwise> + linkend="v4l2-subdev-selection-flags">. >> + > > This sounds a bit confusing to me. One issue is that composing is not formally > defined. I think it would help if you could draw a diagram that shows how the > operations are applied, and modify the text to describe the diagram, using the > natural order of the compose and crop operations on sink and source pads. I drew a diagram based on your suggestion, but I'd prefer the formal definition would come from someone who needs composition and better understands the use cases. Also cc Tomasz. >> +
>> + Order of configuration and format propagation >> + >> + The order of image processing steps will always be from >> + the sink pad towards the source pad. This is also reflected in >> + the order in which the configuration must be performed by the >> + user. The format is propagated within the subdev along the later >> + processing steps. For example, setting the sink pad format >> + causes all the selection rectangles and the source pad format to >> + be set to sink pad format --- if allowed by the hardware, and if >> + not, then closest possible. The coordinates to a step always >> + refer to the active size of the previous step. > > This also sounds a bit ambiguous if I try to ignore the fact that I know how > it works :-) You should at least make it explicit that propagation inside > subdevs is performed by the driver(s), and that propagation outside subdevs is > to be handled by userspace. Agreed. I'll reword it. >> + >> + Sink pad format. The user configures the sink pad >> + format. This format defines the parameters of the image the >> + entity receives through the pad for further processing. >> >> - Cropping behaviour on output pads is not defined. >> + Sink pad active crop selection. The sink pad crop >> + defines the performed to the sink pad format. >> >> + Sink pad active compose selection. The sink pad compose >> + rectangle defines the scaling ratio compared to the size of >> + the sink pad crop rectangle. >> + >> + Source pad active crop selection. Crop on the source >> + pad defines crop performed to the image scaled according to >> + the sink pad compose rectangle. >> + >> + Source pad active compose selection. The source pad >> + compose defines the size and location of the compose >> + rectangle. >> + >> + Source pad format. The source pad format defines the >> + output pixel format of the subdev, as well as the other >> + parameters with the exception of the image width and >> + height. >> + >> + >>
>> + >> >> >> &sub-subdev-formats; > > [snip] > >> diff --git a/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml >> b/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml new file >> mode 100644 >> index 0000000..5fbcd65 >> --- /dev/null >> +++ b/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml >> @@ -0,0 +1,226 @@ > > [snip] > >> + >> + Description >> + >> + >> + Experimental >> + This is an experimental >> + interface and may change in the future. >> + >> + >> + The selections are used to configure various image >> + processing functionality performed by the subdevs which affect the >> + image size. This currently includes cropping, scaling and >> + composition. >> + >> + The selections replace the crop API &sub-subdev-g-crop;. All >> + the function of the crop API, and more, are supported by the >> + selections API. >> + >> + See Sub-device interface for >> + more information on how each selection target affects the image >> + processing pipeline inside the subdevice. >> + >> +
>> + Types of selection targets >> + >> + The are four types of selection targets: active, default, >> + bounds and padding. The ACTIVE targets are the targets which >> + configure the hardware. The DEFAULT target provides the default >> + for the ACTIVE selection. The BOUNDS target will return the >> + maximum width and height of the target. > > What about the minimum ? Good question. We could also specify that the minimum is obtained by using the V4L2_SUBDEV_SEL_FLAG_LE flag with the BOUNDS target. >> The PADDED target >> + provides the width and height for the padded image, > > Is it valid for both crop and compose rectangles ? I think all targets are valid for all rectangles. Should I mention that? The practical use cases may be more limited, though. I wonder if I should remove the padded targets until we get use cases for them. I included them for the reason that they also exist in the V4L2. Tomasz, Sylwester: do you have use for the PADDED targets? I think we also must define what will be done in cases where crop (on either sink or source) / scaling / composition is not supported by the subdev. That's currently undefined. I think it'd be most clear to return an error code but I'm not sure which one --- EINVAL is an obvious candidate but that is also returned when the pad is wrong. It looks still like the best choice to me. >> and is >> + directly affected by the ACTIVE target. The PADDED targets may >> + be configurable depending on the hardware. > > If that's configurable drivers will need a way to store it in the file handle. Good point. I'll add it if we end up defining the padded targets now. >> +
>> + >> + >> + V4L2 subdev selection targets >> + >> + &cs-def; >> + >> + >> + V4L2_SUBDEV_SEL_TGT_CROP_ACTIVE >> + 0 >> + Active crop. Defines the cropping >> + performed by the processing step. >> + >> + >> + V4L2_SUBDEV_SEL_TGT_CROP_DEFAULT >> + 1 >> + Default crop rectangle. >> + >> + >> + V4L2_SUBDEV_SEL_TGT_CROP_BOUNDS >> + 2 >> + Bounds of the crop rectangle. >> + >> + >> + >> V4L2_SUBDEV_SEL_TGT_COMPOSE_ACTIVE + >> 256 >> + Active compose rectangle. Used to configure scaling >> + on sink pads and composition on source pads. >> + >> + >> + >> V4L2_SUBDEV_SEL_TGT_COMPOSE_DEFAULT + >> 257 >> + Default compose rectangle. >> + >> + >> + >> V4L2_SUBDEV_SEL_TGT_COMPOSE_BOUNDS + >> 258 >> + Bounds of the compose rectangle. >> + >> + >> + >> +
>> + >> + >> + V4L2 subdev selection flags >> + >> + &cs-def; >> + >> + >> + V4L2_SUBDEV_SEL_FLAG_SIZE_GE >> + (1 << 0) >> + Suggest the driver it should choose greater or >> + equal rectangle (in size) than was requested. >> + >> + >> + V4L2_SUBDEV_SEL_FLAG_SIZE_LE >> + (1 << 1) >> + Suggest the driver it should choose lesser or >> + equal rectangle (in size) than was requested. >> + >> + >> + V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG >> + (1 << 2) >> + The configuration should not be propagated to any >> + further processing steps. If this flag is not given, the >> + configuration is propagated inside the subdevice to all >> + further processing steps. >> + >> + >> + >> +
>> + >> + >> + struct <structname>v4l2_subdev_selection</structname> >> + >> + &cs-str; >> + >> + >> + __u32 >> + which >> + Active or try selection, from >> + &v4l2-subdev-format-whence;. >> + >> + >> + __u32 >> + pad >> + Pad number as reported by the media framework. >> + >> + >> + __u32 >> + target >> + Target selection rectangle. See >> + .. >> + >> + >> + __u32 >> + flags >> + Flags. See >> + . >> + >> + >> + &v4l2-rect; >> + rect >> + Crop rectangle boundaries, in pixels. >> + >> + >> + __u32 >> + reserved[8] >> + Reserved for future extensions. Applications and drivers must >> + set the array to zero. >> + >> + >> + >> +
>> + >> + >> + >> + >> + &return-value; >> + >> + >> + >> + EBUSY >> + >> + The selection rectangle can't be changed because the >> + pad is currently busy. This can be caused, for instance, by >> + an active video stream on the pad. The ioctl must not be >> + retried without performing another action to fix the problem >> + first. Only returned by >> + VIDIOC_SUBDEV_S_SELECTION >> + >> + >> + >> + EINVAL >> + >> + The &v4l2-subdev-selection; >> + pad references a non-existing >> + pad, the which field references a >> + non-existing format, or the selection target is not >> + supported on the given subdev pad. >> + >> + >> + >> + >> + > -- Sakari Ailus sakari.ailus@maxwell.research.nokia.com