From patchwork Thu Jun 30 00:02:47 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 16445 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 EEA99BD808 for ; Thu, 30 Jun 2022 00:03:32 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9AF0A65655; Thu, 30 Jun 2022 02:03:32 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1656547412; bh=nHf6gwYLwDJnSJ8B2Zk8XcZvNZ2LWnCgfGqPfNJFrMo=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=0GvNVMdmCNwfH3DV0RA2XYPR2ZlbHEEk2OQ0v5+XDnJQymVuNkFRcFDox5aoFMQe/ 3pIP2yC1NZqF+PliA13gS6mkOfknW6QduPscad2Y4H5uA5vX46lsLWJo0RgLRUM9se 4ZhEV5VvHrj9dZfnz8tXzjaQmd5yboZl08Kgjtgn3YzfILGt+JSk7SDgQ1u0gkCz83 PZPR/rKF3klsaXcItI3Gd9KEkJ7uBZRd8vpUwp7Py63ZtaPb0KvPG3F47ZMk9a3LXO IhwPiL+EBtsHSsL58+rJlMO5BBuBtf17QBQ897bWCeCiSyoOxtdKcv7jPyaW5hKXL6 5rcFmiKolgpEg== 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 D5DA965634 for ; Thu, 30 Jun 2022 02:03:18 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="tPc9oeac"; dkim-atps=neutral Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0DD4659D; Thu, 30 Jun 2022 02:03:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1656547398; bh=nHf6gwYLwDJnSJ8B2Zk8XcZvNZ2LWnCgfGqPfNJFrMo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=tPc9oeaciepF73lXkLZ4Hb/7ZW+J64Qt/bjPPc3VfbEa1/f15yjlyJbd9m0R+vkzM bAQP9F138TMcIrB7jgcxGVhh58MmrAq/xcfB6YkV0Z5bVC2QfAzluwkRHVAMHapC9g y6Pygz5WDK67XVEakpaW11HM8xuWC0vM26PI4X7g= To: libcamera-devel@lists.libcamera.org Date: Thu, 30 Jun 2022 03:02:47 +0300 Message-Id: <20220630000251.31295-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220630000251.31295-1-laurent.pinchart@ideasonboard.com> References: <20220630000251.31295-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 08/12] gstreamer: Use dedicated lock for request queues 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Cc: Nicolas Dufresne Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add a new lock to the GstLibcameraSrcState class to protect the queued and completed requests queues. This replaces the GstObject lock, and minimizes the lock contention between the request completion handler and the task run handler as the former must run as fast as possible. Signed-off-by: Laurent Pinchart Reviewed-by: Nicolas Dufresne Reviewed-by: Umang Jain --- Changes since v1: - Add comment about lock_ --- src/gstreamer/gstlibcamerasrc.cpp | 44 +++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index 8c1cd7017d98..6f9a03c515d2 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -32,6 +32,8 @@ #include #include +#include + #include #include #include @@ -111,8 +113,18 @@ struct GstLibcameraSrcState { std::shared_ptr cam_; std::unique_ptr config_; std::vector srcpads_; - std::queue> queuedRequests_; - std::queue> completedRequests_; + + /* + * Contention on this lock_ must be minimized, as it has to be taken in + * the realtime-sensitive requestCompleted() handler to protect + * queuedRequests_ and completedRequests_. + */ + Mutex lock_; + std::queue> queuedRequests_ + LIBCAMERA_TSA_GUARDED_BY(lock_); + std::queue> completedRequests_ + LIBCAMERA_TSA_GUARDED_BY(lock_); + guint group_id_; void requestCompleted(Request *request); @@ -155,12 +167,15 @@ GstStaticPadTemplate request_src_template = { void GstLibcameraSrcState::requestCompleted(Request *request) { - GLibLocker lock(GST_OBJECT(src_)); - GST_DEBUG_OBJECT(src_, "buffers are ready"); - std::unique_ptr wrap = std::move(queuedRequests_.front()); - queuedRequests_.pop(); + std::unique_ptr wrap; + + { + MutexLocker locker(lock_); + wrap = std::move(queuedRequests_.front()); + queuedRequests_.pop(); + } g_return_if_fail(wrap->request_.get() == request); @@ -183,7 +198,10 @@ GstLibcameraSrcState::requestCompleted(Request *request) wrap->latency_ = sys_now - timestamp; } - completedRequests_.push(std::move(wrap)); + { + MutexLocker locker(lock_); + completedRequests_.push(std::move(wrap)); + } gst_task_resume(src_->task); } @@ -289,16 +307,17 @@ gst_libcamera_src_task_run(gpointer user_data) } if (wrap) { - GLibLocker lock(GST_OBJECT(self)); GST_TRACE_OBJECT(self, "Requesting buffers"); state->cam_->queueRequest(wrap->request_.get()); + + MutexLocker locker(state->lock_); state->queuedRequests_.push(std::move(wrap)); /* The RequestWrap will be deleted in the completion handler. */ } { - GLibLocker lock(GST_OBJECT(self)); + MutexLocker locker(state->lock_); if (!state->completedRequests_.empty()) { wrap = std::move(state->completedRequests_.front()); @@ -358,7 +377,7 @@ gst_libcamera_src_task_run(gpointer user_data) bool do_pause; { - GLibLocker lock(GST_OBJECT(self)); + MutexLocker locker(state->lock_); do_pause = state->completedRequests_.empty(); } @@ -513,7 +532,10 @@ gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task, state->cam_->stop(); - state->completedRequests_ = {}; + { + MutexLocker locker(state->lock_); + state->completedRequests_ = {}; + } for (GstPad *srcpad : state->srcpads_) gst_libcamera_pad_set_pool(srcpad, nullptr);