[libcamera-devel,v4,5/5] libcamera: pipeline: Register device numbers with camera
diff mbox series

Message ID 20230615172608.378258-6-kieran.bingham@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Add new Camera devices property
Related show

Commit Message

Kieran Bingham June 15, 2023, 5:26 p.m. UTC 
Register the identified device numbers with each camera as the Devices
property.

This facilitates camera daemons or other systems to identify which
devices are being managed by libcamera, and can prevent duplication of
camera resources.

As the Devices property now provides this list of devices, use it
directly from within the CameraManager when adding a Camera rather than
passing it through the internal API.

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

---
v4
 - Rename property

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 include/libcamera/internal/camera_manager.h |  3 +--
 src/libcamera/camera_manager.cpp            | 14 +++++++++-----
 src/libcamera/pipeline_handler.cpp          | 12 ++++++++++--
 3 files changed, 20 insertions(+), 9 deletions(-)

Comments

Jacopo Mondi June 16, 2023, 6:53 a.m. UTC | #1 
Hi Kieran

On Thu, Jun 15, 2023 at 06:26:08PM +0100, Kieran Bingham wrote:
> Register the identified device numbers with each camera as the Devices

This is now SystemDevices

> property.
>
> This facilitates camera daemons or other systems to identify which
> devices are being managed by libcamera, and can prevent duplication of
> camera resources.
>
> As the Devices property now provides this list of devices, use it

ditto :)

