[libcamera-devel,v2,2/9] libcamera: properties: ColorFilterArrangement draft property
diff mbox series

Message ID 20201218164754.81422-3-jacopo@jmondi.org
State Accepted
Delegated to: Jacopo Mondi
Headers show
Series
  • libcamera: Introduce sensor database
Related show

Commit Message

Jacopo Mondi Dec. 18, 2020, 4:47 p.m. UTC 
Define the 'ColorFilterArrangement' draft property. The property is
currently identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---

This patch introdues the following warnings
property_ids.cpp:716: warning: no matching class member found for
  libcamera::properties::draft::ColorFilterArrangementEnum::RGGB
property_ids.cpp:719: warning: no matching class member found for
  libcamera::properties::draft::ColorFilterArrangementEnum::GRBG
property_ids.cpp:722: warning: no matching class member found for
  libcamera::properties::draft::ColorFilterArrangementEnum::GBRG
property_ids.cpp:725: warning: no matching class member found for
  libcamera::properties::draft::ColorFilterArrangementEnum::BGGR

Removing one letter from the identifiers, or adding one, makes the
warning disapper:

--
2.29.2

Comments

Laurent Pinchart Dec. 21, 2020, 3:44 p.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Fri, Dec 18, 2020 at 05:47:47PM +0100, Jacopo Mondi wrote:
> Define the 'ColorFilterArrangement' draft property. The property is
> currently identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
> 
> This patch introdues the following warnings
> property_ids.cpp:716: warning: no matching class member found for
>   libcamera::properties::draft::ColorFilterArrangementEnum::RGGB
> property_ids.cpp:719: warning: no matching class member found for
>   libcamera::properties::draft::ColorFilterArrangementEnum::GRBG
> property_ids.cpp:722: warning: no matching class member found for
>   libcamera::properties::draft::ColorFilterArrangementEnum::GBRG
> property_ids.cpp:725: warning: no matching class member found for
>   libcamera::properties::draft::ColorFilterArrangementEnum::BGGR
> 
> Removing one letter from the identifiers, or adding one, makes the
> warning disapper:
> 
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 411d5b2a398d..8a19b989e6c4 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -697,16 +697,16 @@ controls:
>          top-left 2x2 section of the sensor, in reading order. Currently
>          identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.
>        enum:
> -        - name: RGGB
> +        - name: RGGBZ
>            value: 0
>            description: RGGB color filter arrangement
> -        - name: GRBG
> +        - name: GRBGZ
>            value: 1
>            description: GRBG color filter arrangement
> -        - name: GBRG
> +        - name: GBRGZ
>            value: 2
>            description: GBRG color filter arrangement
> -        - name: BGGR
> +        - name: BGGRZ
>            value: 3
>            description: BGGR color filter arrangement
>          - name: RGB
> 
> A Doxygen bug maybe ?

I think so. The problem still occurs with the master branch. It's
probably related to https://github.com/doxygen/doxygen/issues/6471,
which I've updated with a test case for this specific issue.

> ---
>  src/libcamera/property_ids.yaml | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index f8e32c1bd8cf..411d5b2a398d 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -689,4 +689,30 @@ controls:
>          The physical dimensions of the full pixel array in millimeters.
>          Currently identical to ANDROID_SENSOR_INFO_PHYSICAL_SIZE.
> 
> +  - 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
> +        - 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.
> +
>  ...

This one should be fairly easy to "undraft", that's nice :-)

Given that we'll rework the control when promoting it to non-draft
status, should we prefix the enumerators to avoid the doxygen warning
for the time being ? For instance s/RGGB/PatternRGGB/.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Jacopo Mondi Dec. 21, 2020, 4:09 p.m. UTC | #2 
Hi Laurent,

