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

Message ID 20230718144301.3612-2-david.plowman@raspberrypi.com
State Accepted
Commit 6fdbf3f38c3148f1ba780286715d9a78ffe1e387
Headers show
Series
  • Add flicker avoidance controls
Related show

Commit Message

David Plowman July 18, 2023, 2:43 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 three controls intended to be used by
AEC/AGC algorithms:

AeFlickerMode to enable flicker avoidance.

AeFlickerPeriod to set the flicker period "manually".

AeFlickerDetected to report any flicker that is currently detected.

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

Comments

Jacopo Mondi July 19, 2023, 12:33 p.m. UTC | #1
Hi David

On Tue, Jul 18, 2023 at 03:43:00PM +0100, David Plowman via libcamera-devel 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 three controls intended to be used by
> AEC/AGC algorithms:
>
> AeFlickerMode to enable flicker avoidance.
>
> AeFlickerPeriod to set the flicker period "manually".
>
> AeFlickerDetected to report any flicker that is currently detected.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/libcamera/control_ids.yaml | 90 +++++++++++++++++++++++++++-------
>  1 file changed, 73 insertions(+), 17 deletions(-)
>
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index 056886e6..f2e542f4 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -156,6 +156,79 @@ controls:
>          control of which features should be automatically adjusted shouldn't
>          better be handled through a separate AE mode control.
>
> +  - 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 this exposure time to integer multiples
> +        of the cycle period, wherever possible.
> +
> +        Implementations may not support all of the flicker modes listed below.

I would drop this, PH/IPA are supposed to register only the modes they
support. This applies generally to all controls

> +
> +        By default the system will start in FlickerAuto mode if this is
> +        supported, otherwise the flicker mode will be set to FlickerOff.
> +
> +      enum:
> +        - name: FlickerOff
> +          value: 0
> +          description: No flicker avoidance is performed.
> +        - name: FlickerManual
> +          value: 1
> +          description: Manual flicker avoidance.
> +            Suppress flicker effects caused by lighting running with a period
> +            specified by the AeFlickerPeriod control.
> +            \sa AeFlickerPeriod
> +        - name: FlickerAuto
> +          value: 2
> +          description: Automatic flicker period detection and avoidance.
> +            The system will automatically determine the most likely value of
> +            flicker period, and avoid flicker of this frequency. Once flicker
> +            is being corrected, it is implementation dependent whether the
> +            system is still able to detect a change in the flicker period.
> +            \sa AeFlickerDetected
> +
> +  - AeFlickerPeriod:
> +      type: int32_t
> +      description: Manual flicker period in microseconds.
> +        This value sets the current flicker period to avoid. It is used when
> +        AeFlickerMode is set to FlickerManual.
> +
> +        To cancel 50Hz mains flicker, this should be set to 10000 (corresponding
> +        to 100Hz), or 8333 (120Hz) for 60Hz mains.
> +
> +        Setting the mode to FlickerManual when no AeFlickerPeriod has ever been
> +        set means that no flicker cancellation occurs (until the value of this
> +        control is updated).
> +
> +        Switching to modes other than FlickerManual has no effect on the
> +        value of the AeFlickerPeriod control.
> +
> +        \sa AeFlickerMode
> +
> +  - AeFlickerDetected:
> +      type: int32_t
> +      description: Flicker period detected in microseconds.
> +        The value reported here indicates the currently detected flicker
> +        period, or zero if no flicker at all is detected.
> +
> +        When AeFlickerMode is set to FlickerAuto, there may be a period during
> +        which the value reported here remains zero. Once a non-zero value is
> +        reported, then this is the flicker period that has been detected and is
> +        now being cancelled.
> +
> +        In the case of 50Hz mains flicker, the value would be 10000
> +        (corresponding to 100Hz), or 8333 (120Hz) for 60Hz mains flicker.
> +
> +        It is implementation dependent whether the system can continue to detect
> +        flicker of different periods when another frequency is already being
> +        cancelled.
> +
> +        \sa AeFlickerMode
> +

