Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

[Laurent Pinchart]

Hi,

(CC'ing linux-media this time, please discard the previous mail)

On Tuesday 14 December 2010 12:27:32 Kim, HeungJun wrote:
> Hi Laurent and Hans,
> 
> I am working on V4L2 subdev for M5MOLS by Fujitsu.
> and I wanna listen your comments about Auto Focus mode of my ideas.
> the details is in the following link discussed at the past.
> Although the situation(adding the more various functions at the M5MOLS
> or any other MEGA camera sensor, I worked.)is changed,
> so I wanna continue this threads for now.
> 
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg03543.html
> 
> First of all, the at least two more mode of auto-focus exists in the
> M5MOLS camera sensor. So, considering defined V4L2 controls and the
> controls in the M5MOLS, I suggest like this:
> 
> +enum  v4l2_focus_auto_type {
> +	V4L2_FOCUS_AUTO_NORMAL = 0,
> +	V4L2_FOCUS_AUTO_MACRO = 1,
> +	V4L2_FOCUS_AUTO_POSITION = 2,
> +};
> +#define V4L2_CID_FOCUS_POSITION			(V4L2_CID_CAMERA_CLASS_BASE+13)
> 
> -#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+13)
> -#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+14)
> +#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+14)
> +#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+15)
> 
> 
> The M5MOLS(or other recent camera sensor) can have at least 2 mode although
> in any cases : *MACRO* and *NORMAL* mode. plus, M5MOLS supports
> positioning focus mode, AKA. POSITION AF mode.
> 
> The MACRO mode scan short range, and this mode can be used at the
> circumstance in the short distance with object and camera lens. So, It has
> fast lens movement, but the command FOCUSING dosen't works well at the
> long distance object.
> 
> On the other hand, NORMAL mode can this. As the words, It's general and
> normal focus mode. The M5MOLS scan fully in the mode.
> 
> In the Position AF mode, the position(expressed x,y) is given at the
> M5MOLS, and then the M5MOLS focus this area. But, the time given the
> position, is normally touch the lcd screen at the mobile device, in my
> case. If the time is given from button, it's no big problem *when*. But,
> in touch-lcd screen case, the position is read at the touch screen driver,
> before command FOCUS to camera sensor. It's the why I add another
> CID(V4L2_CID_FOCUS_POSITION).

I'm pretty sure that some devices would require a rectangle instead of 
coordinates to define the focus point. Even a rectangle might not be enough. 
It would help if we could get feedback from camera designers here.

Hans, should we add a new control type to pass coordinates/rectangles ? :-)

-- 
Regards,

Laurent Pinchart

Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

[Hans Verkuil]

On Wednesday, December 15, 2010 01:19:43 Laurent Pinchart wrote:
> Hi,
> 
> (CC'ing linux-media this time, please discard the previous mail)
> 
> On Tuesday 14 December 2010 12:27:32 Kim, HeungJun wrote:
> > Hi Laurent and Hans,
> > 
> > I am working on V4L2 subdev for M5MOLS by Fujitsu.
> > and I wanna listen your comments about Auto Focus mode of my ideas.
> > the details is in the following link discussed at the past.
> > Although the situation(adding the more various functions at the M5MOLS
> > or any other MEGA camera sensor, I worked.)is changed,
> > so I wanna continue this threads for now.
> > 
> > http://www.mail-archive.com/linux-media@vger.kernel.org/msg03543.html
> > 
> > First of all, the at least two more mode of auto-focus exists in the
> > M5MOLS camera sensor. So, considering defined V4L2 controls and the
> > controls in the M5MOLS, I suggest like this:
> > 
> > +enum  v4l2_focus_auto_type {
> > +	V4L2_FOCUS_AUTO_NORMAL = 0,
> > +	V4L2_FOCUS_AUTO_MACRO = 1,
> > +	V4L2_FOCUS_AUTO_POSITION = 2,
> > +};
> > +#define V4L2_CID_FOCUS_POSITION			(V4L2_CID_CAMERA_CLASS_BASE+13)
> > 
> > -#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+13)
> > -#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+14)
> > +#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+14)
> > +#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+15)
> > 
> > 
> > The M5MOLS(or other recent camera sensor) can have at least 2 mode although
> > in any cases : *MACRO* and *NORMAL* mode. plus, M5MOLS supports
> > positioning focus mode, AKA. POSITION AF mode.
> > 
> > The MACRO mode scan short range, and this mode can be used at the
> > circumstance in the short distance with object and camera lens. So, It has
> > fast lens movement, but the command FOCUSING dosen't works well at the
> > long distance object.
> > 
> > On the other hand, NORMAL mode can this. As the words, It's general and
> > normal focus mode. The M5MOLS scan fully in the mode.
> > 
> > In the Position AF mode, the position(expressed x,y) is given at the
> > M5MOLS, and then the M5MOLS focus this area. But, the time given the
> > position, is normally touch the lcd screen at the mobile device, in my
> > case. If the time is given from button, it's no big problem *when*. But,
> > in touch-lcd screen case, the position is read at the touch screen driver,
> > before command FOCUS to camera sensor. It's the why I add another
> > CID(V4L2_CID_FOCUS_POSITION).
> 
> I'm pretty sure that some devices would require a rectangle instead of 
> coordinates to define the focus point. Even a rectangle might not be enough. 
> It would help if we could get feedback from camera designers here.
> 
> Hans, should we add a new control type to pass coordinates/rectangles ? :-)

