Re: [RFC PATCH 3/4] v4l: Add V4L2_CID_WDR button control

2011-12-30 Thread Sakari Ailus
Hi HeungJun,

On Wed, Dec 28, 2011 at 03:23:47PM +0900, HeungJun, Kim wrote:
> It adds the new CID for setting White Balance Preset. This CID is provided as
> button type. This can commands only if the camera turn on/off this function.
> 
> Signed-off-by: HeungJun, Kim 
> Signed-off-by: Kyungmin Park 
> ---
>  Documentation/DocBook/media/v4l/controls.xml |   12 
>  drivers/media/video/v4l2-ctrls.c |2 ++
>  include/linux/videodev2.h|2 ++
>  3 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml 
> b/Documentation/DocBook/media/v4l/controls.xml
> index afe1845..bed6c66 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -2958,6 +2958,18 @@ it one step further. This is a write-only 
> control.
> 
>  
> 
> + V4L2_CID_WDR

Just a simple comment: how about V4L2_CID_WIDE_DYNAMIC_RANGE instead? I
dont't think it'd be too long.

> + button
> +   
> +   
> + Wide Dynamic Range. It makes
> + the image be more clear by adjusting the image's intensity
> + of the illumination. This function can be provided according to
> + the capability of the hardware(sensor or AP's multimedia block).
> + 
> +   
> +
> +   
>spanname="id">V4L2_CID_PRIVACY 
>   boolean
> Prevent video from being acquired
> diff --git a/drivers/media/video/v4l2-ctrls.c 
> b/drivers/media/video/v4l2-ctrls.c
> index fef58c2..66110bc 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -598,6 +598,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>   case V4L2_CID_IRIS_RELATIVE:return "Iris, Relative";
>   case V4L2_CID_PRESET_WHITE_BALANCE: return "White Balance, Preset";
>   case V4L2_CID_SCENEMODE:return "Scenemode";
> + case V4L2_CID_WDR:  return "Wide Dynamic Range";
>  
>   /* FM Radio Modulator control */
>   /* Keep the order of the 'case's the same as in videodev2.h! */
> @@ -687,6 +688,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
> v4l2_ctrl_type *type,
>   break;
>   case V4L2_CID_PAN_RESET:
>   case V4L2_CID_TILT_RESET:
> + case V4L2_CID_WDR:
>   case V4L2_CID_FLASH_STROBE:
>   case V4L2_CID_FLASH_STROBE_STOP:
>   *type = V4L2_CTRL_TYPE_BUTTON;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index bc14feb..f85ad6c 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1646,6 +1646,8 @@ enum v4l2_scenemode {
>   V4L2_SCENEMODE_CANDLE = 14,
>  };
>  
> +#define V4L2_CID_WDR (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
> 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [RFC PATCH 3/4] v4l: Add V4L2_CID_WDR button control

2011-12-29 Thread HeungJun, Kim
Hi,

> -Original Message-
> From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
> Sent: Friday, December 30, 2011 9:13 AM
> To: HeungJun, Kim
> Cc: linux-media@vger.kernel.org; mche...@redhat.com; hverk...@xs4all.nl;
> sakari.ai...@iki.fi; s.nawro...@samsung.com; kyungmin.p...@samsung.com
> Subject: Re: [RFC PATCH 3/4] v4l: Add V4L2_CID_WDR button control
> 
> Hi,
> 
> On Thursday 29 December 2011 06:52:55 HeungJun, Kim wrote:
> > On Wednesday, December 28, 2011 10:57 PM Laurent Pinchart wrote:
> > > On Wednesday 28 December 2011 07:23:47 HeungJun, Kim wrote:
> > > > It adds the new CID for setting White Balance Preset. This CID is
> > > > provided
> > >
> > > I suppose you mean wide dynamic range here.
> >
> > Right, it's my miss.
> >
> > > > as button type. This can commands only if the camera turn on/off this
> > > > function.
> > >
> > > Shouldn't it be a boolean ? A button can only be activated, for one-shot
> > > auto- focus for instance.
> >
> > Any type can be possible, and fine to me. But, it depends on the whole
> > hardware architecture. The WDR is proceeded and used only in the ISP or
> > another engine processing image. And, the cases I've seen ever, are just
> > one - The ISP exists in the sensor.
> >
> > In M-5MOLS use-case, the ISP is in the M-5MOLS sensor. To the position of
> > developer,
> > it's just ok to turn on/off for using this. But, in the other architecture
> > it might be need more.
> 
> You can't turn a button control on or off. A button control can only be
> activated, it has no state. On/off controls are boolean controls.
Ah, ok. I'll modify this to Boolean for next version. You're right.

> 
> > But, I anticipate if the other architecture use this function, probably
> > any other setting seems be not needed any more. The photographer just says,
> > "turn on the WDR!", not says "adjust parm 1, 2, 3, and turn on WDR!". :)
> >
> > So, IMHO, I think the any other setting is not needed more for now.
> 
> --
> Regards,
> 
> Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 3/4] v4l: Add V4L2_CID_WDR button control

