[libcamera-devel] [PATCH v4 2/4] libcamera: camera_manager, pipeline_handler: allow retrieving cameras by device numbers
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Jan 2 01:46:35 CET 2020
Hi Paul,
Thank you for the patch.
On Mon, Dec 30, 2019 at 11:33:12PM -0600, Paul Elder wrote:
> The V4L2 compatibility layer will need a way to map device numbers to
> libcamera Camera instances. Expose a method in the camera manager to
> retrieve Camera instances by devnum. The mapping from device numbers to
> Camera instances is optionally declared by pipeline handlers when they
> register cameras with the camera manager.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> ---
> Changes in v4:
> - squashed 4/6 from v3
> - simplified registering devnum -> camera mappings
> - old (v3) system: pipeline handlers have their own map, that camera
> manager aggregates
> - new (v4) system: pipeline handlers, when they register the camera
> with the camera manager, (optionally) declare the devnum along with it
>
> New in v3
> ---
> include/libcamera/camera_manager.h | 5 +++-
> src/libcamera/camera_manager.cpp | 37 +++++++++++++++++++++++-
> src/libcamera/include/pipeline_handler.h | 3 +-
> src/libcamera/pipeline_handler.cpp | 13 +++++++--
> 4 files changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> index 8331898c..d2d6e443 100644
> --- a/include/libcamera/camera_manager.h
> +++ b/include/libcamera/camera_manager.h
> @@ -7,6 +7,7 @@
> #ifndef __LIBCAMERA_CAMERA_MANAGER_H__
> #define __LIBCAMERA_CAMERA_MANAGER_H__
>
> +#include <map>
> #include <memory>
> #include <string>
Should you include <sys/types.h> for dev_t ?
> #include <vector>
> @@ -33,8 +34,9 @@ public:
>
> const std::vector<std::shared_ptr<Camera>> &cameras() const { return cameras_; }
> std::shared_ptr<Camera> get(const std::string &name);
> + std::shared_ptr<Camera> get(dev_t devnum);
>
> - void addCamera(std::shared_ptr<Camera> camera);
> + void addCamera(std::shared_ptr<Camera> camera, dev_t devnum);
> void removeCamera(Camera *camera);
>
> static const std::string &version() { return version_; }
> @@ -46,6 +48,7 @@ private:
> std::unique_ptr<DeviceEnumerator> enumerator_;
> std::vector<std::shared_ptr<PipelineHandler>> pipes_;
> std::vector<std::shared_ptr<Camera>> cameras_;
> + std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;
>
> static const std::string version_;
> static CameraManager *self_;
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 7c6f72bb..b6204872 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -180,15 +180,45 @@ std::shared_ptr<Camera> CameraManager::get(const std::string &name)
> return nullptr;
> }
>
> +/**
> + * \brief Retrieve a camera based on device number
> + * \param[in] devnum Device number of camera to get
> + *
> + * Retrieving a camera based on device number is done by the V4L2 compatibility
> + * layer to map device nodes to libcamera Camera instances.
> + *
> + * Regular applications are recommended to retrieve libcamera Camera instances
> + * by name instead.
I'd make this a stronger requirement:
* This method is meant solely for the use of the V4L2 compatibility
* layer, to map device nodes to Camera instances. Applications shall
* not use it and shall instead retrieve cameras by name.
> + *
> + * Before calling this function the caller is responsible for ensuring that
> + * the camera manager is running.
> + *
> + * \return Shared pointer to Camera object, which is empty if the camera is
> + * not found
> + */
> +std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
> +{
> + auto iter = camerasByDevnum_.find(devnum);
> +
I think you can drop this blank line.
> + if (iter == camerasByDevnum_.end())
> + return nullptr;
> +
> + return iter->second.lock();
> +}
> +
> /**
> * \brief Add a camera to the camera manager
> * \param[in] camera The camera to be added
> + * \param[in] devnum The device number 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 devnum is used by the V4L2 compatibility layer to map V4L2 device nodes
> + * to libcamera Camera instances.
s/libcamera // as we're in libcamera, so this is the default :-)
> */
> -void CameraManager::addCamera(std::shared_ptr<Camera> camera)
> +void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)
> {
> for (std::shared_ptr<Camera> c : cameras_) {
> if (c->name() == camera->name()) {
> @@ -200,6 +230,11 @@ void CameraManager::addCamera(std::shared_ptr<Camera> camera)
> }
>
> cameras_.push_back(std::move(camera));
> +
> + if (devnum) {
> + unsigned int index = cameras_.size() - 1;
> + camerasByDevnum_[devnum] = cameras_[index];
> + }
> }
You also need to update CameraManager::removeCamera.
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index 7c6f72bbe120..d45d81effa8d 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -212,15 +212,17 @@ void CameraManager::addCamera(std::shared_ptr<Camera> camera)
*/
void CameraManager::removeCamera(Camera *camera)
{
- for (auto iter = cameras_.begin(); iter != cameras_.end(); ++iter) {
- if (iter->get() == camera) {
- LOG(Camera, Debug)
- << "Unregistering camera '"
- << camera->name() << "'";
- cameras_.erase(iter);
- return;
- }
- }
+ auto iter = std::find_if(cameras_.begin(), cameras_.end(),
+ [camera](std::shared_ptr<Camera> &c) {
+ return c.get() == camera;
+ });
+ if (iter == cameras_.end())
+ return;
+
+ LOG(Camera, Debug)
+ << "Unregistering camera '" << camera->name() << "'";
+
+ cameras_.erase(iter);
}
/**
(untested) and an additional std::find_if at the end for
camerasByDevnum_.
>
> /**
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index f3622631..067baef5 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -12,6 +12,7 @@
> #include <memory>
> #include <set>
> #include <string>
> +#include <sys/sysmacros.h>
> #include <vector>
>
> #include <ipa/ipa_interface.h>
> @@ -86,7 +87,7 @@ public:
>
> protected:
> void registerCamera(std::shared_ptr<Camera> camera,
> - std::unique_ptr<CameraData> data);
> + std::unique_ptr<CameraData> data, dev_t devnum = 0);
> void hotplugMediaDevice(MediaDevice *media);
>
> virtual int queueRequestDevice(Camera *camera, Request *request) = 0;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 5badf31c..942b1a30 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -7,6 +7,8 @@
>
> #include "pipeline_handler.h"
>
> +#include <sys/sysmacros.h>
> +
> #include <libcamera/buffer.h>
> #include <libcamera/camera.h>
> #include <libcamera/camera_manager.h>
> @@ -438,19 +440,26 @@ void PipelineHandler::completeRequest(Camera *camera, Request *request)
> * \brief Register a camera to the camera manager and pipeline handler
> * \param[in] camera The camera to be added
> * \param[in] data Pipeline-specific data for the camera
> + * \param[in] devnum Device number of the camera (optional)
> *
> * This method is called by pipeline handlers to register the cameras they
> * handle with the camera manager. It associates the pipeline-specific \a data
> * with the camera, for later retrieval with cameraData(). Ownership of \a data
> * is transferred to the PipelineHandler.
> + *
> + * \a devnum is the device number (as returned by makedev) that the \a camera
> + * is to be associated with. This is for the V4L2 compatibility layer to map
> + * device nodes to libcamera Camera instances based on the device number
s/libcamera //
> + * registered by this method in \a devnum.
> */
> void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera,
> - std::unique_ptr<CameraData> data)
> + std::unique_ptr<CameraData> data,
> + dev_t devnum)
> {
> data->camera_ = camera.get();
> cameraData_[camera.get()] = std::move(data);
> cameras_.push_back(camera);
> - manager_->addCamera(std::move(camera));
> + manager_->addCamera(std::move(camera), devnum);
> }
>
> /**
With the above issues addressed,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list