It's a bit tricky actually since QUERYCTRL can return only one set of min/max
values. For coordinates/rectangles we need two sets (horizontal and vertical).

And I think it is important to know the min/max values.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

[Kim, HeungJun]

2010-12-15 오후 4:57, Hans Verkuil 쓴 글:
> On Wednesday, December 15, 2010 01:19:43 Laurent Pinchart wrote:
>> Hi,
>>
>> (CC'ing linux-media this time, please discard the previous mail)
>>
>> On Tuesday 14 December 2010 12:27:32 Kim, HeungJun wrote:
>>> Hi Laurent and Hans,
>>>
>>> I am working on V4L2 subdev for M5MOLS by Fujitsu.
>>> and I wanna listen your comments about Auto Focus mode of my ideas.
>>> the details is in the following link discussed at the past.
>>> Although the situation(adding the more various functions at the M5MOLS
>>> or any other MEGA camera sensor, I worked.)is changed,
>>> so I wanna continue this threads for now.
>>>
>>> http://www.mail-archive.com/linux-media@vger.kernel.org/msg03543.html
>>>
>>> First of all, the at least two more mode of auto-focus exists in the
>>> M5MOLS camera sensor. So, considering defined V4L2 controls and the
>>> controls in the M5MOLS, I suggest like this:
>>>
>>> +enum  v4l2_focus_auto_type {
>>> +	V4L2_FOCUS_AUTO_NORMAL = 0,
>>> +	V4L2_FOCUS_AUTO_MACRO = 1,
>>> +	V4L2_FOCUS_AUTO_POSITION = 2,
>>> +};
>>> +#define V4L2_CID_FOCUS_POSITION			(V4L2_CID_CAMERA_CLASS_BASE+13)
>>>
>>> -#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+13)
>>> -#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+14)
>>> +#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+14)
>>> +#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+15)
>>>
>>>
>>> The M5MOLS(or other recent camera sensor) can have at least 2 mode although
>>> in any cases : *MACRO* and *NORMAL* mode. plus, M5MOLS supports
>>> positioning focus mode, AKA. POSITION AF mode.
>>>
>>> The MACRO mode scan short range, and this mode can be used at the
>>> circumstance in the short distance with object and camera lens. So, It has
>>> fast lens movement, but the command FOCUSING dosen't works well at the
>>> long distance object.
>>>
>>> On the other hand, NORMAL mode can this. As the words, It's general and
>>> normal focus mode. The M5MOLS scan fully in the mode.
>>>
>>> In the Position AF mode, the position(expressed x,y) is given at the
>>> M5MOLS, and then the M5MOLS focus this area. But, the time given the
>>> position, is normally touch the lcd screen at the mobile device, in my
>>> case. If the time is given from button, it's no big problem *when*. But,
>>> in touch-lcd screen case, the position is read at the touch screen driver,
>>> before command FOCUS to camera sensor. It's the why I add another
>>> CID(V4L2_CID_FOCUS_POSITION).
>>
>> I'm pretty sure that some devices would require a rectangle instead of 
>> coordinates to define the focus point. Even a rectangle might not be enough. 
>> It would help if we could get feedback from camera designers here.
>>
>> Hans, should we add a new control type to pass coordinates/rectangles ? :-)
> 
> It's a bit tricky actually since QUERYCTRL can return only one set of min/max
> values. For coordinates/rectangles we need two sets (horizontal and vertical).
> 
> And I think it is important to know the min/max values.
> 
> Regards,
> 
> 	Hans
> 

I agree with that.

So, IMHO, my real ideas is that:

1. It aparts two cid for focus POSITIONING.
   (e.g., V4L2_CID_FOCUS_POSX, V4L2_CID_FOCUS_POSY. )
