Re: [PATCH v1 2/3] v4l: Add AUTO option for the V4L2_CID_POWER_LINE_FREQUENCY control

2011-09-21 Thread Sakari Ailus
On Wed, Sep 21, 2011 at 02:47:29PM +0200, Laurent Pinchart wrote:
> Hi Sylwester,

Hi Laurent and Sylwester,

> On Wednesday 21 September 2011 14:28:25 Sylwester Nawrocki wrote:
> > On 09/21/2011 12:17 AM, Sakari Ailus wrote:
> > > On Tue, Sep 20, 2011 at 11:25:31PM +0200, Sylwester Nawrocki wrote:
> > >> On 09/20/2011 10:57 PM, Sakari Ailus wrote:
> > >>> On Tue, Sep 20, 2011 at 01:58:58PM +0200, Sylwester Nawrocki wrote:
> >  V4L2_CID_POWER_LINE_FREQUENCY control allows applications to instruct
> >  a driver what is the power line frequency so an appropriate filter
> >  can be used by the device to cancel flicker by compensating the light
> >  intensity ripple and thus. Currently in the menu we have entries for
> >  50 and 60 Hz and for entirely disabling the anti-flicker filter.
> >  However some devices are capable of automatically detecting the
> >  frequency, so add V4L2_CID_POWER_LINE_FREQUENCY_AUTO entry for them.
> >  
> >  Signed-off-by: Sylwester Nawrocki
> >  Signed-off-by: Kyungmin Park
> >  Acked-by: Laurent Pinchart
> >  ---
> >  
> >    Documentation/DocBook/media/v4l/controls.xml |5 +++--
> >    drivers/media/video/v4l2-ctrls.c |1 +
> >    include/linux/videodev2.h|1 +
> >    3 files changed, 5 insertions(+), 2 deletions(-)
> >  
> >  diff --git a/Documentation/DocBook/media/v4l/controls.xml
> >  b/Documentation/DocBook/media/v4l/controls.xml index 2420e4a..c6b3c46
> >  100644
> >  --- a/Documentation/DocBook/media/v4l/controls.xml
> >  +++ b/Documentation/DocBook/media/v4l/controls.xml
> >  @@ -232,8 +232,9 @@ control is deprecated. New drivers and
> >  applications should use the
> >  
> > Enables a power line frequency filter to avoid
> >    
> >    flicker. Possible values forenum
> >    v4l2_power_line_frequency  are:
> >    V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  (0),
> >  
> >  -V4L2_CID_POWER_LINE_FREQUENCY_50HZ  (1) and
> >  -V4L2_CID_POWER_LINE_FREQUENCY_60HZ  (2).
> >  +V4L2_CID_POWER_LINE_FREQUENCY_50HZ  (1),
> >  +V4L2_CID_POWER_LINE_FREQUENCY_60HZ  (2) and
> >  +V4L2_CID_POWER_LINE_FREQUENCY_AUTO  (3).
> > >>> 
> > >>> A stupid question: wouldn't this be a case for a new control for
> > >>> automatic power line frequency, in other words enabling or disabling
> > >>> it?
> > >> 
> > >> IMO this would complicate things in kernel and user land, without any
> > >> reasonable positive effects. AUTO seems to fit well here, it's just
> > >> another mode of operation of a power line noise filter. Why make things
> > >> more complicated than they need to be ?
> > > 
> > > The advantage would be to be able to get the power line frquency if
> > > that's supported by the hardware. This implementation excludes that.
> > > Such information might be interesting to add e.g. to the image's exif
> > > data.
> > 
> > AFAIU, the power line frequency filter just modifies frame exposure time to
> > be multiple of half of the mains frequency period. So it's the exposure
> > time that gets finally affected. Maybe there is some hardware that
> > supports retrieving of the detected frequency, however I'm not aware of
> > it. And it doesn't seem useful unless you want to use camera as some
> > non-standard measurement tool. It also takes some time until the detection
> > algorithm locks, during this time an undefined frequency value would be
> > read.
> > 
> > I believe the filter settings do not really apply to still capture as it
> > involves periodic operation, like preview. Even if we had this as meta
> > data tag, there are more direct raw image parameters than the PL noise
> > filter frequency.
> > 
> > I feel uncomfortable with having 2 controls, where one can disable the
> > filter and the other enable it with AUTO setting.
> > Let's say the sensor supports 4 distinct settings of the filter: OFF, 50HZ,
> > 60HZ, AUTO. (there is already one sensor driver in mainline that support
> > it - ov519). How do we map this onto 2 controls ?
> > 
> > What do we return from the menu control that covers { OFF, 50HZ, 60HZ }
> > when AUTO mode is enabled through the other control and H/W doesn't allow
> > to read the detected frequency ?
> > 
> > I think, for the 2 controls we would need the DISABLED entry not to belong
> > to V4L2_CID_POWER_LINE_FREQUENCY at first place.
> > 
> > > Not sure if that's important, though.
> > 
> > I would say no, but someone can prove me wrong. And who knows what kind of
> > strange H/W future brings.
> 
> I think it all boils down to whether V4L2_CID_POWER_LINE_FREQUENCY is the 
> power line frequency filter control or the power line frequency control. In 
> the first case it doesn't make sense to use two separate controls. In the 
> second case it could.
> 
> I don't think we need two controls for this, but that's just a personal 
> opinion of the "I don't think we need to bother