On Mon, Dec 21, 2020 at 05:44:11PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Fri, Dec 18, 2020 at 05:47:47PM +0100, Jacopo Mondi wrote:
> > Define the 'ColorFilterArrangement' draft property. The property is
> > currently identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >
> > This patch introdues the following warnings
> > property_ids.cpp:716: warning: no matching class member found for
> >   libcamera::properties::draft::ColorFilterArrangementEnum::RGGB
> > property_ids.cpp:719: warning: no matching class member found for
> >   libcamera::properties::draft::ColorFilterArrangementEnum::GRBG
> > property_ids.cpp:722: warning: no matching class member found for
> >   libcamera::properties::draft::ColorFilterArrangementEnum::GBRG
> > property_ids.cpp:725: warning: no matching class member found for
> >   libcamera::properties::draft::ColorFilterArrangementEnum::BGGR
> >
> > Removing one letter from the identifiers, or adding one, makes the
> > warning disapper:
> >
> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > index 411d5b2a398d..8a19b989e6c4 100644
> > --- a/src/libcamera/property_ids.yaml
> > +++ b/src/libcamera/property_ids.yaml
> > @@ -697,16 +697,16 @@ controls:
> >          top-left 2x2 section of the sensor, in reading order. Currently
> >          identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.
> >        enum:
> > -        - name: RGGB
> > +        - name: RGGBZ
> >            value: 0
> >            description: RGGB color filter arrangement
> > -        - name: GRBG
> > +        - name: GRBGZ
> >            value: 1
> >            description: GRBG color filter arrangement
> > -        - name: GBRG
> > +        - name: GBRGZ
> >            value: 2
> >            description: GBRG color filter arrangement
> > -        - name: BGGR
> > +        - name: BGGRZ
> >            value: 3
> >            description: BGGR color filter arrangement
> >          - name: RGB
> >
> > A Doxygen bug maybe ?
>
> I think so. The problem still occurs with the master branch. It's
> probably related to https://github.com/doxygen/doxygen/issues/6471,
> which I've updated with a test case for this specific issue.
>
> > ---
> >  src/libcamera/property_ids.yaml | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > index f8e32c1bd8cf..411d5b2a398d 100644
> > --- a/src/libcamera/property_ids.yaml
> > +++ b/src/libcamera/property_ids.yaml
> > @@ -689,4 +689,30 @@ controls:
> >          The physical dimensions of the full pixel array in millimeters.
> >          Currently identical to ANDROID_SENSOR_INFO_PHYSICAL_SIZE.
> >
> > +  - 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
> > +        - 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.
> > +
> >  ...
>
> This one should be fairly easy to "undraft", that's nice :-)
>
> Given that we'll rework the control when promoting it to non-draft
> status, should we prefix the enumerators to avoid the doxygen warning
> for the time being ? For instance s/RGGB/PatternRGGB/.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I'll add a \todo to record that we're working around a doxygen bug

>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
index 411d5b2a398d..8a19b989e6c4 100644
--- a/src/libcamera/property_ids.yaml
+++ b/src/libcamera/property_ids.yaml
@@ -697,16 +697,16 @@  controls:
         top-left 2x2 section of the sensor, in reading order. Currently
         identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.
       enum:
-        - name: RGGB
+        - name: RGGBZ
           value: 0
           description: RGGB color filter arrangement
-        - name: GRBG
+        - name: GRBGZ
           value: 1
           description: GRBG color filter arrangement
-        - name: GBRG
+        - name: GBRGZ
           value: 2
           description: GBRG color filter arrangement
-        - name: BGGR
+        - name: BGGRZ
           value: 3
           description: BGGR color filter arrangement
         - name: RGB

A Doxygen bug maybe ?

---
 src/libcamera/property_ids.yaml | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
index f8e32c1bd8cf..411d5b2a398d 100644
--- a/src/libcamera/property_ids.yaml
+++ b/src/libcamera/property_ids.yaml
@@ -689,4 +689,30 @@  controls:
         The physical dimensions of the full pixel array in millimeters.
         Currently identical to ANDROID_SENSOR_INFO_PHYSICAL_SIZE.

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