2. The min/max is changed, responsible for current camera sensor operating mode & size.

The rectangle case is possible for focus mode. But, It can be dealt with this in
the subdev driver, although the camera sensor support rectangle focus.

Thanks.

Regards,
HeungJun Kim

Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

[Laurent Pinchart]

Hi Hans,

On Wednesday 15 December 2010 08:57:29 Hans Verkuil wrote:
> On Wednesday, December 15, 2010 01:19:43 Laurent Pinchart wrote:
> > On Tuesday 14 December 2010 12:27:32 Kim, HeungJun wrote:
> > > Hi Laurent and Hans,
> > > 
> > > I am working on V4L2 subdev for M5MOLS by Fujitsu.
> > > and I wanna listen your comments about Auto Focus mode of my ideas.
> > > the details is in the following link discussed at the past.
> > > Although the situation(adding the more various functions at the M5MOLS
> > > or any other MEGA camera sensor, I worked.)is changed,
> > > so I wanna continue this threads for now.
> > > 
> > > http://www.mail-archive.com/linux-media@vger.kernel.org/msg03543.html
> > > 
> > > First of all, the at least two more mode of auto-focus exists in the
> > > M5MOLS camera sensor. So, considering defined V4L2 controls and the
> > > controls in the M5MOLS, I suggest like this:
> > > 
> > > +enum  v4l2_focus_auto_type {
> > > +	V4L2_FOCUS_AUTO_NORMAL = 0,
> > > +	V4L2_FOCUS_AUTO_MACRO = 1,
> > > +	V4L2_FOCUS_AUTO_POSITION = 2,
> > > +};
> > > +#define V4L2_CID_FOCUS_POSITION			(V4L2_CID_CAMERA_CLASS_BASE+13)
> > > 
> > > -#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+13)
> > > -#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+14)
> > > +#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+14)
> > > +#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+15)
> > > 
> > > 
> > > The M5MOLS(or other recent camera sensor) can have at least 2 mode
> > > although in any cases : *MACRO* and *NORMAL* mode. plus, M5MOLS
> > > supports positioning focus mode, AKA. POSITION AF mode.
> > > 
> > > The MACRO mode scan short range, and this mode can be used at the
> > > circumstance in the short distance with object and camera lens. So, It
> > > has fast lens movement, but the command FOCUSING dosen't works well at
> > > the long distance object.
> > > 
> > > On the other hand, NORMAL mode can this. As the words, It's general and
> > > normal focus mode. The M5MOLS scan fully in the mode.
> > > 
> > > In the Position AF mode, the position(expressed x,y) is given at the
> > > M5MOLS, and then the M5MOLS focus this area. But, the time given the
> > > position, is normally touch the lcd screen at the mobile device, in my
> > > case. If the time is given from button, it's no big problem *when*.
> > > But, in touch-lcd screen case, the position is read at the touch
> > > screen driver, before command FOCUS to camera sensor. It's the why I
> > > add another CID(V4L2_CID_FOCUS_POSITION).
> > 
> > I'm pretty sure that some devices would require a rectangle instead of
> > coordinates to define the focus point. Even a rectangle might not be
> > enough. It would help if we could get feedback from camera designers
> > here.
> > 
> > Hans, should we add a new control type to pass coordinates/rectangles ?
> > :-)
> 
> It's a bit tricky actually since QUERYCTRL can return only one set of
> min/max values. For coordinates/rectangles we need two sets (horizontal
> and vertical).
> 
> And I think it is important to know the min/max values.

Hence my question, should we add a way to pass rectangles (basically a struct 
v4l2_rect) through the control ioctls ? It would make sense.

-- 
Regards,

Laurent Pinchart

Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

[Hans Verkuil]