Re: [PATCH v1 2/3] v4l: Add AUTO option for the V4L2_CID_POWER_LINE_FREQUENCY control

2011-09-21 Thread Laurent Pinchart
Hi Sylwester,

On Wednesday 21 September 2011 14:28:25 Sylwester Nawrocki wrote:
> On 09/21/2011 12:17 AM, Sakari Ailus wrote:
> > On Tue, Sep 20, 2011 at 11:25:31PM +0200, Sylwester Nawrocki wrote:
> >> On 09/20/2011 10:57 PM, Sakari Ailus wrote:
> >>> On Tue, Sep 20, 2011 at 01:58:58PM +0200, Sylwester Nawrocki wrote:
>  V4L2_CID_POWER_LINE_FREQUENCY control allows applications to instruct
>  a driver what is the power line frequency so an appropriate filter
>  can be used by the device to cancel flicker by compensating the light
>  intensity ripple and thus. Currently in the menu we have entries for
>  50 and 60 Hz and for entirely disabling the anti-flicker filter.
>  However some devices are capable of automatically detecting the
>  frequency, so add V4L2_CID_POWER_LINE_FREQUENCY_AUTO entry for them.
>  
>  Signed-off-by: Sylwester Nawrocki
>  Signed-off-by: Kyungmin Park
>  Acked-by: Laurent Pinchart
>  ---
>  
>    Documentation/DocBook/media/v4l/controls.xml |5 +++--
>    drivers/media/video/v4l2-ctrls.c |1 +
>    include/linux/videodev2.h|1 +
>    3 files changed, 5 insertions(+), 2 deletions(-)
>  
>  diff --git a/Documentation/DocBook/media/v4l/controls.xml
>  b/Documentation/DocBook/media/v4l/controls.xml index 2420e4a..c6b3c46
>  100644
>  --- a/Documentation/DocBook/media/v4l/controls.xml
>  +++ b/Documentation/DocBook/media/v4l/controls.xml
>  @@ -232,8 +232,9 @@ control is deprecated. New drivers and
>  applications should use the
>  
>   Enables a power line frequency filter to avoid
>    
>    flicker. Possible values forenum
>    v4l2_power_line_frequency  are:
>    V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  (0),
>  
>  -V4L2_CID_POWER_LINE_FREQUENCY_50HZ  (1) and
>  -V4L2_CID_POWER_LINE_FREQUENCY_60HZ  (2).
>  +V4L2_CID_POWER_LINE_FREQUENCY_50HZ  (1),
>  +V4L2_CID_POWER_LINE_FREQUENCY_60HZ  (2) and
>  +V4L2_CID_POWER_LINE_FREQUENCY_AUTO  (3).
> >>> 
> >>> A stupid question: wouldn't this be a case for a new control for
> >>> automatic power line frequency, in other words enabling or disabling
> >>> it?
> >> 
> >> IMO this would complicate things in kernel and user land, without any
> >> reasonable positive effects. AUTO seems to fit well here, it's just
> >> another mode of operation of a power line noise filter. Why make things
> >> more complicated than they need to be ?
> > 
> > The advantage would be to be able to get the power line frquency if
> > that's supported by the hardware. This implementation excludes that.
> > Such information might be interesting to add e.g. to the image's exif
> > data.
> 
> AFAIU, the power line frequency filter just modifies frame exposure time to
> be multiple of half of the mains frequency period. So it's the exposure
> time that gets finally affected. Maybe there is some hardware that
> supports retrieving of the detected frequency, however I'm not aware of
> it. And it doesn't seem useful unless you want to use camera as some
> non-standard measurement tool. It also takes some time until the detection
> algorithm locks, during this time an undefined frequency value would be
> read.
> 
> I believe the filter settings do not really apply to still capture as it
> involves periodic operation, like preview. Even if we had this as meta
> data tag, there are more direct raw image parameters than the PL noise
> filter frequency.
> 
> I feel uncomfortable with having 2 controls, where one can disable the
> filter and the other enable it with AUTO setting.
> Let's say the sensor supports 4 distinct settings of the filter: OFF, 50HZ,
> 60HZ, AUTO. (there is already one sensor driver in mainline that support
> it - ov519). How do we map this onto 2 controls ?
> 
> What do we return from the menu control that covers { OFF, 50HZ, 60HZ }
> when AUTO mode is enabled through the other control and H/W doesn't allow
> to read the detected frequency ?
> 
> I think, for the 2 controls we would need the DISABLED entry not to belong
> to V4L2_CID_POWER_LINE_FREQUENCY at first place.
> 
> > Not sure if that's important, though.
> 
> I would say no, but someone can prove me wrong. And who knows what kind of
> strange H/W future brings.

