Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Hans Verkuil]

Hi all,

While working on extending v4l2-compliance with cropping/selection test cases
I decided to add support for that to vivi as well (this would give applications
a good test driver to work with).

However, I ran into problems how this should be implemented for V4L2 devices
(we are not talking about complex media controller devices where the video
pipelines are setup manually).

There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.

The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
possible frame sizes, and that can crop, compose and/or scale, then you need
to be able to set the frame size. Currently this is decided by S_FMT which
maps the format size to the closest valid frame size. This however makes
it impossible to e.g. scale up a frame, or compose the image into a larger
buffer.

For video receivers this issue doesn't exist: there the size of the incoming
video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.

I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.

However, this leads to another problem: the current S_FMT behavior is that
it implicitly sets the framesize. That behavior we will have to keep, otherwise
applications will start to behave differently.

I have an idea on how to solve that, but the solution is related to the second
problem I found:

When you set a new format size, then the compose rectangle must be set to the
new format size as well since that has always been the behavior in the past
that applications have come to expect.

But this makes certain operations impossible to execute: if a driver can't
scale, then you can never select a new format size larger than the current
(possibly cropped) frame size, even though you would want to compose the
unscaled image into such a larger buffer.

So what is the behavior that I would expect from drivers?

1) After calling S_STD, S_DV_TIMINGS or S_SELECTION(V4L2_SEL_TGT_FRAMESIZE)
the cropping, composing and format parameters are all adjusted to support the
new input video size (typically they are all set to the new size).

2) After calling S_CROP/S_SELECTION(CROP) the compose and format parameters are all
adjusted to support the new crop rectangle.

3) After calling S_SEL(COMPOSE) the format parameters are adjusted.

4) Calling S_FMT validates the format parameters to support the compose
rectangle.

However, today step 4 does something different: the compose rectangle will be
adjusted to the format size (and in the case of a sensor supporting different
framesizes the whole pipeline will be adjusted).

The only way I see that would solve this (although it isn't perfect) is to
change the behavior of S_FMT only if the selection API was used before by the
filehandle. The core can keep easily keep track of that. When the application
calls S_FMT and no selection API was used in the past by that filehandle, then
the core will call first S_SELECTION(V4L2_SEL_TGT_FRAMESIZE). If that returns
-EINVAL, then it will call S_SELECTION(V4L2_SEL_TGT_COMPOSE). Finally it will
call S_FMT. Note that a similar sequence is needed for the display case.

This means that a driver supporting the selection API can implement the logical
behavior and the core will implement the historically-required illogical part.

So the fix for this would be to add a new selection target and add compatibility
code to the v4l2-core.

With that in place I can easily add crop/compose support to vivi.

One area of uncertainty is how drivers currently implement S_FMT: do they reset
any cropping done before? They should keep the crop rectangle according to the
spec (well, it is implied there). Guennadi, what does soc_camera do?

Sylwester, I am also looking at exynos4-is/fimc-lite.c. I do see that setting
the compose rectangle will adjust it to the format size instead of the other
way around, but I can't tell if setting the format size will also adjust the
compose rectangle if that is now out-of-bounds of the new format size.

Comments? Questions?

Regards,

	Hans

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Sakari Ailus]

Hi Hans,

On Mon, Jun 24, 2013 at 02:48:15PM +0200, Hans Verkuil wrote:
> Hi all,
> 
> While working on extending v4l2-compliance with cropping/selection test cases
> I decided to add support for that to vivi as well (this would give applications
> a good test driver to work with).
> 
> However, I ran into problems how this should be implemented for V4L2 devices
> (we are not talking about complex media controller devices where the video
> pipelines are setup manually).
> 
> There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> 
> The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
> possible frame sizes, and that can crop, compose and/or scale, then you need
> to be able to set the frame size. Currently this is decided by S_FMT which

Sensors have a single "frame size". Other sizes are achieved by using
cropping and scaling (or binning) from the native pixel array size. The
drivers should probably also expose these properties rather than advertise
multiple frame sizes.

> maps the format size to the closest valid frame size. This however makes
> it impossible to e.g. scale up a frame, or compose the image into a larger
> buffer.
> 
> For video receivers this issue doesn't exist: there the size of the incoming
> video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.
> 
> I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.

The smiapp (well, subdev) driver uses V4L2_SEL_TGT_CROP_BOUNDS rectangle for
this purpose. It was agreed to use that instead of creating a separate
"pixel array size" rectangle back then. Could it be used for the same
purpose on video nodes, too? If not, then smiapp should also be switched to
use the new "frame size" rectangle.

-- 
Kind regards,

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

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Hans Verkuil]

On Tue 25 June 2013 10:21:19 Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Jun 24, 2013 at 02:48:15PM +0200, Hans Verkuil wrote:
> > Hi all,
> > 
> > While working on extending v4l2-compliance with cropping/selection test cases
> > I decided to add support for that to vivi as well (this would give applications
> > a good test driver to work with).
> > 
> > However, I ran into problems how this should be implemented for V4L2 devices
> > (we are not talking about complex media controller devices where the video
> > pipelines are setup manually).
> > 
> > There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> > 
> > The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
> > possible frame sizes, and that can crop, compose and/or scale, then you need
> > to be able to set the frame size. Currently this is decided by S_FMT which
> 
> Sensors have a single "frame size". Other sizes are achieved by using
> cropping and scaling (or binning) from the native pixel array size. The
> drivers should probably also expose these properties rather than advertise
> multiple frame sizes.

The problem is that from the point of view of a generic application you really
don't want to know about that. You have a number of possible framesizes and you
just want to pick one.

Also, the hardware may hide how each framesize was achieved and in the case of
vivi or mem2mem devices things are even murkier.

> > maps the format size to the closest valid frame size. This however makes
> > it impossible to e.g. scale up a frame, or compose the image into a larger
> > buffer.
> > 
> > For video receivers this issue doesn't exist: there the size of the incoming
> > video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.
> > 
> > I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
> 
> The smiapp (well, subdev) driver uses V4L2_SEL_TGT_CROP_BOUNDS rectangle for
> this purpose. It was agreed to use that instead of creating a separate
> "pixel array size" rectangle back then. Could it be used for the same
> purpose on video nodes, too? If not, then smiapp should also be switched to
> use the new "frame size" rectangle.

The problem with CROP_BOUNDS is that it may be larger than the actual framesize,
as it can include blanking (for video) or the additional border pixels in a
sensor.

I would prefer a new selection target for this.

Regards,

	Hans

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Guennadi Liakhovetski]

Hi Hans

On Mon, 24 Jun 2013, Hans Verkuil wrote:

> Hi all,
> 
> While working on extending v4l2-compliance with cropping/selection test cases
> I decided to add support for that to vivi as well (this would give applications
> a good test driver to work with).
> 
> However, I ran into problems how this should be implemented for V4L2 devices
> (we are not talking about complex media controller devices where the video
> pipelines are setup manually).
> 
> There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> 
> The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
> possible frame sizes, and that can crop, compose and/or scale, then you need
> to be able to set the frame size. Currently this is decided by S_FMT which
> maps the format size to the closest valid frame size. This however makes
> it impossible to e.g. scale up a frame, or compose the image into a larger
> buffer.
> 
> For video receivers this issue doesn't exist: there the size of the incoming
> video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.

Isn't it a part of the current uncertainty of the type "who should scale?" 
Or what output format should be set on the sensor for any specific final 
user-facing output frame? I thought we decided not to try to become much 
smarter here with the classical V4L2 API, instead, those for whom this is 
really important should support subdevice- or even pad-level 
configuration?

