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