[RFC/PATCH 0/5] v4l: New camera controls

[RFC/PATCH 0/5] v4l: New camera controls

[Sylwester Nawrocki]

Hi All,

I put some effort in preparing a documentation for a couple of new controls
in the camera control class. It's a preeliminary work, it's mainly just
documentation. There is yet no patches for any driver using these controls.
I just wanted to get some possible feedback on them, if this sort of stuff
is welcome and what might need to be done differently.


Thanks,
Sylwester


Heungjun Kim (1):
  uvc: Adapt the driver to new type of V4L2_CID_FOCUS_AUTO control

Sylwester Nawrocki (4):
  v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control
  v4l: Add V4L2_CID_METERING_MODE camera control
  v4l: Add V4L2_CID_EXPOSURE_BIAS camera control
  v4l: Add V4L2_CID_ISO and V4L2_CID_ISO_AUTO controls

 Documentation/DocBook/media/v4l/biblio.xml   |   11 +++
 Documentation/DocBook/media/v4l/controls.xml |  122 ++++++++++++++++++++++++-
 drivers/media/video/uvc/uvc_ctrl.c           |    9 ++-
 drivers/media/video/v4l2-ctrls.c             |   19 ++++-
 include/linux/videodev2.h                    |   19 ++++
 5 files changed, 173 insertions(+), 7 deletions(-)

--
1.7.4.1

[RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sylwester Nawrocki]

Change the V4L2_CID_FOCUS_AUTO control type from boolean to a menu
type. In case of boolean control we had values 0 and 1 corresponding
to manual and automatic focus respectively.

The V4L2_CID_FOCUS_AUTO menu control has currently following items:
  0 - V4L2_FOCUS_MANUAL,
  1 - V4L2_FOCUS_AUTO,
  2 - V4L2_FOCUS_AUTO_MACRO,
  3 - V4L2_FOCUS_AUTO_CONTINUOUS.

To trigger single auto focus action in V4L2_FOCUS_AUTO mode the
V4L2_DO_AUTO_FOCUS control can be used, which is also added in this
patch.

Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---
 Documentation/DocBook/media/v4l/controls.xml |   52 +++++++++++++++++++++++--
 drivers/media/video/v4l2-ctrls.c             |   13 ++++++-
 include/linux/videodev2.h                    |    8 ++++
 3 files changed, 67 insertions(+), 6 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index 3bc5ee8..5ccb0b0 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -2782,12 +2782,54 @@ negative values towards infinity. This is a write-only control.</entry>
 	  </row>
 	  <row><entry></entry></row>
 
-	  <row>
+	  <row id="v4l2-focus-auto-type">
 	    <entry spanname="id"><constant>V4L2_CID_FOCUS_AUTO</constant>&nbsp;</entry>
-	    <entry>boolean</entry>
-	  </row><row><entry spanname="descr">Enables automatic focus
-adjustments. The effect of manual focus adjustments while this feature
-is enabled is undefined, drivers should ignore such requests.</entry>
+	    <entry>enum&nbsp;v4l2_focus_auto_type</entry>
+	  </row><row><entry spanname="descr">Determines the camera
+focus mode. The effect of manual focus adjustments while <constant>
+V4L2_CID_FOCUS_AUTO </constant> is not set to <constant>
+V4L2_FOCUS_MANUAL</constant> is undefined, drivers should ignore such
+requests.</entry>
+	  </row>
+	  <row>
+	    <entrytbl spanname="descr" cols="2">
+	      <tbody valign="top">
+		<row>
+		  <entry><constant>V4L2_FOCUS_MANUAL</constant>&nbsp;</entry>
+		  <entry>Manual focus.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_FOCUS_AUTO</constant>&nbsp;</entry>
+		  <entry>Single shot auto focus. When switched to
+this mode the camera focuses on a subject just once. <constant>
+V4L2_CID_DO_AUTO_FOCUS</constant> control can be used to manually
+invoke auto focusing.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_FOCUS_AUTO_MACRO</constant>&nbsp;</entry>
+		  <entry>Macro (close-up) auto focus. Usually camera
+auto focus algorithms do not attempt to focus on a subject that is
+closer than a given distance. This mode can be used to tell the camera
+to use minimum distance for focus that it is capable of.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_FOCUS_AUTO_CONTINUOUS</constant>&nbsp;</entry>
+		  <entry>Continuous auto focus. When switched to this
+mode the camera continually adjusts focus.</entry>
+		</row>
+	      </tbody>
+	    </entrytbl>
+	  </row>
+	  <row><entry></entry></row>
+
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_DO_AUTO_FOCUS</constant>&nbsp;</entry>
+	    <entry>button</entry>
+	  </row><row><entry spanname="descr">When this control is set
+the camera will perform one shot auto focus. The effect of using this
+control when <constant>V4L2_CID_FOCUS_AUTO</constant> is in mode
+different than <constant>V4L2_FOCUS_AUTO</constant> is undefined,
+drivers should ignore such requests. </entry>
 	  </row>
 	  <row><entry></entry></row>
 
diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 0f415da..7d8862f 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -221,6 +221,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		"Aperture Priority Mode",
 		NULL
 	};
+	static const char * const camera_focus_auto[] = {
+		"Manual Focus",
+		"One-shot Auto Focus",
+		"Macro Auto Focus",
+		"Continuous Auto Focus",
+		NULL
+	};
 	static const char * const colorfx[] = {
 		"None",
 		"Black & White",
@@ -388,6 +395,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:
@@ -567,6 +576,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_PRIVACY:			return "Privacy";
 	case V4L2_CID_IRIS_ABSOLUTE:		return "Iris, Absolute";
 	case V4L2_CID_IRIS_RELATIVE:		return "Iris, Relative";
+	case V4L2_CID_DO_AUTO_FOCUS:		return "Do Auto Focus";
 
 	/* FM Radio Modulator control */
 	/* Keep the order of the 'case's the same as in videodev2.h! */
@@ -633,7 +643,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:
@@ -658,6 +667,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_TILT_RESET:
 	case V4L2_CID_FLASH_STROBE:
 	case V4L2_CID_FLASH_STROBE_STOP:
+	case V4L2_CID_DO_AUTO_FOCUS:
 		*type = V4L2_CTRL_TYPE_BUTTON;
 		*flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
 		*min = *max = *step = *def = 0;
@@ -679,6 +689,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:
 	case V4L2_CID_FLASH_LED_MODE:
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 4b752d5..9acb514 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1608,6 +1608,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_AUTO_MACRO = 2,
+	V4L2_FOCUS_AUTO_CONTINUOUS = 3,
+};
 
 #define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+13)
 #define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+14)
