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

Message ID 1323011776-15967-4-git-send-email-snjw23@gmail.com (mailing list archive)
State RFC, archived
Headers

Commit Message

Sylwester Nawrocki Dec. 4, 2011, 3:16 p.m. UTC
  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(-)
  

Comments

Laurent Pinchart Dec. 6, 2011, 12:32 p.m. UTC | #1
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)
  
Sylwester Nawrocki Dec. 6, 2011, 4:27 p.m. UTC | #2
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 ?
  
Sylwester Nawrocki Dec. 7, 2011, 11:09 a.m. UTC | #3
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.
  
Sakari Ailus Dec. 10, 2011, 10:44 a.m. UTC | #4
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.
  
Sylwester Nawrocki Dec. 10, 2011, 2:14 p.m. UTC | #5
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
  

Patch

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)