Questinons regarding soc_camera / pxa_camera

Questinons regarding soc_camera / pxa_camera

[Stefan Herbrechtsmeier]

Hi,

I'm working on update my ov9655 driver to the current version of the 
v4l-dvb hg tree to publish it.
My systems run a Linux 2.6.26 kernel.

I have some bugs / questions regarding some soc_camera / pxa_camera changes:

--- a/linux/drivers/media/video/pxa_camera.c    Sun Mar 22 08:53:36 2009 
-0300
+++ b/linux/drivers/media/video/pxa_camera.c    Thu Mar 26 15:35:43 2009 
+0100
@@ -36,7 +36,7 @@
 
 #if LINUX_VERSION_CODE > KERNEL_VERSION(2, 6, 28)
 #include <mach/dma.h>
-#elif LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 27)
+#else
 #include <asm/dma.h>
 #endif
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 27)

I think this was a type and can be simplify.

@@ -1443,7 +1443,7 @@ static int pxa_camera_probe(struct platf
         goto exit;
     }
 
-    pcdev->clk = clk_get(&pdev->dev, NULL);
+    pcdev->clk = clk_get(&pdev->dev, "CAMCLK");
     if (IS_ERR(pcdev->clk)) {
         err = PTR_ERR(pcdev->clk);
         goto exit_kfree;

If the driver should support kernel lower 2.6.29 "CAMCLK" is needed.

With this two changes the driver compiles again (with some compiler 
warnings) and
works again on my system with kernel 2.6.26.

--- a/linux/drivers/media/video/soc_camera.c    Sun Mar 22 08:53:36 2009 
-0300
+++ b/linux/drivers/media/video/soc_camera.c    Thu Mar 26 15:35:43 2009 
+0100
@@ -238,7 +238,7 @@ static int soc_camera_init_user_formats(
     icd->num_user_formats = fmts;
     fmts = 0;
 
-    dev_dbg(&icd->dev, "Found %d supported formats.\n", fmts);
+    dev_dbg(&icd->dev, "Found %d supported formats.\n", 
icd->num_user_formats);
 
     /* Second pass - actually fill data formats */
     for (i = 0; i < icd->num_formats; i++)

I thing this was wrong or ' fmts = 0;' must be under the output.

@@ -675,8 +675,8 @@ static int soc_camera_cropcap(struct fil
     a->bounds.height        = icd->height_max;
     a->defrect.left            = icd->x_min;
     a->defrect.top            = icd->y_min;
-    a->defrect.width        = DEFAULT_WIDTH;
-    a->defrect.height        = DEFAULT_HEIGHT;
+    a->defrect.width        = icd->width_max;
+    a->defrect.height        = icd->height_max;
     a->pixelaspect.numerator    = 1;
     a->pixelaspect.denominator    = 1;
 
What was the reason to use fix values? Because of the current 
implementation of crop,
the default value can get bigger than the max value.

Is there some ongoing work regarding the crop implementation on soc_camera?
If I understand the documentation [1] right, the crop vales should 
represent the area
of the capture device before scaling. What was the reason for the 
current implementation
combing crop and fmt values?

Regards
    Stefan

[1] http://www.linuxtv.org/downloads/video4linux/API/V4L2_API/spec/x1707.htm

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: Questinons regarding soc_camera / pxa_camera

[Guennadi Liakhovetski]

(moving to the new v4l list)

On Thu, 26 Mar 2009, Stefan Herbrechtsmeier wrote:

> --- a/linux/drivers/media/video/soc_camera.c    Sun Mar 22 08:53:36 2009 -0300
> +++ b/linux/drivers/media/video/soc_camera.c    Thu Mar 26 15:35:43 2009 +0100
> @@ -238,7 +238,7 @@ static int soc_camera_init_user_formats(
>     icd->num_user_formats = fmts;
>     fmts = 0;
> 
> -    dev_dbg(&icd->dev, "Found %d supported formats.\n", fmts);
> +    dev_dbg(&icd->dev, "Found %d supported formats.\n",
> icd->num_user_formats);
> 
>     /* Second pass - actually fill data formats */
>     for (i = 0; i < icd->num_formats; i++)
> 
> I thing this was wrong or ' fmts = 0;' must be under the output.

Right, I'd prefer the latter though - to move the "fmts = 0;" assignment 
down.

> 
> @@ -675,8 +675,8 @@ static int soc_camera_cropcap(struct fil
>     a->bounds.height        = icd->height_max;
>     a->defrect.left            = icd->x_min;
>     a->defrect.top            = icd->y_min;
> -    a->defrect.width        = DEFAULT_WIDTH;
> -    a->defrect.height        = DEFAULT_HEIGHT;
> +    a->defrect.width        = icd->width_max;
> +    a->defrect.height        = icd->height_max;
>     a->pixelaspect.numerator    = 1;
>     a->pixelaspect.denominator    = 1;
> 
> What was the reason to use fix values? Because of the current implementation
> of crop,
> the default value can get bigger than the max value.

Yes, you're right again, taking scaling into account. Although, setting 
default to maximum doesn't seem a very good idea to me either. Maybe we'd 
have to add an extra parameter to struct soc_camera_device, but I'm 
somewhat reluctant to change this now, because all those fields from the 
struct will have to disappear anyway with the v4l2-subdev transition, at 
which point, I think, all these requests will have to be passed down to 
drivers.

> Is there some ongoing work regarding the crop implementation on soc_camera?
> If I understand the documentation [1] right, the crop vales should represent
> the area
> of the capture device before scaling. What was the reason for the current
> implementation
> combing crop and fmt values?

See this discussion: 
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/68 
besides, my idea was, that the user cannot be bothered with all scalings / 
croppings that take place in host and client devices (on camera 
controllers and / or sensors). My understanding was: the user uses S_FMT 
to request a window of a certain size, say 640x480, the drivers shall try 
to fit as much into it as possible using scaling. How many hardware pixels 
are now used to build this VGA window the user has no idea about. Then you 
can use S_CROP to select sub-windows from _that_ area. If you want a 
different resolution, you use S_FMT again (after stopping the running 
capture), etc. Any other solution seemed too complicated and impractical 
to me.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: Questinons regarding soc_camera / pxa_camera

[Stefan Herbrechtsmeier]

Guennadi Liakhovetski schrieb:
> (moving to the new v4l list)
>
> On Thu, 26 Mar 2009, Stefan Herbrechtsmeier wrote:
>
>   
>> --- a/linux/drivers/media/video/soc_camera.c    Sun Mar 22 08:53:36 2009 -0300
>> +++ b/linux/drivers/media/video/soc_camera.c    Thu Mar 26 15:35:43 2009 +0100
>> @@ -238,7 +238,7 @@ static int soc_camera_init_user_formats(
>>     icd->num_user_formats = fmts;
>>     fmts = 0;
>>
>> -    dev_dbg(&icd->dev, "Found %d supported formats.\n", fmts);
>> +    dev_dbg(&icd->dev, "Found %d supported formats.\n",
>> icd->num_user_formats);
>>
>>     /* Second pass - actually fill data formats */
>>     for (i = 0; i < icd->num_formats; i++)
>>
>> I thing this was wrong or ' fmts = 0;' must be under the output.
>>     
>
> Right, I'd prefer the latter though - to move the "fmts = 0;" assignment 
> down.
>
>   
Should I generate a PATCH or will you change it by our own?
>> @@ -675,8 +675,8 @@ static int soc_camera_cropcap(struct fil
>>     a->bounds.height        = icd->height_max;
>>     a->defrect.left            = icd->x_min;
>>     a->defrect.top            = icd->y_min;
>> -    a->defrect.width        = DEFAULT_WIDTH;
>> -    a->defrect.height        = DEFAULT_HEIGHT;
>> +    a->defrect.width        = icd->width_max;
>> +    a->defrect.height        = icd->height_max;
>>     a->pixelaspect.numerator    = 1;
>>     a->pixelaspect.denominator    = 1;
>>
>> What was the reason to use fix values? Because of the current implementation
>> of crop,
>> the default value can get bigger than the max value.
>>     
>
> Yes, you're right again, taking scaling into account. Although, setting 
> default to maximum doesn't seem a very good idea to me either. Maybe we'd 
> have to add an extra parameter to struct soc_camera_device, but I'm 
> somewhat reluctant to change this now, because all those fields from the 
> struct will have to disappear anyway with the v4l2-subdev transition, at 
> which point, I think, all these requests will have to be passed down to 
> drivers.
>   
OK, what is the timetable / plan for the soc_camera to v4l2-subdev 
transition?
>> Is there some ongoing work regarding the crop implementation on soc_camera?
>> If I understand the documentation [1] right, the crop vales should represent
>> the area
>> of the capture device before scaling. What was the reason for the current
>> implementation
>> combing crop and fmt values?
>>     
>
> See this discussion: 
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/68 
> besides, my idea was, that the user cannot be bothered with all scalings / 
> croppings that take place in host and client devices (on camera 
> controllers and / or sensors). My understanding was: the user uses S_FMT 
> to request a window of a certain size, say 640x480, the drivers shall try 
> to fit as much into it as possible using scaling. How many hardware pixels 
> are now used to build this VGA window the user has no idea about.
If we use the real pixels for CROP, the user can calculate the scale.
(see 1.11.3 Examples for "Image Cropping, Insertion and Scaling" in the 
documentation)

> Then you 
> can use S_CROP to select sub-windows from _that_ area. If you want a 
> different resolution, you use S_FMT again (after stopping the running 
> capture), etc. 
Do you mean you can use S_CROP during a running capture?
What happen if you change the width or height of the sub-windows. This 
will change the resolution
/ size of the image.

I only know the camera driver side of this functions and don't know how 
it is used,  but I would used
S_FMT do set the output format and S_CROP to select the real sensor size 
and position.
For example S_FMT with 320x240 set the sensor area to 1280x960 (max). 
S_CROP with 600x400 set
the sensor area to 640x480, because this is the next supported scale 
(1,2,4,8) for the fix output format.
If I understand the documentation right, S_CROP would use the old scale 
(4) and change the format to
200x100 to get the requested sensor area.

I think for now I take over your implementation of S_FMT and S_CROP.
After the v4l2-subdev transition we can update the implementations.

Regards
    Stefan

Re: Questinons regarding soc_camera / pxa_camera

[Guennadi Liakhovetski]

On Wed, 1 Apr 2009, Stefan Herbrechtsmeier wrote:

> Guennadi Liakhovetski schrieb:
> > (moving to the new v4l list)
> > 
> > On Thu, 26 Mar 2009, Stefan Herbrechtsmeier wrote:
> > 
> >   
> > > --- a/linux/drivers/media/video/soc_camera.c    Sun Mar 22 08:53:36 2009
> > > -0300
> > > +++ b/linux/drivers/media/video/soc_camera.c    Thu Mar 26 15:35:43 2009
> > > +0100
> > > @@ -238,7 +238,7 @@ static int soc_camera_init_user_formats(
> > >     icd->num_user_formats = fmts;
> > >     fmts = 0;
> > > 
> > > -    dev_dbg(&icd->dev, "Found %d supported formats.\n", fmts);
> > > +    dev_dbg(&icd->dev, "Found %d supported formats.\n",
> > > icd->num_user_formats);
> > > 
> > >     /* Second pass - actually fill data formats */
> > >     for (i = 0; i < icd->num_formats; i++)
> > > 
> > > I thing this was wrong or ' fmts = 0;' must be under the output.
> > >     
> > 
> > Right, I'd prefer the latter though - to move the "fmts = 0;" assignment
> > down.
> > 
> >   
> Should I generate a PATCH or will you change it by our own?

Up to you. Patch is welcome. If you don't make one, I'll make it some time 
later with "Reported-by: <you>."

> > > @@ -675,8 +675,8 @@ static int soc_camera_cropcap(struct fil
> > >     a->bounds.height        = icd->height_max;
> > >     a->defrect.left            = icd->x_min;
> > >     a->defrect.top            = icd->y_min;
> > > -    a->defrect.width        = DEFAULT_WIDTH;
> > > -    a->defrect.height        = DEFAULT_HEIGHT;
> > > +    a->defrect.width        = icd->width_max;
> > > +    a->defrect.height        = icd->height_max;
> > >     a->pixelaspect.numerator    = 1;
> > >     a->pixelaspect.denominator    = 1;
> > > 
> > > What was the reason to use fix values? Because of the current
> > > implementation
> > > of crop,
> > > the default value can get bigger than the max value.
> > >     
> > 
> > Yes, you're right again, taking scaling into account. Although, setting
> > default to maximum doesn't seem a very good idea to me either. Maybe we'd
> > have to add an extra parameter to struct soc_camera_device, but I'm somewhat
> > reluctant to change this now, because all those fields from the struct will
> > have to disappear anyway with the v4l2-subdev transition, at which point, I
> > think, all these requests will have to be passed down to drivers.
> >   
> OK, what is the timetable / plan for the soc_camera to v4l2-subdev transition?

There's none. I have (almost) converted all currently in mainline drivers 
to the "soc-camera as a platform device" scheme, which is the first step 
of the conversion. I'll publish it then, so developers of other platforms 
and drivers could test it. Then I'll start the second step. Besides I'm 
constantly interrupted by other tasks, including the currently running 
merge window. So, on the one hand this work profits from me currently not 
having any paid job, on the other hand it suffers from me having to spend 
time looking for paid jobs:-)

> > > Is there some ongoing work regarding the crop implementation on
> > > soc_camera?
> > > If I understand the documentation [1] right, the crop vales should
> > > represent
> > > the area
> > > of the capture device before scaling. What was the reason for the current
> > > implementation
> > > combing crop and fmt values?
> > >     
> > 
> > See this discussion:
> > http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/68
> > besides, my idea was, that the user cannot be bothered with all scalings /
> > croppings that take place in host and client devices (on camera controllers
> > and / or sensors). My understanding was: the user uses S_FMT to request a
> > window of a certain size, say 640x480, the drivers shall try to fit as much
> > into it as possible using scaling. How many hardware pixels are now used to
> > build this VGA window the user has no idea about.
> If we use the real pixels for CROP, the user can calculate the scale.
> (see 1.11.3 Examples for "Image Cropping, Insertion and Scaling" in the
> documentation)

In the thread that I pointed you at there's a reply from Mauro, which, as 
far as we understood each other:-), confirms my understanding of format 
and cropping functionality. And to me it seems easier for the user to only 
work in one scale. But we can revisit this later, sure.

> > Then you can use S_CROP to select sub-windows from _that_ area. If you want
> > a different resolution, you use S_FMT again (after stopping the running
> > capture), etc. 
> Do you mean you can use S_CROP during a running capture?
> What happen if you change the width or height of the sub-windows. This will
> change the resolution
> / size of the image.
> 
> I only know the camera driver side of this functions and don't know how it is
> used,  but I would used
> S_FMT do set the output format and S_CROP to select the real sensor size and
> position.
> For example S_FMT with 320x240 set the sensor area to 1280x960 (max).

Ok, you mean the user retrieves CROP capabilities, sees 1280x960 as max, 
selects scale 4 and hence an output window of 320x240:

User request		Sensor window		Scale	User window
S_FMT(320x240)		1280x960		4:1	320x240

> S_CROP with 600x400 set
> the sensor area to 640x480, because this is the next supported scale (1,2,4,8)
> for the fix output format.
> If I understand the documentation right, S_CROP would use the old scale (4)
> and change the format to 200x100 to get the requested sensor area.

(wouldn't the user window be 160x120?) Here the user wants a window of 
150x100, so she uses the previously remembered scale of 4 to request 
600x400:

S_CROP(600x400)		640x480			4:1	160x120

> I think for now I take over your implementation of S_FMT and S_CROP.
> After the v4l2-subdev transition we can update the implementations.

With my implementation to achieve 160x120 at scale 4:1 you do just

S_FMT(320x240)		1280x960		4:1	320x240
S_CROP(160x120)		640x480			4:1	160x120

i.e., the user doesn't have to remember the scale. Which, IMHO, is easier 
for the user. Think about a visual "agent": if you change the window size, 
you work in terms of your output window size, i.e., you tell the X-server 
to put a window of 160x120 pixels. With the former approach you have to 
use different sizes for X and for the camera, with the latter one you only 
use one unit for both. That's my naive interpretation anyway:-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: Questinons regarding soc_camera / pxa_camera

[Stefan Herbrechtsmeier]

Guennadi Liakhovetski schrieb:
>>>> Is there some ongoing work regarding the crop implementation on
>>>> soc_camera?
>>>> If I understand the documentation [1] right, the crop vales should
>>>> represent
>>>> the area
>>>> of the capture device before scaling. What was the reason for the current
>>>> implementation
>>>> combing crop and fmt values?
>>>>     
>>>>         
>>> See this discussion:
>>> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/68
>>> besides, my idea was, that the user cannot be bothered with all scalings /
>>> croppings that take place in host and client devices (on camera controllers
>>> and / or sensors). My understanding was: the user uses S_FMT to request a
>>> window of a certain size, say 640x480, the drivers shall try to fit as much
>>> into it as possible using scaling. How many hardware pixels are now used to
>>> build this VGA window the user has no idea about.
>>>       
>> If we use the real pixels for CROP, the user can calculate the scale.
>> (see 1.11.3 Examples for "Image Cropping, Insertion and Scaling" in the
>> documentation)
>>     
>
> In the thread that I pointed you at there's a reply from Mauro, which, as 
> far as we understood each other:-), confirms my understanding of format 
> and cropping functionality. And to me it seems easier for the user to only 
> work in one scale. But we can revisit this later, sure.
>
>   
>>> Then you can use S_CROP to select sub-windows from _that_ area. If you want
>>> a different resolution, you use S_FMT again (after stopping the running
>>> capture), etc. 
>>>       
>> Do you mean you can use S_CROP during a running capture?
>> What happen if you change the width or height of the sub-windows. This will
>> change the resolution
>> / size of the image.
>>
>> I only know the camera driver side of this functions and don't know how it is
>> used,  but I would used
>> S_FMT do set the output format and S_CROP to select the real sensor size and
>> position.
>> For example S_FMT with 320x240 set the sensor area to 1280x960 (max).
>>     
>
> Ok, you mean the user retrieves CROP capabilities, sees 1280x960 as max, 
> selects scale 4 and hence an output window of 320x240:
>
> User request		Sensor window		Scale	User window
> S_FMT(320x240)		1280x960		4:1	320x240
>
>   
>> S_CROP with 600x400 set
>> the sensor area to 640x480, because this is the next supported scale (1,2,4,8)
>> for the fix output format.
>> If I understand the documentation right, S_CROP would use the old scale (4)
>> and change the format to 200x100 to get the requested sensor area.
>>     
>
> (wouldn't the user window be 160x120?) Here the user wants a window of 
> 150x100, so she uses the previously remembered scale of 4 to request 
> 600x400:
>
> S_CROP(600x400)		640x480			4:1	160x120
>   
No, exact this is the difference between your implementation and my 
understanding of CROP.
I interpret user window as a fix value, with can only be change by 
S_FMT. The user wants a window
of 320x240 and sensor area of 600x400, which can not reached by the 
sensor, so the driver adjust the
scale to reach the nearest sensor window to the requested one.

User request                    Sensor window        Scale    User window
S_CROP(600x400)        640x480                  2:1        320x240

But now I understand, why you used the scaled value for the sensor 
window and I agree, that it is the
right way for your implementation.

>> I think for now I take over your implementation of S_FMT and S_CROP.
>> After the v4l2-subdev transition we can update the implementations.
>>     
>
> With my implementation to achieve 160x120 at scale 4:1 you do just
>
> S_FMT(320x240)		1280x960		4:1	320x240
> S_CROP(160x120)		640x480			4:1	160x120
>
> i.e., the user doesn't have to remember the scale. Which, IMHO, is easier 
> for the user. Think about a visual "agent": if you change the window size, 
> you work in terms of your output window size, i.e., you tell the X-server 
> to put a window of 160x120 pixels. With the former approach you have to 
> use different sizes for X and for the camera, with the latter one you only 
> use one unit for both. That's my naive interpretation anyway:-)
>   
The user doesn't have to remember the scale anyway. Only the ways a 
different. You interpret S_CROP
as something like a cutting of the S_FMT window. I interpret S_FMT as a 
output format selection
and S_CROP as a sensor window selection.

S_FMT(160x120)        1280x960        8:1    160x120
S_CROP(640x480)      640x480         4:1    160x120

This approach has the advantage, that you can change the scale (sensor 
window) only by using S_CROP
and that you can get the scale by dividing the FMT values through the 
CROP values.

Regards
    Stefan

Re: S_FMT vs. S_CROP

[Guennadi Liakhovetski]

This question - how S_FMT and S_CROP affest image geometry - has been 
discussed at least twice before - that's only with my participation, don't 
know if and how often it has come up before. But the fact, that in two 
discussions we came up with different results seems to suggest, that this 
is not something trivially known by all except me.

First time I asked this question in this thread

http://www.mail-archive.com/linux-media@vger.kernel.org/msg00052.html

and Mauro replied (see above thread for a complete reply):

On Thu, 8 Jan 2009, Mauro Carvalho Chehab wrote:

> On Wed, 7 Jan 2009 10:14:31 +0100 (CET)
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

[snip]

> > For example on mt9t031 
> > binning and skipping are used for that. Whereas CROP uses the current 
> > scaling configuration and selects a sub-window, so, once you've done S_FMT 
> > to 320x240, a crop request for 640x480 might well fail.
> 
> I also understand this way. You cannot crop with a resolution bigger than what
> you've selected.

(Let's call this statement M1:-))

> > For this you have 
> > to issue a S_FMT, i.e., change scaling. Or would one have to re-scale 
> > transparently?
> > 
> > Is this interpretation correct? It seems to reflect the API as documented 
> > on http://v4l2spec.bytesex.org/spec/book1.htm correctly.
> > 
> > If it is correct, then what should CROP_CAP return as maximum supported 
> > window sizes? Should it return them according to the current scaling or 
> > according to scale 1?
> 
> I understand that it should return against the current scaling.

(and this one M2, whereas I understand it as "current scaling" means 
"current scaling coefficient", not "current scaled output windof")

Then in another thread

http://www.mail-archive.com/linux-media@vger.kernel.org/msg03512.html

Stefan motivated for an incomatibly different interpretation of the 
standard:

On Thu, 2 Apr 2009, Stefan Herbrechtsmeier wrote:

[snip]

> The user doesn't have to remember the scale anyway. Only the ways a different.
> You interpret S_CROP
> as something like a cutting of the S_FMT window. I interpret S_FMT as a output
> format selection
> and S_CROP as a sensor window selection.

which I now interpret as

S_FMT(640x480) means "scale whatever rectangle has been selected on the 
sensor to produce an output window of 640x480" and S_CROP(2048x1536) means 
"take a window of 2048x1536 sensor pixels from the sensor and scale it to 
whatever output window has been or will be selected by S_FMT." This 
contradicts M1, because you certainly can crop a larger (sensor) window. 
Also, I now believe, that [GS]_CROP and, logically, CROPCAP operate in 
sensor pixels and shall not depend on any scales, which contradicts (my 
understanding of) M2.

It now seems to be quite simple to me:

{G,S,TRY}_FMT configure output geometry in user pixels
[GS]_CROP, CROPCAP configure input window in sensor pixels

The thus configured input window should be mapped (scaled) into the output 
window.

Now, which one is correct?

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

Re: S_FMT vs. S_CROP

[Hans Verkuil]

On Wednesday 10 June 2009 18:02:39 Guennadi Liakhovetski wrote:
> This question - how S_FMT and S_CROP affest image geometry - has been
> discussed at least twice before - that's only with my participation,
> don't know if and how often it has come up before. But the fact, that in
> two discussions we came up with different results seems to suggest, that
> this is not something trivially known by all except me.
>
> First time I asked this question in this thread
>
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg00052.html
>
> and Mauro replied (see above thread for a complete reply):
>
> On Thu, 8 Jan 2009, Mauro Carvalho Chehab wrote:
> > On Wed, 7 Jan 2009 10:14:31 +0100 (CET)
> > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
>
> [snip]
>
> > > For example on mt9t031
> > > binning and skipping are used for that. Whereas CROP uses the current
> > > scaling configuration and selects a sub-window, so, once you've done
> > > S_FMT to 320x240, a crop request for 640x480 might well fail.
> >
> > I also understand this way. You cannot crop with a resolution bigger
> > than what you've selected.
>
> (Let's call this statement M1:-))

If I read the spec correctly, in particular section 1.11.1, then cropping 
comes before scaling, so you can crop to 640x480 (S_CROP) and scale that to 
320x240 (S_FMT). S_FMT scales the cropped rectangle.

>
> > > For this you have
> > > to issue a S_FMT, i.e., change scaling. Or would one have to re-scale
> > > transparently?
> > >
> > > Is this interpretation correct? It seems to reflect the API as
> > > documented on http://v4l2spec.bytesex.org/spec/book1.htm correctly.
> > >
> > > If it is correct, then what should CROP_CAP return as maximum
> > > supported window sizes? Should it return them according to the
> > > current scaling or according to scale 1?
> >
> > I understand that it should return against the current scaling.
>
> (and this one M2, whereas I understand it as "current scaling" means
> "current scaling coefficient", not "current scaled output windof")
>
> Then in another thread
>
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg03512.html
>
> Stefan motivated for an incomatibly different interpretation of the
> standard:
>
> On Thu, 2 Apr 2009, Stefan Herbrechtsmeier wrote:
>
> [snip]
>
> > The user doesn't have to remember the scale anyway. Only the ways a
> > different. You interpret S_CROP
> > as something like a cutting of the S_FMT window. I interpret S_FMT as a
> > output format selection
> > and S_CROP as a sensor window selection.
>
> which I now interpret as
>
> S_FMT(640x480) means "scale whatever rectangle has been selected on the
> sensor to produce an output window of 640x480" and S_CROP(2048x1536)
> means "take a window of 2048x1536 sensor pixels from the sensor and scale
> it to whatever output window has been or will be selected by S_FMT." This
> contradicts M1, because you certainly can crop a larger (sensor) window.
> Also, I now believe, that [GS]_CROP and, logically, CROPCAP operate in
> sensor pixels and shall not depend on any scales, which contradicts (my
> understanding of) M2.
>
> It now seems to be quite simple to me:
>
> {G,S,TRY}_FMT configure output geometry in user pixels
> [GS]_CROP, CROPCAP configure input window in sensor pixels

Agreed.

> The thus configured input window should be mapped (scaled) into the
> output window.
>
> Now, which one is correct?

Your interpretation is correct to the best of my knowledge. I think the 
cropping API remains one of the worst explained ioctls in the spec. There 
are some weird things you can get into when dealing with S-Video (PAL/NTSC) 
like signals, but for sensors like this it is as you described.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

Re: S_FMT vs. S_CROP

[Trent Piepho]

On Wed, 10 Jun 2009, Hans Verkuil wrote:
> On Wednesday 10 June 2009 18:02:39 Guennadi Liakhovetski wrote:
> > This question - how S_FMT and S_CROP affest image geometry - has been
> > discussed at least twice before - that's only with my participation,
> > don't know if and how often it has come up before. But the fact, that in
> > two discussions we came up with different results seems to suggest, that
> > this is not something trivially known by all except me.
> >
> > First time I asked this question in this thread
> >
> > http://www.mail-archive.com/linux-media@vger.kernel.org/msg00052.html
> >
> > and Mauro replied (see above thread for a complete reply):
> >
> > On Thu, 8 Jan 2009, Mauro Carvalho Chehab wrote:
> > > On Wed, 7 Jan 2009 10:14:31 +0100 (CET)
> > > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> >
> > [snip]
> >
> > > > For example on mt9t031
> > > > binning and skipping are used for that. Whereas CROP uses the current
> > > > scaling configuration and selects a sub-window, so, once you've done
> > > > S_FMT to 320x240, a crop request for 640x480 might well fail.
> > >
> > > I also understand this way. You cannot crop with a resolution bigger
> > > than what you've selected.
> >
> > (Let's call this statement M1:-))
>
> If I read the spec correctly, in particular section 1.11.1, then cropping
> comes before scaling, so you can crop to 640x480 (S_CROP) and scale that to
> 320x240 (S_FMT). S_FMT scales the cropped rectangle.

This is my understanding of how it's supposed to work as well.

Re: S_FMT vs. S_CROP

[Guennadi Liakhovetski]

On Wed, 10 Jun 2009, Hans Verkuil wrote:

> On Wednesday 10 June 2009 18:02:39 Guennadi Liakhovetski wrote:
> >
> > {G,S,TRY}_FMT configure output geometry in user pixels
> > [GS]_CROP, CROPCAP configure input window in sensor pixels
> 
> Agreed.

Now one more related question: if the driver (stack) can only scale down, 
and the user has requested either a crop smaller than current fmt, or fmt 
larger than current crop. What should the driver do? I can think of at 
least two reasonably logical behaviours:

1. adjust the last invalid request - either enlarge crop or reduce fmt
2. give fmt preference and adjust the source rectangle if needed

Which one is prererred?

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

Re: S_FMT vs. S_CROP

[Guennadi Liakhovetski]

Ok, a couple of things have become clearer to me, some others managed to 
confuse me again:

On Wed, 10 Jun 2009, Hans Verkuil wrote:

> On Wednesday 10 June 2009 18:02:39 Guennadi Liakhovetski wrote:

[snip]

> > which I now interpret as
> >
> > S_FMT(640x480) means "scale whatever rectangle has been selected on the
> > sensor to produce an output window of 640x480" and S_CROP(2048x1536)
> > means "take a window of 2048x1536 sensor pixels from the sensor and scale
> > it to whatever output window has been or will be selected by S_FMT." This
> > contradicts M1, because you certainly can crop a larger (sensor) window.
> > Also, I now believe, that [GS]_CROP and, logically, CROPCAP operate in
> > sensor pixels and shall not depend on any scales, which contradicts (my
> > understanding of) M2.
> >
> > It now seems to be quite simple to me:
> >
> > {G,S,TRY}_FMT configure output geometry in user pixels
> > [GS]_CROP, CROPCAP configure input window in sensor pixels
> 
> Agreed.
> 
> > The thus configured input window should be mapped (scaled) into the
> > output window.
> >
> > Now, which one is correct?
> 
> Your interpretation is correct to the best of my knowledge. I think the 
> cropping API remains one of the worst explained ioctls in the spec. There 
> are some weird things you can get into when dealing with S-Video (PAL/NTSC) 
> like signals, but for sensors like this it is as you described.

It seems, my above interpretation contradicts with the following statement 
from the description of S_CROP in the spec:

<quote>
Second the driver adjusts the image size (the opposite rectangle of the 
scaling process, source or target depending on the data direction) to the 
closest size possible while maintaining the current horizontal and 
vertical scaling factor.
</quote>

I read this as "you crop according to the user request and yor new scaled 
image is a result of your new crop area and _old_ scaling factors."

Now, if you set up the crop, and preserve the scaling factors, what the 
heck does "adjusts the image size ... to the closest size possible"... 
What else adjustment freedom does one have here? Shall I be bending the 
sensor in the third dimension or what?...

And btw, the calculations and reasoning in the example in 1.11.2 I cannot 
follow at all. For example this "The present scaling factors limit 
cropping to 640 × 384" I cannot derive however hard I try. And this "the 
driver returns the cropping size 608 × 384 and adjusts the image size to 
closest possible 304 × 192" means the scaling factors are now both 2:1 as 
a result of S_CROP, and before S_CROP the horisontal scale used to be 1:1, 
so, it changed, which again contradicts (IMHO) S_CROP definition above.

HEEEEELP!

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