| Message ID | 20250626095944.1746345-5-paul.elder@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 06. 26. 11:59 keltezéssel, Paul Elder írta: > We want to be able to implement layers in libcamera, which conceptually > sit in between the Camera class and the application. This can be useful > for implementing things that don't belong inside the Camera/IPA nor inside > the application, such as intercepting and translation the AeEnable > control, or implementing the Sync algorithm. > > To achieve this, first add a LayerManager implementation, which searches > for and loads layers from shared object files, and orchestrates > executing them. Actually calling into these functions from the Camera > class will be added in the following patch. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > include/libcamera/internal/layer_manager.h | 74 +++++ > include/libcamera/internal/meson.build | 1 + > include/libcamera/layer.h | 56 ++++ > include/libcamera/meson.build | 1 + > src/layer/meson.build | 10 + > src/libcamera/layer_manager.cpp | 314 +++++++++++++++++++++ > src/libcamera/meson.build | 1 + > src/meson.build | 1 + > 8 files changed, 458 insertions(+) > create mode 100644 include/libcamera/internal/layer_manager.h > create mode 100644 include/libcamera/layer.h > create mode 100644 src/layer/meson.build > create mode 100644 src/libcamera/layer_manager.cpp > > diff --git a/include/libcamera/internal/layer_manager.h b/include/libcamera/internal/layer_manager.h > new file mode 100644 > index 000000000000..73ccad01bca0 > --- /dev/null > +++ b/include/libcamera/internal/layer_manager.h > @@ -0,0 +1,74 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board Oy > + * > + * Layer manager interface > + */ > + > +#pragma once > + > +#include <deque> > +#include <memory> > +#include <set> > +#include <string> > + > +#include <libcamera/base/log.h> > +#include <libcamera/base/span.h> > + > +#include <libcamera/camera.h> > +#include <libcamera/controls.h> > +#include <libcamera/framebuffer.h> > +#include <libcamera/layer.h> > +#include <libcamera/request.h> > +#include <libcamera/stream.h> > + > +namespace libcamera { > + > +LOG_DECLARE_CATEGORY(LayerManager) > + > +class LayerManager > +{ > +public: > + LayerManager(); > + ~LayerManager(); > + > + void bufferCompleted(Request *request, FrameBuffer *buffer); > + void requestCompleted(Request *request); > + void disconnected(); > + > + void acquire(); > + void release(); > + > + const ControlInfoMap &controls(const ControlInfoMap &controlInfoMap); > + const ControlList &properties(const ControlList &properties); > + const std::set<Stream *> &streams(const std::set<Stream *> &streams); > + > + void generateConfiguration(Span<const StreamRole> &roles, > + CameraConfiguration *config); > + > + void configure(CameraConfiguration *config); > + > + void createRequest(uint64_t cookie, Request *request); > + > + void queueRequest(Request *request); > + > + void start(const ControlList *controls); > + void stop(); > + > +private: > + /* Extend the layer with information specific to load-handling */ > + struct LayerLoaded > + { > + Layer layer; Any reason for not just storing a pointer? The struct is not useful without the DSO loaded, so I feel like a pointer is fine. > + void *dlHandle; The destructor should probably close this. And please disable the copy constructor/assignment, and implement move construction/assignment. If that is done, I think you won't need `unique_ptr<LayerLoaded>` anywhere. Might even make sense to add something simple since this is now the second user of the dl*() API, e.g. struct DLCloser { void operator()(void *p) { ::dlclose(p); } }; using DSOPointer = std::unique_ptr<void, DLCloser>; But maybe we should wait for a third... > + }; > + > + std::unique_ptr<LayerLoaded> createLayer(const std::string &file); > + std::deque<std::unique_ptr<LayerLoaded>> executionQueue_; > + > + ControlInfoMap controlInfoMap_; > + ControlList properties_; > + std::set<Stream *> streams_; > +}; > + > +} /* namespace libcamera */ > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > index 690f5c5ec9f6..20e6c295601f 100644 > --- a/include/libcamera/internal/meson.build > +++ b/include/libcamera/internal/meson.build > @@ -29,6 +29,7 @@ libcamera_internal_headers = files([ > 'ipa_proxy.h', > 'ipc_pipe.h', > 'ipc_unixsocket.h', > + 'layer_manager.h', > 'mapped_framebuffer.h', > 'matrix.h', > 'media_device.h', > diff --git a/include/libcamera/layer.h b/include/libcamera/layer.h > new file mode 100644 > index 000000000000..8fa0ee7d24e6 > --- /dev/null > +++ b/include/libcamera/layer.h > @@ -0,0 +1,56 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board Oy > + * > + * Layer interface > + */ > + > +#pragma once > + > +#include <set> > +#include <stdint.h> > + > +#include <libcamera/base/span.h> > + > +namespace libcamera { > + > +class CameraConfiguration; > +class ControlInfoMap; > +class ControlList; > +class FrameBuffer; > +class Request; > +class Stream; > +enum class StreamRole; > + > +struct Layer > +{ > + const char *name; > + int layerAPIVersion; > + > + void (*init)(const std::string &id); I haven't seen it mentioned, sorry if it was; but I think every method here will definitely need some kind of closure pointer, like a `void *`. And I think I would separate the vtable from the other fields. Or given that this is currently C++ dependent, I think using C++ virtual might not be not an issue. > + > + void (*bufferCompleted)(Request *, FrameBuffer *); > + void (*requestCompleted)(Request *); > + void (*disconnected)(); > + > + void (*acquire)(); > + void (*release)(); > + > + ControlInfoMap::Map (*controls)(ControlInfoMap &); > + ControlList (*properties)(ControlList &); > + std::set<Stream *> (*streams)(std::set<Stream *> &); > + > + void (*generateConfiguration)(Span<const StreamRole> &, > + CameraConfiguration *); > + > + void (*configure)(CameraConfiguration *); > + > + void (*createRequest)(uint64_t, Request *); > + > + void (*queueRequest)(Request *); > + > + void (*start)(const ControlList *); > + void (*stop)(); > +} __attribute__((packed)); I am not sure if this needs to be packed? But if it does, could you use `struct [[gnu::packed]] Layer {` notation? > + > +} /* namespace libcamera */ > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index 30ea76f9470a..552af112abb5 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -11,6 +11,7 @@ libcamera_public_headers = files([ > 'framebuffer.h', > 'framebuffer_allocator.h', > 'geometry.h', > + 'layer.h', > 'logging.h', > 'orientation.h', > 'pixel_format.h', > diff --git a/src/layer/meson.build b/src/layer/meson.build > new file mode 100644 > index 000000000000..dee5e5ac5804 > --- /dev/null > +++ b/src/layer/meson.build > @@ -0,0 +1,10 @@ > +# SPDX-License-Identifier: CC0-1.0 > + > +layer_includes = [ > + libcamera_includes, > +] > + > +layer_install_dir = libcamera_libdir / 'layers' > + > +config_h.set('LAYER_DIR', > + '"' + get_option('prefix') / layer_install_dir + '"') > diff --git a/src/libcamera/layer_manager.cpp b/src/libcamera/layer_manager.cpp > new file mode 100644 > index 000000000000..96d53d4fc75d > --- /dev/null > +++ b/src/libcamera/layer_manager.cpp > @@ -0,0 +1,314 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board Oy > + * > + * Layer manager > + */ > + > +#include "libcamera/internal/layer_manager.h" > + > +#include <algorithm> > +#include <dirent.h> > +#include <dlfcn.h> > +#include <map> > +#include <memory> > +#include <set> > +#include <string.h> > +#include <string> > +#include <sys/types.h> > + > +#include <libcamera/base/file.h> > +#include <libcamera/base/log.h> > +#include <libcamera/base/utils.h> > +#include <libcamera/base/span.h> > + > +#include <libcamera/control_ids.h> > +#include <libcamera/layer.h> > + > +#include "libcamera/internal/utils.h" > + > +/** > + * \file layer_manager.h > + * \brief Layer manager > + */ > + > +namespace libcamera { > + > +LOG_DEFINE_CATEGORY(LayerManager) > + > +/** > + * \class LayerManager > + * \brief Layer manager > + * > + * The Layer manager discovers layer implementations from disk, and creates > + * execution queues for every function that is implemented by each layer and > + * executes them. A layer is a layer that sits between libcamera and the > + * application, and hooks into the public Camera interface. > + */ > + > +/** > + * \brief Construct a LayerManager instance > + * > + * The LayerManager class is meant be instantiated by the Camera. Is there any issue with loading all desired layers at `CameraManager` construction time, and instantiating them it for each camera that is registered? > + */ > +LayerManager::LayerManager() > +{ > + std::map<std::string, std::unique_ptr<LayerLoaded>> layers; > + > + /* This returns the number of "modules" successfully loaded */ > + std::function<int(const std::string &)> addDirHandler = > + [this, &layers](const std::string &file) { > + std::unique_ptr<LayerManager::LayerLoaded> layer = createLayer(file); > + if (!layer) > + return 0; > + > + LOG(LayerManager, Debug) << "Loaded layer '" << file << "'"; > + > + layers.insert({std::string(layer->layer.name), std::move(layer)}); > + > + return 1; > + }; > + > + /* User-specified paths take precedence. */ > + const char *layerPaths = utils::secure_getenv("LIBCAMERA_LAYER_PATH"); Can we please make sure that `LIBCAMERA_LAYER_PATH` can be used in the build directory as well? > + if (layerPaths) { > + for (const auto &dir : utils::split(layerPaths, ":")) { > + if (dir.empty()) > + continue; > + > + utils::addDir(dir.c_str(), 0, addDirHandler); > + } > + } > + > + /* > + * When libcamera is used before it is installed, load layers from the > + * same build directory as the libcamera library itself. > + */ > + std::string root = utils::libcameraBuildPath(); > + if (!root.empty()) { > + std::string layerBuildPath = root + "src/layer"; > + constexpr int maxDepth = 2; > + > + LOG(LayerManager, Info) > + << "libcamera is not installed. Adding '" > + << layerBuildPath << "' to the layer search path"; > + > + utils::addDir(layerBuildPath.c_str(), maxDepth, addDirHandler); > + } > + > + /* Finally try to load layers from the installed system path. */ > + utils::addDir(LAYER_DIR, 0, addDirHandler); > + > + /* Order the layers */ > + /* \todo Document this. First is closer to application, last is closer to libcamera */ > + const char *layerList = utils::secure_getenv("LIBCAMERA_LAYERS_ENABLE"); > + if (layerList) { > + for (const auto &layerName : utils::split(layerList, ":")) { > + if (layerName.empty()) > + continue; > + > + const auto &it = layers.find(layerName); > + if (it == layers.end()) > + continue; > + > + executionQueue_.push_back(std::move(it->second)); > + } > + } Layers still in `layers` leak their DSOs on destruction. Or am I missing something? > +} > + > +LayerManager::~LayerManager() > +{ > + for (auto &layer : executionQueue_) > + dlclose(layer->dlHandle); This should be handled in the destructor of each individual object in my opinion. > +} > + > +std::unique_ptr<LayerManager::LayerLoaded> LayerManager::createLayer(const std::string &filename) > +{ > + File file{ filename }; > + if (!file.open(File::OpenModeFlag::ReadOnly)) { > + LOG(LayerManager, Error) << "Failed to open layer: " > + << strerror(-file.error()); > + return nullptr; > + } > + > + Span<const uint8_t> data = file.map(); > + int ret = utils::elfVerifyIdent(data); > + if (ret) { > + LOG(LayerManager, Error) << "Layer is not an ELF file"; > + return nullptr; > + } > + > + Span<const uint8_t> info = utils::elfLoadSymbol(data, "layerInfo"); > + if (info.size() < sizeof(Layer)) { > + LOG(LayerManager, Error) << "Layer has no valid info"; > + return nullptr; > + } > + > + void *dlHandle = dlopen(file.fileName().c_str(), RTLD_LAZY); > + if (!dlHandle) { > + LOG(LayerManager, Error) > + << "Failed to open layer shared object: " > + << dlerror(); > + return nullptr; > + } > + > + void *symbol = dlsym(dlHandle, "layerInfo"); > + if (!symbol) { > + LOG(LayerManager, Error) > + << "Failed to load layerInfo from layer shared object: " > + << dlerror(); > + dlclose(dlHandle); > + dlHandle = nullptr; > + return nullptr; > + } > + > + std::unique_ptr<LayerManager::LayerLoaded> layer = > + std::make_unique<LayerManager::LayerLoaded>(); > + layer->layer = *reinterpret_cast<Layer *>(symbol); > + > + /* \todo Implement this. It should come from the libcamera version */ > + if (layer->layer.layerAPIVersion != 1) { > + LOG(LayerManager, Error) << "Layer API version mismatch"; > + return nullptr; > + } > + > + /* \todo Validate the layer name. */ > + > + layer->dlHandle = dlHandle; > + > + return layer; > +} > [...] > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 8e2aa921a620..226a94768514 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -40,6 +40,7 @@ libcamera_internal_sources = files([ > 'ipc_pipe.cpp', > 'ipc_pipe_unixsocket.cpp', > 'ipc_unixsocket.cpp', > + 'layer_manager.cpp', > 'mapped_framebuffer.cpp', > 'matrix.cpp', > 'media_device.cpp', > diff --git a/src/meson.build b/src/meson.build > index 8eb8f05b362f..37368b01cbf2 100644 > --- a/src/meson.build > +++ b/src/meson.build > @@ -63,6 +63,7 @@ subdir('libcamera') > > subdir('android') > subdir('ipa') > +subdir('layer') > > subdir('apps') > Regards, Barnabás Pőcze
Quoting Paul Elder (2025-06-26 10:59:39) > We want to be able to implement layers in libcamera, which conceptually > sit in between the Camera class and the application. This can be useful > for implementing things that don't belong inside the Camera/IPA nor inside > the application, such as intercepting and translation the AeEnable > control, or implementing the Sync algorithm. > > To achieve this, first add a LayerManager implementation, which searches > for and loads layers from shared object files, and orchestrates > executing them. Actually calling into these functions from the Camera > class will be added in the following patch. At some point - I wonder if we need module signatures too to decide which layers we trust ... or potentially have different levels of capabilities. Particularly important when we start looking at loadable layers that can 'see' the captured images ... My "definitely-doesnt-stream-video-live-to-youtube.so" is sure to be trustedworthy ... But for now this is a great start on the framework and plumbing! > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > include/libcamera/internal/layer_manager.h | 74 +++++ > include/libcamera/internal/meson.build | 1 + > include/libcamera/layer.h | 56 ++++ > include/libcamera/meson.build | 1 + > src/layer/meson.build | 10 + > src/libcamera/layer_manager.cpp | 314 +++++++++++++++++++++ > src/libcamera/meson.build | 1 + > src/meson.build | 1 + > 8 files changed, 458 insertions(+) > create mode 100644 include/libcamera/internal/layer_manager.h > create mode 100644 include/libcamera/layer.h > create mode 100644 src/layer/meson.build > create mode 100644 src/libcamera/layer_manager.cpp > > diff --git a/include/libcamera/internal/layer_manager.h b/include/libcamera/internal/layer_manager.h > new file mode 100644 > index 000000000000..73ccad01bca0 > --- /dev/null > +++ b/include/libcamera/internal/layer_manager.h > @@ -0,0 +1,74 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board Oy > + * > + * Layer manager interface > + */ > + > +#pragma once > + > +#include <deque> > +#include <memory> > +#include <set> > +#include <string> > + > +#include <libcamera/base/log.h> > +#include <libcamera/base/span.h> > + > +#include <libcamera/camera.h> > +#include <libcamera/controls.h> > +#include <libcamera/framebuffer.h> > +#include <libcamera/layer.h> > +#include <libcamera/request.h> > +#include <libcamera/stream.h> > + > +namespace libcamera { > + > +LOG_DECLARE_CATEGORY(LayerManager) > + > +class LayerManager > +{ > +public: > + LayerManager(); > + ~LayerManager(); > + > + void bufferCompleted(Request *request, FrameBuffer *buffer); > + void requestCompleted(Request *request); > + void disconnected(); > + > + void acquire(); > + void release(); > + > + const ControlInfoMap &controls(const ControlInfoMap &controlInfoMap); > + const ControlList &properties(const ControlList &properties); > + const std::set<Stream *> &streams(const std::set<Stream *> &streams); > + > + void generateConfiguration(Span<const StreamRole> &roles, > + CameraConfiguration *config); > + > + void configure(CameraConfiguration *config); > + > + void createRequest(uint64_t cookie, Request *request); > + > + void queueRequest(Request *request); > + > + void start(const ControlList *controls); > + void stop(); > + > +private: > + /* Extend the layer with information specific to load-handling */ > + struct LayerLoaded > + { > + Layer layer; > + void *dlHandle; > + }; > + > + std::unique_ptr<LayerLoaded> createLayer(const std::string &file); > + std::deque<std::unique_ptr<LayerLoaded>> executionQueue_; > + > + ControlInfoMap controlInfoMap_; > + ControlList properties_; > + std::set<Stream *> streams_; > +}; > + > +} /* namespace libcamera */ > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > index 690f5c5ec9f6..20e6c295601f 100644 > --- a/include/libcamera/internal/meson.build > +++ b/include/libcamera/internal/meson.build > @@ -29,6 +29,7 @@ libcamera_internal_headers = files([ > 'ipa_proxy.h', > 'ipc_pipe.h', > 'ipc_unixsocket.h', > + 'layer_manager.h', > 'mapped_framebuffer.h', > 'matrix.h', > 'media_device.h', > diff --git a/include/libcamera/layer.h b/include/libcamera/layer.h > new file mode 100644 > index 000000000000..8fa0ee7d24e6 > --- /dev/null > +++ b/include/libcamera/layer.h > @@ -0,0 +1,56 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board Oy > + * > + * Layer interface > + */ > + > +#pragma once > + > +#include <set> > +#include <stdint.h> > + > +#include <libcamera/base/span.h> > + > +namespace libcamera { > + > +class CameraConfiguration; > +class ControlInfoMap; > +class ControlList; > +class FrameBuffer; > +class Request; > +class Stream; > +enum class StreamRole; > + > +struct Layer > +{ > + const char *name; > + int layerAPIVersion; > + > + void (*init)(const std::string &id); > + > + void (*bufferCompleted)(Request *, FrameBuffer *); > + void (*requestCompleted)(Request *); > + void (*disconnected)(); > + > + void (*acquire)(); > + void (*release)(); > + > + ControlInfoMap::Map (*controls)(ControlInfoMap &); > + ControlList (*properties)(ControlList &); > + std::set<Stream *> (*streams)(std::set<Stream *> &); > + > + void (*generateConfiguration)(Span<const StreamRole> &, > + CameraConfiguration *); > + > + void (*configure)(CameraConfiguration *); > + > + void (*createRequest)(uint64_t, Request *); > + > + void (*queueRequest)(Request *); > + > + void (*start)(const ControlList *); > + void (*stop)(); > +} __attribute__((packed)); I'm curious, Why is this packed ? > + > +} /* namespace libcamera */ > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index 30ea76f9470a..552af112abb5 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -11,6 +11,7 @@ libcamera_public_headers = files([ > 'framebuffer.h', > 'framebuffer_allocator.h', > 'geometry.h', > + 'layer.h', > 'logging.h', > 'orientation.h', > 'pixel_format.h', > diff --git a/src/layer/meson.build b/src/layer/meson.build > new file mode 100644 > index 000000000000..dee5e5ac5804 > --- /dev/null > +++ b/src/layer/meson.build > @@ -0,0 +1,10 @@ > +# SPDX-License-Identifier: CC0-1.0 > + > +layer_includes = [ > + libcamera_includes, > +] > + > +layer_install_dir = libcamera_libdir / 'layers' > + > +config_h.set('LAYER_DIR', > + '"' + get_option('prefix') / layer_install_dir + '"') > diff --git a/src/libcamera/layer_manager.cpp b/src/libcamera/layer_manager.cpp > new file mode 100644 > index 000000000000..96d53d4fc75d > --- /dev/null > +++ b/src/libcamera/layer_manager.cpp > @@ -0,0 +1,314 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board Oy > + * > + * Layer manager > + */ > + > +#include "libcamera/internal/layer_manager.h" > + > +#include <algorithm> > +#include <dirent.h> > +#include <dlfcn.h> > +#include <map> > +#include <memory> > +#include <set> > +#include <string.h> > +#include <string> > +#include <sys/types.h> > + > +#include <libcamera/base/file.h> > +#include <libcamera/base/log.h> > +#include <libcamera/base/utils.h> > +#include <libcamera/base/span.h> > + > +#include <libcamera/control_ids.h> > +#include <libcamera/layer.h> > + > +#include "libcamera/internal/utils.h" > + > +/** > + * \file layer_manager.h > + * \brief Layer manager > + */ > + > +namespace libcamera { > + > +LOG_DEFINE_CATEGORY(LayerManager) > + > +/** > + * \class LayerManager > + * \brief Layer manager > + * > + * The Layer manager discovers layer implementations from disk, and creates > + * execution queues for every function that is implemented by each layer and > + * executes them. A layer is a layer that sits between libcamera and the > + * application, and hooks into the public Camera interface. > + */ > + > +/** > + * \brief Construct a LayerManager instance > + * > + * The LayerManager class is meant be instantiated by the Camera. > + */ > +LayerManager::LayerManager() > +{ > + std::map<std::string, std::unique_ptr<LayerLoaded>> layers; > + > + /* This returns the number of "modules" successfully loaded */ > + std::function<int(const std::string &)> addDirHandler = > + [this, &layers](const std::string &file) { > + std::unique_ptr<LayerManager::LayerLoaded> layer = createLayer(file); > + if (!layer) > + return 0; > + > + LOG(LayerManager, Debug) << "Loaded layer '" << file << "'"; > + > + layers.insert({std::string(layer->layer.name), std::move(layer)}); > + > + return 1; > + }; > + > + /* User-specified paths take precedence. */ > + const char *layerPaths = utils::secure_getenv("LIBCAMERA_LAYER_PATH"); > + if (layerPaths) { > + for (const auto &dir : utils::split(layerPaths, ":")) { > + if (dir.empty()) > + continue; > + > + utils::addDir(dir.c_str(), 0, addDirHandler); > + } > + } > + > + /* > + * When libcamera is used before it is installed, load layers from the > + * same build directory as the libcamera library itself. > + */ > + std::string root = utils::libcameraBuildPath(); > + if (!root.empty()) { > + std::string layerBuildPath = root + "src/layer"; > + constexpr int maxDepth = 2; > + > + LOG(LayerManager, Info) > + << "libcamera is not installed. Adding '" > + << layerBuildPath << "' to the layer search path"; > + > + utils::addDir(layerBuildPath.c_str(), maxDepth, addDirHandler); > + } > + > + /* Finally try to load layers from the installed system path. */ > + utils::addDir(LAYER_DIR, 0, addDirHandler); > + > + /* Order the layers */ > + /* \todo Document this. First is closer to application, last is closer to libcamera */ > + const char *layerList = utils::secure_getenv("LIBCAMERA_LAYERS_ENABLE"); > + if (layerList) { > + for (const auto &layerName : utils::split(layerList, ":")) { > + if (layerName.empty()) > + continue; > + > + const auto &it = layers.find(layerName); > + if (it == layers.end()) > + continue; > + > + executionQueue_.push_back(std::move(it->second)); > + } > + } > +} > + > +LayerManager::~LayerManager() > +{ > + for (auto &layer : executionQueue_) > + dlclose(layer->dlHandle); > +} > + > +std::unique_ptr<LayerManager::LayerLoaded> LayerManager::createLayer(const std::string &filename) > +{ > + File file{ filename }; > + if (!file.open(File::OpenModeFlag::ReadOnly)) { > + LOG(LayerManager, Error) << "Failed to open layer: " > + << strerror(-file.error()); > + return nullptr; > + } > + > + Span<const uint8_t> data = file.map(); > + int ret = utils::elfVerifyIdent(data); > + if (ret) { > + LOG(LayerManager, Error) << "Layer is not an ELF file"; > + return nullptr; > + } > + > + Span<const uint8_t> info = utils::elfLoadSymbol(data, "layerInfo"); > + if (info.size() < sizeof(Layer)) { > + LOG(LayerManager, Error) << "Layer has no valid info"; > + return nullptr; > + } > + > + void *dlHandle = dlopen(file.fileName().c_str(), RTLD_LAZY); > + if (!dlHandle) { > + LOG(LayerManager, Error) > + << "Failed to open layer shared object: " > + << dlerror(); > + return nullptr; > + } > + > + void *symbol = dlsym(dlHandle, "layerInfo"); > + if (!symbol) { > + LOG(LayerManager, Error) > + << "Failed to load layerInfo from layer shared object: " > + << dlerror(); > + dlclose(dlHandle); > + dlHandle = nullptr; > + return nullptr; > + } > + > + std::unique_ptr<LayerManager::LayerLoaded> layer = > + std::make_unique<LayerManager::LayerLoaded>(); > + layer->layer = *reinterpret_cast<Layer *>(symbol); > + > + /* \todo Implement this. It should come from the libcamera version */ > + if (layer->layer.layerAPIVersion != 1) { > + LOG(LayerManager, Error) << "Layer API version mismatch"; > + return nullptr; > + } > + > + /* \todo Validate the layer name. */ > + > + layer->dlHandle = dlHandle; > + > + return layer; > +} > + > +void LayerManager::bufferCompleted(Request *request, FrameBuffer *buffer) > +{ > + /* Reverse order because this comes from a Signal emission */ > + for (auto it = executionQueue_.rbegin(); > + it != executionQueue_.rend(); it++) { > + if ((*it)->layer.bufferCompleted) > + (*it)->layer.bufferCompleted(request, buffer); > + } > +} > + > +void LayerManager::requestCompleted(Request *request) > +{ > + /* Reverse order because this comes from a Signal emission */ > + for (auto it = executionQueue_.rbegin(); > + it != executionQueue_.rend(); it++) { > + if ((*it)->layer.requestCompleted) > + (*it)->layer.requestCompleted(request); > + } > +} > + > +void LayerManager::disconnected() > +{ > + /* Reverse order because this comes from a Signal emission */ > + for (auto it = executionQueue_.rbegin(); > + it != executionQueue_.rend(); it++) { > + if ((*it)->layer.disconnected) > + (*it)->layer.disconnected(); > + } > +} > + > +void LayerManager::acquire() > +{ > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > + if (layer->layer.acquire) > + layer->layer.acquire(); > +} > + > +void LayerManager::release() > +{ > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > + if (layer->layer.release) > + layer->layer.release(); > +} > + > +const ControlInfoMap &LayerManager::controls(const ControlInfoMap &controlInfoMap) > +{ I'm a bit surprised here actually - What happens if the applications calls Camera->controls() multiple times here? I don't think I would manipulate controls in this call - rather I would suspect we should have a LayerManager::init(ControlInfoMap &controlInfoMap) which would be called after the IPA has initialised it's controls into the same map ... Then there would be no indirection with LayerManager::controls() that's simply returns the single configured map. > + controlInfoMap_ = controlInfoMap; > + > + /* \todo Simplify this once ControlInfoMaps become easier to modify */ > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) { > + if (layer->layer.controls) { > + ControlInfoMap::Map ret = layer->layer.controls(controlInfoMap_); > + ControlInfoMap::Map map; > + /* Merge the layer's ret later so that layers can overwrite */ > + for (auto &pair : controlInfoMap_) > + map.insert(pair); > + for (auto &pair : ret) > + map.insert(pair); > + controlInfoMap_ = ControlInfoMap(std::move(map), > + libcamera::controls::controls); > + } > + } > + return controlInfoMap_; > +} > + > +const ControlList &LayerManager::properties(const ControlList &properties) > +{ Similar here I think - this should probably just happen 'once' at ::init() and leave the accessor to return the const data without modifying it? > + properties_ = properties; > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) { > + if (layer->layer.properties) { > + ControlList ret = layer->layer.properties(properties_); > + properties_.merge(ret, ControlList::MergePolicy::OverwriteExisting); > + } > + } > + return properties_; > +} > + > +const std::set<Stream *> &LayerManager::streams(const std::set<Stream *> &streams) > +{ > + streams_ = streams; > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) { > + if (layer->layer.streams) { > + std::set<Stream *> ret = layer->layer.streams(streams_); > + streams_.insert(ret.begin(), ret.end()); > + } > + } > + return streams_; Ohhhh I was about to ask why would we have streams ... but indeed I could imagine a layer that could generate processed streams ... Eeek though for again with the modifying 'const' state. > +} > + > +void LayerManager::generateConfiguration(Span<const StreamRole> &roles, > + CameraConfiguration *config) > +{ > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > + if (layer->layer.generateConfiguration) > + layer->layer.generateConfiguration(roles, config); I don't think we can have generateConfiguration without validate configuration ... Also - maybe we should consider extra stream handling 'later', as both this and streams() is going to be tricky. Layers are going to do configuration validation and see extra streams that they don't know about and they'll just chop them out of the config. So I'm not sure how these parts could work yet... > +} > + > +void LayerManager::configure(CameraConfiguration *config) > +{ > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > + if (layer->layer.configure) > + layer->layer.configure(config); I think this is valid info for a layer to be told though so this is a good hook. > +} > + > +void LayerManager::createRequest(uint64_t cookie, Request *request) > +{ > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > + if (layer->layer.createRequest) > + layer->layer.createRequest(cookie, request); I'm not sure I'd hook this - What would a layer do when creating a request ? Or perhaps it's so they can store some private data or a map ? but I suspect cookie and request should be const here if so. Unless layers could replace requests on the way in an back out :S > +} > + > +void LayerManager::queueRequest(Request *request) > +{ > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > + if (layer->layer.queueRequest) > + layer->layer.queueRequest(request); > +} > + > +void LayerManager::start(const ControlList *controls) > +{ > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > + if (layer->layer.start) > + layer->layer.start(controls); > +} > + > +void LayerManager::stop() > +{ > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > + if (layer->layer.stop) > + layer->layer.stop(); > +} > + > +} /* namespace libcamera */ > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 8e2aa921a620..226a94768514 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -40,6 +40,7 @@ libcamera_internal_sources = files([ > 'ipc_pipe.cpp', > 'ipc_pipe_unixsocket.cpp', > 'ipc_unixsocket.cpp', > + 'layer_manager.cpp', > 'mapped_framebuffer.cpp', > 'matrix.cpp', > 'media_device.cpp', > diff --git a/src/meson.build b/src/meson.build > index 8eb8f05b362f..37368b01cbf2 100644 > --- a/src/meson.build > +++ b/src/meson.build > @@ -63,6 +63,7 @@ subdir('libcamera') > > subdir('android') > subdir('ipa') > +subdir('layer') > > subdir('apps') > > -- > 2.47.2 >
Hi Barnabás, Thanks for the review. Quoting Barnabás Pőcze (2025-06-26 19:38:49) > Hi > > 2025. 06. 26. 11:59 keltezéssel, Paul Elder írta: > > We want to be able to implement layers in libcamera, which conceptually > > sit in between the Camera class and the application. This can be useful > > for implementing things that don't belong inside the Camera/IPA nor inside > > the application, such as intercepting and translation the AeEnable > > control, or implementing the Sync algorithm. > > > > To achieve this, first add a LayerManager implementation, which searches > > for and loads layers from shared object files, and orchestrates > > executing them. Actually calling into these functions from the Camera > > class will be added in the following patch. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > include/libcamera/internal/layer_manager.h | 74 +++++ > > include/libcamera/internal/meson.build | 1 + > > include/libcamera/layer.h | 56 ++++ > > include/libcamera/meson.build | 1 + > > src/layer/meson.build | 10 + > > src/libcamera/layer_manager.cpp | 314 +++++++++++++++++++++ > > src/libcamera/meson.build | 1 + > > src/meson.build | 1 + > > 8 files changed, 458 insertions(+) > > create mode 100644 include/libcamera/internal/layer_manager.h > > create mode 100644 include/libcamera/layer.h > > create mode 100644 src/layer/meson.build > > create mode 100644 src/libcamera/layer_manager.cpp > > > > diff --git a/include/libcamera/internal/layer_manager.h b/include/libcamera/internal/layer_manager.h > > new file mode 100644 > > index 000000000000..73ccad01bca0 > > --- /dev/null > > +++ b/include/libcamera/internal/layer_manager.h > > @@ -0,0 +1,74 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board Oy > > + * > > + * Layer manager interface > > + */ > > + > > +#pragma once > > + > > +#include <deque> > > +#include <memory> > > +#include <set> > > +#include <string> > > + > > +#include <libcamera/base/log.h> > > +#include <libcamera/base/span.h> > > + > > +#include <libcamera/camera.h> > > +#include <libcamera/controls.h> > > +#include <libcamera/framebuffer.h> > > +#include <libcamera/layer.h> > > +#include <libcamera/request.h> > > +#include <libcamera/stream.h> > > + > > +namespace libcamera { > > + > > +LOG_DECLARE_CATEGORY(LayerManager) > > + > > +class LayerManager > > +{ > > +public: > > + LayerManager(); > > + ~LayerManager(); > > + > > + void bufferCompleted(Request *request, FrameBuffer *buffer); > > + void requestCompleted(Request *request); > > + void disconnected(); > > + > > + void acquire(); > > + void release(); > > + > > + const ControlInfoMap &controls(const ControlInfoMap &controlInfoMap); > > + const ControlList &properties(const ControlList &properties); > > + const std::set<Stream *> &streams(const std::set<Stream *> &streams); > > + > > + void generateConfiguration(Span<const StreamRole> &roles, > > + CameraConfiguration *config); > > + > > + void configure(CameraConfiguration *config); > > + > > + void createRequest(uint64_t cookie, Request *request); > > + > > + void queueRequest(Request *request); > > + > > + void start(const ControlList *controls); > > + void stop(); > > + > > +private: > > + /* Extend the layer with information specific to load-handling */ > > + struct LayerLoaded > > + { > > + Layer layer; > > Any reason for not just storing a pointer? The struct is not useful without > the DSO loaded, so I feel like a pointer is fine. Good point, I missed that. > > > > + void *dlHandle; > > The destructor should probably close this. And please disable the copy > constructor/assignment, and implement move construction/assignment. If > that is done, I think you won't need `unique_ptr<LayerLoaded>` anywhere. That's a good idea. > > Might even make sense to add something simple since this is now the second user > of the dl*() API, e.g. > > struct DLCloser { void operator()(void *p) { ::dlclose(p); } }; > using DSOPointer = std::unique_ptr<void, DLCloser>; > > But maybe we should wait for a third... I think two might be enough... this does look nice. > > > > + }; > > + > > + std::unique_ptr<LayerLoaded> createLayer(const std::string &file); > > + std::deque<std::unique_ptr<LayerLoaded>> executionQueue_; > > + > > + ControlInfoMap controlInfoMap_; > > + ControlList properties_; > > + std::set<Stream *> streams_; > > +}; > > + > > +} /* namespace libcamera */ > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > > index 690f5c5ec9f6..20e6c295601f 100644 > > --- a/include/libcamera/internal/meson.build > > +++ b/include/libcamera/internal/meson.build > > @@ -29,6 +29,7 @@ libcamera_internal_headers = files([ > > 'ipa_proxy.h', > > 'ipc_pipe.h', > > 'ipc_unixsocket.h', > > + 'layer_manager.h', > > 'mapped_framebuffer.h', > > 'matrix.h', > > 'media_device.h', > > diff --git a/include/libcamera/layer.h b/include/libcamera/layer.h > > new file mode 100644 > > index 000000000000..8fa0ee7d24e6 > > --- /dev/null > > +++ b/include/libcamera/layer.h > > @@ -0,0 +1,56 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board Oy > > + * > > + * Layer interface > > + */ > > + > > +#pragma once > > + > > +#include <set> > > +#include <stdint.h> > > + > > +#include <libcamera/base/span.h> > > + > > +namespace libcamera { > > + > > +class CameraConfiguration; > > +class ControlInfoMap; > > +class ControlList; > > +class FrameBuffer; > > +class Request; > > +class Stream; > > +enum class StreamRole; > > + > > +struct Layer > > +{ > > + const char *name; > > + int layerAPIVersion; > > + > > + void (*init)(const std::string &id); > > I haven't seen it mentioned, sorry if it was; but I think every method here > will definitely need some kind of closure pointer, like a `void *`. Instead of using closure pointers I instantiate one Layer per Camera. That's why there's one LayerManager per Camera. Hm but yes closures would probably make things cleaner, and we wouldn't have to do so many dlopens... > And I think I would separate the vtable from the other fields. Yeah that's probably better. It did feel a bit weird mixing info and vtable. > Or given that this is currently C++ dependent, I think using C++ > virtual might not be not an issue. > > > > + > > + void (*bufferCompleted)(Request *, FrameBuffer *); > > + void (*requestCompleted)(Request *); > > + void (*disconnected)(); > > + > > + void (*acquire)(); > > + void (*release)(); > > + > > + ControlInfoMap::Map (*controls)(ControlInfoMap &); > > + ControlList (*properties)(ControlList &); > > + std::set<Stream *> (*streams)(std::set<Stream *> &); > > + > > + void (*generateConfiguration)(Span<const StreamRole> &, > > + CameraConfiguration *); > > + > > + void (*configure)(CameraConfiguration *); > > + > > + void (*createRequest)(uint64_t, Request *); > > + > > + void (*queueRequest)(Request *); > > + > > + void (*start)(const ControlList *); > > + void (*stop)(); > > +} __attribute__((packed)); > > I am not sure if this needs to be packed? But if it does, could you use > `struct [[gnu::packed]] Layer {` notation? I thought it was because we're dlsyming the struct. > > + > > +} /* namespace libcamera */ > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > index 30ea76f9470a..552af112abb5 100644 > > --- a/include/libcamera/meson.build > > +++ b/include/libcamera/meson.build > > @@ -11,6 +11,7 @@ libcamera_public_headers = files([ > > 'framebuffer.h', > > 'framebuffer_allocator.h', > > 'geometry.h', > > + 'layer.h', > > 'logging.h', > > 'orientation.h', > > 'pixel_format.h', > > diff --git a/src/layer/meson.build b/src/layer/meson.build > > new file mode 100644 > > index 000000000000..dee5e5ac5804 > > --- /dev/null > > +++ b/src/layer/meson.build > > @@ -0,0 +1,10 @@ > > +# SPDX-License-Identifier: CC0-1.0 > > + > > +layer_includes = [ > > + libcamera_includes, > > +] > > + > > +layer_install_dir = libcamera_libdir / 'layers' > > + > > +config_h.set('LAYER_DIR', > > + '"' + get_option('prefix') / layer_install_dir + '"') > > diff --git a/src/libcamera/layer_manager.cpp b/src/libcamera/layer_manager.cpp > > new file mode 100644 > > index 000000000000..96d53d4fc75d > > --- /dev/null > > +++ b/src/libcamera/layer_manager.cpp > > @@ -0,0 +1,314 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board Oy > > + * > > + * Layer manager > > + */ > > + > > +#include "libcamera/internal/layer_manager.h" > > + > > +#include <algorithm> > > +#include <dirent.h> > > +#include <dlfcn.h> > > +#include <map> > > +#include <memory> > > +#include <set> > > +#include <string.h> > > +#include <string> > > +#include <sys/types.h> > > + > > +#include <libcamera/base/file.h> > > +#include <libcamera/base/log.h> > > +#include <libcamera/base/utils.h> > > +#include <libcamera/base/span.h> > > + > > +#include <libcamera/control_ids.h> > > +#include <libcamera/layer.h> > > + > > +#include "libcamera/internal/utils.h" > > + > > +/** > > + * \file layer_manager.h > > + * \brief Layer manager > > + */ > > + > > +namespace libcamera { > > + > > +LOG_DEFINE_CATEGORY(LayerManager) > > + > > +/** > > + * \class LayerManager > > + * \brief Layer manager > > + * > > + * The Layer manager discovers layer implementations from disk, and creates > > + * execution queues for every function that is implemented by each layer and > > + * executes them. A layer is a layer that sits between libcamera and the > > + * application, and hooks into the public Camera interface. > > + */ > > + > > +/** > > + * \brief Construct a LayerManager instance > > + * > > + * The LayerManager class is meant be instantiated by the Camera. > > Is there any issue with loading all desired layers at `CameraManager` > construction time, and instantiating them it for each camera that > is registered? Without closure pointers, yes. > > > > + */ > > +LayerManager::LayerManager() > > +{ > > + std::map<std::string, std::unique_ptr<LayerLoaded>> layers; > > + > > + /* This returns the number of "modules" successfully loaded */ > > + std::function<int(const std::string &)> addDirHandler = > > + [this, &layers](const std::string &file) { > > + std::unique_ptr<LayerManager::LayerLoaded> layer = createLayer(file); > > + if (!layer) > > + return 0; > > + > > + LOG(LayerManager, Debug) << "Loaded layer '" << file << "'"; > > + > > + layers.insert({std::string(layer->layer.name), std::move(layer)}); > > + > > + return 1; > > + }; > > + > > + /* User-specified paths take precedence. */ > > + const char *layerPaths = utils::secure_getenv("LIBCAMERA_LAYER_PATH"); > > Can we please make sure that `LIBCAMERA_LAYER_PATH` can be used in the build > directory as well? Does this not work? > > > > + if (layerPaths) { > > + for (const auto &dir : utils::split(layerPaths, ":")) { > > + if (dir.empty()) > > + continue; > > + > > + utils::addDir(dir.c_str(), 0, addDirHandler); > > + } > > + } > > + > > + /* > > + * When libcamera is used before it is installed, load layers from the > > + * same build directory as the libcamera library itself. > > + */ > > + std::string root = utils::libcameraBuildPath(); > > + if (!root.empty()) { > > + std::string layerBuildPath = root + "src/layer"; > > + constexpr int maxDepth = 2; > > + > > + LOG(LayerManager, Info) > > + << "libcamera is not installed. Adding '" > > + << layerBuildPath << "' to the layer search path"; > > + > > + utils::addDir(layerBuildPath.c_str(), maxDepth, addDirHandler); > > + } > > + > > + /* Finally try to load layers from the installed system path. */ > > + utils::addDir(LAYER_DIR, 0, addDirHandler); > > + > > + /* Order the layers */ > > + /* \todo Document this. First is closer to application, last is closer to libcamera */ > > + const char *layerList = utils::secure_getenv("LIBCAMERA_LAYERS_ENABLE"); > > + if (layerList) { > > + for (const auto &layerName : utils::split(layerList, ":")) { > > + if (layerName.empty()) > > + continue; > > + > > + const auto &it = layers.find(layerName); > > + if (it == layers.end()) > > + continue; > > + > > + executionQueue_.push_back(std::move(it->second)); > > + } > > + } > > Layers still in `layers` leak their DSOs on destruction. Or am I missing something? They are indeed leaked... > > > > +} > > + > > +LayerManager::~LayerManager() > > +{ > > + for (auto &layer : executionQueue_) > > + dlclose(layer->dlHandle); > > This should be handled in the destructor of each individual object in my opinion. Yes. Thanks, Paul > > > > +} > > + > > +std::unique_ptr<LayerManager::LayerLoaded> LayerManager::createLayer(const std::string &filename) > > +{ > > + File file{ filename }; > > + if (!file.open(File::OpenModeFlag::ReadOnly)) { > > + LOG(LayerManager, Error) << "Failed to open layer: " > > + << strerror(-file.error()); > > + return nullptr; > > + } > > + > > + Span<const uint8_t> data = file.map(); > > + int ret = utils::elfVerifyIdent(data); > > + if (ret) { > > + LOG(LayerManager, Error) << "Layer is not an ELF file"; > > + return nullptr; > > + } > > + > > + Span<const uint8_t> info = utils::elfLoadSymbol(data, "layerInfo"); > > + if (info.size() < sizeof(Layer)) { > > + LOG(LayerManager, Error) << "Layer has no valid info"; > > + return nullptr; > > + } > > + > > + void *dlHandle = dlopen(file.fileName().c_str(), RTLD_LAZY); > > + if (!dlHandle) { > > + LOG(LayerManager, Error) > > + << "Failed to open layer shared object: " > > + << dlerror(); > > + return nullptr; > > + } > > + > > + void *symbol = dlsym(dlHandle, "layerInfo"); > > + if (!symbol) { > > + LOG(LayerManager, Error) > > + << "Failed to load layerInfo from layer shared object: " > > + << dlerror(); > > + dlclose(dlHandle); > > + dlHandle = nullptr; > > + return nullptr; > > + } > > + > > + std::unique_ptr<LayerManager::LayerLoaded> layer = > > + std::make_unique<LayerManager::LayerLoaded>(); > > + layer->layer = *reinterpret_cast<Layer *>(symbol); > > + > > + /* \todo Implement this. It should come from the libcamera version */ > > + if (layer->layer.layerAPIVersion != 1) { > > + LOG(LayerManager, Error) << "Layer API version mismatch"; > > + return nullptr; > > + } > > + > > + /* \todo Validate the layer name. */ > > + > > + layer->dlHandle = dlHandle; > > + > > + return layer; > > +} > > [...] > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index 8e2aa921a620..226a94768514 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -40,6 +40,7 @@ libcamera_internal_sources = files([ > > 'ipc_pipe.cpp', > > 'ipc_pipe_unixsocket.cpp', > > 'ipc_unixsocket.cpp', > > + 'layer_manager.cpp', > > 'mapped_framebuffer.cpp', > > 'matrix.cpp', > > 'media_device.cpp', > > diff --git a/src/meson.build b/src/meson.build > > index 8eb8f05b362f..37368b01cbf2 100644 > > --- a/src/meson.build > > +++ b/src/meson.build > > @@ -63,6 +63,7 @@ subdir('libcamera') > > > > subdir('android') > > subdir('ipa') > > +subdir('layer') > > > > subdir('apps') > > > > Regards, > Barnabás Pőcze
Quoting Kieran Bingham (2025-06-26 19:38:59) > Quoting Paul Elder (2025-06-26 10:59:39) > > We want to be able to implement layers in libcamera, which conceptually > > sit in between the Camera class and the application. This can be useful > > for implementing things that don't belong inside the Camera/IPA nor inside > > the application, such as intercepting and translation the AeEnable > > control, or implementing the Sync algorithm. > > > > To achieve this, first add a LayerManager implementation, which searches > > for and loads layers from shared object files, and orchestrates > > executing them. Actually calling into these functions from the Camera > > class will be added in the following patch. > > At some point - I wonder if we need module signatures too to decide > which layers we trust ... or potentially have different levels of > capabilities. > > Particularly important when we start looking at loadable layers that can > 'see' the captured images ... > > My "definitely-doesnt-stream-video-live-to-youtube.so" is sure to > be trustedworthy ... Just don't load "definitely-doesnt-stream-video-live-to-youtube.so" :) But yes I think we'll need some sort of capabilities or signing or something. > > But for now this is a great start on the framework and plumbing! > > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > include/libcamera/internal/layer_manager.h | 74 +++++ > > include/libcamera/internal/meson.build | 1 + > > include/libcamera/layer.h | 56 ++++ > > include/libcamera/meson.build | 1 + > > src/layer/meson.build | 10 + > > src/libcamera/layer_manager.cpp | 314 +++++++++++++++++++++ > > src/libcamera/meson.build | 1 + > > src/meson.build | 1 + > > 8 files changed, 458 insertions(+) > > create mode 100644 include/libcamera/internal/layer_manager.h > > create mode 100644 include/libcamera/layer.h > > create mode 100644 src/layer/meson.build > > create mode 100644 src/libcamera/layer_manager.cpp > > > > diff --git a/include/libcamera/internal/layer_manager.h b/include/libcamera/internal/layer_manager.h > > new file mode 100644 > > index 000000000000..73ccad01bca0 > > --- /dev/null > > +++ b/include/libcamera/internal/layer_manager.h > > @@ -0,0 +1,74 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board Oy > > + * > > + * Layer manager interface > > + */ > > + > > +#pragma once > > + > > +#include <deque> > > +#include <memory> > > +#include <set> > > +#include <string> > > + > > +#include <libcamera/base/log.h> > > +#include <libcamera/base/span.h> > > + > > +#include <libcamera/camera.h> > > +#include <libcamera/controls.h> > > +#include <libcamera/framebuffer.h> > > +#include <libcamera/layer.h> > > +#include <libcamera/request.h> > > +#include <libcamera/stream.h> > > + > > +namespace libcamera { > > + > > +LOG_DECLARE_CATEGORY(LayerManager) > > + > > +class LayerManager > > +{ > > +public: > > + LayerManager(); > > + ~LayerManager(); > > + > > + void bufferCompleted(Request *request, FrameBuffer *buffer); > > + void requestCompleted(Request *request); > > + void disconnected(); > > + > > + void acquire(); > > + void release(); > > + > > + const ControlInfoMap &controls(const ControlInfoMap &controlInfoMap); > > + const ControlList &properties(const ControlList &properties); > > + const std::set<Stream *> &streams(const std::set<Stream *> &streams); > > + > > + void generateConfiguration(Span<const StreamRole> &roles, > > + CameraConfiguration *config); > > + > > + void configure(CameraConfiguration *config); > > + > > + void createRequest(uint64_t cookie, Request *request); > > + > > + void queueRequest(Request *request); > > + > > + void start(const ControlList *controls); > > + void stop(); > > + > > +private: > > + /* Extend the layer with information specific to load-handling */ > > + struct LayerLoaded > > + { > > + Layer layer; > > + void *dlHandle; > > + }; > > + > > + std::unique_ptr<LayerLoaded> createLayer(const std::string &file); > > + std::deque<std::unique_ptr<LayerLoaded>> executionQueue_; > > + > > + ControlInfoMap controlInfoMap_; > > + ControlList properties_; > > + std::set<Stream *> streams_; > > +}; > > + > > +} /* namespace libcamera */ > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > > index 690f5c5ec9f6..20e6c295601f 100644 > > --- a/include/libcamera/internal/meson.build > > +++ b/include/libcamera/internal/meson.build > > @@ -29,6 +29,7 @@ libcamera_internal_headers = files([ > > 'ipa_proxy.h', > > 'ipc_pipe.h', > > 'ipc_unixsocket.h', > > + 'layer_manager.h', > > 'mapped_framebuffer.h', > > 'matrix.h', > > 'media_device.h', > > diff --git a/include/libcamera/layer.h b/include/libcamera/layer.h > > new file mode 100644 > > index 000000000000..8fa0ee7d24e6 > > --- /dev/null > > +++ b/include/libcamera/layer.h > > @@ -0,0 +1,56 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board Oy > > + * > > + * Layer interface > > + */ > > + > > +#pragma once > > + > > +#include <set> > > +#include <stdint.h> > > + > > +#include <libcamera/base/span.h> > > + > > +namespace libcamera { > > + > > +class CameraConfiguration; > > +class ControlInfoMap; > > +class ControlList; > > +class FrameBuffer; > > +class Request; > > +class Stream; > > +enum class StreamRole; > > + > > +struct Layer > > +{ > > + const char *name; > > + int layerAPIVersion; > > + > > + void (*init)(const std::string &id); > > + > > + void (*bufferCompleted)(Request *, FrameBuffer *); > > + void (*requestCompleted)(Request *); > > + void (*disconnected)(); > > + > > + void (*acquire)(); > > + void (*release)(); > > + > > + ControlInfoMap::Map (*controls)(ControlInfoMap &); > > + ControlList (*properties)(ControlList &); > > + std::set<Stream *> (*streams)(std::set<Stream *> &); > > + > > + void (*generateConfiguration)(Span<const StreamRole> &, > > + CameraConfiguration *); > > + > > + void (*configure)(CameraConfiguration *); > > + > > + void (*createRequest)(uint64_t, Request *); > > + > > + void (*queueRequest)(Request *); > > + > > + void (*start)(const ControlList *); > > + void (*stop)(); > > +} __attribute__((packed)); > > I'm curious, Why is this packed ? The ideal reason: we need it for dlsyming it The real reason: I copied it from IPAModule > > > > + > > +} /* namespace libcamera */ > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > index 30ea76f9470a..552af112abb5 100644 > > --- a/include/libcamera/meson.build > > +++ b/include/libcamera/meson.build > > @@ -11,6 +11,7 @@ libcamera_public_headers = files([ > > 'framebuffer.h', > > 'framebuffer_allocator.h', > > 'geometry.h', > > + 'layer.h', > > 'logging.h', > > 'orientation.h', > > 'pixel_format.h', > > diff --git a/src/layer/meson.build b/src/layer/meson.build > > new file mode 100644 > > index 000000000000..dee5e5ac5804 > > --- /dev/null > > +++ b/src/layer/meson.build > > @@ -0,0 +1,10 @@ > > +# SPDX-License-Identifier: CC0-1.0 > > + > > +layer_includes = [ > > + libcamera_includes, > > +] > > + > > +layer_install_dir = libcamera_libdir / 'layers' > > + > > +config_h.set('LAYER_DIR', > > + '"' + get_option('prefix') / layer_install_dir + '"') > > diff --git a/src/libcamera/layer_manager.cpp b/src/libcamera/layer_manager.cpp > > new file mode 100644 > > index 000000000000..96d53d4fc75d > > --- /dev/null > > +++ b/src/libcamera/layer_manager.cpp > > @@ -0,0 +1,314 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board Oy > > + * > > + * Layer manager > > + */ > > + > > +#include "libcamera/internal/layer_manager.h" > > + > > +#include <algorithm> > > +#include <dirent.h> > > +#include <dlfcn.h> > > +#include <map> > > +#include <memory> > > +#include <set> > > +#include <string.h> > > +#include <string> > > +#include <sys/types.h> > > + > > +#include <libcamera/base/file.h> > > +#include <libcamera/base/log.h> > > +#include <libcamera/base/utils.h> > > +#include <libcamera/base/span.h> > > + > > +#include <libcamera/control_ids.h> > > +#include <libcamera/layer.h> > > + > > +#include "libcamera/internal/utils.h" > > + > > +/** > > + * \file layer_manager.h > > + * \brief Layer manager > > + */ > > + > > +namespace libcamera { > > + > > +LOG_DEFINE_CATEGORY(LayerManager) > > + > > +/** > > + * \class LayerManager > > + * \brief Layer manager > > + * > > + * The Layer manager discovers layer implementations from disk, and creates > > + * execution queues for every function that is implemented by each layer and > > + * executes them. A layer is a layer that sits between libcamera and the > > + * application, and hooks into the public Camera interface. > > + */ > > + > > +/** > > + * \brief Construct a LayerManager instance > > + * > > + * The LayerManager class is meant be instantiated by the Camera. > > + */ > > +LayerManager::LayerManager() > > +{ > > + std::map<std::string, std::unique_ptr<LayerLoaded>> layers; > > + > > + /* This returns the number of "modules" successfully loaded */ > > + std::function<int(const std::string &)> addDirHandler = > > + [this, &layers](const std::string &file) { > > + std::unique_ptr<LayerManager::LayerLoaded> layer = createLayer(file); > > + if (!layer) > > + return 0; > > + > > + LOG(LayerManager, Debug) << "Loaded layer '" << file << "'"; > > + > > + layers.insert({std::string(layer->layer.name), std::move(layer)}); > > + > > + return 1; > > + }; > > + > > + /* User-specified paths take precedence. */ > > + const char *layerPaths = utils::secure_getenv("LIBCAMERA_LAYER_PATH"); > > + if (layerPaths) { > > + for (const auto &dir : utils::split(layerPaths, ":")) { > > + if (dir.empty()) > > + continue; > > + > > + utils::addDir(dir.c_str(), 0, addDirHandler); > > + } > > + } > > + > > + /* > > + * When libcamera is used before it is installed, load layers from the > > + * same build directory as the libcamera library itself. > > + */ > > + std::string root = utils::libcameraBuildPath(); > > + if (!root.empty()) { > > + std::string layerBuildPath = root + "src/layer"; > > + constexpr int maxDepth = 2; > > + > > + LOG(LayerManager, Info) > > + << "libcamera is not installed. Adding '" > > + << layerBuildPath << "' to the layer search path"; > > + > > + utils::addDir(layerBuildPath.c_str(), maxDepth, addDirHandler); > > + } > > + > > + /* Finally try to load layers from the installed system path. */ > > + utils::addDir(LAYER_DIR, 0, addDirHandler); > > + > > + /* Order the layers */ > > + /* \todo Document this. First is closer to application, last is closer to libcamera */ > > + const char *layerList = utils::secure_getenv("LIBCAMERA_LAYERS_ENABLE"); > > + if (layerList) { > > + for (const auto &layerName : utils::split(layerList, ":")) { > > + if (layerName.empty()) > > + continue; > > + > > + const auto &it = layers.find(layerName); > > + if (it == layers.end()) > > + continue; > > + > > + executionQueue_.push_back(std::move(it->second)); > > + } > > + } > > +} > > + > > +LayerManager::~LayerManager() > > +{ > > + for (auto &layer : executionQueue_) > > + dlclose(layer->dlHandle); > > +} > > + > > +std::unique_ptr<LayerManager::LayerLoaded> LayerManager::createLayer(const std::string &filename) > > +{ > > + File file{ filename }; > > + if (!file.open(File::OpenModeFlag::ReadOnly)) { > > + LOG(LayerManager, Error) << "Failed to open layer: " > > + << strerror(-file.error()); > > + return nullptr; > > + } > > + > > + Span<const uint8_t> data = file.map(); > > + int ret = utils::elfVerifyIdent(data); > > + if (ret) { > > + LOG(LayerManager, Error) << "Layer is not an ELF file"; > > + return nullptr; > > + } > > + > > + Span<const uint8_t> info = utils::elfLoadSymbol(data, "layerInfo"); > > + if (info.size() < sizeof(Layer)) { > > + LOG(LayerManager, Error) << "Layer has no valid info"; > > + return nullptr; > > + } > > + > > + void *dlHandle = dlopen(file.fileName().c_str(), RTLD_LAZY); > > + if (!dlHandle) { > > + LOG(LayerManager, Error) > > + << "Failed to open layer shared object: " > > + << dlerror(); > > + return nullptr; > > + } > > + > > + void *symbol = dlsym(dlHandle, "layerInfo"); > > + if (!symbol) { > > + LOG(LayerManager, Error) > > + << "Failed to load layerInfo from layer shared object: " > > + << dlerror(); > > + dlclose(dlHandle); > > + dlHandle = nullptr; > > + return nullptr; > > + } > > + > > + std::unique_ptr<LayerManager::LayerLoaded> layer = > > + std::make_unique<LayerManager::LayerLoaded>(); > > + layer->layer = *reinterpret_cast<Layer *>(symbol); > > + > > + /* \todo Implement this. It should come from the libcamera version */ > > + if (layer->layer.layerAPIVersion != 1) { > > + LOG(LayerManager, Error) << "Layer API version mismatch"; > > + return nullptr; > > + } > > + > > + /* \todo Validate the layer name. */ > > + > > + layer->dlHandle = dlHandle; > > + > > + return layer; > > +} > > + > > +void LayerManager::bufferCompleted(Request *request, FrameBuffer *buffer) > > +{ > > + /* Reverse order because this comes from a Signal emission */ > > + for (auto it = executionQueue_.rbegin(); > > + it != executionQueue_.rend(); it++) { > > + if ((*it)->layer.bufferCompleted) > > + (*it)->layer.bufferCompleted(request, buffer); > > + } > > +} > > + > > +void LayerManager::requestCompleted(Request *request) > > +{ > > + /* Reverse order because this comes from a Signal emission */ > > + for (auto it = executionQueue_.rbegin(); > > + it != executionQueue_.rend(); it++) { > > + if ((*it)->layer.requestCompleted) > > + (*it)->layer.requestCompleted(request); > > + } > > +} > > + > > +void LayerManager::disconnected() > > +{ > > + /* Reverse order because this comes from a Signal emission */ > > + for (auto it = executionQueue_.rbegin(); > > + it != executionQueue_.rend(); it++) { > > + if ((*it)->layer.disconnected) > > + (*it)->layer.disconnected(); > > + } > > +} > > + > > +void LayerManager::acquire() > > +{ > > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > > + if (layer->layer.acquire) > > + layer->layer.acquire(); > > +} > > + > > +void LayerManager::release() > > +{ > > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > > + if (layer->layer.release) > > + layer->layer.release(); > > +} > > + > > +const ControlInfoMap &LayerManager::controls(const ControlInfoMap &controlInfoMap) > > +{ > > I'm a bit surprised here actually - > > What happens if the applications calls Camera->controls() multiple > times here? > > I don't think I would manipulate controls in this call - rather I would > suspect we should have a LayerManager::init(ControlInfoMap > &controlInfoMap) which would be called after the IPA has initialised > it's controls into the same map ... Most IPAs have updateControls() that trigger on configure()... so I thought we need to run this multiple times... Ok I guess we can generate it on init() and configure() then. > Then there would be no indirection with LayerManager::controls() that's > simply returns the single configured map. Yeah. > > > + controlInfoMap_ = controlInfoMap; > > + > > + /* \todo Simplify this once ControlInfoMaps become easier to modify */ > > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) { > > + if (layer->layer.controls) { > > + ControlInfoMap::Map ret = layer->layer.controls(controlInfoMap_); > > + ControlInfoMap::Map map; > > + /* Merge the layer's ret later so that layers can overwrite */ > > + for (auto &pair : controlInfoMap_) > > + map.insert(pair); > > + for (auto &pair : ret) > > + map.insert(pair); > > + controlInfoMap_ = ControlInfoMap(std::move(map), > > + libcamera::controls::controls); > > + } > > + } > > + return controlInfoMap_; > > +} > > + > > +const ControlList &LayerManager::properties(const ControlList &properties) > > +{ > > Similar here I think - this should probably just happen 'once' at > ::init() and leave the accessor to return the const data without > modifying it? Yes. > > > + properties_ = properties; > > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) { > > + if (layer->layer.properties) { > > + ControlList ret = layer->layer.properties(properties_); > > + properties_.merge(ret, ControlList::MergePolicy::OverwriteExisting); > > + } > > + } > > + return properties_; > > +} > > + > > +const std::set<Stream *> &LayerManager::streams(const std::set<Stream *> &streams) > > +{ > > + streams_ = streams; > > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) { > > + if (layer->layer.streams) { > > + std::set<Stream *> ret = layer->layer.streams(streams_); > > + streams_.insert(ret.begin(), ret.end()); > > + } > > + } > > + return streams_; > > Ohhhh I was about to ask why would we have streams ... but indeed I > could imagine a layer that could generate processed streams ... > > Eeek though for again with the modifying 'const' state. That's what controls() and properties() does too... and we at least need the former for AeEnable/FPS and Sync... > > > > +} > > + > > +void LayerManager::generateConfiguration(Span<const StreamRole> &roles, > > + CameraConfiguration *config) > > +{ > > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > > + if (layer->layer.generateConfiguration) > > + layer->layer.generateConfiguration(roles, config); > > I don't think we can have generateConfiguration without validate > configuration ... Good point. > Also - maybe we should consider extra stream handling 'later', as both > this and streams() is going to be tricky. Layers are going to do > configuration validation and see extra streams that they don't know > about and they'll just chop them out of the config. So I'm not sure how > these parts could work yet... > > > > +} > > + > > +void LayerManager::configure(CameraConfiguration *config) > > +{ > > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > > + if (layer->layer.configure) > > + layer->layer.configure(config); > > I think this is valid info for a layer to be told though so this is a > good hook. > > > +} > > + > > +void LayerManager::createRequest(uint64_t cookie, Request *request) > > +{ > > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > > + if (layer->layer.createRequest) > > + layer->layer.createRequest(cookie, request); > > I'm not sure I'd hook this - What would a layer do when creating a > request ? Or perhaps it's so they can store some private data or a map ? That was the idea. > but I suspect cookie and request should be const here if so. True. > Unless layers could replace requests on the way in an back out :S Ok maybe we don't quite want that. Thanks, Paul > > > +} > > + > > +void LayerManager::queueRequest(Request *request) > > +{ > > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > > + if (layer->layer.queueRequest) > > + layer->layer.queueRequest(request); > > +} > > + > > +void LayerManager::start(const ControlList *controls) > > +{ > > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > > + if (layer->layer.start) > > + layer->layer.start(controls); > > +} > > + > > +void LayerManager::stop() > > +{ > > + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) > > + if (layer->layer.stop) > > + layer->layer.stop(); > > +} > > + > > +} /* namespace libcamera */ > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index 8e2aa921a620..226a94768514 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -40,6 +40,7 @@ libcamera_internal_sources = files([ > > 'ipc_pipe.cpp', > > 'ipc_pipe_unixsocket.cpp', > > 'ipc_unixsocket.cpp', > > + 'layer_manager.cpp', > > 'mapped_framebuffer.cpp', > > 'matrix.cpp', > > 'media_device.cpp', > > diff --git a/src/meson.build b/src/meson.build > > index 8eb8f05b362f..37368b01cbf2 100644 > > --- a/src/meson.build > > +++ b/src/meson.build > > @@ -63,6 +63,7 @@ subdir('libcamera') > > > > subdir('android') > > subdir('ipa') > > +subdir('layer') > > > > subdir('apps') > > > > -- > > 2.47.2 > >
Hi 2025. 06. 27. 10:36 keltezéssel, Paul Elder írta: > Hi Barnabás, > > Thanks for the review. > > Quoting Barnabás Pőcze (2025-06-26 19:38:49) >> Hi >> >> 2025. 06. 26. 11:59 keltezéssel, Paul Elder írta: >>> We want to be able to implement layers in libcamera, which conceptually >>> sit in between the Camera class and the application. This can be useful >>> for implementing things that don't belong inside the Camera/IPA nor inside >>> the application, such as intercepting and translation the AeEnable >>> control, or implementing the Sync algorithm. >>> >>> To achieve this, first add a LayerManager implementation, which searches >>> for and loads layers from shared object files, and orchestrates >>> executing them. Actually calling into these functions from the Camera >>> class will be added in the following patch. >>> >>> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> >>> --- >>> include/libcamera/internal/layer_manager.h | 74 +++++ >>> include/libcamera/internal/meson.build | 1 + >>> include/libcamera/layer.h | 56 ++++ >>> include/libcamera/meson.build | 1 + >>> src/layer/meson.build | 10 + >>> src/libcamera/layer_manager.cpp | 314 +++++++++++++++++++++ >>> src/libcamera/meson.build | 1 + >>> src/meson.build | 1 + >>> 8 files changed, 458 insertions(+) >>> create mode 100644 include/libcamera/internal/layer_manager.h >>> create mode 100644 include/libcamera/layer.h >>> create mode 100644 src/layer/meson.build >>> create mode 100644 src/libcamera/layer_manager.cpp >>> > [...] >>> diff --git a/include/libcamera/layer.h b/include/libcamera/layer.h >>> new file mode 100644 >>> index 000000000000..8fa0ee7d24e6 >>> --- /dev/null >>> +++ b/include/libcamera/layer.h >>> @@ -0,0 +1,56 @@ >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >>> +/* >>> + * Copyright (C) 2025, Ideas On Board Oy >>> + * >>> + * Layer interface >>> + */ >>> + >>> +#pragma once >>> + >>> +#include <set> >>> +#include <stdint.h> >>> + >>> +#include <libcamera/base/span.h> >>> + >>> +namespace libcamera { >>> + >>> +class CameraConfiguration; >>> +class ControlInfoMap; >>> +class ControlList; >>> +class FrameBuffer; >>> +class Request; >>> +class Stream; >>> +enum class StreamRole; >>> + >>> +struct Layer >>> +{ >>> + const char *name; >>> + int layerAPIVersion; >>> + >>> + void (*init)(const std::string &id); >> >> I haven't seen it mentioned, sorry if it was; but I think every method here >> will definitely need some kind of closure pointer, like a `void *`. > > Instead of using closure pointers I instantiate one Layer per Camera. That's > why there's one LayerManager per Camera. > > Hm but yes closures would probably make things cleaner, and we wouldn't have to > do so many dlopens... Yes, but dlopen()ing the same DSO in the same namespace will just return the same handle. So using static variables in the layer wouldn't work with multiple cameras. So I think this is pretty much required? > >> And I think I would separate the vtable from the other fields. > > Yeah that's probably better. It did feel a bit weird mixing info and vtable. > >> Or given that this is currently C++ dependent, I think using C++ >> virtual might not be not an issue. >> >> >>> + >>> + void (*bufferCompleted)(Request *, FrameBuffer *); >>> + void (*requestCompleted)(Request *); >>> + void (*disconnected)(); >>> + >>> + void (*acquire)(); >>> + void (*release)(); >>> + >>> + ControlInfoMap::Map (*controls)(ControlInfoMap &); >>> + ControlList (*properties)(ControlList &); >>> + std::set<Stream *> (*streams)(std::set<Stream *> &); >>> + >>> + void (*generateConfiguration)(Span<const StreamRole> &, >>> + CameraConfiguration *); >>> + >>> + void (*configure)(CameraConfiguration *); >>> + >>> + void (*createRequest)(uint64_t, Request *); >>> + >>> + void (*queueRequest)(Request *); >>> + >>> + void (*start)(const ControlList *); >>> + void (*stop)(); >>> +} __attribute__((packed)); >> >> I am not sure if this needs to be packed? But if it does, could you use >> `struct [[gnu::packed]] Layer {` notation? > > I thought it was because we're dlsyming the struct. Then I don't think that's needed. > >>> + >>> +} /* namespace libcamera */ > [...] >>> +LayerManager::LayerManager() >>> +{ >>> + std::map<std::string, std::unique_ptr<LayerLoaded>> layers; >>> + >>> + /* This returns the number of "modules" successfully loaded */ >>> + std::function<int(const std::string &)> addDirHandler = >>> + [this, &layers](const std::string &file) { >>> + std::unique_ptr<LayerManager::LayerLoaded> layer = createLayer(file); >>> + if (!layer) >>> + return 0; >>> + >>> + LOG(LayerManager, Debug) << "Loaded layer '" << file << "'"; >>> + >>> + layers.insert({std::string(layer->layer.name), std::move(layer)}); >>> + >>> + return 1; >>> + }; >>> + >>> + /* User-specified paths take precedence. */ >>> + const char *layerPaths = utils::secure_getenv("LIBCAMERA_LAYER_PATH"); >> >> Can we please make sure that `LIBCAMERA_LAYER_PATH` can be used in the build >> directory as well? > > Does this not work? I think you'll have to modify 0 -> 1 in the `addDir()` call since in the build dir each DSO is in its own folder (as far as I can see, not tested, sorry). > [...] Regards, Barnabás Pőcze
diff --git a/include/libcamera/internal/layer_manager.h b/include/libcamera/internal/layer_manager.h new file mode 100644 index 000000000000..73ccad01bca0 --- /dev/null +++ b/include/libcamera/internal/layer_manager.h @@ -0,0 +1,74 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas On Board Oy + * + * Layer manager interface + */ + +#pragma once + +#include <deque> +#include <memory> +#include <set> +#include <string> + +#include <libcamera/base/log.h> +#include <libcamera/base/span.h> + +#include <libcamera/camera.h> +#include <libcamera/controls.h> +#include <libcamera/framebuffer.h> +#include <libcamera/layer.h> +#include <libcamera/request.h> +#include <libcamera/stream.h> + +namespace libcamera { + +LOG_DECLARE_CATEGORY(LayerManager) + +class LayerManager +{ +public: + LayerManager(); + ~LayerManager(); + + void bufferCompleted(Request *request, FrameBuffer *buffer); + void requestCompleted(Request *request); + void disconnected(); + + void acquire(); + void release(); + + const ControlInfoMap &controls(const ControlInfoMap &controlInfoMap); + const ControlList &properties(const ControlList &properties); + const std::set<Stream *> &streams(const std::set<Stream *> &streams); + + void generateConfiguration(Span<const StreamRole> &roles, + CameraConfiguration *config); + + void configure(CameraConfiguration *config); + + void createRequest(uint64_t cookie, Request *request); + + void queueRequest(Request *request); + + void start(const ControlList *controls); + void stop(); + +private: + /* Extend the layer with information specific to load-handling */ + struct LayerLoaded + { + Layer layer; + void *dlHandle; + }; + + std::unique_ptr<LayerLoaded> createLayer(const std::string &file); + std::deque<std::unique_ptr<LayerLoaded>> executionQueue_; + + ControlInfoMap controlInfoMap_; + ControlList properties_; + std::set<Stream *> streams_; +}; + +} /* namespace libcamera */ diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index 690f5c5ec9f6..20e6c295601f 100644 --- a/include/libcamera/internal/meson.build +++ b/include/libcamera/internal/meson.build @@ -29,6 +29,7 @@ libcamera_internal_headers = files([ 'ipa_proxy.h', 'ipc_pipe.h', 'ipc_unixsocket.h', + 'layer_manager.h', 'mapped_framebuffer.h', 'matrix.h', 'media_device.h', diff --git a/include/libcamera/layer.h b/include/libcamera/layer.h new file mode 100644 index 000000000000..8fa0ee7d24e6 --- /dev/null +++ b/include/libcamera/layer.h @@ -0,0 +1,56 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas On Board Oy + * + * Layer interface + */ + +#pragma once + +#include <set> +#include <stdint.h> + +#include <libcamera/base/span.h> + +namespace libcamera { + +class CameraConfiguration; +class ControlInfoMap; +class ControlList; +class FrameBuffer; +class Request; +class Stream; +enum class StreamRole; + +struct Layer +{ + const char *name; + int layerAPIVersion; + + void (*init)(const std::string &id); + + void (*bufferCompleted)(Request *, FrameBuffer *); + void (*requestCompleted)(Request *); + void (*disconnected)(); + + void (*acquire)(); + void (*release)(); + + ControlInfoMap::Map (*controls)(ControlInfoMap &); + ControlList (*properties)(ControlList &); + std::set<Stream *> (*streams)(std::set<Stream *> &); + + void (*generateConfiguration)(Span<const StreamRole> &, + CameraConfiguration *); + + void (*configure)(CameraConfiguration *); + + void (*createRequest)(uint64_t, Request *); + + void (*queueRequest)(Request *); + + void (*start)(const ControlList *); + void (*stop)(); +} __attribute__((packed)); + +} /* namespace libcamera */ diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index 30ea76f9470a..552af112abb5 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -11,6 +11,7 @@ libcamera_public_headers = files([ 'framebuffer.h', 'framebuffer_allocator.h', 'geometry.h', + 'layer.h', 'logging.h', 'orientation.h', 'pixel_format.h', diff --git a/src/layer/meson.build b/src/layer/meson.build new file mode 100644 index 000000000000..dee5e5ac5804 --- /dev/null +++ b/src/layer/meson.build @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: CC0-1.0 + +layer_includes = [ + libcamera_includes, +] + +layer_install_dir = libcamera_libdir / 'layers' + +config_h.set('LAYER_DIR', + '"' + get_option('prefix') / layer_install_dir + '"') diff --git a/src/libcamera/layer_manager.cpp b/src/libcamera/layer_manager.cpp new file mode 100644 index 000000000000..96d53d4fc75d --- /dev/null +++ b/src/libcamera/layer_manager.cpp @@ -0,0 +1,314 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas On Board Oy + * + * Layer manager + */ + +#include "libcamera/internal/layer_manager.h" + +#include <algorithm> +#include <dirent.h> +#include <dlfcn.h> +#include <map> +#include <memory> +#include <set> +#include <string.h> +#include <string> +#include <sys/types.h> + +#include <libcamera/base/file.h> +#include <libcamera/base/log.h> +#include <libcamera/base/utils.h> +#include <libcamera/base/span.h> + +#include <libcamera/control_ids.h> +#include <libcamera/layer.h> + +#include "libcamera/internal/utils.h" + +/** + * \file layer_manager.h + * \brief Layer manager + */ + +namespace libcamera { + +LOG_DEFINE_CATEGORY(LayerManager) + +/** + * \class LayerManager + * \brief Layer manager + * + * The Layer manager discovers layer implementations from disk, and creates + * execution queues for every function that is implemented by each layer and + * executes them. A layer is a layer that sits between libcamera and the + * application, and hooks into the public Camera interface. + */ + +/** + * \brief Construct a LayerManager instance + * + * The LayerManager class is meant be instantiated by the Camera. + */ +LayerManager::LayerManager() +{ + std::map<std::string, std::unique_ptr<LayerLoaded>> layers; + + /* This returns the number of "modules" successfully loaded */ + std::function<int(const std::string &)> addDirHandler = + [this, &layers](const std::string &file) { + std::unique_ptr<LayerManager::LayerLoaded> layer = createLayer(file); + if (!layer) + return 0; + + LOG(LayerManager, Debug) << "Loaded layer '" << file << "'"; + + layers.insert({std::string(layer->layer.name), std::move(layer)}); + + return 1; + }; + + /* User-specified paths take precedence. */ + const char *layerPaths = utils::secure_getenv("LIBCAMERA_LAYER_PATH"); + if (layerPaths) { + for (const auto &dir : utils::split(layerPaths, ":")) { + if (dir.empty()) + continue; + + utils::addDir(dir.c_str(), 0, addDirHandler); + } + } + + /* + * When libcamera is used before it is installed, load layers from the + * same build directory as the libcamera library itself. + */ + std::string root = utils::libcameraBuildPath(); + if (!root.empty()) { + std::string layerBuildPath = root + "src/layer"; + constexpr int maxDepth = 2; + + LOG(LayerManager, Info) + << "libcamera is not installed. Adding '" + << layerBuildPath << "' to the layer search path"; + + utils::addDir(layerBuildPath.c_str(), maxDepth, addDirHandler); + } + + /* Finally try to load layers from the installed system path. */ + utils::addDir(LAYER_DIR, 0, addDirHandler); + + /* Order the layers */ + /* \todo Document this. First is closer to application, last is closer to libcamera */ + const char *layerList = utils::secure_getenv("LIBCAMERA_LAYERS_ENABLE"); + if (layerList) { + for (const auto &layerName : utils::split(layerList, ":")) { + if (layerName.empty()) + continue; + + const auto &it = layers.find(layerName); + if (it == layers.end()) + continue; + + executionQueue_.push_back(std::move(it->second)); + } + } +} + +LayerManager::~LayerManager() +{ + for (auto &layer : executionQueue_) + dlclose(layer->dlHandle); +} + +std::unique_ptr<LayerManager::LayerLoaded> LayerManager::createLayer(const std::string &filename) +{ + File file{ filename }; + if (!file.open(File::OpenModeFlag::ReadOnly)) { + LOG(LayerManager, Error) << "Failed to open layer: " + << strerror(-file.error()); + return nullptr; + } + + Span<const uint8_t> data = file.map(); + int ret = utils::elfVerifyIdent(data); + if (ret) { + LOG(LayerManager, Error) << "Layer is not an ELF file"; + return nullptr; + } + + Span<const uint8_t> info = utils::elfLoadSymbol(data, "layerInfo"); + if (info.size() < sizeof(Layer)) { + LOG(LayerManager, Error) << "Layer has no valid info"; + return nullptr; + } + + void *dlHandle = dlopen(file.fileName().c_str(), RTLD_LAZY); + if (!dlHandle) { + LOG(LayerManager, Error) + << "Failed to open layer shared object: " + << dlerror(); + return nullptr; + } + + void *symbol = dlsym(dlHandle, "layerInfo"); + if (!symbol) { + LOG(LayerManager, Error) + << "Failed to load layerInfo from layer shared object: " + << dlerror(); + dlclose(dlHandle); + dlHandle = nullptr; + return nullptr; + } + + std::unique_ptr<LayerManager::LayerLoaded> layer = + std::make_unique<LayerManager::LayerLoaded>(); + layer->layer = *reinterpret_cast<Layer *>(symbol); + + /* \todo Implement this. It should come from the libcamera version */ + if (layer->layer.layerAPIVersion != 1) { + LOG(LayerManager, Error) << "Layer API version mismatch"; + return nullptr; + } + + /* \todo Validate the layer name. */ + + layer->dlHandle = dlHandle; + + return layer; +} + +void LayerManager::bufferCompleted(Request *request, FrameBuffer *buffer) +{ + /* Reverse order because this comes from a Signal emission */ + for (auto it = executionQueue_.rbegin(); + it != executionQueue_.rend(); it++) { + if ((*it)->layer.bufferCompleted) + (*it)->layer.bufferCompleted(request, buffer); + } +} + +void LayerManager::requestCompleted(Request *request) +{ + /* Reverse order because this comes from a Signal emission */ + for (auto it = executionQueue_.rbegin(); + it != executionQueue_.rend(); it++) { + if ((*it)->layer.requestCompleted) + (*it)->layer.requestCompleted(request); + } +} + +void LayerManager::disconnected() +{ + /* Reverse order because this comes from a Signal emission */ + for (auto it = executionQueue_.rbegin(); + it != executionQueue_.rend(); it++) { + if ((*it)->layer.disconnected) + (*it)->layer.disconnected(); + } +} + +void LayerManager::acquire() +{ + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) + if (layer->layer.acquire) + layer->layer.acquire(); +} + +void LayerManager::release() +{ + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) + if (layer->layer.release) + layer->layer.release(); +} + +const ControlInfoMap &LayerManager::controls(const ControlInfoMap &controlInfoMap) +{ + controlInfoMap_ = controlInfoMap; + + /* \todo Simplify this once ControlInfoMaps become easier to modify */ + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) { + if (layer->layer.controls) { + ControlInfoMap::Map ret = layer->layer.controls(controlInfoMap_); + ControlInfoMap::Map map; + /* Merge the layer's ret later so that layers can overwrite */ + for (auto &pair : controlInfoMap_) + map.insert(pair); + for (auto &pair : ret) + map.insert(pair); + controlInfoMap_ = ControlInfoMap(std::move(map), + libcamera::controls::controls); + } + } + return controlInfoMap_; +} + +const ControlList &LayerManager::properties(const ControlList &properties) +{ + properties_ = properties; + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) { + if (layer->layer.properties) { + ControlList ret = layer->layer.properties(properties_); + properties_.merge(ret, ControlList::MergePolicy::OverwriteExisting); + } + } + return properties_; +} + +const std::set<Stream *> &LayerManager::streams(const std::set<Stream *> &streams) +{ + streams_ = streams; + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) { + if (layer->layer.streams) { + std::set<Stream *> ret = layer->layer.streams(streams_); + streams_.insert(ret.begin(), ret.end()); + } + } + return streams_; +} + +void LayerManager::generateConfiguration(Span<const StreamRole> &roles, + CameraConfiguration *config) +{ + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) + if (layer->layer.generateConfiguration) + layer->layer.generateConfiguration(roles, config); +} + +void LayerManager::configure(CameraConfiguration *config) +{ + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) + if (layer->layer.configure) + layer->layer.configure(config); +} + +void LayerManager::createRequest(uint64_t cookie, Request *request) +{ + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) + if (layer->layer.createRequest) + layer->layer.createRequest(cookie, request); +} + +void LayerManager::queueRequest(Request *request) +{ + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) + if (layer->layer.queueRequest) + layer->layer.queueRequest(request); +} + +void LayerManager::start(const ControlList *controls) +{ + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) + if (layer->layer.start) + layer->layer.start(controls); +} + +void LayerManager::stop() +{ + for (std::unique_ptr<LayerManager::LayerLoaded> &layer : executionQueue_) + if (layer->layer.stop) + layer->layer.stop(); +} + +} /* namespace libcamera */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 8e2aa921a620..226a94768514 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -40,6 +40,7 @@ libcamera_internal_sources = files([ 'ipc_pipe.cpp', 'ipc_pipe_unixsocket.cpp', 'ipc_unixsocket.cpp', + 'layer_manager.cpp', 'mapped_framebuffer.cpp', 'matrix.cpp', 'media_device.cpp', diff --git a/src/meson.build b/src/meson.build index 8eb8f05b362f..37368b01cbf2 100644 --- a/src/meson.build +++ b/src/meson.build @@ -63,6 +63,7 @@ subdir('libcamera') subdir('android') subdir('ipa') +subdir('layer') subdir('apps')
We want to be able to implement layers in libcamera, which conceptually sit in between the Camera class and the application. This can be useful for implementing things that don't belong inside the Camera/IPA nor inside the application, such as intercepting and translation the AeEnable control, or implementing the Sync algorithm. To achieve this, first add a LayerManager implementation, which searches for and loads layers from shared object files, and orchestrates executing them. Actually calling into these functions from the Camera class will be added in the following patch. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- include/libcamera/internal/layer_manager.h | 74 +++++ include/libcamera/internal/meson.build | 1 + include/libcamera/layer.h | 56 ++++ include/libcamera/meson.build | 1 + src/layer/meson.build | 10 + src/libcamera/layer_manager.cpp | 314 +++++++++++++++++++++ src/libcamera/meson.build | 1 + src/meson.build | 1 + 8 files changed, 458 insertions(+) create mode 100644 include/libcamera/internal/layer_manager.h create mode 100644 include/libcamera/layer.h create mode 100644 src/layer/meson.build create mode 100644 src/libcamera/layer_manager.cpp