From patchwork Fri Dec 6 11:27:43 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stanislaw Gruszka X-Patchwork-Id: 22210 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 B684CBE173 for ; Fri, 6 Dec 2024 11:27:56 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EC154618B3; Fri, 6 Dec 2024 12:27:55 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="ejVhYtDk"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.21]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 79C39618B3 for ; Fri, 6 Dec 2024 12:27:53 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1733484474; x=1765020474; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=Wffql4qqGjdjWvZyiPrfbQ/hqno93NstFihPL2ssJm0=; b=ejVhYtDkvxgsXEazwG8r2LWJzdP661+YaWpbCeY8XxbBDuqAeRuZIp+l iuVKTGx4iMqY6K+j+4WoBLrN8Xrg7fmy/REPcozheN8uwnJtUjaUf8Xux 01ItfAvx5EJgb7OzDY+5MgDNh/xRuGcaWgYh7x33PmFKpsFJOkp2yiWxp khs4YAHxM23KVbOu0v8e5l6dv8n2XhHaTQFCCjUbmVxxSKoFsXQO97mpr FDydQrbzYi6B5Qzud8dUnkfOaG+AtkhHpXjjloXdDXwuRg9tFY5p7GCKo hKd15sqCveifUWtI+oa+VTSgwqSisiKJ2Z433b0NBDe2zKzxs7SfG6Vbl w==; X-CSE-ConnectionGUID: rgPI+y0gRBqwg9gh7ChKRw== X-CSE-MsgGUID: 2g0FX7BLQDmdO6+AQ+LDUA== X-IronPort-AV: E=McAfee;i="6700,10204,11277"; a="33755252" X-IronPort-AV: E=Sophos;i="6.12,213,1728975600"; d="scan'208";a="33755252" Received: from fmviesa001.fm.intel.com ([10.60.135.141]) by orvoesa113.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Dec 2024 03:27:51 -0800 X-CSE-ConnectionGUID: hpHGRAAxQR+c+gZCWyyPuQ== X-CSE-MsgGUID: y06yRQ9TSAutFZlrje5vlg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.12,213,1728975600"; d="scan'208";a="125292742" Received: from sgruszka-mobl.ger.corp.intel.com (HELO localhost) ([10.245.85.43]) by smtpauth.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Dec 2024 03:27:50 -0800 From: Stanislaw Gruszka To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal Subject: [RFC] delayed_controls: avoid reading undefined control value Date: Fri, 6 Dec 2024 12:27:43 +0100 Message-Id: <20241206112743.95435-1-stanislaw.gruszka@linux.intel.com> X-Mailer: git-send-email 2.34.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" Limit ControlRingBuffer index by number of queued entries in order do avoid reading undefined control value with type ControlTypeNone . It can happen at the begging of streaming when ControlRingBuffer is not yet filled and there are errors on CSI-2 bus resulting dropping frames. Then we can call to DelayedControls::get() with frame number that exceed number of saved entries and get below assertion failure: ../src/libcamera/delayed_controls.cpp:227: libcamera::ControlList libcamera::DelayedControls::get(uint32_t): Assertion `info.type() != ControlTypeNone' failed Bug: https://bugs.libcamera.org/show_bug.cgi?id=241 Signed-off-by: Stanislaw Gruszka Reviewed-by: Kieran Bingham Signed-off-by: Stanislaw Gruszka --- Notes: When debugging this I notice that the push() and get() are done in different threads. Maybe mutex should be used? Not sure how synchronization works for mojom signals handlers. Also I'm not sure if similar change like in this patch is not needed for rpi DelayedControls Alternative fix would be to pre-fill ControlRingBuffer with valid control values. Should we care about possibility of queueCount_ overflowing ? src/libcamera/delayed_controls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.43.0 diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 94d0a575..78b0b8f7 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -202,7 +202,7 @@ bool DelayedControls::push(const ControlList &controls) */ ControlList DelayedControls::get(uint32_t sequence) { - unsigned int index = std::max(0, sequence - maxDelay_); + unsigned int index = std::clamp(sequence - maxDelay_, 0, queueCount_ - 1); ControlList out(device_->controls()); for (const auto &ctrl : values_) {