[libcamera-devel] [PATCH v2 1/2] libcamera: ipa_manager: Only parse IPA modules

[Kieran Bingham]

Quoting Kieran Bingham (2023-05-04 20:03:56)
> On 04/05/2023 16:10, Laurent Pinchart wrote:
> > Hi Kieran,
> > 
> > Thank you for the patch.
> > 
> > On Thu, May 04, 2023 at 03:48:00PM +0100, Kieran Bingham via libcamera-devel wrote:
> >> Ensure that when we iterate the libcamera libdir we only select shared
> >> objects which are expected to be IPA modules, with an ipa_ prefix.
> >>
> >> Any shared object not matching this filter is ignored and not processed
> >> by the IPA Manager.
> >>
> >> Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>
> >> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> > I wonder what this protects against, as there shouldn't be other files
> > in this directory, especially if we move IPA modules to an ipa/
> > subdirectory. I don't mind much, as this isn't a hot path, so I have no
> > objection if you want to merge this, but is it useful ?
> 
> Only that it prevents us trying to load any files that might be there 
> otherwise.
> 
> 
> 
> [76:32:31.897778214] [2776180] ERROR IPAModule ipa_module.cpp:286 
> fake.so: IPA module is not an ELF file
> [76:32:31.899112311] [2776180] ERROR IPAModule ipa_module.cpp:172 Symbol 
> ipaModuleInfo not found
> [76:32:31.899119975] [2776180] ERROR IPAModule ipa_module.cpp:292 
> v4l2-compat.so: IPA module has no valid info
> 
> 
> We already filter on .so objects, and we do other checks so it's not 
> crucial, and it's not going to prevent any attempt to get 
> 'non-libcamera' code parsed as anyone dropping a file here could equally 
> call it 'ipa_fake.so' ... so no not really.
> 
> I'll drop this and collect 2/2

I've started seeing various logs that report similar to:
```
$ wireplumber 
[12:34:30.015061894] [206341] ERROR IPAModule ipa_module.cpp:172 Symbol ipaModuleInfo not found
[12:34:30.015085339] [206341] ERROR IPAModule ipa_module.cpp:292 v4l2-compat.so: IPA module has no valid info
[12:34:30.015125806] [206341]  INFO Camera camera_manager.cpp:284 libcamera v0.1.0
```

I believe this is because some distributions are probably overriding the
installation path of the v4l2-compat.so.

Anyway, that makes me think that we /should/ ignore files that are not
prefixed by ipa_ ...


--
Kieran


> 
> --
> Kieran
> 
> 
> 
> > 
> >> ---
> >>   src/libcamera/ipa_manager.cpp | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> >> index 030ef43fb994..c0c2f027e902 100644
> >> --- a/src/libcamera/ipa_manager.cpp
> >> +++ b/src/libcamera/ipa_manager.cpp
> >> @@ -206,6 +206,10 @@ void IPAManager::parseDir(const char *libDir, unsigned int maxDepth,
> >>              if (strcmp(&ent->d_name[offset], ".so"))
> >>                      continue;
> >>   
> >> +            /* Ignore any modules which are not IPAs. */
> >> +            if (strncmp(ent->d_name, "ipa_", 4) != 0)
> >> +                    continue;
> >> +
> >>              files.push_back(std::string(libDir) + "/" + ent->d_name);
> >>      }
> >>   
> >