[libcamera-devel,RFC,2/6] libcamera: property_ids: Define draft properties

Message ID 20200911162039.61933-3-jacopo@jmondi.org
State Superseded, archived
Delegated to: Jacopo Mondi
Headers show
Series
  • libcamera: Define draft controls and properties
Related show

Commit Message

Jacopo Mondi Sept. 11, 2020, 4:20 p.m. UTC
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 Android.

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

Comments

Niklas Söderlund Sept. 12, 2020, 3:26 p.m. UTC | #1
Hi Jacopo,

Thanks for your work.

On 2020-09-11 18:20:35 +0200, 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
> 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 Android.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/property_ids.yaml | 70 +++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 74ad0195d631..62bed9f3844d 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -640,4 +640,74 @@ controls:
>          \todo Rename this property to ActiveAreas once we will have property
>                categories (i.e. Properties::PixelArray::ActiveAreas)
>  
> +  - DraftAvailableNoiseReductionModes:
> +      type: int32_t
> +      size: [n]
> +      description: |
> +        Draft property to report the list of supported noise reduction modes.
> +        Currently identical to
> +        ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES.
> +      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.
> +
> +  - DraftAvailableColorCorrectionAberrationModes:
> +      type: int32_t
> +      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: 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.
> +
> +  - DraftAvailableLensShadingMapModes:
> +      type: int32_t
> +      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: 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.
> +
> +  - DraftPipelineMaxDepth:
> +      type: int32_t
> +      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.
> +
>  ...
> -- 
> 2.28.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham Oct. 7, 2020, 2:23 p.m. UTC | #2
Hi Jacopo,

On 11/09/2020 17: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 Android.
> 

I had it in my head that these draft controls were going to be
namespaced such that it was accessible through:

switch(props.get(properties::draft::AvailableNoiseReductionModes) {
	case properties::draft::NOISE_REDUCTION_MODE_OFF:
	case properties::draft::NOISE_REDUCTION_MODE_FAST:
		// We don't do noise reduction
		break;
	case properties::draft::NOISE_REDUCTION_MODE_HIGH_QUALITY:
DRAFT_NOISE_REDUCTION_MODE_MINIMAL
}

And that inserting (or removing) a control to/from that draft namespace
would be about setting a property on the control definition like:

+  - AvailableNoiseReductionModes:
+      draft: true
+      type: int32_t
+      size: [n]
+      description: |

Or of course it could be:
	status: draft
or
	status: staging

But that's all bikeshed colours.


Is the reason you've used a prefix (on every value) because that's the
way you want it? or because you haven't looked at updating the control
generation scripts to support the 'draft' property?

If it's the latter, I'll tackle that now for you.
(I'm going to have a quick go to see how hard it would be to implement).


> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/property_ids.yaml | 70 +++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 74ad0195d631..62bed9f3844d 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -640,4 +640,74 @@ controls:
>          \todo Rename this property to ActiveAreas once we will have property
>                categories (i.e. Properties::PixelArray::ActiveAreas)
>  
> +  - DraftAvailableNoiseReductionModes:
> +      type: int32_t
> +      size: [n]
> +      description: |
> +        Draft property to report the list of supported noise reduction modes.
> +        Currently identical to
> +        ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES.
> +      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.

This implies that different streams can process noise reduction
separately? but it's confusing because it's referencing ZeroShutterLag?
So - I don't (yet) understand how that corresponds to the noise
reduction. Maybe something for me to learn soon.

Anyway, especially as these are 'DRAFT/::draft::' whatever - I don't
think we need to dwell 'too' much on the detail, as the point of being a
draft is to allow the plumbing to progress, and fix the detail before
moving out of draft status.


The other point I'd like to discuss here, is that you have used
DRAFT_ALL_CAPS_ENUM style for all the enums, which matches the
annonymous enum in
include/libcamera/property_ids.h
enum {
        LOCATION = 1,
        ROTATION = 2,
        MODEL = 3,
        UNIT_CELL_SIZE = 4,
        PIXEL_ARRAY_SIZE = 5,
        PIXEL_ARRAY_OPTICAL_BLACK_RECTANGLES = 6,
        PIXEL_ARRAY_ACTIVE_AREAS = 7,
};


but differs to the LocationValues enum:
enum LocationValues {
        CameraLocationFront = 0,
        CameraLocationBack = 1,
        CameraLocationExternal = 2,
};

My personal preference would be to follow the LocationValues style, as
it feels more C++/libcamery ;-)

But - it's just an open question for the moment.



> +
> +  - DraftAvailableColorCorrectionAberrationModes:
> +      type: int32_t
> +      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: 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.
> +
> +  - DraftAvailableLensShadingMapModes:
> +      type: int32_t
> +      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: 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.
> +
> +  - DraftPipelineMaxDepth:
> +      type: int32_t
> +      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.
> +
>  ...
>
Jacopo Mondi Oct. 7, 2020, 2:40 p.m. UTC | #3
Hi Kieran,

On Wed, Oct 07, 2020 at 03:23:12PM +0100, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 11/09/2020 17: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 Android.
> >
>
> I had it in my head that these draft controls were going to be
> namespaced such that it was accessible through:
>
> switch(props.get(properties::draft::AvailableNoiseReductionModes) {
> 	case properties::draft::NOISE_REDUCTION_MODE_OFF:
> 	case properties::draft::NOISE_REDUCTION_MODE_FAST:
> 		// We don't do noise reduction
> 		break;
> 	case properties::draft::NOISE_REDUCTION_MODE_HIGH_QUALITY:
> DRAFT_NOISE_REDUCTION_MODE_MINIMAL
> }
>
> And that inserting (or removing) a control to/from that draft namespace
> would be about setting a property on the control definition like:
>
> +  - AvailableNoiseReductionModes:
> +      draft: true
> +      type: int32_t
> +      size: [n]
> +      description: |
>
> Or of course it could be:
> 	status: draft
> or
> 	status: staging
>
> But that's all bikeshed colours.
>
>
> Is the reason you've used a prefix (on every value) because that's the
> way you want it? or because you haven't looked at updating the control
> generation scripts to support the 'draft' property?
>
> If it's the latter, I'll tackle that now for you.
> (I'm going to have a quick go to see how hard it would be to implement).
>

I don't think it's worth as I suspect very few draft controls will be made
libcamera controls by just dropping 'draft' from their name.

There are Android defined controls, plain and simple. For each of them
there will be corresponding libcamera control.

Instrumenting the control generation script for this purpose seems a
bit an unjustified effort.

>
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/libcamera/property_ids.yaml | 70 +++++++++++++++++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >
> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > index 74ad0195d631..62bed9f3844d 100644
> > --- a/src/libcamera/property_ids.yaml
> > +++ b/src/libcamera/property_ids.yaml
> > @@ -640,4 +640,74 @@ controls:
> >          \todo Rename this property to ActiveAreas once we will have property
> >                categories (i.e. Properties::PixelArray::ActiveAreas)
> >
> > +  - DraftAvailableNoiseReductionModes:
> > +      type: int32_t
> > +      size: [n]
> > +      description: |
> > +        Draft property to report the list of supported noise reduction modes.
> > +        Currently identical to
> > +        ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES.
> > +      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.
>
> This implies that different streams can process noise reduction
> separately? but it's confusing because it's referencing ZeroShutterLag?
> So - I don't (yet) understand how that corresponds to the noise
> reduction. Maybe something for me to learn soon.

Not sure. Just imported those values from the Android metadata
description.

>
> Anyway, especially as these are 'DRAFT/::draft::' whatever - I don't
> think we need to dwell 'too' much on the detail, as the point of being a
> draft is to allow the plumbing to progress, and fix the detail before
> moving out of draft status.
>
>
> The other point I'd like to discuss here, is that you have used
> DRAFT_ALL_CAPS_ENUM style for all the enums, which matches the
> annonymous enum in
> include/libcamera/property_ids.h
> enum {
>         LOCATION = 1,
>         ROTATION = 2,
>         MODEL = 3,
>         UNIT_CELL_SIZE = 4,
>         PIXEL_ARRAY_SIZE = 5,
>         PIXEL_ARRAY_OPTICAL_BLACK_RECTANGLES = 6,
>         PIXEL_ARRAY_ACTIVE_AREAS = 7,
> };
>

These are controls/properties identifiers

>
> but differs to the LocationValues enum:
> enum LocationValues {
>         CameraLocationFront = 0,
>         CameraLocationBack = 1,
>         CameraLocationExternal = 2,
> };
>
> My personal preference would be to follow the LocationValues style, as
> it feels more C++/libcamery ;-)
>
> But - it's just an open question for the moment.

Again, just because Android named them in this way and staying
consistent with their definition has value to me.

>
>
>
> > +
> > +  - DraftAvailableColorCorrectionAberrationModes:
> > +      type: int32_t
> > +      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: 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.
> > +
> > +  - DraftAvailableLensShadingMapModes:
> > +      type: int32_t
> > +      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: 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.
> > +
> > +  - DraftPipelineMaxDepth:
> > +      type: int32_t
> > +      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.
> > +
> >  ...
> >
>
> --
> Regards
> --
> Kieran
Kieran Bingham Oct. 7, 2020, 3:11 p.m. UTC | #4
Hi Jacopo,

On 07/10/2020 15:40, Jacopo Mondi wrote:
> Hi Kieran,
> 
> On Wed, Oct 07, 2020 at 03:23:12PM +0100, Kieran Bingham wrote:
>> Hi Jacopo,
>>
>> On 11/09/2020 17: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 Android.
>>>
>>
>> I had it in my head that these draft controls were going to be
>> namespaced such that it was accessible through:
>>
>> switch(props.get(properties::draft::AvailableNoiseReductionModes) {
>> 	case properties::draft::NOISE_REDUCTION_MODE_OFF:
>> 	case properties::draft::NOISE_REDUCTION_MODE_FAST:
>> 		// We don't do noise reduction
>> 		break;
>> 	case properties::draft::NOISE_REDUCTION_MODE_HIGH_QUALITY:
>> DRAFT_NOISE_REDUCTION_MODE_MINIMAL
>> }
>>
>> And that inserting (or removing) a control to/from that draft namespace
>> would be about setting a property on the control definition like:
>>
>> +  - AvailableNoiseReductionModes:
>> +      draft: true
>> +      type: int32_t
>> +      size: [n]
>> +      description: |
>>
>> Or of course it could be:
>> 	status: draft
>> or
>> 	status: staging
>>
>> But that's all bikeshed colours.
>>
>>
>> Is the reason you've used a prefix (on every value) because that's the
>> way you want it? or because you haven't looked at updating the control
>> generation scripts to support the 'draft' property?
>>
>> If it's the latter, I'll tackle that now for you.
>> (I'm going to have a quick go to see how hard it would be to implement).
>>
> 
> I don't think it's worth as I suspect very few draft controls will be made
> libcamera controls by just dropping 'draft' from their name.

Well - 'just dropping the DRAFT_' wasn't quite the point ;-) The name
can always be changed at the same time as updating the draft status (or
it could be done in two stages as well, I don't care).

The point of the namespacing in my head was to kind of group, and
clearly mark the controls by nature of their usage.

The namespace really highlights that it's a 'property' of the name, and
not a part of the name.

(I'm getting extreme to try to highlight my angle of course)

 'DRAFT_EXCLUDE_RANGE'

The 'draft exclude' there could be interpreted as a single term (a draft
excluder).

Whereas:

 DRAFT::EXCLUDE_RANGE

Clearly defines the difference/separation ;-0

--
Kieran


> There are Android defined controls, plain and simple. For each of them
> there will be corresponding libcamera control.
> 
> Instrumenting the control generation script for this purpose seems a
> bit an unjustified effort.
> 
>>
>>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>>> ---
>>>  src/libcamera/property_ids.yaml | 70 +++++++++++++++++++++++++++++++++
>>>  1 file changed, 70 insertions(+)
>>>
>>> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
>>> index 74ad0195d631..62bed9f3844d 100644
>>> --- a/src/libcamera/property_ids.yaml
>>> +++ b/src/libcamera/property_ids.yaml
>>> @@ -640,4 +640,74 @@ controls:
>>>          \todo Rename this property to ActiveAreas once we will have property
>>>                categories (i.e. Properties::PixelArray::ActiveAreas)
>>>
>>> +  - DraftAvailableNoiseReductionModes:
>>> +      type: int32_t
>>> +      size: [n]
>>> +      description: |
>>> +        Draft property to report the list of supported noise reduction modes.
>>> +        Currently identical to
>>> +        ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES.
>>> +      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.
>>
>> This implies that different streams can process noise reduction
>> separately? but it's confusing because it's referencing ZeroShutterLag?
>> So - I don't (yet) understand how that corresponds to the noise
>> reduction. Maybe something for me to learn soon.
> 
> Not sure. Just imported those values from the Android metadata
> description.

Indeed, I think for now - it's not something I'm worried about.

> 
>>
>> Anyway, especially as these are 'DRAFT/::draft::' whatever - I don't
>> think we need to dwell 'too' much on the detail, as the point of being a
>> draft is to allow the plumbing to progress, and fix the detail before
>> moving out of draft status.
>>
>>
>> The other point I'd like to discuss here, is that you have used
>> DRAFT_ALL_CAPS_ENUM style for all the enums, which matches the
>> annonymous enum in
>> include/libcamera/property_ids.h
>> enum {
>>         LOCATION = 1,
>>         ROTATION = 2,
>>         MODEL = 3,
>>         UNIT_CELL_SIZE = 4,
>>         PIXEL_ARRAY_SIZE = 5,
>>         PIXEL_ARRAY_OPTICAL_BLACK_RECTANGLES = 6,
>>         PIXEL_ARRAY_ACTIVE_AREAS = 7,
>> };
>>
> 
> These are controls/properties identifiers

Aha - I see - those are the actual enum values that get assigned to the
properties! I see now ;-)

> 
>>
>> but differs to the LocationValues enum:
>> enum LocationValues {
>>         CameraLocationFront = 0,
>>         CameraLocationBack = 1,
>>         CameraLocationExternal = 2,
>> };
>>
>> My personal preference would be to follow the LocationValues style, as
>> it feels more C++/libcamery ;-)
>>
>> But - it's just an open question for the moment.
> 
> Again, just because Android named them in this way and staying
> consistent with their definition has value to me.
> 

I think we need to be consistent in the way we style these properties in
our namespace though ...


>>> +  - DraftAvailableColorCorrectionAberrationModes:
>>> +      type: int32_t
>>> +      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: 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.
>>> +
>>> +  - DraftAvailableLensShadingMapModes:
>>> +      type: int32_t
>>> +      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: 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.
>>> +
>>> +  - DraftPipelineMaxDepth:
>>> +      type: int32_t
>>> +      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.
>>> +
>>>  ...
>>>
>>
>> --
>> Regards
>> --
>> Kieran

Patch

diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
index 74ad0195d631..62bed9f3844d 100644
--- a/src/libcamera/property_ids.yaml
+++ b/src/libcamera/property_ids.yaml
@@ -640,4 +640,74 @@  controls:
         \todo Rename this property to ActiveAreas once we will have property
               categories (i.e. Properties::PixelArray::ActiveAreas)
 
+  - DraftAvailableNoiseReductionModes:
+      type: int32_t
+      size: [n]
+      description: |
+        Draft property to report the list of supported noise reduction modes.
+        Currently identical to
+        ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES.
+      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.
+
+  - DraftAvailableColorCorrectionAberrationModes:
+      type: int32_t
+      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: 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.
+
+  - DraftAvailableLensShadingMapModes:
+      type: int32_t
+      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: 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.
+
+  - DraftPipelineMaxDepth:
+      type: int32_t
+      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.
+
 ...