[libcamera-devel,00/11] libcamera: Add support for IPA configuration

3552 mbox series

Hello,

It is expected that IPA will need configuration files. To standardize
the location of those files and avoid putting too much burden on
pipeline handlers and IPA modules, support for this features is needed
in the libcamera core. This patch series is an attempt to do so.

The series starts in patch 01/11 by changing usage of the IPA module
name, as stored in the IPAModuleInfo structure. The name was a
free-formed string, and is now required to match the directory name of
the IPA module in the source tree for modules part of libcamera. For
external modules, the name is required to be usable as a directory
component.

Patch 02/11 then adds a new function to retrieve the libcamera source
tree directory. This is similar to the build tree directory retrieval
that is already used to locate IPA modules or proxy workers in the build
tree, and will be used to locate configuration files in the source tree.
There's a small issue with that patch in that it hardcodes the path to
the source tree in the libcamera binary. I would like to strip that at
install time, but haven't found a good way to do so yet.

Patch 03/11 and 04/11 are small cleanups. Patch 05/11 starts the bulk of
the work by adding a function to the IPAProxy class to retrieve the
absolute path to a configuration file for an IPA module based on the
name of the file. Please see the patch for more details.

Patch 06/11 changes the IPAManager::createIPA() function to return an
IPAProxy instead of an IPAInterface, giving the pipeline handlers a way
to access the IPAProxy helper functions. This is fairly non-intrusive as
the IPAInterface returned by that function is actually an IPAProxy
instance already.

Patch 07/11 extends the IPAInterface::init() function with an
IPASettings argument, that only contains the configuration file path for
now. This is expected to be extended later with more initialization
data.

Patches 08/11 to 11/11 then update the vimc IPA, and vimc pipeline
handler and the tests to add a (currently dummy) configuration file to
the source tree (08/11), pass it from the pipeline handler and tests to
the IPA (09/11 and 10/11), and verifying it in the IPA (11/11).

Except for the source tree path issue, I'm fairly happy with the result,
but I'm open to reconsidering the approach if needed. Any magic solution
for the source tree path issue will be highly appreciated :-)

On a side note, I've discovered a problem with IPA module signatures
when installing the modules. Both the module and the signature files are
installed in the right location, but the module is modified to strip the
DT_RPATH and DT_RUNPATH tags, which renders the signature invalid. I can
see two options to fix this, regenerating the signature at install time,
and ignoring the dynamic section when generating and verifying the
signature. I'm not sure yet whether the former is possible with meson.
The latter should ideally not ignore the whole dynamic section, as it
may contain data that should be signed, but strip the DT_RPATH and
DT_RUNPATH tags exactly as done by meson when generating the signature
and verifying it. This may however be fragile, so ignoring the whole
section may be the best option for now. I'm also open to ideas.

Laurent Pinchart (11):
  ipa: Name IPA modules after their source directory
  libcamera: utils: Add a function to retrieve the libcamera source tree
  libcamera: ipa_proxy: Add missing space in info message
  libcamera: ipa_proxy: Don't include <iostream>
  libcamera: ipa_proxy: Provide suport for IPA configuration files
  libcamera: ipa_manager: Return an IPAProxy from createIPA()
  ipa: Pass IPA initialization settings to IPAInterface::init()
  ipa: vimc: Add a dummy configuration file to the vimc IPA
  libcamera: pipeline: vimc: Pass configuration file to IPA init()
  test: ipa: ipa_interface: Pass configuration file to IPA init()
  ipa: vimc: Validate configuration file in init()

 include/ipa/ipa_interface.h                 | 13 ++-
 meson.build                                 |  2 +
 src/ipa/libipa/ipa_interface_wrapper.cpp    |  8 +-
 src/ipa/libipa/ipa_interface_wrapper.h      |  3 +-
 src/ipa/meson.build                         |  6 ++
 src/ipa/rkisp1/rkisp1.cpp                   |  4 +-
 src/ipa/vimc/data/meson.build               |  6 ++
 src/ipa/vimc/data/vimc.conf                 |  1 +
 src/ipa/vimc/meson.build                    |  2 +
 src/ipa/vimc/vimc.cpp                       | 17 +++-
 src/libcamera/include/ipa_context_wrapper.h |  2 +-
 src/libcamera/include/ipa_manager.h         |  6 +-
 src/libcamera/include/ipa_proxy.h           | 11 ++-
 src/libcamera/include/pipeline_handler.h    |  5 +-
 src/libcamera/include/utils.h               |  1 +
 src/libcamera/ipa_context_wrapper.cpp       |  9 +-
 src/libcamera/ipa_interface.cpp             | 32 +++++++
 src/libcamera/ipa_manager.cpp               | 14 +--
 src/libcamera/ipa_module.cpp                | 20 +++++
 src/libcamera/ipa_proxy.cpp                 | 95 +++++++++++++++++++--
 src/libcamera/pipeline/rkisp1/rkisp1.cpp    |  2 +-
 src/libcamera/pipeline/vimc/vimc.cpp        |  8 +-
 src/libcamera/proxy/ipa_proxy_linux.cpp     |  4 +-
 src/libcamera/proxy/ipa_proxy_thread.cpp    |  8 +-
 src/libcamera/utils.cpp                     | 37 +++++++-
 test/ipa/ipa_interface_test.cpp             | 10 ++-
 test/ipa/ipa_module_test.cpp                |  2 +-
 test/ipa/ipa_wrappers_test.cpp              | 13 ++-
 28 files changed, 288 insertions(+), 53 deletions(-)
 create mode 100644 src/ipa/vimc/data/meson.build
 create mode 100644 src/ipa/vimc/data/vimc.conf