@@ -1618,6 +1624,8 @@ enum  v4l2_exposure_auto_type {
 #define V4L2_CID_IRIS_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+17)
 #define V4L2_CID_IRIS_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+18)
 
+#define V4L2_CID_DO_AUTO_FOCUS			(V4L2_CID_CAMERA_CLASS_BASE+19)
+
 /* FM Modulator class control IDs */
 #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
 #define V4L2_CID_FM_TX_CLASS			(V4L2_CTRL_CLASS_FM_TX | 1)
-- 
1.7.4.1

[RFC/PATCH 2/5] uvc: Adapt the driver to new type of V4L2_CID_FOCUS_AUTO control

[Sylwester Nawrocki]

From: Heungjun Kim <riverful.kim@samsung.com>

The V4L2_CID_FOCUS_AUTO control has been converted from boolean type,
where control's value 0 and 1 were corresponding to manual and automatic
focus respectively, to menu type with following menu items:
  0 - V4L2_FOCUS_MANUAL,
  1 - V4L2_FOCUS_AUTO,
  2 - V4L2_FOCUS_AUTO_MACRO,
  3 - V4L2_FOCUS_AUTO_CONTINUOUS.

According to this change the uvc control mappings are modified to retain
original sematics, where 0 corresponds to manual and 1 to auto focus.

Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

---
The V4L2_CID_FOCUS_AUTO control in V4L2_FOCUS_AUTO mode does only
a one-shot auto focus, when switched from V4L2_FOCUS_MANUAL.
It might be worth to implement also the V4L2_CID_DO_AUTO_FOCUS button
control in uvc, however I didn't take time yet to better understand
the driver and add this. I also don't have any uvc hardware to test
this patch so it's just compile tested.
---
 drivers/media/video/uvc/uvc_ctrl.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/uvc/uvc_ctrl.c b/drivers/media/video/uvc/uvc_ctrl.c
index 254d326..6860ca1 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -365,6 +365,11 @@ static struct uvc_menu_info exposure_auto_controls[] = {
 	{ 8, "Aperture Priority Mode" },
 };
 
+static struct uvc_menu_info focus_auto_controls[] = {
+	{ 0, "Manual Mode" },
+	{ 1, "Auto Mode" },
+};
+
 static __s32 uvc_ctrl_get_zoom(struct uvc_control_mapping *mapping,
 	__u8 query, const __u8 *data)
 {
@@ -592,8 +597,10 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.selector	= UVC_CT_FOCUS_AUTO_CONTROL,
 		.size		= 1,
 		.offset		= 0,
-		.v4l2_type	= V4L2_CTRL_TYPE_BOOLEAN,
+		.v4l2_type	= V4L2_CTRL_TYPE_MENU,
 		.data_type	= UVC_CTRL_DATA_TYPE_BOOLEAN,
+		.menu_info	= focus_auto_controls,
+		.menu_count	= ARRAY_SIZE(focus_auto_controls),
 	},
 	{
 		.id		= V4L2_CID_IRIS_ABSOLUTE,
-- 
1.7.4.1

[RFC/PATCH 3/5] v4l: Add V4L2_CID_METERING_MODE camera control

[Sylwester Nawrocki]

The V4L2_CID_METERING_MODE control allows to determine what method
is used by the camera to measure the amount of light available for
automatic exposure control.

Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---
 Documentation/DocBook/media/v4l/controls.xml |   31 ++++++++++++++++++++++++++
 drivers/media/video/v4l2-ctrls.c             |    2 +
 include/linux/videodev2.h                    |    7 ++++++
 3 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index 5ccb0b0..53d7c08 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -2893,6 +2893,7 @@ mechanical obturation of the sensor and firmware image processing, but the
 device is not restricted to these methods. Devices that implement the privacy
 control must support read access and may support write access.</entry>
 	  </row>
+	  <row><entry></entry></row>
 
 	  <row>
 	    <entry spanname="id"><constant>V4L2_CID_BAND_STOP_FILTER</constant>&nbsp;</entry>
@@ -2902,6 +2903,36 @@ camera sensor on or off, or specify its strength. Such band-stop filters can
 be used, for example, to filter out the fluorescent light component.</entry>
 	  </row>
 	  <row><entry></entry></row>
+
+	  <row id="v4l2-metering-mode">
+	    <entry spanname="id"><constant>V4L2_CID_METERING_MODE</constant>&nbsp;</entry>
+	    <entry>enum&nbsp;v4l2_metering_mode</entry>
+	  </row><row><entry spanname="descr">Determines how the camera measures
+the amount of light available to expose a frame. Possible values are:</entry>
+	  </row>
+	  <row>
+	    <entrytbl spanname="descr" cols="2">
+	      <tbody valign="top">
+		<row>
+		  <entry><constant>V4L2_METERING_MODE_AVERAGE</constant>&nbsp;</entry>
+		  <entry>Use the light information coming from the entire scene
+and average giving no weighting to any particular portion of the metered area.
+		  </entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_METERING_MODE_CENTER_WEIGHTED</constant>&nbsp;</entry>
+		  <entry>Average the light information coming from the entire scene
+giving priority to the center of the metered area.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_METERING_MODE_SPOT</constant>&nbsp;</entry>
+		  <entry>Measure only very small area at the cent-re of the scene.</entry>
+		</row>
+	      </tbody>
+	    </entrytbl>
+	  </row>
+	  <row><entry></entry></row>
+
 	</tbody>
       </tgroup>
     </table>
diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 7d8862f..8d0cd0e 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -577,6 +577,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_IRIS_ABSOLUTE:		return "Iris, Absolute";
 	case V4L2_CID_IRIS_RELATIVE:		return "Iris, Relative";
 	case V4L2_CID_DO_AUTO_FOCUS:		return "Do Auto Focus";
+	case V4L2_CID_METERING_MODE:		return "Metering Mode";
 
 	/* FM Radio Modulator control */
 	/* Keep the order of the 'case's the same as in videodev2.h! */
@@ -703,6 +704,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC:
 	case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
 	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
+	case V4L2_CID_METERING_MODE:
 		*type = V4L2_CTRL_TYPE_MENU;
 		break;
 	case V4L2_CID_RDS_TX_PS_NAME:
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 9acb514..8956ed6 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1626,6 +1626,13 @@ enum v4l2_focus_auto_type {
 
 #define V4L2_CID_DO_AUTO_FOCUS			(V4L2_CID_CAMERA_CLASS_BASE+19)
 
+#define V4L2_CID_METERING_MODE			(V4L2_CID_CAMERA_CLASS_BASE+20)
+enum v4l2_metering_mode {
+	V4L2_METERING_MODE_AVERAGE,
+	V4L2_METERING_MODE_CENTER_WEIGHTED,
+	V4L2_METERING_MODE_SPOT,
+};
+
 /* FM Modulator class control IDs */
 #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
 #define V4L2_CID_FM_TX_CLASS			(V4L2_CTRL_CLASS_FM_TX | 1)
-- 
1.7.4.1

[RFC/PATCH 4/5] v4l: Add V4L2_CID_EXPOSURE_BIAS camera control

[Sylwester Nawrocki]

The V4L2_CID_EXPOSURE_BIAS control allows for manual exposure
compensation when automatic exposure algorithm is enabled.

Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---
 Documentation/DocBook/media/v4l/controls.xml |   16 ++++++++++++++++
 drivers/media/video/v4l2-ctrls.c             |    1 +
 include/linux/videodev2.h                    |    2 ++
 3 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index 53d7c08..ec5cbc1 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -2691,6 +2691,22 @@ and 100000 for 10 seconds.</entry>
 	  <row><entry></entry></row>
 
 	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_EXPOSURE_BIAS</constant>&nbsp;</entry>
+	    <entry>integer (menu?)</entry>
+	  </row><row><entry spanname="descr"> Determines the exposure
+compensation when <constant>V4L2_CID_EXPOSURE_AUTO</constant> control
+is set to <constant>AUTO</constant>, <constant>SHUTTER_PRIORITY
+</constant> or <constant>APERTURE_PRIORITY</constant>. It is expressed
+in terms of EV, drivers should interpret the values as 0.001 EV units,
+where the value 1000 stands for +1 EV.
+<para>Increasing the exposure compensation value is equivalent to
+decreasing the exposure value (EV) and will increase the amount of
+light at the image sensor. The camera performs the exposure compensation
+by adjusting absolute exposure time and/or aperture.</para></entry>
+	  </row>
+	  <row><entry></entry></row>
+
+	  <row>
 	    <entry spanname="id"><constant>V4L2_CID_EXPOSURE_AUTO_PRIORITY</constant>&nbsp;</entry>
 	    <entry>boolean</entry>
 	  </row><row><entry spanname="descr">When
diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 8d0cd0e..ba636f2 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -578,6 +578,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_IRIS_RELATIVE:		return "Iris, Relative";
 	case V4L2_CID_DO_AUTO_FOCUS:		return "Do Auto Focus";
 	case V4L2_CID_METERING_MODE:		return "Metering Mode";
+	case V4L2_CID_EXPOSURE_BIAS:		return "Exposure, Bias";
 
 	/* FM Radio Modulator control */
 	/* Keep the order of the 'case's the same as in videodev2.h! */
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 8956ed6..37f93cf 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1633,6 +1633,8 @@ enum v4l2_metering_mode {
 	V4L2_METERING_MODE_SPOT,
 };
 
+#define V4L2_CID_EXPOSURE_BIAS			(V4L2_CID_CAMERA_CLASS_BASE+21)
+
 /* FM Modulator class control IDs */
 #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
 #define V4L2_CID_FM_TX_CLASS			(V4L2_CTRL_CLASS_FM_TX | 1)
-- 
1.7.4.1

[RFC/PATCH 5/5] v4l: Add V4L2_CID_ISO and V4L2_CID_ISO_AUTO controls

[Sylwester Nawrocki]

Add manual and automatic ISO controls. The ISO values are related to
the level of amplification of the analog signal between image sensor
and ADC, but some sensors with an integrated SoC ISP expose
an interface to accept the ISO values directly.

Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---

These controls depend on the integer menu control support.
Corresponding patches from Sakari can be found here:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg39733.html
---
 Documentation/DocBook/media/v4l/biblio.xml   |   11 +++++++++++
 Documentation/DocBook/media/v4l/controls.xml |   23 +++++++++++++++++++++++
 drivers/media/video/v4l2-ctrls.c             |    3 +++
 include/linux/videodev2.h                    |    2 ++
 4 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/biblio.xml b/Documentation/DocBook/media/v4l/biblio.xml
index afc8a0d..61f6ff3 100644
--- a/Documentation/DocBook/media/v4l/biblio.xml
+++ b/Documentation/DocBook/media/v4l/biblio.xml
@@ -177,6 +177,17 @@ in the frequency range from 87,5 to 108,0 MHz</title>
       <title>NTSC-4: United States RBDS Standard</title>
     </biblioentry>
 
+    <biblioentry id="iso12232">
+      <abbrev>ISO&nbsp;12232:2006</abbrev>
+      <authorgroup>
+	<corpauthor>International Organization for Standardization
+(<ulink url="http://www.iso.org">http://www.iso.org</ulink>)</corpauthor>
+      </authorgroup>
+      <title>Photography &mdash; Digital still cameras &mdash; Determination
+      of exposure index, ISO speed ratings, standard output sensitivity, and
+      recommended exposure index</title>
+    </biblioentry>
+
   </bibliography>
 
   <!--
diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index ec5cbc1..48a0434 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -2949,6 +2949,29 @@ giving priority to the center of the metered area.</entry>
 	  </row>
 	  <row><entry></entry></row>
 
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_ISO</constant>&nbsp;</entry>
+	    <entry>integer menu</entry>
+	  </row><row><entry spanname="descr">Determines ISO equivalent of an
+image sensor indicating the sensor's sensitivity to light. The numbers are
+expressed in arithmetic scale, as per <xref linkend="iso12232" /> standard,
+where doubling the sensor sensitivity is represented by doubling the numerical
+ISO value. Applications should interpret the values as standard ISO values
+multiplied by 1000, e.g. control value 800 stands for ISO 0.8. Drivers will
+usually support only subset of standard ISO values.
+</entry>
+	  </row>
+	  <row><entry></entry></row>
+
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_ISO_AUTO</constant>&nbsp;</entry>
+	    <entry>boolean</entry>
+	  </row><row><entry spanname="descr">Enables automatic ISO control.
+The effect of setting <constant>V4L2_CID_ISO</constant> while automatic
+ISO control is enabled is undefined, drivers should ignore such request.</entry>
+	  </row>
+	  <row><entry></entry></row>
+
 	</tbody>
       </tgroup>
     </table>
diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index ba636f2..96ec73d 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -579,6 +579,8 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_DO_AUTO_FOCUS:		return "Do Auto Focus";
 	case V4L2_CID_METERING_MODE:		return "Metering Mode";
 	case V4L2_CID_EXPOSURE_BIAS:		return "Exposure, Bias";
+	case V4L2_CID_ISO:			return "Sensitivity (ISO)";
+	case V4L2_CID_ISO_AUTO:			return "Sensitivity (ISO), Auto";
 
 	/* FM Radio Modulator control */
 	/* Keep the order of the 'case's the same as in videodev2.h! */
@@ -638,6 +640,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_HFLIP:
 	case V4L2_CID_VFLIP:
 	case V4L2_CID_HUE_AUTO:
+	case V4L2_CID_ISO_AUTO:
 	case V4L2_CID_CHROMA_AGC:
 	case V4L2_CID_COLOR_KILLER:
 	case V4L2_CID_MPEG_AUDIO_MUTE:
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 37f93cf..d43149c 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1634,6 +1634,8 @@ enum v4l2_metering_mode {
 };
 
 #define V4L2_CID_EXPOSURE_BIAS			(V4L2_CID_CAMERA_CLASS_BASE+21)
+#define V4L2_CID_ISO				(V4L2_CID_CAMERA_CLASS_BASE+22)
+#define V4L2_CID_ISO_AUTO			(V4L2_CID_CAMERA_CLASS_BASE+23)
 
 /* FM Modulator class control IDs */
 #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
-- 
1.7.4.1

Re: [RFC/PATCH 2/5] uvc: Adapt the driver to new type of V4L2_CID_FOCUS_AUTO control

[Laurent Pinchart]

Hi Sylwester and Heungjun,

On Sunday 04 December 2011 16:16:13 Sylwester Nawrocki wrote:
> From: Heungjun Kim <riverful.kim@samsung.com>
> 
> The V4L2_CID_FOCUS_AUTO control has been converted from boolean type,
> where control's value 0 and 1 were corresponding to manual and automatic
> focus respectively, to menu type with following menu items:
>   0 - V4L2_FOCUS_MANUAL,
>   1 - V4L2_FOCUS_AUTO,
>   2 - V4L2_FOCUS_AUTO_MACRO,
>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.
> 
> According to this change the uvc control mappings are modified to retain
> original sematics, where 0 corresponds to manual and 1 to auto focus.

UVC auto-focus works in continuous mode, not single-shot mode. As the existing 
V4L2_CID_FOCUS_AUTO uses 0 to mean manual focus and 1 to mean continuous auto-
focus, shouldn't this patch set define the following values instead ?

   0 - V4L2_FOCUS_MANUAL
   1 - V4L2_FOCUS_AUTO
   2 - V4L2_FOCUS_AUTO_MACRO
   3 - V4L2_FOCUS_AUTO_SINGLE_SHOT

V4L2_FOCUS_AUTO_SINGLE_SHOT could also be named V4L2_FOCUS_SINGLE_SHOT.

> Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> 
> ---
> The V4L2_CID_FOCUS_AUTO control in V4L2_FOCUS_AUTO mode does only
> a one-shot auto focus, when switched from V4L2_FOCUS_MANUAL.
> It might be worth to implement also the V4L2_CID_DO_AUTO_FOCUS button
> control in uvc, however I didn't take time yet to better understand
> the driver and add this. I also don't have any uvc hardware to test
> this patch so it's just compile tested.
> ---
>  drivers/media/video/uvc/uvc_ctrl.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/uvc/uvc_ctrl.c
> b/drivers/media/video/uvc/uvc_ctrl.c index 254d326..6860ca1 100644
> --- a/drivers/media/video/uvc/uvc_ctrl.c
> +++ b/drivers/media/video/uvc/uvc_ctrl.c
> @@ -365,6 +365,11 @@ static struct uvc_menu_info exposure_auto_controls[] =
> { { 8, "Aperture Priority Mode" },
>  };
> 
> +static struct uvc_menu_info focus_auto_controls[] = {
> +	{ 0, "Manual Mode" },
> +	{ 1, "Auto Mode" },
> +};
> +
>  static __s32 uvc_ctrl_get_zoom(struct uvc_control_mapping *mapping,
>  	__u8 query, const __u8 *data)
>  {
> @@ -592,8 +597,10 @@ static struct uvc_control_mapping uvc_ctrl_mappings[]
> = { .selector	= UVC_CT_FOCUS_AUTO_CONTROL,
>  		.size		= 1,
>  		.offset		= 0,
> -		.v4l2_type	= V4L2_CTRL_TYPE_BOOLEAN,
> +		.v4l2_type	= V4L2_CTRL_TYPE_MENU,
>  		.data_type	= UVC_CTRL_DATA_TYPE_BOOLEAN,
> +		.menu_info	= focus_auto_controls,
> +		.menu_count	= ARRAY_SIZE(focus_auto_controls),
>  	},
>  	{
>  		.id		= V4L2_CID_IRIS_ABSOLUTE,

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Laurent Pinchart]

Hi Sylwester,

On Sunday 04 December 2011 16:16:12 Sylwester Nawrocki wrote:
> Change the V4L2_CID_FOCUS_AUTO control type from boolean to a menu
> type. In case of boolean control we had values 0 and 1 corresponding
> to manual and automatic focus respectively.
> 
> The V4L2_CID_FOCUS_AUTO menu control has currently following items:
>   0 - V4L2_FOCUS_MANUAL,
>   1 - V4L2_FOCUS_AUTO,
>   2 - V4L2_FOCUS_AUTO_MACRO,
>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.

I think the mapping is wrong, please see my answer to 2/5.

> To trigger single auto focus action in V4L2_FOCUS_AUTO mode the
> V4L2_DO_AUTO_FOCUS control can be used, which is also added in this
> patch.
> 
> Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
> ---
>  Documentation/DocBook/media/v4l/controls.xml |   52
> +++++++++++++++++++++++-- drivers/media/video/v4l2-ctrls.c             |  
> 13 ++++++-
>  include/linux/videodev2.h                    |    8 ++++
>  3 files changed, 67 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml
> b/Documentation/DocBook/media/v4l/controls.xml index 3bc5ee8..5ccb0b0
> 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -2782,12 +2782,54 @@ negative values towards infinity. This is a
> write-only control.</entry> </row>
>  	  <row><entry></entry></row>
> 
> -	  <row>
> +	  <row id="v4l2-focus-auto-type">
>  	    <entry
> spanname="id"><constant>V4L2_CID_FOCUS_AUTO</constant>&nbsp;</entry> -	   
> <entry>boolean</entry>
> -	  </row><row><entry spanname="descr">Enables automatic focus
> -adjustments. The effect of manual focus adjustments while this feature
> -is enabled is undefined, drivers should ignore such requests.</entry>
> +	    <entry>enum&nbsp;v4l2_focus_auto_type</entry>
> +	  </row><row><entry spanname="descr">Determines the camera
> +focus mode. The effect of manual focus adjustments while <constant>
> +V4L2_CID_FOCUS_AUTO </constant> is not set to <constant>
> +V4L2_FOCUS_MANUAL</constant> is undefined, drivers should ignore such
> +requests.</entry>
> +	  </row>
> +	  <row>
> +	    <entrytbl spanname="descr" cols="2">
> +	      <tbody valign="top">
> +		<row>
> +		  <entry><constant>V4L2_FOCUS_MANUAL</constant>&nbsp;</entry>
> +		  <entry>Manual focus.</entry>
> +		</row>
> +		<row>
> +		  <entry><constant>V4L2_FOCUS_AUTO</constant>&nbsp;</entry>
> +		  <entry>Single shot auto focus. When switched to
> +this mode the camera focuses on a subject just once. <constant>
> +V4L2_CID_DO_AUTO_FOCUS</constant> control can be used to manually
> +invoke auto focusing.</entry>
> +		</row>

Do we need this single-shot auto-focus menu entry ? We could just use 
V4L2_CID_DO_AUTO_FOCUS in V4L2_FOCUS_MANUAL mode to do this. This is what 
happens with one-shot white balance.

> +		<row>
> +		  <entry><constant>V4L2_FOCUS_AUTO_MACRO</constant>&nbsp;</entry>
> +		  <entry>Macro (close-up) auto focus. Usually camera
> +auto focus algorithms do not attempt to focus on a subject that is
> +closer than a given distance. This mode can be used to tell the camera
> +to use minimum distance for focus that it is capable of.</entry>
> +		</row>
> +		<row>
> +		  <entry><constant>V4L2_FOCUS_AUTO_CONTINUOUS</constant>&nbsp;</entry>
> +		  <entry>Continuous auto focus. When switched to this
> +mode the camera continually adjusts focus.</entry>
> +		</row>
> +	      </tbody>
> +	    </entrytbl>
> +	  </row>
> +	  <row><entry></entry></row>
> +
> +	  <row>
> +	    <entry
> spanname="id"><constant>V4L2_CID_DO_AUTO_FOCUS</constant>&nbsp;</entry> +	
>    <entry>button</entry>
> +	  </row><row><entry spanname="descr">When this control is set

Wouldn't "written" be better than "set" here ? I understand "When this control 
is set" as "while the control has a non-zero value" (but it might just be me).

> +the camera will perform one shot auto focus. The effect of using this
> +control when <constant>V4L2_CID_FOCUS_AUTO</constant> is in mode
> +different than <constant>V4L2_FOCUS_AUTO</constant> is undefined,
> +drivers should ignore such requests. </entry>
>  	  </row>
>  	  <row><entry></entry></row>
> 
> diff --git a/drivers/media/video/v4l2-ctrls.c
> b/drivers/media/video/v4l2-ctrls.c index 0f415da..7d8862f 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -221,6 +221,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		"Aperture Priority Mode",
>  		NULL
>  	};
> +	static const char * const camera_focus_auto[] = {
> +		"Manual Focus",
> +		"One-shot Auto Focus",
> +		"Macro Auto Focus",
> +		"Continuous Auto Focus",
> +		NULL
> +	};
>  	static const char * const colorfx[] = {
>  		"None",
>  		"Black & White",
> @@ -388,6 +395,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:
> @@ -567,6 +576,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_PRIVACY:			return "Privacy";
>  	case V4L2_CID_IRIS_ABSOLUTE:		return "Iris, Absolute";
>  	case V4L2_CID_IRIS_RELATIVE:		return "Iris, Relative";
> +	case V4L2_CID_DO_AUTO_FOCUS:		return "Do Auto Focus";
> 
>  	/* FM Radio Modulator control */
>  	/* Keep the order of the 'case's the same as in videodev2.h! */
> @@ -633,7 +643,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:
> @@ -658,6 +667,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type, case V4L2_CID_TILT_RESET:
>  	case V4L2_CID_FLASH_STROBE:
>  	case V4L2_CID_FLASH_STROBE_STOP:
> +	case V4L2_CID_DO_AUTO_FOCUS:
>  		*type = V4L2_CTRL_TYPE_BUTTON;
>  		*flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
>  		*min = *max = *step = *def = 0;
> @@ -679,6 +689,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:
>  	case V4L2_CID_FLASH_LED_MODE:
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 4b752d5..9acb514 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1608,6 +1608,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_AUTO_MACRO = 2,
> +	V4L2_FOCUS_AUTO_CONTINUOUS = 3,
> +};
> 
>  #define V4L2_CID_ZOOM_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+13)
>  #define V4L2_CID_ZOOM_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+14)
> @@ -1618,6 +1624,8 @@ enum  v4l2_exposure_auto_type {
>  #define V4L2_CID_IRIS_ABSOLUTE			(V4L2_CID_CAMERA_CLASS_BASE+17)
>  #define V4L2_CID_IRIS_RELATIVE			(V4L2_CID_CAMERA_CLASS_BASE+18)
> 
> +#define V4L2_CID_DO_AUTO_FOCUS			(V4L2_CID_CAMERA_CLASS_BASE+19)
> +
>  /* FM Modulator class control IDs */
>  #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
>  #define V4L2_CID_FM_TX_CLASS			(V4L2_CTRL_CLASS_FM_TX | 1)

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 3/5] v4l: Add V4L2_CID_METERING_MODE camera control

[Laurent Pinchart]

Hi Sylwester,

On Sunday 04 December 2011 16:16:14 Sylwester Nawrocki wrote:
> The V4L2_CID_METERING_MODE control allows to determine what method
> is used by the camera to measure the amount of light available for
> automatic exposure control.
> 
> Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
> ---
>  Documentation/DocBook/media/v4l/controls.xml |   31
> ++++++++++++++++++++++++++ drivers/media/video/v4l2-ctrls.c             | 
>   2 +
>  include/linux/videodev2.h                    |    7 ++++++
>  3 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml
> b/Documentation/DocBook/media/v4l/controls.xml index 5ccb0b0..53d7c08
> 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -2893,6 +2893,7 @@ mechanical obturation of the sensor and firmware
> image processing, but the device is not restricted to these methods.
> Devices that implement the privacy control must support read access and
> may support write access.</entry> </row>
> +	  <row><entry></entry></row>
> 
>  	  <row>
>  	    <entry
> spanname="id"><constant>V4L2_CID_BAND_STOP_FILTER</constant>&nbsp;</entry>
> @@ -2902,6 +2903,36 @@ camera sensor on or off, or specify its strength.
> Such band-stop filters can be used, for example, to filter out the
> fluorescent light component.</entry> </row>
>  	  <row><entry></entry></row>
> +
> +	  <row id="v4l2-metering-mode">
> +	    <entry
> spanname="id"><constant>V4L2_CID_METERING_MODE</constant>&nbsp;</entry> +	
>    <entry>enum&nbsp;v4l2_metering_mode</entry>
> +	  </row><row><entry spanname="descr">Determines how the camera measures
> +the amount of light available to expose a frame. Possible values
> are:</entry> +	  </row>
> +	  <row>
> +	    <entrytbl spanname="descr" cols="2">
> +	      <tbody valign="top">
> +		<row>
> +		  <entry><constant>V4L2_METERING_MODE_AVERAGE</constant>&nbsp;</entry>
> +		  <entry>Use the light information coming from the entire scene
> +and average giving no weighting to any particular portion of the metered
> area. +		  </entry>
> +		</row>
> +		<row>
> +		 
> <entry><constant>V4L2_METERING_MODE_CENTER_WEIGHTED</constant>&nbsp;</entr
> y> +		  <entry>Average the light information coming from the entire scene
> +giving priority to the center of the metered area.</entry>
> +		</row>
> +		<row>
> +		  <entry><constant>V4L2_METERING_MODE_SPOT</constant>&nbsp;</entry>
> +		  <entry>Measure only very small area at the cent-re of the
> scene.</entry> +		</row>
> +	      </tbody>

For the last two cases, would it also make sense to specify the center of the 
weighted area and the spot location ?

> +	    </entrytbl>
> +	  </row>
> +	  <row><entry></entry></row>
> +
>  	</tbody>
>        </tgroup>
>      </table>
> diff --git a/drivers/media/video/v4l2-ctrls.c
> b/drivers/media/video/v4l2-ctrls.c index 7d8862f..8d0cd0e 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -577,6 +577,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_IRIS_ABSOLUTE:		return "Iris, Absolute";
>  	case V4L2_CID_IRIS_RELATIVE:		return "Iris, Relative";
>  	case V4L2_CID_DO_AUTO_FOCUS:		return "Do Auto Focus";
> +	case V4L2_CID_METERING_MODE:		return "Metering Mode";
> 
>  	/* FM Radio Modulator control */
>  	/* Keep the order of the 'case's the same as in videodev2.h! */
> @@ -703,6 +704,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type, case V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC:
>  	case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
>  	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
> +	case V4L2_CID_METERING_MODE:
>  		*type = V4L2_CTRL_TYPE_MENU;
>  		break;
>  	case V4L2_CID_RDS_TX_PS_NAME:
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 9acb514..8956ed6 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1626,6 +1626,13 @@ enum v4l2_focus_auto_type {
> 
>  #define V4L2_CID_DO_AUTO_FOCUS			(V4L2_CID_CAMERA_CLASS_BASE+19)
> 
> +#define V4L2_CID_METERING_MODE			(V4L2_CID_CAMERA_CLASS_BASE+20)
> +enum v4l2_metering_mode {
> +	V4L2_METERING_MODE_AVERAGE,
> +	V4L2_METERING_MODE_CENTER_WEIGHTED,
> +	V4L2_METERING_MODE_SPOT,
> +};
> +
>  /* FM Modulator class control IDs */
>  #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
>  #define V4L2_CID_FM_TX_CLASS			(V4L2_CTRL_CLASS_FM_TX | 1)

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 0/5] v4l: New camera controls

[Laurent Pinchart]

Hi Sylwester,

On Sunday 04 December 2011 16:16:11 Sylwester Nawrocki wrote:
> Hi All,
> 
> I put some effort in preparing a documentation for a couple of new controls
> in the camera control class. It's a preeliminary work, it's mainly just
> documentation. There is yet no patches for any driver using these controls.
> I just wanted to get some possible feedback on them, if this sort of stuff
> is welcome and what might need to be done differently.

Thanks for the patches.

Regarding patches 3/5, 4/5 and 5/5, we should perhaps try to brainstorm this a 
bit. There's more to exposure setting than just those controls, maybe it's 
time to think about a proper exposure API. We could start by gathering 
requirements on the list, and maybe have an IRC meeting if needed.

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 3/5] v4l: Add V4L2_CID_METERING_MODE camera control

[Sylwester Nawrocki]

Hi Laurent,

thanks for the comments.

On 12/06/2011 01:32 PM, Laurent Pinchart wrote:
> On Sunday 04 December 2011 16:16:14 Sylwester Nawrocki wrote:
>> The V4L2_CID_METERING_MODE control allows to determine what method
>> is used by the camera to measure the amount of light available for
>> automatic exposure control.
>>
>> Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
>> ---
>>  Documentation/DocBook/media/v4l/controls.xml |   31
>> ++++++++++++++++++++++++++ drivers/media/video/v4l2-ctrls.c             | 
>>   2 +
>>  include/linux/videodev2.h                    |    7 ++++++
>>  3 files changed, 40 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/DocBook/media/v4l/controls.xml
>> b/Documentation/DocBook/media/v4l/controls.xml index 5ccb0b0..53d7c08
>> 100644
>> --- a/Documentation/DocBook/media/v4l/controls.xml
>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>> @@ -2893,6 +2893,7 @@ mechanical obturation of the sensor and firmware
>> image processing, but the device is not restricted to these methods.
>> Devices that implement the privacy control must support read access and
>> may support write access.</entry> </row>
>> +	  <row><entry></entry></row>
>>
>>  	  <row>
>>  	    <entry
>> spanname="id"><constant>V4L2_CID_BAND_STOP_FILTER</constant>&nbsp;</entry>
>> @@ -2902,6 +2903,36 @@ camera sensor on or off, or specify its strength.
>> Such band-stop filters can be used, for example, to filter out the
>> fluorescent light component.</entry> </row>
>>  	  <row><entry></entry></row>
>> +
>> +	  <row id="v4l2-metering-mode">
>> +	    <entry
>> spanname="id"><constant>V4L2_CID_METERING_MODE</constant>&nbsp;</entry> +	
>>    <entry>enum&nbsp;v4l2_metering_mode</entry>
>> +	  </row><row><entry spanname="descr">Determines how the camera measures
>> +the amount of light available to expose a frame. Possible values
>> are:</entry> +	  </row>
>> +	  <row>
>> +	    <entrytbl spanname="descr" cols="2">
>> +	      <tbody valign="top">
>> +		<row>
>> +		  <entry><constant>V4L2_METERING_MODE_AVERAGE</constant>&nbsp;</entry>
>> +		  <entry>Use the light information coming from the entire scene
>> +and average giving no weighting to any particular portion of the metered
>> area. +		  </entry>
>> +		</row>
>> +		<row>
>> +		 
>> <entry><constant>V4L2_METERING_MODE_CENTER_WEIGHTED</constant>&nbsp;</entr
>> y> +		  <entry>Average the light information coming from the entire scene
>> +giving priority to the center of the metered area.</entry>
>> +		</row>
>> +		<row>
>> +		  <entry><constant>V4L2_METERING_MODE_SPOT</constant>&nbsp;</entry>
>> +		  <entry>Measure only very small area at the cent-re of the
>> scene.</entry> +		</row>
>> +	      </tbody>
> 
> For the last two cases, would it also make sense to specify the center of the 
> weighted area and the spot location ?

Yes, that's quite basic requirement as well.. A means to determine the 
location would be also needed for some auto focus algorithms.
 
Additionally for V4L2_METERING_MODE_CENTER_WEIGHTED it's also needed to
specify the size of the area (width/height).

What do you think about defining new control for passing pixel position,
i.e. modifying struct v4l2_ext_control to something like:

struct v4l2_ext_control {
	__u32 id;
	__u32 size;
	__u32 reserved2[1];
	union {
		__s32 value;
		__s64 value64;
		struct v4l2_point position;
		char *string;
	};
} __attribute__ ((packed));

where:

struct v4l2_point {
	__s32 x;
	__s32 y;
};


or should we rather use ioctls for things like that ?


-- 
Regards,
Sylwester

Re: [RFC/PATCH 2/5] uvc: Adapt the driver to new type of V4L2_CID_FOCUS_AUTO control

[Sylwester Nawrocki]

Hi Laurent,

On 12/06/2011 01:26 PM, Laurent Pinchart wrote:
> On Sunday 04 December 2011 16:16:13 Sylwester Nawrocki wrote:
>> From: Heungjun Kim <riverful.kim@samsung.com>
>>
>> The V4L2_CID_FOCUS_AUTO control has been converted from boolean type,
>> where control's value 0 and 1 were corresponding to manual and automatic
>> focus respectively, to menu type with following menu items:
>>   0 - V4L2_FOCUS_MANUAL,
>>   1 - V4L2_FOCUS_AUTO,
>>   2 - V4L2_FOCUS_AUTO_MACRO,
>>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.
>>
>> According to this change the uvc control mappings are modified to retain
>> original sematics, where 0 corresponds to manual and 1 to auto focus.
> 
> UVC auto-focus works in continuous mode, not single-shot mode. As the existing

Hmm, I suspected that.

> V4L2_CID_FOCUS_AUTO uses 0 to mean manual focus and 1 to mean continuous auto-
> focus, shouldn't this patch set define the following values instead ?
> 
>    0 - V4L2_FOCUS_MANUAL
>    1 - V4L2_FOCUS_AUTO
>    2 - V4L2_FOCUS_AUTO_MACRO
>    3 - V4L2_FOCUS_AUTO_SINGLE_SHOT

It's not that bad, at least we would not have changed the existing ABI.

It depends how other focus modes are defined, e.g. V4L2_FOCUS_AUTO_MACRO.
There is also an auto focus driven by face detection module output.

The question would be whether we want to append _SINGLE_SHOT or _CONTINUOUS
to the names. And if most of the focus modes are single-shot we will
probably need a common "do auto-focus" control for them.

What do you think about such assignment:

   0 - V4L2_FOCUS_MANUAL,
   1 - V4L2_FOCUS_AUTO_CONTINUOUS,
   2 - V4L2_FOCUS_AUTO,
   3 - V4L2_FOCUS_AUTO_MACRO,

?

I'm not 100% sure right now, but the macro and "face detection"
are rather "single-shot". Single-shot focus might be more common. Perhaps
someone else can put more light on that :-)

Using a "single-shot notation" we might have something like:

   0 - V4L2_FOCUS_MANUAL,
   1 - V4L2_FOCUS_AUTO,
   2 - V4L2_FOCUS_AUTO_SINGLE_SHOT,
   3 - V4L2_FOCUS_AUTO_MACRO_SINGLE_SHOT,
   3 - V4L2_FOCUS_AUTO_FACE_DETECTION_SINGLE_SHOT,

I'm not sure if this convention is the best one.

Alternatively we could define a "do-focus" control for each mode,
but then these controls have to be properly coordinated, for exclusive
operation.

> 
> V4L2_FOCUS_AUTO_SINGLE_SHOT could also be named V4L2_FOCUS_SINGLE_SHOT.
> 
>> Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>>
>> ---
>> The V4L2_CID_FOCUS_AUTO control in V4L2_FOCUS_AUTO mode does only
>> a one-shot auto focus, when switched from V4L2_FOCUS_MANUAL.
>> It might be worth to implement also the V4L2_CID_DO_AUTO_FOCUS button
>> control in uvc, however I didn't take time yet to better understand
>> the driver and add this. I also don't have any uvc hardware to test
>> this patch so it's just compile tested.
>> ---
>>  drivers/media/video/uvc/uvc_ctrl.c |    9 ++++++++-
>>  1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/video/uvc/uvc_ctrl.c
>> b/drivers/media/video/uvc/uvc_ctrl.c index 254d326..6860ca1 100644
>> --- a/drivers/media/video/uvc/uvc_ctrl.c
>> +++ b/drivers/media/video/uvc/uvc_ctrl.c
>> @@ -365,6 +365,11 @@ static struct uvc_menu_info exposure_auto_controls[] =
>> { { 8, "Aperture Priority Mode" },
>>  };
>>
>> +static struct uvc_menu_info focus_auto_controls[] = {
>> +	{ 0, "Manual Mode" },
>> +	{ 1, "Auto Mode" },

Do you think it could be better to change this to:

+	{ 0, "Manual Focus" },
+	{ 1, "Continuous Auto Focus" },

?

--

Regards,
Sylwester

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sylwester Nawrocki]

Hi Laurent,

On 12/06/2011 01:31 PM, Laurent Pinchart wrote:
> Hi Sylwester,
> 
> On Sunday 04 December 2011 16:16:12 Sylwester Nawrocki wrote:
>> Change the V4L2_CID_FOCUS_AUTO control type from boolean to a menu
>> type. In case of boolean control we had values 0 and 1 corresponding
>> to manual and automatic focus respectively.
>>
>> The V4L2_CID_FOCUS_AUTO menu control has currently following items:
>>   0 - V4L2_FOCUS_MANUAL,
>>   1 - V4L2_FOCUS_AUTO,
>>   2 - V4L2_FOCUS_AUTO_MACRO,
>>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.
> 
> I think the mapping is wrong, please see my answer to 2/5.

Yes, agreed. Please see my answer to you answer to 2/5 :)

> 
>> To trigger single auto focus action in V4L2_FOCUS_AUTO mode the
>> V4L2_DO_AUTO_FOCUS control can be used, which is also added in this
>> patch.
>>
>> Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
>> ---
>>  Documentation/DocBook/media/v4l/controls.xml |   52
>> +++++++++++++++++++++++-- drivers/media/video/v4l2-ctrls.c             |  
>> 13 ++++++-
>>  include/linux/videodev2.h                    |    8 ++++
>>  3 files changed, 67 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/DocBook/media/v4l/controls.xml
>> b/Documentation/DocBook/media/v4l/controls.xml index 3bc5ee8..5ccb0b0
>> 100644
>> --- a/Documentation/DocBook/media/v4l/controls.xml
>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>> @@ -2782,12 +2782,54 @@ negative values towards infinity. This is a
>> write-only control.</entry> </row>
>>  	  <row><entry></entry></row>
>>
>> -	  <row>
>> +	  <row id="v4l2-focus-auto-type">
>>  	    <entry
>> spanname="id"><constant>V4L2_CID_FOCUS_AUTO</constant>&nbsp;</entry> -	   
>> <entry>boolean</entry>
>> -	  </row><row><entry spanname="descr">Enables automatic focus
>> -adjustments. The effect of manual focus adjustments while this feature
>> -is enabled is undefined, drivers should ignore such requests.</entry>
>> +	    <entry>enum&nbsp;v4l2_focus_auto_type</entry>
>> +	  </row><row><entry spanname="descr">Determines the camera
>> +focus mode. The effect of manual focus adjustments while <constant>
>> +V4L2_CID_FOCUS_AUTO </constant> is not set to <constant>
>> +V4L2_FOCUS_MANUAL</constant> is undefined, drivers should ignore such
>> +requests.</entry>
>> +	  </row>
>> +	  <row>
>> +	    <entrytbl spanname="descr" cols="2">
>> +	      <tbody valign="top">
>> +		<row>
>> +		  <entry><constant>V4L2_FOCUS_MANUAL</constant>&nbsp;</entry>
>> +		  <entry>Manual focus.</entry>
>> +		</row>
>> +		<row>
>> +		  <entry><constant>V4L2_FOCUS_AUTO</constant>&nbsp;</entry>
>> +		  <entry>Single shot auto focus. When switched to
>> +this mode the camera focuses on a subject just once. <constant>
>> +V4L2_CID_DO_AUTO_FOCUS</constant> control can be used to manually
>> +invoke auto focusing.</entry>
>> +		</row>
> 
> Do we need this single-shot auto-focus menu entry ? We could just use 
> V4L2_CID_DO_AUTO_FOCUS in V4L2_FOCUS_MANUAL mode to do this. This is what 
> happens with one-shot white balance.

There might be various flavours of single-shot auto focus, there may be some
more focus modes than those in this patch.
If we have removed them from the menu entry then we would likely have to
come up with more V4L2_CID_DO_AUTO_FOCUS_* controls.

> 
>> +		<row>
>> +		  <entry><constant>V4L2_FOCUS_AUTO_MACRO</constant>&nbsp;</entry>
>> +		  <entry>Macro (close-up) auto focus. Usually camera
>> +auto focus algorithms do not attempt to focus on a subject that is
>> +closer than a given distance. This mode can be used to tell the camera
>> +to use minimum distance for focus that it is capable of.</entry>
>> +		</row>
>> +		<row>
>> +		  <entry><constant>V4L2_FOCUS_AUTO_CONTINUOUS</constant>&nbsp;</entry>
>> +		  <entry>Continuous auto focus. When switched to this
>> +mode the camera continually adjusts focus.</entry>
>> +		</row>
>> +	      </tbody>
>> +	    </entrytbl>
>> +	  </row>
>> +	  <row><entry></entry></row>
>> +
>> +	  <row>
>> +	    <entry
>> spanname="id"><constant>V4L2_CID_DO_AUTO_FOCUS</constant>&nbsp;</entry> +	
>>    <entry>button</entry>
>> +	  </row><row><entry spanname="descr">When this control is set
> 
> Wouldn't "written" be better than "set" here ? I understand "When this control 
> is set" as "while the control has a non-zero value" (but it might just be me).

Sure, I could change to that. However this is a button control, so "written"
doesn't quite match IMHO. And we do set/get controls, rather than read/write.

Originally I had (copied form V4L2_CID_DO_WHITE_BALANCE):

"This is an action control. When it is set (the value is ignored)..."

How about "When this control is applied.." ?

>
>> +the camera will perform one shot auto focus. The effect of using this
>> +control when <constant>V4L2_CID_FOCUS_AUTO</constant> is in mode
>> +different than <constant>V4L2_FOCUS_AUTO</constant> is undefined,
>> +drivers should ignore such requests. </entry>
>>  	  </row>

--

Thanks,
Sylwester

Re: [RFC/PATCH 0/5] v4l: New camera controls

[Sylwester Nawrocki]

Hi Laurent,

On 12/06/2011 01:34 PM, Laurent Pinchart wrote:
> On Sunday 04 December 2011 16:16:11 Sylwester Nawrocki wrote:
>> Hi All,
>>
>> I put some effort in preparing a documentation for a couple of new controls
>> in the camera control class. It's a preeliminary work, it's mainly just
>> documentation. There is yet no patches for any driver using these controls.
>> I just wanted to get some possible feedback on them, if this sort of stuff
>> is welcome and what might need to be done differently.
> 
> Thanks for the patches.
> 
> Regarding patches 3/5, 4/5 and 5/5, we should perhaps try to brainstorm this a 
> bit. There's more to exposure setting than just those controls, maybe it's 
> time to think about a proper exposure API. We could start by gathering 
> requirements on the list, and maybe have an IRC meeting if needed.

Certainly the existing support for exposure setting in V4L2 is not sufficient
even for mobile camera control. I'll try to prepare a list of requirements.
It would be great to have a brainstorming session with more people experienced
in this field.


Regards,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center

Re: [RFC/PATCH 3/5] v4l: Add V4L2_CID_METERING_MODE camera control

[Sylwester Nawrocki]

On 12/06/2011 05:27 PM, Sylwester Nawrocki wrote:
>>> +		 
>>> <entry><constant>V4L2_METERING_MODE_CENTER_WEIGHTED</constant>&nbsp;</entr
>>> y> +		  <entry>Average the light information coming from the entire scene
>>> +giving priority to the center of the metered area.</entry>
>>> +		</row>
>>> +		<row>
>>> +		  <entry><constant>V4L2_METERING_MODE_SPOT</constant>&nbsp;</entry>
>>> +		  <entry>Measure only very small area at the cent-re of the
>>> scene.</entry> +		</row>
>>> +	      </tbody>
>>
>> For the last two cases, would it also make sense to specify the center of the 
>> weighted area and the spot location ?
> 
> Yes, that's quite basic requirement as well.. A means to determine the 
> location would be also needed for some auto focus algorithms.
>  
> Additionally for V4L2_METERING_MODE_CENTER_WEIGHTED it's also needed to
> specify the size of the area (width/height).
> 
> What do you think about defining new control for passing pixel position,
> i.e. modifying struct v4l2_ext_control to something like:
> 
> struct v4l2_ext_control {
> 	__u32 id;
> 	__u32 size;
> 	__u32 reserved2[1];
> 	union {
> 		__s32 value;
> 		__s64 value64;
> 		struct v4l2_point position;
> 		char *string;
> 	};
> } __attribute__ ((packed));
> 
> where:
> 
> struct v4l2_point {
> 	__s32 x;
> 	__s32 y;
> };

Hmm, that won't work since there is no way to handle the min/max/step for
more than one value. Probably the selection API could be used for specifying
the metering rectangle, or just separate controls for x, y, width, height.
Since we need to specify only locations for some controls and a rectangle for
others, probably separate controls would be more suitable.

-- 
Regards,
Sylwester

Re: [RFC/PATCH 0/5] v4l: New camera controls

[Sakari Ailus]

Hi, all!

On Wed, Dec 07, 2011 at 11:32:38AM +0100, Sylwester Nawrocki wrote:
> Hi Laurent,
> 
> On 12/06/2011 01:34 PM, Laurent Pinchart wrote:
> > On Sunday 04 December 2011 16:16:11 Sylwester Nawrocki wrote:
> >> Hi All,
> >>
> >> I put some effort in preparing a documentation for a couple of new controls
> >> in the camera control class. It's a preeliminary work, it's mainly just
> >> documentation. There is yet no patches for any driver using these controls.
> >> I just wanted to get some possible feedback on them, if this sort of stuff
> >> is welcome and what might need to be done differently.
> > 
> > Thanks for the patches.
> > 
> > Regarding patches 3/5, 4/5 and 5/5, we should perhaps try to brainstorm this a 
> > bit. There's more to exposure setting than just those controls, maybe it's 
> > time to think about a proper exposure API. We could start by gathering 
> > requirements on the list, and maybe have an IRC meeting if needed.
> 
> Certainly the existing support for exposure setting in V4L2 is not sufficient
> even for mobile camera control. I'll try to prepare a list of requirements.
> It would be great to have a brainstorming session with more people experienced
> in this field.

IRC meeting?

The proposed controls seem useful for high level control --- for example,
ISO means a combination of digital and analog gain, and also the unit is
different. In general, I think that even if the three are concerned with
similar issues, the ISO control definitely should belong to a different
class: it requires making policy decisions rather than just providing access
to the sensor features.

Cheers,

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

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sakari Ailus]

