| Message ID | 20190527223540.21855-3-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, On 27/05/2019 23:35, Paul Elder wrote: > In order to match an IPA module with a pipeline handler, the pipeline > handler must have a name and a major and minor version. Add these to > PipelineHandler as functions, so that they can be automatically defined > by REGISTER_PIPELINE_HANDLER. Also update documentation accordingly. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/include/pipeline_handler.h | 20 ++++++++++++++++++-- > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++++- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++++- > src/libcamera/pipeline/uvcvideo.cpp | 6 +++++- > src/libcamera/pipeline/vimc.cpp | 6 +++++- > src/libcamera/pipeline_handler.cpp | 20 ++++++++++++++++++++ > 6 files changed, 58 insertions(+), 6 deletions(-) > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > index 7da6df1..7963e7a 100644 > --- a/src/libcamera/include/pipeline_handler.h > +++ b/src/libcamera/include/pipeline_handler.h > @@ -77,6 +77,10 @@ public: > bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); > void completeRequest(Camera *camera, Request *request); > > + virtual const char *name() = 0; > + virtual int majorVersion() = 0; > + virtual int minorVersion() = 0; > + > protected: > void registerCamera(std::shared_ptr<Camera> camera, > std::unique_ptr<CameraData> data); > @@ -112,7 +116,7 @@ private: > std::string name_; > }; > > -#define REGISTER_PIPELINE_HANDLER(handler) \ > +#define REGISTER_PIPELINE_HANDLER(handler, majorV, minorV) \ > class handler##Factory final : public PipelineHandlerFactory \ > { \ > public: \ > @@ -122,7 +126,19 @@ public: \ > return std::make_shared<handler>(manager); \ > } \ > }; \ > -static handler##Factory global_##handler##Factory; > +static handler##Factory global_##handler##Factory; \ > +const char *handler::name() \ > +{ \ > + return #handler; \ > +} \ > +int handler::majorVersion() \ > +{ \ > + return majorV; \ > +} \ > +int handler::minorVersion() \ > +{ \ > + return minorV; \ > +} Instead of duplicating these in each class, can they be put into the base class at construction time, and handled by member functions there? > } /* namespace libcamera */ > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 05005c4..a1b06fe 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -212,6 +212,10 @@ public: > > bool match(DeviceEnumerator *enumerator) override; > > + const char *name() override; > + int majorVersion() override; > + int minorVersion() override; Then we wouldn't have to add these overrides to each handler... > + > private: > IPU3CameraData *cameraData(const Camera *camera) > { > @@ -1452,6 +1456,6 @@ int CIO2Device::mediaBusToFormat(unsigned int code) > } > } > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3); > +REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, 0, 1); > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 9b3eea2..7042e7f 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -92,6 +92,10 @@ public: > > bool match(DeviceEnumerator *enumerator) override; > > + const char *name() override; > + int majorVersion() override; > + int minorVersion() override; > + > private: > RkISP1CameraData *cameraData(const Camera *camera) > { > @@ -499,6 +503,6 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) > completeRequest(activeCamera_, request); > } > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1); > +REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1, 0, 1); Do we define the rules anywhere on when the version number should be updated? Could we potentially automate this in the future in anyway? (checksum of the PipelineHandler? (probably too crazy, and not linear for matching and tracking backwards compatibility) > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 45260f3..27c731e 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -68,6 +68,10 @@ public: > > bool match(DeviceEnumerator *enumerator) override; > > + const char *name() override; > + int majorVersion() override; > + int minorVersion() override; > + > private: > UVCCameraData *cameraData(const Camera *camera) > { > @@ -263,6 +267,6 @@ void UVCCameraData::bufferReady(Buffer *buffer) > pipe_->completeRequest(camera_, request); > } > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC); > +REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC, 0, 1); > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 0e4eede..d3ff527 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -71,6 +71,10 @@ public: > > bool match(DeviceEnumerator *enumerator) override; > > + const char *name() override; > + int majorVersion() override; > + int minorVersion() override; > + > private: > VimcCameraData *cameraData(const Camera *camera) > { > @@ -274,6 +278,6 @@ void VimcCameraData::bufferReady(Buffer *buffer) > pipe_->completeRequest(camera_, request); > } > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc); > +REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc, 0, 1); > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index dd56907..8f83307 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -505,6 +505,24 @@ CameraData *PipelineHandler::cameraData(const Camera *camera) > * constant for the whole lifetime of the pipeline handler. > */ > > +/** > + * \fn PipelineHandler::name() > + * \brief Retrieve the pipeline handler name > + * \return The pipeline handler name > + */ > + > +/** > + * \fn PipelineHandler::majorVersion() > + * \brief Retrieve the pipeline handler major version > + * \return The pipeline handler major version > + */ > + > +/** > + * \fn PipelineHandler::minorVersion() > + * \brief Retrieve the pipeline handler minor version > + * \return The pipeline handler minor version > + */ > + > /** > * \class PipelineHandlerFactory > * \brief Registration of PipelineHandler classes and creation of instances > @@ -586,6 +604,8 @@ std::vector<PipelineHandlerFactory *> &PipelineHandlerFactory::factories() > * \def REGISTER_PIPELINE_HANDLER > * \brief Register a pipeline handler with the pipeline handler factory > * \param[in] handler Class name of PipelineHandler derived class to register > + * \param[in] majorV Major version of the PipelineHandler > + * \param[in] minorV Minor version of the PipelineHandler > * > * Register a PipelineHandler subclass with the factory and make it available to > * try and match devices. >
Hi Paul, Thanks for your work. On 2019-05-27 18:35:34 -0400, Paul Elder wrote: > In order to match an IPA module with a pipeline handler, the pipeline > handler must have a name and a major and minor version. Add these to > PipelineHandler as functions, so that they can be automatically defined > by REGISTER_PIPELINE_HANDLER. Also update documentation accordingly. I'm a bit curious why you went with a major and minor version identifiers. I understand they can be useful to describe API version (major) and bugfix state (minior). My question is will this be useful for PipelineHandler <-> IPA matching? I have really bad experience working with major.minor versioning. In my experience it's likely to creep in API changes in minor bug fixes even if that is not the intent. Once that happens once, one of two things are likely to follow, 1. On a new release of libcamera all pipeline handlers needs to be tested with all minor versions of major versions IPA's not shipped with libcamera claim to support. 2. Begin to exclusively match on a list of known good major.minior combinations. Preventing libcamera to work with bugfixes of 3rd party IPA who might be well behaved. Maybe I'm being cynical but I would like for us to consider a single integer as a version indicator. This version would describe the API between pipeline handler and IPA and not the version of the components themself. Or maybe I'm misunderstanding the advantages of major.minor design in this problem ;-) > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/include/pipeline_handler.h | 20 ++++++++++++++++++-- > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++++- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++++- > src/libcamera/pipeline/uvcvideo.cpp | 6 +++++- > src/libcamera/pipeline/vimc.cpp | 6 +++++- > src/libcamera/pipeline_handler.cpp | 20 ++++++++++++++++++++ > 6 files changed, 58 insertions(+), 6 deletions(-) > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > index 7da6df1..7963e7a 100644 > --- a/src/libcamera/include/pipeline_handler.h > +++ b/src/libcamera/include/pipeline_handler.h > @@ -77,6 +77,10 @@ public: > bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); > void completeRequest(Camera *camera, Request *request); > > + virtual const char *name() = 0; > + virtual int majorVersion() = 0; > + virtual int minorVersion() = 0; > + > protected: > void registerCamera(std::shared_ptr<Camera> camera, > std::unique_ptr<CameraData> data); > @@ -112,7 +116,7 @@ private: > std::string name_; > }; > > -#define REGISTER_PIPELINE_HANDLER(handler) \ > +#define REGISTER_PIPELINE_HANDLER(handler, majorV, minorV) \ > class handler##Factory final : public PipelineHandlerFactory \ > { \ > public: \ > @@ -122,7 +126,19 @@ public: \ > return std::make_shared<handler>(manager); \ > } \ > }; \ > -static handler##Factory global_##handler##Factory; > +static handler##Factory global_##handler##Factory; \ > +const char *handler::name() \ > +{ \ > + return #handler; \ > +} \ > +int handler::majorVersion() \ > +{ \ > + return majorV; \ > +} \ > +int handler::minorVersion() \ > +{ \ > + return minorV; \ > +} > > } /* namespace libcamera */ > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 05005c4..a1b06fe 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -212,6 +212,10 @@ public: > > bool match(DeviceEnumerator *enumerator) override; > > + const char *name() override; > + int majorVersion() override; > + int minorVersion() override; > + > private: > IPU3CameraData *cameraData(const Camera *camera) > { > @@ -1452,6 +1456,6 @@ int CIO2Device::mediaBusToFormat(unsigned int code) > } > } > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3); > +REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, 0, 1); > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 9b3eea2..7042e7f 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -92,6 +92,10 @@ public: > > bool match(DeviceEnumerator *enumerator) override; > > + const char *name() override; > + int majorVersion() override; > + int minorVersion() override; > + > private: > RkISP1CameraData *cameraData(const Camera *camera) > { > @@ -499,6 +503,6 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) > completeRequest(activeCamera_, request); > } > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1); > +REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1, 0, 1); > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 45260f3..27c731e 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -68,6 +68,10 @@ public: > > bool match(DeviceEnumerator *enumerator) override; > > + const char *name() override; > + int majorVersion() override; > + int minorVersion() override; > + > private: > UVCCameraData *cameraData(const Camera *camera) > { > @@ -263,6 +267,6 @@ void UVCCameraData::bufferReady(Buffer *buffer) > pipe_->completeRequest(camera_, request); > } > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC); > +REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC, 0, 1); > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 0e4eede..d3ff527 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -71,6 +71,10 @@ public: > > bool match(DeviceEnumerator *enumerator) override; > > + const char *name() override; > + int majorVersion() override; > + int minorVersion() override; > + > private: > VimcCameraData *cameraData(const Camera *camera) > { > @@ -274,6 +278,6 @@ void VimcCameraData::bufferReady(Buffer *buffer) > pipe_->completeRequest(camera_, request); > } > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc); > +REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc, 0, 1); > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index dd56907..8f83307 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -505,6 +505,24 @@ CameraData *PipelineHandler::cameraData(const Camera *camera) > * constant for the whole lifetime of the pipeline handler. > */ > > +/** > + * \fn PipelineHandler::name() > + * \brief Retrieve the pipeline handler name > + * \return The pipeline handler name > + */ > + > +/** > + * \fn PipelineHandler::majorVersion() > + * \brief Retrieve the pipeline handler major version > + * \return The pipeline handler major version > + */ > + > +/** > + * \fn PipelineHandler::minorVersion() > + * \brief Retrieve the pipeline handler minor version > + * \return The pipeline handler minor version > + */ > + > /** > * \class PipelineHandlerFactory > * \brief Registration of PipelineHandler classes and creation of instances > @@ -586,6 +604,8 @@ std::vector<PipelineHandlerFactory *> &PipelineHandlerFactory::factories() > * \def REGISTER_PIPELINE_HANDLER > * \brief Register a pipeline handler with the pipeline handler factory > * \param[in] handler Class name of PipelineHandler derived class to register > + * \param[in] majorV Major version of the PipelineHandler > + * \param[in] minorV Minor version of the PipelineHandler > * > * Register a PipelineHandler subclass with the factory and make it available to > * try and match devices. > -- > 2.20.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, On Tue, Jun 04, 2019 at 12:21:10AM +0200, Niklas Söderlund wrote: > On 2019-05-27 18:35:34 -0400, Paul Elder wrote: > > In order to match an IPA module with a pipeline handler, the pipeline > > handler must have a name and a major and minor version. Add these to > > PipelineHandler as functions, so that they can be automatically defined > > by REGISTER_PIPELINE_HANDLER. Also update documentation accordingly. > > I'm a bit curious why you went with a major and minor version > identifiers. I understand they can be useful to describe API version > (major) and bugfix state (minior). > > My question is will this be useful for PipelineHandler <-> IPA matching? > > I have really bad experience working with major.minor versioning. In my > experience it's likely to creep in API changes in minor bug fixes even > if that is not the intent. Once that happens once, one of two things are > likely to follow, > > 1. On a new release of libcamera all pipeline handlers needs to be > tested with all minor versions of major versions IPA's not > shipped with libcamera claim to support. > > 2. Begin to exclusively match on a list of known good major.minior > combinations. Preventing libcamera to work with bugfixes of 3rd > party IPA who might be well behaved. > > Maybe I'm being cynical but I would like for us to consider a single > integer as a version indicator. This version would describe the API > between pipeline handler and IPA and not the version of the components > themself. Or maybe I'm misunderstanding the advantages of major.minor > design in this problem ;-) Our problem here is that some IPAs will be shipped as closed-source .so that we won't be able to update, blocking the update of libcamera. If a small change is needed in the API between a pipeline handler and its IPA, in order to enable a new feature, and that change can be done without breaking backward compatibility, we don't want all the existing IPAs to stop working. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > src/libcamera/include/pipeline_handler.h | 20 ++++++++++++++++++-- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++++- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++++- > > src/libcamera/pipeline/uvcvideo.cpp | 6 +++++- > > src/libcamera/pipeline/vimc.cpp | 6 +++++- > > src/libcamera/pipeline_handler.cpp | 20 ++++++++++++++++++++ > > 6 files changed, 58 insertions(+), 6 deletions(-) > > > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > > index 7da6df1..7963e7a 100644 > > --- a/src/libcamera/include/pipeline_handler.h > > +++ b/src/libcamera/include/pipeline_handler.h > > @@ -77,6 +77,10 @@ public: > > bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); > > void completeRequest(Camera *camera, Request *request); > > > > + virtual const char *name() = 0; > > + virtual int majorVersion() = 0; > > + virtual int minorVersion() = 0; > > + > > protected: > > void registerCamera(std::shared_ptr<Camera> camera, > > std::unique_ptr<CameraData> data); > > @@ -112,7 +116,7 @@ private: > > std::string name_; > > }; > > > > -#define REGISTER_PIPELINE_HANDLER(handler) \ > > +#define REGISTER_PIPELINE_HANDLER(handler, majorV, minorV) \ > > class handler##Factory final : public PipelineHandlerFactory \ > > { \ > > public: \ > > @@ -122,7 +126,19 @@ public: \ > > return std::make_shared<handler>(manager); \ > > } \ > > }; \ > > -static handler##Factory global_##handler##Factory; > > +static handler##Factory global_##handler##Factory; \ > > +const char *handler::name() \ > > +{ \ > > + return #handler; \ > > +} \ > > +int handler::majorVersion() \ > > +{ \ > > + return majorV; \ > > +} \ > > +int handler::minorVersion() \ > > +{ \ > > + return minorV; \ > > +} > > > > } /* namespace libcamera */ > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index 05005c4..a1b06fe 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -212,6 +212,10 @@ public: > > > > bool match(DeviceEnumerator *enumerator) override; > > > > + const char *name() override; > > + int majorVersion() override; > > + int minorVersion() override; > > + > > private: > > IPU3CameraData *cameraData(const Camera *camera) > > { > > @@ -1452,6 +1456,6 @@ int CIO2Device::mediaBusToFormat(unsigned int code) > > } > > } > > > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3); > > +REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, 0, 1); > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > index 9b3eea2..7042e7f 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > @@ -92,6 +92,10 @@ public: > > > > bool match(DeviceEnumerator *enumerator) override; > > > > + const char *name() override; > > + int majorVersion() override; > > + int minorVersion() override; > > + > > private: > > RkISP1CameraData *cameraData(const Camera *camera) > > { > > @@ -499,6 +503,6 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) > > completeRequest(activeCamera_, request); > > } > > > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1); > > +REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1, 0, 1); > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > > index 45260f3..27c731e 100644 > > --- a/src/libcamera/pipeline/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo.cpp > > @@ -68,6 +68,10 @@ public: > > > > bool match(DeviceEnumerator *enumerator) override; > > > > + const char *name() override; > > + int majorVersion() override; > > + int minorVersion() override; > > + > > private: > > UVCCameraData *cameraData(const Camera *camera) > > { > > @@ -263,6 +267,6 @@ void UVCCameraData::bufferReady(Buffer *buffer) > > pipe_->completeRequest(camera_, request); > > } > > > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC); > > +REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC, 0, 1); > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > > index 0e4eede..d3ff527 100644 > > --- a/src/libcamera/pipeline/vimc.cpp > > +++ b/src/libcamera/pipeline/vimc.cpp > > @@ -71,6 +71,10 @@ public: > > > > bool match(DeviceEnumerator *enumerator) override; > > > > + const char *name() override; > > + int majorVersion() override; > > + int minorVersion() override; > > + > > private: > > VimcCameraData *cameraData(const Camera *camera) > > { > > @@ -274,6 +278,6 @@ void VimcCameraData::bufferReady(Buffer *buffer) > > pipe_->completeRequest(camera_, request); > > } > > > > -REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc); > > +REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc, 0, 1); > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > index dd56907..8f83307 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -505,6 +505,24 @@ CameraData *PipelineHandler::cameraData(const Camera *camera) > > * constant for the whole lifetime of the pipeline handler. > > */ > > > > +/** > > + * \fn PipelineHandler::name() > > + * \brief Retrieve the pipeline handler name > > + * \return The pipeline handler name > > + */ > > + > > +/** > > + * \fn PipelineHandler::majorVersion() > > + * \brief Retrieve the pipeline handler major version > > + * \return The pipeline handler major version > > + */ > > + > > +/** > > + * \fn PipelineHandler::minorVersion() > > + * \brief Retrieve the pipeline handler minor version > > + * \return The pipeline handler minor version > > + */ > > + > > /** > > * \class PipelineHandlerFactory > > * \brief Registration of PipelineHandler classes and creation of instances > > @@ -586,6 +604,8 @@ std::vector<PipelineHandlerFactory *> &PipelineHandlerFactory::factories() > > * \def REGISTER_PIPELINE_HANDLER > > * \brief Register a pipeline handler with the pipeline handler factory > > * \param[in] handler Class name of PipelineHandler derived class to register > > + * \param[in] majorV Major version of the PipelineHandler > > + * \param[in] minorV Minor version of the PipelineHandler > > * > > * Register a PipelineHandler subclass with the factory and make it available to > > * try and match devices.
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 7da6df1..7963e7a 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -77,6 +77,10 @@ public: bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); void completeRequest(Camera *camera, Request *request); + virtual const char *name() = 0; + virtual int majorVersion() = 0; + virtual int minorVersion() = 0; + protected: void registerCamera(std::shared_ptr<Camera> camera, std::unique_ptr<CameraData> data); @@ -112,7 +116,7 @@ private: std::string name_; }; -#define REGISTER_PIPELINE_HANDLER(handler) \ +#define REGISTER_PIPELINE_HANDLER(handler, majorV, minorV) \ class handler##Factory final : public PipelineHandlerFactory \ { \ public: \ @@ -122,7 +126,19 @@ public: \ return std::make_shared<handler>(manager); \ } \ }; \ -static handler##Factory global_##handler##Factory; +static handler##Factory global_##handler##Factory; \ +const char *handler::name() \ +{ \ + return #handler; \ +} \ +int handler::majorVersion() \ +{ \ + return majorV; \ +} \ +int handler::minorVersion() \ +{ \ + return minorV; \ +} } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 05005c4..a1b06fe 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -212,6 +212,10 @@ public: bool match(DeviceEnumerator *enumerator) override; + const char *name() override; + int majorVersion() override; + int minorVersion() override; + private: IPU3CameraData *cameraData(const Camera *camera) { @@ -1452,6 +1456,6 @@ int CIO2Device::mediaBusToFormat(unsigned int code) } } -REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3); +REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, 0, 1); } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 9b3eea2..7042e7f 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -92,6 +92,10 @@ public: bool match(DeviceEnumerator *enumerator) override; + const char *name() override; + int majorVersion() override; + int minorVersion() override; + private: RkISP1CameraData *cameraData(const Camera *camera) { @@ -499,6 +503,6 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) completeRequest(activeCamera_, request); } -REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1); +REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1, 0, 1); } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 45260f3..27c731e 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -68,6 +68,10 @@ public: bool match(DeviceEnumerator *enumerator) override; + const char *name() override; + int majorVersion() override; + int minorVersion() override; + private: UVCCameraData *cameraData(const Camera *camera) { @@ -263,6 +267,6 @@ void UVCCameraData::bufferReady(Buffer *buffer) pipe_->completeRequest(camera_, request); } -REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC); +REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC, 0, 1); } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 0e4eede..d3ff527 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -71,6 +71,10 @@ public: bool match(DeviceEnumerator *enumerator) override; + const char *name() override; + int majorVersion() override; + int minorVersion() override; + private: VimcCameraData *cameraData(const Camera *camera) { @@ -274,6 +278,6 @@ void VimcCameraData::bufferReady(Buffer *buffer) pipe_->completeRequest(camera_, request); } -REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc); +REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc, 0, 1); } /* namespace libcamera */ diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index dd56907..8f83307 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -505,6 +505,24 @@ CameraData *PipelineHandler::cameraData(const Camera *camera) * constant for the whole lifetime of the pipeline handler. */ +/** + * \fn PipelineHandler::name() + * \brief Retrieve the pipeline handler name + * \return The pipeline handler name + */ + +/** + * \fn PipelineHandler::majorVersion() + * \brief Retrieve the pipeline handler major version + * \return The pipeline handler major version + */ + +/** + * \fn PipelineHandler::minorVersion() + * \brief Retrieve the pipeline handler minor version + * \return The pipeline handler minor version + */ + /** * \class PipelineHandlerFactory * \brief Registration of PipelineHandler classes and creation of instances @@ -586,6 +604,8 @@ std::vector<PipelineHandlerFactory *> &PipelineHandlerFactory::factories() * \def REGISTER_PIPELINE_HANDLER * \brief Register a pipeline handler with the pipeline handler factory * \param[in] handler Class name of PipelineHandler derived class to register + * \param[in] majorV Major version of the PipelineHandler + * \param[in] minorV Minor version of the PipelineHandler * * Register a PipelineHandler subclass with the factory and make it available to * try and match devices.
In order to match an IPA module with a pipeline handler, the pipeline handler must have a name and a major and minor version. Add these to PipelineHandler as functions, so that they can be automatically defined by REGISTER_PIPELINE_HANDLER. Also update documentation accordingly. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- src/libcamera/include/pipeline_handler.h | 20 ++++++++++++++++++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++++- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++++- src/libcamera/pipeline/uvcvideo.cpp | 6 +++++- src/libcamera/pipeline/vimc.cpp | 6 +++++- src/libcamera/pipeline_handler.cpp | 20 ++++++++++++++++++++ 6 files changed, 58 insertions(+), 6 deletions(-)