[libcamera-devel] Colour spaces (again!)

[David Plowman]

Hi Kieran

Thanks for replying to this. I agree that a video call might be the best
way to unblock all this. Just this week - again! - I was asked by an OEM
about colour spaces, so I really think we need to get this sorted out.

I'll send an email round trying to ascertain a convenient sort of time.

Thanks
David

On Mon, 8 Nov 2021 at 12:17, Kieran Bingham <kieran.bingham at ideasonboard.com>
wrote:

> Quoting David Plowman (2021-11-08 07:28:47)
> > Hi Kieran, Jacopo and everyone
> >
> > Thanks for the recent reviews of the colour space patches. I wanted to
> > leave aside the "coding details" for the moment and discuss the more
> > fundamental ideas that were raised.
> >
> >
> > One ColorSpace field or two?
> >
> > Kieran highlighted this, and it's a good question. I went with two
> > ColorSpace fields because I was uncomfortable with the idea that
> > driver layers (we only have V4L2 at the moment) might be able to use
> > colour spaces that we haven't seen before.
>
> If we come across a colorspace that we haven't seen before, what do we
> want to do about it?
>
> Propagate it along the pipeline, and ensure the application sees the
> same color space representation?
>
> Or something else?
>
> > Again to pick up Kieran's question, colour spaces are not like pixel
> > formats because there could be an infinite number of variations,
> > though in practice it's doubtful we'll ever see more than a handful. I
> > think we have a couple of options.
> >
> > 1. Allow validate() to return an "undefined" ColorSpace. This simply
> > means the driver has chosen a colour space for which the ColorSpace
> > class no entry. At the same time I wanted to disallow applications
> > from requesting this "undefined" colour space because the idea is
> > meaningless, so it is banned.
>
> Ah, so with a validate call, that will cause any un-seen colorspace to
> fail?
>
> Are there any 'rules' that we can validate rather than 'is this an entry
> in the table'?
>
> (I didn't recall seeing a table, only some predefined types - I'll have
> to have another look back at the series).
>
> > This led me to an implementation where the StreamConfiguration has two
> > fields, one the "colour space you want" (where "undefined" is not
> > allowed), and one being "the colour space you'll get", where getting
> > back something you don't understand is possible.
>
> Getting back something you don't understand, but can be fully
> represented by the structures available in the ColorSpace class? Or ...
> not representable by the ColorSpace class?
>
> If it's representable, then I think it's fine - it just gets returned.
>
> If it's something else ... then that's where I guess it needs a specific
> "We don't know how to represent this" and that might be 'Undefined' or
> 'Unrepresentable' ... I guess you're saying there is a subtle difference
> between, something invalid, and something that is valid that we can't
> represent.
>
>
> > 2. Or I think we could do away with the "undefined" return value and
> > insist on a representation for every colour space we might ever
> > encounter. For the moment, covering the V4L2 ones would do. If we ever
> > got to a stage where alternative non-V4L2 driver layers use other
> > colour spaces, then would have to add them to the ColorSpace class.
>
> Certainly if we support beyond V4L2 - then the ColorSpace class would
> need to be extended to support that other thing...
>
> I'm not discounting the requirement for an 'Undefined' yet though ...
> But I haven't seen/understood how it needs to be split into 'what I
> want, and what I get' that can't already be handled through a single
> entry in the StreamConfiguration...
>
> > Should colour spaces be optional?
> >
> > Jacopo raised this, prompted at least in part by an earlier version of
> > my patches where I allowed this, using std::optional.
> >
> > I've actually moved fairly strongly away from this. The more time I've
> > spent with this issue the more convinced I am that letting
> > users/applications "not care" about colour spaces is a bad idea. People
> > tend to start thinking that "colour spaces are optional", but this is
> false -
> > you get some colour space or other whether you choose one or not.
> >
> > Actually I think it contributes to the general confusion about colour
> > spaces, and letting folks accept "whatever the driver chooses" is a
> > recipe for getting the wrong colour space much of the time. This is
> > especially harmful because it's a problem you simply don't notice. So
> > I think that:
> >
> > 1. You must say what colour space you want. Letting
> > generateConfiguration() choose JPEG for stills and Rec.709 for video
> > is a reasonable start, and trivial to do.
>
> What about situations where the user doesn't generate a configuration to
> start with, and they create their own?
>
> I presume the answer there is that they must specify a ColorSpace in
> that case...?
>
> But for example, the Android layer doesn't generate a configuration - it
> just starts blank and validates as far as I understand it.
>
> (There's no requirement for an applciation/layer to use
> generateConfiguration with stream roles).
>
>
> > 2. Colour space problems need to be made obvious, so that you know
> > there is "something you need to investigate". Failing completely seems
> > harsh, but allowing people simply to ignore problems and store them up
> > for later feels counterproductive.
>
> Agreed, but it might have to be 'someone who is able to' investigate.
> I'm not sure I'd expect an end user of some camera app to want to wonder
> why their camera won't capture a JPEG because of something they don't
> understand or care about. But that's probably no different to
> pixelformat issues anyway. It should be handled by the applications (or
> whatever layer does the configuration).
>
> > Finally, I want to stress that colour spaces have to be correct, even
> > though we get lulled into a false sense of security by the fact that
> > you can generally ignore them and things "seem OK" - but they are not.
>
> Yes, I have quite visible color issues between my two monitors. So I can
> see how things can easily 'go wrong silently' and have an impact.
>
> It's subtle, and if there was only one monitor, it wouldn't be noticable
> because it only shows up with a side-by-side comparison. But one of them
> is clearly 'wrong' (Granted, I don't know which one is wrong, and which
> one is right - but that's the point :D)
>
>
> > So thanks to everyone for all the debate. I'd very much like to get
> > these points settled - as you can tell, I think support for colour
> > spaces is long overdue now!
>
> Agreed, I want to help get this going as far as possible.
>
> If you think it would help, would you like to schedule and announce a
> video call/chat this week to get this unblocked?
>
> > Thanks and best regards
> >
> > David
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211112/99fc400e/attachment.htm>