> I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
> 
> However, this leads to another problem: the current S_FMT behavior is that
> it implicitly sets the framesize. That behavior we will have to keep, otherwise
> applications will start to behave differently.
> 
> I have an idea on how to solve that, but the solution is related to the second
> problem I found:
> 
> When you set a new format size, then the compose rectangle must be set to the
> new format size as well since that has always been the behavior in the past
> that applications have come to expect.
> 
> But this makes certain operations impossible to execute: if a driver can't
> scale, then you can never select a new format size larger than the current
> (possibly cropped) frame size, even though you would want to compose the
> unscaled image into such a larger buffer.
> 
> So what is the behavior that I would expect from drivers?
> 
> 1) After calling S_STD, S_DV_TIMINGS or S_SELECTION(V4L2_SEL_TGT_FRAMESIZE)
> the cropping, composing and format parameters are all adjusted to support the
> new input video size (typically they are all set to the new size).
> 
> 2) After calling S_CROP/S_SELECTION(CROP) the compose and format parameters are all
> adjusted to support the new crop rectangle.
> 
> 3) After calling S_SEL(COMPOSE) the format parameters are adjusted.
> 
> 4) Calling S_FMT validates the format parameters to support the compose
> rectangle.
> 
> However, today step 4 does something different: the compose rectangle will be
> adjusted to the format size (and in the case of a sensor supporting different
> framesizes the whole pipeline will be adjusted).
> 
> The only way I see that would solve this (although it isn't perfect) is to
> change the behavior of S_FMT only if the selection API was used before by the
> filehandle. The core can keep easily keep track of that. When the application
> calls S_FMT and no selection API was used in the past by that filehandle, then
> the core will call first S_SELECTION(V4L2_SEL_TGT_FRAMESIZE). If that returns
> -EINVAL, then it will call S_SELECTION(V4L2_SEL_TGT_COMPOSE). Finally it will
> call S_FMT. Note that a similar sequence is needed for the display case.
> 
> This means that a driver supporting the selection API can implement the logical
> behavior and the core will implement the historically-required illogical part.
> 
> So the fix for this would be to add a new selection target and add compatibility
> code to the v4l2-core.
> 
> With that in place I can easily add crop/compose support to vivi.
> 
> One area of uncertainty is how drivers currently implement S_FMT: do they reset
> any cropping done before? They should keep the crop rectangle according to the
> spec (well, it is implied there). Guennadi, what does soc_camera do?

No, soc-camera core doesn't touch cropping parameters in s_fmt. Similarly 
host drivers aren't expected to do that.

Thanks
Guennadi

> Sylwester, I am also looking at exynos4-is/fimc-lite.c. I do see that setting
> the compose rectangle will adjust it to the format size instead of the other
> way around, but I can't tell if setting the format size will also adjust the
> compose rectangle if that is now out-of-bounds of the new format size.
> 
> Comments? Questions?
> 
> Regards,
> 
> 	Hans
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Hans Verkuil]

On Thu 27 June 2013 10:59:20 Guennadi Liakhovetski wrote:
> Hi Hans
> 
> On Mon, 24 Jun 2013, Hans Verkuil wrote:
> 
> > Hi all,
> > 
> > While working on extending v4l2-compliance with cropping/selection test cases
> > I decided to add support for that to vivi as well (this would give applications
> > a good test driver to work with).
> > 
> > However, I ran into problems how this should be implemented for V4L2 devices
> > (we are not talking about complex media controller devices where the video
> > pipelines are setup manually).
> > 
> > There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> > 
> > The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
> > possible frame sizes, and that can crop, compose and/or scale, then you need
> > to be able to set the frame size. Currently this is decided by S_FMT which
> > maps the format size to the closest valid frame size. This however makes
> > it impossible to e.g. scale up a frame, or compose the image into a larger
> > buffer.
> > 
> > For video receivers this issue doesn't exist: there the size of the incoming
> > video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.
> 
> Isn't it a part of the current uncertainty of the type "who should scale?" 
> Or what output format should be set on the sensor for any specific final 
> user-facing output frame? I thought we decided not to try to become much 
> smarter here with the classical V4L2 API, instead, those for whom this is 
> really important should support subdevice- or even pad-level 
> configuration?

I'm actually not trying to do anything complex. And remember that this is not
scaler specific: even if there is no scaler but you just want to crop and/or
compose you end up with the same problem: you can't define which framesize
should be used. You need to be able to do that in order to know how to interpret
the crop/compose rectangles.

The reason I found all these problems is by the simple experiment of adding
crop/compose support to vivi. Hardly a complex driver. But it is impossible
to do this today.

> 
> > I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
> > 
> > However, this leads to another problem: the current S_FMT behavior is that
> > it implicitly sets the framesize. That behavior we will have to keep, otherwise
> > applications will start to behave differently.
> > 
> > I have an idea on how to solve that, but the solution is related to the second
> > problem I found:
> > 
> > When you set a new format size, then the compose rectangle must be set to the
> > new format size as well since that has always been the behavior in the past
> > that applications have come to expect.
> > 
> > But this makes certain operations impossible to execute: if a driver can't
> > scale, then you can never select a new format size larger than the current
> > (possibly cropped) frame size, even though you would want to compose the
> > unscaled image into such a larger buffer.
> > 
> > So what is the behavior that I would expect from drivers?
> > 
> > 1) After calling S_STD, S_DV_TIMINGS or S_SELECTION(V4L2_SEL_TGT_FRAMESIZE)
> > the cropping, composing and format parameters are all adjusted to support the
> > new input video size (typically they are all set to the new size).
> > 
> > 2) After calling S_CROP/S_SELECTION(CROP) the compose and format parameters are all
> > adjusted to support the new crop rectangle.
> > 
> > 3) After calling S_SEL(COMPOSE) the format parameters are adjusted.
> > 
> > 4) Calling S_FMT validates the format parameters to support the compose
> > rectangle.
> > 
> > However, today step 4 does something different: the compose rectangle will be
> > adjusted to the format size (and in the case of a sensor supporting different
> > framesizes the whole pipeline will be adjusted).
> > 
> > The only way I see that would solve this (although it isn't perfect) is to
> > change the behavior of S_FMT only if the selection API was used before by the
> > filehandle. The core can keep easily keep track of that. When the application
> > calls S_FMT and no selection API was used in the past by that filehandle, then
> > the core will call first S_SELECTION(V4L2_SEL_TGT_FRAMESIZE). If that returns
> > -EINVAL, then it will call S_SELECTION(V4L2_SEL_TGT_COMPOSE). Finally it will
> > call S_FMT. Note that a similar sequence is needed for the display case.
> > 
> > This means that a driver supporting the selection API can implement the logical
> > behavior and the core will implement the historically-required illogical part.
> > 
> > So the fix for this would be to add a new selection target and add compatibility
> > code to the v4l2-core.
> > 
> > With that in place I can easily add crop/compose support to vivi.
> > 
> > One area of uncertainty is how drivers currently implement S_FMT: do they reset
> > any cropping done before? They should keep the crop rectangle according to the
> > spec (well, it is implied there). Guennadi, what does soc_camera do?
> 
> No, soc-camera core doesn't touch cropping parameters in s_fmt. Similarly 
> host drivers aren't expected to do that.

OK, good to know.

Thanks,

	Hans

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Sylwester Nawrocki]

Hi Hans,

On 06/24/2013 02:48 PM, Hans Verkuil wrote:
> Hi all,
>
> While working on extending v4l2-compliance with cropping/selection test cases
> I decided to add support for that to vivi as well (this would give applications
> a good test driver to work with).
>
> However, I ran into problems how this should be implemented for V4L2 devices
> (we are not talking about complex media controller devices where the video
> pipelines are setup manually).
>
> There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
>
> The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
> possible frame sizes, and that can crop, compose and/or scale, then you need
> to be able to set the frame size. Currently this is decided by S_FMT which
> maps the format size to the closest valid frame size. This however makes
> it impossible to e.g. scale up a frame, or compose the image into a larger
> buffer.
>
> For video receivers this issue doesn't exist: there the size of the incoming
> video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.
>
> I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.

V4L2_SEL_TGT_FRAMESIZE seems a bit imprecise to me, perhaps:
V4L2_SEL_TGT_SENSOR(_SIZE) or V4L2_SEL_TGT_SOURCE(_SIZE) ? The latter might
be a bit weird when referred to the subdev API though, not sure if defining
it as valid only on V4L2 device nodes makes any difference.