Looks good to me
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>    - Brightness:
>        type: float
>        description: |
> @@ -850,23 +923,6 @@ controls:
>            value: 1
>            description: The lens shading map mode is available.
>
> -  - SceneFlicker:
> -      type: int32_t
> -      draft: true
> -      description: |
> -       Control to report the detected scene light frequency. Currently
> -       identical to ANDROID_STATISTICS_SCENE_FLICKER.
> -      enum:
> -        - name: SceneFickerOff
> -          value: 0
> -          description: No flickering detected.
> -        - name: SceneFicker50Hz
> -          value: 1
> -          description: 50Hz flickering detected.
> -        - name: SceneFicker60Hz
> -          value: 2
> -          description: 60Hz flickering detected.
> -
>    - PipelineDepth:
>        type: int32_t
>        draft: true
> --
> 2.30.2
>
Umang Jain July 21, 2023, 4:22 a.m. UTC | #2
Hi David

On 7/19/23 6:03 PM, Jacopo Mondi via libcamera-devel wrote:
> Hi David
>
> On Tue, Jul 18, 2023 at 03:43:00PM +0100, David Plowman via libcamera-devel 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 three controls intended to be used by
>> AEC/AGC algorithms:
>>
>> AeFlickerMode to enable flicker avoidance.
>>
>> AeFlickerPeriod to set the flicker period "manually".
>>
>> AeFlickerDetected to report any flicker that is currently detected.
>>
>> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
>> ---
>>   src/libcamera/control_ids.yaml | 90 +++++++++++++++++++++++++++-------
>>   1 file changed, 73 insertions(+), 17 deletions(-)
>>
>> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
>> index 056886e6..f2e542f4 100644
>> --- a/src/libcamera/control_ids.yaml
>> +++ b/src/libcamera/control_ids.yaml
>> @@ -156,6 +156,79 @@ controls:
>>           control of which features should be automatically adjusted shouldn't
>>           better be handled through a separate AE mode control.
>>
>> +  - 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 this exposure time to integer multiples
>> +        of the cycle period, wherever possible.
>> +
>> +        Implementations may not support all of the flicker modes listed below.
> I would drop this, PH/IPA are supposed to register only the modes they
> support. This applies generally to all controls

With this addressed,

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

>
>> +
>> +        By default the system will start in FlickerAuto mode if this is
>> +        supported, otherwise the flicker mode will be set to FlickerOff.
>> +
>> +      enum:
>> +        - name: FlickerOff
>> +          value: 0
>> +          description: No flicker avoidance is performed.
>> +        - name: FlickerManual
>> +          value: 1
>> +          description: Manual flicker avoidance.
>> +            Suppress flicker effects caused by lighting running with a period
>> +            specified by the AeFlickerPeriod control.
>> +            \sa AeFlickerPeriod
>> +        - name: FlickerAuto
>> +          value: 2
>> +          description: Automatic flicker period detection and avoidance.
>> +            The system will automatically determine the most likely value of
>> +            flicker period, and avoid flicker of this frequency. Once flicker
>> +            is being corrected, it is implementation dependent whether the
>> +            system is still able to detect a change in the flicker period.
>> +            \sa AeFlickerDetected
>> +
>> +  - AeFlickerPeriod:
>> +      type: int32_t
>> +      description: Manual flicker period in microseconds.
>> +        This value sets the current flicker period to avoid. It is used when
>> +        AeFlickerMode is set to FlickerManual.
>> +
>> +        To cancel 50Hz mains flicker, this should be set to 10000 (corresponding
>> +        to 100Hz), or 8333 (120Hz) for 60Hz mains.
>> +
>> +        Setting the mode to FlickerManual when no AeFlickerPeriod has ever been
>> +        set means that no flicker cancellation occurs (until the value of this
>> +        control is updated).
>> +
>> +        Switching to modes other than FlickerManual has no effect on the
>> +        value of the AeFlickerPeriod control.
>> +
>> +        \sa AeFlickerMode
>> +
>> +  - AeFlickerDetected:
>> +      type: int32_t
>> +      description: Flicker period detected in microseconds.
>> +        The value reported here indicates the currently detected flicker
>> +        period, or zero if no flicker at all is detected.
>> +
>> +        When AeFlickerMode is set to FlickerAuto, there may be a period during
>> +        which the value reported here remains zero. Once a non-zero value is
>> +        reported, then this is the flicker period that has been detected and is
>> +        now being cancelled.
>> +
>> +        In the case of 50Hz mains flicker, the value would be 10000
>> +        (corresponding to 100Hz), or 8333 (120Hz) for 60Hz mains flicker.
>> +
>> +        It is implementation dependent whether the system can continue to detect
>> +        flicker of different periods when another frequency is already being
>> +        cancelled.
>> +
>> +        \sa AeFlickerMode
>> +
> Looks good to me
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
>    j
>
>>     - Brightness:
>>         type: float
>>         description: |
>> @@ -850,23 +923,6 @@ controls:
>>             value: 1
>>             description: The lens shading map mode is available.
>>
>> -  - SceneFlicker:
>> -      type: int32_t
>> -      draft: true
>> -      description: |
>> -       Control to report the detected scene light frequency. Currently
>> -       identical to ANDROID_STATISTICS_SCENE_FLICKER.
>> -      enum:
>> -        - name: SceneFickerOff
>> -          value: 0
>> -          description: No flickering detected.
>> -        - name: SceneFicker50Hz
>> -          value: 1
>> -          description: 50Hz flickering detected.
>> -        - name: SceneFicker60Hz
>> -          value: 2
>> -          description: 60Hz flickering detected.
>> -
>>     - PipelineDepth:
>>         type: int32_t
>>         draft: true
>> --
>> 2.30.2
>>

