{"id":19657,"url":"https://patchwork.libcamera.org/api/patches/19657/?format=json","web_url":"https://patchwork.libcamera.org/patch/19657/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/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":"<20240311123234.32925-2-jacopo.mondi@ideasonboard.com>","date":"2024-03-11T12:32:29","name":"[v2,1/4] libcamera: Allow pipeline to provide a Private request","commit_ref":null,"pull_url":null,"state":"not-applicable","archived":false,"hash":"53ef879cafdf7e1e5ee182bd6b15e3d6df622d21","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/?format=json","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/19657/mbox/","series":[{"id":4211,"url":"https://patchwork.libcamera.org/api/series/4211/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4211","date":"2024-03-11T12:32:28","name":"libcamera: Replace IPU3/RkISP1FrameInfo","version":2,"mbox":"https://patchwork.libcamera.org/series/4211/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19657/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19657/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 9565CBD1F1\n\tfor ;\n\tMon, 11 Mar 2024 12:32:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84F7962C84;\n\tMon, 11 Mar 2024 13:32:47 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CA9FF6286F\n\tfor ;\n\tMon, 11 Mar 2024 13:32:44 +0100 (CET)","from localhost.localdomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7362AF0A;\n\tMon, 11 Mar 2024 13:32:23 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BejItgeX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1710160343;\n\tbh=fl3hRCbieAa2K1342VeSQlfCx4iM3xTYClxBJBCMfq0=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=BejItgeX/6W0Wl062CJDnqunQiOMfG9hLYWPyjGzoKd8g7GQPZY+NUGMwR0BcJ3sr\n\tPyJXejU5Iww12ziy0xhp/9ESJbYeGiwv3CRBkZsxN6STSc/QLMI6hRYIkDMexCYRS8\n\tI3O0wznHcfppzLLXu5m+kCTnChxHhicvAw017luc=","From":"Jacopo Mondi ","To":"libcamera-devel@lists.libcamera.org","Subject":"[PATCH v2 1/4] libcamera: Allow pipeline to provide a Private\n\trequest","Date":"Mon, 11 Mar 2024 13:32:29 +0100","Message-ID":"<20240311123234.32925-2-jacopo.mondi@ideasonboard.com>","X-Mailer":"git-send-email 2.43.2","In-Reply-To":"<20240311123234.32925-1-jacopo.mondi@ideasonboard.com>","References":"<20240311123234.32925-1-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","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","Cc":"Jacopo Mondi ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"In order to allow each pipeline handler to create a Request::Private\nderived class to store ancillary data associated with a capture request,\nmodify the way a Request is created by the PipelineHandler base class.\n\nIntroduce a virtual PipelineHandler::createRequestDevice() that pipeline\nhandler implementation can optionally override to provide a custom\nprivate data type to initialize the Request with and make the existing\nRequest() public constructor not usable by applications.\n\nAs we can't anymore create a Request without going through an active\ncamera, drop the queueRequest() checks from the Camera statemachine\ntest.\n\nSigned-off-by: Jacopo Mondi \nReviewed-by: Stefan Klug \nReviewed-by: Daniel Scally \nTested-by: Daniel Scally \n---\n include/libcamera/internal/pipeline_handler.h | 5 ++-\n include/libcamera/request.h | 3 +-\n src/libcamera/camera.cpp | 8 +---\n src/libcamera/pipeline_handler.cpp | 38 ++++++++++++++++---\n src/libcamera/request.cpp | 15 +++++---\n test/camera/statemachine.cpp | 12 ------\n 6 files changed, 50 insertions(+), 31 deletions(-)","diff":"diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\nindex c96944f4ecc4..bbe74f22e5ae 100644\n--- a/include/libcamera/internal/pipeline_handler.h\n+++ b/include/libcamera/internal/pipeline_handler.h\n@@ -21,6 +21,7 @@\n #include \n \n #include \"libcamera/internal/ipa_proxy.h\"\n+#include \"libcamera/internal/request.h\"\n \n namespace libcamera {\n \n@@ -59,7 +60,7 @@ public:\n \tvoid stop(Camera *camera);\n \tbool hasPendingRequests(const Camera *camera) const;\n \n-\tvoid registerRequest(Request *request);\n+\tstd::unique_ptr createRequest(Camera *camera, uint64_t cookie);\n \tvoid queueRequest(Request *request);\n \n \tbool completeBuffer(Request *request, FrameBuffer *buffer);\n@@ -74,6 +75,8 @@ protected:\n \tvoid registerCamera(std::shared_ptr camera);\n \tvoid hotplugMediaDevice(MediaDevice *media);\n \n+\tvirtual std::unique_ptr createRequestDevice(Camera *camera,\n+\t\t\t\t\t\t\t uint64_t cookie);\n \tvirtual int queueRequestDevice(Camera *camera, Request *request) = 0;\n \tvirtual void stopDevice(Camera *camera) = 0;\n \ndiff --git a/include/libcamera/request.h b/include/libcamera/request.h\nindex dffde1536cad..e16e61a93873 100644\n--- a/include/libcamera/request.h\n+++ b/include/libcamera/request.h\n@@ -45,9 +45,10 @@ public:\n \n \tusing BufferMap = std::map;\n \n-\tRequest(Camera *camera, uint64_t cookie = 0);\n+\tRequest(std::unique_ptr d, uint64_t cookie = 0);\n \t~Request();\n \n+\tstatic std::unique_ptr create(std::unique_ptr d, uint64_t);\n \tvoid reuse(ReuseFlag flags = Default);\n \n \tControlList &controls() { return *controls_; }\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex a71dc933b911..b7053576fe42 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -1236,12 +1236,8 @@ std::unique_ptr Camera::createRequest(uint64_t cookie)\n \tif (ret < 0)\n \t\treturn nullptr;\n \n-\tstd::unique_ptr request = std::make_unique(this, cookie);\n-\n-\t/* Associate the request with the pipeline handler. */\n-\td->pipe_->registerRequest(request.get());\n-\n-\treturn request;\n+\t/* Create a Request from the pipeline handler. */\n+\treturn d->pipe_->createRequest(this, cookie);\n }\n \n /**\ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 29e0c98a6db5..f2a8cdac0408 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -371,20 +371,25 @@ bool PipelineHandler::hasPendingRequests(const Camera *camera) const\n }\n \n /**\n- * \\fn PipelineHandler::registerRequest()\n- * \\brief Register a request for use by the pipeline handler\n- * \\param[in] request The request to register\n+ * \\fn PipelineHandler::createRequest()\n+ * \\brief Create a request and register it for use by the pipeline handler\n+ * \\param[in] camera The camera the Request belongs to\n+ * \\param[in] cookie The Request unique identifier\n *\n- * This function is called when the request is created, and allows the pipeline\n- * handler to perform any one-time initialization it requries for the request.\n+ * This function is called to create a Request by calling createRequestDevice()\n+ * which can be optionally provided by the PipelineHandler derived classes.\n */\n-void PipelineHandler::registerRequest(Request *request)\n+std::unique_ptr PipelineHandler::createRequest(Camera *camera, uint64_t cookie)\n {\n+\tstd::unique_ptr request = createRequestDevice(camera, cookie);\n+\n \t/*\n \t * Connect the request prepared signal to notify the pipeline handler\n \t * when a request is ready to be processed.\n \t */\n \trequest->_d()->prepared.connect(this, &PipelineHandler::doQueueRequests);\n+\n+\treturn request;\n }\n \n /**\n@@ -462,6 +467,27 @@ void PipelineHandler::doQueueRequests()\n \t}\n }\n \n+/**\n+ * \\brief Create a Request from the pipeline handler\n+ * \\param[in] camera The camera the Request belongs to\n+ * \\param[in] cookie The Request unique identifier\n+ *\n+ * A virtual function that PipelineHandler derived classes are free to override\n+ * in order to initialize a Request with a custom Request::Private derived\n+ * class.\n+ *\n+ * This is the base class implementation that use Request::Private to\n+ * initialize the Request.\n+ *\n+ * \\return A unique pointer to a newly created Request\n+ */\n+std::unique_ptr\n+PipelineHandler::createRequestDevice(Camera *camera, uint64_t cookie)\n+{\n+\tauto d = std::make_unique(camera);\n+\treturn Request::create(std::move(d), cookie);\n+}\n+\n /**\n * \\fn PipelineHandler::queueRequestDevice()\n * \\brief Queue a request to the device\ndiff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\nindex 949c556fa437..d1051ad3d25e 100644\n--- a/src/libcamera/request.cpp\n+++ b/src/libcamera/request.cpp\n@@ -336,7 +336,7 @@ void Request::Private::timeout()\n \n /**\n * \\brief Create a capture request for a camera\n- * \\param[in] camera The camera that creates the request\n+ * \\param[in] d The request private data\n * \\param[in] cookie Opaque cookie for application use\n *\n * The \\a cookie is stored in the request and is accessible through the\n@@ -344,12 +344,17 @@ void Request::Private::timeout()\n * the request to an external resource in the request completion handler, and is\n * completely opaque to libcamera.\n */\n-Request::Request(Camera *camera, uint64_t cookie)\n-\t: Extensible(std::make_unique(camera)),\n-\t cookie_(cookie), status_(RequestPending)\n+std::unique_ptr Request::create(std::unique_ptr d,\n+\t\t\t\t\t uint64_t cookie)\n+{\n+\treturn std::make_unique(std::move(d), cookie);\n+}\n+\n+Request::Request(std::unique_ptr d, uint64_t cookie)\n+\t: Extensible(std::move(d)), cookie_(cookie), status_(RequestPending)\n {\n \tcontrols_ = new ControlList(controls::controls,\n-\t\t\t\t camera->_d()->validator());\n+\t\t\t\t _d()->camera()->_d()->validator());\n \n \t/**\n \t * \\todo Add a validator for metadata controls.\ndiff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\nindex 9c2b0c6a7d99..5714061f88b5 100644\n--- a/test/camera/statemachine.cpp\n+++ b/test/camera/statemachine.cpp\n@@ -38,10 +38,6 @@ protected:\n \t\tif (camera_->start() != -EACCES)\n \t\t\treturn TestFail;\n \n-\t\tRequest request(camera_.get());\n-\t\tif (camera_->queueRequest(&request) != -EACCES)\n-\t\t\treturn TestFail;\n-\n \t\t/* Test operations which should pass. */\n \t\tif (camera_->release())\n \t\t\treturn TestFail;\n@@ -68,10 +64,6 @@ protected:\n \t\tif (camera_->start() != -EACCES)\n \t\t\treturn TestFail;\n \n-\t\tRequest request(camera_.get());\n-\t\tif (camera_->queueRequest(&request) != -EACCES)\n-\t\t\treturn TestFail;\n-\n \t\t/* Test operations which should pass. */\n \t\tif (camera_->stop())\n \t\t\treturn TestFail;\n@@ -95,10 +87,6 @@ protected:\n \t\tif (camera_->acquire() != -EBUSY)\n \t\t\treturn TestFail;\n \n-\t\tRequest request1(camera_.get());\n-\t\tif (camera_->queueRequest(&request1) != -EACCES)\n-\t\t\treturn TestFail;\n-\n \t\t/* Test operations which should pass. */\n \t\tstd::unique_ptr request2 = camera_->createRequest();\n \t\tif (!request2)\n","prefixes":["v2","1/4"]}