[libcamera-devel] [RFC PATCH v2 08/12] controls: Add controls necessary for FULL compliance
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Wed Apr 28 12:15:44 CEST 2021
Hi Jacopo,
On Tue, Apr 27, 2021 at 12:21:35PM +0200, Jacopo Mondi wrote:
> Hi Paul,
>
> On Thu, Apr 22, 2021 at 06:40:58PM +0900, Paul Elder wrote:
> > Add controls necessary for FULL compliance:
> > - BlackLevelLocked
> > - EdgeMode
> > - FrameDuration
> > - SensorSensitivity
> > - TonemapMode
> >
> > While at it, change the names of some AwbState values to be consistent
> > and to avoid potential conflicts with AwbLocked.
> >
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> > src/libcamera/control_ids.yaml | 78 +++++++++++++++++++++++++++++++++-
> > 1 file changed, 76 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index f025819a..61c5c96f 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -420,6 +420,13 @@ controls:
> > The camera will cancel any active trigger and the AF routine is
> > reset to its initial state.
> >
> > + - BlackLevelLocked:
> > + type: bool
> > + draft: true
> > + description: |
> > + Whether black-level compensation is locked to its current values, or is
> > + free to vary. Currently identical to ANDROID_BLACK_LEVEL_LOCK.
> > +
> > - NoiseReductionMode:
> > type: int32_t
> > draft: true
> > @@ -554,13 +561,46 @@ controls:
> > - name: AwbStateSearching
> > value: 1
> > description: The AWB algorithm has not converged yet.
> > - - name: AwbConverged
> > + - name: AwbStateConverged
> > value: 2
> > description: The AWB algorithm has converged.
> > - - name: AwbLocked
> > + - name: AwbStateLocked
>
> Do you have build time conflicts ? These values should be properly
> scoped, and should compile fine...
No, but AwbLocked is a draft control so if it ever graduates then it
will conflict. Also all the other values have AwbState while these two
don't so I thought it was a bit inconsistent.
> > value: 3
> > description: The AWB algorithm is locked.
> >
> > + - EdgeMode:
> > + type: int32_t
> > + draft: true
> > + description: |
> > + Control to report the operation mode of edge enhancement. Currently
> > + identical to ANDROID_EDGE_MODE.
> > + enum:
> > + - name: EdgeModeOff
> > + value: 0
> > + description: No edge enhancement is applied.
> > + - name: EdgeModeFast
> > + value: 1
> > + description: |
> > + Apply edge enhancement at a quality level tha does not
> > + slow down frame rate relative to sensor output.
> > + - name: EdgeModeHighQuality
> > + value: 2
> > + description: |
> > + Apply high-quality edge enhancement, at a cost of possible reduced
> > + output frame rate.
> > + - name: EdgeModeZSL
> > + value: 3
> > + description: |
> > + Edge enhancement is applied at different levels for different output
> > + streams, based on resolution.
> > +
> > + - FrameDuration:
>
> We have a FrameDuration control already :)
Oh I didn't notice "A fixed frame duration is achieved by setting the
minimum and maximum values to be the same" :p
> > + type: int64_t
> > + draft: true
> > + description: |
> > + Duration from start of frame exposure to start of next frame exposure, in
> > + nanoseconds. Currently identical to ANDROID_SENSOR_FRAME_DURATION.
> > +
> > - SensorRollingShutterSkew:
> > type: int64_t
> > draft: true
> > @@ -569,6 +609,13 @@ controls:
> > row and the start of exposure of the last row. Currently identical to
> > ANDROID_SENSOR_ROLLING_SHUTTER_SKEW
> >
> > + - SensorSensitivity:
> > + type: int32_t
> > + draft: true
> > + description: |
> > + The amount of gain applied to sensor data before processing. Currently
> > + identical to ANDROID_SENSOR_SENSITIVITY.
>
> I would anyway add
> "The sensitivity is the standard ISO sensitivity value,as defined in
> ISO 12232:2006."
>
> > +
> > - LensShadingMapMode:
> > type: int32_t
> > draft: true
> > @@ -600,6 +647,33 @@ controls:
> > value: 2
> > description: 60Hz flickering detected.
> >
> > + - TonemapMode:
> > + type: int32_t
> > + draft: true
> > + description: |
> > + High-level global contrast/gamma/tonemapping control. Currently identical
> > + to ANDROID_TONEMAP_MODE.
> > + enum:
> > + - name: TonemapModeContrastCurve
> > + value: 0
> > + description: Use the specified tonemapping curve.
>
> The Android description also reports:
> Use the tone mapping curve specified in the android.tonemap.curve* entries.
>
> Do we need a controls::toneMapCurve[float] control to reference here
> and to feed the ph with ?
For this (and the below two), for the purpose of passing CTS, no. iirc
CTS doesn't actually check these values for correctness. For actual
correctness, yeah we probably need them.
Paul
> > + - name: TonemapModeFast
> > + value: 1
> > + description: |
> > + Advanced gamma mapping and color enhancement may be applied, without
> > + reducing frame rate compared to raw sensor output.
> > + - name: TonemapModeHighQuality
> > + value: 2
> > + description: |
> > + High-quality gamma mapping and color enhancement will be applied, at
> > + the cost of possibly reduced frame rate compared to raw sensor output.
> > + - name: TonemapModeGammaValue
> > + value: 3
> > + description: Use the specified gamma value for tonemapping.
>
> This as well reports:
> Use the gamma value specified in android.tonemap.gamma to peform tonemapping.
>
> > + - name: TonemapModePresetCurve
>
> And this one:
> Use the preset tonemapping curve specified in android.tonemap.presetCurve to peform tonemapping.
>
> The same reasoning for Controls::toneMapCurve applies here
> > + value: 4
> > + description: Use the specified preset tonemapping curve.
> > +
> > - PipelineDepth:
> > type: int32_t
> > draft: true
> > --
> > 2.27.0
More information about the libcamera-devel
mailing list