2011-12-29 Thread Laurent Pinchart
Hi,

On Thursday 29 December 2011 06:52:55 HeungJun, Kim wrote:
> On Wednesday, December 28, 2011 10:57 PM Laurent Pinchart wrote:
> > On Wednesday 28 December 2011 07:23:47 HeungJun, Kim wrote:
> > > It adds the new CID for setting White Balance Preset. This CID is
> > > provided
> > 
> > I suppose you mean wide dynamic range here.
> 
> Right, it's my miss.
> 
> > > as button type. This can commands only if the camera turn on/off this
> > > function.
> > 
> > Shouldn't it be a boolean ? A button can only be activated, for one-shot
> > auto- focus for instance.
> 
> Any type can be possible, and fine to me. But, it depends on the whole
> hardware architecture. The WDR is proceeded and used only in the ISP or
> another engine processing image. And, the cases I've seen ever, are just
> one - The ISP exists in the sensor.
> 
> In M-5MOLS use-case, the ISP is in the M-5MOLS sensor. To the position of
> developer,
> it's just ok to turn on/off for using this. But, in the other architecture
> it might be need more.

You can't turn a button control on or off. A button control can only be 
activated, it has no state. On/off controls are boolean controls.

> But, I anticipate if the other architecture use this function, probably
> any other setting seems be not needed any more. The photographer just says,
> "turn on the WDR!", not says "adjust parm 1, 2, 3, and turn on WDR!". :)
> 
> So, IMHO, I think the any other setting is not needed more for now.

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [RFC PATCH 3/4] v4l: Add V4L2_CID_WDR button control

2011-12-28 Thread HeungJun, Kim
Hi,

> -Original Message-
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Laurent Pinchart
> Sent: Wednesday, December 28, 2011 10:57 PM
> To: HeungJun, Kim
> Cc: linux-media@vger.kernel.org; mche...@redhat.com; hverk...@xs4all.nl;
> sakari.ai...@iki.fi; s.nawro...@samsung.com; kyungmin.p...@samsung.com
> Subject: Re: [RFC PATCH 3/4] v4l: Add V4L2_CID_WDR button control
> 
> hi,
> 
> On Wednesday 28 December 2011 07:23:47 HeungJun, Kim wrote:
> > It adds the new CID for setting White Balance Preset. This CID is provided
> 
> I suppose you mean wide dynamic range here.
Right, it's my miss.

> 
> > as button type. This can commands only if the camera turn on/off this
> > function.
> 
> Shouldn't it be a boolean ? A button can only be activated, for one-shot auto-
> focus for instance.
Any type can be possible, and fine to me. But, it depends on the whole hardware
architecture. The WDR is proceeded and used only in the ISP or another engine
processing image. And, the cases I've seen ever, are just one - The ISP exists
in the sensor.

In M-5MOLS use-case, the ISP is in the M-5MOLS sensor. To the position of
developer,
it's just ok to turn on/off for using this. But, in the other architecture
it might be need more.

But, I anticipate if the other architecture use this function, probably
any other setting seems be not needed any more. The photographer just says,
"turn on the WDR!", not says "adjust parm 1, 2, 3, and turn on WDR!". :)

So, IMHO, I think the any other setting is not needed more for now.

