[{"id":38310,"web_url":"https://patchwork.libcamera.org/comment/38310/","msgid":"<854in2udvi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-02-27T20:32:01","subject":"Re: [PATCH] software_isp: Add focus control","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi,\n\nthank you for the patch.\n\nPavel Machek writes:\n\n> From: Vasiliy Doylov \n>\n> This adds manual focus to software_isp.\n\nWe usually use \"libcamera: software_isp: \" prefix in the titles of\ncommits for software ISP changes.\n\nAnd nothing to add about the purpose of the patch etc. in the commit\ndescription?\n\n> Signed-off-by: Vasiliy Doylov \n> Signed-off-by: Pavel Machek \n> --\n>\n> I'm working on millicam -- you can see the results in\n> git@gitlab.com:tui/libcamera.git branch millicam_af_6 . Aim is to\n> eventually take photos and videos and autofocus is required for\n> that. I'm currently tuning/cleaning that up, but this makes sense on\n> its own and it would be nice to get it merged early.\n>\n> Thanks,\n> \t\t\t\t\t\t\t\tPavel\n>\n> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n> index ad89c9b3c..9e5e05fc0 100644\n> --- a/include/libcamera/internal/software_isp/software_isp.h\n> +++ b/include/libcamera/internal/software_isp/software_isp.h\n> @@ -86,11 +86,11 @@ public:\n> \tSignal outputBufferReady;\n> \tSignal ispStatsReady;\n> \tSignal metadataReady;\n> -\tSignal setSensorControls;\n> +\tSignal setSensorControls;\n> \n> private:\n> \tvoid saveIspParams();\n> -\tvoid setSensorCtrls(const ControlList &sensorControls);\n> +\tvoid setSensorCtrls(const ControlList &sensorControls, const ControlList &lensControls);\n> \tvoid statsReady(uint32_t frame, uint32_t bufferId);\n> \tvoid inputReady(FrameBuffer *input);\n> \tvoid outputReady(FrameBuffer *output);\n> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom\n> index 77328c5fd..e5767532c 100644\n> --- a/include/libcamera/ipa/soft.mojom\n> +++ b/include/libcamera/ipa/soft.mojom\n> @@ -10,6 +10,7 @@ import \"include/libcamera/ipa/core.mojom\";\n> \n> struct IPAConfigInfo {\n> \tlibcamera.ControlInfoMap sensorControls;\n> +\tlibcamera.ControlInfoMap lensControls;\n> };\n> \n> interface IPASoftInterface {\n> @@ -32,7 +33,7 @@ interface IPASoftInterface {\n> };\n> \n> interface IPASoftEventInterface {\n> -\tsetSensorControls(libcamera.ControlList sensorControls);\n> +\tsetSensorControls(libcamera.ControlList sensorControls, libcamera.ControlList lensControls);\n> \tsetIspParams();\n> \tmetadataReady(uint32 frame, libcamera.ControlList metadata);\n> };\n> diff --git a/src/ipa/simple/algorithms/af.cpp b/src/ipa/simple/algorithms/af.cpp\n> new file mode 100644\n> index 000000000..b51ed95e4\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/af.cpp\n> @@ -0,0 +1,71 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025 Vasiliy Doylov \n> + *\n> + * Auto focus\n\nThis is actually manual focus, right? So the things should not be named\n\"af\".\n\n> + */\n> +\n> +#include \"af.h\"\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"control_ids.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPASoftAutoFocus)\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +Af::Af()\n> +{\n> +}\n> +\n> +int Af::init(IPAContext &context,\n> +\t [[maybe_unused]] const YamlObject &tuningData)\n> +{\n> +\tcontext.ctrlMap[&controls::LensPosition] = ControlInfo(0.0f, 100.0f, 50.0f);\n\nLensPosition documentation in control_ids_core.yaml says:\n\n The LensPosition control is ignored unless the AfMode is set to\n AfModeManual, though the value is reported back unconditionally in all\n modes.\n\nShould AfMode be added too?\n\n> +\treturn 0;\n> +}\n> +\n> +int Af::configure(IPAContext &context,\n> +\t\t [[maybe_unused]] const IPAConfigInfo &configInfo)\n> +{\n> +\tcontext.activeState.knobs.focus_pos = std::optional();\n> +\n> +\treturn 0;\n> +}\n> +\n> +void Af::queueRequest([[maybe_unused]] typename Module::Context &context,\n\nThis one is used.\n\n> +\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t [[maybe_unused]] typename Module::FrameContext &frameContext,\n> +\t\t const ControlList &controls)\n> +{\n> +\tconst auto &focus_pos = controls.get(controls::LensPosition);\n> +\tif (focus_pos.has_value()) {\n> +\t\tcontext.activeState.knobs.focus_pos = focus_pos;\n> +\t\tLOG(IPASoftAutoFocus, Debug) << \"Setting focus position to \" << focus_pos.value();\n> +\t}\n> +}\n> +\n> +void Af::updateFocus([[maybe_unused]] IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, [[maybe_unused]] double exposureMSV)\n\n`context' and `frameContext' are actually used. `exposureMSV' is indeed\nunused but why is it passed here then?\n\n> +{\n> +\tframeContext.lens.focus_pos = context.activeState.knobs.focus_pos.value_or(50.0) / 100.0 * (context.configuration.focus.focus_max - context.configuration.focus.focus_min);\n> +}\n> +\n> +void Af::process([[maybe_unused]] IPAContext &context,\n\nUsed.\n\n> +\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n\nUsed.\n\n> +\t\t [[maybe_unused]] const SwIspStats *stats,\n> +\t\t [[maybe_unused]] ControlList &metadata)\n> +{\n> +\tupdateFocus(context, frameContext, 0);\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Af, \"Af\")\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/af.h b/src/ipa/simple/algorithms/af.h\n> new file mode 100644\n> index 000000000..a575ef102\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/af.h\n> @@ -0,0 +1,40 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025 Vasiliy Doylov \n> + *\n> + * Auto focus\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +class Af : public Algorithm\n> +{\n> +public:\n> +\tAf();\n> +\t~Af() = default;\n> +\n> +\tint init(IPAContext &context, const YamlObject &tuningData) override;\n> +\tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> +\tvoid queueRequest(typename Module::Context &context,\n> +\t\t\t const uint32_t frame,\n> +\t\t\t typename Module::FrameContext &frameContext,\n> +\t\t\t const ControlList &controls)\n> +\t\toverride;\n> +\tvoid process(IPAContext &context, const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t const SwIspStats *stats,\n> +\t\t ControlList &metadata) override;\n> +\n> +private:\n> +\tvoid updateFocus(IPAContext &context, IPAFrameContext &frameContext, double focus);\n> +};\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n> index 73c637220..8950b008f 100644\n> --- a/src/ipa/simple/algorithms/meson.build\n> +++ b/src/ipa/simple/algorithms/meson.build\n> @@ -6,4 +6,5 @@ soft_simple_ipa_algorithms = files([\n> 'agc.cpp',\n> 'blc.cpp',\n> 'ccm.cpp',\n> + 'af.cpp',\n> ])\n> diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml\n> index fc90ca526..ede277d1d 100644\n> --- a/src/ipa/simple/data/uncalibrated.yaml\n> +++ b/src/ipa/simple/data/uncalibrated.yaml\n> @@ -16,4 +16,5 @@ algorithms:\n> # 0, 0, 1]\n> - Adjust:\n> - Agc:\n> + - Af:\n> ...\n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index 34f7403a4..a436fc76a 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -33,6 +33,9 @@ struct IPASessionConfiguration {\n> \tstruct {\n> \t\tstd::optional level;\n> \t} black;\n> +\tstruct {\n> +\t\tint32_t focus_min, focus_max;\n> +\t} focus;\n> };\n> \n> struct IPAActiveState {\n> @@ -60,6 +63,8 @@ struct IPAActiveState {\n> \t\t/* 0..2 range, 1.0 = normal */\n> \t\tstd::optional contrast;\n> \t\tstd::optional saturation;\n> +\t\t/* 0..100 range, 50.0 = normal */\n\nWhat is this range and what is \"normal\" in the context of focus\nposition? Is there any relationship to LensPosition as described in\ncontrol_ids_core.yaml?\n\n> +\t\tstd::optional focus_pos;\n> \t} knobs;\n> };\n> \n> @@ -71,6 +76,10 @@ struct IPAFrameContext : public FrameContext {\n> \t\tdouble gain;\n> \t} sensor;\n> \n> +\tstruct {\n> +\t\tint32_t focus_pos;\n> +\t} lens;\n> +\n> \tstruct {\n> \t\tdouble red;\n> \t\tdouble blue;\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index 6bef597c8..4f63fa3b4 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -78,6 +78,7 @@ private:\n> \tSwIspStats *stats_;\n> \tstd::unique_ptr camHelper_;\n> \tControlInfoMap sensorInfoMap_;\n> +\tControlInfoMap lensInfoMap_;\n> \n> \t/* Local parameter storage */\n> \tstruct IPAContext context_;\n> @@ -202,6 +203,7 @@ int IPASoftSimple::init(const IPASettings &settings,\n> int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> {\n> \tsensorInfoMap_ = configInfo.sensorControls;\n> +\tlensInfoMap_ = configInfo.lensControls;\n> \n> \tconst ControlInfo &exposureInfo = sensorInfoMap_.find(V4L2_CID_EXPOSURE)->second;\n> \tconst ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second;\n> @@ -211,6 +213,17 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> \tcontext_.activeState = {};\n> \tcontext_.frameContexts.clear();\n> \n> +\tif (lensInfoMap_.empty()) {\n> +\t\tLOG(IPASoft, Warning) << \"No camera leans found! Focus control disabled.\";\n\ns/leans/lens/\n\n> +\t\tcontext_.configuration.focus.focus_min = 0;\n> +\t\tcontext_.configuration.focus.focus_max = 0;\n> +\t} else {\n> +\t\tconst ControlInfo &lensInfo = lensInfoMap_.find(V4L2_CID_FOCUS_ABSOLUTE)->second;\n> +\t\tcontext_.configuration.focus.focus_min = lensInfo.min().get();\n> +\t\tcontext_.configuration.focus.focus_max = lensInfo.max().get();\n> +\t\tLOG(IPASoft, Warning) << \"Camera leans found! Focus: \" << context_.configuration.focus.focus_min << \"-\" << context_.configuration.focus.focus_max;\n\ns/leans/lens/\n\n> +\t}\n> +\n> \tcontext_.configuration.agc.lineDuration =\n> \t\tcontext_.sensorInfo.minLineLength * 1.0s / context_.sensorInfo.pixelRate;\n> \tcontext_.configuration.agc.exposureMin = exposureInfo.min().get();\n> @@ -325,7 +338,10 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> \tctrls.set(V4L2_CID_ANALOGUE_GAIN,\n> \t\t static_cast(camHelper_ ? camHelper_->gainCode(againNew) : againNew));\n> \n> -\tsetSensorControls.emit(ctrls);\n> +\tControlList lens_ctrls(lensInfoMap_);\n> +\tlens_ctrls.set(V4L2_CID_FOCUS_ABSOLUTE, frameContext.lens.focus_pos);\n> +\n> +\tsetSensorControls.emit(ctrls, lens_ctrls);\n> }\n> \n> std::string IPASoftSimple::logPrefix() const\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 4a0b9f58d..4079f0973 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -33,6 +33,7 @@\n> #include \n> \n> #include \"libcamera/internal/camera.h\"\n> +#include \"libcamera/internal/camera_lens.h\"\n> #include \"libcamera/internal/camera_manager.h\"\n> #include \"libcamera/internal/camera_sensor.h\"\n> #include \"libcamera/internal/camera_sensor_properties.h\"\n> @@ -48,6 +49,8 @@\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> \n> +#include \"libcamera/controls.h\"\n> +\n> namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(SimplePipeline)\n> @@ -371,7 +374,7 @@ private:\n> \n> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n> -\tvoid setSensorControls(const ControlList &sensorControls);\n> +\tvoid setSensorControls(const ControlList &sensorControls, const ControlList &lensControls);\n> };\n> \n> class SimpleCameraConfiguration : public CameraConfiguration\n> @@ -1041,7 +1044,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata\n> \ttryCompleteRequest(info->request);\n> }\n> \n> -void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n> +void SimpleCameraData::setSensorControls(const ControlList &sensorControls, const ControlList &lensControls)\n> {\n> \tdelayedCtrls_->push(sensorControls);\n> \t/*\n> @@ -1052,10 +1055,21 @@ void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n> \t * but it also bypasses delayedCtrls_, creating AGC regulation issues.\n> \t * Both problems should be fixed.\n> \t */\n> -\tif (!frameStartEmitter_) {\n> -\t\tControlList ctrls(sensorControls);\n> -\t\tsensor_->setControls(&ctrls);\n> -\t}\n> +\tif (frameStartEmitter_)\n> +\t\treturn;\n> +\n> +\tControlList ctrls(sensorControls);\n> +\tsensor_->setControls(&ctrls);\n> +\n> +\tCameraLens *focusLens = sensor_->focusLens();\n> +\tif (!focusLens)\n> +\t\treturn;\n> +\n> +\tif (!lensControls.contains(V4L2_CID_FOCUS_ABSOLUTE))\n> +\t\treturn;\n> +\n> +\tconst ControlValue &focusValue = lensControls.get(V4L2_CID_FOCUS_ABSOLUTE);\n> +\tfocusLens->setFocusPosition(focusValue.get());\n> }\n> \n> /* Retrieve all source pads connected to a sink pad through active routes. */\n> @@ -1593,6 +1607,10 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \t} else {\n> \t\tipa::soft::IPAConfigInfo configInfo;\n> \t\tconfigInfo.sensorControls = data->sensor_->controls();\n> +\t\tif (data->sensor_->focusLens() != nullptr)\n> +\t\t\tconfigInfo.lensControls = data->sensor_->focusLens()->controls();\n> +\t\telse\n> +\t\t\tconfigInfo.lensControls = ControlInfoMap();\n> \t\treturn data->swIsp_->configure(inputCfg, outputCfgs, configInfo);\n> \t}\n> }\n> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> index c2baaf0bf..df0c5e054 100644\n> --- a/src/libcamera/software_isp/software_isp.cpp\n> +++ b/src/libcamera/software_isp/software_isp.cpp\n> @@ -408,9 +408,9 @@ void SoftwareIsp::saveIspParams()\n> \tdebayerParams_ = *sharedParams_;\n> }\n> \n> -void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)\n> +void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls, const ControlList &lensControls)\n> {\n> -\tsetSensorControls.emit(sensorControls);\n> +\tsetSensorControls.emit(sensorControls, lensControls);\n> }\n> \n> void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)","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 55C48C0DA4\n\tfor ;\n\tFri, 27 Feb 2026 20:32:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9368C622EF;\n\tFri, 27 Feb 2026 21:32:11 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E2728622AD\n\tfor ;\n\tFri, 27 Feb 2026 21:32:09 +0100 (CET)","from mail-wm1-f70.google.com (mail-wm1-f70.google.com\n\t[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-271-WmB6t3DSNKS936QDtKeoVw-1; Fri, 27 Feb 2026 15:32:07 -0500","by mail-wm1-f70.google.com with SMTP id\n\t5b1f17b1804b1-48069a43217so20724935e9.1\n\tfor ;\n\tFri, 27 Feb 2026 12:32:06 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-483bd75df90sm241413695e9.14.2026.02.27.12.32.02\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 27 Feb 2026 12:32:02 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Qn2kM0M+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1772224328;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=sRrVeF8RAVZJvgoqBDYnpUMgatpsJCFeHIujJDol4ZQ=;\n\tb=Qn2kM0M+68o4oJapMKZgkpwOJDtr0dqWwO0jH1stdLOrf1KowWHYvPM8FXccz3k0fnnjXr\n\t+LDVh2VOv1dYxEn1utD4RoiRaETpcwX9kjxFwnhIoRfGpLvZh1RYHucvsa0Zj1a/2eCy+m\n\ttSVl3OkljfAdnkCSwXRKOFqJ40nSRXw=","X-MC-Unique":"WmB6t3DSNKS936QDtKeoVw-1","X-Mimecast-MFC-AGG-ID":"WmB6t3DSNKS936QDtKeoVw_1772224325","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1772224325; x=1772829125;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=sRrVeF8RAVZJvgoqBDYnpUMgatpsJCFeHIujJDol4ZQ=;\n\tb=oki+g/3vBTLjEankEwgNFoNDZcCxHOS9AK82OQuq3QWoe+gWwJdebmmwkhZSYrouLA\n\thi1YmjQg5KtyGhKgwJatDGd//oi/ul4XFmbT9gyjmQ/J3ID6cEdK7iM/Y34u7Vozq0O0\n\ter5Ucd4uoXHCHRp6EvGSM6wSwDYaOGPADpl/QSHxNqwfQybkqk0st1nfn7J40Nq2oPic\n\tkuGwrA70e553zww4mP0DfC4oMQewfqKxKbG/R82+/FjdEf+yCcYx8Xyw8IsGES1HE/e8\n\tdtdIYjE61pULVMMJuwDme7/4snAUTLKosLGYyu/pYNmChtMRWDZj/KTbTXFALBYXMdPT\n\tLsCA==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCX/xe8bhrBTUG87m5Ug4YfrwmqV/e+9tzrVCW9q0UA29cAODrbH59JFyVps/9LbBSK2VZp7amHrmP2ShTcFY+4=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YxjR+SQv8pNa0rsaeswjVv7mz3FJZpKf86FR/2kBY9vEvZ5C8Qh\n\t5AJ+LSKpwm8K5Yy9o/0TDrwc/77LMQWfMaXzHeTa2uwiLJYu+dHgBVAa6g0TlVMjWRSJhekrlhz\n\tajDwRhnyR0pSF7fHP2n+L7qWmCPZsrMSMegCnHSs99kVWQvsabtTIIhKDyvGalNttJikS8V0FKb\n\tk=","X-Gm-Gg":"ATEYQzxJRwyzjALT9PdTT9s/zFn6zlc/CMKRZDqnShwkqQB0TokhQZOaSuI6DybDdsv\n\touZrB2B/CgHA4bH9Pjx2l4vHKsWSg5nLZ3iipiKbgRHYS79SA3VlEERz19XmBHinMm0sLFubjQy\n\tb1H0sGumg2Bxs/Ulx/haKX6T1yzhNuETaig0ZsABiL+hIMleRilEVT5/FuoCtHezGJrXQl3qG4H\n\tzTLVs8TPZNjH0NPVvIzcSdNcYqPob6714bkewcRvjOtMtZRoNe4YiCNappjaYh3WpLzutvA2snb\n\tl1TVfvs5LfpB5pVn9BBhSd2sJ03o7oOnl+IuaBan1m5A4bwGl9x0jMC64z6d81zCS+pi/d9XF3X\n\tOEnK/Dfv4pO+AeuKtcNocvoYzm6w2uJnIDoovDrB2rnu4ncBsC5NJH4txurGsb6aSEOno5huOos\n\ts=","X-Received":["by 2002:a05:600c:6309:b0:480:426e:9d38 with SMTP id\n\t5b1f17b1804b1-483c9bc5b70mr66140785e9.27.1772224324647; \n\tFri, 27 Feb 2026 12:32:04 -0800 (PST)","by 2002:a05:600c:6309:b0:480:426e:9d38 with SMTP id\n\t5b1f17b1804b1-483c9bc5b70mr66140435e9.27.1772224324115; \n\tFri, 27 Feb 2026 12:32:04 -0800 (PST)"],"From":"Milan Zamazal ","To":"Pavel Machek ","Cc":"david@ixit.cz, robert.mader@posteo.de, dorota.czaplejewicz@puri.sm,\n\tmartijn@brixit.nl, robert.mader@collabora.com,\n\tlibcamera-devel@lists.libcamera.org, pisa@cmp.felk.cvut.cz,\n\tnekocwd@mainlining.org","Subject":"Re: [PATCH] software_isp: Add focus control","In-Reply-To":" (Pavel Machek's message of \"Thu,\n\t26 Feb 2026 21:58:22 +0100\")","References":"","Date":"Fri, 27 Feb 2026 21:32:01 +0100","Message-ID":"<854in2udvi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"pIAKf0NnXYIZtlgGmd41jHV8jFwirYxWZaqqkRY_cI4_1772224325","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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":38312,"web_url":"https://patchwork.libcamera.org/comment/38312/","msgid":"<177236581828.1230693.16638569761103113339@ping.linuxembedded.co.uk>","date":"2026-03-01T11:50:18","subject":"Re: [PATCH] software_isp: Add focus control","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2026-02-27 20:32:01)\n> Hi,\n> \n> thank you for the patch.\n> \n> Pavel Machek writes:\n> \n> > From: Vasiliy Doylov \n> >\n> > This adds manual focus to software_isp.\n> \n> We usually use \"libcamera: software_isp: \" prefix in the titles of\n> commits for software ISP changes.\n> \n> And nothing to add about the purpose of the patch etc. in the commit\n> description?\n\nIt would definitely help to split this patch up a bit.\n\n\nPerhaps:\n - Adding an initial (manual) AF algorithm.\n - I think this should go straight into libipa.\n - It's not platform specific.\n - Extending the Simple-IPA interface to support lens controls\n - Adding/Using the Af algorithm in simple pipeline handler.\n - I think it's fine to call it 'Af' ... even if it's only manual for\n now.\n\n> \n> > Signed-off-by: Vasiliy Doylov \n> > Signed-off-by: Pavel Machek \n> > --\n> >\n> > I'm working on millicam -- you can see the results in\n> > git@gitlab.com:tui/libcamera.git branch millicam_af_6 . Aim is to\n> > eventually take photos and videos and autofocus is required for\n> > that. I'm currently tuning/cleaning that up, but this makes sense on\n> > its own and it would be nice to get it merged early.\n> >\n> > Thanks,\n> > Pavel\n> >\n> > diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n> > index ad89c9b3c..9e5e05fc0 100644\n> > --- a/include/libcamera/internal/software_isp/software_isp.h\n> > +++ b/include/libcamera/internal/software_isp/software_isp.h\n> > @@ -86,11 +86,11 @@ public:\n> > Signal outputBufferReady;\n> > Signal ispStatsReady;\n> > Signal metadataReady;\n> > - Signal setSensorControls;\n> > + Signal setSensorControls;\n> > \n> > private:\n> > void saveIspParams();\n> > - void setSensorCtrls(const ControlList &sensorControls);\n> > + void setSensorCtrls(const ControlList &sensorControls, const ControlList &lensControls);\n> > void statsReady(uint32_t frame, uint32_t bufferId);\n> > void inputReady(FrameBuffer *input);\n> > void outputReady(FrameBuffer *output);\n> > diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom\n> > index 77328c5fd..e5767532c 100644\n> > --- a/include/libcamera/ipa/soft.mojom\n> > +++ b/include/libcamera/ipa/soft.mojom\n> > @@ -10,6 +10,7 @@ import \"include/libcamera/ipa/core.mojom\";\n> > \n> > struct IPAConfigInfo {\n> > libcamera.ControlInfoMap sensorControls;\n> > + libcamera.ControlInfoMap lensControls;\n> > };\n> > \n> > interface IPASoftInterface {\n> > @@ -32,7 +33,7 @@ interface IPASoftInterface {\n> > };\n> > \n> > interface IPASoftEventInterface {\n> > - setSensorControls(libcamera.ControlList sensorControls);\n> > + setSensorControls(libcamera.ControlList sensorControls, libcamera.ControlList lensControls);\n> > setIspParams();\n> > metadataReady(uint32 frame, libcamera.ControlList metadata);\n> > };\n> > diff --git a/src/ipa/simple/algorithms/af.cpp b/src/ipa/simple/algorithms/af.cpp\n> > new file mode 100644\n> > index 000000000..b51ed95e4\n> > --- /dev/null\n> > +++ b/src/ipa/simple/algorithms/af.cpp\n> > @@ -0,0 +1,71 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025 Vasiliy Doylov \n> > + *\n> > + * Auto focus\n> \n> This is actually manual focus, right? So the things should not be named\n> \"af\".\n\nI think af will be fine here ... otherwise it's just \"f\" ... and at some\npoint later I would expect the a to be added.\n\n--\nKieran\n\n\n> \n> > + */\n> > +\n> > +#include \"af.h\"\n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"control_ids.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(IPASoftAutoFocus)\n> > +\n> > +namespace ipa::soft::algorithms {\n> > +\n> > +Af::Af()\n> > +{\n> > +}\n> > +\n> > +int Af::init(IPAContext &context,\n> > + [[maybe_unused]] const YamlObject &tuningData)\n> > +{\n> > + context.ctrlMap[&controls::LensPosition] = ControlInfo(0.0f, 100.0f, 50.0f);\n> \n> LensPosition documentation in control_ids_core.yaml says:\n> \n> The LensPosition control is ignored unless the AfMode is set to\n> AfModeManual, though the value is reported back unconditionally in all\n> modes.\n> \n> Should AfMode be added too?\n> \n> > + return 0;\n> > +}\n> > +\n> > +int Af::configure(IPAContext &context,\n> > + [[maybe_unused]] const IPAConfigInfo &configInfo)\n> > +{\n> > + context.activeState.knobs.focus_pos = std::optional();\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +void Af::queueRequest([[maybe_unused]] typename Module::Context &context,\n> \n> This one is used.\n> \n> > + [[maybe_unused]] const uint32_t frame,\n> > + [[maybe_unused]] typename Module::FrameContext &frameContext,\n> > + const ControlList &controls)\n> > +{\n> > + const auto &focus_pos = controls.get(controls::LensPosition);\n> > + if (focus_pos.has_value()) {\n> > + context.activeState.knobs.focus_pos = focus_pos;\n> > + LOG(IPASoftAutoFocus, Debug) << \"Setting focus position to \" << focus_pos.value();\n> > + }\n> > +}\n> > +\n> > +void Af::updateFocus([[maybe_unused]] IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, [[maybe_unused]] double exposureMSV)\n> \n> `context' and `frameContext' are actually used. `exposureMSV' is indeed\n> unused but why is it passed here then?\n> \n> > +{\n> > + frameContext.lens.focus_pos = context.activeState.knobs.focus_pos.value_or(50.0) / 100.0 * (context.configuration.focus.focus_max - context.configuration.focus.focus_min);\n> > +}\n> > +\n> > +void Af::process([[maybe_unused]] IPAContext &context,\n> \n> Used.\n> \n> > + [[maybe_unused]] const uint32_t frame,\n> > + [[maybe_unused]] IPAFrameContext &frameContext,\n> \n> Used.\n> \n> > + [[maybe_unused]] const SwIspStats *stats,\n> > + [[maybe_unused]] ControlList &metadata)\n> > +{\n> > + updateFocus(context, frameContext, 0);\n> > +}\n> > +\n> > +REGISTER_IPA_ALGORITHM(Af, \"Af\")\n> > +\n> > +} /* namespace ipa::soft::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/simple/algorithms/af.h b/src/ipa/simple/algorithms/af.h\n> > new file mode 100644\n> > index 000000000..a575ef102\n> > --- /dev/null\n> > +++ b/src/ipa/simple/algorithms/af.h\n> > @@ -0,0 +1,40 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025 Vasiliy Doylov \n> > + *\n> > + * Auto focus\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \"algorithm.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::soft::algorithms {\n> > +\n> > +class Af : public Algorithm\n> > +{\n> > +public:\n> > + Af();\n> > + ~Af() = default;\n> > +\n> > + int init(IPAContext &context, const YamlObject &tuningData) override;\n> > + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> > + void queueRequest(typename Module::Context &context,\n> > + const uint32_t frame,\n> > + typename Module::FrameContext &frameContext,\n> > + const ControlList &controls)\n> > + override;\n> > + void process(IPAContext &context, const uint32_t frame,\n> > + IPAFrameContext &frameContext,\n> > + const SwIspStats *stats,\n> > + ControlList &metadata) override;\n> > +\n> > +private:\n> > + void updateFocus(IPAContext &context, IPAFrameContext &frameContext, double focus);\n> > +};\n> > +\n> > +} /* namespace ipa::soft::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n> > index 73c637220..8950b008f 100644\n> > --- a/src/ipa/simple/algorithms/meson.build\n> > +++ b/src/ipa/simple/algorithms/meson.build\n> > @@ -6,4 +6,5 @@ soft_simple_ipa_algorithms = files([\n> > 'agc.cpp',\n> > 'blc.cpp',\n> > 'ccm.cpp',\n> > + 'af.cpp',\n> > ])\n> > diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml\n> > index fc90ca526..ede277d1d 100644\n> > --- a/src/ipa/simple/data/uncalibrated.yaml\n> > +++ b/src/ipa/simple/data/uncalibrated.yaml\n> > @@ -16,4 +16,5 @@ algorithms:\n> > # 0, 0, 1]\n> > - Adjust:\n> > - Agc:\n> > + - Af:\n> > ...\n> > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> > index 34f7403a4..a436fc76a 100644\n> > --- a/src/ipa/simple/ipa_context.h\n> > +++ b/src/ipa/simple/ipa_context.h\n> > @@ -33,6 +33,9 @@ struct IPASessionConfiguration {\n> > struct {\n> > std::optional level;\n> > } black;\n> > + struct {\n> > + int32_t focus_min, focus_max;\n> > + } focus;\n> > };\n> > \n> > struct IPAActiveState {\n> > @@ -60,6 +63,8 @@ struct IPAActiveState {\n> > /* 0..2 range, 1.0 = normal */\n> > std::optional contrast;\n> > std::optional saturation;\n> > + /* 0..100 range, 50.0 = normal */\n> \n> What is this range and what is \"normal\" in the context of focus\n> position? Is there any relationship to LensPosition as described in\n> control_ids_core.yaml?\n> \n> > + std::optional focus_pos;\n> > } knobs;\n> > };\n> > \n> > @@ -71,6 +76,10 @@ struct IPAFrameContext : public FrameContext {\n> > double gain;\n> > } sensor;\n> > \n> > + struct {\n> > + int32_t focus_pos;\n> > + } lens;\n> > +\n> > struct {\n> > double red;\n> > double blue;\n> > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> > index 6bef597c8..4f63fa3b4 100644\n> > --- a/src/ipa/simple/soft_simple.cpp\n> > +++ b/src/ipa/simple/soft_simple.cpp\n> > @@ -78,6 +78,7 @@ private:\n> > SwIspStats *stats_;\n> > std::unique_ptr camHelper_;\n> > ControlInfoMap sensorInfoMap_;\n> > + ControlInfoMap lensInfoMap_;\n> > \n> > /* Local parameter storage */\n> > struct IPAContext context_;\n> > @@ -202,6 +203,7 @@ int IPASoftSimple::init(const IPASettings &settings,\n> > int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> > {\n> > sensorInfoMap_ = configInfo.sensorControls;\n> > + lensInfoMap_ = configInfo.lensControls;\n> > \n> > const ControlInfo &exposureInfo = sensorInfoMap_.find(V4L2_CID_EXPOSURE)->second;\n> > const ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second;\n> > @@ -211,6 +213,17 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> > context_.activeState = {};\n> > context_.frameContexts.clear();\n> > \n> > + if (lensInfoMap_.empty()) {\n> > + LOG(IPASoft, Warning) << \"No camera leans found! Focus control disabled.\";\n> \n> s/leans/lens/\n> \n> > + context_.configuration.focus.focus_min = 0;\n> > + context_.configuration.focus.focus_max = 0;\n> > + } else {\n> > + const ControlInfo &lensInfo = lensInfoMap_.find(V4L2_CID_FOCUS_ABSOLUTE)->second;\n> > + context_.configuration.focus.focus_min = lensInfo.min().get();\n> > + context_.configuration.focus.focus_max = lensInfo.max().get();\n> > + LOG(IPASoft, Warning) << \"Camera leans found! Focus: \" << context_.configuration.focus.focus_min << \"-\" << context_.configuration.focus.focus_max;\n> \n> s/leans/lens/\n> \n> > + }\n> > +\n> > context_.configuration.agc.lineDuration =\n> > context_.sensorInfo.minLineLength * 1.0s / context_.sensorInfo.pixelRate;\n> > context_.configuration.agc.exposureMin = exposureInfo.min().get();\n> > @@ -325,7 +338,10 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> > ctrls.set(V4L2_CID_ANALOGUE_GAIN,\n> > static_cast(camHelper_ ? camHelper_->gainCode(againNew) : againNew));\n> > \n> > - setSensorControls.emit(ctrls);\n> > + ControlList lens_ctrls(lensInfoMap_);\n> > + lens_ctrls.set(V4L2_CID_FOCUS_ABSOLUTE, frameContext.lens.focus_pos);\n> > +\n> > + setSensorControls.emit(ctrls, lens_ctrls);\n> > }\n> > \n> > std::string IPASoftSimple::logPrefix() const\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 4a0b9f58d..4079f0973 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -33,6 +33,7 @@\n> > #include \n> > \n> > #include \"libcamera/internal/camera.h\"\n> > +#include \"libcamera/internal/camera_lens.h\"\n> > #include \"libcamera/internal/camera_manager.h\"\n> > #include \"libcamera/internal/camera_sensor.h\"\n> > #include \"libcamera/internal/camera_sensor_properties.h\"\n> > @@ -48,6 +49,8 @@\n> > #include \"libcamera/internal/v4l2_subdevice.h\"\n> > #include \"libcamera/internal/v4l2_videodevice.h\"\n> > \n> > +#include \"libcamera/controls.h\"\n> > +\n> > namespace libcamera {\n> > \n> > LOG_DEFINE_CATEGORY(SimplePipeline)\n> > @@ -371,7 +374,7 @@ private:\n> > \n> > void ispStatsReady(uint32_t frame, uint32_t bufferId);\n> > void metadataReady(uint32_t frame, const ControlList &metadata);\n> > - void setSensorControls(const ControlList &sensorControls);\n> > + void setSensorControls(const ControlList &sensorControls, const ControlList &lensControls);\n> > };\n> > \n> > class SimpleCameraConfiguration : public CameraConfiguration\n> > @@ -1041,7 +1044,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata\n> > tryCompleteRequest(info->request);\n> > }\n> > \n> > -void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n> > +void SimpleCameraData::setSensorControls(const ControlList &sensorControls, const ControlList &lensControls)\n> > {\n> > delayedCtrls_->push(sensorControls);\n> > /*\n> > @@ -1052,10 +1055,21 @@ void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n> > * but it also bypasses delayedCtrls_, creating AGC regulation issues.\n> > * Both problems should be fixed.\n> > */\n> > - if (!frameStartEmitter_) {\n> > - ControlList ctrls(sensorControls);\n> > - sensor_->setControls(&ctrls);\n> > - }\n> > + if (frameStartEmitter_)\n> > + return;\n> > +\n> > + ControlList ctrls(sensorControls);\n> > + sensor_->setControls(&ctrls);\n> > +\n> > + CameraLens *focusLens = sensor_->focusLens();\n> > + if (!focusLens)\n> > + return;\n> > +\n> > + if (!lensControls.contains(V4L2_CID_FOCUS_ABSOLUTE))\n> > + return;\n> > +\n> > + const ControlValue &focusValue = lensControls.get(V4L2_CID_FOCUS_ABSOLUTE);\n> > + focusLens->setFocusPosition(focusValue.get());\n> > }\n> > \n> > /* Retrieve all source pads connected to a sink pad through active routes. */\n> > @@ -1593,6 +1607,10 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > } else {\n> > ipa::soft::IPAConfigInfo configInfo;\n> > configInfo.sensorControls = data->sensor_->controls();\n> > + if (data->sensor_->focusLens() != nullptr)\n> > + configInfo.lensControls = data->sensor_->focusLens()->controls();\n> > + else\n> > + configInfo.lensControls = ControlInfoMap();\n> > return data->swIsp_->configure(inputCfg, outputCfgs, configInfo);\n> > }\n> > }\n> > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> > index c2baaf0bf..df0c5e054 100644\n> > --- a/src/libcamera/software_isp/software_isp.cpp\n> > +++ b/src/libcamera/software_isp/software_isp.cpp\n> > @@ -408,9 +408,9 @@ void SoftwareIsp::saveIspParams()\n> > debayerParams_ = *sharedParams_;\n> > }\n> > \n> > -void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)\n> > +void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls, const ControlList &lensControls)\n> > {\n> > - setSensorControls.emit(sensorControls);\n> > + setSensorControls.emit(sensorControls, lensControls);\n> > }\n> > \n> > void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)\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 956C5C0DA4\n\tfor ;\n\tSun, 1 Mar 2026 11:50:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A19216237B;\n\tSun, 1 Mar 2026 12:50:23 +0100 (CET)","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 26085622E9\n\tfor ;\n\tSun, 1 Mar 2026 12:50:21 +0100 (CET)","from monstersaurus.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 38AD656D;\n\tSun, 1 Mar 2026 12:49:20 +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=\"YlUYiIOB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1772365760;\n\tbh=HuOCyWAd+0SPfjfDp8+tw7bDlhGnHlt8YEyzudmaDj0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=YlUYiIOB/bB306TsuPYqFeKhocqnqBSSbDam8cOA6dEPUn+QVctVEONKdEgVcHl2a\n\t9f+RmzV8co76LHmSXWN46zqbQ1jSqL4h4BN3KQeiYo4/SjdkJEES8qWoBS72dNw8ZE\n\tGzpw+goNuHIIsG5pKhFeBa26uCi9ZvpCjjwTRFIg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<854in2udvi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"\n\t<854in2udvi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Subject":"Re: [PATCH] software_isp: Add focus control","From":"Kieran Bingham ","Cc":"david@ixit.cz, robert.mader@posteo.de, dorota.czaplejewicz@puri.sm,\n\tmartijn@brixit.nl, robert.mader@collabora.com,\n\tlibcamera-devel@lists.libcamera.org, pisa@cmp.felk.cvut.cz,\n\tnekocwd@mainlining.org","To":"Milan Zamazal , Pavel Machek ","Date":"Sun, 01 Mar 2026 11:50:18 +0000","Message-ID":"<177236581828.1230693.16638569761103113339@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":38313,"web_url":"https://patchwork.libcamera.org/comment/38313/","msgid":"","date":"2026-03-01T16:35:34","subject":"Re: [PATCH] software_isp: Add focus control","submitter":{"id":49,"url":"https://patchwork.libcamera.org/api/people/49/","name":"Pavel Machek","email":"pavel@ucw.cz"},"content":"Hi!\n\n> > And nothing to add about the purpose of the patch etc. in the commit\n> > description?\n> \n> It would definitely help to split this patch up a bit.\n> \n> \n> Perhaps:\n> - Adding an initial (manual) AF algorithm.\n> - I think this should go straight into libipa.\n> - It's not platform specific.\n\nThis patch is \"add a control for manual focus\" only. (Additional\nautofocus bits can be found in mcam repository.)\n\n> - Extending the Simple-IPA interface to support lens controls\n\nCan you take a second look? This extends interface\nIPASoftEventInterface::setSensorControls() to take lens\ncontrols. Is that ok or should it go somewhere else? If so, where?\n\nFinal autofocus code will need statistics collection from\nsoftware_isp, but we are not there, yet.\n\n> - Adding/Using the Af algorithm in simple pipeline handler.\n> - I think it's fine to call it 'Af' ... even if it's only manual for\n> now.\n\nI hoped so, too :-).\n\nThanks and best regards,\n\t\t\t\t\t\t\t\tPavel\n\t\t\t\t\t\t\t\t\n> > > +++ b/src/ipa/simple/ipa_context.h\n> > > @@ -33,6 +33,9 @@ struct IPASessionConfiguration {\n> > > struct {\n> > > std::optional level;\n> > > } black;\n> > > + struct {\n> > > + int32_t focus_min, focus_max;\n> > > + } focus;\n> > > };\n> > > \n> > > struct IPAActiveState {\n> > > @@ -60,6 +63,8 @@ struct IPAActiveState {\n> > > /* 0..2 range, 1.0 = normal */\n> > > std::optional contrast;\n> > > std::optional saturation;\n> > > + /* 0..100 range, 50.0 = normal */\n> > \n> > What is this range and what is \"normal\" in the context of focus\n> > position? Is there any relationship to LensPosition as described in\n> > control_ids_core.yaml?\n> > \n> > > + std::optional focus_pos;\n> > > } knobs;\n> > > };\n> > > \n> > > @@ -71,6 +76,10 @@ struct IPAFrameContext : public FrameContext {\n> > > double gain;\n> > > } sensor;\n> > > \n> > > + struct {\n> > > + int32_t focus_pos;\n> > > + } lens;\n> > > +\n> > > struct {\n> > > double red;\n> > > double blue;\n> > > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> > > index 6bef597c8..4f63fa3b4 100644\n> > > --- a/src/ipa/simple/soft_simple.cpp\n> > > +++ b/src/ipa/simple/soft_simple.cpp\n> > > @@ -78,6 +78,7 @@ private:\n> > > SwIspStats *stats_;\n> > > std::unique_ptr camHelper_;\n> > > ControlInfoMap sensorInfoMap_;\n> > > + ControlInfoMap lensInfoMap_;\n> > > \n> > > /* Local parameter storage */\n> > > struct IPAContext context_;\n> > > @@ -202,6 +203,7 @@ int IPASoftSimple::init(const IPASettings &settings,\n> > > int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> > > {\n> > > sensorInfoMap_ = configInfo.sensorControls;\n> > > + lensInfoMap_ = configInfo.lensControls;\n> > > \n> > > const ControlInfo &exposureInfo = sensorInfoMap_.find(V4L2_CID_EXPOSURE)->second;\n> > > const ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second;\n> > > @@ -211,6 +213,17 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> > > context_.activeState = {};\n> > > context_.frameContexts.clear();\n> > > \n> > > + if (lensInfoMap_.empty()) {\n> > > + LOG(IPASoft, Warning) << \"No camera leans found! Focus control disabled.\";\n> > \n> > s/leans/lens/\n> > \n> > > + context_.configuration.focus.focus_min = 0;\n> > > + context_.configuration.focus.focus_max = 0;\n> > > + } else {\n> > > + const ControlInfo &lensInfo = lensInfoMap_.find(V4L2_CID_FOCUS_ABSOLUTE)->second;\n> > > + context_.configuration.focus.focus_min = lensInfo.min().get();\n> > > + context_.configuration.focus.focus_max = lensInfo.max().get();\n> > > + LOG(IPASoft, Warning) << \"Camera leans found! Focus: \" << context_.configuration.focus.focus_min << \"-\" << context_.configuration.focus.focus_max;\n> > \n> > s/leans/lens/\n> > \n> > > + }\n> > > +\n> > > context_.configuration.agc.lineDuration =\n> > > context_.sensorInfo.minLineLength * 1.0s / context_.sensorInfo.pixelRate;\n> > > context_.configuration.agc.exposureMin = exposureInfo.min().get();","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 E8357BE086\n\tfor ;\n\tSun, 1 Mar 2026 16:35:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F283762375;\n\tSun, 1 Mar 2026 17:35:36 +0100 (CET)","from jabberwock.ucw.cz (jabberwock.ucw.cz [46.255.230.98])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E35DB622E9\n\tfor ;\n\tSun, 1 Mar 2026 17:35:34 +0100 (CET)","by jabberwock.ucw.cz (Postfix, from userid 1017)\n\tid 3FB0B327EB8; Sun, 01 Mar 2026 17:35:34 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ucw.cz header.i=@ucw.cz header.b=\"rfGYCo6L\";\n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=ucw.cz; s=gen1;\n\tt=1772382934;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=JuBOokHIPx7sp5LdVMJCggJ751eruStUI48wqVH3tAU=;\n\tb=rfGYCo6LxvVYEqK8btv10V/UC7y/9sCKF0SDHW72YM8i7LQM0nXpI6gBWaNpX//fP4bI/W\n\t6at/mepzKvHZy5wBZzqWTjoxliEPBDDJ2pmgfyBEibYOdAbSnCEujSl/cqZn15kqVXKG/u\n\twnDS1vKtMJnfNyUtEfvtZ4OFayt+47o=","Date":"Sun, 1 Mar 2026 17:35:34 +0100","From":"Pavel Machek ","To":"Kieran Bingham ","Cc":"Milan Zamazal , david@ixit.cz,\n\trobert.mader@posteo.de, dorota.czaplejewicz@puri.sm,\n\tmartijn@brixit.nl, robert.mader@collabora.com,\n\tlibcamera-devel@lists.libcamera.org, pisa@cmp.felk.cvut.cz,\n\tnekocwd@mainlining.org","Subject":"Re: [PATCH] software_isp: Add focus control","Message-ID":"","References":"\n\t<854in2udvi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<177236581828.1230693.16638569761103113339@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<177236581828.1230693.16638569761103113339@ping.linuxembedded.co.uk>","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":38314,"web_url":"https://patchwork.libcamera.org/comment/38314/","msgid":"<177238460284.2708452.4664116211405237815@ping.linuxembedded.co.uk>","date":"2026-03-01T17:03:22","subject":"Re: [PATCH] software_isp: Add focus control","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Pavel Machek (2026-02-26 20:58:22)\n> From: Vasiliy Doylov \n> \n> This adds manual focus to software_isp.\n> \n> Signed-off-by: Vasiliy Doylov \n> Signed-off-by: Pavel Machek \n> \n> --\n> \n> I'm working on millicam -- you can see the results in\n> git@gitlab.com:tui/libcamera.git branch millicam_af_6 . Aim is to\n> eventually take photos and videos and autofocus is required for\n> that. I'm currently tuning/cleaning that up, but this makes sense on\n> its own and it would be nice to get it merged early.\n> \n> Thanks,\n> Pavel\n> \n> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n> index ad89c9b3c..9e5e05fc0 100644\n> --- a/include/libcamera/internal/software_isp/software_isp.h\n> +++ b/include/libcamera/internal/software_isp/software_isp.h\n> @@ -86,11 +86,11 @@ public:\n> Signal outputBufferReady;\n> Signal ispStatsReady;\n> Signal metadataReady;\n> - Signal setSensorControls;\n> + Signal setSensorControls;\n\nI don't think this should be connected to the software ISP. The simple\npipeline handler should be in control of this somehow - and it should be\npossible to set manual focus even if the software isp is disabled.\n\n\nThough it looks like somehow controls for the sensor are currently going\nthrough the 'ISP' ... that doesn't feel right either... and might need\nlooking at.\n\n\n> \n> private:\n> void saveIspParams();\n> - void setSensorCtrls(const ControlList &sensorControls);\n> + void setSensorCtrls(const ControlList &sensorControls, const ControlList &lensControls);\n> void statsReady(uint32_t frame, uint32_t bufferId);\n> void inputReady(FrameBuffer *input);\n> void outputReady(FrameBuffer *output);\n\n--\nKieran","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 92A0FC0DA4\n\tfor ;\n\tSun, 1 Mar 2026 17:03:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AB94562375;\n\tSun, 1 Mar 2026 18:03:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9941A622E9\n\tfor ;\n\tSun, 1 Mar 2026 18:03:26 +0100 (CET)","from monstersaurus.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 B82C856D;\n\tSun, 1 Mar 2026 18:02:25 +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=\"IRMuJAGJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1772384545;\n\tbh=32uKYMEKZkZuUFQdw9CmFufUc+8anJtgjJInCogt48Y=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=IRMuJAGJoMy+N2sSzT3yYuivEcwk8k8YbjsCdKIMnRSn0uHRJSqC2vGBIKPwZ/JH4\n\t9SvOOfvQJYHhttpQOuHMZawI/r+8oRnuF8tKKcZc1a2Ir2SJg0buBLlIMji2Rh9D7J\n\tCRLFQ4KwOrgHpqYI3SCpI9UsDBEva93xeJ2N+EBI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"","Subject":"Re: [PATCH] software_isp: Add focus control","From":"Kieran Bingham ","To":"Pavel Machek , david@ixit.cz, dorota.czaplejewicz@puri.sm, \n\tlibcamera-devel@lists.libcamera.org, martijn@brixit.nl,\n\tnekocwd@mainlining.org, \n\tpisa@cmp.felk.cvut.cz, robert.mader@collabora.com, robert.mader@posteo.de","Date":"Sun, 01 Mar 2026 17:03:22 +0000","Message-ID":"<177238460284.2708452.4664116211405237815@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":38315,"web_url":"https://patchwork.libcamera.org/comment/38315/","msgid":"","date":"2026-03-01T18:20:42","subject":"Re: [PATCH] software_isp: Add focus control","submitter":{"id":49,"url":"https://patchwork.libcamera.org/api/people/49/","name":"Pavel Machek","email":"pavel@ucw.cz"},"content":"Hi!\n\n> > This adds manual focus to software_isp.\n> > \n> > Signed-off-by: Vasiliy Doylov \n> > Signed-off-by: Pavel Machek \n> > \n> > --\n> > \n> > I'm working on millicam -- you can see the results in\n> > git@gitlab.com:tui/libcamera.git branch millicam_af_6 . Aim is to\n> > eventually take photos and videos and autofocus is required for\n> > that. I'm currently tuning/cleaning that up, but this makes sense on\n> > its own and it would be nice to get it merged early.\n> > \n> > Thanks,\n> > Pavel\n> > \n> > diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n> > index ad89c9b3c..9e5e05fc0 100644\n> > --- a/include/libcamera/internal/software_isp/software_isp.h\n> > +++ b/include/libcamera/internal/software_isp/software_isp.h\n> > @@ -86,11 +86,11 @@ public:\n> > Signal outputBufferReady;\n> > Signal ispStatsReady;\n> > Signal metadataReady;\n> > - Signal setSensorControls;\n> > + Signal setSensorControls;\n> \n> I don't think this should be connected to the software ISP. The simple\n> pipeline handler should be in control of this somehow - and it should be\n> possible to set manual focus even if the software isp is disabled.\n\nOk, agreed that would be nice. But I'm afraid that if I try to fix\nlibcamera design, you may not like the results.\n\n> Though it looks like somehow controls for the sensor are currently going\n> through the 'ISP' ... that doesn't feel right either... and might need\n> looking at.\n\nCan I somehow make you do the looking? :-) It should be immediately\nuseful on PinePhone Pro, test code is in millicam_af_6 allowing to do\nmanual focus interactively. I don't have PP Pro, but Librem 5 should\nbe similar enough (as should be OnePlus 6).\n\nSimple autofocus code is not that hard, say 500LoC, but integrating it\nwith rest of the libcamera seems to be tricky. If I had, say, patch\nwith /* fill in the gaps */, I could fill in simple auto-focus\nalgorithm.\n\nThanks and best regards,\n\t\t\t\t\t\t\t\tPavel","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 C761DBE086\n\tfor ;\n\tSun, 1 Mar 2026 18:20:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E6EB162384;\n\tSun, 1 Mar 2026 19:20:43 +0100 (CET)","from jabberwock.ucw.cz (jabberwock.ucw.cz [46.255.230.98])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CC430622E9\n\tfor ;\n\tSun, 1 Mar 2026 19:20:42 +0100 (CET)","by jabberwock.ucw.cz (Postfix, from userid 1017)\n\tid 39E43327EC7; Sun, 01 Mar 2026 19:20:42 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ucw.cz header.i=@ucw.cz header.b=\"ZHfi7xe4\";\n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=ucw.cz; s=gen1;\n\tt=1772389242;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=Vrky5xC3xW6m6VG6gJKvfBG2L0B8rp4MmThFvCdpjMQ=;\n\tb=ZHfi7xe4KUUNZp0QJKJifj/DahOEoS4MGGtnXYpjmtQXvCz4cgLwBoQzuo5NrB9z/X/ESV\n\tJT8Mky/SuX3JhdPQI7ymVMGzvGNRkg1b/wgLWqk/6OZSR7EI1zxQo+Tspe/ah37fh+Ytgp\n\tYVfQOPLoB4wva20329uyjZbLilsK5Bs=","Date":"Sun, 1 Mar 2026 19:20:42 +0100","From":"Pavel Machek ","To":"Kieran Bingham ","Cc":"david@ixit.cz, dorota.czaplejewicz@puri.sm,\n\tlibcamera-devel@lists.libcamera.org, martijn@brixit.nl,\n\tnekocwd@mainlining.org, pisa@cmp.felk.cvut.cz,\n\trobert.mader@collabora.com, robert.mader@posteo.de","Subject":"Re: [PATCH] software_isp: Add focus control","Message-ID":"","References":"\n\t<177238460284.2708452.4664116211405237815@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<177238460284.2708452.4664116211405237815@ping.linuxembedded.co.uk>","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\" "}}]