[{"id":18418,"web_url":"https://patchwork.libcamera.org/comment/18418/","msgid":"<20210729102359.GK2167@pyrite.rasen.tech>","date":"2021-07-29T10:23:59","subject":"Re: [libcamera-devel] [PATCH v2 3/5] test: control serialization:\n\tTest lookup by ControlId","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Jul 28, 2021 at 06:11:14PM +0200, Jacopo Mondi wrote:\n> Test that lookup by ControlId reference works in the control\n> serialization test.\n> \n> Also make sure that the control limits are not changed by\n> de-serialization.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> ---\n>  test/serialization/control_serialization.cpp | 22 ++++++++++++++++++++\n>  1 file changed, 22 insertions(+)\n> \n> diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp\n> index e23383d13bd6..45a706d27ba7 100644\n> --- a/test/serialization/control_serialization.cpp\n> +++ b/test/serialization/control_serialization.cpp\n> @@ -140,6 +140,28 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> +\t\t/* Test lookup by ControlId * on the de-serialized info map. */\n> +\t\tauto newLimitsIter = newInfoMap.find(&controls::Brightness);\n> +\t\tif (newLimitsIter == newInfoMap.end()) {\n> +\t\t\tcerr << \"Lookup by ControlId * failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tauto initialLimitsIter = infoMap.find(&controls::Brightness);\n> +\t\tif (initialLimitsIter == infoMap.end()) {\n> +\t\t\tcerr << \"Unable to retrieve the original control limits\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Make sure control limits looked up by id are not changed. */\n> +\t\tconst ControlInfo &newLimits = newLimitsIter->second;\n> +\t\tconst ControlInfo &initialLimits = initialLimitsIter->second;\n> +\t\tif (newLimits.min().get<float>() != initialLimits.min().get<float>() ||\n> +\t\t    newLimits.max().get<float>() != initialLimits.max().get<float>()) {\n> +\t\t\tcerr << \"The brightness control limits have changed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n>  \t\t/* Deserialize the control list and verify the contents. */\n>  \t\tbuffer = ByteStreamBuffer(const_cast<const uint8_t *>(listData.data()),\n>  \t\t\t\t\t  listData.size());\n> -- \n> 2.32.0\n>","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 2DC98C3230\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 29 Jul 2021 10:24:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 93C41687BF;\n\tThu, 29 Jul 2021 12:24:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 73236687BA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 29 Jul 2021 12:24:07 +0200 (CEST)","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 E8ADD9FB;\n\tThu, 29 Jul 2021 12:24:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"hWw6Z947\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627554247;\n\tbh=yrZogcYU+zUo8oH4baLfNqdi7CbkJAHw02lcA6WI6yo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hWw6Z947Gkp0NAtqFXH3Rj2OYaE3pi43RLsxdanjDhWo/2cvaeTfd+WWvoLEsdS7Z\n\t6qTcPCtqz1nwnPD6taKk58H9971+GOB4tMU4rwts3FYl0Pe4aY8gTABTex+IauvsSk\n\tR4wLKF0oqqAtntww2xufMKadO1ghsdXH99a4d5bA=","Date":"Thu, 29 Jul 2021 19:23:59 +0900","From":"paul.elder@ideasonboard.com","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20210729102359.GK2167@pyrite.rasen.tech>","References":"<20210728161116.64489-1-jacopo@jmondi.org>\n\t<20210728161116.64489-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210728161116.64489-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] test: control serialization:\n\tTest lookup by ControlId","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":18524,"web_url":"https://patchwork.libcamera.org/comment/18524/","msgid":"<YQlw3tkoKQPc+CIY@pendragon.ideasonboard.com>","date":"2021-08-03T16:37:50","subject":"Re: [libcamera-devel] [PATCH v2 3/5] test: control serialization:\n\tTest lookup by ControlId","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, Jul 28, 2021 at 06:11:14PM +0200, Jacopo Mondi wrote:\n> Test that lookup by ControlId reference works in the control\n> serialization test.\n> \n> Also make sure that the control limits are not changed by\n> de-serialization.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  test/serialization/control_serialization.cpp | 22 ++++++++++++++++++++\n>  1 file changed, 22 insertions(+)\n> \n> diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp\n> index e23383d13bd6..45a706d27ba7 100644\n> --- a/test/serialization/control_serialization.cpp\n> +++ b/test/serialization/control_serialization.cpp\n> @@ -140,6 +140,28 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> +\t\t/* Test lookup by ControlId * on the de-serialized info map. */\n> +\t\tauto newLimitsIter = newInfoMap.find(&controls::Brightness);\n> +\t\tif (newLimitsIter == newInfoMap.end()) {\n> +\t\t\tcerr << \"Lookup by ControlId * failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tauto initialLimitsIter = infoMap.find(&controls::Brightness);\n> +\t\tif (initialLimitsIter == infoMap.end()) {\n> +\t\t\tcerr << \"Unable to retrieve the original control limits\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n\nI think you can drop the check here, as this test is about\nserialization, not about testing basic usage of ControlInfoMap. You can\nthus write\n\n\t\tconst ControlInfo &initialLimits = infoMap[&controls::Brightness];\n\nbelow and drop initialLimitsIter.\n\n> +\n> +\t\t/* Make sure control limits looked up by id are not changed. */\n> +\t\tconst ControlInfo &newLimits = newLimitsIter->second;\n> +\t\tconst ControlInfo &initialLimits = initialLimitsIter->second;\n> +\t\tif (newLimits.min().get<float>() != initialLimits.min().get<float>() ||\n> +\t\t    newLimits.max().get<float>() != initialLimits.max().get<float>()) {\n\nYou could write\n\n\t\tif (newLimits.min() != initialLimits.min() ||\n\t\t    newLimits.max()) != initialLimits.max()) {\n\nCould you also move this patch to the beginning of the series to make\nsure the test fails ?\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\t\t\tcerr << \"The brightness control limits have changed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n>  \t\t/* Deserialize the control list and verify the contents. */\n>  \t\tbuffer = ByteStreamBuffer(const_cast<const uint8_t *>(listData.data()),\n>  \t\t\t\t\t  listData.size());","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 ECCB0C3235\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  3 Aug 2021 16:38:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5AEA1687CC;\n\tTue,  3 Aug 2021 18:38:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3DCDC6026A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  3 Aug 2021 18:38:03 +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 B1F4D3F0;\n\tTue,  3 Aug 2021 18:38:02 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZuTOYJPS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628008682;\n\tbh=eIoDF8pOzgA635tyKvaGJNQc9r4g227T51R3SYQMvwQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZuTOYJPSxR0lwP2D8lE5hmLOMf4ewFuBxOgPQYp0uJcZwr9T4bdsgfrgN1Ul1fzKK\n\tGxLH9pL1pH1/LSmy1CuFrwj2LKF1grfFPSMeKQu6cYECdwdG9fpWpetGJUspBU9BA2\n\tUQQgfx9UqUeoIeTNrE+KdMD/1BjonL8gfmJHZYqE=","Date":"Tue, 3 Aug 2021 19:37:50 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YQlw3tkoKQPc+CIY@pendragon.ideasonboard.com>","References":"<20210728161116.64489-1-jacopo@jmondi.org>\n\t<20210728161116.64489-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210728161116.64489-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] test: control serialization:\n\tTest lookup by ControlId","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":18623,"web_url":"https://patchwork.libcamera.org/comment/18623/","msgid":"<20210809141419.yosyja22dmahbs3p@uno.localdomain>","date":"2021-08-09T14:14:19","subject":"Re: [libcamera-devel] [PATCH v2 3/5] test: control serialization:\n\tTest lookup by ControlId","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Aug 03, 2021 at 07:37:50PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Wed, Jul 28, 2021 at 06:11:14PM +0200, Jacopo Mondi wrote:\n> > Test that lookup by ControlId reference works in the control\n> > serialization test.\n> >\n> > Also make sure that the control limits are not changed by\n> > de-serialization.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  test/serialization/control_serialization.cpp | 22 ++++++++++++++++++++\n> >  1 file changed, 22 insertions(+)\n> >\n> > diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp\n> > index e23383d13bd6..45a706d27ba7 100644\n> > --- a/test/serialization/control_serialization.cpp\n> > +++ b/test/serialization/control_serialization.cpp\n> > @@ -140,6 +140,28 @@ protected:\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >\n> > +\t\t/* Test lookup by ControlId * on the de-serialized info map. */\n> > +\t\tauto newLimitsIter = newInfoMap.find(&controls::Brightness);\n> > +\t\tif (newLimitsIter == newInfoMap.end()) {\n> > +\t\t\tcerr << \"Lookup by ControlId * failed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tauto initialLimitsIter = infoMap.find(&controls::Brightness);\n> > +\t\tif (initialLimitsIter == infoMap.end()) {\n> > +\t\t\tcerr << \"Unable to retrieve the original control limits\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n>\n> I think you can drop the check here, as this test is about\n> serialization, not about testing basic usage of ControlInfoMap. You can\n> thus write\n>\n> \t\tconst ControlInfo &initialLimits = infoMap[&controls::Brightness];\n\nNo I can't, we do not expose std::unordered_map::operator[] in\nControlInfoMap as it would allow to modify the ControlInfoMap. So I\nhave to go through at().\n\nI wonder if all the emphasis on making ControlInfoMap un-mutable once\nconstructed is still justified. The next item on my list is to update\nthe ControlInfo limits in response to a camera configuration, and we can only\ndo so by overwriting the full ControlInfoMap because of that...\n\n>\n> below and drop initialLimitsIter.\n>\n> > +\n> > +\t\t/* Make sure control limits looked up by id are not changed. */\n> > +\t\tconst ControlInfo &newLimits = newLimitsIter->second;\n> > +\t\tconst ControlInfo &initialLimits = initialLimitsIter->second;\n> > +\t\tif (newLimits.min().get<float>() != initialLimits.min().get<float>() ||\n> > +\t\t    newLimits.max().get<float>() != initialLimits.max().get<float>()) {\n>\n> You could write\n>\n> \t\tif (newLimits.min() != initialLimits.min() ||\n> \t\t    newLimits.max()) != initialLimits.max()) {\n>\n> Could you also move this patch to the beginning of the series to make\n> sure the test fails ?\n\nSure\n\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nThanks\n   j\n\n>\n> > +\t\t\tcerr << \"The brightness control limits have changed\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> >  \t\t/* Deserialize the control list and verify the contents. */\n> >  \t\tbuffer = ByteStreamBuffer(const_cast<const uint8_t *>(listData.data()),\n> >  \t\t\t\t\t  listData.size());\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 94C8AC3240\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  9 Aug 2021 14:13:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1A7F96884F;\n\tMon,  9 Aug 2021 16:13:33 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7B30160269\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  9 Aug 2021 16:13:31 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id E58AC100007;\n\tMon,  9 Aug 2021 14:13:30 +0000 (UTC)"],"Date":"Mon, 9 Aug 2021 16:14:19 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210809141419.yosyja22dmahbs3p@uno.localdomain>","References":"<20210728161116.64489-1-jacopo@jmondi.org>\n\t<20210728161116.64489-4-jacopo@jmondi.org>\n\t<YQlw3tkoKQPc+CIY@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<YQlw3tkoKQPc+CIY@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] test: control serialization:\n\tTest lookup by ControlId","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":18627,"web_url":"https://patchwork.libcamera.org/comment/18627/","msgid":"<YRE+aDPwMMd6NoVq@pendragon.ideasonboard.com>","date":"2021-08-09T14:40:40","subject":"Re: [libcamera-devel] [PATCH v2 3/5] test: control serialization:\n\tTest lookup by ControlId","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Aug 09, 2021 at 04:14:19PM +0200, Jacopo Mondi wrote:\n> On Tue, Aug 03, 2021 at 07:37:50PM +0300, Laurent Pinchart wrote:\n> > On Wed, Jul 28, 2021 at 06:11:14PM +0200, Jacopo Mondi wrote:\n> > > Test that lookup by ControlId reference works in the control\n> > > serialization test.\n> > >\n> > > Also make sure that the control limits are not changed by\n> > > de-serialization.\n> > >\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > ---\n> > >  test/serialization/control_serialization.cpp | 22 ++++++++++++++++++++\n> > >  1 file changed, 22 insertions(+)\n> > >\n> > > diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp\n> > > index e23383d13bd6..45a706d27ba7 100644\n> > > --- a/test/serialization/control_serialization.cpp\n> > > +++ b/test/serialization/control_serialization.cpp\n> > > @@ -140,6 +140,28 @@ protected:\n> > >  \t\t\treturn TestFail;\n> > >  \t\t}\n> > >\n> > > +\t\t/* Test lookup by ControlId * on the de-serialized info map. */\n> > > +\t\tauto newLimitsIter = newInfoMap.find(&controls::Brightness);\n> > > +\t\tif (newLimitsIter == newInfoMap.end()) {\n> > > +\t\t\tcerr << \"Lookup by ControlId * failed\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\tauto initialLimitsIter = infoMap.find(&controls::Brightness);\n> > > +\t\tif (initialLimitsIter == infoMap.end()) {\n> > > +\t\t\tcerr << \"Unable to retrieve the original control limits\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> >\n> > I think you can drop the check here, as this test is about\n> > serialization, not about testing basic usage of ControlInfoMap. You can\n> > thus write\n> >\n> > \t\tconst ControlInfo &initialLimits = infoMap[&controls::Brightness];\n> \n> No I can't, we do not expose std::unordered_map::operator[] in\n> ControlInfoMap as it would allow to modify the ControlInfoMap. So I\n> have to go through at().\n\nOops indeed.\n\n> I wonder if all the emphasis on making ControlInfoMap un-mutable once\n> constructed is still justified. The next item on my list is to update\n> the ControlInfo limits in response to a camera configuration, and we can only\n> do so by overwriting the full ControlInfoMap because of that...\n\nThat's a good question. It should certainly be read-only from an\napplication point of view, but that's enforced by returning a const\nreference from Camera::controls(). It would also be nice if the API\ncould lower the risk of making modifications by mistake in pipeline\nhandlers, but there we'll need to find a middle-ground anyway, as\nmodifications are needed. I believe we'll revisit this in any case,\ngiven that the updates to Camera::controls() that we're doing now if a\nbit of a hack and needs to be better designed.\n\n> > below and drop initialLimitsIter.\n> >\n> > > +\n> > > +\t\t/* Make sure control limits looked up by id are not changed. */\n> > > +\t\tconst ControlInfo &newLimits = newLimitsIter->second;\n> > > +\t\tconst ControlInfo &initialLimits = initialLimitsIter->second;\n> > > +\t\tif (newLimits.min().get<float>() != initialLimits.min().get<float>() ||\n> > > +\t\t    newLimits.max().get<float>() != initialLimits.max().get<float>()) {\n> >\n> > You could write\n> >\n> > \t\tif (newLimits.min() != initialLimits.min() ||\n> > \t\t    newLimits.max()) != initialLimits.max()) {\n> >\n> > Could you also move this patch to the beginning of the series to make\n> > sure the test fails ?\n> \n> Sure\n> \n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> Thanks\n> \n> > > +\t\t\tcerr << \"The brightness control limits have changed\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > >  \t\t/* Deserialize the control list and verify the contents. */\n> > >  \t\tbuffer = ByteStreamBuffer(const_cast<const uint8_t *>(listData.data()),\n> > >  \t\t\t\t\t  listData.size());","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 B6552C3240\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  9 Aug 2021 14:40:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2375A68826;\n\tMon,  9 Aug 2021 16:40:44 +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 9B28C60269\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  9 Aug 2021 16:40:42 +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 17FDEDD;\n\tMon,  9 Aug 2021 16:40:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oIsrAH/f\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628520042;\n\tbh=1bz3TEg0BhKxtFtnXpQ3dqUzt2oVbF6wUFAu/sOB7eE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oIsrAH/fLzEsFbRzwXap7udqFUAM24YN6ZJwC94YwCUoVCdg3B+NhoaIuFoBPsbFf\n\tsxEeT6GNCR7tZuFE8KPpDEHDqQjtWkodoR1IMJt3uXy6k25uxYvqF+V0dzITYhsQR0\n\tjurbrfPs8FJAlbNxLMNvv08clGHQtIsnZqiaE3S0=","Date":"Mon, 9 Aug 2021 17:40:40 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YRE+aDPwMMd6NoVq@pendragon.ideasonboard.com>","References":"<20210728161116.64489-1-jacopo@jmondi.org>\n\t<20210728161116.64489-4-jacopo@jmondi.org>\n\t<YQlw3tkoKQPc+CIY@pendragon.ideasonboard.com>\n\t<20210809141419.yosyja22dmahbs3p@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210809141419.yosyja22dmahbs3p@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] test: control serialization:\n\tTest lookup by ControlId","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>"}}]