[libcamera-devel,v4,4/5] libcamera: properties: Provide a Devices camera property

18741 diff mbox series

Provide a new Camera property that allows pipeline handlers to list any
kernel device used to operate the camera. This allows other frameworks
and daemons such as PipeWire to better understand the resources consumed
by a Camera and consider ignoring those resources when enumerating
camera devices on a system.

Tested-by: Ashok Sidipotu <ashok.sidipotu@collabora.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---
v4:
 - Report that different cameras may report identical devices
 - Rename to SystemDevices

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/property_ids.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

Hi


2023. június 15., csütörtök 19:26 keltezéssel, Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org> írta:

> Provide a new Camera property that allows pipeline handlers to list any
> kernel device used to operate the camera. This allows other frameworks
> and daemons such as PipeWire to better understand the resources consumed
> by a Camera and consider ignoring those resources when enumerating
> camera devices on a system.
> 
> Tested-by: Ashok Sidipotu <ashok.sidipotu@collabora.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> ---
> v4:
>  - Report that different cameras may report identical devices
>  - Rename to SystemDevices
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/property_ids.yaml | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index cb55e0ed2283..ef1dfd322db1 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -690,6 +690,15 @@ controls:
>          that is twice that of the full resolution mode. This value will be valid
>          after the configure method has returned successfully.
> 
> +  - SystemDevices:
> +      type: int64_t
> +      size: [n]
> +      description: |
> +        A list of integer values of type dev_t denoting major and minor device
> +        number of the underlying devices used in the operation of this camera.

I am no native speaker of English, but to me any of

 - ... denoting [the] major and minor device number of the underlying ...
 - ... denoting major and minor device number[s] of the underlying ...

sounds a bit better.


> +
> +        Different cameras may report identical devices.

I am wondering how this statement is reconciled with the fact that `camerasByDevnum_`
is just a simple map, and not a multimap; and that `CameraManager::get(dev_t)`
returns a single camera. I know this is not really relevant for this patch, sorry,
but maybe it's worth looking into.


> +
>    # ----------------------------------------------------------------------------
>    # Draft properties section
> 
> --
> 2.34.1


Regards,
Barnabás Pőcze

Quoting Barnabás Pőcze (2023-06-15 22:21:12)
> Hi
> 
> 
> 2023. június 15., csütörtök 19:26 keltezéssel, Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org> írta:
> 
> > Provide a new Camera property that allows pipeline handlers to list any
> > kernel device used to operate the camera. This allows other frameworks
> > and daemons such as PipeWire to better understand the resources consumed
> > by a Camera and consider ignoring those resources when enumerating
> > camera devices on a system.
> > 
> > Tested-by: Ashok Sidipotu <ashok.sidipotu@collabora.com>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> > ---
> > v4:
> >  - Report that different cameras may report identical devices
> >  - Rename to SystemDevices
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  src/libcamera/property_ids.yaml | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > index cb55e0ed2283..ef1dfd322db1 100644
> > --- a/src/libcamera/property_ids.yaml
> > +++ b/src/libcamera/property_ids.yaml
> > @@ -690,6 +690,15 @@ controls:
> >          that is twice that of the full resolution mode. This value will be valid
> >          after the configure method has returned successfully.
> > 
> > +  - SystemDevices:
> > +      type: int64_t
> > +      size: [n]
> > +      description: |
> > +        A list of integer values of type dev_t denoting major and minor device
> > +        number of the underlying devices used in the operation of this camera.
> 
> I am no native speaker of English, but to me any of
> 
>  - ... denoting [the] major and minor device number of the underlying ...
>  - ... denoting major and minor device number[s] of the underlying ...
> 
> sounds a bit better.

Yes, I'll add those.

> 
> 
> > +
> > +        Different cameras may report identical devices.
> 
> I am wondering how this statement is reconciled with the fact that `camerasByDevnum_`
> is just a simple map, and not a multimap; and that `CameraManager::get(dev_t)`
> returns a single camera. I know this is not really relevant for this patch, sorry,
> but maybe it's worth looking into.

Hopefully [0] will remove all concerns about camerasByDevnum_ by
removing it and CameraManager::get(dev_t) entirely.

[0] https://patchwork.libcamera.org/project/libcamera/list/?series=3927

Thanks

Kieran


> 
> 
> > +
> >    # ----------------------------------------------------------------------------
> >    # Draft properties section
> > 
> > --
> > 2.34.1
> 
> 
> Regards,
> Barnabás Pőcze