> However, this leads to another problem: the current S_FMT behavior is that
> it implicitly sets the framesize. That behavior we will have to keep, otherwise
> applications will start to behave differently.
>
> I have an idea on how to solve that, but the solution is related to the second
> problem I found:
>
> When you set a new format size, then the compose rectangle must be set to the
> new format size as well since that has always been the behavior in the past
> that applications have come to expect.
>
> But this makes certain operations impossible to execute: if a driver can't
> scale, then you can never select a new format size larger than the current
> (possibly cropped) frame size, even though you would want to compose the
> unscaled image into such a larger buffer.
>
> So what is the behavior that I would expect from drivers?
>
> 1) After calling S_STD, S_DV_TIMINGS or S_SELECTION(V4L2_SEL_TGT_FRAMESIZE)
> the cropping, composing and format parameters are all adjusted to support the
> new input video size (typically they are all set to the new size).
>
> 2) After calling S_CROP/S_SELECTION(CROP) the compose and format parameters are all
> adjusted to support the new crop rectangle.
>
> 3) After calling S_SEL(COMPOSE) the format parameters are adjusted.
>
> 4) Calling S_FMT validates the format parameters to support the compose
> rectangle.

Then, if one wants to change the COMPOSE rectangle while streaming, step 3)
would limit possible COMPOSE rectangle to the current format only ?

> However, today step 4 does something different: the compose rectangle will be
> adjusted to the format size (and in the case of a sensor supporting different
> framesizes the whole pipeline will be adjusted).
>
> The only way I see that would solve this (although it isn't perfect) is to
> change the behavior of S_FMT only if the selection API was used before by the
> filehandle. The core can keep easily keep track of that. When the application
> calls S_FMT and no selection API was used in the past by that filehandle, then
> the core will call first S_SELECTION(V4L2_SEL_TGT_FRAMESIZE). If that returns
> -EINVAL, then it will call S_SELECTION(V4L2_SEL_TGT_COMPOSE). Finally it will
> call S_FMT. Note that a similar sequence is needed for the display case.

Sounds sane to me.

> This means that a driver supporting the selection API can implement the logical
> behavior and the core will implement the historically-required illogical part.
>
> So the fix for this would be to add a new selection target and add compatibility
> code to the v4l2-core.
>
> With that in place I can easily add crop/compose support to vivi.
>
> One area of uncertainty is how drivers currently implement S_FMT: do they reset
> any cropping done before? They should keep the crop rectangle according to the
> spec (well, it is implied there). Guennadi, what does soc_camera do?
>
> Sylwester, I am also looking at exynos4-is/fimc-lite.c. I do see that setting
> the compose rectangle will adjust it to the format size instead of the other
> way around, but I can't tell if setting the format size will also adjust the
> compose rectangle if that is now out-of-bounds of the new format size.

Hmm, yes, adjusting compose rectangle to the format size should be fixed in
this driver, especially given that this video device can't be used 
standalone,
only as a part of a pipeline including at least the FIMC-LITE and an image
sensor subdev. And no, currently S_FMT doesn't adjust the compose 
rectangle's
size, as it is always equal to the size of the crop rectangle on the camera
interface input (FIMC-LITE.n subdev's sink pad crop rectangle size or 
source
pad format of this subdev). This is a bug, because the driver can be in an
incorrect state for certain configuration sequence in user space. But
I don't want to be setting any FRAMESIZE on FIMC-LITE video node, it 
belongs
to the subdev API (the subdev sink pad format). This would mean the rule of
configuring the video pipeline from data source to video node is not
conformed to. All that is needed in this case is just to compose the image
rectangle, of which size is determined by the FIMC-LITE.n subdev source pad
format, onto whatever location in the memory buffer set up with 
VIDIOC_S_FMT.
I guess this is not a problem WRT your approach as long as this driver
doesn't implement V4L2_SEL_TGT_FRAMESIZE/SENSOR selection target.

The selection API documentation currently says that the
V4L2_SEL_TGT_COMPOSE_BOUNDS rectangle is set by VIDIOC_S_FMT:

"The composing targets refer to a memory buffer. The limits of composing
coordinates are obtained using V4L2_SEL_TGT_COMPOSE_BOUNDS. All coordinates
are expressed in pixels. The rectangle's top/left corner must be located at
position (0,0) . The width and height are equal to the image size set by
VIDIOC_S_FMT."

So I understand it as one first must do S_FMT to define COMPOSE rectangle's
bounds and then COMPOSE rectangle should be set, however it's not said
explicitly.

Additionally, when a device is streaming and compose rectangle is adjusted
the format size cannot be changed. I guess the solution is simply to adjust
COMPOSE rectangle to the current format in such case.

Thanks,
Sylwester

[1] http://linuxtv.org/downloads/v4l-dvb-apis/selection-api.html

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Mauro Carvalho Chehab]

Em Sun, 30 Jun 2013 22:28:55 +0200
Sylwester Nawrocki <sylvester.nawrocki@gmail.com> escreveu:

> Hi Hans,
> 
> On 06/24/2013 02:48 PM, Hans Verkuil wrote:
> > Hi all,
> >
> > While working on extending v4l2-compliance with cropping/selection test cases
> > I decided to add support for that to vivi as well (this would give applications
> > a good test driver to work with).
> >
> > However, I ran into problems how this should be implemented for V4L2 devices
> > (we are not talking about complex media controller devices where the video
> > pipelines are setup manually).
> >
> > There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> >
> > The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
> > possible frame sizes, and that can crop, compose and/or scale, then you need
> > to be able to set the frame size. Currently this is decided by S_FMT which
> > maps the format size to the closest valid frame size. This however makes
> > it impossible to e.g. scale up a frame, or compose the image into a larger
> > buffer.
> >
> > For video receivers this issue doesn't exist: there the size of the incoming
> > video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.
> >
> > I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
> 
> V4L2_SEL_TGT_FRAMESIZE seems a bit imprecise to me, perhaps:
> V4L2_SEL_TGT_SENSOR(_SIZE) or V4L2_SEL_TGT_SOURCE(_SIZE) ? The latter might
> be a bit weird when referred to the subdev API though, not sure if defining
> it as valid only on V4L2 device nodes makes any difference.

Both name proposals seem weird and confuse.

If the issue is only to do scale up, then why not create a VIDIOC_S_SCALEUP
ioctl (or something similar), when we start having any real case where this
is needed. Please, let's not overbloat the API just due to vivi driver issues.

In the specific case of vivi, I can't see why you would need something like
that for crop/selection: it should just assume that the S_FMT represents the
full frame, and crop/selection will apply on it.

Btw, I can't remember a single (non-embedded) capture device that can do
scaleup. On embedded devices, this is probably already solved by a mem2mem
driver or via the media controller API.

Ok, for output devices this could be more common.

Do you have any real case where this feature is needed?

Regards,
Mauro

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Sylwester Nawrocki]

On 06/30/2013 10:55 PM, Mauro Carvalho Chehab wrote:
> Em Sun, 30 Jun 2013 22:28:55 +0200
> Sylwester Nawrocki<sylvester.nawrocki@gmail.com>  escreveu:
>
>> Hi Hans,
>>
>> On 06/24/2013 02:48 PM, Hans Verkuil wrote:
>>> Hi all,
>>>
>>> While working on extending v4l2-compliance with cropping/selection test cases
>>> I decided to add support for that to vivi as well (this would give applications
>>> a good test driver to work with).
>>>
>>> However, I ran into problems how this should be implemented for V4L2 devices
>>> (we are not talking about complex media controller devices where the video
>>> pipelines are setup manually).
>>>
>>> There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
>>>
>>> The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
>>> possible frame sizes, and that can crop, compose and/or scale, then you need
>>> to be able to set the frame size. Currently this is decided by S_FMT which
>>> maps the format size to the closest valid frame size. This however makes
>>> it impossible to e.g. scale up a frame, or compose the image into a larger
>>> buffer.
>>>
>>> For video receivers this issue doesn't exist: there the size of the incoming
>>> video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.
>>>
>>> I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
>>
>> V4L2_SEL_TGT_FRAMESIZE seems a bit imprecise to me, perhaps:
>> V4L2_SEL_TGT_SENSOR(_SIZE) or V4L2_SEL_TGT_SOURCE(_SIZE) ? The latter might
>> be a bit weird when referred to the subdev API though, not sure if defining
>> it as valid only on V4L2 device nodes makes any difference.
>
> Both name proposals seem weird and confuse.
>
> If the issue is only to do scale up, then why not create a VIDIOC_S_SCALEUP
> ioctl (or something similar), when we start having any real case where this
> is needed. Please, let's not overbloat the API just due to vivi driver issues.
>
> In the specific case of vivi, I can't see why you would need something like
> that for crop/selection: it should just assume that the S_FMT represents the
> full frame, and crop/selection will apply on it.
>
> Btw, I can't remember a single (non-embedded) capture device that can do
> scaleup. On embedded devices, this is probably already solved by a mem2mem
> driver or via the media controller API.

