[libcamera-devel] [PATCH v2 1/7] controls: Reorganize the AE-related controls
Jacopo Mondi
jacopo at jmondi.org
Fri Oct 1 18:08:19 CEST 2021
Hi Paul,
On Fri, Oct 01, 2021 at 07:33:19PM +0900, Paul Elder wrote:
> We have multiple goals:
> - we need a lock of some sort, to instruct the AEGC to not update output
> results
> - we need manual modes, to override the values computed by the AEGC
> - we need to support seamless transitions from auto -> manual, and do so
> without flickering
> - we need custom minimum values for the manual controls, that is no
> magic values for enabling/disabling auto
> - all of these need to be done with AE sub-controls (exposure time,
> analogue gain)
>
> To achieve these goals, we introduce mode controls for the AE
> sub-controls: ExposureTimeMode and AnalogueGainMode. These have an auto
> state, and a disabled state. The disabled state has an internal one-way
> state change from locked to manual, triggered by the presence of the
> value-controls (ExposureTime and AnalogueGain).
>
> We then remove the AeEnable control, as it is a redundant control in the
> face of these two mode controls.
>
> We also remove AeLocked, as it is insufficient for reporting the AE
> state, and we promote AeState to non-draft to fill its role. Notably,
> the locked state is removed, since this information can be obtained from
> the aforementioned mode controls.
>
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=42
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=43
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=47
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v2:
> - No changes, just resubmitting at the head of this series so that it's
> together and so that /people will actually see it/
>
> Initial version:
> Still RFC as I haven't updated the users of the control yet, and I want
> to check that these are the controls and docs that we want.
>
> We've decided that the "master AE control" will be implemented by a
> helper... but looking at uvcvideo and the V4L2 controls I'm wondering if
> such helper should come earlier than later?
> ---
> src/libcamera/control_ids.yaml | 215 +++++++++++++++++++++++----------
> 1 file changed, 150 insertions(+), 65 deletions(-)
>
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index 9d4638ae..18b186e8 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -7,23 +7,61 @@
> # Unless otherwise stated, all controls are bi-directional, i.e. they can be
> # set through Request::controls() and returned out through Request::metadata().
> controls:
> - - AeEnable:
> - type: bool
> - description: |
> - Enable or disable the AE.
> -
> - \sa ExposureTime AnalogueGain
> -
> - - AeLocked:
> - type: bool
> + - AeState:
Now that we have two separate controls for the exposure time and
analogue gain, do we want a single state for the whole AEGC block ?
> + type: int32_t
> description: |
> - Report the lock status of a running AE algorithm.
> -
> - If the AE algorithm is locked the value shall be set to true, if it's
> - converging it shall be set to false. If the AE algorithm is not
> - running the control shall not be present in the metadata control list.
> + Control to report the current AE algorithm state. Enabling or disabling
> + any of the AE-related mode controls (eg. AnalogueGainMode,
> + ExposureTimeMode) is not required to reset the state to
> + AeStateInactive. The camera device can do several state transitions
I understand that, from where we come from the
"Enabling or disabling any of the AE-related mode controls (eg. AnalogueGainMode,
ExposureTimeMode) is not required to reset the state to
AeStateInactive."
makes sense, but you're reading this for the first time you would be
wondering why is this information here, even before what the inactive
state is has been defined. If you consider this relevant, I would
place this in the Inactive state description itself
> + between two results, if it is allowed by the state transition table.
which table ? :)
> + For example, AeStateInactive may never actually be seen in a result.
I would place this information in the transition table if we have one
> +
> + The state in the result is the state for this image (in sync with this
The () part is a repetition, and doesn't this apply to all controls
part of a result ? Don't they all apply to the frames part of the
request they are part of ? (Also I would refer to requests, not
images)
> + image). If AE state becomes AeStateConverged, then the image data
> + associated with the result should be good to use.
> +
> + As some AE algorithms may still be running when any of the AE-related
> + controls are in manual mode, the states are valid even when the mode
What is manual mode ? Don't we have an "enabled"/"disabled" state only
?
I would just say that the state is reported even if the exposure time
and the analogue gain algorithms are disabled
> + controls are set to Disabled. To know if this image image used AE or
> + manual (or a mixture), check the relevant modes (eg. AnalogueGainMode,
> + ExposureTimeMode).
> +
> + \sa AnalogueGain
> + \sa AnalogueGainMode
> + \sa ExposureTime
> + \sa ExposureTimeMode
>
> - \sa AeEnable
> + enum:
> + - name: AeStateInactive
> + value: 0
> + description: |
> + The AE algorithm is inactive.
> + If the camera initiates an AE scan, the state shall go to Searching.
> + - name: AeStateSearching
> + value: 1
> + description: |
> + The AE algorithm has not converged yet.
> + If the camera finishes an AE scan, the state shall go to Converged.
> + If the camera finishes an AE scan, but flash is required, the state
> + shall go to FlashRequired.
> + - name: AeStateConverged
> + value: 2
> + description: |
> + The AE algorithm has converged.
> + If the camera initiates an AE scan, the state shall go to Searching.
> + - name: AeStateFlashRequired
> + value: 3
> + description: |
> + The AE algorithm would need a flash for good results
> + If the camera initiates an AE scan, the state shall go to Searching.
> + If AeLock is on, the state shall go to Locked.
> + - name: AeStatePrecapture
> + value: 4
> + description: |
> + The AE algorithm has started a pre-capture metering session.
> + After the sequence is finished, the state shall go to Converged if
> + \sa AePrecaptureTrigger
We don't have AE capture triggers, nor a transition table defined at
all. And as said above, now we have two controls for exposure and
analog gain, and a single state for the whole AE.
Do we want AeState at all ?
>
> # AeMeteringMode needs further attention:
> # - Auto-generate max enum value.
> @@ -93,6 +131,17 @@ controls:
> how the desired total exposure is divided between the shutter time
> and the sensor's analogue gain. The exposure modes are platform
> specific, and not all exposure modes may be supported.
> +
> + When one of AnalogueGainMode or ExposureTimeMode is set to Disabled and
> + a corresponding manual control is provided, AeExposureMode may be
> + ignored. This is because, for example, if AeExposureMode is set to
> + ExposureLong but ExposureTimeMode is Disabled and a short ExposureTime
> + is provided (AnalogueGainMode set to Auto), then the AeExposureMode
> + doesn't make sense.
Why not just:
This controls takes effect only if ExposureTimeMode is set
to ExposureTimeModeAuto and it's ignored otherwise
> +
> + \sa AnalogueGainMode
> + \sa ExposureTimeMode
> +
> enum:
> - name: ExposureNormal
> value: 0
> @@ -111,13 +160,15 @@ controls:
> type: float
> description: |
> Specify an Exposure Value (EV) parameter. The EV parameter will only be
> - applied if the AE algorithm is currently enabled.
> + applied if the AE algorithm is currently enabled, that is, at least one
> + of AnalogueGainMode and ExposureTimeMode are auto.
Mostly a question for RPi who upstreamed this, but
I don't fully get how ExposureValue works. Seems like it is used as a
global multiplier for the analogue gain in AGC::computeGain. Is it
worth reworking it or is it just me that I'm missing some part ?
>
> By convention EV adjusts the exposure as log2. For example
> EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure adjustment
> of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x].
>
> - \sa AeEnable
> + \sa AnalogueGainMode
> + \sa ExposureTimeMode
>
> - ExposureTime:
> type: int32_t
> @@ -125,17 +176,49 @@ controls:
> Exposure time (shutter speed) for the frame applied in the sensor
> device. This value is specified in micro-seconds.
>
> - Setting this value means that it is now fixed and the AE algorithm may
> - not change it. Setting it back to zero returns it to the control of the
> - AE algorithm.
> + This control will only take effect if ExposureTimeMode is Disabled. Its
> + presence in a request acts as a trigger to switch to the internal
> + manual mode within ExposureTimeModeDisabled.
I find the concept of "internal manual mode" confusing, and I think we
should not insert it in the documentation.
This control will only take effect if ExposureTimeMode is Disabled.
^ take or takes ?
and is ignored when ExposureTimeMode is set to Auto.
> + When reported in metadata, this control indicates what exposure time
> + was used for the current request, regardless of ExposureTimeMode.
> + ExposureTimeMode will indicate the source of the exposure time value,
> + whether it came from the AE algorithm or not.
>
> - \sa AnalogueGain AeEnable
> + \sa AnalogueGain
> + \sa ExposureTimeMode
>
> - \todo Document the interactions between AeEnable and setting a fixed
> - value for this control. Consider interactions with other AE features,
> - such as aperture and aperture/shutter priority mode, and decide if
> - control of which features should be automatically adjusted shouldn't
> - better be handled through a separate AE mode control.
> + - ExposureTimeMode:
So, I don't want to be too picky, but does it make sense to say a
"Mode" is either "Auto" or "Disabled" ? Aren't "AutoExposure = {Enable,
Disabled}" or "ExposureMode = {Auto, Manual}" better ? I'm asking
without having a real answer to the question.
> + type: int32_t
> + description: |
> + Control to set and report the source of the exposure time that will be
> + applied.
Controls how the frame exposure time is computed. When set
to "Auto" the auto-exposure algorithm computes the exposure
time of the next frame and configures the image sensor
accordingly. When set to "Disabled" the auto-exposure
algorithm stops computing the exposure time.
> + \sa ExposureTime
> + enum:
> + - name: ExposureTimeModeAuto
> + value: 0
> + description: |
> + The exposure time will be calculated automatically and set by the
> + AE algorithm.
> + - name: ExposureTimeModeDisabled
> + value: 1
> + description: |
> + The exposure time will not be updated by the AE algorithm. It will
> + come from the last calculated value when the mode was Auto, or from
> + the value specified in ExposureTime.
> +
> + This Disabled state has two internal states; locked and manual.
> + When the Disabled state is first entered, the internal state will
> + be locked, and the latest exposure time value set by the AE
> + algorithm will be used (or the default ExposureTime value, if the
> + mode started out as Disabled). When an ExposureTime is provided,
> + then the internal state will go to manual, and the provided value
> + will be used for the exposure time. Going from manual to locked is
> + not possible. If an ExposureTime value is provided in the same
> + request as going into the Disabled state, then the internal locked
> + state will be skipped, and the internal state will start out as
> + manual.
Is it useful to define such internal states ? Isn't the same expressed
with ?
When transitioning from Auto to Disabled mode the last
computed exposure value is used until a new value is
specified through the ExposureTime control. If an
ExposureTime value is specified in the same request where
the ExposureTimeMode is set to Disabled, the provided
ExposureTime is applied immediately.
I also think we need to describe how to perform a 'flickerless'
transition, but that depends on what we decide to do with AeState. If
I'm not mistaken we can deduce that the exposure value is 'locked' by
inspecting the ExposureTimeMode value in the result.
We can have a dedicated section in the control description. I'll make
an attempt but I'm sure this can be vastly improved.
- Flicker-less manual exposure time transition:
Applications that transition from ExposureTimeMode
Auto mode to the direct control of the exposure time through
the ExposureTime control should aim to do so by selecting
an ExposureTime value as close as possible to the last
value computed by the auto exposure algorithm to avoid
any visible flickering.
In order to select the correct value to use in the first
Request with the ExposureTime control specified,
applications should accommodate the natural delay in
applying controls caused by the capture pipeline frame
depth.
When switching to manual exposure time control,
applications should not immediately specify an
ExposureTime value in the same request where
ExposureTimeMode is set to Disabled. They should instead
wait for the first Request where ExposureTimeMode is
returned as Disabled in the Request metadata and use the
there returned exposure time to populate the ExposureTime
value in the next queued Request.
The implementation of the auto-exposure time algorithm
should equally try to minimize flickering and when
transitioning from manual exposure time control to auto
exposure, the last used value should be used as starting
point.
I'm not sure about the last paragraph though, does it match the
outcome of our last discussion ?
>
> - AnalogueGain:
> type: float
> @@ -144,17 +227,49 @@ controls:
> The value of the control specifies the gain multiplier applied to all
> colour channels. This value cannot be lower than 1.0.
>
> - Setting this value means that it is now fixed and the AE algorithm may
> - not change it. Setting it back to zero returns it to the control of the
> - AE algorithm.
> + This control will only take effect if ExposureTimeMode is Disabled. Its
> + presence in a request acts as a trigger to switch to the internal
> + manual mode within ExposureTimeModeDisabled.
s/ExposureTimeMode/AnalogueGainMode ?
>
> - \sa ExposureTime AeEnable
> + When reported in metadata, this control indicates what exposure time
> + was used for the current request, regardless of ExposureTimeMode.
> + ExposureTimeMode will indicate the source of the exposure time value,
> + whether it came from the AE algorithm or not.
Again, all the 'exposure' should be 'analogue gain' instead ?
> +
> + \sa ExposureTime
> + \sa AnalogueGainMode
> +
> + - AnalogueGainMode:
> + type: int32_t
> + description: |
> + Control to set and report the source of the analogue gain that will be
> + applied.
>
> - \todo Document the interactions between AeEnable and setting a fixed
> - value for this control. Consider interactions with other AE features,
> - such as aperture and aperture/shutter priority mode, and decide if
> - control of which features should be automatically adjusted shouldn't
> - better be handled through a separate AE mode control.
> + \sa AnalogueGain
> + enum:
> + - name: AnalogueGainModeAuto
> + value: 0
> + description: |
> + The analogue gain will be calculated automatically and set by the
> + AE algorithm.
> + - name: AnalogueGainModeDisabled
> + value: 1
> + description: |
> + The analogue gain will not be updated by the AE algorithm. It will
> + come from the last calculated value when the mode was Auto, or from
> + the value specified in AnalogueGain.
> +
> + This Disabled state has two internal states; locked and manual.
> + When the Disabled state is first entered, the internal state will
> + be locked, and the latest analogue gain value set by the AE
> + algorithm will be used (or the default AnalogueGain value, if the
> + mode started out as Disabled). When an AnalogueGain is provided,
> + then the internal state will go to manual, and the provided value
> + will be used for the analogue gain. Going from manual to locked is
> + not possible. If an AnalogueGain value is provided in the same
> + request as going into the Disabled state, then the internal locked
> + state will be skipped, and the internal state will start out as
> + manual.
Same comments as per the ExposureTimeMode control.
>
> - Brightness:
> type: float
> @@ -477,36 +592,6 @@ controls:
> High quality aberration correction which might reduce the frame
> rate.
>
> - - AeState:
> - type: int32_t
> - draft: true
> - description: |
> - Control to report the current AE algorithm state. Currently identical to
> - ANDROID_CONTROL_AE_STATE.
> -
> - Current state of the AE algorithm.
> - enum:
> - - name: AeStateInactive
> - value: 0
> - description: The AE algorithm is inactive.
> - - name: AeStateSearching
> - value: 1
> - description: The AE algorithm has not converged yet.
> - - name: AeStateConverged
> - value: 2
> - description: The AE algorithm has converged.
> - - name: AeStateLocked
> - value: 3
> - description: The AE algorithm is locked.
> - - name: AeStateFlashRequired
> - value: 4
> - description: The AE algorithm would need a flash for good results
> - - name: AeStatePrecapture
> - value: 5
> - description: |
> - The AE algorithm has started a pre-capture metering session.
> - \sa AePrecaptureTrigger
> -
> - AfState:
> type: int32_t
> draft: true
> --
> 2.27.0
>
More information about the libcamera-devel
mailing list