> Hi Hans,
>
> On Wednesday 15 December 2010 08:57:29 Hans Verkuil wrote:
>> On Wednesday, December 15, 2010 01:19:43 Laurent Pinchart wrote:
>> > On Tuesday 14 December 2010 12:27:32 Kim, HeungJun wrote:
>> > > Hi Laurent and Hans,
>> > >
>> > > I am working on V4L2 subdev for M5MOLS by Fujitsu.
>> > > and I wanna listen your comments about Auto Focus mode of my ideas.
>> > > the details is in the following link discussed at the past.
>> > > Although the situation(adding the more various functions at the
>> M5MOLS
>> > > or any other MEGA camera sensor, I worked.)is changed,
>> > > so I wanna continue this threads for now.
>> > >
>> > > http://www.mail-archive.com/linux-media@vger.kernel.org/msg03543.html
>> > >
>> > > First of all, the at least two more mode of auto-focus exists in the
>> > > M5MOLS camera sensor. So, considering defined V4L2 controls and the
>> > > controls in the M5MOLS, I suggest like this:
>> > >
>> > > +enum  v4l2_focus_auto_type {
>> > > +	V4L2_FOCUS_AUTO_NORMAL = 0,
>> > > +	V4L2_FOCUS_AUTO_MACRO = 1,
>> > > +	V4L2_FOCUS_AUTO_POSITION = 2,
>> > > +};
>> > > +#define V4L2_CID_FOCUS_POSITION			(V4L2_CID_CAMERA_CLASS_BASE+13)
>> > >
>> > > -#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+13)
>> > > -#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+14)
>> > > +#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+14)
>> > > +#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+15)
>> > >
>> > >
>> > > The M5MOLS(or other recent camera sensor) can have at least 2 mode
>> > > although in any cases : *MACRO* and *NORMAL* mode. plus, M5MOLS
>> > > supports positioning focus mode, AKA. POSITION AF mode.
>> > >
>> > > The MACRO mode scan short range, and this mode can be used at the
>> > > circumstance in the short distance with object and camera lens. So,
>> It
>> > > has fast lens movement, but the command FOCUSING dosen't works well
>> at
>> > > the long distance object.
>> > >
>> > > On the other hand, NORMAL mode can this. As the words, It's general
>> and
>> > > normal focus mode. The M5MOLS scan fully in the mode.
>> > >
>> > > In the Position AF mode, the position(expressed x,y) is given at the
>> > > M5MOLS, and then the M5MOLS focus this area. But, the time given the
>> > > position, is normally touch the lcd screen at the mobile device, in
>> my
>> > > case. If the time is given from button, it's no big problem *when*.
>> > > But, in touch-lcd screen case, the position is read at the touch
>> > > screen driver, before command FOCUS to camera sensor. It's the why I
>> > > add another CID(V4L2_CID_FOCUS_POSITION).
>> >
>> > I'm pretty sure that some devices would require a rectangle instead of
>> > coordinates to define the focus point. Even a rectangle might not be
>> > enough. It would help if we could get feedback from camera designers
>> > here.
>> >
>> > Hans, should we add a new control type to pass coordinates/rectangles
>> ?
>> > :-)
>>
>> It's a bit tricky actually since QUERYCTRL can return only one set of
>> min/max values. For coordinates/rectangles we need two sets (horizontal
>> and vertical).
>>
>> And I think it is important to know the min/max values.
>
> Hence my question, should we add a way to pass rectangles (basically a
> struct
> v4l2_rect) through the control ioctls ? It would make sense.

I thought it over and came to the conclusion that we should not do that.
Instead we can create four separate controls.

The problem we run into when adding more complex types is that we can no
longer communicate min and max values (something that we definitely want
when dealing with coordinates).

Another reason is how the control mechanism is designed: they only support
the basic types (int, bool, string, enum, int64 and a 'button' aka
action). And the controls are grouped into classes which are named through
the 'ctrl_class' control.

So effectively controls represent a field in a class (or struct) and each
class can be presented as a tab page in a control panel.

Simple and straightforward.

If we start to add complex types, then it becomes really hard to define
the meta data of the control since you are really defining a 'mini-class'.

It sounds nice initially, but we really should not do this since I believe
it will lead to chaos later on. You want complex types, then use ioctls,
not controls. Or split up the complex type into multiple simple types.

Regards,

        Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

[Laurent Pinchart]

Hi Hans,

On Wednesday 15 December 2010 11:03:37 Hans Verkuil wrote:
> > On Wednesday 15 December 2010 08:57:29 Hans Verkuil wrote:
> >
> > Hence my question, should we add a way to pass rectangles (basically a
> > structv4l2_rect) through the control ioctls ? It would make sense.
> 
> I thought it over and came to the conclusion that we should not do that.
> Instead we can create four separate controls.

That's not very clean, is it ?

> The problem we run into when adding more complex types is that we can no
> longer communicate min and max values (something that we definitely want
> when dealing with coordinates).

Why not ? We should still support querying min/max/def values on a struct 
v4l2_ctrl. This would of course require an extended queryctrl ioctl.

> Another reason is how the control mechanism is designed: they only support
> the basic types (int, bool, string, enum, int64 and a 'button' aka
> action). And the controls are grouped into classes which are named through
> the 'ctrl_class' control.
> 
> So effectively controls represent a field in a class (or struct) and each
> class can be presented as a tab page in a control panel.
> 
> Simple and straightforward.
> 
> If we start to add complex types, then it becomes really hard to define
> the meta data of the control since you are really defining a 'mini-class'.

