[libcamera-devel] [PATCH v3 2/6] libcamera: pipeline_handler: Add map from devnum to cameras

[Jacopo Mondi]

Hi Paul,

On Mon, Dec 23, 2019 at 01:26:16AM -0600, Paul Elder wrote:
> The V4L2 compatibility layer will need a way to map device numbers to
> libcamera Cameras. As a first step, we add a map from devnum to Cameras
> to PipelineHandler, as well as an overloaded registerCamera() method to
> add to this map.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> New in v3
> ---
>  src/libcamera/include/pipeline_handler.h |  6 +++++
>  src/libcamera/pipeline_handler.cpp       | 30 ++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
>
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index f3622631..563de72a 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>
> @@ -84,9 +85,13 @@ public:
>
>  	const char *name() const { return name_; }
>
> +	const std::map<dev_t, std::weak_ptr<Camera>> &camerasByDevnum() const { return camerasByDevnum_; }
> +
>  protected:
>  	void registerCamera(std::shared_ptr<Camera> camera,
>  			    std::unique_ptr<CameraData> data);
> +	void registerCamera(std::shared_ptr<Camera> camera,
> +			    std::unique_ptr<CameraData> data, dev_t devnum);

Just wondering if it would not be better to have a single
implementation with devnum = 0 (it can't be 0, right ? )

>  	void hotplugMediaDevice(MediaDevice *media);
>
>  	virtual int queueRequestDevice(Camera *camera, Request *request) = 0;
> @@ -102,6 +107,7 @@ private:
>  	std::vector<std::shared_ptr<MediaDevice>> mediaDevices_;
>  	std::vector<std::weak_ptr<Camera>> cameras_;
>  	std::map<const Camera *, std::unique_ptr<CameraData>> cameraData_;
> +	std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;
>
>  	const char *name_;
>
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 5badf31c..90211f12 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>
> @@ -453,6 +455,28 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera,
>  	manager_->addCamera(std::move(camera));
>  }
>
> +/**
> + * \brief Register a camera to the camera manager and pipeline handler

Nit:
If you keep it separate, I would add "Register a camera associated
with \a devnum to... "

> + * \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
> + *
> + * 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.
> + */
> +void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera,
> +				     std::unique_ptr<CameraData> data,
> +				     dev_t devnum)
> +{
> +	registerCamera(camera, std::move(data));
> +	camerasByDevnum_[devnum] = camera;
> +}
> +
>  /**
>   * \brief Enable hotplug handling for a media device
>   * \param[in] media The media device
> @@ -538,6 +562,12 @@ CameraData *PipelineHandler::cameraData(const Camera *camera)
>   * \return The pipeline handler name
>   */
>
> +/**
> + * \fn PipelineHandler::camerasByDevnum()
> + * \brief Retrieve the map of devnums to cameras
> + * \return The map of devnums to cameras

s/devnums/device numbers/ ?

Nit aparts:
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> + */
> +
>  /**
>   * \class PipelineHandlerFactory
>   * \brief Registration of PipelineHandler classes and creation of instances
> --
> 2.23.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191227/aa0d92e3/attachment.sig>