[{"id":14742,"web_url":"https://patchwork.libcamera.org/comment/14742/","msgid":"","date":"2021-01-25T09:59:45","subject":"Re: [libcamera-devel] [PATCH v4 7/8] android: camera_device: Cache\n\trequest metadata","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Mon, Jan 25, 2021 at 04:14:43PM +0900, Paul Elder wrote:\n> The settings in an android capture request may be null, in which case\n> the settings from the most recently submitted capture request should be\n> used. Cache the request settings to achieve this.\n> \n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n> \n> ---\n> Changes in v4:\n> - use default CameraMetadata constructor for lastSettings_ (so no\n> initialization in CameraDevice constructor)\n> - add todo for incremental caching of android request settings\n> \n> Changes in v3:\n> - rebase on \"android: camera device and metatada improvements\", so it's\n> a bit cleaner\n> \n> New in v2\n> ---\n> src/android/camera_device.cpp | 6 +++++-\n> src/android/camera_device.h | 2 ++\n> 2 files changed, 7 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 3068f89f..9330db39 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1688,9 +1688,13 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \t * The descriptor and the associated memory reserved here are freed\n> \t * at request complete time.\n> \t */\n> -\t/* \\todo Handle null request settings */\n> \tCamera3RequestDescriptor *descriptor =\n> \t\tnew Camera3RequestDescriptor(camera_.get(), camera3Request);\n> +\t/* \\todo Set cache incrementally? */\n\nI'm not sure we'll remember what this means in a few months :-)\n\n\t/*\n\t * \\todo The Android request model is incremental, settings passed in\n\t * previous requests are to be effective until overridden explicitly in\n\t * a new request. Do we need to cache settings incrementally here, or is\n\t * it handled by the Android camera service ?\n\t */\n\n> +\tif (camera3Request->settings)\n> +\t\tlastSettings_ = camera3Request->settings;\n> +\telse\n> +\t\tdescriptor->settings_ = lastSettings_;\n> \n> \tLOG(HAL, Debug) << \"Queueing Request to libcamera with \"\n> \t\t\t<< descriptor->numBuffers_ << \" HAL streams\";\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 058a3f9a..fa4fb544 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -134,6 +134,8 @@ private:\n> \tint orientation_;\n> \n> \tunsigned int maxJpegBufferSize_;\n> +\n> +\tCameraMetadata lastSettings_;\n> };\n> \n> #endif /* __ANDROID_CAMERA_DEVICE_H__ */","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 C6D53C0F2B\n\tfor ;\n\tMon, 25 Jan 2021 10:00:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 589F0682AD;\n\tMon, 25 Jan 2021 11:00:06 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 535A36030B\n\tfor ;\n\tMon, 25 Jan 2021 11:00:05 +0100 (CET)","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 C181E3E;\n\tMon, 25 Jan 2021 11:00:04 +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=\"quZv6a8/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611568805;\n\tbh=wqcl6MH9mL/v/wDwTArQPrLajZ6BL5XV697wauhhz6Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=quZv6a8/v12N88J5khKtg2AREJLkdr298hMuef7IdXYSDrrw8wUiwS2o/KxL96xKc\n\tltLcbWV7CPA0qctvD9stG3S+Iz2LK6PkibCh1O6T9TQgtNW5bZtNtEC0MUyGhceo6Q\n\tQHgO221Aal0wrMSSvz3nqVlt7WUKzWYSZcmPXdCw=","Date":"Mon, 25 Jan 2021 11:59:45 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210125071444.26252-1-paul.elder@ideasonboard.com>\n\t<20210125071444.26252-8-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210125071444.26252-8-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 7/8] android: camera_device: Cache\n\trequest metadata","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":14760,"web_url":"https://patchwork.libcamera.org/comment/14760/","msgid":"<20210125121100.fkmd65wdgazpiyqo@uno.localdomain>","date":"2021-01-25T12:11:00","subject":"Re: [libcamera-devel] [PATCH v4 7/8] android: camera_device: Cache\n\trequest metadata","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul\n\nOn Mon, Jan 25, 2021 at 04:14:43PM +0900, Paul Elder wrote:\n> The settings in an android capture request may be null, in which case\n> the settings from the most recently submitted capture request should be\n> used. Cache the request settings to achieve this.\n>\n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n\nThis scares me a bit, as it's not totally clear to me what the\nsemantic of a request without control impacts the other metadata\ncontructed using the values passed to us as settings.\n\nFor the time being though, I think it's ok\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> ---\n> Changes in v4:\n> - use default CameraMetadata constructor for lastSettings_ (so no\n> initialization in CameraDevice constructor)\n> - add todo for incremental caching of android request settings\n>\n> Changes in v3:\n> - rebase on \"android: camera device and metatada improvements\", so it's\n> a bit cleaner\n>\n> New in v2\n> ---\n> src/android/camera_device.cpp | 6 +++++-\n> src/android/camera_device.h | 2 ++\n> 2 files changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 3068f89f..9330db39 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1688,9 +1688,13 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \t * The descriptor and the associated memory reserved here are freed\n> \t * at request complete time.\n> \t */\n> -\t/* \\todo Handle null request settings */\n> \tCamera3RequestDescriptor *descriptor =\n> \t\tnew Camera3RequestDescriptor(camera_.get(), camera3Request);\n> +\t/* \\todo Set cache incrementally? */\n> +\tif (camera3Request->settings)\n> +\t\tlastSettings_ = camera3Request->settings;\n> +\telse\n> +\t\tdescriptor->settings_ = lastSettings_;\n>\n> \tLOG(HAL, Debug) << \"Queueing Request to libcamera with \"\n> \t\t\t<< descriptor->numBuffers_ << \" HAL streams\";\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 058a3f9a..fa4fb544 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -134,6 +134,8 @@ private:\n> \tint orientation_;\n>\n> \tunsigned int maxJpegBufferSize_;\n> +\n> +\tCameraMetadata lastSettings_;\n> };\n>\n> #endif /* __ANDROID_CAMERA_DEVICE_H__ */\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 DB709C0F2B\n\tfor ;\n\tMon, 25 Jan 2021 12:10:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 571F6682C6;\n\tMon, 25 Jan 2021 13:10:42 +0100 (CET)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E38B3682BB\n\tfor ;\n\tMon, 25 Jan 2021 13:10:40 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 607EF40006;\n\tMon, 25 Jan 2021 12:10:40 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 25 Jan 2021 13:11:00 +0100","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20210125121100.fkmd65wdgazpiyqo@uno.localdomain>","References":"<20210125071444.26252-1-paul.elder@ideasonboard.com>\n\t<20210125071444.26252-8-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210125071444.26252-8-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 7/8] android: camera_device: Cache\n\trequest metadata","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\" "}}]