[{"id":34780,"web_url":"https://patchwork.libcamera.org/comment/34780/","msgid":"<175159622338.2127280.3041812220180984712@neptunite.rasen.tech>","date":"2025-07-04T02:30:23","subject":"Re: [PATCH v2 4/8] libcamera: layer_manager: Add LayerManager\n\timplementation","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi me,\n\nQuoting Paul Elder (2025-07-03 20:42:19)\n> We want to be able to implement layers in libcamera, which conceptually\n> sit in between the Camera class and the application. This can be useful\n> for implementing things that don't belong inside the Camera/IPA nor inside\n> the application, such as intercepting and translation the AeEnable\n> control, or implementing the Sync algorithm.\n> \n> To achieve this, first add a LayerManager implementation, which searches\n> for and loads layers from shared object files, and orchestrates\n> executing them. Actually calling into these functions from the Camera\n> class will be added in the following patch.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v2:\n> - add closures to the layer interface\n> - separate Layer into LayerInfo and LayerInterface, to separate out\n> layer data and the vtable\n> - remove generateConfiguration() and streams() from the layer interface\n> - add init() and terminate() to the layer interface, and make them\n> required (mainly for preparing and destroying the closure)\n> - make LayerLoaded automatically dlclose on deconstruction, remove copy,\n> and implement move constructors\n> - cache controls and properties in the LayerManager\n> ---\n> include/libcamera/internal/layer_manager.h | 117 +++++++\n> include/libcamera/internal/meson.build | 1 +\n> include/libcamera/layer.h | 54 +++\n> include/libcamera/meson.build | 1 +\n> src/layer/meson.build | 10 +\n> src/libcamera/layer_manager.cpp | 383 +++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> src/meson.build | 1 +\n> 8 files changed, 568 insertions(+)\n> create mode 100644 include/libcamera/internal/layer_manager.h\n> create mode 100644 include/libcamera/layer.h\n> create mode 100644 src/layer/meson.build\n> create mode 100644 src/libcamera/layer_manager.cpp\n> \n> diff --git a/include/libcamera/internal/layer_manager.h b/include/libcamera/internal/layer_manager.h\n> new file mode 100644\n> index 000000000000..0d108bcddf3d\n> --- /dev/null\n> +++ b/include/libcamera/internal/layer_manager.h\n> @@ -0,0 +1,117 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + *\n> + * Layer manager interface\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(LayerManager)\n> +\n> +class LayerManager\n> +{\n> +public:\n> + LayerManager();\n> + ~LayerManager() = default;\n> +\n> + void init(const Camera *camera, const ControlList &properties,\n> + const ControlInfoMap &controlInfoMap);\n> + void terminate(const Camera *camera);\n> +\n> + void bufferCompleted(const Camera *camera,\n> + Request *request, FrameBuffer *buffer);\n> + void requestCompleted(const Camera *camera, Request *request);\n> + void disconnected(const Camera *camera);\n> +\n> + void acquire(const Camera *camera);\n> + void release(const Camera *camera);\n> +\n> + const ControlInfoMap &controls(const Camera *camera) const { return controls_.at(camera); }\n> + const ControlList &properties(const Camera *camera) const { return properties_.at(camera); }\n> +\n> + void configure(const Camera *camera, const CameraConfiguration *config,\n> + const ControlInfoMap &controlInfoMap);\n> +\n> + void createRequest(const Camera *camera,\n> + uint64_t cookie, const Request *request);\n> +\n> + void queueRequest(const Camera *camera, Request *request);\n> +\n> + void start(const Camera *camera, const ControlList *controls);\n\ncontrols should probably be non-const, otherwise layers won't be able to\nintercept and modify it.\n\n> + void stop(const Camera *camera);\n> +\n> +private:\n> + /* Extend the layer with information specific to load-handling */\n> + struct LayerLoaded\n> + {\n> + LayerLoaded()\n> + : info(nullptr), vtable(nullptr), dlHandle(nullptr)\n> + {\n> + }\n> +\n> + LayerLoaded(LayerLoaded &&other)\n> + : info(other.info), vtable(other.vtable),\n> + dlHandle(other.dlHandle)\n> + {\n> + other.dlHandle = nullptr;\n> + }\n> +\n> + LayerLoaded &operator=(LayerLoaded &&other)\n> + {\n> + info = other.info;\n> + vtable = other.vtable;\n> + dlHandle = other.dlHandle;\n> + other.dlHandle = nullptr;\n> + return *this;\n> + }\n> +\n> + ~LayerLoaded()\n> + {\n> + if (dlHandle)\n> + dlclose(dlHandle);\n> + }\n> +\n> + LayerInfo *info;\n> + LayerInterface *vtable;\n> + void *dlHandle;\n> +\n> + private:\n> + LIBCAMERA_DISABLE_COPY(LayerLoaded)\n> + };\n> +\n> + using ClosureKey = std::tuple;\n> +\n> + void updateProperties(const Camera *camera,\n> + const ControlList &properties);\n> + void updateControls(const Camera *camera,\n> + const ControlInfoMap &controlInfoMap);\n> +\n> + LayerLoaded createLayer(const std::string &file);\n> + std::deque executionQueue_;\n> + std::map closures_;\n> +\n> + std::map controls_;\n> + std::map properties_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 690f5c5ec9f6..20e6c295601f 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -29,6 +29,7 @@ libcamera_internal_headers = files([\n> 'ipa_proxy.h',\n> 'ipc_pipe.h',\n> 'ipc_unixsocket.h',\n> + 'layer_manager.h',\n> 'mapped_framebuffer.h',\n> 'matrix.h',\n> 'media_device.h',\n> diff --git a/include/libcamera/layer.h b/include/libcamera/layer.h\n> new file mode 100644\n> index 000000000000..cd0e26a3b72b\n> --- /dev/null\n> +++ b/include/libcamera/layer.h\n> @@ -0,0 +1,54 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + *\n> + * Layer interface\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class CameraConfiguration;\n> +class FrameBuffer;\n> +class Request;\n> +class Stream;\n> +enum class StreamRole;\n> +\n> +struct LayerInfo {\n> + const char *name;\n> + int layerAPIVersion;\n> +};\n> +\n> +struct LayerInterface {\n> + void *(*init)(const std::string &id);\n> + void (*terminate)(void *);\n> +\n> + void (*bufferCompleted)(void *, Request *, FrameBuffer *);\n> + void (*requestCompleted)(void *, Request *);\n> + void (*disconnected)(void *);\n> +\n> + void (*acquire)(void *);\n> + void (*release)(void *);\n> +\n> + ControlInfoMap::Map (*controls)(void *, ControlInfoMap &);\n> + ControlList (*properties)(void *, ControlList &);\n> +\n> + void (*configure)(void *, const CameraConfiguration *);\n> +\n> + void (*createRequest)(void *, uint64_t, const Request *);\n> +\n> + void (*queueRequest)(void *, Request *);\n> +\n> + void (*start)(void *, const ControlList *);\n> + void (*stop)(void *);\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 30ea76f9470a..552af112abb5 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -11,6 +11,7 @@ libcamera_public_headers = files([\n> 'framebuffer.h',\n> 'framebuffer_allocator.h',\n> 'geometry.h',\n> + 'layer.h',\n> 'logging.h',\n> 'orientation.h',\n> 'pixel_format.h',\n> diff --git a/src/layer/meson.build b/src/layer/meson.build\n> new file mode 100644\n> index 000000000000..dee5e5ac5804\n> --- /dev/null\n> +++ b/src/layer/meson.build\n> @@ -0,0 +1,10 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +layer_includes = [\n> + libcamera_includes,\n> +]\n> +\n> +layer_install_dir = libcamera_libdir / 'layers'\n> +\n> +config_h.set('LAYER_DIR',\n> + '\"' + get_option('prefix') / layer_install_dir + '\"')\n> diff --git a/src/libcamera/layer_manager.cpp b/src/libcamera/layer_manager.cpp\n> new file mode 100644\n> index 000000000000..d707d4e12a53\n> --- /dev/null\n> +++ b/src/libcamera/layer_manager.cpp\n> @@ -0,0 +1,383 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + *\n> + * Layer manager\n> + */\n> +\n> +#include \"libcamera/internal/layer_manager.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/utils.h\"\n> +\n> +/**\n> + * \\file layer_manager.h\n> + * \\brief Layer manager\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(LayerManager)\n> +\n> +/**\n> + * \\class LayerManager\n> + * \\brief Layer manager\n> + *\n> + * The Layer manager discovers layer implementations from disk, and creates\n> + * execution queues for every function that is implemented by each layer and\n> + * executes them. A layer is a layer that sits between libcamera and the\n> + * application, and hooks into the public Camera interface.\n> + */\n> +\n> +/**\n> + * \\brief Construct a LayerManager instance\n> + *\n> + * The LayerManager class is meant be instantiated by the Camera.\n> + */\n> +LayerManager::LayerManager()\n> +{\n> + std::map layers;\n> +\n> + /* \\todo Implement built-in layers */\n> +\n> + /* This returns the number of \"modules\" successfully loaded */\n> + std::function addDirHandler =\n> + [this, &layers](const std::string &file) {\n> + LayerManager::LayerLoaded layer = createLayer(file);\n> + if (!layer.info)\n> + return 0;\n> +\n> + LOG(LayerManager, Debug) << \"Loaded layer '\" << file << \"'\";\n> +\n> + layers.emplace(std::string(layer.info->name), std::move(layer));\n> +\n> + return 1;\n> + };\n> +\n> + /* User-specified paths take precedence. */\n> + /* \\todo Document this */\n> + const char *layerPaths = utils::secure_getenv(\"LIBCAMERA_LAYER_PATH\");\n> + if (layerPaths) {\n> + for (const auto &dir : utils::split(layerPaths, \":\")) {\n> + if (dir.empty())\n> + continue;\n> +\n> + /*\n> + * \\todo Move the shared objects into one directory\n> + * instead of each in their own subdir\n> + */\n> + utils::addDir(dir.c_str(), 1, addDirHandler);\n> + }\n> + }\n> +\n> + /*\n> + * When libcamera is used before it is installed, load layers from the\n> + * same build directory as the libcamera library itself.\n> + */\n> + std::string root = utils::libcameraBuildPath();\n> + if (!root.empty()) {\n> + std::string layerBuildPath = root + \"src/layer\";\n> + constexpr int maxDepth = 2;\n> +\n> + LOG(LayerManager, Info)\n> + << \"libcamera is not installed. Adding '\"\n> + << layerBuildPath << \"' to the layer search path\";\n> +\n> + utils::addDir(layerBuildPath.c_str(), maxDepth, addDirHandler);\n> + }\n> +\n> + /* Finally try to load layers from the installed system path. */\n> + utils::addDir(LAYER_DIR, 1, addDirHandler);\n> +\n> + /* Order the layers */\n> + /* \\todo Document this. First is closer to application, last is closer to libcamera */\n> + const char *layerList = utils::secure_getenv(\"LIBCAMERA_LAYERS_ENABLE\");\n> + if (layerList) {\n> + for (const auto &layerName : utils::split(layerList, \":\")) {\n> + if (layerName.empty())\n> + continue;\n> +\n> + const auto &it = layers.find(layerName);\n> + if (it == layers.end())\n> + continue;\n> +\n> + executionQueue_.push_back(std::move(it->second));\n> + }\n> + }\n> +}\n> +\n> +void LayerManager::init(const Camera *camera, const ControlList &properties,\n> + const ControlInfoMap &controlInfoMap)\n> +{\n> + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> + void *closure = layer.vtable->init(camera->id());\n> + closures_[std::make_tuple(camera, &layer)] = closure;\n> + }\n> +\n> + /*\n> + * We need to iterate over the layers individually to merge all of\n> + * their controls, so we'll factor out updateControls() as it needs to be\n> + * run again at configure().\n> + */\n> + updateProperties(camera, properties);\n> + updateControls(camera, controlInfoMap);\n> +}\n> +\n> +void LayerManager::terminate(const Camera *camera)\n> +{\n> + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> + layer.vtable->terminate(closure);\n> + }\n> +}\n> +\n> +LayerManager::LayerLoaded LayerManager::createLayer(const std::string &filename)\n> +{\n> + LayerLoaded layer;\n> +\n> + File file{ filename };\n> + if (!file.open(File::OpenModeFlag::ReadOnly)) {\n> + LOG(LayerManager, Error) << \"Failed to open layer: \"\n> + << strerror(-file.error());\n> + return layer;\n> + }\n> +\n> + Span data = file.map();\n> + int ret = utils::elfVerifyIdent(data);\n> + if (ret) {\n> + LOG(LayerManager, Error) << \"Layer is not an ELF file\";\n> + return layer;\n> + }\n> +\n> + Span info = utils::elfLoadSymbol(data, \"layerInfo\");\n> + if (info.size() < sizeof(LayerInfo)) {\n> + LOG(LayerManager, Error) << \"Layer has no valid info\";\n> + return layer;\n> + }\n> +\n> + void *dlHandle = dlopen(file.fileName().c_str(), RTLD_LAZY);\n> + if (!dlHandle) {\n> + LOG(LayerManager, Error)\n> + << \"Failed to open layer shared object: \"\n> + << dlerror();\n> + return layer;\n> + }\n\nMaybe we can already populate layer.dlHandle here and remove the dlclose()s\nbelow, as the deconstructor will handle it.\n\n\nPaul\n\n> +\n> + void *layerInfo = dlsym(dlHandle, \"layerInfo\");\n> + if (!layerInfo) {\n> + LOG(LayerManager, Error)\n> + << \"Failed to load layerInfo from layer shared object: \"\n> + << dlerror();\n> + dlclose(dlHandle);\n> + return layer;\n> + }\n> +\n> + void *vtable = dlsym(dlHandle, \"layerInterface\");\n> + if (!vtable) {\n> + LOG(LayerManager, Error)\n> + << \"Failed to load layerInterface from layer shared object: \"\n> + << dlerror();\n> + dlclose(dlHandle);\n> + return layer;\n> + }\n> +\n> + layer.info = static_cast(layerInfo);\n> + layer.vtable = static_cast(vtable);\n> + layer.dlHandle = dlHandle;\n> +\n> + /*\n> + * No need to dlclose after this as the LayerLoaded deconstructor will\n> + * handle it\n> + */\n> +\n> + /* \\todo Implement this. It should come from the libcamera version */\n> + if (layer.info->layerAPIVersion != 1) {\n> + LOG(LayerManager, Error) << \"Layer API version mismatch\";\n> + layer.info = nullptr;\n> + return layer;\n> + }\n> +\n> + /* \\todo Document these requirements */\n> + if (!layer.vtable->init) {\n> + LOG(LayerManager, Error) << \"Layer doesn't implement init\";\n> + layer.info = nullptr;\n> + return layer;\n> + }\n> +\n> + /* \\todo Document these requirements */\n> + if (!layer.vtable->terminate) {\n> + LOG(LayerManager, Error) << \"Layer doesn't implement terminate\";\n> + layer.info = nullptr;\n> + return layer;\n> + }\n> +\n> + /* \\todo Validate the layer name. */\n> +\n> + return layer;\n> +}\n> +\n> +void LayerManager::bufferCompleted(const Camera *camera, Request *request, FrameBuffer *buffer)\n> +{\n> + /* Reverse order because this comes from a Signal emission */\n> + for (auto it = executionQueue_.rbegin();\n> + it != executionQueue_.rend(); it++) {\n> + if ((*it).vtable->bufferCompleted) {\n> + void *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> + (*it).vtable->bufferCompleted(closure, request, buffer);\n> + }\n> + }\n> +}\n> +\n> +void LayerManager::requestCompleted(const Camera *camera, Request *request)\n> +{\n> + /* Reverse order because this comes from a Signal emission */\n> + for (auto it = executionQueue_.rbegin();\n> + it != executionQueue_.rend(); it++) {\n> + if ((*it).vtable->requestCompleted) {\n> + void *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> + (*it).vtable->requestCompleted(closure, request);\n> + }\n> + }\n> +}\n> +\n> +void LayerManager::disconnected(const Camera *camera)\n> +{\n> + /* Reverse order because this comes from a Signal emission */\n> + for (auto it = executionQueue_.rbegin();\n> + it != executionQueue_.rend(); it++) {\n> + if ((*it).vtable->disconnected) {\n> + void *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> + (*it).vtable->disconnected(closure);\n> + }\n> + }\n> +}\n> +\n> +void LayerManager::acquire(const Camera *camera)\n> +{\n> + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> + if (layer.vtable->acquire) {\n> + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> + layer.vtable->acquire(closure);\n> + }\n> + }\n> +}\n> +\n> +void LayerManager::release(const Camera *camera)\n> +{\n> + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> + if (layer.vtable->release) {\n> + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> + layer.vtable->release(closure);\n> + }\n> + }\n> +}\n> +\n> +void LayerManager::updateProperties(const Camera *camera,\n> + const ControlList &properties)\n> +{\n> + ControlList props = properties;\n> + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> + if (layer.vtable->properties) {\n> + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> + ControlList ret = layer.vtable->properties(closure, props);\n> + props.merge(ret, ControlList::MergePolicy::OverwriteExisting);\n> + }\n> + }\n> + properties_[camera] = props;\n> +}\n> +\n> +void LayerManager::updateControls(const Camera *camera,\n> + const ControlInfoMap &controlInfoMap)\n> +{\n> + ControlInfoMap infoMap = controlInfoMap;\n> + /* \\todo Simplify this once ControlInfoMaps become easier to modify */\n> + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> + if (layer.vtable->controls) {\n> + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> + ControlInfoMap::Map ret = layer.vtable->controls(closure, infoMap);\n> + ControlInfoMap::Map map;\n> + /* Merge the layer's ret later so that layers can overwrite */\n> + for (auto &pair : infoMap)\n> + map.insert(pair);\n> + for (auto &pair : ret)\n> + map.insert(pair);\n> + infoMap = ControlInfoMap(std::move(map),\n> + libcamera::controls::controls);\n> + }\n> + }\n> + controls_[camera] = infoMap;\n> +}\n> +\n> +void LayerManager::configure(const Camera *camera,\n> + const CameraConfiguration *config,\n> + const ControlInfoMap &controlInfoMap)\n> +{\n> + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> + if (layer.vtable->configure) {\n> + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> + layer.vtable->configure(closure, config);\n> + }\n> + }\n> +\n> + updateControls(camera, controlInfoMap);\n> +}\n> +\n> +void LayerManager::createRequest(const Camera *camera, uint64_t cookie, const Request *request)\n> +{\n> + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> + if (layer.vtable->createRequest) {\n> + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> + layer.vtable->createRequest(closure, cookie, request);\n> + }\n> + }\n> +}\n> +\n> +void LayerManager::queueRequest(const Camera *camera, Request *request)\n> +{\n> + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> + if (layer.vtable->queueRequest) {\n> + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> + layer.vtable->queueRequest(closure, request);\n> + }\n> + }\n> +}\n> +\n> +void LayerManager::start(const Camera *camera, const ControlList *controls)\n> +{\n> + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> + if (layer.vtable->start) {\n> + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> + layer.vtable->start(closure, controls);\n> + }\n> + }\n> +}\n> +\n> +void LayerManager::stop(const Camera *camera)\n> +{\n> + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> + if (layer.vtable->stop) {\n> + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> + layer.vtable->stop(closure);\n> + }\n> + }\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 6a71b2903d27..0c2086a8399c 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -40,6 +40,7 @@ libcamera_internal_sources = files([\n> 'ipc_pipe.cpp',\n> 'ipc_pipe_unixsocket.cpp',\n> 'ipc_unixsocket.cpp',\n> + 'layer_manager.cpp',\n> 'mapped_framebuffer.cpp',\n> 'matrix.cpp',\n> 'media_device.cpp',\n> diff --git a/src/meson.build b/src/meson.build\n> index 8eb8f05b362f..37368b01cbf2 100644\n> --- a/src/meson.build\n> +++ b/src/meson.build\n> @@ -63,6 +63,7 @@ subdir('libcamera')\n> \n> subdir('android')\n> subdir('ipa')\n> +subdir('layer')\n> \n> subdir('apps')\n> \n> -- \n> 2.47.2\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 27741BDCBF\n\tfor ;\n\tFri, 4 Jul 2025 02:30:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 59CBB68E55;\n\tFri, 4 Jul 2025 04:30:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6539F6151C\n\tfor ;\n\tFri, 4 Jul 2025 04:30:30 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:dee9:10d6:1205:4e64])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 3B2FE82A;\n\tFri, 4 Jul 2025 04:30:04 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kteNPL1r\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751596206;\n\tbh=8uLHY4OB0/ivOW/HjTuVujwUWXgJeo6SzI+iEqPonJw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=kteNPL1riuEIeB+RMvZJ8+WDjWeYXxCp6yfOiVKomxGV2gTJ/Fnx3nLaR142BiJOg\n\tTVlvEuqgHxF0DkaQiCy2MPPUGE3OLFUZohzonYe+OtSrNbdp1d2zEZceh/QvZyePGO\n\tiuXgMqRj8e3hs8b+3EuSHpwIsriyVuxNdvQTAxU0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250703114225.2074071-5-paul.elder@ideasonboard.com>","References":"<20250703114225.2074071-1-paul.elder@ideasonboard.com>\n\t<20250703114225.2074071-5-paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v2 4/8] libcamera: layer_manager: Add LayerManager\n\timplementation","From":"Paul Elder ","Cc":"kieran.bingham@ideasonboard.com, barnabas.pocze@ideasonboard.com","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 04 Jul 2025 11:30:23 +0900","Message-ID":"<175159622338.2127280.3041812220180984712@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35057,"web_url":"https://patchwork.libcamera.org/comment/35057/","msgid":"<175329260060.50296.13730329434404882348@ping.linuxembedded.co.uk>","date":"2025-07-23T17:43:20","subject":"Re: [PATCH v2 4/8] libcamera: layer_manager: Add LayerManager\n\timplementation","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Paul Elder (2025-07-04 03:30:23)\n> Hi me,\n> \n> Quoting Paul Elder (2025-07-03 20:42:19)\n> > We want to be able to implement layers in libcamera, which conceptually\n> > sit in between the Camera class and the application. This can be useful\n> > for implementing things that don't belong inside the Camera/IPA nor inside\n> > the application, such as intercepting and translation the AeEnable\n> > control, or implementing the Sync algorithm.\n> > \n> > To achieve this, first add a LayerManager implementation, which searches\n> > for and loads layers from shared object files, and orchestrates\n> > executing them. Actually calling into these functions from the Camera\n> > class will be added in the following patch.\n> > \n> > Signed-off-by: Paul Elder \n> > \n> > ---\n> > Changes in v2:\n> > - add closures to the layer interface\n> > - separate Layer into LayerInfo and LayerInterface, to separate out\n> > layer data and the vtable\n> > - remove generateConfiguration() and streams() from the layer interface\n> > - add init() and terminate() to the layer interface, and make them\n> > required (mainly for preparing and destroying the closure)\n> > - make LayerLoaded automatically dlclose on deconstruction, remove copy,\n> > and implement move constructors\n> > - cache controls and properties in the LayerManager\n> > ---\n> > include/libcamera/internal/layer_manager.h | 117 +++++++\n> > include/libcamera/internal/meson.build | 1 +\n> > include/libcamera/layer.h | 54 +++\n> > include/libcamera/meson.build | 1 +\n> > src/layer/meson.build | 10 +\n> > src/libcamera/layer_manager.cpp | 383 +++++++++++++++++++++\n> > src/libcamera/meson.build | 1 +\n> > src/meson.build | 1 +\n> > 8 files changed, 568 insertions(+)\n> > create mode 100644 include/libcamera/internal/layer_manager.h\n> > create mode 100644 include/libcamera/layer.h\n> > create mode 100644 src/layer/meson.build\n> > create mode 100644 src/libcamera/layer_manager.cpp\n> > \n> > diff --git a/include/libcamera/internal/layer_manager.h b/include/libcamera/internal/layer_manager.h\n> > new file mode 100644\n> > index 000000000000..0d108bcddf3d\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/layer_manager.h\n> > @@ -0,0 +1,117 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board Oy\n> > + *\n> > + * Layer manager interface\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DECLARE_CATEGORY(LayerManager)\n> > +\n> > +class LayerManager\n> > +{\n> > +public:\n> > + LayerManager();\n> > + ~LayerManager() = default;\n> > +\n> > + void init(const Camera *camera, const ControlList &properties,\n> > + const ControlInfoMap &controlInfoMap);\n> > + void terminate(const Camera *camera);\n> > +\n> > + void bufferCompleted(const Camera *camera,\n> > + Request *request, FrameBuffer *buffer);\n> > + void requestCompleted(const Camera *camera, Request *request);\n> > + void disconnected(const Camera *camera);\n> > +\n> > + void acquire(const Camera *camera);\n> > + void release(const Camera *camera);\n> > +\n> > + const ControlInfoMap &controls(const Camera *camera) const { return controls_.at(camera); }\n> > + const ControlList &properties(const Camera *camera) const { return properties_.at(camera); }\n> > +\n> > + void configure(const Camera *camera, const CameraConfiguration *config,\n> > + const ControlInfoMap &controlInfoMap);\n> > +\n> > + void createRequest(const Camera *camera,\n> > + uint64_t cookie, const Request *request);\n> > +\n> > + void queueRequest(const Camera *camera, Request *request);\n> > +\n> > + void start(const Camera *camera, const ControlList *controls);\n> \n> controls should probably be non-const, otherwise layers won't be able to\n> intercept and modify it.\n\nIndeed.\n\n> \n> > + void stop(const Camera *camera);\n> > +\n> > +private:\n> > + /* Extend the layer with information specific to load-handling */\n> > + struct LayerLoaded\n> > + {\n> > + LayerLoaded()\n> > + : info(nullptr), vtable(nullptr), dlHandle(nullptr)\n> > + {\n> > + }\n> > +\n> > + LayerLoaded(LayerLoaded &&other)\n> > + : info(other.info), vtable(other.vtable),\n> > + dlHandle(other.dlHandle)\n> > + {\n> > + other.dlHandle = nullptr;\n> > + }\n> > +\n> > + LayerLoaded &operator=(LayerLoaded &&other)\n> > + {\n> > + info = other.info;\n> > + vtable = other.vtable;\n> > + dlHandle = other.dlHandle;\n> > + other.dlHandle = nullptr;\n> > + return *this;\n> > + }\n> > +\n> > + ~LayerLoaded()\n> > + {\n> > + if (dlHandle)\n> > + dlclose(dlHandle);\n> > + }\n> > +\n> > + LayerInfo *info;\n> > + LayerInterface *vtable;\n> > + void *dlHandle;\n> > +\n> > + private:\n> > + LIBCAMERA_DISABLE_COPY(LayerLoaded)\n> > + };\n> > +\n> > + using ClosureKey = std::tuple;\n> > +\n> > + void updateProperties(const Camera *camera,\n> > + const ControlList &properties);\n> > + void updateControls(const Camera *camera,\n> > + const ControlInfoMap &controlInfoMap);\n> > +\n> > + LayerLoaded createLayer(const std::string &file);\n> > + std::deque executionQueue_;\n> > + std::map closures_;\n> > +\n> > + std::map controls_;\n> > + std::map properties_;\n\n\nRe-sending these comments because my mail client seemed to hang?\n\nThis feels ... wrong somehow.\n\nI don't think we should keep copies of these here - but somehow in some\nCamera specific instance of the Layer handler, which is deleted along\nwith the camera when the camera is removed.\n\nI'm not sure of the impact yet here - but I guess it correlates with my\nother comments about there likely being a global loader that parses the\n.so files - but then we should instantiate an instance of (the ones that\nare going to be used) in a per-camera fasion.\n\n\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index 690f5c5ec9f6..20e6c295601f 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -29,6 +29,7 @@ libcamera_internal_headers = files([\n> > 'ipa_proxy.h',\n> > 'ipc_pipe.h',\n> > 'ipc_unixsocket.h',\n> > + 'layer_manager.h',\n> > 'mapped_framebuffer.h',\n> > 'matrix.h',\n> > 'media_device.h',\n> > diff --git a/include/libcamera/layer.h b/include/libcamera/layer.h\n> > new file mode 100644\n> > index 000000000000..cd0e26a3b72b\n> > --- /dev/null\n> > +++ b/include/libcamera/layer.h\n> > @@ -0,0 +1,54 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board Oy\n> > + *\n> > + * Layer interface\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +class CameraConfiguration;\n> > +class FrameBuffer;\n> > +class Request;\n> > +class Stream;\n> > +enum class StreamRole;\n> > +\n> > +struct LayerInfo {\n> > + const char *name;\n> > + int layerAPIVersion;\n> > +};\n> > +\n> > +struct LayerInterface {\n> > + void *(*init)(const std::string &id);\n> > + void (*terminate)(void *);\n> > +\n> > + void (*bufferCompleted)(void *, Request *, FrameBuffer *);\n> > + void (*requestCompleted)(void *, Request *);\n> > + void (*disconnected)(void *);\n> > +\n> > + void (*acquire)(void *);\n> > + void (*release)(void *);\n> > +\n> > + ControlInfoMap::Map (*controls)(void *, ControlInfoMap &);\n> > + ControlList (*properties)(void *, ControlList &);\n> > +\n> > + void (*configure)(void *, const CameraConfiguration *);\n> > +\n> > + void (*createRequest)(void *, uint64_t, const Request *);\n> > +\n> > + void (*queueRequest)(void *, Request *);\n> > +\n> > + void (*start)(void *, const ControlList *);\n\nSame here for non-const.\n\n> > + void (*stop)(void *);\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > index 30ea76f9470a..552af112abb5 100644\n> > --- a/include/libcamera/meson.build\n> > +++ b/include/libcamera/meson.build\n> > @@ -11,6 +11,7 @@ libcamera_public_headers = files([\n> > 'framebuffer.h',\n> > 'framebuffer_allocator.h',\n> > 'geometry.h',\n> > + 'layer.h',\n> > 'logging.h',\n> > 'orientation.h',\n> > 'pixel_format.h',\n> > diff --git a/src/layer/meson.build b/src/layer/meson.build\n> > new file mode 100644\n> > index 000000000000..dee5e5ac5804\n> > --- /dev/null\n> > +++ b/src/layer/meson.build\n> > @@ -0,0 +1,10 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +\n> > +layer_includes = [\n> > + libcamera_includes,\n> > +]\n> > +\n> > +layer_install_dir = libcamera_libdir / 'layers'\n> > +\n> > +config_h.set('LAYER_DIR',\n> > + '\"' + get_option('prefix') / layer_install_dir + '\"')\n> > diff --git a/src/libcamera/layer_manager.cpp b/src/libcamera/layer_manager.cpp\n> > new file mode 100644\n> > index 000000000000..d707d4e12a53\n> > --- /dev/null\n> > +++ b/src/libcamera/layer_manager.cpp\n> > @@ -0,0 +1,383 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board Oy\n> > + *\n> > + * Layer manager\n> > + */\n> > +\n> > +#include \"libcamera/internal/layer_manager.h\"\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \"libcamera/internal/utils.h\"\n> > +\n> > +/**\n> > + * \\file layer_manager.h\n> > + * \\brief Layer manager\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(LayerManager)\n> > +\n> > +/**\n> > + * \\class LayerManager\n> > + * \\brief Layer manager\n> > + *\n> > + * The Layer manager discovers layer implementations from disk, and creates\n> > + * execution queues for every function that is implemented by each layer and\n> > + * executes them. A layer is a layer that sits between libcamera and the\n> > + * application, and hooks into the public Camera interface.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Construct a LayerManager instance\n> > + *\n> > + * The LayerManager class is meant be instantiated by the Camera.\n> > + */\n> > +LayerManager::LayerManager()\n> > +{\n> > + std::map layers;\n> > +\n> > + /* \\todo Implement built-in layers */\n> > +\n> > + /* This returns the number of \"modules\" successfully loaded */\n> > + std::function addDirHandler =\n> > + [this, &layers](const std::string &file) {\n> > + LayerManager::LayerLoaded layer = createLayer(file);\n> > + if (!layer.info)\n> > + return 0;\n> > +\n> > + LOG(LayerManager, Debug) << \"Loaded layer '\" << file << \"'\";\n> > +\n> > + layers.emplace(std::string(layer.info->name), std::move(layer));\n> > +\n> > + return 1;\n> > + };\n> > +\n> > + /* User-specified paths take precedence. */\n> > + /* \\todo Document this */\n> > + const char *layerPaths = utils::secure_getenv(\"LIBCAMERA_LAYER_PATH\");\n> > + if (layerPaths) {\n> > + for (const auto &dir : utils::split(layerPaths, \":\")) {\n> > + if (dir.empty())\n> > + continue;\n> > +\n> > + /*\n> > + * \\todo Move the shared objects into one directory\n> > + * instead of each in their own subdir\n> > + */\n> > + utils::addDir(dir.c_str(), 1, addDirHandler);\n> > + }\n> > + }\n> > +\n> > + /*\n> > + * When libcamera is used before it is installed, load layers from the\n> > + * same build directory as the libcamera library itself.\n> > + */\n> > + std::string root = utils::libcameraBuildPath();\n> > + if (!root.empty()) {\n> > + std::string layerBuildPath = root + \"src/layer\";\n> > + constexpr int maxDepth = 2;\n> > +\n> > + LOG(LayerManager, Info)\n> > + << \"libcamera is not installed. Adding '\"\n> > + << layerBuildPath << \"' to the layer search path\";\n> > +\n> > + utils::addDir(layerBuildPath.c_str(), maxDepth, addDirHandler);\n> > + }\n> > +\n> > + /* Finally try to load layers from the installed system path. */\n> > + utils::addDir(LAYER_DIR, 1, addDirHandler);\n> > +\n> > + /* Order the layers */\n> > + /* \\todo Document this. First is closer to application, last is closer to libcamera */\n> > + const char *layerList = utils::secure_getenv(\"LIBCAMERA_LAYERS_ENABLE\");\n> > + if (layerList) {\n> > + for (const auto &layerName : utils::split(layerList, \":\")) {\n> > + if (layerName.empty())\n> > + continue;\n> > +\n> > + const auto &it = layers.find(layerName);\n> > + if (it == layers.end())\n> > + continue;\n> > +\n> > + executionQueue_.push_back(std::move(it->second));\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::init(const Camera *camera, const ControlList &properties,\n> > + const ControlInfoMap &controlInfoMap)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + void *closure = layer.vtable->init(camera->id());\n> > + closures_[std::make_tuple(camera, &layer)] = closure;\n> > + }\n> > +\n> > + /*\n> > + * We need to iterate over the layers individually to merge all of\n> > + * their controls, so we'll factor out updateControls() as it needs to be\n> > + * run again at configure().\n> > + */\n> > + updateProperties(camera, properties);\n> > + updateControls(camera, controlInfoMap);\n> > +}\n> > +\n> > +void LayerManager::terminate(const Camera *camera)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->terminate(closure);\n> > + }\n\n\nIf we do end up keeping maps of the camera make sure we remove the map\nentries when cameras are removed ?\n\n(Think hotplugging UVC cameras - each time it's removed and re-inserted\nthere would be a 'new' Camera*)\n\n\n> > +}\n> > +\n> > +LayerManager::LayerLoaded LayerManager::createLayer(const std::string &filename)\n> > +{\n> > + LayerLoaded layer;\n> > +\n> > + File file{ filename };\n> > + if (!file.open(File::OpenModeFlag::ReadOnly)) {\n> > + LOG(LayerManager, Error) << \"Failed to open layer: \"\n> > + << strerror(-file.error());\n> > + return layer;\n> > + }\n> > +\n> > + Span data = file.map();\n> > + int ret = utils::elfVerifyIdent(data);\n> > + if (ret) {\n> > + LOG(LayerManager, Error) << \"Layer is not an ELF file\";\n> > + return layer;\n> > + }\n> > +\n> > + Span info = utils::elfLoadSymbol(data, \"layerInfo\");\n> > + if (info.size() < sizeof(LayerInfo)) {\n> > + LOG(LayerManager, Error) << \"Layer has no valid info\";\n> > + return layer;\n> > + }\n> > +\n> > + void *dlHandle = dlopen(file.fileName().c_str(), RTLD_LAZY);\n> > + if (!dlHandle) {\n> > + LOG(LayerManager, Error)\n> > + << \"Failed to open layer shared object: \"\n> > + << dlerror();\n> > + return layer;\n> > + }\n> \n> Maybe we can already populate layer.dlHandle here and remove the dlclose()s\n> below, as the deconstructor will handle it.\n> \n> \n> Paul\n> \n> > +\n> > + void *layerInfo = dlsym(dlHandle, \"layerInfo\");\n> > + if (!layerInfo) {\n> > + LOG(LayerManager, Error)\n> > + << \"Failed to load layerInfo from layer shared object: \"\n> > + << dlerror();\n> > + dlclose(dlHandle);\n> > + return layer;\n> > + }\n> > +\n> > + void *vtable = dlsym(dlHandle, \"layerInterface\");\n> > + if (!vtable) {\n> > + LOG(LayerManager, Error)\n> > + << \"Failed to load layerInterface from layer shared object: \"\n> > + << dlerror();\n> > + dlclose(dlHandle);\n> > + return layer;\n> > + }\n> > +\n> > + layer.info = static_cast(layerInfo);\n> > + layer.vtable = static_cast(vtable);\n> > + layer.dlHandle = dlHandle;\n> > +\n> > + /*\n> > + * No need to dlclose after this as the LayerLoaded deconstructor will\n> > + * handle it\n> > + */\n> > +\n> > + /* \\todo Implement this. It should come from the libcamera version */\n> > + if (layer.info->layerAPIVersion != 1) {\n> > + LOG(LayerManager, Error) << \"Layer API version mismatch\";\n> > + layer.info = nullptr;\n> > + return layer;\n> > + }\n> > +\n> > + /* \\todo Document these requirements */\n> > + if (!layer.vtable->init) {\n> > + LOG(LayerManager, Error) << \"Layer doesn't implement init\";\n> > + layer.info = nullptr;\n> > + return layer;\n> > + }\n> > +\n> > + /* \\todo Document these requirements */\n> > + if (!layer.vtable->terminate) {\n> > + LOG(LayerManager, Error) << \"Layer doesn't implement terminate\";\n> > + layer.info = nullptr;\n> > + return layer;\n> > + }\n> > +\n> > + /* \\todo Validate the layer name. */\n> > +\n> > + return layer;\n> > +}\n> > +\n> > +void LayerManager::bufferCompleted(const Camera *camera, Request *request, FrameBuffer *buffer)\n> > +{\n> > + /* Reverse order because this comes from a Signal emission */\n> > + for (auto it = executionQueue_.rbegin();\n> > + it != executionQueue_.rend(); it++) {\n> > + if ((*it).vtable->bufferCompleted) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> > + (*it).vtable->bufferCompleted(closure, request, buffer);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::requestCompleted(const Camera *camera, Request *request)\n> > +{\n> > + /* Reverse order because this comes from a Signal emission */\n> > + for (auto it = executionQueue_.rbegin();\n> > + it != executionQueue_.rend(); it++) {\n> > + if ((*it).vtable->requestCompleted) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> > + (*it).vtable->requestCompleted(closure, request);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::disconnected(const Camera *camera)\n> > +{\n> > + /* Reverse order because this comes from a Signal emission */\n> > + for (auto it = executionQueue_.rbegin();\n> > + it != executionQueue_.rend(); it++) {\n> > + if ((*it).vtable->disconnected) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> > + (*it).vtable->disconnected(closure);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::acquire(const Camera *camera)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->acquire) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->acquire(closure);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::release(const Camera *camera)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->release) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->release(closure);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::updateProperties(const Camera *camera,\n> > + const ControlList &properties)\n> > +{\n> > + ControlList props = properties;\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->properties) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + ControlList ret = layer.vtable->properties(closure, props);\n> > + props.merge(ret, ControlList::MergePolicy::OverwriteExisting);\n> > + }\n> > + }\n> > + properties_[camera] = props;\n> > +}\n> > +\n> > +void LayerManager::updateControls(const Camera *camera,\n> > + const ControlInfoMap &controlInfoMap)\n> > +{\n> > + ControlInfoMap infoMap = controlInfoMap;\n> > + /* \\todo Simplify this once ControlInfoMaps become easier to modify */\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->controls) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + ControlInfoMap::Map ret = layer.vtable->controls(closure, infoMap);\n> > + ControlInfoMap::Map map;\n> > + /* Merge the layer's ret later so that layers can overwrite */\n> > + for (auto &pair : infoMap)\n> > + map.insert(pair);\n> > + for (auto &pair : ret)\n> > + map.insert(pair);\n> > + infoMap = ControlInfoMap(std::move(map),\n> > + libcamera::controls::controls);\n> > + }\n> > + }\n> > + controls_[camera] = infoMap;\n> > +}\n> > +\n> > +void LayerManager::configure(const Camera *camera,\n> > + const CameraConfiguration *config,\n> > + const ControlInfoMap &controlInfoMap)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->configure) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->configure(closure, config);\n> > + }\n> > + }\n> > +\n> > + updateControls(camera, controlInfoMap);\n> > +}\n> > +\n> > +void LayerManager::createRequest(const Camera *camera, uint64_t cookie, const Request *request)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->createRequest) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->createRequest(closure, cookie, request);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::queueRequest(const Camera *camera, Request *request)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->queueRequest) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->queueRequest(closure, request);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::start(const Camera *camera, const ControlList *controls)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->start) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->start(closure, controls);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::stop(const Camera *camera)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->stop) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->stop(closure);\n> > + }\n> > + }\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 6a71b2903d27..0c2086a8399c 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -40,6 +40,7 @@ libcamera_internal_sources = files([\n> > 'ipc_pipe.cpp',\n> > 'ipc_pipe_unixsocket.cpp',\n> > 'ipc_unixsocket.cpp',\n> > + 'layer_manager.cpp',\n> > 'mapped_framebuffer.cpp',\n> > 'matrix.cpp',\n> > 'media_device.cpp',\n> > diff --git a/src/meson.build b/src/meson.build\n> > index 8eb8f05b362f..37368b01cbf2 100644\n> > --- a/src/meson.build\n> > +++ b/src/meson.build\n> > @@ -63,6 +63,7 @@ subdir('libcamera')\n> > \n> > subdir('android')\n> > subdir('ipa')\n> > +subdir('layer')\n> > \n> > subdir('apps')\n> > \n> > -- \n> > 2.47.2\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8D4CCC3237\n\tfor ;\n\tWed, 23 Jul 2025 17:43:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 557C769062;\n\tWed, 23 Jul 2025 19:43:24 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EDBC7614ED\n\tfor ;\n\tWed, 23 Jul 2025 19:43:22 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 89AA9E91;\n\tWed, 23 Jul 2025 19:42:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dhJ3uQAd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753292564;\n\tbh=0uG1Byt6fqaXlKOoN9Ad2RWhGqc8FIhtCCwQBqR5N+g=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=dhJ3uQAdZSnU2N+p2FoPjtyDvC3eYChpVdSufoofM8Nw9TYd4iLuiePv2XqbK2sun\n\tPwjs/r1CTegvZ502MvPZ0CAN4XQAN+vf/2InYSnc0GVosmEMQWHtlNlwobLRPUGGNK\n\t9l3iCD2E0e1mAqMZVXZQo9F1+DFZi8XCM2Jiz2g8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<175159622338.2127280.3041812220180984712@neptunite.rasen.tech>","References":"<20250703114225.2074071-1-paul.elder@ideasonboard.com>\n\t<20250703114225.2074071-5-paul.elder@ideasonboard.com>\n\t<175159622338.2127280.3041812220180984712@neptunite.rasen.tech>","Subject":"Re: [PATCH v2 4/8] libcamera: layer_manager: Add LayerManager\n\timplementation","From":"Kieran Bingham ","Cc":"barnabas.pocze@ideasonboard.com","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 23 Jul 2025 18:43:20 +0100","Message-ID":"<175329260060.50296.13730329434404882348@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35163,"web_url":"https://patchwork.libcamera.org/comment/35163/","msgid":"<61fd9a80-0d8e-431b-b16e-80457ac62957@ideasonboard.com>","date":"2025-07-27T17:42:19","subject":"Re: [PATCH v2 4/8] libcamera: layer_manager: Add LayerManager\n\timplementation","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 07. 03. 13:42 keltezéssel, Paul Elder írta:\n> We want to be able to implement layers in libcamera, which conceptually\n> sit in between the Camera class and the application. This can be useful\n> for implementing things that don't belong inside the Camera/IPA nor inside\n> the application, such as intercepting and translation the AeEnable\n> control, or implementing the Sync algorithm.\n> \n> To achieve this, first add a LayerManager implementation, which searches\n> for and loads layers from shared object files, and orchestrates\n> executing them. Actually calling into these functions from the Camera\n> class will be added in the following patch.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v2:\n> - add closures to the layer interface\n> - separate Layer into LayerInfo and LayerInterface, to separate out\n> layer data and the vtable\n> - remove generateConfiguration() and streams() from the layer interface\n> - add init() and terminate() to the layer interface, and make them\n> required (mainly for preparing and destroying the closure)\n> - make LayerLoaded automatically dlclose on deconstruction, remove copy,\n> and implement move constructors\n> - cache controls and properties in the LayerManager\n> ---\n> include/libcamera/internal/layer_manager.h | 117 +++++++\n> include/libcamera/internal/meson.build | 1 +\n> include/libcamera/layer.h | 54 +++\n> include/libcamera/meson.build | 1 +\n> src/layer/meson.build | 10 +\n> src/libcamera/layer_manager.cpp | 383 +++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> src/meson.build | 1 +\n> 8 files changed, 568 insertions(+)\n> create mode 100644 include/libcamera/internal/layer_manager.h\n> create mode 100644 include/libcamera/layer.h\n> create mode 100644 src/layer/meson.build\n> create mode 100644 src/libcamera/layer_manager.cpp\n> \n> diff --git a/include/libcamera/internal/layer_manager.h b/include/libcamera/internal/layer_manager.h\n> new file mode 100644\n> index 000000000000..0d108bcddf3d\n> --- /dev/null\n> +++ b/include/libcamera/internal/layer_manager.h\n> @@ -0,0 +1,117 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + *\n> + * Layer manager interface\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(LayerManager)\n> +\n> +class LayerManager\n> +{\n> +public:\n> +\tLayerManager();\n> +\t~LayerManager() = default;\n> +\n> +\tvoid init(const Camera *camera, const ControlList &properties,\n> +\t\t const ControlInfoMap &controlInfoMap);\n> +\tvoid terminate(const Camera *camera);\n> +\n> +\tvoid bufferCompleted(const Camera *camera,\n> +\t\t\t Request *request, FrameBuffer *buffer);\n> +\tvoid requestCompleted(const Camera *camera, Request *request);\n> +\tvoid disconnected(const Camera *camera);\n> +\n> +\tvoid acquire(const Camera *camera);\n> +\tvoid release(const Camera *camera);\n> +\n> +\tconst ControlInfoMap &controls(const Camera *camera) const { return controls_.at(camera); }\n> +\tconst ControlList &properties(const Camera *camera) const { return properties_.at(camera); }\n> +\n> +\tvoid configure(const Camera *camera, const CameraConfiguration *config,\n> +\t\t const ControlInfoMap &controlInfoMap);\n> +\n> +\tvoid createRequest(const Camera *camera,\n> +\t\t\t uint64_t cookie, const Request *request);\n> +\n> +\tvoid queueRequest(const Camera *camera, Request *request);\n> +\n> +\tvoid start(const Camera *camera, const ControlList *controls);\n> +\tvoid stop(const Camera *camera);\n> +\n> +private:\n> +\t/* Extend the layer with information specific to load-handling */\n> +\tstruct LayerLoaded\n> +\t{\n> +\t\tLayerLoaded()\n> +\t\t\t: info(nullptr), vtable(nullptr), dlHandle(nullptr)\n\nI would initialize the members where they are defined, and have `LayerLoaded() = default;`.\n\n\n> +\t\t{\n> +\t\t}\n> +\n> +\t\tLayerLoaded(LayerLoaded &&other)\n> +\t\t\t: info(other.info), vtable(other.vtable),\n> +\t\t\t dlHandle(other.dlHandle)\n> +\t\t{\n> +\t\t\tother.dlHandle = nullptr;\n> +\t\t}\n> +\n> +\t\tLayerLoaded &operator=(LayerLoaded &&other)\n> +\t\t{\n> +\t\t\tinfo = other.info;\n> +\t\t\tvtable = other.vtable;\n> +\t\t\tdlHandle = other.dlHandle;\n> +\t\t\tother.dlHandle = nullptr;\n> +\t\t\treturn *this;\n> +\t\t}\n\nI like to use `std::exchange()` for move constr/assignment because it makes things\neasier in my opinion; but the current version also looks ok.\n\n\n> +\n> +\t\t~LayerLoaded()\n> +\t\t{\n> +\t\t\tif (dlHandle)\n> +\t\t\t\tdlclose(dlHandle);\n> +\t\t}\n> +\n> +\t\tLayerInfo *info;\n> +\t\tLayerInterface *vtable;\n> +\t\tvoid *dlHandle;\n> +\n> +\tprivate:\n> +\t\tLIBCAMERA_DISABLE_COPY(LayerLoaded)\n> +\t};\n> +\n> +\tusing ClosureKey = std::tuple;\n> +\n> +\tvoid updateProperties(const Camera *camera,\n> +\t\t\t const ControlList &properties);\n> +\tvoid updateControls(const Camera *camera,\n> +\t\t\t const ControlInfoMap &controlInfoMap);\n> +\n> +\tLayerLoaded createLayer(const std::string &file);\n> +\tstd::deque executionQueue_;\n> +\tstd::map closures_;\n\nI feel like we should bike shed the actual interface a bit more.\n\nI would like to suggest something like the following:\n\n// -----------------\n// plugin interface:\n\nstruct libcamera_layer_vtable {\n void (*destroy)(libcamera_layer *);\n ...\n};\n\nstruct libcamera_layer_info {\n const char *name;\n ...\n};\n\nstruct libcamera_layer {\n const struct libcamera_layer_info *info;\n const struct libcamera_layer_vtable *vtable;\n};\n\nstruct libcamera_layer *layer_instantiate(Camera *, ...)\n{\n // create an instance for the given camera\n}\n\n// or we could allow having multiple layers in the same dso by having one function:\n\nconst struct libcamera_layer_info *layer_enumerate(uint32_t *idx);\n\nand then the info struct could have the `instantiate()` function ptr\n\n// ----------------\n// libcamera parts:\n\nstruct LayerPlugin {\n void *dso;\n\n ~LayerPlugin() { dlclose(dso); }\n};\n\nstruct LayerLoaded {\n std::shared_ptr plugin;\n libcamera_layer *layer;\n\n ~LayerLoaded() { layer->vtable->destroy(layer); }\n};\n\n\nstruct Camera::Private {\n ...\n std::vector layers;\n};\n\nstruct LayerManager {\n // some kind of mapping/list of `LayerPlugin`s\n\n std::vector instantiateFor(Camera &camera)\n {\n // as long as there is no configuration file support, instantiate all of them or similar\n }\n};\n\nor something along these lines. I feel like we should get rid of the closure std::map.\nIn this setup the plugin could do something like this:\n\nstruct MyLayer : libcamera_layer {\n ...\n};\n\nlibcamera_layer *layer_instantiate(Camera *camera)\n{\n return new MyLayer(camera);\n}\n\nand then in the implementations, `static_cast(...)` can be used.\nOr e.g. in C, the same could be done, or if it's not the first member, then\nan implementation of `container_of()`.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> +\n> +\tstd::map controls_;\n> +\tstd::map properties_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 690f5c5ec9f6..20e6c295601f 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -29,6 +29,7 @@ libcamera_internal_headers = files([\n> 'ipa_proxy.h',\n> 'ipc_pipe.h',\n> 'ipc_unixsocket.h',\n> + 'layer_manager.h',\n> 'mapped_framebuffer.h',\n> 'matrix.h',\n> 'media_device.h',\n> diff --git a/include/libcamera/layer.h b/include/libcamera/layer.h\n> new file mode 100644\n> index 000000000000..cd0e26a3b72b\n> --- /dev/null\n> +++ b/include/libcamera/layer.h\n> @@ -0,0 +1,54 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + *\n> + * Layer interface\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class CameraConfiguration;\n> +class FrameBuffer;\n> +class Request;\n> +class Stream;\n> +enum class StreamRole;\n> +\n> +struct LayerInfo {\n> +\tconst char *name;\n> +\tint layerAPIVersion;\n> +};\n> +\n> +struct LayerInterface {\n> +\tvoid *(*init)(const std::string &id);\n> +\tvoid (*terminate)(void *);\n> +\n> +\tvoid (*bufferCompleted)(void *, Request *, FrameBuffer *);\n> +\tvoid (*requestCompleted)(void *, Request *);\n> +\tvoid (*disconnected)(void *);\n> +\n> +\tvoid (*acquire)(void *);\n> +\tvoid (*release)(void *);\n> +\n> +\tControlInfoMap::Map (*controls)(void *, ControlInfoMap &);\n> +\tControlList (*properties)(void *, ControlList &);\n> +\n> +\tvoid (*configure)(void *, const CameraConfiguration *);\n> +\n> +\tvoid (*createRequest)(void *, uint64_t, const Request *);\n> +\n> +\tvoid (*queueRequest)(void *, Request *);\n> +\n> +\tvoid (*start)(void *, const ControlList *);\n> +\tvoid (*stop)(void *);\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 30ea76f9470a..552af112abb5 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -11,6 +11,7 @@ libcamera_public_headers = files([\n> 'framebuffer.h',\n> 'framebuffer_allocator.h',\n> 'geometry.h',\n> + 'layer.h',\n> 'logging.h',\n> 'orientation.h',\n> 'pixel_format.h',\n> diff --git a/src/layer/meson.build b/src/layer/meson.build\n> new file mode 100644\n> index 000000000000..dee5e5ac5804\n> --- /dev/null\n> +++ b/src/layer/meson.build\n> @@ -0,0 +1,10 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +layer_includes = [\n> + libcamera_includes,\n> +]\n> +\n> +layer_install_dir = libcamera_libdir / 'layers'\n> +\n> +config_h.set('LAYER_DIR',\n> + '\"' + get_option('prefix') / layer_install_dir + '\"')\n> diff --git a/src/libcamera/layer_manager.cpp b/src/libcamera/layer_manager.cpp\n> new file mode 100644\n> index 000000000000..d707d4e12a53\n> --- /dev/null\n> +++ b/src/libcamera/layer_manager.cpp\n> @@ -0,0 +1,383 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + *\n> + * Layer manager\n> + */\n> +\n> +#include \"libcamera/internal/layer_manager.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/utils.h\"\n> +\n> +/**\n> + * \\file layer_manager.h\n> + * \\brief Layer manager\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(LayerManager)\n> +\n> +/**\n> + * \\class LayerManager\n> + * \\brief Layer manager\n> + *\n> + * The Layer manager discovers layer implementations from disk, and creates\n> + * execution queues for every function that is implemented by each layer and\n> + * executes them. A layer is a layer that sits between libcamera and the\n> + * application, and hooks into the public Camera interface.\n> + */\n> +\n> +/**\n> + * \\brief Construct a LayerManager instance\n> + *\n> + * The LayerManager class is meant be instantiated by the Camera.\n> + */\n> +LayerManager::LayerManager()\n> +{\n> +\tstd::map layers;\n> +\n> +\t/* \\todo Implement built-in layers */\n> +\n> +\t/* This returns the number of \"modules\" successfully loaded */\n> +\tstd::function addDirHandler =\n> +\t[this, &layers](const std::string &file) {\n> +\t\tLayerManager::LayerLoaded layer = createLayer(file);\n> +\t\tif (!layer.info)\n> +\t\t\treturn 0;\n> +\n> +\t\tLOG(LayerManager, Debug) << \"Loaded layer '\" << file << \"'\";\n> +\n> +\t\tlayers.emplace(std::string(layer.info->name), std::move(layer));\n> +\n> +\t\treturn 1;\n> +\t};\n> +\n> +\t/* User-specified paths take precedence. */\n> +\t/* \\todo Document this */\n> +\tconst char *layerPaths = utils::secure_getenv(\"LIBCAMERA_LAYER_PATH\");\n> +\tif (layerPaths) {\n> +\t\tfor (const auto &dir : utils::split(layerPaths, \":\")) {\n> +\t\t\tif (dir.empty())\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\t/*\n> +\t\t\t * \\todo Move the shared objects into one directory\n> +\t\t\t * instead of each in their own subdir\n> +\t\t\t */\n> +\t\t\tutils::addDir(dir.c_str(), 1, addDirHandler);\n> +\t\t}\n> +\t}\n> +\n> +\t/*\n> +\t * When libcamera is used before it is installed, load layers from the\n> +\t * same build directory as the libcamera library itself.\n> +\t */\n> +\tstd::string root = utils::libcameraBuildPath();\n> +\tif (!root.empty()) {\n> +\t\tstd::string layerBuildPath = root + \"src/layer\";\n> +\t\tconstexpr int maxDepth = 2;\n> +\n> +\t\tLOG(LayerManager, Info)\n> +\t\t\t<< \"libcamera is not installed. Adding '\"\n> +\t\t\t<< layerBuildPath << \"' to the layer search path\";\n> +\n> +\t\tutils::addDir(layerBuildPath.c_str(), maxDepth, addDirHandler);\n> +\t}\n> +\n> +\t/* Finally try to load layers from the installed system path. */\n> +\tutils::addDir(LAYER_DIR, 1, addDirHandler);\n> +\n> +\t/* Order the layers */\n> +\t/* \\todo Document this. First is closer to application, last is closer to libcamera */\n> +\tconst char *layerList = utils::secure_getenv(\"LIBCAMERA_LAYERS_ENABLE\");\n> +\tif (layerList) {\n> +\t\tfor (const auto &layerName : utils::split(layerList, \":\")) {\n> +\t\t\tif (layerName.empty())\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tconst auto &it = layers.find(layerName);\n> +\t\t\tif (it == layers.end())\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\texecutionQueue_.push_back(std::move(it->second));\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void LayerManager::init(const Camera *camera, const ControlList &properties,\n> +\t\t\tconst ControlInfoMap &controlInfoMap)\n> +{\n> +\tfor (LayerManager::LayerLoaded &layer : executionQueue_) {\n> +\t\tvoid *closure = layer.vtable->init(camera->id());\n> +\t\tclosures_[std::make_tuple(camera, &layer)] = closure;\n> +\t}\n> +\n> +\t/*\n> +\t * We need to iterate over the layers individually to merge all of\n> +\t * their controls, so we'll factor out updateControls() as it needs to be\n> +\t * run again at configure().\n> +\t */\n> +\tupdateProperties(camera, properties);\n> +\tupdateControls(camera, controlInfoMap);\n> +}\n> +\n> +void LayerManager::terminate(const Camera *camera)\n> +{\n> +\tfor (LayerManager::LayerLoaded &layer : executionQueue_) {\n> +\t\tvoid *closure = closures_.at(std::make_tuple(camera, &layer));\n> +\t\tlayer.vtable->terminate(closure);\n> +\t}\n> +}\n> +\n> +LayerManager::LayerLoaded LayerManager::createLayer(const std::string &filename)\n> +{\n> +\tLayerLoaded layer;\n> +\n> +\tFile file{ filename };\n> +\tif (!file.open(File::OpenModeFlag::ReadOnly)) {\n> +\t\tLOG(LayerManager, Error) << \"Failed to open layer: \"\n> +\t\t\t\t\t << strerror(-file.error());\n> +\t\treturn layer;\n> +\t}\n> +\n> +\tSpan data = file.map();\n> +\tint ret = utils::elfVerifyIdent(data);\n> +\tif (ret) {\n> +\t\tLOG(LayerManager, Error) << \"Layer is not an ELF file\";\n> +\t\treturn layer;\n> +\t}\n> +\n> +\tSpan info = utils::elfLoadSymbol(data, \"layerInfo\");\n> +\tif (info.size() < sizeof(LayerInfo)) {\n> +\t\tLOG(LayerManager, Error) << \"Layer has no valid info\";\n> +\t\treturn layer;\n> +\t}\n> +\n> +\tvoid *dlHandle = dlopen(file.fileName().c_str(), RTLD_LAZY);\n> +\tif (!dlHandle) {\n> +\t\tLOG(LayerManager, Error)\n> +\t\t\t<< \"Failed to open layer shared object: \"\n> +\t\t\t<< dlerror();\n> +\t\treturn layer;\n> +\t}\n> +\n> +\tvoid *layerInfo = dlsym(dlHandle, \"layerInfo\");\n> +\tif (!layerInfo) {\n> +\t\tLOG(LayerManager, Error)\n> +\t\t\t<< \"Failed to load layerInfo from layer shared object: \"\n> +\t\t\t<< dlerror();\n> +\t\tdlclose(dlHandle);\n> +\t\treturn layer;\n> +\t}\n> +\n> +\tvoid *vtable = dlsym(dlHandle, \"layerInterface\");\n> +\tif (!vtable) {\n> +\t\tLOG(LayerManager, Error)\n> +\t\t\t<< \"Failed to load layerInterface from layer shared object: \"\n> +\t\t\t<< dlerror();\n> +\t\tdlclose(dlHandle);\n> +\t\treturn layer;\n> +\t}\n> +\n> +\tlayer.info = static_cast(layerInfo);\n> +\tlayer.vtable = static_cast(vtable);\n> +\tlayer.dlHandle = dlHandle;\n> +\n> +\t/*\n> +\t * No need to dlclose after this as the LayerLoaded deconstructor will\n> +\t * handle it\n> +\t */\n> +\n> +\t/* \\todo Implement this. It should come from the libcamera version */\n> +\tif (layer.info->layerAPIVersion != 1) {\n> +\t\tLOG(LayerManager, Error) << \"Layer API version mismatch\";\n> +\t\tlayer.info = nullptr;\n> +\t\treturn layer;\n> +\t}\n> +\n> +\t/* \\todo Document these requirements */\n> +\tif (!layer.vtable->init) {\n> +\t\tLOG(LayerManager, Error) << \"Layer doesn't implement init\";\n> +\t\tlayer.info = nullptr;\n> +\t\treturn layer;\n> +\t}\n> +\n> +\t/* \\todo Document these requirements */\n> +\tif (!layer.vtable->terminate) {\n> +\t\tLOG(LayerManager, Error) << \"Layer doesn't implement terminate\";\n> +\t\tlayer.info = nullptr;\n> +\t\treturn layer;\n> +\t}\n> +\n> +\t/* \\todo Validate the layer name. */\n> +\n> +\treturn layer;\n> +}\n> +\n> +void LayerManager::bufferCompleted(const Camera *camera, Request *request, FrameBuffer *buffer)\n> +{\n> +\t/* Reverse order because this comes from a Signal emission */\n> +\tfor (auto it = executionQueue_.rbegin();\n> +\t it != executionQueue_.rend(); it++) {\n> +\t\tif ((*it).vtable->bufferCompleted) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> +\t\t\t(*it).vtable->bufferCompleted(closure, request, buffer);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void LayerManager::requestCompleted(const Camera *camera, Request *request)\n> +{\n> +\t/* Reverse order because this comes from a Signal emission */\n> +\tfor (auto it = executionQueue_.rbegin();\n> +\t it != executionQueue_.rend(); it++) {\n> +\t\tif ((*it).vtable->requestCompleted) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> +\t\t\t(*it).vtable->requestCompleted(closure, request);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void LayerManager::disconnected(const Camera *camera)\n> +{\n> +\t/* Reverse order because this comes from a Signal emission */\n> +\tfor (auto it = executionQueue_.rbegin();\n> +\t it != executionQueue_.rend(); it++) {\n> +\t\tif ((*it).vtable->disconnected) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> +\t\t\t(*it).vtable->disconnected(closure);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void LayerManager::acquire(const Camera *camera)\n> +{\n> +\tfor (LayerManager::LayerLoaded &layer : executionQueue_) {\n> +\t\tif (layer.vtable->acquire) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &layer));\n> +\t\t\tlayer.vtable->acquire(closure);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void LayerManager::release(const Camera *camera)\n> +{\n> +\tfor (LayerManager::LayerLoaded &layer : executionQueue_) {\n> +\t\tif (layer.vtable->release) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &layer));\n> +\t\t\tlayer.vtable->release(closure);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void LayerManager::updateProperties(const Camera *camera,\n> +\t\t\t\t const ControlList &properties)\n> +{\n> +\tControlList props = properties;\n> +\tfor (LayerManager::LayerLoaded &layer : executionQueue_) {\n> +\t\tif (layer.vtable->properties) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &layer));\n> +\t\t\tControlList ret = layer.vtable->properties(closure, props);\n> +\t\t\tprops.merge(ret, ControlList::MergePolicy::OverwriteExisting);\n> +\t\t}\n> +\t}\n> +\tproperties_[camera] = props;\n> +}\n> +\n> +void LayerManager::updateControls(const Camera *camera,\n> +\t\t\t\t const ControlInfoMap &controlInfoMap)\n> +{\n> +\tControlInfoMap infoMap = controlInfoMap;\n> +\t/* \\todo Simplify this once ControlInfoMaps become easier to modify */\n> +\tfor (LayerManager::LayerLoaded &layer : executionQueue_) {\n> +\t\tif (layer.vtable->controls) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &layer));\n> +\t\t\tControlInfoMap::Map ret = layer.vtable->controls(closure, infoMap);\n> +\t\t\tControlInfoMap::Map map;\n> +\t\t\t/* Merge the layer's ret later so that layers can overwrite */\n> +\t\t\tfor (auto &pair : infoMap)\n> +\t\t\t\tmap.insert(pair);\n> +\t\t\tfor (auto &pair : ret)\n> +\t\t\t\tmap.insert(pair);\n> +\t\t\tinfoMap = ControlInfoMap(std::move(map),\n> +\t\t\t\t\t\t libcamera::controls::controls);\n> +\t\t}\n> +\t}\n> +\tcontrols_[camera] = infoMap;\n> +}\n> +\n> +void LayerManager::configure(const Camera *camera,\n> +\t\t\t const CameraConfiguration *config,\n> +\t\t\t const ControlInfoMap &controlInfoMap)\n> +{\n> +\tfor (LayerManager::LayerLoaded &layer : executionQueue_) {\n> +\t\tif (layer.vtable->configure) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &layer));\n> +\t\t\tlayer.vtable->configure(closure, config);\n> +\t\t}\n> +\t}\n> +\n> +\tupdateControls(camera, controlInfoMap);\n> +}\n> +\n> +void LayerManager::createRequest(const Camera *camera, uint64_t cookie, const Request *request)\n> +{\n> +\tfor (LayerManager::LayerLoaded &layer : executionQueue_) {\n> +\t\tif (layer.vtable->createRequest) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &layer));\n> +\t\t\tlayer.vtable->createRequest(closure, cookie, request);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void LayerManager::queueRequest(const Camera *camera, Request *request)\n> +{\n> +\tfor (LayerManager::LayerLoaded &layer : executionQueue_) {\n> +\t\tif (layer.vtable->queueRequest) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &layer));\n> +\t\t\tlayer.vtable->queueRequest(closure, request);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void LayerManager::start(const Camera *camera, const ControlList *controls)\n> +{\n> +\tfor (LayerManager::LayerLoaded &layer : executionQueue_) {\n> +\t\tif (layer.vtable->start) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &layer));\n> +\t\t\tlayer.vtable->start(closure, controls);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void LayerManager::stop(const Camera *camera)\n> +{\n> +\tfor (LayerManager::LayerLoaded &layer : executionQueue_) {\n> +\t\tif (layer.vtable->stop) {\n> +\t\t\tvoid *closure = closures_.at(std::make_tuple(camera, &layer));\n> +\t\t\tlayer.vtable->stop(closure);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 6a71b2903d27..0c2086a8399c 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -40,6 +40,7 @@ libcamera_internal_sources = files([\n> 'ipc_pipe.cpp',\n> 'ipc_pipe_unixsocket.cpp',\n> 'ipc_unixsocket.cpp',\n> + 'layer_manager.cpp',\n> 'mapped_framebuffer.cpp',\n> 'matrix.cpp',\n> 'media_device.cpp',\n> diff --git a/src/meson.build b/src/meson.build\n> index 8eb8f05b362f..37368b01cbf2 100644\n> --- a/src/meson.build\n> +++ b/src/meson.build\n> @@ -63,6 +63,7 @@ subdir('libcamera')\n> \n> subdir('android')\n> subdir('ipa')\n> +subdir('layer')\n> \n> subdir('apps')\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 04244C3237\n\tfor ;\n\tSun, 27 Jul 2025 17:42:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA05169128;\n\tSun, 27 Jul 2025 19:42:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D30C669080\n\tfor ;\n\tSun, 27 Jul 2025 19:42:24 +0200 (CEST)","from [192.168.33.14] (185.221.140.39.nat.pool.zt.hu\n\t[185.221.140.39])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D74DA4C9;\n\tSun, 27 Jul 2025 19:41:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"UzB3FDoY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753638103;\n\tbh=afJipalVYe8Z5FF395SPOyN9vSYJDGNEX2BllYeUrHI=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=UzB3FDoYhU2UTRabFuDMdjZoU0alHbB/+lOzJ/d9InGf8gtmUYKz5uM+MLxy2eA2+\n\thEgsqggx0MkjmH8u920+nloXRwa/y90F48NmyJerGPf1FbCtz/+j7JvIyoQEZCRvaJ\n\tFlgcPgAs+9VkzfMp0XMqOauktNfxoHiLkWcjE5Qk=","Message-ID":"<61fd9a80-0d8e-431b-b16e-80457ac62957@ideasonboard.com>","Date":"Sun, 27 Jul 2025 19:42:19 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 4/8] libcamera: layer_manager: Add LayerManager\n\timplementation","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"kieran.bingham@ideasonboard.com","References":"<20250703114225.2074071-1-paul.elder@ideasonboard.com>\n\t<20250703114225.2074071-5-paul.elder@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250703114225.2074071-5-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35228,"web_url":"https://patchwork.libcamera.org/comment/35228/","msgid":"<175377667472.3836966.5766200675588374795@neptunite.rasen.tech>","date":"2025-07-29T08:11:14","subject":"Re: [PATCH v2 4/8] libcamera: layer_manager: Add LayerManager\n\timplementation","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-07-28 02:42:19)\n> Hi\n> \n> 2025. 07. 03. 13:42 keltezéssel, Paul Elder írta:\n> > We want to be able to implement layers in libcamera, which conceptually\n> > sit in between the Camera class and the application. This can be useful\n> > for implementing things that don't belong inside the Camera/IPA nor inside\n> > the application, such as intercepting and translation the AeEnable\n> > control, or implementing the Sync algorithm.\n> > \n> > To achieve this, first add a LayerManager implementation, which searches\n> > for and loads layers from shared object files, and orchestrates\n> > executing them. Actually calling into these functions from the Camera\n> > class will be added in the following patch.\n> > \n> > Signed-off-by: Paul Elder \n> > \n> > ---\n> > Changes in v2:\n> > - add closures to the layer interface\n> > - separate Layer into LayerInfo and LayerInterface, to separate out\n> > layer data and the vtable\n> > - remove generateConfiguration() and streams() from the layer interface\n> > - add init() and terminate() to the layer interface, and make them\n> > required (mainly for preparing and destroying the closure)\n> > - make LayerLoaded automatically dlclose on deconstruction, remove copy,\n> > and implement move constructors\n> > - cache controls and properties in the LayerManager\n> > ---\n> > include/libcamera/internal/layer_manager.h | 117 +++++++\n> > include/libcamera/internal/meson.build | 1 +\n> > include/libcamera/layer.h | 54 +++\n> > include/libcamera/meson.build | 1 +\n> > src/layer/meson.build | 10 +\n> > src/libcamera/layer_manager.cpp | 383 +++++++++++++++++++++\n> > src/libcamera/meson.build | 1 +\n> > src/meson.build | 1 +\n> > 8 files changed, 568 insertions(+)\n> > create mode 100644 include/libcamera/internal/layer_manager.h\n> > create mode 100644 include/libcamera/layer.h\n> > create mode 100644 src/layer/meson.build\n> > create mode 100644 src/libcamera/layer_manager.cpp\n> > \n> > diff --git a/include/libcamera/internal/layer_manager.h b/include/libcamera/internal/layer_manager.h\n> > new file mode 100644\n> > index 000000000000..0d108bcddf3d\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/layer_manager.h\n> > @@ -0,0 +1,117 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board Oy\n> > + *\n> > + * Layer manager interface\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DECLARE_CATEGORY(LayerManager)\n> > +\n> > +class LayerManager\n> > +{\n> > +public:\n> > + LayerManager();\n> > + ~LayerManager() = default;\n> > +\n> > + void init(const Camera *camera, const ControlList &properties,\n> > + const ControlInfoMap &controlInfoMap);\n> > + void terminate(const Camera *camera);\n> > +\n> > + void bufferCompleted(const Camera *camera,\n> > + Request *request, FrameBuffer *buffer);\n> > + void requestCompleted(const Camera *camera, Request *request);\n> > + void disconnected(const Camera *camera);\n> > +\n> > + void acquire(const Camera *camera);\n> > + void release(const Camera *camera);\n> > +\n> > + const ControlInfoMap &controls(const Camera *camera) const { return controls_.at(camera); }\n> > + const ControlList &properties(const Camera *camera) const { return properties_.at(camera); }\n> > +\n> > + void configure(const Camera *camera, const CameraConfiguration *config,\n> > + const ControlInfoMap &controlInfoMap);\n> > +\n> > + void createRequest(const Camera *camera,\n> > + uint64_t cookie, const Request *request);\n> > +\n> > + void queueRequest(const Camera *camera, Request *request);\n> > +\n> > + void start(const Camera *camera, const ControlList *controls);\n> > + void stop(const Camera *camera);\n> > +\n> > +private:\n> > + /* Extend the layer with information specific to load-handling */\n> > + struct LayerLoaded\n> > + {\n> > + LayerLoaded()\n> > + : info(nullptr), vtable(nullptr), dlHandle(nullptr)\n> \n> I would initialize the members where they are defined, and have `LayerLoaded() = default;`.\n\nAh ok. (I put this in v3)\n\n> \n> \n> > + {\n> > + }\n> > +\n> > + LayerLoaded(LayerLoaded &&other)\n> > + : info(other.info), vtable(other.vtable),\n> > + dlHandle(other.dlHandle)\n> > + {\n> > + other.dlHandle = nullptr;\n> > + }\n> > +\n> > + LayerLoaded &operator=(LayerLoaded &&other)\n> > + {\n> > + info = other.info;\n> > + vtable = other.vtable;\n> > + dlHandle = other.dlHandle;\n> > + other.dlHandle = nullptr;\n> > + return *this;\n> > + }\n> \n> I like to use `std::exchange()` for move constr/assignment because it makes things\n> easier in my opinion; but the current version also looks ok.\n> \n> \n> > +\n> > + ~LayerLoaded()\n> > + {\n> > + if (dlHandle)\n> > + dlclose(dlHandle);\n> > + }\n> > +\n> > + LayerInfo *info;\n> > + LayerInterface *vtable;\n> > + void *dlHandle;\n> > +\n> > + private:\n> > + LIBCAMERA_DISABLE_COPY(LayerLoaded)\n> > + };\n> > +\n> > + using ClosureKey = std::tuple;\n> > +\n> > + void updateProperties(const Camera *camera,\n> > + const ControlList &properties);\n> > + void updateControls(const Camera *camera,\n> > + const ControlInfoMap &controlInfoMap);\n> > +\n> > + LayerLoaded createLayer(const std::string &file);\n> > + std::deque executionQueue_;\n> > + std::map closures_;\n> \n> I feel like we should bike shed the actual interface a bit more.\n> \n> I would like to suggest something like the following:\n> \n> // -----------------\n> // plugin interface:\n> \n> struct libcamera_layer_vtable {\n> void (*destroy)(libcamera_layer *);\n> ...\n> };\n> \n> struct libcamera_layer_info {\n> const char *name;\n> ...\n> };\n\nI added something similar to this to v3 and had it ready to send already so I\nsent it to see what you think of it or if you want to bikeshed it more over\nthere :)\n\n> \n> struct libcamera_layer {\n> const struct libcamera_layer_info *info;\n> const struct libcamera_layer_vtable *vtable;\n> };\n> \n> struct libcamera_layer *layer_instantiate(Camera *, ...)\n> {\n> // create an instance for the given camera\n> }\n> \n> // or we could allow having multiple layers in the same dso by having one function:\n> \n> const struct libcamera_layer_info *layer_enumerate(uint32_t *idx);\n> \n> and then the info struct could have the `instantiate()` function ptr\n> \n> // ----------------\n> // libcamera parts:\n> \n> struct LayerPlugin {\n> void *dso;\n> \n> ~LayerPlugin() { dlclose(dso); }\n> };\n> \n> struct LayerLoaded {\n> std::shared_ptr plugin;\n> libcamera_layer *layer;\n> \n> ~LayerLoaded() { layer->vtable->destroy(layer); }\n> };\n> \n> \n> struct Camera::Private {\n> ...\n> std::vector layers;\n> };\n> \n> struct LayerManager {\n> // some kind of mapping/list of `LayerPlugin`s\n> \n> std::vector instantiateFor(Camera &camera)\n> {\n> // as long as there is no configuration file support, instantiate all of them or similar\n> }\n> };\n> \n> or something along these lines. I feel like we should get rid of the closure std::map.\n\nI did that in v3, by adding a LayerController per-Camera.\n\n\nThanks,\n\nPaul\n\n> In this setup the plugin could do something like this:\n> \n> struct MyLayer : libcamera_layer {\n> ...\n> };\n> \n> libcamera_layer *layer_instantiate(Camera *camera)\n> {\n> return new MyLayer(camera);\n> }\n> \n> and then in the implementations, `static_cast(...)` can be used.\n> Or e.g. in C, the same could be done, or if it's not the first member, then\n> an implementation of `container_of()`.\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n> \n> > +\n> > + std::map controls_;\n> > + std::map properties_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index 690f5c5ec9f6..20e6c295601f 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -29,6 +29,7 @@ libcamera_internal_headers = files([\n> > 'ipa_proxy.h',\n> > 'ipc_pipe.h',\n> > 'ipc_unixsocket.h',\n> > + 'layer_manager.h',\n> > 'mapped_framebuffer.h',\n> > 'matrix.h',\n> > 'media_device.h',\n> > diff --git a/include/libcamera/layer.h b/include/libcamera/layer.h\n> > new file mode 100644\n> > index 000000000000..cd0e26a3b72b\n> > --- /dev/null\n> > +++ b/include/libcamera/layer.h\n> > @@ -0,0 +1,54 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board Oy\n> > + *\n> > + * Layer interface\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +class CameraConfiguration;\n> > +class FrameBuffer;\n> > +class Request;\n> > +class Stream;\n> > +enum class StreamRole;\n> > +\n> > +struct LayerInfo {\n> > + const char *name;\n> > + int layerAPIVersion;\n> > +};\n> > +\n> > +struct LayerInterface {\n> > + void *(*init)(const std::string &id);\n> > + void (*terminate)(void *);\n> > +\n> > + void (*bufferCompleted)(void *, Request *, FrameBuffer *);\n> > + void (*requestCompleted)(void *, Request *);\n> > + void (*disconnected)(void *);\n> > +\n> > + void (*acquire)(void *);\n> > + void (*release)(void *);\n> > +\n> > + ControlInfoMap::Map (*controls)(void *, ControlInfoMap &);\n> > + ControlList (*properties)(void *, ControlList &);\n> > +\n> > + void (*configure)(void *, const CameraConfiguration *);\n> > +\n> > + void (*createRequest)(void *, uint64_t, const Request *);\n> > +\n> > + void (*queueRequest)(void *, Request *);\n> > +\n> > + void (*start)(void *, const ControlList *);\n> > + void (*stop)(void *);\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > index 30ea76f9470a..552af112abb5 100644\n> > --- a/include/libcamera/meson.build\n> > +++ b/include/libcamera/meson.build\n> > @@ -11,6 +11,7 @@ libcamera_public_headers = files([\n> > 'framebuffer.h',\n> > 'framebuffer_allocator.h',\n> > 'geometry.h',\n> > + 'layer.h',\n> > 'logging.h',\n> > 'orientation.h',\n> > 'pixel_format.h',\n> > diff --git a/src/layer/meson.build b/src/layer/meson.build\n> > new file mode 100644\n> > index 000000000000..dee5e5ac5804\n> > --- /dev/null\n> > +++ b/src/layer/meson.build\n> > @@ -0,0 +1,10 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +\n> > +layer_includes = [\n> > + libcamera_includes,\n> > +]\n> > +\n> > +layer_install_dir = libcamera_libdir / 'layers'\n> > +\n> > +config_h.set('LAYER_DIR',\n> > + '\"' + get_option('prefix') / layer_install_dir + '\"')\n> > diff --git a/src/libcamera/layer_manager.cpp b/src/libcamera/layer_manager.cpp\n> > new file mode 100644\n> > index 000000000000..d707d4e12a53\n> > --- /dev/null\n> > +++ b/src/libcamera/layer_manager.cpp\n> > @@ -0,0 +1,383 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board Oy\n> > + *\n> > + * Layer manager\n> > + */\n> > +\n> > +#include \"libcamera/internal/layer_manager.h\"\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \"libcamera/internal/utils.h\"\n> > +\n> > +/**\n> > + * \\file layer_manager.h\n> > + * \\brief Layer manager\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(LayerManager)\n> > +\n> > +/**\n> > + * \\class LayerManager\n> > + * \\brief Layer manager\n> > + *\n> > + * The Layer manager discovers layer implementations from disk, and creates\n> > + * execution queues for every function that is implemented by each layer and\n> > + * executes them. A layer is a layer that sits between libcamera and the\n> > + * application, and hooks into the public Camera interface.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Construct a LayerManager instance\n> > + *\n> > + * The LayerManager class is meant be instantiated by the Camera.\n> > + */\n> > +LayerManager::LayerManager()\n> > +{\n> > + std::map layers;\n> > +\n> > + /* \\todo Implement built-in layers */\n> > +\n> > + /* This returns the number of \"modules\" successfully loaded */\n> > + std::function addDirHandler =\n> > + [this, &layers](const std::string &file) {\n> > + LayerManager::LayerLoaded layer = createLayer(file);\n> > + if (!layer.info)\n> > + return 0;\n> > +\n> > + LOG(LayerManager, Debug) << \"Loaded layer '\" << file << \"'\";\n> > +\n> > + layers.emplace(std::string(layer.info->name), std::move(layer));\n> > +\n> > + return 1;\n> > + };\n> > +\n> > + /* User-specified paths take precedence. */\n> > + /* \\todo Document this */\n> > + const char *layerPaths = utils::secure_getenv(\"LIBCAMERA_LAYER_PATH\");\n> > + if (layerPaths) {\n> > + for (const auto &dir : utils::split(layerPaths, \":\")) {\n> > + if (dir.empty())\n> > + continue;\n> > +\n> > + /*\n> > + * \\todo Move the shared objects into one directory\n> > + * instead of each in their own subdir\n> > + */\n> > + utils::addDir(dir.c_str(), 1, addDirHandler);\n> > + }\n> > + }\n> > +\n> > + /*\n> > + * When libcamera is used before it is installed, load layers from the\n> > + * same build directory as the libcamera library itself.\n> > + */\n> > + std::string root = utils::libcameraBuildPath();\n> > + if (!root.empty()) {\n> > + std::string layerBuildPath = root + \"src/layer\";\n> > + constexpr int maxDepth = 2;\n> > +\n> > + LOG(LayerManager, Info)\n> > + << \"libcamera is not installed. Adding '\"\n> > + << layerBuildPath << \"' to the layer search path\";\n> > +\n> > + utils::addDir(layerBuildPath.c_str(), maxDepth, addDirHandler);\n> > + }\n> > +\n> > + /* Finally try to load layers from the installed system path. */\n> > + utils::addDir(LAYER_DIR, 1, addDirHandler);\n> > +\n> > + /* Order the layers */\n> > + /* \\todo Document this. First is closer to application, last is closer to libcamera */\n> > + const char *layerList = utils::secure_getenv(\"LIBCAMERA_LAYERS_ENABLE\");\n> > + if (layerList) {\n> > + for (const auto &layerName : utils::split(layerList, \":\")) {\n> > + if (layerName.empty())\n> > + continue;\n> > +\n> > + const auto &it = layers.find(layerName);\n> > + if (it == layers.end())\n> > + continue;\n> > +\n> > + executionQueue_.push_back(std::move(it->second));\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::init(const Camera *camera, const ControlList &properties,\n> > + const ControlInfoMap &controlInfoMap)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + void *closure = layer.vtable->init(camera->id());\n> > + closures_[std::make_tuple(camera, &layer)] = closure;\n> > + }\n> > +\n> > + /*\n> > + * We need to iterate over the layers individually to merge all of\n> > + * their controls, so we'll factor out updateControls() as it needs to be\n> > + * run again at configure().\n> > + */\n> > + updateProperties(camera, properties);\n> > + updateControls(camera, controlInfoMap);\n> > +}\n> > +\n> > +void LayerManager::terminate(const Camera *camera)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->terminate(closure);\n> > + }\n> > +}\n> > +\n> > +LayerManager::LayerLoaded LayerManager::createLayer(const std::string &filename)\n> > +{\n> > + LayerLoaded layer;\n> > +\n> > + File file{ filename };\n> > + if (!file.open(File::OpenModeFlag::ReadOnly)) {\n> > + LOG(LayerManager, Error) << \"Failed to open layer: \"\n> > + << strerror(-file.error());\n> > + return layer;\n> > + }\n> > +\n> > + Span data = file.map();\n> > + int ret = utils::elfVerifyIdent(data);\n> > + if (ret) {\n> > + LOG(LayerManager, Error) << \"Layer is not an ELF file\";\n> > + return layer;\n> > + }\n> > +\n> > + Span info = utils::elfLoadSymbol(data, \"layerInfo\");\n> > + if (info.size() < sizeof(LayerInfo)) {\n> > + LOG(LayerManager, Error) << \"Layer has no valid info\";\n> > + return layer;\n> > + }\n> > +\n> > + void *dlHandle = dlopen(file.fileName().c_str(), RTLD_LAZY);\n> > + if (!dlHandle) {\n> > + LOG(LayerManager, Error)\n> > + << \"Failed to open layer shared object: \"\n> > + << dlerror();\n> > + return layer;\n> > + }\n> > +\n> > + void *layerInfo = dlsym(dlHandle, \"layerInfo\");\n> > + if (!layerInfo) {\n> > + LOG(LayerManager, Error)\n> > + << \"Failed to load layerInfo from layer shared object: \"\n> > + << dlerror();\n> > + dlclose(dlHandle);\n> > + return layer;\n> > + }\n> > +\n> > + void *vtable = dlsym(dlHandle, \"layerInterface\");\n> > + if (!vtable) {\n> > + LOG(LayerManager, Error)\n> > + << \"Failed to load layerInterface from layer shared object: \"\n> > + << dlerror();\n> > + dlclose(dlHandle);\n> > + return layer;\n> > + }\n> > +\n> > + layer.info = static_cast(layerInfo);\n> > + layer.vtable = static_cast(vtable);\n> > + layer.dlHandle = dlHandle;\n> > +\n> > + /*\n> > + * No need to dlclose after this as the LayerLoaded deconstructor will\n> > + * handle it\n> > + */\n> > +\n> > + /* \\todo Implement this. It should come from the libcamera version */\n> > + if (layer.info->layerAPIVersion != 1) {\n> > + LOG(LayerManager, Error) << \"Layer API version mismatch\";\n> > + layer.info = nullptr;\n> > + return layer;\n> > + }\n> > +\n> > + /* \\todo Document these requirements */\n> > + if (!layer.vtable->init) {\n> > + LOG(LayerManager, Error) << \"Layer doesn't implement init\";\n> > + layer.info = nullptr;\n> > + return layer;\n> > + }\n> > +\n> > + /* \\todo Document these requirements */\n> > + if (!layer.vtable->terminate) {\n> > + LOG(LayerManager, Error) << \"Layer doesn't implement terminate\";\n> > + layer.info = nullptr;\n> > + return layer;\n> > + }\n> > +\n> > + /* \\todo Validate the layer name. */\n> > +\n> > + return layer;\n> > +}\n> > +\n> > +void LayerManager::bufferCompleted(const Camera *camera, Request *request, FrameBuffer *buffer)\n> > +{\n> > + /* Reverse order because this comes from a Signal emission */\n> > + for (auto it = executionQueue_.rbegin();\n> > + it != executionQueue_.rend(); it++) {\n> > + if ((*it).vtable->bufferCompleted) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> > + (*it).vtable->bufferCompleted(closure, request, buffer);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::requestCompleted(const Camera *camera, Request *request)\n> > +{\n> > + /* Reverse order because this comes from a Signal emission */\n> > + for (auto it = executionQueue_.rbegin();\n> > + it != executionQueue_.rend(); it++) {\n> > + if ((*it).vtable->requestCompleted) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> > + (*it).vtable->requestCompleted(closure, request);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::disconnected(const Camera *camera)\n> > +{\n> > + /* Reverse order because this comes from a Signal emission */\n> > + for (auto it = executionQueue_.rbegin();\n> > + it != executionQueue_.rend(); it++) {\n> > + if ((*it).vtable->disconnected) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &(*it)));\n> > + (*it).vtable->disconnected(closure);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::acquire(const Camera *camera)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->acquire) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->acquire(closure);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::release(const Camera *camera)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->release) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->release(closure);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::updateProperties(const Camera *camera,\n> > + const ControlList &properties)\n> > +{\n> > + ControlList props = properties;\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->properties) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + ControlList ret = layer.vtable->properties(closure, props);\n> > + props.merge(ret, ControlList::MergePolicy::OverwriteExisting);\n> > + }\n> > + }\n> > + properties_[camera] = props;\n> > +}\n> > +\n> > +void LayerManager::updateControls(const Camera *camera,\n> > + const ControlInfoMap &controlInfoMap)\n> > +{\n> > + ControlInfoMap infoMap = controlInfoMap;\n> > + /* \\todo Simplify this once ControlInfoMaps become easier to modify */\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->controls) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + ControlInfoMap::Map ret = layer.vtable->controls(closure, infoMap);\n> > + ControlInfoMap::Map map;\n> > + /* Merge the layer's ret later so that layers can overwrite */\n> > + for (auto &pair : infoMap)\n> > + map.insert(pair);\n> > + for (auto &pair : ret)\n> > + map.insert(pair);\n> > + infoMap = ControlInfoMap(std::move(map),\n> > + libcamera::controls::controls);\n> > + }\n> > + }\n> > + controls_[camera] = infoMap;\n> > +}\n> > +\n> > +void LayerManager::configure(const Camera *camera,\n> > + const CameraConfiguration *config,\n> > + const ControlInfoMap &controlInfoMap)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->configure) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->configure(closure, config);\n> > + }\n> > + }\n> > +\n> > + updateControls(camera, controlInfoMap);\n> > +}\n> > +\n> > +void LayerManager::createRequest(const Camera *camera, uint64_t cookie, const Request *request)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->createRequest) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->createRequest(closure, cookie, request);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::queueRequest(const Camera *camera, Request *request)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->queueRequest) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->queueRequest(closure, request);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::start(const Camera *camera, const ControlList *controls)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->start) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->start(closure, controls);\n> > + }\n> > + }\n> > +}\n> > +\n> > +void LayerManager::stop(const Camera *camera)\n> > +{\n> > + for (LayerManager::LayerLoaded &layer : executionQueue_) {\n> > + if (layer.vtable->stop) {\n> > + void *closure = closures_.at(std::make_tuple(camera, &layer));\n> > + layer.vtable->stop(closure);\n> > + }\n> > + }\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 6a71b2903d27..0c2086a8399c 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -40,6 +40,7 @@ libcamera_internal_sources = files([\n> > 'ipc_pipe.cpp',\n> > 'ipc_pipe_unixsocket.cpp',\n> > 'ipc_unixsocket.cpp',\n> > + 'layer_manager.cpp',\n> > 'mapped_framebuffer.cpp',\n> > 'matrix.cpp',\n> > 'media_device.cpp',\n> > diff --git a/src/meson.build b/src/meson.build\n> > index 8eb8f05b362f..37368b01cbf2 100644\n> > --- a/src/meson.build\n> > +++ b/src/meson.build\n> > @@ -63,6 +63,7 @@ subdir('libcamera')\n> > \n> > subdir('android')\n> > subdir('ipa')\n> > +subdir('layer')\n> > \n> > subdir('apps')\n> > \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6C0AFBDC71\n\tfor ;\n\tTue, 29 Jul 2025 08:11:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 45D71691CC;\n\tTue, 29 Jul 2025 10:11:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 21A8769154\n\tfor ;\n\tTue, 29 Jul 2025 10:11:23 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:1bd7:17bc:587c:d8a0])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 4F45B42B;\n\tTue, 29 Jul 2025 10:10:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"G7nzQrO2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753776640;\n\tbh=//H96UjbSHYwnRKErwCxquGwX206QMfLlspqie7HUAs=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=G7nzQrO23ePrMaI+WGBRmexQuOUEPbm0BDNvtqXpOm6BH1X9ZjM8P2w5pYGmrRkiw\n\ts+qHBDFQ1kPv0aAoDv8yoe9G1q7rCnzuSKvwX3Ix9dR0VOYfNUi5jWYNR97f8RZg8w\n\tV0FRNGIVCzVDjmHTvVDym+71diYQul5JZjSEqI7w=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<61fd9a80-0d8e-431b-b16e-80457ac62957@ideasonboard.com>","References":"<20250703114225.2074071-1-paul.elder@ideasonboard.com>\n\t<20250703114225.2074071-5-paul.elder@ideasonboard.com>\n\t<61fd9a80-0d8e-431b-b16e-80457ac62957@ideasonboard.com>","Subject":"Re: [PATCH v2 4/8] libcamera: layer_manager: Add LayerManager\n\timplementation","From":"Paul Elder ","Cc":"kieran.bingham@ideasonboard.com","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 29 Jul 2025 17:11:14 +0900","Message-ID":"<175377667472.3836966.5766200675588374795@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]