Yes, for example in case of the Samsung SoCs most, if not all, the 
camera host
interfaces can't do scaling up themselves, only scaling down. But the 
DMA can
compose the source image onto a buffer, which could be larger that the frame
size set on the camera interface input. When the camera interface input 
(sensor)
resolution and the capture buffer resolution are both set by S_FMT, this 
feature
cannot be used. Anyway, this is now handled there by the MC/subdev API.

> Ok, for output devices this could be more common.
>
> Do you have any real case where this feature is needed?

Regards,
Sylwester

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Laurent Pinchart]

Hi Hans,

On Tuesday 25 June 2013 11:02:51 Hans Verkuil wrote:
> On Tue 25 June 2013 10:21:19 Sakari Ailus wrote:
> > On Mon, Jun 24, 2013 at 02:48:15PM +0200, Hans Verkuil wrote:
> > > Hi all,
> > > 
> > > While working on extending v4l2-compliance with cropping/selection test
> > > cases I decided to add support for that to vivi as well (this would
> > > give applications a good test driver to work with).
> > > 
> > > However, I ran into problems how this should be implemented for V4L2
> > > devices (we are not talking about complex media controller devices
> > > where the video pipelines are setup manually).
> > > 
> > > There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> > > 
> > > The ENUM_FRAMESIZES issue is simple: if you have a sensor that has
> > > several possible frame sizes, and that can crop, compose and/or scale,
> > > then you need to be able to set the frame size. Currently this is
> > > decided by S_FMT which
> >
> > Sensors have a single "frame size". Other sizes are achieved by using
> > cropping and scaling (or binning) from the native pixel array size. The
> > drivers should probably also expose these properties rather than advertise
> > multiple frame sizes.
> 
> The problem is that from the point of view of a generic application you
> really don't want to know about that. You have a number of possible
> framesizes and you just want to pick one.
> 
> Also, the hardware may hide how each framesize was achieved and in the case
> of vivi or mem2mem devices things are even murkier.

ENUM_FRAMESIZES has been introduced for the uvcvideo driver. UVC devices 
expose a list of possible frame sizes, without telling anything about how the 
frame size is achieved (depending on the devices various combinations of 
cropping and scaling have been seen in practice). This is a shortcoming of the 
UVC specification in my opinion, but we need to live with it.

On the other hand, the fact that some hardware tries and fails to be clever 
shouldn't force us to implement bad APIs for sane devices. Sure, making the 
complete pipeline configurable by userspace in simple cases is overkill, but I 
don't think it would be expecting too much of userspace to support the format 
and selection APIs properly and compute the possible frame sizes (especially 
if we perform that computation in libv4l).

> > > maps the format size to the closest valid frame size. This however makes
> > > it impossible to e.g. scale up a frame, or compose the image into a
> > > larger buffer.
> > > 
> > > For video receivers this issue doesn't exist: there the size of the
> > > incoming video is decided by S_STD or S_DV_TIMINGS, but no equivalent
> > > exists for sensors.
> > > 
> > > I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
> > 
> > The smiapp (well, subdev) driver uses V4L2_SEL_TGT_CROP_BOUNDS rectangle
> > for this purpose. It was agreed to use that instead of creating a
> > separate "pixel array size" rectangle back then. Could it be used for the
> > same purpose on video nodes, too? If not, then smiapp should also be
> > switched to use the new "frame size" rectangle.
> 
> The problem with CROP_BOUNDS is that it may be larger than the actual
> framesize, as it can include blanking (for video) or the additional border
> pixels in a sensor.

*can* it include blanking and borders, or *does* it include it ? It's time to 
write those rules down in the documentation.

> I would prefer a new selection target for this.

-- 
Regards,

Laurent Pinchart

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Laurent Pinchart]

Hi Hans,

On Monday 24 June 2013 14:48:15 Hans Verkuil wrote:
> Hi all,
> 
> While working on extending v4l2-compliance with cropping/selection test
> cases I decided to add support for that to vivi as well (this would give
> applications a good test driver to work with).
> 
> However, I ran into problems how this should be implemented for V4L2 devices
> (we are not talking about complex media controller devices where the video
> pipelines are setup manually).
> 
> There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> 
> The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
> possible frame sizes, and that can crop, compose and/or scale, then you need
> to be able to set the frame size.

You mentioned that this discussion relates to simple pipelines controlled 
through a video node only. I'd like to take a step back here and first define 
what pipelines we want to support in such a way, and what pipelines requires 
the media controller API. Based on that information we can list the use cases 
we need to support, and then decide on the S_FMT/S_SELECTION APIs behaviour.

I vaguely remember to have discussed this topic previously in a meeting but I 
can't find any related information in my notes at the moment. Would anyone 
happen to have a better memory here ?

> Currently this is decided by S_FMT which maps the format size to the closest
> valid frame size. This however makes it impossible to e.g. scale up a frame,
> or compose the image into a larger buffer.

It also makes it impossible to scale a frame down without composing it into a 
larger buffer. That's definitely a bad limitation of the API.

> For video receivers this issue doesn't exist: there the size of the incoming
> video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for
> sensors.
> 
> I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.

Just to make sure I understand you correctly, are you proposing a new 
selection target valid on video nodes only, that would control the format at 
the source pad of the sensor ?

> However, this leads to another problem: the current S_FMT behavior is that
> it implicitly sets the framesize. That behavior we will have to keep,
> otherwise applications will start to behave differently.

Which frame size are you talking about ? S_FMT definitely sets the frame size 
in memory, do you mean it also implicitly sets the frame size at the sensor 
source pad ?

> I have an idea on how to solve that, but the solution is related to the
> second problem I found:
> 
> When you set a new format size, then the compose rectangle must be set to
> the new format size as well since that has always been the behavior in the
> past that applications have come to expect.

That's the behaviour applications have come to expect from devices that can't 
compose. From a quick look at the kernel source only Samsung devices implement 
the composition API. Does this behaviour need to be preserved there ?

> But this makes certain operations impossible to execute: if a driver can't
> scale, then you can never select a new format size larger than the current
> (possibly cropped) frame size, even though you would want to compose the
> unscaled image into such a larger buffer.
> 
> So what is the behavior that I would expect from drivers?
> 
> 1) After calling S_STD, S_DV_TIMINGS or S_SELECTION(V4L2_SEL_TGT_FRAMESIZE)
> the cropping, composing and format parameters are all adjusted to support
> the new input video size (typically they are all set to the new size).
> 
> 2) After calling S_CROP/S_SELECTION(CROP) the compose and format parameters
> are all adjusted to support the new crop rectangle.
> 
> 3) After calling S_SEL(COMPOSE) the format parameters are adjusted.
> 
> 4) Calling S_FMT validates the format parameters to support the compose
> rectangle.
> 
> However, today step 4 does something different: the compose rectangle will
> be adjusted to the format size (and in the case of a sensor supporting
> different framesizes the whole pipeline will be adjusted).
> 
> The only way I see that would solve this (although it isn't perfect) is to
> change the behavior of S_FMT only if the selection API was used before by
> the filehandle. The core can keep easily keep track of that. When the
> application calls S_FMT and no selection API was used in the past by that
> filehandle, then the core will call first
> S_SELECTION(V4L2_SEL_TGT_FRAMESIZE). If that returns -EINVAL, then it will
> call S_SELECTION(V4L2_SEL_TGT_COMPOSE). Finally it will call S_FMT. Note
> that a similar sequence is needed for the display case.
> 
> This means that a driver supporting the selection API can implement the
> logical behavior and the core will implement the historically-required
> illogical part.
> 
> So the fix for this would be to add a new selection target and add
> compatibility code to the v4l2-core.
> 
> With that in place I can easily add crop/compose support to vivi.
> 
> One area of uncertainty is how drivers currently implement S_FMT: do they
> reset any cropping done before? They should keep the crop rectangle
> according to the spec (well, it is implied there). Guennadi, what does
> soc_camera do?
> 
> Sylwester, I am also looking at exynos4-is/fimc-lite.c. I do see that
> setting the compose rectangle will adjust it to the format size instead of
> the other way around, but I can't tell if setting the format size will also
> adjust the compose rectangle if that is now out-of-bounds of the new format
> size.
> 
> Comments? Questions?

How should we handle devices for which supported sizes (crop, compose, ...) 
are restricted by selected pixel format ?

-- 
Regards,

Laurent Pinchart

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Hans Verkuil]

