[libcamera-devel] [PATCH v5 2/7] libcamera: Add ColorSpace fields to StreamConfiguration

[Kieran Bingham]

Quoting David Plowman (2021-11-05 14:38:13)
> Hi again Kieran
> 
> Sorry, forgot to "reply all" on my previous reply, hopefully folks can
> find the missing email trail below!
> 
> On Fri, 5 Nov 2021 at 13:29, Kieran Bingham
> <kieran.bingham at ideasonboard.com> wrote:
> >
> > Quoting David Plowman (2021-11-05 13:16:44)
> > > Hi Kieran
> > >
> > > Thanks for looking at this patch set!
> > >
> > > On Fri, 5 Nov 2021 at 12:02, Kieran Bingham
> > > <kieran.bingham at ideasonboard.com> wrote:
> > > >
> > > > Hi David,
> > > >
> > > > Quoting David Plowman (2021-11-04 13:58:00)
> > > > > This is so that applications can choose appropriate color spaces which
> > > > > will then be passed down to the V4L2 devices.
> > > > >
> > > > > There are two fields: the "requestedColorSpace" which is the one an
> > > > > application wants, and the "actualColorSpace" which is what the
> > > > > underlying hardware can deliver, and which is filled in by the
> > > > > validate() method.
> > > >
> > > > Is it necessary to store both? We don't do that with the other fields...
> > > > the StreamConfiguration structure is used in different states to handle
> > > > that I think.
> > > >
> > > > When a CameraConfiguration is 'generated' Potential StreamConfigurations
> > > > are 'offered' to the application.
> > > >
> > > > Applications can either fill in those StreamConfigurations, (or start
> > > > from an empty one if it will set all fields anyway...) and 'validate'
> > > > the overall CameraConfiguration.
> > > >
> > > > The StreamConfiguration at that point contains the 'requested' values
> > > > ... and when returned back from validate() - it has the values that can
> > > > be used by hardware...
> > >
> > > Yes, I think this is one of the critical points that we need to agree
> > > on. So my reasoning was as follows:
> > >
> > > You specify the colour space that you want, but what if the driver
> > > gives you something else. Specifically, what if the driver gives us
> > > some wacky colour space that the ColorSpace class can't represent at
> > > all?
> > >
> > > It feels a bit onerous to insist that we have a representation for
> > > every colour space that we might ever encounter, particularly if there
> > > might be non-V4L2 things in future, so I've gone with the notion of an
> > > "undefined" colour space, which can be returned to us from the driver
> > > layer.
> > >
> > > Now this is a dangerous thing so I've explicitly forbidden this as a
> > > value that you can request (what would it mean? sounds like confusion
> > > all round). So the worry is that we make a reasonable request, but the
> > > driver changes it to "undefined", and now we can't call validate with
> > > the same configuration.
> >
> > I suspect that at that point it's up to the pipeline handler to provide
> > a reasonable color space?
> 
> So I think this is the crux of the matter. Do we want to insist that
> we have a representation for every colour space that we might ever
> encounter? If we do, then yes - we can promise to pass a meaningful
> ColorSpace back up the stack and we never have to worry about
> anything.
> 
> But colour spaces are in fact infinitely variable (unlike pixel
> formats), though in practice V4L2 doesn't represent them so we'll
> almost never encounter any. For the moment, anyway. Do we want to
> pretend we can add every colour space we will ever encounter? In
> practice it's probably true, but are we sure it will always be like
> this?

Maybe the bit I haven't understood is:

 Do we expect 'named' defined color spaces, or are colorspaces (of the
 ColorSpace class) also widely constuctable?

Can I do:

 // Use the JPEG as a start point
 ColorSpace myColorSpace = ColorSpace::Jpeg;
 // Except I use the Linear TF for $REASONS
 myColorSpace.transferFunction = TransferFunction::Linear;

I thought we don't have to pre-name/define the colorspaces explicitly -
they can be interacted with as a struct as long as they Satisfy the 'isFullyDefined' check?

> > Can drivers really return an undefined colorspace? What does that mean?
> > (In the same regards as what does 'asking' for an undefined color space
> > mean?).
> 
> Note that a colour space is never actually "undefined", the driver
> will have chosen one, it just means "we don't have a ColorSpace to
> describe what it chose". So after validate() we could be told, "it's
> going to use a colour space that I can't describe to you", but at the
> same time it's nonsensical to request a colour space "that I can't
> describe", which is why that is banned.
> 
> I don't know if any of this helps. Perhaps we need a face-to-face.

Maybe ;-) I'll go continue the discussion on the top level discussion
mail, and we can see who we need to get together.

--
Kieran