Patch
diff mbox series

diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
index 056886e6..f2e542f4 100644
--- a/src/libcamera/control_ids.yaml
+++ b/src/libcamera/control_ids.yaml
@@ -156,6 +156,79 @@  controls:
         control of which features should be automatically adjusted shouldn't
         better be handled through a separate AE mode control.
 
+  - 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 this exposure time to integer multiples
+        of the cycle period, wherever possible.
+
+        Implementations may not support all of the flicker modes listed below.
+
+        By default the system will start in FlickerAuto mode if this is
+        supported, otherwise the flicker mode will be set to FlickerOff.
+
+      enum:
+        - name: FlickerOff
+          value: 0
+          description: No flicker avoidance is performed.
+        - name: FlickerManual
+          value: 1
+          description: Manual flicker avoidance.
+            Suppress flicker effects caused by lighting running with a period
+            specified by the AeFlickerPeriod control.
+            \sa AeFlickerPeriod
+        - name: FlickerAuto
+          value: 2
+          description: Automatic flicker period detection and avoidance.
+            The system will automatically determine the most likely value of
+            flicker period, and avoid flicker of this frequency. Once flicker
+            is being corrected, it is implementation dependent whether the
+            system is still able to detect a change in the flicker period.
+            \sa AeFlickerDetected
+
+  - AeFlickerPeriod:
+      type: int32_t
+      description: Manual flicker period in microseconds.
+        This value sets the current flicker period to avoid. It is used when
+        AeFlickerMode is set to FlickerManual.
+
+        To cancel 50Hz mains flicker, this should be set to 10000 (corresponding
+        to 100Hz), or 8333 (120Hz) for 60Hz mains.
+
+        Setting the mode to FlickerManual when no AeFlickerPeriod has ever been
+        set means that no flicker cancellation occurs (until the value of this
+        control is updated).
+
+        Switching to modes other than FlickerManual has no effect on the
+        value of the AeFlickerPeriod control.
+
+        \sa AeFlickerMode
+
+  - AeFlickerDetected:
+      type: int32_t
+      description: Flicker period detected in microseconds.
+        The value reported here indicates the currently detected flicker
+        period, or zero if no flicker at all is detected.
+
+        When AeFlickerMode is set to FlickerAuto, there may be a period during
+        which the value reported here remains zero. Once a non-zero value is
+        reported, then this is the flicker period that has been detected and is
+        now being cancelled.
+
+        In the case of 50Hz mains flicker, the value would be 10000
+        (corresponding to 100Hz), or 8333 (120Hz) for 60Hz mains flicker.
+
+        It is implementation dependent whether the system can continue to detect
+        flicker of different periods when another frequency is already being
+        cancelled.
+
+        \sa AeFlickerMode
+
   - Brightness:
       type: float
       description: |
@@ -850,23 +923,6 @@  controls:
           value: 1
           description: The lens shading map mode is available.
 
-  - SceneFlicker:
-      type: int32_t
-      draft: true
-      description: |
-       Control to report the detected scene light frequency. Currently
-       identical to ANDROID_STATISTICS_SCENE_FLICKER.
-      enum:
-        - name: SceneFickerOff
-          value: 0
-          description: No flickering detected.
-        - name: SceneFicker50Hz
-          value: 1
-          description: 50Hz flickering detected.
-        - name: SceneFicker60Hz
-          value: 2
-          description: 60Hz flickering detected.
-
   - PipelineDepth:
       type: int32_t
       draft: true