[{"id":21839,"web_url":"https://patchwork.libcamera.org/comment/21839/","msgid":"<20211221085342.tbnwoqfbx5v4lyov@uno.localdomain>","date":"2021-12-21T08:53:42","subject":"Re: [libcamera-devel] [PATCH v2 3/6] android: camera_metadata: Add\n\tappendEntry helper","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Mon, Dec 20, 2021 at 05:26:26PM -0600, Paul Elder wrote:\n> Add appendEntry() helper, that automatically detects if updateEntry() or\n> addEntry() should be used.\n>\n> Note that updateEntry() will fail if the entry was not yet added, and\n> addEntry() will fail is the entry was already added. They are silent\n> failures that cause unexpected values to find their way into the android\n> metadata instance.\n>\n> Previously this helper was not necessary, as (with respect to the\n> current use case) the preview template generator would always add a key\n> so the other template generators that used the preview template as\n> boilerplate could reliably use updateEntry(). The preview template\n> generator will soon decide based on capabilities whether or not to add\n> keys, so the other template generators need a helper to decide whether\n> to use updateEntry() or addEntry().\n>\n> For now only implement it for enums and arithmetic values, as they will\n> mainly be used in populating templates.\n\nI'm just not sure if doing this for enum and arithmetic types is a\ngood idea, or we would need a template specialization for each\nspecialization of addEntry() (and it seems to me updateEntry() and\naddEntry() are already mis-aligned).\n\n>\n> Signed-off-by: Paul Elder \n> Reviewed-by: Kieran Bingham \n>\n> ---\n> Changes in v2:\n> - expand on correctness of updateEntry() vs addEntry(), and the\n> rationale for the patch\n> ---\n> src/android/camera_metadata.h | 11 +++++++++++\n> 1 file changed, 11 insertions(+)\n>\n> diff --git a/src/android/camera_metadata.h b/src/android/camera_metadata.h\n> index e70f60af..f3b7c91e 100644\n> --- a/src/android/camera_metadata.h\n> +++ b/src/android/camera_metadata.h\n> @@ -33,6 +33,17 @@ public:\n>\n> \tbool hasEntry(uint32_t tag) const;\n>\n> +\ttemplate +\t\t std::enable_if_t ||\n> +\t\t\t\t std::is_enum_v> * = nullptr>\n> +\tbool appendEntry(uint32_t tag, const T &data)\n\nUsual bikeshedding about name: is append the best name to explain what\nthis function does ? Isn't 'setEntry()' more appropriate ?\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> +\t{\n> +\t\tif (hasEntry(tag))\n> +\t\t\treturn updateEntry(tag, &data, 1, sizeof(T));\n> +\t\telse\n> +\t\t\treturn addEntry(tag, &data, 1, sizeof(T));\n> +\t}\n> +\n> \ttemplate \t\t std::enable_if_t ||\n> \t\t\t\t std::is_enum_v> * = nullptr>\n> --\n> 2.27.0\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 30022BE080\n\tfor ;\n\tTue, 21 Dec 2021 08:52:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3F94F60901;\n\tTue, 21 Dec 2021 09:52:49 +0100 (CET)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 05E7E60113\n\tfor ;\n\tTue, 21 Dec 2021 09:52:48 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 7AAD4C000D;\n\tTue, 21 Dec 2021 08:52:47 +0000 (UTC)"],"Date":"Tue, 21 Dec 2021 09:53:42 +0100","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20211221085342.tbnwoqfbx5v4lyov@uno.localdomain>","References":"<20211220232629.1485890-1-paul.elder@ideasonboard.com>\n\t<20211220232629.1485890-4-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211220232629.1485890-4-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] android: camera_metadata: Add\n\tappendEntry helper","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]