| Message ID | 20210811232625.17280-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@ideasonboard.com> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> Reviewed-by: Jacopo Mondi <jacopo@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 >
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;