[PATCH v3 1/8] controls: Introduce AEGC-related controls
David Plowman
david.plowman at raspberrypi.com
Tue Nov 19 14:44:13 CET 2024
Hi Jacopo
On Mon, 18 Nov 2024 at 14:43, Jacopo Mondi
<jacopo.mondi at ideasonboard.com> wrote:
>
> Hi David
>
> On Mon, Nov 18, 2024 at 02:29:15PM +0000, David Plowman wrote:
> > Hi Paul
> >
> > I would just like to make sure we're looking after some of our (many)
> > less-expert Raspberry Pi users, and don't want to cause code they're
> > using to break with minimal warning or explanation. (These are not
> > people who read libcamera mailing lists!)
> >
>
> One clarification here: are you concerned about pi users that has
> custom applications interfacing directly with libcamera (either
> through the C++ or Python APIs) ?
I'm not particularly worried about anyone using C++. It's the Python
users, who may be using simple scripts they've written or been given,
who could get confused and end up clogging the Raspberry Pi forums!
>
> > For example, it could be helpful to us to maintain some degree of
> > backward compatibility. Maybe switching to "manual mode" automatically
> > would help. As I commented previously, AF lens position works like
> > this, so it would be more consistent.
> >
> > I understand that folks don't like the "zero means auto" value, on the
> > other hand I'm not totally persuaded that breaking a single control
> > into two, now with the risk of a race hazard, is so very much better.
> > Maybe we could have a switchover period where the old method works but
> > issues a warning?
> >
> > Another thought would be for us to "translate" control lists from the
> > older API to the newer one at runtime. It's hardly a big deal, but
> > feels a teeny weeny bit inelegant!
> >
>
> Because this seems to suggest you could handle the translation, I
> though you were considering doing that in your applications, but
> now I wonder if you're suggesting to do that in the IPA
Yes, I can handle the translation for Python users easily enough. It's
not lovely, but I think that's probably the least bad alternative for
us.
Sorry if I've been sounding a bit grumpy about this!!
David
>
> Thanks
> j
>
> > Thanks!
> > David
> >
> > 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.
> > >
> > >
> > > Thanks,
> > >
> > > 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