| Message ID | 20210805175848.24188-2-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Thu, Aug 05, 2021 at 08:58:38PM +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> > --- > include/libcamera/camera.h | 5 ++-- > 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 | 4 ++- > src/libcamera/pipeline/vimc/vimc.cpp | 4 ++- > 9 files changed, 37 insertions(+), 21 deletions(-) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index b081907e0cb1..17ddddc2722a 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -78,8 +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, > - const std::string &id, > + static std::shared_ptr<Camera> create(Private *d, const std::string &id, > const std::set<Stream *> &streams); > > const std::string &id() const; > @@ -107,7 +106,7 @@ public: > private: > LIBCAMERA_DISABLE_COPY(Camera) > > - Camera(PipelineHandler *pipe, const std::string &id, > + Camera(Private *d, const std::string &id, As discussed offline, is passing a raw pointer the best solution here ? If the creation of the Private instance is now disjointed from the construction of the Extensible derived class, shouldn't its lifetime be managed by the class that constructed it, or rather moved here. This probably does not only affect Camera but it might apply to the Extensible in full. Thanks j > 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 4b5bc891fc37..a5bb60c698bc 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(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(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(Private *d, const std::string &id, > const std::set<Stream *> &streams) > - : Extensible(new Private(pipe, id, streams)) > + : Extensible(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 76c3bb3d8aa9..15d6cca609ff 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" > @@ -1185,7 +1186,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(new 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 0bab3bedd402..33cd5765b52e 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -30,6 +30,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" > @@ -1106,7 +1107,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(new 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..4a8ac97d5ef0 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(new 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..43af3fafa475 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(new 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..63cb1daaae22 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,8 @@ 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(new 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..d63562b1ee54 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(new Camera::Private(this), data->sensor_->id(), > + streams); > registerCamera(std::move(camera), std::move(data)); > > return true; > -- > Regards, > > Laurent Pinchart >
Hi Jacopo, On Tue, Aug 10, 2021 at 02:53:44PM +0200, Jacopo Mondi wrote: > On Thu, Aug 05, 2021 at 08:58:38PM +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> > > --- > > include/libcamera/camera.h | 5 ++-- > > 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 | 4 ++- > > src/libcamera/pipeline/vimc/vimc.cpp | 4 ++- > > 9 files changed, 37 insertions(+), 21 deletions(-) > > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > index b081907e0cb1..17ddddc2722a 100644 > > --- a/include/libcamera/camera.h > > +++ b/include/libcamera/camera.h > > @@ -78,8 +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, > > - const std::string &id, > > + static std::shared_ptr<Camera> create(Private *d, const std::string &id, > > const std::set<Stream *> &streams); > > > > const std::string &id() const; > > @@ -107,7 +106,7 @@ public: > > private: > > LIBCAMERA_DISABLE_COPY(Camera) > > > > - Camera(PipelineHandler *pipe, const std::string &id, > > + Camera(Private *d, const std::string &id, > > As discussed offline, is passing a raw pointer the best solution here ? Absolutely not :-) I'll add a new patch at the bottom of this series to replace the Private * passed to the Extensible constructor with a std::unique_ptr<Private>. The rest of the series will be adapted accordingly. > If the creation of the Private instance is now disjointed from the > construction of the Extensible derived class, shouldn't its lifetime > be managed by the class that constructed it, or rather moved here. Yes, it should be moved. Managing it in the caller would be cumbersome and error prone. > This probably does not only affect Camera but it might apply to > the Extensible in full. > > > 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 4b5bc891fc37..a5bb60c698bc 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(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(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(Private *d, const std::string &id, > > const std::set<Stream *> &streams) > > - : Extensible(new Private(pipe, id, streams)) > > + : Extensible(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 76c3bb3d8aa9..15d6cca609ff 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" > > @@ -1185,7 +1186,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(new 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 0bab3bedd402..33cd5765b52e 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -30,6 +30,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" > > @@ -1106,7 +1107,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(new 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..4a8ac97d5ef0 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(new 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..43af3fafa475 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(new 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..63cb1daaae22 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,8 @@ 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(new 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..d63562b1ee54 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(new Camera::Private(this), data->sensor_->id(), > > + streams); > > registerCamera(std::move(camera), std::move(data)); > > > > return true;
diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index b081907e0cb1..17ddddc2722a 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -78,8 +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, - const std::string &id, + static std::shared_ptr<Camera> create(Private *d, const std::string &id, const std::set<Stream *> &streams); const std::string &id() const; @@ -107,7 +106,7 @@ public: private: LIBCAMERA_DISABLE_COPY(Camera) - Camera(PipelineHandler *pipe, const std::string &id, + Camera(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 4b5bc891fc37..a5bb60c698bc 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(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(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(Private *d, const std::string &id, const std::set<Stream *> &streams) - : Extensible(new Private(pipe, id, streams)) + : Extensible(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 76c3bb3d8aa9..15d6cca609ff 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" @@ -1185,7 +1186,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(new 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 0bab3bedd402..33cd5765b52e 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -30,6 +30,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" @@ -1106,7 +1107,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(new 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..4a8ac97d5ef0 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(new 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..43af3fafa475 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(new 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..63cb1daaae22 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,8 @@ 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(new 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..d63562b1ee54 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(new Camera::Private(this), data->sensor_->id(), + streams); registerCamera(std::move(camera), std::move(data)); return true;