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

Fri Feb 14 11:37:54 CET 2025

Hi Dylan,

Quoting Dylan Aissi (2025-02-13 22:17:45)
> Hi Kieran,
> 
> On 2/13/25 18:36, Kieran Bingham wrote:
> > Hi Dylan, Javier,
> > 
> > I've got some questions for you both at the bottom of this mail
> > regarding libcamera packaging.
> > 
> > Quoting Laurent Pinchart (2025-02-13 17:01:26)
> >> On Wed, Feb 12, 2025 at 09:42:07AM +0000, Kieran Bingham wrote:
> >>> Quoting Laurent Pinchart (2023-08-28 20:13:13)
> >>>> On Mon, Aug 28, 2023 at 01:28:37PM +0100, Kieran Bingham wrote:
> >>>>> Quoting Kieran Bingham (2023-05-04 20:03:56)
> >>>>>> On 04/05/2023 16:10, Laurent Pinchart wrote:
> >>>>>>> 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_ ...
> >>>>
> >>>> Or move IPA modules to an ipa/ subdirectory ?
> >>>
> >>> And I've just seen this in some logs from a raspberry pi issue:
> >>>
> >>> ```
> >>>    (AGS_311_venv) @AGS-RPI-0004:~$ libcamera-hello
> >>>    [0:01:23.242753930] [811] ERROR IPAModule ipa_module.cpp:171 Symbol ipaModuleInfo not found
> >>>    [0:01:23.242802797] [811] ERROR IPAModule ipa_module.cpp:291 v4l2-compat.so: IPA module has
> >>>    no valid info
> >>>    [0:01:23.242827435] [811]  INFO Camera camera_manager.cpp:327 libcamera v0.4.0+51-ca36c77f
> >>> ```
> >>>
> >>> So I think it's time to move the IPA modules to a subdirectory. I like
> >>> that better than arbitrary filtering of the name.
> >>
> >> It's really a distribution issue, by default IPA modules go to (on an
> >> Arm64 system) /usr/local/lib64 (and only IPA modules are installed
> >> there), and v4l2-compat.so go to /usr/local/libexec/libcamera/. We can
> >> create a subdirectory for IPA modules, but will distributions then
> >> decide to install v4l2-compat.so there ? :-)
> > 
> > A distribution issue that's impacting multiple distributions ...
> > 
> > So the question is /why/ ?
> > 
> > 
> > https://salsa.debian.org/multimedia-team/libcamera/-/blob/debian/unstable/debian/libcamera-ipa.install?ref_type=heads
> > shows:
> > 
> > 
> > usr/lib/*/libcamera/ipa_*.so
> > usr/lib/*/libcamera/ipa_*.so.sign
> > usr/lib/*/libcamera/*_ipa_proxy
> > usr/share/libcamera
> > 
> > so it's capturing the ipa files into a package based on where they get
> > installed.
> > 
> > https://salsa.debian.org/multimedia-team/libcamera/-/blob/debian/unstable/debian/libcamera-v4l2.install?ref_type=heads
> > 
> > shows
> > 
> > usr/lib/*/libcamera/v4l2-compat.so
> > 
> > for the V4L2 side.
> > 
> > 
> > And finally:
> > 
> > https://salsa.debian.org/multimedia-team/libcamera/-/blob/debian/unstable/debian/rules?ref_type=heads
> > 
> > Shows:
> > 
> > override_dh_auto_configure:
> >       dh_auto_configure -- \
> >               --libexecdir=lib/${DEB_HOST_MULTIARCH} \
> > 
> > 
> > Which is overridding the libexecdir for multiarch.
> > 
> > 
> > Trying to search for debian guidance on this I hit:
> > 
> > https://linux.debian.devel.mentors.narkive.com/hkilz1si/using-usr-libexec-directory
> > which is a mentor on the debian lists giving the following guidance:
> > 
> >> Move the files to use "usr/lib" instead. Nothing on debian uses libexec.
> >>
> >> Usually there is a configure option one can pass, e.g. --libexecdir=/usr/lib
> > 
> > and
> > https://lists.debian.org/debian-devel/2005/05/msg00404.html
> >>> Should we change some of these to /usr/libexec?
> >>
> >> well, it would be against the FHS, I think.
> >>
> >> The BSDs use libexec but I don't really see a good reason why it exists.
> >>
> > 
> > And this one:
> > https://lists.debian.org/debian-devel/2005/05/msg00565.html
> >> The BSDs use libexec but I don't really see a good reason why it exists.
> >>
> >> The only reason we don't have it is because of petty bickering and
> >> politics between the FHS folks (several years ago).  There were few
> >> technical objections to it on the FHS list, but it was dropped for
> >> non-technical reasons.  Given that the FHS is supposed to codify
> >> existing practice, it should be in there on that count alone.  Every
> >> libexec-using package in Debian has been reconfigured not to use it;
> >> upstreams do use it, and I'd like to use it myself.
> >>
> >> I'd personally be very glad to have it, and would support using it in
> >> Debian.
> > 
> > 
> > But so far I haven't found any documentation to 'specify' this.
> > 
> > Dylan, Do you know /why/ the v4l2-compat.so is moved out of libexec and
> > into lib/libcamera ? (Noting that it causes a spurious error to be
> > reported to users who have libcamera-v4l2 installed).
> 
> I think the answer is just "bad sequence of events" and there is no real 
> reason.
> Let's do some git archeology to understand what happened:
> [1] In 2020, libexecdir was overridden to make the package multiarch 
> compatible.
> [2] ~ 10 days later, v4l2 compat was enabled, so with libexecdir already 
> overridden.
> [3] In 2022, I moved v4l2 into its own binary package, but unrelated 
> with our current issue.
> [4] In 2023, v4l2 was moved upstream from libexec to libexec/libcamera.
> [5] This was reflected in the debian package but with the override of 
> libexecdir, v4l2 has landed into usr/lib/*/libcamera/ which is the same 
> directory than ipa modules.

Thank you - that is incredibly helpful! And now it's much clearer what
was going on.

> I don't see a good reason other than trying to make v4l2 and *_ipa_proxy 
> multiarch co-installable.
> Now the question is, does this make sense? I don't know. I can drop the 
> libexecdir override, both v4l2 and *_ipa_proxy will go back in 
> usr/libexec/libcamera/
> and both binary packages libcamera-v4l2 and libcamera-ipa will no longer 
> be multiarch co-installable, but I am not sure there was a real use case 
> anyway.

The tricky part here will likely be the IPA proxies. I think that's part
of 'libcamera' but could be handled as part of libcamera-ipa indeed, and
then that component shouldn't be multiarch - the package itself would be
architecture specific. Same for the libcamera-v4l2.

Having libcamera core multi-arch facilitates cross compiling which is
great. libcamera-v4l2 and libcamera-ipa are not required for that.

Are there other use-cases for having multi-arch libcamera?

> 
> [1] 
> https://salsa.debian.org/multimedia-team/libcamera/-/commit/04fda66ee27bfacd07a39acb86bbaba376bcc050
> [2] 
> https://salsa.debian.org/multimedia-team/libcamera/-/commit/89ab628d6d9605547423c7696f4dbc8b10ce7708
> [3] 
> https://salsa.debian.org/multimedia-team/libcamera/-/commit/7bfebf853bb18004ed26c822a0b8a1cb3fb1f0d7
> [4] 
> https://gitlab.freedesktop.org/camera/libcamera/-/commit/1c512d406536d72a393c38c3f6a75fe0fdb9ecb2
> [5] 
> https://salsa.debian.org/multimedia-team/libcamera/-/commit/a991999c4c935d150c04a88f401cf2dcf5314c71
> 
> 
> > I know this same issue is occuring on Alpine and Arch systems too. So it
> > seems like "all the major distributions" are doing roughly the same
> > thing here.
> 
> Maybe for the same reason, or maybe they just copied our mistakes.

Indeed, I'll chase those distributions with the same rationale now that
we understand it though.

Thank you very much for helping get this clear.

Will you make an update to the debian packages?

--
Kieran

> > Javier, Do you know if this is happening on Fedora or if there is any
> > requirements regarding /usr/libexec ?
> > 
> > --
> > Kieran
> 
> Best regards,
> Dylan