All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] SUBDEV_S/G_SELECTION IOCTLs
@ 2011-11-08 21:55 Sakari Ailus
  2011-11-10 11:57 ` Sylwester Nawrocki
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Sakari Ailus @ 2011-11-08 21:55 UTC (permalink / raw)
  To: linux-media
  Cc: laurent.pinchart, t.stanislaws, sylvester.nawrocki,
	g.liakhovetski, hverkuil, dacohen, andriy.shevchenko

Hi all,

This RFC discusses the SUBDEV_S_SELECTION/SUBDEV_G_SELECTION API which is
intended to amend and replace the existing SUBDEV_[SG]_CROP API. These
IOCTLs have previously been discussed in the Cambridge V4L2 brainstorming
meeting [0] and their intent is to provide more configurability for subdevs,
including cropping on the source pads and composing for a display.

The S_SELECTION patches for V4L2 nodes are available here [1] and the
existing documentation for the V4L2 subdev pad format configuration can be
found in [2].

SUBDEV_[SG]_SELECTION is intended to fully replace SUBDEV_[SG]_CROP in
drivers as the latter can be implemented in SUBDEV_[SG]_SELECTION using
active CROP target on sink pads. That can be done in v4l2-ioctl.c so drivers
only will need to implement SUBDEV_[SG]_SELECTION.


Questions, comments and thoughts are welcome, especially regarding new use
cases.


Order of configuration
======================

The proposed order of the subdev configuration is as follows. Individual
steps may be omitted since any of the steps will reset the rectangles /
sizes for any following step.

1. SUBDEV_S_FMT on the SINK pad. The user will issue SUBDEV_S_FMT to set the
subdev sink pad image size and media bus format code and other parameters in
v4l2_mbus_framefmt as necessary.

2. SUBDEV_S_SELECTION with CROP target on the SINK pad. The crop rectangle
is set related to the image size given in step 1).

3. SUBDEV_S_SELECTION with COMPOSE target on the SINK pad. The size of the
compose rectangle, if it differs from the size of the rectangle given in 2),
signifies user's wish to perform scaling.

4. SUBDEV_S_SELECTION with CROP target on the SOURCE pad. Configure cropping
performed by the subdev after scaling.

5. SUBDEV_S_SELECTION with COMPOSE target on the SOURCE pad. This configures
composition on the display if relevant for the subdevice. (In this case the
COMPOSE bounds will yield to the size of the display.)

6. SUBDEV_S_FMT on the SOURCE pad. The size of the image is defined by
setting CROP on the SOURCE pad, so SUBDEV_S_FMT only has an effect of
changing other parameters than size.

As defined in [2], when performing any of the configuration phases above,
the formats and selections are reset to defaults from each phase onwards.
For example, SUBDEV_S_SELECTION with CROP target on the SINK pad will
--- beyond its obvious function of setting CROP selection target on the SINK
pad --- reset the COMPOSE selection target on SINK pad, as well as the CROP
selection target and format on the SOURCE pad.


Definitions
===========

/**
 * struct v4l2_subdev_selection - selection info
 *
 * @which: either V4L2_SUBDEV_FORMAT_ACTIVE or V4L2_SUBDEV_FORMAT_TRY
 * @pad: pad number, as reported by the media API
 * @target: selection target, used to choose one of possible rectangles
 * @flags: constraints flags
 * @r: coordinates of selection window
 * @reserved: for future use, rounds structure size to 64 bytes, set to zero
 *
 * Hardware may use multiple helper window to process a video stream.
 * The structure is used to exchange this selection areas between
 * an application and a driver.
 */
struct v4l2_subdev_selection {
	__u32 which;
	__u32 pad;
	__u32 target;
	__u32 flags;
	struct v4l2_rect r;
	__u32 reserved[8];
};

Both SUBDEV_S_SELECTION and SUBDEV_G_SELECTION would take struct
v4l2_subdev_selection as the IOCTL argument (RW).

The same target definitions and flags apply as in [1], with possible
exception of the PADDED targets --- see below. The flags will gain _SUBDEV
prefix after the existing V4L2 prefix.


Sample use cases
================


OMAP 3 ISP preview
------------------

The OMAP 3 ISP preview block provides cropping on preview sink pad, but also
horizontal averaging. The horizontal averaging may scale the image
horizontally by factor 1/n, where n is either 1, 2, 4 or 8.

The preview block also performs pixel format conversion from raw bayer to
YUV. Other image processing operations crops maximum of 12 columns and 8 rows
before the format conversion. After the format conversion, further 2 columns
are cropped.

To make this easy for the user, the driver assumes that all the features
affecting cropping are enabled at all times so the size stays constant for
the user.


This example only includes the preview subdev since it can be shown
independently. In the example, the preview subdev is configured to crop the
image by 100 pixels on all sides of the image. The horizontal averaging is
configured by factor 1/2, which translates 100 pixels from each side of the
original image. The preview sink pad format is 800x600 SGRBG10.


[crop] 0:preview:1 [crop (static), compose]

	The initial state of the pipeline is:

	preview:0	preview:1
compose (0,0)/788x592	(0,0)/788x592
crop	(7,4)/788x592	(1,0)/786x592
fmt	800x600/SGRBG10	786x592/YUYV

	This is due to hardware imposed cropping performed in the sink pad
	as well as on the source pad.

	To crop 100 pixels on all sides:

	SUBDEV_S_SELECTION(preview:0, CROP_ACTIVE, (99,100)/602x400);

	The hardware mandated crop on the sink pad is thus one pixel on left
	and right sides of the image. (This would also be shown in the
	CROP_BOUNDS target.)

	preview:0	preview:1
compose (0,0)/602x400	(0,0)/602x400
crop	(99,100)/602x400 (1,0)/600x400
fmt	800x600/SGRBG10	600x400/YUYV


A sensor
--------

The intent is to obtain a VGA image from a 8 MP sensor which provides
following pipeline:

pixel_array:0 [crop] ---> 0:binner:1 ---> [crop] 0:scaler:1 [crop]

Binner is an entity which can perform scaling, but only in factor of 1/n,
where n is a positive integer. No cropping is needed. The intent is to get a
640x480 image from such sensor. (This doesn't involve any other
configuration as the image size related one.)

	The initial state of the pipeline

	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
compose (0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464
crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464
fmt	3600x2464	3600x2464	3600x2464	3600x2464	3600x2464

	This will configure the binning on the binner subdev sink pad:

	SUBDEV_S_SELECTION(binner:0, COMPOSE_ACTIVE, (0,0)/1800x1232);

	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
compose (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/3600x2464	(0,0)/3600x2464
crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/3600x2464	(0,0)/3600x2464
fmt	3600x2464	3600x2464	1800x1232	3600x2464	3600x2464

	The same format must be set on the scaler pad 0 as well. This will
	reset the size inside the scaler to a sane default, which is no
	scaling:

	SUBDEV_S_FMT(scaler:0, 1800x1232);

	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
compose (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232
crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232
fmt	3600x2464	3600x2464	1800x1232	1800x1232	1800x1232

	To perform further scaling on the scaler, the COMPOSE target is used
	on the scaler subdev's SOURCE pad:

	SUBDEV_S_SELECTION(scaler:0, COMPOSE_ACTIVE, (0,0)/640x480);

	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
compose (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/640x480	(0,0)/640x480
crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/640x480
fmt	3600x2464	3600x2464	1800x1232	1800x1232	640x480


The result is a 640x480 image from the scaler's output pad. The aspect ratio
of the resulting image is different from 4/3 since no cropping was
performed in this example.


Applications which do not recognise SUBDEV_S_SELECTION 
======================================================

The current spec [2] tells that the scaling factor is defined by using
SUBDEV_S_FMT in the source pad. This method would not be supported in the
future, possibly affecting applications which use SUBDEV_S_FMT to configure
the scaling factor.

If supporting this is seen necessary, it can be implemented by e.g.
reverting to the old behaviour if the SOURCE crop rectangle width or height
is different from width and height specified in SOURCE S_FMT.


Open questions
==============

1. Keep subdev configuration flag. In Cambourne meeting the case of the OMAP
3 ISP resizer configuration dilemma was discussed, and the proposal was to
add a flag to disable propagating the configuration inside a single subdev.
Propagating inside a single subdev is the default. Where do we need this
flag; is just SUBDEV_S_SELECTION enough? [0]

2. Are PADDED targets relevant for media bus formats? [3]


References
==========

[0] http://www.mail-archive.com/linux-media@vger.kernel.org/msg35361.html

[1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36206.html

[2] http://hverkuil.home.xs4all.nl/spec/media.html#subdev

[3] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36203.html


-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] SUBDEV_S/G_SELECTION IOCTLs
  2011-11-08 21:55 [RFC] SUBDEV_S/G_SELECTION IOCTLs Sakari Ailus
@ 2011-11-10 11:57 ` Sylwester Nawrocki
  2011-11-10 21:01   ` Sakari Ailus
  2011-11-10 15:23 ` Tomasz Stanislawski
  2011-11-11 14:03 ` HeungJun Kim
  2 siblings, 1 reply; 9+ messages in thread
From: Sylwester Nawrocki @ 2011-11-10 11:57 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, laurent.pinchart, t.stanislaws, g.liakhovetski,
	hverkuil, dacohen, andriy.shevchenko

Hi Sakari,

On 11/08/2011 10:55 PM, Sakari Ailus wrote:
> Hi all,
> 
> This RFC discusses the SUBDEV_S_SELECTION/SUBDEV_G_SELECTION API which is
> intended to amend and replace the existing SUBDEV_[SG]_CROP API. These
> IOCTLs have previously been discussed in the Cambridge V4L2 brainstorming
> meeting [0] and their intent is to provide more configurability for subdevs,
> including cropping on the source pads and composing for a display.
> 
> The S_SELECTION patches for V4L2 nodes are available here [1] and the
> existing documentation for the V4L2 subdev pad format configuration can be
> found in [2].
> 
> SUBDEV_[SG]_SELECTION is intended to fully replace SUBDEV_[SG]_CROP in
> drivers as the latter can be implemented in SUBDEV_[SG]_SELECTION using
> active CROP target on sink pads. That can be done in v4l2-ioctl.c so drivers
> only will need to implement SUBDEV_[SG]_SELECTION.
> 
> 
> Questions, comments and thoughts are welcome, especially regarding new use
> cases.
> 
> 
> Order of configuration
> ======================
> 
> The proposed order of the subdev configuration is as follows. Individual
> steps may be omitted since any of the steps will reset the rectangles /
> sizes for any following step.
> 
> 1. SUBDEV_S_FMT on the SINK pad. The user will issue SUBDEV_S_FMT to set the
> subdev sink pad image size and media bus format code and other parameters in
> v4l2_mbus_framefmt as necessary.
> 
> 2. SUBDEV_S_SELECTION with CROP target on the SINK pad. The crop rectangle
> is set related to the image size given in step 1).
> 
> 3. SUBDEV_S_SELECTION with COMPOSE target on the SINK pad. The size of the
> compose rectangle, if it differs from the size of the rectangle given in 2),
> signifies user's wish to perform scaling.
> 
> 4. SUBDEV_S_SELECTION with CROP target on the SOURCE pad. Configure cropping
> performed by the subdev after scaling.
> 
> 5. SUBDEV_S_SELECTION with COMPOSE target on the SOURCE pad. This configures
> composition on the display if relevant for the subdevice. (In this case the
> COMPOSE bounds will yield to the size of the display.)
> 
> 6. SUBDEV_S_FMT on the SOURCE pad. The size of the image is defined by
> setting CROP on the SOURCE pad, so SUBDEV_S_FMT only has an effect of
> changing other parameters than size.
> 
> As defined in [2], when performing any of the configuration phases above,
> the formats and selections are reset to defaults from each phase onwards.
> For example, SUBDEV_S_SELECTION with CROP target on the SINK pad will
> --- beyond its obvious function of setting CROP selection target on the SINK
> pad --- reset the COMPOSE selection target on SINK pad, as well as the CROP
> selection target and format on the SOURCE pad.

I thought we agreed that the spec will not be enforcing resetting parameters
to defaults from each phase onwards. Also I couldn't find anything explicitly
telling this in [2]. Let's consider simple use case: video pipeline with image
sensor, scaler, composer and DMA engine.
Initially user performs the configuration in the above described order.
Then video stream is started and user wants to change the area cropped at
image sensor, which will then appear in the configured compose window. We assume
the H/W supports changing crop window position during streaming.

We can't reset existing configuration below/after CROP phase at the scaler sink
pad, because we want the compose window unchanged. 

I guess this is where we need the flag to disable propagating the configuration
inside single subdev, don't we ?

[snip]

> Open questions
> ==============
> 
> 1. Keep subdev configuration flag. In Cambourne meeting the case of the OMAP
> 3 ISP resizer configuration dilemma was discussed, and the proposal was to
> add a flag to disable propagating the configuration inside a single subdev.
> Propagating inside a single subdev is the default. Where do we need this
> flag; is just SUBDEV_S_SELECTION enough? [0]
> 
> 2. Are PADDED targets relevant for media bus formats? [3]
> 
> 
> References
> ==========
> 
> [0] http://www.mail-archive.com/linux-media@vger.kernel.org/msg35361.html
> 
> [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36206.html
> 
> [2] http://hverkuil.home.xs4all.nl/spec/media.html#subdev
> 
> [3] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36203.html

---
Regards,
Sylwester

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] SUBDEV_S/G_SELECTION IOCTLs
  2011-11-08 21:55 [RFC] SUBDEV_S/G_SELECTION IOCTLs Sakari Ailus
  2011-11-10 11:57 ` Sylwester Nawrocki