I agree that arbitrary complex controls might not be a good idea, but 
v4l2_rect is a pretty standard data structure in V4L2 and something that can 
be useful for different controls.

> It sounds nice initially, but we really should not do this since I believe
> it will lead to chaos later on. You want complex types, then use ioctls,
> not controls. Or split up the complex type into multiple simple types.

Then let's use an ioctl for focus control. I don't like it.

-- 
Regards,

Laurent Pinchart

Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

[Hans Verkuil]

On Thursday, December 23, 2010 10:19:38 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Wednesday 15 December 2010 11:03:37 Hans Verkuil wrote:
> > > On Wednesday 15 December 2010 08:57:29 Hans Verkuil wrote:
> > >
> > > Hence my question, should we add a way to pass rectangles (basically a
> > > structv4l2_rect) through the control ioctls ? It would make sense.
> > 
> > I thought it over and came to the conclusion that we should not do that.
> > Instead we can create four separate controls.
> 
> That's not very clean, is it ?

Why not? It's perfectly consistent with the way controls work.

> > The problem we run into when adding more complex types is that we can no
> > longer communicate min and max values (something that we definitely want
> > when dealing with coordinates).
> 
> Why not ? We should still support querying min/max/def values on a struct 
> v4l2_ctrl. This would of course require an extended queryctrl ioctl.

Which will make apps even more complicated. I thought about this as well, but
it's a road that will lead to chaos.
 
> > Another reason is how the control mechanism is designed: they only support
> > the basic types (int, bool, string, enum, int64 and a 'button' aka
> > action). And the controls are grouped into classes which are named through
> > the 'ctrl_class' control.
> > 
> > So effectively controls represent a field in a class (or struct) and each
> > class can be presented as a tab page in a control panel.
> > 
> > Simple and straightforward.
> > 
> > If we start to add complex types, then it becomes really hard to define
> > the meta data of the control since you are really defining a 'mini-class'.
> 
> I agree that arbitrary complex controls might not be a good idea, but 
> v4l2_rect is a pretty standard data structure in V4L2 and something that can 
> be useful for different controls.
> 
> > It sounds nice initially, but we really should not do this since I believe
> > it will lead to chaos later on. You want complex types, then use ioctls,
> > not controls. Or split up the complex type into multiple simple types.
> 
> Then let's use an ioctl for focus control. I don't like it.

I would vote for 4 controls. Since FOCUS_MODE_AUTO is a control it makes no
sense to make an ioctl to set the rectangle. That would be a strange mix.

And having 4 controls for the rectangle will actually look good in GUIs, and
with S_EXT_CTRLS you can set all focus-related controls in one call.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

[Laurent Pinchart]

Hi Hans,

On Thursday 23 December 2010 10:31:19 Hans Verkuil wrote:
> On Thursday, December 23, 2010 10:19:38 Laurent Pinchart wrote:
> > On Wednesday 15 December 2010 11:03:37 Hans Verkuil wrote:
> > > > On Wednesday 15 December 2010 08:57:29 Hans Verkuil wrote:
> > > > 
> > > > Hence my question, should we add a way to pass rectangles (basically
> > > > a structv4l2_rect) through the control ioctls ? It would make sense.
> > > 
> > > I thought it over and came to the conclusion that we should not do
> > > that. Instead we can create four separate controls.
> > 
> > That's not very clean, is it ?
> 
> Why not? It's perfectly consistent with the way controls work.

Because it multiplies the number of required controls by 4. There's also no 
way for applications to know that the controls are grouped together.

> > > The problem we run into when adding more complex types is that we can
> > > no longer communicate min and max values (something that we definitely
> > > want when dealing with coordinates).
> > 
> > Why not ? We should still support querying min/max/def values on a struct
> > v4l2_ctrl. This would of course require an extended queryctrl ioctl.
> 
> Which will make apps even more complicated. I thought about this as well,
> but it's a road that will lead to chaos.

Sorry, I don't see why a v4l2_rect type will lead to chaos. That doesn't mean 
we need to support any data structure as control data, just a very limited 
number V4L2 standard ones.

