Patch Detail
Show a patch.
GET /api/1.1/patches/19657/?format=api
{ "id": 19657, "url": "https://patchwork.libcamera.org/api/1.1/patches/19657/?format=api", "web_url": "https://patchwork.libcamera.org/patch/19657/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "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/1.1/people/143/?format=api", "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/1.1/series/4211/?format=api", "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": "<libcamera-devel-bounces@lists.libcamera.org>", "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 <parsemail@patchwork.libcamera.org>;\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 <libcamera-devel@lists.libcamera.org>;\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 <jacopo.mondi@ideasonboard.com>", "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": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Cc": "Jacopo Mondi <jacopo.mondi@ideasonboard.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "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 <jacopo.mondi@ideasonboard.com>\nReviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>\nReviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\nTested-by: Daniel Scally <dan.scally@ideasonboard.com>\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 <libcamera/stream.h>\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<Request> 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> camera);\n \tvoid hotplugMediaDevice(MediaDevice *media);\n \n+\tvirtual std::unique_ptr<Request> 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<const Stream *, FrameBuffer *>;\n \n-\tRequest(Camera *camera, uint64_t cookie = 0);\n+\tRequest(std::unique_ptr<Private> d, uint64_t cookie = 0);\n \t~Request();\n \n+\tstatic std::unique_ptr<Request> create(std::unique_ptr<Private> 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<Request> Camera::createRequest(uint64_t cookie)\n \tif (ret < 0)\n \t\treturn nullptr;\n \n-\tstd::unique_ptr<Request> request = std::make_unique<Request>(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<Request> PipelineHandler::createRequest(Camera *camera, uint64_t cookie)\n {\n+\tstd::unique_ptr<Request> 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<Request>\n+PipelineHandler::createRequestDevice(Camera *camera, uint64_t cookie)\n+{\n+\tauto d = std::make_unique<Request::Private>(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<Private>(camera)),\n-\t cookie_(cookie), status_(RequestPending)\n+std::unique_ptr<Request> Request::create(std::unique_ptr<Private> d,\n+\t\t\t\t\t uint64_t cookie)\n+{\n+\treturn std::make_unique<Request>(std::move(d), cookie);\n+}\n+\n+Request::Request(std::unique_ptr<Request::Private> 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<Request> request2 = camera_->createRequest();\n \t\tif (!request2)\n", "prefixes": [ "v2", "1/4" ] }