Patch Detail
Show a patch.
GET /api/patches/23112/?format=api
{ "id": 23112, "url": "https://patchwork.libcamera.org/api/patches/23112/?format=api", "web_url": "https://patchwork.libcamera.org/patch/23112/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/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": "<20250403074551.263496-4-stanislaw.gruszka@linux.intel.com>", "date": "2025-04-03T07:45:49", "name": "[v7,3/5] pipeline: simple: Enable frame start events", "commit_ref": "183bab1643466c5d62d2cb93feea286571c73fbb", "pull_url": null, "state": "accepted", "archived": false, "hash": "2b33405506984bec765a8064bcb059160f047aeb", "submitter": { "id": 211, "url": "https://patchwork.libcamera.org/api/people/211/?format=api", "name": "Stanislaw Gruszka", "email": "stanislaw.gruszka@linux.intel.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/23112/mbox/", "series": [ { "id": 5109, "url": "https://patchwork.libcamera.org/api/series/5109/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5109", "date": "2025-04-03T07:45:46", "name": "libcamera: start frame events changes", "version": 7, "mbox": "https://patchwork.libcamera.org/series/5109/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/23112/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/23112/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 8EFCCC3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 3 Apr 2025 07:46:14 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3661F689A3;\n\tThu, 3 Apr 2025 09:46:14 +0200 (CEST)", "from mgamail.intel.com (mgamail.intel.com [192.198.163.7])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4CB86689A0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 3 Apr 2025 09:46:12 +0200 (CEST)", "from orviesa003.jf.intel.com ([10.64.159.143])\n\tby fmvoesa101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n\t03 Apr 2025 00:46:11 -0700", "from sgruszka-mobl.ger.corp.intel.com (HELO localhost)\n\t([10.246.8.237]) by ORVIESA003-auth.jf.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Apr 2025 00:46:08 -0700" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=intel.com header.i=@intel.com\n\theader.b=\"fuiT0hsy\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple;\n\td=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n\tt=1743666373; x=1775202373;\n\th=from:to:cc:subject:date:message-id:in-reply-to:\n\treferences:mime-version:content-transfer-encoding;\n\tbh=HjL273VvDMxhXp4GAhM+gjF4hrSKP81TEziKHH7wfz8=;\n\tb=fuiT0hsykT1PuyL/jvQydb2mNYKpsD1JPeOaN+VYgn+1wIQPDBdpkvbF\n\tsgY4vDcRBVIB5vRt8Hrb97aZwLlIFsTUuR/LZvg18F+Ik1Hv4UVtQTAGB\n\twxszasxVX6Xsfk6daUMhLRJQJR9CoXCBD06JLTM4Jn3eL+ueYI/hpN8Zi\n\tL0VaDde9duM6NWonGTAfExPter/SNN3Gz6LGJ62aMpXsM5dBjQmhqTl5E\n\tVG3tkPfcNEIOoEjP4Y1snxiCY2PRqQHkYmwHkl+gY4aMNH33Wd07hZvJK\n\t1Tv4EIgD4ASJnZ9Nh43GyhiK11Y+jw4ObYeY1J9gs4EJ267uoXU0zKaXg Q==;", "X-CSE-ConnectionGUID": [ "4zjc4ZpTQGGX1h0RuGDDmw==", "Dde6mqE9R3aPFte2MkACnw==" ], "X-CSE-MsgGUID": [ "vcuxuCaaQ3qOHDckvCcPKQ==", "Uv2eG27fQZCmszA7u5RnlQ==" ], "X-IronPort-AV": [ "E=McAfee;i=\"6700,10204,11392\"; a=\"70425162\"", "E=Sophos;i=\"6.15,184,1739865600\"; d=\"scan'208\";a=\"70425162\"", "E=Sophos;i=\"6.15,184,1739865600\"; d=\"scan'208\";a=\"131785192\"" ], "X-ExtLoop1": "1", "From": "Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Milan Zamazal <mzamazal@redhat.com>, Hans de Goede <hdegoede@redhat.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tSakari Ailus <sakari.ailus@linux.intel.com>,\n\tStefan Klug <stefan.klug@ideasonboard.com>", "Subject": "[PATCH v7 3/5] pipeline: simple: Enable frame start events", "Date": "Thu, 3 Apr 2025 09:45:49 +0200", "Message-Id": "<20250403074551.263496-4-stanislaw.gruszka@linux.intel.com>", "X-Mailer": "git-send-email 2.34.1", "In-Reply-To": "<20250403074551.263496-1-stanislaw.gruszka@linux.intel.com>", "References": "<20250403074551.263496-1-stanislaw.gruszka@linux.intel.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": "The simple pipeline handler uses frame start events to apply sensor\ncontrols through the DelayedControls class. The setSensorControls()\nfunction applies the controls directly, which would result in controls\nbeing applied twice, if it wasn't for the fact that the pipeline handler\nforgot to enable the frame start events in the first place. Those two\nissues cancel each other, but cause controls to always be applied\ndirectly.\n\nFix the issue by only applying controls directly in setSensorControls()\nif no frame start event emitter is available, and by enabling the frame\nstart events in startDevice() otherwise. Disable them in stopDevice()\nfor symmetry.\n\nReviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> # v6\nCo-developed-by: Hans de Goede <hdegoede@redhat.com>\nSigned-off-by: Hans de Goede <hdegoede@redhat.com>\nCo-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nSigned-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>\n---\n src/libcamera/pipeline/simple/simple.cpp | 49 +++++++++++++++++++++---\n 1 file changed, 43 insertions(+), 6 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 06e805d89caa..c97904076b63 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -327,6 +327,7 @@ public:\n \tstd::list<Entity> entities_;\n \tstd::unique_ptr<CameraSensor> sensor_;\n \tV4L2VideoDevice *video_;\n+\tV4L2Subdevice *frameStartEmitter_;\n \n \tstd::vector<Configuration> configs_;\n \tstd::map<PixelFormat, std::vector<const Configuration *>> formats_;\n@@ -633,6 +634,20 @@ int SimpleCameraData::init()\n \n \tproperties_ = sensor_->properties();\n \n+\t/* Find the first subdev that can generate a frame start signal, if any. */\n+\tframeStartEmitter_ = nullptr;\n+\tfor (const Entity &entity : entities_) {\n+\t\tV4L2Subdevice *sd = pipe->subdev(entity.entity);\n+\t\tif (!sd || !sd->supportsFrameStartEvent())\n+\t\t\tcontinue;\n+\n+\t\tLOG(SimplePipeline, Debug)\n+\t\t\t<< \"Using frameStart signal from '\"\n+\t\t\t<< entity.entity->name() << \"'\";\n+\t\tframeStartEmitter_ = sd;\n+\t\tbreak;\n+\t}\n+\n \treturn 0;\n }\n \n@@ -983,8 +998,18 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata\n void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n {\n \tdelayedCtrls_->push(sensorControls);\n-\tControlList ctrls(sensorControls);\n-\tsensor_->setControls(&ctrls);\n+\t/*\n+ * Directly apply controls now if there is no frameStart signal.\n+\t *\n+\t * \\todo Applying controls directly not only increases the risk of\n+\t * applying them to the wrong frame (or across a frame boundary),\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 }\n \n /* Retrieve all source pads connected to a sink pad through active routes. */\n@@ -1409,6 +1434,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n {\n \tSimpleCameraData *data = cameraData(camera);\n \tV4L2VideoDevice *video = data->video_;\n+\tV4L2Subdevice *frameStartEmitter = data->frameStartEmitter_;\n \tint ret;\n \n \tconst MediaPad *pad = acquirePipeline(data);\n@@ -1438,8 +1464,15 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n \n \tvideo->bufferReady.connect(data, &SimpleCameraData::imageBufferReady);\n \n-\tdata->video_->frameStart.connect(data->delayedCtrls_.get(),\n-\t\t\t\t\t &DelayedControls::applyControls);\n+\tif (frameStartEmitter) {\n+\t\tret = frameStartEmitter->setFrameStartEnabled(true);\n+\t\tif (ret) {\n+\t\t\tstop(camera);\n+\t\t\treturn ret;\n+\t\t}\n+\t\tframeStartEmitter->frameStart.connect(data->delayedCtrls_.get(),\n+\t\t\t\t\t\t &DelayedControls::applyControls);\n+\t}\n \n \tret = video->streamOn();\n \tif (ret < 0) {\n@@ -1472,9 +1505,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n {\n \tSimpleCameraData *data = cameraData(camera);\n \tV4L2VideoDevice *video = data->video_;\n+\tV4L2Subdevice *frameStartEmitter = data->frameStartEmitter_;\n \n-\tdata->video_->frameStart.disconnect(data->delayedCtrls_.get(),\n-\t\t\t\t\t &DelayedControls::applyControls);\n+\tif (frameStartEmitter) {\n+\t\tframeStartEmitter->setFrameStartEnabled(false);\n+\t\tframeStartEmitter->frameStart.disconnect(data->delayedCtrls_.get(),\n+\t\t\t\t\t\t\t &DelayedControls::applyControls);\n+\t}\n \n \tif (data->useConversion_) {\n \t\tif (data->converter_)\n", "prefixes": [ "v7", "3/5" ] }