Hi Sylwester,

On Sun, Dec 04, 2011 at 04:16:12PM +0100, Sylwester Nawrocki wrote:
> Change the V4L2_CID_FOCUS_AUTO control type from boolean to a menu
> type. In case of boolean control we had values 0 and 1 corresponding
> to manual and automatic focus respectively.
> 
> The V4L2_CID_FOCUS_AUTO menu control has currently following items:
>   0 - V4L2_FOCUS_MANUAL,
>   1 - V4L2_FOCUS_AUTO,
>   2 - V4L2_FOCUS_AUTO_MACRO,
>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.

I would put the macro mode to a separate menu since it's configuration for
how the regular AF works rather than really different mode.

...

> @@ -567,6 +576,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_PRIVACY:			return "Privacy";
>  	case V4L2_CID_IRIS_ABSOLUTE:		return "Iris, Absolute";
>  	case V4L2_CID_IRIS_RELATIVE:		return "Iris, Relative";
> +	case V4L2_CID_DO_AUTO_FOCUS:		return "Do Auto Focus";

I'd perhaps use "begin" or "start". How does the user learn the autofocus
has finished? I think this looks like quite similar problem than telling the
flash strobe status to the user. The solution could also be similar to that.

Cheers,

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

Re: [RFC/PATCH 3/5] v4l: Add V4L2_CID_METERING_MODE camera control

