[libcamera-devel] [PATCH 01/10] libcamera: pipeline_handler: Store the camera manager pointer
Niklas Söderlund
niklas.soderlund at ragnatech.se
Thu Jan 24 17:38:00 CET 2019
Hi Laurent,
Thanks for your work.
On 2019-01-24 12:16:42 +0200, Laurent Pinchart wrote:
> Instead of passing the camera manager pointer to the match() function,
> and later to more PipelineHandler functions, store it in the
> PipelineHandler::manager_ member variable at construction time and
> access it from there.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/libcamera/camera_manager.cpp | 4 +--
> src/libcamera/include/pipeline_handler.h | 20 ++++++++-----
> src/libcamera/pipeline/ipu3/ipu3.cpp | 18 ++++++------
> src/libcamera/pipeline/uvcvideo.cpp | 12 ++++----
> src/libcamera/pipeline/vimc.cpp | 12 ++++----
> src/libcamera/pipeline_handler.cpp | 36 ++++++++++++++++++++----
> 6 files changed, 66 insertions(+), 36 deletions(-)
>
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 4ea7ed44cc31..14410d4dcda7 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -98,8 +98,8 @@ int CameraManager::start()
> * all pipelines it can provide.
> */
> while (1) {
> - PipelineHandler *pipe = factory->create();
> - if (!pipe->match(this, enumerator_.get())) {
> + PipelineHandler *pipe = factory->create(this);
> + if (!pipe->match(enumerator_.get())) {
> delete pipe;
> break;
> }
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index b03217d5bff8..7bb07d1ec5c7 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -19,9 +19,13 @@ class DeviceEnumerator;
> class PipelineHandler
> {
> public:
> - virtual ~PipelineHandler() { };
> + PipelineHandler(CameraManager *manager);
> + virtual ~PipelineHandler();
>
> - virtual bool match(CameraManager *manager, DeviceEnumerator *enumerator) = 0;
> + virtual bool match(DeviceEnumerator *enumerator) = 0;
> +
> +protected:
> + CameraManager *manager_;
> };
>
> class PipelineHandlerFactory
> @@ -30,7 +34,7 @@ public:
> PipelineHandlerFactory(const char *name);
> virtual ~PipelineHandlerFactory() { };
>
> - virtual PipelineHandler *create() = 0;
> + virtual PipelineHandler *create(CameraManager *manager) = 0;
>
> const std::string &name() const { return name_; }
>
> @@ -42,11 +46,13 @@ private:
> };
>
> #define REGISTER_PIPELINE_HANDLER(handler) \
> -class handler##Factory : public PipelineHandlerFactory { \
> +class handler##Factory : public PipelineHandlerFactory \
> +{ \
> public: \
> - handler##Factory() : PipelineHandlerFactory(#handler) { } \
> - PipelineHandler *create() final { \
> - return new handler(); \
> + handler##Factory() : PipelineHandlerFactory(#handler) {} \
> + PipelineHandler *create(CameraManager *manager) final \
> + { \
> + return new handler(manager); \
> } \
> }; \
> static handler##Factory global_##handler##Factory;
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 8cbc10acfbb5..589b3078f301 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -23,20 +23,20 @@ LOG_DEFINE_CATEGORY(IPU3)
> class PipelineHandlerIPU3 : public PipelineHandler
> {
> public:
> - PipelineHandlerIPU3();
> + PipelineHandlerIPU3(CameraManager *manager);
> ~PipelineHandlerIPU3();
>
> - bool match(CameraManager *manager, DeviceEnumerator *enumerator);
> + bool match(DeviceEnumerator *enumerator);
>
> private:
> MediaDevice *cio2_;
> MediaDevice *imgu_;
>
> - void registerCameras(CameraManager *manager);
> + void registerCameras();
> };
>
> -PipelineHandlerIPU3::PipelineHandlerIPU3()
> - : cio2_(nullptr), imgu_(nullptr)
> +PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)
> + : PipelineHandler(manager), cio2_(nullptr), imgu_(nullptr)
> {
> }
>
> @@ -52,7 +52,7 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()
> imgu_ = nullptr;
> }
>
> -bool PipelineHandlerIPU3::match(CameraManager *manager, DeviceEnumerator *enumerator)
> +bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
> {
> DeviceMatch cio2_dm("ipu3-cio2");
> cio2_dm.add("ipu3-csi2 0");
> @@ -106,7 +106,7 @@ bool PipelineHandlerIPU3::match(CameraManager *manager, DeviceEnumerator *enumer
> if (cio2_->disableLinks())
> goto error_close_cio2;
>
> - registerCameras(manager);
> + registerCameras();
>
> cio2_->close();
>
> @@ -127,7 +127,7 @@ error_release_mdev:
> * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four
> * CIO2 CSI-2 receivers.
> */
> -void PipelineHandlerIPU3::registerCameras(CameraManager *manager)
> +void PipelineHandlerIPU3::registerCameras()
> {
> /*
> * For each CSI-2 receiver on the IPU3, create a Camera if an
> @@ -172,7 +172,7 @@ void PipelineHandlerIPU3::registerCameras(CameraManager *manager)
>
> std::string cameraName = sensor->name() + " " + std::to_string(id);
> std::shared_ptr<Camera> camera = Camera::create(cameraName);
> - manager->addCamera(std::move(camera));
> + manager_->addCamera(std::move(camera));
>
> LOG(IPU3, Info)
> << "Registered Camera[" << numCameras << "] \""
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 3ba69da8b775..92b23da73901 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -17,17 +17,17 @@ namespace libcamera {
> class PipelineHandlerUVC : public PipelineHandler
> {
> public:
> - PipelineHandlerUVC();
> + PipelineHandlerUVC(CameraManager *manager);
> ~PipelineHandlerUVC();
>
> - bool match(CameraManager *manager, DeviceEnumerator *enumerator);
> + bool match(DeviceEnumerator *enumerator);
>
> private:
> MediaDevice *dev_;
> };
>
> -PipelineHandlerUVC::PipelineHandlerUVC()
> - : dev_(nullptr)
> +PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)
> + : PipelineHandler(manager), dev_(nullptr)
> {
> }
>
> @@ -37,7 +37,7 @@ PipelineHandlerUVC::~PipelineHandlerUVC()
> dev_->release();
> }
>
> -bool PipelineHandlerUVC::match(CameraManager *manager, DeviceEnumerator *enumerator)
> +bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> {
> DeviceMatch dm("uvcvideo");
>
> @@ -49,7 +49,7 @@ bool PipelineHandlerUVC::match(CameraManager *manager, DeviceEnumerator *enumera
> dev_->acquire();
>
> std::shared_ptr<Camera> camera = Camera::create(dev_->model());
> - manager->addCamera(std::move(camera));
> + manager_->addCamera(std::move(camera));
>
> return true;
> }
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 82b9237a3d7d..f12d007cd956 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -17,17 +17,17 @@ namespace libcamera {
> class PipeHandlerVimc : public PipelineHandler
> {
> public:
> - PipeHandlerVimc();
> + PipeHandlerVimc(CameraManager *manager);
> ~PipeHandlerVimc();
>
> - bool match(CameraManager *manager, DeviceEnumerator *enumerator);
> + bool match(DeviceEnumerator *enumerator);
>
> private:
> MediaDevice *dev_;
> };
>
> -PipeHandlerVimc::PipeHandlerVimc()
> - : dev_(nullptr)
> +PipeHandlerVimc::PipeHandlerVimc(CameraManager *manager)
> + : PipelineHandler(manager), dev_(nullptr)
> {
> }
>
> @@ -37,7 +37,7 @@ PipeHandlerVimc::~PipeHandlerVimc()
> dev_->release();
> }
>
> -bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator)
> +bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)
> {
> DeviceMatch dm("vimc");
>
> @@ -65,7 +65,7 @@ bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator
> * object is modeled.
> */
> std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera");
> - manager->addCamera(std::move(camera));
> + manager_->addCamera(std::move(camera));
>
> return true;
> }
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index c24feeafc503..45788487b5c0 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -34,18 +34,30 @@ LOG_DEFINE_CATEGORY(Pipeline)
> * with the pipelines it supports and creates corresponding Camera devices.
> */
>
> +/**
> + * \brief Construct a PipelineHandler instance
> + * \param[in] manager The camera manager
> + */
> +PipelineHandler::PipelineHandler(CameraManager *manager)
> + : manager_(manager)
> +{
> +}
> +
> +PipelineHandler::~PipelineHandler()
> +{
> +}
> +
> /**
> * \fn PipelineHandler::match(DeviceEnumerator *enumerator)
> * \brief Match media devices and create camera instances
> - * \param manager The camera manager
> * \param enumerator The enumerator providing all media devices found in the
> * system
> *
> * This function is the main entry point of the pipeline handler. It is called
> - * by the camera manager with the \a manager and \a enumerator passed as
> - * arguments. It shall acquire from the \a enumerator all the media devices it
> - * needs for a single pipeline, create one or multiple Camera instances and
> - * register them with the \a manager.
> + * by the camera manager with the \a enumerator passed as an argument. It shall
> + * acquire from the \a enumerator all the media devices it needs for a single
> + * pipeline, create one or multiple Camera instances and register them with the
> + * camera manager.
> *
> * If all media devices needed by the pipeline handler are found, they must all
> * be acquired by a call to MediaDevice::acquire(). This function shall then
> @@ -66,6 +78,15 @@ LOG_DEFINE_CATEGORY(Pipeline)
> * created, or false otherwise
> */
>
> +/**
> + * \var PipelineHandler::manager_
> + * \brief The Camera manager associated with the pipeline handler
> + *
> + * The camera manager pointer is stored in the pipeline handler for the
> + * convenience of pipeline handler implementations. It remains valid and
> + * constant for the whole lifetime of the pipeline handler.
> + */
> +
> /**
> * \class PipelineHandlerFactory
> * \brief Registration of PipelineHandler classes and creation of instances
> @@ -96,8 +117,11 @@ PipelineHandlerFactory::PipelineHandlerFactory(const char *name)
> /**
> * \fn PipelineHandlerFactory::create()
> * \brief Create an instance of the PipelineHandler corresponding to the factory
> + * \param[in] manager The camera manager
> *
> - * This virtual function is implemented by the REGISTER_PIPELINE_HANDLER() macro.
> + * This virtual function is implemented by the REGISTER_PIPELINE_HANDLER()
> + * macro. It creates a pipeline handler instance associated with the camera
> + * \a manager.
> *
> * \return a pointer to a newly constructed instance of the PipelineHandler
> * subclass corresponding to the factory
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list