On Sun 30 June 2013 22:55:24 Mauro Carvalho Chehab wrote:
> Em Sun, 30 Jun 2013 22:28:55 +0200
> Sylwester Nawrocki <sylvester.nawrocki@gmail.com> escreveu:
> 
> > Hi Hans,
> > 
> > On 06/24/2013 02:48 PM, Hans Verkuil wrote:
> > > Hi all,
> > >
> > > While working on extending v4l2-compliance with cropping/selection test cases
> > > I decided to add support for that to vivi as well (this would give applications
> > > a good test driver to work with).
> > >
> > > However, I ran into problems how this should be implemented for V4L2 devices
> > > (we are not talking about complex media controller devices where the video
> > > pipelines are setup manually).
> > >
> > > There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> > >
> > > The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
> > > possible frame sizes, and that can crop, compose and/or scale, then you need
> > > to be able to set the frame size. Currently this is decided by S_FMT which
> > > maps the format size to the closest valid frame size. This however makes
> > > it impossible to e.g. scale up a frame, or compose the image into a larger
> > > buffer.
> > >
> > > For video receivers this issue doesn't exist: there the size of the incoming
> > > video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.
> > >
> > > I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
> > 
> > V4L2_SEL_TGT_FRAMESIZE seems a bit imprecise to me, perhaps:
> > V4L2_SEL_TGT_SENSOR(_SIZE) or V4L2_SEL_TGT_SOURCE(_SIZE) ? The latter might
> > be a bit weird when referred to the subdev API though, not sure if defining
> > it as valid only on V4L2 device nodes makes any difference.
> 
> Both name proposals seem weird and confuse.
> 
> If the issue is only to do scale up, then why not create a VIDIOC_S_SCALEUP
> ioctl (or something similar), when we start having any real case where this
> is needed. Please, let's not overbloat the API just due to vivi driver issues.

It has nothing to do with vivi, that's just an example. I'm trying to write
tests for v4l2-compliance for S_SELECTION (both cropping and composing), and
to test that I want to use vivi since that would make a nice enhancement for
it. And the bottom line it, what the behavior should be when you combine
cropping, composing and optionally scaling is just undefined. Part of the reason
is that for drivers supporting ENUM_FRAMESIZES you need a way to tell the
driver which frame size should be used.

> In the specific case of vivi, I can't see why you would need something like
> that for crop/selection: it should just assume that the S_FMT represents the
> full frame, and crop/selection will apply on it.

OK, say I call S_FMT with 800x600 for vivi. That selects the 800x600 format.
Now I set the crop to a 400x300 rectangle at 100x100. Now what should happen?
Should it scale 400x300 up to 800x600? But vivi doesn't support scaling, so
should it modify the format to 400x300 instead? But what is S_FMT is called
again with that new 400x300 format? That would suddenly select a 400x300 frame.

Or should it compose the 400x300 rectangle into the 800x600 format? But
that's not allowed by the spec (or supported by any of todays applications).

Note that this issue is not vivi specific, it's true for any driver supporting
ENUM_FRAMESIZES that want to support cropping, composing and/or scaling. This
includes sensors (and I believe some of the soc-camera drivers can do upscaling
as well), codecs and virtual drivers like vivi.

> Btw, I can't remember a single (non-embedded) capture device that can do
> scaleup. On embedded devices, this is probably already solved by a mem2mem
> driver

mem2mem doesn't solve this.

> or via the media controller API.

This discussion only applies to drivers that use the V4L2 API (i.e. have the
capture or output capabilities set), not if you have to setup the pipeline
using the subdev API. In the case of the subdev API if you are dealing with a
pad that enumerates framesizes, then SUBDEV_S_FMT will indeed select which
framesize you are going to use, but in that case there cannot be any scaling,
cropping or composing between the sensor and the source pad (i.e. pad going
out of the sensor with the image data). If the sensor has a scaler, then that
becomes a separate subdev (Laurent, Sylwester, I hope I got that right).

In the V4L2 API you do not have that luxury.

Today you simply cannot use composing at all for devices with ENUM_FRAMESIZES,
and crop only if there is a scaler, or if the crop width/height is identical
to the capture format.

If we all decide that that is a limitation we can accept, then that's fine.
Then v4l2-compliance will verify that the driver implements that correctly.

But it seems very inconsistent behavior to me. All you need is a way to select
one of the possible frame sizes to fix it.

> 
> Ok, for output devices this could be more common.
> 
> Do you have any real case where this feature is needed?

There are no drivers at this moment where this is an issue, except perhaps
soc_camera to some extent, but I think that although soc_camera attempts to
support compose, none of the actual soc_camera drivers supports it.

But I do need to know what we are going to do for the v4l2-compliance checks.
Right now it is extremely fuzzy how enum_framesizes, s_crop, s_selection and
s_fmt are supposed to work together. And that makes it hard to write tests.

Regards,

	Hans

PS: this discussion works much better in person with a nice whiteboard to
draw on!

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Hans Verkuil]

On Mon 1 July 2013 14:42:34 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Monday 24 June 2013 14:48:15 Hans Verkuil wrote:
> > Hi all,
> > 
> > While working on extending v4l2-compliance with cropping/selection test
> > cases I decided to add support for that to vivi as well (this would give
> > applications a good test driver to work with).
> > 
> > However, I ran into problems how this should be implemented for V4L2 devices
> > (we are not talking about complex media controller devices where the video
> > pipelines are setup manually).
> > 
> > There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> > 
> > The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
> > possible frame sizes, and that can crop, compose and/or scale, then you need
> > to be able to set the frame size.
> 
> You mentioned that this discussion relates to simple pipelines controlled 
> through a video node only. I'd like to take a step back here and first define 
> what pipelines we want to support in such a way, and what pipelines requires 
> the media controller API. Based on that information we can list the use cases 
> we need to support, and then decide on the S_FMT/S_SELECTION APIs behaviour.

It's fairly simple. If I have a video capture device, either using S_STD or
S_DV_TIMINGS to define the resolution of the incoming video, then I can do
cropping, composing and setting the final format without problem. I have all
the information I need to do the calculations.

On the other hand, replace the video receiver by a sensor or by a software
or hardware image generator that supports a range of resolutions and everything
falls down just because you don't have the equivalent of S_STD/S_DV_TIMINGS
for this type of device. All you need is a way to select which resolution should
be produced at the beginning/source of the video pipeline. That's exactly why
S_STD/S_DV_TIMINGS exist.

> I vaguely remember to have discussed this topic previously in a meeting but I 
> can't find any related information in my notes at the moment. Would anyone 
> happen to have a better memory here ?
> 
> > Currently this is decided by S_FMT which maps the format size to the closest
> > valid frame size. This however makes it impossible to e.g. scale up a frame,
> > or compose the image into a larger buffer.
> 
> It also makes it impossible to scale a frame down without composing it into a 
> larger buffer. That's definitely a bad limitation of the API.
> 
> > For video receivers this issue doesn't exist: there the size of the incoming
> > video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for
> > sensors.
> > 
> > I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
> 
> Just to make sure I understand you correctly, are you proposing a new 
> selection target valid on video nodes only, that would control the format at 
> the source pad of the sensor ?

Yes. So this would be valid for an input that:

- Does not set V4L2_IN_CAP_DV_TIMINGS or CAP_STD in ENUMINPUT
- Does support ENUM_FRAMESIZES