[Sakari Ailus]

Hi Laurent and Sylwester,

On Wed, Dec 07, 2011 at 12:09:09PM +0100, Sylwester Nawrocki wrote:
> On 12/06/2011 05:27 PM, Sylwester Nawrocki wrote:
> >>> +		 
> >>> <entry><constant>V4L2_METERING_MODE_CENTER_WEIGHTED</constant>&nbsp;</entr
> >>> y> +		  <entry>Average the light information coming from the entire scene
> >>> +giving priority to the center of the metered area.</entry>
> >>> +		</row>
> >>> +		<row>
> >>> +		  <entry><constant>V4L2_METERING_MODE_SPOT</constant>&nbsp;</entry>
> >>> +		  <entry>Measure only very small area at the cent-re of the
> >>> scene.</entry> +		</row>
> >>> +	      </tbody>
> >>
> >> For the last two cases, would it also make sense to specify the center of the 
> >> weighted area and the spot location ?
> > 
> > Yes, that's quite basic requirement as well.. A means to determine the 
> > location would be also needed for some auto focus algorithms.
> >  
> > Additionally for V4L2_METERING_MODE_CENTER_WEIGHTED it's also needed to
> > specify the size of the area (width/height).
> > 
> > What do you think about defining new control for passing pixel position,
> > i.e. modifying struct v4l2_ext_control to something like:
> > 
> > struct v4l2_ext_control {
> > 	__u32 id;
> > 	__u32 size;
> > 	__u32 reserved2[1];
> > 	union {
> > 		__s32 value;
> > 		__s64 value64;
> > 		struct v4l2_point position;
> > 		char *string;
> > 	};
> > } __attribute__ ((packed));
> > 
> > where:
> > 
> > struct v4l2_point {
> > 	__s32 x;
> > 	__s32 y;
> > };
> 
> Hmm, that won't work since there is no way to handle the min/max/step for
> more than one value. Probably the selection API could be used for specifying
> the metering rectangle, or just separate controls for x, y, width, height.
> Since we need to specify only locations for some controls and a rectangle for
> others, probably separate controls would be more suitable.

