[libcamera-devel] [PATCH 2/2] libcamera: control_ids: Define draft controls

Jacopo Mondi jacopo at jmondi.org
Thu Sep 3 12:19:49 CEST 2020


Hi Kieran,

On Thu, Sep 03, 2020 at 11:04:24AM +0100, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 08/08/2020 11:32, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2020-08-08 12:00:46 +0200, Jacopo Mondi wrote:
> >> Libcamera is in the process of defining its own set of controls and
> >> properties, to advertise the camera capabilities, allow applications to
> >> control the image capture process and return information on the captured
> >> frames.
> >>
> >> However, we're still not there yet to have all the controls we need to
> >> support the Android Camera3 HAL requisites.
> >
> > I would drop this paragraph.
> >
> >>
> >> To temporary close the gap, and support all controls required in the
> >> LIMITED hw level, define a set of Draft controls whose values are taken
> >> from their Android definition, in order to allow pipeline handlers to
> >> support Android natively.
> >>
> >> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> >> ---
> >>  src/libcamera/control_ids.yaml | 242 +++++++++++++++++++++++++++++++++
> >>  1 file changed, 242 insertions(+)
> >>
> >> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> >> index 4c415545dcae..cdd51911c400 100644
> >> --- a/src/libcamera/control_ids.yaml
> >> +++ b/src/libcamera/control_ids.yaml
> >> @@ -284,4 +284,246 @@ controls:
> >>          order in an array of 9 floating point values.
> >>
> >>        size: [3x3]
> >> +
> >> +  - DraftAePrecaptureTrigger:
>
> I thought I recalled that we would have a property on the control that
> stated it was a staging control. Wouldn't that be better/less invasive
> than having to rename all the controls?

It's not just renaming to drop the 'Draft' part that makes a control
a non-draft one.

The here proposed definitions point to the Android documentation, we
might end up expressing the control's semantic through one or more
different libcamera controls.

>
> Or is the point that the controls have to be renamed to highlight them
> through breakage when they are 'formalised' ?
>

I suspect most of the controls here will be re-implemented in a
different fashion

> Or have them in a separate sub-namespace?
>
> With a
> 	state: staging
>
> or
> 	state: draft
>
> property on the controls, we can have the generation script put them
> into a sub-namespace ?

I don't think it's worth the effort, give that in the long term we
plan to get rid of these, not make them part of the standard controls
definition.

