[libcamera-devel] [PATCH v3 4/5] libcamera: properties: Provide a Devices camera property

[Barnabás Pőcze]

Hi


2023. június 13., kedd 19:10 keltezéssel, Kieran Bingham <kieran.bingham at ideasonboard.com> írta:

> Quoting Barnabás Pőcze (2023-06-13 16:48:39)
> > Hi
> >
> >
> > 2023. június 12., hétfő 18:28 keltezéssel, Kieran Bingham <kieran.bingham at ideasonboard.com> írta:
> >
> > > [...]
> > > > > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > > > > index cb55e0ed2283..6141942969f9 100644
> > > > > --- a/src/libcamera/property_ids.yaml
> > > > > +++ b/src/libcamera/property_ids.yaml
> > > > > @@ -690,6 +690,13 @@ controls:
> > > > >          that is twice that of the full resolution mode. This value will be valid
> > > > >          after the configure method has returned successfully.
> > > > >
> > > > > +  - Devices:
> > > > > +      type: int64_t
> > > >
> > > > I am wondering why the type is `int64_t`. POSIX only says `dev_t` is an integer type[0].
> > > > Both glibc[1] and musl[2] define it as an unsigned (64-bit) integer (at least as far as I checked).
> > >
> > > Indeed every implementation I can see define it as a 64-bit, but the
> > > kernel uses only 32 bits.
> > >
> > > This is a libcamera control limitation ultimately, as we don't have
> > > unsigned integer controls. I'm not sure yet if adding would be helpful
> > > or difficult. It might be worth trying out.
> > > [...]
> >
> > Ahh, I wasn't aware that libcamera does not have uint controls. Thanks for the clarification.
> 
> Have you reviewed the patches enough to consider providing a set of
> Reviewed-by tags perhaps?
> 
> Do you have any thoughts or preferences on the control name? Laurent has
> discussed the option between "Devices" and "SystemDevices" - I'm really
> hoping someone will weigh in on which they prefer to be able to move
> forwards and merge this series.
> 
> (I prefer Devices, so that makes it 1 vs 1)
> [...]

Well, I can certainly weigh in, but I don't think you will be pleased, because
my preference leans towards "SystemDevices". But I would even go as far as to
suggest "{,Underlying}{Kernel,System}DeviceIDs" as potential candidates.
Admittedly some of those are long, but consider that

 - most programs won't need this, and
 - those that need will very likely only query it in a single place.

So I think even something like "UnderlyingKernelDeviceIDs" could work,
and I think that name is quite descriptive.

But, of course, since this is likely a very rarely used control, I think both
"Devices" and "SystemDevices" would work just fine.


Regards,
Barnabás Pőcze