[{"id":14357,"web_url":"https://patchwork.libcamera.org/comment/14357/","msgid":"","date":"2020-12-27T11:36:15","subject":"Re: [libcamera-devel] [PATCH v6 1/9] libcamera: control_serializer:\n\tSave serialized ControlInfoMap in a cache","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nThanks for your work.\n\nOn 2020-12-24 17:15:26 +0900, Paul Elder wrote:\n> The ControlSerializer saves all ControlInfoMaps that it has already\n> (de)serialized, in order to (de)serialize ControlLists that contain the\n> ControlInfoMaps. Leverage this to cache ControlInfoMaps, such that the\n> ControlSerializer will not re-(de)serialize a ControlInfoMap that it has\n> previously (de)serialized.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> New in v6\n> ---\n> .../libcamera/internal/control_serializer.h | 1 +\n> src/libcamera/control_serializer.cpp | 30 +++++++++++++++++++\n> 2 files changed, 31 insertions(+)\n> \n> diff --git a/include/libcamera/internal/control_serializer.h b/include/libcamera/internal/control_serializer.h\n> index 0ab29d9a..76cb3c10 100644\n> --- a/include/libcamera/internal/control_serializer.h\n> +++ b/include/libcamera/internal/control_serializer.h\n> @@ -33,6 +33,7 @@ public:\n> \ttemplate\n> \tT deserialize(ByteStreamBuffer &buffer);\n> \n> +\tbool isCached(const ControlInfoMap *infoMap);\n> private:\n> \tstatic size_t binarySize(const ControlValue &value);\n> \tstatic size_t binarySize(const ControlInfo &info);\n> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> index 258db6df..4cf1c720 100644\n> --- a/src/libcamera/control_serializer.cpp\n> +++ b/src/libcamera/control_serializer.cpp\n> @@ -173,6 +173,11 @@ void ControlSerializer::store(const ControlInfo &info, ByteStreamBuffer &buffer)\n> int ControlSerializer::serialize(const ControlInfoMap &infoMap,\n> \t\t\t\t ByteStreamBuffer &buffer)\n> {\n> +\tif (isCached(&infoMap)) {\n> +\t\tLOG(Serializer, Info) << \"Serializing ControlInfoMap from cache\";\n\nI wonder if this and the one below should be Debug messages?\n\n> +\t\treturn 0;\n> +\t}\n> +\n> \t/* Compute entries and data required sizes. */\n> \tsize_t entriesSize = infoMap.size()\n> \t\t\t * sizeof(struct ipa_control_info_entry);\n> @@ -347,6 +352,12 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &\n> \t\treturn {};\n> \t}\n> \n> +\tauto iter = infoMaps_.find(hdr->handle);\n> +\tif (iter != infoMaps_.end()) {\n> +\t\tLOG(Serializer, Info) << \"Deserializing ControlInfoMap from cache\";\n> +\t\treturn iter->second;\n> +\t}\n\nI think you should either fold the check from isChached() in above or \nadd a isCached(unsigned int) and use it here. I'm leaning towards the \nformer as then if we switch to C++20 we could use \nstd::map<>::contains().\n\n> +\n> \tif (hdr->version != IPA_CONTROLS_FORMAT_VERSION) {\n> \t\tLOG(Serializer, Error)\n> \t\t\t<< \"Unsupported controls format version \"\n> @@ -485,4 +496,23 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n> \treturn ctrls;\n> }\n> \n> +/**\n> + * \\brief Check if some ControlInfoMap is cached\n\ns/some/a/\n\n> + * \\param[in] infoMap The ControlInfoMap to check\n> + *\n> + * The ControlSerializer caches all ControlInfoMaps that it has \n> (de)serialized.\n> + * This function checks if \\a infoMap is in the cache.\n> + *\n> + * \\return True if \\a infoMap is in the cache or if \\a infoMap is\n> + * controls::controls, false otherwise\n\ncontrols::controls ?\n\n> + */\n> +bool ControlSerializer::isCached(const ControlInfoMap *infoMap)\n> +{\n> +\tif (!infoMap)\n> +\t\treturn true;\n> +\n> +\tauto iter = infoMapHandles_.find(infoMap);\n> +\treturn iter != infoMapHandles_.end();\n> +}\n> +\n> } /* namespace libcamera */\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 9A4E5C0F1B\n\tfor ;\n\tSun, 27 Dec 2020 11:36:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 283A461FFA;\n\tSun, 27 Dec 2020 12:36:37 +0100 (CET)","from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 068E9615D2\n\tfor ;\n\tSun, 27 Dec 2020 12:36:36 +0100 (CET)","by mail-lf1-x142.google.com with SMTP id h205so18155637lfd.5\n\tfor ;\n\tSun, 27 Dec 2020 03:36:35 -0800 (PST)","from localhost ([185.224.57.161]) by smtp.gmail.com with ESMTPSA id\n\t192sm5001009lfa.219.2020.12.27.03.36.33\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 27 Dec 2020 03:36:34 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"a8WcOpFJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=pbUyE5NEHLdoeeeM0pzxrdoaSrmPllZi2TT3BcZ66lk=;\n\tb=a8WcOpFJACo30P7OE1w8Ass2w6bLtzg3+qSYCgzPiMPtctnUC5qtzY1tDT3H6Gug/d\n\tdTqgak14eLksRw4q1wPaDsY6dzO9GR1jP3FA6WoFutzGETiUQ02IuaLju6MPHlvtxd2x\n\thLjoUlextWGQEBsfEjjLGo8OqGRSWogXUylQEwWPuxYatgXA4GHZShb5syZjTQBdj5Q/\n\tFOOyPc4qq6SEsJsxSoP4bPoO0060FNYSkJUXVq+zaho2PGphMS6Gi1aD7Xo6s6GoqNGC\n\teQIkfUyGJ9Fhsr12XvSEAZtfxbO0GO407B/g022BtERK8VwDn0jQRurhLLn255ET7rFa\n\tD9vg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=pbUyE5NEHLdoeeeM0pzxrdoaSrmPllZi2TT3BcZ66lk=;\n\tb=N7CvXqrcIGqcmwVC5KnzPCx96g3Td6AR561hb+Rw9XAn8exnzG/qb47nawi7bnvJ1P\n\tNfK8gQHp9h5V05qU6E0u0b2/eaJFL21AgIr3VQlBiZWwHqrsGdpM7uGAfPjVtJRxa3Pb\n\tY3hZMZFBR2OxZDEJPp+ayndUn/TF/d30jggNnq4Vkz0VG3fQ775oTgg1cDzuyALbVi87\n\tq1yOVTy2e0yfp1hZUN1en07ikO3wcHEfzZlCGFRWr72OYE/wYgyRCeFY/73bYpz95IRx\n\tPfU7NYasalEcJGKgYQGwlfNuTI8Kuabmiv+KukY/JI5UMxL5YxiKFnpjPnJrP4aB/aAx\n\tZu+A==","X-Gm-Message-State":"AOAM531XVxVu4s4s8/9c5fyYOrovySp3KZgQ6xqJIGlUYSe3hCkT4N1D\n\tqx0wZ0ELz5rSHIbBXpGMWrD0GqIY6vgwSg==","X-Google-Smtp-Source":"ABdhPJyn7Xsor8qmIA9cuII7E4uZPpFwzmeD5FF1wndu9alvhHecYLzrx7T+BgU3nhxVErAv6cLTYw==","X-Received":"by 2002:a05:6512:208b:: with SMTP id\n\tt11mr16967948lfr.647.1609068995419; \n\tSun, 27 Dec 2020 03:36:35 -0800 (PST)","Date":"Sun, 27 Dec 2020 12:36:15 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Paul Elder ","Message-ID":"","References":"<20201224081534.41601-1-paul.elder@ideasonboard.com>\n\t<20201224081534.41601-2-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201224081534.41601-2-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v6 1/9] libcamera: control_serializer:\n\tSave serialized ControlInfoMap in a cache","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14376,"web_url":"https://patchwork.libcamera.org/comment/14376/","msgid":"<20201228094811.GJ1933@pyrite.rasen.tech>","date":"2020-12-28T09:48:11","subject":"Re: [libcamera-devel] [PATCH v6 1/9] libcamera: control_serializer:\n\tSave serialized ControlInfoMap in a cache","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the review.\n\nOn Sun, Dec 27, 2020 at 12:36:15PM +0100, Niklas Söderlund wrote:\n> Hi Paul,\n> \n> Thanks for your work.\n> \n> On 2020-12-24 17:15:26 +0900, Paul Elder wrote:\n> > The ControlSerializer saves all ControlInfoMaps that it has already\n> > (de)serialized, in order to (de)serialize ControlLists that contain the\n> > ControlInfoMaps. Leverage this to cache ControlInfoMaps, such that the\n> > ControlSerializer will not re-(de)serialize a ControlInfoMap that it has\n> > previously (de)serialized.\n> > \n> > Signed-off-by: Paul Elder \n> > \n> > ---\n> > New in v6\n> > ---\n> > .../libcamera/internal/control_serializer.h | 1 +\n> > src/libcamera/control_serializer.cpp | 30 +++++++++++++++++++\n> > 2 files changed, 31 insertions(+)\n> > \n> > diff --git a/include/libcamera/internal/control_serializer.h b/include/libcamera/internal/control_serializer.h\n> > index 0ab29d9a..76cb3c10 100644\n> > --- a/include/libcamera/internal/control_serializer.h\n> > +++ b/include/libcamera/internal/control_serializer.h\n> > @@ -33,6 +33,7 @@ public:\n> > \ttemplate\n> > \tT deserialize(ByteStreamBuffer &buffer);\n> > \n> > +\tbool isCached(const ControlInfoMap *infoMap);\n> > private:\n> > \tstatic size_t binarySize(const ControlValue &value);\n> > \tstatic size_t binarySize(const ControlInfo &info);\n> > diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> > index 258db6df..4cf1c720 100644\n> > --- a/src/libcamera/control_serializer.cpp\n> > +++ b/src/libcamera/control_serializer.cpp\n> > @@ -173,6 +173,11 @@ void ControlSerializer::store(const ControlInfo &info, ByteStreamBuffer &buffer)\n> > int ControlSerializer::serialize(const ControlInfoMap &infoMap,\n> > \t\t\t\t ByteStreamBuffer &buffer)\n> > {\n> > +\tif (isCached(&infoMap)) {\n> > +\t\tLOG(Serializer, Info) << \"Serializing ControlInfoMap from cache\";\n> \n> I wonder if this and the one below should be Debug messages?\n\nYeah it should be.\n\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > \t/* Compute entries and data required sizes. */\n> > \tsize_t entriesSize = infoMap.size()\n> > \t\t\t * sizeof(struct ipa_control_info_entry);\n> > @@ -347,6 +352,12 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &\n> > \t\treturn {};\n> > \t}\n> > \n> > +\tauto iter = infoMaps_.find(hdr->handle);\n> > +\tif (iter != infoMaps_.end()) {\n> > +\t\tLOG(Serializer, Info) << \"Deserializing ControlInfoMap from cache\";\n> > +\t\treturn iter->second;\n> > +\t}\n> \n> I think you should either fold the check from isChached() in above or \n> add a isCached(unsigned int) and use it here. I'm leaning towards the \n> former as then if we switch to C++20 we could use \n> std::map<>::contains().\n\nI don't see how the former would work. The latter would just be the same\nas what's here, and it's only used once.\n\n> > +\n> > \tif (hdr->version != IPA_CONTROLS_FORMAT_VERSION) {\n> > \t\tLOG(Serializer, Error)\n> > \t\t\t<< \"Unsupported controls format version \"\n> > @@ -485,4 +496,23 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n> > \treturn ctrls;\n> > }\n> > \n> > +/**\n> > + * \\brief Check if some ControlInfoMap is cached\n> \n> s/some/a/\n> \n> > + * \\param[in] infoMap The ControlInfoMap to check\n> > + *\n> > + * The ControlSerializer caches all ControlInfoMaps that it has \n> > (de)serialized.\n> > + * This function checks if \\a infoMap is in the cache.\n> > + *\n> > + * \\return True if \\a infoMap is in the cache or if \\a infoMap is\n> > + * controls::controls, false otherwise\n> \n> controls::controls ?\n\nOh, it should be nullptr here. I was thinking about ControlLists that\nuse controls::controls.\n\n\nThanks,\n\nPaul\n\n> > + */\n> > +bool ControlSerializer::isCached(const ControlInfoMap *infoMap)\n> > +{\n> > +\tif (!infoMap)\n> > +\t\treturn true;\n> > +\n> > +\tauto iter = infoMapHandles_.find(infoMap);\n> > +\treturn iter != infoMapHandles_.end();\n> > +}\n> > +\n> > } /* namespace libcamera */\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\n> \n> -- \n> Regards,\n> Niklas Söderlund","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 3DA29C0F1B\n\tfor ;\n\tMon, 28 Dec 2020 09:48:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A92C0615B2;\n\tMon, 28 Dec 2020 10:48:21 +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 97E7E60525\n\tfor ;\n\tMon, 28 Dec 2020 10:48:19 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E6E0225C;\n\tMon, 28 Dec 2020 10:48:17 +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=\"OjUZAo+4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1609148899;\n\tbh=Xizcb9+L6GSio+UmYxdSf7BvpWuXR4ESe9c5sT0+vFw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OjUZAo+4bBTFfr2xV3wy5Wi79AO1MSgcX3oLWWhZC5IyqIAOoWa+oz0848w/r/Sg+\n\tjj/M/L74RvZKsDBr8zPalDjyP82tA2iKQfOoDIlb31RbDLblvsjAOkGTM2IhuCzjHx\n\tNPkXCM4+Lac8s5hdPiOmc3oQJfSHeKQ93Iluq08M=","Date":"Mon, 28 Dec 2020 18:48:11 +0900","From":"paul.elder@ideasonboard.com","To":"Niklas =?iso-8859-1?q?S=F6derlund?= ","Message-ID":"<20201228094811.GJ1933@pyrite.rasen.tech>","References":"<20201224081534.41601-1-paul.elder@ideasonboard.com>\n\t<20201224081534.41601-2-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v6 1/9] libcamera: control_serializer:\n\tSave serialized ControlInfoMap in a cache","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14892,"web_url":"https://patchwork.libcamera.org/comment/14892/","msgid":"","date":"2021-02-01T21:40:41","subject":"Re: [libcamera-devel] [PATCH v6 1/9] libcamera: control_serializer:\n\tSave serialized ControlInfoMap in a cache","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, Dec 28, 2020 at 06:48:11PM +0900, paul.elder@ideasonboard.com wrote:\n> On Sun, Dec 27, 2020 at 12:36:15PM +0100, Niklas Söderlund wrote:\n> > On 2020-12-24 17:15:26 +0900, Paul Elder wrote:\n> > > The ControlSerializer saves all ControlInfoMaps that it has already\n> > > (de)serialized, in order to (de)serialize ControlLists that contain the\n> > > ControlInfoMaps. Leverage this to cache ControlInfoMaps, such that the\n> > > ControlSerializer will not re-(de)serialize a ControlInfoMap that it has\n> > > previously (de)serialized.\n> > > \n> > > Signed-off-by: Paul Elder \n> > > \n> > > ---\n> > > New in v6\n> > > ---\n> > > .../libcamera/internal/control_serializer.h | 1 +\n> > > src/libcamera/control_serializer.cpp | 30 +++++++++++++++++++\n> > > 2 files changed, 31 insertions(+)\n> > > \n> > > diff --git a/include/libcamera/internal/control_serializer.h b/include/libcamera/internal/control_serializer.h\n> > > index 0ab29d9a..76cb3c10 100644\n> > > --- a/include/libcamera/internal/control_serializer.h\n> > > +++ b/include/libcamera/internal/control_serializer.h\n> > > @@ -33,6 +33,7 @@ public:\n> > > \ttemplate\n> > > \tT deserialize(ByteStreamBuffer &buffer);\n> > > \n> > > +\tbool isCached(const ControlInfoMap *infoMap);\n\nBlank line here ?\n\n> > > private:\n> > > \tstatic size_t binarySize(const ControlValue &value);\n> > > \tstatic size_t binarySize(const ControlInfo &info);\n> > > diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> > > index 258db6df..4cf1c720 100644\n> > > --- a/src/libcamera/control_serializer.cpp\n> > > +++ b/src/libcamera/control_serializer.cpp\n> > > @@ -173,6 +173,11 @@ void ControlSerializer::store(const ControlInfo &info, ByteStreamBuffer &buffer)\n> > > int ControlSerializer::serialize(const ControlInfoMap &infoMap,\n> > > \t\t\t\t ByteStreamBuffer &buffer)\n> > > {\n> > > +\tif (isCached(&infoMap)) {\n> > > +\t\tLOG(Serializer, Info) << \"Serializing ControlInfoMap from cache\";\n> > \n> > I wonder if this and the one below should be Debug messages?\n> \n> Yeah it should be.\n\nAnd I think should mention that serialization is skipped, not that the\nControlInfoMap is serialized from the cache.\n\n\t\tLOG(Serializer, Debug)\n\t\t\t<< \"Skipping already serialized ControlInfoMap\";\n\n> > > +\t\treturn 0;\n> > > +\t}\n> > > +\n> > > \t/* Compute entries and data required sizes. */\n> > > \tsize_t entriesSize = infoMap.size()\n> > > \t\t\t * sizeof(struct ipa_control_info_entry);\n> > > @@ -347,6 +352,12 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &\n> > > \t\treturn {};\n> > > \t}\n> > > \n> > > +\tauto iter = infoMaps_.find(hdr->handle);\n> > > +\tif (iter != infoMaps_.end()) {\n> > > +\t\tLOG(Serializer, Info) << \"Deserializing ControlInfoMap from cache\";\n\nDebug here too, and maybe\n\n\t\tLOG(Serializer, Info) << \"Use cached ControlInfoMap\";\n\nas you don't deserialize it.\n\n> > > +\t\treturn iter->second;\n> > > +\t}\n> > \n> > I think you should either fold the check from isChached() in above or \n> > add a isCached(unsigned int) and use it here. I'm leaning towards the \n> > former as then if we switch to C++20 we could use \n> > std::map<>::contains().\n> \n> I don't see how the former would work. The latter would just be the same\n> as what's here, and it's only used once.\n> \n> > > +\n> > > \tif (hdr->version != IPA_CONTROLS_FORMAT_VERSION) {\n> > > \t\tLOG(Serializer, Error)\n> > > \t\t\t<< \"Unsupported controls format version \"\n> > > @@ -485,4 +496,23 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n> > > \treturn ctrls;\n> > > }\n> > > \n> > > +/**\n> > > + * \\brief Check if some ControlInfoMap is cached\n> > \n> > s/some/a/\n> > \n> > > + * \\param[in] infoMap The ControlInfoMap to check\n> > > + *\n> > > + * The ControlSerializer caches all ControlInfoMaps that it has (de)serialized.\n> > > + * This function checks if \\a infoMap is in the cache.\n> > > + *\n> > > + * \\return True if \\a infoMap is in the cache or if \\a infoMap is\n> > > + * controls::controls, false otherwise\n> > \n> > controls::controls ?\n> \n> Oh, it should be nullptr here. I was thinking about ControlLists that\n> use controls::controls.\n\nAnd I'd drop this, as you never pass nullptr to this function.\n\n * \\return True if \\a infoMap is in the cache or false otherwise\n\n> > > + */\n> > > +bool ControlSerializer::isCached(const ControlInfoMap *infoMap)\n\nAnd I'd pass a const reference instead of a const pointer, to make sure\nit can never be null.\n\n> > > +{\n> > > +\tif (!infoMap)\n> > > +\t\treturn true;\n> > > +\n\nThis can be dropped.\n\n> > > +\tauto iter = infoMapHandles_.find(infoMap);\n> > > +\treturn iter != infoMapHandles_.end();\n\nYou can just return\n\n\treturn infoMapHandles_.count(infoMap);\n\nReviewed-by: Laurent Pinchart \n\n> > > +}\n> > > +\n> > > } /* namespace libcamera */","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 E1704C33BB\n\tfor ;\n\tMon, 1 Feb 2021 21:41:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7642268417;\n\tMon, 1 Feb 2021 22:41:04 +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 687E268414\n\tfor ;\n\tMon, 1 Feb 2021 22:41:03 +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 CA05B556;\n\tMon, 1 Feb 2021 22:41:02 +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=\"YqQw2tHo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612215663;\n\tbh=BujnV+VYgQP3h1B9mWeXiC5sne566uQJiqV0w56V3b0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YqQw2tHoAVdxZv1+ihPNQCVE/chbXM9VPOTZNO4xbEVt2w5mCAeG446TcrsmKiOIP\n\tJFsm1MpD1cIXBoyD0KTJPavB2bHMCNDw6LLoM1pSW2S0pegZD2Nqv+nNn5ZKuJcZmS\n\tTdjUB6cCu1te1CVl7C0wcU5ksGYSH0sohRl3A/eg=","Date":"Mon, 1 Feb 2021 23:40:41 +0200","From":"Laurent Pinchart ","To":"paul.elder@ideasonboard.com","Message-ID":"","References":"<20201224081534.41601-1-paul.elder@ideasonboard.com>\n\t<20201224081534.41601-2-paul.elder@ideasonboard.com>\n\t <20201228094811.GJ1933@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201228094811.GJ1933@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH v6 1/9] libcamera: control_serializer:\n\tSave serialized ControlInfoMap in a cache","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]