I think it all boils down to whether V4L2_CID_POWER_LINE_FREQUENCY is the 
power line frequency filter control or the power line frequency control. In 
the first case it doesn't make sense to use two separate controls. In the 
second case it could.

I don't think we need two controls for this, but that's just a personal 
opinion of the "I don't think we need to bother" type :-)

-- 
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: [PATCH v1 2/3] v4l: Add AUTO option for the V4L2_CID_POWER_LINE_FREQUENCY control

2011-09-21 Thread Sylwester Nawrocki
Hi Sakari,

On 09/21/2011 12:17 AM, Sakari Ailus wrote:
> On Tue, Sep 20, 2011 at 11:25:31PM +0200, Sylwester Nawrocki wrote:
>> On 09/20/2011 10:57 PM, Sakari Ailus wrote:
>>> On Tue, Sep 20, 2011 at 01:58:58PM +0200, Sylwester Nawrocki wrote:
 V4L2_CID_POWER_LINE_FREQUENCY control allows applications to instruct
 a driver what is the power line frequency so an appropriate filter
 can be used by the device to cancel flicker by compensating the light
 intensity ripple and thus. Currently in the menu we have entries for
 50 and 60 Hz and for entirely disabling the anti-flicker filter.
 However some devices are capable of automatically detecting the
 frequency, so add V4L2_CID_POWER_LINE_FREQUENCY_AUTO entry for them.

 Signed-off-by: Sylwester Nawrocki
 Signed-off-by: Kyungmin Park
 Acked-by: Laurent Pinchart
 ---
   Documentation/DocBook/media/v4l/controls.xml |5 +++--
   drivers/media/video/v4l2-ctrls.c |1 +
   include/linux/videodev2.h|1 +
   3 files changed, 5 insertions(+), 2 deletions(-)

 diff --git a/Documentation/DocBook/media/v4l/controls.xml 
 b/Documentation/DocBook/media/v4l/controls.xml
 index 2420e4a..c6b3c46 100644
 --- a/Documentation/DocBook/media/v4l/controls.xml
 +++ b/Documentation/DocBook/media/v4l/controls.xml
 @@ -232,8 +232,9 @@ control is deprecated. New drivers and applications 
 should use the
