[libcamera-devel,03/10] libcamera: property_ids: Define draft properties

10030 diff mbox series

Libcamera is in the process of defining its own set of properties, to
advertise the camera capabilities.

To temporary close the gap in the Android camera HAL and support all
static metadata required in the LIMITED hw level, define a set of Draft
properties whose values are taken from their Android definition, in order
to allow pipeline handlers to support them.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/property_ids.yaml | 74 +++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

Hi Jacopo,

On 09/10/2020 13:20, Jacopo Mondi wrote:
> Libcamera is in the process of defining its own set of properties, to
> advertise the camera capabilities.
> 
> To temporary close the gap in the Android camera HAL and support all

s/temporary/temporarily/

> static metadata required in the LIMITED hw level, define a set of Draft
> properties whose values are taken from their Android definition, in order
> to allow pipeline handlers to support them.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/property_ids.yaml | 74 +++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 7261263a9ba3..e524a0718dc3 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -663,4 +663,78 @@ controls:
>          \todo Rename this property to ActiveAreas once we will have property
>                categories (i.e. Properties::PixelArray::ActiveAreas)
>  
> +  - AvailableNoiseReductionModes:
> +      type: int32_t
> +      draft: true
> +      size: [n]
> +      description: |
> +        Draft property to report the list of supported noise reduction modes.
> +        Currently identical to
> +        ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES.

I don't think it's necessary to say "Draft property" in the
descriptions, and that's defined by the draft flag.

However - I interpret "draft: true", to be a statement that all/any of
the implementation of the control will/can be updated, and therefore
that includes the description - and thus ... you can put in anything you
like that you feel is appropriate to get started ;-)

For me - the parts I would check on draft controls are that the styles
are consistent and no glaring errors to tackle - so that makes adding
draft controls easy.

(And getting out of draft status is where the important stage is).

So -

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +      enum:
> +        - name: NoiseReductionModeOff
> +          value: 0
> +          description: No noise reduction is applied
> +        - name: NoiseReductionModeFast
> +          value: 1
> +          description: |
> +            Noise reduction is applied without reducing the frame rate.
> +        - name: NoiseReductionModeHighQuality
> +          value: 2
> +          description: |
> +            High quality noise reduction at the expense of frame rate.
> +        - name: NoiseReductionModeMinimal
> +          value: 3
> +          description: |
> +            Minimal noise reduction is applied without reducing the frame rate.
> +        - name: NoiseReductionModeZSL
> +          value: 4
> +          description: |
> +            Noise reduction is applied at different levels to different streams.
> +
> +  - AvailableColorCorrectionAberrationModes:
> +      type: int32_t
> +      draft: true
> +      size: [n]
> +      description: |
> +        Draft property to report the list of supported color correction
> +        aberration modes. Currently identical to
> +        ANDROID_COLOR_CORRECTION_AVAILABLE_ABERRATION_MODES.
> +      enum:
> +        - name: ColorCorrectionAberrationOff
> +          value: 0
> +          description: No aberration correction is applied.
> +        - name: ColorCorrectionAberrationFast
> +          value: 1
> +          description: Aberration correction will not slow down the frame rate.
> +        - name: ColorCorrectionAberrationHighQuality
> +          value: 2
> +          description: |
> +            High quality aberration correction which might reduce the frame
> +            rate.
> +
> +  - AvailableLensShadingMapModes:
> +      type: int32_t
> +      draft: true
> +      size: [n]
> +      description: |
> +        Draft property to report the list of supported lens shading map modes.
> +        Currently identical to
> +        ANDROID_STATISTICS_INFO_AVAILABLE_LENS_SHADING_MAP_MODES.
> +      enum:
> +        - name: LensShadingMapModeOff
> +          value: 0
> +          description: No lens shading map mode is available.
> +        - name: LensShadingMapModeOn
> +          value: 1
> +          description: The lens shading map mode is available.
> +
> +  - PipelineMaxDepth:
> +      type: int32_t
> +      draft: true
> +      description: |
> +        Draft control to report the maximum number of pipeline stages a frame
> +        has to go through from when it is exposed to when it is available to
> +        applications. Currently identical to ANDROID_REQUEST_PIPELINE_MAX_DEPTH.
> +
>  ...
>

Hi Jacopo,

Thank you for the patch.

On Fri, Oct 09, 2020 at 01:26:00PM +0100, Kieran Bingham wrote:
> On 09/10/2020 13:20, Jacopo Mondi wrote:
> > Libcamera is in the process of defining its own set of properties, to

s/Libcamera/libcamera/