> > However, this leads to another problem: the current S_FMT behavior is that
> > it implicitly sets the framesize. That behavior we will have to keep,
> > otherwise applications will start to behave differently.
> 
> Which frame size are you talking about ? S_FMT definitely sets the frame size 
> in memory, do you mean it also implicitly sets the frame size at the sensor 
> source pad ?

For such devices, yes. How else can you select today which frame size the sensor
should produce?

> > I have an idea on how to solve that, but the solution is related to the
> > second problem I found:
> > 
> > When you set a new format size, then the compose rectangle must be set to
> > the new format size as well since that has always been the behavior in the
> > past that applications have come to expect.
> 
> That's the behaviour applications have come to expect from devices that can't 
> compose. From a quick look at the kernel source only Samsung devices implement 
> the composition API. Does this behaviour need to be preserved there ?

I believe so. I plan on adding composing capabilities to vivi. Any existing
apps should keep working as expected.

> > But this makes certain operations impossible to execute: if a driver can't
> > scale, then you can never select a new format size larger than the current
> > (possibly cropped) frame size, even though you would want to compose the
> > unscaled image into such a larger buffer.
> > 
> > So what is the behavior that I would expect from drivers?
> > 
> > 1) After calling S_STD, S_DV_TIMINGS or S_SELECTION(V4L2_SEL_TGT_FRAMESIZE)
> > the cropping, composing and format parameters are all adjusted to support
> > the new input video size (typically they are all set to the new size).
> > 
> > 2) After calling S_CROP/S_SELECTION(CROP) the compose and format parameters
> > are all adjusted to support the new crop rectangle.
> > 
> > 3) After calling S_SEL(COMPOSE) the format parameters are adjusted.
> > 
> > 4) Calling S_FMT validates the format parameters to support the compose
> > rectangle.
> > 
> > However, today step 4 does something different: the compose rectangle will
> > be adjusted to the format size (and in the case of a sensor supporting
> > different framesizes the whole pipeline will be adjusted).
> > 
> > The only way I see that would solve this (although it isn't perfect) is to
> > change the behavior of S_FMT only if the selection API was used before by
> > the filehandle. The core can keep easily keep track of that. When the
> > application calls S_FMT and no selection API was used in the past by that
> > filehandle, then the core will call first
> > S_SELECTION(V4L2_SEL_TGT_FRAMESIZE). If that returns -EINVAL, then it will
> > call S_SELECTION(V4L2_SEL_TGT_COMPOSE). Finally it will call S_FMT. Note
> > that a similar sequence is needed for the display case.
> > 
> > This means that a driver supporting the selection API can implement the
> > logical behavior and the core will implement the historically-required
> > illogical part.
> > 
> > So the fix for this would be to add a new selection target and add
> > compatibility code to the v4l2-core.
> > 
> > With that in place I can easily add crop/compose support to vivi.
> > 
> > One area of uncertainty is how drivers currently implement S_FMT: do they
> > reset any cropping done before? They should keep the crop rectangle
> > according to the spec (well, it is implied there). Guennadi, what does
> > soc_camera do?
> > 
> > Sylwester, I am also looking at exynos4-is/fimc-lite.c. I do see that
> > setting the compose rectangle will adjust it to the format size instead of
> > the other way around, but I can't tell if setting the format size will also
> > adjust the compose rectangle if that is now out-of-bounds of the new format
> > size.
> > 
> > Comments? Questions?
> 
> How should we handle devices for which supported sizes (crop, compose, ...) 
> are restricted by selected pixel format ?

Good question. ENUM_FRAMESIZES returns the available resolutions dependent on
the pixelformat. That means that when you select a resolution you need to
specify a pixelformat as well. So just a rectangle isn't enough.

I need to think some more about this.

Regards,

	Hans

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Mauro Carvalho Chehab]

Em Mon, 1 Jul 2013 14:56:58 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

...

> PS: this discussion works much better in person with a nice whiteboard to
> draw on!

Well, let's then postpone it to either LinuxCon NA or EU. 

>From this threadt, it seems that new selection API failed to fix the issues 
we had with the crop API, as we're still noticing the same issues that 
motivated the new API. So, we need to go one step back, and try to have
a bigger picture of the issues.

Cheers,
Mauro

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Laurent Pinchart]

Hi Hans,

On Monday 01 July 2013 16:38:34 Hans Verkuil wrote:
> On Mon 1 July 2013 14:42:34 Laurent Pinchart wrote:
> > On Monday 24 June 2013 14:48:15 Hans Verkuil wrote:
> > > Hi all,
> > > 
> > > While working on extending v4l2-compliance with cropping/selection test
> > > cases I decided to add support for that to vivi as well (this would give
> > > applications a good test driver to work with).
> > > 
> > > However, I ran into problems how this should be implemented for V4L2
> > > devices (we are not talking about complex media controller devices
> > > where the video pipelines are setup manually).
> > > 
> > > There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> > > 
> > > The ENUM_FRAMESIZES issue is simple: if you have a sensor that has
> > > several possible frame sizes, and that can crop, compose and/or scale,
> > > then you need to be able to set the frame size.
> > 
> > You mentioned that this discussion relates to simple pipelines controlled
> > through a video node only. I'd like to take a step back here and first
> > define what pipelines we want to support in such a way, and what
> > pipelines requires the media controller API. Based on that information we
> > can list the use cases we need to support, and then decide on the
> > S_FMT/S_SELECTION APIs behaviour.
>
> It's fairly simple. If I have a video capture device, either using S_STD or
> S_DV_TIMINGS to define the resolution of the incoming video, then I can do
> cropping, composing and setting the final format without problem. I have all
> the information I need to do the calculations.
> 
> On the other hand, replace the video receiver by a sensor or by a software
> or hardware image generator that supports a range of resolutions and
> everything falls down just because you don't have the equivalent of
> S_STD/S_DV_TIMINGS for this type of device. All you need is a way to select
> which resolution should be produced at the beginning/source of the video
> pipeline. That's exactly why S_STD/S_DV_TIMINGS exist.

I understand that, but it wasn't my point. What I'd like to do is to define 
what hardware pipelines are supported by pure V4L2 drivers (with a video node 
only) and what hardware pipelines are supported by a combination of V4L2 and 
MC.

> > I vaguely remember to have discussed this topic previously in a meeting
> > but I can't find any related information in my notes at the moment. Would
> > anyone happen to have a better memory here ?
> > 
> > > Currently this is decided by S_FMT which maps the format size to the
> > > closest valid frame size. This however makes it impossible to e.g.
> > > scale up a frame, or compose the image into a larger buffer.
> > 
> > It also makes it impossible to scale a frame down without composing it
> > into a larger buffer. That's definitely a bad limitation of the API.
> > 
> > > For video receivers this issue doesn't exist: there the size of the
> > > incoming video is decided by S_STD or S_DV_TIMINGS, but no equivalent
> > > exists for sensors.
> > > 
> > > I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
> > 
> > Just to make sure I understand you correctly, are you proposing a new
> > selection target valid on video nodes only, that would control the format
> > at the source pad of the sensor ?
> 
> Yes. So this would be valid for an input that:
> 
> - Does not set V4L2_IN_CAP_DV_TIMINGS or CAP_STD in ENUMINPUT
> - Does support ENUM_FRAMESIZES

And ENUM_FRAMESIZES would then report the list of available FRAMESIZE 
rectangles, not the list of available output formats ?

> > > However, this leads to another problem: the current S_FMT behavior is
> > > that it implicitly sets the framesize. That behavior we will have to
> > > keep, otherwise applications will start to behave differently.
> > 
> > Which frame size are you talking about ? S_FMT definitely sets the frame
> > size in memory, do you mean it also implicitly sets the frame size at the
> > sensor source pad ?
> 
> For such devices, yes. How else can you select today which frame size the
> sensor should produce?

Right, I just wanted to make sure to understand your point properly.

> > > I have an idea on how to solve that, but the solution is related to the
> > > second problem I found:
> > > 
> > > When you set a new format size, then the compose rectangle must be set
> > > to the new format size as well since that has always been the behavior
> > > in the past that applications have come to expect.
> > 
> > That's the behaviour applications have come to expect from devices that
> > can't compose. From a quick look at the kernel source only Samsung
> > devices implement the composition API. Does this behaviour need to be
> > preserved there ?
>
> I believe so. I plan on adding composing capabilities to vivi. Any existing
> apps should keep working as expected.