> 
> > Signed-off-by: HeungJun, Kim 
> > Signed-off-by: Kyungmin Park 
> > ---
> >  Documentation/DocBook/media/v4l/controls.xml |   12 
> >  drivers/media/video/v4l2-ctrls.c |2 ++
> >  include/linux/videodev2.h|2 ++
> >  3 files changed, 16 insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/DocBook/media/v4l/controls.xml
> > b/Documentation/DocBook/media/v4l/controls.xml index afe1845..bed6c66
> > 100644
> > --- a/Documentation/DocBook/media/v4l/controls.xml
> > +++ b/Documentation/DocBook/media/v4l/controls.xml
> > @@ -2958,6 +2958,18 @@ it one step further. This is a write-only
> > control. 
> >
> >   
> > +   V4L2_CID_WDR
> > +   button
> > + 
> > + 
> > +   Wide Dynamic Range. It makes
> > +   the image be more clear by adjusting the image's intensity
> > +   of the illumination. This function can be provided according to
> > +   the capability of the hardware(sensor or AP's multimedia block).
> > +   
> > + 
> > +
> > + 
> >  > spanname="id">V4L2_CID_PRIVACY 
> > boolean
> >   Prevent video from being acquired
> > diff --git a/drivers/media/video/v4l2-ctrls.c
> > b/drivers/media/video/v4l2-ctrls.c index fef58c2..66110bc 100644
> > --- a/drivers/media/video/v4l2-ctrls.c
> > +++ b/drivers/media/video/v4l2-ctrls.c
> > @@ -598,6 +598,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> > case V4L2_CID_IRIS_RELATIVE:return "Iris, Relative";
> > case V4L2_CID_PRESET_WHITE_BALANCE: return "White Balance, Preset";
> > case V4L2_CID_SCENEMODE:return "Scenemode";
> > +   case V4L2_CID_WDR:  return "Wide Dynamic Range";
> >
> > /* FM Radio Modulator control */
> > /* Keep the order of the 'case's the same as in videodev2.h! */
> > @@ -687,6 +688,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> > v4l2_ctrl_type *type, break;
> > case V4L2_CID_PAN_RESET:
> > case V4L2_CID_TILT_RESET:
> > +   case V4L2_CID_WDR:
> > case V4L2_CID_FLASH_STROBE:
> > case V4L2_CID_FLASH_STROBE_STOP:
> > *type = V4L2_CTRL_TYPE_BUTTON;
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index bc14feb..f85ad6c 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -1646,6 +1646,8 @@ enum v4l2_scenemode {
> > V4L2_SCENEMODE_CANDLE = 14,
> >  };
> >
> > +#define V4L2_CID_WDR
(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)
> 
> --
> Regards,
> 
> Laurent Pinchart
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 3/4] v4l: Add V4L2_CID_WDR button control

2011-12-28 Thread Laurent Pinchart
hi,

On Wednesday 28 December 2011 07:23:47 HeungJun, Kim wrote:
> It adds the new CID for setting White Balance Preset. This CID is provided

I suppose you mean wide dynamic range here.

> as button type. This can commands only if the camera turn on/off this
> function.

Shouldn't it be a boolean ? A button can only be activated, for one-shot auto-
focus for instance.

> Signed-off-by: HeungJun, Kim 
> Signed-off-by: Kyungmin Park 
> ---
>  Documentation/DocBook/media/v4l/controls.xml |   12 
>  drivers/media/video/v4l2-ctrls.c |2 ++
>  include/linux/videodev2.h|2 ++
>  3 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml
> b/Documentation/DocBook/media/v4l/controls.xml index afe1845..bed6c66
> 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -2958,6 +2958,18 @@ it one step further. This is a write-only
> control. 
> 
> 
> + V4L2_CID_WDR
> + button
> +   
> +   
> + Wide Dynamic Range. It makes
> + the image be more clear by adjusting the image's intensity
> + of the illumination. This function can be provided according to
> + the capability of the hardware(sensor or AP's multimedia block).
> + 
> +   
> +
> +   
>spanname="id">V4L2_CID_PRIVACY 
> boolean
> Prevent video from being acquired
> diff --git a/drivers/media/video/v4l2-ctrls.c
> b/drivers/media/video/v4l2-ctrls.c index fef58c2..66110bc 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -598,6 +598,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>   case V4L2_CID_IRIS_RELATIVE:return "Iris, Relative";
>   case V4L2_CID_PRESET_WHITE_BALANCE: return "White Balance, Preset";
>   case V4L2_CID_SCENEMODE:return "Scenemode";
> + case V4L2_CID_WDR:  return "Wide Dynamic Range";
> 
>   /* FM Radio Modulator control */
>   /* Keep the order of the 'case's the same as in videodev2.h! */
> @@ -687,6 +688,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type, break;
>   case V4L2_CID_PAN_RESET:
>   case V4L2_CID_TILT_RESET:
> + case V4L2_CID_WDR:
>   case V4L2_CID_FLASH_STROBE:
>   case V4L2_CID_FLASH_STROBE_STOP:
>   *type = V4L2_CTRL_TYPE_BUTTON;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index bc14feb..f85ad6c 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1646,6 +1646,8 @@ enum v4l2_scenemode {
>   V4L2_SCENEMODE_CANDLE = 14,
>  };
> 
> +#define V4L2_CID_WDR (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)

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html