From patchwork Fri Sep 18 09:10:50 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 28336 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 7B8B6C3356 for ; Fri, 18 Sep 2026 09:19:04 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DF7EC68718; Fri, 18 Sep 2026 11:19:03 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="gwtG5+pr"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C0E368702 for ; Fri, 18 Sep 2026 11:19:02 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6c1f:355d:1c19:aba6]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A0832C23; Fri, 18 Sep 2026 11:17:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1789723039; bh=vlhUCeThgFduav+ZPKVXHnNyxQIJmLDgsDKUU1A5MYw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=gwtG5+prM+TdhGLUqNj6ETPJSP8rJORsXofg4vGg1cxl2zajkebCa/rOeWOsAA76j g8YCIm1wF53xn88ssP1Dvh9fBJiNmXy2T31hgIouO90XdqWH8rCVCGsQU4O2Rzddrn FehBkr6sHgZsuXmlotUF8+svqKSaYbJ0KydBPXfc= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Naushir Patuck , Jacopo Mondi , David Plowman Subject: [PATCH v4 3/4] libcamera: ipa: core: Use native coordinates for IPACameraSensorInfo::analogCrop Date: Fri, 18 Sep 2026 11:10:50 +0200 Message-ID: <20260918091835.2332386-4-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260918091835.2332386-1-stefan.klug@ideasonboard.com> References: <20260918091835.2332386-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Now that the camera properties use the physical pixel array as reference coordinate system, do the same with IPACameraSensorInfo::analogCrop. Replace IPACameraSensorInfo::activeAreaSize with IPACameraSensorInfo::activeArea to be able to keep the existing calculations. Co-developed-by: Naushir Patuck Signed-off-by: Naushir Patuck Signed-off-by: Stefan Klug Reviewed-by: Jacopo Mondi Reviewed-by: David Plowman --- Changes in v4: - Collected tag - Reformatted changes in the lsc.cpp files to be checkstyle compatible Changes in v3: - Prevent negative crops and issue a warning in the rpi ipa - Reflowed docs for IPACameraSensorInfo::analogCrop - Added a fix for a typo in an comment that showed up in the patch Changes in v2: - Moved all hunks related to IPACameraSensorInfo from the previous patch to this one - Fixed some missing documentation changes - Moved IPACameraSensorInfo::pixelArraySize to a separate patch - Pulled in changes by Naush - Kept the rkisp1 lsc logic. The related changes will come one top --- include/libcamera/ipa/core.mojom | 29 ++++++++++++------- src/ipa/mali-c55/algorithms/lsc.cpp | 11 ++++--- src/ipa/rkisp1/algorithms/lsc.cpp | 11 ++++--- src/ipa/rpi/common/ipa_base.cpp | 20 ++++++++++--- src/libcamera/sensor/camera_sensor_legacy.cpp | 19 +++--------- src/libcamera/sensor/camera_sensor_raw.cpp | 17 ++--------- 6 files changed, 51 insertions(+), 56 deletions(-) diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom index bce797245829..32ff25bbfc2b 100644 --- a/include/libcamera/ipa/core.mojom +++ b/include/libcamera/ipa/core.mojom @@ -153,21 +153,28 @@ module libcamera; */ /** - * \var IPACameraSensorInfo::activeAreaSize - * \brief The size of the pixel array active area of the sensor + * \var IPACameraSensorInfo::activeArea + * \brief The active pixel area of the sensor, relative to the physical pixel + * array + * + * The active area rectangle is expressed relative to the top-left corner of the + * physical pixel array. It describes the region of the pixel array which + * produces valid image data, excluding non-readable and optical black pixels. + * + * \note New IPA implementations should *not* use this property and instead rely + * on coordinates relative to the physical sensor array */ /** * \var IPACameraSensorInfo::analogCrop - * \brief The portion of the pixel array active area which is read-out and - * processed + * \brief The portion of the pixel array which is read-out and processed * - * The analog crop rectangle top-left corner is defined as the displacement - * from the top-left corner of the pixel array active area. The rectangle - * horizontal and vertical sizes define the portion of the pixel array which - * is read-out and provided to the sensor's internal processing pipeline, before - * any pixel sub-sampling method, such as pixel binning, skipping and averaging - * take place. + * The analog crop rectangle top-left corner is defined as the displacement from + * the top-left corner of the physical pixel array. The rectangle horizontal and + * vertical sizes define the portion of the pixel array which is read-out and + * provided to the sensor's internal processing pipeline, before any pixel + * sub-sampling method, such as pixel binning, skipping and averaging take + * place. */ /** @@ -243,7 +250,7 @@ struct IPACameraSensorInfo { uint32 bitsPerPixel; uint32 cfaPattern; - Size activeAreaSize; + Rectangle activeArea; Rectangle analogCrop; Size outputSize; diff --git a/src/ipa/mali-c55/algorithms/lsc.cpp b/src/ipa/mali-c55/algorithms/lsc.cpp index fff0dc7d0e64..a79d8c80fa9f 100644 --- a/src/ipa/mali-c55/algorithms/lsc.cpp +++ b/src/ipa/mali-c55/algorithms/lsc.cpp @@ -53,12 +53,11 @@ int Lsc::init(IPAContext &context, const ValueNode &tuningData) { gridPos_ = segmentsToPosition(); - return lscAlgo_.init(tuningData, context.ctrlMap, { - .keys = { "r", "g", "b" }, - .numHSamples = kMeshSize, - .numVSamples = kMeshSize, - .sensorSize = context.sensorInfo.activeAreaSize - }); + return lscAlgo_.init(tuningData, context.ctrlMap, + { .keys = { "r", "g", "b" }, + .numHSamples = kMeshSize, + .numVSamples = kMeshSize, + .sensorSize = context.sensorInfo.activeArea.size() }); } int Lsc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp index 36f9a344ed3b..7994ad111ba2 100644 --- a/src/ipa/rkisp1/algorithms/lsc.cpp +++ b/src/ipa/rkisp1/algorithms/lsc.cpp @@ -124,12 +124,11 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, xPos_ = sizesListToPositions(xSize_); yPos_ = sizesListToPositions(ySize_); - return lscAlgo_.init(tuningData, context.ctrlMap, { - .keys = { "r", "gr", "gb", "b" }, - .numHSamples = RKISP1_CIF_ISP_LSC_SAMPLES_MAX, - .numVSamples = RKISP1_CIF_ISP_LSC_SAMPLES_MAX, - .sensorSize = context.sensorInfo.activeAreaSize - }); + return lscAlgo_.init(tuningData, context.ctrlMap, + { .keys = { "r", "gr", "gb", "b" }, + .numHSamples = RKISP1_CIF_ISP_LSC_SAMPLES_MAX, + .numVSamples = RKISP1_CIF_ISP_LSC_SAMPLES_MAX, + .sensorSize = context.sensorInfo.activeArea.size() }); } /** diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index bc8e65cb0811..a5d149036a37 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -561,10 +561,22 @@ void IpaBase::setMode(const IPACameraSensorInfo &sensorInfo) mode_.bitdepth = sensorInfo.bitsPerPixel; mode_.width = sensorInfo.outputSize.width; mode_.height = sensorInfo.outputSize.height; - mode_.sensorWidth = sensorInfo.activeAreaSize.width; - mode_.sensorHeight = sensorInfo.activeAreaSize.height; - mode_.cropX = sensorInfo.analogCrop.x; - mode_.cropY = sensorInfo.analogCrop.y; + mode_.sensorWidth = sensorInfo.activeArea.width; + mode_.sensorHeight = sensorInfo.activeArea.height; + /* + * CameraMode::cropX/Y are defined relative to the active pixel area, + * whereas IPACameraSensorInfo::analogCrop is relative to the physical + * pixel array. Rebase the crop origin onto the active area. + */ + mode_.cropX = sensorInfo.analogCrop.x - std::min(sensorInfo.activeArea.x, + sensorInfo.analogCrop.x); + mode_.cropY = sensorInfo.analogCrop.y - std::min(sensorInfo.activeArea.y, + sensorInfo.analogCrop.y); + if (sensorInfo.analogCrop.x < sensorInfo.activeArea.x || + sensorInfo.analogCrop.y < sensorInfo.activeArea.y) { + LOG(IPARPI, Warning) << "Analog crop lies outside the active area." + << " This can result in image artifacts."; + } mode_.pixelRate = sensorInfo.pixelRate; /* diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp index f8d05487202c..62f346aa06df 100644 --- a/src/libcamera/sensor/camera_sensor_legacy.cpp +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp @@ -888,14 +888,13 @@ int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const info->model = model(); /* - * The active area size is a static property, while the crop - * rectangle needs to be re-read as it depends on the sensor - * configuration. + * The active area is a static property, while the crop rectangle needs + * to be re-read as it depends on the sensor configuration. */ - info->activeAreaSize = { activeArea_.width, activeArea_.height }; + info->activeArea = activeArea_; /* - * \todo Support for retreiving the crop rectangle is scheduled to + * \todo Support for retrieving the crop rectangle is scheduled to * become mandatory. For the time being use the default value if it has * been initialized at sensor driver validation time. */ @@ -906,16 +905,6 @@ int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const << "The analogue crop rectangle has been defaulted to the active area size"; } - /* - * IPACameraSensorInfo::analogCrop::x and IPACameraSensorInfo::analogCrop::y - * are defined relatively to the active pixel area, while V4L2's - * TGT_CROP target is defined in respect to the full pixel array. - * - * Compensate it by subtracting the active area offset. - */ - info->analogCrop.x -= activeArea_.x; - info->analogCrop.y -= activeArea_.y; - /* The bit depth and image size depend on the currently applied format. */ V4L2SubdeviceFormat format{}; ret = subdev_->getFormat(pad_, &format); diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp index 117da63ef364..940e7514ae93 100644 --- a/src/libcamera/sensor/camera_sensor_raw.cpp +++ b/src/libcamera/sensor/camera_sensor_raw.cpp @@ -1009,27 +1009,16 @@ int CameraSensorRaw::sensorInfo(IPACameraSensorInfo *info) const info->model = model(); /* - * The active area size is a static property, while the crop - * rectangle needs to be re-read as it depends on the sensor - * configuration. + * The active area is a static property, while the crop rectangle needs + * to be re-read as it depends on the sensor configuration. */ - info->activeAreaSize = { activeArea_.width, activeArea_.height }; + info->activeArea = activeArea_; int ret = subdev_->getSelection(streams_.image.sink, V4L2_SEL_TGT_CROP, &info->analogCrop); if (ret) return ret; - /* - * IPACameraSensorInfo::analogCrop::x and IPACameraSensorInfo::analogCrop::y - * are defined relatively to the active pixel area, while V4L2's - * TGT_CROP target is defined in respect to the full pixel array. - * - * Compensate it by subtracting the active area offset. - */ - info->analogCrop.x -= activeArea_.x; - info->analogCrop.y -= activeArea_.y; - /* The bit depth and image size depend on the currently applied format. */ V4L2SubdeviceFormat format{}; ret = subdev_->getFormat(streams_.image.source, &format);