I prefer the use of the selection API over controls. Also consider you may
have multiple areas for metering. Also the areas may well be different for
focus, white balance and exxposure --- they often actually are.

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

Re: [RFC/PATCH 3/5] v4l: Add V4L2_CID_METERING_MODE camera control

[Sylwester Nawrocki]

Hi Sakari,

On 12/10/2011 11:44 AM, Sakari Ailus wrote:
> On Wed, Dec 07, 2011 at 12:09:09PM +0100, Sylwester Nawrocki wrote:
>> On 12/06/2011 05:27 PM, Sylwester Nawrocki wrote:
>>>>> +		 
>>>>> <entry><constant>V4L2_METERING_MODE_CENTER_WEIGHTED</constant>&nbsp;</entr
>>>>> y> +		  <entry>Average the light information coming from the entire scene
>>>>> +giving priority to the center of the metered area.</entry>
>>>>> +		</row>
>>>>> +		<row>
>>>>> +		  <entry><constant>V4L2_METERING_MODE_SPOT</constant>&nbsp;</entry>
>>>>> +		  <entry>Measure only very small area at the cent-re of the
>>>>> scene.</entry> +		</row>
>>>>> +	      </tbody>
>>>>
>>>> For the last two cases, would it also make sense to specify the center of the 
>>>> weighted area and the spot location ?
>>>
>>> Yes, that's quite basic requirement as well.. A means to determine the 
>>> location would be also needed for some auto focus algorithms.
>>>  
>>> Additionally for V4L2_METERING_MODE_CENTER_WEIGHTED it's also needed to
>>> specify the size of the area (width/height).
>>>
>>> What do you think about defining new control for passing pixel position,
>>> i.e. modifying struct v4l2_ext_control to something like:
>>>
>>> struct v4l2_ext_control {
>>> 	__u32 id;
>>> 	__u32 size;
>>> 	__u32 reserved2[1];
>>> 	union {
>>> 		__s32 value;
>>> 		__s64 value64;
>>> 		struct v4l2_point position;
>>> 		char *string;
>>> 	};
>>> } __attribute__ ((packed));
>>>
>>> where:
>>>
>>> struct v4l2_point {
>>> 	__s32 x;
>>> 	__s32 y;
>>> };
>>
>> Hmm, that won't work since there is no way to handle the min/max/step for
>> more than one value. Probably the selection API could be used for specifying
>> the metering rectangle, or just separate controls for x, y, width, height.
>> Since we need to specify only locations for some controls and a rectangle for
>> others, probably separate controls would be more suitable.
> 
> I prefer the use of the selection API over controls. Also consider you may
> have multiple areas for metering. Also the areas may well be different for
> focus, white balance and exxposure --- they often actually are.

Yes, AFAIR Laurent expressed similar opinion on that [1].

I talked to Tomasz and Marek about configuring of the metering areas and we came
to conclusion that we could designate a group of targets for exposure, auto-focus,
etc. Define a base and target pool size so it is possible to define multiple
rectangles. And single point (pixel) would be described by struct v4l2_rect with
width=1, height=1.
Then we would probably need to be able to enumerate the targets somehow.

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

-- 
Thanks,
Sylwester

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sylwester Nawrocki]

Hi Sakari,

On 12/10/2011 11:33 AM, Sakari Ailus wrote:
> On Sun, Dec 04, 2011 at 04:16:12PM +0100, Sylwester Nawrocki wrote:
>> Change the V4L2_CID_FOCUS_AUTO control type from boolean to a menu
>> type. In case of boolean control we had values 0 and 1 corresponding
>> to manual and automatic focus respectively.
>>
>> The V4L2_CID_FOCUS_AUTO menu control has currently following items:
>>   0 - V4L2_FOCUS_MANUAL,
>>   1 - V4L2_FOCUS_AUTO,
>>   2 - V4L2_FOCUS_AUTO_MACRO,
>>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.
> 
> I would put the macro mode to a separate menu since it's configuration for
> how the regular AF works rather than really different mode.

Yes, makes sense. Most likely there could be also continuous macro auto focus..
I don't have yet an idea what could be a name for that new menu though.

Many Samsung devices have also something like guided auto focus, where the
application can specify location in the frame for focusing on. IIRC this could
be also single-shot or continuous. So it could make sense to group MACRO and
"guided" auto focus in one menu, what do you think ?


>> @@ -567,6 +576,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  	case V4L2_CID_PRIVACY:			return "Privacy";
>>  	case V4L2_CID_IRIS_ABSOLUTE:		return "Iris, Absolute";
>>  	case V4L2_CID_IRIS_RELATIVE:		return "Iris, Relative";
>> +	case V4L2_CID_DO_AUTO_FOCUS:		return "Do Auto Focus";
> 
> I'd perhaps use "begin" or "start". How does the user learn the autofocus

I agree, it's not something than is finished after G_CTRL call returns.

V4L2_CID_START_AUTO_FOCUS sounds better to me.

> has finished? I think this looks like quite similar problem than telling the
> flash strobe status to the user. The solution could also be similar to that.

I guess you're suggesting an auto focus status control? Together with the control
events it would be nice interface for notifying the applications.


-- 
Regards,
Sylwester

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sylwester Nawrocki]

On 12/10/2011 11:33 AM, Sakari Ailus wrote:
> On Sun, Dec 04, 2011 at 04:16:12PM +0100, Sylwester Nawrocki wrote:
>> Change the V4L2_CID_FOCUS_AUTO control type from boolean to a menu
>> type. In case of boolean control we had values 0 and 1 corresponding
>> to manual and automatic focus respectively.
>>
>> The V4L2_CID_FOCUS_AUTO menu control has currently following items:
>>   0 - V4L2_FOCUS_MANUAL,
>>   1 - V4L2_FOCUS_AUTO,
>>   2 - V4L2_FOCUS_AUTO_MACRO,
>>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.
> 
> I would put the macro mode to a separate menu since it's configuration for
> how the regular AF works rather than really different mode.
> 
> ...
> 
>> @@ -567,6 +576,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  	case V4L2_CID_PRIVACY:			return "Privacy";
>>  	case V4L2_CID_IRIS_ABSOLUTE:		return "Iris, Absolute";
>>  	case V4L2_CID_IRIS_RELATIVE:		return "Iris, Relative";
>> +	case V4L2_CID_DO_AUTO_FOCUS:		return "Do Auto Focus";
> 
> I'd perhaps use "begin" or "start". How does the user learn the autofocus
> has finished? I think this looks like quite similar problem than telling the
> flash strobe status to the user. The solution could also be similar to that.

Hi,

considering your previous comments, how about the below focusing control set ?

Controls for starting/stopping auto focusing (for V4L2_CID_FOCUS_AUTO ==
V4L2_FOCUS_MANUAL):

V4L2_CID_START_AUTO_FOCUS  - (button) - start auto focusing,
V4L2_CID_STOP_AUTO_FOCUS   - (button) - stop auto focusing (might be also
                                        useful in V4L2_FOCUS_AUTO_CONTINUOUS
                                        mode)
and auto focus status:

V4L2_CID_AUTO_FOCUS_STATUS - (boolean) - whether focusing is in progress or not
                             (when V4L2_CID_FOCUS_AUTO == V4L2_FOCUS_MANUAL)