Right. My point was that we could possibly break the way S_FMT works for 
drivers to implement composition, given that existing applications not aware 
of composition wouldn't work with those drivers anyway. But I wasn't 
considering drivers upgraded to support composition, which can't break 
anything. As we need a solution for those, we can as well extend the solution 
to all drivers.
 
> > > But this makes certain operations impossible to execute: if a driver
> > > can't scale, then you can never select a new format size larger than the
> > > current (possibly cropped) frame size, even though you would want to
> > > compose the unscaled image into such a larger buffer.
> > > 
> > > So what is the behavior that I would expect from drivers?
> > > 
> > > 1) After calling S_STD, S_DV_TIMINGS or
> > > S_SELECTION(V4L2_SEL_TGT_FRAMESIZE) the cropping, composing and format
> > > parameters are all adjusted to support the new input video size
> > > (typically they are all set to the new size).
> > > 
> > > 2) After calling S_CROP/S_SELECTION(CROP) the compose and format
> > > parameters are all adjusted to support the new crop rectangle.
> > > 
> > > 3) After calling S_SEL(COMPOSE) the format parameters are adjusted.
> > > 
> > > 4) Calling S_FMT validates the format parameters to support the compose
> > > rectangle.
> > > 
> > > However, today step 4 does something different: the compose rectangle
> > > will be adjusted to the format size (and in the case of a sensor
> > > supporting different framesizes the whole pipeline will be adjusted).
> > > 
> > > The only way I see that would solve this (although it isn't perfect) is
> > > to change the behavior of S_FMT only if the selection API was used
> > > before by the filehandle. The core can keep easily keep track of that.
> > > When the application calls S_FMT and no selection API was used in the
> > > past by that filehandle, then the core will call first
> > > S_SELECTION(V4L2_SEL_TGT_FRAMESIZE). If that returns -EINVAL, then it
> > > will call S_SELECTION(V4L2_SEL_TGT_COMPOSE). Finally it will call S_FMT.
> > > Note that a similar sequence is needed for the display case.
> > > 
> > > This means that a driver supporting the selection API can implement the
> > > logical behavior and the core will implement the historically-required
> > > illogical part.
> > > 
> > > So the fix for this would be to add a new selection target and add
> > > compatibility code to the v4l2-core.
> > > 
> > > With that in place I can easily add crop/compose support to vivi.
> > > 
> > > One area of uncertainty is how drivers currently implement S_FMT: do
> > > they reset any cropping done before? They should keep the crop rectangle
> > > according to the spec (well, it is implied there). Guennadi, what does
> > > soc_camera do?
> > > 
> > > Sylwester, I am also looking at exynos4-is/fimc-lite.c. I do see that
> > > setting the compose rectangle will adjust it to the format size instead
> > > of the other way around, but I can't tell if setting the format size
> > > will also adjust the compose rectangle if that is now out-of-bounds of
> > > the new format size.
> > > 
> > > Comments? Questions?
> > 
> > How should we handle devices for which supported sizes (crop, compose,
> > ...) are restricted by selected pixel format ?
> 
> Good question. ENUM_FRAMESIZES returns the available resolutions dependent
> on the pixelformat. That means that when you select a resolution you need
> to specify a pixelformat as well. So just a rectangle isn't enough.
>
> I need to think some more about this.

We could first set the pixel format using S_FMT, then proceed to set all 
resolutions, and finally call S_FMT again as in point 4 above, but that looks 
a bit like a hack to me.

-- 
Regards,

Laurent Pinchart

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Tomasz Stanislawski]

Hi Hans,
Maybe it is a good time to separate configuration of a data source from
configuration of a data sink. Currently, everything is pretty mixed
for videodev API. The magic ioctl called S_FMT does to much.
As I understand it includes:
  a) setting pixelformat
  b) setting size of a buffer
  c) setting/adjusting crop rectangle previously defined using S_CROP
  d) if S_CROP is not used then S_FMT sets the framesize
     changing framesize ahs huge impact on a pipeline for capture device
  e) setting composing rectangle to the whole buffer

IMO, it would a good idea to split S_FMT into separate calls for a sink and
a source.

In case of a capture device, the source is TV grabber or sensor array.
They are controlled using S_STD for anlog TV, S_DV_TIMING for digital TV
or hypothetical S_FRAMESIZE for sensor arrays.
All mentioned medias are pretty different so they deserve dedicated ioctl.
I think it might be much better to introduce an ioctl dedicated
only for sensor arrays like {S/G}_SENSOR(_ARRAY). This ioctl would take width,
height as parameters (updating to defaults if only one resolution is supported).
All binning, skipping, etc. magic might have dedicated fields in S_SENSOR.
Introducing new selection target like SEL_TGT_FRAMESIZE or even
SEL_TGT_SOURCE is a nice abstraction but it is not need to provide HW
functionality to userspace.

In case of a capture device, the sink is a buffer. IMO, it would be nice
to have an ioctl dedicated only for configuration of buffers in memory.
Let's call it S_BUFFER. It will take width, height and pixelformat
as parameters. This ioctl would be very similar to S_FMT expect
much smaller influence on other parts of the pipeline. This call would
not change neither framesize nor cropping rectangle. There would be no
requirement that the buffer is fully filled without margins. This would
greatly simplify interactions with composing rectangle and scaling setup.

The interactions between composing rectangle and S_BUFFER are
an interesting topic. During the discussion about pipeline
configuration [1] it was mentioned that all dependencies
should be executed in to-memory order. It means that S_BUFFER
would adjust width and height to be assure that composing
rectangle fits. The same way adjusting composing rectangle will
change buffer size if necessary.

Finally S_FMT could be emulated in libv4l2 or v4l2-core as series of
of S_SELECTION and S_BUFFER calls, optionally S_STD, S_DV_TIMING or S_SENSOR
if possible. The same emulation would have to be performed for S_CROP.
The emulation would be used only to handle support for legacy applications.
All new and experimental applications must not use S_FMT or S_CROP.

I know that the changes are dramatic but fixing issues with pipeline
configuration is worth it.

What do you think about the proposed idea?

Regards,
Tomasz Stanislawski

[1] http://www.spinics.net/lists/linux-media/msg34541.html

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Sakari Ailus]

Hi Hans,

On Tue, Jun 25, 2013 at 11:02:51AM +0200, Hans Verkuil wrote:
> On Tue 25 June 2013 10:21:19 Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Mon, Jun 24, 2013 at 02:48:15PM +0200, Hans Verkuil wrote:
> > > Hi all,
> > > 
> > > While working on extending v4l2-compliance with cropping/selection test cases
> > > I decided to add support for that to vivi as well (this would give applications
> > > a good test driver to work with).
> > > 
> > > However, I ran into problems how this should be implemented for V4L2 devices
> > > (we are not talking about complex media controller devices where the video
> > > pipelines are setup manually).
> > > 
> > > There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> > > 
> > > The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
> > > possible frame sizes, and that can crop, compose and/or scale, then you need
> > > to be able to set the frame size. Currently this is decided by S_FMT which
> > 
> > Sensors have a single "frame size". Other sizes are achieved by using
> > cropping and scaling (or binning) from the native pixel array size. The
> > drivers should probably also expose these properties rather than advertise
> > multiple frame sizes.
> 
> The problem is that from the point of view of a generic application you really
> don't want to know about that. You have a number of possible framesizes and you
> just want to pick one.
> 
> Also, the hardware may hide how each framesize was achieved and in the case of
> vivi or mem2mem devices things are even murkier.
> 
> > > maps the format size to the closest valid frame size. This however makes
> > > it impossible to e.g. scale up a frame, or compose the image into a larger
> > > buffer.
> > > 
> > > For video receivers this issue doesn't exist: there the size of the incoming
> > > video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.
> > > 
> > > I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
> > 
> > The smiapp (well, subdev) driver uses V4L2_SEL_TGT_CROP_BOUNDS rectangle for
> > this purpose. It was agreed to use that instead of creating a separate
> > "pixel array size" rectangle back then. Could it be used for the same
> > purpose on video nodes, too? If not, then smiapp should also be switched to
> > use the new "frame size" rectangle.
> 
> The problem with CROP_BOUNDS is that it may be larger than the actual framesize,
> as it can include blanking (for video) or the additional border pixels in a
> sensor.

