| Message ID | 20260612071117.109431-1-tju_cooyun@163.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series |
|
| Related | show |
Quoting tju_cooyun@163.com (2026-06-12 08:11:17) > From: zcy <tju_cooyun@163.com> > > Make Camera::create a private implementation detail since it is only > used by pipeline handler subclasses. This prevents app users from > misunderstanding or misusing the function. Add a protected > PipelineHandler::createCamera helper so pipeline implementations can > create Camera instances internally. > --- > Documentation/guides/pipeline-handler.rst | 13 +++++++------ > include/libcamera/camera.h | 8 ++++---- On a first glance, all of this looks quite reasonable, but with the ABI breakage (and public API, but we expect it's not used anyway) we'll have to work out when a good time to merge this is. I've previously wondered if we should or could have an ABI/API break tag too on the patches so I can make it clear in release notes. > include/libcamera/internal/pipeline_handler.h | 5 +++++ > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 2 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 3 +-- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- > src/libcamera/pipeline/rpi/pisp/pisp.cpp | 2 +- > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 2 +- > src/libcamera/pipeline/simple/simple.cpp | 2 +- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +- > src/libcamera/pipeline/vimc/vimc.cpp | 2 +- > src/libcamera/pipeline/virtual/virtual.cpp | 2 +- > src/libcamera/pipeline_handler.cpp | 7 +++++++ > 14 files changed, 33 insertions(+), 21 deletions(-) > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst > index 083cdb745..2a661cf24 100644 > --- a/Documentation/guides/pipeline-handler.rst > +++ b/Documentation/guides/pipeline-handler.rst > @@ -489,10 +489,11 @@ handler. > > Once the camera data has been initialized, the Camera device instances and the > associated streams have to be registered. Create a set of streams for the > -camera, which for this device is only one. You create a camera using the static > -:doxy-int:`Camera::create` function, passing the Camera::Private instance, the > -id of the camera, and the streams available. Then register the camera with the > -pipeline handler and camera manager using :doxy-int:`PipelineHandler::registerCamera`. > +camera, which for this device is only one. You create a camera using the > +protected :doxy-int:`PipelineHandler::createCamera` helper, passing the > +Camera::Private instance, the id of the camera, and the streams available. > +Then register the camera with the pipeline handler and camera manager using > +:doxy-int:`PipelineHandler::registerCamera`. > > Finally with a successful construction, we return 'true' indicating that the > PipelineHandler successfully matched and constructed a device. > @@ -501,7 +502,7 @@ PipelineHandler successfully matched and constructed a device. > > std::set<Stream *> streams{ &data->stream_ }; > const char *id = data->video_->deviceName(); > - std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams); > + std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams); > registerCamera(std::move(camera)); > > return true; > @@ -529,7 +530,7 @@ Our match function should now look like the following: > /* Create and register the camera. */ > std::set<Stream *> streams{ &data->stream_ }; > const char *id = data->video_->deviceName(); > - std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams); > + std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams); > registerCamera(std::move(camera)); > > return true; > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index b24a29740..a5e8ba03a 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -116,10 +116,6 @@ class Camera final : public Object, public std::enable_shared_from_this<Camera>, > LIBCAMERA_DECLARE_PRIVATE() > > public: > - static std::shared_ptr<Camera> create(std::unique_ptr<Private> d, > - const std::string &id, > - const std::set<Stream *> &streams); > - > const std::string &id() const; > > Signal<Request *, FrameBuffer *> bufferCompleted; > @@ -154,6 +150,10 @@ public: > private: > LIBCAMERA_DISABLE_COPY(Camera) > > + static std::shared_ptr<Camera> create(std::unique_ptr<Private> d, > + const std::string &id, > + const std::set<Stream *> &streams); > + I guess this works because somewhere camera's and pipeline handlers are already friends? > Camera(std::unique_ptr<Private> d, const std::string &id, > const std::set<Stream *> &streams); > ~Camera(); > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h > index 6922ce18e..69531b836 100644 > --- a/include/libcamera/internal/pipeline_handler.h > +++ b/include/libcamera/internal/pipeline_handler.h > @@ -14,6 +14,7 @@ > > #include <libcamera/base/object.h> > > +#include <libcamera/camera.h> > #include <libcamera/controls.h> > #include <libcamera/stream.h> > > @@ -80,6 +81,10 @@ public: > } > > protected: > + std::shared_ptr<Camera> createCamera(std::unique_ptr<Camera::Private> d, > + const std::string &id, > + const std::set<Stream *> &streams); > + > void registerCamera(std::shared_ptr<Camera> camera); > void hotplugMediaDevice(std::shared_ptr<MediaDevice> media); > unsigned int useCount() const { return useCount_; } > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > index 706681fce..36a42eb8e 100644 > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > @@ -1101,7 +1101,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator) > } > > std::shared_ptr<Camera> camera = > - Camera::create(std::move(data), id, streams); > + createCamera(std::move(data), id, streams); > > registerCamera(std::move(camera)); > numCameras++; > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index bf4c2921f..7d36c148b 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -1133,7 +1133,7 @@ int PipelineHandlerIPU3::registerCameras() > /* Create and register the Camera instance. */ > const std::string &cameraId = cio2->sensor()->id(); > std::shared_ptr<Camera> camera = > - Camera::create(std::move(data), cameraId, streams); > + createCamera(std::move(data), cameraId, streams); > > registerCamera(std::move(camera)); > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > index ce2469ed1..62f7fafe1 100644 > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > @@ -1778,8 +1778,7 @@ bool PipelineHandlerMaliC55::registerMaliCamera(std::unique_ptr<MaliC55CameraDat > if (dsFitted_) > streams.insert(&data->dsStream_); > > - std::shared_ptr<Camera> camera = Camera::create(std::move(data), > - name, streams); > + std::shared_ptr<Camera> camera = createCamera(std::move(data), name, streams); > registerCamera(std::move(camera)); > > return true; > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 9606459fc..48505c6b4 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -1504,7 +1504,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) > }; > const std::string &id = data->sensor_->id(); > std::shared_ptr<Camera> camera = > - Camera::create(std::move(data), id, streams); > + createCamera(std::move(data), id, streams); > > registerCamera(std::move(camera)); > > diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp > index b744c901f..62bd4ad1d 100644 > --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp > +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp > @@ -1175,7 +1175,7 @@ int PipelineHandlerPiSP::platformRegister(std::unique_ptr<RPi::CameraData> &came > /* Create and register the camera. */ > const std::string &id = data->sensor_->id(); > std::shared_ptr<Camera> camera = > - Camera::create(std::move(cameraData), id, streams); > + createCamera(std::move(cameraData), id, streams); > PipelineHandler::registerCamera(std::move(camera)); > > LOG(RPI, Info) << "Registered camera " << id > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > index 3e9a49058..bf520b3a0 100644 > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > @@ -400,7 +400,7 @@ int PipelineHandlerVc4::platformRegister(std::unique_ptr<RPi::CameraData> &camer > /* Create and register the camera. */ > const std::string &id = data->sensor_->id(); > std::shared_ptr<Camera> camera = > - Camera::create(std::move(cameraData), id, streams); > + createCamera(std::move(cameraData), id, streams); > PipelineHandler::registerCamera(std::move(camera)); > > LOG(RPI, Info) << "Registered camera " << id > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index c6fe12d65..4e672067c 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -2002,7 +2002,7 @@ bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media, > > const std::string &id = data->sensor_->id(); > std::shared_ptr<Camera> camera = > - Camera::create(std::move(data), id, streams); > + createCamera(std::move(data), id, streams); > registerCamera(std::move(camera)); > registered = true; > } > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index 3bd51733d..88b0f23a1 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -485,7 +485,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) > std::string id = data->id(); > std::set<Stream *> streams{ &data->stream_ }; > std::shared_ptr<Camera> camera = > - Camera::create(std::move(data), id, streams); > + createCamera(std::move(data), id, streams); > registerCamera(std::move(camera)); > > /* Enable hot-unplug notifications. */ > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > index 6681c74ee..b5518e6e8 100644 > --- a/src/libcamera/pipeline/vimc/vimc.cpp > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > @@ -510,7 +510,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) > std::set<Stream *> streams{ &data->stream_ }; > const std::string &id = data->sensor_->id(); > std::shared_ptr<Camera> camera = > - Camera::create(std::move(data), id, streams); > + createCamera(std::move(data), id, streams); > registerCamera(std::move(camera)); > > return true; > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > index 81d2dddab..1ecfe9d13 100644 > --- a/src/libcamera/pipeline/virtual/virtual.cpp > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > @@ -402,7 +402,7 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator > for (auto &streamConfig : data->streamConfigs_) > streams.insert(&streamConfig.stream); > std::string id = data->config_.id; > - std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams); > + std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams); > > if (!initFrameGenerator(camera.get())) { > LOG(Virtual, Error) << "Failed to initialize frame " > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index e7145c1d4..c5cdbfda1 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -76,6 +76,13 @@ PipelineHandler::PipelineHandler(CameraManager *manager, > { > } > > +std::shared_ptr<Camera> PipelineHandler::createCamera(std::unique_ptr<Camera::Private> d, > + const std::string &id, > + const std::set<Stream *> &streams) > +{ > + return Camera::create(std::move(d), id, streams); > +} > + > PipelineHandler::~PipelineHandler() > { > for (std::shared_ptr<MediaDevice> &media : mediaDevices_) > -- > 2.43.0 >
On Fri, Jun 12, 2026 at 12:59:55PM +0100, Kieran Bingham wrote: > Quoting tju_cooyun@163.com (2026-06-12 08:11:17) > > From: zcy <tju_cooyun@163.com> > > > > Make Camera::create a private implementation detail since it is only > > used by pipeline handler subclasses. This prevents app users from > > misunderstanding or misusing the function. Add a protected > > PipelineHandler::createCamera helper so pipeline implementations can > > create Camera instances internally. > > --- > > Documentation/guides/pipeline-handler.rst | 13 +++++++------ > > include/libcamera/camera.h | 8 ++++---- > > On a first glance, all of this looks quite reasonable, but with the ABI > breakage (and public API, but we expect it's not used anyway) we'll have to > work out when a good time to merge this is. The only change in API is that the Camera::create() function can be called by applications any more. As no application should do that, I don't see it as a problem. Camera::create() takes a unique pointer to a Camera::Private class, which is not exposed to applications. While it would be possible for an application to pass a null pointer, that would cause an immediate crash. And if an application decided to define a Camera::Private class of its own... well, I don't think it's even worth discussing it :-) While the Camera::create() function is visible in the public camera.h header, it's not documented in the public API, and applications have no meaningful way to call it. Is there really a risk of anyone misunderstanding or misusing the function ? > I've previously wondered if we should or could have an ABI/API break tag > too on the patches so I can make it clear in release notes. > > > include/libcamera/internal/pipeline_handler.h | 5 +++++ > > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 2 +- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 3 +-- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- > > src/libcamera/pipeline/rpi/pisp/pisp.cpp | 2 +- > > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 2 +- > > src/libcamera/pipeline/simple/simple.cpp | 2 +- > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +- > > src/libcamera/pipeline/vimc/vimc.cpp | 2 +- > > src/libcamera/pipeline/virtual/virtual.cpp | 2 +- > > src/libcamera/pipeline_handler.cpp | 7 +++++++ > > 14 files changed, 33 insertions(+), 21 deletions(-) > > > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst > > index 083cdb745..2a661cf24 100644 > > --- a/Documentation/guides/pipeline-handler.rst > > +++ b/Documentation/guides/pipeline-handler.rst > > @@ -489,10 +489,11 @@ handler. > > > > Once the camera data has been initialized, the Camera device instances and the > > associated streams have to be registered. Create a set of streams for the > > -camera, which for this device is only one. You create a camera using the static > > -:doxy-int:`Camera::create` function, passing the Camera::Private instance, the > > -id of the camera, and the streams available. Then register the camera with the > > -pipeline handler and camera manager using :doxy-int:`PipelineHandler::registerCamera`. > > +camera, which for this device is only one. You create a camera using the > > +protected :doxy-int:`PipelineHandler::createCamera` helper, passing the > > +Camera::Private instance, the id of the camera, and the streams available. > > +Then register the camera with the pipeline handler and camera manager using > > +:doxy-int:`PipelineHandler::registerCamera`. > > > > Finally with a successful construction, we return 'true' indicating that the > > PipelineHandler successfully matched and constructed a device. > > @@ -501,7 +502,7 @@ PipelineHandler successfully matched and constructed a device. > > > > std::set<Stream *> streams{ &data->stream_ }; > > const char *id = data->video_->deviceName(); > > - std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams); > > + std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams); > > registerCamera(std::move(camera)); > > > > return true; > > @@ -529,7 +530,7 @@ Our match function should now look like the following: > > /* Create and register the camera. */ > > std::set<Stream *> streams{ &data->stream_ }; > > const char *id = data->video_->deviceName(); > > - std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams); > > + std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams); > > registerCamera(std::move(camera)); > > > > return true; > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > index b24a29740..a5e8ba03a 100644 > > --- a/include/libcamera/camera.h > > +++ b/include/libcamera/camera.h > > @@ -116,10 +116,6 @@ class Camera final : public Object, public std::enable_shared_from_this<Camera>, > > LIBCAMERA_DECLARE_PRIVATE() > > > > public: > > - static std::shared_ptr<Camera> create(std::unique_ptr<Private> d, > > - const std::string &id, > > - const std::set<Stream *> &streams); > > - > > const std::string &id() const; > > > > Signal<Request *, FrameBuffer *> bufferCompleted; > > @@ -154,6 +150,10 @@ public: > > private: > > LIBCAMERA_DISABLE_COPY(Camera) > > > > + static std::shared_ptr<Camera> create(std::unique_ptr<Private> d, > > + const std::string &id, > > + const std::set<Stream *> &streams); > > + > > I guess this works because somewhere camera's and pipeline handlers are > already friends? > > > Camera(std::unique_ptr<Private> d, const std::string &id, > > const std::set<Stream *> &streams); > > ~Camera(); > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h > > index 6922ce18e..69531b836 100644 > > --- a/include/libcamera/internal/pipeline_handler.h > > +++ b/include/libcamera/internal/pipeline_handler.h > > @@ -14,6 +14,7 @@ > > > > #include <libcamera/base/object.h> > > > > +#include <libcamera/camera.h> > > #include <libcamera/controls.h> > > #include <libcamera/stream.h> > > > > @@ -80,6 +81,10 @@ public: > > } > > > > protected: > > + std::shared_ptr<Camera> createCamera(std::unique_ptr<Camera::Private> d, > > + const std::string &id, > > + const std::set<Stream *> &streams); > > + > > void registerCamera(std::shared_ptr<Camera> camera); > > void hotplugMediaDevice(std::shared_ptr<MediaDevice> media); > > unsigned int useCount() const { return useCount_; } > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > index 706681fce..36a42eb8e 100644 > > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > @@ -1101,7 +1101,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator) > > } > > > > std::shared_ptr<Camera> camera = > > - Camera::create(std::move(data), id, streams); > > + createCamera(std::move(data), id, streams); > > > > registerCamera(std::move(camera)); > > numCameras++; > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index bf4c2921f..7d36c148b 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -1133,7 +1133,7 @@ int PipelineHandlerIPU3::registerCameras() > > /* Create and register the Camera instance. */ > > const std::string &cameraId = cio2->sensor()->id(); > > std::shared_ptr<Camera> camera = > > - Camera::create(std::move(data), cameraId, streams); > > + createCamera(std::move(data), cameraId, streams); > > > > registerCamera(std::move(camera)); > > > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > index ce2469ed1..62f7fafe1 100644 > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > @@ -1778,8 +1778,7 @@ bool PipelineHandlerMaliC55::registerMaliCamera(std::unique_ptr<MaliC55CameraDat > > if (dsFitted_) > > streams.insert(&data->dsStream_); > > > > - std::shared_ptr<Camera> camera = Camera::create(std::move(data), > > - name, streams); > > + std::shared_ptr<Camera> camera = createCamera(std::move(data), name, streams); > > registerCamera(std::move(camera)); > > > > return true; > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > index 9606459fc..48505c6b4 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > @@ -1504,7 +1504,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) > > }; > > const std::string &id = data->sensor_->id(); > > std::shared_ptr<Camera> camera = > > - Camera::create(std::move(data), id, streams); > > + createCamera(std::move(data), id, streams); > > > > registerCamera(std::move(camera)); > > > > diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp > > index b744c901f..62bd4ad1d 100644 > > --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp > > +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp > > @@ -1175,7 +1175,7 @@ int PipelineHandlerPiSP::platformRegister(std::unique_ptr<RPi::CameraData> &came > > /* Create and register the camera. */ > > const std::string &id = data->sensor_->id(); > > std::shared_ptr<Camera> camera = > > - Camera::create(std::move(cameraData), id, streams); > > + createCamera(std::move(cameraData), id, streams); > > PipelineHandler::registerCamera(std::move(camera)); > > > > LOG(RPI, Info) << "Registered camera " << id > > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > > index 3e9a49058..bf520b3a0 100644 > > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp > > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > > @@ -400,7 +400,7 @@ int PipelineHandlerVc4::platformRegister(std::unique_ptr<RPi::CameraData> &camer > > /* Create and register the camera. */ > > const std::string &id = data->sensor_->id(); > > std::shared_ptr<Camera> camera = > > - Camera::create(std::move(cameraData), id, streams); > > + createCamera(std::move(cameraData), id, streams); > > PipelineHandler::registerCamera(std::move(camera)); > > > > LOG(RPI, Info) << "Registered camera " << id > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > > index c6fe12d65..4e672067c 100644 > > --- a/src/libcamera/pipeline/simple/simple.cpp > > +++ b/src/libcamera/pipeline/simple/simple.cpp > > @@ -2002,7 +2002,7 @@ bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media, > > > > const std::string &id = data->sensor_->id(); > > std::shared_ptr<Camera> camera = > > - Camera::create(std::move(data), id, streams); > > + createCamera(std::move(data), id, streams); > > registerCamera(std::move(camera)); > > registered = true; > > } > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > index 3bd51733d..88b0f23a1 100644 > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > @@ -485,7 +485,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) > > std::string id = data->id(); > > std::set<Stream *> streams{ &data->stream_ }; > > std::shared_ptr<Camera> camera = > > - Camera::create(std::move(data), id, streams); > > + createCamera(std::move(data), id, streams); > > registerCamera(std::move(camera)); > > > > /* Enable hot-unplug notifications. */ > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > > index 6681c74ee..b5518e6e8 100644 > > --- a/src/libcamera/pipeline/vimc/vimc.cpp > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > > @@ -510,7 +510,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) > > std::set<Stream *> streams{ &data->stream_ }; > > const std::string &id = data->sensor_->id(); > > std::shared_ptr<Camera> camera = > > - Camera::create(std::move(data), id, streams); > > + createCamera(std::move(data), id, streams); > > registerCamera(std::move(camera)); > > > > return true; > > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > > index 81d2dddab..1ecfe9d13 100644 > > --- a/src/libcamera/pipeline/virtual/virtual.cpp > > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > > @@ -402,7 +402,7 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator > > for (auto &streamConfig : data->streamConfigs_) > > streams.insert(&streamConfig.stream); > > std::string id = data->config_.id; > > - std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams); > > + std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams); > > > > if (!initFrameGenerator(camera.get())) { > > LOG(Virtual, Error) << "Failed to initialize frame " > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > index e7145c1d4..c5cdbfda1 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -76,6 +76,13 @@ PipelineHandler::PipelineHandler(CameraManager *manager, > > { > > } > > > > +std::shared_ptr<Camera> PipelineHandler::createCamera(std::unique_ptr<Camera::Private> d, > > + const std::string &id, > > + const std::set<Stream *> &streams) > > +{ > > + return Camera::create(std::move(d), id, streams); > > +} > > + > > PipelineHandler::~PipelineHandler() > > { > > for (std::shared_ptr<MediaDevice> &media : mediaDevices_)
diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 083cdb745..2a661cf24 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -489,10 +489,11 @@ handler. Once the camera data has been initialized, the Camera device instances and the associated streams have to be registered. Create a set of streams for the -camera, which for this device is only one. You create a camera using the static -:doxy-int:`Camera::create` function, passing the Camera::Private instance, the -id of the camera, and the streams available. Then register the camera with the -pipeline handler and camera manager using :doxy-int:`PipelineHandler::registerCamera`. +camera, which for this device is only one. You create a camera using the +protected :doxy-int:`PipelineHandler::createCamera` helper, passing the +Camera::Private instance, the id of the camera, and the streams available. +Then register the camera with the pipeline handler and camera manager using +:doxy-int:`PipelineHandler::registerCamera`. Finally with a successful construction, we return 'true' indicating that the PipelineHandler successfully matched and constructed a device. @@ -501,7 +502,7 @@ PipelineHandler successfully matched and constructed a device. std::set<Stream *> streams{ &data->stream_ }; const char *id = data->video_->deviceName(); - std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams); + std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams); registerCamera(std::move(camera)); return true; @@ -529,7 +530,7 @@ Our match function should now look like the following: /* Create and register the camera. */ std::set<Stream *> streams{ &data->stream_ }; const char *id = data->video_->deviceName(); - std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams); + std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams); registerCamera(std::move(camera)); return true; diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index b24a29740..a5e8ba03a 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -116,10 +116,6 @@ class Camera final : public Object, public std::enable_shared_from_this<Camera>, LIBCAMERA_DECLARE_PRIVATE() public: - static std::shared_ptr<Camera> create(std::unique_ptr<Private> d, - const std::string &id, - const std::set<Stream *> &streams); - const std::string &id() const; Signal<Request *, FrameBuffer *> bufferCompleted; @@ -154,6 +150,10 @@ public: private: LIBCAMERA_DISABLE_COPY(Camera) + static std::shared_ptr<Camera> create(std::unique_ptr<Private> d, + const std::string &id, + const std::set<Stream *> &streams); + Camera(std::unique_ptr<Private> d, const std::string &id, const std::set<Stream *> &streams); ~Camera(); diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index 6922ce18e..69531b836 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -14,6 +14,7 @@ #include <libcamera/base/object.h> +#include <libcamera/camera.h> #include <libcamera/controls.h> #include <libcamera/stream.h> @@ -80,6 +81,10 @@ public: } protected: + std::shared_ptr<Camera> createCamera(std::unique_ptr<Camera::Private> d, + const std::string &id, + const std::set<Stream *> &streams); + void registerCamera(std::shared_ptr<Camera> camera); void hotplugMediaDevice(std::shared_ptr<MediaDevice> media); unsigned int useCount() const { return useCount_; } diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index 706681fce..36a42eb8e 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -1101,7 +1101,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator) } std::shared_ptr<Camera> camera = - Camera::create(std::move(data), id, streams); + createCamera(std::move(data), id, streams); registerCamera(std::move(camera)); numCameras++; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index bf4c2921f..7d36c148b 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -1133,7 +1133,7 @@ int PipelineHandlerIPU3::registerCameras() /* Create and register the Camera instance. */ const std::string &cameraId = cio2->sensor()->id(); std::shared_ptr<Camera> camera = - Camera::create(std::move(data), cameraId, streams); + createCamera(std::move(data), cameraId, streams); registerCamera(std::move(camera)); diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index ce2469ed1..62f7fafe1 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -1778,8 +1778,7 @@ bool PipelineHandlerMaliC55::registerMaliCamera(std::unique_ptr<MaliC55CameraDat if (dsFitted_) streams.insert(&data->dsStream_); - std::shared_ptr<Camera> camera = Camera::create(std::move(data), - name, streams); + std::shared_ptr<Camera> camera = createCamera(std::move(data), name, streams); registerCamera(std::move(camera)); return true; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 9606459fc..48505c6b4 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1504,7 +1504,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) }; const std::string &id = data->sensor_->id(); std::shared_ptr<Camera> camera = - Camera::create(std::move(data), id, streams); + createCamera(std::move(data), id, streams); registerCamera(std::move(camera)); diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp index b744c901f..62bd4ad1d 100644 --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp @@ -1175,7 +1175,7 @@ int PipelineHandlerPiSP::platformRegister(std::unique_ptr<RPi::CameraData> &came /* Create and register the camera. */ const std::string &id = data->sensor_->id(); std::shared_ptr<Camera> camera = - Camera::create(std::move(cameraData), id, streams); + createCamera(std::move(cameraData), id, streams); PipelineHandler::registerCamera(std::move(camera)); LOG(RPI, Info) << "Registered camera " << id diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp index 3e9a49058..bf520b3a0 100644 --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp @@ -400,7 +400,7 @@ int PipelineHandlerVc4::platformRegister(std::unique_ptr<RPi::CameraData> &camer /* Create and register the camera. */ const std::string &id = data->sensor_->id(); std::shared_ptr<Camera> camera = - Camera::create(std::move(cameraData), id, streams); + createCamera(std::move(cameraData), id, streams); PipelineHandler::registerCamera(std::move(camera)); LOG(RPI, Info) << "Registered camera " << id diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index c6fe12d65..4e672067c 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -2002,7 +2002,7 @@ bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media, const std::string &id = data->sensor_->id(); std::shared_ptr<Camera> camera = - Camera::create(std::move(data), id, streams); + createCamera(std::move(data), id, streams); registerCamera(std::move(camera)); registered = true; } diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 3bd51733d..88b0f23a1 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -485,7 +485,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) std::string id = data->id(); std::set<Stream *> streams{ &data->stream_ }; std::shared_ptr<Camera> camera = - Camera::create(std::move(data), id, streams); + createCamera(std::move(data), id, streams); registerCamera(std::move(camera)); /* Enable hot-unplug notifications. */ diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 6681c74ee..b5518e6e8 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -510,7 +510,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) std::set<Stream *> streams{ &data->stream_ }; const std::string &id = data->sensor_->id(); std::shared_ptr<Camera> camera = - Camera::create(std::move(data), id, streams); + createCamera(std::move(data), id, streams); registerCamera(std::move(camera)); return true; diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp index 81d2dddab..1ecfe9d13 100644 --- a/src/libcamera/pipeline/virtual/virtual.cpp +++ b/src/libcamera/pipeline/virtual/virtual.cpp @@ -402,7 +402,7 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator for (auto &streamConfig : data->streamConfigs_) streams.insert(&streamConfig.stream); std::string id = data->config_.id; - std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams); + std::shared_ptr<Camera> camera = createCamera(std::move(data), id, streams); if (!initFrameGenerator(camera.get())) { LOG(Virtual, Error) << "Failed to initialize frame " diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index e7145c1d4..c5cdbfda1 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -76,6 +76,13 @@ PipelineHandler::PipelineHandler(CameraManager *manager, { } +std::shared_ptr<Camera> PipelineHandler::createCamera(std::unique_ptr<Camera::Private> d, + const std::string &id, + const std::set<Stream *> &streams) +{ + return Camera::create(std::move(d), id, streams); +} + PipelineHandler::~PipelineHandler() { for (std::shared_ptr<MediaDevice> &media : mediaDevices_)
From: zcy <tju_cooyun@163.com> Make Camera::create a private implementation detail since it is only used by pipeline handler subclasses. This prevents app users from misunderstanding or misusing the function. Add a protected PipelineHandler::createCamera helper so pipeline implementations can create Camera instances internally. --- Documentation/guides/pipeline-handler.rst | 13 +++++++------ include/libcamera/camera.h | 8 ++++---- include/libcamera/internal/pipeline_handler.h | 5 +++++ src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 2 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- src/libcamera/pipeline/mali-c55/mali-c55.cpp | 3 +-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- src/libcamera/pipeline/rpi/pisp/pisp.cpp | 2 +- src/libcamera/pipeline/rpi/vc4/vc4.cpp | 2 +- src/libcamera/pipeline/simple/simple.cpp | 2 +- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc/vimc.cpp | 2 +- src/libcamera/pipeline/virtual/virtual.cpp | 2 +- src/libcamera/pipeline_handler.cpp | 7 +++++++ 14 files changed, 33 insertions(+), 21 deletions(-)