[libcamera-devel] [PATCH 2/2] libcamera: control_ids: Define draft controls
Jacopo Mondi
jacopo at jmondi.org
Mon Aug 10 09:23:15 CEST 2020
Hi Niklas,
On Sat, Aug 08, 2020 at 12:32:09PM +0200, 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:
> > + 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 ;-)
>
Ouch, not a nit indeed. Sorry, I started placing and empy line
randomly to "see how it looks like" then I forgot and didn't notice
when I generated patches. I think I like the empy line though :)
> > + - 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?
>
Being draft controls, I think it's very much relevant to take notes
what's their purpose and the connections with the existing ones.
Those are meant to be here indeed. Don't you agree ?
> > +
> > + - 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.
>
The right process to me would be to define a libcamera control (named
SensorTimestamp or similar) and use it then drop the draft one. The
control value would then be filled in with the
FrameMetadatata::timestamp value (I know nothing about that part, I
trust your judgment there) and the value of the libcamera control used
to fill the Android one in the Camera HAL.
Should we add a todo note ?
> > +
> > + - 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?
Intentional indeed. I don't mind to have notes on draft controls, but
we can keep them out if necessary.
>
> 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>
Thanks
j
>
> > + 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,
> Niklas Söderlund
More information about the libcamera-devel
mailing list