[{"id":12378,"web_url":"https://patchwork.libcamera.org/comment/12378/","msgid":"<095af0f2-9115-6cb1-892e-1418f004fbe2@ideasonboard.com>","date":"2020-09-08T12:28:07","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 08/09/2020 08:54, Laurent Pinchart wrote:\n> In most cases the last reference to a Camera instance will be the one\n> held by the CameraManager. That reference gets released when the\n> CameraManager thread cleans up, just before it stops. There's no need to\n> delete the camera with deleteLater() in that case.\n> \n> To optimize this case, use deleteLater() only when the camera gets\n> deleted from a different thread, and delete is synchronously otherwise.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> Umang, would you be able to test this patch with the Android HAL ?\n> \n> src/libcamera/camera.cpp | 6 +++++-\n> 1 file changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 4a9c19c33cfb..ae16a64a3b44 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -16,6 +16,7 @@\n> \n> #include \"libcamera/internal/log.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> +#include \"libcamera/internal/thread.h\"\n> #include \"libcamera/internal/utils.h\"\n> \n> /**\n> @@ -470,7 +471,10 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> \tstruct Deleter : std::default_delete {\n> \t\tvoid operator()(Camera *camera)\n> \t\t{\n> -\t\t\tcamera->deleteLater();\n> +\t\t\tif (Thread::current() == camera->thread())\n> +\t\t\t\tdelete camera;\n> +\t\t\telse\n> +\t\t\t\tcamera->deleteLater();\n\nShouldn't/Couldn't this logic be handled in Object::deleteLater() directly?\n\n> \t\t}\n> \t};\n> \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7D784BDB1C\n\tfor ;\n\tTue, 8 Sep 2020 12:28:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E3B2662C3B;\n\tTue, 8 Sep 2020 14:28:11 +0200 (CEST)","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 6F19562C30\n\tfor ;\n\tTue, 8 Sep 2020 14:28:11 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E36FD39;\n\tTue, 8 Sep 2020 14:28:10 +0200 (CEST)"],"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=\"imfDZyoM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599568091;\n\tbh=0NxVXl8uaCN+FeafmcZ7iUSBG4qXlFciMKtJRINjVN0=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=imfDZyoM8cCPs23LSpf66B565zvx3Jyd4NSCsFQFkPxZOC3PO5sj6sTf3erHvlr9O\n\tC5v0AHx1XNOA1r7l7NKr1q88rdMIdZmujzISo/0NnTd9x6jfcgS5t/6E8atFwVIJPV\n\tqk005c56qbWf3FUvO6vx6AIEx/LkNLSzSHuN5oQ0=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200908075406.14039-1-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<095af0f2-9115-6cb1-892e-1418f004fbe2@ideasonboard.com>","Date":"Tue, 8 Sep 2020 13:28:07 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200908075406.14039-1-laurent.pinchart@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12379,"web_url":"https://patchwork.libcamera.org/comment/12379/","msgid":"<20200908123035.GF6047@pendragon.ideasonboard.com>","date":"2020-09-08T12:30:35","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Sep 08, 2020 at 01:28:07PM +0100, Kieran Bingham wrote:\n> On 08/09/2020 08:54, Laurent Pinchart wrote:\n> > In most cases the last reference to a Camera instance will be the one\n> > held by the CameraManager. That reference gets released when the\n> > CameraManager thread cleans up, just before it stops. There's no need to\n> > delete the camera with deleteLater() in that case.\n> > \n> > To optimize this case, use deleteLater() only when the camera gets\n> > deleted from a different thread, and delete is synchronously otherwise.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Umang, would you be able to test this patch with the Android HAL ?\n> > \n> > src/libcamera/camera.cpp | 6 +++++-\n> > 1 file changed, 5 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 4a9c19c33cfb..ae16a64a3b44 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -16,6 +16,7 @@\n> > \n> > #include \"libcamera/internal/log.h\"\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> > +#include \"libcamera/internal/thread.h\"\n> > #include \"libcamera/internal/utils.h\"\n> > \n> > /**\n> > @@ -470,7 +471,10 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > \tstruct Deleter : std::default_delete {\n> > \t\tvoid operator()(Camera *camera)\n> > \t\t{\n> > -\t\t\tcamera->deleteLater();\n> > +\t\t\tif (Thread::current() == camera->thread())\n> > +\t\t\t\tdelete camera;\n> > +\t\t\telse\n> > +\t\t\t\tcamera->deleteLater();\n> \n> Shouldn't/Couldn't this logic be handled in Object::deleteLater() directly?\n\nI've considered that, but there can be valid use cases for deleting an\nobject from its thread when control returns to the event loop. For\ninstance the decision to delete the object could be located at a point\nwhere functions up the call stack will still access the object. This\ncould be a sign of bad software design overall, but I think there are\nvalid use cases.\n\n> > \t\t}\n> > \t};\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 145F5BDB1C\n\tfor ;\n\tTue, 8 Sep 2020 12:31:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 979C762C3B;\n\tTue, 8 Sep 2020 14:31:01 +0200 (CEST)","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 4A8E762C30\n\tfor ;\n\tTue, 8 Sep 2020 14:31:00 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C750C39;\n\tTue, 8 Sep 2020 14:30:59 +0200 (CEST)"],"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=\"YNsAgT71\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599568260;\n\tbh=0TRQGFJSYhHjCyuII2Vmw8m7WSssWJEo8ELfFd21nBo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YNsAgT711p25SME7t8bRt6df2Z8T9NghrsE7NzBcZpizZSgLzgO/M9alomHBAt96h\n\teJhgivgdo57skOFar5Lh7KUeDEpugbugQ/80Ai7HpG9ac23evGTCsc3b9o89wf/O5H\n\tOTeqDGjHxmQcPH9rOnASNs0UT23zqKMA5NUdiUS8=","Date":"Tue, 8 Sep 2020 15:30:35 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20200908123035.GF6047@pendragon.ideasonboard.com>","References":"<20200908075406.14039-1-laurent.pinchart@ideasonboard.com>\n\t<095af0f2-9115-6cb1-892e-1418f004fbe2@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<095af0f2-9115-6cb1-892e-1418f004fbe2@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12384,"web_url":"https://patchwork.libcamera.org/comment/12384/","msgid":"<17ad3536-3478-0be2-e1e4-03cd8905bcbd@uajain.com>","date":"2020-09-08T13:58:04","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Laurent,\n\nOn 9/8/20 1:24 PM, Laurent Pinchart wrote:\n> In most cases the last reference to a Camera instance will be the one\n> held by the CameraManager. That reference gets released when the\n> CameraManager thread cleans up, just before it stops. There's no need to\n> delete the camera with deleteLater() in that case.\n>\n> To optimize this case, use deleteLater() only when the camera gets\n> deleted from a different thread, and delete is synchronously otherwise.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> Umang, would you be able to test this patch with the Android HAL ?\nI agree for the optimization, but do you have any specific use case in \nmind. I tested it briefly with cros_camera_service cli to test both `if` \nbranches on hot-unplug but as you know my android build/test setup is \nfairly limited with UVC. deleteLater() was brought in, when a currently \nstreaming camera in an app, is hot-unplugged from the system. Although I \nam confident that this patch will hold correctly in such a situation as \nwell.\n\nReviewed-by: Umang Jain \n>\n> src/libcamera/camera.cpp | 6 +++++-\n> 1 file changed, 5 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 4a9c19c33cfb..ae16a64a3b44 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -16,6 +16,7 @@\n> \n> #include \"libcamera/internal/log.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> +#include \"libcamera/internal/thread.h\"\n> #include \"libcamera/internal/utils.h\"\n> \n> /**\n> @@ -470,7 +471,10 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> \tstruct Deleter : std::default_delete {\n> \t\tvoid operator()(Camera *camera)\n> \t\t{\n> -\t\t\tcamera->deleteLater();\n> +\t\t\tif (Thread::current() == camera->thread())\n> +\t\t\t\tdelete camera;\n> +\t\t\telse\n> +\t\t\t\tcamera->deleteLater();\n> \t\t}\n> \t};\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 29764BDB1C\n\tfor ;\n\tTue, 8 Sep 2020 13:58:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AF5616056C;\n\tTue, 8 Sep 2020 15:58:10 +0200 (CEST)","from mail.uajain.com (static.126.159.217.95.clients.your-server.de\n\t[95.217.159.126])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B4BCB6036E\n\tfor ;\n\tTue, 8 Sep 2020 15:58:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"ej6ZELid\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=uajain.com; s=mail;\n\tt=1599573488; bh=b1cz4ixFS0ZTzDSLjBYSDIS0vJiNv8o9MBnWwaO7e+w=;\n\th=Subject:To:References:From:In-Reply-To;\n\tb=ej6ZELidjrRnqQS0n9EuhqYGLbq4ttwx+lKLCzLEummW1sSdagrSkTpKqcvMb8PUz\n\ttPfWM5PsfKr7Tp1t4k1UqnUlAlh+0Edismpd7aVyokt1Hoqn4f9yyVbtatZ6n3pFWG\n\tztI6ZRjnWxddv5TTfE7av4mKpRMqeIjKK2NW862Q90nA0/LkmldwCL+lTKEkNpOlRD\n\tF40Bd+DTvSzFcSjEMX8H+LaFdDqEvrLCuKNOzPrmCJs19MUKJSgwSIFG9/2AszDPaw\n\tI/M+oePo1unnO2b6eplAwewfJdqLrWQYvNjYstEMfyAhIL/AvHTFFidJiS4+lEX5Dy\n\tAJuum7pyTSeUQ==","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200908075406.14039-1-laurent.pinchart@ideasonboard.com>","From":"Umang Jain ","Message-ID":"<17ad3536-3478-0be2-e1e4-03cd8905bcbd@uajain.com>","Date":"Tue, 8 Sep 2020 19:28:04 +0530","Mime-Version":"1.0","In-Reply-To":"<20200908075406.14039-1-laurent.pinchart@ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12385,"web_url":"https://patchwork.libcamera.org/comment/12385/","msgid":"<20200908141102.GN6047@pendragon.ideasonboard.com>","date":"2020-09-08T14:11:02","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Tue, Sep 08, 2020 at 07:28:04PM +0530, Umang Jain wrote:\n> On 9/8/20 1:24 PM, Laurent Pinchart wrote:\n> > In most cases the last reference to a Camera instance will be the one\n> > held by the CameraManager. That reference gets released when the\n> > CameraManager thread cleans up, just before it stops. There's no need to\n> > delete the camera with deleteLater() in that case.\n> >\n> > To optimize this case, use deleteLater() only when the camera gets\n> > deleted from a different thread, and delete is synchronously otherwise.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Umang, would you be able to test this patch with the Android HAL ?\n>\n> I agree for the optimization, but do you have any specific use case in \n> mind. I tested it briefly with cros_camera_service cli to test both `if` \n> branches on hot-unplug but as you know my android build/test setup is \n> fairly limited with UVC. deleteLater() was brought in, when a currently \n> streaming camera in an app, is hot-unplugged from the system. Although I \n> am confident that this patch will hold correctly in such a situation as \n> well.\n\nI expect deleteLater() to be called in the hot-unplug case, and the\nregular delete when the camera manager is stopped, without unplug.\n\n> Reviewed-by: Umang Jain \n>\n> > src/libcamera/camera.cpp | 6 +++++-\n> > 1 file changed, 5 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 4a9c19c33cfb..ae16a64a3b44 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -16,6 +16,7 @@\n> > \n> > #include \"libcamera/internal/log.h\"\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> > +#include \"libcamera/internal/thread.h\"\n> > #include \"libcamera/internal/utils.h\"\n> > \n> > /**\n> > @@ -470,7 +471,10 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > \tstruct Deleter : std::default_delete {\n> > \t\tvoid operator()(Camera *camera)\n> > \t\t{\n> > -\t\t\tcamera->deleteLater();\n> > +\t\t\tif (Thread::current() == camera->thread())\n> > +\t\t\t\tdelete camera;\n> > +\t\t\telse\n> > +\t\t\t\tcamera->deleteLater();\n> > \t\t}\n> > \t};\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7B387BDB1D\n\tfor ;\n\tTue, 8 Sep 2020 14:11:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0831462C37;\n\tTue, 8 Sep 2020 16:11:29 +0200 (CEST)","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 99D4D6036E\n\tfor ;\n\tTue, 8 Sep 2020 16:11:27 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2743439;\n\tTue, 8 Sep 2020 16:11:27 +0200 (CEST)"],"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=\"s6oN+97s\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599574287;\n\tbh=1SDdYOsaMAt8hen8ZC3PRSwfUfdLtKWcLmLpJrHrE0Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=s6oN+97sieN4d4zsiRj0zGPCYLrcWxypS0IHZgrbNxpKDvZamVBWxs4pEAefAYeHR\n\tYnmaei5dxJg+47ygM1koYQpIRA3uwdwiTO1NF+jctAVA8vRArIJX2L15sLMYKylkjv\n\tf+Znssali9zQpcJn0++J21g7wy4Hwjz0iHd74obo=","Date":"Tue, 8 Sep 2020 17:11:02 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"<20200908141102.GN6047@pendragon.ideasonboard.com>","References":"<20200908075406.14039-1-laurent.pinchart@ideasonboard.com>\n\t<17ad3536-3478-0be2-e1e4-03cd8905bcbd@uajain.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<17ad3536-3478-0be2-e1e4-03cd8905bcbd@uajain.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12387,"web_url":"https://patchwork.libcamera.org/comment/12387/","msgid":"<7d05ab55-cd5f-2c01-8444-e6333398f766@uajain.com>","date":"2020-09-08T15:04:59","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Laurent,\n\nOn 9/8/20 7:41 PM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> On Tue, Sep 08, 2020 at 07:28:04PM +0530, Umang Jain wrote:\n>> On 9/8/20 1:24 PM, Laurent Pinchart wrote:\n>>> In most cases the last reference to a Camera instance will be the one\n>>> held by the CameraManager. That reference gets released when the\n>>> CameraManager thread cleans up, just before it stops. There's no need to\n>>> delete the camera with deleteLater() in that case.\n>>>\n>>> To optimize this case, use deleteLater() only when the camera gets\n>>> deleted from a different thread, and delete is synchronously otherwise.\n>>>\n>>> Signed-off-by: Laurent Pinchart \n>>> ---\n>>> Umang, would you be able to test this patch with the Android HAL ?\n>> I agree for the optimization, but do you have any specific use case in\n>> mind. I tested it briefly with cros_camera_service cli to test both `if`\n>> branches on hot-unplug but as you know my android build/test setup is\n>> fairly limited with UVC. deleteLater() was brought in, when a currently\n>> streaming camera in an app, is hot-unplugged from the system. Although I\n>> am confident that this patch will hold correctly in such a situation as\n>> well.\n> I expect deleteLater() to be called in the hot-unplug case, and the\n> regular delete when the camera manager is stopped, without unplug.\nYes, regular delete when camera manager is stopped. It is working as \nexpected.\n>\n>> Reviewed-by: Umang Jain \n>>\n>>> src/libcamera/camera.cpp | 6 +++++-\n>>> 1 file changed, 5 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>>> index 4a9c19c33cfb..ae16a64a3b44 100644\n>>> --- a/src/libcamera/camera.cpp\n>>> +++ b/src/libcamera/camera.cpp\n>>> @@ -16,6 +16,7 @@\n>>> \n>>> #include \"libcamera/internal/log.h\"\n>>> #include \"libcamera/internal/pipeline_handler.h\"\n>>> +#include \"libcamera/internal/thread.h\"\n>>> #include \"libcamera/internal/utils.h\"\n>>> \n>>> /**\n>>> @@ -470,7 +471,10 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n>>> \tstruct Deleter : std::default_delete {\n>>> \t\tvoid operator()(Camera *camera)\n>>> \t\t{\n>>> -\t\t\tcamera->deleteLater();\n>>> +\t\t\tif (Thread::current() == camera->thread())\n>>> +\t\t\t\tdelete camera;\n>>> +\t\t\telse\n>>> +\t\t\t\tcamera->deleteLater();\n>>> \t\t}\n>>> \t};\n>>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3BE70BDB1D\n\tfor ;\n\tTue, 8 Sep 2020 15:05:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C7B746056C;\n\tTue, 8 Sep 2020 17:05:06 +0200 (CEST)","from mail.uajain.com (static.126.159.217.95.clients.your-server.de\n\t[95.217.159.126])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 798796037B\n\tfor ;\n\tTue, 8 Sep 2020 17:05:04 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"AohMcGFS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=uajain.com; s=mail;\n\tt=1599577503; bh=0koMndymHadPRR+jegYVYDAns4BIyAVkComB2y39r5k=;\n\th=Subject:To:Cc:References:From:In-Reply-To;\n\tb=AohMcGFSx7Hj65FpdYULcQSuUBtUYzTHAaLT44QD5WPJt2t6CHJlEFERjKr83Lrgr\n\twvH2OrYP0S+R9UedqKi2sOfxqJhHDM3ZSROpM6g0hVrAhmp0mnDtY4OtWns6zk/05V\n\tIGXeI9M0yWDqLuykZSOzh+7Mqr9VebJcnalGzl+yhFg61Goxjj88O1zT6+CYIHxo7g\n\tGuuFed/slx3c8oxp1kCt9uNPcjpe7cW2AtyrGBxSQySOLCFS8+I3F/8Yxo1X/KZpJc\n\tnCFshnMpF0YuREirGMUz54y2thdWapSKH1u5UqkH3KM8faasAyqFaK0NhZ8WlRCndo\n\tk8b9XoinfgtTg==","To":"Laurent Pinchart ","References":"<20200908075406.14039-1-laurent.pinchart@ideasonboard.com>\n\t<17ad3536-3478-0be2-e1e4-03cd8905bcbd@uajain.com>\n\t<20200908141102.GN6047@pendragon.ideasonboard.com>","From":"Umang Jain ","Message-ID":"<7d05ab55-cd5f-2c01-8444-e6333398f766@uajain.com>","Date":"Tue, 8 Sep 2020 20:34:59 +0530","Mime-Version":"1.0","In-Reply-To":"<20200908141102.GN6047@pendragon.ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12541,"web_url":"https://patchwork.libcamera.org/comment/12541/","msgid":"<3ae4761c-1347-b6c0-bae7-98848de3d9e7@ideasonboard.com>","date":"2020-09-16T11:11:58","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 08/09/2020 13:30, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Tue, Sep 08, 2020 at 01:28:07PM +0100, Kieran Bingham wrote:\n>> On 08/09/2020 08:54, Laurent Pinchart wrote:\n>>> In most cases the last reference to a Camera instance will be the one\n>>> held by the CameraManager. That reference gets released when the\n>>> CameraManager thread cleans up, just before it stops. There's no need to\n>>> delete the camera with deleteLater() in that case.\n>>>\n>>> To optimize this case, use deleteLater() only when the camera gets\n>>> deleted from a different thread, and delete is synchronously otherwise.\n>>>\n>>> Signed-off-by: Laurent Pinchart \n>>> ---\n>>> Umang, would you be able to test this patch with the Android HAL ?\n>>>\n>>> src/libcamera/camera.cpp | 6 +++++-\n>>> 1 file changed, 5 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>>> index 4a9c19c33cfb..ae16a64a3b44 100644\n>>> --- a/src/libcamera/camera.cpp\n>>> +++ b/src/libcamera/camera.cpp\n>>> @@ -16,6 +16,7 @@\n>>> \n>>> #include \"libcamera/internal/log.h\"\n>>> #include \"libcamera/internal/pipeline_handler.h\"\n>>> +#include \"libcamera/internal/thread.h\"\n>>> #include \"libcamera/internal/utils.h\"\n>>> \n>>> /**\n>>> @@ -470,7 +471,10 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n>>> \tstruct Deleter : std::default_delete {\n>>> \t\tvoid operator()(Camera *camera)\n>>> \t\t{\n>>> -\t\t\tcamera->deleteLater();\n>>> +\t\t\tif (Thread::current() == camera->thread())\n>>> +\t\t\t\tdelete camera;\n>>> +\t\t\telse\n>>> +\t\t\t\tcamera->deleteLater();\n>>\n>> Shouldn't/Couldn't this logic be handled in Object::deleteLater() directly?\n> \n> I've considered that, but there can be valid use cases for deleting an\n> object from its thread when control returns to the event loop. For\n> instance the decision to delete the object could be located at a point\n> where functions up the call stack will still access the object. This\n> could be a sign of bad software design overall, but I think there are\n> valid use cases.\n\nWell, this is the only current user - so it's hard to compare other\nuse-cases, or whether they are valid or not yet\n\nBut adding specific code to handle a case which is specifically only\nused this way feels a bit odd.\n\nAnyway, Didn't this fix some issue in particular (I can't see anything\nmentioned in the commit message though)? so lets just get it in if you\nneed it.\n\n\n\n>>> \t\t}\n>>> \t};\n>>> \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3BE96C3B5B\n\tfor ;\n\tWed, 16 Sep 2020 11:12:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DF2B962E1F;\n\tWed, 16 Sep 2020 13:12:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9141260369\n\tfor ;\n\tWed, 16 Sep 2020 13:12:01 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BD99A57F;\n\tWed, 16 Sep 2020 13:12:00 +0200 (CEST)"],"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=\"O3lnWphE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600254720;\n\tbh=hKolFIHn4H1hsorNdKU+0liSfoQOtsvqoNcKVwZgrKs=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=O3lnWphEJrfS9kHa64t7TcjkhjwtAz6dYnKj6EiL40wqYWpka4wKollhzr9+/drw5\n\tYx/NOk06F+iJ5liM3KRdxm5QwFbHhxDUb2DkCybTH7OrRxL8zfPE1tR4EYb3mFcLqZ\n\tOSUE7MA7Fxv3S3GNRBTRIG6PthHiBbSjbZkMmsvE=","To":"Laurent Pinchart ","References":"<20200908075406.14039-1-laurent.pinchart@ideasonboard.com>\n\t<095af0f2-9115-6cb1-892e-1418f004fbe2@ideasonboard.com>\n\t<20200908123035.GF6047@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<3ae4761c-1347-b6c0-bae7-98848de3d9e7@ideasonboard.com>","Date":"Wed, 16 Sep 2020 12:11:58 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200908123035.GF6047@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12542,"web_url":"https://patchwork.libcamera.org/comment/12542/","msgid":"<20200916111704.GA3853@pendragon.ideasonboard.com>","date":"2020-09-16T11:17:04","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Sep 16, 2020 at 12:11:58PM +0100, Kieran Bingham wrote:\n> On 08/09/2020 13:30, Laurent Pinchart wrote:\n> > On Tue, Sep 08, 2020 at 01:28:07PM +0100, Kieran Bingham wrote:\n> >> On 08/09/2020 08:54, Laurent Pinchart wrote:\n> >>> In most cases the last reference to a Camera instance will be the one\n> >>> held by the CameraManager. That reference gets released when the\n> >>> CameraManager thread cleans up, just before it stops. There's no need to\n> >>> delete the camera with deleteLater() in that case.\n> >>>\n> >>> To optimize this case, use deleteLater() only when the camera gets\n> >>> deleted from a different thread, and delete is synchronously otherwise.\n> >>>\n> >>> Signed-off-by: Laurent Pinchart \n> >>> ---\n> >>> Umang, would you be able to test this patch with the Android HAL ?\n> >>>\n> >>> src/libcamera/camera.cpp | 6 +++++-\n> >>> 1 file changed, 5 insertions(+), 1 deletion(-)\n> >>>\n> >>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> >>> index 4a9c19c33cfb..ae16a64a3b44 100644\n> >>> --- a/src/libcamera/camera.cpp\n> >>> +++ b/src/libcamera/camera.cpp\n> >>> @@ -16,6 +16,7 @@\n> >>> \n> >>> #include \"libcamera/internal/log.h\"\n> >>> #include \"libcamera/internal/pipeline_handler.h\"\n> >>> +#include \"libcamera/internal/thread.h\"\n> >>> #include \"libcamera/internal/utils.h\"\n> >>> \n> >>> /**\n> >>> @@ -470,7 +471,10 @@ std::shared_ptr Camera::create(PipelineHandler *pipe,\n> >>> \tstruct Deleter : std::default_delete {\n> >>> \t\tvoid operator()(Camera *camera)\n> >>> \t\t{\n> >>> -\t\t\tcamera->deleteLater();\n> >>> +\t\t\tif (Thread::current() == camera->thread())\n> >>> +\t\t\t\tdelete camera;\n> >>> +\t\t\telse\n> >>> +\t\t\t\tcamera->deleteLater();\n> >>\n> >> Shouldn't/Couldn't this logic be handled in Object::deleteLater() directly?\n> > \n> > I've considered that, but there can be valid use cases for deleting an\n> > object from its thread when control returns to the event loop. For\n> > instance the decision to delete the object could be located at a point\n> > where functions up the call stack will still access the object. This\n> > could be a sign of bad software design overall, but I think there are\n> > valid use cases.\n> \n> Well, this is the only current user - so it's hard to compare other\n> use-cases, or whether they are valid or not yet\n\nSure. Regardless of the option we pick now, I'm sure we'll change it at\nleast twice ;-) As a default, I've thought that following the\ndeleteLater() API implemented by Qt wouldn't be a bad idea.\n\n> But adding specific code to handle a case which is specifically only\n> used this way feels a bit odd.\n> \n> Anyway, Didn't this fix some issue in particular (I can't see anything\n> mentioned in the commit message though)? so lets just get it in if you\n> need it.\n\nIt's an optimization, it doesn't fix a functional issue.\n\n> >>> \t\t}\n> >>> \t};\n> >>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 37A18BF01C\n\tfor ;\n\tWed, 16 Sep 2020 11:17:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A444962E1F;\n\tWed, 16 Sep 2020 13:17:36 +0200 (CEST)","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 1812960369\n\tfor ;\n\tWed, 16 Sep 2020 13:17:35 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 842FE57F;\n\tWed, 16 Sep 2020 13:17:34 +0200 (CEST)"],"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=\"unNfSZzA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600255054;\n\tbh=52zJ15o7JAd938PfGNy+qh0hxF2r3Y1vuzDd5dczUj8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=unNfSZzACrqC44Ig04i0kApLvaJ0uT6q97nGyteY7L1ZgEnJamvStDaCbhfXznVGG\n\taBcokb8x2tHaq2+5Cwy9cGn9NG0/4IndRAYPirEgzwFpTpdQX4dnxImdNlWoE9Ep/u\n\t7gVRVbUMcPgPX9dfIx7NsDfbgJF0AUB1jo69h0+s=","Date":"Wed, 16 Sep 2020 14:17:04 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20200916111704.GA3853@pendragon.ideasonboard.com>","References":"<20200908075406.14039-1-laurent.pinchart@ideasonboard.com>\n\t<095af0f2-9115-6cb1-892e-1418f004fbe2@ideasonboard.com>\n\t<20200908123035.GF6047@pendragon.ideasonboard.com>\n\t<3ae4761c-1347-b6c0-bae7-98848de3d9e7@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<3ae4761c-1347-b6c0-bae7-98848de3d9e7@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera: Optimize camera\n\tdeletion","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]