[{"id":28861,"web_url":"https://patchwork.libcamera.org/comment/28861/","msgid":"<5d26a4fc-c6c2-4a7f-bae4-629f00faefba@ideasonboard.com>","date":"2024-03-05T11:10:02","subject":"Re: [PATCH 1/5] libcamera: Allow pipeline to provide a Private\n\trequest","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jacopo\n\nOn 21/02/2024 17:40, Jacopo Mondi wrote:\n> In order to allow each pipeline handler to create a Request::Private\n> derived class to store ancillary data associated with a capture request,\n> modify the way a Request is created by the PipelineHandler base class.\n>\n> Introduce a virtual PipelineHandler::createRequestDevice() that pipeline\n> handler implementation can optionally override to provide a custom\n> private data type to initialize the Request with.\n>\n> As we can't anymore create a Request without going through an active\n> camera, drop the queueRequest() checks from the Camera statemachine\n> test.\n>\n> Signed-off-by: Jacopo Mondi <jacopo.mondi@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(-)\n>\n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 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>   \n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index 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_; }\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 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>   /**\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 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\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index 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\n\nI'm sure this is just me not understanding c++ properly; but what's the reason for this function? \nWhy not just use the class constructor?\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.\n> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> index 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)","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 65D2BC326B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Mar 2024 11:10:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8584D61C92;\n\tTue,  5 Mar 2024 12:10:07 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D088561C8D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Mar 2024 12:10:05 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AD4202B3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Mar 2024 12:09:48 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"hEpmKwq2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709636988;\n\tbh=9ns7hdyne5Dx/pg+2SGgf7ehkJ+30gH0nrvH+spyIus=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=hEpmKwq2y06vMbPxaAbzkoslmau6i4BqvJIHrccOXvJUdXHCkKtIrxadSH4TCmwZb\n\tXeEHc5dLqDtZ5zTlvON7CqjCDdtB3fPm1aX/fj/KZbOyDfkGaQz4PGOPCNIg1tL/4p\n\tZTWcUiZ6U/JbLdXdr0TcO0nYosEwvqOL/UwOeV7o=","Message-ID":"<5d26a4fc-c6c2-4a7f-bae4-629f00faefba@ideasonboard.com>","Date":"Tue, 5 Mar 2024 11:10:02 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/5] libcamera: Allow pipeline to provide a Private\n\trequest","Content-Language":"en-US","To":"libcamera-devel@lists.libcamera.org","References":"<20240221174015.52958-1-jacopo.mondi@ideasonboard.com>\n\t<20240221174015.52958-2-jacopo.mondi@ideasonboard.com>","From":"Dan Scally <dan.scally@ideasonboard.com>","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20240221174015.52958-2-jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28864,"web_url":"https://patchwork.libcamera.org/comment/28864/","msgid":"<6mfd7lnokfvovwi7szcysr7kkelnln6il3u5p37uvzuzlxda3t@6a2nwyuyo2gm>","date":"2024-03-05T11:28:14","subject":"Re: [PATCH 1/5] libcamera: Allow pipeline to provide a Private\n\trequest","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Tue, Mar 05, 2024 at 11:10:02AM +0000, Dan Scally wrote:\n> Hi Jacopo\n>\n> On 21/02/2024 17:40, Jacopo Mondi wrote:\n> > In order to allow each pipeline handler to create a Request::Private\n> > derived class to store ancillary data associated with a capture request,\n> > modify the way a Request is created by the PipelineHandler base class.\n> >\n> > Introduce a virtual PipelineHandler::createRequestDevice() that pipeline\n> > handler implementation can optionally override to provide a custom\n> > private data type to initialize the Request with.\n> >\n> > As we can't anymore create a Request without going through an active\n> > camera, drop the queueRequest() checks from the Camera statemachine\n> > test.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo.mondi@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(-)\n> >\n> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > index 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> >   #include \"libcamera/internal/ipa_proxy.h\"\n> > +#include \"libcamera/internal/request.h\"\n> >   namespace libcamera {\n> > @@ -59,7 +60,7 @@ public:\n> >   \tvoid stop(Camera *camera);\n> >   \tbool hasPendingRequests(const Camera *camera) const;\n> > -\tvoid registerRequest(Request *request);\n> > +\tstd::unique_ptr<Request> createRequest(Camera *camera, uint64_t cookie);\n> >   \tvoid queueRequest(Request *request);\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> > +\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> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > index dffde1536cad..e16e61a93873 100644\n> > --- a/include/libcamera/request.h\n> > +++ b/include/libcamera/request.h\n> > @@ -45,9 +45,10 @@ public:\n> >   \tusing BufferMap = std::map<const Stream *, FrameBuffer *>;\n> > -\tRequest(Camera *camera, uint64_t cookie = 0);\n> > +\tRequest(std::unique_ptr<Private> d, uint64_t cookie = 0);\n> >   \t~Request();\n> > +\tstatic std::unique_ptr<Request> create(std::unique_ptr<Private> d, uint64_t);\n> >   \tvoid reuse(ReuseFlag flags = Default);\n> >   \tControlList &controls() { return *controls_; }\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 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> > -\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> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 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> > - * \\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> > @@ -462,6 +467,27 @@ void PipelineHandler::doQueueRequests()\n> >   \t}\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\n> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> > index 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> >    * \\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>\n>\n> I'm sure this is just me not understanding c++ properly; but what's the\n> reason for this function? Why not just use the class constructor?\n>\n\nIt's an helper, to easily create a unique_ptr<> from a Request. I\nthink pipeline handlers can live without it\n\n\n std::unique_ptr<Request> PipelineHandlerRkISP1::createRequestDevice(Camera *camera,\n                                                                    uint64_t cookie)\n {\n-       auto request = std::make_unique<RkISP1Request>(camera);\n-       return Request::create(std::move(request), cookie);\n+       return std::make_unique<Request>(std::make_unique<RkISP1Request>(camera),\n+                                        cookie);\n }\n\nIt just feels nicer\n\nAlso note that applications won't be able to use this function or neither the\nconstructor has they don't have access to internal headers so they\ncan't provide a Request::Private instance.\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> >   \t/**\n> >   \t * \\todo Add a validator for metadata controls.\n> > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> > index 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> > -\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> > -\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> > -\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)","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 39CE0C326B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Mar 2024 11:28:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 86AA262871;\n\tTue,  5 Mar 2024 12:28:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C56961C8D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Mar 2024 12:28:17 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8618CCC8;\n\tTue,  5 Mar 2024 12:28:00 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"n5gdHqoX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709638080;\n\tbh=tXETcuSbza1ik7jAUqlgTDgTBxfBlf5cS9Sdzg99F9I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=n5gdHqoXRPhBXTmiJFn/7NJTK2rollMH4IggJ4zRGLls6rWNy/5iIiG1Ys8iSko9Z\n\tpGeDOa+t6jYvW9uyzI96csIto0c8Tel1GG39FQ1GbVWT/mNKvrb3QOfF+VjVQ9sd6q\n\tyUfmneouF9xJ7yWG0Nbkn1wg77WJEINbATOL/HE0=","Date":"Tue, 5 Mar 2024 12:28:14 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Dan Scally <dan.scally@ideasonboard.com>","Subject":"Re: [PATCH 1/5] libcamera: Allow pipeline to provide a Private\n\trequest","Message-ID":"<6mfd7lnokfvovwi7szcysr7kkelnln6il3u5p37uvzuzlxda3t@6a2nwyuyo2gm>","References":"<20240221174015.52958-1-jacopo.mondi@ideasonboard.com>\n\t<20240221174015.52958-2-jacopo.mondi@ideasonboard.com>\n\t<5d26a4fc-c6c2-4a7f-bae4-629f00faefba@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<5d26a4fc-c6c2-4a7f-bae4-629f00faefba@ideasonboard.com>","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":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28865,"web_url":"https://patchwork.libcamera.org/comment/28865/","msgid":"<58693ed3-1a39-49b4-b91a-ab69693ec0df@ideasonboard.com>","date":"2024-03-05T11:40:31","subject":"Re: [PATCH 1/5] libcamera: Allow pipeline to provide a Private\n\trequest","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jacopo\n\nOn 05/03/2024 11:28, Jacopo Mondi wrote:\n> Hi Dan\n>\n> On Tue, Mar 05, 2024 at 11:10:02AM +0000, Dan Scally wrote:\n>> Hi Jacopo\n>>\n>> On 21/02/2024 17:40, Jacopo Mondi wrote:\n>>> In order to allow each pipeline handler to create a Request::Private\n>>> derived class to store ancillary data associated with a capture request,\n>>> modify the way a Request is created by the PipelineHandler base class.\n>>>\n>>> Introduce a virtual PipelineHandler::createRequestDevice() that pipeline\n>>> handler implementation can optionally override to provide a custom\n>>> private data type to initialize the Request with.\n>>>\n>>> As we can't anymore create a Request without going through an active\n>>> camera, drop the queueRequest() checks from the Camera statemachine\n>>> test.\n>>>\n>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@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(-)\n>>>\n>>> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n>>> index 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>>>    #include \"libcamera/internal/ipa_proxy.h\"\n>>> +#include \"libcamera/internal/request.h\"\n>>>    namespace libcamera {\n>>> @@ -59,7 +60,7 @@ public:\n>>>    \tvoid stop(Camera *camera);\n>>>    \tbool hasPendingRequests(const Camera *camera) const;\n>>> -\tvoid registerRequest(Request *request);\n>>> +\tstd::unique_ptr<Request> createRequest(Camera *camera, uint64_t cookie);\n>>>    \tvoid queueRequest(Request *request);\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>>> +\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>>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n>>> index dffde1536cad..e16e61a93873 100644\n>>> --- a/include/libcamera/request.h\n>>> +++ b/include/libcamera/request.h\n>>> @@ -45,9 +45,10 @@ public:\n>>>    \tusing BufferMap = std::map<const Stream *, FrameBuffer *>;\n>>> -\tRequest(Camera *camera, uint64_t cookie = 0);\n>>> +\tRequest(std::unique_ptr<Private> d, uint64_t cookie = 0);\n>>>    \t~Request();\n>>> +\tstatic std::unique_ptr<Request> create(std::unique_ptr<Private> d, uint64_t);\n>>>    \tvoid reuse(ReuseFlag flags = Default);\n>>>    \tControlList &controls() { return *controls_; }\n>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>>> index 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>>> -\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>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>>> index 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>>> - * \\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>>> @@ -462,6 +467,27 @@ void PipelineHandler::doQueueRequests()\n>>>    \t}\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\n>>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n>>> index 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>>>     * \\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>>\n>> I'm sure this is just me not understanding c++ properly; but what's the\n>> reason for this function? Why not just use the class constructor?\n>>\n> It's an helper, to easily create a unique_ptr<> from a Request. I\n> think pipeline handlers can live without it\n>\n>\n>   std::unique_ptr<Request> PipelineHandlerRkISP1::createRequestDevice(Camera *camera,\n>                                                                      uint64_t cookie)\n>   {\n> -       auto request = std::make_unique<RkISP1Request>(camera);\n> -       return Request::create(std::move(request), cookie);\n> +       return std::make_unique<Request>(std::make_unique<RkISP1Request>(camera),\n> +                                        cookie);\n>   }\nYeah that's what I was expecting\n> It just feels nicer\n\nFair enough, I was worried I wasn't understanding something! Anyway; I think the patch is good:\n\n\nReviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n\nand\n\nTested-by: Daniel Scally <dan.scally@ideasonboard.com>\n\n> Also note that applications won't be able to use this function or neither the\n> constructor has they don't have access to internal headers so they\n> can't provide a Request::Private instance.\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>>>    \t/**\n>>>    \t * \\todo Add a validator for metadata controls.\n>>> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n>>> index 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>>> -\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>>> -\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>>> -\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)","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 0A442BD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Mar 2024 11:40:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 406FF61C92;\n\tTue,  5 Mar 2024 12:40:36 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 35EF461C8D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Mar 2024 12:40:34 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 38D7722A;\n\tTue,  5 Mar 2024 12:40:17 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fQnN+2UB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709638817;\n\tbh=qfxZoDrUspjLzdPE9AhFAbzmyh2qjREJ8xpA09KorCs=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=fQnN+2UB3c6eIGkg3O9/Uv81mzPnuhTMw879n9XHZ8uA2W5Dq5nm7ZI8jcg+GULuy\n\tzzEo246LCrQ8eEvoFldc0MI8FSiq/9fzrq83iQ1VKytobPB7vnDKE2puRpm1Y3FtHT\n\tBHvWm+8IpbVJM3vm64MY7W0B9pzBc4nJXPv8sVkU=","Message-ID":"<58693ed3-1a39-49b4-b91a-ab69693ec0df@ideasonboard.com>","Date":"Tue, 5 Mar 2024 11:40:31 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/5] libcamera: Allow pipeline to provide a Private\n\trequest","Content-Language":"en-US","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","References":"<20240221174015.52958-1-jacopo.mondi@ideasonboard.com>\n\t<20240221174015.52958-2-jacopo.mondi@ideasonboard.com>\n\t<5d26a4fc-c6c2-4a7f-bae4-629f00faefba@ideasonboard.com>\n\t<6mfd7lnokfvovwi7szcysr7kkelnln6il3u5p37uvzuzlxda3t@6a2nwyuyo2gm>","From":"Dan Scally <dan.scally@ideasonboard.com>","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<6mfd7lnokfvovwi7szcysr7kkelnln6il3u5p37uvzuzlxda3t@6a2nwyuyo2gm>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28880,"web_url":"https://patchwork.libcamera.org/comment/28880/","msgid":"<20240306154203.mpbp2xyudas5iy4x@jasper>","date":"2024-03-06T15:42:03","subject":"Re: [PATCH 1/5] libcamera: Allow pipeline to provide a Private\n\trequest","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Jacopo,\n\nmaybe it would be nice to mention in the commit message, that this breaks the ABI.\n(I know we don't really track this, but hey :-)\n\nOn Tue, Mar 05, 2024 at 12:28:14PM +0100, Jacopo Mondi wrote:\n> Hi Dan\n> \n> On Tue, Mar 05, 2024 at 11:10:02AM +0000, Dan Scally wrote:\n> > Hi Jacopo\n> >\n> > On 21/02/2024 17:40, Jacopo Mondi wrote:\n> > > In order to allow each pipeline handler to create a Request::Private\n> > > derived class to store ancillary data associated with a capture request,\n> > > modify the way a Request is created by the PipelineHandler base class.\n> > >\n> > > Introduce a virtual PipelineHandler::createRequestDevice() that pipeline\n> > > handler implementation can optionally override to provide a custom\n> > > private data type to initialize the Request with.\n> > >\n> > > As we can't anymore create a Request without going through an active\n> > > camera, drop the queueRequest() checks from the Camera statemachine\n> > > test.\n> > >\n> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@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(-)\n> > >\n> > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > > index 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> > >   #include \"libcamera/internal/ipa_proxy.h\"\n> > > +#include \"libcamera/internal/request.h\"\n> > >   namespace libcamera {\n> > > @@ -59,7 +60,7 @@ public:\n> > >   \tvoid stop(Camera *camera);\n> > >   \tbool hasPendingRequests(const Camera *camera) const;\n> > > -\tvoid registerRequest(Request *request);\n> > > +\tstd::unique_ptr<Request> createRequest(Camera *camera, uint64_t cookie);\n> > >   \tvoid queueRequest(Request *request);\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> > > +\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> > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > > index dffde1536cad..e16e61a93873 100644\n> > > --- a/include/libcamera/request.h\n> > > +++ b/include/libcamera/request.h\n> > > @@ -45,9 +45,10 @@ public:\n> > >   \tusing BufferMap = std::map<const Stream *, FrameBuffer *>;\n> > > -\tRequest(Camera *camera, uint64_t cookie = 0);\n> > > +\tRequest(std::unique_ptr<Private> d, uint64_t cookie = 0);\n> > >   \t~Request();\n> > > +\tstatic std::unique_ptr<Request> create(std::unique_ptr<Private> d, uint64_t);\n> > >   \tvoid reuse(ReuseFlag flags = Default);\n> > >   \tControlList &controls() { return *controls_; }\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index 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> > > -\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> > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > index 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> > > - * \\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> > > @@ -462,6 +467,27 @@ void PipelineHandler::doQueueRequests()\n> > >   \t}\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\n> > > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> > > index 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> > >    * \\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> >\n> >\n> > I'm sure this is just me not understanding c++ properly; but what's the\n> > reason for this function? Why not just use the class constructor?\n> >\n> \n> It's an helper, to easily create a unique_ptr<> from a Request. I\n> think pipeline handlers can live without it\n> \n> \n>  std::unique_ptr<Request> PipelineHandlerRkISP1::createRequestDevice(Camera *camera,\n>                                                                     uint64_t cookie)\n>  {\n> -       auto request = std::make_unique<RkISP1Request>(camera);\n> -       return Request::create(std::move(request), cookie);\n> +       return std::make_unique<Request>(std::make_unique<RkISP1Request>(camera),\n> +                                        cookie);\n>  }\n> \n> It just feels nicer\n> \n> Also note that applications won't be able to use this function or neither the\n> constructor has they don't have access to internal headers so they\n> can't provide a Request::Private instance.\n\nAs this is on the public API and not much hassle to do manually in the\nPipelineHandler I suggest to remove that function. In the docs for the \nconstructor we could add a note, that Requests can not be constructed \ndirectly but only through a camera.\n\nWith or without that change:\n\nReviewed-by: Stefan Klug<stefan.klug@ideasonboard.com>\n\nCheers,\nStefan\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> > >   \t/**\n> > >   \t * \\todo Add a validator for metadata controls.\n> > > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> > > index 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> > > -\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> > > -\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> > > -\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)","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 4A49EBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  6 Mar 2024 15:42:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9B9D662867;\n\tWed,  6 Mar 2024 16:42:08 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F52E61C8F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  6 Mar 2024 16:42:06 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:dd65:a3ea:5f4d:989a])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9C8A6552;\n\tWed,  6 Mar 2024 16:41:48 +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=\"ducZYf15\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709739708;\n\tbh=RsOGvPJcTQ7JD+Sh0CvctFYq3e6EPMQFx6wSXUM+0x0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ducZYf153EXrl3dy19GUsKrT+zq7h56Lj5ZKcAxXoGinzOU9BB7zU9RsMkRYw8bue\n\t9L0Ewe27TZ1FeDJckcKlkOVRzSuR0U0NeKAwDqpGUnl9ZfAXQgUshnHZ5hdoPXIeux\n\tL65MAZJFt61SMdWJT4+mvT2vTRQ/xveBEC8KY3Hk=","Date":"Wed, 6 Mar 2024 16:42:03 +0100","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Subject":"Re: [PATCH 1/5] libcamera: Allow pipeline to provide a Private\n\trequest","Message-ID":"<20240306154203.mpbp2xyudas5iy4x@jasper>","References":"<20240221174015.52958-1-jacopo.mondi@ideasonboard.com>\n\t<20240221174015.52958-2-jacopo.mondi@ideasonboard.com>\n\t<5d26a4fc-c6c2-4a7f-bae4-629f00faefba@ideasonboard.com>\n\t<6mfd7lnokfvovwi7szcysr7kkelnln6il3u5p37uvzuzlxda3t@6a2nwyuyo2gm>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<6mfd7lnokfvovwi7szcysr7kkelnln6il3u5p37uvzuzlxda3t@6a2nwyuyo2gm>","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":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28883,"web_url":"https://patchwork.libcamera.org/comment/28883/","msgid":"<cn25lk76m6yv5364cua473r6ujl5u77jepxe2dqvc5nw3ebohq@hwfrrhnijye6>","date":"2024-03-06T16:05:02","subject":"Re: [PATCH 1/5] libcamera: Allow pipeline to provide a Private\n\trequest","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n  thanks for review\n\nOn Wed, Mar 06, 2024 at 04:42:03PM +0100, Stefan Klug wrote:\n> Hi Jacopo,\n>\n> maybe it would be nice to mention in the commit message, that this breaks the ABI.\n> (I know we don't really track this, but hey :-)\n\nIndeed this changes the public Request interface. Applications were not\nsupposed to create a Request directly (however it seems to me it was\npossible, unfortunately) but they were meant to go through\nCamera::createRequest() and this series doesn't change that.\n\n>\n> On Tue, Mar 05, 2024 at 12:28:14PM +0100, Jacopo Mondi wrote:\n> > Hi Dan\n> >\n> > On Tue, Mar 05, 2024 at 11:10:02AM +0000, Dan Scally wrote:\n> > > Hi Jacopo\n> > >\n> > > On 21/02/2024 17:40, Jacopo Mondi wrote:\n> > > > In order to allow each pipeline handler to create a Request::Private\n> > > > derived class to store ancillary data associated with a capture request,\n> > > > modify the way a Request is created by the PipelineHandler base class.\n> > > >\n> > > > Introduce a virtual PipelineHandler::createRequestDevice() that pipeline\n> > > > handler implementation can optionally override to provide a custom\n> > > > private data type to initialize the Request with.\n> > > >\n> > > > As we can't anymore create a Request without going through an active\n> > > > camera, drop the queueRequest() checks from the Camera statemachine\n> > > > test.\n> > > >\n> > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@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(-)\n> > > >\n> > > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > > > index 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> > > >   #include \"libcamera/internal/ipa_proxy.h\"\n> > > > +#include \"libcamera/internal/request.h\"\n> > > >   namespace libcamera {\n> > > > @@ -59,7 +60,7 @@ public:\n> > > >   \tvoid stop(Camera *camera);\n> > > >   \tbool hasPendingRequests(const Camera *camera) const;\n> > > > -\tvoid registerRequest(Request *request);\n> > > > +\tstd::unique_ptr<Request> createRequest(Camera *camera, uint64_t cookie);\n> > > >   \tvoid queueRequest(Request *request);\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> > > > +\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> > > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > > > index dffde1536cad..e16e61a93873 100644\n> > > > --- a/include/libcamera/request.h\n> > > > +++ b/include/libcamera/request.h\n> > > > @@ -45,9 +45,10 @@ public:\n> > > >   \tusing BufferMap = std::map<const Stream *, FrameBuffer *>;\n> > > > -\tRequest(Camera *camera, uint64_t cookie = 0);\n> > > > +\tRequest(std::unique_ptr<Private> d, uint64_t cookie = 0);\n> > > >   \t~Request();\n> > > > +\tstatic std::unique_ptr<Request> create(std::unique_ptr<Private> d, uint64_t);\n> > > >   \tvoid reuse(ReuseFlag flags = Default);\n> > > >   \tControlList &controls() { return *controls_; }\n> > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > > index 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> > > > -\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> > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > > index 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> > > > - * \\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> > > > @@ -462,6 +467,27 @@ void PipelineHandler::doQueueRequests()\n> > > >   \t}\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\n> > > > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> > > > index 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> > > >    * \\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> > >\n> > >\n> > > I'm sure this is just me not understanding c++ properly; but what's the\n> > > reason for this function? Why not just use the class constructor?\n> > >\n> >\n> > It's an helper, to easily create a unique_ptr<> from a Request. I\n> > think pipeline handlers can live without it\n> >\n> >\n> >  std::unique_ptr<Request> PipelineHandlerRkISP1::createRequestDevice(Camera *camera,\n> >                                                                     uint64_t cookie)\n> >  {\n> > -       auto request = std::make_unique<RkISP1Request>(camera);\n> > -       return Request::create(std::move(request), cookie);\n> > +       return std::make_unique<Request>(std::make_unique<RkISP1Request>(camera),\n> > +                                        cookie);\n> >  }\n> >\n> > It just feels nicer\n> >\n> > Also note that applications won't be able to use this function or neither the\n> > constructor has they don't have access to internal headers so they\n> > can't provide a Request::Private instance.\n>\n> As this is on the public API and not much hassle to do manually in the\n> PipelineHandler I suggest to remove that function. In the docs for the\n> constructor we could add a note, that Requests can not be constructed\n> directly but only through a camera.\n>\n\nPlease not that the existing Request constructor cannot be unsed\nanymore by applications (and as said, they shouldn't have had to in\nfirst place) because they cannot provide a Request::Private instance\nas it's not part of public headers.\n\nWhen it comes to the interface between the Camera and the\nPipelineHandler (this is were the new PipelineHandler::createRequest()\nis introduced in place of PipelineHandler::registerRequest()) it's\npurely internal to the library.\n\n> With or without that change:\n\nIf most people is bothered by this I can indeed remove it. Please note\nit's the same pattern implemented by the Camera::create().\n\n\n>\n> Reviewed-by: Stefan Klug<stefan.klug@ideasonboard.com>\n>\n> Cheers,\n> Stefan\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> > > >   \t/**\n> > > >   \t * \\todo Add a validator for metadata controls.\n> > > > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> > > > index 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> > > > -\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> > > > -\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> > > > -\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)","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 77D0FBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  6 Mar 2024 16:05:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D782462867;\n\tWed,  6 Mar 2024 17:05:06 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4660D61C8F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  6 Mar 2024 17:05:05 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5E2FEBD1;\n\tWed,  6 Mar 2024 17:04:47 +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=\"S3i7nrTE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709741087;\n\tbh=yzHLB8B6tv+y5btpzwP8MX6Q5bPZw7/lk1jyH3Mat+E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=S3i7nrTEBmAcqB690TYKBVchmYdxhaY+JDTK6ZgbohfiuSfDIyflJZdqBNiBNEdVG\n\tv9CT1IUQBxyDhkb2AtAQ7jJQgMDRIwQkcTG+F9KHuELEDHt1EOVJ5QY598+u94BCU+\n\tZkbFem9acp3E7LW/rcCyOJnv28b0IPy7D/pK+FvY=","Date":"Wed, 6 Mar 2024 17:05:02 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH 1/5] libcamera: Allow pipeline to provide a Private\n\trequest","Message-ID":"<cn25lk76m6yv5364cua473r6ujl5u77jepxe2dqvc5nw3ebohq@hwfrrhnijye6>","References":"<20240221174015.52958-1-jacopo.mondi@ideasonboard.com>\n\t<20240221174015.52958-2-jacopo.mondi@ideasonboard.com>\n\t<5d26a4fc-c6c2-4a7f-bae4-629f00faefba@ideasonboard.com>\n\t<6mfd7lnokfvovwi7szcysr7kkelnln6il3u5p37uvzuzlxda3t@6a2nwyuyo2gm>\n\t<20240306154203.mpbp2xyudas5iy4x@jasper>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240306154203.mpbp2xyudas5iy4x@jasper>","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>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]