| Message ID | 20230419085821.2682901-1-kieran.bingham@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran On Wed, Apr 19, 2023 at 09:58:18AM +0100, Kieran Bingham via libcamera-devel wrote: > It can be beneficial to allow reporting the devices used by a camera to > facilitate de-duplication of resources when handling cameras from a > centralised infrastructure such as PipeWire. > This series allows to retrieve the list of dev_t a libcamera::Camera handles by using a camera properties. The CameraManager class however maintains a camerasByDevnum_ map that associates a dev_t with the Camera it is owned by. It would be trivial to reverse the map. Have you considered that ? Getting dev_t from the manager seems more natural than doing so from the Camera (and would avoid the additional template specialization for dev_t you don't like) > Expose a new property on all cameras which reports a list of dev_t > values representing devices used by the camera. > > Pipelines supported by media-controller will automatically register an > entry for any video node present in the media graphs registered with the > Pipeline Handler. > > Kieran Bingham (3): > libcamera: controls: Support dev_t in an Integer32 type. > libcamera: properties: Provide a Devices camera property > libcamera: pipeline: Register device numbers with camera > > include/libcamera/controls.h | 5 +++++ > src/libcamera/pipeline_handler.cpp | 8 ++++++++ > src/libcamera/property_ids.yaml | 8 ++++++++ > 3 files changed, 21 insertions(+) > > -- > 2.34.1 >
Hi Jacopo On 19/04/2023 10:45, Jacopo Mondi wrote: > Hi Kieran > > On Wed, Apr 19, 2023 at 09:58:18AM +0100, Kieran Bingham via libcamera-devel wrote: >> It can be beneficial to allow reporting the devices used by a camera to >> facilitate de-duplication of resources when handling cameras from a >> centralised infrastructure such as PipeWire. >> > > This series allows to retrieve the list of dev_t a libcamera::Camera > handles by using a camera properties. > > The CameraManager class however maintains a camerasByDevnum_ map that > associates a dev_t with the Camera it is owned by. It would be trivial > to reverse the map. Have you considered that ? Getting dev_t from the > manager seems more natural than doing so from the Camera (and would > avoid the additional template specialization for dev_t you don't like) No I haven't, so that's an interesting point. I'll take a look. Robert/Wim, Do you have any specific requirements here? Do you need to be able to map which devices are used by each camera? Or is a single list sufficient? Or maybe returning a map of camera (id?) to device list is best all round? My only thought on the separation is the list would have to be re-obtained for hotplugged cameras, while it's available at the 'camera' when on the object. Conversely, perhaps hotplug is a reason to centralize the whole list as after a camera is removed it would need to be updated anyway. So - I'll see if I can add in a call on the Camera Manager that returns a map of camera id / dev_t. -- Kieran
On Wed, Apr 19, 2023 at 01:11:39PM +0100, Kieran Bingham via libcamera-devel wrote: > On 19/04/2023 10:45, Jacopo Mondi wrote: > > On Wed, Apr 19, 2023 at 09:58:18AM +0100, Kieran Bingham via libcamera-devel wrote: > >> It can be beneficial to allow reporting the devices used by a camera to > >> facilitate de-duplication of resources when handling cameras from a > >> centralised infrastructure such as PipeWire. > > > > This series allows to retrieve the list of dev_t a libcamera::Camera > > handles by using a camera properties. > > > > The CameraManager class however maintains a camerasByDevnum_ map that > > associates a dev_t with the Camera it is owned by. It would be trivial > > to reverse the map. Have you considered that ? Getting dev_t from the > > manager seems more natural than doing so from the Camera (and would > > avoid the additional template specialization for dev_t you don't like) > > No I haven't, so that's an interesting point. I'll take a look. > > Robert/Wim, > > Do you have any specific requirements here? Do you need to be able to > map which devices are used by each camera? Or is a single list sufficient? > > Or maybe returning a map of camera (id?) to device list is best all round? > > My only thought on the separation is the list would have to be > re-obtained for hotplugged cameras, while it's available at the 'camera' > when on the object. Conversely, perhaps hotplug is a reason to > centralize the whole list as after a camera is removed it would need to > be updated anyway. > > So - I'll see if I can add in a call on the Camera Manager that returns > a map of camera id / dev_t. The map we expose through the CameraManager is meant for the V4L2 adaptation layer only. I'm actually thinking about how we could avoid exposing it in the public API. A camera property seems a nicer solution for the problem at hand.
HI Laurent On Thu, Apr 20, 2023 at 05:57:37AM +0300, Laurent Pinchart wrote: > On Wed, Apr 19, 2023 at 01:11:39PM +0100, Kieran Bingham via libcamera-devel wrote: > > On 19/04/2023 10:45, Jacopo Mondi wrote: > > > On Wed, Apr 19, 2023 at 09:58:18AM +0100, Kieran Bingham via libcamera-devel wrote: > > >> It can be beneficial to allow reporting the devices used by a camera to > > >> facilitate de-duplication of resources when handling cameras from a > > >> centralised infrastructure such as PipeWire. > > > > > > This series allows to retrieve the list of dev_t a libcamera::Camera > > > handles by using a camera properties. > > > > > > The CameraManager class however maintains a camerasByDevnum_ map that > > > associates a dev_t with the Camera it is owned by. It would be trivial > > > to reverse the map. Have you considered that ? Getting dev_t from the > > > manager seems more natural than doing so from the Camera (and would > > > avoid the additional template specialization for dev_t you don't like) > > > > No I haven't, so that's an interesting point. I'll take a look. > > > > Robert/Wim, > > > > Do you have any specific requirements here? Do you need to be able to > > map which devices are used by each camera? Or is a single list sufficient? > > > > Or maybe returning a map of camera (id?) to device list is best all round? > > > > My only thought on the separation is the list would have to be > > re-obtained for hotplugged cameras, while it's available at the 'camera' > > when on the object. Conversely, perhaps hotplug is a reason to > > centralize the whole list as after a camera is removed it would need to > > be updated anyway. > > > > So - I'll see if I can add in a call on the Camera Manager that returns > > a map of camera id / dev_t. > > The map we expose through the CameraManager is meant for the V4L2 > adaptation layer only. I'm actually thinking about how we could avoid It's for V4L2 layer only because so far we had no other use cases, isn't it ? > exposing it in the public API. A camera property seems a nicer solution > for the problem at hand. > However it will require getting hold of all camera intances, fetch and read the properties. As Kieran said with hotplug it gets even worse. Isn't it better to get it from the manager which maintains an overall view of all resources in the system ? > -- > Regards, > > Laurent Pinchart
On Thu, Apr 20, 2023 at 08:48:14AM +0200, Jacopo Mondi wrote: > On Thu, Apr 20, 2023 at 05:57:37AM +0300, Laurent Pinchart wrote: > > On Wed, Apr 19, 2023 at 01:11:39PM +0100, Kieran Bingham via libcamera-devel wrote: > > > On 19/04/2023 10:45, Jacopo Mondi wrote: > > > > On Wed, Apr 19, 2023 at 09:58:18AM +0100, Kieran Bingham via libcamera-devel wrote: > > > >> It can be beneficial to allow reporting the devices used by a camera to > > > >> facilitate de-duplication of resources when handling cameras from a > > > >> centralised infrastructure such as PipeWire. > > > > > > > > This series allows to retrieve the list of dev_t a libcamera::Camera > > > > handles by using a camera properties. > > > > > > > > The CameraManager class however maintains a camerasByDevnum_ map that > > > > associates a dev_t with the Camera it is owned by. It would be trivial > > > > to reverse the map. Have you considered that ? Getting dev_t from the > > > > manager seems more natural than doing so from the Camera (and would > > > > avoid the additional template specialization for dev_t you don't like) > > > > > > No I haven't, so that's an interesting point. I'll take a look. > > > > > > Robert/Wim, > > > > > > Do you have any specific requirements here? Do you need to be able to > > > map which devices are used by each camera? Or is a single list sufficient? > > > > > > Or maybe returning a map of camera (id?) to device list is best all round? > > > > > > My only thought on the separation is the list would have to be > > > re-obtained for hotplugged cameras, while it's available at the 'camera' > > > when on the object. Conversely, perhaps hotplug is a reason to > > > centralize the whole list as after a camera is removed it would need to > > > be updated anyway. > > > > > > So - I'll see if I can add in a call on the Camera Manager that returns > > > a map of camera id / dev_t. > > > > The map we expose through the CameraManager is meant for the V4L2 > > adaptation layer only. I'm actually thinking about how we could avoid > > It's for V4L2 layer only because so far we had no other use cases, > isn't it ? Yes, but I also want to avoid new ue cases :-) V4L2 should be an implementation detail that applications shouldn't care about. There are some transitional use cases that we have to support, and I'd like them to be exceptions, not a norm. > > exposing it in the public API. A camera property seems a nicer solution > > for the problem at hand. > > However it will require getting hold of all camera intances, fetch and > read the properties. As Kieran said with hotplug it gets even worse. > Isn't it better to get it from the manager which maintains an overall > view of all resources in the system ? Hotplug is already supported by libcamera, through a signal that indicates when a camera is plugged in. You can then get the list of dev_t from the camera. If you want to expose that information in a different way, you will need a separate hotplug notification to tell when new dev_t are added or removed.
Cc + George who I think may also be looking at this Quoting Laurent Pinchart (2023-04-20 09:08:57) > On Thu, Apr 20, 2023 at 08:48:14AM +0200, Jacopo Mondi wrote: > > On Thu, Apr 20, 2023 at 05:57:37AM +0300, Laurent Pinchart wrote: > > > On Wed, Apr 19, 2023 at 01:11:39PM +0100, Kieran Bingham via libcamera-devel wrote: > > > > On 19/04/2023 10:45, Jacopo Mondi wrote: > > > > > On Wed, Apr 19, 2023 at 09:58:18AM +0100, Kieran Bingham via libcamera-devel wrote: > > > > >> It can be beneficial to allow reporting the devices used by a camera to > > > > >> facilitate de-duplication of resources when handling cameras from a > > > > >> centralised infrastructure such as PipeWire. > > > > > > > > > > This series allows to retrieve the list of dev_t a libcamera::Camera > > > > > handles by using a camera properties. > > > > > > > > > > The CameraManager class however maintains a camerasByDevnum_ map that > > > > > associates a dev_t with the Camera it is owned by. It would be trivial > > > > > to reverse the map. Have you considered that ? Getting dev_t from the > > > > > manager seems more natural than doing so from the Camera (and would > > > > > avoid the additional template specialization for dev_t you don't like) > > > > > > > > No I haven't, so that's an interesting point. I'll take a look. > > > > > > > > Robert/Wim, > > > > > > > > Do you have any specific requirements here? Do you need to be able to > > > > map which devices are used by each camera? Or is a single list sufficient? > > > > > > > > Or maybe returning a map of camera (id?) to device list is best all round? > > > > > > > > My only thought on the separation is the list would have to be > > > > re-obtained for hotplugged cameras, while it's available at the 'camera' > > > > when on the object. Conversely, perhaps hotplug is a reason to > > > > centralize the whole list as after a camera is removed it would need to > > > > be updated anyway. > > > > > > > > So - I'll see if I can add in a call on the Camera Manager that returns > > > > a map of camera id / dev_t. > > > > > > The map we expose through the CameraManager is meant for the V4L2 > > > adaptation layer only. I'm actually thinking about how we could avoid > > > > It's for V4L2 layer only because so far we had no other use cases, > > isn't it ? > > Yes, but I also want to avoid new ue cases :-) V4L2 should be an > implementation detail that applications shouldn't care about. There are > some transitional use cases that we have to support, and I'd like them > to be exceptions, not a norm. > > > > exposing it in the public API. A camera property seems a nicer solution > > > for the problem at hand. > > > > However it will require getting hold of all camera intances, fetch and > > read the properties. As Kieran said with hotplug it gets even worse. > > Isn't it better to get it from the manager which maintains an overall > > view of all resources in the system ? > > Hotplug is already supported by libcamera, through a signal that > indicates when a camera is plugged in. You can then get the list of > dev_t from the camera. If you want to expose that information in a > different way, you will need a separate hotplug notification to tell > when new dev_t are added or removed. I envisaged that upon hotplug, an application can call the camera manager to get the revised complete list on both hotplug and hotunplug. For a while I thought otherwise, the application would have to iterate all the cameras again to re-fresh the list. However - on hotunplug - the Camera object is passed in, so the unplug call already reports which dev_t is now being 'released' and could be handled by the app. So with: --- a/src/apps/cam/main.cpp +++ b/src/apps/cam/main.cpp @@ -187,11 +187,23 @@ int CamApp::parseOptions(int argc, char *argv[]) void CamApp::cameraAdded(std::shared_ptr<Camera> cam) { std::cout << "Camera Added: " << cam->id() << std::endl; + + std::cout << " Utilising the following devices: " << std::endl; + for (auto d : cam->properties() + .get(properties::Devices) + .value_or(std::vector<dev_t>{})) + std::cout << " - " << d << std::endl; } void CamApp::cameraRemoved(std::shared_ptr<Camera> cam) { std::cout << "Camera Removed: " << cam->id() << std::endl; + + std::cout << " Utilising the following devices: " << std::endl; + for (auto d : cam->properties() + .get(properties::Devices) + .value_or(std::vector<dev_t>{})) + std::cout << " - " << d << std::endl; } void CamApp::captureDone() We (helpfully) get: ./build/gcc/src/apps/cam/cam -m ... Camera Removed: \_SB_.PCI0.GP13.XHC0.RHUB.PRT4-4.1:1.0-0525:a4a2 Utilising the following devices: - 20736 Camera Added: \_SB_.PCI0.GP13.XHC0.RHUB.PRT4-4.1:1.0-0525:a4a2 Utilising the following devices: - 20736 So that leaves my only concern about how we 'store' the dev_t in a ControlInfo. -- Kieran > > -- > Regards, > > Laurent Pinchart
Hi Kieran, On Thu, Apr 20, 2023 at 12:42:28PM +0100, Kieran Bingham wrote: > Cc + George who I think may also be looking at this > > Quoting Laurent Pinchart (2023-04-20 09:08:57) > > On Thu, Apr 20, 2023 at 08:48:14AM +0200, Jacopo Mondi wrote: > > > On Thu, Apr 20, 2023 at 05:57:37AM +0300, Laurent Pinchart wrote: > > > > On Wed, Apr 19, 2023 at 01:11:39PM +0100, Kieran Bingham via libcamera-devel wrote: > > > > > On 19/04/2023 10:45, Jacopo Mondi wrote: > > > > > > On Wed, Apr 19, 2023 at 09:58:18AM +0100, Kieran Bingham via libcamera-devel wrote: > > > > > >> It can be beneficial to allow reporting the devices used by a camera to > > > > > >> facilitate de-duplication of resources when handling cameras from a > > > > > >> centralised infrastructure such as PipeWire. > > > > > > > > > > > > This series allows to retrieve the list of dev_t a libcamera::Camera > > > > > > handles by using a camera properties. > > > > > > > > > > > > The CameraManager class however maintains a camerasByDevnum_ map that > > > > > > associates a dev_t with the Camera it is owned by. It would be trivial > > > > > > to reverse the map. Have you considered that ? Getting dev_t from the > > > > > > manager seems more natural than doing so from the Camera (and would > > > > > > avoid the additional template specialization for dev_t you don't like) > > > > > > > > > > No I haven't, so that's an interesting point. I'll take a look. > > > > > > > > > > Robert/Wim, > > > > > > > > > > Do you have any specific requirements here? Do you need to be able to > > > > > map which devices are used by each camera? Or is a single list sufficient? > > > > > > > > > > Or maybe returning a map of camera (id?) to device list is best all round? > > > > > > > > > > My only thought on the separation is the list would have to be > > > > > re-obtained for hotplugged cameras, while it's available at the 'camera' > > > > > when on the object. Conversely, perhaps hotplug is a reason to > > > > > centralize the whole list as after a camera is removed it would need to > > > > > be updated anyway. > > > > > > > > > > So - I'll see if I can add in a call on the Camera Manager that returns > > > > > a map of camera id / dev_t. > > > > > > > > The map we expose through the CameraManager is meant for the V4L2 > > > > adaptation layer only. I'm actually thinking about how we could avoid > > > > > > It's for V4L2 layer only because so far we had no other use cases, > > > isn't it ? > > > > Yes, but I also want to avoid new ue cases :-) V4L2 should be an > > implementation detail that applications shouldn't care about. There are > > some transitional use cases that we have to support, and I'd like them > > to be exceptions, not a norm. > > > > > > exposing it in the public API. A camera property seems a nicer solution > > > > for the problem at hand. > > > > > > However it will require getting hold of all camera intances, fetch and > > > read the properties. As Kieran said with hotplug it gets even worse. > > > Isn't it better to get it from the manager which maintains an overall > > > view of all resources in the system ? > > > > Hotplug is already supported by libcamera, through a signal that > > indicates when a camera is plugged in. You can then get the list of > > dev_t from the camera. If you want to expose that information in a > > different way, you will need a separate hotplug notification to tell > > when new dev_t are added or removed. > > I envisaged that upon hotplug, an application can call the camera > manager to get the revised complete list on both hotplug and hotunplug. > > For a while I thought otherwise, the application would have to iterate > all the cameras again to re-fresh the list. However - on hotunplug - the > Camera object is passed in, so the unplug call already reports which > dev_t is now being 'released' and could be handled by the app. > > So with: > > --- a/src/apps/cam/main.cpp > +++ b/src/apps/cam/main.cpp > @@ -187,11 +187,23 @@ int CamApp::parseOptions(int argc, char *argv[]) > void CamApp::cameraAdded(std::shared_ptr<Camera> cam) > { > std::cout << "Camera Added: " << cam->id() << std::endl; > + > + std::cout << " Utilising the following devices: " << std::endl; > + for (auto d : cam->properties() > + .get(properties::Devices) > + .value_or(std::vector<dev_t>{})) > + std::cout << " - " << d << std::endl; > } > > void CamApp::cameraRemoved(std::shared_ptr<Camera> cam) > { > std::cout << "Camera Removed: " << cam->id() << std::endl; > + > + std::cout << " Utilising the following devices: " << std::endl; > + for (auto d : cam->properties() > + .get(properties::Devices) > + .value_or(std::vector<dev_t>{})) > + std::cout << " - " << d << std::endl; > } > > void CamApp::captureDone() > > > We (helpfully) get: > > ./build/gcc/src/apps/cam/cam -m > ... > Camera Removed: \_SB_.PCI0.GP13.XHC0.RHUB.PRT4-4.1:1.0-0525:a4a2 > Utilising the following devices: > - 20736 > > Camera Added: \_SB_.PCI0.GP13.XHC0.RHUB.PRT4-4.1:1.0-0525:a4a2 > Utilising the following devices: > - 20736 That's also what I had in mind :-) > So that leaves my only concern about how we 'store' the dev_t in a > ControlInfo. How about switching internal storage of all existing dev_t to 64-bit integers ? That will be large enough for all platforms and will avoid having to add a dev_t type to the control API.