[libcamera-devel] libcamera: controls: Add controls for AEC/AGC flicker avoidance
diff mbox series

Message ID 20230125132018.5725-1-david.plowman@raspberrypi.com
State Superseded
Headers show
Series
  • [libcamera-devel] libcamera: controls: Add controls for AEC/AGC flicker avoidance
Related show

Commit Message

David Plowman Jan. 25, 2023, 1:20 p.m. UTC
Flicker is the term used to describe brightness banding or oscillation
of images caused typically by artificial lighting driven by a 50 or
60Hz mains supply. We add two controls intended to be used by AEC/AGC
algorithms:

AeFlickerMode to determine whether flicker avoidance is active or not.

AeFlickerPeriod to specify a custom flicker period when the period is
other than 50 or 60Hz.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/libcamera/control_ids.yaml | 52 ++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

Comments

Kieran Bingham Jan. 25, 2023, 3:47 p.m. UTC | #1
Hi David,

Quoting David Plowman via libcamera-devel (2023-01-25 13:20:18)
> Flicker is the term used to describe brightness banding or oscillation
> of images caused typically by artificial lighting driven by a 50 or
> 60Hz mains supply. We add two controls intended to be used by AEC/AGC
> algorithms:
> 
> AeFlickerMode to determine whether flicker avoidance is active or not.
> 
> AeFlickerPeriod to specify a custom flicker period when the period is
> other than 50 or 60Hz.

Oh excellent, this had been flickering in the back of my mind for some
time.

I expect you're going to be providing an implementation for
pipeline/raspberrypi, but this should be easy to add to
pipeline/uvcvideo too with appropriate mapping to:

#define V4L2_CID_POWER_LINE_FREQUENCY   (V4L2_CID_BASE+24)
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,
};


> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/libcamera/control_ids.yaml | 52 ++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index adea5f90..5e5428ea 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -14,6 +14,58 @@ controls:
>  
>          \sa ExposureTime AnalogueGain
>  
> +  - AeFlickerMode:
> +      type: int32_t
> +      description: |
> +        Set the flicker mode, which determines whether, and how, the AGC/AEC
> +        algorithm attempts to hide flicker effects caused by the duty cycle of
> +        artificial lighting.
> +
> +        Although implementation dependent, many algorithms for "flicker
> +        avoidance" work by restricting the exposure time to integer multiples
> +        of this cycle period, wherever possible.
> +
> +        Implementations may not support all of the flicker modes listed below.

Hrm... this makes me wonder how an application can determine what is
possible, and what we would expect if the application tries to set a
mode that isn't possible on that device.

I don't think we have anything yet at the framework level to be able to
expose which subset of a mode are valid for a given camera...

For example, the UVC pipeline wouldn't support FlickerCustom.