> > > Another reason is how the control mechanism is designed: they only
> > > support the basic types (int, bool, string, enum, int64 and a 'button'
> > > aka action). And the controls are grouped into classes which are named
> > > through the 'ctrl_class' control.
> > > 
> > > So effectively controls represent a field in a class (or struct) and
> > > each class can be presented as a tab page in a control panel.
> > > 
> > > Simple and straightforward.
> > > 
> > > If we start to add complex types, then it becomes really hard to define
> > > the meta data of the control since you are really defining a
> > > 'mini-class'.
> > 
> > I agree that arbitrary complex controls might not be a good idea, but
> > v4l2_rect is a pretty standard data structure in V4L2 and something that
> > can be useful for different controls.
> > 
> > > It sounds nice initially, but we really should not do this since I
> > > believe it will lead to chaos later on. You want complex types, then
> > > use ioctls, not controls. Or split up the complex type into multiple
> > > simple types.
> > 
> > Then let's use an ioctl for focus control. I don't like it.
> 
> I would vote for 4 controls. Since FOCUS_MODE_AUTO is a control it makes no
> sense to make an ioctl to set the rectangle. That would be a strange mix.
> 
> And having 4 controls for the rectangle will actually look good in GUIs,

Applications will display the controls without knowing they are related. 
Having a v4l2_rect type would actually allow applications to display the 
control better.

> and with S_EXT_CTRLS you can set all focus-related controls in one call.

Then don't tell me that an extended queryctrl ioctl would make apps even more 
complicated, when you advocate using four separate values to change a single 
control ;-)

-- 
Regards,

Laurent Pinchart

Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

[Hans Verkuil]

On Thursday, December 23, 2010 10:38:31 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Thursday 23 December 2010 10:31:19 Hans Verkuil wrote:
> > On Thursday, December 23, 2010 10:19:38 Laurent Pinchart wrote:
> > > On Wednesday 15 December 2010 11:03:37 Hans Verkuil wrote:
> > > > > On Wednesday 15 December 2010 08:57:29 Hans Verkuil wrote:
> > > > > 
> > > > > Hence my question, should we add a way to pass rectangles (basically
> > > > > a structv4l2_rect) through the control ioctls ? It would make sense.
> > > > 
> > > > I thought it over and came to the conclusion that we should not do
> > > > that. Instead we can create four separate controls.
> > > 
> > > That's not very clean, is it ?
> > 
> > Why not? It's perfectly consistent with the way controls work.
> 
> Because it multiplies the number of required controls by 4. There's also no 
> way for applications to know that the controls are grouped together.

4 values, 4 controls. Simple.

> > > > The problem we run into when adding more complex types is that we can
> > > > no longer communicate min and max values (something that we definitely
> > > > want when dealing with coordinates).
> > > 
> > > Why not ? We should still support querying min/max/def values on a struct
> > > v4l2_ctrl. This would of course require an extended queryctrl ioctl.
> > 
> > Which will make apps even more complicated. I thought about this as well,
> > but it's a road that will lead to chaos.
> 
> Sorry, I don't see why a v4l2_rect type will lead to chaos. That doesn't mean 
> we need to support any data structure as control data, just a very limited 
> number V4L2 standard ones.

For every new control type applications will have to add support for that.
Especially for GUIs that is actually a lot of work. Do you want to take on the
job of modifying all V4L2 apps whenever you add a new type? I don't. Adding new
controls is easy and cheap, adding new types is hard and difficult.

The STRING and INTEGER64 types were the first types added in a long time (and
I'm sorry I added INTEGER64 since it is actually not used by any driver at the
moment). And it was a lot of work adding string support to v4l2-ctl and qv4l2.

And I think these are still the only apps in existence that support these two
types.
 
> > > > Another reason is how the control mechanism is designed: they only
> > > > support the basic types (int, bool, string, enum, int64 and a 'button'
> > > > aka action). And the controls are grouped into classes which are named
> > > > through the 'ctrl_class' control.
> > > > 
> > > > So effectively controls represent a field in a class (or struct) and
> > > > each class can be presented as a tab page in a control panel.
> > > > 
> > > > Simple and straightforward.
> > > > 
> > > > If we start to add complex types, then it becomes really hard to define
> > > > the meta data of the control since you are really defining a
> > > > 'mini-class'.
> > > 
> > > I agree that arbitrary complex controls might not be a good idea, but
> > > v4l2_rect is a pretty standard data structure in V4L2 and something that
> > > can be useful for different controls.
> > > 
> > > > It sounds nice initially, but we really should not do this since I
> > > > believe it will lead to chaos later on. You want complex types, then
> > > > use ioctls, not controls. Or split up the complex type into multiple
> > > > simple types.
> > > 
> > > Then let's use an ioctl for focus control. I don't like it.
> > 
> > I would vote for 4 controls. Since FOCUS_MODE_AUTO is a control it makes no
> > sense to make an ioctl to set the rectangle. That would be a strange mix.
> > 
> > And having 4 controls for the rectangle will actually look good in GUIs,
> 
> Applications will display the controls without knowing they are related. 
> Having a v4l2_rect type would actually allow applications to display the 
> control better.

