[{"id":35603,"web_url":"https://patchwork.libcamera.org/comment/35603/","msgid":"<c48fe6d6-6832-47dd-9b38-52465a474306@ideasonboard.com>","date":"2025-08-28T13:44:51","subject":"Re: [PATCH 2/5] libcamera: Add support for camera flash devices","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. 08. 28. 15:09 keltezéssel, Matthias Fend írta:\n> Add a class to model camera flash devices. Currently, only v4l2 flash\n> devices are supported. The v4l2 flash devices are implemented similar to\n> the camera lenses.\n> \n> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>\n> ---\n>   include/libcamera/internal/camera_flash.h     |  75 ++++++++\n>   include/libcamera/internal/camera_sensor.h    |   2 +\n>   src/libcamera/camera_flash.cpp                | 248 ++++++++++++++++++++++++++\n>   src/libcamera/meson.build                     |   1 +\n>   src/libcamera/sensor/camera_sensor_legacy.cpp |  13 ++\n>   src/libcamera/sensor/camera_sensor_raw.cpp    |  13 ++\n>   6 files changed, 352 insertions(+)\n> \n> diff --git a/include/libcamera/internal/camera_flash.h b/include/libcamera/internal/camera_flash.h\n> new file mode 100644\n> index 0000000000000000000000000000000000000000..e41afef2ab84852a340a12a012e3994f00cac27a\n> --- /dev/null\n> +++ b/include/libcamera/internal/camera_flash.h\n> @@ -0,0 +1,75 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, matthias.fend@emfend.at\n> + *\n> + * Camera flash support\n> + */\n> +#pragma once\n> +\n> +#include <memory>\n> +#include <stdint.h>\n> +#include <string>\n> +\n> +#include <libcamera/base/class.h>\n> +#include <libcamera/base/log.h>\n> +\n> +#include <libcamera/controls.h>\n> +\n> +namespace libcamera {\n> +\n> +class MediaEntity;\n> +class V4L2Subdevice;\n> +\n> +class CameraFlash : protected Loggable\n> +{\n> +public:\n> +\tenum Mode {\n> +\t\tNone,\n> +\t\tFlash,\n> +\t\tTorch,\n> +\t};\n> +\n> +\tenum StrobeSource {\n> +\t\tSoftware,\n> +\t\tExternal,\n> +\t};\n\nI think instead of defining these again, you should use the generated enums from\n`control_ids.h` since the return values of `getMode()` and `getStrobeSource()` are\ndirectly set as the value for `controls::draft::Flash{Mode,StrobeSource}`. Or at the\nvery least some `static_assert()`s would be good. But I think using the generated\nenumerators would be preferable.\n\n\n> +\n> +\texplicit CameraFlash(const MediaEntity *entity);\n> +\t~CameraFlash();\n> +\tint init();\n> +\tMode getMode() const;\n> +\tint setMode(Mode mode);\n> +\tconst ControlInfo &getFlashIntensityInfo() const;\n> +\tint32_t getFlashIntensity() const;\n> +\tint setFlashIntensity(int32_t intensity);\n> +\tconst ControlInfo &getFlashTimeoutInfo() const;\n> +\tint32_t getFlashTimeout() const;\n> +\tint setFlashTimeout(int32_t timeout_us);\n> +\tStrobeSource getStrobeSource() const;\n> +\tint setStrobeSource(StrobeSource source);\n> +\tint startStrobe();\n> +\tint stopStrobe();\n> +\tconst ControlInfo &getTorchIntensityInfo() const;\n> +\tint32_t getTorchIntensity() const;\n> +\tint setTorchIntensity(int32_t intensity);\n> +\n> +\tconst std::string &model() const;\n> +\tconst ControlInfoMap &controls() const;\n> +\n> +protected:\n> +\tstd::string logPrefix() const override;\n> +\n> +private:\n> +\tLIBCAMERA_DISABLE_COPY_AND_MOVE(CameraFlash)\n> +\n> +\tint32_t getSubdevControl(uint32_t id) const;\n> +\tint setSubdevControl(uint32_t id, int32_t value);\n> +\tint validateDriver();\n> +\n> +\tconst MediaEntity *entity_;\n> +\tstd::unique_ptr<V4L2Subdevice> subdev_;\n> +\tstd::string model_;\n> +\tconst ControlInfoMap *controlInfoMap_ = nullptr;\n\nWhat is the motivation for caching this value?\n\n\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index f6ef4df170d43500bc652762e9a575010a6c1cbd..faeecf244c8d42a43eca52ae04813e0c2f80e516 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -28,6 +28,7 @@\n>   \n>   namespace libcamera {\n>   \n> +class CameraFlash;\n>   class CameraLens;\n>   class MediaEntity;\n>   class SensorConfiguration;\n> @@ -48,6 +49,7 @@ public:\n>   \tvirtual V4L2Subdevice *device() = 0;\n>   \n>   \tvirtual CameraLens *focusLens() = 0;\n> +\tvirtual CameraFlash *flash() = 0;\n>   \n>   \tvirtual const std::vector<unsigned int> &mbusCodes() const = 0;\n>   \tvirtual std::vector<Size> sizes(unsigned int mbusCode) const = 0;\n> diff --git a/src/libcamera/camera_flash.cpp b/src/libcamera/camera_flash.cpp\n> new file mode 100644\n> index 0000000000000000000000000000000000000000..4702c590c91e0bcb20d173e7ce03608e1ae6ecfd\n> --- /dev/null\n> +++ b/src/libcamera/camera_flash.cpp\n> @@ -0,0 +1,248 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, matthias.fend@emfend.at\n> + *\n> + * Camera flash support\n> + */\n> +\n> +#include \"libcamera/internal/camera_flash.h\"\n> +\n> +#include <libcamera/base/utils.h>\n> +\n> +#include \"libcamera/internal/v4l2_subdevice.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(CameraFlash)\n> +\n> +CameraFlash::CameraFlash(const MediaEntity *entity)\n> +\t: entity_(entity)\n> +{\n> +}\n> +\n> +CameraFlash::~CameraFlash() = default;\n> +\n> +int CameraFlash::init()\n> +{\n> +\tif (entity_->function() != MEDIA_ENT_F_FLASH) {\n> +\t\tLOG(CameraFlash, Error)\n> +\t\t\t<< \"Invalid flash function \"\n> +\t\t\t<< utils::hex(entity_->function());\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tsubdev_ = std::make_unique<V4L2Subdevice>(entity_);\n> +\tint ret = subdev_->open();\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tcontrolInfoMap_ = &subdev_->controls();\n> +\n> +\tret = validateDriver();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tmodel_ = subdev_->model();\n> +\n> +\treturn 0;\n> +}\n> +\n> +CameraFlash::Mode CameraFlash::getMode() const\n> +{\n> +\tMode m;\n> +\n> +\tswitch (getSubdevControl(V4L2_CID_FLASH_LED_MODE)) {\n> +\tcase V4L2_FLASH_LED_MODE_FLASH:\n> +\t\tm = Flash;\n> +\t\tbreak;\n> +\tcase V4L2_FLASH_LED_MODE_TORCH:\n> +\t\tm = Torch;\n> +\t\tbreak;\n> +\tcase V4L2_FLASH_LED_MODE_NONE:\n> +\tdefault:\n> +\t\tm = None;\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn m;\n> +}\n> +\n> +int CameraFlash::setMode(Mode mode)\n> +{\n> +\tint32_t m;\n> +\n> +\tswitch (mode) {\n> +\tcase Flash:\n> +\t\tm = V4L2_FLASH_LED_MODE_FLASH;\n> +\t\tbreak;\n> +\tcase Torch:\n> +\t\tm = V4L2_FLASH_LED_MODE_TORCH;\n> +\t\tbreak;\n> +\tcase None:\n> +\t\tm = V4L2_FLASH_LED_MODE_NONE;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\treturn setSubdevControl(V4L2_CID_FLASH_LED_MODE, m);\n> +}\n> +\n> +const ControlInfo &CameraFlash::getFlashIntensityInfo() const\n> +{\n> +\treturn controlInfoMap_->find(V4L2_CID_FLASH_INTENSITY)->second;\n> +}\n> +\n> +int32_t CameraFlash::getFlashIntensity() const\n> +{\n> +\treturn getSubdevControl(V4L2_CID_FLASH_INTENSITY);\n> +}\n> +\n> +int CameraFlash::setFlashIntensity(int32_t intensity)\n> +{\n> +\treturn setSubdevControl(V4L2_CID_FLASH_INTENSITY, intensity);\n> +}\n> +\n> +const ControlInfo &CameraFlash::getFlashTimeoutInfo() const\n> +{\n> +\treturn controlInfoMap_->find(V4L2_CID_FLASH_TIMEOUT)->second;\n> +}\n> +\n> +int32_t CameraFlash::getFlashTimeout() const\n> +{\n> +\treturn getSubdevControl(V4L2_CID_FLASH_TIMEOUT);\n> +}\n> +\n> +int CameraFlash::setFlashTimeout(int32_t timeout)\n> +{\n> +\treturn setSubdevControl(V4L2_CID_FLASH_TIMEOUT, timeout);\n> +}\n> +\n> +CameraFlash::StrobeSource CameraFlash::getStrobeSource() const\n> +{\n> +\tStrobeSource s;\n> +\n> +\tswitch (getSubdevControl(V4L2_CID_FLASH_STROBE_SOURCE)) {\n> +\tcase V4L2_FLASH_STROBE_SOURCE_EXTERNAL:\n> +\t\ts = External;\n> +\t\tbreak;\n> +\tcase V4L2_FLASH_STROBE_SOURCE_SOFTWARE:\n> +\tdefault:\n> +\t\ts = Software;\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn s;\n> +}\n> +\n> +int CameraFlash::setStrobeSource(StrobeSource source)\n> +{\n> +\tint32_t s;\n> +\n> +\tswitch (source) {\n> +\tcase External:\n> +\t\ts = V4L2_FLASH_STROBE_SOURCE_EXTERNAL;\n> +\t\tbreak;\n> +\tcase Software:\n> +\t\ts = V4L2_FLASH_STROBE_SOURCE_SOFTWARE;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\treturn setSubdevControl(V4L2_CID_FLASH_STROBE_SOURCE, s);\n> +}\n> +\n> +int CameraFlash::startStrobe()\n> +{\n> +\treturn setSubdevControl(V4L2_CID_FLASH_STROBE, 1);\n> +}\n> +\n> +int CameraFlash::stopStrobe()\n> +{\n> +\treturn setSubdevControl(V4L2_CID_FLASH_STROBE_STOP, 1);\n> +}\n> +\n> +const ControlInfo &CameraFlash::getTorchIntensityInfo() const\n> +{\n> +\treturn controlInfoMap_->find(V4L2_CID_FLASH_TORCH_INTENSITY)->second;\n> +}\n> +\n> +int32_t CameraFlash::getTorchIntensity() const\n> +{\n> +\treturn getSubdevControl(V4L2_CID_FLASH_TORCH_INTENSITY);\n> +}\n> +\n> +int CameraFlash::setTorchIntensity(int32_t intensity)\n> +{\n> +\treturn setSubdevControl(V4L2_CID_FLASH_TORCH_INTENSITY, intensity);\n> +}\n> +\n> +const std::string &CameraFlash::model() const\n> +{\n> +\treturn model_;\n> +}\n> +\n> +const ControlInfoMap &CameraFlash::controls() const\n> +{\n> +\treturn subdev_->controls();\n\nWhy not `controlInfoMap_` here?\n\n\n> +}\n> +\n> +std::string CameraFlash::logPrefix() const\n> +{\n> +\treturn \"'\" + entity_->name() + \"'\";\n> +}\n> +\n> +int32_t CameraFlash::getSubdevControl(uint32_t id) const\n> +{\n> +\tControlList controlList = subdev_->getControls(std::vector<uint32_t>{ id });\n\n   getControls(std::array{ id })\n\nshould work.\n\n\n> +\n> +\treturn controlList.get(id).get<int32_t>();\n\nI think some level of error checking would be useful when\nquerying the device.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> +}\n> +\n> +int CameraFlash::setSubdevControl(uint32_t id, int32_t value)\n> +{\n> +\tControlList flashCtrls(subdev_->controls());\n> +\n> +\tflashCtrls.set(id, value);\n> +\n> +\tif (subdev_->setControls(&flashCtrls))\n> +\t\treturn -EINVAL;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int CameraFlash::validateDriver()\n> +{\n> +\tint ret = 0;\n> +\tstatic constexpr uint32_t mandatoryControls[] = {\n> +\t\tV4L2_CID_FLASH_LED_MODE,\n> +\t\tV4L2_CID_FLASH_STROBE_SOURCE,\n> +\t\tV4L2_CID_FLASH_STROBE,\n> +\t\tV4L2_CID_FLASH_TIMEOUT,\n> +\t\tV4L2_CID_FLASH_INTENSITY,\n> +\t\tV4L2_CID_FLASH_TORCH_INTENSITY,\n> +\t};\n> +\n> +\tfor (uint32_t ctrl : mandatoryControls) {\n> +\t\tif (!controlInfoMap_->count(ctrl)) {\n> +\t\t\tLOG(CameraFlash, Error)\n> +\t\t\t\t<< \"Mandatory V4L2 control \" << utils::hex(ctrl)\n> +\t\t\t\t<< \" not available\";\n> +\t\t\tret = -EINVAL;\n> +\t\t}\n> +\t}\n> +\n> +\tif (ret) {\n> +\t\tLOG(CameraFlash, Error)\n> +\t\t\t<< \"The flash kernel driver needs to be fixed\";\n> +\t\tLOG(CameraFlash, Error)\n> +\t\t\t<< \"See Documentation/flash_driver_requirements.rst in\"\n> +\t\t\t<< \" the libcamera sources for more information\";\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn ret;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index b3ca27f217da4ba3a896ef7cbfb5502fa82a4907..0f125661a51e2431c1febc353cef30a1219f9ce7 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -20,6 +20,7 @@ libcamera_internal_sources = files([\n>       'bayer_format.cpp',\n>       'byte_stream_buffer.cpp',\n>       'camera_controls.cpp',\n> +    'camera_flash.cpp',\n>       'camera_lens.cpp',\n>       'clock_recovery.cpp',\n>       'control_serializer.cpp',\n> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp\n> index f9e685a9acc499fc91d51ed1d66780a0ad2d2a8f..632b66ea0aa15fcd654e7f0efb50c24cb9b973bf 100644\n> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp\n> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp\n> @@ -31,6 +31,7 @@\n>   #include <libcamera/ipa/core_ipa_interface.h>\n>   \n>   #include \"libcamera/internal/bayer_format.h\"\n> +#include \"libcamera/internal/camera_flash.h\"\n>   #include \"libcamera/internal/camera_lens.h\"\n>   #include \"libcamera/internal/camera_sensor.h\"\n>   #include \"libcamera/internal/camera_sensor_properties.h\"\n> @@ -68,6 +69,7 @@ public:\n>   \tV4L2Subdevice *device() override { return subdev_.get(); }\n>   \n>   \tCameraLens *focusLens() override { return focusLens_.get(); }\n> +\tCameraFlash *flash() override { return flash_.get(); }\n>   \n>   \tconst std::vector<unsigned int> &mbusCodes() const override { return mbusCodes_; }\n>   \tstd::vector<Size> sizes(unsigned int mbusCode) const override;\n> @@ -139,6 +141,7 @@ private:\n>   \tControlList properties_;\n>   \n>   \tstd::unique_ptr<CameraLens> focusLens_;\n> +\tstd::unique_ptr<CameraFlash> flash_;\n>   };\n>   \n>   /**\n> @@ -665,6 +668,16 @@ int CameraSensorLegacy::discoverAncillaryDevices()\n>   \t\t\t}\n>   \t\t\tbreak;\n>   \n> +\t\tcase MEDIA_ENT_F_FLASH:\n> +\t\t\tflash_ = std::make_unique<CameraFlash>(ancillary);\n> +\t\t\tret = flash_->init();\n> +\t\t\tif (ret) {\n> +\t\t\t\tLOG(CameraSensor, Error)\n> +\t\t\t\t\t<< \"Flash initialisation failed, flash disabled\";\n> +\t\t\t\tflash_.reset();\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\n>   \t\tdefault:\n>   \t\t\tLOG(CameraSensor, Warning)\n>   \t\t\t\t<< \"Unsupported ancillary entity function \"\n> diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp\n> index 8ea4423698cd8c1eaae43eb5ba8b5d524b94d515..9d533d814b9df453aa4009a87818c1558bcbd665 100644\n> --- a/src/libcamera/sensor/camera_sensor_raw.cpp\n> +++ b/src/libcamera/sensor/camera_sensor_raw.cpp\n> @@ -32,6 +32,7 @@\n>   #include <libcamera/ipa/core_ipa_interface.h>\n>   \n>   #include \"libcamera/internal/bayer_format.h\"\n> +#include \"libcamera/internal/camera_flash.h\"\n>   #include \"libcamera/internal/camera_lens.h\"\n>   #include \"libcamera/internal/camera_sensor.h\"\n>   #include \"libcamera/internal/camera_sensor_properties.h\"\n> @@ -69,6 +70,7 @@ public:\n>   \tV4L2Subdevice *device() override { return subdev_.get(); }\n>   \n>   \tCameraLens *focusLens() override { return focusLens_.get(); }\n> +\tCameraFlash *flash() override { return flash_.get(); }\n>   \n>   \tconst std::vector<unsigned int> &mbusCodes() const override { return mbusCodes_; }\n>   \tstd::vector<Size> sizes(unsigned int mbusCode) const override;\n> @@ -150,6 +152,7 @@ private:\n>   \tControlList properties_;\n>   \n>   \tstd::unique_ptr<CameraLens> focusLens_;\n> +\tstd::unique_ptr<CameraFlash> flash_;\n>   };\n>   \n>   /**\n> @@ -513,6 +516,16 @@ std::optional<int> CameraSensorRaw::init()\n>   \t\t\t}\n>   \t\t\tbreak;\n>   \n> +\t\tcase MEDIA_ENT_F_FLASH:\n> +\t\t\tflash_ = std::make_unique<CameraFlash>(ancillary);\n> +\t\t\tret = flash_->init();\n> +\t\t\tif (ret) {\n> +\t\t\t\tLOG(CameraSensor, Error)\n> +\t\t\t\t\t<< \"Flash initialisation failed, flash disabled\";\n> +\t\t\t\tflash_.reset();\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\n>   \t\tdefault:\n>   \t\t\tLOG(CameraSensor, Warning)\n>   \t\t\t\t<< \"Unsupported ancillary entity function \"\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 6E6AEBEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Aug 2025 13:44:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 12186692F3;\n\tThu, 28 Aug 2025 15:44:58 +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 EC73F613B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Aug 2025 15:44:55 +0200 (CEST)","from [192.168.33.12] (185.221.143.232.nat.pool.zt.hu\n\t[185.221.143.232])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 497951CE3;\n\tThu, 28 Aug 2025 15:43:51 +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=\"JsFJMbDv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1756388631;\n\tbh=5BAci0f4qSVfzUYCFp3QexYdD4JAgLB4fd1FpbDK09g=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=JsFJMbDvwxer2hWauu6++0//BU6InD9I0ee+gdVU0maECeNl6IptJrJo1TCYpOUkh\n\tr7Eea3KEOdysa5oBVtNiuUOVSES2gfPMJD4qeVamfYc48NtxgKUDgU16TAMeRAaP4C\n\tCJhTCNq9rTnRsmveQnYHhtdIPCDBWuuOuNYbQg5o=","Message-ID":"<c48fe6d6-6832-47dd-9b38-52465a474306@ideasonboard.com>","Date":"Thu, 28 Aug 2025 15:44:51 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/5] libcamera: Add support for camera flash devices","To":"Matthias Fend <matthias.fend@emfend.at>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20250828-flash-support-v1-0-4c5dc674a05b@emfend.at>\n\t<20250828-flash-support-v1-2-4c5dc674a05b@emfend.at>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250828-flash-support-v1-2-4c5dc674a05b@emfend.at>","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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35607,"web_url":"https://patchwork.libcamera.org/comment/35607/","msgid":"<555eb0fa-b9d8-41cb-8c18-0cc607e0f839@emfend.at>","date":"2025-08-28T14:30:31","subject":"Re: [PATCH 2/5] libcamera: Add support for camera flash devices","submitter":{"id":134,"url":"https://patchwork.libcamera.org/api/people/134/","name":"Matthias Fend","email":"matthias.fend@emfend.at"},"content":"Hi Barnabás,\n\nthanks a lot for your feedback!\n\nAm 28.08.2025 um 15:44 schrieb Barnabás Pőcze:\n> Hi\n> \n> 2025. 08. 28. 15:09 keltezéssel, Matthias Fend írta:\n>> Add a class to model camera flash devices. Currently, only v4l2 flash\n>> devices are supported. The v4l2 flash devices are implemented similar to\n>> the camera lenses.\n>>\n>> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>\n>> ---\n>>   include/libcamera/internal/camera_flash.h     |  75 ++++++++\n>>   include/libcamera/internal/camera_sensor.h    |   2 +\n>>   src/libcamera/camera_flash.cpp                | 248 ++++++++++++++++ \n>> ++++++++++\n>>   src/libcamera/meson.build                     |   1 +\n>>   src/libcamera/sensor/camera_sensor_legacy.cpp |  13 ++\n>>   src/libcamera/sensor/camera_sensor_raw.cpp    |  13 ++\n>>   6 files changed, 352 insertions(+)\n>>\n>> diff --git a/include/libcamera/internal/camera_flash.h b/include/ \n>> libcamera/internal/camera_flash.h\n>> new file mode 100644\n>> index \n>> 0000000000000000000000000000000000000000..e41afef2ab84852a340a12a012e3994f00cac27a\n>> --- /dev/null\n>> +++ b/include/libcamera/internal/camera_flash.h\n>> @@ -0,0 +1,75 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2025, matthias.fend@emfend.at\n>> + *\n>> + * Camera flash support\n>> + */\n>> +#pragma once\n>> +\n>> +#include <memory>\n>> +#include <stdint.h>\n>> +#include <string>\n>> +\n>> +#include <libcamera/base/class.h>\n>> +#include <libcamera/base/log.h>\n>> +\n>> +#include <libcamera/controls.h>\n>> +\n>> +namespace libcamera {\n>> +\n>> +class MediaEntity;\n>> +class V4L2Subdevice;\n>> +\n>> +class CameraFlash : protected Loggable\n>> +{\n>> +public:\n>> +    enum Mode {\n>> +        None,\n>> +        Flash,\n>> +        Torch,\n>> +    };\n>> +\n>> +    enum StrobeSource {\n>> +        Software,\n>> +        External,\n>> +    };\n> \n> I think instead of defining these again, you should use the generated \n> enums from\n> `control_ids.h` since the return values of `getMode()` and \n> `getStrobeSource()` are\n> directly set as the value for `controls::draft::Flash{Mode,StrobeSource} \n> `. Or at the\n> very least some `static_assert()`s would be good. But I think using the \n> generated\n> enumerators would be preferable.\n\nI wanted to have a dedicated internal API here and not mix it with the \nexternal control API.\nBut you're right, for the controls in the metadata, the return values \n​​of getMode() and getStrobeSource() should also be mapped (as is \nalready done for the incoming controls).\n\n> \n> \n>> +\n>> +    explicit CameraFlash(const MediaEntity *entity);\n>> +    ~CameraFlash();\n>> +    int init();\n>> +    Mode getMode() const;\n>> +    int setMode(Mode mode);\n>> +    const ControlInfo &getFlashIntensityInfo() const;\n>> +    int32_t getFlashIntensity() const;\n>> +    int setFlashIntensity(int32_t intensity);\n>> +    const ControlInfo &getFlashTimeoutInfo() const;\n>> +    int32_t getFlashTimeout() const;\n>> +    int setFlashTimeout(int32_t timeout_us);\n>> +    StrobeSource getStrobeSource() const;\n>> +    int setStrobeSource(StrobeSource source);\n>> +    int startStrobe();\n>> +    int stopStrobe();\n>> +    const ControlInfo &getTorchIntensityInfo() const;\n>> +    int32_t getTorchIntensity() const;\n>> +    int setTorchIntensity(int32_t intensity);\n>> +\n>> +    const std::string &model() const;\n>> +    const ControlInfoMap &controls() const;\n>> +\n>> +protected:\n>> +    std::string logPrefix() const override;\n>> +\n>> +private:\n>> +    LIBCAMERA_DISABLE_COPY_AND_MOVE(CameraFlash)\n>> +\n>> +    int32_t getSubdevControl(uint32_t id) const;\n>> +    int setSubdevControl(uint32_t id, int32_t value);\n>> +    int validateDriver();\n>> +\n>> +    const MediaEntity *entity_;\n>> +    std::unique_ptr<V4L2Subdevice> subdev_;\n>> +    std::string model_;\n>> +    const ControlInfoMap *controlInfoMap_ = nullptr;\n> \n> What is the motivation for caching this value?\n\nJust to make it a little easier to read and to avoid constructs like this:\nsubdev_->controls().find(V4L2_CID_FLASH_TORCH_INTENSITY)->second;\n\n> \n> \n>> +};\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/ \n>> libcamera/internal/camera_sensor.h\n>> index \n>> f6ef4df170d43500bc652762e9a575010a6c1cbd..faeecf244c8d42a43eca52ae04813e0c2f80e516 100644\n>> --- a/include/libcamera/internal/camera_sensor.h\n>> +++ b/include/libcamera/internal/camera_sensor.h\n>> @@ -28,6 +28,7 @@\n>>   namespace libcamera {\n>> +class CameraFlash;\n>>   class CameraLens;\n>>   class MediaEntity;\n>>   class SensorConfiguration;\n>> @@ -48,6 +49,7 @@ public:\n>>       virtual V4L2Subdevice *device() = 0;\n>>       virtual CameraLens *focusLens() = 0;\n>> +    virtual CameraFlash *flash() = 0;\n>>       virtual const std::vector<unsigned int> &mbusCodes() const = 0;\n>>       virtual std::vector<Size> sizes(unsigned int mbusCode) const = 0;\n>> diff --git a/src/libcamera/camera_flash.cpp b/src/libcamera/ \n>> camera_flash.cpp\n>> new file mode 100644\n>> index \n>> 0000000000000000000000000000000000000000..4702c590c91e0bcb20d173e7ce03608e1ae6ecfd\n>> --- /dev/null\n>> +++ b/src/libcamera/camera_flash.cpp\n>> @@ -0,0 +1,248 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2025, matthias.fend@emfend.at\n>> + *\n>> + * Camera flash support\n>> + */\n>> +\n>> +#include \"libcamera/internal/camera_flash.h\"\n>> +\n>> +#include <libcamera/base/utils.h>\n>> +\n>> +#include \"libcamera/internal/v4l2_subdevice.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +LOG_DEFINE_CATEGORY(CameraFlash)\n>> +\n>> +CameraFlash::CameraFlash(const MediaEntity *entity)\n>> +    : entity_(entity)\n>> +{\n>> +}\n>> +\n>> +CameraFlash::~CameraFlash() = default;\n>> +\n>> +int CameraFlash::init()\n>> +{\n>> +    if (entity_->function() != MEDIA_ENT_F_FLASH) {\n>> +        LOG(CameraFlash, Error)\n>> +            << \"Invalid flash function \"\n>> +            << utils::hex(entity_->function());\n>> +        return -EINVAL;\n>> +    }\n>> +\n>> +    subdev_ = std::make_unique<V4L2Subdevice>(entity_);\n>> +    int ret = subdev_->open();\n>> +    if (ret < 0)\n>> +        return ret;\n>> +\n>> +    controlInfoMap_ = &subdev_->controls();\n>> +\n>> +    ret = validateDriver();\n>> +    if (ret)\n>> +        return ret;\n>> +\n>> +    model_ = subdev_->model();\n>> +\n>> +    return 0;\n>> +}\n>> +\n>> +CameraFlash::Mode CameraFlash::getMode() const\n>> +{\n>> +    Mode m;\n>> +\n>> +    switch (getSubdevControl(V4L2_CID_FLASH_LED_MODE)) {\n>> +    case V4L2_FLASH_LED_MODE_FLASH:\n>> +        m = Flash;\n>> +        break;\n>> +    case V4L2_FLASH_LED_MODE_TORCH:\n>> +        m = Torch;\n>> +        break;\n>> +    case V4L2_FLASH_LED_MODE_NONE:\n>> +    default:\n>> +        m = None;\n>> +        break;\n>> +    }\n>> +\n>> +    return m;\n>> +}\n>> +\n>> +int CameraFlash::setMode(Mode mode)\n>> +{\n>> +    int32_t m;\n>> +\n>> +    switch (mode) {\n>> +    case Flash:\n>> +        m = V4L2_FLASH_LED_MODE_FLASH;\n>> +        break;\n>> +    case Torch:\n>> +        m = V4L2_FLASH_LED_MODE_TORCH;\n>> +        break;\n>> +    case None:\n>> +        m = V4L2_FLASH_LED_MODE_NONE;\n>> +        break;\n>> +    default:\n>> +        return -EINVAL;\n>> +    }\n>> +\n>> +    return setSubdevControl(V4L2_CID_FLASH_LED_MODE, m);\n>> +}\n>> +\n>> +const ControlInfo &CameraFlash::getFlashIntensityInfo() const\n>> +{\n>> +    return controlInfoMap_->find(V4L2_CID_FLASH_INTENSITY)->second;\n>> +}\n>> +\n>> +int32_t CameraFlash::getFlashIntensity() const\n>> +{\n>> +    return getSubdevControl(V4L2_CID_FLASH_INTENSITY);\n>> +}\n>> +\n>> +int CameraFlash::setFlashIntensity(int32_t intensity)\n>> +{\n>> +    return setSubdevControl(V4L2_CID_FLASH_INTENSITY, intensity);\n>> +}\n>> +\n>> +const ControlInfo &CameraFlash::getFlashTimeoutInfo() const\n>> +{\n>> +    return controlInfoMap_->find(V4L2_CID_FLASH_TIMEOUT)->second;\n>> +}\n>> +\n>> +int32_t CameraFlash::getFlashTimeout() const\n>> +{\n>> +    return getSubdevControl(V4L2_CID_FLASH_TIMEOUT);\n>> +}\n>> +\n>> +int CameraFlash::setFlashTimeout(int32_t timeout)\n>> +{\n>> +    return setSubdevControl(V4L2_CID_FLASH_TIMEOUT, timeout);\n>> +}\n>> +\n>> +CameraFlash::StrobeSource CameraFlash::getStrobeSource() const\n>> +{\n>> +    StrobeSource s;\n>> +\n>> +    switch (getSubdevControl(V4L2_CID_FLASH_STROBE_SOURCE)) {\n>> +    case V4L2_FLASH_STROBE_SOURCE_EXTERNAL:\n>> +        s = External;\n>> +        break;\n>> +    case V4L2_FLASH_STROBE_SOURCE_SOFTWARE:\n>> +    default:\n>> +        s = Software;\n>> +        break;\n>> +    }\n>> +\n>> +    return s;\n>> +}\n>> +\n>> +int CameraFlash::setStrobeSource(StrobeSource source)\n>> +{\n>> +    int32_t s;\n>> +\n>> +    switch (source) {\n>> +    case External:\n>> +        s = V4L2_FLASH_STROBE_SOURCE_EXTERNAL;\n>> +        break;\n>> +    case Software:\n>> +        s = V4L2_FLASH_STROBE_SOURCE_SOFTWARE;\n>> +        break;\n>> +    default:\n>> +        return -EINVAL;\n>> +    }\n>> +\n>> +    return setSubdevControl(V4L2_CID_FLASH_STROBE_SOURCE, s);\n>> +}\n>> +\n>> +int CameraFlash::startStrobe()\n>> +{\n>> +    return setSubdevControl(V4L2_CID_FLASH_STROBE, 1);\n>> +}\n>> +\n>> +int CameraFlash::stopStrobe()\n>> +{\n>> +    return setSubdevControl(V4L2_CID_FLASH_STROBE_STOP, 1);\n>> +}\n>> +\n>> +const ControlInfo &CameraFlash::getTorchIntensityInfo() const\n>> +{\n>> +    return controlInfoMap_->find(V4L2_CID_FLASH_TORCH_INTENSITY)- \n>> >second;\n>> +}\n>> +\n>> +int32_t CameraFlash::getTorchIntensity() const\n>> +{\n>> +    return getSubdevControl(V4L2_CID_FLASH_TORCH_INTENSITY);\n>> +}\n>> +\n>> +int CameraFlash::setTorchIntensity(int32_t intensity)\n>> +{\n>> +    return setSubdevControl(V4L2_CID_FLASH_TORCH_INTENSITY, intensity);\n>> +}\n>> +\n>> +const std::string &CameraFlash::model() const\n>> +{\n>> +    return model_;\n>> +}\n>> +\n>> +const ControlInfoMap &CameraFlash::controls() const\n>> +{\n>> +    return subdev_->controls();\n> \n> Why not `controlInfoMap_` here?\n\nTrue, if the variable already exists, it should probably always be used.\n\n> \n> \n>> +}\n>> +\n>> +std::string CameraFlash::logPrefix() const\n>> +{\n>> +    return \"'\" + entity_->name() + \"'\";\n>> +}\n>> +\n>> +int32_t CameraFlash::getSubdevControl(uint32_t id) const\n>> +{\n>> +    ControlList controlList = subdev_- \n>> >getControls(std::vector<uint32_t>{ id });\n> \n>    getControls(std::array{ id })\n> \n> should work.\n\nACK.\n\n> \n> \n>> +\n>> +    return controlList.get(id).get<int32_t>();\n> \n> I think some level of error checking would be useful when\n> querying the device.\n\nOkay. Do you have anything specific in mind that could go wrong here \nthat should be checked?\n\nThanks,\n  ~Matthias\n\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n> \n>> +}\n>> +\n>> +int CameraFlash::setSubdevControl(uint32_t id, int32_t value)\n>> +{\n>> +    ControlList flashCtrls(subdev_->controls());\n>> +\n>> +    flashCtrls.set(id, value);\n>> +\n>> +    if (subdev_->setControls(&flashCtrls))\n>> +        return -EINVAL;\n>> +\n>> +    return 0;\n>> +}\n>> +\n>> +int CameraFlash::validateDriver()\n>> +{\n>> +    int ret = 0;\n>> +    static constexpr uint32_t mandatoryControls[] = {\n>> +        V4L2_CID_FLASH_LED_MODE,\n>> +        V4L2_CID_FLASH_STROBE_SOURCE,\n>> +        V4L2_CID_FLASH_STROBE,\n>> +        V4L2_CID_FLASH_TIMEOUT,\n>> +        V4L2_CID_FLASH_INTENSITY,\n>> +        V4L2_CID_FLASH_TORCH_INTENSITY,\n>> +    };\n>> +\n>> +    for (uint32_t ctrl : mandatoryControls) {\n>> +        if (!controlInfoMap_->count(ctrl)) {\n>> +            LOG(CameraFlash, Error)\n>> +                << \"Mandatory V4L2 control \" << utils::hex(ctrl)\n>> +                << \" not available\";\n>> +            ret = -EINVAL;\n>> +        }\n>> +    }\n>> +\n>> +    if (ret) {\n>> +        LOG(CameraFlash, Error)\n>> +            << \"The flash kernel driver needs to be fixed\";\n>> +        LOG(CameraFlash, Error)\n>> +            << \"See Documentation/flash_driver_requirements.rst in\"\n>> +            << \" the libcamera sources for more information\";\n>> +        return ret;\n>> +    }\n>> +\n>> +    return ret;\n>> +}\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>> index \n>> b3ca27f217da4ba3a896ef7cbfb5502fa82a4907..0f125661a51e2431c1febc353cef30a1219f9ce7 100644\n>> --- a/src/libcamera/meson.build\n>> +++ b/src/libcamera/meson.build\n>> @@ -20,6 +20,7 @@ libcamera_internal_sources = files([\n>>       'bayer_format.cpp',\n>>       'byte_stream_buffer.cpp',\n>>       'camera_controls.cpp',\n>> +    'camera_flash.cpp',\n>>       'camera_lens.cpp',\n>>       'clock_recovery.cpp',\n>>       'control_serializer.cpp',\n>> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/ \n>> libcamera/sensor/camera_sensor_legacy.cpp\n>> index \n>> f9e685a9acc499fc91d51ed1d66780a0ad2d2a8f..632b66ea0aa15fcd654e7f0efb50c24cb9b973bf 100644\n>> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp\n>> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp\n>> @@ -31,6 +31,7 @@\n>>   #include <libcamera/ipa/core_ipa_interface.h>\n>>   #include \"libcamera/internal/bayer_format.h\"\n>> +#include \"libcamera/internal/camera_flash.h\"\n>>   #include \"libcamera/internal/camera_lens.h\"\n>>   #include \"libcamera/internal/camera_sensor.h\"\n>>   #include \"libcamera/internal/camera_sensor_properties.h\"\n>> @@ -68,6 +69,7 @@ public:\n>>       V4L2Subdevice *device() override { return subdev_.get(); }\n>>       CameraLens *focusLens() override { return focusLens_.get(); }\n>> +    CameraFlash *flash() override { return flash_.get(); }\n>>       const std::vector<unsigned int> &mbusCodes() const override \n>> { return mbusCodes_; }\n>>       std::vector<Size> sizes(unsigned int mbusCode) const override;\n>> @@ -139,6 +141,7 @@ private:\n>>       ControlList properties_;\n>>       std::unique_ptr<CameraLens> focusLens_;\n>> +    std::unique_ptr<CameraFlash> flash_;\n>>   };\n>>   /**\n>> @@ -665,6 +668,16 @@ int CameraSensorLegacy::discoverAncillaryDevices()\n>>               }\n>>               break;\n>> +        case MEDIA_ENT_F_FLASH:\n>> +            flash_ = std::make_unique<CameraFlash>(ancillary);\n>> +            ret = flash_->init();\n>> +            if (ret) {\n>> +                LOG(CameraSensor, Error)\n>> +                    << \"Flash initialisation failed, flash disabled\";\n>> +                flash_.reset();\n>> +            }\n>> +            break;\n>> +\n>>           default:\n>>               LOG(CameraSensor, Warning)\n>>                   << \"Unsupported ancillary entity function \"\n>> diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/ \n>> libcamera/sensor/camera_sensor_raw.cpp\n>> index \n>> 8ea4423698cd8c1eaae43eb5ba8b5d524b94d515..9d533d814b9df453aa4009a87818c1558bcbd665 100644\n>> --- a/src/libcamera/sensor/camera_sensor_raw.cpp\n>> +++ b/src/libcamera/sensor/camera_sensor_raw.cpp\n>> @@ -32,6 +32,7 @@\n>>   #include <libcamera/ipa/core_ipa_interface.h>\n>>   #include \"libcamera/internal/bayer_format.h\"\n>> +#include \"libcamera/internal/camera_flash.h\"\n>>   #include \"libcamera/internal/camera_lens.h\"\n>>   #include \"libcamera/internal/camera_sensor.h\"\n>>   #include \"libcamera/internal/camera_sensor_properties.h\"\n>> @@ -69,6 +70,7 @@ public:\n>>       V4L2Subdevice *device() override { return subdev_.get(); }\n>>       CameraLens *focusLens() override { return focusLens_.get(); }\n>> +    CameraFlash *flash() override { return flash_.get(); }\n>>       const std::vector<unsigned int> &mbusCodes() const override \n>> { return mbusCodes_; }\n>>       std::vector<Size> sizes(unsigned int mbusCode) const override;\n>> @@ -150,6 +152,7 @@ private:\n>>       ControlList properties_;\n>>       std::unique_ptr<CameraLens> focusLens_;\n>> +    std::unique_ptr<CameraFlash> flash_;\n>>   };\n>>   /**\n>> @@ -513,6 +516,16 @@ std::optional<int> CameraSensorRaw::init()\n>>               }\n>>               break;\n>> +        case MEDIA_ENT_F_FLASH:\n>> +            flash_ = std::make_unique<CameraFlash>(ancillary);\n>> +            ret = flash_->init();\n>> +            if (ret) {\n>> +                LOG(CameraSensor, Error)\n>> +                    << \"Flash initialisation failed, flash disabled\";\n>> +                flash_.reset();\n>> +            }\n>> +            break;\n>> +\n>>           default:\n>>               LOG(CameraSensor, Warning)\n>>                   << \"Unsupported ancillary entity function \"\n>>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 2F32EBEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Aug 2025 14:30:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 42F63692E1;\n\tThu, 28 Aug 2025 16:30:37 +0200 (CEST)","from lx20.hoststar.hosting (lx20.hoststar.hosting [168.119.41.54])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E56D8613B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Aug 2025 16:30:34 +0200 (CEST)","from 194-208-208-245.tele.net ([194.208.208.245]:60736\n\thelo=[192.168.0.207])\n\tby lx20.hoststar.hosting with esmtpsa (TLS1.3) tls\n\tTLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (Exim 4.93)\n\t(envelope-from <matthias.fend@emfend.at>)\n\tid 1urde4-00DcDm-J4; Thu, 28 Aug 2025 16:30:34 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=emfend.at header.i=@emfend.at\n\theader.b=\"etTIn6KR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=emfend.at;\n\ts=mail;\n\th=Content-Transfer-Encoding:Content-Type:In-Reply-To:From:References\n\t:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To:Cc:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=QmrxkUkL7lxvjCIBHbvP25GKrUTwp2Ov5yW+al7Ldyc=;\n\tb=etTIn6KRGZ+tdK14oZ+viSMFAM\n\t0a1GeviKz/BEa0KMMrWpVElVYR/ftqFTWVfks0uNk0CJVbBsO1etpeIj50B1l8gUu343nkWhABSKW\n\tkFZ18+ipj4QoI7O+9yi1OwZnxvGZugI2RXgzkdKef5haP62d0OhDE7W0Oxnuf8CRZcCo=;","Message-ID":"<555eb0fa-b9d8-41cb-8c18-0cc607e0f839@emfend.at>","Date":"Thu, 28 Aug 2025 16:30:31 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/5] libcamera: Add support for camera flash devices","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20250828-flash-support-v1-0-4c5dc674a05b@emfend.at>\n\t<20250828-flash-support-v1-2-4c5dc674a05b@emfend.at>\n\t<c48fe6d6-6832-47dd-9b38-52465a474306@ideasonboard.com>","Content-Language":"de-DE","From":"Matthias Fend <matthias.fend@emfend.at>","In-Reply-To":"<c48fe6d6-6832-47dd-9b38-52465a474306@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-Spam-Score":"","X-Spam-Bar":"","X-Spam-Report":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35608,"web_url":"https://patchwork.libcamera.org/comment/35608/","msgid":"<cca79c31-22b4-4034-a112-6963d61bc686@ideasonboard.com>","date":"2025-08-28T14:51:51","subject":"Re: [PATCH 2/5] libcamera: Add support for camera flash devices","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 08. 28. 16:30 keltezéssel, Matthias Fend írta:\n> Hi Barnabás,\n> \n> thanks a lot for your feedback!\n> \n> Am 28.08.2025 um 15:44 schrieb Barnabás Pőcze:\n>> Hi\n>>\n>> 2025. 08. 28. 15:09 keltezéssel, Matthias Fend írta:\n>>> Add a class to model camera flash devices. Currently, only v4l2 flash\n>>> devices are supported. The v4l2 flash devices are implemented similar to\n>>> the camera lenses.\n>>>\n>>> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>\n>>> ---\n>>>   include/libcamera/internal/camera_flash.h     |  75 ++++++++\n>>>   include/libcamera/internal/camera_sensor.h    |   2 +\n>>>   src/libcamera/camera_flash.cpp                | 248 ++++++++++++++++ ++++++++++\n>>>   src/libcamera/meson.build                     |   1 +\n>>>   src/libcamera/sensor/camera_sensor_legacy.cpp |  13 ++\n>>>   src/libcamera/sensor/camera_sensor_raw.cpp    |  13 ++\n>>>   6 files changed, 352 insertions(+)\n>>>\n>>> diff --git a/include/libcamera/internal/camera_flash.h b/include/ libcamera/internal/camera_flash.h\n>>> new file mode 100644\n>>> index 0000000000000000000000000000000000000000..e41afef2ab84852a340a12a012e3994f00cac27a\n>>> --- /dev/null\n>>> +++ b/include/libcamera/internal/camera_flash.h\n>>> @@ -0,0 +1,75 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2025, matthias.fend@emfend.at\n>>> + *\n>>> + * Camera flash support\n>>> + */\n>>> +#pragma once\n>>> +\n>>> +#include <memory>\n>>> +#include <stdint.h>\n>>> +#include <string>\n>>> +\n>>> +#include <libcamera/base/class.h>\n>>> +#include <libcamera/base/log.h>\n>>> +\n>>> +#include <libcamera/controls.h>\n>>> +\n>>> +namespace libcamera {\n>>> +\n>>> +class MediaEntity;\n>>> +class V4L2Subdevice;\n>>> +\n>>> +class CameraFlash : protected Loggable\n>>> +{\n>>> +public:\n>>> +    enum Mode {\n>>> +        None,\n>>> +        Flash,\n>>> +        Torch,\n>>> +    };\n>>> +\n>>> +    enum StrobeSource {\n>>> +        Software,\n>>> +        External,\n>>> +    };\n>>\n>> I think instead of defining these again, you should use the generated enums from\n>> `control_ids.h` since the return values of `getMode()` and `getStrobeSource()` are\n>> directly set as the value for `controls::draft::Flash{Mode,StrobeSource} `. Or at the\n>> very least some `static_assert()`s would be good. But I think using the generated\n>> enumerators would be preferable.\n> \n> I wanted to have a dedicated internal API here and not mix it with the external control API.\n> But you're right, for the controls in the metadata, the return values ​​of getMode() and getStrobeSource() should also be mapped (as is already done for the incoming controls).\n> \n\nThe `CameraSensor*` types also reuse `controls::draft::TestPatternModeEnum`, I don't see any\nissues with it if you just end up duplicating the values. In any case, if you want the separate\nenumerators, please try to make them `enum class`.\n\n\n>>\n>>\n>>> +\n>>> +    explicit CameraFlash(const MediaEntity *entity);\n>>> +    ~CameraFlash();\n>>> +    int init();\n>>> +    Mode getMode() const;\n>>> +    int setMode(Mode mode);\n>>> +    const ControlInfo &getFlashIntensityInfo() const;\n>>> +    int32_t getFlashIntensity() const;\n>>> +    int setFlashIntensity(int32_t intensity);\n>>> +    const ControlInfo &getFlashTimeoutInfo() const;\n>>> +    int32_t getFlashTimeout() const;\n>>> +    int setFlashTimeout(int32_t timeout_us);\n>>> +    StrobeSource getStrobeSource() const;\n>>> +    int setStrobeSource(StrobeSource source);\n>>> +    int startStrobe();\n>>> +    int stopStrobe();\n>>> +    const ControlInfo &getTorchIntensityInfo() const;\n>>> +    int32_t getTorchIntensity() const;\n>>> +    int setTorchIntensity(int32_t intensity);\n>>> +\n>>> +    const std::string &model() const;\n>>> +    const ControlInfoMap &controls() const;\n\nAre these two used? In any case, I'd make the inline, they are quite trivial.\n\n\n>>> +\n>>> +protected:\n>>> +    std::string logPrefix() const override;\n>>> +\n>>> +private:\n>>> +    LIBCAMERA_DISABLE_COPY_AND_MOVE(CameraFlash)\n>>> +\n>>> +    int32_t getSubdevControl(uint32_t id) const;\n>>> +    int setSubdevControl(uint32_t id, int32_t value);\n>>> +    int validateDriver();\n>>> +\n>>> +    const MediaEntity *entity_;\n>>> +    std::unique_ptr<V4L2Subdevice> subdev_;\n>>> +    std::string model_;\n>>> +    const ControlInfoMap *controlInfoMap_ = nullptr;\n>>\n>> What is the motivation for caching this value?\n> \n> Just to make it a little easier to read and to avoid constructs like this:\n> subdev_->controls().find(V4L2_CID_FLASH_TORCH_INTENSITY)->second;\n\nACK. In any case, I think what you wrote or even just `controls().find(...)` is fine.\n\n\n> \n>>\n>>\n>>> +};\n>>> +\n>>> +} /* namespace libcamera */\n>>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/ libcamera/internal/camera_sensor.h\n>>> index f6ef4df170d43500bc652762e9a575010a6c1cbd..faeecf244c8d42a43eca52ae04813e0c2f80e516 100644\n>>> --- a/include/libcamera/internal/camera_sensor.h\n>>> +++ b/include/libcamera/internal/camera_sensor.h\n>>> @@ -28,6 +28,7 @@\n>>>   namespace libcamera {\n>>> +class CameraFlash;\n>>>   class CameraLens;\n>>>   class MediaEntity;\n>>>   class SensorConfiguration;\n>>> @@ -48,6 +49,7 @@ public:\n>>>       virtual V4L2Subdevice *device() = 0;\n>>>       virtual CameraLens *focusLens() = 0;\n>>> +    virtual CameraFlash *flash() = 0;\n>>>       virtual const std::vector<unsigned int> &mbusCodes() const = 0;\n>>>       virtual std::vector<Size> sizes(unsigned int mbusCode) const = 0;\n>>> diff --git a/src/libcamera/camera_flash.cpp b/src/libcamera/ camera_flash.cpp\n>>> new file mode 100644\n>>> index 0000000000000000000000000000000000000000..4702c590c91e0bcb20d173e7ce03608e1ae6ecfd\n>>> --- /dev/null\n>>> +++ b/src/libcamera/camera_flash.cpp\n>>> @@ -0,0 +1,248 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2025, matthias.fend@emfend.at\n>>> + *\n>>> + * Camera flash support\n>>> + */\n>>> +\n>>> +#include \"libcamera/internal/camera_flash.h\"\n>>> +\n>>> +#include <libcamera/base/utils.h>\n>>> +\n>>> +#include \"libcamera/internal/v4l2_subdevice.h\"\n>>> +\n>>> +namespace libcamera {\n>>> +\n>>> +LOG_DEFINE_CATEGORY(CameraFlash)\n>>> +\n>>> +CameraFlash::CameraFlash(const MediaEntity *entity)\n>>> +    : entity_(entity)\n>>> +{\n>>> +}\n>>> +\n>>> +CameraFlash::~CameraFlash() = default;\n>>> +\n>>> +int CameraFlash::init()\n>>> +{\n>>> +    if (entity_->function() != MEDIA_ENT_F_FLASH) {\n>>> +        LOG(CameraFlash, Error)\n>>> +            << \"Invalid flash function \"\n>>> +            << utils::hex(entity_->function());\n>>> +        return -EINVAL;\n>>> +    }\n>>> +\n>>> +    subdev_ = std::make_unique<V4L2Subdevice>(entity_);\n>>> +    int ret = subdev_->open();\n>>> +    if (ret < 0)\n>>> +        return ret;\n>>> +\n>>> +    controlInfoMap_ = &subdev_->controls();\n>>> +\n>>> +    ret = validateDriver();\n>>> +    if (ret)\n>>> +        return ret;\n>>> +\n>>> +    model_ = subdev_->model();\n>>> +\n>>> +    return 0;\n>>> +}\n>>> +\n>>> +CameraFlash::Mode CameraFlash::getMode() const\n>>> +{\n>>> +    Mode m;\n>>> +\n>>> +    switch (getSubdevControl(V4L2_CID_FLASH_LED_MODE)) {\n>>> +    case V4L2_FLASH_LED_MODE_FLASH:\n>>> +        m = Flash;\n>>> +        break;\n>>> +    case V4L2_FLASH_LED_MODE_TORCH:\n>>> +        m = Torch;\n>>> +        break;\n>>> +    case V4L2_FLASH_LED_MODE_NONE:\n>>> +    default:\n>>> +        m = None;\n>>> +        break;\n>>> +    }\n>>> +\n>>> +    return m;\n>>> +}\n>>> +\n>>> +int CameraFlash::setMode(Mode mode)\n>>> +{\n>>> +    int32_t m;\n>>> +\n>>> +    switch (mode) {\n>>> +    case Flash:\n>>> +        m = V4L2_FLASH_LED_MODE_FLASH;\n>>> +        break;\n>>> +    case Torch:\n>>> +        m = V4L2_FLASH_LED_MODE_TORCH;\n>>> +        break;\n>>> +    case None:\n>>> +        m = V4L2_FLASH_LED_MODE_NONE;\n>>> +        break;\n>>> +    default:\n>>> +        return -EINVAL;\n>>> +    }\n>>> +\n>>> +    return setSubdevControl(V4L2_CID_FLASH_LED_MODE, m);\n>>> +}\n>>> +\n>>> +const ControlInfo &CameraFlash::getFlashIntensityInfo() const\n>>> +{\n>>> +    return controlInfoMap_->find(V4L2_CID_FLASH_INTENSITY)->second;\n>>> +}\n>>> +\n>>> +int32_t CameraFlash::getFlashIntensity() const\n>>> +{\n>>> +    return getSubdevControl(V4L2_CID_FLASH_INTENSITY);\n>>> +}\n>>> +\n>>> +int CameraFlash::setFlashIntensity(int32_t intensity)\n>>> +{\n>>> +    return setSubdevControl(V4L2_CID_FLASH_INTENSITY, intensity);\n>>> +}\n>>> +\n>>> +const ControlInfo &CameraFlash::getFlashTimeoutInfo() const\n>>> +{\n>>> +    return controlInfoMap_->find(V4L2_CID_FLASH_TIMEOUT)->second;\n>>> +}\n>>> +\n>>> +int32_t CameraFlash::getFlashTimeout() const\n>>> +{\n>>> +    return getSubdevControl(V4L2_CID_FLASH_TIMEOUT);\n>>> +}\n>>> +\n>>> +int CameraFlash::setFlashTimeout(int32_t timeout)\n>>> +{\n>>> +    return setSubdevControl(V4L2_CID_FLASH_TIMEOUT, timeout);\n>>> +}\n>>> +\n>>> +CameraFlash::StrobeSource CameraFlash::getStrobeSource() const\n>>> +{\n>>> +    StrobeSource s;\n>>> +\n>>> +    switch (getSubdevControl(V4L2_CID_FLASH_STROBE_SOURCE)) {\n>>> +    case V4L2_FLASH_STROBE_SOURCE_EXTERNAL:\n>>> +        s = External;\n>>> +        break;\n>>> +    case V4L2_FLASH_STROBE_SOURCE_SOFTWARE:\n>>> +    default:\n>>> +        s = Software;\n>>> +        break;\n>>> +    }\n>>> +\n>>> +    return s;\n>>> +}\n>>> +\n>>> +int CameraFlash::setStrobeSource(StrobeSource source)\n>>> +{\n>>> +    int32_t s;\n>>> +\n>>> +    switch (source) {\n>>> +    case External:\n>>> +        s = V4L2_FLASH_STROBE_SOURCE_EXTERNAL;\n>>> +        break;\n>>> +    case Software:\n>>> +        s = V4L2_FLASH_STROBE_SOURCE_SOFTWARE;\n>>> +        break;\n>>> +    default:\n>>> +        return -EINVAL;\n>>> +    }\n>>> +\n>>> +    return setSubdevControl(V4L2_CID_FLASH_STROBE_SOURCE, s);\n>>> +}\n>>> +\n>>> +int CameraFlash::startStrobe()\n>>> +{\n>>> +    return setSubdevControl(V4L2_CID_FLASH_STROBE, 1);\n>>> +}\n>>> +\n>>> +int CameraFlash::stopStrobe()\n>>> +{\n>>> +    return setSubdevControl(V4L2_CID_FLASH_STROBE_STOP, 1);\n>>> +}\n>>> +\n>>> +const ControlInfo &CameraFlash::getTorchIntensityInfo() const\n>>> +{\n>>> +    return controlInfoMap_->find(V4L2_CID_FLASH_TORCH_INTENSITY)- >second;\n>>> +}\n>>> +\n>>> +int32_t CameraFlash::getTorchIntensity() const\n>>> +{\n>>> +    return getSubdevControl(V4L2_CID_FLASH_TORCH_INTENSITY);\n>>> +}\n>>> +\n>>> +int CameraFlash::setTorchIntensity(int32_t intensity)\n>>> +{\n>>> +    return setSubdevControl(V4L2_CID_FLASH_TORCH_INTENSITY, intensity);\n>>> +}\n>>> +\n>>> +const std::string &CameraFlash::model() const\n>>> +{\n>>> +    return model_;\n>>> +}\n>>> +\n>>> +const ControlInfoMap &CameraFlash::controls() const\n>>> +{\n>>> +    return subdev_->controls();\n>>\n>> Why not `controlInfoMap_` here?\n> \n> True, if the variable already exists, it should probably always be used.\n> \n>>\n>>\n>>> +}\n>>> +\n>>> +std::string CameraFlash::logPrefix() const\n>>> +{\n>>> +    return \"'\" + entity_->name() + \"'\";\n>>> +}\n>>> +\n>>> +int32_t CameraFlash::getSubdevControl(uint32_t id) const\n>>> +{\n>>> +    ControlList controlList = subdev_- >getControls(std::vector<uint32_t>{ id });\n>>\n>>    getControls(std::array{ id })\n>>\n>> should work.\n> \n> ACK.\n> \n>>\n>>\n>>> +\n>>> +    return controlList.get(id).get<int32_t>();\n>>\n>> I think some level of error checking would be useful when\n>> querying the device.\n> \n> Okay. Do you have anything specific in mind that could go wrong here that should be checked?\n\nSomething like `controlList.empty()` or `controlList.contains(id)` at least.\nPossibly making it return `std::optional`.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> Thanks,\n>   ~Matthias\n> \n>>\n>>\n>> Regards,\n>> Barnabás Pőcze\n>>\n>>\n>>> +}\n>>> +\n>>> +int CameraFlash::setSubdevControl(uint32_t id, int32_t value)\n>>> +{\n>>> +    ControlList flashCtrls(subdev_->controls());\n>>> +\n>>> +    flashCtrls.set(id, value);\n>>> +\n>>> +    if (subdev_->setControls(&flashCtrls))\n>>> +        return -EINVAL;\n>>> +\n>>> +    return 0;\n>>> +}\n>>> +\n>>> +int CameraFlash::validateDriver()\n>>> +{\n>>> +    int ret = 0;\n>>> +    static constexpr uint32_t mandatoryControls[] = {\n>>> +        V4L2_CID_FLASH_LED_MODE,\n>>> +        V4L2_CID_FLASH_STROBE_SOURCE,\n>>> +        V4L2_CID_FLASH_STROBE,\n>>> +        V4L2_CID_FLASH_TIMEOUT,\n>>> +        V4L2_CID_FLASH_INTENSITY,\n>>> +        V4L2_CID_FLASH_TORCH_INTENSITY,\n>>> +    };\n>>> +\n>>> +    for (uint32_t ctrl : mandatoryControls) {\n>>> +        if (!controlInfoMap_->count(ctrl)) {\n>>> +            LOG(CameraFlash, Error)\n>>> +                << \"Mandatory V4L2 control \" << utils::hex(ctrl)\n>>> +                << \" not available\";\n>>> +            ret = -EINVAL;\n>>> +        }\n>>> +    }\n>>> +\n>>> +    if (ret) {\n>>> +        LOG(CameraFlash, Error)\n>>> +            << \"The flash kernel driver needs to be fixed\";\n>>> +        LOG(CameraFlash, Error)\n>>> +            << \"See Documentation/flash_driver_requirements.rst in\"\n>>> +            << \" the libcamera sources for more information\";\n>>> +        return ret;\n>>> +    }\n>>> +\n>>> +    return ret;\n>>> +}\n>>> +\n>>> +} /* namespace libcamera */\n>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>>> index b3ca27f217da4ba3a896ef7cbfb5502fa82a4907..0f125661a51e2431c1febc353cef30a1219f9ce7 100644\n>>> --- a/src/libcamera/meson.build\n>>> +++ b/src/libcamera/meson.build\n>>> @@ -20,6 +20,7 @@ libcamera_internal_sources = files([\n>>>       'bayer_format.cpp',\n>>>       'byte_stream_buffer.cpp',\n>>>       'camera_controls.cpp',\n>>> +    'camera_flash.cpp',\n>>>       'camera_lens.cpp',\n>>>       'clock_recovery.cpp',\n>>>       'control_serializer.cpp',\n>>> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/ libcamera/sensor/camera_sensor_legacy.cpp\n>>> index f9e685a9acc499fc91d51ed1d66780a0ad2d2a8f..632b66ea0aa15fcd654e7f0efb50c24cb9b973bf 100644\n>>> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp\n>>> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp\n>>> @@ -31,6 +31,7 @@\n>>>   #include <libcamera/ipa/core_ipa_interface.h>\n>>>   #include \"libcamera/internal/bayer_format.h\"\n>>> +#include \"libcamera/internal/camera_flash.h\"\n>>>   #include \"libcamera/internal/camera_lens.h\"\n>>>   #include \"libcamera/internal/camera_sensor.h\"\n>>>   #include \"libcamera/internal/camera_sensor_properties.h\"\n>>> @@ -68,6 +69,7 @@ public:\n>>>       V4L2Subdevice *device() override { return subdev_.get(); }\n>>>       CameraLens *focusLens() override { return focusLens_.get(); }\n>>> +    CameraFlash *flash() override { return flash_.get(); }\n>>>       const std::vector<unsigned int> &mbusCodes() const override { return mbusCodes_; }\n>>>       std::vector<Size> sizes(unsigned int mbusCode) const override;\n>>> @@ -139,6 +141,7 @@ private:\n>>>       ControlList properties_;\n>>>       std::unique_ptr<CameraLens> focusLens_;\n>>> +    std::unique_ptr<CameraFlash> flash_;\n>>>   };\n>>>   /**\n>>> @@ -665,6 +668,16 @@ int CameraSensorLegacy::discoverAncillaryDevices()\n>>>               }\n>>>               break;\n>>> +        case MEDIA_ENT_F_FLASH:\n>>> +            flash_ = std::make_unique<CameraFlash>(ancillary);\n>>> +            ret = flash_->init();\n>>> +            if (ret) {\n>>> +                LOG(CameraSensor, Error)\n>>> +                    << \"Flash initialisation failed, flash disabled\";\n>>> +                flash_.reset();\n>>> +            }\n>>> +            break;\n>>> +\n>>>           default:\n>>>               LOG(CameraSensor, Warning)\n>>>                   << \"Unsupported ancillary entity function \"\n>>> diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/ libcamera/sensor/camera_sensor_raw.cpp\n>>> index 8ea4423698cd8c1eaae43eb5ba8b5d524b94d515..9d533d814b9df453aa4009a87818c1558bcbd665 100644\n>>> --- a/src/libcamera/sensor/camera_sensor_raw.cpp\n>>> +++ b/src/libcamera/sensor/camera_sensor_raw.cpp\n>>> @@ -32,6 +32,7 @@\n>>>   #include <libcamera/ipa/core_ipa_interface.h>\n>>>   #include \"libcamera/internal/bayer_format.h\"\n>>> +#include \"libcamera/internal/camera_flash.h\"\n>>>   #include \"libcamera/internal/camera_lens.h\"\n>>>   #include \"libcamera/internal/camera_sensor.h\"\n>>>   #include \"libcamera/internal/camera_sensor_properties.h\"\n>>> @@ -69,6 +70,7 @@ public:\n>>>       V4L2Subdevice *device() override { return subdev_.get(); }\n>>>       CameraLens *focusLens() override { return focusLens_.get(); }\n>>> +    CameraFlash *flash() override { return flash_.get(); }\n>>>       const std::vector<unsigned int> &mbusCodes() const override { return mbusCodes_; }\n>>>       std::vector<Size> sizes(unsigned int mbusCode) const override;\n>>> @@ -150,6 +152,7 @@ private:\n>>>       ControlList properties_;\n>>>       std::unique_ptr<CameraLens> focusLens_;\n>>> +    std::unique_ptr<CameraFlash> flash_;\n>>>   };\n>>>   /**\n>>> @@ -513,6 +516,16 @@ std::optional<int> CameraSensorRaw::init()\n>>>               }\n>>>               break;\n>>> +        case MEDIA_ENT_F_FLASH:\n>>> +            flash_ = std::make_unique<CameraFlash>(ancillary);\n>>> +            ret = flash_->init();\n>>> +            if (ret) {\n>>> +                LOG(CameraSensor, Error)\n>>> +                    << \"Flash initialisation failed, flash disabled\";\n>>> +                flash_.reset();\n>>> +            }\n>>> +            break;\n>>> +\n>>>           default:\n>>>               LOG(CameraSensor, Warning)\n>>>                   << \"Unsupported ancillary entity function \"\n>>>\n>>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7F28FBD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Aug 2025 14:51:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 351E7692F3;\n\tThu, 28 Aug 2025 16:51:57 +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 7F561613B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Aug 2025 16:51:55 +0200 (CEST)","from [192.168.33.12] (185.221.143.232.nat.pool.zt.hu\n\t[185.221.143.232])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C658720EE;\n\tThu, 28 Aug 2025 16:50:50 +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=\"mpyYw4jm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1756392651;\n\tbh=6522ki6R4ljYo7pPUwkQWIPtRCs0iQ8JkDzIydvEDmY=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=mpyYw4jm4sMi6Xp2m2PurmD5t8Cqao9xo3+BJQFHzNk6OLfitjB/szQcHkK8RiJQV\n\tjcM8FwQF5eYDXSZh+uO7zgxxlKnyM+fhRDVv4RF7eb/YtBeCzszYZTcfwq2YUptnq9\n\tiD5BldFfaWwXVafaFHm4sS/hiswGpzFZ7H/VgQsA=","Message-ID":"<cca79c31-22b4-4034-a112-6963d61bc686@ideasonboard.com>","Date":"Thu, 28 Aug 2025 16:51:51 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/5] libcamera: Add support for camera flash devices","To":"Matthias Fend <matthias.fend@emfend.at>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20250828-flash-support-v1-0-4c5dc674a05b@emfend.at>\n\t<20250828-flash-support-v1-2-4c5dc674a05b@emfend.at>\n\t<c48fe6d6-6832-47dd-9b38-52465a474306@ideasonboard.com>\n\t<555eb0fa-b9d8-41cb-8c18-0cc607e0f839@emfend.at>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<555eb0fa-b9d8-41cb-8c18-0cc607e0f839@emfend.at>","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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]