[libcamera-devel] [PATCH v2 2/9] libcamera: properties: ColorFilterArrangement draft property

[Jacopo Mondi]

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 at 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 at ideasonboard.com>

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

>
> --
> Regards,
>
> Laurent Pinchart