(I don't think that itself blocks the definition of these options though)

> +
> +      enum:
> +        - name: FlickerOff
> +          value: 0
> +          description: No flicker avoidance is performed.
> +        - name: Flicker50Hz
> +          value: 1
> +          description: 50Hz flicker avoidance.
> +            Suppress flicker effects caused by lighting runing with a
> +            100Hz period (such as produced by 50Hz mains electricity).
> +        - name: Flicker60Hz
> +          value: 2
> +          description: 60Hz flicker avoidance.
> +            Suppress flicker effects caused by lighting running with a
> +            120Hz period (such as produced by 60Hz mains electricity).
> +        - name: FlickerCustom
> +          value: 3
> +          description: Custom flicker avoidance.
> +            Suppress flicker effects caused by lighting running with a
> +            period specified by AeFlickerPeriod.
> +
> +            \sa AeFlickerPeriod
> +        - name: FlickerAuto
> +          value: 4
> +          description: Automatic flicker period detection and avoidance.
> +            The system will automatically determine the most likely value
> +            of the flicker period, and avoid flicker of this frequency.
> +        
> +  - AeFlickerPeriod:
> +      type: int32_t
> +      description: Custom flicker period in microseconds.
> +        This value is taken as the current flicker period to avoid. It
> +        is used when the AeFlickerMode is set to FlickerCustom.
> +
> +        For example, to avoid 50Hz mains flicker, you could set the period
> +        to 10000, corresponding to 10ms (twice the mains frequency), and
> +        AeFlickerMode to FlickerCustom.
> +
> +        \sa AeFlickerMode

I like that applications would be able to control this so precisely,
rather than just the standard 50/60Hz modes.

--
Kieran


> +
>    - AeLocked:
>        type: bool
>        description: |
> -- 
> 2.30.2
>
David Plowman Jan. 25, 2023, 5:22 p.m. UTC | #2
Hi Kieran

Thanks for the reply.

On Wed, 25 Jan 2023 at 15:47, Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Hi David,
>
> Quoting David Plowman via libcamera-devel (2023-01-25 13:20:18)
> > Flicker is the term used to describe brightness banding or oscillation
> > of images caused typically by artificial lighting driven by a 50 or
> > 60Hz mains supply. We add two controls intended to be used by AEC/AGC
> > algorithms:
> >
> > AeFlickerMode to determine whether flicker avoidance is active or not.
> >
> > AeFlickerPeriod to specify a custom flicker period when the period is
> > other than 50 or 60Hz.
>
> Oh excellent, this had been flickering in the back of my mind for some
> time.
>
> I expect you're going to be providing an implementation for
> pipeline/raspberrypi, but this should be easy to add to
> pipeline/uvcvideo too with appropriate mapping to:
>
> #define V4L2_CID_POWER_LINE_FREQUENCY   (V4L2_CID_BASE+24)
> 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,
> };

I wonder what "AUTO" is intended to do. Perhaps it selects based on
location, perhaps it does something cleverer. I guess it's up to the
driver.

