{"id":16728,"url":"https://patchwork.libcamera.org/api/1.1/patches/16728/?format=json","web_url":"https://patchwork.libcamera.org/patch/16728/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20220721210351.1908-1-laurent.pinchart@ideasonboard.com>","date":"2022-07-21T21:03:51","name":"[libcamera-devel] libcamera: Allow concurrent use of cameras from same pipeline handler","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"0cb175056af51844e956ae0e56453735928037fb","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/16728/mbox/","series":[{"id":3316,"url":"https://patchwork.libcamera.org/api/1.1/series/3316/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3316","date":"2022-07-21T21:03:51","name":"[libcamera-devel] libcamera: Allow concurrent use of cameras from same pipeline handler","version":1,"mbox":"https://patchwork.libcamera.org/series/3316/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/16728/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/16728/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A548DBD1F1\n\tfor ;\n\tThu, 21 Jul 2022 21:03:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 274E96330F;\n\tThu, 21 Jul 2022 23:03:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6F57A601B8\n\tfor ;\n\tThu, 21 Jul 2022 23:03:53 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D9372496;\n\tThu, 21 Jul 2022 23:03:52 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658437435;\n\tbh=XiK4AZ0YODet26M7qUZJ6up+kfd0EPLphcdJuHxwkag=;\n\th=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post:\n\tList-Help:List-Subscribe:From:Reply-To:From;\n\tb=omb1WHnRPtlePSAtOyyCCGFI8VDy7wosSjNolcLGEgZEYI9nZPpELjKlQTc5zFk8A\n\t6s6bL9UbfjGdvnd7TTSdnpgtN19DbYmgK+vWU1SAsPxvMAf7HaVnfzHunGq/sOTNuJ\n\tc2d1bkqKJhMv2ynOf8UGahYPaU9izjZgIdejACcPB6H9Ns0/IWfKwAmIp/MyjBrqR8\n\tDKllvBA9FN6I8lbaMRyLiCBlshgjIx2H+Ola9zi2EDKf/HDgMKT1zyLuOd2X0iROxz\n\tY2gaQxLfY5iOWtk+qOja/bB/3yBBSqxT4xNfHH89Qolou3yyqavuoCQNMtBDkHF3JP\n\t6/YMMxEAHM0xg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658437433;\n\tbh=XiK4AZ0YODet26M7qUZJ6up+kfd0EPLphcdJuHxwkag=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=FwnkIsNsqgW+lMddZ7JN7s6sgMUQMe2W2ZPhqS/0kxqTtsl44C60zt6q53T1t48wu\n\tuJq2Raelimw4amgHflad8NIaJNU5+PD5sG4sEtxbNKfBO2PsrOUU5l0fO6X9UNVsRR\n\tIihwHAyorwvF8K3u09sG/tJ+B0ZHt8Q8SqN+Uf6g="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"FwnkIsNs\"; dkim-atps=neutral","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 22 Jul 2022 00:03:51 +0300","Message-Id":"<20220721210351.1908-1-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.35.1","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH] libcamera: Allow concurrent use of\n\tcameras from same pipeline handler","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"libcamera implements a pipeline handler locking mechanism based on\nadvisory locks on media devices, to prevent concurrent access to cameras\nfrom the same pipeline handler from different processes (this only works\nbetween multiple libcamera instances, as other processes won't use\nadvisory locks on media devices).\n\nA side effect of the implementation prevents multiple cameras created by\nthe same pipeline handler from being used concurrently. Fix this by\nturning the PipelineHandler lock() and unlock() functions into acquire()\nand release(), with a use count to replace the boolean lock flag. The\nCamera class is updated accordingly.\n\nSigned-off-by: Laurent Pinchart \n---\nThis may help addressing the problem reported in \"[RFC PATCH] qcam: Fix\nIPU3 camera switching\".\n---\n include/libcamera/internal/camera.h | 1 +\n include/libcamera/internal/pipeline_handler.h | 8 ++-\n src/libcamera/camera.cpp | 12 +++-\n src/libcamera/pipeline_handler.cpp | 63 ++++++++++++-------\n 4 files changed, 55 insertions(+), 29 deletions(-)\n\n\nbase-commit: d4eee094e684806dbdb54fbe1bc02c9c590aa7f4","diff":"diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h\nindex 597426a6f9d2..38dd94ff8156 100644\n--- a/include/libcamera/internal/camera.h\n+++ b/include/libcamera/internal/camera.h\n@@ -50,6 +50,7 @@ private:\n \t\tCameraRunning,\n \t};\n \n+\tbool isAcquired() const;\n \tbool isRunning() const;\n \tint isAccessAllowed(State state, bool allowDisconnected = false,\n \t\t\t const char *from = __builtin_FUNCTION()) const;\ndiff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\nindex c3e4c2587ecd..20f1cbb07fea 100644\n--- a/include/libcamera/internal/pipeline_handler.h\n+++ b/include/libcamera/internal/pipeline_handler.h\n@@ -45,8 +45,8 @@ public:\n \tMediaDevice *acquireMediaDevice(DeviceEnumerator *enumerator,\n \t\t\t\t\tconst DeviceMatch &dm);\n \n-\tbool lock();\n-\tvoid unlock();\n+\tbool acquire();\n+\tvoid release();\n \n \tvirtual CameraConfiguration *generateConfiguration(Camera *camera,\n \t\tconst StreamRoles &roles) = 0;\n@@ -77,6 +77,8 @@ protected:\n \tCameraManager *manager_;\n \n private:\n+\tvoid unlockMediaDevices();\n+\n \tvoid mediaDeviceDisconnected(MediaDevice *media);\n \tvirtual void disconnect();\n \n@@ -91,7 +93,7 @@ private:\n \tconst char *name_;\n \n \tMutex lock_;\n-\tbool lockOwner_ LIBCAMERA_TSA_GUARDED_BY(lock_); /* *Not* ownership of lock_ */\n+\tunsigned int useCount_ LIBCAMERA_TSA_GUARDED_BY(lock_);\n \n \tfriend class PipelineHandlerFactory;\n };\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 2a8ef60e3862..9ce32db1ac78 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -508,6 +508,11 @@ static const char *const camera_state_names[] = {\n \t\"Running\",\n };\n \n+bool Camera::Private::isAcquired() const\n+{\n+\treturn state_.load(std::memory_order_acquire) == CameraRunning;\n+}\n+\n bool Camera::Private::isRunning() const\n {\n \treturn state_.load(std::memory_order_acquire) == CameraRunning;\n@@ -811,7 +816,7 @@ int Camera::exportFrameBuffers(Stream *stream,\n * not blocking, if the device has already been acquired (by the same or another\n * process) the -EBUSY error code is returned.\n *\n- * Acquiring a camera will limit usage of any other camera(s) provided by the\n+ * Acquiring a camera may limit usage of any other camera(s) provided by the\n * same pipeline handler to the same instance of libcamera. The limit is in\n * effect until all cameras from the pipeline handler are released. Other\n * instances of libcamera can still list and examine the cameras but will fail\n@@ -839,7 +844,7 @@ int Camera::acquire()\n \tif (ret < 0)\n \t\treturn ret == -EACCES ? -EBUSY : ret;\n \n-\tif (!d->pipe_->lock()) {\n+\tif (!d->pipe_->acquire()) {\n \t\tLOG(Camera, Info)\n \t\t\t<< \"Pipeline handler in use by another process\";\n \t\treturn -EBUSY;\n@@ -873,7 +878,8 @@ int Camera::release()\n \tif (ret < 0)\n \t\treturn ret == -EACCES ? -EBUSY : ret;\n \n-\td->pipe_->unlock();\n+\tif (d->isAcquired())\n+\t\td->pipe_->release();\n \n \td->setState(Private::CameraAvailable);\n \ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 675405330f45..7d2d00ef45e6 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -68,7 +68,7 @@ LOG_DEFINE_CATEGORY(Pipeline)\n * respective factories.\n */\n PipelineHandler::PipelineHandler(CameraManager *manager)\n-\t: manager_(manager), lockOwner_(false)\n+\t: manager_(manager), useCount_(0)\n {\n }\n \n@@ -143,58 +143,75 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator,\n }\n \n /**\n- * \\brief Lock all media devices acquired by the pipeline\n+ * \\brief Acquire exclusive access to the pipeline handler for the process\n *\n- * This function shall not be called from pipeline handler implementation, as\n- * the Camera class handles locking directly.\n+ * This function locks all the media devices used by the pipeline to ensure\n+ * that no other process can access them concurrently.\n+ *\n+ * Access to a pipeline handler may be acquired recursively from within the\n+ * same process. Every successful acquire() call shall be matched with a\n+ * release() call. This allows concurrent access to the same pipeline handler\n+ * from different cameras within the same process.\n+ *\n+ * Pipeline handlers shall not call this function directly as the Camera class\n+ * handles access internally.\n *\n * \\context This function is \\threadsafe.\n *\n- * \\return True if the devices could be locked, false otherwise\n- * \\sa unlock()\n- * \\sa MediaDevice::lock()\n+ * \\return True if the pipeline handler was acquired, false if another process\n+ * has already acquired it\n+ * \\sa release()\n */\n-bool PipelineHandler::lock()\n+bool PipelineHandler::acquire()\n {\n \tMutexLocker locker(lock_);\n \n-\t/* Do not allow nested locking in the same libcamera instance. */\n-\tif (lockOwner_)\n-\t\treturn false;\n+\tif (useCount_) {\n+\t\t++useCount_;\n+\t\treturn true;\n+\t}\n \n \tfor (std::shared_ptr &media : mediaDevices_) {\n \t\tif (!media->lock()) {\n-\t\t\tunlock();\n+\t\t\tunlockMediaDevices();\n \t\t\treturn false;\n \t\t}\n \t}\n \n-\tlockOwner_ = true;\n-\n+\t++useCount_;\n \treturn true;\n }\n \n /**\n- * \\brief Unlock all media devices acquired by the pipeline\n+ * \\brief Release exclusive access to the pipeline handler\n *\n- * This function shall not be called from pipeline handler implementation, as\n- * the Camera class handles locking directly.\n+ * This function releases access to the pipeline handler previously acquired by\n+ * a call to acquire(). Every release() call shall match a previous successful\n+ * acquire() call. Calling this function on a pipeline handler that hasn't been\n+ * acquired results in undefined behaviour.\n+ *\n+ * Pipeline handlers shall not call this function directly as the Camera class\n+ * handles access internally.\n *\n * \\context This function is \\threadsafe.\n *\n- * \\sa lock()\n+ * \\sa acquire()\n */\n-void PipelineHandler::unlock()\n+void PipelineHandler::release()\n {\n \tMutexLocker locker(lock_);\n \n-\tif (!lockOwner_)\n-\t\treturn;\n+\tASSERT(useCount_);\n \n+\tunlockMediaDevices();\n+\n+\t--useCount_;\n+}\n+\n+void PipelineHandler::unlockMediaDevices()\n+{\n \tfor (std::shared_ptr &media : mediaDevices_)\n \t\tmedia->unlock();\n-\n-\tlockOwner_ = false;\n }\n \n /**\n","prefixes":["libcamera-devel"]}