[{"id":18675,"web_url":"https://patchwork.libcamera.org/comment/18675/","msgid":"<20210810125344.nmbmjz7jvrk6pkdp@uno.localdomain>","date":"2021-08-10T12:53:44","subject":"Re: [libcamera-devel] [PATCH v2 01/11] libcamera: camera: Pass\n\tPrivate pointer to Camera constructor","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Aug 05, 2021 at 08:58:38PM +0300, Laurent Pinchart wrote:\n> In order to allow subclassing Camera::Private in pipeline handlers, pass\n> the pointer to the private data to the Camera constructor, and to the\n> Camera::createCamera() function.\n>\n> The Camera::Private id_ and streams_ members now need to be initialized\n> by the Camera constructor instead of the Camera::Private constructor, to\n> allow storage of the streams in a pipeline handler-specific subclass of\n> Camera::Private.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  include/libcamera/camera.h                    |  5 ++--\n>  include/libcamera/internal/camera.h           |  3 +--\n>  src/libcamera/camera.cpp                      | 26 ++++++++++++-------\n>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  4 ++-\n>  .../pipeline/raspberrypi/raspberrypi.cpp      |  4 ++-\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  4 ++-\n>  src/libcamera/pipeline/simple/simple.cpp      |  4 ++-\n>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  4 ++-\n>  src/libcamera/pipeline/vimc/vimc.cpp          |  4 ++-\n>  9 files changed, 37 insertions(+), 21 deletions(-)\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index b081907e0cb1..17ddddc2722a 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -78,8 +78,7 @@ class Camera final : public Object, public std::enable_shared_from_this<Camera>,\n>  \tLIBCAMERA_DECLARE_PRIVATE()\n>\n>  public:\n> -\tstatic std::shared_ptr<Camera> create(PipelineHandler *pipe,\n> -\t\t\t\t\t      const std::string &id,\n> +\tstatic std::shared_ptr<Camera> create(Private *d, const std::string &id,\n>  \t\t\t\t\t      const std::set<Stream *> &streams);\n>\n>  \tconst std::string &id() const;\n> @@ -107,7 +106,7 @@ public:\n>  private:\n>  \tLIBCAMERA_DISABLE_COPY(Camera)\n>\n> -\tCamera(PipelineHandler *pipe, const std::string &id,\n> +\tCamera(Private *d, const std::string &id,\n\nAs discussed offline, is passing a raw pointer the best solution here ?\n\nIf the creation of the Private instance is now disjointed from the\nconstruction of the Extensible derived class, shouldn't its lifetime\nbe managed by the class that constructed it, or rather moved here.\n\nThis probably does not only affect Camera but it might apply to\nthe Extensible in full.\n\nThanks\n   j\n\n>  \t       const std::set<Stream *> &streams);\n>  \t~Camera();\n>\n> diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h\n> index b60ed140356a..9ec8321a9a21 100644\n> --- a/include/libcamera/internal/camera.h\n> +++ b/include/libcamera/internal/camera.h\n> @@ -26,8 +26,7 @@ class Camera::Private : public Extensible::Private\n>  \tLIBCAMERA_DECLARE_PUBLIC(Camera)\n>\n>  public:\n> -\tPrivate(PipelineHandler *pipe, const std::string &id,\n> -\t\tconst std::set<Stream *> &streams);\n> +\tPrivate(PipelineHandler *pipe);\n>  \t~Private();\n>\n>  private:\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 4b5bc891fc37..a5bb60c698bc 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -332,11 +332,13 @@ std::size_t CameraConfiguration::size() const\n>   * \\brief The vector of stream configurations\n>   */\n>\n> -Camera::Private::Private(PipelineHandler *pipe,\n> -\t\t\t const std::string &id,\n> -\t\t\t const std::set<Stream *> &streams)\n> -\t: pipe_(pipe->shared_from_this()), id_(id), streams_(streams),\n> -\t  disconnected_(false), state_(CameraAvailable)\n> +/**\n> + * \\brief Construct a Camera::Private instance\n> + * \\param[in] pipe The pipeline handler responsible for the camera device\n> + */\n> +Camera::Private::Private(PipelineHandler *pipe)\n> +\t: pipe_(pipe->shared_from_this()), disconnected_(false),\n> +\t  state_(CameraAvailable)\n>  {\n>  }\n>\n> @@ -513,7 +515,7 @@ void Camera::Private::setState(State state)\n>\n>  /**\n>   * \\brief Create a camera instance\n> - * \\param[in] pipe The pipeline handler responsible for the camera device\n> + * \\param[in] d Camera private data\n>   * \\param[in] id The ID of the camera device\n>   * \\param[in] streams Array of streams the camera provides\n>   *\n> @@ -527,10 +529,12 @@ void Camera::Private::setState(State state)\n>   *\n>   * \\return A shared pointer to the newly created camera object\n>   */\n> -std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,\n> +std::shared_ptr<Camera> Camera::create(Private *d,\n>  \t\t\t\t       const std::string &id,\n>  \t\t\t\t       const std::set<Stream *> &streams)\n>  {\n> +\tASSERT(d);\n> +\n>  \tstruct Deleter : std::default_delete<Camera> {\n>  \t\tvoid operator()(Camera *camera)\n>  \t\t{\n> @@ -541,7 +545,7 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,\n>  \t\t}\n>  \t};\n>\n> -\tCamera *camera = new Camera(pipe, id, streams);\n> +\tCamera *camera = new Camera(d, id, streams);\n>\n>  \treturn std::shared_ptr<Camera>(camera, Deleter());\n>  }\n> @@ -594,10 +598,12 @@ const std::string &Camera::id() const\n>   * application API calls by returning errors immediately.\n>   */\n>\n> -Camera::Camera(PipelineHandler *pipe, const std::string &id,\n> +Camera::Camera(Private *d, const std::string &id,\n>  \t       const std::set<Stream *> &streams)\n> -\t: Extensible(new Private(pipe, id, streams))\n> +\t: Extensible(d)\n>  {\n> +\td->id_ = id;\n> +\td->streams_ = streams;\n>  }\n>\n>  Camera::~Camera()\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 76c3bb3d8aa9..15d6cca609ff 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -23,6 +23,7 @@\n>  #include <libcamera/request.h>\n>  #include <libcamera/stream.h>\n>\n> +#include \"libcamera/internal/camera.h\"\n>  #include \"libcamera/internal/camera_sensor.h\"\n>  #include \"libcamera/internal/delayed_controls.h\"\n>  #include \"libcamera/internal/device_enumerator.h\"\n> @@ -1185,7 +1186,8 @@ int PipelineHandlerIPU3::registerCameras()\n>  \t\t/* Create and register the Camera instance. */\n>  \t\tstd::string cameraId = cio2->sensor()->id();\n>  \t\tstd::shared_ptr<Camera> camera =\n> -\t\t\tCamera::create(this, cameraId, streams);\n> +\t\t\tCamera::create(new Camera::Private(this), cameraId,\n> +\t\t\t\t       streams);\n>\n>  \t\tregisterCamera(std::move(camera), std::move(data));\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 0bab3bedd402..33cd5765b52e 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -30,6 +30,7 @@\n>  #include <linux/videodev2.h>\n>\n>  #include \"libcamera/internal/bayer_format.h\"\n> +#include \"libcamera/internal/camera.h\"\n>  #include \"libcamera/internal/camera_sensor.h\"\n>  #include \"libcamera/internal/delayed_controls.h\"\n>  #include \"libcamera/internal/device_enumerator.h\"\n> @@ -1106,7 +1107,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n>\n>  \t/* Create and register the camera. */\n>  \tstd::shared_ptr<Camera> camera =\n> -\t\tCamera::create(this, data->sensor_->id(), streams);\n> +\t\tCamera::create(new Camera::Private(this), data->sensor_->id(),\n> +\t\t\t       streams);\n>  \tregisterCamera(std::move(camera), std::move(data));\n>\n>  \treturn true;\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 42911a8fdfdb..4a8ac97d5ef0 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -27,6 +27,7 @@\n>  #include <libcamera/request.h>\n>  #include <libcamera/stream.h>\n>\n> +#include \"libcamera/internal/camera.h\"\n>  #include \"libcamera/internal/camera_sensor.h\"\n>  #include \"libcamera/internal/delayed_controls.h\"\n>  #include \"libcamera/internal/device_enumerator.h\"\n> @@ -970,7 +971,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n>  \t\t&data->selfPathStream_,\n>  \t};\n>  \tstd::shared_ptr<Camera> camera =\n> -\t\tCamera::create(this, data->sensor_->id(), streams);\n> +\t\tCamera::create(new Camera::Private(this), data->sensor_->id(),\n> +\t\t\t       streams);\n>  \tregisterCamera(std::move(camera), std::move(data));\n>\n>  \treturn 0;\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index b29fff9820e5..43af3fafa475 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -28,6 +28,7 @@\n>  #include <libcamera/request.h>\n>  #include <libcamera/stream.h>\n>\n> +#include \"libcamera/internal/camera.h\"\n>  #include \"libcamera/internal/camera_sensor.h\"\n>  #include \"libcamera/internal/device_enumerator.h\"\n>  #include \"libcamera/internal/media_device.h\"\n> @@ -1046,7 +1047,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n>  \t\t\t       [](Stream &stream) { return &stream; });\n>\n>  \t\tstd::shared_ptr<Camera> camera =\n> -\t\t\tCamera::create(this, data->sensor_->id(), streams);\n> +\t\t\tCamera::create(new Camera::Private(this),\n> +\t\t\t\t       data->sensor_->id(), streams);\n>  \t\tregisterCamera(std::move(camera), std::move(data));\n>  \t\tregistered = true;\n>  \t}\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 0f634b8da609..63cb1daaae22 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -22,6 +22,7 @@\n>  #include <libcamera/request.h>\n>  #include <libcamera/stream.h>\n>\n> +#include \"libcamera/internal/camera.h\"\n>  #include \"libcamera/internal/device_enumerator.h\"\n>  #include \"libcamera/internal/media_device.h\"\n>  #include \"libcamera/internal/pipeline_handler.h\"\n> @@ -470,7 +471,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>  \t}\n>\n>  \tstd::set<Stream *> streams{ &data->stream_ };\n> -\tstd::shared_ptr<Camera> camera = Camera::create(this, id, streams);\n> +\tstd::shared_ptr<Camera> camera =\n> +\t\tCamera::create(new Camera::Private(this), id, streams);\n>  \tregisterCamera(std::move(camera), std::move(data));\n>\n>  \t/* Enable hot-unplug notifications. */\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 12f7517fd0ae..d63562b1ee54 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -29,6 +29,7 @@\n>  #include <libcamera/ipa/vimc_ipa_interface.h>\n>  #include <libcamera/ipa/vimc_ipa_proxy.h>\n>\n> +#include \"libcamera/internal/camera.h\"\n>  #include \"libcamera/internal/camera_sensor.h\"\n>  #include \"libcamera/internal/device_enumerator.h\"\n>  #include \"libcamera/internal/ipa_manager.h\"\n> @@ -438,7 +439,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n>  \t/* Create and register the camera. */\n>  \tstd::set<Stream *> streams{ &data->stream_ };\n>  \tstd::shared_ptr<Camera> camera =\n> -\t\tCamera::create(this, data->sensor_->id(), streams);\n> +\t\tCamera::create(new Camera::Private(this), data->sensor_->id(),\n> +\t\t\t       streams);\n>  \tregisterCamera(std::move(camera), std::move(data));\n>\n>  \treturn true;\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F014CC3240\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 10 Aug 2021 12:52:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D2B16884D;\n\tTue, 10 Aug 2021 14:52:58 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 89749687F0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 10 Aug 2021 14:52:57 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 086F520005;\n\tTue, 10 Aug 2021 12:52:56 +0000 (UTC)"],"Date":"Tue, 10 Aug 2021 14:53:44 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210810125344.nmbmjz7jvrk6pkdp@uno.localdomain>","References":"<20210805175848.24188-1-laurent.pinchart@ideasonboard.com>\n\t<20210805175848.24188-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210805175848.24188-2-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 01/11] libcamera: camera: Pass\n\tPrivate pointer to Camera constructor","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18707,"web_url":"https://patchwork.libcamera.org/comment/18707/","msgid":"<YRQASb7pE7XO4w03@pendragon.ideasonboard.com>","date":"2021-08-11T16:52:25","subject":"Re: [libcamera-devel] [PATCH v2 01/11] libcamera: camera: Pass\n\tPrivate pointer to Camera constructor","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Aug 10, 2021 at 02:53:44PM +0200, Jacopo Mondi wrote:\n> On Thu, Aug 05, 2021 at 08:58:38PM +0300, Laurent Pinchart wrote:\n> > In order to allow subclassing Camera::Private in pipeline handlers, pass\n> > the pointer to the private data to the Camera constructor, and to the\n> > Camera::createCamera() function.\n> >\n> > The Camera::Private id_ and streams_ members now need to be initialized\n> > by the Camera constructor instead of the Camera::Private constructor, to\n> > allow storage of the streams in a pipeline handler-specific subclass of\n> > Camera::Private.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  include/libcamera/camera.h                    |  5 ++--\n> >  include/libcamera/internal/camera.h           |  3 +--\n> >  src/libcamera/camera.cpp                      | 26 ++++++++++++-------\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp          |  4 ++-\n> >  .../pipeline/raspberrypi/raspberrypi.cpp      |  4 ++-\n> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  4 ++-\n> >  src/libcamera/pipeline/simple/simple.cpp      |  4 ++-\n> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  4 ++-\n> >  src/libcamera/pipeline/vimc/vimc.cpp          |  4 ++-\n> >  9 files changed, 37 insertions(+), 21 deletions(-)\n> >\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index b081907e0cb1..17ddddc2722a 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -78,8 +78,7 @@ class Camera final : public Object, public std::enable_shared_from_this<Camera>,\n> >  \tLIBCAMERA_DECLARE_PRIVATE()\n> >\n> >  public:\n> > -\tstatic std::shared_ptr<Camera> create(PipelineHandler *pipe,\n> > -\t\t\t\t\t      const std::string &id,\n> > +\tstatic std::shared_ptr<Camera> create(Private *d, const std::string &id,\n> >  \t\t\t\t\t      const std::set<Stream *> &streams);\n> >\n> >  \tconst std::string &id() const;\n> > @@ -107,7 +106,7 @@ public:\n> >  private:\n> >  \tLIBCAMERA_DISABLE_COPY(Camera)\n> >\n> > -\tCamera(PipelineHandler *pipe, const std::string &id,\n> > +\tCamera(Private *d, const std::string &id,\n> \n> As discussed offline, is passing a raw pointer the best solution here ?\n\nAbsolutely not :-) I'll add a new patch at the bottom of this series to\nreplace the Private * passed to the Extensible constructor with a\nstd::unique_ptr<Private>. The rest of the series will be adapted\naccordingly.\n\n> If the creation of the Private instance is now disjointed from the\n> construction of the Extensible derived class, shouldn't its lifetime\n> be managed by the class that constructed it, or rather moved here.\n\nYes, it should be moved. Managing it in the caller would be cumbersome\nand error prone.\n\n> This probably does not only affect Camera but it might apply to\n> the Extensible in full.\n> \n> >  \t       const std::set<Stream *> &streams);\n> >  \t~Camera();\n> >\n> > diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h\n> > index b60ed140356a..9ec8321a9a21 100644\n> > --- a/include/libcamera/internal/camera.h\n> > +++ b/include/libcamera/internal/camera.h\n> > @@ -26,8 +26,7 @@ class Camera::Private : public Extensible::Private\n> >  \tLIBCAMERA_DECLARE_PUBLIC(Camera)\n> >\n> >  public:\n> > -\tPrivate(PipelineHandler *pipe, const std::string &id,\n> > -\t\tconst std::set<Stream *> &streams);\n> > +\tPrivate(PipelineHandler *pipe);\n> >  \t~Private();\n> >\n> >  private:\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 4b5bc891fc37..a5bb60c698bc 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -332,11 +332,13 @@ std::size_t CameraConfiguration::size() const\n> >   * \\brief The vector of stream configurations\n> >   */\n> >\n> > -Camera::Private::Private(PipelineHandler *pipe,\n> > -\t\t\t const std::string &id,\n> > -\t\t\t const std::set<Stream *> &streams)\n> > -\t: pipe_(pipe->shared_from_this()), id_(id), streams_(streams),\n> > -\t  disconnected_(false), state_(CameraAvailable)\n> > +/**\n> > + * \\brief Construct a Camera::Private instance\n> > + * \\param[in] pipe The pipeline handler responsible for the camera device\n> > + */\n> > +Camera::Private::Private(PipelineHandler *pipe)\n> > +\t: pipe_(pipe->shared_from_this()), disconnected_(false),\n> > +\t  state_(CameraAvailable)\n> >  {\n> >  }\n> >\n> > @@ -513,7 +515,7 @@ void Camera::Private::setState(State state)\n> >\n> >  /**\n> >   * \\brief Create a camera instance\n> > - * \\param[in] pipe The pipeline handler responsible for the camera device\n> > + * \\param[in] d Camera private data\n> >   * \\param[in] id The ID of the camera device\n> >   * \\param[in] streams Array of streams the camera provides\n> >   *\n> > @@ -527,10 +529,12 @@ void Camera::Private::setState(State state)\n> >   *\n> >   * \\return A shared pointer to the newly created camera object\n> >   */\n> > -std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,\n> > +std::shared_ptr<Camera> Camera::create(Private *d,\n> >  \t\t\t\t       const std::string &id,\n> >  \t\t\t\t       const std::set<Stream *> &streams)\n> >  {\n> > +\tASSERT(d);\n> > +\n> >  \tstruct Deleter : std::default_delete<Camera> {\n> >  \t\tvoid operator()(Camera *camera)\n> >  \t\t{\n> > @@ -541,7 +545,7 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,\n> >  \t\t}\n> >  \t};\n> >\n> > -\tCamera *camera = new Camera(pipe, id, streams);\n> > +\tCamera *camera = new Camera(d, id, streams);\n> >\n> >  \treturn std::shared_ptr<Camera>(camera, Deleter());\n> >  }\n> > @@ -594,10 +598,12 @@ const std::string &Camera::id() const\n> >   * application API calls by returning errors immediately.\n> >   */\n> >\n> > -Camera::Camera(PipelineHandler *pipe, const std::string &id,\n> > +Camera::Camera(Private *d, const std::string &id,\n> >  \t       const std::set<Stream *> &streams)\n> > -\t: Extensible(new Private(pipe, id, streams))\n> > +\t: Extensible(d)\n> >  {\n> > +\td->id_ = id;\n> > +\td->streams_ = streams;\n> >  }\n> >\n> >  Camera::~Camera()\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 76c3bb3d8aa9..15d6cca609ff 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -23,6 +23,7 @@\n> >  #include <libcamera/request.h>\n> >  #include <libcamera/stream.h>\n> >\n> > +#include \"libcamera/internal/camera.h\"\n> >  #include \"libcamera/internal/camera_sensor.h\"\n> >  #include \"libcamera/internal/delayed_controls.h\"\n> >  #include \"libcamera/internal/device_enumerator.h\"\n> > @@ -1185,7 +1186,8 @@ int PipelineHandlerIPU3::registerCameras()\n> >  \t\t/* Create and register the Camera instance. */\n> >  \t\tstd::string cameraId = cio2->sensor()->id();\n> >  \t\tstd::shared_ptr<Camera> camera =\n> > -\t\t\tCamera::create(this, cameraId, streams);\n> > +\t\t\tCamera::create(new Camera::Private(this), cameraId,\n> > +\t\t\t\t       streams);\n> >\n> >  \t\tregisterCamera(std::move(camera), std::move(data));\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 0bab3bedd402..33cd5765b52e 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -30,6 +30,7 @@\n> >  #include <linux/videodev2.h>\n> >\n> >  #include \"libcamera/internal/bayer_format.h\"\n> > +#include \"libcamera/internal/camera.h\"\n> >  #include \"libcamera/internal/camera_sensor.h\"\n> >  #include \"libcamera/internal/delayed_controls.h\"\n> >  #include \"libcamera/internal/device_enumerator.h\"\n> > @@ -1106,7 +1107,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> >\n> >  \t/* Create and register the camera. */\n> >  \tstd::shared_ptr<Camera> camera =\n> > -\t\tCamera::create(this, data->sensor_->id(), streams);\n> > +\t\tCamera::create(new Camera::Private(this), data->sensor_->id(),\n> > +\t\t\t       streams);\n> >  \tregisterCamera(std::move(camera), std::move(data));\n> >\n> >  \treturn true;\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 42911a8fdfdb..4a8ac97d5ef0 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -27,6 +27,7 @@\n> >  #include <libcamera/request.h>\n> >  #include <libcamera/stream.h>\n> >\n> > +#include \"libcamera/internal/camera.h\"\n> >  #include \"libcamera/internal/camera_sensor.h\"\n> >  #include \"libcamera/internal/delayed_controls.h\"\n> >  #include \"libcamera/internal/device_enumerator.h\"\n> > @@ -970,7 +971,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> >  \t\t&data->selfPathStream_,\n> >  \t};\n> >  \tstd::shared_ptr<Camera> camera =\n> > -\t\tCamera::create(this, data->sensor_->id(), streams);\n> > +\t\tCamera::create(new Camera::Private(this), data->sensor_->id(),\n> > +\t\t\t       streams);\n> >  \tregisterCamera(std::move(camera), std::move(data));\n> >\n> >  \treturn 0;\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index b29fff9820e5..43af3fafa475 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -28,6 +28,7 @@\n> >  #include <libcamera/request.h>\n> >  #include <libcamera/stream.h>\n> >\n> > +#include \"libcamera/internal/camera.h\"\n> >  #include \"libcamera/internal/camera_sensor.h\"\n> >  #include \"libcamera/internal/device_enumerator.h\"\n> >  #include \"libcamera/internal/media_device.h\"\n> > @@ -1046,7 +1047,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> >  \t\t\t       [](Stream &stream) { return &stream; });\n> >\n> >  \t\tstd::shared_ptr<Camera> camera =\n> > -\t\t\tCamera::create(this, data->sensor_->id(), streams);\n> > +\t\t\tCamera::create(new Camera::Private(this),\n> > +\t\t\t\t       data->sensor_->id(), streams);\n> >  \t\tregisterCamera(std::move(camera), std::move(data));\n> >  \t\tregistered = true;\n> >  \t}\n> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 0f634b8da609..63cb1daaae22 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -22,6 +22,7 @@\n> >  #include <libcamera/request.h>\n> >  #include <libcamera/stream.h>\n> >\n> > +#include \"libcamera/internal/camera.h\"\n> >  #include \"libcamera/internal/device_enumerator.h\"\n> >  #include \"libcamera/internal/media_device.h\"\n> >  #include \"libcamera/internal/pipeline_handler.h\"\n> > @@ -470,7 +471,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> >  \t}\n> >\n> >  \tstd::set<Stream *> streams{ &data->stream_ };\n> > -\tstd::shared_ptr<Camera> camera = Camera::create(this, id, streams);\n> > +\tstd::shared_ptr<Camera> camera =\n> > +\t\tCamera::create(new Camera::Private(this), id, streams);\n> >  \tregisterCamera(std::move(camera), std::move(data));\n> >\n> >  \t/* Enable hot-unplug notifications. */\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index 12f7517fd0ae..d63562b1ee54 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -29,6 +29,7 @@\n> >  #include <libcamera/ipa/vimc_ipa_interface.h>\n> >  #include <libcamera/ipa/vimc_ipa_proxy.h>\n> >\n> > +#include \"libcamera/internal/camera.h\"\n> >  #include \"libcamera/internal/camera_sensor.h\"\n> >  #include \"libcamera/internal/device_enumerator.h\"\n> >  #include \"libcamera/internal/ipa_manager.h\"\n> > @@ -438,7 +439,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> >  \t/* Create and register the camera. */\n> >  \tstd::set<Stream *> streams{ &data->stream_ };\n> >  \tstd::shared_ptr<Camera> camera =\n> > -\t\tCamera::create(this, data->sensor_->id(), streams);\n> > +\t\tCamera::create(new Camera::Private(this), data->sensor_->id(),\n> > +\t\t\t       streams);\n> >  \tregisterCamera(std::move(camera), std::move(data));\n> >\n> >  \treturn true;","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 14801C3240\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 11 Aug 2021 16:52:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4E6F76884F;\n\tWed, 11 Aug 2021 18:52:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E69268826\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 11 Aug 2021 18:52:28 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2636DEE;\n\tWed, 11 Aug 2021 18:52:28 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OR6xFWdO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628700748;\n\tbh=kg4daK08cyujE/iZRwofoF5EDvSl8Iw7F7BJvM37bTI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OR6xFWdO0FYEp5toAThkngZ+CqIzwBzbTh+mJWmt2ayLdO25X119u3/+9TWA7ibr4\n\tn7b4x4FdJ7uOztmGP+U9Ty6fBpZLiXcM6mlUa1SFvPVySpxFoVCJbZimhewBnz1I3X\n\toD1WDHzg6MKgLRzJBvXv4AZN98xSIPZpXAxMLIxU=","Date":"Wed, 11 Aug 2021 19:52:25 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YRQASb7pE7XO4w03@pendragon.ideasonboard.com>","References":"<20210805175848.24188-1-laurent.pinchart@ideasonboard.com>\n\t<20210805175848.24188-2-laurent.pinchart@ideasonboard.com>\n\t<20210810125344.nmbmjz7jvrk6pkdp@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210810125344.nmbmjz7jvrk6pkdp@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 01/11] libcamera: camera: Pass\n\tPrivate pointer to Camera constructor","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]