> > advertise the camera capabilities.
> > 
> > To temporary close the gap in the Android camera HAL and support all
> 
> s/temporary/temporarily/
> 
> > static metadata required in the LIMITED hw level, define a set of Draft

s/hw/hardware/ ?

> > properties whose values are taken from their Android definition, in order
> > to allow pipeline handlers to support them.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/libcamera/property_ids.yaml | 74 +++++++++++++++++++++++++++++++++
> >  1 file changed, 74 insertions(+)
> > 
> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > index 7261263a9ba3..e524a0718dc3 100644
> > --- a/src/libcamera/property_ids.yaml
> > +++ b/src/libcamera/property_ids.yaml
> > @@ -663,4 +663,78 @@ controls:
> >          \todo Rename this property to ActiveAreas once we will have property
> >                categories (i.e. Properties::PixelArray::ActiveAreas)
> >  
> > +  - AvailableNoiseReductionModes:
> > +      type: int32_t
> > +      draft: true
> > +      size: [n]
> > +      description: |
> > +        Draft property to report the list of supported noise reduction modes.
> > +        Currently identical to
> > +        ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES.
> 
> I don't think it's necessary to say "Draft property" in the
> descriptions, and that's defined by the draft flag.
> 
> However - I interpret "draft: true", to be a statement that all/any of
> the implementation of the control will/can be updated, and therefore
> that includes the description - and thus ... you can put in anything you
> like that you feel is appropriate to get started ;-)
> 
> For me - the parts I would check on draft controls are that the styles
> are consistent and no glaring errors to tackle - so that makes adding
> draft controls easy.
> 
> (And getting out of draft status is where the important stage is).
> 
> So -
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

I would also drop draft in the description, but both option are fine
with me.

> > +      enum:
> > +        - name: NoiseReductionModeOff
> > +          value: 0
> > +          description: No noise reduction is applied
> > +        - name: NoiseReductionModeFast
> > +          value: 1
> > +          description: |
> > +            Noise reduction is applied without reducing the frame rate.
> > +        - name: NoiseReductionModeHighQuality
> > +          value: 2
> > +          description: |
> > +            High quality noise reduction at the expense of frame rate.
> > +        - name: NoiseReductionModeMinimal
> > +          value: 3
> > +          description: |
> > +            Minimal noise reduction is applied without reducing the frame rate.
> > +        - name: NoiseReductionModeZSL
> > +          value: 4
> > +          description: |
> > +            Noise reduction is applied at different levels to different streams.

This one will likely be handled using per-stream controls, which will
then influence the other values.

> > +
> > +  - AvailableColorCorrectionAberrationModes:
> > +      type: int32_t
> > +      draft: true
> > +      size: [n]
> > +      description: |
> > +        Draft property to report the list of supported color correction
> > +        aberration modes. Currently identical to
> > +        ANDROID_COLOR_CORRECTION_AVAILABLE_ABERRATION_MODES.
> > +      enum:
> > +        - name: ColorCorrectionAberrationOff
> > +          value: 0
> > +          description: No aberration correction is applied.
> > +        - name: ColorCorrectionAberrationFast
> > +          value: 1
> > +          description: Aberration correction will not slow down the frame rate.
> > +        - name: ColorCorrectionAberrationHighQuality
> > +          value: 2
> > +          description: |
> > +            High quality aberration correction which might reduce the frame
> > +            rate.
> > +
> > +  - AvailableLensShadingMapModes:
> > +      type: int32_t
> > +      draft: true
> > +      size: [n]
> > +      description: |
> > +        Draft property to report the list of supported lens shading map modes.
> > +        Currently identical to
> > +        ANDROID_STATISTICS_INFO_AVAILABLE_LENS_SHADING_MAP_MODES.
> > +      enum:
> > +        - name: LensShadingMapModeOff
> > +          value: 0
> > +          description: No lens shading map mode is available.
> > +        - name: LensShadingMapModeOn
> > +          value: 1
> > +          description: The lens shading map mode is available.

Note that these three properties should be handled through ControlInfo
instead of being manually defined. For instance, the LensShadingMapMode
control will have a ControlInfo that should list the supported values.
We don't have this yet in ControlInfo for enum controls, so it's not a
candidate for this patch series, but let's keep the direction in mind.

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > +
> > +  - PipelineMaxDepth:
> > +      type: int32_t
> > +      draft: true
> > +      description: |
> > +        Draft control to report the maximum number of pipeline stages a frame
> > +        has to go through from when it is exposed to when it is available to
> > +        applications. Currently identical to ANDROID_REQUEST_PIPELINE_MAX_DEPTH.
> > +
> >  ...
> >