[libcamera-devel] [PATCH v4 1/3] controls: Reorganize the AE-related controls
Naushir Patuck
naush at raspberrypi.com
Tue Jul 5 10:06:39 CEST 2022
Hi Jacopo and Paul,
On Mon, 4 Jul 2022 at 16:30, Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hi Naush,
> sorry Paul if I reply in your place, but I just got through this so
> I might have it slightly fresher
>
> On Mon, Jul 04, 2022 at 11:02:47AM +0100, Naushir Patuck via
libcamera-devel wrote:
> > Hi Paul,
> >
> > Thank you for your work. It's nice to see these changes to solidify
the AE
> > controls.
> > I do have a few thoughts/comments below.
>
> Can I ask you to have a look at the final result with the fixups I
> sent in reply applied on top of this patch ?
Yes will do!
>
> >
> >
> > On Wed, 18 May 2022 at 14:47, Paul Elder via libcamera-devel <
> > libcamera-devel at lists.libcamera.org> 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 v4:
> > > - remove FlashRequired and Precapture from AeState
> > > - upgrade documentation of all the controls
> > >
> > > Changes in v3:
> > > - improve wording of the control descriptions
> > > - make more succinct and clear
> > > - add description of how to do a flickerless transition
> > >
> > > 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?
> > >
> >
> > Yes, I agree having the "master AE control" earlier will be beneficial
for
> > application developers.
> >
>
> Do you envision this as something that could be part of your
> applications in example, or in a layer part of libcamera itself ?
My preference would be to have a helper in libcamera do this if possible.
This way, applications don't need to duplicate code to set all AE controls.
But either way, it's not too big of a deal to put it in the application.
>
>
> >
> > > ---
> > > src/libcamera/control_ids.yaml | 262
+++++++++++++++++++++++++--------
> > > 1 file changed, 200 insertions(+), 62 deletions(-)
> > >
> > > diff --git a/src/libcamera/control_ids.yaml
> > > b/src/libcamera/control_ids.yaml
> > > index 9d4638ae..9f5ce5e8 100644
> > > --- a/src/libcamera/control_ids.yaml
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -7,23 +7,46 @@
> > > # 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
> > > + - AeState:
> > > + type: int32_t
> > > description: |
> > > - Enable or disable the AE.
> > > + Control to report the AE algorithm state associated with the
> > > capture
> > > + result.
> > >
> > > - \sa ExposureTime AnalogueGain
> > > + The state is still reported even if ExposureTimeMode or
> > > + AnalogueGainMode is set to Disabled.
> > >
> > > - - AeLocked:
> > > - type: bool
> > > - description: |
> > > - Report the lock status of a running AE algorithm.
> > > + \sa AnalogueGain
> > > + \sa AnalogueGainMode
> > > + \sa ExposureTime
> > > + \sa ExposureTimeMode
> > >
> > > - 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.
> > > + enum:
> > > + - name: AeStateInactive
> > > + value: 0
> > > + description: |
> > > + The AE algorithm is inactive.
> > >
> > > - \sa AeEnable
> > > + This state should be returned if both AnalogueGainMode
and
> > > + ExposureTimeMode are set to disabled (or one, if the
camera
> > > only
> > > + supports one of the two controls).
> > > + - name: AeStateSearching
> > > + value: 1
> > > + description: |
> > > + The AE algorithm has not converged yet.
> > > +
> > > + This state should be returned if at least one of
> > > AnalogueGainMode
> > > + or ExposureTimeMode is set to auto, and the AE algorithm
> > > hasn't
> > > + converged yet. If the AE algorithm converges, the state
shall
> > > go to
> > > + AeStateConverged.
> > > + - name: AeStateConverged
> > > + value: 2
> > > + description: |
> > > + The AE algorithm has converged.
> > > +
> > > + This state should be returned if at least one of
> > > AnalogueGainMode
> > > + or ExposureTimeMode is set to auto, and the AE algorithm
has
> > > + converged.
> > >
> > > # AeMeteringMode needs further attention:
> > > # - Auto-generate max enum value.
> > > @@ -93,6 +116,13 @@ 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,
> > > + the fixed values will override any choices made by
AeExposureMode.
> > > +
> > > + \sa AnalogueGainMode
> > > + \sa ExposureTimeMode
> > > +
> > > enum:
> > > - name: ExposureNormal
> > > value: 0
> > > @@ -111,13 +141,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.
> > >
> > > 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 +157,85 @@ 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. If
> > > + this control is set when ExposureTimeMode is Auto, the value
will
> > > be
> > > + ignored and will not be retained.
> > > +
> > > + 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
> > > + \sa ExposureTimeMode
> > > +
> > > + - ExposureTimeMode:
> > > + type: int32_t
> > > + description: |
> > > + Controls the source of the exposure time that is applied to
the
> > > image
> > > + sensor. When set to Auto, the AE algorithm computes the
exposure
> > > time
> > > + and configures the image sensor accordingly. When set to
Disabled,
> > > + exposure time specified in ExposureTime is applied to the
image
> > > sensor.
> > > + If ExposureTime is not set, then the value last computed by
the AE
> > > + algorithm when the mode was Auto will be used.
> > >
> >
> > Can we un-set ExposureTime? If it ever gets set once at any point in
the
> > application,
> > then ExposureTimeModeDisabled will always use the last set value for
> > ExposureTime.
> >
>
> If I interpret your question right, are you wondering if the
> ExposureTime value is retained if an application sends it when the
> AEGC is actually in auto mode (and so the ExposureTime from application
> is not applied) ?
>
> We discussed this, and I think Paul tried to clarify it in the
> ExposureTime documentation by saying:
>
> - name: ExposureTimeModeDisabled
> If ExposureTime is set while this mode is active, it
> will be ignored, and it will also not be retained.
>
> which means that by design, the ExposureTime is just ignored if sent
> when the AEGC is in auto mode.
>
> Do you think that's not the expected behaviour ?
Not exactly... I was considering the following sequence:
1) ExposureTimeMode is Auto - AE adjusts exposure time as needed.
2) ExposureTimeMode set to Disabled - AE stops adjusting exposure time.
No ExposureTime value set yet, so keep the last AE exposure time.
3) Set ExposureTime to some desired value.
4) ExposureTimeMode set to Auto - AE adjusts exposure time as needed again.
5) ExposureTimeMode set to Disabled - AE stops adjusting exposure time.
In step 5, does the IPA switch back to the ExposureTime set in step 3, or
does
setting ExposureTimeMode to Auto invalidate the existing ExposureTime
value? If
the ExposureTime value is still valid, should we consider a way to
invalidate
(unset) it if the application wanted to just use the AE selected exposure
time
at step 5?
>
>
> >
> > > +
> > > + If ExposureTime is not set and the mode is
> > > ExposureTimeModeDisabled and
> > > + AE was never Auto (either because the camera started in
Disabled
> > > mode,
> > > + or Auto is not supported by the camera), the camera should
use a
> > > + best-effort default value.
> > > +
> > > + When ExposureTimeMode is set Auto, the value set in
ExposureTime
> > > is
> > > + ignored and is not retained. This means that if
ExposureTimeMode
> > > is set
> > > + to Disabled and ExposureTime is not also set, the exposure
time
> > > that
> > > + was last computed by the AE algorithm while the mode was Auto
> > > will be
> > > + applied to the sensor.
> > > +
> > > + If ExposureTimeModeDisabled is supported, the ExposureTime
> > > control must
> > > + also be supported.
> > > +
> > > + The set of ExposureTimeMode modes that are supported by the
> > > camera must
> > > + have an intersection with the supported set of
AnalogueGainMode
> > > modes.
> > >
> > > - \sa AnalogueGain AeEnable
> > > + As it takes a few frames to apply the exposure time, there
is a
> > > period of
> > > + time between submitting a request with ExposureTimeMode set
to
> > > Disabled
> > > + and the exposure time component of the AE actually being
disabled,
> > > + during which the AE algorithm can still update the exposure
time.
> > > If an
> > > + application is switching from automatic and manual control
and
> > > wishes
> > > + to eliminate any flicker during the switch, the following
> > > procedure is
> > > + recommended.
> > >
> >
> > I'm a bit confused by this bit to be honest. If a user switches
> > ExposureTimeMode from
> > Auto to Disabled with the intention of setting a manual ExposureTime,
how
> > can we ever
> > avoid a glitch in the brightness (unless we also change AnalogueGain
> > appropriately)?
> >
>
> See below
>
> >
> > > - \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.
> > > + 1. Start with ExposureTimeMode set to Auto
> > > +
> > > + 2. Set ExposureTimeMode to Disabled
> > > +
> > > + 3. Wait for the first request to be output that has
> > > ExposureTimeMode
> > > + set to Disabled
> > >
> >
> > How would the application know this time point? Would the AE algorithm
> > have to
> > count frames once it has been given a ExposureTimeModeDisabled ctrl then
> > return out the same in the metadata when it knows that it's last
requested
> > exposure
> > time change has been applied?
> >
>
> Not sure this is going to answer your question, but let's start by
> defining what a "glitch" is for us in this definition. I think it's
> useful to validate our understanding against your experience of providing
> this features to the vast number of users you have.
>
> The idea is that applications willing to control the exposure time
> explicitly might want to do so by minimizing the difference between
> the last value computed by the AEGC algorithm and their newly set
> value, to avoid a sudden change in exposure and gain which result in a
> visible "glitch". En example, suddenly moving the exposure time and
> gain to the opposite of the spectrum of what the AEGC was computing
> will result in images going very bright or very dark in just a few
> frames.
>
> The way to implement a smooth transition is to start from the values
> lastly computed by the AEGC (as available in metadata) and then "slowly"
> moving towards the desired manual value. Of course this is not
> mandatory, application might desire a sudden change of exposure, or simply
> won't care about smooth transitions. If they do, however, they have to
> consider that there will always be a number of requests in the queue
> that will be processed by the camera before the one with
> "ExposureTimeDisabled" gets to be processed.
>
> During the processing of those requests in the queue, the AEGC will still
> be active and might still change the exposure time (and gain) to values
quite
> different from the ones visible at the application at the time it
> queued the request with "ExposureTimeModeDisabled".
>
> The steps proposed here suggest to applications to wait until the
> "ExposureTimeModeDisabled" request is returned and the AEGC is
> actually off. From the definitions we gave here, this mean the
> exposure time (and gain) won't be updated by the now inactive AEGC
> block until an "ExposureTime" value is submitted by applications (more
> or less like your agc::pause()/resume() work, if I recall correctly).
>
> When the "ExposureTimeModeDisabled" request has completed,
> applications know that the exposure time won't be updated from that
> point on, and can use the ExposureTime and AnalogueGain metadata values
> as a "stable" starting point for their values.
>
> Does this make sense to you ?
Yes it does, thanks for the clarification!
One question still remains - how does the application know when the AE has
actullay finished - i.e. all the AE adjusted frames have been delivered?
Should
the IPA report "ExposureTimeModeDisabled" only when the AE adjust frames
have
been completed, or report it immediately? The former will require AE to
hold
extra state and keep "running" even when it might be disabled, but it's not
too
big of a problem I suppose.
>
> In my fixups I proposed a rework of the introduction section of this
> part, could you have a look to see if that's more clear ?
>
> >
> > > +
> > > + 4. Copy the value reported in ExposureTime into a new
request, and
> > > + submit it
> > > +
> > > + 5. Proceed to run manual exposure time
> > >
> >
> > Again, I am unclear how this avoids glitches. Say the AE chooses an
> > exposure
> > time of 33ms, then the user wants to switch to 15ms. There is always
going
> > to
> > be a jump in brightness. Perhaps my interpretation of this glitch is
not
> > the same
> > as what you are describing?
>
> If an application decides not to care and halves the exposure time
> from one request to the following one, the above procedure is useless
> indeed.
>
> But as explained above, an application might want to approach 15ms more
> smoothly and the above text suggests how to do so.
>
> I feel like this is mostly directed to applications that wants to
> drive the AEGC with some sort of algorithm instead of application
> that simply take a value in from users and apply it directly. In
> this latter case the values input from the user might very well be 1ms
> hence approaching it slowly might not even be desired.
Thanks, I do understand when this might be needed now. But I struggle to
see
why any application might want to do something like this, but that's not to
say
they won't :-)
Regards,
Naush
>
> Thanks for commenting!
>
> >
> > Ditto comments for the AnalogueGain changes.
> >
> > Regards,
> > Naush
> >
> >
> > > +
> > > + \sa ExposureTime
> > > + enum:
> > > + - name: ExposureTimeModeAuto
> > > + value: 0
> > > + description: |
> > > + The exposure time will be calculated automatically and
set by
> > > the
> > > + AE algorithm. If ExposureTime is set while this mode is
> > > active, it
> > > + will be ignored, and it will also not be retained.
> > > + - 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.
> > > +
> > > + 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 changed from
Auto
> > > to
> > > + Disabled, the provided ExposureTime is applied.
> > >
> > > - AnalogueGain:
> > > type: float
> > > @@ -144,17 +244,85 @@ 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 AnalogueGainMode is
> > > Disabled. If
> > > + this control is set when AnalogueGainMode is Auto, the value
will
> > > be
> > > + ignored and will not be retained.
> > > +
> > > + When reported in metadata, this control indicates what
analogue
> > > gain
> > > + was used for the current request, regardless of
AnalogueGainMode.
> > > + AnalogueGainMode will indicate the source of the analogue
gain
> > > value,
> > > + whether it came from the AE algorithm or not.
> > > +
> > > + \sa ExposureTime
> > > + \sa AnalogueGainMode
> > > +
> > > + - AnalogueGainMode:
> > > + type: int32_t
> > > + description: |
> > > + Controls the source of the analogue gain that is applied to
the
> > > image
> > > + sensor. When set to Auto, the AE algorithm computes the
analogue
> > > gain
> > > + and configures the image sensor accordingly. When set to
Disabled,
> > > + analogue gain specified in AnalogueGain is applied to the
image
> > > sensor.
> > > + If AnalogueGain is not set, then the value last computed by
the AE
> > > + algorithm when the mode was Auto will be used.
> > > +
> > > + If AnalogueGain is not set and the mode is
> > > AnalogueGainModeDisabled and
> > > + AE was never Auto (either because the camera started in
Disabled
> > > mode,
> > > + or Auto is not supported by the camera), the camera should
use a
> > > + best-effort default value.
> > > +
> > > + When AnalogueGainMode is set Auto, the value set in
AnalogueGain
> > > is
> > > + ignored and is not retained. This means that if
AnalogueGainMode
> > > is set
> > > + to Disabled and AnalogueGain is not also set, the analogue
gain
> > > that
> > > + was last computed by the AE algorithm while the mode was Auto
> > > will be
> > > + applied to the sensor.
> > >
> > > - \sa ExposureTime AeEnable
> > > + If AnalogueGainModeDisabled is supported, the AnalogueGain
> > > control must
> > > + also be supported.
> > > +
> > > + The set of AnalogueGainMode modes that are supported by the
> > > camera must
> > > + have an intersection with the supported set of
ExposureTimeMode
> > > modes.
> > > +
> > > + As it takes a few frames to apply the analogue gain, there
is a
> > > period of
> > > + time between submitting a request with AnalogueGainMode set
to
> > > Disabled
> > > + and the analogue gain component of the AE actually being
disabled,
> > > + during which the AE algorithm can still update the analogue
gain.
> > > If an
> > > + application is switching from automatic and manual control
and
> > > wishes
> > > + to eliminate any flicker during the switch, the following
> > > procedure is
> > > + recommended.
> > > +
> > > + 1. Start with AnalogueGainMode set to Auto
> > > +
> > > + 2. Set AnalogueGainMode to Disabled
> > > +
> > > + 3. Wait for the first request to be output that has
> > > AnalogueGainMode
> > > + set to Disabled
> > > +
> > > + 4. Copy the value reported in AnalogueGain into a new
request, and
> > > + submit it
> > > +
> > > + 5. Proceed to run manual analogue gain
> > >
> > +
> > > + \sa AnalogueGain
> > > + enum:
> > > + - name: AnalogueGainModeAuto
> > > + value: 0
> > > + description: |
> > > + The analogue gain will be calculated automatically and
set by
> > > the
> > > + AE algorithm. If AnalogueGain is set while this mode is
> > > active, it
> > > + will be ignored, and it will also not be retained.
> > > + - 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.
> > >
> > > - \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.
> > > + When transitioning from Auto to Disabled mode the last
> > > computed
> > > + gain value is used until a new value is specified
through the
> > > + AnalogueGain control. If an AnalogueGain value is
specified
> > > in the
> > > + same request where the AnalogueGainMode is set to
Disabled,
> > > the
> > > + provided AnalogueGain is applied.
> > >
> > > - Brightness:
> > > type: float
> > > @@ -477,36 +645,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.30.2
> > >
> > >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220705/4bfa37ff/attachment.htm>
More information about the libcamera-devel
mailing list