I don't think it should include anything else than just the image.

Blanking isn't valid image data, and I'd also leave any possible borders
out: this is hardly interesting to the user, nor is really part of the image.
That's what the user expects, right? The rest can't be meaningfully
processed in anyway by hardware blocks, which would mix badly with
configuring the pipeline from the user space.

A lower level mechanism is needed: frame descriptors.

> I would prefer a new selection target for this.

It was decided to use the crop bounds for the purpose a few years back, and
I don't see a need to change it. (I actually was for having such a rectangle
back then, but the rough concensus was different. :))

-- 
Kind regards,

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

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Hans Verkuil]

On Thu 4 July 2013 00:37:46 Sakari Ailus wrote:
> Hi Hans,
> 
> On Tue, Jun 25, 2013 at 11:02:51AM +0200, Hans Verkuil wrote:
> > On Tue 25 June 2013 10:21:19 Sakari Ailus wrote:
> > > Hi Hans,
> > > 
> > > On Mon, Jun 24, 2013 at 02:48:15PM +0200, Hans Verkuil wrote:
> > > > Hi all,
> > > > 
> > > > While working on extending v4l2-compliance with cropping/selection test cases
> > > > I decided to add support for that to vivi as well (this would give applications
> > > > a good test driver to work with).
> > > > 
> > > > However, I ran into problems how this should be implemented for V4L2 devices
> > > > (we are not talking about complex media controller devices where the video
> > > > pipelines are setup manually).
> > > > 
> > > > There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
> > > > 
> > > > The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
> > > > possible frame sizes, and that can crop, compose and/or scale, then you need
> > > > to be able to set the frame size. Currently this is decided by S_FMT which
> > > 
> > > Sensors have a single "frame size". Other sizes are achieved by using
> > > cropping and scaling (or binning) from the native pixel array size. The
> > > drivers should probably also expose these properties rather than advertise
> > > multiple frame sizes.
> > 
> > The problem is that from the point of view of a generic application you really
> > don't want to know about that. You have a number of possible framesizes and you
> > just want to pick one.
> > 
> > Also, the hardware may hide how each framesize was achieved and in the case of
> > vivi or mem2mem devices things are even murkier.
> > 
> > > > maps the format size to the closest valid frame size. This however makes
> > > > it impossible to e.g. scale up a frame, or compose the image into a larger
> > > > buffer.
> > > > 
> > > > For video receivers this issue doesn't exist: there the size of the incoming
> > > > video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.
> > > > 
> > > > I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
> > > 
> > > The smiapp (well, subdev) driver uses V4L2_SEL_TGT_CROP_BOUNDS rectangle for
> > > this purpose. It was agreed to use that instead of creating a separate
> > > "pixel array size" rectangle back then. Could it be used for the same
> > > purpose on video nodes, too? If not, then smiapp should also be switched to
> > > use the new "frame size" rectangle.
> > 
> > The problem with CROP_BOUNDS is that it may be larger than the actual framesize,
> > as it can include blanking (for video) or the additional border pixels in a
> > sensor.
> 
> I don't think it should include anything else than just the image.
> 
> Blanking isn't valid image data, and I'd also leave any possible borders
> out: this is hardly interesting to the user, nor is really part of the image.
> That's what the user expects, right? The rest can't be meaningfully
> processed in anyway by hardware blocks, which would mix badly with
> configuring the pipeline from the user space.

It's not so easy: I'm pretty sure bttv allows messing with the blanking area,
and in the case of analog it can be useful in case of misalignment of syncs.

The question is: does anyone actually still use it like that?

> A lower level mechanism is needed: frame descriptors.
> 
> > I would prefer a new selection target for this.
> 
> It was decided to use the crop bounds for the purpose a few years back, and
> I don't see a need to change it. (I actually was for having such a rectangle
> back then, but the rough concensus was different. :))
> 
> 

Regards,

	Hans

Re: Question: interaction between selection API, ENUM_FRAMESIZES and S_FMT?

[Sakari Ailus]

Hi Hans,

Hans Verkuil wrote:
> On Thu 4 July 2013 00:37:46 Sakari Ailus wrote:
>> Hi Hans,
>>
>> On Tue, Jun 25, 2013 at 11:02:51AM +0200, Hans Verkuil wrote:
>>> On Tue 25 June 2013 10:21:19 Sakari Ailus wrote:
>>>> Hi Hans,
>>>>
>>>> On Mon, Jun 24, 2013 at 02:48:15PM +0200, Hans Verkuil wrote:
>>>>> Hi all,
>>>>>
>>>>> While working on extending v4l2-compliance with cropping/selection test cases
>>>>> I decided to add support for that to vivi as well (this would give applications
>>>>> a good test driver to work with).
>>>>>
>>>>> However, I ran into problems how this should be implemented for V4L2 devices
>>>>> (we are not talking about complex media controller devices where the video
>>>>> pipelines are setup manually).
>>>>>
>>>>> There are two problems, one related to ENUM_FRAMESIZES and one to S_FMT.
>>>>>
>>>>> The ENUM_FRAMESIZES issue is simple: if you have a sensor that has several
>>>>> possible frame sizes, and that can crop, compose and/or scale, then you need
>>>>> to be able to set the frame size. Currently this is decided by S_FMT which
>>>>
>>>> Sensors have a single "frame size". Other sizes are achieved by using
>>>> cropping and scaling (or binning) from the native pixel array size. The
>>>> drivers should probably also expose these properties rather than advertise
>>>> multiple frame sizes.
>>>
>>> The problem is that from the point of view of a generic application you really
>>> don't want to know about that. You have a number of possible framesizes and you
>>> just want to pick one.
>>>
>>> Also, the hardware may hide how each framesize was achieved and in the case of
>>> vivi or mem2mem devices things are even murkier.
>>>
>>>>> maps the format size to the closest valid frame size. This however makes
>>>>> it impossible to e.g. scale up a frame, or compose the image into a larger
>>>>> buffer.
>>>>>
>>>>> For video receivers this issue doesn't exist: there the size of the incoming
>>>>> video is decided by S_STD or S_DV_TIMINGS, but no equivalent exists for sensors.
>>>>>
>>>>> I propose that a new selection target is added: V4L2_SEL_TGT_FRAMESIZE.
>>>>
>>>> The smiapp (well, subdev) driver uses V4L2_SEL_TGT_CROP_BOUNDS rectangle for
>>>> this purpose. It was agreed to use that instead of creating a separate
>>>> "pixel array size" rectangle back then. Could it be used for the same
>>>> purpose on video nodes, too? If not, then smiapp should also be switched to
>>>> use the new "frame size" rectangle.
>>>
>>> The problem with CROP_BOUNDS is that it may be larger than the actual framesize,
>>> as it can include blanking (for video) or the additional border pixels in a
>>> sensor.
>>
>> I don't think it should include anything else than just the image.
>>
>> Blanking isn't valid image data, and I'd also leave any possible borders
>> out: this is hardly interesting to the user, nor is really part of the image.
>> That's what the user expects, right? The rest can't be meaningfully
>> processed in anyway by hardware blocks, which would mix badly with
>> configuring the pipeline from the user space.
>
> It's not so easy: I'm pretty sure bttv allows messing with the blanking area,
> and in the case of analog it can be useful in case of misalignment of syncs.

Oh right --- TVs can be different. I only thought about sensors and 
other devices. AFAIR also the analogue text TV data is transferred in 
the VBI. But there's a separate way to access it.

> The question is: does anyone actually still use it like that?

I have to admit I have absolutely no idea about that. But I still have a 
bttv card: I use it to receive video from my C-64 that I connect to my 
PC once every five years or so. ;-)

If we add a new selection target for the purpose we also must define how 
it interacts with the existing ones. Just to tell the size of the pixel 
array in the coordinates of the crop bounds rectangle on the source pad 
would appear sound to me. To keep things generic, the crop bounds 
rectangle should still be supported by the drivers.

-- 
Cheers,

Sakari Ailus
sakari.ailus@iki.fi