| Message ID | 20240301212121.9072-20-laurent.pinchart@ideasonboard.com |
|---|---|
| State | RFC |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Fri, Mar 01, 2024 at 11:21:08PM +0200, Laurent Pinchart wrote: > From: Jacopo Mondi <jacopo@jmondi.org> > > Introduce a factory to create CameraSensor derived classes instances by > inspecting the sensor media entity name and provide a convenience macro > to register specialized sensor handlers. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> Cheers, Stefan > --- > include/libcamera/internal/camera_sensor.h | 48 +++++- > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 9 +- > src/libcamera/pipeline/ipu3/cio2.cpp | 7 +- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 +- > .../pipeline/rpi/common/pipeline_base.cpp | 5 +- > src/libcamera/pipeline/simple/simple.cpp | 9 +- > src/libcamera/pipeline/vimc/vimc.cpp | 7 +- > src/libcamera/sensor/camera_sensor.cpp | 162 ++++++++++++++++++ > test/camera-sensor.cpp | 7 +- > .../v4l2_videodevice_test.cpp | 5 +- > test/v4l2_videodevice/v4l2_videodevice_test.h | 2 +- > 11 files changed, 231 insertions(+), 37 deletions(-) > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > index d05f48ebeebe..577af779cd6e 100644 > --- a/include/libcamera/internal/camera_sensor.h > +++ b/include/libcamera/internal/camera_sensor.h > @@ -39,7 +39,6 @@ enum class Orientation; > class CameraSensor : protected Loggable > { > public: > - explicit CameraSensor(const MediaEntity *entity); > ~CameraSensor(); > > int init(); > @@ -82,6 +81,7 @@ public: > int setTestPatternMode(controls::draft::TestPatternModeEnum mode); > > protected: > + explicit CameraSensor(const MediaEntity *entity); > std::string logPrefix() const override; > > private: > @@ -123,4 +123,50 @@ private: > std::unique_ptr<CameraLens> focusLens_; > }; > > +class CameraSensorFactoryBase > +{ > +public: > + CameraSensorFactoryBase(); > + virtual ~CameraSensorFactoryBase() = default; > + > + static std::unique_ptr<CameraSensor> create(MediaEntity *entity); > + > +private: > + LIBCAMERA_DISABLE_COPY_AND_MOVE(CameraSensorFactoryBase) > + > + static std::vector<CameraSensorFactoryBase *> &factories(); > + > + static void registerFactory(CameraSensorFactoryBase *factory); > + > + virtual bool match(const MediaEntity *entity) const = 0; > + > + virtual std::unique_ptr<CameraSensor> > + createInstance(MediaEntity *entity) const = 0; > +}; > + > +template<typename _CameraSensor> > +class CameraSensorFactory final : public CameraSensorFactoryBase > +{ > +public: > + CameraSensorFactory() > + : CameraSensorFactoryBase() > + { > + } > + > +private: > + bool match(const MediaEntity *entity) const override > + { > + return _CameraSensor::match(entity); > + } > + > + std::unique_ptr<CameraSensor> > + createInstance(MediaEntity *entity) const override > + { > + return _CameraSensor::create(entity); > + } > +}; > + > +#define REGISTER_CAMERA_SENSOR(sensor) \ > + static CameraSensorFactory<sensor> global_##sensor##Factory{}; > + > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > index a3782aea2ba9..a00567c6873c 100644 > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > @@ -157,11 +157,10 @@ PipelineHandlerISI *ISICameraData::pipe() > /* Open and initialize pipe components. */ > int ISICameraData::init() > { > - int ret = sensor_->init(); > - if (ret) > - return ret; > + if (!sensor_) > + return -ENODEV; > > - ret = csis_->open(); > + int ret = csis_->open(); > if (ret) > return ret; > > @@ -1063,7 +1062,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator) > std::unique_ptr<ISICameraData> data = > std::make_unique<ISICameraData>(this); > > - data->sensor_ = std::make_unique<CameraSensor>(sensor); > + data->sensor_ = CameraSensorFactoryBase::create(sensor); > data->csis_ = std::make_unique<V4L2Subdevice>(csi); > data->xbarSink_ = sink; > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > index 43c816baf6ef..e09583ea418f 100644 > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > @@ -134,10 +134,9 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) > > MediaLink *link = links[0]; > MediaEntity *sensorEntity = link->source()->entity(); > - sensor_ = std::make_unique<CameraSensor>(sensorEntity); > - ret = sensor_->init(); > - if (ret) > - return ret; > + sensor_ = CameraSensorFactoryBase::create(sensorEntity); > + if (!sensor_) > + return -ENODEV; > > ret = link->setEnabled(true); > if (ret) > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index abb21968413a..1a3e7938fa91 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -1109,10 +1109,9 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) > std::make_unique<RkISP1CameraData>(this, &mainPath_, > hasSelfPath_ ? &selfPath_ : nullptr); > > - data->sensor_ = std::make_unique<CameraSensor>(sensor); > - ret = data->sensor_->init(); > - if (ret) > - return ret; > + data->sensor_ = CameraSensorFactoryBase::create(sensor); > + if (!data->sensor_) > + return -ENODEV; > > /* Initialize the camera properties. */ > data->properties_ = data->sensor_->properties(); > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > index 7e420b3f90a4..d662c8f12145 100644 > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > @@ -772,13 +772,10 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera > CameraData *data = cameraData.get(); > int ret; > > - data->sensor_ = std::make_unique<CameraSensor>(sensorEntity); > + data->sensor_ = CameraSensorFactoryBase::create(sensorEntity); > if (!data->sensor_) > return -EINVAL; > > - if (data->sensor_->init()) > - return -EINVAL; > - > /* Populate the map of sensor supported formats and sizes. */ > for (auto const mbusCode : data->sensor_->mbusCodes()) > data->sensorFormats_.emplace(mbusCode, > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 01f2a97798ba..0bd021388c8f 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -371,8 +371,6 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, > MediaEntity *sensor) > : Camera::Private(pipe), streams_(numStreams) > { > - int ret; > - > /* > * Find the shortest path from the camera sensor to a video capture > * device using the breadth-first search algorithm. This heuristic will > @@ -463,12 +461,9 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, > } > > /* Finally also remember the sensor. */ > - sensor_ = std::make_unique<CameraSensor>(sensor); > - ret = sensor_->init(); > - if (ret) { > - sensor_.reset(); > + sensor_ = CameraSensorFactoryBase::create(sensor); > + if (!sensor_) > return; > - } > > LOG(SimplePipeline, Debug) > << "Found pipeline: " > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > index 5e66ee1d26c1..ae0ca21907ec 100644 > --- a/src/libcamera/pipeline/vimc/vimc.cpp > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > @@ -510,10 +510,9 @@ int VimcCameraData::init() > return ret; > > /* Create and open the camera sensor, debayer, scaler and video device. */ > - sensor_ = std::make_unique<CameraSensor>(media_->getEntityByName("Sensor B")); > - ret = sensor_->init(); > - if (ret) > - return ret; > + sensor_ = CameraSensorFactoryBase::create(media_->getEntityByName("Sensor B")); > + if (!sensor_) > + return -ENODEV; > > debayer_ = V4L2Subdevice::fromEntityName(media_, "Debayer B"); > if (debayer_->open()) > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp > index 5c4f35324055..a35683eb4b58 100644 > --- a/src/libcamera/sensor/camera_sensor.cpp > +++ b/src/libcamera/sensor/camera_sensor.cpp > @@ -12,6 +12,7 @@ > #include <float.h> > #include <iomanip> > #include <limits.h> > +#include <map> > #include <math.h> > #include <string.h> > > @@ -1204,4 +1205,165 @@ std::string CameraSensor::logPrefix() const > return "'" + entity_->name() + "'"; > } > > +namespace { > + > +/* Transitory default camera sensor implementation */ > +class CameraSensorDefault : public CameraSensor > +{ > +public: > + CameraSensorDefault(MediaEntity *entity) > + : CameraSensor(entity) > + { > + } > + > + static bool match([[maybe_unused]] const MediaEntity *entity) > + { > + return true; > + } > + > + static std::unique_ptr<CameraSensorDefault> create(MediaEntity *entity) > + { > + std::unique_ptr<CameraSensorDefault> sensor = > + std::make_unique<CameraSensorDefault>(entity); > + > + if (sensor->init()) > + return nullptr; > + > + return sensor; > + } > +}; > + > +REGISTER_CAMERA_SENSOR(CameraSensorDefault) > + > +}; /* namespace */ > + > +/** > + * \class CameraSensorFactoryBase > + * \brief Base class for camera sensor factories > + * > + * The CameraSensorFactoryBase class is the base of all specializations of > + * the CameraSensorFactory class template. It implements the factory > + * registration, maintains a registry of factories, and provides access to the > + * registered factories. > + */ > + > +/** > + * \brief Construct a camera sensor factory base > + * > + * Creating an instance of the factory base registers it with the global list of > + * factories, accessible through the factories() function. > + */ > +CameraSensorFactoryBase::CameraSensorFactoryBase() > +{ > + registerFactory(this); > +} > + > +/** > + * \brief Create an instance of the CameraSensor corresponding to a media entity > + * \param[in] entity The media entity on the source end of the sensor > + * > + * \return A unique pointer to a new instance of the CameraSensor subclass > + * matching the entity, or a null pointer if no such factory exists > + */ > +std::unique_ptr<CameraSensor> CameraSensorFactoryBase::create(MediaEntity *entity) > +{ > + const std::vector<CameraSensorFactoryBase *> &factories = > + CameraSensorFactoryBase::factories(); > + > + for (const CameraSensorFactoryBase *factory : factories) { > + if (!factory->match(entity)) > + continue; > + > + std::unique_ptr<CameraSensor> sensor = factory->createInstance(entity); > + if (!sensor) { > + LOG(CameraSensor, Error) > + << "Failed to create sensor for '" > + << entity->name(); > + return nullptr; > + } > + > + return sensor; > + } > + > + return nullptr; > +} > + > +/** > + * \brief Retrieve the list of all camera sensor factories > + * \return The list of camera sensor factories > + */ > +std::vector<CameraSensorFactoryBase *> &CameraSensorFactoryBase::factories() > +{ > + /* > + * The static factories map is defined inside the function to ensure > + * it gets initialized on first use, without any dependency on link > + * order. > + */ > + static std::vector<CameraSensorFactoryBase *> factories; > + return factories; > +} > + > +/** > + * \brief Add a camera sensor class to the registry > + * \param[in] factory Factory to use to construct the camera sensor > + */ > +void CameraSensorFactoryBase::registerFactory(CameraSensorFactoryBase *factory) > +{ > + std::vector<CameraSensorFactoryBase *> &factories = > + CameraSensorFactoryBase::factories(); > + > + factories.push_back(factory); > +} > + > +/** > + * \class CameraSensorFactory > + * \brief Registration of CameraSensorFactory classes and creation of instances > + * \tparam _CameraSensor The camera sensor class type for this factory > + * > + * To facilitate discovery and instantiation of CameraSensor classes, the > + * CameraSensorFactory class implements auto-registration of camera sensors. > + * Each CameraSensor subclass shall register itself using the > + * REGISTER_CAMERA_SENSOR() macro, which will create a corresponding instance > + * of a CameraSensorFactory subclass and register it with the static list of > + * factories. > + */ > + > +/** > + * \fn CameraSensorFactory::CameraSensorFactory() > + * \brief Construct a camera sensor factory > + * > + * Creating an instance of the factory registers it with the global list of > + * factories, accessible through the CameraSensorFactoryBase::factories() > + * function. > + */ > + > +/** > + * \fn CameraSensorFactory::createInstance() const > + * \brief Create an instance of the CameraSensor corresponding to the factory > + * > + * \return A unique pointer to a newly constructed instance of the CameraSensor > + * subclass corresponding to the factory > + */ > + > +/** > + * \def REGISTER_CAMERA_SENSOR(sensor) > + * \brief Register a camera sensor type to the sensor factory > + * \param[in] sensor Class name of the CameraSensor derived class to register > + * > + * Register a CameraSensor subclass with the factory and make it available to > + * try and match sensors. The subclass needs to implement two static functions: > + * > + * \code{.cpp} > + * static bool match(const MediaEntity *entity); > + * static std::unique_ptr<sensor> create(MediaEntity *entity); > + * \endcode > + * > + * The match() function tests if the sensor class supports the camera sensor > + * identified by a MediaEntity. > + * > + * The create() function creates a new instance of the sensor class. It may > + * return a null pointer if initialization of the instance fails. It will only > + * be called if the match() function has returned true for the given entity. > + */ > + > } /* namespace libcamera */ > diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp > index 9503d7753fb9..acd3a1241ae1 100644 > --- a/test/camera-sensor.cpp > +++ b/test/camera-sensor.cpp > @@ -52,8 +52,8 @@ protected: > return TestFail; > } > > - sensor_ = new CameraSensor(entity); > - if (sensor_->init() < 0) { > + sensor_ = CameraSensorFactoryBase::create(entity); > + if (!sensor_) { > cerr << "Unable to initialise camera sensor" << endl; > return TestFail; > } > @@ -118,13 +118,12 @@ protected: > > void cleanup() > { > - delete sensor_; > } > > private: > std::unique_ptr<DeviceEnumerator> enumerator_; > std::shared_ptr<MediaDevice> media_; > - CameraSensor *sensor_; > + std::unique_ptr<CameraSensor> sensor_; > CameraLens *lens_; > }; > > diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp > index 1113cf5bf8cf..9fbd24cc91ea 100644 > --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp > +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp > @@ -64,8 +64,8 @@ int V4L2VideoDeviceTest::init() > format.size.height = 480; > > if (driver_ == "vimc") { > - sensor_ = new CameraSensor(media_->getEntityByName("Sensor A")); > - if (sensor_->init()) > + sensor_ = CameraSensorFactoryBase::create(media_->getEntityByName("Sensor A")); > + if (!sensor_) > return TestFail; > > debayer_ = new V4L2Subdevice(media_->getEntityByName("Debayer A")); > @@ -98,6 +98,5 @@ void V4L2VideoDeviceTest::cleanup() > capture_->close(); > > delete debayer_; > - delete sensor_; > delete capture_; > } > diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h > index d2de1a6de29f..f06db72a9df5 100644 > --- a/test/v4l2_videodevice/v4l2_videodevice_test.h > +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h > @@ -36,7 +36,7 @@ protected: > std::string entity_; > std::unique_ptr<libcamera::DeviceEnumerator> enumerator_; > std::shared_ptr<libcamera::MediaDevice> media_; > - libcamera::CameraSensor *sensor_; > + std::unique_ptr<libcamera::CameraSensor> sensor_; > libcamera::V4L2Subdevice *debayer_; > libcamera::V4L2VideoDevice *capture_; > std::vector<std::unique_ptr<libcamera::FrameBuffer>> buffers_; > -- > Regards, > > Laurent Pinchart >
diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index d05f48ebeebe..577af779cd6e 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -39,7 +39,6 @@ enum class Orientation; class CameraSensor : protected Loggable { public: - explicit CameraSensor(const MediaEntity *entity); ~CameraSensor(); int init(); @@ -82,6 +81,7 @@ public: int setTestPatternMode(controls::draft::TestPatternModeEnum mode); protected: + explicit CameraSensor(const MediaEntity *entity); std::string logPrefix() const override; private: @@ -123,4 +123,50 @@ private: std::unique_ptr<CameraLens> focusLens_; }; +class CameraSensorFactoryBase +{ +public: + CameraSensorFactoryBase(); + virtual ~CameraSensorFactoryBase() = default; + + static std::unique_ptr<CameraSensor> create(MediaEntity *entity); + +private: + LIBCAMERA_DISABLE_COPY_AND_MOVE(CameraSensorFactoryBase) + + static std::vector<CameraSensorFactoryBase *> &factories(); + + static void registerFactory(CameraSensorFactoryBase *factory); + + virtual bool match(const MediaEntity *entity) const = 0; + + virtual std::unique_ptr<CameraSensor> + createInstance(MediaEntity *entity) const = 0; +}; + +template<typename _CameraSensor> +class CameraSensorFactory final : public CameraSensorFactoryBase +{ +public: + CameraSensorFactory() + : CameraSensorFactoryBase() + { + } + +private: + bool match(const MediaEntity *entity) const override + { + return _CameraSensor::match(entity); + } + + std::unique_ptr<CameraSensor> + createInstance(MediaEntity *entity) const override + { + return _CameraSensor::create(entity); + } +}; + +#define REGISTER_CAMERA_SENSOR(sensor) \ + static CameraSensorFactory<sensor> global_##sensor##Factory{}; + } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index a3782aea2ba9..a00567c6873c 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -157,11 +157,10 @@ PipelineHandlerISI *ISICameraData::pipe() /* Open and initialize pipe components. */ int ISICameraData::init() { - int ret = sensor_->init(); - if (ret) - return ret; + if (!sensor_) + return -ENODEV; - ret = csis_->open(); + int ret = csis_->open(); if (ret) return ret; @@ -1063,7 +1062,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator) std::unique_ptr<ISICameraData> data = std::make_unique<ISICameraData>(this); - data->sensor_ = std::make_unique<CameraSensor>(sensor); + data->sensor_ = CameraSensorFactoryBase::create(sensor); data->csis_ = std::make_unique<V4L2Subdevice>(csi); data->xbarSink_ = sink; diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 43c816baf6ef..e09583ea418f 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -134,10 +134,9 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) MediaLink *link = links[0]; MediaEntity *sensorEntity = link->source()->entity(); - sensor_ = std::make_unique<CameraSensor>(sensorEntity); - ret = sensor_->init(); - if (ret) - return ret; + sensor_ = CameraSensorFactoryBase::create(sensorEntity); + if (!sensor_) + return -ENODEV; ret = link->setEnabled(true); if (ret) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index abb21968413a..1a3e7938fa91 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1109,10 +1109,9 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) std::make_unique<RkISP1CameraData>(this, &mainPath_, hasSelfPath_ ? &selfPath_ : nullptr); - data->sensor_ = std::make_unique<CameraSensor>(sensor); - ret = data->sensor_->init(); - if (ret) - return ret; + data->sensor_ = CameraSensorFactoryBase::create(sensor); + if (!data->sensor_) + return -ENODEV; /* Initialize the camera properties. */ data->properties_ = data->sensor_->properties(); diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index 7e420b3f90a4..d662c8f12145 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -772,13 +772,10 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera CameraData *data = cameraData.get(); int ret; - data->sensor_ = std::make_unique<CameraSensor>(sensorEntity); + data->sensor_ = CameraSensorFactoryBase::create(sensorEntity); if (!data->sensor_) return -EINVAL; - if (data->sensor_->init()) - return -EINVAL; - /* Populate the map of sensor supported formats and sizes. */ for (auto const mbusCode : data->sensor_->mbusCodes()) data->sensorFormats_.emplace(mbusCode, diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 01f2a97798ba..0bd021388c8f 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -371,8 +371,6 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, MediaEntity *sensor) : Camera::Private(pipe), streams_(numStreams) { - int ret; - /* * Find the shortest path from the camera sensor to a video capture * device using the breadth-first search algorithm. This heuristic will @@ -463,12 +461,9 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, } /* Finally also remember the sensor. */ - sensor_ = std::make_unique<CameraSensor>(sensor); - ret = sensor_->init(); - if (ret) { - sensor_.reset(); + sensor_ = CameraSensorFactoryBase::create(sensor); + if (!sensor_) return; - } LOG(SimplePipeline, Debug) << "Found pipeline: " diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 5e66ee1d26c1..ae0ca21907ec 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -510,10 +510,9 @@ int VimcCameraData::init() return ret; /* Create and open the camera sensor, debayer, scaler and video device. */ - sensor_ = std::make_unique<CameraSensor>(media_->getEntityByName("Sensor B")); - ret = sensor_->init(); - if (ret) - return ret; + sensor_ = CameraSensorFactoryBase::create(media_->getEntityByName("Sensor B")); + if (!sensor_) + return -ENODEV; debayer_ = V4L2Subdevice::fromEntityName(media_, "Debayer B"); if (debayer_->open()) diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp index 5c4f35324055..a35683eb4b58 100644 --- a/src/libcamera/sensor/camera_sensor.cpp +++ b/src/libcamera/sensor/camera_sensor.cpp @@ -12,6 +12,7 @@ #include <float.h> #include <iomanip> #include <limits.h> +#include <map> #include <math.h> #include <string.h> @@ -1204,4 +1205,165 @@ std::string CameraSensor::logPrefix() const return "'" + entity_->name() + "'"; } +namespace { + +/* Transitory default camera sensor implementation */ +class CameraSensorDefault : public CameraSensor +{ +public: + CameraSensorDefault(MediaEntity *entity) + : CameraSensor(entity) + { + } + + static bool match([[maybe_unused]] const MediaEntity *entity) + { + return true; + } + + static std::unique_ptr<CameraSensorDefault> create(MediaEntity *entity) + { + std::unique_ptr<CameraSensorDefault> sensor = + std::make_unique<CameraSensorDefault>(entity); + + if (sensor->init()) + return nullptr; + + return sensor; + } +}; + +REGISTER_CAMERA_SENSOR(CameraSensorDefault) + +}; /* namespace */ + +/** + * \class CameraSensorFactoryBase + * \brief Base class for camera sensor factories + * + * The CameraSensorFactoryBase class is the base of all specializations of + * the CameraSensorFactory class template. It implements the factory + * registration, maintains a registry of factories, and provides access to the + * registered factories. + */ + +/** + * \brief Construct a camera sensor factory base + * + * Creating an instance of the factory base registers it with the global list of + * factories, accessible through the factories() function. + */ +CameraSensorFactoryBase::CameraSensorFactoryBase() +{ + registerFactory(this); +} + +/** + * \brief Create an instance of the CameraSensor corresponding to a media entity + * \param[in] entity The media entity on the source end of the sensor + * + * \return A unique pointer to a new instance of the CameraSensor subclass + * matching the entity, or a null pointer if no such factory exists + */ +std::unique_ptr<CameraSensor> CameraSensorFactoryBase::create(MediaEntity *entity) +{ + const std::vector<CameraSensorFactoryBase *> &factories = + CameraSensorFactoryBase::factories(); + + for (const CameraSensorFactoryBase *factory : factories) { + if (!factory->match(entity)) + continue; + + std::unique_ptr<CameraSensor> sensor = factory->createInstance(entity); + if (!sensor) { + LOG(CameraSensor, Error) + << "Failed to create sensor for '" + << entity->name(); + return nullptr; + } + + return sensor; + } + + return nullptr; +} + +/** + * \brief Retrieve the list of all camera sensor factories + * \return The list of camera sensor factories + */ +std::vector<CameraSensorFactoryBase *> &CameraSensorFactoryBase::factories() +{ + /* + * The static factories map is defined inside the function to ensure + * it gets initialized on first use, without any dependency on link + * order. + */ + static std::vector<CameraSensorFactoryBase *> factories; + return factories; +} + +/** + * \brief Add a camera sensor class to the registry + * \param[in] factory Factory to use to construct the camera sensor + */ +void CameraSensorFactoryBase::registerFactory(CameraSensorFactoryBase *factory) +{ + std::vector<CameraSensorFactoryBase *> &factories = + CameraSensorFactoryBase::factories(); + + factories.push_back(factory); +} + +/** + * \class CameraSensorFactory + * \brief Registration of CameraSensorFactory classes and creation of instances + * \tparam _CameraSensor The camera sensor class type for this factory + * + * To facilitate discovery and instantiation of CameraSensor classes, the + * CameraSensorFactory class implements auto-registration of camera sensors. + * Each CameraSensor subclass shall register itself using the + * REGISTER_CAMERA_SENSOR() macro, which will create a corresponding instance + * of a CameraSensorFactory subclass and register it with the static list of + * factories. + */ + +/** + * \fn CameraSensorFactory::CameraSensorFactory() + * \brief Construct a camera sensor factory + * + * Creating an instance of the factory registers it with the global list of + * factories, accessible through the CameraSensorFactoryBase::factories() + * function. + */ + +/** + * \fn CameraSensorFactory::createInstance() const + * \brief Create an instance of the CameraSensor corresponding to the factory + * + * \return A unique pointer to a newly constructed instance of the CameraSensor + * subclass corresponding to the factory + */ + +/** + * \def REGISTER_CAMERA_SENSOR(sensor) + * \brief Register a camera sensor type to the sensor factory + * \param[in] sensor Class name of the CameraSensor derived class to register + * + * Register a CameraSensor subclass with the factory and make it available to + * try and match sensors. The subclass needs to implement two static functions: + * + * \code{.cpp} + * static bool match(const MediaEntity *entity); + * static std::unique_ptr<sensor> create(MediaEntity *entity); + * \endcode + * + * The match() function tests if the sensor class supports the camera sensor + * identified by a MediaEntity. + * + * The create() function creates a new instance of the sensor class. It may + * return a null pointer if initialization of the instance fails. It will only + * be called if the match() function has returned true for the given entity. + */ + } /* namespace libcamera */ diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp index 9503d7753fb9..acd3a1241ae1 100644 --- a/test/camera-sensor.cpp +++ b/test/camera-sensor.cpp @@ -52,8 +52,8 @@ protected: return TestFail; } - sensor_ = new CameraSensor(entity); - if (sensor_->init() < 0) { + sensor_ = CameraSensorFactoryBase::create(entity); + if (!sensor_) { cerr << "Unable to initialise camera sensor" << endl; return TestFail; } @@ -118,13 +118,12 @@ protected: void cleanup() { - delete sensor_; } private: std::unique_ptr<DeviceEnumerator> enumerator_; std::shared_ptr<MediaDevice> media_; - CameraSensor *sensor_; + std::unique_ptr<CameraSensor> sensor_; CameraLens *lens_; }; diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp index 1113cf5bf8cf..9fbd24cc91ea 100644 --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp @@ -64,8 +64,8 @@ int V4L2VideoDeviceTest::init() format.size.height = 480; if (driver_ == "vimc") { - sensor_ = new CameraSensor(media_->getEntityByName("Sensor A")); - if (sensor_->init()) + sensor_ = CameraSensorFactoryBase::create(media_->getEntityByName("Sensor A")); + if (!sensor_) return TestFail; debayer_ = new V4L2Subdevice(media_->getEntityByName("Debayer A")); @@ -98,6 +98,5 @@ void V4L2VideoDeviceTest::cleanup() capture_->close(); delete debayer_; - delete sensor_; delete capture_; } diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h index d2de1a6de29f..f06db72a9df5 100644 --- a/test/v4l2_videodevice/v4l2_videodevice_test.h +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h @@ -36,7 +36,7 @@ protected: std::string entity_; std::unique_ptr<libcamera::DeviceEnumerator> enumerator_; std::shared_ptr<libcamera::MediaDevice> media_; - libcamera::CameraSensor *sensor_; + std::unique_ptr<libcamera::CameraSensor> sensor_; libcamera::V4L2Subdevice *debayer_; libcamera::V4L2VideoDevice *capture_; std::vector<std::unique_ptr<libcamera::FrameBuffer>> buffers_;