From patchwork Wed Mar 5 13:52:54 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stanislaw Gruszka X-Patchwork-Id: 22928 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 3C295C3257 for ; Wed, 5 Mar 2025 13:53:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E4C5B68823; Wed, 5 Mar 2025 14:53:18 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="E2wbPZ2w"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.13]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B011568823 for ; Wed, 5 Mar 2025 14:53:16 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1741182797; x=1772718797; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=idfQbkN/xGzKp15crcBiqWgyKpcofe1e6bWptz43C2I=; b=E2wbPZ2wG3o/Gl9RMKCbod3vLsnXQ1RV9YumfOPhim3jlrWoYWa+YWlX bUDPVS1U5Ss8TUt+CeDB3yGeLaEsGXY17PbMGpy4VjQml0K8S8ZwcgIHE OPTmq2cPmcpKWHcbNIxATvQ+cRrj7XHJRNSzwxI6eMwSCtgkfgFA3MkBh oBpoMer/5v0UpA0DrEs6tgPGrT+lhE5C7aYOXGvYBEyB6OShDyu2Oupy1 0d+IvX6d5SbGMUkDPly/dKe37d9eo9czt3edm0YwzFhrJWi8KZ2MSZJNn wM3fQQFiL4kriTzckvwG/ntEiLeC8sCZewKDebfsMrHVHDjKV/csEtjHz w==; X-CSE-ConnectionGUID: q5j2Qv4wRkmebpK97Pf1jQ== X-CSE-MsgGUID: Fw/i/xVJQtiI99fUxfzsPA== X-IronPort-AV: E=McAfee;i="6700,10204,11363"; a="53134786" X-IronPort-AV: E=Sophos;i="6.14,223,1736841600"; d="scan'208";a="53134786" Received: from orviesa004.jf.intel.com ([10.64.159.144]) by orvoesa105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Mar 2025 05:53:17 -0800 X-CSE-ConnectionGUID: vM1BAb64SRa6mOUyyw7Qzg== X-CSE-MsgGUID: 6mjgmGQMTXuq3UTCmzgJXQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.14,223,1736841600"; d="scan'208";a="123735793" Received: from sgruszka-mobl.ger.corp.intel.com (HELO localhost) ([10.245.112.97]) by orviesa004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Mar 2025 05:53:14 -0800 From: Stanislaw Gruszka To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal , Laurent Pinchart , Kieran Bingham , Naushir Patuck , Sakari Ailus , Hans de Goede Subject: [PATCH v6 3/5] pipeline: simple: Enable frame start events Date: Wed, 5 Mar 2025 14:52:54 +0100 Message-Id: <20250305135256.801351-4-stanislaw.gruszka@linux.intel.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20250305135256.801351-1-stanislaw.gruszka@linux.intel.com> References: <20250305135256.801351-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. 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: Stefan Klug --- 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 8345a771..7be49017 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -277,6 +277,7 @@ public: std::list entities_; std::unique_ptr sensor_; V4L2VideoDevice *video_; + V4L2Subdevice *frameStartEmitter_; std::vector configs_; std::map> formats_; @@ -579,6 +580,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; } @@ -897,8 +912,18 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId) 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. */ @@ -1323,6 +1348,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); @@ -1352,8 +1378,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) { @@ -1386,9 +1419,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_)