[libcamera-devel] Colour space adjustment

[Laurent Pinchart]

Hello,

On Mon, Sep 05, 2022 at 01:37:00PM +0100, Naushir Patuck wrote:
> Hi,
> 
> On a related note, we are also getting this warning on the Raspberry Pi
> platform with commit e297673e7686:
> 
> WARN V4L2 v4l2_subdevice.cpp:512 'imx477 10-001a': Unknown subdev format
> 0x7002, defaulting to RGB encoding
> 
> This is because the embedded buffer stream format (i.e. a non-image data
> format) does not map to a libcamera PixelFormat* at present.  In the past,
> we've had similar issues and decided not to use WARN level to flag this.
> Can we do the same here please?

Absolutely. I recall a discussion about this, and thought we had solved
the problem already, but it seems I'm wrong.

> Not directly related to the logging message, but I suppose the same
> question should also be asked here - should we be overwriting the
> colorEncoding in these circumstances?

It depends what you mean by "these cirscumstances". For metadata formats
colorspace doesn't make sense, so we should set the colorSpace field to
nullopt without any warning. For unknown media bus codes in general,
that's different, as the simple pipeline handler can propagate them
through the pipeline in a generic way without caring what they are
exactly.

The imx477 driver seems to always report V4L2_COLORSPACE_DEFAULT for the
colorspace field. I think setting colorSpace to nullopt in that case
would be the right thing to do. I'll prepare a patch and will CC you.

> On Mon, 5 Sept 2022 at 13:25, David Plowman via libcamera-devel wrote:
> 
> > Hi everyone
> >
> > We've recently checked out the latest version of libcamera and are having
> > a few colour space issues. Obviously I'm sorry to be raising this at a
> > stage where the commits have already been applied, but maybe we can think
> > of some kind of acceptable workaround.

Or even an acceptable good solution :-) Sorry for breaking it for you in
the first place.

> > The problems come with commit 4aa71c ("libcamera: color_space: Move color
> > space adjustment to ColorSpace class"). The specific behaviour that causes
> > us grief is where it overwrites the YCbCr encoding matrix to "None" when
> > the pixel format is an RGB one.
> >
> > In the Pi's pipeline, pixels fundamentally end up as YUV and there's an
> > extra conversion that happens if you want RGB. So this means we do need to
> > know that YCbCr matrix.

Does that mean that the ISP will internally encode RGB to YUV and then
decode YUV to RGB ? If so, does the YCbCr matrix matter, or os the only
requirement that both conversions should use the same matrix ?

> > More generally, I'm not sure how I feel about taking "standard colour
> > spaces" and  then overwriting some parts of them, so that afterwards
> > something that is equivalent to (for example) sYCC is then no longer ==
> > Sycc(). Possibly I'm being a bit paranoid here, I'm not sure.

>From an application point of view, an RGB format should always have its
YCbCrEncoding set to None, as the data isn't encoded in YCbCr. I don't
really see what other good alternatives we would have.

> > Anyway, I'm not quite sure what to do about this. Is this behaviour now
> > important to other platforms? I guess we have the option to override the
> > validateColorSpaces method for the RPiCameraConfiguration. Or add another
> > flag "don't-touch-my-colour-spaces". But I'm not sensing great elegance
> > here...
> >
> > Any suggestions?

I'm not sure to see what the problem is. Could you describe what issues
you see in the application-facing behaviour and in the V4L2-facing
behaviour ?

-- 
Regards,

Laurent Pinchart