>
>
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  src/libcamera/control_ids.yaml | 52 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 52 insertions(+)
> >
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index adea5f90..5e5428ea 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -14,6 +14,58 @@ controls:
> >
> >          \sa ExposureTime AnalogueGain
> >
> > +  - AeFlickerMode:
> > +      type: int32_t
> > +      description: |
> > +        Set the flicker mode, which determines whether, and how, the AGC/AEC
> > +        algorithm attempts to hide flicker effects caused by the duty cycle of
> > +        artificial lighting.
> > +
> > +        Although implementation dependent, many algorithms for "flicker
> > +        avoidance" work by restricting the exposure time to integer multiples
> > +        of this cycle period, wherever possible.
> > +
> > +        Implementations may not support all of the flicker modes listed below.
>
> Hrm... this makes me wonder how an application can determine what is
> possible, and what we would expect if the application tries to set a
> mode that isn't possible on that device.
>
> I don't think we have anything yet at the framework level to be able to
> expose which subset of a mode are valid for a given camera...
>
> For example, the UVC pipeline wouldn't support FlickerCustom.
>
> (I don't think that itself blocks the definition of these options though)

You could set a max control value that excludes the "top" choices, I
think I've seen that done before. Maybe one might swap the "custom"
and "auto" values. But a better solution for unhandled control values
would be nice!

David

>
> > +
> > +      enum:
> > +        - name: FlickerOff
> > +          value: 0
> > +          description: No flicker avoidance is performed.
> > +        - name: Flicker50Hz
> > +          value: 1
> > +          description: 50Hz flicker avoidance.
> > +            Suppress flicker effects caused by lighting runing with a
> > +            100Hz period (such as produced by 50Hz mains electricity).
> > +        - name: Flicker60Hz
> > +          value: 2
> > +          description: 60Hz flicker avoidance.
> > +            Suppress flicker effects caused by lighting running with a
> > +            120Hz period (such as produced by 60Hz mains electricity).
> > +        - name: FlickerCustom
> > +          value: 3
> > +          description: Custom flicker avoidance.
> > +            Suppress flicker effects caused by lighting running with a
> > +            period specified by AeFlickerPeriod.
> > +
> > +            \sa AeFlickerPeriod
> > +        - name: FlickerAuto
> > +          value: 4
> > +          description: Automatic flicker period detection and avoidance.
> > +            The system will automatically determine the most likely value
> > +            of the flicker period, and avoid flicker of this frequency.
> > +
> > +  - AeFlickerPeriod:
> > +      type: int32_t
> > +      description: Custom flicker period in microseconds.
> > +        This value is taken as the current flicker period to avoid. It
> > +        is used when the AeFlickerMode is set to FlickerCustom.
> > +
> > +        For example, to avoid 50Hz mains flicker, you could set the period
> > +        to 10000, corresponding to 10ms (twice the mains frequency), and
> > +        AeFlickerMode to FlickerCustom.
> > +
> > +        \sa AeFlickerMode
>
> I like that applications would be able to control this so precisely,
> rather than just the standard 50/60Hz modes.
>
> --
> Kieran
>
>
> > +
> >    - AeLocked:
> >        type: bool
> >        description: |
> > --
> > 2.30.2
> >
Kieran Bingham Jan. 25, 2023, 5:36 p.m. UTC | #3
Quoting David Plowman (2023-01-25 17:22:27)
> Hi Kieran
> 
> Thanks for the reply.
> 
> On Wed, 25 Jan 2023 at 15:47, Kieran Bingham
> <kieran.bingham@ideasonboard.com> wrote:
> >
> > Hi David,
> >
> > Quoting David Plowman via libcamera-devel (2023-01-25 13:20:18)
> > > Flicker is the term used to describe brightness banding or oscillation
> > > of images caused typically by artificial lighting driven by a 50 or
> > > 60Hz mains supply. We add two controls intended to be used by AEC/AGC
> > > algorithms:
> > >
> > > AeFlickerMode to determine whether flicker avoidance is active or not.
> > >
> > > AeFlickerPeriod to specify a custom flicker period when the period is
> > > other than 50 or 60Hz.
> >
> > Oh excellent, this had been flickering in the back of my mind for some
> > time.
> >
> > I expect you're going to be providing an implementation for
> > pipeline/raspberrypi, but this should be easy to add to
> > pipeline/uvcvideo too with appropriate mapping to:
> >
> > #define V4L2_CID_POWER_LINE_FREQUENCY   (V4L2_CID_BASE+24)
> > 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,
> > };
> 
> I wonder what "AUTO" is intended to do. Perhaps it selects based on
> location, perhaps it does something cleverer. I guess it's up to the
> driver.

https://lore.kernel.org/all/1316519939-22540-3-git-send-email-s.nawrocki@samsung.com/

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

I guess the same applies to FlickerAuto ?

