[libcamera-devel] [PATCH v2 3/6] libcamera: controls: Add AE related controls
Naushir Patuck
naush at raspberrypi.com
Mon Mar 23 16:29:18 CET 2020
Hi Kieran,
On Fri, 20 Mar 2020 at 14:58, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Hi Naush,
>
> On 09/03/2020 12:33, Naushir Patuck wrote:
> > AeMeteringMode, AeConstraintMode, and AeExposureMode are new enum type
> > controls used to specify operating modes in the AE algorithm. All modes
> > may not be supported by all platforms.
> >
> > ExposureValue is a new double type control used to set the log2 exposure
> > adjustment for the AE algorithm.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> > src/libcamera/control_ids.yaml | 121 +++++++++++++++++++++++++++++----
> > 1 file changed, 109 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index 5bbe65ae..da1a7b43 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -23,24 +23,104 @@ controls:
> >
> > \sa AeEnable
> >
> > - - AwbEnable:
> > - type: bool
> > + - AeMeteringMode:
> > + type: int32_t
> > description: |
> > - Enable or disable the AWB.
> > -
> > - \sa ManualGain
> > + Specify a metering mode for the AE algorithm to use. The metering
> > + modes determine which parts of the image are used to determine the
> > + scene brightness. Metering modes may be platform-specific and not
>
> Only caught because of the spelling error below, but in the other
> instances you do not hyphenate platform-specific. Maybe the simple
> option is to not hyphenate here either :-)
>
> > + all metering modes may be supported.
> > + enum:
> > + - name: MeteringCentreWeighted
> > + value: 0
> > + description: Centre-weighted metering mode.
> > + - name: MeteringSpot
> > + value: 1
> > + description: Spot metering mode.
> > + - name: MeteringMatrix
> > + value: 2
> > + description: Matrix metering mode.
> > + - name: MeteringCustom1
> > + value: 3
>
> This feels a bit painful if we want to add more (non-custom, or custom)
> modes.
>
> We would only be able to append to prevent ABI breakage - Then the enums
> could seem a bit ugly:
>
> enum {
> MeteringCentreWeighted,
> MeteringSpot,
> MeteringMatrix,
> MeteringCustom1,
> MeteringCustom2,
> MeteringCustom3,
> MeteringMadeUpGeneralControl,
> MeteringCustom4,
> MeteringMadeupButGeneralControl2,
> }
>
> But I fear that may not be something we can easily avoid unless we know
> *all* of the metering mode names in advance...
>
> Of course we're intentionally not yet ABI stable anyway, so this could
> still be an intermediate step...
>
>
> One option could be to have MeteringCustom, with the actual 'custom'
> choice provided by another means ('yet another' control?), but maybe
> that doesn't scale well either. I'm not sure yet...
>
> > + description: Custom metering mode 1.
> > + - name: MeteringCustom2
> > + value: 4
> > + description: Custom metering mode 2.
> > + - name: MeteringCustom3
> > + value: 5
> > + description: Custom metering mode 3.
>
> Are you able to express what you currently envisage the 3 custom modes
> would be used for?
>
The intention here was to allow users to add new modes to the
algorithm, perhaps something very specific to their application. If
the tuning parameters for the mode could be added/tweaked via a
text/config file, then the user could directly use the new mode
without ever having to download/re-compile any code. Initially (as in
patch v1), these modes were defined as std::string types, but we
concluded that added too much vagueness to the list of options.
Perhaps 3 custom modes is too much, and we can make do with just one?
> > + - name: MeteringModeMax
> > + value: 5
> It's a shame we have to express the enum max value manually here :-(
> I haven't looked at if it could be identified through code though.
>
This is primarily needed for setting up ControlRange for the control.
It should be entirely possible for the gen_controls script to add the
Max item to the end of the enum?
> > + description: Maximum allowed value (place any new values above here).
> >
> > - - Brightness:
> > + - AeConstraintMode:
> > type: int32_t
> > - description: Specify a fixed brightness parameter
> > + description: |
> > + Specify a constraint mode for the AE algorithm to use. These determine
> > + how the measured scene brightness is adjusted to reach the desired
> > + target exposure. Constraint modes may be platform specific, and not
> > + all constaint modes may be supported.
>
> /constaint/constraint/
>
> > + enum:
> > + - name: ConstraintNormal
> > + value: 0
> > + description: Default constraint mode.
> > + - name: ConstraintHighlight
> > + value: 1
> > + description: Highlight constraint mode.
> > + - name: ConstraintCustom1
> > + value: 2
> > + description: Custom constraint mode 1.
> > + - name: ConstraintCustom2
> > + value: 3
> > + description: Custom constraint mode 2.
> > + - name: ConstraintCustom3
> > + value: 4
> > + description: Custom constraint mode 3.
> > + - name: ConstraintModeMax
> > + value: 4
> > + description: Maximum allowed value (place any new values above here).
>
> Same comments apply here of course.
>
> Perhaps we should expand the initial set from the modes/constraints that
> we will need for supporting the Android HAL...
>
>
> >
> > - - Contrast:
> > + - AeExposureMode:
> > type: int32_t
> > - description: Specify a fixed contrast parameter
> > + description: |
> > + Specify an exposure mode for the AE algorithm to use. These specify
> > + how the desired total exposure is divided between the shutter time
> > + and the sensor's analogue gain. The exposure modes are platform
> > + spcific, and not all exposure modes may be supported.
>
> s/spcific/specific/
>
> > + enum:
> > + - name: ExposureNormal
> > + value: 0
> > + description: Default metering mode.
> > + - name: ExposureSport
> > + value: 1
> > + description: Sport metering mode (favouring short shutter times).
> > + - name: ExposureLong
> > + value: 2
> > + description: Exposure mode allowing long exposure times.
> > + - name: ExposureCustom1
> > + value: 3
> > + description: Custom exposure mode 1.
> > + - name: ExposureCustom2
> > + value: 4
> > + description: Custom exposure mode 2.
> > + - name: ExposureCustom3
> > + value: 5
> > + description: Custom exposure mode 3.
> > + - name: ExposureModeMax
> > + value: 5
> > + description: Maximum allowed value (place any new values above here).
> >
> > - - Saturation:
> > - type: int32_t
> > - description: Specify a fixed saturation parameter
> > + - ExposureValue:
> > + type: float
> > + description: |
> > + Specify an Exposure Value (EV) parameter. The EV parameter will only be
> > + applied if the AE alogrithm is currently enabled.
> > +
> > + 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
> >
> > - ManualExposure:
> > type: int32_t
> > @@ -57,4 +137,21 @@ controls:
> > applied to all colour channels.
> >
> > \sa ManualExposure
> > +
>
> What changes were made to the properties below ? (I don't see anything
> in particular) or is that just git being a pain and adding churn?
>
I only moved them around - so all the controls in the file are ordered
in appropriate groups (AGC, AWB, General...).
>
>
> > + - AwbEnable:
> > + type: bool
> > + description: |
> > + Enable or disable the AWB.
> > +
> > + - Brightness:
> > + type: int32_t
> > + description: Specify a fixed brightness parameter
> > +
> > + - Contrast:
> > + type: int32_t
> > + description: Specify a fixed contrast parameter
> > +
> > + - Saturation:
> > + type: int32_t
> > + description: Specify a fixed saturation parameter
> > ...
> >
>
> --
> Regards
> --
> Kieran
Regards,
Naush
More information about the libcamera-devel
mailing list