[{"id":37057,"web_url":"https://patchwork.libcamera.org/comment/37057/","msgid":"<176409166149.567526.3964367410159734360@ping.linuxembedded.co.uk>","date":"2025-11-25T17:27:41","subject":"Re: [PATCH v3 16/29] libcamera: converter: Add dw100 converter\n\tmodule","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-11-25 16:28:28)\n> The DW100 Dewarp engine is present on i.MX8MP SoC and possibly others.\n> This patch provides a dedicated converter module that allows easy\n> integration of such a dewarper into a pipeline handler.\n> \n> In this patch only the ScalerCrop control is implemented. Support for\n> additional functionality will be added in later patches.\n> \n> Signed-off-by: Stefan Klug \n> \n> ---\n> \n> Chnages in v3:\n> - Major rewrite to implement the libcamera controls in the converter class\n> instead of leaving that to the pipeline handler.\n\n\n \\o/ \\o/ \\o/ \\o/\n\nthank you :-)\n\n> - Moved V4L2 requests handling into the class instead of leaving that to\n> the pipeline handler\n> \n\nAnd this!\n\n> Changes in v2:\n> - Drop validateSize() as it is not used anywhere\n> \n> Changes in v0.9\n> - Use shared_ptr in constructor\n> ---\n> .../internal/converter/converter_dw100.h | 85 ++++\n> .../libcamera/internal/converter/meson.build | 1 +\n> src/libcamera/converter/converter_dw100.cpp | 413 ++++++++++++++++++\n> src/libcamera/converter/meson.build | 1 +\n> 4 files changed, 500 insertions(+)\n> create mode 100644 include/libcamera/internal/converter/converter_dw100.h\n> create mode 100644 src/libcamera/converter/converter_dw100.cpp\n> \n> diff --git a/include/libcamera/internal/converter/converter_dw100.h b/include/libcamera/internal/converter/converter_dw100.h\n> new file mode 100644\n> index 000000000000..8dd21a6228f1\n> --- /dev/null\n> +++ b/include/libcamera/internal/converter/converter_dw100.h\n> @@ -0,0 +1,85 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board Oy\n> + *\n> + * i.MX8MP Dewarp Engine integration\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/converter/converter_dw100_vertexmap.h\"\n> +#include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> +#include \"libcamera/internal/device_enumerator.h\"\n> +\n> +namespace libcamera {\n> +\n> +class MediaDevice;\n> +class Rectangle;\n> +class Stream;\n> +\n> +class ConverterDW100Module\n> +{\n> +public:\n> + virtual ~ConverterDW100Module() = default;\n> +\n> + static std::unique_ptr createModule(DeviceEnumerator *enumerator);\n> +\n> + int configure(const StreamConfiguration &inputCfg,\n> + const std::vector>\n> + &outputCfg);\n> + bool isConfigured(const Stream *stream) const;\n> +\n> + Size adjustInputSize(const PixelFormat &pixFmt, const Size &size,\n> + Converter::Alignment align = Converter::Alignment::Down);\n> + Size adjustOutputSize(const PixelFormat &pixFmt, const Size &size,\n> + Converter::Alignment align = Converter::Alignment::Down);\n> +\n> + int exportBuffers(const Stream *stream, unsigned int count,\n> + std::vector> *buffers);\n> + int validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> + Converter::Alignment align = Converter::Alignment::Down);\n> + int queueBuffers(FrameBuffer *input,\n> + const std::map &outputs);\n> +\n> + int start();\n> + void stop();\n> +\n> + void updateControlInfos(const Stream *stream, ControlInfoMap::Map &infos);\n> + void setControls(const Stream *stream, const ControlList &controls);\n> + void populateMetadata(const Stream *stream, ControlList &meta);\n> +\n> + void setSensorCrop(const Rectangle &rect);\n> + void setTransform(const Stream *stream, const Transform &transform);\n> +\n> + Signal inputBufferReady;\n> + Signal outputBufferReady;\n> +\n> +private:\n> + ConverterDW100Module(std::shared_ptr media);\n> +\n> + int applyControls(const Stream *stream, const V4L2Request *request);\n> + void reinitRequest(V4L2Request *request);\n> +\n> + struct VertexMapInfo {\n> + Dw100VertexMap map;\n> + bool update;\n> + };\n> +\n> + std::map vertexMaps_;\n> + unsigned int inputBufferCount_;\n> + V4L2M2MConverter converter_;\n> + Rectangle sensorCrop_;\n> + bool running_;\n> +\n> + std::vector> requests_;\n> + std::queue availableRequests_;\n> +};\n\nHard to know how 'modular' this is until we add more modules - but I\nvery much appreciate pulling it out to be something we can start\nworking towards.\n\nAnd that this will be easier to integrate in say the ISI pipeline\nhandler if desired later.\n\n\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/converter/meson.build b/include/libcamera/internal/converter/meson.build\n> index 9d586293f63a..128c644cb73f 100644\n> --- a/include/libcamera/internal/converter/meson.build\n> +++ b/include/libcamera/internal/converter/meson.build\n> @@ -1,6 +1,7 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> libcamera_internal_headers += files([\n> + 'converter_dw100.h',\n> 'converter_dw100_vertexmap.h',\n> 'converter_v4l2_m2m.h',\n> ])\n> diff --git a/src/libcamera/converter/converter_dw100.cpp b/src/libcamera/converter/converter_dw100.cpp\n> new file mode 100644\n> index 000000000000..cba7cc9f709b\n> --- /dev/null\n> +++ b/src/libcamera/converter/converter_dw100.cpp\n> @@ -0,0 +1,413 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board Oy\n> + *\n> + * i.MX8MP Dewarp Engine integration\n> + */\n> +\n> +#include \"libcamera/internal/converter/converter_dw100.h\"\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/converter.h\"\n> +#include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> +#include \"libcamera/internal/media_device.h\"\n> +#include \"libcamera/internal/v4l2_videodevice.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(Converter)\n> +\n> +/**\n> + * \\class libcamera::ConverterDW100Module\n> + * \\brief A converter module for the dw100 dewarper\n> + *\n> + * This class implements a converter module with direct support for libcamera\n> + * controls. Functionality wise it closely resembles the libcamera::Converter\n> + * interface. The main difference is that V4L2 requests are handled internally\n> + * and it has direct support for libcamera controls.\n> + */\n> +\n> +ConverterDW100Module::ConverterDW100Module(std::shared_ptr media)\n> + : converter_(media), running_(false)\n> +{\n> + converter_.outputBufferReady.connect(&this->outputBufferReady, &Signal::emit);\n> + converter_.inputBufferReady.connect(&this->inputBufferReady, &Signal::emit);\n> +}\n> +\n> +/**\n> + * \\brief Create a ConverterDW100Module\n> + * \\param[in] enumerator The enumerator\n> + *\n> + * Static factory function that searches for the dw100 device using the provided\n> + * \\a enumerator. If found, a ConverterDW100Module is instantiated and returned.\n> + *\n> + * \\return A ConverterDW100Module or null if no converter was found\n> + */\n> +std::unique_ptr\n> +ConverterDW100Module::createModule(DeviceEnumerator *enumerator)\n> +{\n> + DeviceMatch dwp(\"dw100\");\n> + dwp.add(\"dw100-source\");\n> + dwp.add(\"dw100-sink\");\n> +\n> + std::shared_ptr dwpMediaDevice = enumerator->search(dwp);\n> + if (!dwpMediaDevice)\n> + return {};\n> +\n> + std::unique_ptr dwpModule{ new ConverterDW100Module(dwpMediaDevice) };\n> + if (dwpModule->converter_.isValid())\n> + return dwpModule;\n> +\n> + LOG(Converter, Warning)\n> + << \"Found DW100 dewarper \" << dwpMediaDevice->deviceNode()\n> + << \" but invalid\";\n> + return {};\n> +}\n\nNice.\n\n\n> +\n> +/**\n> + * \\copydoc libcamera::V4L2M2MConverter::configure\n> + */\n> +int ConverterDW100Module::configure(const StreamConfiguration &inputCfg,\n> + const std::vector>\n> + &outputCfgs)\n> +{\n> + int ret;\n> +\n> + vertexMaps_.clear();\n> + ret = converter_.configure(inputCfg, outputCfgs);\n> + if (ret)\n> + return ret;\n> +\n> + inputBufferCount_ = inputCfg.bufferCount;\n> +\n> + for (auto &ref : outputCfgs) {\n> + const auto &outputCfg = ref.get();\n> + auto &info = vertexMaps_[outputCfg.stream()];\n> + auto &vertexMap = info.map;\n> + vertexMap.setInputSize(inputCfg.size);\n> + vertexMap.setOutputSize(outputCfg.size);\n> + vertexMap.setSensorCrop(sensorCrop_);\n> + info.update = true;\n> + }\n\nLooking forward to seeing multi-stream outputs extended for the i.MX8MP\n... but that's definitely later.\n\n> +\n> + return 0;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::V4L2M2MConverter::isConfigured\n> + */\n> +bool ConverterDW100Module::isConfigured(const Stream *stream) const\n> +{\n> + return vertexMaps_.find(stream) != vertexMaps_.end();\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::V4L2M2MConverter::adjustInputSize\n> + */\n> +Size ConverterDW100Module::adjustInputSize(const PixelFormat &pixFmt,\n> + const Size &size,\n> + Converter::Alignment align)\n> +{\n> + return converter_.adjustInputSize(pixFmt, size, align);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::V4L2M2MConverter::adjustOutputSize\n> + */\n> +Size ConverterDW100Module::adjustOutputSize(const PixelFormat &pixFmt,\n> + const Size &size,\n> + Converter::Alignment align)\n> +{\n> + return converter_.adjustOutputSize(pixFmt, size, align);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::V4L2M2MConverter::exportBuffers\n> + */\n> +int ConverterDW100Module::exportBuffers(const Stream *stream, unsigned int count,\n> + std::vector> *buffers)\n> +{\n> + return converter_.exportBuffers(stream, count, buffers);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::V4L2M2MConverter::validateOutput\n> + */\n> +int ConverterDW100Module::validateOutput(StreamConfiguration *cfg,\n> + bool *adjusted,\n> + Converter::Alignment align)\n> +{\n> + return converter_.validateOutput(cfg, adjusted, align);\n> +}\n> +\n> +/**\n> + * \\brief Queue buffers to converter device\n> + * \\param[in] input The frame buffer to apply the conversion\n> + * \\param[out] outputs The container holding the output stream pointers and\n> + * their respective frame buffer outputs.\n> + *\n> + * This function queues the \\a input frame buffer and the output frame buffers\n> + * contained in \\a outputs to the device for processing.\n> + *\n> + * Controls are automatically applied to the device before queuing buffers. V4L2\n> + * requests are used to atomically apply the controls if the kernel supports it.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int ConverterDW100Module::queueBuffers(FrameBuffer *input,\n> + const std::map &outputs)\n> +{\n> + int ret;\n> +\n> + V4L2Request *request = nullptr;\n> + if (!requests_.empty()) {\n> + /* If we have requests support, there must be one available */\n> + ASSERT(!availableRequests_.empty());\n> + request = availableRequests_.front();\n> + availableRequests_.pop();\n> + }\n> +\n> + for (auto &[stream, buffer] : outputs) {\n> + ret = applyControls(stream, request);\n> + if (ret) {\n> + reinitRequest(request);\n> + return ret;\n> + }\n> + }\n> +\n> + ret = converter_.queueBuffers(input, outputs, request);\n> + if (ret) {\n> + reinitRequest(request);\n> + return ret;\n> + }\n> +\n> + if (!request)\n> + return 0;\n> +\n> + ret = request->queue();\n> + if (ret < 0) {\n> + LOG(Converter, Error) << \"Failed to queue dewarp request: -\"\n> + << strerror(-ret);\n> + /* Push it back into the queue. */\n> + reinitRequest(request);\n> + }\n> +\n> + return ret;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::V4L2M2MConverter::start\n> + */\n> +int ConverterDW100Module::start()\n> +{\n> + int ret;\n> +\n> + if (converter_.supportsRequests()) {\n> + ret = converter_.allocateRequests(inputBufferCount_,\n> + &requests_);\n> + if (ret < 0) {\n> + LOG(Converter, Error) << \"Failed to allocate requests.\";\n> + return ret;\n> + }\n> + }\n> +\n> + for (std::unique_ptr &request : requests_) {\n> + request->requestDone.connect(this, &ConverterDW100Module::reinitRequest);\n> + availableRequests_.push(request.get());\n> + }\n> +\n> + /*\n> + * Apply controls on all streams, to support older kernels without\n> + * request and dynamic vertex map support.\n> + */\n> + for (auto &[stream, info] : vertexMaps_)\n> + applyControls(stream, nullptr);\n\nWhat controls are we applying here? Should we be passing controls\nthrough ::start(ControlList ...) or such?\n\n\n> +\n> + ret = converter_.start();\n> + if (!ret) {\n> + running_ = true;\n> + return 0;\n> + }\n> +\n> + availableRequests_ = {};\n> + requests_.clear();\n> + return ret;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::V4L2M2MConverter::stop\n> + */\n> +void ConverterDW100Module::stop()\n> +{\n> + running_ = false;\n> + converter_.stop();\n> + availableRequests_ = {};\n> + requests_.clear();\n> +}\n> +\n> +/**\n> + * \\brief Update the controls\n> + * \\param[in] stream The stream\n> + * \\param[inout] controls The controls info map to update\n> + *\n> + * Updated the \\a controls map with all the controls and limits provided by this\n> + * class.\n> + */\n> +void ConverterDW100Module::updateControlInfos(const Stream *stream, ControlInfoMap::Map &controls)\n> +{\n> + ControlValue scalerCropDefault = sensorCrop_;\n> +\n> + if (isConfigured(stream)) {\n> + auto &info = vertexMaps_[stream];\n> + info.map.applyLimits();\n> + scalerCropDefault = info.map.effectiveScalerCrop();\n> + }\n> +\n> + controls[&controls::ScalerCrop] = ControlInfo(Rectangle(sensorCrop_.x, sensorCrop_.y, 1, 1),\n> + sensorCrop_, sensorCrop_);\n> +\n> + if (!converter_.supportsRequests())\n> + LOG(Converter, Warning)\n> + << \"dw100 kernel driver has no requests support.\"\n> + \" Dynamic configuration is not possible.\";\n> +}\n> +\n> +/**\n> + * \\brief Set libcamera controls\n> + * \\param[in] stream The stream to update\n> + * \\param[in] controls The controls\n> + *\n> + * Looks up all supported controls in \\a controls and sets them on stream \\a\n> + * stream. The controls will be applied to the device on the next call to\n> + * queueBuffers().\n> + */\n> +void ConverterDW100Module::setControls(const Stream *stream, const ControlList &controls)\n> +{\n> + if (!isConfigured(stream))\n> + return;\n> +\n> + auto &info = vertexMaps_[stream];\n> + auto &vertexMap = info.map;\n> +\n> + const auto &crop = controls.get(controls::ScalerCrop);\n> + if (crop) {\n> + vertexMap.setScalerCrop(*crop);\n> + info.update = true;\n> + }\n> +\n> + if (info.update && running_ && !converter_.supportsRequests())\n> + LOG(Converter, Error)\n> + << \"Dynamically setting dw100 specific controls requires\"\n> + \" a dw100 kernel driver with requests support\";\n> +}\n> +\n> +/**\n> + * \\brief Retrieve updated metadata\n> + * \\param[in] stream The stream\n> + * \\param[in] meta The metadata list\n> + *\n> + * This function retrieves the metadata for the provided \\a stream and writes it\n> + * to \\a list. It shall be called after queueBuffers().\n> + */\n> +void ConverterDW100Module::populateMetadata(const Stream *stream, ControlList &meta)\n> +{\n> + if (!isConfigured(stream))\n> + return;\n> +\n> + auto &vertexMap = vertexMaps_[stream].map;\n> +\n> + meta.set(controls::ScalerCrop, vertexMap.effectiveScalerCrop());\n> +}\n> +\n> +/**\n> + * \\var ConverterDW100Module::inputBufferReady\n> + * \\brief A signal emitted when the input frame buffer completes\n> + */\n> +\n> +/**\n> + * \\var ConverterDW100Module::outputBufferReady\n> + * \\brief A signal emitted on each frame buffer completion of the output queue\n> + */\n> +\n> +/**\n> + * \\brief Set sensor crop rectangle\n> + * \\param[in] rect The crop rectangle\n> + *\n> + * Set the sensor crop rectangle to \\a rect. This rectangle describes the area\n> + * covered by the input buffers in sensor coordinates. It is used internally\n> + * to handle the ScalerCrop control and related metadata.\n> + */\n> +void ConverterDW100Module::setSensorCrop(const Rectangle &rect)\n> +{\n> + sensorCrop_ = rect;\n> + for (auto &[stream, vertexMap] : vertexMaps_) {\n> + vertexMap.map.setSensorCrop(rect);\n> + vertexMap.update = true;\n> + }\n> +}\n> +\n> +/**\n> + * \\brief Set transform\n> + * \\param[in] stream The stream\n> + * \\param[in] transform The transform\n> + *\n> + * Set the transform that shall be applied by the dewarper on the given stream.\n> + * As orientation is a property of libcamera::CameraConfiguration, the transform\n> + * needs to be set at configure time.\n> + */\n> +void ConverterDW100Module::setTransform(const Stream *stream, const Transform &transform)\n> +{\n> + if (!isConfigured(stream))\n> + return;\n> +\n> + vertexMaps_[stream].map.setTransform(transform);\n> +}\n> +\n> +void ConverterDW100Module::reinitRequest(V4L2Request *request)\n> +{\n> + if (!request)\n> + return;\n> +\n> + request->reinit();\n> + availableRequests_.push(request);\n> +}\n> +\n> +/**\n> + * \\brief Apply the vertex map for a given stream\n> + * \\param[in] stream The stream to update\n> + * \\param[in] request An optional request\n> + *\n> + * This function updates the vertex map on the stream \\a stream. If \\a request\n> + * is provided, the updated happens in that request.\n\ns/updated/update/\n\nReviewed-by: Kieran Bingham \n\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int ConverterDW100Module::applyControls(const Stream *stream, const V4L2Request *request)\n> +{\n> + if (!isConfigured(stream))\n> + return -EINVAL;\n> +\n> + auto &info = vertexMaps_[stream];\n> + if (!info.update)\n> + return 0;\n> +\n> + std::vector map = info.map.getVertexMap();\n> + auto value = Span(reinterpret_cast(&map[0]), map.size());\n> +\n> + ControlList ctrls;\n> + ctrls.set(V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP, value);\n> +\n> + int ret = converter_.applyControls(stream, ctrls, request);\n> + if (!ret)\n> + info.update = false;\n> +\n> + return ret;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/converter/meson.build b/src/libcamera/converter/meson.build\n> index 558d63a1bdd4..48f27cad5997 100644\n> --- a/src/libcamera/converter/meson.build\n> +++ b/src/libcamera/converter/meson.build\n> @@ -1,6 +1,7 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> libcamera_internal_sources += files([\n> + 'converter_dw100.cpp',\n> 'converter_dw100_vertexmap.cpp',\n> 'converter_v4l2_m2m.cpp'\n> ])\n> -- \n> 2.51.0\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 4E553C32DE\n\tfor ;\n\tTue, 25 Nov 2025 17:27:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 96A6060A9E;\n\tTue, 25 Nov 2025 18:27:46 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9F31A609D8\n\tfor ;\n\tTue, 25 Nov 2025 18:27:44 +0100 (CET)","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 5786C6AF;\n\tTue, 25 Nov 2025 18:25:35 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZYI9mTP0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764091535;\n\tbh=05GGiZg7hiHcAc7Fu02UPf3qsKBz1zLVmkqqvCautgA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ZYI9mTP0APXfEbA5qXsB5zD2MseI/2IpyI2tC7/i9wrmz6WecIKoCS6pOiHMWrVwD\n\timagH3oL7gU55GRTAsPuREadTWz/q/cERHsGB6677AAIAn6v/ztpj6xbC56MrkvANk\n\tzfxVqZSQVOi38RD2TLkNy6l8Li6rn1NVsq1kIYyY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251125162851.2301793-17-stefan.klug@ideasonboard.com>","References":"<20251125162851.2301793-1-stefan.klug@ideasonboard.com>\n\t<20251125162851.2301793-17-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v3 16/29] libcamera: converter: Add dw100 converter\n\tmodule","From":"Kieran Bingham ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 25 Nov 2025 17:27:41 +0000","Message-ID":"<176409166149.567526.3964367410159734360@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\" "}}]