As I said, are you going to add support for this in all apps? Especially since
many apps just show a list of controls, so if you just ensure that the four
rect controls come one after the other, then they will show up just fine.

> > and with S_EXT_CTRLS you can set all focus-related controls in one call.
> 
> Then don't tell me that an extended queryctrl ioctl would make apps even more 
> complicated, when you advocate using four separate values to change a single 
> control ;-)

???

Most generic apps just enumerate controls, so 1 or 100 controls doesn't matter
to them.

Adding a new type does, though.

Just add four controls for this. It works everywhere and doesn't require any new
APIs and application changes.

Regards,

	Hans

Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

[Kim, HeungJun]

Hi Laurent,

2010-12-15 오전 9:19, Laurent Pinchart 쓴 글:
> Hi,
> 
> (CC'ing linux-media this time, please discard the previous mail)
> 
> On Tuesday 14 December 2010 12:27:32 Kim, HeungJun wrote:
>> Hi Laurent and Hans,
>>
>> I am working on V4L2 subdev for M5MOLS by Fujitsu.
>> and I wanna listen your comments about Auto Focus mode of my ideas.
>> the details is in the following link discussed at the past.
>> Although the situation(adding the more various functions at the M5MOLS
>> or any other MEGA camera sensor, I worked.)is changed,
>> so I wanna continue this threads for now.
>>
>> http://www.mail-archive.com/linux-media@vger.kernel.org/msg03543.html
>>
>> First of all, the at least two more mode of auto-focus exists in the
>> M5MOLS camera sensor. So, considering defined V4L2 controls and the
>> controls in the M5MOLS, I suggest like this:
>>
>> +enum  v4l2_focus_auto_type {
>> +	V4L2_FOCUS_AUTO_NORMAL = 0,
>> +	V4L2_FOCUS_AUTO_MACRO = 1,
>> +	V4L2_FOCUS_AUTO_POSITION = 2,
>> +};
>> +#define V4L2_CID_FOCUS_POSITION			(V4L2_CID_CAMERA_CLASS_BASE+13)
>>
>> -#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+13)
>> -#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+14)
>> +#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+14)
>> +#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+15)
>>
>>
>> The M5MOLS(or other recent camera sensor) can have at least 2 mode although
>> in any cases : *MACRO* and *NORMAL* mode. plus, M5MOLS supports
>> positioning focus mode, AKA. POSITION AF mode.
>>
>> The MACRO mode scan short range, and this mode can be used at the
>> circumstance in the short distance with object and camera lens. So, It has
>> fast lens movement, but the command FOCUSING dosen't works well at the
>> long distance object.
>>
>> On the other hand, NORMAL mode can this. As the words, It's general and
>> normal focus mode. The M5MOLS scan fully in the mode.
>>
>> In the Position AF mode, the position(expressed x,y) is given at the
>> M5MOLS, and then the M5MOLS focus this area. But, the time given the
>> position, is normally touch the lcd screen at the mobile device, in my
>> case. If the time is given from button, it's no big problem *when*. But,
>> in touch-lcd screen case, the position is read at the touch screen driver,
>> before command FOCUS to camera sensor. It's the why I add another
>> CID(V4L2_CID_FOCUS_POSITION).
> 
> I'm pretty sure that some devices would require a rectangle instead of 
> coordinates to define the focus point. Even a rectangle might not be enough. 
> It would help if we could get feedback from camera designers here.
> 
> Hans, should we add a new control type to pass coordinates/rectangles ? :-)
> 

Very glad to be sure that.

As you know, the recent camera sensor embedded in mobile devices has evoluted 
rapidly in a decade. It's not digital camera, but it operates like digital
camera. Actually, the camera sensor module with ISP in the recent mobile device
use the same one in the digital camera. And I can let you know this newer
control types, like in a uppper FOCUS case.(e.g.,iso, exposure, wb, wdr(wide
dynamic range), effects, the method to get jpeg bulk streams with sync, even
face detections.)

So, I'll make general patch or RFC patch about new control types which is needed at
the the mobile device, based on M5MOLS and some sensors else, for generality.
(considering another ISP like a NEC, Samsung sensor modules. It is available for me.)

