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

Message ID 20201228164003.53051-2-jacopo@jmondi.org
State Accepted
Commit 5d3d0dcedb36f991b7f395c15b9112694f44d87d
Delegated to: Jacopo Mondi
Headers show
Series
  • android: support ColorFilterArrangement property
Related show

Commit Message

Jacopo Mondi Dec. 28, 2020, 4:39 p.m. UTC 
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(+)

Comments

Paul Elder Dec. 29, 2020, 4:56 a.m. UTC | #1 
Hi Jacopo,

On Mon, Dec 28, 2020 at 05:39:59PM +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>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  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..104e9aaf4fa3 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 Bayer pattern
> +        - name: GRBG
> +          value: 1
> +          description: GRBG Bayer pattern
> +        - name: GBRG
> +          value: 2
> +          description: GBRG Bayer pattern
> +        - name: BGGR
> +          value: 3
> +          description: BGGR Bayer pattern
> +        - 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.
> +
>  ...
> -- 
> 2.29.2
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
David Plowman Dec. 29, 2020, 9:25 a.m. UTC | #2 
Hi Jacopo

Thanks for this patch! Just one little question - I suppose Android
(and libcamera) have some other way of signalling a monochrome
sensor... I think I might have expected to find a
"monochrome/non-Bayer" option in the list?

Best regards
David

On Tue, 29 Dec 2020 at 04:56, <paul.elder@ideasonboard.com> wrote:
>
> Hi Jacopo,
>
> On Mon, Dec 28, 2020 at 05:39:59PM +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>
>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
>
> > ---
> >  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..104e9aaf4fa3 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 Bayer pattern
> > +        - name: GRBG
> > +          value: 1
> > +          description: GRBG Bayer pattern
> > +        - name: GBRG
> > +          value: 2
> > +          description: GBRG Bayer pattern
> > +        - name: BGGR
> > +          value: 3
> > +          description: BGGR Bayer pattern
> > +        - 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.
> > +
> >  ...
> > --
> > 2.29.2
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi Dec. 29, 2020, 10:26 a.m. UTC | #3 
Hi David,

On Tue, Dec 29, 2020 at 09:25:14AM +0000, David Plowman wrote:
> Hi Jacopo
>
> Thanks for this patch! Just one little question - I suppose Android
> (and libcamera) have some other way of signalling a monochrome
> sensor... I think I might have expected to find a
> "monochrome/non-Bayer" option in the list?

Please bare in mind this is a draft property, whose main purpose is
to temporary close the gap with Android and its definition come
straight from there.

This property only makes sensor for RAW sensors, although there's a
more generic 'RGB' value that sounds like a catch-all for non-Bayer
sensors.

Simply put: non-RAW sensor won't register this property at all.
When we'll un-draft the property we can discuss if that makes sense
at all as a choice, or we might want to have a more generic
definition.

Do you perhaps have any usage for such a more generic property in mind ?

>
> Best regards
> David
>
> On Tue, 29 Dec 2020 at 04:56, <paul.elder@ideasonboard.com> wrote:
> >
> > Hi Jacopo,
> >
> > On Mon, Dec 28, 2020 at 05:39:59PM +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>
> >
> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> >
> > > ---
> > >  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..104e9aaf4fa3 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 Bayer pattern
> > > +        - name: GRBG
> > > +          value: 1
> > > +          description: GRBG Bayer pattern
> > > +        - name: GBRG
> > > +          value: 2
> > > +          description: GBRG Bayer pattern
> > > +        - name: BGGR
> > > +          value: 3
> > > +          description: BGGR Bayer pattern
> > > +        - 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.
> > > +
> > >  ...
> > > --
> > > 2.29.2
> > >
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel@lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
David Plowman Dec. 29, 2020, 10:32 p.m. UTC | #4 
Hi Jacopo

Thanks for the reply!