@ 2011-11-10 15:23 ` Tomasz Stanislawski
  2011-11-10 22:29   ` Sakari Ailus
  2011-11-11 14:03 ` HeungJun Kim
  2 siblings, 1 reply; 9+ messages in thread
From: Tomasz Stanislawski @ 2011-11-10 15:23 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, laurent.pinchart, sylvester.nawrocki,
	g.liakhovetski, hverkuil, dacohen, andriy.shevchenko

Hi Sakari,


On 11/08/2011 10:55 PM, Sakari Ailus wrote:
> Hi all,
>
> This RFC discusses the SUBDEV_S_SELECTION/SUBDEV_G_SELECTION API which is
> intended to amend and replace the existing SUBDEV_[SG]_CROP API. These
> IOCTLs have previously been discussed in the Cambridge V4L2 brainstorming
> meeting [0] and their intent is to provide more configurability for subdevs,
> including cropping on the source pads and composing for a display.
>
> The S_SELECTION patches for V4L2 nodes are available here [1] and the
> existing documentation for the V4L2 subdev pad format configuration can be
> found in [2].
>
> SUBDEV_[SG]_SELECTION is intended to fully replace SUBDEV_[SG]_CROP in
> drivers as the latter can be implemented in SUBDEV_[SG]_SELECTION using
> active CROP target on sink pads. That can be done in v4l2-ioctl.c so drivers
> only will need to implement SUBDEV_[SG]_SELECTION.
>
>
> Questions, comments and thoughts are welcome, especially regarding new use
> cases.
>
>
> Order of configuration
> ======================

Sorry, what is exactly the SOURCE pad and SINK pad? The spec says "Data 
flows from a source pad to a sink pad.". Does in refer to a subdev or to 
a link? Look at the image below:

data ---- link0 --> (pad0) [subdev] (pad1) ----> link1

 From subdev's point of view, pad0 is data source, pad1 is the sink. 
 From link0's point of view, pad0 is a data sink. Which interpretation 
is correct?

>
> The proposed order of the subdev configuration is as follows. Individual
> steps may be omitted since any of the steps will reset the rectangles /
> sizes for any following step.

I assume that SOURCE and SINK are defined from the link's point of view. 
Otherwise it would mean that configuration goes in order opposite to 
data flow order.

I do not think that resetting is a good idea. It is better to state in 
spec that change of a given target guarantee that targets/formats 
earlier in pipeline are not modified. Part below pipeline may change or 
not. The application should check if the configuration of lower parts of 
pipeline is suitable. For example

Change of COMPOSE target on SINK pad must not modify
- format on the SINK pad
- CROP on the SINK pad

All other parameters may change. Of course the configuration of lower 
part must be consistent with higher part of the pipeline.

>
> 1. SUBDEV_S_FMT on the SINK pad. The user will issue SUBDEV_S_FMT to set the
> subdev sink pad image size and media bus format code and other parameters in
> v4l2_mbus_framefmt as necessary.
>
> 2. SUBDEV_S_SELECTION with CROP target on the SINK pad. The crop rectangle
> is set related to the image size given in step 1).
>
> 3. SUBDEV_S_SELECTION with COMPOSE target on the SINK pad. The size of the
> compose rectangle, if it differs from the size of the rectangle given in 2),
> signifies user's wish to perform scaling.
>
> 4. SUBDEV_S_SELECTION with CROP target on the SOURCE pad. Configure cropping
> performed by the subdev after scaling.
>
> 5. SUBDEV_S_SELECTION with COMPOSE target on the SOURCE pad. This configures
> composition on the display if relevant for the subdevice. (In this case the
> COMPOSE bounds will yield to the size of the display.)
>
> 6. SUBDEV_S_FMT on the SOURCE pad. The size of the image is defined by
> setting CROP on the SOURCE pad, so SUBDEV_S_FMT only has an effect of
> changing other parameters than size.
>
> As defined in [2], when performing any of the configuration phases above,
> the formats and selections are reset to defaults from each phase onwards.
> For example, SUBDEV_S_SELECTION with CROP target on the SINK pad will
> --- beyond its obvious function of setting CROP selection target on the SINK
> pad --- reset the COMPOSE selection target on SINK pad, as well as the CROP
> selection target and format on the SOURCE pad.
>
>

I think that formal definitions of CROP,COMPOSE for pads are needed.
As I remember from Cambridge brainstorming we agreed that SINK.COMPOSE 
and SOURCE.CROP are expressed in subdev's internal coordinate system.
The SINK.CROP is expressed in link0's coordinate system and 
SOURCE.COMPOSE is expressed in link1's coordinate system.

> Definitions
> ===========
>
> /**
>   * struct v4l2_subdev_selection - selection info
>   *
>   * @which: either V4L2_SUBDEV_FORMAT_ACTIVE or V4L2_SUBDEV_FORMAT_TRY
>   * @pad: pad number, as reported by the media API
>   * @target: selection target, used to choose one of possible rectangles
>   * @flags: constraints flags
>   * @r: coordinates of selection window
>   * @reserved: for future use, rounds structure size to 64 bytes, set to zero
>   *
>   * Hardware may use multiple helper window to process a video stream.
>   * The structure is used to exchange this selection areas between
>   * an application and a driver.
>   */
> struct v4l2_subdev_selection {
> 	__u32 which;
> 	__u32 pad;
> 	__u32 target;
> 	__u32 flags;
> 	struct v4l2_rect r;
> 	__u32 reserved[8];
> };
>
> Both SUBDEV_S_SELECTION and SUBDEV_G_SELECTION would take struct
> v4l2_subdev_selection as the IOCTL argument (RW).
>
> The same target definitions and flags apply as in [1], with possible
> exception of the PADDED targets --- see below. The flags will gain _SUBDEV
> prefix after the existing V4L2 prefix.
>
>
> Sample use cases
> ================
>
>
> OMAP 3 ISP preview
> ------------------
>
> The OMAP 3 ISP preview block provides cropping on preview sink pad, but also
> horizontal averaging. The horizontal averaging may scale the image
> horizontally by factor 1/n, where n is either 1, 2, 4 or 8.
>
> The preview block also performs pixel format conversion from raw bayer to
> YUV. Other image processing operations crops maximum of 12 columns and 8 rows
> before the format conversion. After the format conversion, further 2 columns
> are cropped.
>
> To make this easy for the user, the driver assumes that all the features
> affecting cropping are enabled at all times so the size stays constant for
> the user.
>
>
> This example only includes the preview subdev since it can be shown
> independently. In the example, the preview subdev is configured to crop the
> image by 100 pixels on all sides of the image. The horizontal averaging is
> configured by factor 1/2, which translates 100 pixels from each side of the
> original image. The preview sink pad format is 800x600 SGRBG10.
>
>
> [crop] 0:preview:1 [crop (static), compose]
>
> 	The initial state of the pipeline is:
>
> 	preview:0	preview:1
> compose (0,0)/788x592	(0,0)/788x592
> crop	(7,4)/788x592	(1,0)/786x592
> fmt	800x600/SGRBG10	786x592/YUYV
>
> 	This is due to hardware imposed cropping performed in the sink pad
> 	as well as on the source pad.
>
> 	To crop 100 pixels on all sides:
>
> 	SUBDEV_S_SELECTION(preview:0, CROP_ACTIVE, (99,100)/602x400);
>
> 	The hardware mandated crop on the sink pad is thus one pixel on left
> 	and right sides of the image. (This would also be shown in the
> 	CROP_BOUNDS target.)
>
> 	preview:0	preview:1
> compose (0,0)/602x400	(0,0)/602x400
> crop	(99,100)/602x400 (1,0)/600x400
> fmt	800x600/SGRBG10	600x400/YUYV
>
>
> A sensor
> --------
>
> The intent is to obtain a VGA image from a 8 MP sensor which provides
> following pipeline:
>
> pixel_array:0 [crop] --->  0:binner:1 --->  [crop] 0:scaler:1 [crop]
>
> Binner is an entity which can perform scaling, but only in factor of 1/n,
> where n is a positive integer. No cropping is needed. The intent is to get a
> 640x480 image from such sensor. (This doesn't involve any other
> configuration as the image size related one.)
>
> 	The initial state of the pipeline
>
> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> compose (0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464
> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464
> fmt	3600x2464	3600x2464	3600x2464	3600x2464	3600x2464
>
> 	This will configure the binning on the binner subdev sink pad:
>
> 	SUBDEV_S_SELECTION(binner:0, COMPOSE_ACTIVE, (0,0)/1800x1232);
>
> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> compose (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/3600x2464	(0,0)/3600x2464
> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/3600x2464	(0,0)/3600x2464
> fmt	3600x2464	3600x2464	1800x1232	3600x2464	3600x2464
>
> 	The same format must be set on the scaler pad 0 as well. This will
> 	reset the size inside the scaler to a sane default, which is no
> 	scaling:
>
> 	SUBDEV_S_FMT(scaler:0, 1800x1232);
>
> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> compose (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232
> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232
> fmt	3600x2464	3600x2464	1800x1232	1800x1232	1800x1232

I assume that scaler can upscale image 1800x1232 on scaler:0 to 
3600x2464 on pad scaler:1. Therefore the format and compose targets on 
scaler:1 should not be changed.

>
> 	To perform further scaling on the scaler, the COMPOSE target is used
> 	on the scaler subdev's SOURCE pad:
>
> 	SUBDEV_S_SELECTION(scaler:0, COMPOSE_ACTIVE, (0,0)/640x480);
>
> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> compose (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/640x480	(0,0)/640x480
> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/640x480
> fmt	3600x2464	3600x2464	1800x1232	1800x1232	640x480
>

It is possible to compose 640x480 image into 1800x1232 data stream 
produced on scaler:1. Therefore the format on scaler:1 should not be 
changed. The area outside 640x480 would left undefined or filled by some 
pattern configured using controls. This situation was the reason of 
introducing PADDED target.

Best regards,
Tomasz Stanislawski

>
> The result is a 640x480 image from the scaler's output pad. The aspect ratio
> of the resulting image is different from 4/3 since no cropping was
> performed in this example.
>
>
> Applications which do not recognise SUBDEV_S_SELECTION
> ======================================================
>
> The current spec [2] tells that the scaling factor is defined by using
> SUBDEV_S_FMT in the source pad. This method would not be supported in the
> future, possibly affecting applications which use SUBDEV_S_FMT to configure
> the scaling factor.
>
> If supporting this is seen necessary, it can be implemented by e.g.
> reverting to the old behaviour if the SOURCE crop rectangle width or height
> is different from width and height specified in SOURCE S_FMT.
>
>
> Open questions
> ==============
>
> 1. Keep subdev configuration flag. In Cambourne meeting the case of the OMAP
> 3 ISP resizer configuration dilemma was discussed, and the proposal was to
> add a flag to disable propagating the configuration inside a single subdev.
> Propagating inside a single subdev is the default. Where do we need this
> flag; is just SUBDEV_S_SELECTION enough? [0]
>
> 2. Are PADDED targets relevant for media bus formats? [3]
>
>
> References
> ==========
>
> [0] http://www.mail-archive.com/linux-media@vger.kernel.org/msg35361.html
>
> [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36206.html
>
> [2] http://hverkuil.home.xs4all.nl/spec/media.html#subdev
>
> [3] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36203.html
>
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] SUBDEV_S/G_SELECTION IOCTLs
  2011-11-10 11:57 ` Sylwester Nawrocki
