On Monday 30 April 2012 17:50:57 Hans Verkuil wrote:
> On Friday 27 April 2012 16:23:21 Sylwester Nawrocki wrote:
> > Add V4L2_CID_WIDE_DYNAMIC_RANGE camera class control for camera wide
> > dynamic range (WDR, HDR) feature. This control has now only menu entries
> > for enabling and disabling WDR. It can be extended when the wide dynamic
> > range technique selection is needed.

Never mind, I get it. It's for future expansion.

That said, I find it dubious to make this an enum.

I would go with a boolean control and perhaps make a remark that it might become
an enum in the future if more options are needed.

Regards,

        Hans

> > 
> > Signed-off-by: HeungJun Kim <riverful....@samsung.com>
> > Signed-off-by: Sylwester Nawrocki <s.nawro...@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
> > ---
> >  Documentation/DocBook/media/v4l/controls.xml |   28 
> > ++++++++++++++++++++++++++
> >  drivers/media/video/v4l2-ctrls.c             |    9 +++++++++
> >  include/linux/videodev2.h                    |    6 ++++++
> >  3 files changed, 43 insertions(+)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/controls.xml 
> > b/Documentation/DocBook/media/v4l/controls.xml
> > index b671a70..487b7b5 100644
> > --- a/Documentation/DocBook/media/v4l/controls.xml
> > +++ b/Documentation/DocBook/media/v4l/controls.xml
> > @@ -3018,6 +3018,34 @@ sky. It corresponds approximately to 9000...10000 K 
> > color temperature.
> >       </row>
> >       <row><entry></entry></row>
> >  
> > +     <row id="v4l2-wide-dynamic-range-type">
> > +       <entry 
> > spanname="id"><constant>V4L2_CID_WIDE_DYNAMIC_RANGE</constant></entry>
> > +       <entry>enum&nbsp;v4l2_wide_dynamic_range_type</entry>
> > +     </row>
> > +     <row>
> > +       <entry spanname="descr">Enables or disables the camera's wide 
> > dynamic
> > +range feature. This feature allows to obtain clear images in situations 
> > where
> > +intensity of the illumination varies significantly throughout the scene, 
> > i.e.
> > +there are simultaneously very dark and very bright areas. It is most 
> > commonly
> > +realized in cameras by combining two subsequent frames with different 
> > exposure
> > +times.</entry>
> > +     </row>
> > +     <row>
> > +       <entrytbl spanname="descr" cols="2">
> > +         <tbody valign="top">
> > +           <row>
> > +             
> > <entry><constant>V4L2_WIDE_DYNAMIC_RANGE_DISABLED</constant></entry>
> > +             <entry>Wide dynamic range is disabled.</entry>
> > +           </row>
> > +           <row>
> > +             
> > <entry><constant>V4L2_WIDE_DYNAMIC_RANGE_ENABLED</constant></entry>
> > +             <entry>Wide dynamic range is enabled.</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 02fa9b0..ad2f035 100644
> > --- a/drivers/media/video/v4l2-ctrls.c
> > +++ b/drivers/media/video/v4l2-ctrls.c
> > @@ -256,6 +256,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> >             "Shade",
> >             NULL,
> >     };
> > +   static const char * const camera_wide_dynamic_range[] = {
> > +           "Disabled",
> > +           "Enabled",
> > +           NULL
> > +   };
> 
> Huh? Why isn't this a boolean control? This looks very weird.
> 
> Regards,
> 
>       Hans
> 
> >     static const char * const tune_preemphasis[] = {
> >             "No Preemphasis",
> >             "50 Microseconds",
> > @@ -427,6 +432,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> >             return colorfx;
> >     case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE:
> >             return auto_n_preset_white_balance;
> > +   case V4L2_CID_WIDE_DYNAMIC_RANGE:
> > +           return camera_wide_dynamic_range;
> >     case V4L2_CID_TUNE_PREEMPHASIS:
> >             return tune_preemphasis;
> >     case V4L2_CID_FLASH_LED_MODE:
> > @@ -614,6 +621,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> >     case V4L2_CID_IRIS_RELATIVE:            return "Iris, Relative";
> >     case V4L2_CID_AUTO_EXPOSURE_BIAS:       return "Auto Exposure, Bias";
> >     case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE: return "White Balance, Auto 
> > & Preset";
> > +   case V4L2_CID_WIDE_DYNAMIC_RANGE:       return "Wide Dynamic Range";
> >  
> >     /* FM Radio Modulator control */
> >     /* Keep the order of the 'case's the same as in videodev2.h! */
> > @@ -751,6 +759,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
> > v4l2_ctrl_type *type,
> >     case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
> >     case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
> >     case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
> > +   case V4L2_CID_WIDE_DYNAMIC_RANGE:
> >             *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 08891e6..3ca9b10 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -1709,6 +1709,12 @@ enum v4l2_auto_n_preset_white_balance_type {
> >     V4L2_WHITE_BALANCE_SHADE                = 9,
> >  };
> >  
> > +#define V4L2_CID_WIDE_DYNAMIC_RANGE                
> > (V4L2_CID_CAMERA_CLASS_BASE+21)
> > +enum v4l2_wide_dynamic_range_type {
> > +   V4L2_WIDE_DYNAMIC_RANGE_DISABLED        = 0,
> > +   V4L2_WIDE_DYNAMIC_RANGE_ENABLED         = 1,
> > +};
> > +
> >  /* 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)
> > 
> 
--
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

Reply via email to