Enables a power line frequency filter to avoid
   flicker. Possible values forenum 
 v4l2_power_line_frequency  are:
   V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  (0),
 -V4L2_CID_POWER_LINE_FREQUENCY_50HZ  (1) and
 -V4L2_CID_POWER_LINE_FREQUENCY_60HZ  (2).
 +V4L2_CID_POWER_LINE_FREQUENCY_50HZ  (1),
 +V4L2_CID_POWER_LINE_FREQUENCY_60HZ  (2) and
 +V4L2_CID_POWER_LINE_FREQUENCY_AUTO  (3).
>>>
>>> A stupid question: wouldn't this be a case for a new control for automatic
>>> power line frequency, in other words enabling or disabling it?
>>
>> IMO this would complicate things in kernel and user land, without any 
>> reasonable
>> positive effects. AUTO seems to fit well here, it's just another mode of 
>> operation
>> of a power line noise filter. Why make things more complicated than they 
>> need to be ? 
> 
> The advantage would be to be able to get the power line frquency if that's
> supported by the hardware. This implementation excludes that. Such
> information might be interesting to add e.g. to the image's exif data.

AFAIU, the power line frequency filter just modifies frame exposure time to be
multiple of half of the mains frequency period. So it's the exposure time that 
gets
finally affected. Maybe there is some hardware that supports retrieving of the 
detected
frequency, however I'm not aware of it. And it doesn't seem useful unless you 
want
to use camera as some non-standard measurement tool. It also takes some time 
until
the detection algorithm locks, during this time an undefined frequency value 
would
be read. 

I believe the filter settings do not really apply to still capture as it 
involves
periodic operation, like preview. Even if we had this as meta data tag, there 
are
more direct raw image parameters than the PL noise filter frequency.

I feel uncomfortable with having 2 controls, where one can disable the filter 
and
the other enable it with AUTO setting. 
Let's say the sensor supports 4 distinct settings of the filter: OFF, 50HZ, 
60HZ, AUTO.
(there is already one sensor driver in mainline that support it - ov519).
How do we map this onto 2 controls ?

What do we return from the menu control that covers { OFF, 50HZ, 60HZ } when 
AUTO
mode is enabled through the other control and H/W doesn't allow to read the 
detected
frequency ?

I think, for the 2 controls we would need the DISABLED entry not to belong to
V4L2_CID_POWER_LINE_FREQUENCY at first place.

> 
> Not sure if that's important, though.

I would say no, but someone can prove me wrong. And who knows what kind of 
strange
H/W future brings.


Regards,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center
--
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: [PATCH v1 2/3] v4l: Add AUTO option for the V4L2_CID_POWER_LINE_FREQUENCY control

2011-09-20 Thread Sakari Ailus
On Tue, Sep 20, 2011 at 11:25:31PM +0200, Sylwester Nawrocki wrote:
> Hi Sakari,

Hi,

> On 09/20/2011 10:57 PM, Sakari Ailus wrote:
> > Hi Sylwester,
> > 
> > On Tue, Sep 20, 2011 at 01:58:58PM +0200, Sylwester Nawrocki wrote:
> >> V4L2_CID_POWER_LINE_FREQUENCY control allows applications to instruct
> >> a driver what is the power line frequency so an appropriate filter
> >> can be used by the device to cancel flicker by compensating the light
> >> intensity ripple and thus. Currently in the menu we have entries for
> >> 50 and 60 Hz and for entirely disabling the anti-flicker filter.
> >> However some devices are capable of automatically detecting the
> >> frequency, so add V4L2_CID_POWER_LINE_FREQUENCY_AUTO entry for them.
> >>
> >> Signed-off-by: Sylwester Nawrocki
> >> Signed-off-by: Kyungmin Park
> >> Acked-by: Laurent Pinchart
> >> ---
> >>   Documentation/DocBook/media/v4l/controls.xml |5 +++--
> >>   drivers/media/video/v4l2-ctrls.c |1 +
> >>   include/linux/videodev2.h|1 +
> >>   3 files changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/DocBook/media/v4l/controls.xml 
> >> b/Documentation/DocBook/media/v4l/controls.xml
> >> index 2420e4a..c6b3c46 100644
> >> --- a/Documentation/DocBook/media/v4l/controls.xml
> >> +++ b/Documentation/DocBook/media/v4l/controls.xml
> >> @@ -232,8 +232,9 @@ control is deprecated. New drivers and applications 
> >> should use the
> >>Enables a power line frequency filter to avoid
> >>   flicker. Possible values forenum 
> >> v4l2_power_line_frequency  are:
> >>   V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  (0),
> >> -V4L2_CID_POWER_LINE_FREQUENCY_50HZ  (1) and
> >> -V4L2_CID_POWER_LINE_FREQUENCY_60HZ  (2).
> >> +V4L2_CID_POWER_LINE_FREQUENCY_50HZ  (1),
> >> +V4L2_CID_POWER_LINE_FREQUENCY_60HZ  (2) and
> >> +V4L2_CID_POWER_LINE_FREQUENCY_AUTO  (3).
> > 
> > A stupid question: wouldn't this be a case for a new control for automatic
> > power line frequency, in other words enabling or disabling it?
> 
> IMO this would complicate things in kernel and user land, without any 
> reasonable
> positive effects. AUTO seems to fit well here, it's just another mode of 
> operation
> of a power line noise filter. Why make things more complicated than they need 
> to be ? 

