[PATCH v3 1/8] controls: Introduce AEGC-related controls
Paul Elder
paul.elder at ideasonboard.com
Mon Nov 18 14:51:02 CET 2024
On Mon, Nov 18, 2024 at 05:30:39PM +0900, Paul Elder wrote:
> On Fri, Nov 15, 2024 at 01:51:00PM +0000, Naushir Patuck wrote:
> > On Fri, 15 Nov 2024 at 13:41, Paul Elder <paul.elder at ideasonboard.com> wrote:
> > >
> > > On Thu, Nov 14, 2024 at 03:16:20PM +0100, Stefan Klug wrote:
> > > > On Wed, Nov 13, 2024 at 02:18:26PM +0000, David Plowman wrote:
> > > > Hi Paul,
> > > >
> > > > Thank you for the patch.
> > > >
> > > > > Hi Paul
> > > > >
> > > > > Thanks for posting all this!
> > > > >
> > > > > On Wed, 13 Nov 2024 at 13:13, Paul Elder <paul.elder at ideasonboard.com> wrote:
> > > > > >
> > > > > > Introduce the AeState, ExposureTimeMode and AnalogueGainMode controls
> > > > > > to model the AEGC algorithm block.
> > > > > >
> > > > > > The three controls allow applications to select the exposure time and
> > > > > > analogue gain computation calculation mode (auto vs manual)
> > > > > > independently from one another, while the AeState control reports the
> > > > > > global state for the AEGC algorithm.
> > > > > >
> > > > > > The new controls are meant to replace the existing AeEnable and AeLocked
> > > > > > controls, which are momentarily kept not to break compilation of
> > > > > > platforms making use of them.
> > > > > >
> > > > > > 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>
> > > > > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > > > >
> > > > > > ---
> > > > > > Changes in v3:
> > > > > > - recovered from 2-year-old bitrot
> > > > >
> > > > > Indeed, I think I remember discussing all this _years_ ago!!
> > > > >
> > > > > > ---
> > > > > > src/libcamera/control_ids_core.yaml | 243 ++++++++++++++++++++++++---
> > > > > > src/libcamera/control_ids_draft.yaml | 29 ----
> > > > > > 2 files changed, 223 insertions(+), 49 deletions(-)
> > > > > >
> > > > > > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> > > > > > index 1b1bd9507d25..98ef0736aa1b 100644
> > > > > > --- a/src/libcamera/control_ids_core.yaml
> > > > > > +++ b/src/libcamera/control_ids_core.yaml
> > > > > > @@ -26,6 +26,75 @@ controls:
> > > > > >
> > > > > > \sa AeEnable
> > > > > >
> > > > > > + - AeState:
> > > > > > + type: int32_t
> > > > > > + description: |
> > > > > > + Control to report the AEGC algorithm state.
> > > > > > +
> > > > > > + The AEGC algorithm computes the exposure time and the analogue gain
> > > > > > + values to be applied to the image sensor.
> > > > > > +
> > > > > > + The AEGC algorithm behaviour is controlled by the ExposureTimeMode and
> > > > > > + AnalogueGainMode controls, which allow applications to decide how
> > > > > > + the exposure time and gain are computed, in Auto or Manual mode,
> > > > > > + independently from one another.
> > > > > > +
> > > > > > + The AeState control reports the AEGC algorithm state through a single
> > > > > > + value and describes it as a single computation block which computes
> > > > > > + both the exposure time and the analogue gain values.
> > > > > > +
> > > > > > + When both the exposure time and analogue gain values are configured to
> > > > > > + be in Manual mode, the AEGC algorithm is quiescent and does not actively
> > > > > > + compute any value and the AeState control will report AeStateIdle.
> > > > > > +
> > > > > > + When at least the exposure time or analogue gain are configured to be
> > > > > > + computed by the AEGC algorithm, the AeState control will report if the
> > > > > > + algorithm has converged to stable values for all of the controls set
> > > > > > + to be computed in Auto mode.
> > > > > > +
> > > > > > + \sa AnalogueGainMode
> > > > > > + \sa ExposureTimeMode
> > > > > > +
> > > > > > + enum:
> > > > > > + - name: AeStateIdle
> > > > > > + value: 0
> > > > > > + description: |
> > > > > > + The AEGC algorithm is inactive.
> > > > > > +
> > > > > > + This state is returned when both AnalogueGainMode and
> > > > > > + ExposureTimeMode are set to Manual and the algorithm is not
> > > > > > + actively computing any value.
> > > > > > +
> > > > > > + When ExposureTimeMode or AnalogueGainMode are set to Auto mode, the
> > > > > > + AEGC algorithm might spontaneously initiate a new scan, in which
> > > > >
> > > > > Maybe "might spontaneously start adjusting again"? The word "scan"
> > > > > isn't really wrong, just sounds a bit like it was copy-pasted from an
> > > > > autofocus description! But I'm not too bothered.
> > > > >
> > > > > > + case the AeState control is moved to AeStateSearching.
> > > > > > + - name: AeStateSearching
> > > >
> > > > A bit of bikeshedding here. Somehow "searching" sound strange to me (may
> > > > well be because I'm not a native speaker). How does "regulating" or
> > > > "controlling" sound to native ears? In my mind a search doesn't really
> > > > converge, but a regulation does.
> > >
> > > Hm to be "regulating" and "controlling" aren't the right words either...
> > > Searching I think works fine because you're searching in a numerical
> > > space for the value that maximizes the score.
> > >
> > > >
> > > > > > + value: 1
> > > > > > + description: |
> > > > > > + The AEGC algorithm is actively computing new values, for either the
> > > > > > + exposure time or the analogue gain, but has not converged to a
> > > > > > + stable result yet.
> > > > > > +
> > > > > > + This state is returned if at least one of AnalogueGainMode
> > > > > > + or ExposureTimeMode is set to auto and the algorithm hasn't
> > > > > > + converged yet.
> > > > > > +
> > > > > > + The AEGC algorithm converges once stable values are computed for
> > > > > > + any of the controls set to be computed in Auto mode.
> > > >
> > > > Should that be "all of the"?
> > >
> > > Oh yeah it should be.
> > >
> > > >
> > > > > > +
> > > > > > + Once the algorithm converges the state is moved to AeStateConverged.
> > > > > > + - name: AeStateConverged
> > > > > > + value: 2
> > > > > > + description: |
> > > > > > + The AEGC algorithm has converged.
> > > > > > +
> > > > > > + This state is returned if at least one of AnalogueGainMode
> > > > > > + or ExposureTimeMode is set to Auto, and the AEGC algorithm has
> > > > > > + converged to stable value.
> > > > > > +
> > > > > > + The AEGC algorithm might spontaneously re-initiate an AE scan, in
> > > > > > + which case the state is moved to AeStateSearching.
> > > >
> > > > This sounds like it is based on a random event. What about something
> > > > like: When the statistics/measurements move too far away from the
> > > > convergence point the AEGC algo automatically restarts
> > > > regulation/scanning...
> > >
> > > In a way I think it's spontaneous to the user :)
> > >
> > > But indeed your description would be more accurate.
> > >
> > > >
> > > > > > +
> > > > > > # AeMeteringMode needs further attention:
> > > > > > # - Auto-generate max enum value.
> > > > > > # - Better handling of custom types.
> > > > > > @@ -104,6 +173,13 @@ controls:
> > > > > > The exposure modes specify how the desired total exposure is divided
> > > > > > between the shutter time and the sensor's analogue gain. They are
> > > > > > platform specific, and not all exposure modes may be supported.
> > > > > > +
> > > > > > + When one of AnalogueGainMode or ExposureTimeMode is set to Manual,
> > > > > > + the fixed values will override any choices made by AeExposureMode.
> > > > > > +
> > > > > > + \sa AnalogueGainMode
> > > > > > + \sa ExposureTimeMode
> > > > > > +
> > > > > > enum:
> > > > > > - name: ExposureNormal
> > > > > > value: 0
> > > > > > @@ -124,13 +200,15 @@ controls:
> > > > > > Specify an Exposure Value (EV) parameter.
> > > > > >
> > > > > > The EV parameter will only be applied if the AE algorithm is currently
> > > > > > - enabled.
> > > > > > + enabled, that is, at least one of AnalogueGainMode and ExposureTimeMode
> > > > > > + are in Auto mode.
> > > > > >
> > > > > > 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
> > > > > > @@ -140,17 +218,95 @@ controls:
> > > > > >
> > > > > > 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 Manual. If
> > > > > > + this control is set when ExposureTimeMode is Auto, the value will be
> > > > > > + ignored and will not be retained.
> > > > >
> > > > > I'm not sure about this, it seems like a change of API behaviour which
> > > > > will require users to be re-educated and application code to be hunted
> > > > > down and changed. Previously you could just set the exposure time to X
> > > > > ms, and it "automatically went to manual mode". Auto focus behaves the
> > > > > same, I think, with respect to setting the lens position. It's just
> > > > > more convenient.
> > > >
> > > > This is an interesting discussion. I wasn't aware of the old
> > > > specification. I see that just setting one value and deducing what the
> > > > user wants is convenient. On the other hand one control changing the
> > > > value of another control feels quite difficult to explain and in my
> > > > opinion should be prevented whenever possible. I believe it is easier to
> > > > educate the user and have an explicit interface than to have hidden
> > > > magic whenever possible.
> > >
> > > Yeah that was my thought process too when I designed this.
> >
> > There is one minor annoyance this change will introduce because of the
> > lack of ordering in the ControlList map.
> >
> > Going from, say, auto control into manual will require 2 controls, and
> > the presence of the AnalogueGainModeManual enum + the AnalogueGain
> > value to use. Since Controls are not ordered, if you iterate through
> > the ControlList, you will have to check for the presence of one, then
> > the other programmatically.
> >
>
> Oh good point, I totally forgot about that. Back to the drawing board...
Ok I'm back from the drawing board.
At the moment I see two easy options/solutions. One is to check each the
prescence of individual control instead of iterating over the control
list, like what rkisp1 does. The other option is to directly check the
prescence of the enable control when handling the manual control in the
iteration. Something like (using what's in the Raspberry Pi IPA):
for (auto const &ctrl : controls) {
switch(ctrl.first) {
case controls::EXPOSURE_TIME_MODE: {
exposureEnableFlag_ =
(*exposureEnable == controls::ExposureTimeModeAuto);
}
case controls::EXPOSURE_TIME: {
const auto &exposureEnable =
controls.get(controls::ExposureTimeMode);
if (exposureEnable &&
(*exposureEnable == controls::ExposureTimeModeAuto))
exposureEnableFlag_ = true;
if (!exposureEnable && !exposureEnableFlag_)
continue;
}
}
}
I suppose they are indeed minor annoyances, but I think it's a necessary
byproduct of consistency and correctness that the new controls provide
over what we had previously.
Paul
> > >
> > > >
> > > > >
> > > > > [There's also the question of how confident you are that pipeline
> > > > > handlers correctly deal with two controls like this that are
> > > > > effectively ordered...]
> > > > >
> > > > > > +
> > > > > > + 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 Manual,
> > > > > > + the value of the ExposureTime control is used.
> > > > > > +
> > > > > > + When transitioning from Auto to Manual mode and no ExposureTime control
> > > > > > + is provided by the application, the last value computed by the AE
> > > > > > + algorithm when the mode was Auto will be used. If the ExposureTimeMode
> > > > > > + was never set to Auto (either because the camera started in Manual mode,
> > > > > > + or Auto is not supported by the camera), the camera should use a
> > > > > > + best-effort default value.
> > > > >
> > > > > Yes, I think this sounds OK. So setting it to manual mode is what
> > > > > setting AeEnable to zero used to do, except that it can be done
> > > > > separately for exposure time and gain now.
> > > > >
> > > > > > +
> > > > > > + If ExposureTimeModeManual 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.
> > > > >
> > > > > That had me scratching my head for a moment! Maybe
> > > > >
> > > > > "At least one of the modes, auto or manual, must be supported by both
> > > > > ExposureTimeMode and AnalogueGainMode."
> > > > >
> > > > > Is that clearer? (Also, is it what you meant?)
> > > > >
> > > > > > +
> > > > > > + Flickerless exposure mode transitions
> > > > > > +
> > > > > > + Applications that transition from ExposureTimeModeAuto to the direct
> > > > > > + control of the exposure time should aim to do so by selecting an
> > > > > > + ExposureTime value as close as possible to the last value computed by
> > > > > > + the auto exposure algorithm in order to avoid any visible flickering.
> > > > >
> > > > > Sounds a bit like you _have_ to do this. Maybe
> > > > >
> > > > > "Applications that wish to transition from ..Auto to direct control of
> > > > > the exposure time without causing extra flicker, should do so by
> > > > > selecting an ExposureTime value as close as possible to the last value
> > > > > computed by the auto exposure algorithm."
> > > > >
> > > > > Don't know if that's better or not.
> > > > >
> > > > > > +
> > > > > > + To select the correct value to use as ExposureTime value, applications
> > > > > > + should accommodate the natural delay in applying controls caused by the
> > > > > > + capture pipeline frame depth.
> > > > > > +
> > > > > > + When switching to manual exposure mode, . I'm struggling a bit with that wording!!applications should not
> > > > > > + immediately specify an ExposureTime value in the same request where
> > > > > > + ExposureTimeMode is set to Manual. They should instead wait for the
> > > > > > + first Request where ExposureTimeMode is reported as
> > > > > > + ExposureTimeModeManual in the Request metadata, and use the reported
> > > > > > + ExposureTime to populate the control value in the next Request to be
> > > > > > + queued to the Camera.
> > > > > > +
> > > > > > + The implementation of the auto-exposure algorithm should equally try to
> > > > > > + minimize flickering and when transitioning from manual exposure mode to
> > > > > > + auto exposure use the last value provided by the application as starting
> > > > > > + point.
> > > > > > +
> > > > > > + 1. Start with ExposureTimeMode set to Auto
> > > > > > +
> > > > > > + 2. Set ExposureTimeMode to Manual
> > > > > > +
> > > > > > + 3. Wait for the first completed request that has ExposureTimeMode
> > > > > > + set to Manual
> > > > > > +
> > > > > > + 4. Copy the value reported in ExposureTime into a new request, and
> > > > > > + submit it
> > > > > > +
> > > > > > + 5. Proceed to run manual exposure time
> > > > > > +
> > > > > > + \sa ExposureTime
> > > > > > + enum:
> > > > > > + - name: ExposureTimeModeAuto
> > > > > > + value: 0
> > > > > > + description: |
> > > > > > + The exposure time will be calculated automatically and set by the
> > > > > > + AE algorithm.
> > > > > >
> > > > > > - \sa AnalogueGain AeEnable
> > > > > > + If ExposureTime is set while this mode is active, it will be
> > > > > > + ignored, and it will also not be retained.
> > > > > > + - name: ExposureTimeModeManual
> > > > > > + value: 1
> > > > > > + description: |
> > > > > > + The exposure time will not be updated by the AE algorithm.
> > > > > >
> > > > > > - \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 Manual 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
> > > > > > + Manual, the provided ExposureTime is applied immediately.
> > > > >
> > > > > Did we say anywhere what happens when you go from manual back to auto?
> > > > > I'm assuming the adjustments restart from the manual values that you
> > > > > currently have.
> > > > >
> > > > > >
> > > > > > - AnalogueGain:
> > > > > > type: float
> > > > > > @@ -160,17 +316,64 @@ 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 Manual. If
> > > > > > + this control is set when AnalogueGainMode is Auto, the value will be
> > > > > > + ignored and will not be retained.
> > > > >
> > > > > As above. It would be user-friendly to switch automatically to manual, I think.
> > > > >
> > > > > > +
> > > > > > + 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 AEGC 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 AEGC algorithm computes the analogue gain
> > > > > > + and configures the image sensor accordingly. When set to Manual,
> > > > > > + the value of the AnalogueGain control is used.
> > > > > > +
> > > > > > + When transitioning from Auto to Manual mode and no AnalogueGain control
> > > > > > + is provided by the application, the last value computed by the AEGC
> > > > > > + algorithm when the mode was Auto will be used. If the AnalogueGainMode
> > > > > > + was never set to Auto (either because the camera started in Manual mode,
> > > > > > + or Auto is not supported by the camera), the camera should use a
> > > > > > + best-effort default value.
> > > > > > +
> > > > > > + If AnalogueGainModeManual is supported, the AnalogueGain control must
> > > > > > + also be supported.
> > > > > > +
> > > > > > + The set of AnalogueGainMode modes that are . I'm struggling a bit with that wording!!supported by the camera must
> > > > > > + have an intersection with the supported set of ExposureTimeMode modes.
> > > > >
> > > > > As above!
> > > > >
> > > > > Thanks again
> > > > >
> > > > > David
> > > > >
> > > > > > +
> > > > > > + The same procedure described for performing flickerless transitions in
> > > > > > + the ExposureTimeMode control documentation should be applied to
> > > > > > + analogue gain.
> > > > > > +
> > > > > > + \sa ExposureTimeMode
> > > > > > + \sa AnalogueGain
> > > > > > + enum:
> > > > > > + - name: AnalogueGainModeAuto
> > > > > > + value: 0
> > > > > > + description: |
> > > > > > + The analogue gain will be calculated automatically and set by the
> > > > > > + AEGC algorithm.
> > > > > >
> > > > > > - \sa ExposureTime AeEnable
> > > > > > + If AnalogueGain is set while this mode is active, it will be
> > > > > > + ignored, and it will also not be retained.
> > > > > > + - name: AnalogueGainModeManual
> > > > > > + value: 1
> > > > > > + description: |
> > > > > > + The analogue gain will not be updated by the AEGC algorithm.
> > > > > >
> > > > > > - \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 Manual 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 changed from Auto to
> > > > > > + Manual, the provided AnalogueGain is applied immediately.
> > > > > >
> > > > > > - AeFlickerMode:
> > > > > > type: int32_t
> > > > > > diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml
> > > > > > index 1b284257f601..32c2b3feaf65 100644
> > > > > > --- a/src/libcamera/control_ids_draft.yaml
> > > > > > +++ b/src/libcamera/control_ids_draft.yaml
> > > > > > @@ -77,35 +77,6 @@ controls:
> > > > > > High quality aberration correction which might reduce the frame
> > > > > > rate.
> > > > > >
> > > > > > - - AeState:
> > > > > > - type: int32_t
> > > > > > - 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
> > > > > > -
> > > > > > - AwbState:
> > > > > > type: int32_t
> > > > > > description: |
> > > > > > --
> > > > > > 2.39.2
> > > > > >
More information about the libcamera-devel
mailing list