@ 2011-11-10 21:01   ` Sakari Ailus
  0 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2011-11-10 21:01 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-media, laurent.pinchart, t.stanislaws, g.liakhovetski,
	hverkuil, dacohen, andriy.shevchenko

Hi Sylwester,

Thanks for the comments!

On Thu, Nov 10, 2011 at 12:57:10PM +0100, Sylwester Nawrocki wrote:
> Hi Sakari,
> 
> On 11/08/2011 10:55 PM, Sakari Ailus wrote:
> > Hi all,
> > 
> > This RFC discusses the SUBDEV_S_SELECTION/SUBDEV_G_SELECTION API which is
> > intended to amend and replace the existing SUBDEV_[SG]_CROP API. These
> > IOCTLs have previously been discussed in the Cambridge V4L2 brainstorming
> > meeting [0] and their intent is to provide more configurability for subdevs,
> > including cropping on the source pads and composing for a display.
> > 
> > The S_SELECTION patches for V4L2 nodes are available here [1] and the
> > existing documentation for the V4L2 subdev pad format configuration can be
> > found in [2].
> > 
> > SUBDEV_[SG]_SELECTION is intended to fully replace SUBDEV_[SG]_CROP in
> > drivers as the latter can be implemented in SUBDEV_[SG]_SELECTION using
> > active CROP target on sink pads. That can be done in v4l2-ioctl.c so drivers
> > only will need to implement SUBDEV_[SG]_SELECTION.
> > 
> > 
> > Questions, comments and thoughts are welcome, especially regarding new use
> > cases.
> > 
> > 
> > Order of configuration
> > ======================
> > 
> > The proposed order of the subdev configuration is as follows. Individual
> > steps may be omitted since any of the steps will reset the rectangles /
> > sizes for any following step.
> > 
> > 1. SUBDEV_S_FMT on the SINK pad. The user will issue SUBDEV_S_FMT to set the
> > subdev sink pad image size and media bus format code and other parameters in
> > v4l2_mbus_framefmt as necessary.
> > 
> > 2. SUBDEV_S_SELECTION with CROP target on the SINK pad. The crop rectangle
> > is set related to the image size given in step 1).
> > 
> > 3. SUBDEV_S_SELECTION with COMPOSE target on the SINK pad. The size of the
> > compose rectangle, if it differs from the size of the rectangle given in 2),
> > signifies user's wish to perform scaling.
> > 
> > 4. SUBDEV_S_SELECTION with CROP target on the SOURCE pad. Configure cropping
> > performed by the subdev after scaling.
> > 
> > 5. SUBDEV_S_SELECTION with COMPOSE target on the SOURCE pad. This configures
> > composition on the display if relevant for the subdevice. (In this case the
> > COMPOSE bounds will yield to the size of the display.)
> > 
> > 6. SUBDEV_S_FMT on the SOURCE pad. The size of the image is defined by
> > setting CROP on the SOURCE pad, so SUBDEV_S_FMT only has an effect of
> > changing other parameters than size.
> > 
> > As defined in [2], when performing any of the configuration phases above,
> > the formats and selections are reset to defaults from each phase onwards.
> > For example, SUBDEV_S_SELECTION with CROP target on the SINK pad will
> > --- beyond its obvious function of setting CROP selection target on the SINK
> > pad --- reset the COMPOSE selection target on SINK pad, as well as the CROP
> > selection target and format on the SOURCE pad.
> 
> I thought we agreed that the spec will not be enforcing resetting parameters
> to defaults from each phase onwards. Also I couldn't find anything explicitly
> telling this in [2]. Let's consider simple use case: video pipeline with image
> sensor, scaler, composer and DMA engine.
> Initially user performs the configuration in the above described order.
> Then video stream is started and user wants to change the area cropped at
> image sensor, which will then appear in the configured compose window. We assume
> the H/W supports changing crop window position during streaming.
> 
> We can't reset existing configuration below/after CROP phase at the scaler sink
> pad, because we want the compose window unchanged. 
> 
> I guess this is where we need the flag to disable propagating the configuration
> inside single subdev, don't we ?

Yes, the intent is to define one. See open question number 1.

I think a note of the behaviour must be added to the spec. It might have
been left open on purpose since no consensus had been reached on how to best
handle it.

I'll make the needed changes to the next version of the RFC.

> 
> [snip]
> 
> > Open questions
> > ==============
> > 
> > 1. Keep subdev configuration flag. In Cambourne meeting the case of the OMAP
> > 3 ISP resizer configuration dilemma was discussed, and the proposal was to
> > add a flag to disable propagating the configuration inside a single subdev.
> > Propagating inside a single subdev is the default. Where do we need this
> > flag; is just SUBDEV_S_SELECTION enough? [0]
> > 
> > 2. Are PADDED targets relevant for media bus formats? [3]
> > 
> > 
> > References
> > ==========
> > 
> > [0] http://www.mail-archive.com/linux-media@vger.kernel.org/msg35361.html
> > 
> > [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36206.html
> > 
> > [2] http://hverkuil.home.xs4all.nl/spec/media.html#subdev
> > 
> > [3] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36203.html
> 
> ---
> Regards,
> Sylwester

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] SUBDEV_S/G_SELECTION IOCTLs
  2011-11-10 15:23 ` Tomasz Stanislawski
@ 2011-11-10 22:29   ` Sakari Ailus
  2011-11-16 17:07     ` Laurent Pinchart
  0 siblings, 1 reply; 9+ messages in thread
From: Sakari Ailus @ 2011-11-10 22:29 UTC (permalink / raw)
  To: Tomasz Stanislawski
  Cc: linux-media, laurent.pinchart, sylvester.nawrocki,
	g.liakhovetski, hverkuil, dacohen, andriy.shevchenko

Hi Tomasz,

Thanks for your comments!

On Thu, Nov 10, 2011 at 04:23:19PM +0100, Tomasz Stanislawski wrote:
> Hi Sakari,
> 
> 
> On 11/08/2011 10:55 PM, Sakari Ailus wrote:
> >Hi all,
> >
> >This RFC discusses the SUBDEV_S_SELECTION/SUBDEV_G_SELECTION API which is
> >intended to amend and replace the existing SUBDEV_[SG]_CROP API. These
> >IOCTLs have previously been discussed in the Cambridge V4L2 brainstorming
> >meeting [0] and their intent is to provide more configurability for subdevs,
> >including cropping on the source pads and composing for a display.
> >
> >The S_SELECTION patches for V4L2 nodes are available here [1] and the
> >existing documentation for the V4L2 subdev pad format configuration can be
> >found in [2].
> >
> >SUBDEV_[SG]_SELECTION is intended to fully replace SUBDEV_[SG]_CROP in
> >drivers as the latter can be implemented in SUBDEV_[SG]_SELECTION using
> >active CROP target on sink pads. That can be done in v4l2-ioctl.c so drivers
> >only will need to implement SUBDEV_[SG]_SELECTION.
> >
> >
> >Questions, comments and thoughts are welcome, especially regarding new use
> >cases.
> >
> >
> >Order of configuration
> >======================
> 
> Sorry, what is exactly the SOURCE pad and SINK pad? The spec says
> "Data flows from a source pad to a sink pad.". Does in refer to a
> subdev or to a link? Look at the image below:
> 
> data ---- link0 --> (pad0) [subdev] (pad1) ----> link1
> 
> From subdev's point of view, pad0 is data source, pad1 is the sink.
> From link0's point of view, pad0 is a data sink. Which
> interpretation is correct?

It should say "link" in the spec. That needs to be clarified in the spec...

> >The proposed order of the subdev configuration is as follows. Individual
> >steps may be omitted since any of the steps will reset the rectangles /
> >sizes for any following step.
> 
> I assume that SOURCE and SINK are defined from the link's point of
> view. Otherwise it would mean that configuration goes in order
> opposite to data flow order.
> 
> I do not think that resetting is a good idea. It is better to state
> in spec that change of a given target guarantee that targets/formats
> earlier in pipeline are not modified. Part below pipeline may change
> or not. The application should check if the configuration of lower
> parts of pipeline is suitable. For example

I don't think the above text in the RFC would be a change on how it works at
the moment. Inside a subdev the following stages of configuration (as in the
flow of data) are indeed reset.

Much of the hardware actually needs this unless they're scalers.

> Change of COMPOSE target on SINK pad must not modify
> - format on the SINK pad
> - CROP on the SINK pad
> 
> All other parameters may change. Of course the configuration of
> lower part must be consistent with higher part of the pipeline.

The above two would not be changed since they are before the compose window
in the sink pad.

If the user wishes the following stages also not to be modified, (s)he can
specify it using a flag. We could call it V4L2_SUBDEV_SEL_FLAG_LOCALCHANGE.
Better names are always welcome. :)

See also open question 1.

> >
> >1. SUBDEV_S_FMT on the SINK pad. The user will issue SUBDEV_S_FMT to set the
> >subdev sink pad image size and media bus format code and other parameters in
> >v4l2_mbus_framefmt as necessary.
> >
> >2. SUBDEV_S_SELECTION with CROP target on the SINK pad. The crop rectangle
> >is set related to the image size given in step 1).
> >
> >3. SUBDEV_S_SELECTION with COMPOSE target on the SINK pad. The size of the
> >compose rectangle, if it differs from the size of the rectangle given in 2),
> >signifies user's wish to perform scaling.
> >
> >4. SUBDEV_S_SELECTION with CROP target on the SOURCE pad. Configure cropping
> >performed by the subdev after scaling.
> >
> >5. SUBDEV_S_SELECTION with COMPOSE target on the SOURCE pad. This configures
> >composition on the display if relevant for the subdevice. (In this case the
> >COMPOSE bounds will yield to the size of the display.)
> >
> >6. SUBDEV_S_FMT on the SOURCE pad. The size of the image is defined by
> >setting CROP on the SOURCE pad, so SUBDEV_S_FMT only has an effect of
> >changing other parameters than size.
> >
> >As defined in [2], when performing any of the configuration phases above,
> >the formats and selections are reset to defaults from each phase onwards.
> >For example, SUBDEV_S_SELECTION with CROP target on the SINK pad will
> >--- beyond its obvious function of setting CROP selection target on the SINK
> >pad --- reset the COMPOSE selection target on SINK pad, as well as the CROP
> >selection target and format on the SOURCE pad.
> >
> >
> 
> I think that formal definitions of CROP,COMPOSE for pads are needed.
> As I remember from Cambridge brainstorming we agreed that
> SINK.COMPOSE and SOURCE.CROP are expressed in subdev's internal
> coordinate system.
> The SINK.CROP is expressed in link0's coordinate system and
> SOURCE.COMPOSE is expressed in link1's coordinate system.