The advantage would be to be able to get the power line frquency if that's
supported by the hardware. This implementation excludes that. Such
information might be interesting to add e.g. to the image's exif data.

Not sure if that's important, though.

> > 
> >>
> >>
> >>V4L2_CID_HUE_AUTO
> >> diff --git a/drivers/media/video/v4l2-ctrls.c 
> >> b/drivers/media/video/v4l2-ctrls.c
> >> index 06b6014..20abe5d 100644
> >> --- a/drivers/media/video/v4l2-ctrls.c
> >> +++ b/drivers/media/video/v4l2-ctrls.c
> >> @@ -210,6 +210,7 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> >>"Disabled",
> >>"50 Hz",
> >>"60 Hz",
> >> +  "Auto",
> >>NULL
> >>};
> >>static const char * const camera_exposure_auto[] = {
> >> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> >> index c33f462..87210f1 100644
> >> --- a/include/linux/videodev2.h
> >> +++ b/include/linux/videodev2.h
> >> @@ -1125,6 +1125,7 @@ enum v4l2_power_line_frequency {
> >>V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  = 0,
> >>V4L2_CID_POWER_LINE_FREQUENCY_50HZ  = 1,
> >>V4L2_CID_POWER_LINE_FREQUENCY_60HZ  = 2,
> >> +  V4L2_CID_POWER_LINE_FREQUENCY_AUTO  = 3,
> >>   };
> >>   #define V4L2_CID_HUE_AUTO(V4L2_CID_BASE+25)
> >>   #define V4L2_CID_WHITE_BALANCE_TEMPERATURE   (V4L2_CID_BASE+26)
> >> -- 
> >> 1.7.6.3
> >>
> > 
> 
> --
> Thanks,
> Sylwester

-- 
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: [PATCH v1 2/3] v4l: Add AUTO option for the V4L2_CID_POWER_LINE_FREQUENCY control

2011-09-20 Thread Sylwester Nawrocki
Hi Sakari,

On 09/20/2011 10:57 PM, Sakari Ailus wrote:
> Hi Sylwester,
> 
> On Tue, Sep 20, 2011 at 01:58:58PM +0200, Sylwester Nawrocki wrote:
>> V4L2_CID_POWER_LINE_FREQUENCY control allows applications to instruct
>> a driver what is the power line frequency so an appropriate filter
>> can be used by the device to cancel flicker by compensating the light
>> intensity ripple and thus. Currently in the menu we have entries for
>> 50 and 60 Hz and for entirely disabling the anti-flicker filter.
>> However some devices are capable of automatically detecting the
>> frequency, so add V4L2_CID_POWER_LINE_FREQUENCY_AUTO entry for them.
>>
>> Signed-off-by: Sylwester Nawrocki
>> Signed-off-by: Kyungmin Park
>> Acked-by: Laurent Pinchart
>> ---
>>   Documentation/DocBook/media/v4l/controls.xml |5 +++--
>>   drivers/media/video/v4l2-ctrls.c |1 +
>>   include/linux/videodev2.h|1 +
>>   3 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/DocBook/media/v4l/controls.xml 
>> b/Documentation/DocBook/media/v4l/controls.xml
>> index 2420e4a..c6b3c46 100644
>> --- a/Documentation/DocBook/media/v4l/controls.xml
>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>> @@ -232,8 +232,9 @@ control is deprecated. New drivers and applications 
>> should use the
>>  Enables a power line frequency filter to avoid
>>   flicker. Possible values forenum 
>> v4l2_power_line_frequency  are:
>>   V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  (0),
>> -V4L2_CID_POWER_LINE_FREQUENCY_50HZ  (1) and
>> -V4L2_CID_POWER_LINE_FREQUENCY_60HZ  (2).
>> +V4L2_CID_POWER_LINE_FREQUENCY_50HZ  (1),
>> +V4L2_CID_POWER_LINE_FREQUENCY_60HZ  (2) and
>> +V4L2_CID_POWER_LINE_FREQUENCY_AUTO  (3).
> 
> A stupid question: wouldn't this be a case for a new control for automatic
> power line frequency, in other words enabling or disabling it?

IMO this would complicate things in kernel and user land, without any reasonable
positive effects. AUTO seems to fit well here, it's just another mode of 
operation
of a power line noise filter. Why make things more complicated than they need 
to be ? 

> 
>>  
>>  
>>  V4L2_CID_HUE_AUTO
>> diff --git a/drivers/media/video/v4l2-ctrls.c 
>> b/drivers/media/video/v4l2-ctrls.c
>> index 06b6014..20abe5d 100644
>> --- a/drivers/media/video/v4l2-ctrls.c
>> +++ b/drivers/media/video/v4l2-ctrls.c
>> @@ -210,6 +210,7 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>  "Disabled",
>>  "50 Hz",
>>  "60 Hz",
>> +"Auto",
>>  NULL
>>  };
>>  static const char * const camera_exposure_auto[] = {
>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>> index c33f462..87210f1 100644
>> --- a/include/linux/videodev2.h
>> +++ b/include/linux/videodev2.h
>> @@ -1125,6 +1125,7 @@ enum v4l2_power_line_frequency {
>>  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  = 0,
>>  V4L2_CID_POWER_LINE_FREQUENCY_50HZ  = 1,
>>  V4L2_CID_POWER_LINE_FREQUENCY_60HZ  = 2,
>> +V4L2_CID_POWER_LINE_FREQUENCY_AUTO  = 3,
>>   };
>>   #define V4L2_CID_HUE_AUTO  (V4L2_CID_BASE+25)
>>   #define V4L2_CID_WHITE_BALANCE_TEMPERATURE (V4L2_CID_BASE+26)
>> -- 
>> 1.7.6.3
>>
> 

--
Thanks,
Sylwester
--
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: [PATCH v1 2/3] v4l: Add AUTO option for the V4L2_CID_POWER_LINE_FREQUENCY control

2011-09-20 Thread Sakari Ailus
Hi Sylwester,

On Tue, Sep 20, 2011 at 01:58:58PM +0200, Sylwester Nawrocki wrote:
> V4L2_CID_POWER_LINE_FREQUENCY control allows applications to instruct
> a driver what is the power line frequency so an appropriate filter
> can be used by the device to cancel flicker by compensating the light
> intensity ripple and thus. Currently in the menu we have entries for
> 50 and 60 Hz and for entirely disabling the anti-flicker filter.
> However some devices are capable of automatically detecting the
> frequency, so add V4L2_CID_POWER_LINE_FREQUENCY_AUTO entry for them.
> 
> Signed-off-by: Sylwester Nawrocki 
> Signed-off-by: Kyungmin Park 
> Acked-by: Laurent Pinchart 
> ---
>  Documentation/DocBook/media/v4l/controls.xml |5 +++--
>  drivers/media/video/v4l2-ctrls.c |1 +
>  include/linux/videodev2.h|1 +
>  3 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml 
> b/Documentation/DocBook/media/v4l/controls.xml
> index 2420e4a..c6b3c46 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -232,8 +232,9 @@ control is deprecated. New drivers and applications 
> should use the
>   Enables a power line frequency filter to avoid
>  flicker. Possible values for enum 
> v4l2_power_line_frequency are:
>  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED (0),
> -V4L2_CID_POWER_LINE_FREQUENCY_50HZ (1) and
> -V4L2_CID_POWER_LINE_FREQUENCY_60HZ (2).
> +V4L2_CID_POWER_LINE_FREQUENCY_50HZ (1),
> +V4L2_CID_POWER_LINE_FREQUENCY_60HZ (2) and
> +V4L2_CID_POWER_LINE_FREQUENCY_AUTO (3).

A stupid question: wouldn't this be a case for a new control for automatic
power line frequency, in other words enabling or disabling it?

> 
> 
>   V4L2_CID_HUE_AUTO
> diff --git a/drivers/media/video/v4l2-ctrls.c 
> b/drivers/media/video/v4l2-ctrls.c
> index 06b6014..20abe5d 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -210,6 +210,7 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>   "Disabled",
>   "50 Hz",
>   "60 Hz",
> + "Auto",
>   NULL
>   };
>   static const char * const camera_exposure_auto[] = {
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index c33f462..87210f1 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1125,6 +1125,7 @@ enum v4l2_power_line_frequency {
>   V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  = 0,
>   V4L2_CID_POWER_LINE_FREQUENCY_50HZ  = 1,
>   V4L2_CID_POWER_LINE_FREQUENCY_60HZ  = 2,
> + V4L2_CID_POWER_LINE_FREQUENCY_AUTO  = 3,
>  };
>  #define V4L2_CID_HUE_AUTO(V4L2_CID_BASE+25)
>  #define V4L2_CID_WHITE_BALANCE_TEMPERATURE   (V4L2_CID_BASE+26)
> -- 
> 1.7.6.3
> 

-- 
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


[PATCH v1 2/3] v4l: Add AUTO option for the V4L2_CID_POWER_LINE_FREQUENCY control

2011-09-20 Thread Sylwester Nawrocki
V4L2_CID_POWER_LINE_FREQUENCY control allows applications to instruct
a driver what is the power line frequency so an appropriate filter
can be used by the device to cancel flicker by compensating the light
intensity ripple and thus. Currently in the menu we have entries for
50 and 60 Hz and for entirely disabling the anti-flicker filter.
However some devices are capable of automatically detecting the
frequency, so add V4L2_CID_POWER_LINE_FREQUENCY_AUTO entry for them.

Signed-off-by: Sylwester Nawrocki 
Signed-off-by: Kyungmin Park 
Acked-by: Laurent Pinchart 
---
 Documentation/DocBook/media/v4l/controls.xml |5 +++--
 drivers/media/video/v4l2-ctrls.c |1 +
 include/linux/videodev2.h|1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/controls.xml 
b/Documentation/DocBook/media/v4l/controls.xml
index 2420e4a..c6b3c46 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -232,8 +232,9 @@ control is deprecated. New drivers and applications should 
use the
Enables a power line frequency filter to avoid
 flicker. Possible values for enum 
v4l2_power_line_frequency are:
 V4L2_CID_POWER_LINE_FREQUENCY_DISABLED (0),
-V4L2_CID_POWER_LINE_FREQUENCY_50HZ (1) and
-V4L2_CID_POWER_LINE_FREQUENCY_60HZ (2).
+V4L2_CID_POWER_LINE_FREQUENCY_50HZ (1),
+V4L2_CID_POWER_LINE_FREQUENCY_60HZ (2) and
+V4L2_CID_POWER_LINE_FREQUENCY_AUTO (3).
  
  
V4L2_CID_HUE_AUTO
diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 06b6014..20abe5d 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -210,6 +210,7 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
"Disabled",
"50 Hz",
"60 Hz",
+   "Auto",
NULL
};
static const char * const camera_exposure_auto[] = {
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index c33f462..87210f1 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1125,6 +1125,7 @@ enum v4l2_power_line_frequency {
V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  = 0,
V4L2_CID_POWER_LINE_FREQUENCY_50HZ  = 1,
V4L2_CID_POWER_LINE_FREQUENCY_60HZ  = 2,
+   V4L2_CID_POWER_LINE_FREQUENCY_AUTO  = 3,
 };
 #define V4L2_CID_HUE_AUTO  (V4L2_CID_BASE+25)
 #define V4L2_CID_WHITE_BALANCE_TEMPERATURE (V4L2_CID_BASE+26)
-- 
1.7.6.3

--
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