Patch Detail
Show a patch.
GET /api/1.1/patches/22773/?format=api
{ "id": 22773, "url": "https://patchwork.libcamera.org/api/1.1/patches/22773/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22773/", "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": "<20250207133743.1074164-1-kieran.bingham@ideasonboard.com>", "date": "2025-02-07T13:37:43", "name": "libcamera: rkisp1: Only connect delayed controls at start/stop", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "13d0e6774d30c06c7b66bed1e703deb642cecaba", "submitter": { "id": 4, "url": "https://patchwork.libcamera.org/api/1.1/people/4/?format=api", "name": "Kieran Bingham", "email": "kieran.bingham@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22773/mbox/", "series": [ { "id": 4995, "url": "https://patchwork.libcamera.org/api/1.1/series/4995/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4995", "date": "2025-02-07T13:37:43", "name": "libcamera: rkisp1: Only connect delayed controls at start/stop", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4995/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22773/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22773/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 0B544C32EA\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 7 Feb 2025 13:37:50 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A844C68600;\n\tFri, 7 Feb 2025 14:37:48 +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 DB417685AF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 7 Feb 2025 14:37:46 +0100 (CET)", "from Monstersaurus.tail69b4.ts.net\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 79C27F0C;\n\tFri, 7 Feb 2025 14:36:32 +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=\"qAVvDwn7\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1738935392;\n\tbh=238Qdfxwdr9loQmkeBJL+HbZ2vupAb5uysFQItRud08=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=qAVvDwn7UIuBD+nT+2OMOUYvDxMJZKqNy4T1W1BSPVjFat+n1KkfU/0vw0QO/Uh8I\n\tVmsWSm0vfJgcP6W+L2FhMqTLHoW0y+2LGhlfg4KAW8+dqa1X97gEQ2d9cOwjbYNSna\n\tTS5VpOhYmGMFf2Pz4o/9ivlkaS/fHqCo21RPCZuE=", "From": "Kieran Bingham <kieran.bingham@ideasonboard.com>", "To": "libcamera devel <libcamera-devel@lists.libcamera.org>", "Cc": "Kieran Bingham <kieran.bingham@ideasonboard.com>", "Subject": "[PATCH] libcamera: rkisp1: Only connect delayed controls at\n\tstart/stop", "Date": "Fri, 7 Feb 2025 13:37:43 +0000", "Message-ID": "<20250207133743.1074164-1-kieran.bingham@ideasonboard.com>", "X-Mailer": "git-send-email 2.47.1", "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": "The RKISP1 path may potentially have multiple cameras connected through\ncomplex pipelines such as video multiplexors or multiple FPGA paths.\n\nThe RKISP1 pipeline handler notifies DelayedControls that a new frame is\ncommencing by using the frameStart event on the ISP and using that to\nsignal to DelayedControls that it is time to process the controls for\nthe next frame.\n\nWhen more than one camera is connected to an ISP it is important not to\nsignal events to an inactive Camera.\n\nMove the frameStart signal connection from CreateCamera() to start() and\nintroduce a corresponding disconnect at stop().\n\nSigned-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n---\n\nWhen developing on an i.MX8MP, with multiple cameras connected to a\nsingle ISP this caused a horrible bug which I eventually found using\nUBSAN/ASAN to spot that actaully there was a use-after-free in calling\ndelayedControls on a camera object that wasn't active.\n\nOne other alternative here, could be to move the connection of the\nsignal into the setFrameStartEnabled() call, if we pass in the\nCameraData to be explicit on where we are connecting and disconnect. But\nmaybe that's overkill too.\n\n\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 1ac8d8ae7ed9..b1b0bde7fd00 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -1071,6 +1071,11 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n \tutils::ScopeExitActions actions;\n \tint ret;\n \n+\tisp_->frameStart.connect(data->delayedCtrls_.get(),\n+\t\t\t\t &DelayedControls::applyControls);\n+\n+\tactions += [&]() { isp_->frameStart.disconnect(data->delayedCtrls_.get()); };\n+\n \t/* Allocate buffers for internal pipeline usage. */\n \tret = allocateBuffers(camera);\n \tif (ret)\n@@ -1142,6 +1147,8 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)\n \n \tisp_->setFrameStartEnabled(false);\n \n+\tisp_->frameStart.disconnect(data->delayedCtrls_.get());\n+\n \tdata->ipa_->stop();\n \n \tif (hasSelfPath_)\n@@ -1330,8 +1337,6 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n \tdata->delayedCtrls_ =\n \t\tstd::make_unique<DelayedControls>(data->sensor_->device(),\n \t\t\t\t\t\t params);\n-\tisp_->frameStart.connect(data->delayedCtrls_.get(),\n-\t\t\t\t &DelayedControls::applyControls);\n \n \tret = data->loadIPA(media_->hwRevision());\n \tif (ret)\n", "prefixes": [] }