V4L2_CID_FOCUS_AUTO would basically retain it's current semantics:

V4L2_CID_FOCUS_AUTO        - (boolean) - select focus type (manual/auto
                                         continuous)
        V4L2_FOCUS_MANUAL              - manual focus
        V4L2_FOCUS_AUTO_CONTINUOUS     - continuous auto focus
                                         (or V4L2_FOCUS_AUTO)

New menu control to choose behaviour of auto focus (either single-shot
or continuous):

V4L2_CID_AF_MODE                        - select auto focus mode
        V4L2_AF_MODE_NORMAL             - "normal" auto focus
        V4L2_AF_MODE_MACRO              - macro (close-up)
        V4L2_AF_MODE_SPOT               - spot location passed with
                                          selection API or other control
        V4L2_AF_MODE_FACE_DETECTION	- focus point indicated by internal
                                          face detector


I might try it out and implement in M-5MOLS driver, to see how it works
in practice.

Does it make sense for you ?

--

Regards,
Sylwester

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sakari Ailus]

Hi Sylwester,

Apologies for my late answer.

On Sat, Dec 10, 2011 at 03:42:41PM +0100, Sylwester Nawrocki wrote:
> Hi Sakari,
> 
> On 12/10/2011 11:33 AM, Sakari Ailus wrote:
> > On Sun, Dec 04, 2011 at 04:16:12PM +0100, Sylwester Nawrocki wrote:
> >> Change the V4L2_CID_FOCUS_AUTO control type from boolean to a menu
> >> type. In case of boolean control we had values 0 and 1 corresponding
> >> to manual and automatic focus respectively.
> >>
> >> The V4L2_CID_FOCUS_AUTO menu control has currently following items:
> >>   0 - V4L2_FOCUS_MANUAL,
> >>   1 - V4L2_FOCUS_AUTO,
> >>   2 - V4L2_FOCUS_AUTO_MACRO,
> >>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.
> > 
> > I would put the macro mode to a separate menu since it's configuration for
> > how the regular AF works rather than really different mode.
> 
> Yes, makes sense. Most likely there could be also continuous macro auto focus..
> I don't have yet an idea what could be a name for that new menu though.

V4L2_CID_FOCUS_AUTO_DISTANCE? It could then have choices FULL or MACRO.

> Many Samsung devices have also something like guided auto focus, where the
> application can specify location in the frame for focusing on. IIRC this could
> be also single-shot or continuous. So it could make sense to group MACRO and
> "guided" auto focus in one menu, what do you think ?

I think it could be a separate menu. It's not connected to the distance
parameter. We also need to discuss how the af statistics window
configuration is done. I'm not certain there could even be a standardised
interface which could be used to control it all.

> >> @@ -567,6 +576,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> >>  	case V4L2_CID_PRIVACY:			return "Privacy";
> >>  	case V4L2_CID_IRIS_ABSOLUTE:		return "Iris, Absolute";
> >>  	case V4L2_CID_IRIS_RELATIVE:		return "Iris, Relative";
> >> +	case V4L2_CID_DO_AUTO_FOCUS:		return "Do Auto Focus";
> > 
> > I'd perhaps use "begin" or "start". How does the user learn the autofocus
> 
> I agree, it's not something than is finished after G_CTRL call returns.
> 
> V4L2_CID_START_AUTO_FOCUS sounds better to me.
> 
> > has finished? I think this looks like quite similar problem than telling the
> > flash strobe status to the user. The solution could also be similar to that.
> 
> I guess you're suggesting an auto focus status control? Together with the control
> events it would be nice interface for notifying the applications.

I agree.

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

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sylwester Nawrocki]

On 12/31/2011 01:00 PM, Sakari Ailus wrote:
> Hi Sylwester,
> 
> Apologies for my late answer.

No problem, thanks for your comments!

> On Sat, Dec 10, 2011 at 03:42:41PM +0100, Sylwester Nawrocki wrote:
>> Hi Sakari,
>>
>> On 12/10/2011 11:33 AM, Sakari Ailus wrote:
>>> On Sun, Dec 04, 2011 at 04:16:12PM +0100, Sylwester Nawrocki wrote:
>>>> Change the V4L2_CID_FOCUS_AUTO control type from boolean to a menu
>>>> type. In case of boolean control we had values 0 and 1 corresponding
>>>> to manual and automatic focus respectively.
>>>>
>>>> The V4L2_CID_FOCUS_AUTO menu control has currently following items:
>>>>   0 - V4L2_FOCUS_MANUAL,
>>>>   1 - V4L2_FOCUS_AUTO,
>>>>   2 - V4L2_FOCUS_AUTO_MACRO,
>>>>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.
>>>
>>> I would put the macro mode to a separate menu since it's configuration for
>>> how the regular AF works rather than really different mode.
>>
>> Yes, makes sense. Most likely there could be also continuous macro auto focus..
>> I don't have yet an idea what could be a name for that new menu though.
> 
> V4L2_CID_FOCUS_AUTO_DISTANCE? It could then have choices FULL or MACRO.

How about V4L2_CID_FOCUS_AUTO_SCAN_RANGE ? Which would then have choices:
	NORMAL,
	MACRO,
	INFINITY
?

>> Many Samsung devices have also something like guided auto focus, where the
>> application can specify location in the frame for focusing on. IIRC this could
>> be also single-shot or continuous. So it could make sense to group MACRO and
>> "guided" auto focus in one menu, what do you think ?
> 
> I think it could be a separate menu. It's not connected to the distance

OK, let me summarize

* controls for starting/stopping auto focusing (V4L2_CID_FOCUS_AUTO == false)

  V4L2_CID_START_AUTO_FOCUS (button) - start auto focusing,
  V4L2_CID_STOP_AUTO_FOCUS  (button) - stop auto focusing (might be also
                                       useful in V4L2_FOCUS_AUTO == true),
* auto focus status

  V4L2_CID_AUTO_FOCUS_STATUS (menu, read-only) - whether focusing is in
                                                 progress or not,
  possible entries:

  - V4L2_AUTO_FOCUS_STATUS_IDLE,    // auto focusing not enabled or force stopped
  - V4L2_AUTO_FOCUS_STATUS_BUSY,    // focusing in progress
  - V4L2_AUTO_FOCUS_STATUS_SUCCESS, // single-shot auto focusing succeed
                                    // or continuous AF in progress
  - V4L2_AUTO_FOCUS_STATUS_FAIL,    // auto focusing failed


* V4L2_CID_FOCUS_AUTO would retain its current semantics:

  V4L2_CID_FOCUS_AUTO (boolean) - selects auto/manual focus
      false - manual
      true  - auto continuous

* AF algorithm scan range, V4L2_CID_FOCUS_AUTO_SCAN_RANGE with choices:

  - V4L2_AUTO_FOCUS_SCAN_RANGE_NORMAL,
  - V4L2_AUTO_FOCUS_SCAN_RANGE_MACRO,
  - V4L2_AUTO_FOCUS_SCAN_RANGE_INFINITY


New menu control to choose behaviour of auto focus (either single-shot
or continuous):

* select auto focus mode

V4L2_CID_AUTO_FOCUS_MODE
        V4L2_AUTO_FOCUS_MODE_NORMAL     - "normal" auto focus (whole frame?)
        V4L2_AUTO_FOCUS_MODE_SPOT       - spot location passed with other
                                          controls or selection API
        V4L2_AUTO_FOCUS_MODE_RECTANGLE  - rectangle passed with other
                                          controls or selection API


> parameter. We also need to discuss how the af statistics window
> configuration is done. I'm not certain there could even be a standardised

Do we need multiple windows for AF statistics ?

If not, I'm inclined to use four separate controls for window configuration.
(X, Y, WIDTH, HEIGHT). This was Hans' preference in previous discussions [1].

> interface which could be used to control it all.


[1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg25647.html

-- 
Regards,
Sylwester

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Laurent Pinchart]

Hi Sylwester,

On Sunday 01 January 2012 17:49:11 Sylwester Nawrocki wrote:
> On 12/31/2011 01:00 PM, Sakari Ailus wrote:
> > On Sat, Dec 10, 2011 at 03:42:41PM +0100, Sylwester Nawrocki wrote:
> >> On 12/10/2011 11:33 AM, Sakari Ailus wrote:
> >>> On Sun, Dec 04, 2011 at 04:16:12PM +0100, Sylwester Nawrocki wrote:
> >>>> Change the V4L2_CID_FOCUS_AUTO control type from boolean to a menu
> >>>> type. In case of boolean control we had values 0 and 1 corresponding
> >>>> to manual and automatic focus respectively.
> >>>> 
> >>>> The V4L2_CID_FOCUS_AUTO menu control has currently following items:
> >>>>   0 - V4L2_FOCUS_MANUAL,
> >>>>   1 - V4L2_FOCUS_AUTO,
> >>>>   2 - V4L2_FOCUS_AUTO_MACRO,
> >>>>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.
> >>> 
> >>> I would put the macro mode to a separate menu since it's configuration
> >>> for how the regular AF works rather than really different mode.
> >> 
> >> Yes, makes sense. Most likely there could be also continuous macro auto
> >> focus.. I don't have yet an idea what could be a name for that new menu
> >> though.
> > 
> > V4L2_CID_FOCUS_AUTO_DISTANCE? It could then have choices FULL or MACRO.
> 
> How about V4L2_CID_FOCUS_AUTO_SCAN_RANGE ? Which would then have choices:
> 	NORMAL,
> 	MACRO,
> 	INFINITY
> ?
> 
> >> Many Samsung devices have also something like guided auto focus, where
> >> the application can specify location in the frame for focusing on. IIRC
> >> this could be also single-shot or continuous. So it could make sense to
> >> group MACRO and "guided" auto focus in one menu, what do you think ?
> > 
> > I think it could be a separate menu. It's not connected to the distance
> 
> OK, let me summarize
> 
> * controls for starting/stopping auto focusing (V4L2_CID_FOCUS_AUTO ==
> false)
> 
>   V4L2_CID_START_AUTO_FOCUS (button) - start auto focusing,
>   V4L2_CID_STOP_AUTO_FOCUS  (button) - stop auto focusing (might be also
>                                        useful in V4L2_FOCUS_AUTO == true),

Maybe V4L2_CID_AUTO_FOCUS_START and V4L2_CID_AUTO_FOCUS_STOP to be consistent 
with the other proposed controls ?

> * auto focus status
> 
>   V4L2_CID_AUTO_FOCUS_STATUS (menu, read-only) - whether focusing is in
>                                                  progress or not,
>   possible entries:
> 
>   - V4L2_AUTO_FOCUS_STATUS_IDLE,    // auto focusing not enabled or force
> stopped - V4L2_AUTO_FOCUS_STATUS_BUSY,    // focusing in progress
>   - V4L2_AUTO_FOCUS_STATUS_SUCCESS, // single-shot auto focusing succeed
>                                     // or continuous AF in progress
>   - V4L2_AUTO_FOCUS_STATUS_FAIL,    // auto focusing failed
> 
> 
> * V4L2_CID_FOCUS_AUTO would retain its current semantics:
> 
>   V4L2_CID_FOCUS_AUTO (boolean) - selects auto/manual focus
>       false - manual
>       true  - auto continuous
> 
> * AF algorithm scan range, V4L2_CID_FOCUS_AUTO_SCAN_RANGE with choices:
> 
>   - V4L2_AUTO_FOCUS_SCAN_RANGE_NORMAL,
>   - V4L2_AUTO_FOCUS_SCAN_RANGE_MACRO,
>   - V4L2_AUTO_FOCUS_SCAN_RANGE_INFINITY
> 
> 
> New menu control to choose behaviour of auto focus (either single-shot
> or continuous):
> 
> * select auto focus mode
> 
> V4L2_CID_AUTO_FOCUS_MODE
>         V4L2_AUTO_FOCUS_MODE_NORMAL     - "normal" auto focus (whole frame?)
>         V4L2_AUTO_FOCUS_MODE_SPOT       - spot location passed with other
>         controls or selection API
>         V4L2_AUTO_FOCUS_MODE_RECTANGLE  - rectangle passed with other
>         controls or selection API

Soudns good to me.

> > parameter. We also need to discuss how the af statistics window
> > configuration is done. I'm not certain there could even be a standardised
> 
> Do we need multiple windows for AF statistics ?
>
> If not, I'm inclined to use four separate controls for window
> configuration. (X, Y, WIDTH, HEIGHT). This was Hans' preference in
> previous discussions [1].

For the OMAP3 ISP we need multiple statistics windows. AEWB can use more than 
32 windows. Having separate controls for that wouldn't be practical.

> > interface which could be used to control it all.
> 
> [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg25647.html

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sylwester Nawrocki]

Hi Laurent,

On 01/02/2012 12:16 PM, Laurent Pinchart wrote:
>> * controls for starting/stopping auto focusing (V4L2_CID_FOCUS_AUTO ==
>> false)
>>
>>   V4L2_CID_START_AUTO_FOCUS (button) - start auto focusing,
>>   V4L2_CID_STOP_AUTO_FOCUS  (button) - stop auto focusing (might be also
>>                                        useful in V4L2_FOCUS_AUTO == true),
> 
> Maybe V4L2_CID_AUTO_FOCUS_START and V4L2_CID_AUTO_FOCUS_STOP to be consistent 
> with the other proposed controls ?