On Tue, 29 Dec 2020 at 10:26, Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi David,
>
> On Tue, Dec 29, 2020 at 09:25:14AM +0000, David Plowman wrote:
> > Hi Jacopo
> >
> > Thanks for this patch! Just one little question - I suppose Android
> > (and libcamera) have some other way of signalling a monochrome
> > sensor... I think I might have expected to find a
> > "monochrome/non-Bayer" option in the list?
>
> Please bare in mind this is a draft property, whose main purpose is
> to temporary close the gap with Android and its definition come
> straight from there.
>
> This property only makes sensor for RAW sensors, although there's a
> more generic 'RGB' value that sounds like a catch-all for non-Bayer
> sensors.
>
> Simply put: non-RAW sensor won't register this property at all.
> When we'll un-draft the property we can discuss if that makes sense
> at all as a choice, or we might want to have a more generic
> definition.
>
> Do you perhaps have any usage for such a more generic property in mind ?

Not especially, my thoughts on the subject are only "draft" too!  :)

I guess my only observations are that a monochrome ("no CFA at all")
format would probably be useful (it's still a RAW format, though not a
Bayer one). Also, that single non-RAW format (3x16-bit RGB) seems a
bit lonely, for example YUV422 interleaved is probably more common,
but I doubt we want to list all the possible pixel formats all over
again, we have enough of that already!

Thanks!
David

>
> >
> > Best regards
> > David
> >
> > On Tue, 29 Dec 2020 at 04:56, <paul.elder@ideasonboard.com> wrote:
> > >
> > > Hi Jacopo,
> > >
> > > On Mon, Dec 28, 2020 at 05:39:59PM +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>
> > >
> > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> > >
> > > > ---
> > > >  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..104e9aaf4fa3 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 Bayer pattern
> > > > +        - name: GRBG
> > > > +          value: 1
> > > > +          description: GRBG Bayer pattern
> > > > +        - name: GBRG
> > > > +          value: 2
> > > > +          description: GBRG Bayer pattern
> > > > +        - name: BGGR
> > > > +          value: 3
> > > > +          description: BGGR Bayer pattern
> > > > +        - 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.
> > > > +
> > > >  ...
> > > > --
> > > > 2.29.2
> > > >
> > > > _______________________________________________
> > > > libcamera-devel mailing list
> > > > libcamera-devel@lists.libcamera.org
> > > > https://lists.libcamera.org/listinfo/libcamera-devel
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel@lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi Dec. 30, 2020, 7:59 a.m. UTC | #5 
Hi David,

On Tue, Dec 29, 2020 at 10:32:21PM +0000, David Plowman wrote:
> Hi Jacopo
>
> Thanks for the reply!
>
> On Tue, 29 Dec 2020 at 10:26, Jacopo Mondi <jacopo@jmondi.org> wrote:
> >
> > Hi David,
> >
> > On Tue, Dec 29, 2020 at 09:25:14AM +0000, David Plowman wrote:
> > > Hi Jacopo
> > >
> > > Thanks for this patch! Just one little question - I suppose Android
> > > (and libcamera) have some other way of signalling a monochrome
> > > sensor... I think I might have expected to find a
> > > "monochrome/non-Bayer" option in the list?
> >
> > Please bare in mind this is a draft property, whose main purpose is
> > to temporary close the gap with Android and its definition come
> > straight from there.
> >
> > This property only makes sensor for RAW sensors, although there's a
> > more generic 'RGB' value that sounds like a catch-all for non-Bayer
> > sensors.
> >
> > Simply put: non-RAW sensor won't register this property at all.
> > When we'll un-draft the property we can discuss if that makes sense
> > at all as a choice, or we might want to have a more generic
> > definition.
> >
> > Do you perhaps have any usage for such a more generic property in mind ?
>
> Not especially, my thoughts on the subject are only "draft" too!  :)
>
> I guess my only observations are that a monochrome ("no CFA at all")
> format would probably be useful (it's still a RAW format, though not a

My bad, I always thought 'mono-chrome' as a YUV sensor with only the Y
channel being sent out.

> Bayer one). Also, that single non-RAW format (3x16-bit RGB) seems a
> bit lonely, for example YUV422 interleaved is probably more common,
> but I doubt we want to list all the possible pixel formats all over
> again, we have enough of that already!

I'm not sure I follow. How does a processed format like YUV422 relates
to the sensor's color filters arrangements ?

Thanks
   j

>
> Thanks!
> David
>
> >
> > >
> > > Best regards
> > > David
> > >
> > > On Tue, 29 Dec 2020 at 04:56, <paul.elder@ideasonboard.com> wrote:
> > > >
> > > > Hi Jacopo,
> > > >
> > > > On Mon, Dec 28, 2020 at 05:39:59PM +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>
> > > >
> > > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> > > >
> > > > > ---
> > > > >  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..104e9aaf4fa3 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 Bayer pattern
> > > > > +        - name: GRBG
> > > > > +          value: 1
> > > > > +          description: GRBG Bayer pattern
> > > > > +        - name: GBRG
> > > > > +          value: 2
> > > > > +          description: GBRG Bayer pattern
> > > > > +        - name: BGGR
> > > > > +          value: 3
> > > > > +          description: BGGR Bayer pattern
> > > > > +        - 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.
> > > > > +
> > > > >  ...
> > > > > --
> > > > > 2.29.2
> > > > >
> > > > > _______________________________________________
> > > > > libcamera-devel mailing list
> > > > > libcamera-devel@lists.libcamera.org
> > > > > https://lists.libcamera.org/listinfo/libcamera-devel
> > > > _______________________________________________
> > > > libcamera-devel mailing list
> > > > libcamera-devel@lists.libcamera.org
> > > > https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi Dec. 30, 2020, 9:38 a.m. UTC | #6 
Hi David,

On Wed, Dec 30, 2020 at 08:59:31AM +0100, Jacopo Mondi wrote:
> Hi David,
>
> On Tue, Dec 29, 2020 at 10:32:21PM +0000, David Plowman wrote:
> > Hi Jacopo
> >
> > Thanks for the reply!
> >
> > On Tue, 29 Dec 2020 at 10:26, Jacopo Mondi <jacopo@jmondi.org> wrote:
> > >
> > > Hi David,
> > >
> > > On Tue, Dec 29, 2020 at 09:25:14AM +0000, David Plowman wrote:
> > > > Hi Jacopo
> > > >
> > > > Thanks for this patch! Just one little question - I suppose Android
> > > > (and libcamera) have some other way of signalling a monochrome
> > > > sensor... I think I might have expected to find a
> > > > "monochrome/non-Bayer" option in the list?
> > >
> > > Please bare in mind this is a draft property, whose main purpose is
> > > to temporary close the gap with Android and its definition come
> > > straight from there.
> > >
> > > This property only makes sensor for RAW sensors, although there's a
> > > more generic 'RGB' value that sounds like a catch-all for non-Bayer
> > > sensors.
> > >
> > > Simply put: non-RAW sensor won't register this property at all.
> > > When we'll un-draft the property we can discuss if that makes sense
> > > at all as a choice, or we might want to have a more generic
> > > definition.
> > >
> > > Do you perhaps have any usage for such a more generic property in mind ?
> >
> > Not especially, my thoughts on the subject are only "draft" too!  :)
> >
> > I guess my only observations are that a monochrome ("no CFA at all")
> > format would probably be useful (it's still a RAW format, though not a
>
> My bad, I always thought 'mono-chrome' as a YUV sensor with only the Y
> channel being sent out.
>
> > Bayer one). Also, that single non-RAW format (3x16-bit RGB) seems a
> > bit lonely, for example YUV422 interleaved is probably more common,
> > but I doubt we want to list all the possible pixel formats all over
> > again, we have enough of that already!
>
> I'm not sure I follow. How does a processed format like YUV422 relates
> to the sensor's color filters arrangements ?

Laurent helped clarify this to me, and yes, we probably need a
definition for monochrome going forward. I would however keep the draft
property version as close as possible to Android's definition and
expand it when un-drafting.

Thanks
  j
David Plowman Dec. 30, 2020, 10 a.m. UTC | #7 
Hi Jacopo

Thanks for that. Actually I think I was a bit confused too. I was
thinking that the 3x16-bit was somehow supposed to include non-RAW
sensors, but I guess it doesn't. Maybe Android invented it to account
for those Foveon sensors that we had a few years back, which did give
you R, G and B at each location, but were still RAW?

But anyway, I agree it will probably make sense to add a "no CFA"
option for those monochrome RAW sensors at some point. In fact, I
wonder if in future we may want to add that to our BayerFormat
class... at which point "BayerFormat" isn't necessarily the right name
any more. Oh dear! Anyway, that's a question for another time...

Best regards
David

On Wed, 30 Dec 2020 at 09:38, Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi David,
>
> On Wed, Dec 30, 2020 at 08:59:31AM +0100, Jacopo Mondi wrote:
> > Hi David,
> >
> > On Tue, Dec 29, 2020 at 10:32:21PM +0000, David Plowman wrote:
> > > Hi Jacopo
> > >
> > > Thanks for the reply!
> > >
> > > On Tue, 29 Dec 2020 at 10:26, Jacopo Mondi <jacopo@jmondi.org> wrote:
> > > >
> > > > Hi David,
> > > >
> > > > On Tue, Dec 29, 2020 at 09:25:14AM +0000, David Plowman wrote:
> > > > > Hi Jacopo
> > > > >
> > > > > Thanks for this patch! Just one little question - I suppose Android
> > > > > (and libcamera) have some other way of signalling a monochrome
> > > > > sensor... I think I might have expected to find a
> > > > > "monochrome/non-Bayer" option in the list?
> > > >
> > > > Please bare in mind this is a draft property, whose main purpose is
> > > > to temporary close the gap with Android and its definition come
> > > > straight from there.
> > > >
> > > > This property only makes sensor for RAW sensors, although there's a
> > > > more generic 'RGB' value that sounds like a catch-all for non-Bayer
> > > > sensors.
> > > >
> > > > Simply put: non-RAW sensor won't register this property at all.
> > > > When we'll un-draft the property we can discuss if that makes sense
> > > > at all as a choice, or we might want to have a more generic
> > > > definition.
> > > >
> > > > Do you perhaps have any usage for such a more generic property in mind ?
> > >
> > > Not especially, my thoughts on the subject are only "draft" too!  :)
> > >
> > > I guess my only observations are that a monochrome ("no CFA at all")
> > > format would probably be useful (it's still a RAW format, though not a
> >
> > My bad, I always thought 'mono-chrome' as a YUV sensor with only the Y
> > channel being sent out.
> >
> > > Bayer one). Also, that single non-RAW format (3x16-bit RGB) seems a
> > > bit lonely, for example YUV422 interleaved is probably more common,
> > > but I doubt we want to list all the possible pixel formats all over
> > > again, we have enough of that already!
> >
> > I'm not sure I follow. How does a processed format like YUV422 relates
> > to the sensor's color filters arrangements ?
>
> Laurent helped clarify this to me, and yes, we probably need a
> definition for monochrome going forward. I would however keep the draft
> property version as close as possible to Android's definition and
> expand it when un-drafting.
>
> Thanks
>   j
Laurent Pinchart Dec. 30, 2020, 12:50 p.m. UTC | #8 
Hi David,

On Wed, Dec 30, 2020 at 10:00:07AM +0000, David Plowman wrote:
> Hi Jacopo
> 
> Thanks for that. Actually I think I was a bit confused too. I was
> thinking that the 3x16-bit was somehow supposed to include non-RAW
> sensors, but I guess it doesn't. Maybe Android invented it to account
> for those Foveon sensors that we had a few years back, which did give
> you R, G and B at each location, but were still RAW?
> 
> But anyway, I agree it will probably make sense to add a "no CFA"
> option for those monochrome RAW sensors at some point. In fact, I
> wonder if in future we may want to add that to our BayerFormat
> class... at which point "BayerFormat" isn't necessarily the right name
> any more. Oh dear! Anyway, that's a question for another time...

ColorFilterArray may be a better name. It depends on what the
BayerFormat class ends up being used for. If it's just a helper to
transpose the component ordering, it shouldn't be too much of an issue.
Android, in its android.sensor.info.colorFilterArrangement property,
differentiates monochrome from NIR. There's a slippery slope toards
exposing sensor frequency response information, which likely doesn't
belong to whatever BayerFormat will end up being.

In any case, it's a topic for later indeed.

> On Wed, 30 Dec 2020 at 09:38, Jacopo Mondi <jacopo@jmondi.org> wrote:
> > On Wed, Dec 30, 2020 at 08:59:31AM +0100, Jacopo Mondi wrote:
> > > On Tue, Dec 29, 2020 at 10:32:21PM +0000, David Plowman wrote:
> > > > On Tue, 29 Dec 2020 at 10:26, Jacopo Mondi <jacopo@jmondi.org> wrote:
> > > > > On Tue, Dec 29, 2020 at 09:25:14AM +0000, David Plowman wrote:
> > > > > > Hi Jacopo
> > > > > >
> > > > > > Thanks for this patch! Just one little question - I suppose Android
> > > > > > (and libcamera) have some other way of signalling a monochrome
> > > > > > sensor... I think I might have expected to find a
> > > > > > "monochrome/non-Bayer" option in the list?
> > > > >
> > > > > Please bare in mind this is a draft property, whose main purpose is
> > > > > to temporary close the gap with Android and its definition come
> > > > > straight from there.
> > > > >
> > > > > This property only makes sensor for RAW sensors, although there's a
> > > > > more generic 'RGB' value that sounds like a catch-all for non-Bayer
> > > > > sensors.
> > > > >
> > > > > Simply put: non-RAW sensor won't register this property at all.
> > > > > When we'll un-draft the property we can discuss if that makes sense
> > > > > at all as a choice, or we might want to have a more generic
> > > > > definition.
> > > > >
> > > > > Do you perhaps have any usage for such a more generic property in mind ?
> > > >
> > > > Not especially, my thoughts on the subject are only "draft" too!  :)
> > > >
> > > > I guess my only observations are that a monochrome ("no CFA at all")
> > > > format would probably be useful (it's still a RAW format, though not a
> > >
> > > My bad, I always thought 'mono-chrome' as a YUV sensor with only the Y
> > > channel being sent out.
> > >
> > > > Bayer one). Also, that single non-RAW format (3x16-bit RGB) seems a
> > > > bit lonely, for example YUV422 interleaved is probably more common,
> > > > but I doubt we want to list all the possible pixel formats all over
> > > > again, we have enough of that already!
> > >
> > > I'm not sure I follow. How does a processed format like YUV422 relates
> > > to the sensor's color filters arrangements ?
> >
> > Laurent helped clarify this to me, and yes, we probably need a
> > definition for monochrome going forward. I would however keep the draft
> > property version as close as possible to Android's definition and
> > expand it when un-drafting.

Patch
diff mbox series 

diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
index 64e88f0361d6..104e9aaf4fa3 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 Bayer pattern
+        - name: GRBG
+          value: 1
+          description: GRBG Bayer pattern
+        - name: GBRG
+          value: 2
+          description: GBRG Bayer pattern
+        - name: BGGR
+          value: 3
+          description: BGGR Bayer pattern
+        - 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.
+
 ...