Hi Sakari, On Sunday 26 February 2012 23:42:19 Sakari Ailus wrote: > Laurent Pinchart wrote: > > On Monday 20 February 2012 03:56:48 Sakari Ailus wrote: > >> Add documentation for V4L2 subdev selection API. This changes also > >> experimental V4L2 subdev API so that scaling now works through selection > >> API only. > >> > >> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx> > >> > >> > >> diff --git a/Documentation/DocBook/media/v4l/dev-subdev.xml > >> b/Documentation/DocBook/media/v4l/dev-subdev.xml index 0916a73..9d5e7da > >> 100644 > >> --- a/Documentation/DocBook/media/v4l/dev-subdev.xml > >> +++ b/Documentation/DocBook/media/v4l/dev-subdev.xml > > > > [snip] > > > >> + <para>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 > >> + <constant>V4L2_SUBDEV_SEL_COMPOSE_ACTIVE</constant> 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.</para> > > > > I'm not sure if that would be very clear for readers who are not yet > > familiar with the API. What about the following text instead ? > > > > "The scaling operation changes the size of the image by scaling it to new > > dimensions. The scaling ratio isn't specified explicitly, but is implied > > from the original and scaled image sizes. Both sizes are represented by > > &v4l2- rect;. > > > > Scaling support is optional. When supported by a subdev, the crop > > rectangle on the subdev's sink pad is scaled to the size configured using > > &VIDIOC-SUBDEV-G- SELECTION; and > > <constant>V4L2_SUBDEV_SEL_COMPOSE_ACTIVE</constant> selection target on > > the same pad. If the subdev supports scaling but no composing, the top > > and left values are not used and must always be set to zero." > > > > (note that &sub-subdev-g-selection; has been replaced with > > &VIDIOC-SUBDEV-G- SELECTION;) > > > > I would also move this text after the sink pad crop description to follow > > the order in which operations are applied by subdevs. > > I'm fine with that change, so I did it. However, I won't replace > &sub-subdev-g-selection; with &VIDIOC-SUBDEV-G-SELECTION; simply because > it won't work: > > dev-subdev.xml:310: parser error : Entity 'VIDIOC-SUBDEV-G-SELECTION' > not defined > size configured using &VIDIOC-SUBDEV-G-SELECTION; and > > It's beyond me why not; similar references are being used elsewhere with > otherwise equivalent definitions. Perhaps the name is just too long? > That's the only difference I could think of: xmlto typically segfaults > on errors so I wouldn't be surprised of something so simple. Don't give up so fast :-) diff --git a/Documentation/DocBook/media/Makefile b/Documentation/DocBook/media/Makefile index 729f840..77ead85 100644 --- a/Documentation/DocBook/media/Makefile +++ b/Documentation/DocBook/media/Makefile @@ -65,6 +65,8 @@ IOCTLS = \ $(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' $(srctree)/include/linux/dvb/video.h) \ $(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' $(srctree)/include/linux/media.h) \ $(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' $(srctree)/include/linux/v4l2-subdev.h) \ + VIDIOC_SUBDEV_G_SELECTION \ + VIDIOC_SUBDEV_S_SELECTION \ VIDIOC_SUBDEV_G_FRAME_INTERVAL \ VIDIOC_SUBDEV_S_FRAME_INTERVAL \ VIDIOC_SUBDEV_ENUM_MBUS_CODE \ and rm Documentation/DocBook/media-entities.tmpl before you compile the documentation. > >> + <para>As for pad formats, drivers store try and active > >> + rectangles for the selection targets of ACTIVE type <xref > >> + linkend="v4l2-subdev-selection-targets">.</xref></para> > >> + > >> + <para>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.</para> > >> + > >> + <para>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.</para> > >> + > >> + <para>The drivers should always use the closest possible > >> + rectangle the user requests on all selection targets, unless > >> + specificly told otherwise<xref > >> + linkend="v4l2-subdev-selection-flags">.</xref></para> > >> + </section> > >> + > >> + <section> > >> + <title>Types of selection targets</title> > >> + > >> + <section> > >> + <title>ACTIVE targets</title> > >> + > >> + <para>ACTIVE targets reflect the actual hardware configuration > >> + at any point of time.</para> > >> + </section> > >> + > >> + <section> > >> + <title>BOUNDS targets</title> > >> + > >> + <para>BOUNDS targets is the smallest rectangle within which > >> + contains all valid ACTIVE rectangles. > > > > s/within which/that/ ? > > Ack. > > >> It may not be possible > >> + to set the ACTIVE rectangle as large as the BOUNDS rectangle, > >> + however.</para> > > > > What about > > > > "The BOUNDS rectangle might not itself be a valid ACTIVE rectangle when > > all possible ACTIVE pixels do not form a rectangular shape (e.g. > > cross-shaped or round sensors)." > > There are cases where the active size is limited, even if it's > rectangular. I can add the above case there, sure, if you think such > devices exist --- I've never heard of nor seen them. Some sensors are > documented to be cross-shaped but the only thing separating these from > the rest is that the manufacturer doesn't guarantee the quality of the > pixels in the corners. At least on those I've seen. You can still > capture the full pixel matrix. Those are just examples. I think it's useful to add them, otherwise the reader won't know why and under what kind of circumstances the ACTIVE rectangle can't be as large at the BOUNDS rectangle. > >> + </section> > >> > >> - <para>Cropping behaviour on output pads is not defined.</para> > >> + </section> > >> + > >> + <section> > >> + <title>Order of configuration and format propagation</title> > >> + > >> + <para>Inside subdevs, 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 changes made will be propagated to > >> + any subsequent stages. If this behaviour is not desired, the > >> + user must set > >> + <constant>V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG</constant> flag. > > > > Could you explain what happens when V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG is > > set ? Just stating that it doesn't follow the propagation behaviour > > previously described could be understood in many different ways. > > Good point. How about this: > > "This flag causes that no propagation of the changes are allowed in any > circumstances. This may also lead the accessed rectangle not being > changed at all, "The accessed rectangle will likely be adjusted by the driver," > depending on the properties of the underlying hardware. > Some drivers may not support this flag." What should happen then ? Should the flag be ignored, or should the driver return an error ? > >> The > >> + coordinates to a step always refer to the active size of the > >> + previous step. The exception to this rule is the source compose > >> + rectangle, which refers to the sink compose bounds rectangle --- > >> + if it is supported by the hardware.</para> > >> + > >> + <orderedlist> > >> + <listitem>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.</listitem> > >> + > >> + <listitem>Sink pad active crop selection. The sink pad crop > >> + defines the performed to the sink pad format.</listitem> > > > > s/defines the/defines the cropping/ ? > > Fixed. > > >> + > >> + <listitem>Sink pad active compose selection. The size of the > >> + sink pad compose rectangle defines the scaling ratio compared > >> + to the size of the sink pad crop rectangle. The location of > >> + the compose rectangle specifies the location of the active > >> + sink compose rectangle in the sink compose bounds > >> + rectangle.</listitem> > >> + > >> + <listitem>Source pad active crop selection. Crop on the source > >> + pad defines crop performed to the image in the sink compose > >> + bounds rectangle.</listitem> > >> + > >> + <listitem>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. > >> + Width and height are defined by the size of the source pad > >> + active crop selection.</listitem> > >> + </orderedlist> > >> + > >> + <para>Accessing any of the above rectangles not supported by the > >> + subdev will return <constant>EINVAL</constant>. > > > > Do drivers have to support BOUNDS rectangles for every ACTIVE rectangle > > they support (and the other way around) ? If so, I think it should be > > specified. > I think the answer to that should be "yes", albeit this hasn't been > discussed previously. I see no reason not to support equivalent BOUNDS > rectangles. > > I'll change the documentation to state that. > > > Is EINVAL returned for any other error case in the selection API ? If so, > > it might make enumeration of the supported rectangles difficult. > > Good point. There are two other reasons to return EINVAL and those are > invalid parameters in either pad or which fields. I think that should be > fine. I'd keep it EINVAL. > > Alternatively we could use e.g. ENOENT. > > What do you think? I'm OK with EINVAL then, as applications shouldn't pass an invalid pad or which value in the first place. > >> Any rectangle > >> + referring to a previous unsupported rectangle coordinates will > >> + instead refer to the previous supported rectangle. For example, > >> + if sink crop is not supported, the compose selection will refer > >> + to the sink pad format dimensions instead.</para> > > > > Should we add a list of the rectangles a subdev must/should/can support > > for the different possible use cases ? > > I think this should be rather obvious with the examples and the > statement on BOUNDS rectangles. Such a list, if we make it very > detailed, will be unnecessarily redundant and will soon become > out-of-date. In principle I don't like redundancy whether it's code or > documentation. It easily leads to contradictions. > > If it turns out it'll be needed we can easily add it later on. OK. [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..033077a > >> --- /dev/null > >> +++ b/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml [snip] > >> + <table pgwide="1" frame="none" id="v4l2-subdev-selection-flags"> > >> + <title>V4L2 subdev selection flags</title> > >> + <tgroup cols="3"> > >> + &cs-def; > >> + <tbody valign="top"> > >> + <row> > >> + <entry><constant>V4L2_SUBDEV_SEL_FLAG_SIZE_GE</constant></entry> > >> + <entry>(1 << 0)</entry> > >> + <entry>Suggest the driver it should choose greater or > >> + equal rectangle (in size) than was requested.</entry> > >> + </row> > >> + <row> > >> + <entry><constant>V4L2_SUBDEV_SEL_FLAG_SIZE_LE</constant></entry> > >> + <entry>(1 << 1)</entry> > >> + <entry>Suggest the driver it should choose lesser or > >> + equal rectangle (in size) than was requested.</entry> > >> + </row> > > > > Those two flags are only briefly described here, could you add a more > > detailed description either here or in > > Documentation/DocBook/media/v4l/dev-subdev.xml ? > Added: > > " Albeit the driver may choose a lesser size, > it will only do so due to hardware limitations. Without > this flag (and > <constant>V4L2_SUBDEV_SEL_FLAG_SIZE_LE</constant>) the > behaviour is to choose the closest possible > rectangle." > > Does that cover your understanding of a more detailed description? :-) I was thinking about something in media/v4l/dev-subdev.xml that would explain what the flags are used for. media/v4l/vidioc-subdev-g-selection.xml is more like reference documentation. -- Regards, Laurent Pinchart -- 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