> >
> >
> > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > ---
> > >  src/libcamera/control_ids.yaml | 52 ++++++++++++++++++++++++++++++++++
> > >  1 file changed, 52 insertions(+)
> > >
> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > > index adea5f90..5e5428ea 100644
> > > --- a/src/libcamera/control_ids.yaml
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -14,6 +14,58 @@ controls:
> > >
> > >          \sa ExposureTime AnalogueGain
> > >
> > > +  - AeFlickerMode:
> > > +      type: int32_t
> > > +      description: |
> > > +        Set the flicker mode, which determines whether, and how, the AGC/AEC
> > > +        algorithm attempts to hide flicker effects caused by the duty cycle of
> > > +        artificial lighting.
> > > +
> > > +        Although implementation dependent, many algorithms for "flicker
> > > +        avoidance" work by restricting the exposure time to integer multiples
> > > +        of this cycle period, wherever possible.
> > > +
> > > +        Implementations may not support all of the flicker modes listed below.
> >
> > Hrm... this makes me wonder how an application can determine what is
> > possible, and what we would expect if the application tries to set a
> > mode that isn't possible on that device.
> >
> > I don't think we have anything yet at the framework level to be able to
> > expose which subset of a mode are valid for a given camera...
> >
> > For example, the UVC pipeline wouldn't support FlickerCustom.
> >
> > (I don't think that itself blocks the definition of these options though)
> 
> You could set a max control value that excludes the "top" choices, I
> think I've seen that done before. Maybe one might swap the "custom"
> and "auto" values. But a better solution for unhandled control values
> would be nice!

Yes, I just wondered about this trying to implement this quickly in UVC.
Swapping Custom to be last might be reasonable, and then have max at
auto. (It would work for UVC at least, but may not be generic).

The other point that would inform the application is that the camera can
choose to not expose the AeFlickerPeriod control. Then it can't set the
custom mode!

--
Kieran


> 
> David
> 
> >
> > > +
> > > +      enum:
> > > +        - name: FlickerOff
> > > +          value: 0
> > > +          description: No flicker avoidance is performed.
> > > +        - name: Flicker50Hz
> > > +          value: 1
> > > +          description: 50Hz flicker avoidance.
> > > +            Suppress flicker effects caused by lighting runing with a
> > > +            100Hz period (such as produced by 50Hz mains electricity).
> > > +        - name: Flicker60Hz
> > > +          value: 2
> > > +          description: 60Hz flicker avoidance.
> > > +            Suppress flicker effects caused by lighting running with a
> > > +            120Hz period (such as produced by 60Hz mains electricity).
> > > +        - name: FlickerCustom
> > > +          value: 3
> > > +          description: Custom flicker avoidance.
> > > +            Suppress flicker effects caused by lighting running with a
> > > +            period specified by AeFlickerPeriod.
> > > +
> > > +            \sa AeFlickerPeriod
> > > +        - name: FlickerAuto
> > > +          value: 4
> > > +          description: Automatic flicker period detection and avoidance.
> > > +            The system will automatically determine the most likely value
> > > +            of the flicker period, and avoid flicker of this frequency.
> > > +
> > > +  - AeFlickerPeriod:
> > > +      type: int32_t
> > > +      description: Custom flicker period in microseconds.
> > > +        This value is taken as the current flicker period to avoid. It
> > > +        is used when the AeFlickerMode is set to FlickerCustom.
> > > +
> > > +        For example, to avoid 50Hz mains flicker, you could set the period
> > > +        to 10000, corresponding to 10ms (twice the mains frequency), and
> > > +        AeFlickerMode to FlickerCustom.
> > > +
> > > +        \sa AeFlickerMode
> >
> > I like that applications would be able to control this so precisely,
> > rather than just the standard 50/60Hz modes.
> >
> > --
> > Kieran
> >
> >
> > > +
> > >    - AeLocked:
> > >        type: bool
> > >        description: |
> > > --
> > > 2.30.2
> > >
Naushir Patuck March 13, 2023, 9:04 a.m. UTC | #4
Hi David,

Thanks for this work!

On Wed, 25 Jan 2023 at 13:20, David Plowman via libcamera-devel <
libcamera-devel@lists.libcamera.org> wrote:

> Flicker is the term used to describe brightness banding or oscillation
> of images caused typically by artificial lighting driven by a 50 or
> 60Hz mains supply. We add two controls intended to be used by AEC/AGC
> algorithms:
>
> AeFlickerMode to determine whether flicker avoidance is active or not.
>
> AeFlickerPeriod to specify a custom flicker period when the period is
> other than 50 or 60Hz.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
>

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>



> ---
>  src/libcamera/control_ids.yaml | 52 ++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>
> diff --git a/src/libcamera/control_ids.yaml
> b/src/libcamera/control_ids.yaml
> index adea5f90..5e5428ea 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -14,6 +14,58 @@ controls:
>
>          \sa ExposureTime AnalogueGain
>
> +  - AeFlickerMode:
> +      type: int32_t
> +      description: |
> +        Set the flicker mode, which determines whether, and how, the
> AGC/AEC
> +        algorithm attempts to hide flicker effects caused by the duty
> cycle of
> +        artificial lighting.
> +
> +        Although implementation dependent, many algorithms for "flicker
> +        avoidance" work by restricting the exposure time to integer
> multiples
> +        of this cycle period, wherever possible.
> +
> +        Implementations may not support all of the flicker modes listed
> below.
> +
> +      enum:
> +        - name: FlickerOff
> +          value: 0
> +          description: No flicker avoidance is performed.
> +        - name: Flicker50Hz
> +          value: 1
> +          description: 50Hz flicker avoidance.
> +            Suppress flicker effects caused by lighting runing with a
> +            100Hz period (such as produced by 50Hz mains electricity).
> +        - name: Flicker60Hz
> +          value: 2
> +          description: 60Hz flicker avoidance.
> +            Suppress flicker effects caused by lighting running with a
> +            120Hz period (such as produced by 60Hz mains electricity).
> +        - name: FlickerCustom
> +          value: 3
> +          description: Custom flicker avoidance.
> +            Suppress flicker effects caused by lighting running with a
> +            period specified by AeFlickerPeriod.
> +
> +            \sa AeFlickerPeriod
> +        - name: FlickerAuto
> +          value: 4
> +          description: Automatic flicker period detection and avoidance.
> +            The system will automatically determine the most likely value
> +            of the flicker period, and avoid flicker of this frequency.
> +
> +  - AeFlickerPeriod:
> +      type: int32_t
> +      description: Custom flicker period in microseconds.
> +        This value is taken as the current flicker period to avoid. It
> +        is used when the AeFlickerMode is set to FlickerCustom.
> +
> +        For example, to avoid 50Hz mains flicker, you could set the period
> +        to 10000, corresponding to 10ms (twice the mains frequency), and
> +        AeFlickerMode to FlickerCustom.
> +
> +        \sa AeFlickerMode
> +
>    - AeLocked:
>        type: bool
>        description: |
> --
> 2.30.2
>
>

Patch
diff mbox series

diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
index adea5f90..5e5428ea 100644
--- a/src/libcamera/control_ids.yaml
+++ b/src/libcamera/control_ids.yaml
@@ -14,6 +14,58 @@  controls:
 
         \sa ExposureTime AnalogueGain
 
+  - AeFlickerMode:
+      type: int32_t
+      description: |
+        Set the flicker mode, which determines whether, and how, the AGC/AEC
+        algorithm attempts to hide flicker effects caused by the duty cycle of
+        artificial lighting.
+
+        Although implementation dependent, many algorithms for "flicker
+        avoidance" work by restricting the exposure time to integer multiples
+        of this cycle period, wherever possible.
+
+        Implementations may not support all of the flicker modes listed below.
+
+      enum:
+        - name: FlickerOff
+          value: 0
+          description: No flicker avoidance is performed.
+        - name: Flicker50Hz
+          value: 1
+          description: 50Hz flicker avoidance.
+            Suppress flicker effects caused by lighting runing with a
+            100Hz period (such as produced by 50Hz mains electricity).
+        - name: Flicker60Hz
+          value: 2
+          description: 60Hz flicker avoidance.
+            Suppress flicker effects caused by lighting running with a
+            120Hz period (such as produced by 60Hz mains electricity).
+        - name: FlickerCustom
+          value: 3
+          description: Custom flicker avoidance.
+            Suppress flicker effects caused by lighting running with a
+            period specified by AeFlickerPeriod.
+
+            \sa AeFlickerPeriod
+        - name: FlickerAuto
+          value: 4
+          description: Automatic flicker period detection and avoidance.
+            The system will automatically determine the most likely value
+            of the flicker period, and avoid flicker of this frequency.
+        
+  - AeFlickerPeriod:
+      type: int32_t
+      description: Custom flicker period in microseconds.
+        This value is taken as the current flicker period to avoid. It
+        is used when the AeFlickerMode is set to FlickerCustom.
+
+        For example, to avoid 50Hz mains flicker, you could set the period
+        to 10000, corresponding to 10ms (twice the mains frequency), and
+        AeFlickerMode to FlickerCustom.
+
+        \sa AeFlickerMode
+
   - AeLocked:
       type: bool
       description: |