[{"id":34652,"web_url":"https://patchwork.libcamera.org/comment/34652/","msgid":"<882d1a34-d548-4d04-a44a-2bb545131834@ideasonboard.com>","date":"2025-06-26T10:38:49","subject":"Re: [RFC PATCH 4/7] 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. 06. 26. 11:59 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> include/libcamera/internal/layer_manager.h | 74 +++++\n> include/libcamera/internal/meson.build | 1 +\n> include/libcamera/layer.h | 56 ++++\n> include/libcamera/meson.build | 1 +\n> src/layer/meson.build | 10 +\n> src/libcamera/layer_manager.cpp | 314 +++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> src/meson.build | 1 +\n> 8 files changed, 458 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..73ccad01bca0\n> --- /dev/null\n> +++ b/include/libcamera/internal/layer_manager.h\n> @@ -0,0 +1,74 @@\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> +\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();\n> +\n> +\tvoid bufferCompleted(Request *request, FrameBuffer *buffer);\n> +\tvoid requestCompleted(Request *request);\n> +\tvoid disconnected();\n> +\n> +\tvoid acquire();\n> +\tvoid release();\n> +\n> +\tconst ControlInfoMap &controls(const ControlInfoMap &controlInfoMap);\n> +\tconst ControlList &properties(const ControlList &properties);\n> +\tconst std::set &streams(const std::set &streams);\n> +\n> +\tvoid generateConfiguration(Span &roles,\n> +\t\t\t\t CameraConfiguration *config);\n> +\n> +\tvoid configure(CameraConfiguration *config);\n> +\n> +\tvoid createRequest(uint64_t cookie, Request *request);\n> +\n> +\tvoid queueRequest(Request *request);\n> +\n> +\tvoid start(const ControlList *controls);\n> +\tvoid stop();\n> +\n> +private:\n> +\t/* Extend the layer with information specific to load-handling */\n> +\tstruct LayerLoaded\n> +\t{\n> +\t\tLayer layer;\n\nAny reason for not just storing a pointer? The struct is not useful without\nthe DSO loaded, so I feel like a pointer is fine.\n\n\n> +\t\tvoid *dlHandle;\n\nThe destructor should probably close this. And please disable the copy\nconstructor/assignment, and implement move construction/assignment. If\nthat is done, I think you won't need `unique_ptr` anywhere.\n\nMight even make sense to add something simple since this is now the second user\nof the dl*() API, e.g.\n\n struct DLCloser { void operator()(void *p) { ::dlclose(p); } };\n using DSOPointer = std::unique_ptr;\n\nBut maybe we should wait for a third...\n\n\n> +\t};\n> +\n> +\tstd::unique_ptr createLayer(const std::string &file);\n> +\tstd::deque> executionQueue_;\n> +\n> +\tControlInfoMap controlInfoMap_;\n> +\tControlList properties_;\n> +\tstd::set streams_;\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..8fa0ee7d24e6\n> --- /dev/null\n> +++ b/include/libcamera/layer.h\n> @@ -0,0 +1,56 @@\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> +namespace libcamera {\n> +\n> +class CameraConfiguration;\n> +class ControlInfoMap;\n> +class ControlList;\n> +class FrameBuffer;\n> +class Request;\n> +class Stream;\n> +enum class StreamRole;\n> +\n> +struct Layer\n> +{\n> +\tconst char *name;\n> +\tint layerAPIVersion;\n> +\n> +\tvoid (*init)(const std::string &id);\n\nI haven't seen it mentioned, sorry if it was; but I think every method here\nwill definitely need some kind of closure pointer, like a `void *`.\n\nAnd I think I would separate the vtable from the other fields.\n\nOr given that this is currently C++ dependent, I think using C++\nvirtual might not be not an issue.\n\n\n> +\n> +\tvoid (*bufferCompleted)(Request *, FrameBuffer *);\n> +\tvoid (*requestCompleted)(Request *);\n> +\tvoid (*disconnected)();\n> +\n> +\tvoid (*acquire)();\n> +\tvoid (*release)();\n> +\n> +\tControlInfoMap::Map (*controls)(ControlInfoMap &);\n> +\tControlList (*properties)(ControlList &);\n> +\tstd::set (*streams)(std::set &);\n> +\n> +\tvoid (*generateConfiguration)(Span &,\n> +\t\t\t\t CameraConfiguration *);\n> +\n> +\tvoid (*configure)(CameraConfiguration *);\n> +\n> +\tvoid (*createRequest)(uint64_t, Request *);\n> +\n> +\tvoid (*queueRequest)(Request *);\n> +\n> +\tvoid (*start)(const ControlList *);\n> +\tvoid (*stop)();\n> +} __attribute__((packed));\n\nI am not sure if this needs to be packed? But if it does, could you use\n`struct [[gnu::packed]] Layer {` notation?\n\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..96d53d4fc75d\n> --- /dev/null\n> +++ b/src/libcamera/layer_manager.cpp\n> @@ -0,0 +1,314 @@\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> +\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\nIs there any issue with loading all desired layers at `CameraManager`\nconstruction time, and instantiating them it for each camera that\nis registered?\n\n\n> + */\n> +LayerManager::LayerManager()\n> +{\n> +\tstd::map> 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\tstd::unique_ptr layer = createLayer(file);\n> +\t\tif (!layer)\n> +\t\t\treturn 0;\n> +\n> +\t\tLOG(LayerManager, Debug) << \"Loaded layer '\" << file << \"'\";\n> +\n> +\t\tlayers.insert({std::string(layer->layer.name), std::move(layer)});\n> +\n> +\t\treturn 1;\n> +\t};\n> +\n> +\t/* User-specified paths take precedence. */\n> +\tconst char *layerPaths = utils::secure_getenv(\"LIBCAMERA_LAYER_PATH\");\n\nCan we please make sure that `LIBCAMERA_LAYER_PATH` can be used in the build\ndirectory as well?\n\n\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\tutils::addDir(dir.c_str(), 0, 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, 0, 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\nLayers still in `layers` leak their DSOs on destruction. Or am I missing something?\n\n\n> +}\n> +\n> +LayerManager::~LayerManager()\n> +{\n> +\tfor (auto &layer : executionQueue_)\n> +\t\tdlclose(layer->dlHandle);\n\nThis should be handled in the destructor of each individual object in my opinion.\n\n\n> +}\n> +\n> +std::unique_ptr LayerManager::createLayer(const std::string &filename)\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 nullptr;\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 nullptr;\n> +\t}\n> +\n> +\tSpan info = utils::elfLoadSymbol(data, \"layerInfo\");\n> +\tif (info.size() < sizeof(Layer)) {\n> +\t\tLOG(LayerManager, Error) << \"Layer has no valid info\";\n> +\t\treturn nullptr;\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 nullptr;\n> +\t}\n> +\n> +\tvoid *symbol = dlsym(dlHandle, \"layerInfo\");\n> +\tif (!symbol) {\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\tdlHandle = nullptr;\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tstd::unique_ptr layer =\n> +\t\tstd::make_unique();\n> +\tlayer->layer = *reinterpret_cast(symbol);\n> +\n> +\t/* \\todo Implement this. It should come from the libcamera version */\n> +\tif (layer->layer.layerAPIVersion != 1) {\n> +\t\tLOG(LayerManager, Error) << \"Layer API version mismatch\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\t/* \\todo Validate the layer name. */\n> +\n> +\tlayer->dlHandle = dlHandle;\n> +\n> +\treturn layer;\n> +}\n> [...]\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 8e2aa921a620..226a94768514 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\nRegards,\nBarnabás Pőcze","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 BAF89C3237\n\tfor ;\n\tThu, 26 Jun 2025 10:38:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A937B68DF9;\n\tThu, 26 Jun 2025 12:38:55 +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 6737F68DF3\n\tfor ;\n\tThu, 26 Jun 2025 12:38:53 +0200 (CEST)","from [192.168.33.8] (185.221.143.107.nat.pool.zt.hu\n\t[185.221.143.107])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 72B32743;\n\tThu, 26 Jun 2025 12:38:34 +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=\"bQI8T/a8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750934314;\n\tbh=R7bA0rrbjwYagaJ6QIqvoUzTtLGOAaXi6cjWwb4xvMY=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=bQI8T/a8bQl4w036b6KUB1wIc0V3+8Te0F+A5obBPPF36UapNGKJAOaXmOsveuPgf\n\tGOkkvZmEpWxTGKLV1NsUf32C3u/aowCv/ac4/R7lL6hwoTeryGUe+cN3hllE7iaNH0\n\trU+iLCIO7zkIHOgmpDgbb9bqmwYNVE9OYcLCYSlE=","Message-ID":"<882d1a34-d548-4d04-a44a-2bb545131834@ideasonboard.com>","Date":"Thu, 26 Jun 2025 12:38:49 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH 4/7] libcamera: layer_manager: Add LayerManager\n\timplementation","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"kieran.bingham@ideasonboard.com","References":"<20250626095944.1746345-1-paul.elder@ideasonboard.com>\n\t<20250626095944.1746345-5-paul.elder@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250626095944.1746345-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":34653,"web_url":"https://patchwork.libcamera.org/comment/34653/","msgid":"<175093433919.3962368.15084412623261115761@ping.linuxembedded.co.uk>","date":"2025-06-26T10:38:59","subject":"Re: [RFC PATCH 4/7] 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-06-26 10:59:39)\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\nAt some point - I wonder if we need module signatures too to decide\nwhich layers we trust ... or potentially have different levels of\ncapabilities.\n\nParticularly important when we start looking at loadable layers that can\n'see' the captured images ...\n\nMy \"definitely-doesnt-stream-video-live-to-youtube.so\" is sure to\nbe trustedworthy ...\n\nBut for now this is a great start on the framework and plumbing!\n\n> \n> Signed-off-by: Paul Elder \n> ---\n> include/libcamera/internal/layer_manager.h | 74 +++++\n> include/libcamera/internal/meson.build | 1 +\n> include/libcamera/layer.h | 56 ++++\n> include/libcamera/meson.build | 1 +\n> src/layer/meson.build | 10 +\n> src/libcamera/layer_manager.cpp | 314 +++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> src/meson.build | 1 +\n> 8 files changed, 458 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..73ccad01bca0\n> --- /dev/null\n> +++ b/include/libcamera/internal/layer_manager.h\n> @@ -0,0 +1,74 @@\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> +\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();\n> +\n> + void bufferCompleted(Request *request, FrameBuffer *buffer);\n> + void requestCompleted(Request *request);\n> + void disconnected();\n> +\n> + void acquire();\n> + void release();\n> +\n> + const ControlInfoMap &controls(const ControlInfoMap &controlInfoMap);\n> + const ControlList &properties(const ControlList &properties);\n> + const std::set &streams(const std::set &streams);\n> +\n> + void generateConfiguration(Span &roles,\n> + CameraConfiguration *config);\n> +\n> + void configure(CameraConfiguration *config);\n> +\n> + void createRequest(uint64_t cookie, Request *request);\n> +\n> + void queueRequest(Request *request);\n> +\n> + void start(const ControlList *controls);\n> + void stop();\n> +\n> +private:\n> + /* Extend the layer with information specific to load-handling */\n> + struct LayerLoaded\n> + {\n> + Layer layer;\n> + void *dlHandle;\n> + };\n> +\n> + std::unique_ptr createLayer(const std::string &file);\n> + std::deque> executionQueue_;\n> +\n> + ControlInfoMap controlInfoMap_;\n> + ControlList properties_;\n> + std::set streams_;\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..8fa0ee7d24e6\n> --- /dev/null\n> +++ b/include/libcamera/layer.h\n> @@ -0,0 +1,56 @@\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> +namespace libcamera {\n> +\n> +class CameraConfiguration;\n> +class ControlInfoMap;\n> +class ControlList;\n> +class FrameBuffer;\n> +class Request;\n> +class Stream;\n> +enum class StreamRole;\n> +\n> +struct Layer\n> +{\n> + const char *name;\n> + int layerAPIVersion;\n> +\n> + void (*init)(const std::string &id);\n> +\n> + void (*bufferCompleted)(Request *, FrameBuffer *);\n> + void (*requestCompleted)(Request *);\n> + void (*disconnected)();\n> +\n> + void (*acquire)();\n> + void (*release)();\n> +\n> + ControlInfoMap::Map (*controls)(ControlInfoMap &);\n> + ControlList (*properties)(ControlList &);\n> + std::set (*streams)(std::set &);\n> +\n> + void (*generateConfiguration)(Span &,\n> + CameraConfiguration *);\n> +\n> + void (*configure)(CameraConfiguration *);\n> +\n> + void (*createRequest)(uint64_t, Request *);\n> +\n> + void (*queueRequest)(Request *);\n> +\n> + void (*start)(const ControlList *);\n> + void (*stop)();\n> +} __attribute__((packed));\n\nI'm curious, Why is this packed ?\n\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..96d53d4fc75d\n> --- /dev/null\n> +++ b/src/libcamera/layer_manager.cpp\n> @@ -0,0 +1,314 @@\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> +\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> + /* This returns the number of \"modules\" successfully loaded */\n> + std::function addDirHandler =\n> + [this, &layers](const std::string &file) {\n> + std::unique_ptr layer = createLayer(file);\n> + if (!layer)\n> + return 0;\n> +\n> + LOG(LayerManager, Debug) << \"Loaded layer '\" << file << \"'\";\n> +\n> + layers.insert({std::string(layer->layer.name), std::move(layer)});\n> +\n> + return 1;\n> + };\n> +\n> + /* User-specified paths take precedence. */\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> + utils::addDir(dir.c_str(), 0, 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, 0, 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> +LayerManager::~LayerManager()\n> +{\n> + for (auto &layer : executionQueue_)\n> + dlclose(layer->dlHandle);\n> +}\n> +\n> +std::unique_ptr LayerManager::createLayer(const std::string &filename)\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 nullptr;\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 nullptr;\n> + }\n> +\n> + Span info = utils::elfLoadSymbol(data, \"layerInfo\");\n> + if (info.size() < sizeof(Layer)) {\n> + LOG(LayerManager, Error) << \"Layer has no valid info\";\n> + return nullptr;\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 nullptr;\n> + }\n> +\n> + void *symbol = dlsym(dlHandle, \"layerInfo\");\n> + if (!symbol) {\n> + LOG(LayerManager, Error)\n> + << \"Failed to load layerInfo from layer shared object: \"\n> + << dlerror();\n> + dlclose(dlHandle);\n> + dlHandle = nullptr;\n> + return nullptr;\n> + }\n> +\n> + std::unique_ptr layer =\n> + std::make_unique();\n> + layer->layer = *reinterpret_cast(symbol);\n> +\n> + /* \\todo Implement this. It should come from the libcamera version */\n> + if (layer->layer.layerAPIVersion != 1) {\n> + LOG(LayerManager, Error) << \"Layer API version mismatch\";\n> + return nullptr;\n> + }\n> +\n> + /* \\todo Validate the layer name. */\n> +\n> + layer->dlHandle = dlHandle;\n> +\n> + return layer;\n> +}\n> +\n> +void LayerManager::bufferCompleted(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)->layer.bufferCompleted)\n> + (*it)->layer.bufferCompleted(request, buffer);\n> + }\n> +}\n> +\n> +void LayerManager::requestCompleted(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)->layer.requestCompleted)\n> + (*it)->layer.requestCompleted(request);\n> + }\n> +}\n> +\n> +void LayerManager::disconnected()\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)->layer.disconnected)\n> + (*it)->layer.disconnected();\n> + }\n> +}\n> +\n> +void LayerManager::acquire()\n> +{\n> + for (std::unique_ptr &layer : executionQueue_)\n> + if (layer->layer.acquire)\n> + layer->layer.acquire();\n> +}\n> +\n> +void LayerManager::release()\n> +{\n> + for (std::unique_ptr &layer : executionQueue_)\n> + if (layer->layer.release)\n> + layer->layer.release();\n> +}\n> +\n> +const ControlInfoMap &LayerManager::controls(const ControlInfoMap &controlInfoMap)\n> +{\n\nI'm a bit surprised here actually - \n\nWhat happens if the applications calls Camera->controls() multiple\ntimes here?\n\nI don't think I would manipulate controls in this call - rather I would\nsuspect we should have a LayerManager::init(ControlInfoMap\n&controlInfoMap) which would be called after the IPA has initialised\nit's controls into the same map ...\n\nThen there would be no indirection with LayerManager::controls() that's\nsimply returns the single configured map.\n\n\n> + controlInfoMap_ = controlInfoMap;\n> +\n> + /* \\todo Simplify this once ControlInfoMaps become easier to modify */\n> + for (std::unique_ptr &layer : executionQueue_) {\n> + if (layer->layer.controls) {\n> + ControlInfoMap::Map ret = layer->layer.controls(controlInfoMap_);\n> + ControlInfoMap::Map map;\n> + /* Merge the layer's ret later so that layers can overwrite */\n> + for (auto &pair : controlInfoMap_)\n> + map.insert(pair);\n> + for (auto &pair : ret)\n> + map.insert(pair);\n> + controlInfoMap_ = ControlInfoMap(std::move(map),\n> + libcamera::controls::controls);\n> + }\n> + }\n> + return controlInfoMap_;\n> +}\n> +\n> +const ControlList &LayerManager::properties(const ControlList &properties)\n> +{\n\nSimilar here I think - this should probably just happen 'once' at\n::init() and leave the accessor to return the const data without\nmodifying it?\n\n> + properties_ = properties;\n> + for (std::unique_ptr &layer : executionQueue_) {\n> + if (layer->layer.properties) {\n> + ControlList ret = layer->layer.properties(properties_);\n> + properties_.merge(ret, ControlList::MergePolicy::OverwriteExisting);\n> + }\n> + }\n> + return properties_;\n> +}\n> +\n> +const std::set &LayerManager::streams(const std::set &streams)\n> +{\n> + streams_ = streams;\n> + for (std::unique_ptr &layer : executionQueue_) {\n> + if (layer->layer.streams) {\n> + std::set ret = layer->layer.streams(streams_);\n> + streams_.insert(ret.begin(), ret.end());\n> + }\n> + }\n> + return streams_;\n\nOhhhh I was about to ask why would we have streams ... but indeed I\ncould imagine a layer that could generate processed streams ...\n\nEeek though for again with the modifying 'const' state.\n\n\n> +}\n> +\n> +void LayerManager::generateConfiguration(Span &roles,\n> + CameraConfiguration *config)\n> +{\n> + for (std::unique_ptr &layer : executionQueue_)\n> + if (layer->layer.generateConfiguration)\n> + layer->layer.generateConfiguration(roles, config);\n\nI don't think we can have generateConfiguration without validate\nconfiguration ...\n\nAlso - maybe we should consider extra stream handling 'later', as both\nthis and streams() is going to be tricky. Layers are going to do\nconfiguration validation and see extra streams that they don't know\nabout and they'll just chop them out of the config. So I'm not sure how\nthese parts could work yet...\n\n\n> +}\n> +\n> +void LayerManager::configure(CameraConfiguration *config)\n> +{\n> + for (std::unique_ptr &layer : executionQueue_)\n> + if (layer->layer.configure)\n> + layer->layer.configure(config);\n\nI think this is valid info for a layer to be told though so this is a\ngood hook.\n\n> +}\n> +\n> +void LayerManager::createRequest(uint64_t cookie, Request *request)\n> +{\n> + for (std::unique_ptr &layer : executionQueue_)\n> + if (layer->layer.createRequest)\n> + layer->layer.createRequest(cookie, request);\n\nI'm not sure I'd hook this - What would a layer do when creating a\nrequest ? Or perhaps it's so they can store some private data or a map ?\nbut I suspect cookie and request should be const here if so.\n\nUnless layers could replace requests on the way in an back out :S\n\n\n> +}\n> +\n> +void LayerManager::queueRequest(Request *request)\n> +{\n> + for (std::unique_ptr &layer : executionQueue_)\n> + if (layer->layer.queueRequest)\n> + layer->layer.queueRequest(request);\n> +}\n> +\n> +void LayerManager::start(const ControlList *controls)\n> +{\n> + for (std::unique_ptr &layer : executionQueue_)\n> + if (layer->layer.start)\n> + layer->layer.start(controls);\n> +}\n> +\n> +void LayerManager::stop()\n> +{\n> + for (std::unique_ptr &layer : executionQueue_)\n> + if (layer->layer.stop)\n> + layer->layer.stop();\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 8e2aa921a620..226a94768514 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 0F509C3237\n\tfor ;\n\tThu, 26 Jun 2025 10:39:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B0A6068DFD;\n\tThu, 26 Jun 2025 12:39:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BEEE668DF6\n\tfor ;\n\tThu, 26 Jun 2025 12:39:01 +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 15F2D743;\n\tThu, 26 Jun 2025 12:38:43 +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=\"qK+DeHAv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750934323;\n\tbh=TRA41p7ves4rDfJohcdkWjQWOOztAk/UwP/O0lXKHD0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=qK+DeHAvMdLsOFiLNuTFAQUy992azoDyUYzO5FlBWYqInQqrm17uX40Z301USYrNy\n\tmifdHDyFyp/JQ1yo1fJYXyezmbKK+w8BDBWmCdiw0HZzZJxGSnXIfrKruPRfjg0cYa\n\tVYnCrf2CCulhJC5KYfb2NVtCueKuKV4Q1cJGPDaQ=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250626095944.1746345-5-paul.elder@ideasonboard.com>","References":"<20250626095944.1746345-1-paul.elder@ideasonboard.com>\n\t<20250626095944.1746345-5-paul.elder@ideasonboard.com>","Subject":"Re: [RFC PATCH 4/7] libcamera: layer_manager: Add LayerManager\n\timplementation","From":"Kieran Bingham ","Cc":"Paul Elder ","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 26 Jun 2025 11:38:59 +0100","Message-ID":"<175093433919.3962368.15084412623261115761@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":34684,"web_url":"https://patchwork.libcamera.org/comment/34684/","msgid":"<175101336938.2836253.15600344020193899175@neptunite.rasen.tech>","date":"2025-06-27T08:36:09","subject":"Re: [RFC PATCH 4/7] 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 Barnabás,\n\nThanks for the review.\n\nQuoting Barnabás Pőcze (2025-06-26 19:38:49)\n> Hi\n> \n> 2025. 06. 26. 11:59 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> > include/libcamera/internal/layer_manager.h | 74 +++++\n> > include/libcamera/internal/meson.build | 1 +\n> > include/libcamera/layer.h | 56 ++++\n> > include/libcamera/meson.build | 1 +\n> > src/layer/meson.build | 10 +\n> > src/libcamera/layer_manager.cpp | 314 +++++++++++++++++++++\n> > src/libcamera/meson.build | 1 +\n> > src/meson.build | 1 +\n> > 8 files changed, 458 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..73ccad01bca0\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/layer_manager.h\n> > @@ -0,0 +1,74 @@\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> > +\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();\n> > +\n> > + void bufferCompleted(Request *request, FrameBuffer *buffer);\n> > + void requestCompleted(Request *request);\n> > + void disconnected();\n> > +\n> > + void acquire();\n> > + void release();\n> > +\n> > + const ControlInfoMap &controls(const ControlInfoMap &controlInfoMap);\n> > + const ControlList &properties(const ControlList &properties);\n> > + const std::set &streams(const std::set &streams);\n> > +\n> > + void generateConfiguration(Span &roles,\n> > + CameraConfiguration *config);\n> > +\n> > + void configure(CameraConfiguration *config);\n> > +\n> > + void createRequest(uint64_t cookie, Request *request);\n> > +\n> > + void queueRequest(Request *request);\n> > +\n> > + void start(const ControlList *controls);\n> > + void stop();\n> > +\n> > +private:\n> > + /* Extend the layer with information specific to load-handling */\n> > + struct LayerLoaded\n> > + {\n> > + Layer layer;\n> \n> Any reason for not just storing a pointer? The struct is not useful without\n> the DSO loaded, so I feel like a pointer is fine.\n\nGood point, I missed that.\n\n> \n> \n> > + void *dlHandle;\n> \n> The destructor should probably close this. And please disable the copy\n> constructor/assignment, and implement move construction/assignment. If\n> that is done, I think you won't need `unique_ptr` anywhere.\n\nThat's a good idea.\n\n> \n> Might even make sense to add something simple since this is now the second user\n> of the dl*() API, e.g.\n> \n> struct DLCloser { void operator()(void *p) { ::dlclose(p); } };\n> using DSOPointer = std::unique_ptr;\n> \n> But maybe we should wait for a third...\n\nI think two might be enough... this does look nice.\n\n> \n> \n> > + };\n> > +\n> > + std::unique_ptr createLayer(const std::string &file);\n> > + std::deque> executionQueue_;\n> > +\n> > + ControlInfoMap controlInfoMap_;\n> > + ControlList properties_;\n> > + std::set streams_;\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..8fa0ee7d24e6\n> > --- /dev/null\n> > +++ b/include/libcamera/layer.h\n> > @@ -0,0 +1,56 @@\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> > +namespace libcamera {\n> > +\n> > +class CameraConfiguration;\n> > +class ControlInfoMap;\n> > +class ControlList;\n> > +class FrameBuffer;\n> > +class Request;\n> > +class Stream;\n> > +enum class StreamRole;\n> > +\n> > +struct Layer\n> > +{\n> > + const char *name;\n> > + int layerAPIVersion;\n> > +\n> > + void (*init)(const std::string &id);\n> \n> I haven't seen it mentioned, sorry if it was; but I think every method here\n> will definitely need some kind of closure pointer, like a `void *`.\n\nInstead of using closure pointers I instantiate one Layer per Camera. That's\nwhy there's one LayerManager per Camera.\n\nHm but yes closures would probably make things cleaner, and we wouldn't have to\ndo so many dlopens...\n\n> And I think I would separate the vtable from the other fields.\n\nYeah that's probably better. It did feel a bit weird mixing info and vtable.\n\n> Or given that this is currently C++ dependent, I think using C++\n> virtual might not be not an issue.\n> \n> \n> > +\n> > + void (*bufferCompleted)(Request *, FrameBuffer *);\n> > + void (*requestCompleted)(Request *);\n> > + void (*disconnected)();\n> > +\n> > + void (*acquire)();\n> > + void (*release)();\n> > +\n> > + ControlInfoMap::Map (*controls)(ControlInfoMap &);\n> > + ControlList (*properties)(ControlList &);\n> > + std::set (*streams)(std::set &);\n> > +\n> > + void (*generateConfiguration)(Span &,\n> > + CameraConfiguration *);\n> > +\n> > + void (*configure)(CameraConfiguration *);\n> > +\n> > + void (*createRequest)(uint64_t, Request *);\n> > +\n> > + void (*queueRequest)(Request *);\n> > +\n> > + void (*start)(const ControlList *);\n> > + void (*stop)();\n> > +} __attribute__((packed));\n> \n> I am not sure if this needs to be packed? But if it does, could you use\n> `struct [[gnu::packed]] Layer {` notation?\n\nI thought it was because we're dlsyming the struct.\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..96d53d4fc75d\n> > --- /dev/null\n> > +++ b/src/libcamera/layer_manager.cpp\n> > @@ -0,0 +1,314 @@\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> > +\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> Is there any issue with loading all desired layers at `CameraManager`\n> construction time, and instantiating them it for each camera that\n> is registered?\n\nWithout closure pointers, yes.\n\n> \n> \n> > + */\n> > +LayerManager::LayerManager()\n> > +{\n> > + std::map> layers;\n> > +\n> > + /* This returns the number of \"modules\" successfully loaded */\n> > + std::function addDirHandler =\n> > + [this, &layers](const std::string &file) {\n> > + std::unique_ptr layer = createLayer(file);\n> > + if (!layer)\n> > + return 0;\n> > +\n> > + LOG(LayerManager, Debug) << \"Loaded layer '\" << file << \"'\";\n> > +\n> > + layers.insert({std::string(layer->layer.name), std::move(layer)});\n> > +\n> > + return 1;\n> > + };\n> > +\n> > + /* User-specified paths take precedence. */\n> > + const char *layerPaths = utils::secure_getenv(\"LIBCAMERA_LAYER_PATH\");\n> \n> Can we please make sure that `LIBCAMERA_LAYER_PATH` can be used in the build\n> directory as well?\n\nDoes this not work?\n\n> \n> \n> > + if (layerPaths) {\n> > + for (const auto &dir : utils::split(layerPaths, \":\")) {\n> > + if (dir.empty())\n> > + continue;\n> > +\n> > + utils::addDir(dir.c_str(), 0, 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, 0, 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> Layers still in `layers` leak their DSOs on destruction. Or am I missing something?\n\nThey are indeed leaked...\n\n> \n> \n> > +}\n> > +\n> > +LayerManager::~LayerManager()\n> > +{\n> > + for (auto &layer : executionQueue_)\n> > + dlclose(layer->dlHandle);\n> \n> This should be handled in the destructor of each individual object in my opinion.\n\nYes.\n\n\nThanks,\n\nPaul\n\n> \n> \n> > +}\n> > +\n> > +std::unique_ptr LayerManager::createLayer(const std::string &filename)\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 nullptr;\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 nullptr;\n> > + }\n> > +\n> > + Span info = utils::elfLoadSymbol(data, \"layerInfo\");\n> > + if (info.size() < sizeof(Layer)) {\n> > + LOG(LayerManager, Error) << \"Layer has no valid info\";\n> > + return nullptr;\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 nullptr;\n> > + }\n> > +\n> > + void *symbol = dlsym(dlHandle, \"layerInfo\");\n> > + if (!symbol) {\n> > + LOG(LayerManager, Error)\n> > + << \"Failed to load layerInfo from layer shared object: \"\n> > + << dlerror();\n> > + dlclose(dlHandle);\n> > + dlHandle = nullptr;\n> > + return nullptr;\n> > + }\n> > +\n> > + std::unique_ptr layer =\n> > + std::make_unique();\n> > + layer->layer = *reinterpret_cast(symbol);\n> > +\n> > + /* \\todo Implement this. It should come from the libcamera version */\n> > + if (layer->layer.layerAPIVersion != 1) {\n> > + LOG(LayerManager, Error) << \"Layer API version mismatch\";\n> > + return nullptr;\n> > + }\n> > +\n> > + /* \\todo Validate the layer name. */\n> > +\n> > + layer->dlHandle = dlHandle;\n> > +\n> > + return layer;\n> > +}\n> > [...]\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 8e2aa921a620..226a94768514 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> Regards,\n> Barnabás Pőcze","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 1BF82BDCBF\n\tfor ;\n\tFri, 27 Jun 2025 08:36:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E447A68DFC;\n\tFri, 27 Jun 2025 10:36:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CB8C462C43\n\tfor ;\n\tFri, 27 Jun 2025 10:36:14 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:e17c:62b2:2597:dc1b])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id B93847E1;\n\tFri, 27 Jun 2025 10:35:54 +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=\"BnIw2enX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751013355;\n\tbh=TUr8FfBKImM66QboATo5n8AtJ79aXN9EWWyxitrQVIk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=BnIw2enXWDI9QjthHVrtCoNRHDxYGvs3QHFmLwxPg/BqzC91FGA1OzwIWY+HMbY6D\n\taNwIMOKlcIOXFVQ+ULEu/fMPhaH7xmRENelJlQ4YF6IhUsOWx476U8QQsNuCJ5q0Zb\n\txikncEM1vSHSGDaC+r2+R8j2L+NGOszEE0qFKizk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<882d1a34-d548-4d04-a44a-2bb545131834@ideasonboard.com>","References":"<20250626095944.1746345-1-paul.elder@ideasonboard.com>\n\t<20250626095944.1746345-5-paul.elder@ideasonboard.com>\n\t<882d1a34-d548-4d04-a44a-2bb545131834@ideasonboard.com>","Subject":"Re: [RFC PATCH 4/7] 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":"Fri, 27 Jun 2025 17:36:09 +0900","Message-ID":"<175101336938.2836253.15600344020193899175@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":34685,"web_url":"https://patchwork.libcamera.org/comment/34685/","msgid":"<175101374583.2836253.4237184301562237114@neptunite.rasen.tech>","date":"2025-06-27T08:42:25","subject":"Re: [RFC PATCH 4/7] 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 Kieran Bingham (2025-06-26 19:38:59)\n> Quoting Paul Elder (2025-06-26 10:59:39)\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> At some point - I wonder if we need module signatures too to decide\n> which layers we trust ... or potentially have different levels of\n> capabilities.\n> \n> Particularly important when we start looking at loadable layers that can\n> 'see' the captured images ...\n> \n> My \"definitely-doesnt-stream-video-live-to-youtube.so\" is sure to\n> be trustedworthy ...\n\nJust don't load \"definitely-doesnt-stream-video-live-to-youtube.so\" :)\n\nBut yes I think we'll need some sort of capabilities or signing or something.\n\n> \n> But for now this is a great start on the framework and plumbing!\n> \n> > \n> > Signed-off-by: Paul Elder \n> > ---\n> > include/libcamera/internal/layer_manager.h | 74 +++++\n> > include/libcamera/internal/meson.build | 1 +\n> > include/libcamera/layer.h | 56 ++++\n> > include/libcamera/meson.build | 1 +\n> > src/layer/meson.build | 10 +\n> > src/libcamera/layer_manager.cpp | 314 +++++++++++++++++++++\n> > src/libcamera/meson.build | 1 +\n> > src/meson.build | 1 +\n> > 8 files changed, 458 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..73ccad01bca0\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/layer_manager.h\n> > @@ -0,0 +1,74 @@\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> > +\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();\n> > +\n> > + void bufferCompleted(Request *request, FrameBuffer *buffer);\n> > + void requestCompleted(Request *request);\n> > + void disconnected();\n> > +\n> > + void acquire();\n> > + void release();\n> > +\n> > + const ControlInfoMap &controls(const ControlInfoMap &controlInfoMap);\n> > + const ControlList &properties(const ControlList &properties);\n> > + const std::set &streams(const std::set &streams);\n> > +\n> > + void generateConfiguration(Span &roles,\n> > + CameraConfiguration *config);\n> > +\n> > + void configure(CameraConfiguration *config);\n> > +\n> > + void createRequest(uint64_t cookie, Request *request);\n> > +\n> > + void queueRequest(Request *request);\n> > +\n> > + void start(const ControlList *controls);\n> > + void stop();\n> > +\n> > +private:\n> > + /* Extend the layer with information specific to load-handling */\n> > + struct LayerLoaded\n> > + {\n> > + Layer layer;\n> > + void *dlHandle;\n> > + };\n> > +\n> > + std::unique_ptr createLayer(const std::string &file);\n> > + std::deque> executionQueue_;\n> > +\n> > + ControlInfoMap controlInfoMap_;\n> > + ControlList properties_;\n> > + std::set streams_;\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..8fa0ee7d24e6\n> > --- /dev/null\n> > +++ b/include/libcamera/layer.h\n> > @@ -0,0 +1,56 @@\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> > +namespace libcamera {\n> > +\n> > +class CameraConfiguration;\n> > +class ControlInfoMap;\n> > +class ControlList;\n> > +class FrameBuffer;\n> > +class Request;\n> > +class Stream;\n> > +enum class StreamRole;\n> > +\n> > +struct Layer\n> > +{\n> > + const char *name;\n> > + int layerAPIVersion;\n> > +\n> > + void (*init)(const std::string &id);\n> > +\n> > + void (*bufferCompleted)(Request *, FrameBuffer *);\n> > + void (*requestCompleted)(Request *);\n> > + void (*disconnected)();\n> > +\n> > + void (*acquire)();\n> > + void (*release)();\n> > +\n> > + ControlInfoMap::Map (*controls)(ControlInfoMap &);\n> > + ControlList (*properties)(ControlList &);\n> > + std::set (*streams)(std::set &);\n> > +\n> > + void (*generateConfiguration)(Span &,\n> > + CameraConfiguration *);\n> > +\n> > + void (*configure)(CameraConfiguration *);\n> > +\n> > + void (*createRequest)(uint64_t, Request *);\n> > +\n> > + void (*queueRequest)(Request *);\n> > +\n> > + void (*start)(const ControlList *);\n> > + void (*stop)();\n> > +} __attribute__((packed));\n> \n> I'm curious, Why is this packed ?\n\nThe ideal reason: we need it for dlsyming it\n\nThe real reason: I copied it from IPAModule\n\n> \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..96d53d4fc75d\n> > --- /dev/null\n> > +++ b/src/libcamera/layer_manager.cpp\n> > @@ -0,0 +1,314 @@\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> > +\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> > + /* This returns the number of \"modules\" successfully loaded */\n> > + std::function addDirHandler =\n> > + [this, &layers](const std::string &file) {\n> > + std::unique_ptr layer = createLayer(file);\n> > + if (!layer)\n> > + return 0;\n> > +\n> > + LOG(LayerManager, Debug) << \"Loaded layer '\" << file << \"'\";\n> > +\n> > + layers.insert({std::string(layer->layer.name), std::move(layer)});\n> > +\n> > + return 1;\n> > + };\n> > +\n> > + /* User-specified paths take precedence. */\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> > + utils::addDir(dir.c_str(), 0, 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, 0, 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> > +LayerManager::~LayerManager()\n> > +{\n> > + for (auto &layer : executionQueue_)\n> > + dlclose(layer->dlHandle);\n> > +}\n> > +\n> > +std::unique_ptr LayerManager::createLayer(const std::string &filename)\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 nullptr;\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 nullptr;\n> > + }\n> > +\n> > + Span info = utils::elfLoadSymbol(data, \"layerInfo\");\n> > + if (info.size() < sizeof(Layer)) {\n> > + LOG(LayerManager, Error) << \"Layer has no valid info\";\n> > + return nullptr;\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 nullptr;\n> > + }\n> > +\n> > + void *symbol = dlsym(dlHandle, \"layerInfo\");\n> > + if (!symbol) {\n> > + LOG(LayerManager, Error)\n> > + << \"Failed to load layerInfo from layer shared object: \"\n> > + << dlerror();\n> > + dlclose(dlHandle);\n> > + dlHandle = nullptr;\n> > + return nullptr;\n> > + }\n> > +\n> > + std::unique_ptr layer =\n> > + std::make_unique();\n> > + layer->layer = *reinterpret_cast(symbol);\n> > +\n> > + /* \\todo Implement this. It should come from the libcamera version */\n> > + if (layer->layer.layerAPIVersion != 1) {\n> > + LOG(LayerManager, Error) << \"Layer API version mismatch\";\n> > + return nullptr;\n> > + }\n> > +\n> > + /* \\todo Validate the layer name. */\n> > +\n> > + layer->dlHandle = dlHandle;\n> > +\n> > + return layer;\n> > +}\n> > +\n> > +void LayerManager::bufferCompleted(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)->layer.bufferCompleted)\n> > + (*it)->layer.bufferCompleted(request, buffer);\n> > + }\n> > +}\n> > +\n> > +void LayerManager::requestCompleted(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)->layer.requestCompleted)\n> > + (*it)->layer.requestCompleted(request);\n> > + }\n> > +}\n> > +\n> > +void LayerManager::disconnected()\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)->layer.disconnected)\n> > + (*it)->layer.disconnected();\n> > + }\n> > +}\n> > +\n> > +void LayerManager::acquire()\n> > +{\n> > + for (std::unique_ptr &layer : executionQueue_)\n> > + if (layer->layer.acquire)\n> > + layer->layer.acquire();\n> > +}\n> > +\n> > +void LayerManager::release()\n> > +{\n> > + for (std::unique_ptr &layer : executionQueue_)\n> > + if (layer->layer.release)\n> > + layer->layer.release();\n> > +}\n> > +\n> > +const ControlInfoMap &LayerManager::controls(const ControlInfoMap &controlInfoMap)\n> > +{\n> \n> I'm a bit surprised here actually - \n> \n> What happens if the applications calls Camera->controls() multiple\n> times here?\n> \n> I don't think I would manipulate controls in this call - rather I would\n> suspect we should have a LayerManager::init(ControlInfoMap\n> &controlInfoMap) which would be called after the IPA has initialised\n> it's controls into the same map ...\n\nMost IPAs have updateControls() that trigger on configure()... so I\nthought we need to run this multiple times...\n\nOk I guess we can generate it on init() and configure() then.\n\n> Then there would be no indirection with LayerManager::controls() that's\n> simply returns the single configured map.\n\nYeah.\n\n> \n> > + controlInfoMap_ = controlInfoMap;\n> > +\n> > + /* \\todo Simplify this once ControlInfoMaps become easier to modify */\n> > + for (std::unique_ptr &layer : executionQueue_) {\n> > + if (layer->layer.controls) {\n> > + ControlInfoMap::Map ret = layer->layer.controls(controlInfoMap_);\n> > + ControlInfoMap::Map map;\n> > + /* Merge the layer's ret later so that layers can overwrite */\n> > + for (auto &pair : controlInfoMap_)\n> > + map.insert(pair);\n> > + for (auto &pair : ret)\n> > + map.insert(pair);\n> > + controlInfoMap_ = ControlInfoMap(std::move(map),\n> > + libcamera::controls::controls);\n> > + }\n> > + }\n> > + return controlInfoMap_;\n> > +}\n> > +\n> > +const ControlList &LayerManager::properties(const ControlList &properties)\n> > +{\n> \n> Similar here I think - this should probably just happen 'once' at\n> ::init() and leave the accessor to return the const data without\n> modifying it?\n\nYes.\n\n> \n> > + properties_ = properties;\n> > + for (std::unique_ptr &layer : executionQueue_) {\n> > + if (layer->layer.properties) {\n> > + ControlList ret = layer->layer.properties(properties_);\n> > + properties_.merge(ret, ControlList::MergePolicy::OverwriteExisting);\n> > + }\n> > + }\n> > + return properties_;\n> > +}\n> > +\n> > +const std::set &LayerManager::streams(const std::set &streams)\n> > +{\n> > + streams_ = streams;\n> > + for (std::unique_ptr &layer : executionQueue_) {\n> > + if (layer->layer.streams) {\n> > + std::set ret = layer->layer.streams(streams_);\n> > + streams_.insert(ret.begin(), ret.end());\n> > + }\n> > + }\n> > + return streams_;\n> \n> Ohhhh I was about to ask why would we have streams ... but indeed I\n> could imagine a layer that could generate processed streams ...\n> \n> Eeek though for again with the modifying 'const' state.\n\nThat's what controls() and properties() does too... and we at least need the\nformer for AeEnable/FPS and Sync...\n\n> \n> \n> > +}\n> > +\n> > +void LayerManager::generateConfiguration(Span &roles,\n> > + CameraConfiguration *config)\n> > +{\n> > + for (std::unique_ptr &layer : executionQueue_)\n> > + if (layer->layer.generateConfiguration)\n> > + layer->layer.generateConfiguration(roles, config);\n> \n> I don't think we can have generateConfiguration without validate\n> configuration ...\n\nGood point.\n\n> Also - maybe we should consider extra stream handling 'later', as both\n> this and streams() is going to be tricky. Layers are going to do\n> configuration validation and see extra streams that they don't know\n> about and they'll just chop them out of the config. So I'm not sure how\n> these parts could work yet...\n> \n> \n> > +}\n> > +\n> > +void LayerManager::configure(CameraConfiguration *config)\n> > +{\n> > + for (std::unique_ptr &layer : executionQueue_)\n> > + if (layer->layer.configure)\n> > + layer->layer.configure(config);\n> \n> I think this is valid info for a layer to be told though so this is a\n> good hook.\n> \n> > +}\n> > +\n> > +void LayerManager::createRequest(uint64_t cookie, Request *request)\n> > +{\n> > + for (std::unique_ptr &layer : executionQueue_)\n> > + if (layer->layer.createRequest)\n> > + layer->layer.createRequest(cookie, request);\n> \n> I'm not sure I'd hook this - What would a layer do when creating a\n> request ? Or perhaps it's so they can store some private data or a map ?\n\nThat was the idea.\n\n> but I suspect cookie and request should be const here if so.\n\nTrue.\n\n> Unless layers could replace requests on the way in an back out :S\n\nOk maybe we don't quite want that.\n\n\nThanks,\n\nPaul\n\n> \n> > +}\n> > +\n> > +void LayerManager::queueRequest(Request *request)\n> > +{\n> > + for (std::unique_ptr &layer : executionQueue_)\n> > + if (layer->layer.queueRequest)\n> > + layer->layer.queueRequest(request);\n> > +}\n> > +\n> > +void LayerManager::start(const ControlList *controls)\n> > +{\n> > + for (std::unique_ptr &layer : executionQueue_)\n> > + if (layer->layer.start)\n> > + layer->layer.start(controls);\n> > +}\n> > +\n> > +void LayerManager::stop()\n> > +{\n> > + for (std::unique_ptr &layer : executionQueue_)\n> > + if (layer->layer.stop)\n> > + layer->layer.stop();\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 8e2aa921a620..226a94768514 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 9AD30C3237\n\tfor ;\n\tFri, 27 Jun 2025 08:42:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6539B68DFC;\n\tFri, 27 Jun 2025 10:42:34 +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 DCFFD62C43\n\tfor ;\n\tFri, 27 Jun 2025 10:42:32 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:e17c:62b2:2597:dc1b])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id F15197E1;\n\tFri, 27 Jun 2025 10:42:12 +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=\"ZDXCWMpF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751013733;\n\tbh=lZ3SgxFos1V2pmSk1uPLPGI02MjRYzE/W1VlKoybyEY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ZDXCWMpFBzAR4WpVdNls7676lkSzPt2UMM7RRQtsAVMno8U4+B2xVZKrsY8e9OIQT\n\twl5QDxD8iFwOEIOmQDUanwjq2z6Z+QEasP9XJLNMilTF1sY99QV5Pbkf6EXr606dDd\n\tVvOZyV4Gk5q/bXAY3SuCFU24bViFDbyaRK8YPxVE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<175093433919.3962368.15084412623261115761@ping.linuxembedded.co.uk>","References":"<20250626095944.1746345-1-paul.elder@ideasonboard.com>\n\t<20250626095944.1746345-5-paul.elder@ideasonboard.com>\n\t<175093433919.3962368.15084412623261115761@ping.linuxembedded.co.uk>","Subject":"Re: [RFC PATCH 4/7] libcamera: layer_manager: Add LayerManager\n\timplementation","From":"Paul Elder ","Cc":"","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 27 Jun 2025 17:42:25 +0900","Message-ID":"<175101374583.2836253.4237184301562237114@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":34687,"web_url":"https://patchwork.libcamera.org/comment/34687/","msgid":"<87146bb1-b7e8-4be5-9def-d4272369f76e@ideasonboard.com>","date":"2025-06-27T08:51:18","subject":"Re: [RFC PATCH 4/7] 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. 06. 27. 10:36 keltezéssel, Paul Elder írta:\n> Hi Barnabás,\n> \n> Thanks for the review.\n> \n> Quoting Barnabás Pőcze (2025-06-26 19:38:49)\n>> Hi\n>>\n>> 2025. 06. 26. 11:59 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>>> include/libcamera/internal/layer_manager.h | 74 +++++\n>>> include/libcamera/internal/meson.build | 1 +\n>>> include/libcamera/layer.h | 56 ++++\n>>> include/libcamera/meson.build | 1 +\n>>> src/layer/meson.build | 10 +\n>>> src/libcamera/layer_manager.cpp | 314 +++++++++++++++++++++\n>>> src/libcamera/meson.build | 1 +\n>>> src/meson.build | 1 +\n>>> 8 files changed, 458 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> [...]\n>>> diff --git a/include/libcamera/layer.h b/include/libcamera/layer.h\n>>> new file mode 100644\n>>> index 000000000000..8fa0ee7d24e6\n>>> --- /dev/null\n>>> +++ b/include/libcamera/layer.h\n>>> @@ -0,0 +1,56 @@\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>>> +namespace libcamera {\n>>> +\n>>> +class CameraConfiguration;\n>>> +class ControlInfoMap;\n>>> +class ControlList;\n>>> +class FrameBuffer;\n>>> +class Request;\n>>> +class Stream;\n>>> +enum class StreamRole;\n>>> +\n>>> +struct Layer\n>>> +{\n>>> + const char *name;\n>>> + int layerAPIVersion;\n>>> +\n>>> + void (*init)(const std::string &id);\n>>\n>> I haven't seen it mentioned, sorry if it was; but I think every method here\n>> will definitely need some kind of closure pointer, like a `void *`.\n> \n> Instead of using closure pointers I instantiate one Layer per Camera. That's\n> why there's one LayerManager per Camera.\n> \n> Hm but yes closures would probably make things cleaner, and we wouldn't have to\n> do so many dlopens...\n\nYes, but dlopen()ing the same DSO in the same namespace will just return the same handle.\nSo using static variables in the layer wouldn't work with multiple cameras. So I think\nthis is pretty much required?\n\n\n> \n>> And I think I would separate the vtable from the other fields.\n> \n> Yeah that's probably better. It did feel a bit weird mixing info and vtable.\n> \n>> Or given that this is currently C++ dependent, I think using C++\n>> virtual might not be not an issue.\n>>\n>>\n>>> +\n>>> + void (*bufferCompleted)(Request *, FrameBuffer *);\n>>> + void (*requestCompleted)(Request *);\n>>> + void (*disconnected)();\n>>> +\n>>> + void (*acquire)();\n>>> + void (*release)();\n>>> +\n>>> + ControlInfoMap::Map (*controls)(ControlInfoMap &);\n>>> + ControlList (*properties)(ControlList &);\n>>> + std::set (*streams)(std::set &);\n>>> +\n>>> + void (*generateConfiguration)(Span &,\n>>> + CameraConfiguration *);\n>>> +\n>>> + void (*configure)(CameraConfiguration *);\n>>> +\n>>> + void (*createRequest)(uint64_t, Request *);\n>>> +\n>>> + void (*queueRequest)(Request *);\n>>> +\n>>> + void (*start)(const ControlList *);\n>>> + void (*stop)();\n>>> +} __attribute__((packed));\n>>\n>> I am not sure if this needs to be packed? But if it does, could you use\n>> `struct [[gnu::packed]] Layer {` notation?\n> \n> I thought it was because we're dlsyming the struct.\n\nThen I don't think that's needed.\n\n\n\n> \n>>> +\n>>> +} /* namespace libcamera */\n> [...]\n>>> +LayerManager::LayerManager()\n>>> +{\n>>> + std::map> layers;\n>>> +\n>>> + /* This returns the number of \"modules\" successfully loaded */\n>>> + std::function addDirHandler =\n>>> + [this, &layers](const std::string &file) {\n>>> + std::unique_ptr layer = createLayer(file);\n>>> + if (!layer)\n>>> + return 0;\n>>> +\n>>> + LOG(LayerManager, Debug) << \"Loaded layer '\" << file << \"'\";\n>>> +\n>>> + layers.insert({std::string(layer->layer.name), std::move(layer)});\n>>> +\n>>> + return 1;\n>>> + };\n>>> +\n>>> + /* User-specified paths take precedence. */\n>>> + const char *layerPaths = utils::secure_getenv(\"LIBCAMERA_LAYER_PATH\");\n>>\n>> Can we please make sure that `LIBCAMERA_LAYER_PATH` can be used in the build\n>> directory as well?\n> \n> Does this not work?\n\nI think you'll have to modify 0 -> 1 in the `addDir()` call since in the build\ndir each DSO is in its own folder (as far as I can see, not tested, sorry).\n\n\n> [...]\n\n\nRegards,\nBarnabás Pőcze","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 DA9EFC3237\n\tfor ;\n\tFri, 27 Jun 2025 08:51:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8CCF168DF4;\n\tFri, 27 Jun 2025 10:51:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C765E62C43\n\tfor ;\n\tFri, 27 Jun 2025 10:51:21 +0200 (CEST)","from [192.168.33.12] (185.221.143.107.nat.pool.zt.hu\n\t[185.221.143.107])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 533A6E7C;\n\tFri, 27 Jun 2025 10:51:02 +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=\"q7artQlx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751014262;\n\tbh=TRDOO8aovyXSgA8tkBT4vNifI5FtdrMV7kdhhx/mocI=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=q7artQlxKBMa/PtWhPrfGZ+lHNvdclcVJpZHVz2iJfc+sM2276B30HFMqKh68Dgwn\n\tNhYI4bs8YAfJ7twWcdb00QyPDUtyhpiPYz2aNW2S2Am83pNK43EdVyqEAAwRjTMkOU\n\tchY0nL78mOjYBZwF1oKt2oOqDG7zIdsnPGECSp5c=","Message-ID":"<87146bb1-b7e8-4be5-9def-d4272369f76e@ideasonboard.com>","Date":"Fri, 27 Jun 2025 10:51:18 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH 4/7] libcamera: layer_manager: Add LayerManager\n\timplementation","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"kieran.bingham@ideasonboard.com","References":"<20250626095944.1746345-1-paul.elder@ideasonboard.com>\n\t<20250626095944.1746345-5-paul.elder@ideasonboard.com>\n\t<882d1a34-d548-4d04-a44a-2bb545131834@ideasonboard.com>\n\t<175101336938.2836253.15600344020193899175@neptunite.rasen.tech>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<175101336938.2836253.15600344020193899175@neptunite.rasen.tech>","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\" "}}]