> directly from within the CameraManager when adding a Camera rather than
> passing it through the internal API.
>
> Tested-by: Ashok Sidipotu <ashok.sidipotu@collabora.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> ---
> v4
>  - Rename property
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  include/libcamera/internal/camera_manager.h |  3 +--
>  src/libcamera/camera_manager.cpp            | 14 +++++++++-----
>  src/libcamera/pipeline_handler.cpp          | 12 ++++++++++--
>  3 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h
> index 84aac499ea13..cdf009a9c626 100644
> --- a/include/libcamera/internal/camera_manager.h
> +++ b/include/libcamera/internal/camera_manager.h
> @@ -35,8 +35,7 @@ public:
>  	Private();
>
>  	int start();
> -	void addCamera(std::shared_ptr<Camera> camera,
> -		       const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);
> +	void addCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);
>  	void removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);
>
>  	/*
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index cafd7bce574e..31d45c42fde0 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -11,7 +11,9 @@
>  #include <libcamera/base/utils.h>
>
>  #include <libcamera/camera.h>
> +#include <libcamera/property_ids.h>
>
> +#include "libcamera/internal/camera.h"
>  #include "libcamera/internal/device_enumerator.h"
>  #include "libcamera/internal/pipeline_handler.h"
>
> @@ -151,19 +153,17 @@ void CameraManager::Private::cleanup()
>  /**
>   * \brief Add a camera to the camera manager
>   * \param[in] camera The camera to be added
> - * \param[in] devnums The device numbers to associate with \a camera
>   *
>   * This function is called by pipeline handlers to register the cameras they
>   * handle with the camera manager. Registered cameras are immediately made
>   * available to the system.
>   *
> - * \a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes
> - * to Camera instances.
> + * Device numbers from the Devices property are used by the V4L2 compatibility

here as well

> + * layer to map V4L2 device nodes to Camera instances.
>   *
>   * \context This function shall be called from the CameraManager thread.
>   */
> -void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
> -				       const std::vector<dev_t> &devnums)
> +void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)
>  {
>  	ASSERT(Thread::current() == this);
>
> @@ -178,6 +178,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
>  		}
>  	}
>
> +	auto devnums = camera->properties()
> +			       .get(properties::SystemDevices)
> +			       .value_or(Span<int64_t>{});
> +
>  	cameras_.push_back(std::move(camera));
>
>  	unsigned int index = cameras_.size() - 1;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 49092ea88a58..9c74c6cfda70 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -17,6 +17,7 @@
>
>  #include <libcamera/camera.h>
>  #include <libcamera/framebuffer.h>
> +#include <libcamera/property_ids.h>
>
>  #include "libcamera/internal/camera.h"
>  #include "libcamera/internal/camera_manager.h"
> @@ -612,7 +613,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
>  	 * Walk the entity list and map the devnums of all capture video nodes
>  	 * to the camera.
>  	 */
> -	std::vector<dev_t> devnums;
> +	std::vector<int64_t> devnums;
>  	for (const std::shared_ptr<MediaDevice> &media : mediaDevices_) {
>  		for (const MediaEntity *entity : media->entities()) {
>  			if (entity->pads().size() == 1 &&
> @@ -624,7 +625,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
>  		}
>  	}
>
> -	manager_->_d()->addCamera(std::move(camera), devnums);
> +	/*
> +	 * Store the associated devices as a property of the camera to allow
> +	 * systems to identify which devices are managed by libcamera.
> +	 */
> +	Camera::Private *data = camera->_d();
> +	data->properties_.set(properties::SystemDevices, devnums);
> +
> +	manager_->_d()->addCamera(std::move(camera));
>  }
>
>  /**
> --
> 2.34.1
>
Kieran Bingham June 16, 2023, 7:51 a.m. UTC | #2 
Quoting Jacopo Mondi (2023-06-16 07:53:14)
> Hi Kieran
> 
> On Thu, Jun 15, 2023 at 06:26:08PM +0100, Kieran Bingham wrote:
> > Register the identified device numbers with each camera as the Devices
> 
> This is now SystemDevices
> 
> > property.
> >
> > This facilitates camera daemons or other systems to identify which
> > devices are being managed by libcamera, and can prevent duplication of
> > camera resources.
> >
> > As the Devices property now provides this list of devices, use it
> 
> ditto :)
> 
> > directly from within the CameraManager when adding a Camera rather than
> > passing it through the internal API.
> >
> > Tested-by: Ashok Sidipotu <ashok.sidipotu@collabora.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >
> > ---
> > v4
> >  - Rename property
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  include/libcamera/internal/camera_manager.h |  3 +--
> >  src/libcamera/camera_manager.cpp            | 14 +++++++++-----
> >  src/libcamera/pipeline_handler.cpp          | 12 ++++++++++--
> >  3 files changed, 20 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h
> > index 84aac499ea13..cdf009a9c626 100644
> > --- a/include/libcamera/internal/camera_manager.h
> > +++ b/include/libcamera/internal/camera_manager.h
> > @@ -35,8 +35,7 @@ public:
> >       Private();
> >
> >       int start();
> > -     void addCamera(std::shared_ptr<Camera> camera,
> > -                    const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);
> > +     void addCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);
> >       void removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);
> >
> >       /*
> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> > index cafd7bce574e..31d45c42fde0 100644
> > --- a/src/libcamera/camera_manager.cpp
> > +++ b/src/libcamera/camera_manager.cpp
> > @@ -11,7 +11,9 @@
> >  #include <libcamera/base/utils.h>
> >
> >  #include <libcamera/camera.h>
> > +#include <libcamera/property_ids.h>
> >
> > +#include "libcamera/internal/camera.h"
> >  #include "libcamera/internal/device_enumerator.h"
> >  #include "libcamera/internal/pipeline_handler.h"
> >
> > @@ -151,19 +153,17 @@ void CameraManager::Private::cleanup()
> >  /**
> >   * \brief Add a camera to the camera manager
> >   * \param[in] camera The camera to be added
> > - * \param[in] devnums The device numbers to associate with \a camera
> >   *
> >   * This function is called by pipeline handlers to register the cameras they
> >   * handle with the camera manager. Registered cameras are immediately made
> >   * available to the system.
> >   *
> > - * \a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes
> > - * to Camera instances.
> > + * Device numbers from the Devices property are used by the V4L2 compatibility
> 
> here as well

Ayeee thanks.

I'll hold of a bit longer, but I think that's the last fix before I push
these. If there's nothing else I'll fix those and push.

--
Kieran


> 
> > + * layer to map V4L2 device nodes to Camera instances.
> >   *
> >   * \context This function shall be called from the CameraManager thread.
> >   */
> > -void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
> > -                                    const std::vector<dev_t> &devnums)
> > +void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)
> >  {
> >       ASSERT(Thread::current() == this);
> >
> > @@ -178,6 +178,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
> >               }
> >       }
> >
> > +     auto devnums = camera->properties()
> > +                            .get(properties::SystemDevices)
> > +                            .value_or(Span<int64_t>{});
> > +
> >       cameras_.push_back(std::move(camera));
> >
> >       unsigned int index = cameras_.size() - 1;
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index 49092ea88a58..9c74c6cfda70 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -17,6 +17,7 @@
> >
> >  #include <libcamera/camera.h>
> >  #include <libcamera/framebuffer.h>
> > +#include <libcamera/property_ids.h>
> >
> >  #include "libcamera/internal/camera.h"
> >  #include "libcamera/internal/camera_manager.h"
> > @@ -612,7 +613,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
> >        * Walk the entity list and map the devnums of all capture video nodes
> >        * to the camera.
> >        */
> > -     std::vector<dev_t> devnums;
> > +     std::vector<int64_t> devnums;
> >       for (const std::shared_ptr<MediaDevice> &media : mediaDevices_) {
> >               for (const MediaEntity *entity : media->entities()) {
> >                       if (entity->pads().size() == 1 &&
> > @@ -624,7 +625,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
> >               }
> >       }
> >
> > -     manager_->_d()->addCamera(std::move(camera), devnums);
> > +     /*
> > +      * Store the associated devices as a property of the camera to allow
> > +      * systems to identify which devices are managed by libcamera.
> > +      */
> > +     Camera::Private *data = camera->_d();
> > +     data->properties_.set(properties::SystemDevices, devnums);
> > +
> > +     manager_->_d()->addCamera(std::move(camera));
> >  }
> >
> >  /**
> > --
> > 2.34.1
> >

Patch
diff mbox series 

diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h
index 84aac499ea13..cdf009a9c626 100644
--- a/include/libcamera/internal/camera_manager.h
+++ b/include/libcamera/internal/camera_manager.h
@@ -35,8 +35,7 @@  public:
 	Private();
 
 	int start();
-	void addCamera(std::shared_ptr<Camera> camera,
-		       const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);
+	void addCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);
 	void removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);
 
 	/*
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index cafd7bce574e..31d45c42fde0 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -11,7 +11,9 @@ 
 #include <libcamera/base/utils.h>
 
 #include <libcamera/camera.h>
+#include <libcamera/property_ids.h>
 
+#include "libcamera/internal/camera.h"
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/pipeline_handler.h"
 
@@ -151,19 +153,17 @@  void CameraManager::Private::cleanup()
 /**
  * \brief Add a camera to the camera manager
  * \param[in] camera The camera to be added
- * \param[in] devnums The device numbers to associate with \a camera
  *
  * This function is called by pipeline handlers to register the cameras they
  * handle with the camera manager. Registered cameras are immediately made
  * available to the system.
  *
- * \a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes
- * to Camera instances.
+ * Device numbers from the Devices property are used by the V4L2 compatibility
+ * layer to map V4L2 device nodes to Camera instances.
  *
  * \context This function shall be called from the CameraManager thread.
  */
