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

Kieran Bingham kieran.bingham at ideasonboard.com
Thu Sep 3 12:04:24 CEST 2020


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?

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

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 ?
	

>> +      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