[libcamera-devel,2/6] libcamera: properties: ColorFilterArrangement draft property

10703 diff mbox series

Define the 'ColorFilterArrangement' draft property. The property is
currently identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/property_ids.yaml | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

Hi Jacopo,

Thanks for your work.

On 2020-12-23 18:47:05 +0100, Jacopo Mondi wrote:
> Define the 'ColorFilterArrangement' draft property. The property is
> currently identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/property_ids.yaml | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 64e88f0361d6..7f0e9c87a408 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -678,4 +678,33 @@ controls:
>          \todo Turn this property into a "maximum control value" for the
>          ScalerCrop control once "dynamic" controls have been implemented.
>  
> +  # ----------------------------------------------------------------------------
> +  # Draft properties section
> +
> +  - ColorFilterArrangement:
> +      type: int32_t
> +      draft: true
> +      description: |
> +        The arrangement of color filters on sensor; represents the colors in the
> +        top-left 2x2 section of the sensor, in reading order. Currently
> +        identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.
> +      enum:
> +        - name: RGGB
> +          value: 0
> +          description: RGGB color filter arrangement
> +        - name: GRBG
> +          value: 1
> +          description: GRBG color filter arrangement
> +        - name: GBRG
> +          value: 2
> +          description: GBRG color filter arrangement
> +        - name: BGGR
> +          value: 3
> +          description: BGGR color filter arrangement

How about mentioning that these are Bayer patterns? Something like,

    Sensor outputs a Bayer patter in BGGR color filter arrangement.

> +        - name: RGB
> +          value: 4
> +          description: |
> +            Sensor is not Bayer; output has 3 16-bit values for each pixel,
> +            instead of just 1 16-bit value per pixel.

Should we mention the sample size here? Something like,

    Sensor outputs 3 samples for each pixel, one for each RGB color 
    component.

> +
>  ...
> -- 
> 2.29.2
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Niklas,

I've lost your email, so I'll reply in-line here copying your
questions

On Wed, Dec 23, 2020 at 06:47:05PM +0100, Jacopo Mondi wrote:
> Define the 'ColorFilterArrangement' draft property. The property is
> currently identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/property_ids.yaml | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 64e88f0361d6..7f0e9c87a408 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -678,4 +678,33 @@ controls:
>          \todo Turn this property into a "maximum control value" for the
>          ScalerCrop control once "dynamic" controls have been implemented.
>
> +  # ----------------------------------------------------------------------------
> +  # Draft properties section
> +
> +  - ColorFilterArrangement:
> +      type: int32_t
> +      draft: true
> +      description: |
> +        The arrangement of color filters on sensor; represents the colors in the
> +        top-left 2x2 section of the sensor, in reading order. Currently
> +        identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.
> +      enum:
> +        - name: RGGB
> +          value: 0
> +          description: RGGB color filter arrangement

Niklas:
How about mentioning that these are Bayer patterns? Something like,
    Sensor outputs a Bayer patter in BGGR color filter arrangement.

Me:
To be honest I found the concept of "output a bayer pattern in some
filter arrangement" not very precise. The filters arrangement is a
sensor construction property, which defines how the samples are then
output. What about just "RGGB Bayer pattern" ?

> +        - name: GRBG
> +          value: 1
> +          description: GRBG color filter arrangement
> +        - name: GBRG
> +          value: 2
> +          description: GBRG color filter arrangement
> +        - name: BGGR
> +          value: 3
> +          description: BGGR color filter arrangement
> +        - name: RGB
> +          value: 4
> +          description: |
> +            Sensor is not Bayer; output has 3 16-bit values for each pixel,
> +            instead of just 1 16-bit value per pixel.

Niklas:
Should we mention the sample size here? Something like,
    Sensor outputs 3 samples for each pixel, one for each RGB color
    component.

This comes straight from Android definition, to which this property is
currently identical. I would keep it as it is and decide what to do
when we undraft it.

Thanks
  j

> +


>  ...
> --
> 2.29.2
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel