[libcamera-devel] [PATCH 1/2] libcamera: formats: Fix colour encoding for "R" raw greyscale formats

[David Plowman]

Hi Laurent

Thanks for the feedback.

On Thu, 4 Aug 2022 at 19:25, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> On Thu, Aug 04, 2022 at 11:45:49AM +0100, David Plowman via libcamera-devel wrote:
> > These are being used for raw monochrome sensors and so the colour
> > encoding should be "raw".
>
> Ouch. This is something I didn't foresee, but we have a problem here.
> These can refer to both raw data from monochrome sensors, but also
> greyscale data processed by an ISP. We currently don't consider as RAW
> formats, which means that
>
> - The CameraConfiguration::validateColorSpaces() will accept other color
>   spaces that ColorSpace::Raw.
> - The IPU3 and Raspberry Pi pipeline handlers, and the Android HAL, will
>   consider them as processed streams, not raw streams.
>
> This patch will change that, quite likely introducing breakages.
>
> Now, the interesting question is how to support both raw monochrome data
> and processed monochrome data. Any idea ? :-)

Hmm. That's all a bit awkward. Am I right in thinking that this comes
fundamentally from the V4L2 drivers which tell us a format (e.g.
"10-bit greyscale") but not whether it's raw or processed? Which
presumably makes it a bit awkward to fix "properly".

Alternatively, I suppose we could add (slightly ugly) workarounds in
the Pi pipeline handler, along the lines of "oh, it says R10, I know
that's really a raw format", because that's true for us. I don't
really think that I'm seeing anything better than this at the
moment...

Thanks!

David

>
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  src/libcamera/formats.cpp | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> > index 283ecb3d..7b98fef2 100644
> > --- a/src/libcamera/formats.cpp
> > +++ b/src/libcamera/formats.cpp
> > @@ -531,7 +531,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> >                       .multi = V4L2PixelFormat(),
> >               },
> >               .bitsPerPixel = 8,
> > -             .colourEncoding = PixelFormatInfo::ColourEncodingYUV,
> > +             .colourEncoding = PixelFormatInfo::ColourEncodingRAW,
> >               .packed = false,
> >               .pixelsPerGroup = 1,
> >               .planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }},
> > @@ -544,7 +544,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> >                       .multi = V4L2PixelFormat(),
> >               },
> >               .bitsPerPixel = 10,
> > -             .colourEncoding = PixelFormatInfo::ColourEncodingYUV,
> > +             .colourEncoding = PixelFormatInfo::ColourEncodingRAW,
> >               .packed = false,
> >               .pixelsPerGroup = 1,
> >               .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> > @@ -557,7 +557,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> >                       .multi = V4L2PixelFormat(),
> >               },
> >               .bitsPerPixel = 12,
> > -             .colourEncoding = PixelFormatInfo::ColourEncodingYUV,
> > +             .colourEncoding = PixelFormatInfo::ColourEncodingRAW,
> >               .packed = false,
> >               .pixelsPerGroup = 1,
> >               .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> > @@ -570,7 +570,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> >                       .multi = V4L2PixelFormat(),
> >               },
> >               .bitsPerPixel = 10,
> > -             .colourEncoding = PixelFormatInfo::ColourEncodingYUV,
> > +             .colourEncoding = PixelFormatInfo::ColourEncodingRAW,
> >               .packed = true,
> >               .pixelsPerGroup = 4,
> >               .planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},
>
> --
> Regards,
>
> Laurent Pinchart