| Message ID | 9c7e79d2-1b12-4a30-9126-019607673bb7@oss.qualcomm.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Hans, On Mon, Jul 27, 2026 at 01:58:23PM +0200, Hans de Goede wrote: > Hi All, > > As discussed I've been looking into loading / creating IPAs by (proxy) type name. > > A way to do this was proposed by Barnabás here: > > https://patchwork.libcamera.org/patch/26503/ > > Barnabás Pőcze wrote: > > """ > > --- a/include/libcamera/internal/ipa_manager.h > > +++ b/include/libcamera/internal/ipa_manager.h > > @@ -34,12 +34,13 @@ public: > > > > template<typename T> > > static std::unique_ptr<T> createIPA(PipelineHandler *pipe, > > + const char *name, > > uint32_t minVersion, > > uint32_t maxVersion) > > { > > I agree with this change, but I wish that the name could be derived from `T`. > And that is quite easy to do, just adding a > > static constexpr const char *name() { return "..."; } > > and then using `T::name()` here works mostly. > > But unfortunately the raspberry pi ipa modules are not compatible at the moment > with this approach. I have tried to come up with something, but so for has failed. > > Maybe the `name` argument could be kept like this: > > const char *name = T::name() > > and then the rpi pipeline handler could be changed to use the specific names. Not > a fan of this in any case. > """ > > This sounds nice and simple, but the <typename T> being passed to > the template here is an IPA proxy which gets auto-generated by mojo. > > mojo does support const variables so I've tried this: As discussed on IRC, you don't need to modify the .mojom files. The proxy class is generated by a jinja2 template, see utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl. Adding a static name() function there should be enough. > diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom > index d9a50b01d..b430ae5fd 100644 > --- a/include/libcamera/ipa/ipu3.mojom > +++ b/include/libcamera/ipa/ipu3.mojom > @@ -9,6 +9,8 @@ module ipa.ipu3; > import "include/libcamera/ipa/core.mojom"; > > struct IPAConfigInfo { > + const string kInterfaceName = "ipu3"; > + > libcamera.IPACameraSensorInfo sensorInfo; > libcamera.ControlInfoMap sensorControls; > libcamera.ControlInfoMap lensControls; > @@ -17,6 +19,8 @@ struct IPAConfigInfo { > }; > > interface IPAIPU3Interface { > + const string kInterfaceName = "ipu3"; > + > init(libcamera.IPASettings settings, > libcamera.IPACameraSensorInfo sensorInfo, > libcamera.ControlInfoMap sensorControls) > > > Unfortunately this results in the follow autogenerated code: > > struct IPAConfigInfo > { > public: > #ifndef __DOXYGEN__ > IPAConfigInfo() = default; > > template< > typename T1 = IPACameraSensorInfo, > typename T2 = ControlInfoMap, > typename T3 = ControlInfoMap, > typename T4 = Size, > typename T5 = Size, > std::enable_if_t<std::is_convertible_v<T1&&, IPACameraSensorInfo>> * = nullptr, > std::enable_if_t<std::is_convertible_v<T2&&, ControlInfoMap>> * = nullptr, > std::enable_if_t<std::is_convertible_v<T3&&, ControlInfoMap>> * = nullptr, > std::enable_if_t<std::is_convertible_v<T4&&, Size>> * = nullptr, > std::enable_if_t<std::is_convertible_v<T5&&, Size>> * = nullptr > > > IPAConfigInfo(T1 &&_sensorInfo, T2 &&_sensorControls, T3 &&_lensControls, T4 &&_bdsOutputSize, T5 &&_iif) > : sensorInfo(std::forward<T1>(_sensorInfo)) > , sensorControls(std::forward<T2>(_sensorControls)) > , lensControls(std::forward<T3>(_lensControls)) > , bdsOutputSize(std::forward<T4>(_bdsOutputSize)) > , iif(std::forward<T5>(_iif)) > { > } > #endif > > > IPACameraSensorInfo sensorInfo; > ControlInfoMap sensorControls; > ControlInfoMap lensControls; > Size bdsOutputSize; > Size iif; > }; > > class IPAIPU3Interface : public IPAInterface > { > public: > > virtual int32_t init( > const IPASettings &settings, > const IPACameraSensorInfo &sensorInfo, > const ControlInfoMap &sensorControls, > ControlInfoMap *ipaControls) = 0; > > virtual int32_t start() = 0; > > virtual void stop() = 0; > > virtual int32_t configure( > const IPAConfigInfo &configInfo, > ControlInfoMap *ipaControls) = 0; > > virtual void mapBuffers( > const std::vector<libcamera::IPABuffer> &buffers) = 0; > > virtual void unmapBuffers( > const std::vector<uint32_t> &ids) = 0; > > virtual void queueRequest( > const uint32_t frame, > const ControlList &controls) = 0; > > virtual void computeParams( > const uint32_t frame, > const uint32_t bufferId) = 0; > > virtual void processStats( > const uint32_t frame, > const int64_t frameTimestamp, > const uint32_t bufferId, > const ControlList &sensorControls) = 0; > > Signal<uint32_t, const ControlList &, const ControlList &> setSensorControls; > > Signal<uint32_t> paramsComputed; > > Signal<uint32_t, const ControlList &> metadataReady; > }; > > which does not change by adding the const lines to the .mojo file. > > it seems the const declarations inside mojo files are only intended for use > inside the mojo files (for e.g. arg default values) and are nor propagated > to the generic code. > > So AFAICT this means that the whole load/create IPA by IPA-proxy type idea > will not work.
diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom index d9a50b01d..b430ae5fd 100644 --- a/include/libcamera/ipa/ipu3.mojom +++ b/include/libcamera/ipa/ipu3.mojom @@ -9,6 +9,8 @@ module ipa.ipu3; import "include/libcamera/ipa/core.mojom"; struct IPAConfigInfo { + const string kInterfaceName = "ipu3"; + libcamera.IPACameraSensorInfo sensorInfo; libcamera.ControlInfoMap sensorControls; libcamera.ControlInfoMap lensControls; @@ -17,6 +19,8 @@ struct IPAConfigInfo { }; interface IPAIPU3Interface { + const string kInterfaceName = "ipu3"; + init(libcamera.IPASettings settings, libcamera.IPACameraSensorInfo sensorInfo, libcamera.ControlInfoMap sensorControls)