[libcamera-devel] [RFC PATCH 16/17] libcamera: pipeline_handler: Drop CameraData class
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sat Jul 24 09:29:09 CEST 2021
Hi Laurent,
Thanks for your work.
On 2021-07-23 07:00:35 +0300, Laurent Pinchart wrote:
> The CameraData class isn't used anymore. Drop it.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/internal/pipeline_handler.h | 26 +----
> src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
> .../pipeline/raspberrypi/raspberrypi.cpp | 2 +-
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
> src/libcamera/pipeline/simple/simple.cpp | 2 +-
> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +-
> src/libcamera/pipeline/vimc/vimc.cpp | 2 +-
> src/libcamera/pipeline_handler.cpp | 94 +------------------
> 8 files changed, 9 insertions(+), 123 deletions(-)
What's not to love?
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index 86727f90bb65..2f814753f2ae 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -8,14 +8,12 @@
> #define __LIBCAMERA_INTERNAL_PIPELINE_HANDLER_H__
>
> #include <list>
> -#include <map>
> #include <memory>
> #include <set>
> #include <string>
> #include <sys/types.h>
> #include <vector>
>
> -#include <libcamera/base/class.h>
> #include <libcamera/base/object.h>
>
> #include <libcamera/controls.h>
> @@ -35,23 +33,6 @@ class MediaDevice;
> class PipelineHandler;
> class Request;
>
> -class CameraData
> -{
> -public:
> - explicit CameraData(PipelineHandler *pipe)
> - : pipe_(pipe)
> - {
> - }
> - virtual ~CameraData() = default;
> -
> - PipelineHandler *pipe_;
> - ControlInfoMap controlInfo_;
> - ControlList properties_;
> -
> -private:
> - LIBCAMERA_DISABLE_COPY(CameraData)
> -};
> -
> class PipelineHandler : public std::enable_shared_from_this<PipelineHandler>,
> public Object
> {
> @@ -88,15 +69,11 @@ public:
> const char *name() const { return name_; }
>
> protected:
> - void registerCamera(std::shared_ptr<Camera> camera,
> - std::unique_ptr<CameraData> data);
> + void registerCamera(std::shared_ptr<Camera> camera);
> void hotplugMediaDevice(MediaDevice *media);
>
> virtual int queueRequestDevice(Camera *camera, Request *request) = 0;
>
> - CameraData *cameraData(const Camera *camera);
> - const CameraData *cameraData(const Camera *camera) const;
> -
> CameraManager *manager_;
>
> private:
> @@ -105,7 +82,6 @@ private:
>
> std::vector<std::shared_ptr<MediaDevice>> mediaDevices_;
> std::vector<std::weak_ptr<Camera>> cameras_;
> - std::map<const Camera *, std::unique_ptr<CameraData>> cameraData_;
>
> const char *name_;
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 6d097ac91d2e..713fbec4daae 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -1187,7 +1187,7 @@ int PipelineHandlerIPU3::registerCameras()
> std::shared_ptr<Camera> camera =
> Camera::create(data.release(), cameraId, streams);
>
> - registerCamera(std::move(camera), nullptr);
> + registerCamera(std::move(camera));
>
> LOG(IPU3, Info)
> << "Registered Camera[" << numCameras << "] \""
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index be8dcdc67a15..895ab3fee329 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1109,7 +1109,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> const std::string &id = data->sensor_->id();
> std::shared_ptr<Camera> camera =
> Camera::create(data.release(), id, streams);
> - registerCamera(std::move(camera), nullptr);
> + registerCamera(std::move(camera));
>
> return true;
> }
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index eab892785fb0..22be1a03e4dc 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -973,7 +973,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> const std::string &id = data->sensor_->id();
> std::shared_ptr<Camera> camera =
> Camera::create(data.release(), id, streams);
> - registerCamera(std::move(camera), nullptr);
> + registerCamera(std::move(camera));
>
> return 0;
> }
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 81d342a3fa73..8594520f520c 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -1050,7 +1050,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> const std::string &id = data->sensor_->id();
> std::shared_ptr<Camera> camera =
> Camera::create(data.release(), id, streams);
> - registerCamera(std::move(camera), nullptr);
> + registerCamera(std::move(camera));
> registered = true;
> }
>
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index bbeeda97a91a..882d3f82839a 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -472,7 +472,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> std::set<Stream *> streams{ &data->stream_ };
> std::shared_ptr<Camera> camera =
> Camera::create(data.release(), id, streams);
> - registerCamera(std::move(camera), nullptr);
> + registerCamera(std::move(camera));
>
> /* Enable hot-unplug notifications. */
> hotplugMediaDevice(media);
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 866c993d7e50..bd7cbcebe3ff 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -440,7 +440,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
> const std::string &id = data->sensor_->id();
> std::shared_ptr<Camera> camera =
> Camera::create(data.release(), id, streams);
> - registerCamera(std::move(camera), nullptr);
> + registerCamera(std::move(camera));
>
> return true;
> }
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 1f1de19d81c5..28e09bc00771 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -39,55 +39,6 @@ namespace libcamera {
>
> LOG_DEFINE_CATEGORY(Pipeline)
>
> -/**
> - * \class CameraData
> - * \brief Base class for platform-specific data associated with a camera
> - *
> - * The CameraData base abstract class represents platform specific-data
> - * a pipeline handler might want to associate with a Camera to access them
> - * at a later time.
> - *
> - * Pipeline handlers are expected to extend this base class with platform
> - * specific implementation, associate instances of the derived classes
> - * using the registerCamera() method, and access them at a later time
> - * with cameraData().
> - */
> -
> -/**
> - * \fn CameraData::CameraData(PipelineHandler *pipe)
> - * \brief Construct a CameraData instance for the given pipeline handler
> - * \param[in] pipe The pipeline handler
> - *
> - * The reference to the pipeline handler is stored internally, the caller shall
> - * guarantee that the pointer remains valid as long as the CameraData instance
> - * exists.
> - */
> -
> -/**
> - * \var CameraData::pipe_
> - * \brief The pipeline handler related to this CameraData instance
> - *
> - * The pipe_ pointer provides access to the PipelineHandler object that this
> - * instance is related to. It is set when the CameraData instance is created
> - * and remains valid until the instance is destroyed.
> - */
> -
> -/**
> - * \var CameraData::controlInfo_
> - * \brief The set of controls supported by the camera
> - *
> - * The control information shall be initialised by the pipeline handler when
> - * creating the camera, and shall not be modified afterwards.
> - */
> -
> -/**
> - * \var CameraData::properties_
> - * \brief The list of properties supported by the camera
> - *
> - * The list of camera properties shall be initialised by the pipeline handler
> - * when creating the camera, and shall not be modified afterwards.
> - */
> -
> /**
> * \class PipelineHandler
> * \brief Create and manage cameras based on a set of media devices
> @@ -471,28 +422,14 @@ void PipelineHandler::completeRequest(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
> *
> * 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.
> + * handle with the camera manager.
> *
> * \context This function shall be called from the CameraManager thread.
> */
> -void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera,
> - std::unique_ptr<CameraData> data)
> +void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
> {
> - /*
> - * To be removed once all pipeline handlers will have migrated from
> - * CameraData to Camera::Private.
> - */
> - if (data) {
> - camera->_d()->controlInfo_ = std::move(data->controlInfo_);
> - camera->_d()->properties_ = std::move(data->properties_);
> - }
> -
> - cameraData_[camera.get()] = std::move(data);
> cameras_.push_back(camera);
>
> if (mediaDevices_.empty())
> @@ -586,33 +523,6 @@ void PipelineHandler::disconnect()
> }
> }
>
> -/**
> - * \brief Retrieve the pipeline-specific data associated with a Camera
> - * \param[in] camera The camera whose data to retrieve
> - * \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)
> -{
> - ASSERT(cameraData_.count(camera));
> - return cameraData_[camera].get();
> -}
> -
> -/**
> - * \brief Retrieve the pipeline-specific data associated with a Camera
> - * \param[in] camera The camera whose data to retrieve
> - * \sa cameraData()
> - * \return A const pointer to the pipeline-specific data passed to
> - * registerCamera().
> - */
> -const CameraData *PipelineHandler::cameraData(const Camera *camera) const
> -{
> - ASSERT(cameraData_.count(camera));
> - return cameraData_.at(camera).get();
> -}
> -
> /**
> * \var PipelineHandler::manager_
> * \brief The Camera manager associated with the pipeline handler
> --
> Regards,
>
> Laurent Pinchart
>
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list