[{"id":39057,"web_url":"https://patchwork.libcamera.org/comment/39057/","msgid":"<178126559543.1400742.12367811826925283464@ping.linuxembedded.co.uk>","date":"2026-06-12T11:59:55","subject":"Re: [PATCH] libcamera: camera: make Camera::create private","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting tju_cooyun@163.com (2026-06-12 08:11:17)\n> From: zcy <tju_cooyun@163.com>\n> \n> Make Camera::create a private implementation detail since it is only\n> used by pipeline handler subclasses. This prevents app users from\n> misunderstanding or misusing the function. Add a protected\n> PipelineHandler::createCamera helper so pipeline implementations can\n> create Camera instances internally.\n> ---\n>  Documentation/guides/pipeline-handler.rst     | 13 +++++++------\n>  include/libcamera/camera.h                    |  8 ++++----\n\nOn a first glance, all of this looks quite reasonable, but with the ABI\nbreakage (and public API, but we expect it's not used anyway) we'll have to\nwork out when a good time to merge this is.\n\nI've previously wondered if we should or could have an ABI/API break tag\ntoo on the patches so I can make it clear in release notes.\n\n\n>  include/libcamera/internal/pipeline_handler.h |  5 +++++\n>  src/libcamera/pipeline/imx8-isi/imx8-isi.cpp  |  2 +-\n>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  2 +-\n>  src/libcamera/pipeline/mali-c55/mali-c55.cpp  |  3 +--\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-\n>  src/libcamera/pipeline/rpi/pisp/pisp.cpp      |  2 +-\n>  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |  2 +-\n>  src/libcamera/pipeline/simple/simple.cpp      |  2 +-\n>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  2 +-\n>  src/libcamera/pipeline/vimc/vimc.cpp          |  2 +-\n>  src/libcamera/pipeline/virtual/virtual.cpp    |  2 +-\n>  src/libcamera/pipeline_handler.cpp            |  7 +++++++\n>  14 files changed, 33 insertions(+), 21 deletions(-)\n> \n> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> index 083cdb745..2a661cf24 100644\n> --- a/Documentation/guides/pipeline-handler.rst\n> +++ b/Documentation/guides/pipeline-handler.rst\n> @@ -489,10 +489,11 @@ handler.\n>  \n>  Once the camera data has been initialized, the Camera device instances and the\n>  associated streams have to be registered. Create a set of streams for the\n> -camera, which for this device is only one. You create a camera using the static\n> -:doxy-int:`Camera::create` function, passing the Camera::Private instance, the\n> -id of the camera, and the streams available. Then register the camera with the\n> -pipeline handler and camera manager using :doxy-int:`PipelineHandler::registerCamera`.\n> +camera, which for this device is only one. You create a camera using the\n> +protected :doxy-int:`PipelineHandler::createCamera` helper, passing the\n> +Camera::Private instance, the id of the camera, and the streams available.\n> +Then register the camera with the pipeline handler and camera manager using\n> +:doxy-int:`PipelineHandler::registerCamera`.\n>  \n>  Finally with a successful construction, we return 'true' indicating that the\n>  PipelineHandler successfully matched and constructed a device.\n> @@ -501,7 +502,7 @@ PipelineHandler successfully matched and constructed a device.\n>  \n>     std::set<Stream *> streams{ &data->stream_ };\n>     const char *id = data->video_->deviceName();\n> -   std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams);\n> +   std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams);\n>     registerCamera(std::move(camera));\n>  \n>     return true;\n> @@ -529,7 +530,7 @@ Our match function should now look like the following:\n>         /* Create and register the camera. */\n>         std::set<Stream *> streams{ &data->stream_ };\n>         const char *id = data->video_->deviceName();\n> -       std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams);\n> +       std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams);\n>         registerCamera(std::move(camera));\n>  \n>         return true;\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index b24a29740..a5e8ba03a 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -116,10 +116,6 @@ class Camera final : public Object, public std::enable_shared_from_this<Camera>,\n>         LIBCAMERA_DECLARE_PRIVATE()\n>  \n>  public:\n> -       static std::shared_ptr<Camera> create(std::unique_ptr<Private> d,\n> -                                             const std::string &id,\n> -                                             const std::set<Stream *> &streams);\n> -\n>         const std::string &id() const;\n>  \n>         Signal<Request *, FrameBuffer *> bufferCompleted;\n> @@ -154,6 +150,10 @@ public:\n>  private:\n>         LIBCAMERA_DISABLE_COPY(Camera)\n>  \n> +       static std::shared_ptr<Camera> create(std::unique_ptr<Private> d,\n> +                                             const std::string &id,\n> +                                             const std::set<Stream *> &streams);\n> +\n\nI guess this works because somewhere camera's and pipeline handlers are\nalready friends?\n\n\n\n>         Camera(std::unique_ptr<Private> d, const std::string &id,\n>                const std::set<Stream *> &streams);\n>         ~Camera();\n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 6922ce18e..69531b836 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -14,6 +14,7 @@\n>  \n>  #include <libcamera/base/object.h>\n>  \n> +#include <libcamera/camera.h>\n>  #include <libcamera/controls.h>\n>  #include <libcamera/stream.h>\n>  \n> @@ -80,6 +81,10 @@ public:\n>         }\n>  \n>  protected:\n> +       std::shared_ptr<Camera> createCamera(std::unique_ptr<Camera::Private> d,\n> +                                            const std::string &id,\n> +                                            const std::set<Stream *> &streams);\n> +\n>         void registerCamera(std::shared_ptr<Camera> camera);\n>         void hotplugMediaDevice(std::shared_ptr<MediaDevice> media);\n>         unsigned int useCount() const { return useCount_; }\n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index 706681fce..36a42eb8e 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -1101,7 +1101,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator)\n>                 }\n>  \n>                 std::shared_ptr<Camera> camera =\n> -                       Camera::create(std::move(data), id, streams);\n> +                       createCamera(std::move(data), id, streams);\n>  \n>                 registerCamera(std::move(camera));\n>                 numCameras++;\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index bf4c2921f..7d36c148b 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1133,7 +1133,7 @@ int PipelineHandlerIPU3::registerCameras()\n>                 /* Create and register the Camera instance. */\n>                 const std::string &cameraId = cio2->sensor()->id();\n>                 std::shared_ptr<Camera> camera =\n> -                       Camera::create(std::move(data), cameraId, streams);\n> +                       createCamera(std::move(data), cameraId, streams);\n>  \n>                 registerCamera(std::move(camera));\n>  \n> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> index ce2469ed1..62f7fafe1 100644\n> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> @@ -1778,8 +1778,7 @@ bool PipelineHandlerMaliC55::registerMaliCamera(std::unique_ptr<MaliC55CameraDat\n>         if (dsFitted_)\n>                 streams.insert(&data->dsStream_);\n>  \n> -       std::shared_ptr<Camera> camera = Camera::create(std::move(data),\n> -                                                       name, streams);\n> +       std::shared_ptr<Camera> camera = createCamera(std::move(data), name, streams);\n>         registerCamera(std::move(camera));\n>  \n>         return true;\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 9606459fc..48505c6b4 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -1504,7 +1504,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n>         };\n>         const std::string &id = data->sensor_->id();\n>         std::shared_ptr<Camera> camera =\n> -               Camera::create(std::move(data), id, streams);\n> +               createCamera(std::move(data), id, streams);\n>  \n>         registerCamera(std::move(camera));\n>  \n> diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> index b744c901f..62bd4ad1d 100644\n> --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> @@ -1175,7 +1175,7 @@ int PipelineHandlerPiSP::platformRegister(std::unique_ptr<RPi::CameraData> &came\n>         /* Create and register the camera. */\n>         const std::string &id = data->sensor_->id();\n>         std::shared_ptr<Camera> camera =\n> -               Camera::create(std::move(cameraData), id, streams);\n> +               createCamera(std::move(cameraData), id, streams);\n>         PipelineHandler::registerCamera(std::move(camera));\n>  \n>         LOG(RPI, Info) << \"Registered camera \" << id\n> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> index 3e9a49058..bf520b3a0 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> @@ -400,7 +400,7 @@ int PipelineHandlerVc4::platformRegister(std::unique_ptr<RPi::CameraData> &camer\n>         /* Create and register the camera. */\n>         const std::string &id = data->sensor_->id();\n>         std::shared_ptr<Camera> camera =\n> -               Camera::create(std::move(cameraData), id, streams);\n> +               createCamera(std::move(cameraData), id, streams);\n>         PipelineHandler::registerCamera(std::move(camera));\n>  \n>         LOG(RPI, Info) << \"Registered camera \" << id\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index c6fe12d65..4e672067c 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -2002,7 +2002,7 @@ bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media,\n>  \n>                 const std::string &id = data->sensor_->id();\n>                 std::shared_ptr<Camera> camera =\n> -                       Camera::create(std::move(data), id, streams);\n> +                       createCamera(std::move(data), id, streams);\n>                 registerCamera(std::move(camera));\n>                 registered = true;\n>         }\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 3bd51733d..88b0f23a1 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -485,7 +485,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>         std::string id = data->id();\n>         std::set<Stream *> streams{ &data->stream_ };\n>         std::shared_ptr<Camera> camera =\n> -               Camera::create(std::move(data), id, streams);\n> +               createCamera(std::move(data), id, streams);\n>         registerCamera(std::move(camera));\n>  \n>         /* Enable hot-unplug notifications. */\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 6681c74ee..b5518e6e8 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -510,7 +510,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n>         std::set<Stream *> streams{ &data->stream_ };\n>         const std::string &id = data->sensor_->id();\n>         std::shared_ptr<Camera> camera =\n> -               Camera::create(std::move(data), id, streams);\n> +               createCamera(std::move(data), id, streams);\n>         registerCamera(std::move(camera));\n>  \n>         return true;\n> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> index 81d2dddab..1ecfe9d13 100644\n> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> @@ -402,7 +402,7 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator\n>                 for (auto &streamConfig : data->streamConfigs_)\n>                         streams.insert(&streamConfig.stream);\n>                 std::string id = data->config_.id;\n> -               std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams);\n> +               std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams);\n>  \n>                 if (!initFrameGenerator(camera.get())) {\n>                         LOG(Virtual, Error) << \"Failed to initialize frame \"\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index e7145c1d4..c5cdbfda1 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -76,6 +76,13 @@ PipelineHandler::PipelineHandler(CameraManager *manager,\n>  {\n>  }\n>  \n> +std::shared_ptr<Camera> PipelineHandler::createCamera(std::unique_ptr<Camera::Private> d,\n> +                                                     const std::string &id,\n> +                                                     const std::set<Stream *> &streams)\n> +{\n> +       return Camera::create(std::move(d), id, streams);\n> +}\n> +\n>  PipelineHandler::~PipelineHandler()\n>  {\n>         for (std::shared_ptr<MediaDevice> &media : mediaDevices_)\n> -- \n> 2.43.0\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 CB9C2C328C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 12 Jun 2026 12:00:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6CFC6623C9;\n\tFri, 12 Jun 2026 14:00:00 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CB2F1623C2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Jun 2026 13:59:58 +0200 (CEST)","from monstersaurus.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1BE5319EC;\n\tFri, 12 Jun 2026 13:59: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=\"u3DExA0i\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1781265568;\n\tbh=jD4j2wWZv2sr/JIe15NpSolj1eb6k9p1chACPJYmT+8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=u3DExA0iBJGwkeVBZJFTGKOj0iaw/Oj+5DLSORcMZ9LUC1ZC7o2PZ8NUP0VvEcXEA\n\tESJQ8iPWU5JXAe3qgFq5BBibwODbR3pO/d0bRYeUqkAsLlcT7UQtmPQs3YzSwyID+0\n\tA8EGJ42B3xoH7tHRqandMRPpNMcjCxorELYLSxWw=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260612071117.109431-1-tju_cooyun@163.com>","References":"<20260612071117.109431-1-tju_cooyun@163.com>","Subject":"Re: [PATCH] libcamera: camera: make Camera::create private","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"zcy <tju_cooyun@163.com>","To":"libcamera-devel@lists.libcamera.org, tju_cooyun@163.com","Date":"Fri, 12 Jun 2026 12:59:55 +0100","Message-ID":"<178126559543.1400742.12367811826925283464@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":39058,"web_url":"https://patchwork.libcamera.org/comment/39058/","msgid":"<20260612125342.GB1982714@killaraus.ideasonboard.com>","date":"2026-06-12T12:53:42","subject":"Re: [PATCH] libcamera: camera: make Camera::create private","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Jun 12, 2026 at 12:59:55PM +0100, Kieran Bingham wrote:\n> Quoting tju_cooyun@163.com (2026-06-12 08:11:17)\n> > From: zcy <tju_cooyun@163.com>\n> > \n> > Make Camera::create a private implementation detail since it is only\n> > used by pipeline handler subclasses. This prevents app users from\n> > misunderstanding or misusing the function. Add a protected\n> > PipelineHandler::createCamera helper so pipeline implementations can\n> > create Camera instances internally.\n> > ---\n> >  Documentation/guides/pipeline-handler.rst     | 13 +++++++------\n> >  include/libcamera/camera.h                    |  8 ++++----\n> \n> On a first glance, all of this looks quite reasonable, but with the ABI\n> breakage (and public API, but we expect it's not used anyway) we'll have to\n> work out when a good time to merge this is.\n\nThe only change in API is that the Camera::create() function can be\ncalled by applications any more. As no application should do that, I\ndon't see it as a problem.\n\nCamera::create() takes a unique pointer to a Camera::Private class,\nwhich is not exposed to applications. While it would be possible for an\napplication to pass a null pointer, that would cause an immediate crash.\nAnd if an application decided to define a Camera::Private class of its\nown... well, I don't think it's even worth discussing it :-)\n\nWhile the Camera::create() function is visible in the public camera.h\nheader, it's not documented in the public API, and applications have no\nmeaningful way to call it. Is there really a risk of anyone\nmisunderstanding or misusing the function ?\n\n> I've previously wondered if we should or could have an ABI/API break tag\n> too on the patches so I can make it clear in release notes.\n> \n> >  include/libcamera/internal/pipeline_handler.h |  5 +++++\n> >  src/libcamera/pipeline/imx8-isi/imx8-isi.cpp  |  2 +-\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp          |  2 +-\n> >  src/libcamera/pipeline/mali-c55/mali-c55.cpp  |  3 +--\n> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-\n> >  src/libcamera/pipeline/rpi/pisp/pisp.cpp      |  2 +-\n> >  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |  2 +-\n> >  src/libcamera/pipeline/simple/simple.cpp      |  2 +-\n> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  2 +-\n> >  src/libcamera/pipeline/vimc/vimc.cpp          |  2 +-\n> >  src/libcamera/pipeline/virtual/virtual.cpp    |  2 +-\n> >  src/libcamera/pipeline_handler.cpp            |  7 +++++++\n> >  14 files changed, 33 insertions(+), 21 deletions(-)\n> > \n> > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> > index 083cdb745..2a661cf24 100644\n> > --- a/Documentation/guides/pipeline-handler.rst\n> > +++ b/Documentation/guides/pipeline-handler.rst\n> > @@ -489,10 +489,11 @@ handler.\n> >  \n> >  Once the camera data has been initialized, the Camera device instances and the\n> >  associated streams have to be registered. Create a set of streams for the\n> > -camera, which for this device is only one. You create a camera using the static\n> > -:doxy-int:`Camera::create` function, passing the Camera::Private instance, the\n> > -id of the camera, and the streams available. Then register the camera with the\n> > -pipeline handler and camera manager using :doxy-int:`PipelineHandler::registerCamera`.\n> > +camera, which for this device is only one. You create a camera using the\n> > +protected :doxy-int:`PipelineHandler::createCamera` helper, passing the\n> > +Camera::Private instance, the id of the camera, and the streams available.\n> > +Then register the camera with the pipeline handler and camera manager using\n> > +:doxy-int:`PipelineHandler::registerCamera`.\n> >  \n> >  Finally with a successful construction, we return 'true' indicating that the\n> >  PipelineHandler successfully matched and constructed a device.\n> > @@ -501,7 +502,7 @@ PipelineHandler successfully matched and constructed a device.\n> >  \n> >     std::set<Stream *> streams{ &data->stream_ };\n> >     const char *id = data->video_->deviceName();\n> > -   std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams);\n> > +   std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams);\n> >     registerCamera(std::move(camera));\n> >  \n> >     return true;\n> > @@ -529,7 +530,7 @@ Our match function should now look like the following:\n> >         /* Create and register the camera. */\n> >         std::set<Stream *> streams{ &data->stream_ };\n> >         const char *id = data->video_->deviceName();\n> > -       std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams);\n> > +       std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams);\n> >         registerCamera(std::move(camera));\n> >  \n> >         return true;\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index b24a29740..a5e8ba03a 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -116,10 +116,6 @@ class Camera final : public Object, public std::enable_shared_from_this<Camera>,\n> >         LIBCAMERA_DECLARE_PRIVATE()\n> >  \n> >  public:\n> > -       static std::shared_ptr<Camera> create(std::unique_ptr<Private> d,\n> > -                                             const std::string &id,\n> > -                                             const std::set<Stream *> &streams);\n> > -\n> >         const std::string &id() const;\n> >  \n> >         Signal<Request *, FrameBuffer *> bufferCompleted;\n> > @@ -154,6 +150,10 @@ public:\n> >  private:\n> >         LIBCAMERA_DISABLE_COPY(Camera)\n> >  \n> > +       static std::shared_ptr<Camera> create(std::unique_ptr<Private> d,\n> > +                                             const std::string &id,\n> > +                                             const std::set<Stream *> &streams);\n> > +\n> \n> I guess this works because somewhere camera's and pipeline handlers are\n> already friends?\n> \n> >         Camera(std::unique_ptr<Private> d, const std::string &id,\n> >                const std::set<Stream *> &streams);\n> >         ~Camera();\n> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > index 6922ce18e..69531b836 100644\n> > --- a/include/libcamera/internal/pipeline_handler.h\n> > +++ b/include/libcamera/internal/pipeline_handler.h\n> > @@ -14,6 +14,7 @@\n> >  \n> >  #include <libcamera/base/object.h>\n> >  \n> > +#include <libcamera/camera.h>\n> >  #include <libcamera/controls.h>\n> >  #include <libcamera/stream.h>\n> >  \n> > @@ -80,6 +81,10 @@ public:\n> >         }\n> >  \n> >  protected:\n> > +       std::shared_ptr<Camera> createCamera(std::unique_ptr<Camera::Private> d,\n> > +                                            const std::string &id,\n> > +                                            const std::set<Stream *> &streams);\n> > +\n> >         void registerCamera(std::shared_ptr<Camera> camera);\n> >         void hotplugMediaDevice(std::shared_ptr<MediaDevice> media);\n> >         unsigned int useCount() const { return useCount_; }\n> > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > index 706681fce..36a42eb8e 100644\n> > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > @@ -1101,7 +1101,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator)\n> >                 }\n> >  \n> >                 std::shared_ptr<Camera> camera =\n> > -                       Camera::create(std::move(data), id, streams);\n> > +                       createCamera(std::move(data), id, streams);\n> >  \n> >                 registerCamera(std::move(camera));\n> >                 numCameras++;\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index bf4c2921f..7d36c148b 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -1133,7 +1133,7 @@ int PipelineHandlerIPU3::registerCameras()\n> >                 /* Create and register the Camera instance. */\n> >                 const std::string &cameraId = cio2->sensor()->id();\n> >                 std::shared_ptr<Camera> camera =\n> > -                       Camera::create(std::move(data), cameraId, streams);\n> > +                       createCamera(std::move(data), cameraId, streams);\n> >  \n> >                 registerCamera(std::move(camera));\n> >  \n> > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > index ce2469ed1..62f7fafe1 100644\n> > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > @@ -1778,8 +1778,7 @@ bool PipelineHandlerMaliC55::registerMaliCamera(std::unique_ptr<MaliC55CameraDat\n> >         if (dsFitted_)\n> >                 streams.insert(&data->dsStream_);\n> >  \n> > -       std::shared_ptr<Camera> camera = Camera::create(std::move(data),\n> > -                                                       name, streams);\n> > +       std::shared_ptr<Camera> camera = createCamera(std::move(data), name, streams);\n> >         registerCamera(std::move(camera));\n> >  \n> >         return true;\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 9606459fc..48505c6b4 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -1504,7 +1504,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> >         };\n> >         const std::string &id = data->sensor_->id();\n> >         std::shared_ptr<Camera> camera =\n> > -               Camera::create(std::move(data), id, streams);\n> > +               createCamera(std::move(data), id, streams);\n> >  \n> >         registerCamera(std::move(camera));\n> >  \n> > diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> > index b744c901f..62bd4ad1d 100644\n> > --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> > +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> > @@ -1175,7 +1175,7 @@ int PipelineHandlerPiSP::platformRegister(std::unique_ptr<RPi::CameraData> &came\n> >         /* Create and register the camera. */\n> >         const std::string &id = data->sensor_->id();\n> >         std::shared_ptr<Camera> camera =\n> > -               Camera::create(std::move(cameraData), id, streams);\n> > +               createCamera(std::move(cameraData), id, streams);\n> >         PipelineHandler::registerCamera(std::move(camera));\n> >  \n> >         LOG(RPI, Info) << \"Registered camera \" << id\n> > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > index 3e9a49058..bf520b3a0 100644\n> > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > @@ -400,7 +400,7 @@ int PipelineHandlerVc4::platformRegister(std::unique_ptr<RPi::CameraData> &camer\n> >         /* Create and register the camera. */\n> >         const std::string &id = data->sensor_->id();\n> >         std::shared_ptr<Camera> camera =\n> > -               Camera::create(std::move(cameraData), id, streams);\n> > +               createCamera(std::move(cameraData), id, streams);\n> >         PipelineHandler::registerCamera(std::move(camera));\n> >  \n> >         LOG(RPI, Info) << \"Registered camera \" << id\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index c6fe12d65..4e672067c 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -2002,7 +2002,7 @@ bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media,\n> >  \n> >                 const std::string &id = data->sensor_->id();\n> >                 std::shared_ptr<Camera> camera =\n> > -                       Camera::create(std::move(data), id, streams);\n> > +                       createCamera(std::move(data), id, streams);\n> >                 registerCamera(std::move(camera));\n> >                 registered = true;\n> >         }\n> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 3bd51733d..88b0f23a1 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -485,7 +485,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> >         std::string id = data->id();\n> >         std::set<Stream *> streams{ &data->stream_ };\n> >         std::shared_ptr<Camera> camera =\n> > -               Camera::create(std::move(data), id, streams);\n> > +               createCamera(std::move(data), id, streams);\n> >         registerCamera(std::move(camera));\n> >  \n> >         /* Enable hot-unplug notifications. */\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index 6681c74ee..b5518e6e8 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -510,7 +510,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> >         std::set<Stream *> streams{ &data->stream_ };\n> >         const std::string &id = data->sensor_->id();\n> >         std::shared_ptr<Camera> camera =\n> > -               Camera::create(std::move(data), id, streams);\n> > +               createCamera(std::move(data), id, streams);\n> >         registerCamera(std::move(camera));\n> >  \n> >         return true;\n> > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> > index 81d2dddab..1ecfe9d13 100644\n> > --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> > +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> > @@ -402,7 +402,7 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator\n> >                 for (auto &streamConfig : data->streamConfigs_)\n> >                         streams.insert(&streamConfig.stream);\n> >                 std::string id = data->config_.id;\n> > -               std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams);\n> > +               std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams);\n> >  \n> >                 if (!initFrameGenerator(camera.get())) {\n> >                         LOG(Virtual, Error) << \"Failed to initialize frame \"\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index e7145c1d4..c5cdbfda1 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -76,6 +76,13 @@ PipelineHandler::PipelineHandler(CameraManager *manager,\n> >  {\n> >  }\n> >  \n> > +std::shared_ptr<Camera> PipelineHandler::createCamera(std::unique_ptr<Camera::Private> d,\n> > +                                                     const std::string &id,\n> > +                                                     const std::set<Stream *> &streams)\n> > +{\n> > +       return Camera::create(std::move(d), id, streams);\n> > +}\n> > +\n> >  PipelineHandler::~PipelineHandler()\n> >  {\n> >         for (std::shared_ptr<MediaDevice> &media : mediaDevices_)","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 DA643C324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 12 Jun 2026 12:53:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 05984623CA;\n\tFri, 12 Jun 2026 14:53:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 872BE623C2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Jun 2026 14:53:44 +0200 (CEST)","from killaraus.ideasonboard.com\n\t(2001-14ba-70f3-e800--a06.rev.dnainternet.fi\n\t[IPv6:2001:14ba:70f3:e800::a06])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 87A1E512;\n\tFri, 12 Jun 2026 14:53:13 +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=\"TXd7UpmW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1781268793;\n\tbh=EKC3hQf4oB/Jw5PoWwkziMcT6mH7B4T3ck0uRcb5pkw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TXd7UpmW7VWY2fWpbBesv9w3T4lS+39Prk+yVA76xaXMEEgIiioZO1ouEq+Ac3GO/\n\t4+zE//8vd+Lpb+tLudh4fGGJm5N46MF9LkRcvnfRmlTHehlkdB2/oToGwOFx5WBkbv\n\tx9V0mpqmu7rx2ORaWN6DPYAQi2tiK7R8IfN/nuvw=","Date":"Fri, 12 Jun 2026 15:53:42 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, tju_cooyun@163.com","Subject":"Re: [PATCH] libcamera: camera: make Camera::create private","Message-ID":"<20260612125342.GB1982714@killaraus.ideasonboard.com>","References":"<20260612071117.109431-1-tju_cooyun@163.com>\n\t<178126559543.1400742.12367811826925283464@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<178126559543.1400742.12367811826925283464@ping.linuxembedded.co.uk>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]