Yes, you're right, I'll change them to make consistent with others.
I've noticed that too, but a little bit too late:)

>> * auto focus status
>>
>>   V4L2_CID_AUTO_FOCUS_STATUS (menu, read-only) - whether focusing is in
>>                                                  progress or not,
>>   possible entries:
>>
>>   - V4L2_AUTO_FOCUS_STATUS_IDLE,    // auto focusing not enabled or force
>>                                        stopped 
>>   - V4L2_AUTO_FOCUS_STATUS_BUSY,    // focusing in progress
>>   - V4L2_AUTO_FOCUS_STATUS_SUCCESS, // single-shot auto focusing succeed
>>                                     // or continuous AF in progress
>>   - V4L2_AUTO_FOCUS_STATUS_FAIL,    // auto focusing failed
>>
>>
>> * V4L2_CID_FOCUS_AUTO would retain its current semantics:
>>
>>   V4L2_CID_FOCUS_AUTO (boolean) - selects auto/manual focus
>>       false - manual
>>       true  - auto continuous
>>
>> * AF algorithm scan range, V4L2_CID_FOCUS_AUTO_SCAN_RANGE with choices:
>>
>>   - V4L2_AUTO_FOCUS_SCAN_RANGE_NORMAL,
>>   - V4L2_AUTO_FOCUS_SCAN_RANGE_MACRO,
>>   - V4L2_AUTO_FOCUS_SCAN_RANGE_INFINITY
>>
...
>>
>> * select auto focus mode
>>
>> V4L2_CID_AUTO_FOCUS_MODE
>>         V4L2_AUTO_FOCUS_MODE_NORMAL     - "normal" auto focus (whole frame?)
>>         V4L2_AUTO_FOCUS_MODE_SPOT       - spot location passed with other
>>         controls or selection API
>>         V4L2_AUTO_FOCUS_MODE_RECTANGLE  - rectangle passed with other
>>         controls or selection API
> 
> Soudns good to me.
>
>>> parameter. We also need to discuss how the af statistics window
>>> configuration is done. I'm not certain there could even be a standardised
>>
>> Do we need multiple windows for AF statistics ?
>>
>> If not, I'm inclined to use four separate controls for window
>> configuration. (X, Y, WIDTH, HEIGHT). This was Hans' preference in
>> previous discussions [1].
> 
> For the OMAP3 ISP we need multiple statistics windows. AEWB can use more than 
> 32 windows. Having separate controls for that wouldn't be practical.

OK, so the control API in current form doesn't seem capable of setting up the
statistics windows. There is also little space in struct v4l2_ext_control for
any major extensions.

We might need to define dedicated set of selection targets in the selection
API for handling multiple windows.

Yet, to avoid forcing applications to use the selection API where rectangles
aren't needed - only single spot coordinates, how about defining following
two controls ?

* AF spot coordinates when focus mode is set to V4L2_AUTO_FOCUS_MODE_SPOT

 - V4L2_CID_AUTO_FOCUS_POSITION_X - horizontal position in pixels relative
                                    to the left of frame
 - V4L2_CID_AUTO_FOCUS_POSITION_Y - vertical position in pixels relative
                                    to the top of frame

--

Thanks,
Sylwester

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Laurent Pinchart]

Hi Sylwester,

On Monday 02 January 2012 21:55:31 Sylwester Nawrocki wrote:
> On 01/02/2012 12:16 PM, Laurent Pinchart wrote:
> >> * controls for starting/stopping auto focusing (V4L2_CID_FOCUS_AUTO ==
> >> false)
> >> 
> >>   V4L2_CID_START_AUTO_FOCUS (button) - start auto focusing,
> >>   V4L2_CID_STOP_AUTO_FOCUS  (button) - stop auto focusing (might be also
> >>   
> >>                                        useful in V4L2_FOCUS_AUTO ==
> >>                                        true),
> > 
> > Maybe V4L2_CID_AUTO_FOCUS_START and V4L2_CID_AUTO_FOCUS_STOP to be
> > consistent with the other proposed controls ?
> 
> Yes, you're right, I'll change them to make consistent with others.
> I've noticed that too, but a little bit too late:)
> 
> >> * auto focus status
> >> 
> >>   V4L2_CID_AUTO_FOCUS_STATUS (menu, read-only) - whether focusing is in
> >>   
> >>                                                  progress or not,
> >>   
> >>   possible entries:
> >>   
> >>   - V4L2_AUTO_FOCUS_STATUS_IDLE,    // auto focusing not enabled or
> >>   force
> >>   
> >>                                        stopped
> >>   
> >>   - V4L2_AUTO_FOCUS_STATUS_BUSY,    // focusing in progress
> >>   - V4L2_AUTO_FOCUS_STATUS_SUCCESS, // single-shot auto focusing succeed
> >>   
> >>                                     // or continuous AF in progress
> >>   
> >>   - V4L2_AUTO_FOCUS_STATUS_FAIL,    // auto focusing failed
> >> 
> >> * V4L2_CID_FOCUS_AUTO would retain its current semantics:
> >>   V4L2_CID_FOCUS_AUTO (boolean) - selects auto/manual focus
> >>   
> >>       false - manual
> >>       true  - auto continuous
> >> 
> >> * AF algorithm scan range, V4L2_CID_FOCUS_AUTO_SCAN_RANGE with choices:
> >>   - V4L2_AUTO_FOCUS_SCAN_RANGE_NORMAL,
> >>   - V4L2_AUTO_FOCUS_SCAN_RANGE_MACRO,
> >>   - V4L2_AUTO_FOCUS_SCAN_RANGE_INFINITY
> 
> ...
> 
> >> * select auto focus mode
> >> 
> >> V4L2_CID_AUTO_FOCUS_MODE
> >> 
> >>         V4L2_AUTO_FOCUS_MODE_NORMAL     - "normal" auto focus (whole
> >>         frame?) V4L2_AUTO_FOCUS_MODE_SPOT       - spot location passed
> >>         with other controls or selection API
> >>         V4L2_AUTO_FOCUS_MODE_RECTANGLE  - rectangle passed with other
> >>         controls or selection API
> > 
> > Soudns good to me.
> > 
> >>> parameter. We also need to discuss how the af statistics window
> >>> configuration is done. I'm not certain there could even be a
> >>> standardised
> >> 
> >> Do we need multiple windows for AF statistics ?
> >> 
> >> If not, I'm inclined to use four separate controls for window
> >> configuration. (X, Y, WIDTH, HEIGHT). This was Hans' preference in
> >> previous discussions [1].
> > 
> > For the OMAP3 ISP we need multiple statistics windows. AEWB can use more
> > than 32 windows. Having separate controls for that wouldn't be
> > practical.
> 
> OK, so the control API in current form doesn't seem capable of setting up
> the statistics windows. There is also little space in struct
> v4l2_ext_control for any major extensions.
> 
> We might need to define dedicated set of selection targets in the selection
> API for handling multiple windows.
> 
> Yet, to avoid forcing applications to use the selection API where
> rectangles aren't needed - only single spot coordinates, how about
> defining following two controls ?
> 
> * AF spot coordinates when focus mode is set to V4L2_AUTO_FOCUS_MODE_SPOT
> 
>  - V4L2_CID_AUTO_FOCUS_POSITION_X - horizontal position in pixels relative
>                                     to the left of frame
>  - V4L2_CID_AUTO_FOCUS_POSITION_Y - vertical position in pixels relative
>                                     to the top of frame

What about a point control type instead ? :-) X and Y coordinates could be 
stored on 32 bits each.

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sakari Ailus]

Hi Sylwester,

On Sun, Jan 01, 2012 at 05:49:11PM +0100, Sylwester Nawrocki wrote:
> On 12/31/2011 01:00 PM, Sakari Ailus wrote:
> > Hi Sylwester,
> > 
> > Apologies for my late answer.
> 
> No problem, thanks for your comments!
> 
> > On Sat, Dec 10, 2011 at 03:42:41PM +0100, Sylwester Nawrocki wrote:
> >> Hi Sakari,
> >>
> >> On 12/10/2011 11:33 AM, Sakari Ailus wrote:
> >>> On Sun, Dec 04, 2011 at 04:16:12PM +0100, Sylwester Nawrocki wrote:
> >>>> Change the V4L2_CID_FOCUS_AUTO control type from boolean to a menu
> >>>> type. In case of boolean control we had values 0 and 1 corresponding
> >>>> to manual and automatic focus respectively.
> >>>>
> >>>> The V4L2_CID_FOCUS_AUTO menu control has currently following items:
> >>>>   0 - V4L2_FOCUS_MANUAL,
> >>>>   1 - V4L2_FOCUS_AUTO,
> >>>>   2 - V4L2_FOCUS_AUTO_MACRO,
> >>>>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.
> >>>
> >>> I would put the macro mode to a separate menu since it's configuration for
> >>> how the regular AF works rather than really different mode.
> >>
> >> Yes, makes sense. Most likely there could be also continuous macro auto focus..
> >> I don't have yet an idea what could be a name for that new menu though.
> > 
> > V4L2_CID_FOCUS_AUTO_DISTANCE? It could then have choices FULL or MACRO.
> 
> How about V4L2_CID_FOCUS_AUTO_SCAN_RANGE ? Which would then have choices:
> 	NORMAL,
> 	MACRO,
> 	INFINITY
> ?

What does INFINITY signify? That lens is permanently positioned there?

The name of the control sounds good to me, but I might change the order of
FOCUS and AUTO in it.

> >> Many Samsung devices have also something like guided auto focus, where the
> >> application can specify location in the frame for focusing on. IIRC this could
> >> be also single-shot or continuous. So it could make sense to group MACRO and
> >> "guided" auto focus in one menu, what do you think ?
> > 
> > I think it could be a separate menu. It's not connected to the distance
> 
> OK, let me summarize
> 
> * controls for starting/stopping auto focusing (V4L2_CID_FOCUS_AUTO == false)
> 
>   V4L2_CID_START_AUTO_FOCUS (button) - start auto focusing,
>   V4L2_CID_STOP_AUTO_FOCUS  (button) - stop auto focusing (might be also
>                                        useful in V4L2_FOCUS_AUTO == true),
> * auto focus status
> 
>   V4L2_CID_AUTO_FOCUS_STATUS (menu, read-only) - whether focusing is in
>                                                  progress or not,
>   possible entries:
> 
>   - V4L2_AUTO_FOCUS_STATUS_IDLE,    // auto focusing not enabled or force stopped
>   - V4L2_AUTO_FOCUS_STATUS_BUSY,    // focusing in progress
>   - V4L2_AUTO_FOCUS_STATUS_SUCCESS, // single-shot auto focusing succeed
>                                     // or continuous AF in progress
>   - V4L2_AUTO_FOCUS_STATUS_FAIL,    // auto focusing failed
> 
> 
> * V4L2_CID_FOCUS_AUTO would retain its current semantics:
> 
>   V4L2_CID_FOCUS_AUTO (boolean) - selects auto/manual focus
>       false - manual
>       true  - auto continuous
> 
> * AF algorithm scan range, V4L2_CID_FOCUS_AUTO_SCAN_RANGE with choices:
> 
>   - V4L2_AUTO_FOCUS_SCAN_RANGE_NORMAL,
>   - V4L2_AUTO_FOCUS_SCAN_RANGE_MACRO,
>   - V4L2_AUTO_FOCUS_SCAN_RANGE_INFINITY
> 
> 
> New menu control to choose behaviour of auto focus (either single-shot
> or continuous):
> 
> * select auto focus mode
> 
> V4L2_CID_AUTO_FOCUS_MODE
>         V4L2_AUTO_FOCUS_MODE_NORMAL     - "normal" auto focus (whole frame?)
>         V4L2_AUTO_FOCUS_MODE_SPOT       - spot location passed with other
>                                           controls or selection API
>         V4L2_AUTO_FOCUS_MODE_RECTANGLE  - rectangle passed with other
>                                           controls or selection API
> 
> 
> > parameter. We also need to discuss how the af statistics window
> > configuration is done. I'm not certain there could even be a standardised
> 
> Do we need multiple windows for AF statistics ?
> 
> If not, I'm inclined to use four separate controls for window configuration.
> (X, Y, WIDTH, HEIGHT). This was Hans' preference in previous discussions [1].
> 
> > interface which could be used to control it all.
> 
> 
> [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg25647.html
> 
> -- 
> Regards,
> Sylwester

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

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sakari Ailus]

Hi Sylwester,

