[libcamera-devel] [PATCH v3 02/14] libcamera: camera: Pass Private pointer to Camera constructor
Jacopo Mondi
jacopo at jmondi.org
Thu Aug 12 10:12:56 CEST 2021
Hi Laurent,
On Thu, Aug 12, 2021 at 02:26:13AM +0300, Laurent Pinchart wrote:
> In order to allow subclassing Camera::Private in pipeline handlers, pass
> the pointer to the private data to the Camera constructor, and to the
> Camera::createCamera() function.
>
> The Camera::Private id_ and streams_ members now need to be initialized
> by the Camera constructor instead of the Camera::Private constructor, to
> allow storage of the streams in a pipeline handler-specific subclass of
> Camera::Private.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> ---
> include/libcamera/camera.h | 4 +--
> include/libcamera/internal/camera.h | 3 +--
> src/libcamera/camera.cpp | 26 ++++++++++++-------
> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++-
> .../pipeline/raspberrypi/raspberrypi.cpp | 4 ++-
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-
> src/libcamera/pipeline/simple/simple.cpp | 4 ++-
> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 5 +++-
> src/libcamera/pipeline/vimc/vimc.cpp | 4 ++-
> 9 files changed, 38 insertions(+), 20 deletions(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index b081907e0cb1..05cdab724b10 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -78,7 +78,7 @@ class Camera final : public Object, public std::enable_shared_from_this<Camera>,
> LIBCAMERA_DECLARE_PRIVATE()
>
> public:
> - static std::shared_ptr<Camera> create(PipelineHandler *pipe,
> + static std::shared_ptr<Camera> create(std::unique_ptr<Private> d,
> const std::string &id,
> const std::set<Stream *> &streams);
>
> @@ -107,7 +107,7 @@ public:
> private:
> LIBCAMERA_DISABLE_COPY(Camera)
>
> - Camera(PipelineHandler *pipe, const std::string &id,
> + Camera(std::unique_ptr<Private> d, const std::string &id,
> const std::set<Stream *> &streams);
> ~Camera();
>
> diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
> index b60ed140356a..9ec8321a9a21 100644
> --- a/include/libcamera/internal/camera.h
> +++ b/include/libcamera/internal/camera.h
> @@ -26,8 +26,7 @@ class Camera::Private : public Extensible::Private
> LIBCAMERA_DECLARE_PUBLIC(Camera)
>
> public:
> - Private(PipelineHandler *pipe, const std::string &id,
> - const std::set<Stream *> &streams);
> + Private(PipelineHandler *pipe);
> ~Private();
>
> private:
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 8e99e2a96eaf..d9f6b784e0dc 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -332,11 +332,13 @@ std::size_t CameraConfiguration::size() const
> * \brief The vector of stream configurations
> */
>
> -Camera::Private::Private(PipelineHandler *pipe,
> - const std::string &id,
> - const std::set<Stream *> &streams)
> - : pipe_(pipe->shared_from_this()), id_(id), streams_(streams),
> - disconnected_(false), state_(CameraAvailable)
> +/**
> + * \brief Construct a Camera::Private instance
> + * \param[in] pipe The pipeline handler responsible for the camera device
> + */
> +Camera::Private::Private(PipelineHandler *pipe)
> + : pipe_(pipe->shared_from_this()), disconnected_(false),
> + state_(CameraAvailable)
> {
> }
>
> @@ -513,7 +515,7 @@ void Camera::Private::setState(State state)
>
> /**
> * \brief Create a camera instance
> - * \param[in] pipe The pipeline handler responsible for the camera device
> + * \param[in] d Camera private data
> * \param[in] id The ID of the camera device
> * \param[in] streams Array of streams the camera provides
> *
> @@ -527,10 +529,12 @@ void Camera::Private::setState(State state)
> *
> * \return A shared pointer to the newly created camera object
> */
> -std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
> +std::shared_ptr<Camera> Camera::create(std::unique_ptr<Private> d,
> const std::string &id,
> const std::set<Stream *> &streams)
> {
> + ASSERT(d);
> +
> struct Deleter : std::default_delete<Camera> {
> void operator()(Camera *camera)
> {
> @@ -541,7 +545,7 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
> }
> };
>
> - Camera *camera = new Camera(pipe, id, streams);
> + Camera *camera = new Camera(std::move(d), id, streams);
>
> return std::shared_ptr<Camera>(camera, Deleter());
> }
> @@ -594,10 +598,12 @@ const std::string &Camera::id() const
> * application API calls by returning errors immediately.
> */
>
> -Camera::Camera(PipelineHandler *pipe, const std::string &id,
> +Camera::Camera(std::unique_ptr<Private> d, const std::string &id,
> const std::set<Stream *> &streams)
> - : Extensible(std::make_unique<Private>(pipe, id, streams))
> + : Extensible(std::move(d))
> {
> + _d()->id_ = id;
> + _d()->streams_ = streams;
> }
>
> Camera::~Camera()
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 19cb5c4ec9c3..b9c0941c5ea8 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -23,6 +23,7 @@
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> +#include "libcamera/internal/camera.h"
> #include "libcamera/internal/camera_sensor.h"
> #include "libcamera/internal/delayed_controls.h"
> #include "libcamera/internal/device_enumerator.h"
> @@ -1199,7 +1200,8 @@ int PipelineHandlerIPU3::registerCameras()
> /* Create and register the Camera instance. */
> std::string cameraId = cio2->sensor()->id();
> std::shared_ptr<Camera> camera =
> - Camera::create(this, cameraId, streams);
> + Camera::create(std::make_unique<Camera::Private>(this),
> + cameraId, streams);
>
> registerCamera(std::move(camera), std::move(data));
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 2e8774ae360d..91e906ef14ce 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -29,6 +29,7 @@
> #include <linux/videodev2.h>
>
> #include "libcamera/internal/bayer_format.h"
> +#include "libcamera/internal/camera.h"
> #include "libcamera/internal/camera_sensor.h"
> #include "libcamera/internal/delayed_controls.h"
> #include "libcamera/internal/device_enumerator.h"
> @@ -1105,7 +1106,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>
> /* Create and register the camera. */
> std::shared_ptr<Camera> camera =
> - Camera::create(this, data->sensor_->id(), streams);
> + Camera::create(std::make_unique<Camera::Private>(this),
> + data->sensor_->id(), streams);
> registerCamera(std::move(camera), std::move(data));
>
> return true;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 42911a8fdfdb..c2872289337b 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -27,6 +27,7 @@
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> +#include "libcamera/internal/camera.h"
> #include "libcamera/internal/camera_sensor.h"
> #include "libcamera/internal/delayed_controls.h"
> #include "libcamera/internal/device_enumerator.h"
> @@ -970,7 +971,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> &data->selfPathStream_,
> };
> std::shared_ptr<Camera> camera =
> - Camera::create(this, data->sensor_->id(), streams);
> + Camera::create(std::make_unique<Camera::Private>(this),
> + data->sensor_->id(), streams);
> registerCamera(std::move(camera), std::move(data));
>
> return 0;
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index b29fff9820e5..ad581969fca1 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -28,6 +28,7 @@
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> +#include "libcamera/internal/camera.h"
> #include "libcamera/internal/camera_sensor.h"
> #include "libcamera/internal/device_enumerator.h"
> #include "libcamera/internal/media_device.h"
> @@ -1046,7 +1047,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> [](Stream &stream) { return &stream; });
>
> std::shared_ptr<Camera> camera =
> - Camera::create(this, data->sensor_->id(), streams);
> + Camera::create(std::make_unique<Camera::Private>(this),
> + data->sensor_->id(), streams);
> registerCamera(std::move(camera), std::move(data));
> registered = true;
> }
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 0f634b8da609..a9b6af4b5bdf 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -22,6 +22,7 @@
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> +#include "libcamera/internal/camera.h"
> #include "libcamera/internal/device_enumerator.h"
> #include "libcamera/internal/media_device.h"
> #include "libcamera/internal/pipeline_handler.h"
> @@ -470,7 +471,9 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> }
>
> std::set<Stream *> streams{ &data->stream_ };
> - std::shared_ptr<Camera> camera = Camera::create(this, id, streams);
> + std::shared_ptr<Camera> camera =
> + Camera::create(std::make_unique<Camera::Private>(this), id,
> + streams);
> registerCamera(std::move(camera), std::move(data));
>
> /* Enable hot-unplug notifications. */
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 12f7517fd0ae..999d975716fa 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -29,6 +29,7 @@
> #include <libcamera/ipa/vimc_ipa_interface.h>
> #include <libcamera/ipa/vimc_ipa_proxy.h>
>
> +#include "libcamera/internal/camera.h"
> #include "libcamera/internal/camera_sensor.h"
> #include "libcamera/internal/device_enumerator.h"
> #include "libcamera/internal/ipa_manager.h"
> @@ -438,7 +439,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
> /* Create and register the camera. */
> std::set<Stream *> streams{ &data->stream_ };
> std::shared_ptr<Camera> camera =
> - Camera::create(this, data->sensor_->id(), streams);
> + Camera::create(std::make_unique<Camera::Private>(this),
> + data->sensor_->id(), streams);
> registerCamera(std::move(camera), std::move(data));
>
> return true;
> --
> Regards,
>
> Laurent Pinchart
>
More information about the libcamera-devel
mailing list