Message ID | 20251003-ipa-match-by-name-v1-1-07b796729412@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi 2025. 10. 03. 11:27 keltezéssel, Jacopo Mondi írta: > From: Hans de Goede <hdegoede@redhat.com> > > Currently createIPA() / IPAManager::module() assume that there is a 1:1 > relationship between pipeline handlers and IPAs and IPA matching is done > based on matching the pipe to ipaModuleInfo.pipelineName[]. > > One way to allow using a single IPA with multiple pipelines would be to > allow the IPA to declare itself compatible with more than one pipeline, > turning ipaModuleInfo.pipelineName[] into e.g. a vector. But the way > ipaModuleInfo is loaded as an ELF symbol requires it to be a simple flat > C-struct. > > Instead, move the IPA creation procedure to be name-based, introducing > an overload to IPAManager::createIPA(pipe, name, minVer, maxVer) that > allows to specify the name of the IPA module to match. Pipeline handlers > that wants to use their name as matching criteria can continue doing so > using the already existing createIPA(pipe, minVer, maxVer) overload. When this was first proposed, I looked at the possibility of adding the name to the generated proxy type as a static constexpr member to avoid any potential type confusion. But sadly the way the raspberry pi modules are handled makes that more complicated. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > include/libcamera/internal/ipa_manager.h | 14 +++++++++++-- > include/libcamera/internal/ipa_module.h | 4 ++-- > src/libcamera/ipa_manager.cpp | 34 ++++++++++++++++++++++++++------ > src/libcamera/ipa_module.cpp | 12 +++++------ > 4 files changed, 48 insertions(+), 16 deletions(-) > > diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h > index f8ce780169e617088557888d7c8dae2d3f10ec08..8bc8680be0a4a6db0cdb57c1a9400c23efecda0c 100644 > --- a/include/libcamera/internal/ipa_manager.h > +++ b/include/libcamera/internal/ipa_manager.h > @@ -34,12 +34,14 @@ public: > > template<typename T> > static std::unique_ptr<T> createIPA(PipelineHandler *pipe, > + const char *name, > uint32_t minVersion, > uint32_t maxVersion) > + > { > CameraManager *cm = pipe->cameraManager(); Since `pipe` is only used to get the camera manager, could we take that as argument instead? Regards, Barnabás Pőcze > IPAManager *self = cm->_d()->ipaManager(); > - IPAModule *m = self->module(pipe, minVersion, maxVersion); > + IPAModule *m = self->module(name, minVersion, maxVersion); > if (!m) > return nullptr; > > @@ -60,6 +62,14 @@ public: > return proxy; > } > > + template<typename T> > + static std::unique_ptr<T> createIPA(PipelineHandler *pipe, > + uint32_t minVersion, > + uint32_t maxVersion) > + { > + return createIPA<T>(pipe, pipe->name(), minVersion, maxVersion); > + } > + > #if HAVE_IPA_PUBKEY > static const PubKey &pubKey() > { > @@ -72,7 +82,7 @@ private: > std::vector<std::string> &files); > unsigned int addDir(const char *libDir, unsigned int maxDepth = 0); > > - IPAModule *module(PipelineHandler *pipe, uint32_t minVersion, > + IPAModule *module(const char *name, uint32_t minVersion, > uint32_t maxVersion); > > bool isSignatureValid(IPAModule *ipa) const; > diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h > index 15f19492c3a027a0bc4f9572188d13af41fcd450..a0a53764e1394abed3bab92cdde9f33a86441c5f 100644 > --- a/include/libcamera/internal/ipa_module.h > +++ b/include/libcamera/internal/ipa_module.h > @@ -36,8 +36,8 @@ public: > > IPAInterface *createInterface(); > > - bool match(PipelineHandler *pipe, > - uint32_t minVersion, uint32_t maxVersion) const; > + bool match(const char *name, uint32_t minVersion, > + uint32_t maxVersion) const; > > protected: > std::string logPrefix() const override; > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp > index 35171d097136a6d85b8f518c099f8228f9eacd6f..f62a4ee5fd012d23ce178d59f84c6ab49513376b 100644 > --- a/src/libcamera/ipa_manager.cpp > +++ b/src/libcamera/ipa_manager.cpp > @@ -247,15 +247,15 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth) > > /** > * \brief Retrieve an IPA module that matches a given pipeline handler > - * \param[in] pipe The pipeline handler > + * \param[in] name The IPA module string identifier > * \param[in] minVersion Minimum acceptable version of IPA module > * \param[in] maxVersion Maximum acceptable version of IPA module > */ > -IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, > +IPAModule *IPAManager::module(const char *name, uint32_t minVersion, > uint32_t maxVersion) > { > for (const auto &module : modules_) { > - if (module->match(pipe, minVersion, maxVersion)) > + if (module->match(name, minVersion, maxVersion)) > return module.get(); > } > > @@ -263,12 +263,34 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, > } > > /** > - * \fn IPAManager::createIPA() > - * \brief Create an IPA proxy that matches a given pipeline handler > - * \param[in] pipe The pipeline handler that wants a matching IPA proxy > + * \fn IPAManager::createIPA(PipelineHandler *pipe, const char *ipaName, uint32_t minVersion, uint32_t maxVersion) > + * \brief Create an IPA proxy that matches the requested name and version > + * \param[in] pipe The pipeline handler that wants to create the IPA module > + * \param[in] ipaName The IPA module name > * \param[in] minVersion Minimum acceptable version of IPA module > * \param[in] maxVersion Maximum acceptable version of IPA module > * > + * Create an IPA module using \a name as the matching identifier. This overload > + * allows pipeline handlers to create an IPA module by specifying its name > + * instead of relying on the fact that the IPA module matches the pipeline > + * handler's one. > + * > + * \return A newly created IPA proxy, or nullptr if no matching IPA module is > + * found or if the IPA proxy fails to initialize > + */ > + > +/** > + * \fn IPAManager::createIPA(PipelineHandler *pipe, uint32_t minVersion, uint32_t maxVersion) > + * \brief Create an IPA proxy that matches the pipeline handler name and the > + * requested version > + * \param[in] pipe The pipeline handler that wants to create the IPA module > + * \param[in] minVersion Minimum acceptable version of IPA module > + * \param[in] maxVersion Maximum acceptable version of IPA module > + * > + * Create an IPA module using the pipeline handler name as the matching > + * identifier. This overload allows pipeline handler to create an IPA module > + * whose name matches the pipeline handler one. > + * > * \return A newly created IPA proxy, or nullptr if no matching IPA module is > * found or if the IPA proxy fails to initialize > */ > diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp > index e6ea61e4482983a83b4185c4309f2c514ed24fc2..0bd6f14626fe2038072f48b70ca4341b0eb8cef5 100644 > --- a/src/libcamera/ipa_module.cpp > +++ b/src/libcamera/ipa_module.cpp > @@ -463,21 +463,21 @@ IPAInterface *IPAModule::createInterface() > > /** > * \brief Verify if the IPA module matches a given pipeline handler > - * \param[in] pipe Pipeline handler to match with > + * \param[in] name The IPA module name > * \param[in] minVersion Minimum acceptable version of IPA module > * \param[in] maxVersion Maximum acceptable version of IPA module > * > - * This function checks if this IPA module matches the \a pipe pipeline handler, > + * This function checks if this IPA module matches the requested \a name > * and the input version range. > * > - * \return True if the pipeline handler matches the IPA module, or false otherwise > + * \return True if the IPA module matches, or false otherwise > */ > -bool IPAModule::match(PipelineHandler *pipe, > - uint32_t minVersion, uint32_t maxVersion) const > +bool IPAModule::match(const char *name, uint32_t minVersion, > + uint32_t maxVersion) const > { > return info_.pipelineVersion >= minVersion && > info_.pipelineVersion <= maxVersion && > - !strcmp(info_.pipelineName, pipe->name()); > + !strcmp(info_.name, name); > } > > std::string IPAModule::logPrefix() const >
Quoting Barnabás Pőcze (2025-10-03 10:52:14) > Hi > > > 2025. 10. 03. 11:27 keltezéssel, Jacopo Mondi írta: > > From: Hans de Goede <hdegoede@redhat.com> > > > > Currently createIPA() / IPAManager::module() assume that there is a 1:1 > > relationship between pipeline handlers and IPAs and IPA matching is done > > based on matching the pipe to ipaModuleInfo.pipelineName[]. > > > > One way to allow using a single IPA with multiple pipelines would be to > > allow the IPA to declare itself compatible with more than one pipeline, > > turning ipaModuleInfo.pipelineName[] into e.g. a vector. But the way > > ipaModuleInfo is loaded as an ELF symbol requires it to be a simple flat > > C-struct. > > > > Instead, move the IPA creation procedure to be name-based, introducing > > an overload to IPAManager::createIPA(pipe, name, minVer, maxVer) that > > allows to specify the name of the IPA module to match. Pipeline handlers > > that wants to use their name as matching criteria can continue doing so > > using the already existing createIPA(pipe, minVer, maxVer) overload. > > When this was first proposed, I looked at the possibility of adding the name to the > generated proxy type as a static constexpr member to avoid any potential type confusion. > But sadly the way the raspberry pi modules are handled makes that more complicated. > > > > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > --- > > include/libcamera/internal/ipa_manager.h | 14 +++++++++++-- > > include/libcamera/internal/ipa_module.h | 4 ++-- > > src/libcamera/ipa_manager.cpp | 34 ++++++++++++++++++++++++++------ > > src/libcamera/ipa_module.cpp | 12 +++++------ > > 4 files changed, 48 insertions(+), 16 deletions(-) > > > > diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h > > index f8ce780169e617088557888d7c8dae2d3f10ec08..8bc8680be0a4a6db0cdb57c1a9400c23efecda0c 100644 > > --- a/include/libcamera/internal/ipa_manager.h > > +++ b/include/libcamera/internal/ipa_manager.h > > @@ -34,12 +34,14 @@ public: > > > > template<typename T> > > static std::unique_ptr<T> createIPA(PipelineHandler *pipe, > > + const char *name, > > uint32_t minVersion, > > uint32_t maxVersion) > > + I don't think we want this extraneous line here. > > { > > CameraManager *cm = pipe->cameraManager(); > > Since `pipe` is only used to get the camera manager, could we take that as argument instead? > > > Regards, > Barnabás Pőcze > > > IPAManager *self = cm->_d()->ipaManager(); > > - IPAModule *m = self->module(pipe, minVersion, maxVersion); > > + IPAModule *m = self->module(name, minVersion, maxVersion); > > if (!m) > > return nullptr; > > > > @@ -60,6 +62,14 @@ public: > > return proxy; > > } > > > > + template<typename T> > > + static std::unique_ptr<T> createIPA(PipelineHandler *pipe, > > + uint32_t minVersion, > > + uint32_t maxVersion) > > + { > > + return createIPA<T>(pipe, pipe->name(), minVersion, maxVersion); > > + } > > + > > #if HAVE_IPA_PUBKEY > > static const PubKey &pubKey() > > { > > @@ -72,7 +82,7 @@ private: > > std::vector<std::string> &files); > > unsigned int addDir(const char *libDir, unsigned int maxDepth = 0); > > > > - IPAModule *module(PipelineHandler *pipe, uint32_t minVersion, > > + IPAModule *module(const char *name, uint32_t minVersion, > > uint32_t maxVersion); > > > > bool isSignatureValid(IPAModule *ipa) const; > > diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h > > index 15f19492c3a027a0bc4f9572188d13af41fcd450..a0a53764e1394abed3bab92cdde9f33a86441c5f 100644 > > --- a/include/libcamera/internal/ipa_module.h > > +++ b/include/libcamera/internal/ipa_module.h > > @@ -36,8 +36,8 @@ public: > > > > IPAInterface *createInterface(); > > > > - bool match(PipelineHandler *pipe, > > - uint32_t minVersion, uint32_t maxVersion) const; > > + bool match(const char *name, uint32_t minVersion, > > + uint32_t maxVersion) const; > > > > protected: > > std::string logPrefix() const override; > > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp > > index 35171d097136a6d85b8f518c099f8228f9eacd6f..f62a4ee5fd012d23ce178d59f84c6ab49513376b 100644 > > --- a/src/libcamera/ipa_manager.cpp > > +++ b/src/libcamera/ipa_manager.cpp > > @@ -247,15 +247,15 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth) > > > > /** > > * \brief Retrieve an IPA module that matches a given pipeline handler > > - * \param[in] pipe The pipeline handler > > + * \param[in] name The IPA module string identifier > > * \param[in] minVersion Minimum acceptable version of IPA module > > * \param[in] maxVersion Maximum acceptable version of IPA module > > */ > > -IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, > > +IPAModule *IPAManager::module(const char *name, uint32_t minVersion, > > uint32_t maxVersion) > > { > > for (const auto &module : modules_) { > > - if (module->match(pipe, minVersion, maxVersion)) > > + if (module->match(name, minVersion, maxVersion)) > > return module.get(); > > } > > > > @@ -263,12 +263,34 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, > > } > > > > /** > > - * \fn IPAManager::createIPA() > > - * \brief Create an IPA proxy that matches a given pipeline handler > > - * \param[in] pipe The pipeline handler that wants a matching IPA proxy > > + * \fn IPAManager::createIPA(PipelineHandler *pipe, const char *ipaName, uint32_t minVersion, uint32_t maxVersion) > > + * \brief Create an IPA proxy that matches the requested name and version > > + * \param[in] pipe The pipeline handler that wants to create the IPA module > > + * \param[in] ipaName The IPA module name > > * \param[in] minVersion Minimum acceptable version of IPA module > > * \param[in] maxVersion Maximum acceptable version of IPA module > > * > > + * Create an IPA module using \a name as the matching identifier. This overload > > + * allows pipeline handlers to create an IPA module by specifying its name > > + * instead of relying on the fact that the IPA module matches the pipeline > > + * handler's one. > > + * > > + * \return A newly created IPA proxy, or nullptr if no matching IPA module is > > + * found or if the IPA proxy fails to initialize > > + */ > > + > > +/** > > + * \fn IPAManager::createIPA(PipelineHandler *pipe, uint32_t minVersion, uint32_t maxVersion) > > + * \brief Create an IPA proxy that matches the pipeline handler name and the > > + * requested version > > + * \param[in] pipe The pipeline handler that wants to create the IPA module > > + * \param[in] minVersion Minimum acceptable version of IPA module > > + * \param[in] maxVersion Maximum acceptable version of IPA module > > + * > > + * Create an IPA module using the pipeline handler name as the matching > > + * identifier. This overload allows pipeline handler to create an IPA module > > + * whose name matches the pipeline handler one. > > + * > > * \return A newly created IPA proxy, or nullptr if no matching IPA module is > > * found or if the IPA proxy fails to initialize > > */ > > diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp > > index e6ea61e4482983a83b4185c4309f2c514ed24fc2..0bd6f14626fe2038072f48b70ca4341b0eb8cef5 100644 > > --- a/src/libcamera/ipa_module.cpp > > +++ b/src/libcamera/ipa_module.cpp > > @@ -463,21 +463,21 @@ IPAInterface *IPAModule::createInterface() > > > > /** > > * \brief Verify if the IPA module matches a given pipeline handler > > - * \param[in] pipe Pipeline handler to match with > > + * \param[in] name The IPA module name > > * \param[in] minVersion Minimum acceptable version of IPA module > > * \param[in] maxVersion Maximum acceptable version of IPA module > > * > > - * This function checks if this IPA module matches the \a pipe pipeline handler, > > + * This function checks if this IPA module matches the requested \a name > > * and the input version range. > > * > > - * \return True if the pipeline handler matches the IPA module, or false otherwise > > + * \return True if the IPA module matches, or false otherwise > > */ > > -bool IPAModule::match(PipelineHandler *pipe, > > - uint32_t minVersion, uint32_t maxVersion) const > > +bool IPAModule::match(const char *name, uint32_t minVersion, > > + uint32_t maxVersion) const > > { > > return info_.pipelineVersion >= minVersion && > > info_.pipelineVersion <= maxVersion && > > - !strcmp(info_.pipelineName, pipe->name()); > > + !strcmp(info_.name, name); Otherwise nothing stands out to me, and with Barnabás comment handled if needed or as a separate patch? Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > } > > > > std::string IPAModule::logPrefix() const > > >
Hi Jacopo, On 3-Oct-25 11:27 AM, Jacopo Mondi wrote: > From: Hans de Goede <hdegoede@redhat.com> > > Currently createIPA() / IPAManager::module() assume that there is a 1:1 > relationship between pipeline handlers and IPAs and IPA matching is done > based on matching the pipe to ipaModuleInfo.pipelineName[]. > > One way to allow using a single IPA with multiple pipelines would be to > allow the IPA to declare itself compatible with more than one pipeline, > turning ipaModuleInfo.pipelineName[] into e.g. a vector. But the way > ipaModuleInfo is loaded as an ELF symbol requires it to be a simple flat > C-struct. > > Instead, move the IPA creation procedure to be name-based, introducing > an overload to IPAManager::createIPA(pipe, name, minVer, maxVer) that > allows to specify the name of the IPA module to match. Pipeline handlers > that wants to use their name as matching criteria can continue doing so > using the already existing createIPA(pipe, minVer, maxVer) overload. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thank you for dusting off this patch. Note that hdegoede@redhat.com will stop working after October 31st. For v2 can you please update my S-o-b to: Reviewed-by: Hans de Goede <hansg@kernel.org> And also do a git commit --amend --author="Hans de Goede <hansg@kernel.org>" to update the author / From: field to match ? Regards, Hans > --- > include/libcamera/internal/ipa_manager.h | 14 +++++++++++-- > include/libcamera/internal/ipa_module.h | 4 ++-- > src/libcamera/ipa_manager.cpp | 34 ++++++++++++++++++++++++++------ > src/libcamera/ipa_module.cpp | 12 +++++------ > 4 files changed, 48 insertions(+), 16 deletions(-) > > diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h > index f8ce780169e617088557888d7c8dae2d3f10ec08..8bc8680be0a4a6db0cdb57c1a9400c23efecda0c 100644 > --- a/include/libcamera/internal/ipa_manager.h > +++ b/include/libcamera/internal/ipa_manager.h > @@ -34,12 +34,14 @@ public: > > template<typename T> > static std::unique_ptr<T> createIPA(PipelineHandler *pipe, > + const char *name, > uint32_t minVersion, > uint32_t maxVersion) > + > { > CameraManager *cm = pipe->cameraManager(); > IPAManager *self = cm->_d()->ipaManager(); > - IPAModule *m = self->module(pipe, minVersion, maxVersion); > + IPAModule *m = self->module(name, minVersion, maxVersion); > if (!m) > return nullptr; > > @@ -60,6 +62,14 @@ public: > return proxy; > } > > + template<typename T> > + static std::unique_ptr<T> createIPA(PipelineHandler *pipe, > + uint32_t minVersion, > + uint32_t maxVersion) > + { > + return createIPA<T>(pipe, pipe->name(), minVersion, maxVersion); > + } > + > #if HAVE_IPA_PUBKEY > static const PubKey &pubKey() > { > @@ -72,7 +82,7 @@ private: > std::vector<std::string> &files); > unsigned int addDir(const char *libDir, unsigned int maxDepth = 0); > > - IPAModule *module(PipelineHandler *pipe, uint32_t minVersion, > + IPAModule *module(const char *name, uint32_t minVersion, > uint32_t maxVersion); > > bool isSignatureValid(IPAModule *ipa) const; > diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h > index 15f19492c3a027a0bc4f9572188d13af41fcd450..a0a53764e1394abed3bab92cdde9f33a86441c5f 100644 > --- a/include/libcamera/internal/ipa_module.h > +++ b/include/libcamera/internal/ipa_module.h > @@ -36,8 +36,8 @@ public: > > IPAInterface *createInterface(); > > - bool match(PipelineHandler *pipe, > - uint32_t minVersion, uint32_t maxVersion) const; > + bool match(const char *name, uint32_t minVersion, > + uint32_t maxVersion) const; > > protected: > std::string logPrefix() const override; > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp > index 35171d097136a6d85b8f518c099f8228f9eacd6f..f62a4ee5fd012d23ce178d59f84c6ab49513376b 100644 > --- a/src/libcamera/ipa_manager.cpp > +++ b/src/libcamera/ipa_manager.cpp > @@ -247,15 +247,15 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth) > > /** > * \brief Retrieve an IPA module that matches a given pipeline handler > - * \param[in] pipe The pipeline handler > + * \param[in] name The IPA module string identifier > * \param[in] minVersion Minimum acceptable version of IPA module > * \param[in] maxVersion Maximum acceptable version of IPA module > */ > -IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, > +IPAModule *IPAManager::module(const char *name, uint32_t minVersion, > uint32_t maxVersion) > { > for (const auto &module : modules_) { > - if (module->match(pipe, minVersion, maxVersion)) > + if (module->match(name, minVersion, maxVersion)) > return module.get(); > } > > @@ -263,12 +263,34 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, > } > > /** > - * \fn IPAManager::createIPA() > - * \brief Create an IPA proxy that matches a given pipeline handler > - * \param[in] pipe The pipeline handler that wants a matching IPA proxy > + * \fn IPAManager::createIPA(PipelineHandler *pipe, const char *ipaName, uint32_t minVersion, uint32_t maxVersion) > + * \brief Create an IPA proxy that matches the requested name and version > + * \param[in] pipe The pipeline handler that wants to create the IPA module > + * \param[in] ipaName The IPA module name > * \param[in] minVersion Minimum acceptable version of IPA module > * \param[in] maxVersion Maximum acceptable version of IPA module > * > + * Create an IPA module using \a name as the matching identifier. This overload > + * allows pipeline handlers to create an IPA module by specifying its name > + * instead of relying on the fact that the IPA module matches the pipeline > + * handler's one. > + * > + * \return A newly created IPA proxy, or nullptr if no matching IPA module is > + * found or if the IPA proxy fails to initialize > + */ > + > +/** > + * \fn IPAManager::createIPA(PipelineHandler *pipe, uint32_t minVersion, uint32_t maxVersion) > + * \brief Create an IPA proxy that matches the pipeline handler name and the > + * requested version > + * \param[in] pipe The pipeline handler that wants to create the IPA module > + * \param[in] minVersion Minimum acceptable version of IPA module > + * \param[in] maxVersion Maximum acceptable version of IPA module > + * > + * Create an IPA module using the pipeline handler name as the matching > + * identifier. This overload allows pipeline handler to create an IPA module > + * whose name matches the pipeline handler one. > + * > * \return A newly created IPA proxy, or nullptr if no matching IPA module is > * found or if the IPA proxy fails to initialize > */ > diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp > index e6ea61e4482983a83b4185c4309f2c514ed24fc2..0bd6f14626fe2038072f48b70ca4341b0eb8cef5 100644 > --- a/src/libcamera/ipa_module.cpp > +++ b/src/libcamera/ipa_module.cpp > @@ -463,21 +463,21 @@ IPAInterface *IPAModule::createInterface() > > /** > * \brief Verify if the IPA module matches a given pipeline handler > - * \param[in] pipe Pipeline handler to match with > + * \param[in] name The IPA module name > * \param[in] minVersion Minimum acceptable version of IPA module > * \param[in] maxVersion Maximum acceptable version of IPA module > * > - * This function checks if this IPA module matches the \a pipe pipeline handler, > + * This function checks if this IPA module matches the requested \a name > * and the input version range. > * > - * \return True if the pipeline handler matches the IPA module, or false otherwise > + * \return True if the IPA module matches, or false otherwise > */ > -bool IPAModule::match(PipelineHandler *pipe, > - uint32_t minVersion, uint32_t maxVersion) const > +bool IPAModule::match(const char *name, uint32_t minVersion, > + uint32_t maxVersion) const > { > return info_.pipelineVersion >= minVersion && > info_.pipelineVersion <= maxVersion && > - !strcmp(info_.pipelineName, pipe->name()); > + !strcmp(info_.name, name); > } > > std::string IPAModule::logPrefix() const >
Hi Barnabás On Fri, Oct 03, 2025 at 11:52:14AM +0200, Barnabás Pőcze wrote: > Hi > > > 2025. 10. 03. 11:27 keltezéssel, Jacopo Mondi írta: > > From: Hans de Goede <hdegoede@redhat.com> > > > > Currently createIPA() / IPAManager::module() assume that there is a 1:1 > > relationship between pipeline handlers and IPAs and IPA matching is done > > based on matching the pipe to ipaModuleInfo.pipelineName[]. > > > > One way to allow using a single IPA with multiple pipelines would be to > > allow the IPA to declare itself compatible with more than one pipeline, > > turning ipaModuleInfo.pipelineName[] into e.g. a vector. But the way > > ipaModuleInfo is loaded as an ELF symbol requires it to be a simple flat > > C-struct. > > > > Instead, move the IPA creation procedure to be name-based, introducing > > an overload to IPAManager::createIPA(pipe, name, minVer, maxVer) that > > allows to specify the name of the IPA module to match. Pipeline handlers > > that wants to use their name as matching criteria can continue doing so > > using the already existing createIPA(pipe, minVer, maxVer) overload. > > When this was first proposed, I looked at the possibility of adding the name to the > generated proxy type as a static constexpr member to avoid any potential type confusion. > But sadly the way the raspberry pi modules are handled makes that more complicated. > > > > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > --- > > include/libcamera/internal/ipa_manager.h | 14 +++++++++++-- > > include/libcamera/internal/ipa_module.h | 4 ++-- > > src/libcamera/ipa_manager.cpp | 34 ++++++++++++++++++++++++++------ > > src/libcamera/ipa_module.cpp | 12 +++++------ > > 4 files changed, 48 insertions(+), 16 deletions(-) > > > > diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h > > index f8ce780169e617088557888d7c8dae2d3f10ec08..8bc8680be0a4a6db0cdb57c1a9400c23efecda0c 100644 > > --- a/include/libcamera/internal/ipa_manager.h > > +++ b/include/libcamera/internal/ipa_manager.h > > @@ -34,12 +34,14 @@ public: > > template<typename T> > > static std::unique_ptr<T> createIPA(PipelineHandler *pipe, > > + const char *name, > > uint32_t minVersion, > > uint32_t maxVersion) > > + > > { > > CameraManager *cm = pipe->cameraManager(); > > Since `pipe` is only used to get the camera manager, could we take that as argument instead? Isn't it more natural for callers to pass "this" instead of accessing the camera manager themselves ? DO you feel strongly about this or can I leave it as it is ? > > > Regards, > Barnabás Pőcze > > > IPAManager *self = cm->_d()->ipaManager(); > > - IPAModule *m = self->module(pipe, minVersion, maxVersion); > > + IPAModule *m = self->module(name, minVersion, maxVersion); > > if (!m) > > return nullptr; > > @@ -60,6 +62,14 @@ public: > > return proxy; > > } > > + template<typename T> > > + static std::unique_ptr<T> createIPA(PipelineHandler *pipe, > > + uint32_t minVersion, > > + uint32_t maxVersion) > > + { > > + return createIPA<T>(pipe, pipe->name(), minVersion, maxVersion); > > + } > > + > > #if HAVE_IPA_PUBKEY > > static const PubKey &pubKey() > > { > > @@ -72,7 +82,7 @@ private: > > std::vector<std::string> &files); > > unsigned int addDir(const char *libDir, unsigned int maxDepth = 0); > > - IPAModule *module(PipelineHandler *pipe, uint32_t minVersion, > > + IPAModule *module(const char *name, uint32_t minVersion, > > uint32_t maxVersion); > > bool isSignatureValid(IPAModule *ipa) const; > > diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h > > index 15f19492c3a027a0bc4f9572188d13af41fcd450..a0a53764e1394abed3bab92cdde9f33a86441c5f 100644 > > --- a/include/libcamera/internal/ipa_module.h > > +++ b/include/libcamera/internal/ipa_module.h > > @@ -36,8 +36,8 @@ public: > > IPAInterface *createInterface(); > > - bool match(PipelineHandler *pipe, > > - uint32_t minVersion, uint32_t maxVersion) const; > > + bool match(const char *name, uint32_t minVersion, > > + uint32_t maxVersion) const; > > protected: > > std::string logPrefix() const override; > > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp > > index 35171d097136a6d85b8f518c099f8228f9eacd6f..f62a4ee5fd012d23ce178d59f84c6ab49513376b 100644 > > --- a/src/libcamera/ipa_manager.cpp > > +++ b/src/libcamera/ipa_manager.cpp > > @@ -247,15 +247,15 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth) > > /** > > * \brief Retrieve an IPA module that matches a given pipeline handler > > - * \param[in] pipe The pipeline handler > > + * \param[in] name The IPA module string identifier > > * \param[in] minVersion Minimum acceptable version of IPA module > > * \param[in] maxVersion Maximum acceptable version of IPA module > > */ > > -IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, > > +IPAModule *IPAManager::module(const char *name, uint32_t minVersion, > > uint32_t maxVersion) > > { > > for (const auto &module : modules_) { > > - if (module->match(pipe, minVersion, maxVersion)) > > + if (module->match(name, minVersion, maxVersion)) > > return module.get(); > > } > > @@ -263,12 +263,34 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, > > } > > /** > > - * \fn IPAManager::createIPA() > > - * \brief Create an IPA proxy that matches a given pipeline handler > > - * \param[in] pipe The pipeline handler that wants a matching IPA proxy > > + * \fn IPAManager::createIPA(PipelineHandler *pipe, const char *ipaName, uint32_t minVersion, uint32_t maxVersion) > > + * \brief Create an IPA proxy that matches the requested name and version > > + * \param[in] pipe The pipeline handler that wants to create the IPA module > > + * \param[in] ipaName The IPA module name > > * \param[in] minVersion Minimum acceptable version of IPA module > > * \param[in] maxVersion Maximum acceptable version of IPA module > > * > > + * Create an IPA module using \a name as the matching identifier. This overload > > + * allows pipeline handlers to create an IPA module by specifying its name > > + * instead of relying on the fact that the IPA module matches the pipeline > > + * handler's one. > > + * > > + * \return A newly created IPA proxy, or nullptr if no matching IPA module is > > + * found or if the IPA proxy fails to initialize > > + */ > > + > > +/** > > + * \fn IPAManager::createIPA(PipelineHandler *pipe, uint32_t minVersion, uint32_t maxVersion) > > + * \brief Create an IPA proxy that matches the pipeline handler name and the > > + * requested version > > + * \param[in] pipe The pipeline handler that wants to create the IPA module > > + * \param[in] minVersion Minimum acceptable version of IPA module > > + * \param[in] maxVersion Maximum acceptable version of IPA module > > + * > > + * Create an IPA module using the pipeline handler name as the matching > > + * identifier. This overload allows pipeline handler to create an IPA module > > + * whose name matches the pipeline handler one. > > + * > > * \return A newly created IPA proxy, or nullptr if no matching IPA module is > > * found or if the IPA proxy fails to initialize > > */ > > diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp > > index e6ea61e4482983a83b4185c4309f2c514ed24fc2..0bd6f14626fe2038072f48b70ca4341b0eb8cef5 100644 > > --- a/src/libcamera/ipa_module.cpp > > +++ b/src/libcamera/ipa_module.cpp > > @@ -463,21 +463,21 @@ IPAInterface *IPAModule::createInterface() > > /** > > * \brief Verify if the IPA module matches a given pipeline handler > > - * \param[in] pipe Pipeline handler to match with > > + * \param[in] name The IPA module name > > * \param[in] minVersion Minimum acceptable version of IPA module > > * \param[in] maxVersion Maximum acceptable version of IPA module > > * > > - * This function checks if this IPA module matches the \a pipe pipeline handler, > > + * This function checks if this IPA module matches the requested \a name > > * and the input version range. > > * > > - * \return True if the pipeline handler matches the IPA module, or false otherwise > > + * \return True if the IPA module matches, or false otherwise > > */ > > -bool IPAModule::match(PipelineHandler *pipe, > > - uint32_t minVersion, uint32_t maxVersion) const > > +bool IPAModule::match(const char *name, uint32_t minVersion, > > + uint32_t maxVersion) const > > { > > return info_.pipelineVersion >= minVersion && > > info_.pipelineVersion <= maxVersion && > > - !strcmp(info_.pipelineName, pipe->name()); > > + !strcmp(info_.name, name); > > } > > std::string IPAModule::logPrefix() const > > >
2025. 10. 03. 18:12 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Fri, Oct 03, 2025 at 11:52:14AM +0200, Barnabás Pőcze wrote: >> Hi >> >> >> 2025. 10. 03. 11:27 keltezéssel, Jacopo Mondi írta: >>> From: Hans de Goede <hdegoede@redhat.com> >>> >>> Currently createIPA() / IPAManager::module() assume that there is a 1:1 >>> relationship between pipeline handlers and IPAs and IPA matching is done >>> based on matching the pipe to ipaModuleInfo.pipelineName[]. >>> >>> One way to allow using a single IPA with multiple pipelines would be to >>> allow the IPA to declare itself compatible with more than one pipeline, >>> turning ipaModuleInfo.pipelineName[] into e.g. a vector. But the way >>> ipaModuleInfo is loaded as an ELF symbol requires it to be a simple flat >>> C-struct. >>> >>> Instead, move the IPA creation procedure to be name-based, introducing >>> an overload to IPAManager::createIPA(pipe, name, minVer, maxVer) that >>> allows to specify the name of the IPA module to match. Pipeline handlers >>> that wants to use their name as matching criteria can continue doing so >>> using the already existing createIPA(pipe, minVer, maxVer) overload. >> >> When this was first proposed, I looked at the possibility of adding the name to the >> generated proxy type as a static constexpr member to avoid any potential type confusion. >> But sadly the way the raspberry pi modules are handled makes that more complicated. >> >> >>> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>> --- >>> include/libcamera/internal/ipa_manager.h | 14 +++++++++++-- >>> include/libcamera/internal/ipa_module.h | 4 ++-- >>> src/libcamera/ipa_manager.cpp | 34 ++++++++++++++++++++++++++------ >>> src/libcamera/ipa_module.cpp | 12 +++++------ >>> 4 files changed, 48 insertions(+), 16 deletions(-) >>> >>> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h >>> index f8ce780169e617088557888d7c8dae2d3f10ec08..8bc8680be0a4a6db0cdb57c1a9400c23efecda0c 100644 >>> --- a/include/libcamera/internal/ipa_manager.h >>> +++ b/include/libcamera/internal/ipa_manager.h >>> @@ -34,12 +34,14 @@ public: >>> template<typename T> >>> static std::unique_ptr<T> createIPA(PipelineHandler *pipe, >>> + const char *name, >>> uint32_t minVersion, >>> uint32_t maxVersion) >>> + >>> { >>> CameraManager *cm = pipe->cameraManager(); >> >> Since `pipe` is only used to get the camera manager, could we take that as argument instead? > > Isn't it more natural for callers to pass "this" instead of accessing > the camera manager themselves ? I suppose it is. > > DO you feel strongly about this or can I leave it as it is ? Let us leave it as is, if a use case appears, it can always be modified. > >> >> >> Regards, >> Barnabás Pőcze >> >>> IPAManager *self = cm->_d()->ipaManager(); >>> - IPAModule *m = self->module(pipe, minVersion, maxVersion); >>> + IPAModule *m = self->module(name, minVersion, maxVersion); >>> if (!m) >>> return nullptr; >>> @@ -60,6 +62,14 @@ public: >>> return proxy; >>> } >>> + template<typename T> >>> + static std::unique_ptr<T> createIPA(PipelineHandler *pipe, >>> + uint32_t minVersion, >>> + uint32_t maxVersion) >>> + { >>> + return createIPA<T>(pipe, pipe->name(), minVersion, maxVersion); >>> + } >>> + >>> #if HAVE_IPA_PUBKEY >>> static const PubKey &pubKey() >>> { >>> @@ -72,7 +82,7 @@ private: >>> std::vector<std::string> &files); >>> unsigned int addDir(const char *libDir, unsigned int maxDepth = 0); >>> - IPAModule *module(PipelineHandler *pipe, uint32_t minVersion, >>> + IPAModule *module(const char *name, uint32_t minVersion, >>> uint32_t maxVersion); >>> bool isSignatureValid(IPAModule *ipa) const; >>> diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h >>> index 15f19492c3a027a0bc4f9572188d13af41fcd450..a0a53764e1394abed3bab92cdde9f33a86441c5f 100644 >>> --- a/include/libcamera/internal/ipa_module.h >>> +++ b/include/libcamera/internal/ipa_module.h >>> @@ -36,8 +36,8 @@ public: >>> IPAInterface *createInterface(); >>> - bool match(PipelineHandler *pipe, >>> - uint32_t minVersion, uint32_t maxVersion) const; >>> + bool match(const char *name, uint32_t minVersion, >>> + uint32_t maxVersion) const; >>> protected: >>> std::string logPrefix() const override; >>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp >>> index 35171d097136a6d85b8f518c099f8228f9eacd6f..f62a4ee5fd012d23ce178d59f84c6ab49513376b 100644 >>> --- a/src/libcamera/ipa_manager.cpp >>> +++ b/src/libcamera/ipa_manager.cpp >>> @@ -247,15 +247,15 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth) >>> /** >>> * \brief Retrieve an IPA module that matches a given pipeline handler >>> - * \param[in] pipe The pipeline handler >>> + * \param[in] name The IPA module string identifier >>> * \param[in] minVersion Minimum acceptable version of IPA module >>> * \param[in] maxVersion Maximum acceptable version of IPA module >>> */ >>> -IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, >>> +IPAModule *IPAManager::module(const char *name, uint32_t minVersion, >>> uint32_t maxVersion) >>> { >>> for (const auto &module : modules_) { >>> - if (module->match(pipe, minVersion, maxVersion)) >>> + if (module->match(name, minVersion, maxVersion)) >>> return module.get(); >>> } >>> @@ -263,12 +263,34 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, >>> } >>> /** >>> - * \fn IPAManager::createIPA() >>> - * \brief Create an IPA proxy that matches a given pipeline handler >>> - * \param[in] pipe The pipeline handler that wants a matching IPA proxy >>> + * \fn IPAManager::createIPA(PipelineHandler *pipe, const char *ipaName, uint32_t minVersion, uint32_t maxVersion) >>> + * \brief Create an IPA proxy that matches the requested name and version >>> + * \param[in] pipe The pipeline handler that wants to create the IPA module >>> + * \param[in] ipaName The IPA module name >>> * \param[in] minVersion Minimum acceptable version of IPA module >>> * \param[in] maxVersion Maximum acceptable version of IPA module >>> * >>> + * Create an IPA module using \a name as the matching identifier. This overload >>> + * allows pipeline handlers to create an IPA module by specifying its name >>> + * instead of relying on the fact that the IPA module matches the pipeline >>> + * handler's one. >>> + * >>> + * \return A newly created IPA proxy, or nullptr if no matching IPA module is >>> + * found or if the IPA proxy fails to initialize >>> + */ >>> + >>> +/** >>> + * \fn IPAManager::createIPA(PipelineHandler *pipe, uint32_t minVersion, uint32_t maxVersion) >>> + * \brief Create an IPA proxy that matches the pipeline handler name and the >>> + * requested version >>> + * \param[in] pipe The pipeline handler that wants to create the IPA module >>> + * \param[in] minVersion Minimum acceptable version of IPA module >>> + * \param[in] maxVersion Maximum acceptable version of IPA module >>> + * >>> + * Create an IPA module using the pipeline handler name as the matching >>> + * identifier. This overload allows pipeline handler to create an IPA module >>> + * whose name matches the pipeline handler one. >>> + * >>> * \return A newly created IPA proxy, or nullptr if no matching IPA module is >>> * found or if the IPA proxy fails to initialize >>> */ >>> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp >>> index e6ea61e4482983a83b4185c4309f2c514ed24fc2..0bd6f14626fe2038072f48b70ca4341b0eb8cef5 100644 >>> --- a/src/libcamera/ipa_module.cpp >>> +++ b/src/libcamera/ipa_module.cpp >>> @@ -463,21 +463,21 @@ IPAInterface *IPAModule::createInterface() >>> /** >>> * \brief Verify if the IPA module matches a given pipeline handler >>> - * \param[in] pipe Pipeline handler to match with >>> + * \param[in] name The IPA module name >>> * \param[in] minVersion Minimum acceptable version of IPA module >>> * \param[in] maxVersion Maximum acceptable version of IPA module >>> * >>> - * This function checks if this IPA module matches the \a pipe pipeline handler, >>> + * This function checks if this IPA module matches the requested \a name >>> * and the input version range. >>> * >>> - * \return True if the pipeline handler matches the IPA module, or false otherwise >>> + * \return True if the IPA module matches, or false otherwise >>> */ >>> -bool IPAModule::match(PipelineHandler *pipe, >>> - uint32_t minVersion, uint32_t maxVersion) const >>> +bool IPAModule::match(const char *name, uint32_t minVersion, >>> + uint32_t maxVersion) const >>> { >>> return info_.pipelineVersion >= minVersion && >>> info_.pipelineVersion <= maxVersion && >>> - !strcmp(info_.pipelineName, pipe->name()); >>> + !strcmp(info_.name, name); >>> } >>> std::string IPAModule::logPrefix() const >>> >>
diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h index f8ce780169e617088557888d7c8dae2d3f10ec08..8bc8680be0a4a6db0cdb57c1a9400c23efecda0c 100644 --- a/include/libcamera/internal/ipa_manager.h +++ b/include/libcamera/internal/ipa_manager.h @@ -34,12 +34,14 @@ public: template<typename T> static std::unique_ptr<T> createIPA(PipelineHandler *pipe, + const char *name, uint32_t minVersion, uint32_t maxVersion) + { CameraManager *cm = pipe->cameraManager(); IPAManager *self = cm->_d()->ipaManager(); - IPAModule *m = self->module(pipe, minVersion, maxVersion); + IPAModule *m = self->module(name, minVersion, maxVersion); if (!m) return nullptr; @@ -60,6 +62,14 @@ public: return proxy; } + template<typename T> + static std::unique_ptr<T> createIPA(PipelineHandler *pipe, + uint32_t minVersion, + uint32_t maxVersion) + { + return createIPA<T>(pipe, pipe->name(), minVersion, maxVersion); + } + #if HAVE_IPA_PUBKEY static const PubKey &pubKey() { @@ -72,7 +82,7 @@ private: std::vector<std::string> &files); unsigned int addDir(const char *libDir, unsigned int maxDepth = 0); - IPAModule *module(PipelineHandler *pipe, uint32_t minVersion, + IPAModule *module(const char *name, uint32_t minVersion, uint32_t maxVersion); bool isSignatureValid(IPAModule *ipa) const; diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h index 15f19492c3a027a0bc4f9572188d13af41fcd450..a0a53764e1394abed3bab92cdde9f33a86441c5f 100644 --- a/include/libcamera/internal/ipa_module.h +++ b/include/libcamera/internal/ipa_module.h @@ -36,8 +36,8 @@ public: IPAInterface *createInterface(); - bool match(PipelineHandler *pipe, - uint32_t minVersion, uint32_t maxVersion) const; + bool match(const char *name, uint32_t minVersion, + uint32_t maxVersion) const; protected: std::string logPrefix() const override; diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index 35171d097136a6d85b8f518c099f8228f9eacd6f..f62a4ee5fd012d23ce178d59f84c6ab49513376b 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -247,15 +247,15 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth) /** * \brief Retrieve an IPA module that matches a given pipeline handler - * \param[in] pipe The pipeline handler + * \param[in] name The IPA module string identifier * \param[in] minVersion Minimum acceptable version of IPA module * \param[in] maxVersion Maximum acceptable version of IPA module */ -IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, +IPAModule *IPAManager::module(const char *name, uint32_t minVersion, uint32_t maxVersion) { for (const auto &module : modules_) { - if (module->match(pipe, minVersion, maxVersion)) + if (module->match(name, minVersion, maxVersion)) return module.get(); } @@ -263,12 +263,34 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion, } /** - * \fn IPAManager::createIPA() - * \brief Create an IPA proxy that matches a given pipeline handler - * \param[in] pipe The pipeline handler that wants a matching IPA proxy + * \fn IPAManager::createIPA(PipelineHandler *pipe, const char *ipaName, uint32_t minVersion, uint32_t maxVersion) + * \brief Create an IPA proxy that matches the requested name and version + * \param[in] pipe The pipeline handler that wants to create the IPA module + * \param[in] ipaName The IPA module name * \param[in] minVersion Minimum acceptable version of IPA module * \param[in] maxVersion Maximum acceptable version of IPA module * + * Create an IPA module using \a name as the matching identifier. This overload + * allows pipeline handlers to create an IPA module by specifying its name + * instead of relying on the fact that the IPA module matches the pipeline + * handler's one. + * + * \return A newly created IPA proxy, or nullptr if no matching IPA module is + * found or if the IPA proxy fails to initialize + */ + +/** + * \fn IPAManager::createIPA(PipelineHandler *pipe, uint32_t minVersion, uint32_t maxVersion) + * \brief Create an IPA proxy that matches the pipeline handler name and the + * requested version + * \param[in] pipe The pipeline handler that wants to create the IPA module + * \param[in] minVersion Minimum acceptable version of IPA module + * \param[in] maxVersion Maximum acceptable version of IPA module + * + * Create an IPA module using the pipeline handler name as the matching + * identifier. This overload allows pipeline handler to create an IPA module + * whose name matches the pipeline handler one. + * * \return A newly created IPA proxy, or nullptr if no matching IPA module is * found or if the IPA proxy fails to initialize */ diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp index e6ea61e4482983a83b4185c4309f2c514ed24fc2..0bd6f14626fe2038072f48b70ca4341b0eb8cef5 100644 --- a/src/libcamera/ipa_module.cpp +++ b/src/libcamera/ipa_module.cpp @@ -463,21 +463,21 @@ IPAInterface *IPAModule::createInterface() /** * \brief Verify if the IPA module matches a given pipeline handler - * \param[in] pipe Pipeline handler to match with + * \param[in] name The IPA module name * \param[in] minVersion Minimum acceptable version of IPA module * \param[in] maxVersion Maximum acceptable version of IPA module * - * This function checks if this IPA module matches the \a pipe pipeline handler, + * This function checks if this IPA module matches the requested \a name * and the input version range. * - * \return True if the pipeline handler matches the IPA module, or false otherwise + * \return True if the IPA module matches, or false otherwise */ -bool IPAModule::match(PipelineHandler *pipe, - uint32_t minVersion, uint32_t maxVersion) const +bool IPAModule::match(const char *name, uint32_t minVersion, + uint32_t maxVersion) const { return info_.pipelineVersion >= minVersion && info_.pipelineVersion <= maxVersion && - !strcmp(info_.pipelineName, pipe->name()); + !strcmp(info_.name, name); } std::string IPAModule::logPrefix() const