[libcamera-devel] [PATCH 06/10] libcamera: pipeline_handler: Make pipeline-specific data mandatory
Niklas Söderlund
niklas.soderlund at ragnatech.se
Thu Feb 28 18:29:25 CET 2019
Hi Laurent,
Thanks for your patch.
On 2019-02-28 18:29:09 +0200, Laurent Pinchart wrote:
> Mandate creationg of pipeline-specific data by pipeline handlers. This
> allows simplifying the API by passing the pipeline-specific data to the
> registerCamera() method and removing the separate setCameraData()
> method.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
I like the diffstat :-)
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/libcamera/include/pipeline_handler.h | 4 +-
> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-
> src/libcamera/pipeline/uvcvideo.cpp | 4 +-
> src/libcamera/pipeline/vimc.cpp | 4 +-
> src/libcamera/pipeline_handler.cpp | 56 +++++-------------------
> 5 files changed, 15 insertions(+), 56 deletions(-)
>
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index bc4da5820ac4..b2b9c94cebdc 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -63,11 +63,11 @@ public:
> virtual int queueRequest(Camera *camera, Request *request) = 0;
>
> protected:
> - void registerCamera(std::shared_ptr<Camera> camera);
> + void registerCamera(std::shared_ptr<Camera> camera,
> + std::unique_ptr<CameraData> data);
> void hotplugMediaDevice(MediaDevice *media);
>
> CameraData *cameraData(const Camera *camera);
> - void setCameraData(const Camera *camera, std::unique_ptr<CameraData> data);
>
> CameraManager *manager_;
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 347ee657fddf..776b7f07f78d 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -406,8 +406,7 @@ void PipelineHandlerIPU3::registerCameras()
> if (ret)
> continue;
>
> - setCameraData(camera.get(), std::move(data));
> - registerCamera(std::move(camera));
> + registerCamera(std::move(camera), std::move(data));
>
> LOG(IPU3, Info)
> << "Registered Camera[" << numCameras << "] \""
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 9af4838891f4..f121d3c5633e 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -201,9 +201,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> /* Create and register the camera. */
> std::vector<Stream *> streams{ &data->stream_ };
> std::shared_ptr<Camera> camera = Camera::create(this, media_->model(), streams);
> -
> - setCameraData(camera.get(), std::move(data));
> - registerCamera(std::move(camera));
> + registerCamera(std::move(camera), std::move(data));
>
> /* Enable hot-unplug notifications. */
> hotplugMediaDevice(media_.get());
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index c4c1eb0dc19f..6d022c37eb9f 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -209,9 +209,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
> std::vector<Stream *> streams{ &data->stream_ };
> std::shared_ptr<Camera> camera = Camera::create(this, "VIMC Sensor B",
> streams);
> -
> - setCameraData(camera.get(), std::move(data));
> - registerCamera(std::move(camera));
> + registerCamera(std::move(camera), std::move(data));
>
> return true;
> }
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index ac1acea5a318..54f237942a48 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -247,20 +247,18 @@ PipelineHandler::~PipelineHandler()
> /**
> * \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
> *
> * This method is called by pipeline handlers to register the cameras they
> - * handle with the camera manager. If no CameraData has been associated with
> - * the camera with setCameraData() by the pipeline handler, the method creates
> - * a default CameraData instance for the \a camera.
> + * 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.
> */
> -void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
> +void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera,
> + std::unique_ptr<CameraData> data)
> {
> - if (!cameraData_.count(camera.get())) {
> - std::unique_ptr<CameraData> data = utils::make_unique<CameraData>(this);
> - setCameraData(camera.get(), std::move(data));
> - }
> -
> - cameraData(camera.get())->camera_ = camera.get();
> + data->camera_ = camera.get();
> + cameraData_[camera.get()] = std::move(data);
> cameras_.push_back(camera);
> manager_->addCamera(std::move(camera));
> }
> @@ -325,51 +323,17 @@ void PipelineHandler::disconnect()
> * \brief Retrieve the pipeline-specific data associated with a Camera
> * \param camera The camera whose data to retrieve
> *
> - * \return A pointer to the pipeline-specific data set with setCameraData().
> + * \return A pointer to the pipeline-specific data passed to registerCamera().
> * The returned pointer is a borrowed reference and is guaranteed to remain
> * valid until the pipeline handler is destroyed. It shall not be deleted
> * manually by the caller.
> */
> CameraData *PipelineHandler::cameraData(const Camera *camera)
> {
> - if (!cameraData_.count(camera)) {
> - LOG(Pipeline, Error)
> - << "Cannot get data associated with camera "
> - << camera->name();
> - return nullptr;
> - }
> -
> + ASSERT(cameraData_.count(camera));
> return cameraData_[camera].get();
> }
>
> -/**
> - * \brief Set pipeline-specific data for the camera
> - * \param camera The camera to associate data to
> - * \param data The pipeline-specific data
> - *
> - * This method allows pipeline handlers to associate pipeline-specific
> - * information with \a camera. Ownership of \a data is transferred to
> - * the PipelineHandler.
> - *
> - * Pipeline-specific data can only be set once, and shall be set before
> - * registering the camera with registerCamera(). Any attempt to call this
> - * method more than once for the same camera, or to call it after registering
> - * the camera, will not change the pipeline-specific data.
> - *
> - * The data can be retrieved by pipeline handlers using the cameraData() method.
> - */
> -void PipelineHandler::setCameraData(const Camera *camera,
> - std::unique_ptr<CameraData> data)
> -{
> - if (cameraData_.find(camera) != cameraData_.end()) {
> - LOG(Pipeline, Error)
> - << "Replacing data associated with a camera is forbidden";
> - return;
> - }
> -
> - cameraData_[camera] = std::move(data);
> -}
> -
> /**
> * \var PipelineHandler::manager_
> * \brief The Camera manager associated with the pipeline handler
> --
> 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