From patchwork Thu Apr 3 07:45:49 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stanislaw Gruszka X-Patchwork-Id: 23112 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 8EFCCC3213 for ; Thu, 3 Apr 2025 07:46:14 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3661F689A3; Thu, 3 Apr 2025 09:46:14 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="fuiT0hsy"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.7]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4CB86689A0 for ; Thu, 3 Apr 2025 09:46:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1743666373; x=1775202373; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=HjL273VvDMxhXp4GAhM+gjF4hrSKP81TEziKHH7wfz8=; b=fuiT0hsykT1PuyL/jvQydb2mNYKpsD1JPeOaN+VYgn+1wIQPDBdpkvbF sgY4vDcRBVIB5vRt8Hrb97aZwLlIFsTUuR/LZvg18F+Ik1Hv4UVtQTAGB wxszasxVX6Xsfk6daUMhLRJQJR9CoXCBD06JLTM4Jn3eL+ueYI/hpN8Zi L0VaDde9duM6NWonGTAfExPter/SNN3Gz6LGJ62aMpXsM5dBjQmhqTl5E VG3tkPfcNEIOoEjP4Y1snxiCY2PRqQHkYmwHkl+gY4aMNH33Wd07hZvJK 1Tv4EIgD4ASJnZ9Nh43GyhiK11Y+jw4ObYeY1J9gs4EJ267uoXU0zKaXg Q==; X-CSE-ConnectionGUID: 4zjc4ZpTQGGX1h0RuGDDmw== X-CSE-MsgGUID: vcuxuCaaQ3qOHDckvCcPKQ== X-IronPort-AV: E=McAfee;i="6700,10204,11392"; a="70425162" X-IronPort-AV: E=Sophos;i="6.15,184,1739865600"; d="scan'208";a="70425162" Received: from orviesa003.jf.intel.com ([10.64.159.143]) by fmvoesa101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Apr 2025 00:46:11 -0700 X-CSE-ConnectionGUID: Dde6mqE9R3aPFte2MkACnw== X-CSE-MsgGUID: Uv2eG27fQZCmszA7u5RnlQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.15,184,1739865600"; d="scan'208";a="131785192" Received: from sgruszka-mobl.ger.corp.intel.com (HELO localhost) ([10.246.8.237]) by ORVIESA003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Apr 2025 00:46:08 -0700 From: Stanislaw Gruszka To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal , Hans de Goede , Laurent Pinchart , Kieran Bingham , Sakari Ailus , Stefan Klug 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 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" The simple pipeline handler uses frame start events to apply sensor controls through the DelayedControls class. The setSensorControls() function applies the controls directly, which would result in controls being applied twice, if it wasn't for the fact that the pipeline handler forgot to enable the frame start events in the first place. Those two issues cancel each other, but cause controls to always be applied directly. Fix the issue by only applying controls directly in setSensorControls() if no frame start event emitter is available, and by enabling the frame start events in startDevice() otherwise. Disable them in stopDevice() for symmetry. Reviewed-by: Stefan Klug # v6 Co-developed-by: Hans de Goede Signed-off-by: Hans de Goede Co-developed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart Signed-off-by: Stanislaw Gruszka Reviewed-by: Kieran Bingham --- src/libcamera/pipeline/simple/simple.cpp | 49 +++++++++++++++++++++--- 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 06e805d89caa..c97904076b63 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -327,6 +327,7 @@ public: std::list entities_; std::unique_ptr sensor_; V4L2VideoDevice *video_; + V4L2Subdevice *frameStartEmitter_; std::vector configs_; std::map> formats_; @@ -633,6 +634,20 @@ int SimpleCameraData::init() properties_ = sensor_->properties(); + /* Find the first subdev that can generate a frame start signal, if any. */ + frameStartEmitter_ = nullptr; + for (const Entity &entity : entities_) { + V4L2Subdevice *sd = pipe->subdev(entity.entity); + if (!sd || !sd->supportsFrameStartEvent()) + continue; + + LOG(SimplePipeline, Debug) + << "Using frameStart signal from '" + << entity.entity->name() << "'"; + frameStartEmitter_ = sd; + break; + } + return 0; } @@ -983,8 +998,18 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata void SimpleCameraData::setSensorControls(const ControlList &sensorControls) { delayedCtrls_->push(sensorControls); - ControlList ctrls(sensorControls); - sensor_->setControls(&ctrls); + /* + * Directly apply controls now if there is no frameStart signal. + * + * \todo Applying controls directly not only increases the risk of + * applying them to the wrong frame (or across a frame boundary), + * but it also bypasses delayedCtrls_, creating AGC regulation issues. + * Both problems should be fixed. + */ + if (!frameStartEmitter_) { + ControlList ctrls(sensorControls); + sensor_->setControls(&ctrls); + } } /* Retrieve all source pads connected to a sink pad through active routes. */ @@ -1409,6 +1434,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL { SimpleCameraData *data = cameraData(camera); V4L2VideoDevice *video = data->video_; + V4L2Subdevice *frameStartEmitter = data->frameStartEmitter_; int ret; const MediaPad *pad = acquirePipeline(data); @@ -1438,8 +1464,15 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL video->bufferReady.connect(data, &SimpleCameraData::imageBufferReady); - data->video_->frameStart.connect(data->delayedCtrls_.get(), - &DelayedControls::applyControls); + if (frameStartEmitter) { + ret = frameStartEmitter->setFrameStartEnabled(true); + if (ret) { + stop(camera); + return ret; + } + frameStartEmitter->frameStart.connect(data->delayedCtrls_.get(), + &DelayedControls::applyControls); + } ret = video->streamOn(); if (ret < 0) { @@ -1472,9 +1505,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera) { SimpleCameraData *data = cameraData(camera); V4L2VideoDevice *video = data->video_; + V4L2Subdevice *frameStartEmitter = data->frameStartEmitter_; - data->video_->frameStart.disconnect(data->delayedCtrls_.get(), - &DelayedControls::applyControls); + if (frameStartEmitter) { + frameStartEmitter->setFrameStartEnabled(false); + frameStartEmitter->frameStart.disconnect(data->delayedCtrls_.get(), + &DelayedControls::applyControls); + } if (data->useConversion_) { if (data->converter_)