>
>
> >> +      type: int32_t
> >> +      description: |
> >> +        Draft control for AE metering trigger. Currently identical to
> >> +        ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER.
> >> +
> >> +        Whether the camera device will trigger a precapture metering sequence
> >> +        when it processes this request.
> >> +      enum:
> >
> > nit: My OCD is tingling, sometimes you have a new line between
> > description and enum, and sometimes you don't ;-)
> >
> >> +        - name: DRAFT_AE_PRECAPTURE_TRIGGER_IDLE
> >> +          value: 0
> >> +          description: The trigger is idle.
> >> +        - name: DRAFT_AE_PRECAPTURE_TRIGGER_START
> >> +          value: 1
> >> +          description: The pre-capture AE metering is started by the camera.
> >> +        - name: DRAFT_AE_PRECAPTURE_TRIGGER_CANCEL
> >> +          value: 2
> >> +          description: |
> >> +            The camera will cancel any active or completed metering sequence.
> >> +            The AE algorithm is reset to its initial state.
> >> +
> >> +  - DraftAfTrigger:
> >> +      type: int32_t
> >> +      description: |
> >> +        Draft control for AF trigger. Currently identical to
> >> +        ANDROID_CONTROL_AF_TRIGGER.
> >> +
> >> +        Whether the camera device will trigger autofocus for this request.
> >> +
> >> +      enum:
> >> +        - name: DRAFT_AF_TRIGGER_IDLE
> >> +          value: 0
> >> +          description: The trigger is idle.
> >> +        - name: DRAFT_AF_TRIGGER_START
> >> +          value: 1
> >> +          description: The AF routine is started by the camera.
> >> +        - name: DRAFT_AF_TRIGGER_CANCEL
> >> +          value: 2
> >> +          description: |
> >> +            The camera will cancel any active trigger and the AF routine is
> >> +            reset to its initial state.
> >> +
> >> +  - DraftNoiseReductionMode:
> >> +      type: int32_t
> >> +      description: |
> >> +        Draft control to select the noise reduction algorithm mode. Currently
> >> +        identical to ANDROID_NOISE_REDUCTION_MODE.
> >> +
> >> +        Mode of operation for the noise reduction algorithm.
> >> +
> >> +      enum:
> >> +        - name: DRAFT_NOISE_REDUCTION_MODE_OFF
> >> +          value: 0
> >> +          description: No noise reduction is applied
> >> +        - name: DRAFT_NOISE_REDUCTION_MODE_FAST
> >> +          value: 1
> >> +          description: |
> >> +            Noise reduction is applied without reducing the frame rate.
> >> +        - name: DRAFT_NOISE_REDUCTION_MODE_HIGH_QUALITY
> >> +          value: 2
> >> +          description: |
> >> +            High quality noise reduction at the expense of frame rate.
> >> +        - name: DRAFT_NOISE_REDUCTION_MODE_MINIMAL
> >> +          value: 3
> >> +          description: |
> >> +            Minimal noise reduction is applied without reducing the frame rate.
> >> +        - name: DRAFT_NOISE_REDUCTION_MODE_ZSL
> >> +          value: 4
> >> +          description: |
> >> +            Noise reduction is applied at different levels to different streams.
> >> +
> >> +  - DraftColorCorrectionAberrationMode:
> >> +      type: int32_t
> >> +      description: |
> >> +        Draft control to select the color correction aberration mode. Currently
> >> +        identical to ANDROID_COLOR_CORRECTION_ABERRATION_MODE.
> >> +
> >> +        Mode of operation for the chromatic aberration correction algorithm.
> >> +
> >> +      enum:
> >> +        - name: DRAFT_COLOR_CORRECTION_ABERRATION_OFF
> >> +          value: 0
> >> +          description: No aberration correction is applied.
> >> +        - name: DRAFT_COLOR_CORRECTION_ABERRATION_FAST
> >> +          value: 1
> >> +          description: Aberration correction will not slow down the frame rate.
> >> +        - name: DRAFT_COLOR_CORRECTION_ABERRATION_HIGH_QUALITY
> >> +          value: 2
> >> +          description: |
> >> +            High quality aberration correction which might reduce the frame
> >> +            rate.
> >> +
> >> +  - DraftAeState:
> >> +      type: int32_t
> >> +      description: |
> >> +        Draft control to report the current AE algorithm state. Currently
> >> +        identical to ANDROID_CONTROL_AE_STATE.
> >> +
> >> +        Current state of the AE algorithm.
> >> +
> >> +      enum:
> >> +        - name: DRAFT_AE_STATE_INACTIVE
> >> +          value: 0
> >> +          description: The AE algorithm is inactive.
> >> +        - name: DRAFT_AE_STATE_SEARCHING
> >> +          value: 1
> >> +          description: The AE algorithm has not converged yet.
> >> +        - name: DRAFT_AE_STATE_CONVERGED
> >> +          value: 2
> >> +          description: The AE algorithm has converged.
> >> +        - name: DRAFT_AE_STATE_LOCKED
> >> +          value: 3
> >> +          description: The AE algorithm is locked.
> >> +        - name: DRAFT_AE_STATE_FLASH_REQUIRED
> >> +          value: 4
> >> +          description: The AE algorithm would need a flash for good results
> >> +        - name: DRAFT_AE_STATE_PRECAPTURE
> >> +          value: 5
> >> +          description: |
> >> +            The AE algorithm has started a pre-capture metering session.
> >> +            \sa DraftAePrecaptureTrigger
> >> +
> >> +  - DraftAfState:
> >> +      type: int32_t
> >> +      description: |
> >> +        Draft control to report the current AF algorithm state. Currently
> >> +        identical to ANDROID_CONTROL_AF_STATE.
> >> +
> >> +        Current state of the AF algorithm.
> >> +      enum:
> >> +        - name: DRAFT_AF_STATE_INACTIVE
> >> +          value: 0
> >> +          description: The AF algorithm is inactive.
> >> +        - name: DRAFT_AF_STATE_PASSIVE_SCAN
> >> +          value: 1
> >> +          description: |
> >> +            AF is performing a passive scan of the scene in continuous
> >> +            auto-focus mode.
> >> +        - name: DRAFT_AF_STATE_PASSIVE_FOCUSED
> >> +          value: 2
> >> +          description: |
> >> +            AF believes the scene is in focus, but might restart scanning.
> >> +        - name: DRAFT_AF_STATE_ACTIVE_SCAN
> >> +          value: 3
> >> +          description: |
> >> +            AF is performing a scan triggered by an AF trigger request.
> >> +            \sa DraftAfTrigger
> >> +        - name: DRAFT_AF_STATE_FOCUSED_LOCK
> >> +          value: 4
> >> +          description: |
> >> +            AF believes has focused correctly and has locked focus.
> >> +        - name: DRAFT_AF_STATE_NOT_FOCUSED_LOCK
> >> +          value: 5
> >> +          description: |
> >> +            AF has not been able to focus and has locked.
> >> +        - name: DRAFT_AF_STATE_PASSIVE_UNFOCUSED
> >> +          value: 6
> >> +          description: |
> >> +            AF has completed a passive scan without finding focus.
> >> +
> >> +  - DraftAwbState:
> >> +      type: int32_t
> >> +      description: |
> >> +        Draft control to report the current AWB algorithm state. Currently
> >> +        identical to ANDROID_CONTROL_AWB_STATE.
> >> +
> >> +        Current state of the AWB algorithm.
> >> +      enum:
> >> +        - name: DRAFT_AWB_STATE_INACTIVE
> >> +          value: 0
> >> +          description: The AWB algorithm is inactive.
> >> +        - name: DRAFT_AWB_STATE_SEARCHING
> >> +          value: 1
> >> +          description: The AWB algorithm has not converged yet.
> >> +        - name: DRAFT_AWB_CONVERGED
> >> +          value: 2
> >> +          description: The AWB algorithm has converged.
> >> +        - name: DRAFT_AWB_LOCKED
> >> +          value: 3
> >> +          description: The AWB algorithm is locked.
> >> +
> >> +  - DraftScalerCropRegion:
> >> +      type: Rectangle
> >> +      description: |
> >> +        Draft control to report the region of the sensor that has been read-out.
> >> +        Currently identical to ANDROID_SCALER_CROP_REGION.
> >> +
> >> +        The area of the sensor that has been read out, defined relatively to
> >> +        the active pixel array size.
> >> +
> >> +        \sa Properties::PixelArrayActiveAreas
> >> +        \todo This will soon be required to implement digital zoom
> >
> > Does the \sa and \todo really belong here or are they left over
> > development notes?
> >
> >> +
> >> +  - DraftSensorTimestamp:
> >> +      type: int64_t
> >> +      description: |
> >> +        Draft control to report the start of exposure of the first row of the
> >> +        captured image. Currently identical to ANDROID_SENSOR_TIMESTAMP.
> >
> > Could we replace the FrameMetadata::timestamp with this control on the
> > request? This definition is really nice and it would move the timestamp
> > from buffer to Request which is something we have been thinking about
> > for some time already.
>
> Moving the timestamp to the request does sound like it would be helpful
> indeed.
>
> >
> >> +
> >> +  - DraftSensorRollingShutterSkew:
> >> +      type: int64_t
> >> +      description: |
> >> +        Draft control to report the time between the start of exposure of the
> >> +        first row and the start of exposure of the last row. Currently
> >> +        identical to ANDROID_SENSOR_ROLLING_SHUTTER_SKEW
> >> +
> >> +  - DraftLensShadingMapMode:
> >> +      type: int32_t
> >> +      description: |
> >> +        Draft control to report if the lens shading map is available. Currently
> >> +        identical to ANDROID_STATISTICS_LENS_SHADING_MAP_MODE.
> >> +
> >> +        \todo Once we define a control to report the lens shading map, we can
> >> +        deduce if it is available or not. Currently this will always be set to
> >> +        OFF.
> >
> > Same here, is this todo something we wish to record?
> >
> > Over all I'm happy to see this patch as it will allow us to move forward
> > at a much quicker phase using these draft controls, nice work.
> >
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >
> >> +      enum:
> >> +        - name: DRAFT_LENS_SHADING_MAP_MODE_OFF
> >> +          value: 0
> >> +          description: No lens shading map mode is available.
> >> +        - name: DRAFT_LENS_SHADING_MAP_MODE_ON
> >> +          value: 1
> >> +          description: The lens shading map mode is available.
> >> +
> >> +
> >> +  - DraftSceneFlicker:
> >> +      type: int32_t
> >> +      description: |
> >> +        Draft control to report the detected scene light frequency. Currently
> >> +        identical to ANDROID_STATISTICS_SCENE_FLICKER.
> >> +
> >> +      enum:
> >> +        - name: DRAFT_SCENE_FLICKER_OFF
> >> +          value: 0
> >> +          description: No flickering detected.
> >> +        - name: DRAFT_SCENE_FLICKER_50HZ
> >> +          value: 0
> >> +          description: 50Hz flickering detected.
> >> +        - name: DRAFT_SCENE_FLICKER_60HZ
> >> +          value: 0
> >> +          description: 50Hz flickering detected.
> >> +
> >>  ...
> >> --
> >> 2.27.0
> >>
> >> _______________________________________________
> >> libcamera-devel mailing list
> >> libcamera-devel at lists.libcamera.org
> >> https://lists.libcamera.org/listinfo/libcamera-devel
> >
>
> --
> Regards
> --
> Kieran


More information about the libcamera-devel mailing list