On Mon, Jan 02, 2012 at 09:55:31PM +0100, Sylwester Nawrocki wrote:
> Hi Laurent,
> 
> On 01/02/2012 12:16 PM, Laurent Pinchart wrote:
> >> * controls for starting/stopping auto focusing (V4L2_CID_FOCUS_AUTO ==
> >> false)
> >>
> >>   V4L2_CID_START_AUTO_FOCUS (button) - start auto focusing,
> >>   V4L2_CID_STOP_AUTO_FOCUS  (button) - stop auto focusing (might be also
> >>                                        useful in V4L2_FOCUS_AUTO == true),
> > 
> > Maybe V4L2_CID_AUTO_FOCUS_START and V4L2_CID_AUTO_FOCUS_STOP to be consistent 
> > with the other proposed controls ?
> 
> Yes, you're right, I'll change them to make consistent with others.
> I've noticed that too, but a little bit too late:)
> 
> >> * auto focus status
> >>
> >>   V4L2_CID_AUTO_FOCUS_STATUS (menu, read-only) - whether focusing is in
> >>                                                  progress or not,
> >>   possible entries:
> >>
> >>   - V4L2_AUTO_FOCUS_STATUS_IDLE,    // auto focusing not enabled or force
> >>                                        stopped 
> >>   - V4L2_AUTO_FOCUS_STATUS_BUSY,    // focusing in progress
> >>   - V4L2_AUTO_FOCUS_STATUS_SUCCESS, // single-shot auto focusing succeed
> >>                                     // or continuous AF in progress
> >>   - V4L2_AUTO_FOCUS_STATUS_FAIL,    // auto focusing failed
> >>
> >>
> >> * V4L2_CID_FOCUS_AUTO would retain its current semantics:
> >>
> >>   V4L2_CID_FOCUS_AUTO (boolean) - selects auto/manual focus
> >>       false - manual
> >>       true  - auto continuous
> >>
> >> * AF algorithm scan range, V4L2_CID_FOCUS_AUTO_SCAN_RANGE with choices:
> >>
> >>   - V4L2_AUTO_FOCUS_SCAN_RANGE_NORMAL,
> >>   - V4L2_AUTO_FOCUS_SCAN_RANGE_MACRO,
> >>   - V4L2_AUTO_FOCUS_SCAN_RANGE_INFINITY
> >>
> ...
> >>
> >> * select auto focus mode
> >>
> >> V4L2_CID_AUTO_FOCUS_MODE
> >>         V4L2_AUTO_FOCUS_MODE_NORMAL     - "normal" auto focus (whole frame?)
> >>         V4L2_AUTO_FOCUS_MODE_SPOT       - spot location passed with other
> >>         controls or selection API
> >>         V4L2_AUTO_FOCUS_MODE_RECTANGLE  - rectangle passed with other
> >>         controls or selection API
> > 
> > Soudns good to me.
> >
> >>> parameter. We also need to discuss how the af statistics window
> >>> configuration is done. I'm not certain there could even be a standardised
> >>
> >> Do we need multiple windows for AF statistics ?
> >>
> >> If not, I'm inclined to use four separate controls for window
> >> configuration. (X, Y, WIDTH, HEIGHT). This was Hans' preference in
> >> previous discussions [1].
> > 
> > For the OMAP3 ISP we need multiple statistics windows. AEWB can use more than 
> > 32 windows. Having separate controls for that wouldn't be practical.
> 
> OK, so the control API in current form doesn't seem capable of setting up the
> statistics windows. There is also little space in struct v4l2_ext_control for
> any major extensions.
> 
> We might need to define dedicated set of selection targets in the selection
> API for handling multiple windows.
> 
> Yet, to avoid forcing applications to use the selection API where rectangles
> aren't needed - only single spot coordinates, how about defining following
> two controls ?
> 
> * AF spot coordinates when focus mode is set to V4L2_AUTO_FOCUS_MODE_SPOT
> 
>  - V4L2_CID_AUTO_FOCUS_POSITION_X - horizontal position in pixels relative
>                                     to the left of frame
>  - V4L2_CID_AUTO_FOCUS_POSITION_Y - vertical position in pixels relative
>                                     to the top of frame

What's a spot focus mode?

Any AF statistics from a single pixel have no meaning, so there has to be
more. It sounds like a small rectangle to me. :-)

But being able to provide more windows would be rather important. You don't
need to look at too special cameras before you can have seven or so focus
windows.

The selection API could be used for this, as proposed already. We could have
a new V4L2_(SUBDEV_)SELECTION_STAT_AF target to configure windows. We could
add an id field to define the window ID to struct v4l2_(subdev_)selection.
One control would be required to tell how many statistics windows do you
have.

Cheers,

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

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sylwester Nawrocki]

Hi Laurent,

On 01/03/2012 02:55 PM, Laurent Pinchart wrote:
>>>>> parameter. We also need to discuss how the af statistics window
>>>>> configuration is done. I'm not certain there could even be a
>>>>> standardised
>>>>
>>>> Do we need multiple windows for AF statistics ?
>>>>
>>>> If not, I'm inclined to use four separate controls for window
>>>> configuration. (X, Y, WIDTH, HEIGHT). This was Hans' preference in
>>>> previous discussions [1].
>>>
>>> For the OMAP3 ISP we need multiple statistics windows. AEWB can use more
>>> than 32 windows. Having separate controls for that wouldn't be
>>> practical.
>>
>> OK, so the control API in current form doesn't seem capable of setting up
>> the statistics windows. There is also little space in struct
>> v4l2_ext_control for any major extensions.
>>
>> We might need to define dedicated set of selection targets in the selection
>> API for handling multiple windows.
>>
>> Yet, to avoid forcing applications to use the selection API where
>> rectangles aren't needed - only single spot coordinates, how about
>> defining following two controls ?
>>
>> * AF spot coordinates when focus mode is set to V4L2_AUTO_FOCUS_MODE_SPOT
>>
>>  - V4L2_CID_AUTO_FOCUS_POSITION_X - horizontal position in pixels relative
>>                                     to the left of frame
>>  - V4L2_CID_AUTO_FOCUS_POSITION_Y - vertical position in pixels relative
>>                                     to the top of frame
> 
> What about a point control type instead ? :-) X and Y coordinates could be 
> stored on 32 bits each.

That's more appealing than two separate controls :-) If Hans agrees to
add a point control type (fingers crossed :)) I could prepare relevant patch
to see how it looks like. I've analysed roughly what would need to be changed,
the effort is quite significant but not so invasive for drivers.

I thought about using new V4L2_CTRL_FLAG* for VIDIOC_QUERYCTRL to indicate
which field of the point data structure is queried.

The only real problem seem to be events, I can't see simple method for adding
two sets of min/max/step/def values to the control event payload. There would
probably have to be two separate control change events for each point structure
field.

-- 

Regards,
Sylwester

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sylwester Nawrocki]

Hi Sakari,

On 01/04/2012 02:22 PM, Sakari Ailus wrote:
>>>>>> The V4L2_CID_FOCUS_AUTO menu control has currently following items:
>>>>>>   0 - V4L2_FOCUS_MANUAL,
>>>>>>   1 - V4L2_FOCUS_AUTO,
>>>>>>   2 - V4L2_FOCUS_AUTO_MACRO,
>>>>>>   3 - V4L2_FOCUS_AUTO_CONTINUOUS.
>>>>>
>>>>> I would put the macro mode to a separate menu since it's configuration for
>>>>> how the regular AF works rather than really different mode.
>>>>
>>>> Yes, makes sense. Most likely there could be also continuous macro auto focus..
>>>> I don't have yet an idea what could be a name for that new menu though.
>>>
>>> V4L2_CID_FOCUS_AUTO_DISTANCE? It could then have choices FULL or MACRO.
>>
>> How about V4L2_CID_FOCUS_AUTO_SCAN_RANGE ? Which would then have choices:
>> 	NORMAL,
>> 	MACRO,
>> 	INFINITY
>> ?
> 
> What does INFINITY signify? That lens is permanently positioned there?

Yes, I think that's just what it is supposed to mean, but I would have to double
check in the specs.

HeungJun, would you have some more details about the INFINITY setting ?

> The name of the control sounds good to me, but I might change the order of
> FOCUS and AUTO in it.

Ok, I'll fix it.

--

Thanks,
Sylwester

Re: [RFC/PATCH 1/5] v4l: Convert V4L2_CID_FOCUS_AUTO control to a menu control

[Sylwester Nawrocki]

Hi Sakari,

On 01/04/2012 03:04 PM, Sakari Ailus wrote:
>> On 01/02/2012 12:16 PM, Laurent Pinchart wrote:
>>>> * controls for starting/stopping auto focusing (V4L2_CID_FOCUS_AUTO ==
>>>> false)
>>>>
>>>>    V4L2_CID_START_AUTO_FOCUS (button) - start auto focusing,
>>>>    V4L2_CID_STOP_AUTO_FOCUS  (button) - stop auto focusing (might be also
>>>>                                         useful in V4L2_FOCUS_AUTO == true),
>>>
>>> Maybe V4L2_CID_AUTO_FOCUS_START and V4L2_CID_AUTO_FOCUS_STOP to be consistent
>>> with the other proposed controls ?
>>
>> Yes, you're right, I'll change them to make consistent with others.
>> I've noticed that too, but a little bit too late:)
>>
>>>> * auto focus status
>>>>
>>>>    V4L2_CID_AUTO_FOCUS_STATUS (menu, read-only) - whether focusing is in
>>>>                                                   progress or not,
>>>>    possible entries:
>>>>
>>>>    - V4L2_AUTO_FOCUS_STATUS_IDLE,    // auto focusing not enabled or force
>>>>                                         stopped
>>>>    - V4L2_AUTO_FOCUS_STATUS_BUSY,    // focusing in progress
>>>>    - V4L2_AUTO_FOCUS_STATUS_SUCCESS, // single-shot auto focusing succeed
>>>>                                      // or continuous AF in progress
>>>>    - V4L2_AUTO_FOCUS_STATUS_FAIL,    // auto focusing failed
>>>>
>>>>
>>>> * V4L2_CID_FOCUS_AUTO would retain its current semantics:
>>>>
>>>>    V4L2_CID_FOCUS_AUTO (boolean) - selects auto/manual focus
>>>>        false - manual
>>>>        true  - auto continuous
>>>>
>>>> * AF algorithm scan range, V4L2_CID_FOCUS_AUTO_SCAN_RANGE with choices:
>>>>
>>>>    - V4L2_AUTO_FOCUS_SCAN_RANGE_NORMAL,
>>>>    - V4L2_AUTO_FOCUS_SCAN_RANGE_MACRO,
>>>>    - V4L2_AUTO_FOCUS_SCAN_RANGE_INFINITY
>>>>
>> ...
>>>>
>>>> * select auto focus mode
>>>>
>>>> V4L2_CID_AUTO_FOCUS_MODE
>>>>          V4L2_AUTO_FOCUS_MODE_NORMAL     - "normal" auto focus (whole frame?)
>>>>          V4L2_AUTO_FOCUS_MODE_SPOT       - spot location passed with other
>>>>          controls or selection API
>>>>          V4L2_AUTO_FOCUS_MODE_RECTANGLE  - rectangle passed with other
>>>>          controls or selection API
>>>
>>> Soudns good to me.
>>>
>>>>> parameter. We also need to discuss how the af statistics window
>>>>> configuration is done. I'm not certain there could even be a standardised
>>>>
>>>> Do we need multiple windows for AF statistics ?
>>>>
>>>> If not, I'm inclined to use four separate controls for window
>>>> configuration. (X, Y, WIDTH, HEIGHT). This was Hans' preference in
>>>> previous discussions [1].
>>>
>>> For the OMAP3 ISP we need multiple statistics windows. AEWB can use more than
>>> 32 windows. Having separate controls for that wouldn't be practical.
>>
>> OK, so the control API in current form doesn't seem capable of setting up the
>> statistics windows. There is also little space in struct v4l2_ext_control for
>> any major extensions.
>>
>> We might need to define dedicated set of selection targets in the selection
>> API for handling multiple windows.
>>
>> Yet, to avoid forcing applications to use the selection API where rectangles
>> aren't needed - only single spot coordinates, how about defining following
>> two controls ?
>>
>> * AF spot coordinates when focus mode is set to V4L2_AUTO_FOCUS_MODE_SPOT
>>
>>   - V4L2_CID_AUTO_FOCUS_POSITION_X - horizontal position in pixels relative
>>                                      to the left of frame
>>   - V4L2_CID_AUTO_FOCUS_POSITION_Y - vertical position in pixels relative
>>                                      to the top of frame
> 
> What's a spot focus mode?
> 
> Any AF statistics from a single pixel have no meaning, so there has to be
> more. It sounds like a small rectangle to me. :-)

Yes, it must be a small rectangle. But there may be no way to configure or 
read this rectangle's parameters, hence this menu option. If some hardware 
doesn't implement such feature it can just skip the spot focus menu item.

And in case camera doesn't provide any interface for AF statistics windows 
configuration I don't see a good reason to use the selection API. 

> But being able to provide more windows would be rather important. You don't
> need to look at too special cameras before you can have seven or so focus
> windows.
> 
> The selection API could be used for this, as proposed already. We could have
> a new V4L2_(SUBDEV_)SELECTION_STAT_AF target to configure windows. We could
> add an id field to define the window ID to struct v4l2_(subdev_)selection.
> One control would be required to tell how many statistics windows do you
> have.

Yeah, sounds good to me, better than defining selection target base and
the target pool size. :) Ack.

--
Regards,
Sylwester