| Message ID | 1355147019-25375-3-git-send-email-a.hajda@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andrzej and Sylwester, Thanks for the patch! On Mon, Dec 10, 2012 at 02:43:39PM +0100, Andrzej Hajda wrote: > From: Sylwester Nawrocki <s.nawrocki@samsung.com> > > Add control for automatic focus area selection. > This control determines the area of the frame that the camera uses > for automatic focus. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > Documentation/DocBook/media/v4l/compat.xml | 9 +++-- > Documentation/DocBook/media/v4l/controls.xml | 47 +++++++++++++++++++++++++- > Documentation/DocBook/media/v4l/v4l2.xml | 7 ++++ > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ > include/uapi/linux/v4l2-controls.h | 6 ++++ > 5 files changed, 76 insertions(+), 3 deletions(-) > > diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml > index 4fdf6b5..e8b53da 100644 > --- a/Documentation/DocBook/media/v4l/compat.xml > +++ b/Documentation/DocBook/media/v4l/compat.xml > @@ -2452,8 +2452,9 @@ that used it. It was originally scheduled for removal in 2.6.35. > <constant>V4L2_CID_3A_LOCK</constant>, > <constant>V4L2_CID_AUTO_FOCUS_START</constant>, > <constant>V4L2_CID_AUTO_FOCUS_STOP</constant>, > - <constant>V4L2_CID_AUTO_FOCUS_STATUS</constant> and > - <constant>V4L2_CID_AUTO_FOCUS_RANGE</constant>. > + <constant>V4L2_CID_AUTO_FOCUS_STATUS</constant>, > + <constant>V4L2_CID_AUTO_FOCUS_RANGE</constant> and > + <constant>V4L2_CID_AUTO_FOCUS_AREA</constant>. > </para> > </listitem> > </orderedlist> > @@ -2586,6 +2587,10 @@ ioctls.</para> > <para>Vendor and device specific media bus pixel formats. > <xref linkend="v4l2-mbus-vendor-spec-fmts" />.</para> > </listitem> > + <listitem> > + <para><link linkend="v4l2-auto-focus-area"><constant> > + V4L2_CID_AUTO_FOCUS_AREA</constant></link> control.</para> > + </listitem> > </itemizedlist> > </section> > > diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml > index 7fe5be1..9d4af8a 100644 > --- a/Documentation/DocBook/media/v4l/controls.xml > +++ b/Documentation/DocBook/media/v4l/controls.xml > @@ -3347,6 +3347,51 @@ use its minimum possible distance for auto focus.</entry> > </row> > <row><entry></entry></row> > > + <row id="v4l2-auto-focus-area"> > + <entry spanname="id"> > + <constant>V4L2_CID_AUTO_FOCUS_AREA</constant> </entry> > + <entry>enum v4l2_auto_focus_area</entry> > + </row> > + <row><entry spanname="descr">Determines the area of the frame that > +the camera uses for automatic focus. The corresponding coordinates of the > +focusing spot or rectangle can be specified and queried using the selection API. > +To change the auto focus region of interest applications first select required > +mode of this control and then set the rectangle or spot coordinates by means > +of the &VIDIOC-SUBDEV-S-SELECTION; or &VIDIOC-S-SELECTION; ioctl. In order to > +trigger again a one shot auto focus with same coordinates applications should > +use the <constant>V4L2_CID_AUTO_FOCUS_START </constant> control. Or alternatively Extra space above. ^ > +invoke a &VIDIOC-SUBDEV-S-SELECTION; or a &VIDIOC-S-SELECTION; ioctl again. How about requiring explicit V4L2_CID_AUTO_FOCUS_START? If you need to specify several AF selection windows, then on which one do you start the algorithm? A bitmask control explicitly telling which ones are active would also be needed --- but that's for the future. I think now we just need to ascertain we don't make that difficult. :-) > +In the latter case the new pixel coordinates are applied to hardware only when > +the focus area control was set to > +<constant>V4L2_AUTO_FOCUS_AREA_RECTANGLE</constant>.</entry> > + </row> > + <row> > + <entrytbl spanname="descr" cols="2"> > + <tbody valign="top"> > + <row> > + <entry><constant>V4L2_AUTO_FOCUS_AREA_ALL</constant> </entry> > + <entry>Normal auto focus, the focusing area extends over the > +entire frame.</entry> Does this need to be explicitly specified? Shouldn't the user just choose the largest possible AF window instead? I'd even expect that the AF window might span the whole frame by default (up to driver, hardware etc.). > + </row> > + <row> > + <entry><constant>V4L2_AUTO_FOCUS_AREA_RECTANGLE</constant> </entry> > + <entry>The auto focus region of interest is determined by the > +<constant>V4L2_SEL_TGT_AUTO_FOCUS</constant> selection rectangle.</entry> It's not strictly required in documentation (and that shows) but it'd be nice to align the paragraphs at equal indentation. > + </row> > + <row> > + <entry><constant>V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION</constant> </entry> > + <entry>The auto focus region of interest is determined > +by an object (e.g. face) detection engine.</entry> > + </row> > + </tbody> > + </entrytbl> > + </row> > + <row><entry spanname="descr"> > + This is an <link linkend="experimental">experimental</link> > +control and may change in the future.</entry> > + </row> > + <row><entry></entry></row> > + > <row> > <entry spanname="id"><constant>V4L2_CID_ZOOM_ABSOLUTE</constant> </entry> > <entry>integer</entry> > @@ -3986,7 +4031,7 @@ interface and may change in the future.</para> > > <table pgwide="1" frame="none" id="flash-control-id"> > <title>Flash Control IDs</title> > - > + > <tgroup cols="4"> > <colspec colname="c1" colwidth="1*" /> > <colspec colname="c2" colwidth="6*" /> > diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml > index 10ccde9..1477540 100644 > --- a/Documentation/DocBook/media/v4l/v4l2.xml > +++ b/Documentation/DocBook/media/v4l/v4l2.xml > @@ -140,6 +140,13 @@ structs, ioctls) must be noted in more detail in the history chapter > applications. --> > > <revision> > + <revnumber>3.9</revnumber> > + <date>2012-12-10</date> > + <authorinitials>sn</authorinitials> > + <revremark>Added V4L2_CID_AUTO_FOCUS_AREA control.</revremark> > + </revision> > + > + <revision> > <revnumber>3.6</revnumber> > <date>2012-07-02</date> > <authorinitials>hv</authorinitials> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c > index f6ee201..9cdf4b8 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -236,6 +236,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id) > "Spot", > NULL > }; > + static const char * const camera_auto_focus_area[] = { > + "All", > + "Rectangle", > + "Object Detection", > + NULL > + }; > static const char * const camera_auto_focus_range[] = { > "Auto", > "Normal", > @@ -497,6 +503,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id) > return camera_exposure_auto; > case V4L2_CID_EXPOSURE_METERING: > return camera_exposure_metering; > + case V4L2_CID_AUTO_FOCUS_AREA: > + return camera_auto_focus_area; > case V4L2_CID_AUTO_FOCUS_RANGE: > return camera_auto_focus_range; > case V4L2_CID_COLORFX: > @@ -732,6 +740,7 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_AUTO_FOCUS_STOP: return "Auto Focus, Stop"; > case V4L2_CID_AUTO_FOCUS_STATUS: return "Auto Focus, Status"; > case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range"; > + case V4L2_CID_AUTO_FOCUS_AREA: return "Auto Focus, Area"; > > /* FM Radio Modulator control */ > /* Keep the order of the 'case's the same as in videodev2.h! */ > @@ -881,6 +890,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_AUTO_FOCUS_AREA: > case V4L2_CID_AUTO_FOCUS_RANGE: > case V4L2_CID_COLORFX: > case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE: > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h > index f56c945..0eb1c1a 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -683,6 +683,12 @@ enum v4l2_auto_focus_range { > V4L2_AUTO_FOCUS_RANGE_INFINITY = 3, > }; > > +#define V4L2_CID_AUTO_FOCUS_AREA (V4L2_CID_CAMERA_CLASS_BASE+32) > +enum v4l2_auto_focus_area { > + V4L2_AUTO_FOCUS_AREA_ALL = 0, > + V4L2_AUTO_FOCUS_AREA_RECTANGLE = 1, > + V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION = 2, How about using #defines? It's easier for the user space when it can just test #ifdef instead of relying on interesting hacks such as those in VLC source in modules/access/v4l2/v4l2.h. You can easily either provide a substitute or just not compile in the feature. > +}; > > /* FM Modulator class control IDs */ >
Hi Sakari, Thanks for the review. On 11.12.2012 22:34, Sakari Ailus wrote: > Hi Andrzej and Sylwester, > > Thanks for the patch! > > On Mon, Dec 10, 2012 at 02:43:39PM +0100, Andrzej Hajda wrote: >> From: Sylwester Nawrocki <s.nawrocki@samsung.com> >> >> Add control for automatic focus area selection. >> This control determines the area of the frame that the camera uses >> for automatic focus. >> >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> >> --- >> Documentation/DocBook/media/v4l/compat.xml | 9 +++-- >> Documentation/DocBook/media/v4l/controls.xml | 47 +++++++++++++++++++++++++- >> Documentation/DocBook/media/v4l/v4l2.xml | 7 ++++ >> drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ >> include/uapi/linux/v4l2-controls.h | 6 ++++ >> 5 files changed, 76 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml >> index 4fdf6b5..e8b53da 100644 >> --- a/Documentation/DocBook/media/v4l/compat.xml >> +++ b/Documentation/DocBook/media/v4l/compat.xml >> @@ -2452,8 +2452,9 @@ that used it. It was originally scheduled for removal in 2.6.35. >> <constant>V4L2_CID_3A_LOCK</constant>, >> <constant>V4L2_CID_AUTO_FOCUS_START</constant>, >> <constant>V4L2_CID_AUTO_FOCUS_STOP</constant>, >> - <constant>V4L2_CID_AUTO_FOCUS_STATUS</constant> and >> - <constant>V4L2_CID_AUTO_FOCUS_RANGE</constant>. >> + <constant>V4L2_CID_AUTO_FOCUS_STATUS</constant>, >> + <constant>V4L2_CID_AUTO_FOCUS_RANGE</constant> and >> + <constant>V4L2_CID_AUTO_FOCUS_AREA</constant>. >> </para> >> </listitem> >> </orderedlist> >> @@ -2586,6 +2587,10 @@ ioctls.</para> >> <para>Vendor and device specific media bus pixel formats. >> <xref linkend="v4l2-mbus-vendor-spec-fmts" />.</para> >> </listitem> >> + <listitem> >> + <para><link linkend="v4l2-auto-focus-area"><constant> >> + V4L2_CID_AUTO_FOCUS_AREA</constant></link> control.</para> >> + </listitem> >> </itemizedlist> >> </section> >> >> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml >> index 7fe5be1..9d4af8a 100644 >> --- a/Documentation/DocBook/media/v4l/controls.xml >> +++ b/Documentation/DocBook/media/v4l/controls.xml >> @@ -3347,6 +3347,51 @@ use its minimum possible distance for auto focus.</entry> >> </row> >> <row><entry></entry></row> >> >> + <row id="v4l2-auto-focus-area"> >> + <entry spanname="id"> >> + <constant>V4L2_CID_AUTO_FOCUS_AREA</constant> </entry> >> + <entry>enum v4l2_auto_focus_area</entry> >> + </row> >> + <row><entry spanname="descr">Determines the area of the frame that >> +the camera uses for automatic focus. The corresponding coordinates of the >> +focusing spot or rectangle can be specified and queried using the selection API. >> +To change the auto focus region of interest applications first select required >> +mode of this control and then set the rectangle or spot coordinates by means >> +of the &VIDIOC-SUBDEV-S-SELECTION; or &VIDIOC-S-SELECTION; ioctl. In order to >> +trigger again a one shot auto focus with same coordinates applications should >> +use the <constant>V4L2_CID_AUTO_FOCUS_START </constant> control. Or alternatively > Extra space above. ^ > >> +invoke a &VIDIOC-SUBDEV-S-SELECTION; or a &VIDIOC-S-SELECTION; ioctl again. > How about requiring explicit V4L2_CID_AUTO_FOCUS_START? If you need to > specify several AF selection windows, then on which one do you start the > algorithm? A bitmask control explicitly telling which ones are active would > also be needed --- but that's for the future. I think now we just need to > ascertain we don't make that difficult. :-) Do you mean only V4L2_CID_AUTO_FOCUS_START should start AF? What about continuous auto-focus (CAF)? In case of the sensor I am working on, face detection can work in both AF and CAF. Should CAF be restarted (ie stopped and started again), to use face detection? >> +In the latter case the new pixel coordinates are applied to hardware only when >> +the focus area control was set to >> +<constant>V4L2_AUTO_FOCUS_AREA_RECTANGLE</constant>.</entry> >> + </row> >> + <row> >> + <entrytbl spanname="descr" cols="2"> >> + <tbody valign="top"> >> + <row> >> + <entry><constant>V4L2_AUTO_FOCUS_AREA_ALL</constant> </entry> >> + <entry>Normal auto focus, the focusing area extends over the >> +entire frame.</entry> > Does this need to be explicitly specified? Shouldn't the user just choose > the largest possible AF window instead? I'd even expect that the AF window > might span the whole frame by default (up to driver, hardware etc.). Yes it could be removed. There are two reasons I have left it: 1. If hardware support only AF on spots, V4L2_AUTO_FOCUS_AREA_ALL seems to be more natural than focusing on the whole image. 2. (Hypothetical) Instructing HW to area-focusing on the whole are could have different results than just starting default auto-focus, ie there could be different algorithms involved. It is just a prediction based on my current experience :) > >> + </row> >> + <row> >> + <entry><constant>V4L2_AUTO_FOCUS_AREA_RECTANGLE</constant> </entry> >> + <entry>The auto focus region of interest is determined by the >> +<constant>V4L2_SEL_TGT_AUTO_FOCUS</constant> selection rectangle.</entry> > It's not strictly required in documentation (and that shows) but it'd be > nice to align the paragraphs at equal indentation. OK > >> + </row> >> + <row> >> + <entry><constant>V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION</constant> </entry> >> + <entry>The auto focus region of interest is determined >> +by an object (e.g. face) detection engine.</entry> >> + </row> >> + </tbody> >> + </entrytbl> >> + </row> >> + <row><entry spanname="descr"> >> + This is an <link linkend="experimental">experimental</link> >> +control and may change in the future.</entry> >> + </row> >> + <row><entry></entry></row> >> + >> <row> >> <entry spanname="id"><constant>V4L2_CID_ZOOM_ABSOLUTE</constant> </entry> >> <entry>integer</entry> >> @@ -3986,7 +4031,7 @@ interface and may change in the future.</para> >> >> <table pgwide="1" frame="none" id="flash-control-id"> >> <title>Flash Control IDs</title> >> - >> + >> <tgroup cols="4"> >> <colspec colname="c1" colwidth="1*" /> >> <colspec colname="c2" colwidth="6*" /> >> diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml >> index 10ccde9..1477540 100644 >> --- a/Documentation/DocBook/media/v4l/v4l2.xml >> +++ b/Documentation/DocBook/media/v4l/v4l2.xml >> @@ -140,6 +140,13 @@ structs, ioctls) must be noted in more detail in the history chapter >> applications. --> >> >> <revision> >> + <revnumber>3.9</revnumber> >> + <date>2012-12-10</date> >> + <authorinitials>sn</authorinitials> >> + <revremark>Added V4L2_CID_AUTO_FOCUS_AREA control.</revremark> >> + </revision> >> + >> + <revision> >> <revnumber>3.6</revnumber> >> <date>2012-07-02</date> >> <authorinitials>hv</authorinitials> >> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c >> index f6ee201..9cdf4b8 100644 >> --- a/drivers/media/v4l2-core/v4l2-ctrls.c >> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c >> @@ -236,6 +236,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id) >> "Spot", >> NULL >> }; >> + static const char * const camera_auto_focus_area[] = { >> + "All", >> + "Rectangle", >> + "Object Detection", >> + NULL >> + }; >> static const char * const camera_auto_focus_range[] = { >> "Auto", >> "Normal", >> @@ -497,6 +503,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id) >> return camera_exposure_auto; >> case V4L2_CID_EXPOSURE_METERING: >> return camera_exposure_metering; >> + case V4L2_CID_AUTO_FOCUS_AREA: >> + return camera_auto_focus_area; >> case V4L2_CID_AUTO_FOCUS_RANGE: >> return camera_auto_focus_range; >> case V4L2_CID_COLORFX: >> @@ -732,6 +740,7 @@ const char *v4l2_ctrl_get_name(u32 id) >> case V4L2_CID_AUTO_FOCUS_STOP: return "Auto Focus, Stop"; >> case V4L2_CID_AUTO_FOCUS_STATUS: return "Auto Focus, Status"; >> case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range"; >> + case V4L2_CID_AUTO_FOCUS_AREA: return "Auto Focus, Area"; >> >> /* FM Radio Modulator control */ >> /* Keep the order of the 'case's the same as in videodev2.h! */ >> @@ -881,6 +890,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_AUTO_FOCUS_AREA: >> case V4L2_CID_AUTO_FOCUS_RANGE: >> case V4L2_CID_COLORFX: >> case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE: >> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h >> index f56c945..0eb1c1a 100644 >> --- a/include/uapi/linux/v4l2-controls.h >> +++ b/include/uapi/linux/v4l2-controls.h >> @@ -683,6 +683,12 @@ enum v4l2_auto_focus_range { >> V4L2_AUTO_FOCUS_RANGE_INFINITY = 3, >> }; >> >> +#define V4L2_CID_AUTO_FOCUS_AREA (V4L2_CID_CAMERA_CLASS_BASE+32) >> +enum v4l2_auto_focus_area { >> + V4L2_AUTO_FOCUS_AREA_ALL = 0, >> + V4L2_AUTO_FOCUS_AREA_RECTANGLE = 1, >> + V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION = 2, > How about using #defines? It's easier for the user space when it can just > test #ifdef instead of relying on interesting hacks such as those in VLC > source in modules/access/v4l2/v4l2.h. > > You can easily either provide a substitute or just not compile in the > feature. Kernel also uses this hack/feature, grep started in kernel sources shows some of them: grep -C3 -Prn '^#define (.*) \1$' include/ > >> +}; >> >> /* FM Modulator class control IDs */ >> Regards Andrzej -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Andrzej, On Wed, Dec 12, 2012 at 04:14:22PM +0100, Andrzej Hajda wrote: > On 11.12.2012 22:34, Sakari Ailus wrote: > >Hi Andrzej and Sylwester, > > > >Thanks for the patch! > > > >On Mon, Dec 10, 2012 at 02:43:39PM +0100, Andrzej Hajda wrote: > >>From: Sylwester Nawrocki <s.nawrocki@samsung.com> > >> > >>Add control for automatic focus area selection. > >>This control determines the area of the frame that the camera uses > >>for automatic focus. > >> > >>Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > >>Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> > >>Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > >>--- > >> Documentation/DocBook/media/v4l/compat.xml | 9 +++-- > >> Documentation/DocBook/media/v4l/controls.xml | 47 +++++++++++++++++++++++++- > >> Documentation/DocBook/media/v4l/v4l2.xml | 7 ++++ > >> drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ > >> include/uapi/linux/v4l2-controls.h | 6 ++++ > >> 5 files changed, 76 insertions(+), 3 deletions(-) > >> > >>diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml > >>index 4fdf6b5..e8b53da 100644 > >>--- a/Documentation/DocBook/media/v4l/compat.xml > >>+++ b/Documentation/DocBook/media/v4l/compat.xml > >>@@ -2452,8 +2452,9 @@ that used it. It was originally scheduled for removal in 2.6.35. > >> <constant>V4L2_CID_3A_LOCK</constant>, > >> <constant>V4L2_CID_AUTO_FOCUS_START</constant>, > >> <constant>V4L2_CID_AUTO_FOCUS_STOP</constant>, > >>- <constant>V4L2_CID_AUTO_FOCUS_STATUS</constant> and > >>- <constant>V4L2_CID_AUTO_FOCUS_RANGE</constant>. > >>+ <constant>V4L2_CID_AUTO_FOCUS_STATUS</constant>, > >>+ <constant>V4L2_CID_AUTO_FOCUS_RANGE</constant> and > >>+ <constant>V4L2_CID_AUTO_FOCUS_AREA</constant>. > >> </para> > >> </listitem> > >> </orderedlist> > >>@@ -2586,6 +2587,10 @@ ioctls.</para> > >> <para>Vendor and device specific media bus pixel formats. > >> <xref linkend="v4l2-mbus-vendor-spec-fmts" />.</para> > >> </listitem> > >>+ <listitem> > >>+ <para><link linkend="v4l2-auto-focus-area"><constant> > >>+ V4L2_CID_AUTO_FOCUS_AREA</constant></link> control.</para> > >>+ </listitem> > >> </itemizedlist> > >> </section> > >>diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml > >>index 7fe5be1..9d4af8a 100644 > >>--- a/Documentation/DocBook/media/v4l/controls.xml > >>+++ b/Documentation/DocBook/media/v4l/controls.xml > >>@@ -3347,6 +3347,51 @@ use its minimum possible distance for auto focus.</entry> > >> </row> > >> <row><entry></entry></row> > >>+ <row id="v4l2-auto-focus-area"> > >>+ <entry spanname="id"> > >>+ <constant>V4L2_CID_AUTO_FOCUS_AREA</constant> </entry> > >>+ <entry>enum v4l2_auto_focus_area</entry> > >>+ </row> > >>+ <row><entry spanname="descr">Determines the area of the frame that > >>+the camera uses for automatic focus. The corresponding coordinates of the > >>+focusing spot or rectangle can be specified and queried using the selection API. > >>+To change the auto focus region of interest applications first select required > >>+mode of this control and then set the rectangle or spot coordinates by means > >>+of the &VIDIOC-SUBDEV-S-SELECTION; or &VIDIOC-S-SELECTION; ioctl. In order to > >>+trigger again a one shot auto focus with same coordinates applications should > >>+use the <constant>V4L2_CID_AUTO_FOCUS_START </constant> control. Or alternatively > >Extra space above. ^ > > > >>+invoke a &VIDIOC-SUBDEV-S-SELECTION; or a &VIDIOC-S-SELECTION; ioctl again. > >How about requiring explicit V4L2_CID_AUTO_FOCUS_START? If you need to > >specify several AF selection windows, then on which one do you start the > >algorithm? A bitmask control explicitly telling which ones are active would > >also be needed --- but that's for the future. I think now we just need to > >ascertain we don't make that difficult. :-) > Do you mean only V4L2_CID_AUTO_FOCUS_START should start AF? > What about continuous auto-focus (CAF)? In case of the sensor I am > working on, face detection can work in both AF and CAF. Continuous AF needs to be an exception to that. It's controlled by V4L2_CID_FOCUS_AUTO, which interestingly doesn't even mention continuous AF. > Should CAF be restarted (ie stopped and started again), to use face > detection? I wonder if V4L2_CID_AUTO_FOCUS_START should be defined to restart CAF when V4L2_CID_FOCUS_AUTO is enabled. I don't think we currently have a way to do that; the current definition says that using V4L2_CID_AUTO_FOCUS_START is undefined then. What do you think? > >>+In the latter case the new pixel coordinates are applied to hardware only when > >>+the focus area control was set to > >>+<constant>V4L2_AUTO_FOCUS_AREA_RECTANGLE</constant>.</entry> > >>+ </row> > >>+ <row> > >>+ <entrytbl spanname="descr" cols="2"> > >>+ <tbody valign="top"> > >>+ <row> > >>+ <entry><constant>V4L2_AUTO_FOCUS_AREA_ALL</constant> </entry> > >>+ <entry>Normal auto focus, the focusing area extends over the > >>+entire frame.</entry> > >Does this need to be explicitly specified? Shouldn't the user just choose > >the largest possible AF window instead? I'd even expect that the AF window > >might span the whole frame by default (up to driver, hardware etc.). > Yes it could be removed. There are two reasons I have left it: > 1. If hardware support only AF on spots, V4L2_AUTO_FOCUS_AREA_ALL > seems to be more > natural than focusing on the whole image. If the hardware only supports spots, then wouldn't V4L2_AUTO_FOCUS_AREA_ALL give false information to the user, suggesting the focus area is actually the whole image? > 2. (Hypothetical) Instructing HW to area-focusing on the whole are > could have different results than just starting default auto-focus, > ie there could be different algorithms involved. It is just a > prediction based on my current experience :) If the algorithm is different in that case, then it should be made a new control, not implicitly throught a seemingly unrelated control. We currently don't have one, and this kind of things could be hardware specific, so this could be a private control IMO.
Hi Sakari, On 12/16/2012 04:00 PM, Sakari Ailus wrote: >>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml >>>> index 7fe5be1..9d4af8a 100644 >>>> --- a/Documentation/DocBook/media/v4l/controls.xml >>>> +++ b/Documentation/DocBook/media/v4l/controls.xml >>>> @@ -3347,6 +3347,51 @@ use its minimum possible distance for auto focus.</entry> >>>> </row> >>>> <row><entry></entry></row> >>>> + <row id="v4l2-auto-focus-area"> >>>> + <entry spanname="id"> >>>> + <constant>V4L2_CID_AUTO_FOCUS_AREA</constant> </entry> >>>> + <entry>enum v4l2_auto_focus_area</entry> >>>> + </row> >>>> + <row><entry spanname="descr">Determines the area of the frame that >>>> +the camera uses for automatic focus. The corresponding coordinates of the >>>> +focusing spot or rectangle can be specified and queried using the selection API. >>>> +To change the auto focus region of interest applications first select required >>>> +mode of this control and then set the rectangle or spot coordinates by means >>>> +of the&VIDIOC-SUBDEV-S-SELECTION; or&VIDIOC-S-SELECTION; ioctl. In order to >>>> +trigger again a one shot auto focus with same coordinates applications should >>>> +use the<constant>V4L2_CID_AUTO_FOCUS_START</constant> control. Or alternatively >>> Extra space above. ^ >>> >>>> +invoke a&VIDIOC-SUBDEV-S-SELECTION; or a&VIDIOC-S-SELECTION; ioctl again. >>> How about requiring explicit V4L2_CID_AUTO_FOCUS_START? If you need to >>> specify several AF selection windows, then on which one do you start the >>> algorithm? A bitmask control explicitly telling which ones are active would >>> also be needed --- but that's for the future. I think now we just need to >>> ascertain we don't make that difficult. :-) >> Do you mean only V4L2_CID_AUTO_FOCUS_START should start AF? >> What about continuous auto-focus (CAF)? In case of the sensor I am >> working on, face detection can work in both AF and CAF. > > Continuous AF needs to be an exception to that. It's controlled by > V4L2_CID_FOCUS_AUTO, which interestingly doesn't even mention continuous AF. I think it does, maybe not exactly in these words, but "continuous automatic focus adjustments" doesn't sound like a difference thing to me. >> Should CAF be restarted (ie stopped and started again), to use face >> detection? > > I wonder if V4L2_CID_AUTO_FOCUS_START should be defined to restart CAF when > V4L2_CID_FOCUS_AUTO is enabled. I don't think we currently have a way to do > that; the current definition says that using V4L2_CID_AUTO_FOCUS_START is > undefined then. What do you think? Yes, it might be worth to reconsider this. However, I would like to see real use cases first where V4L2_CID_AUTO_FOCUS_START control is needed in continuous AF mode. All in all, we have V4L2_AUTO_FOCUS_STATUS_FAILED AF status control value and I can't see anything preventing it to be applicable to CAF. So it might make sense to define in the API what needs to be done to bring CAF out of this state. >>>> +In the latter case the new pixel coordinates are applied to hardware only when >>>> +the focus area control was set to >>>> +<constant>V4L2_AUTO_FOCUS_AREA_RECTANGLE</constant>.</entry> >>>> + </row> >>>> + <row> >>>> + <entrytbl spanname="descr" cols="2"> >>>> + <tbody valign="top"> >>>> + <row> >>>> + <entry><constant>V4L2_AUTO_FOCUS_AREA_ALL</constant> </entry> >>>> + <entry>Normal auto focus, the focusing area extends over the >>>> +entire frame.</entry> >>> Does this need to be explicitly specified? Shouldn't the user just choose >>> the largest possible AF window instead? I'd even expect that the AF window >>> might span the whole frame by default (up to driver, hardware etc.). >> Yes it could be removed. There are two reasons I have left it: >> 1. If hardware support only AF on spots, V4L2_AUTO_FOCUS_AREA_ALL >> seems to be more >> natural than focusing on the whole image. > > If the hardware only supports spots, then wouldn't V4L2_AUTO_FOCUS_AREA_ALL > give false information to the user, suggesting the focus area is actually > the whole image? I think Andrzej meant to say that there can be hardware that supports: a. AF where region of interest is whole frame, b. AF where region of interest is some rectangle of size that may be not known exactly, and position (center) of that rectangle only is defined through AF selections. So you would be really switching AF algorithms by manipulating AF selection rectangle only. That said I really think V4L2_AUTO_FOCUS_AREA_ALL is a bad name here. I originally started with single AF mode control and then after discussions we came up with V4L2_AUTO_FOCUS_RANGE and V4L2_AUTO_FOCUS_AREA controls. My motivation behind V4L2_AUTO_FOCUS_AREA_ALL was to provide a menu item that would allow to select "normal" AF, with supposedly whole frame being the AF region of interest. "Normal" AF might mean really any area of the frame, so I propose to just replace V4L2_AUTO_FOCUS_AREA_ALL with V4L2_AUTO_FOCUS_AREA_AUTO. This entry would naturally mean that AF area is automatically selected by an ISP and it might not be known exactly to user. Like in case of those superb AF algorithms that many companies value to keep secret... >> 2. (Hypothetical) Instructing HW to area-focusing on the whole are >> could have different results than just starting default auto-focus, >> ie there could be different algorithms involved. It is just a >> prediction based on my current experience :) > > If the algorithm is different in that case, then it should be made a new > control, not implicitly throught a seemingly unrelated control. > > We currently don't have one, and this kind of things could be hardware > specific, so this could be a private control IMO. We have already something like V4L2_CID_AUTO_FOCUS_MODE private control, common to multiple Samsung camera sensors. And each device has mostly different set of options in such a control. Not sure if it wouldn't make more sense to have standard menu control ID with driver specific entries for all AF modes. It likely makes sense to have common patterns expressed in standard controls though. It seems current set of AF controls, together with V4L2_AUTO_FOCUS_AREA covers pretty much of the functionality, without resorting to the private controls interface, that is so awkward to use when you have to deal with multiple different devices... -- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester and Sakari, On 17.12.2012 01:11, Sylwester Nawrocki wrote: > Hi Sakari, > > On 12/16/2012 04:00 PM, Sakari Ailus wrote: >>>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml >>>>> b/Documentation/DocBook/media/v4l/controls.xml >>>>> index 7fe5be1..9d4af8a 100644 >>>>> --- a/Documentation/DocBook/media/v4l/controls.xml >>>>> +++ b/Documentation/DocBook/media/v4l/controls.xml >>>>> @@ -3347,6 +3347,51 @@ use its minimum possible distance for auto >>>>> focus.</entry> >>>>> </row> >>>>> <row><entry></entry></row> >>>>> + <row id="v4l2-auto-focus-area"> >>>>> + <entry spanname="id"> >>>>> + <constant>V4L2_CID_AUTO_FOCUS_AREA</constant> </entry> >>>>> + <entry>enum v4l2_auto_focus_area</entry> >>>>> + </row> >>>>> + <row><entry spanname="descr">Determines the area of the frame >>>>> that >>>>> +the camera uses for automatic focus. The corresponding >>>>> coordinates of the >>>>> +focusing spot or rectangle can be specified and queried using the >>>>> selection API. >>>>> +To change the auto focus region of interest applications first >>>>> select required >>>>> +mode of this control and then set the rectangle or spot >>>>> coordinates by means >>>>> +of the&VIDIOC-SUBDEV-S-SELECTION; or&VIDIOC-S-SELECTION; ioctl. >>>>> In order to >>>>> +trigger again a one shot auto focus with same coordinates >>>>> applications should >>>>> +use the<constant>V4L2_CID_AUTO_FOCUS_START</constant> control. Or >>>>> alternatively >>>> Extra space above. ^ >>>> >>>>> +invoke a&VIDIOC-SUBDEV-S-SELECTION; or a&VIDIOC-S-SELECTION; >>>>> ioctl again. >>>> How about requiring explicit V4L2_CID_AUTO_FOCUS_START? If you need to >>>> specify several AF selection windows, then on which one do you >>>> start the >>>> algorithm? A bitmask control explicitly telling which ones are >>>> active would >>>> also be needed --- but that's for the future. I think now we just >>>> need to >>>> ascertain we don't make that difficult. :-) >>> Do you mean only V4L2_CID_AUTO_FOCUS_START should start AF? >>> What about continuous auto-focus (CAF)? In case of the sensor I am >>> working on, face detection can work in both AF and CAF. >> >> Continuous AF needs to be an exception to that. It's controlled by >> V4L2_CID_FOCUS_AUTO, which interestingly doesn't even mention >> continuous AF. > > I think it does, maybe not exactly in these words, but "continuous > automatic focus > adjustments" doesn't sound like a difference thing to me. > >>> Should CAF be restarted (ie stopped and started again), to use face >>> detection? >> >> I wonder if V4L2_CID_AUTO_FOCUS_START should be defined to restart >> CAF when >> V4L2_CID_FOCUS_AUTO is enabled. I don't think we currently have a way >> to do >> that; the current definition says that using >> V4L2_CID_AUTO_FOCUS_START is >> undefined then. What do you think? > > Yes, it might be worth to reconsider this. However, I would like to > see real > use cases first where V4L2_CID_AUTO_FOCUS_START control is needed in > continuous > AF mode. > All in all, we have V4L2_AUTO_FOCUS_STATUS_FAILED AF status control > value and > I can't see anything preventing it to be applicable to CAF. So it > might make > sense to define in the API what needs to be done to bring CAF out of > this state. And what about using again V4L2_CID_FOCUS_AUTO=true? Regarding the proposition that setting V4L2_CID_AUTO_FOCUS_AREA should not trigger AF/CAF. I am not sure if it will be good in case of CAF. After setting this control but before restarting CAF driver will not reflect hardware state. It does not seem to be dangerous but I do not see why we should allow for such situation anyway. In case of AF it seems to me OK. FOCUS_AREA is a setting which will be used only when AF action is started, ie. only in V4L2_CID_AUTO_FOCUS_START. Documentation should clearly state that only V4L2_CID_AUTO_FOCUS_START starts AF. >>>>> +In the latter case the new pixel coordinates are applied to >>>>> hardware only when >>>>> +the focus area control was set to >>>>> +<constant>V4L2_AUTO_FOCUS_AREA_RECTANGLE</constant>.</entry> >>>>> + </row> >>>>> + <row> >>>>> + <entrytbl spanname="descr" cols="2"> >>>>> + <tbody valign="top"> >>>>> + <row> >>>>> + <entry><constant>V4L2_AUTO_FOCUS_AREA_ALL</constant> </entry> >>>>> + <entry>Normal auto focus, the focusing area extends over the >>>>> +entire frame.</entry> >>>> Does this need to be explicitly specified? Shouldn't the user just >>>> choose >>>> the largest possible AF window instead? I'd even expect that the AF >>>> window >>>> might span the whole frame by default (up to driver, hardware etc.). >>> Yes it could be removed. There are two reasons I have left it: >>> 1. If hardware support only AF on spots, V4L2_AUTO_FOCUS_AREA_ALL >>> seems to be more >>> natural than focusing on the whole image. >> >> If the hardware only supports spots, then wouldn't >> V4L2_AUTO_FOCUS_AREA_ALL >> give false information to the user, suggesting the focus area is >> actually >> the whole image? > > I think Andrzej meant to say that there can be hardware that supports: > > a. AF where region of interest is whole frame, > b. AF where region of interest is some rectangle of size that may be not > known exactly, and position (center) of that rectangle only is defined > through AF selections. > > So you would be really switching AF algorithms by manipulating AF > selection > rectangle only. > > That said I really think V4L2_AUTO_FOCUS_AREA_ALL is a bad name here. > I originally started with single AF mode control and then after > discussions > we came up with V4L2_AUTO_FOCUS_RANGE and V4L2_AUTO_FOCUS_AREA controls. > > My motivation behind V4L2_AUTO_FOCUS_AREA_ALL was to provide a menu > item that would allow to select "normal" AF, with supposedly whole frame > being the AF region of interest. "Normal" AF might mean really any area > of the frame, so I propose to just replace V4L2_AUTO_FOCUS_AREA_ALL with > V4L2_AUTO_FOCUS_AREA_AUTO. This entry would naturally mean that AF area > is automatically selected by an ISP and it might not be known exactly to > user. Like in case of those superb AF algorithms that many companies > value to keep secret... > >>> 2. (Hypothetical) Instructing HW to area-focusing on the whole are >>> could have different results than just starting default auto-focus, >>> ie there could be different algorithms involved. It is just a >>> prediction based on my current experience :) >> >> If the algorithm is different in that case, then it should be made a new >> control, not implicitly throught a seemingly unrelated control. >> >> We currently don't have one, and this kind of things could be hardware >> specific, so this could be a private control IMO. > > We have already something like V4L2_CID_AUTO_FOCUS_MODE private control, > common to multiple Samsung camera sensors. And each device has mostly > different set of options in such a control. Not sure if it wouldn't make > more sense to have standard menu control ID with driver specific entries > for all AF modes. It likely makes sense to have common patterns expressed > in standard controls though. It seems current set of AF controls, > together > with V4L2_AUTO_FOCUS_AREA covers pretty much of the functionality, > without resorting to the private controls interface, that is so awkward > to use when you have to deal with multiple different devices... > > -- > > Thanks, > Sylwester > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester, My apologies for the delayed answer. Sylwester Nawrocki wrote: > On 12/16/2012 04:00 PM, Sakari Ailus wrote: >>>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml >>>>> b/Documentation/DocBook/media/v4l/controls.xml >>>>> index 7fe5be1..9d4af8a 100644 >>>>> --- a/Documentation/DocBook/media/v4l/controls.xml >>>>> +++ b/Documentation/DocBook/media/v4l/controls.xml >>>>> @@ -3347,6 +3347,51 @@ use its minimum possible distance for auto >>>>> focus.</entry> >>>>> </row> >>>>> <row><entry></entry></row> >>>>> + <row id="v4l2-auto-focus-area"> >>>>> + <entry spanname="id"> >>>>> + <constant>V4L2_CID_AUTO_FOCUS_AREA</constant> </entry> >>>>> + <entry>enum v4l2_auto_focus_area</entry> >>>>> + </row> >>>>> + <row><entry spanname="descr">Determines the area of the frame >>>>> that >>>>> +the camera uses for automatic focus. The corresponding coordinates >>>>> of the >>>>> +focusing spot or rectangle can be specified and queried using the >>>>> selection API. >>>>> +To change the auto focus region of interest applications first >>>>> select required >>>>> +mode of this control and then set the rectangle or spot >>>>> coordinates by means >>>>> +of the&VIDIOC-SUBDEV-S-SELECTION; or&VIDIOC-S-SELECTION; ioctl. In >>>>> order to >>>>> +trigger again a one shot auto focus with same coordinates >>>>> applications should >>>>> +use the<constant>V4L2_CID_AUTO_FOCUS_START</constant> control. Or >>>>> alternatively >>>> Extra space above. ^ >>>> >>>>> +invoke a&VIDIOC-SUBDEV-S-SELECTION; or a&VIDIOC-S-SELECTION; ioctl >>>>> again. >>>> How about requiring explicit V4L2_CID_AUTO_FOCUS_START? If you need to >>>> specify several AF selection windows, then on which one do you start >>>> the >>>> algorithm? A bitmask control explicitly telling which ones are >>>> active would >>>> also be needed --- but that's for the future. I think now we just >>>> need to >>>> ascertain we don't make that difficult. :-) >>> Do you mean only V4L2_CID_AUTO_FOCUS_START should start AF? >>> What about continuous auto-focus (CAF)? In case of the sensor I am >>> working on, face detection can work in both AF and CAF. >> >> Continuous AF needs to be an exception to that. It's controlled by >> V4L2_CID_FOCUS_AUTO, which interestingly doesn't even mention >> continuous AF. > > I think it does, maybe not exactly in these words, but "continuous > automatic focus > adjustments" doesn't sound like a difference thing to me. Oh. I must have missed that. It's ok then. >>> Should CAF be restarted (ie stopped and started again), to use face >>> detection? >> >> I wonder if V4L2_CID_AUTO_FOCUS_START should be defined to restart CAF >> when >> V4L2_CID_FOCUS_AUTO is enabled. I don't think we currently have a way >> to do >> that; the current definition says that using V4L2_CID_AUTO_FOCUS_START is >> undefined then. What do you think? > > Yes, it might be worth to reconsider this. However, I would like to see > real > use cases first where V4L2_CID_AUTO_FOCUS_START control is needed in > continuous > AF mode. If the CAF focus window is changed, I think it may make sense to tell the CAF algorithm this. If the window moves around based on e.g. output from face detection algorithm it's unlikely there's a need to perform a full search every time this happens. > All in all, we have V4L2_AUTO_FOCUS_STATUS_FAILED AF status control > value and > I can't see anything preventing it to be applicable to CAF. So it might > make > sense to define in the API what needs to be done to bring CAF out of > this state. How would CAF fail? Would this mean that a pre-defined amount of time has been spent on searching focus and none has been found? Shouldn't it be application's decision to tell how long is too long, rather than driver's? >>>>> +In the latter case the new pixel coordinates are applied to >>>>> hardware only when >>>>> +the focus area control was set to >>>>> +<constant>V4L2_AUTO_FOCUS_AREA_RECTANGLE</constant>.</entry> >>>>> + </row> >>>>> + <row> >>>>> + <entrytbl spanname="descr" cols="2"> >>>>> + <tbody valign="top"> >>>>> + <row> >>>>> + >>>>> <entry><constant>V4L2_AUTO_FOCUS_AREA_ALL</constant> </entry> >>>>> + <entry>Normal auto focus, the focusing area extends over the >>>>> +entire frame.</entry> >>>> Does this need to be explicitly specified? Shouldn't the user just >>>> choose >>>> the largest possible AF window instead? I'd even expect that the AF >>>> window >>>> might span the whole frame by default (up to driver, hardware etc.). >>> Yes it could be removed. There are two reasons I have left it: >>> 1. If hardware support only AF on spots, V4L2_AUTO_FOCUS_AREA_ALL >>> seems to be more >>> natural than focusing on the whole image. >> >> If the hardware only supports spots, then wouldn't >> V4L2_AUTO_FOCUS_AREA_ALL >> give false information to the user, suggesting the focus area is actually >> the whole image? > > I think Andrzej meant to say that there can be hardware that supports: > > a. AF where region of interest is whole frame, > b. AF where region of interest is some rectangle of size that may be not > known exactly, and position (center) of that rectangle only is defined > through AF selections. > > So you would be really switching AF algorithms by manipulating AF selection > rectangle only. I would keep the two separate: selecting the algorithm and the area of interest. When the host software runs the algorithm itself the selection is just about the area and nothing else. I think it'd be more flexible not to make other decisions based on it in the driver. Well, an individual driver could still do this if really, really needed but I don't think it should end up to the V4L2 spec. > That said I really think V4L2_AUTO_FOCUS_AREA_ALL is a bad name here. > I originally started with single AF mode control and then after discussions > we came up with V4L2_AUTO_FOCUS_RANGE and V4L2_AUTO_FOCUS_AREA controls. > > My motivation behind V4L2_AUTO_FOCUS_AREA_ALL was to provide a menu > item that would allow to select "normal" AF, with supposedly whole frame > being the AF region of interest. "Normal" AF might mean really any area > of the frame, so I propose to just replace V4L2_AUTO_FOCUS_AREA_ALL with > V4L2_AUTO_FOCUS_AREA_AUTO. This entry would naturally mean that AF area > is automatically selected by an ISP and it might not be known exactly to > user. Like in case of those superb AF algorithms that many companies > value to keep secret... There might be cases where the user doesn't wish to choose the area, and the user shouldn't be forced to do so. In this case the driver either knows the rectangle or it doesn't. For the user this shouldn't matter. If the area isn't known, the driver shouldn't provide access to the whole selection rectangle, or the control. If the area is known to the driver, there likely should be a sane default, at least for the cases where the AF algorithm isn't in the host user space. The sane default most probably would be something else than the full image area. What makes sense likely also depends on the hardware (resolution etc.) and the algorithm used, too. >>> 2. (Hypothetical) Instructing HW to area-focusing on the whole are >>> could have different results than just starting default auto-focus, >>> ie there could be different algorithms involved. It is just a >>> prediction based on my current experience :) >> >> If the algorithm is different in that case, then it should be made a new >> control, not implicitly throught a seemingly unrelated control. >> >> We currently don't have one, and this kind of things could be hardware >> specific, so this could be a private control IMO. > > We have already something like V4L2_CID_AUTO_FOCUS_MODE private control, > common to multiple Samsung camera sensors. And each device has mostly > different set of options in such a control. Not sure if it wouldn't make > more sense to have standard menu control ID with driver specific entries > for all AF modes. It likely makes sense to have common patterns expressed > in standard controls though. It seems current set of AF controls, together > with V4L2_AUTO_FOCUS_AREA covers pretty much of the functionality, > without resorting to the private controls interface, that is so awkward > to use when you have to deal with multiple different devices... How many drivers are there and how many (and which) modes do they provide? This sounds still a little bit specific to Samsung camera modules, but if it's present in many then it could make sense to make it a standard control. I can't say for sure without knowing more, though.
diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml index 4fdf6b5..e8b53da 100644 --- a/Documentation/DocBook/media/v4l/compat.xml +++ b/Documentation/DocBook/media/v4l/compat.xml @@ -2452,8 +2452,9 @@ that used it. It was originally scheduled for removal in 2.6.35. <constant>V4L2_CID_3A_LOCK</constant>, <constant>V4L2_CID_AUTO_FOCUS_START</constant>, <constant>V4L2_CID_AUTO_FOCUS_STOP</constant>, - <constant>V4L2_CID_AUTO_FOCUS_STATUS</constant> and - <constant>V4L2_CID_AUTO_FOCUS_RANGE</constant>. + <constant>V4L2_CID_AUTO_FOCUS_STATUS</constant>, + <constant>V4L2_CID_AUTO_FOCUS_RANGE</constant> and + <constant>V4L2_CID_AUTO_FOCUS_AREA</constant>. </para> </listitem> </orderedlist> @@ -2586,6 +2587,10 @@ ioctls.</para> <para>Vendor and device specific media bus pixel formats. <xref linkend="v4l2-mbus-vendor-spec-fmts" />.</para> </listitem> + <listitem> + <para><link linkend="v4l2-auto-focus-area"><constant> + V4L2_CID_AUTO_FOCUS_AREA</constant></link> control.</para> + </listitem> </itemizedlist> </section> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index 7fe5be1..9d4af8a 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -3347,6 +3347,51 @@ use its minimum possible distance for auto focus.</entry> </row> <row><entry></entry></row> + <row id="v4l2-auto-focus-area"> + <entry spanname="id"> + <constant>V4L2_CID_AUTO_FOCUS_AREA</constant> </entry> + <entry>enum v4l2_auto_focus_area</entry> + </row> + <row><entry spanname="descr">Determines the area of the frame that +the camera uses for automatic focus. The corresponding coordinates of the +focusing spot or rectangle can be specified and queried using the selection API. +To change the auto focus region of interest applications first select required +mode of this control and then set the rectangle or spot coordinates by means +of the &VIDIOC-SUBDEV-S-SELECTION; or &VIDIOC-S-SELECTION; ioctl. In order to +trigger again a one shot auto focus with same coordinates applications should +use the <constant>V4L2_CID_AUTO_FOCUS_START </constant> control. Or alternatively +invoke a &VIDIOC-SUBDEV-S-SELECTION; or a &VIDIOC-S-SELECTION; ioctl again. +In the latter case the new pixel coordinates are applied to hardware only when +the focus area control was set to +<constant>V4L2_AUTO_FOCUS_AREA_RECTANGLE</constant>.</entry> + </row> + <row> + <entrytbl spanname="descr" cols="2"> + <tbody valign="top"> + <row> + <entry><constant>V4L2_AUTO_FOCUS_AREA_ALL</constant> </entry> + <entry>Normal auto focus, the focusing area extends over the +entire frame.</entry> + </row> + <row> + <entry><constant>V4L2_AUTO_FOCUS_AREA_RECTANGLE</constant> </entry> + <entry>The auto focus region of interest is determined by the +<constant>V4L2_SEL_TGT_AUTO_FOCUS</constant> selection rectangle.</entry> + </row> + <row> + <entry><constant>V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION</constant> </entry> + <entry>The auto focus region of interest is determined +by an object (e.g. face) detection engine.</entry> + </row> + </tbody> + </entrytbl> + </row> + <row><entry spanname="descr"> + This is an <link linkend="experimental">experimental</link> +control and may change in the future.</entry> + </row> + <row><entry></entry></row> + <row> <entry spanname="id"><constant>V4L2_CID_ZOOM_ABSOLUTE</constant> </entry> <entry>integer</entry> @@ -3986,7 +4031,7 @@ interface and may change in the future.</para> <table pgwide="1" frame="none" id="flash-control-id"> <title>Flash Control IDs</title> - + <tgroup cols="4"> <colspec colname="c1" colwidth="1*" /> <colspec colname="c2" colwidth="6*" /> diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml index 10ccde9..1477540 100644 --- a/Documentation/DocBook/media/v4l/v4l2.xml +++ b/Documentation/DocBook/media/v4l/v4l2.xml @@ -140,6 +140,13 @@ structs, ioctls) must be noted in more detail in the history chapter applications. --> <revision> + <revnumber>3.9</revnumber> + <date>2012-12-10</date> + <authorinitials>sn</authorinitials> + <revremark>Added V4L2_CID_AUTO_FOCUS_AREA control.</revremark> + </revision> + + <revision> <revnumber>3.6</revnumber> <date>2012-07-02</date> <authorinitials>hv</authorinitials> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index f6ee201..9cdf4b8 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -236,6 +236,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id) "Spot", NULL }; + static const char * const camera_auto_focus_area[] = { + "All", + "Rectangle", + "Object Detection", + NULL + }; static const char * const camera_auto_focus_range[] = { "Auto", "Normal", @@ -497,6 +503,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id) return camera_exposure_auto; case V4L2_CID_EXPOSURE_METERING: return camera_exposure_metering; + case V4L2_CID_AUTO_FOCUS_AREA: + return camera_auto_focus_area; case V4L2_CID_AUTO_FOCUS_RANGE: return camera_auto_focus_range; case V4L2_CID_COLORFX: @@ -732,6 +740,7 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_AUTO_FOCUS_STOP: return "Auto Focus, Stop"; case V4L2_CID_AUTO_FOCUS_STATUS: return "Auto Focus, Status"; case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range"; + case V4L2_CID_AUTO_FOCUS_AREA: return "Auto Focus, Area"; /* FM Radio Modulator control */ /* Keep the order of the 'case's the same as in videodev2.h! */ @@ -881,6 +890,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_AUTO_FOCUS_AREA: case V4L2_CID_AUTO_FOCUS_RANGE: case V4L2_CID_COLORFX: case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE: diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index f56c945..0eb1c1a 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -683,6 +683,12 @@ enum v4l2_auto_focus_range { V4L2_AUTO_FOCUS_RANGE_INFINITY = 3, }; +#define V4L2_CID_AUTO_FOCUS_AREA (V4L2_CID_CAMERA_CLASS_BASE+32) +enum v4l2_auto_focus_area { + V4L2_AUTO_FOCUS_AREA_ALL = 0, + V4L2_AUTO_FOCUS_AREA_RECTANGLE = 1, + V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION = 2, +}; /* FM Modulator class control IDs */