| Message ID | 20190228162913.6508-7-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@ideasonboard.com> I like the diffstat :-) Reviewed-by: Niklas Söderlund <niklas.soderlund@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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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
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@ideasonboard.com> --- 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(-)