From patchwork Fri Feb 7 13:37:43 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 22773 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 0B544C32EA for ; Fri, 7 Feb 2025 13:37:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A844C68600; Fri, 7 Feb 2025 14:37:48 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="qAVvDwn7"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DB417685AF for ; Fri, 7 Feb 2025 14:37:46 +0100 (CET) Received: from Monstersaurus.tail69b4.ts.net (cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 79C27F0C; Fri, 7 Feb 2025 14:36:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1738935392; bh=238Qdfxwdr9loQmkeBJL+HbZ2vupAb5uysFQItRud08=; h=From:To:Cc:Subject:Date:From; b=qAVvDwn7UIuBD+nT+2OMOUYvDxMJZKqNy4T1W1BSPVjFat+n1KkfU/0vw0QO/Uh8I VmsWSm0vfJgcP6W+L2FhMqTLHoW0y+2LGhlfg4KAW8+dqa1X97gEQ2d9cOwjbYNSna TS5VpOhYmGMFf2Pz4o/9ivlkaS/fHqCo21RPCZuE= From: Kieran Bingham To: libcamera devel Cc: Kieran Bingham Subject: [PATCH] libcamera: rkisp1: Only connect delayed controls at start/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 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 RKISP1 path may potentially have multiple cameras connected through complex pipelines such as video multiplexors or multiple FPGA paths. The RKISP1 pipeline handler notifies DelayedControls that a new frame is commencing by using the frameStart event on the ISP and using that to signal to DelayedControls that it is time to process the controls for the next frame. When more than one camera is connected to an ISP it is important not to signal events to an inactive Camera. Move the frameStart signal connection from CreateCamera() to start() and introduce a corresponding disconnect at stop(). Signed-off-by: Kieran Bingham --- When developing on an i.MX8MP, with multiple cameras connected to a single ISP this caused a horrible bug which I eventually found using UBSAN/ASAN to spot that actaully there was a use-after-free in calling delayedControls on a camera object that wasn't active. One other alternative here, could be to move the connection of the signal into the setFrameStartEnabled() call, if we pass in the CameraData to be explicit on where we are connecting and disconnect. But maybe that's overkill too. src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 1ac8d8ae7ed9..b1b0bde7fd00 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1071,6 +1071,11 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL utils::ScopeExitActions actions; int ret; + isp_->frameStart.connect(data->delayedCtrls_.get(), + &DelayedControls::applyControls); + + actions += [&]() { isp_->frameStart.disconnect(data->delayedCtrls_.get()); }; + /* Allocate buffers for internal pipeline usage. */ ret = allocateBuffers(camera); if (ret) @@ -1142,6 +1147,8 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera) isp_->setFrameStartEnabled(false); + isp_->frameStart.disconnect(data->delayedCtrls_.get()); + data->ipa_->stop(); if (hasSelfPath_) @@ -1330,8 +1337,6 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) data->delayedCtrls_ = std::make_unique(data->sensor_->device(), params); - isp_->frameStart.connect(data->delayedCtrls_.get(), - &DelayedControls::applyControls); ret = data->loadIPA(media_->hwRevision()); if (ret)