After that, I'm glad with being reviewed it to Hans and Laurent.
(Actually, I don't know who is the maintainer of CID of camera. Let me know, plz. :-) )

If Laurent and Hans agree with that, I'll prepare patch works.

Thanks for reading.

ps. I wanna know where the recent v4l2 control is described, as already told 
at the previous my mail. 


Regards,
HeungJun Kim

Re: What if add enumerations at the V4L2_FOCUS_MODE_AUTO?

[Hans Verkuil]

> Hi Laurent,
>
> 2010-12-15 ¿ÀÀü 9:19, Laurent Pinchart ¾´ ±Û:
>> Hi,
>>
>> (CC'ing linux-media this time, please discard the previous mail)
>>
>> On Tuesday 14 December 2010 12:27:32 Kim, HeungJun wrote:
>>> Hi Laurent and Hans,
>>>
>>> I am working on V4L2 subdev for M5MOLS by Fujitsu.
>>> and I wanna listen your comments about Auto Focus mode of my ideas.
>>> the details is in the following link discussed at the past.
>>> Although the situation(adding the more various functions at the M5MOLS
>>> or any other MEGA camera sensor, I worked.)is changed,
>>> so I wanna continue this threads for now.
>>>
>>> http://www.mail-archive.com/linux-media@vger.kernel.org/msg03543.html
>>>
>>> First of all, the at least two more mode of auto-focus exists in the
>>> M5MOLS camera sensor. So, considering defined V4L2 controls and the
>>> controls in the M5MOLS, I suggest like this:
>>>
>>> +enum  v4l2_focus_auto_type {
>>> +	V4L2_FOCUS_AUTO_NORMAL = 0,
>>> +	V4L2_FOCUS_AUTO_MACRO = 1,
>>> +	V4L2_FOCUS_AUTO_POSITION = 2,
>>> +};
>>> +#define V4L2_CID_FOCUS_POSITION			(V4L2_CID_CAMERA_CLASS_BASE+13)

Something I forgot to mention before: new controls should be added at the
end of the control class. These IDs can be used directly in applications,
so if you shift them, then apps will start to fail.

>>>
>>> -#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+13)
>>> -#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+14)
>>> +#define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+14)
>>> +#define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+15)
>>>
>>>
>>> The M5MOLS(or other recent camera sensor) can have at least 2 mode
>>> although
>>> in any cases : *MACRO* and *NORMAL* mode. plus, M5MOLS supports
>>> positioning focus mode, AKA. POSITION AF mode.
>>>
>>> The MACRO mode scan short range, and this mode can be used at the
>>> circumstance in the short distance with object and camera lens. So, It
>>> has
>>> fast lens movement, but the command FOCUSING dosen't works well at the
>>> long distance object.
>>>
>>> On the other hand, NORMAL mode can this. As the words, It's general and
>>> normal focus mode. The M5MOLS scan fully in the mode.
>>>
>>> In the Position AF mode, the position(expressed x,y) is given at the
>>> M5MOLS, and then the M5MOLS focus this area. But, the time given the
>>> position, is normally touch the lcd screen at the mobile device, in my
>>> case. If the time is given from button, it's no big problem *when*.
>>> But,
>>> in touch-lcd screen case, the position is read at the touch screen
>>> driver,
>>> before command FOCUS to camera sensor. It's the why I add another
>>> CID(V4L2_CID_FOCUS_POSITION).
>>
>> I'm pretty sure that some devices would require a rectangle instead of
>> coordinates to define the focus point. Even a rectangle might not be
>> enough.
>> It would help if we could get feedback from camera designers here.
>>
>> Hans, should we add a new control type to pass coordinates/rectangles ?
>> :-)
>>
>
> Very glad to be sure that.
>
> As you know, the recent camera sensor embedded in mobile devices has
> evoluted
> rapidly in a decade. It's not digital camera, but it operates like digital
> camera. Actually, the camera sensor module with ISP in the recent mobile
> device
> use the same one in the digital camera. And I can let you know this newer
> control types, like in a uppper FOCUS case.(e.g.,iso, exposure, wb,
> wdr(wide
> dynamic range), effects, the method to get jpeg bulk streams with sync,
> even
> face detections.)
>
> So, I'll make general patch or RFC patch about new control types which is
> needed at
> the the mobile device, based on M5MOLS and some sensors else, for
> generality.
> (considering another ISP like a NEC, Samsung sensor modules. It is
> available for me.)

Good plan.

> After that, I'm glad with being reviewed it to Hans and Laurent.
> (Actually, I don't know who is the maintainer of CID of camera. Let me
> know, plz. :-) )

You have the right people :-)

> If Laurent and Hans agree with that, I'll prepare patch works.
>
> Thanks for reading.
>
> ps. I wanna know where the recent v4l2 control is described, as already
> told
> at the previous my mail.

The latest spec build from the git tree is always available here:

www.xs4all.nl/~hverkuil/spec/media.html

And of course in the media_tree git repository itself.

Regards,

       Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco