[{"id":18304,"web_url":"https://patchwork.libcamera.org/comment/18304/","msgid":"","date":"2021-07-24T07:09:22","subject":"Re: [libcamera-devel] [RFC PATCH 08/17] libcamera: camera: Pass\n\tPrivate pointer to Camera constructor","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2021-07-23 07:00:27 +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 \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,\n> \tLIBCAMERA_DECLARE_PRIVATE()\n> \n> public:\n> -\tstatic std::shared_ptr create(PipelineHandler *pipe,\n> -\t\t\t\t\t const std::string &id,\n> +\tstatic std::shared_ptr create(Private *d, const std::string &id,\n> \t\t\t\t\t const std::set &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> \t const std::set &streams);\n\nGoing forward would it make sens to move the streams (and other standard \nbut camera specific) property inside the Private class? I'm thinking it \ncould be useful in the core code to have standard properties available \nby the Camera id_. This is unrelated to this patch however and if \nthought to be a good idea should be done on-top.\n\nReviewed-by: Niklas Söderlund \n\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 &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 &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::create(PipelineHandler *pipe,\n> +std::shared_ptr Camera::create(Private *d,\n> \t\t\t\t const std::string &id,\n> \t\t\t\t const std::set &streams)\n> {\n> +\tASSERT(d);\n> +\n> \tstruct Deleter : std::default_delete {\n> \t\tvoid operator()(Camera *camera)\n> \t\t{\n> @@ -541,7 +545,7 @@ std::shared_ptr 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, 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 &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 \n> #include \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 =\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 f821d8fe1b6c..2411f73f48e0 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -30,6 +30,7 @@\n> #include \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 =\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 \n> #include \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 =\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 \n> #include \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 =\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 \n> #include \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 streams{ &data->stream_ };\n> -\tstd::shared_ptr camera = Camera::create(this, id, streams);\n> +\tstd::shared_ptr 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 \n> #include \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 streams{ &data->stream_ };\n> \tstd::shared_ptr 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":"","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 162CBC322C\n\tfor ;\n\tSat, 24 Jul 2021 07:09:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6ABEB687AC;\n\tSat, 24 Jul 2021 09:09:25 +0200 (CEST)","from mail-lf1-x133.google.com (mail-lf1-x133.google.com\n\t[IPv6:2a00:1450:4864:20::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9962468540\n\tfor ;\n\tSat, 24 Jul 2021 09:09:24 +0200 (CEST)","by mail-lf1-x133.google.com with SMTP id y34so5793716lfa.8\n\tfor ;\n\tSat, 24 Jul 2021 00:09:24 -0700 (PDT)","from localhost (h-46-59-88-219.A463.priv.bahnhof.se.\n\t[46.59.88.219]) by smtp.gmail.com with ESMTPSA id\n\tb3sm2425533lfb.109.2021.07.24.00.09.23\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 24 Jul 2021 00:09:23 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"J8Q6w7mH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=SAVq0Tcr8jRFalitM8SWQ4ABCJvHZRwLoYmP5W735KQ=;\n\tb=J8Q6w7mH5EErDx6W7lwpv7UbqR2sMMDZl5hCaIw9PNKT4r20b5OHN+acQgN1Z30KWW\n\tfLNxuvsEsAa6Cao05d0wgapyb+CT93VuRho4rihQocPsOe030wplgzCTpI9BIfbhU06m\n\t1kEt4pP8kA1cd0CPrr8eYEYztXF8yKhm2zulwAimtTltYAdRusoBS/9C6gR8zWCFqWlr\n\tRc+D+YUo7ediSHW++nWSE+3QNeKvaUtGL397iCPSbM+uDfpYX2RHwRaG27Ka4v6UX/8N\n\te90n0472i+CqdCEIeb0Z2cjb5Z+82lFNtlB+KNYVPO6vomHH8oO5716EGbSH7E+eVcC0\n\t++qg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=SAVq0Tcr8jRFalitM8SWQ4ABCJvHZRwLoYmP5W735KQ=;\n\tb=oWXh2s0dNe7cwwcGxOJo4EmxIE77ylisUgkJI5rUQHlIv1V1IgYR9OrUKyiblUQtAk\n\t0aMRjdWGH3bWTL0NN9kXjcOyPFifPL7gHWGc5slANT6tEQtc8hYimxFieUJs6qIwfg3/\n\tuvncqTlGw0RsZMwL2YnEC/efmc0cCRB7gGh6hONCFaRTBfMnQvHjT/Nztc8gUexmhfCe\n\tF2aJQ/uzrMq05EWs2RKOX49q8o9Wyq8jfrz2YRqQsqg3PNtN37i85dnYQn0p5RbFzV+a\n\tF4xjLHdLi1BB+QKKwWVuQssprZAu5lydTYcAPWJdv6e9qGJuBMS6B+jaimHkNF82cOtb\n\ty5Pw==","X-Gm-Message-State":"AOAM530UwclzJ44vybU+/T+zxZG5YgxCcERGrbBMGMn/+zB4q7wYz8bA\n\tShki84lJzTP5nnDypVPHfkgIMQ==","X-Google-Smtp-Source":"ABdhPJwI52yLMUPUZ11OzhFEe/uxy6neUSwskBn3aLJuf/vZHxJHrO7s+TbsBhqZYH7xDzXLVFK52Q==","X-Received":"by 2002:a19:f015:: with SMTP id\n\tp21mr5659306lfc.181.1627110563816; \n\tSat, 24 Jul 2021 00:09:23 -0700 (PDT)","Date":"Sat, 24 Jul 2021 09:09:22 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"","References":"<20210723040036.32346-1-laurent.pinchart@ideasonboard.com>\n\t<20210723040036.32346-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210723040036.32346-9-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 08/17] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18316,"web_url":"https://patchwork.libcamera.org/comment/18316/","msgid":"","date":"2021-07-24T19:00:10","subject":"Re: [libcamera-devel] [RFC PATCH 08/17] 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 Niklas,\n\nOn Sat, Jul 24, 2021 at 09:09:22AM +0200, Niklas Söderlund wrote:\n> On 2021-07-23 07:00:27 +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 \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,\n> > \tLIBCAMERA_DECLARE_PRIVATE()\n> > \n> > public:\n> > -\tstatic std::shared_ptr create(PipelineHandler *pipe,\n> > -\t\t\t\t\t const std::string &id,\n> > +\tstatic std::shared_ptr create(Private *d, const std::string &id,\n> > \t\t\t\t\t const std::set &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> > \t const std::set &streams);\n> \n> Going forward would it make sens to move the streams (and other standard \n> but camera specific) property inside the Private class? I'm thinking it \n> could be useful in the core code to have standard properties available \n> by the Camera id_. This is unrelated to this patch however and if \n> thought to be a good idea should be done on-top.\n\nIt's an idea I'm experimenting at the moment :-)\n\n> Reviewed-by: Niklas Söderlund \n> \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 &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 &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::create(PipelineHandler *pipe,\n> > +std::shared_ptr Camera::create(Private *d,\n> > \t\t\t\t const std::string &id,\n> > \t\t\t\t const std::set &streams)\n> > {\n> > +\tASSERT(d);\n> > +\n> > \tstruct Deleter : std::default_delete {\n> > \t\tvoid operator()(Camera *camera)\n> > \t\t{\n> > @@ -541,7 +545,7 @@ std::shared_ptr 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, 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 &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 \n> > #include \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 =\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 f821d8fe1b6c..2411f73f48e0 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -30,6 +30,7 @@\n> > #include \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 =\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 \n> > #include \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 =\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 \n> > #include \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 =\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 \n> > #include \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 streams{ &data->stream_ };\n> > -\tstd::shared_ptr camera = Camera::create(this, id, streams);\n> > +\tstd::shared_ptr 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 \n> > #include \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 streams{ &data->stream_ };\n> > \tstd::shared_ptr 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":"","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 C7BECC322C\n\tfor ;\n\tSat, 24 Jul 2021 19:00:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 270D9687AF;\n\tSat, 24 Jul 2021 21:00:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 535B568536\n\tfor ;\n\tSat, 24 Jul 2021 21:00:13 +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 CF9E83D7;\n\tSat, 24 Jul 2021 21:00:12 +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=\"Ydwevb/E\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627153213;\n\tbh=x4Sj6rsyUuPuIl+vEiHtplDiDRMvHwzlYWgVSsFETSs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ydwevb/EhcNbGyDUl6jex/G2ehNG7Ph/J1h+jie+wuqHcTus7khW4f+t86pi3r5ih\n\tvu3wUqnMlY1ArHCBxOXIDlikKnNu54sfTp+CUYJp0Vl2RZuiipfSC2czm4D/ICLjQN\n\t2lJTVsCLYMZBCepOzCwR1xU9/2cV23FVf/iJ0leM=","Date":"Sat, 24 Jul 2021 22:00:10 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"","References":"<20210723040036.32346-1-laurent.pinchart@ideasonboard.com>\n\t<20210723040036.32346-9-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH 08/17] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18429,"web_url":"https://patchwork.libcamera.org/comment/18429/","msgid":"<20210729210423.qroy3bodcv4tc4xn@uno.localdomain>","date":"2021-07-29T21:04:23","subject":"Re: [libcamera-devel] [RFC PATCH 08/17] 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 Fri, Jul 23, 2021 at 07:00:27AM +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\nAs suggested by Niklas, shouldn't stream and id be part of the Private\nclass and the public API rely on d_->streams() ?\n\nIt will expose streams and id to the internal API but that shouldn't\nbe an issue\n>\n> Signed-off-by: Laurent Pinchart \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,\n> \tLIBCAMERA_DECLARE_PRIVATE()\n>\n> public:\n> -\tstatic std::shared_ptr create(PipelineHandler *pipe,\n> -\t\t\t\t\t const std::string &id,\n> +\tstatic std::shared_ptr create(Private *d, const std::string &id,\n> \t\t\t\t\t const std::set &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> \t const std::set &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 &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 &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::create(PipelineHandler *pipe,\n> +std::shared_ptr Camera::create(Private *d,\n> \t\t\t\t const std::string &id,\n> \t\t\t\t const std::set &streams)\n> {\n> +\tASSERT(d);\n> +\n> \tstruct Deleter : std::default_delete {\n> \t\tvoid operator()(Camera *camera)\n> \t\t{\n> @@ -541,7 +545,7 @@ std::shared_ptr 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, 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 &streams)\n> -\t: Extensible(new Private(pipe, id, streams))\n> +\t: Extensible(d)\n> {\n> +\td->id_ = id;\no> +\td->streams_ = streams;\n\nThat's a consequence of the\n friend Extensible;\ndeclaration in Extensible::Private ?\n\nIn this case, it really makes the interface between the two classes\npermeable, I mean, as long as it's the public one that can access the\nprivate it's ok, but isn't it a too much of a shortcut that allow to\ncircumvent the proper definition of a Public/Private interface ?\n\nThanks\n j\n\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 \n> #include \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 =\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 f821d8fe1b6c..2411f73f48e0 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -30,6 +30,7 @@\n> #include \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 =\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 \n> #include \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 =\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 \n> #include \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 =\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 \n> #include \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 streams{ &data->stream_ };\n> -\tstd::shared_ptr camera = Camera::create(this, id, streams);\n> +\tstd::shared_ptr 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 \n> #include \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 streams{ &data->stream_ };\n> \tstd::shared_ptr 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":"","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 F26C5C322E\n\tfor ;\n\tThu, 29 Jul 2021 21:03:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B68C687BF;\n\tThu, 29 Jul 2021 23:03:38 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9DBEF687BA\n\tfor ;\n\tThu, 29 Jul 2021 23:03:37 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 883F940002;\n\tThu, 29 Jul 2021 21:03:36 +0000 (UTC)"],"Date":"Thu, 29 Jul 2021 23:04:23 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20210729210423.qroy3bodcv4tc4xn@uno.localdomain>","References":"<20210723040036.32346-1-laurent.pinchart@ideasonboard.com>\n\t<20210723040036.32346-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210723040036.32346-9-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 08/17] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18436,"web_url":"https://patchwork.libcamera.org/comment/18436/","msgid":"","date":"2021-07-30T02:49:20","subject":"Re: [libcamera-devel] [RFC PATCH 08/17] 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 Thu, Jul 29, 2021 at 11:04:23PM +0200, Jacopo Mondi wrote:\n> On Fri, Jul 23, 2021 at 07:00:27AM +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> As suggested by Niklas, shouldn't stream and id be part of the Private\n> class and the public API rely on d_->streams() ?\n\nThe id is already stored in the Camera::Private class. The pointers to\nthe streams are stored there too, but not the stream instances\nthemselves. This is what I believe Niklas was referring to, moving the\nactual stream storage from the CameraData subclasses (turned into\nCamera::Private subclasses in this series) into the Camera::Private\nclass itself. I've considered this, but it would prevent pipeline\nhandlers from subclassing the Stream class, which isn't nice. I've also\ntoyed with the idea of storing a std::vector> in\nCamera::Private, which would allow pipeline handlers to subclass Stream,\nbut would make Camera::Private manage the lifetime of stream objects. It\nended up requiring pipeline handlers to always cast from Stream to their\nStream subclass, and didn't really bring much added value.\n\n> It will expose streams and id to the internal API but that shouldn't\n> be an issue\n>\n> >\n> > Signed-off-by: Laurent Pinchart \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,\n> > \tLIBCAMERA_DECLARE_PRIVATE()\n> >\n> > public:\n> > -\tstatic std::shared_ptr create(PipelineHandler *pipe,\n> > -\t\t\t\t\t const std::string &id,\n> > +\tstatic std::shared_ptr create(Private *d, const std::string &id,\n> > \t\t\t\t\t const std::set &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> > \t const std::set &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 &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 &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::create(PipelineHandler *pipe,\n> > +std::shared_ptr Camera::create(Private *d,\n> > \t\t\t\t const std::string &id,\n> > \t\t\t\t const std::set &streams)\n> > {\n> > +\tASSERT(d);\n> > +\n> > \tstruct Deleter : std::default_delete {\n> > \t\tvoid operator()(Camera *camera)\n> > \t\t{\n> > @@ -541,7 +545,7 @@ std::shared_ptr 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, 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 &streams)\n> > -\t: Extensible(new Private(pipe, id, streams))\n> > +\t: Extensible(d)\n> > {\n> > +\td->id_ = id;\n> > +\td->streams_ = streams;\n> \n> That's a consequence of the\n> friend Extensible;\n> declaration in Extensible::Private ?\n\nCorrect.\n\n> In this case, it really makes the interface between the two classes\n> permeable, I mean, as long as it's the public one that can access the\n> private it's ok, but isn't it a too much of a shortcut that allow to\n> circumvent the proper definition of a Public/Private interface ?\n\nFrom the point of view of the Camera, the Camera and Camera::Private\nsplit is internal, and both classes form a whole. They can access each\nother, in the same way that functions of the Camera class could access\nall members, public, protected or private, if the Extensible mechanism\nwasn't used. The Camera and Camera::Private classes are tightly coupled,\ndeveloped and maintained together, are not used separately. I thus don't\nsee a need to artificially prevent them from accessing some members of\ntheir counterpart class.\n\nThe access specifiers to the Camera class are used to control what\nmembers can be accessed by users of the public API, and to some extent\nby internal users too. Similarly, the access specifiers to the\nCamera::Private class are used to control what members can be accessed\nby internal users.\n\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 \n> > #include \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 =\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 f821d8fe1b6c..2411f73f48e0 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -30,6 +30,7 @@\n> > #include \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 =\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 \n> > #include \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 =\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 \n> > #include \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 =\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 \n> > #include \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 streams{ &data->stream_ };\n> > -\tstd::shared_ptr camera = Camera::create(this, id, streams);\n> > +\tstd::shared_ptr 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 \n> > #include \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 streams{ &data->stream_ };\n> > \tstd::shared_ptr 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":"","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 713F1C3230\n\tfor ;\n\tFri, 30 Jul 2021 02:49:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EEB01687C4;\n\tFri, 30 Jul 2021 04:49:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 32032687BD\n\tfor ;\n\tFri, 30 Jul 2021 04:49: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 A8EC989B;\n\tFri, 30 Jul 2021 04:49:27 +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=\"koGPWvhq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627613367;\n\tbh=eyqUQxlH3z/50TwSR7TY8vzYDvQYUVyeYKmdCmNXzNA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=koGPWvhq+i7AiUYkVm1bkXwtFoJIocUEhIElXLmq1su7OzJnB9Pu2b3Hsxcz8Wymf\n\tFgvUJ7UwM4N4rpxKg+5cGkZPTN6A5svDpN7Fwoqdyg9usM/0d53Ie90usNEMvXqo2B\n\tyu8KN/GvZ0yGYdXlj9PQcTBVgMG0brL4Hwu17LCo=","Date":"Fri, 30 Jul 2021 05:49:20 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210723040036.32346-1-laurent.pinchart@ideasonboard.com>\n\t<20210723040036.32346-9-laurent.pinchart@ideasonboard.com>\n\t<20210729210423.qroy3bodcv4tc4xn@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210729210423.qroy3bodcv4tc4xn@uno.localdomain>","Subject":"Re: [libcamera-devel] [RFC PATCH 08/17] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]