Links have no coordinate system. They're just either enabled or disabled,
and only the formats at the ends of the link must match.

Externally subdevs have media bus formats in pads. Other than that,
s_selection which is to be defined, would be used to define what subdevs do
internally.

> >Definitions
> >===========
> >
> >/**
> >  * struct v4l2_subdev_selection - selection info
> >  *
> >  * @which: either V4L2_SUBDEV_FORMAT_ACTIVE or V4L2_SUBDEV_FORMAT_TRY
> >  * @pad: pad number, as reported by the media API
> >  * @target: selection target, used to choose one of possible rectangles
> >  * @flags: constraints flags
> >  * @r: coordinates of selection window
> >  * @reserved: for future use, rounds structure size to 64 bytes, set to zero
> >  *
> >  * Hardware may use multiple helper window to process a video stream.
> >  * The structure is used to exchange this selection areas between
> >  * an application and a driver.
> >  */
> >struct v4l2_subdev_selection {
> >	__u32 which;
> >	__u32 pad;
> >	__u32 target;
> >	__u32 flags;
> >	struct v4l2_rect r;
> >	__u32 reserved[8];
> >};
> >
> >Both SUBDEV_S_SELECTION and SUBDEV_G_SELECTION would take struct
> >v4l2_subdev_selection as the IOCTL argument (RW).
> >
> >The same target definitions and flags apply as in [1], with possible
> >exception of the PADDED targets --- see below. The flags will gain _SUBDEV
> >prefix after the existing V4L2 prefix.
> >
> >
> >Sample use cases
> >================
> >
> >
> >OMAP 3 ISP preview
> >------------------
> >
> >The OMAP 3 ISP preview block provides cropping on preview sink pad, but also
> >horizontal averaging. The horizontal averaging may scale the image
> >horizontally by factor 1/n, where n is either 1, 2, 4 or 8.
> >
> >The preview block also performs pixel format conversion from raw bayer to
> >YUV. Other image processing operations crops maximum of 12 columns and 8 rows
> >before the format conversion. After the format conversion, further 2 columns
> >are cropped.
> >
> >To make this easy for the user, the driver assumes that all the features
> >affecting cropping are enabled at all times so the size stays constant for
> >the user.
> >
> >
> >This example only includes the preview subdev since it can be shown
> >independently. In the example, the preview subdev is configured to crop the
> >image by 100 pixels on all sides of the image. The horizontal averaging is
> >configured by factor 1/2, which translates 100 pixels from each side of the
> >original image. The preview sink pad format is 800x600 SGRBG10.
> >
> >
> >[crop] 0:preview:1 [crop (static), compose]
> >
> >	The initial state of the pipeline is:
> >
> >	preview:0	preview:1
> >compose (0,0)/788x592	(0,0)/788x592
> >crop	(7,4)/788x592	(1,0)/786x592
> >fmt	800x600/SGRBG10	786x592/YUYV
> >
> >	This is due to hardware imposed cropping performed in the sink pad
> >	as well as on the source pad.
> >
> >	To crop 100 pixels on all sides:
> >
> >	SUBDEV_S_SELECTION(preview:0, CROP_ACTIVE, (99,100)/602x400);
> >
> >	The hardware mandated crop on the sink pad is thus one pixel on left
> >	and right sides of the image. (This would also be shown in the
> >	CROP_BOUNDS target.)
> >
> >	preview:0	preview:1
> >compose (0,0)/602x400	(0,0)/602x400
> >crop	(99,100)/602x400 (1,0)/600x400
> >fmt	800x600/SGRBG10	600x400/YUYV
> >
> >
> >A sensor
> >--------
> >
> >The intent is to obtain a VGA image from a 8 MP sensor which provides
> >following pipeline:
> >
> >pixel_array:0 [crop] --->  0:binner:1 --->  [crop] 0:scaler:1 [crop]
> >
> >Binner is an entity which can perform scaling, but only in factor of 1/n,
> >where n is a positive integer. No cropping is needed. The intent is to get a
> >640x480 image from such sensor. (This doesn't involve any other
> >configuration as the image size related one.)
> >
> >	The initial state of the pipeline
> >
> >	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> >compose (0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464
> >crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464
> >fmt	3600x2464	3600x2464	3600x2464	3600x2464	3600x2464
> >
> >	This will configure the binning on the binner subdev sink pad:
> >
> >	SUBDEV_S_SELECTION(binner:0, COMPOSE_ACTIVE, (0,0)/1800x1232);
> >
> >	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> >compose (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/3600x2464	(0,0)/3600x2464
> >crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/3600x2464	(0,0)/3600x2464
> >fmt	3600x2464	3600x2464	1800x1232	3600x2464	3600x2464
> >
> >	The same format must be set on the scaler pad 0 as well. This will
> >	reset the size inside the scaler to a sane default, which is no
> >	scaling:
> >
> >	SUBDEV_S_FMT(scaler:0, 1800x1232);
> >
> >	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> >compose (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232
> >crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232
> >fmt	3600x2464	3600x2464	1800x1232	1800x1232	1800x1232
> 
> I assume that scaler can upscale image 1800x1232 on scaler:0 to
> 3600x2464 on pad scaler:1. Therefore the format and compose targets
> on scaler:1 should not be changed.

Open question one: do we need a flag for other than s_selection to not to
reset the following stages?

That said, we also need to define a behaviour for that: if changes must be
made e.g. to crop and compose  rectangle on both sink and source pads, then
how are they made?

> >	To perform further scaling on the scaler, the COMPOSE target is used
> >	on the scaler subdev's SOURCE pad:
> >
> >	SUBDEV_S_SELECTION(scaler:0, COMPOSE_ACTIVE, (0,0)/640x480);
> >
> >	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> >compose (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/640x480	(0,0)/640x480
> >crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/640x480
> >fmt	3600x2464	3600x2464	1800x1232	1800x1232	640x480
> >
> 
> It is possible to compose 640x480 image into 1800x1232 data stream
> produced on scaler:1. Therefore the format on scaler:1 should not be
> changed. The area outside 640x480 would left undefined or filled by
> some pattern configured using controls. This situation was the
> reason of introducing PADDED target.

Consider the same example but scaling factor larger than 1. Should there be
cropping or should the compose rectangle be changed?

Would it make sense to do as few changes as possible if the aforementioned
flag is given?

> 
> Best regards,
> Tomasz Stanislawski
> 
> >
> >The result is a 640x480 image from the scaler's output pad. The aspect ratio
> >of the resulting image is different from 4/3 since no cropping was
> >performed in this example.
> >
> >
> >Applications which do not recognise SUBDEV_S_SELECTION
> >======================================================
> >
> >The current spec [2] tells that the scaling factor is defined by using
> >SUBDEV_S_FMT in the source pad. This method would not be supported in the
> >future, possibly affecting applications which use SUBDEV_S_FMT to configure
> >the scaling factor.
> >
> >If supporting this is seen necessary, it can be implemented by e.g.
> >reverting to the old behaviour if the SOURCE crop rectangle width or height
> >is different from width and height specified in SOURCE S_FMT.
> >
> >
> >Open questions
> >==============
> >
> >1. Keep subdev configuration flag. In Cambourne meeting the case of the OMAP
> >3 ISP resizer configuration dilemma was discussed, and the proposal was to
> >add a flag to disable propagating the configuration inside a single subdev.
> >Propagating inside a single subdev is the default. Where do we need this
> >flag; is just SUBDEV_S_SELECTION enough? [0]
> >
> >2. Are PADDED targets relevant for media bus formats? [3]
> >
> >
> >References
> >==========
> >
> >[0] http://www.mail-archive.com/linux-media@vger.kernel.org/msg35361.html
> >
> >[1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36206.html
> >
> >[2] http://hverkuil.home.xs4all.nl/spec/media.html#subdev
> >
> >[3] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36203.html
> >
> >
> 

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] SUBDEV_S/G_SELECTION IOCTLs
  2011-11-08 21:55 [RFC] SUBDEV_S/G_SELECTION IOCTLs Sakari Ailus
  2011-11-10 11:57 ` Sylwester Nawrocki
  2011-11-10 15:23 ` Tomasz Stanislawski
@ 2011-11-11 14:03 ` HeungJun Kim
  2 siblings, 0 replies; 9+ messages in thread
From: HeungJun Kim @ 2011-11-11 14:03 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, laurent.pinchart, t.stanislaws, sylvester.nawrocki,
	g.liakhovetski, hverkuil, dacohen, andriy.shevchenko

Hi Sakari,

It looks nice trial for setting the image and stream size.

I'm curious just one thing. Does it help to use the way setting size "easily"?

The most media drivers has its own negotiation way, and at every time using such media drivers, the application developers should learn the way to be negotiated. They generally wants the size which they want, not to be negotiated. 

But, this new API looks to fix the sizes, to be able to make negotiation codes easy, "not to negotiate".

So, in such meaning, how about your thoughts? Does it help the user to handle the size more easily?

Regards,
Heungjun Kim

2011. 11. 9. 오전 6:55 Sakari Ailus <sakari.ailus@iki.fi> 작성:

> Hi all,
> 
> This RFC discusses the SUBDEV_S_SELECTION/SUBDEV_G_SELECTION API which is
> intended to amend and replace the existing SUBDEV_[SG]_CROP API. These
> IOCTLs have previously been discussed in the Cambridge V4L2 brainstorming
> meeting [0] and their intent is to provide more configurability for subdevs,
> including cropping on the source pads and composing for a display.
> 
> The S_SELECTION patches for V4L2 nodes are available here [1] and the
> existing documentation for the V4L2 subdev pad format configuration can be
> found in [2].
> 
> SUBDEV_[SG]_SELECTION is intended to fully replace SUBDEV_[SG]_CROP in
> drivers as the latter can be implemented in SUBDEV_[SG]_SELECTION using
> active CROP target on sink pads. That can be done in v4l2-ioctl.c so drivers
> only will need to implement SUBDEV_[SG]_SELECTION.
> 
> 
> Questions, comments and thoughts are welcome, especially regarding new use
> cases.
> 
> 
> Order of configuration
> ======================
> 
> The proposed order of the subdev configuration is as follows. Individual
> steps may be omitted since any of the steps will reset the rectangles /
> sizes for any following step.
> 
> 1. SUBDEV_S_FMT on the SINK pad. The user will issue SUBDEV_S_FMT to set the
> subdev sink pad image size and media bus format code and other parameters in
> v4l2_mbus_framefmt as necessary.
> 
> 2. SUBDEV_S_SELECTION with CROP target on the SINK pad. The crop rectangle
> is set related to the image size given in step 1).
> 
> 3. SUBDEV_S_SELECTION with COMPOSE target on the SINK pad. The size of the
> compose rectangle, if it differs from the size of the rectangle given in 2),
> signifies user's wish to perform scaling.
> 
> 4. SUBDEV_S_SELECTION with CROP target on the SOURCE pad. Configure cropping
> performed by the subdev after scaling.
> 
> 5. SUBDEV_S_SELECTION with COMPOSE target on the SOURCE pad. This configures
> composition on the display if relevant for the subdevice. (In this case the
> COMPOSE bounds will yield to the size of the display.)
> 
> 6. SUBDEV_S_FMT on the SOURCE pad. The size of the image is defined by
> setting CROP on the SOURCE pad, so SUBDEV_S_FMT only has an effect of
> changing other parameters than size.
> 
> As defined in [2], when performing any of the configuration phases above,
> the formats and selections are reset to defaults from each phase onwards.
> For example, SUBDEV_S_SELECTION with CROP target on the SINK pad will
> --- beyond its obvious function of setting CROP selection target on the SINK
> pad --- reset the COMPOSE selection target on SINK pad, as well as the CROP
> selection target and format on the SOURCE pad.
> 
> 
> Definitions
> ===========
> 
> /**
> * struct v4l2_subdev_selection - selection info
> *
> * @which: either V4L2_SUBDEV_FORMAT_ACTIVE or V4L2_SUBDEV_FORMAT_TRY
> * @pad: pad number, as reported by the media API
> * @target: selection target, used to choose one of possible rectangles
> * @flags: constraints flags
> * @r: coordinates of selection window
> * @reserved: for future use, rounds structure size to 64 bytes, set to zero
> *
> * Hardware may use multiple helper window to process a video stream.
> * The structure is used to exchange this selection areas between
> * an application and a driver.
> */
> struct v4l2_subdev_selection {
>    __u32 which;
>    __u32 pad;
>    __u32 target;
>    __u32 flags;
>    struct v4l2_rect r;
>    __u32 reserved[8];
> };
> 
> Both SUBDEV_S_SELECTION and SUBDEV_G_SELECTION would take struct
> v4l2_subdev_selection as the IOCTL argument (RW).
> 
> The same target definitions and flags apply as in [1], with possible
> exception of the PADDED targets --- see below. The flags will gain _SUBDEV
> prefix after the existing V4L2 prefix.
> 
> 
> Sample use cases
> ================
> 
> 
> OMAP 3 ISP preview
> ------------------
> 
> The OMAP 3 ISP preview block provides cropping on preview sink pad, but also
> horizontal averaging. The horizontal averaging may scale the image
> horizontally by factor 1/n, where n is either 1, 2, 4 or 8.
> 
> The preview block also performs pixel format conversion from raw bayer to
> YUV. Other image processing operations crops maximum of 12 columns and 8 rows
> before the format conversion. After the format conversion, further 2 columns
> are cropped.
> 
> To make this easy for the user, the driver assumes that all the features
> affecting cropping are enabled at all times so the size stays constant for
> the user.
> 
> 
> This example only includes the preview subdev since it can be shown
> independently. In the example, the preview subdev is configured to crop the
> image by 100 pixels on all sides of the image. The horizontal averaging is
> configured by factor 1/2, which translates 100 pixels from each side of the
> original image. The preview sink pad format is 800x600 SGRBG10.
> 
> 
> [crop] 0:preview:1 [crop (static), compose]
> 
>    The initial state of the pipeline is:
> 
>    preview:0    preview:1
> compose (0,0)/788x592    (0,0)/788x592
> crop    (7,4)/788x592    (1,0)/786x592
> fmt    800x600/SGRBG10    786x592/YUYV
> 
>    This is due to hardware imposed cropping performed in the sink pad
>    as well as on the source pad.
> 
>    To crop 100 pixels on all sides:
> 
>    SUBDEV_S_SELECTION(preview:0, CROP_ACTIVE, (99,100)/602x400);
> 
>    The hardware mandated crop on the sink pad is thus one pixel on left
>    and right sides of the image. (This would also be shown in the
>    CROP_BOUNDS target.)
> 
>    preview:0    preview:1
> compose (0,0)/602x400    (0,0)/602x400
> crop    (99,100)/602x400 (1,0)/600x400
> fmt    800x600/SGRBG10    600x400/YUYV
> 
> 
> A sensor
> --------
> 
> The intent is to obtain a VGA image from a 8 MP sensor which provides
> following pipeline:
> 
> pixel_array:0 [crop] ---> 0:binner:1 ---> [crop] 0:scaler:1 [crop]
> 
> Binner is an entity which can perform scaling, but only in factor of 1/n,
> where n is a positive integer. No cropping is needed. The intent is to get a
> 640x480 image from such sensor. (This doesn't involve any other
> configuration as the image size related one.)
> 
>    The initial state of the pipeline
> 
>    pixel_array:0    binner:0    binner:1    scaler:0    scaler:1
> compose (0,0)/3600x2464    (0,0)/3600x2464    (0,0)/3600x2464    (0,0)/3600x2464    (0,0)/3600x2464
> crop    (0,0)/3600x2464    (0,0)/3600x2464    (0,0)/3600x2464    (0,0)/3600x2464    (0,0)/3600x2464
> fmt    3600x2464    3600x2464    3600x2464    3600x2464    3600x2464
> 
>    This will configure the binning on the binner subdev sink pad:
> 
>    SUBDEV_S_SELECTION(binner:0, COMPOSE_ACTIVE, (0,0)/1800x1232);
> 
>    pixel_array:0    binner:0    binner:1    scaler:0    scaler:1
> compose (0,0)/3600x2464    (0,0)/1800x1232    (0,0)/1800x1232    (0,0)/3600x2464    (0,0)/3600x2464
> crop    (0,0)/3600x2464    (0,0)/3600x2464    (0,0)/1800x1232    (0,0)/3600x2464    (0,0)/3600x2464
> fmt    3600x2464    3600x2464    1800x1232    3600x2464    3600x2464
> 
>    The same format must be set on the scaler pad 0 as well. This will
>    reset the size inside the scaler to a sane default, which is no
>    scaling:
> 
>    SUBDEV_S_FMT(scaler:0, 1800x1232);
> 
>    pixel_array:0    binner:0    binner:1    scaler:0    scaler:1
> compose (0,0)/3600x2464    (0,0)/1800x1232    (0,0)/1800x1232    (0,0)/1800x1232    (0,0)/1800x1232
> crop    (0,0)/3600x2464    (0,0)/3600x2464    (0,0)/1800x1232    (0,0)/1800x1232    (0,0)/1800x1232
> fmt    3600x2464    3600x2464    1800x1232    1800x1232    1800x1232
> 
>    To perform further scaling on the scaler, the COMPOSE target is used
>    on the scaler subdev's SOURCE pad:
> 
>    SUBDEV_S_SELECTION(scaler:0, COMPOSE_ACTIVE, (0,0)/640x480);
> 
>    pixel_array:0    binner:0    binner:1    scaler:0    scaler:1
> compose (0,0)/3600x2464    (0,0)/1800x1232    (0,0)/1800x1232    (0,0)/640x480    (0,0)/640x480
> crop    (0,0)/3600x2464    (0,0)/3600x2464    (0,0)/1800x1232    (0,0)/1800x1232    (0,0)/640x480
> fmt    3600x2464    3600x2464    1800x1232    1800x1232    640x480
> 
> 
> The result is a 640x480 image from the scaler's output pad. The aspect ratio
> of the resulting image is different from 4/3 since no cropping was
> performed in this example.
> 
> 
> Applications which do not recognise SUBDEV_S_SELECTION 
> ======================================================
> 
> The current spec [2] tells that the scaling factor is defined by using
> SUBDEV_S_FMT in the source pad. This method would not be supported in the
> future, possibly affecting applications which use SUBDEV_S_FMT to configure
> the scaling factor.
> 
> If supporting this is seen necessary, it can be implemented by e.g.
> reverting to the old behaviour if the SOURCE crop rectangle width or height
> is different from width and height specified in SOURCE S_FMT.
> 
> 
> Open questions
> ==============
> 
> 1. Keep subdev configuration flag. In Cambourne meeting the case of the OMAP
> 3 ISP resizer configuration dilemma was discussed, and the proposal was to
> add a flag to disable propagating the configuration inside a single subdev.
> Propagating inside a single subdev is the default. Where do we need this
> flag; is just SUBDEV_S_SELECTION enough? [0]
> 
> 2. Are PADDED targets relevant for media bus formats? [3]
> 
> 
> References
> ==========
> 
> [0] http://www.mail-archive.com/linux-media@vger.kernel.org/msg35361.html
> 
> [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36206.html
> 
> [2] http://hverkuil.home.xs4all.nl/spec/media.html#subdev
> 
> [3] http://www.mail-archive.com/linux-media@vger.kernel.org/msg36203.html
> 
> 
> -- 
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi    jabber/XMPP/Gmail: sailus@retiisi.org.uk
> --
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] SUBDEV_S/G_SELECTION IOCTLs
  2011-11-10 22:29   ` Sakari Ailus
@ 2011-11-16 17:07     ` Laurent Pinchart
  2011-11-26  9:25       ` Sakari Ailus
  0 siblings, 1 reply; 9+ messages in thread
From: Laurent Pinchart @ 2011-11-16 17:07 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Tomasz Stanislawski, linux-media, sylvester.nawrocki,
	g.liakhovetski, hverkuil, dacohen, andriy.shevchenko

Hi Sakari,

On Thursday 10 November 2011 23:29:34 Sakari Ailus wrote:
> On Thu, Nov 10, 2011 at 04:23:19PM +0100, Tomasz Stanislawski wrote:
> > On 11/08/2011 10:55 PM, Sakari Ailus wrote:
> > >Hi all,
> > >
> > >This RFC discusses the SUBDEV_S_SELECTION/SUBDEV_G_SELECTION API which
> > >is intended to amend and replace the existing SUBDEV_[SG]_CROP API.
> > >These IOCTLs have previously been discussed in the Cambridge V4L2
> > >brainstorming meeting [0] and their intent is to provide more
> > >configurability for subdevs, including cropping on the source pads and
> > >composing for a display.
> > >
> > >The S_SELECTION patches for V4L2 nodes are available here [1] and the
> > >existing documentation for the V4L2 subdev pad format configuration can
> > >be found in [2].
> > >
> > >SUBDEV_[SG]_SELECTION is intended to fully replace SUBDEV_[SG]_CROP in
> > >drivers as the latter can be implemented in SUBDEV_[SG]_SELECTION using
> > >active CROP target on sink pads. That can be done in v4l2-ioctl.c so
> > >drivers only will need to implement SUBDEV_[SG]_SELECTION.
> > >
> > >
> > >Questions, comments and thoughts are welcome, especially regarding new
> > >use cases.
> > >
> > >
> > >Order of configuration
> > >======================
> > 
> > Sorry, what is exactly the SOURCE pad and SINK pad? The spec says
> > "Data flows from a source pad to a sink pad.". Does in refer to a
> > subdev or to a link? Look at the image below:
> > 
> > data ---- link0 --> (pad0) [subdev] (pad1) ----> link1
> > 
> > From subdev's point of view, pad0 is data source, pad1 is the sink.
> > From link0's point of view, pad0 is a data sink. Which
> > interpretation is correct?
> 
> It should say "link" in the spec. That needs to be clarified in the spec...
> 
> > >The proposed order of the subdev configuration is as follows. Individual
> > >steps may be omitted since any of the steps will reset the rectangles /
> > >sizes for any following step.
> > 
> > I assume that SOURCE and SINK are defined from the link's point of
> > view. Otherwise it would mean that configuration goes in order
> > opposite to data flow order.
> > 
> > I do not think that resetting is a good idea. It is better to state
> > in spec that change of a given target guarantee that targets/formats
> > earlier in pipeline are not modified. Part below pipeline may change
> > or not. The application should check if the configuration of lower
> > parts of pipeline is suitable. For example
> 
> I don't think the above text in the RFC would be a change on how it works
> at the moment. Inside a subdev the following stages of configuration (as
> in the flow of data) are indeed reset.

Nit-picking here, I wouldn't talk about resetting but about propagating.

Changing a rectangle should be guaranteed not to change any other rectangle 
"upstream" in the pipeline, and may modify rectangles "downstream" (limited to 
the same subdev) as required by format propagation. A flag is required to 
allow/prevent propagation.

I don't think there's any disagreement here though, my understanding is that 
we all agree to this.

> Much of the hardware actually needs this unless they're scalers.
>
> > Change of COMPOSE target on SINK pad must not modify
> > - format on the SINK pad
> > - CROP on the SINK pad
> > 
> > All other parameters may change. Of course the configuration of
> > lower part must be consistent with higher part of the pipeline.
> 
> The above two would not be changed since they are before the compose window
> in the sink pad.
> 
> If the user wishes the following stages also not to be modified, (s)he can
> specify it using a flag. We could call it V4L2_SUBDEV_SEL_FLAG_LOCALCHANGE.
> Better names are always welcome. :)
> 
> See also open question 1.
> 
> > >1. SUBDEV_S_FMT on the SINK pad. The user will issue SUBDEV_S_FMT to set
> > >the subdev sink pad image size and media bus format code and other
> > >parameters in v4l2_mbus_framefmt as necessary.
> > >
> > >2. SUBDEV_S_SELECTION with CROP target on the SINK pad. The crop
> > >rectangle is set related to the image size given in step 1).
> > >
> > >3. SUBDEV_S_SELECTION with COMPOSE target on the SINK pad. The size of
> > >the compose rectangle, if it differs from the size of the rectangle
> > >given in 2), signifies user's wish to perform scaling.
> > >
> > >4. SUBDEV_S_SELECTION with CROP target on the SOURCE pad. Configure
> > >cropping performed by the subdev after scaling.
> > >
> > >5. SUBDEV_S_SELECTION with COMPOSE target on the SOURCE pad. This
> > >configures composition on the display if relevant for the subdevice.
> > >(In this case the COMPOSE bounds will yield to the size of the
> > >display.)
> > >
> > >6. SUBDEV_S_FMT on the SOURCE pad. The size of the image is defined by
> > >setting CROP on the SOURCE pad, so SUBDEV_S_FMT only has an effect of
> > >changing other parameters than size.
> > >
> > >As defined in [2], when performing any of the configuration phases
> > >above, the formats and selections are reset to defaults from each phase
> > >onwards. For example, SUBDEV_S_SELECTION with CROP target on the SINK
> > >pad will --- beyond its obvious function of setting CROP selection
> > >target on the SINK pad --- reset the COMPOSE selection target on SINK
> > >pad, as well as the CROP selection target and format on the SOURCE pad.
> > 
> > I think that formal definitions of CROP,COMPOSE for pads are needed.
> > As I remember from Cambridge brainstorming we agreed that
> > SINK.COMPOSE and SOURCE.CROP are expressed in subdev's internal
> > coordinate system.
> > The SINK.CROP is expressed in link0's coordinate system and
> > SOURCE.COMPOSE is expressed in link1's coordinate system.
> 
> Links have no coordinate system. They're just either enabled or disabled,
> and only the formats at the ends of the link must match.
> 
> Externally subdevs have media bus formats in pads. Other than that,
> s_selection which is to be defined, would be used to define what subdevs do
> internally.
> 
> > >Definitions
> > >===========
> > >
> > >/**
> > >
> > >  * struct v4l2_subdev_selection - selection info
> > >  *
> > >  * @which: either V4L2_SUBDEV_FORMAT_ACTIVE or V4L2_SUBDEV_FORMAT_TRY
> > >  * @pad: pad number, as reported by the media API
> > >  * @target: selection target, used to choose one of possible rectangles
> > >  * @flags: constraints flags
> > >  * @r: coordinates of selection window
> > >  * @reserved: for future use, rounds structure size to 64 bytes, set to
> > >  zero *
> > >  * Hardware may use multiple helper window to process a video stream.
> > >  * The structure is used to exchange this selection areas between
> > >  * an application and a driver.
> > >  */
> > >
> > >struct v4l2_subdev_selection {
> > >
> > >	__u32 which;
> > >	__u32 pad;
> > >	__u32 target;
> > >	__u32 flags;
> > >	struct v4l2_rect r;
> > >	__u32 reserved[8];
> > >
> > >};
> > >
> > >Both SUBDEV_S_SELECTION and SUBDEV_G_SELECTION would take struct
> > >v4l2_subdev_selection as the IOCTL argument (RW).
> > >
> > >The same target definitions and flags apply as in [1], with possible
> > >exception of the PADDED targets --- see below. The flags will gain
> > >_SUBDEV prefix after the existing V4L2 prefix.
> > >
> > >
> > >Sample use cases
> > >================
> > >
> > >
> > >OMAP 3 ISP preview
> > >------------------
> > >
> > >The OMAP 3 ISP preview block provides cropping on preview sink pad, but
> > >also horizontal averaging. The horizontal averaging may scale the image
> > >horizontally by factor 1/n, where n is either 1, 2, 4 or 8.
> > >
> > >The preview block also performs pixel format conversion from raw bayer
> > >to YUV. Other image processing operations crops maximum of 12 columns
> > >and 8 rows before the format conversion. After the format conversion,
> > >further 2 columns are cropped.
> > >
> > >To make this easy for the user, the driver assumes that all the features
> > >affecting cropping are enabled at all times so the size stays constant
> > >for the user.
> > >
> > >
> > >This example only includes the preview subdev since it can be shown
> > >independently. In the example, the preview subdev is configured to crop
> > >the image by 100 pixels on all sides of the image. The horizontal
> > >averaging is configured by factor 1/2, which translates 100 pixels from
> > >each side of the original image. The preview sink pad format is 800x600
> > >SGRBG10.
> > >
> > >
> > >[crop] 0:preview:1 [crop (static), compose]
> > >
> > >	The initial state of the pipeline is:
> > >	
> > >	preview:0	preview:1
> > >
> > >compose (0,0)/788x592	(0,0)/788x592
> > >crop	(7,4)/788x592	(1,0)/786x592
> > >fmt	800x600/SGRBG10	786x592/YUYV
> > >
> > >	This is due to hardware imposed cropping performed in the sink pad
> > >	as well as on the source pad.
> > >	
> > >	To crop 100 pixels on all sides:
> > >	
> > >	SUBDEV_S_SELECTION(preview:0, CROP_ACTIVE, (99,100)/602x400);
> > >	
> > >	The hardware mandated crop on the sink pad is thus one pixel on left
> > >	and right sides of the image. (This would also be shown in the
> > >	CROP_BOUNDS target.)
> > >	
> > >	preview:0	preview:1
> > >
> > >compose (0,0)/602x400	(0,0)/602x400
> > >crop	(99,100)/602x400 (1,0)/600x400
> > >fmt	800x600/SGRBG10	600x400/YUYV
> > >
> > >
> > >A sensor
> > >--------
> > >
> > >The intent is to obtain a VGA image from a 8 MP sensor which provides
> > >following pipeline:
> > >
> > >pixel_array:0 [crop] --->  0:binner:1 --->  [crop] 0:scaler:1 [crop]
> > >
> > >Binner is an entity which can perform scaling, but only in factor of
> > >1/n, where n is a positive integer. No cropping is needed. The intent
> > >is to get a 640x480 image from such sensor. (This doesn't involve any
> > >other configuration as the image size related one.)
> > >
> > >	The initial state of the pipeline
> > >	
> > >	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> > >
> > >compose
> > >(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3
> > >600x2464
> > >crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0
> > >,0)/3600x2464 fmt	3600x2464	3600x2464	3600x2464	3600x2464	3600x2464
> > >
> > >	This will configure the binning on the binner subdev sink pad:
> > >	
> > >	SUBDEV_S_SELECTION(binner:0, COMPOSE_ACTIVE, (0,0)/1800x1232);
> > >	
> > >	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> > >
> > >compose
> > >(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/3600x2464	(0,0)/3
> > >600x2464
> > >crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/3600x2464	(0
> > >,0)/3600x2464 fmt	3600x2464	3600x2464	1800x1232	3600x2464	3600x2464
> > >
> > >	The same format must be set on the scaler pad 0 as well. This will
> > >	reset the size inside the scaler to a sane default, which is no
> > >	scaling:
> > >	
> > >	SUBDEV_S_FMT(scaler:0, 1800x1232);
> > >	
> > >	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> > >
> > >compose
> > >(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1
> > >800x1232
> > >crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0
> > >,0)/1800x1232 fmt	3600x2464	3600x2464	1800x1232	1800x1232	1800x1232
> > 
> > I assume that scaler can upscale image 1800x1232 on scaler:0 to
> > 3600x2464 on pad scaler:1. Therefore the format and compose targets
> > on scaler:1 should not be changed.
> 
> Open question one: do we need a flag for other than s_selection to not to
> reset the following stages?
> 
> That said, we also need to define a behaviour for that: if changes must be
> made e.g. to crop and compose  rectangle on both sink and source pads, then
> how are they made?

Shouldn't that be left to the drivers to decide ? Different devices will 
likely have different requirements.

> > >	To perform further scaling on the scaler, the COMPOSE target is used
> > >	on the scaler subdev's SOURCE pad:
> > >	
> > >	SUBDEV_S_SELECTION(scaler:0, COMPOSE_ACTIVE, (0,0)/640x480);
> > >	
> > >	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> > >
> > >compose
> > >(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/640x480	
(0,0)/640
> > >x480
> > >crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0
> > >,0)/640x480 fmt	3600x2464	3600x2464	1800x1232	1800x1232	640x480
> > 
> > It is possible to compose 640x480 image into 1800x1232 data stream
> > produced on scaler:1. Therefore the format on scaler:1 should not be
> > changed. The area outside 640x480 would left undefined or filled by
> > some pattern configured using controls. This situation was the
> > reason of introducing PADDED target.
> 
> Consider the same example but scaling factor larger than 1. Should there be
> cropping or should the compose rectangle be changed?
> 
> Would it make sense to do as few changes as possible if the aforementioned
> flag is given?
> 
> > Best regards,
> > Tomasz Stanislawski
> > 
> > >The result is a 640x480 image from the scaler's output pad. The aspect
> > >ratio of the resulting image is different from 4/3 since no cropping
> > >was performed in this example.
> > >
> > >
> > >Applications which do not recognise SUBDEV_S_SELECTION
> > >======================================================
> > >
> > >The current spec [2] tells that the scaling factor is defined by using
> > >SUBDEV_S_FMT in the source pad. This method would not be supported in
> > >the future, possibly affecting applications which use SUBDEV_S_FMT to
> > >configure the scaling factor.
> > >
> > >If supporting this is seen necessary, it can be implemented by e.g.
> > >reverting to the old behaviour if the SOURCE crop rectangle width or
> > >height is different from width and height specified in SOURCE S_FMT.
> > >
> > >
> > >Open questions
> > >==============
> > >
> > >1. Keep subdev configuration flag. In Cambourne meeting the case of the
> > >OMAP 3 ISP resizer configuration dilemma was discussed, and the
> > >proposal was to add a flag to disable propagating the configuration
> > >inside a single subdev. Propagating inside a single subdev is the
> > >default. Where do we need this flag; is just SUBDEV_S_SELECTION enough?
> > >[0]
> > >
> > >2. Are PADDED targets relevant for media bus formats? [3]
> > >
> > >
> > >References
> > >==========
> > >
> > >[0]
> > >http://www.mail-archive.com/linux-media@vger.kernel.org/msg35361.html
> > >
> > >[1]
> > >http://www.mail-archive.com/linux-media@vger.kernel.org/msg36206.html
> > >
> > >[2] http://hverkuil.home.xs4all.nl/spec/media.html#subdev
> > >
> > >[3]
> > >http://www.mail-archive.com/linux-media@vger.kernel.org/msg36203.html

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] SUBDEV_S/G_SELECTION IOCTLs
  2011-11-16 17:07     ` Laurent Pinchart
@ 2011-11-26  9:25       ` Sakari Ailus
  2011-11-28 11:06         ` Laurent Pinchart
  0 siblings, 1 reply; 9+ messages in thread
From: Sakari Ailus @ 2011-11-26  9:25 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Tomasz Stanislawski, linux-media, sylvester.nawrocki,
	g.liakhovetski, hverkuil, dacohen, andriy.shevchenko

Laurent Pinchart wrote:
> Hi Sakari,

Hi Laurent,

> On Thursday 10 November 2011 23:29:34 Sakari Ailus wrote:
>> On Thu, Nov 10, 2011 at 04:23:19PM +0100, Tomasz Stanislawski wrote:
>>> On 11/08/2011 10:55 PM, Sakari Ailus wrote:
>>>> Hi all,
>>>>
>>>> This RFC discusses the SUBDEV_S_SELECTION/SUBDEV_G_SELECTION API which
>>>> is intended to amend and replace the existing SUBDEV_[SG]_CROP API.
>>>> These IOCTLs have previously been discussed in the Cambridge V4L2
>>>> brainstorming meeting [0] and their intent is to provide more
>>>> configurability for subdevs, including cropping on the source pads and
>>>> composing for a display.
>>>>
>>>> The S_SELECTION patches for V4L2 nodes are available here [1] and the
>>>> existing documentation for the V4L2 subdev pad format configuration can
>>>> be found in [2].
>>>>
>>>> SUBDEV_[SG]_SELECTION is intended to fully replace SUBDEV_[SG]_CROP in
>>>> drivers as the latter can be implemented in SUBDEV_[SG]_SELECTION using
>>>> active CROP target on sink pads. That can be done in v4l2-ioctl.c so
>>>> drivers only will need to implement SUBDEV_[SG]_SELECTION.
>>>>
>>>>
>>>> Questions, comments and thoughts are welcome, especially regarding new
>>>> use cases.
>>>>
>>>>
>>>> Order of configuration
>>>> ======================
>>>
>>> Sorry, what is exactly the SOURCE pad and SINK pad? The spec says
>>> "Data flows from a source pad to a sink pad.". Does in refer to a
>>> subdev or to a link? Look at the image below:
>>>
>>> data ---- link0 -->  (pad0) [subdev] (pad1) ---->  link1
>>>
>>>  From subdev's point of view, pad0 is data source, pad1 is the sink.
>>>  From link0's point of view, pad0 is a data sink. Which
>>> interpretation is correct?
>>
>> It should say "link" in the spec. That needs to be clarified in the spec...
>>
>>>> The proposed order of the subdev configuration is as follows. Individual
>>>> steps may be omitted since any of the steps will reset the rectangles /
>>>> sizes for any following step.
>>>
>>> I assume that SOURCE and SINK are defined from the link's point of
>>> view. Otherwise it would mean that configuration goes in order
>>> opposite to data flow order.
>>>
>>> I do not think that resetting is a good idea. It is better to state
>>> in spec that change of a given target guarantee that targets/formats
>>> earlier in pipeline are not modified. Part below pipeline may change
>>> or not. The application should check if the configuration of lower
>>> parts of pipeline is suitable. For example
>>
>> I don't think the above text in the RFC would be a change on how it works
>> at the moment. Inside a subdev the following stages of configuration (as
>> in the flow of data) are indeed reset.
>
> Nit-picking here, I wouldn't talk about resetting but about propagating.
>
> Changing a rectangle should be guaranteed not to change any other rectangle
> "upstream" in the pipeline, and may modify rectangles "downstream" (limited to
> the same subdev) as required by format propagation. A flag is required to
> allow/prevent propagation.
>
> I don't think there's any disagreement here though, my understanding is that
> we all agree to this.

I'd think so, too, but it's still good to specify that. I'll change the
term I use in the next version.

>> Much of the hardware actually needs this unless they're scalers.
>>
>>> Change of COMPOSE target on SINK pad must not modify
>>> - format on the SINK pad
>>> - CROP on the SINK pad
>>>
>>> All other parameters may change. Of course the configuration of
>>> lower part must be consistent with higher part of the pipeline.
>>
>> The above two would not be changed since they are before the compose window
>> in the sink pad.
>>
>> If the user wishes the following stages also not to be modified, (s)he can
>> specify it using a flag. We could call it V4L2_SUBDEV_SEL_FLAG_LOCALCHANGE.
>> Better names are always welcome. :)
>>
>> See also open question 1.
>>
>>>> 1. SUBDEV_S_FMT on the SINK pad. The user will issue SUBDEV_S_FMT to set
>>>> the subdev sink pad image size and media bus format code and other
>>>> parameters in v4l2_mbus_framefmt as necessary.
>>>>
>>>> 2. SUBDEV_S_SELECTION with CROP target on the SINK pad. The crop
>>>> rectangle is set related to the image size given in step 1).
>>>>
>>>> 3. SUBDEV_S_SELECTION with COMPOSE target on the SINK pad. The size of
>>>> the compose rectangle, if it differs from the size of the rectangle
>>>> given in 2), signifies user's wish to perform scaling.
>>>>
>>>> 4. SUBDEV_S_SELECTION with CROP target on the SOURCE pad. Configure
>>>> cropping performed by the subdev after scaling.
>>>>
>>>> 5. SUBDEV_S_SELECTION with COMPOSE target on the SOURCE pad. This
>>>> configures composition on the display if relevant for the subdevice.
>>>> (In this case the COMPOSE bounds will yield to the size of the
>>>> display.)
>>>>
>>>> 6. SUBDEV_S_FMT on the SOURCE pad. The size of the image is defined by
>>>> setting CROP on the SOURCE pad, so SUBDEV_S_FMT only has an effect of
>>>> changing other parameters than size.
>>>>
>>>> As defined in [2], when performing any of the configuration phases
>>>> above, the formats and selections are reset to defaults from each phase
>>>> onwards. For example, SUBDEV_S_SELECTION with CROP target on the SINK
>>>> pad will --- beyond its obvious function of setting CROP selection
>>>> target on the SINK pad --- reset the COMPOSE selection target on SINK
>>>> pad, as well as the CROP selection target and format on the SOURCE pad.
>>>
>>> I think that formal definitions of CROP,COMPOSE for pads are needed.
>>> As I remember from Cambridge brainstorming we agreed that
>>> SINK.COMPOSE and SOURCE.CROP are expressed in subdev's internal
>>> coordinate system.
>>> The SINK.CROP is expressed in link0's coordinate system and
>>> SOURCE.COMPOSE is expressed in link1's coordinate system.
>>
>> Links have no coordinate system. They're just either enabled or disabled,
>> and only the formats at the ends of the link must match.
>>
>> Externally subdevs have media bus formats in pads. Other than that,
>> s_selection which is to be defined, would be used to define what subdevs do
>> internally.
>>
>>>> Definitions
>>>> ===========
>>>>
>>>> /**
>>>>
>>>>   * struct v4l2_subdev_selection - selection info
>>>>   *
>>>>   * @which: either V4L2_SUBDEV_FORMAT_ACTIVE or V4L2_SUBDEV_FORMAT_TRY
>>>>   * @pad: pad number, as reported by the media API
>>>>   * @target: selection target, used to choose one of possible rectangles
>>>>   * @flags: constraints flags
>>>>   * @r: coordinates of selection window
>>>>   * @reserved: for future use, rounds structure size to 64 bytes, set to
>>>>   zero *
>>>>   * Hardware may use multiple helper window to process a video stream.
>>>>   * The structure is used to exchange this selection areas between
>>>>   * an application and a driver.
>>>>   */
>>>>
>>>> struct v4l2_subdev_selection {
>>>>
>>>> 	__u32 which;
>>>> 	__u32 pad;
>>>> 	__u32 target;
>>>> 	__u32 flags;
>>>> 	struct v4l2_rect r;
>>>> 	__u32 reserved[8];
>>>>
>>>> };
>>>>
>>>> Both SUBDEV_S_SELECTION and SUBDEV_G_SELECTION would take struct
>>>> v4l2_subdev_selection as the IOCTL argument (RW).
>>>>
>>>> The same target definitions and flags apply as in [1], with possible
>>>> exception of the PADDED targets --- see below. The flags will gain
>>>> _SUBDEV prefix after the existing V4L2 prefix.
>>>>
>>>>
>>>> Sample use cases
>>>> ================
>>>>
>>>>
>>>> OMAP 3 ISP preview
>>>> ------------------
>>>>
>>>> The OMAP 3 ISP preview block provides cropping on preview sink pad, but
>>>> also horizontal averaging. The horizontal averaging may scale the image
>>>> horizontally by factor 1/n, where n is either 1, 2, 4 or 8.
>>>>
>>>> The preview block also performs pixel format conversion from raw bayer
>>>> to YUV. Other image processing operations crops maximum of 12 columns
>>>> and 8 rows before the format conversion. After the format conversion,
>>>> further 2 columns are cropped.
>>>>
>>>> To make this easy for the user, the driver assumes that all the features
>>>> affecting cropping are enabled at all times so the size stays constant
>>>> for the user.
>>>>
>>>>
>>>> This example only includes the preview subdev since it can be shown
>>>> independently. In the example, the preview subdev is configured to crop
>>>> the image by 100 pixels on all sides of the image. The horizontal
>>>> averaging is configured by factor 1/2, which translates 100 pixels from
>>>> each side of the original image. The preview sink pad format is 800x600
>>>> SGRBG10.
>>>>
>>>>
>>>> [crop] 0:preview:1 [crop (static), compose]
>>>>
>>>> 	The initial state of the pipeline is:
>>>> 	
>>>> 	preview:0	preview:1
>>>>
>>>> compose (0,0)/788x592	(0,0)/788x592
>>>> crop	(7,4)/788x592	(1,0)/786x592
>>>> fmt	800x600/SGRBG10	786x592/YUYV
>>>>
>>>> 	This is due to hardware imposed cropping performed in the sink pad
>>>> 	as well as on the source pad.
>>>> 	
>>>> 	To crop 100 pixels on all sides:
>>>> 	
>>>> 	SUBDEV_S_SELECTION(preview:0, CROP_ACTIVE, (99,100)/602x400);
>>>> 	
>>>> 	The hardware mandated crop on the sink pad is thus one pixel on left
>>>> 	and right sides of the image. (This would also be shown in the
>>>> 	CROP_BOUNDS target.)
>>>> 	
>>>> 	preview:0	preview:1
>>>>
>>>> compose (0,0)/602x400	(0,0)/602x400
>>>> crop	(99,100)/602x400 (1,0)/600x400
>>>> fmt	800x600/SGRBG10	600x400/YUYV
>>>>
>>>>
>>>> A sensor
>>>> --------
>>>>
>>>> The intent is to obtain a VGA image from a 8 MP sensor which provides
>>>> following pipeline:
>>>>
>>>> pixel_array:0 [crop] --->   0:binner:1 --->   [crop] 0:scaler:1 [crop]
>>>>
>>>> Binner is an entity which can perform scaling, but only in factor of
>>>> 1/n, where n is a positive integer. No cropping is needed. The intent
>>>> is to get a 640x480 image from such sensor. (This doesn't involve any
>>>> other configuration as the image size related one.)
>>>>
>>>> 	The initial state of the pipeline
>>>> 	
>>>> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
>>>>
>>>> compose
>>>> (0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3
>>>> 600x2464
>>>> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0
>>>> ,0)/3600x2464 fmt	3600x2464	3600x2464	3600x2464	3600x2464	3600x2464
>>>>
>>>> 	This will configure the binning on the binner subdev sink pad:
>>>> 	
>>>> 	SUBDEV_S_SELECTION(binner:0, COMPOSE_ACTIVE, (0,0)/1800x1232);
>>>> 	
>>>> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
>>>>
>>>> compose
>>>> (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/3600x2464	(0,0)/3
>>>> 600x2464
>>>> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/3600x2464	(0
>>>> ,0)/3600x2464 fmt	3600x2464	3600x2464	1800x1232	3600x2464	3600x2464
>>>>
>>>> 	The same format must be set on the scaler pad 0 as well. This will
>>>> 	reset the size inside the scaler to a sane default, which is no
>>>> 	scaling:
>>>> 	
>>>> 	SUBDEV_S_FMT(scaler:0, 1800x1232);
>>>> 	
>>>> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
>>>>
>>>> compose
>>>> (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1
>>>> 800x1232
>>>> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0
>>>> ,0)/1800x1232 fmt	3600x2464	3600x2464	1800x1232	1800x1232	1800x1232
>>>
>>> I assume that scaler can upscale image 1800x1232 on scaler:0 to
>>> 3600x2464 on pad scaler:1. Therefore the format and compose targets
>>> on scaler:1 should not be changed.
>>
>> Open question one: do we need a flag for other than s_selection to not to
>> reset the following stages?
>>
>> That said, we also need to define a behaviour for that: if changes must be
>> made e.g. to crop and compose  rectangle on both sink and source pads, then
>> how are they made?
>
> Shouldn't that be left to the drivers to decide ? Different devices will
> likely have different requirements.

That's quite possible, but still there should be a general rule that
should be obeyed if possible, or if it makes sense; hopefully both.

I think that enabling the keep-pipeline bit should tell that the changes
should be propagated as little as possible while not making too smart
decisions. The expected behaviour should be defined.

Say, if one has configured crop and scaling on the sink pad, how does
the change of the sink pad format affect the two?

How about this: as the sink format still consists of the whole image,
the crop rectangle should be scaled (by the driver) to fit to the new
image and the scaling factor should be adjusted so that result after
scaling in the sink pad changes as little as possible. There still may
be changes to the image size after scaling depending on the properties
of the hardware.

Do you think that would that make sense?

>>>> 	To perform further scaling on the scaler, the COMPOSE target is used
>>>> 	on the scaler subdev's SOURCE pad:
>>>> 	
>>>> 	SUBDEV_S_SELECTION(scaler:0, COMPOSE_ACTIVE, (0,0)/640x480);
>>>> 	
>>>> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
>>>>
>>>> compose
>>>> (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/640x480	
> (0,0)/640
>>>> x480
>>>> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0
>>>> ,0)/640x480 fmt	3600x2464	3600x2464	1800x1232	1800x1232	640x480
>>>
>>> It is possible to compose 640x480 image into 1800x1232 data stream
>>> produced on scaler:1. Therefore the format on scaler:1 should not be
>>> changed. The area outside 640x480 would left undefined or filled by
>>> some pattern configured using controls. This situation was the
>>> reason of introducing PADDED target.
>>
>> Consider the same example but scaling factor larger than 1. Should there be
>> cropping or should the compose rectangle be changed?
>>
>> Would it make sense to do as few changes as possible if the aforementioned
>> flag is given?
>>
>>> Best regards,
>>> Tomasz Stanislawski
>>>
>>>> The result is a 640x480 image from the scaler's output pad. The aspect
>>>> ratio of the resulting image is different from 4/3 since no cropping
>>>> was performed in this example.
>>>>
>>>>
>>>> Applications which do not recognise SUBDEV_S_SELECTION
>>>> ======================================================
>>>>
>>>> The current spec [2] tells that the scaling factor is defined by using
>>>> SUBDEV_S_FMT in the source pad. This method would not be supported in
>>>> the future, possibly affecting applications which use SUBDEV_S_FMT to
>>>> configure the scaling factor.
>>>>
>>>> If supporting this is seen necessary, it can be implemented by e.g.
>>>> reverting to the old behaviour if the SOURCE crop rectangle width or
>>>> height is different from width and height specified in SOURCE S_FMT.
>>>>
>>>>
>>>> Open questions
>>>> ==============
>>>>
>>>> 1. Keep subdev configuration flag. In Cambourne meeting the case of the
>>>> OMAP 3 ISP resizer configuration dilemma was discussed, and the
>>>> proposal was to add a flag to disable propagating the configuration
>>>> inside a single subdev. Propagating inside a single subdev is the
>>>> default. Where do we need this flag; is just SUBDEV_S_SELECTION enough?
>>>> [0]
>>>>
>>>> 2. Are PADDED targets relevant for media bus formats? [3]
>>>>
>>>>
>>>> References
>>>> ==========
>>>>
>>>> [0]
>>>> http://www.mail-archive.com/linux-media@vger.kernel.org/msg35361.html
>>>>
>>>> [1]
>>>> http://www.mail-archive.com/linux-media@vger.kernel.org/msg36206.html
>>>>
>>>> [2] http://hverkuil.home.xs4all.nl/spec/media.html#subdev
>>>>
>>>> [3]
>>>> http://www.mail-archive.com/linux-media@vger.kernel.org/msg36203.html
>

Kind regards,

-- 
Sakari Ailus
sakari.ailus@iki.fi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] SUBDEV_S/G_SELECTION IOCTLs
  2011-11-26  9:25       ` Sakari Ailus
@ 2011-11-28 11:06         ` Laurent Pinchart
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2011-11-28 11:06 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Tomasz Stanislawski, linux-media, sylvester.nawrocki,
	g.liakhovetski, hverkuil, dacohen, andriy.shevchenko

Hi Sakari,

On Saturday 26 November 2011 10:25:19 Sakari Ailus wrote:
> Laurent Pinchart wrote:
> > On Thursday 10 November 2011 23:29:34 Sakari Ailus wrote:
> >> On Thu, Nov 10, 2011 at 04:23:19PM +0100, Tomasz Stanislawski wrote:
> >>> On 11/08/2011 10:55 PM, Sakari Ailus wrote:

[snip]

> >>>> A sensor
> >>>> --------
> >>>> 
> >>>> The intent is to obtain a VGA image from a 8 MP sensor which provides
> >>>> following pipeline:
> >>>> 
> >>>> pixel_array:0 [crop] --->   0:binner:1 --->   [crop] 0:scaler:1 [crop]
> >>>> 
> >>>> Binner is an entity which can perform scaling, but only in factor of
> >>>> 1/n, where n is a positive integer. No cropping is needed. The intent
> >>>> is to get a 640x480 image from such sensor. (This doesn't involve any
> >>>> other configuration as the image size related one.)
> >>>> 
> >>>> 	The initial state of the pipeline
> >>>> 	
> >>>> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> >>>> 
> >>>> compose
> >>>> (0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/
> >>>> 3 600x2464
> >>>> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/3600x2464	(
> >>>> 0 ,0)/3600x2464 fmt	3600x2464	3600x2464	3600x2464	3600x2464	3600x2464
> >>>> 
> >>>> 	This will configure the binning on the binner subdev sink pad:
> >>>> 	
> >>>> 	SUBDEV_S_SELECTION(binner:0, COMPOSE_ACTIVE, (0,0)/1800x1232);
> >>>> 	
> >>>> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> >>>> 
> >>>> compose
> >>>> (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/3600x2464	(0,0)/
> >>>> 3 600x2464
> >>>> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/3600x2464	(
> >>>> 0 ,0)/3600x2464 fmt	3600x2464	3600x2464	1800x1232	3600x2464	3600x2464
> >>>> 
> >>>> 	The same format must be set on the scaler pad 0 as well. This will
> >>>> 	reset the size inside the scaler to a sane default, which is no
> >>>> 	scaling:
> >>>> 	
> >>>> 	SUBDEV_S_FMT(scaler:0, 1800x1232);
> >>>> 	
> >>>> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> >>>> 
> >>>> compose
> >>>> (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/
> >>>> 1 800x1232
> >>>> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(
> >>>> 0 ,0)/1800x1232 fmt	3600x2464	3600x2464	1800x1232	1800x1232	1800x1232
> >>> 
> >>> I assume that scaler can upscale image 1800x1232 on scaler:0 to
> >>> 3600x2464 on pad scaler:1. Therefore the format and compose targets
> >>> on scaler:1 should not be changed.
> >> 
> >> Open question one: do we need a flag for other than s_selection to not
> >> to reset the following stages?
> >> 
> >> That said, we also need to define a behaviour for that: if changes must
> >> be made e.g. to crop and compose  rectangle on both sink and source
> >> pads, then how are they made?
> > 
> > Shouldn't that be left to the drivers to decide ? Different devices will
> > likely have different requirements.
> 
> That's quite possible, but still there should be a general rule that
> should be obeyed if possible, or if it makes sense; hopefully both.
> 
> I think that enabling the keep-pipeline bit should tell that the changes
> should be propagated as little as possible while not making too smart
> decisions. The expected behaviour should be defined.

Shouldn't the keep-pipeline bit prevent any propagation ?

> Say, if one has configured crop and scaling on the sink pad, how does
> the change of the sink pad format affect the two?
> 
> How about this: as the sink format still consists of the whole image,
> the crop rectangle should be scaled (by the driver) to fit to the new
> image and the scaling factor should be adjusted so that result after
> scaling in the sink pad changes as little as possible. There still may
> be changes to the image size after scaling depending on the properties
> of the hardware.
> 
> Do you think that would that make sense?

I think we should make it simpler. What about just setting all rectangles 
downstream to the same size as the sink format ? That's if the keep-pipeline 
bit isn't set, if it's set modifying the sink format should probably be 
disallowed.

> >>>> 	To perform further scaling on the scaler, the COMPOSE target is used
> >>>> 	on the scaler subdev's SOURCE pad:
> >>>> 	
> >>>> 	SUBDEV_S_SELECTION(scaler:0, COMPOSE_ACTIVE, (0,0)/640x480);
> >>>> 	
> >>>> 	pixel_array:0	binner:0	binner:1	scaler:0	scaler:1
> >>>> 
> >>>> compose
> >>>> (0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(0,0)/640x480
> > 
> > (0,0)/640
> > 
> >>>> x480
> >>>> crop	(0,0)/3600x2464	(0,0)/3600x2464	(0,0)/1800x1232	(0,0)/1800x1232	(
> >>>> 0 ,0)/640x480 fmt	3600x2464	3600x2464	1800x1232	1800x1232	640x480
> >>> 
> >>> It is possible to compose 640x480 image into 1800x1232 data stream
> >>> produced on scaler:1. Therefore the format on scaler:1 should not be
> >>> changed. The area outside 640x480 would left undefined or filled by
> >>> some pattern configured using controls. This situation was the
> >>> reason of introducing PADDED target.
> >> 
> >> Consider the same example but scaling factor larger than 1. Should there
> >> be cropping or should the compose rectangle be changed?
> >> 
> >> Would it make sense to do as few changes as possible if the
> >> aforementioned flag is given?
> >> 
> >>>> The result is a 640x480 image from the scaler's output pad. The aspect
> >>>> ratio of the resulting image is different from 4/3 since no cropping
> >>>> was performed in this example.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-11-28 11:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-08 21:55 [RFC] SUBDEV_S/G_SELECTION IOCTLs Sakari Ailus
2011-11-10 11:57 ` Sylwester Nawrocki
2011-11-10 21:01   ` Sakari Ailus
2011-11-10 15:23 ` Tomasz Stanislawski
2011-11-10 22:29   ` Sakari Ailus
2011-11-16 17:07     ` Laurent Pinchart
2011-11-26  9:25       ` Sakari Ailus
2011-11-28 11:06         ` Laurent Pinchart
2011-11-11 14:03 ` HeungJun Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.