[RFC PATCH v2 1/3] v4l2-ctrls: change the boolean type of V4L2_CID_FOCUS_AUTO to menu type

[RFC PATCH v2 1/3] v4l2-ctrls: change the boolean type of V4L2_CID_FOCUS_AUTO to menu type

[Kim, HeungJun]

Support more modes of autofocus, it changes the type of V4L2_CID_FOCUS_AUTO
from boolean to menu. And it includes 4 kinds of enumeration types:

V4L2_FOCUS_AUTO, V4L2_FOCUS_MANUAL, V4L2_FOCUS_MACRO, V4L2_FOCUS_CONTINUOUS

Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/v4l2-ctrls.c |   11 ++++++++++-
 include/linux/videodev2.h        |    6 ++++++
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 2412f08..0b1cce0 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -197,6 +197,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		"Aperture Priority Mode",
 		NULL
 	};
+	static const char * const camera_focus_auto[] = {
+		"Manual Mode",
+		"Auto Mode",
+		"Macro Mode",
+		"Continuous Mode",
+		NULL
+	};
 	static const char * const colorfx[] = {
 		"None",
 		"Black & White",
@@ -252,6 +259,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		return camera_power_line_frequency;
 	case V4L2_CID_EXPOSURE_AUTO:
 		return camera_exposure_auto;
+	case V4L2_CID_FOCUS_AUTO:
+		return camera_focus_auto;
 	case V4L2_CID_COLORFX:
 		return colorfx;
 	case V4L2_CID_TUNE_PREEMPHASIS:
@@ -416,7 +425,6 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_GOP_CLOSURE:
 	case V4L2_CID_MPEG_VIDEO_PULLDOWN:
 	case V4L2_CID_EXPOSURE_AUTO_PRIORITY:
-	case V4L2_CID_FOCUS_AUTO:
 	case V4L2_CID_PRIVACY:
 	case V4L2_CID_AUDIO_LIMITER_ENABLED:
 	case V4L2_CID_AUDIO_COMPRESSION_ENABLED:
@@ -450,6 +458,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_STREAM_TYPE:
 	case V4L2_CID_MPEG_STREAM_VBI_FMT:
 	case V4L2_CID_EXPOSURE_AUTO:
+	case V4L2_CID_FOCUS_AUTO:
 	case V4L2_CID_COLORFX:
 	case V4L2_CID_TUNE_PREEMPHASIS:
 		*type = V4L2_CTRL_TYPE_MENU;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index a94c4d5..959d59e 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1374,6 +1374,12 @@ enum  v4l2_exposure_auto_type {
 #define V4L2_CID_FOCUS_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+10)
 #define V4L2_CID_FOCUS_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+11)
 #define V4L2_CID_FOCUS_AUTO			(V4L2_CID_CAMERA_CLASS_BASE+12)
+enum  v4l2_focus_auto_type {
+	V4L2_FOCUS_MANUAL = 0,
+	V4L2_FOCUS_AUTO = 1,
+	V4L2_FOCUS_MACRO = 2,
+	V4L2_FOCUS_CONTINUOUS = 3
+};
 
 #define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+13)
 #define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+14)
-- 
1.7.0.4

Re: [RFC PATCH v2 1/3] v4l2-ctrls: change the boolean type of V4L2_CID_FOCUS_AUTO to menu type

[Laurent Pinchart]

On Friday 25 February 2011 07:21:27 Kim, HeungJun wrote:
> Support more modes of autofocus, it changes the type of V4L2_CID_FOCUS_AUTO
> from boolean to menu. And it includes 4 kinds of enumeration types:
> 
> V4L2_FOCUS_AUTO, V4L2_FOCUS_MANUAL, V4L2_FOCUS_MACRO, V4L2_FOCUS_CONTINUOUS
> 
> Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/video/v4l2-ctrls.c |   11 ++++++++++-
>  include/linux/videodev2.h        |    6 ++++++
>  2 files changed, 16 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-ctrls.c
> b/drivers/media/video/v4l2-ctrls.c index 2412f08..0b1cce0 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -197,6 +197,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		"Aperture Priority Mode",
>  		NULL
>  	};
> +	static const char * const camera_focus_auto[] = {
> +		"Manual Mode",
> +		"Auto Mode",
> +		"Macro Mode",
> +		"Continuous Mode",

This might be nit-picking, but maybe the menu entries should be named "Manual 
Focus", "Auto Focus", "Macro Focus" and "Continuous Auto Focus". Hans ?

> +		NULL
> +	};
>  	static const char * const colorfx[] = {
>  		"None",
>  		"Black & White",
> @@ -252,6 +259,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		return camera_power_line_frequency;
>  	case V4L2_CID_EXPOSURE_AUTO:
>  		return camera_exposure_auto;
> +	case V4L2_CID_FOCUS_AUTO:
> +		return camera_focus_auto;
>  	case V4L2_CID_COLORFX:
>  		return colorfx;
>  	case V4L2_CID_TUNE_PREEMPHASIS:
> @@ -416,7 +425,6 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type, case V4L2_CID_MPEG_VIDEO_GOP_CLOSURE:
>  	case V4L2_CID_MPEG_VIDEO_PULLDOWN:
>  	case V4L2_CID_EXPOSURE_AUTO_PRIORITY:
> -	case V4L2_CID_FOCUS_AUTO:
>  	case V4L2_CID_PRIVACY:
>  	case V4L2_CID_AUDIO_LIMITER_ENABLED:
>  	case V4L2_CID_AUDIO_COMPRESSION_ENABLED:
> @@ -450,6 +458,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type, case V4L2_CID_MPEG_STREAM_TYPE:
>  	case V4L2_CID_MPEG_STREAM_VBI_FMT:
>  	case V4L2_CID_EXPOSURE_AUTO:
> +	case V4L2_CID_FOCUS_AUTO:
>  	case V4L2_CID_COLORFX:
>  	case V4L2_CID_TUNE_PREEMPHASIS:
>  		*type = V4L2_CTRL_TYPE_MENU;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index a94c4d5..959d59e 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1374,6 +1374,12 @@ enum  v4l2_exposure_auto_type {
>  #define V4L2_CID_FOCUS_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+10)
>  #define V4L2_CID_FOCUS_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+11)
>  #define V4L2_CID_FOCUS_AUTO			(V4L2_CID_CAMERA_CLASS_BASE+12)
> +enum  v4l2_focus_auto_type {
> +	V4L2_FOCUS_MANUAL = 0,
> +	V4L2_FOCUS_AUTO = 1,
> +	V4L2_FOCUS_MACRO = 2,
> +	V4L2_FOCUS_CONTINUOUS = 3
> +};
> 
>  #define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+13)
>  #define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+14)

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH v2 1/3] v4l2-ctrls: change the boolean type of V4L2_CID_FOCUS_AUTO to menu type

[Hans Verkuil]

On Friday, February 25, 2011 10:21:59 Laurent Pinchart wrote:
> On Friday 25 February 2011 07:21:27 Kim, HeungJun wrote:
> > Support more modes of autofocus, it changes the type of V4L2_CID_FOCUS_AUTO
> > from boolean to menu. And it includes 4 kinds of enumeration types:
> > 
> > V4L2_FOCUS_AUTO, V4L2_FOCUS_MANUAL, V4L2_FOCUS_MACRO, V4L2_FOCUS_CONTINUOUS
> > 
> > Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  drivers/media/video/v4l2-ctrls.c |   11 ++++++++++-
> >  include/linux/videodev2.h        |    6 ++++++
> >  2 files changed, 16 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/media/video/v4l2-ctrls.c
> > b/drivers/media/video/v4l2-ctrls.c index 2412f08..0b1cce0 100644
> > --- a/drivers/media/video/v4l2-ctrls.c
> > +++ b/drivers/media/video/v4l2-ctrls.c
> > @@ -197,6 +197,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> >  		"Aperture Priority Mode",
> >  		NULL
> >  	};
> > +	static const char * const camera_focus_auto[] = {
> > +		"Manual Mode",
> > +		"Auto Mode",
> > +		"Macro Mode",
> > +		"Continuous Mode",
> 
> This might be nit-picking, but maybe the menu entries should be named "Manual 
> Focus", "Auto Focus", "Macro Focus" and "Continuous Auto Focus". Hans ?

Yes, that's better. Although I believe that it should be 'Macro Auto Focus',
right?

But if we change this for 'focus' then we need to do the same for the auto
exposure menu which currently also uses the term 'Mode'.

Do you agree?

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: [RFC PATCH v2 1/3] v4l2-ctrls: change the boolean type of V4L2_CID_FOCUS_AUTO to menu type

[Kim, HeungJun]

Hans, and Laurent,

2011-02-25 오후 6:54, Hans Verkuil 쓴 글:
> On Friday, February 25, 2011 10:21:59 Laurent Pinchart wrote:
>> On Friday 25 February 2011 07:21:27 Kim, HeungJun wrote:
>>> Support more modes of autofocus, it changes the type of V4L2_CID_FOCUS_AUTO
>>> from boolean to menu. And it includes 4 kinds of enumeration types:
>>>
>>> V4L2_FOCUS_AUTO, V4L2_FOCUS_MANUAL, V4L2_FOCUS_MACRO, V4L2_FOCUS_CONTINUOUS
>>>
>>> Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>>  drivers/media/video/v4l2-ctrls.c |   11 ++++++++++-
>>>  include/linux/videodev2.h        |    6 ++++++
>>>  2 files changed, 16 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/media/video/v4l2-ctrls.c
>>> b/drivers/media/video/v4l2-ctrls.c index 2412f08..0b1cce0 100644
>>> --- a/drivers/media/video/v4l2-ctrls.c
>>> +++ b/drivers/media/video/v4l2-ctrls.c
>>> @@ -197,6 +197,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>>  		"Aperture Priority Mode",
>>>  		NULL
>>>  	};
>>> +	static const char * const camera_focus_auto[] = {
>>> +		"Manual Mode",
>>> +		"Auto Mode",
>>> +		"Macro Mode",
>>> +		"Continuous Mode",
>>
>> This might be nit-picking, but maybe the menu entries should be named "Manual 
>> Focus", "Auto Focus", "Macro Focus" and "Continuous Auto Focus". Hans ?
> 
> Yes, that's better. Although I believe that it should be 'Macro Auto Focus',
> right?
> 
> But if we change this for 'focus' then we need to do the same for the auto
> exposure menu which currently also uses the term 'Mode'.
> 
> Do you agree?

Although listenning Laurent's opinion first, if my opinion is asked, I agree
using term 'Focus', and agree using term 'Exposure' (is it right??) at the
exposure auto control, too.

If the name is decided, and came to the conclusion, I would modify to maintain
the term 'Focus' in the focus control name.

Regards,
Heungjun Kim

Re: [RFC PATCH v2 1/3] v4l2-ctrls: change the boolean type of V4L2_CID_FOCUS_AUTO to menu type

[Laurent Pinchart]

On Friday 25 February 2011 10:54:02 Hans Verkuil wrote:
> On Friday, February 25, 2011 10:21:59 Laurent Pinchart wrote:
> > On Friday 25 February 2011 07:21:27 Kim, HeungJun wrote:
> > > Support more modes of autofocus, it changes the type of
> > > V4L2_CID_FOCUS_AUTO from boolean to menu. And it includes 4 kinds of
> > > enumeration types:
> > > 
> > > V4L2_FOCUS_AUTO, V4L2_FOCUS_MANUAL, V4L2_FOCUS_MACRO,
> > > V4L2_FOCUS_CONTINUOUS
> > > 
> > > Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > > ---
> > > 
> > >  drivers/media/video/v4l2-ctrls.c |   11 ++++++++++-
> > >  include/linux/videodev2.h        |    6 ++++++
> > >  2 files changed, 16 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/media/video/v4l2-ctrls.c
> > > b/drivers/media/video/v4l2-ctrls.c index 2412f08..0b1cce0 100644
> > > --- a/drivers/media/video/v4l2-ctrls.c
> > > +++ b/drivers/media/video/v4l2-ctrls.c
> > > @@ -197,6 +197,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > > 
> > >  		"Aperture Priority Mode",
> > >  		NULL
> > >  	
> > >  	};
> > > 
> > > +	static const char * const camera_focus_auto[] = {
> > > +		"Manual Mode",
> > > +		"Auto Mode",
> > > +		"Macro Mode",
> > > +		"Continuous Mode",
> > 
> > This might be nit-picking, but maybe the menu entries should be named
> > "Manual Focus", "Auto Focus", "Macro Focus" and "Continuous Auto Focus".
> > Hans ?
> 
> Yes, that's better. Although I believe that it should be 'Macro Auto
> Focus', right?

I suppose so. Heungjun could confirm that.

> But if we change this for 'focus' then we need to do the same for the auto
> exposure menu which currently also uses the term 'Mode'.
> 
> Do you agree?

Auto Mode and Manual Mode could be renamed to Auto Exposure and Manual 
Exposure, but Shutter Priority Exposure and Aperture Priority Exposure don't 
sound right.

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH v2 1/3] v4l2-ctrls: change the boolean type of V4L2_CID_FOCUS_AUTO to menu type

[Kim, HeungJun]

2011-02-25 오후 7:28, Laurent Pinchart 쓴 글:
> On Friday 25 February 2011 10:54:02 Hans Verkuil wrote:
>> On Friday, February 25, 2011 10:21:59 Laurent Pinchart wrote:
>>> On Friday 25 February 2011 07:21:27 Kim, HeungJun wrote:
>>>> Support more modes of autofocus, it changes the type of
>>>> V4L2_CID_FOCUS_AUTO from boolean to menu. And it includes 4 kinds of
>>>> enumeration types:
>>>>
>>>> V4L2_FOCUS_AUTO, V4L2_FOCUS_MANUAL, V4L2_FOCUS_MACRO,
>>>> V4L2_FOCUS_CONTINUOUS
>>>>
>>>> Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
>>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>> ---
>>>>
>>>>  drivers/media/video/v4l2-ctrls.c |   11 ++++++++++-
>>>>  include/linux/videodev2.h        |    6 ++++++
>>>>  2 files changed, 16 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/drivers/media/video/v4l2-ctrls.c
>>>> b/drivers/media/video/v4l2-ctrls.c index 2412f08..0b1cce0 100644
>>>> --- a/drivers/media/video/v4l2-ctrls.c
>>>> +++ b/drivers/media/video/v4l2-ctrls.c
>>>> @@ -197,6 +197,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>>>
>>>>  		"Aperture Priority Mode",
>>>>  		NULL
>>>>  	
>>>>  	};
>>>>
>>>> +	static const char * const camera_focus_auto[] = {
>>>> +		"Manual Mode",
>>>> +		"Auto Mode",
>>>> +		"Macro Mode",
>>>> +		"Continuous Mode",
>>>
>>> This might be nit-picking, but maybe the menu entries should be named
>>> "Manual Focus", "Auto Focus", "Macro Focus" and "Continuous Auto Focus".
>>> Hans ?
>>
>> Yes, that's better. Although I believe that it should be 'Macro Auto
>> Focus', right?
> 
> I suppose so. Heungjun could confirm that.
> 
>> But if we change this for 'focus' then we need to do the same for the auto
>> exposure menu which currently also uses the term 'Mode'.
>>
>> Do you agree?
> 
> Auto Mode and Manual Mode could be renamed to Auto Exposure and Manual 
> Exposure, but Shutter Priority Exposure and Aperture Priority Exposure don't 
> sound right.
> 

I guess the reason why "Shutter Priority Exposure" dosen't always imply auto iris
exposure, but the current name "Shutter Prority Mode" can be imagined including
auto iris. The contrary case seems the same.

Although tt's the different story, the exposure be made up 3 factor
(shutter speed/aperture/iso), but there is not the enum name matching iso factor now.
So, it looks better that the name should be changed all later.

But, it looks good for me the focus name is below:
"Manual Focus"
"Normal Auto Focus"
"Macro Auto Focus"
"Continuous Auto Focus"

If any ideas, I'll re-send the patch series.


Regards,
Heungjun Kim