-void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
-				       const std::vector<dev_t> &devnums)
+void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)
 {
 	ASSERT(Thread::current() == this);
 
@@ -178,6 +178,10 @@  void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
 		}
 	}
 
+	auto devnums = camera->properties()
+			       .get(properties::SystemDevices)
+			       .value_or(Span<int64_t>{});
+
 	cameras_.push_back(std::move(camera));
 
 	unsigned int index = cameras_.size() - 1;
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 49092ea88a58..9c74c6cfda70 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -17,6 +17,7 @@ 
 
 #include <libcamera/camera.h>
 #include <libcamera/framebuffer.h>
+#include <libcamera/property_ids.h>
 
 #include "libcamera/internal/camera.h"
 #include "libcamera/internal/camera_manager.h"
@@ -612,7 +613,7 @@  void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
 	 * Walk the entity list and map the devnums of all capture video nodes
 	 * to the camera.
 	 */
-	std::vector<dev_t> devnums;
+	std::vector<int64_t> devnums;
 	for (const std::shared_ptr<MediaDevice> &media : mediaDevices_) {
 		for (const MediaEntity *entity : media->entities()) {
 			if (entity->pads().size() == 1 &&
@@ -624,7 +625,14 @@  void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
 		}
 	}
 
-	manager_->_d()->addCamera(std::move(camera), devnums);
+	/*
+	 * Store the associated devices as a property of the camera to allow
+	 * systems to identify which devices are managed by libcamera.
+	 */
+	Camera::Private *data = camera->_d();
+	data->properties_.set(properties::SystemDevices, devnums);
+
+	manager_->_d()->addCamera(std::move(camera));
 }
 
 /**