Patch Detail
Show a patch.
GET /api/1.1/patches/22199/?format=api
{ "id": 22199, "url": "https://patchwork.libcamera.org/api/1.1/patches/22199/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22199/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20241206101344.767170-8-stefan.klug@ideasonboard.com>", "date": "2024-12-06T10:13:29", "name": "[v3,07/17] pipeline: rkisp1: Fix ScalerCrop to be in sensor coordinates", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "444e1ee1c6996935698d4e40541d17bffc32a844", "submitter": { "id": 184, "url": "https://patchwork.libcamera.org/api/1.1/people/184/?format=api", "name": "Stefan Klug", "email": "stefan.klug@ideasonboard.com" }, "delegate": { "id": 17, "url": "https://patchwork.libcamera.org/api/1.1/users/17/?format=api", "username": "epaul", "first_name": "Paul", "last_name": "Elder", "email": "paul.elder@ideasonboard.com" }, "mbox": "https://patchwork.libcamera.org/patch/22199/mbox/", "series": [ { "id": 4854, "url": "https://patchwork.libcamera.org/api/1.1/series/4854/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4854", "date": "2024-12-06T10:13:22", "name": "rkisp1: Fix aspect ratio and ScalerCrop", "version": 3, "mbox": "https://patchwork.libcamera.org/series/4854/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22199/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22199/checks/", "tags": {}, "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 7E5E3BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 6 Dec 2024 10:14:13 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2E15566148;\n\tFri, 6 Dec 2024 11:14:13 +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 6E11966147\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 6 Dec 2024 11:14:08 +0100 (CET)", "from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:3543:aebe:e043:ef86])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0A9EF9FC;\n\tFri, 6 Dec 2024 11:13:39 +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=\"qNNW5X0R\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733480019;\n\tbh=lkLu06CK3ndS/9wQqnPGGRn5ROVBHx/zFZHyO4c7MEI=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=qNNW5X0Rosj6hjz2b6nTYLzIUKdIx4lhA7QHT6zw9bdUrkA+d0pnWwRo+jzw9dVQB\n\tV0toFXuVZjGP/XL+6ItecO7f1CcuOb5othYJAvbsoQvrPw9fdjj2uSMF4pat+45T4M\n\tnHP6DQFadc2PhQLAWfpS1P2ysEp7OQTNtUYBwCuM=", "From": "Stefan Klug <stefan.klug@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Stefan Klug <stefan.klug@ideasonboard.com>,\n\tPaul Elder <paul.elder@ideasonboard.com>", "Subject": "[PATCH v3 07/17] pipeline: rkisp1: Fix ScalerCrop to be in sensor\n\tcoordinates", "Date": "Fri, 6 Dec 2024 11:13:29 +0100", "Message-ID": "<20241206101344.767170-8-stefan.klug@ideasonboard.com>", "X-Mailer": "git-send-email 2.43.0", "In-Reply-To": "<20241206101344.767170-1-stefan.klug@ideasonboard.com>", "References": "<20241206101344.767170-1-stefan.klug@ideasonboard.com>", "MIME-Version": "1.0", "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>" }, "content": "ScalerCrop is specified as being in sensor coordinates. The current\ndewarper implementation on the imx8mp handles ScalerCrop in dewarper\ncoordinates. This leads to unexpected results and an unusable ScalerCrop\ncontrol in camshark. Fix that by transforming back and forth between\nsensor coordinates and dewarper coordinates.\n\nSigned-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n---\nChanges in v3:\n- Rename dewarpSensorCrop_ to scalerMaxCrop_\n- Remove unnecessary ScalerCrop max calculation\n\nChanges in v2:\n- Initialize dewarperSensorCrop_ to sane defaults\n- Collect tags\n---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 50 +++++++++++++++++++-----\n 1 file changed, 41 insertions(+), 9 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 89946b782854..ad556ec85a2c 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -205,6 +205,7 @@ private:\n \tRkISP1SelfPath selfPath_;\n \n \tstd::unique_ptr<V4L2M2MConverter> dewarper_;\n+\tRectangle scalerMaxCrop_;\n \tbool useDewarper_;\n \n \tstd::optional<Rectangle> activeCrop_;\n@@ -861,6 +862,15 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n \t\t\t\toutputCfgs.push_back(const_cast<StreamConfiguration &>(cfg));\n \t\t\t\tret = dewarper_->configure(cfg, outputCfgs);\n \t\t\t\tuseDewarper_ = ret ? false : true;\n+\n+\t\t\t\t/*\n+\t\t\t\t * Calculate the crop rectangle of the data\n+\t\t\t\t * flowing into the dewarper in sensor\n+\t\t\t\t * coordinates.\n+\t\t\t\t */\n+\t\t\t\tscalerMaxCrop_ =\n+\t\t\t\t\toutputCrop.transformedBetween(inputCrop,\n+\t\t\t\t\t\t\t\t sensorInfo.analogCrop);\n \t\t\t}\n \t\t} else if (hasSelfPath_) {\n \t\t\tret = selfPath_.configure(cfg, format);\n@@ -1223,10 +1233,19 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n \t\tstd::pair<Rectangle, Rectangle> cropLimits =\n \t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n \n-\t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n-\t\t\t\t\t\t\t cropLimits.second,\n-\t\t\t\t\t\t\t cropLimits.second);\n-\t\tactiveCrop_ = cropLimits.second;\n+\t\t/*\n+\t\t * ScalerCrop is specified to be in Sensor coordinates.\n+\t\t * So we need to transform the limits to sensor coordinates.\n+\t\t * We can safely assume that the maximum crop limit contains the\n+\t\t * full fov of the dewarper.\n+\t\t */\n+\t\tRectangle min = cropLimits.first.transformedBetween(cropLimits.second,\n+\t\t\t\t\t\t\t\t scalerMaxCrop_);\n+\n+\t\tcontrols[&controls::ScalerCrop] = ControlInfo(min,\n+\t\t\t\t\t\t\t scalerMaxCrop_,\n+\t\t\t\t\t\t\t scalerMaxCrop_);\n+\t\tactiveCrop_ = scalerMaxCrop_;\n \t}\n \n \t/* Add the IPA registered controls to list of camera controls. */\n@@ -1254,6 +1273,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n \t/* Initialize the camera properties. */\n \tdata->properties_ = data->sensor_->properties();\n \n+\tscalerMaxCrop_ = Rectangle(data->sensor_->resolution());\n+\n \tconst CameraSensorProperties::SensorDelays &delays = data->sensor_->sensorDelays();\n \tstd::unordered_map<uint32_t, DelayedControls::ControlParams> params = {\n \t\t{ V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },\n@@ -1473,22 +1494,33 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n \t/* Handle scaler crop control. */\n \tconst auto &crop = request->controls().get(controls::ScalerCrop);\n \tif (crop) {\n-\t\tRectangle appliedRect = crop.value();\n+\t\tRectangle rect = crop.value();\n+\n+\t\t/*\n+\t\t * ScalerCrop is specified to be in Sensor coordinates.\n+\t\t * So we need to transform it into dewarper coordinates.\n+\t\t * We can safely assume that the maximum crop limit contains the\n+\t\t * full fov of the dewarper.\n+\t\t */\n+\t\tstd::pair<Rectangle, Rectangle> cropLimits =\n+\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n \n+\t\trect = rect.transformedBetween(scalerMaxCrop_, cropLimits.second);\n \t\tint ret = dewarper_->setInputCrop(&data->mainPathStream_,\n-\t\t\t\t\t\t &appliedRect);\n-\t\tif (!ret && appliedRect != crop.value()) {\n+\t\t\t\t\t\t &rect);\n+\t\trect = rect.transformedBetween(cropLimits.second, scalerMaxCrop_);\n+\t\tif (!ret && rect != crop.value()) {\n \t\t\t/*\n \t\t\t * If the rectangle is changed by setInputCrop on the\n \t\t\t * dewarper, log a debug message and cache the actual\n \t\t\t * applied rectangle for metadata reporting.\n \t\t\t */\n \t\t\tLOG(RkISP1, Debug)\n-\t\t\t\t<< \"Applied rectangle \" << appliedRect.toString()\n+\t\t\t\t<< \"Applied rectangle \" << rect.toString()\n \t\t\t\t<< \" differs from requested \" << crop.value().toString();\n \t\t}\n \n-\t\tactiveCrop_ = appliedRect;\n+\t\tactiveCrop_ = rect;\n \t}\n \n \t/*\n", "prefixes": [ "v3", "07/17" ] }