[{"id":15771,"web_url":"https://patchwork.libcamera.org/comment/15771/","msgid":"<20210319170357.vvgniin54dmduw3z@basti-TUXEDO-Book-XA1510>","date":"2021-03-19T17:03:57","subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Extend docs how\n\tto identify controls from ControlList","submitter":{"id":78,"url":"https://patchwork.libcamera.org/api/people/78/","name":"Sebastian Fricke","email":"sebastian.fricke@posteo.net"},"content":"Hey Marian,\n\nThank you for the patch.\n\nOn 19.03.2021 17:40, Marian Cichy wrote:\n>Informations how to identify Controls from a ControlList is quite\n\ns/Informations how/Informations on how/\ns/is quite/are quite/\n\n>scattered around the documentation and not clear if one reads about the\n\ns/and not clear/and it is not clear/\n\n>ControlList. Referring to ControlId and ControlIdMap right in the\n>detailed description is probably very helpful.\n\nagreed :)\n\n>\n>Signed-off-by: Marian Cichy \n>---\n> src/libcamera/controls.cpp | 4 ++++\n> 1 file changed, 4 insertions(+)\n>\n>diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n>index c58ed394..b5253f83 100644\n>--- a/src/libcamera/controls.cpp\n>+++ b/src/libcamera/controls.cpp\n>@@ -805,6 +805,10 @@ ControlList::ControlList()\n> * For ControlList containing libcamera controls, a global map of all libcamera\n> * controls is provided by controls::controls and can be used as the \\a idmap\n> * argument.\n>+ *\n>+ * To identify a Control from the ControlList, one needs to find the ControlId\n>+ * from the numerical control id saved in this list. A global ControlIdMap\n>+ * of all libcamera controls is provided by controls::controls.\n\nThis sentence seems to be very similar to the sentence two rows above.\nHow about:\n```\n * For ControlList containing libcamera controls, a global map of all libcamera\n * controls is provided by controls::controls and can be used as the \\a idmap\n * argument.\n * In order to identify a single control from the ControlList, one needs to find\n * the ControlId using the contains method in the numerical control IDs of that\n * map.\n```\n\nI feel like when we are at it, just mention how you are supposed to do\nit.\n\n> */\n> ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator)\n> \t: validator_(validator), idmap_(&idmap), infoMap_(nullptr)\n>-- \n>2.29.2\n\nGreetings,\nSebastian\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 094B4C32E1\n\tfor ;\n\tFri, 19 Mar 2021 17:04:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3DDDB68D66;\n\tFri, 19 Mar 2021 18:04:03 +0100 (CET)","from mout01.posteo.de (mout01.posteo.de [185.67.36.65])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C39596051E\n\tfor ;\n\tFri, 19 Mar 2021 18:04:01 +0100 (CET)","from submission (posteo.de [89.146.220.130]) \n\tby mout01.posteo.de (Postfix) with ESMTPS id 48E86160061\n\tfor ;\n\tFri, 19 Mar 2021 18:04:01 +0100 (CET)","from customer (localhost [127.0.0.1])\n\tby submission (posteo.de) with ESMTPSA id 4F29GS5QqNz9rxV;\n\tFri, 19 Mar 2021 18:04:00 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=posteo.net header.i=@posteo.net\n\theader.b=\"KG9MT3xt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017;\n\tt=1616173441; bh=An/g1HT86Kq1wxqLYBs5AhUGnP8Hg5XhfYmtONWgZX0=;\n\th=Date:From:To:Cc:Subject:From;\n\tb=KG9MT3xt4DVOEOpeUtYR7H/N1yCLcLvs1GlbRp4095c0G4zpgyOMBj1AZw/G9RsDi\n\t2LeiglLhhw8RGsBIr91qziKkV+MRNBqT8euZcMnlcCB2ZVB+oelBnsrlzn4OAqtda/\n\tEaadbMEPvow+YUOR9wGR3DlQZ44115+QMWSpSYEE8SZYKXand1AmrydNfKKxZsXPNm\n\tT+By4YY5uiiXc2K2hX43ShQTAkUYPUyqbLptzRL3aTglBULbpaclbEYvEIlA/RozkN\n\tBJpjQDgvOo1aEL62Jn+1e34F6D5xfh4bfz9FqS8r2MEPwpG3D6HtrvJQZctB2hQmtX\n\tUQTKcS+Hwaydg==","Date":"Fri, 19 Mar 2021 18:03:57 +0100","From":"Sebastian Fricke ","To":"Marian Cichy ","Message-ID":"<20210319170357.vvgniin54dmduw3z@basti-TUXEDO-Book-XA1510>","References":"<20210319164046.5626-1-m.cichy@pengutronix.de>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210319164046.5626-1-m.cichy@pengutronix.de>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Extend docs how\n\tto identify controls from ControlList","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, graphics@pengutronix.de","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":15773,"web_url":"https://patchwork.libcamera.org/comment/15773/","msgid":"","date":"2021-03-20T20:31:51","subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Extend docs how\n\tto identify controls from ControlList","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Marian,\n\nThank you for the patch.\n\nOn Fri, Mar 19, 2021 at 06:03:57PM +0100, Sebastian Fricke wrote:\n> On 19.03.2021 17:40, Marian Cichy wrote:\n> > Informations how to identify Controls from a ControlList is quite\n> \n> s/Informations how/Informations on how/\n> s/is quite/are quite/\n> \n> > scattered around the documentation and not clear if one reads about the\n> \n> s/and not clear/and it is not clear/\n> \n> > ControlList. Referring to ControlId and ControlIdMap right in the\n> > detailed description is probably very helpful.\n> \n> agreed :)\n> \n> > Signed-off-by: Marian Cichy \n> > ---\n> > src/libcamera/controls.cpp | 4 ++++\n> > 1 file changed, 4 insertions(+)\n> > \n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index c58ed394..b5253f83 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -805,6 +805,10 @@ ControlList::ControlList()\n> > * For ControlList containing libcamera controls, a global map of all libcamera\n> > * controls is provided by controls::controls and can be used as the \\a idmap\n> > * argument.\n> > + *\n> > + * To identify a Control from the ControlList, one needs to find the ControlId\n> > + * from the numerical control id saved in this list. A global ControlIdMap\n> > + * of all libcamera controls is provided by controls::controls.\n> \n> This sentence seems to be very similar to the sentence two rows above.\n> How about:\n> ```\n> * For ControlList containing libcamera controls, a global map of all libcamera\n> * controls is provided by controls::controls and can be used as the \\a idmap\n> * argument.\n> * In order to identify a single control from the ControlList, one needs to find\n> * the ControlId using the contains method in the numerical control IDs of that\n> * map.\n> ```\n> \n> I feel like when we are at it, just mention how you are supposed to do\n> it.\n\nFurthermore, usage information shouldn't be documented in one particular\nconstructor, but at the class level, or even file level. I think file\nlevel would be more appropriate here, to tie together Control, ControlId\nand ControlList.\n\n> > */\n> > ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator)\n> > \t: validator_(validator), idmap_(&idmap), infoMap_(nullptr)","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 1DB01C32E1\n\tfor ;\n\tSat, 20 Mar 2021 20:32:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 64E4E68D60;\n\tSat, 20 Mar 2021 21:32:33 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AF60C602DB\n\tfor ;\n\tSat, 20 Mar 2021 21:32:31 +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 1FF608D3;\n\tSat, 20 Mar 2021 21:32:31 +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=\"lzvNU4bZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616272351;\n\tbh=2GwN5KPea1pGoUAK8QVFduKCz3y6AIb6VM5HvU1nPVE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lzvNU4bZHDCYROFXyPJeq4tKV73O4jHbdN+7mscItHU6augBn5HY4zF3k8nB2FW5F\n\t64yTZdSOD9OxVJCEccWtd/OqQJ1hK5IWor9znoaAQL84dSHxs0YaZP3yOrLjw36U4j\n\tLMOlgQAMvlBPcqgXGvtAtgnwnJELKu9W5kA/sdGc=","Date":"Sat, 20 Mar 2021 22:31:51 +0200","From":"Laurent Pinchart ","To":"Marian Cichy ","Message-ID":"","References":"<20210319164046.5626-1-m.cichy@pengutronix.de>\n\t<20210319170357.vvgniin54dmduw3z@basti-TUXEDO-Book-XA1510>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210319170357.vvgniin54dmduw3z@basti-TUXEDO-Book-XA1510>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Extend docs how\n\tto identify controls from ControlList","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, graphics@pengutronix.de","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15792,"web_url":"https://patchwork.libcamera.org/comment/15792/","msgid":"<5dad517a-3754-7400-0c44-df6758673f75@pengutronix.de>","date":"2021-03-22T14:06:02","subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Extend docs how\n\tto identify controls from ControlList","submitter":{"id":81,"url":"https://patchwork.libcamera.org/api/people/81/","name":"Marian Cichy","email":"mci@pengutronix.de"},"content":"On 3/19/21 6:03 PM, Sebastian Fricke wrote:\n> Hey Marian,\n>\n> Thank you for the patch.\n>\n> On 19.03.2021 17:40, Marian Cichy wrote:\n>> Informations how to identify Controls from a ControlList is quite\n>\n> s/Informations how/Informations on how/\n> s/is quite/are quite/\n>\n>> scattered around the documentation and not clear if one reads about the\n>\n> s/and not clear/and it is not clear/\n>\n>> ControlList. Referring to ControlId and ControlIdMap right in the\n>> detailed description is probably very helpful.\n>\n> agreed :)\n>\n>>\n>> Signed-off-by: Marian Cichy \n>> ---\n>> src/libcamera/controls.cpp | 4 ++++\n>> 1 file changed, 4 insertions(+)\n>>\n>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n>> index c58ed394..b5253f83 100644\n>> --- a/src/libcamera/controls.cpp\n>> +++ b/src/libcamera/controls.cpp\n>> @@ -805,6 +805,10 @@ ControlList::ControlList()\n>>  * For ControlList containing libcamera controls, a global map of all \n>> libcamera\n>>  * controls is provided by controls::controls and can be used as the \n>> \\a idmap\n>>  * argument.\n>> + *\n>> + * To identify a Control from the ControlList, one needs to find the \n>> ControlId\n>> + * from the numerical control id saved in this list. A global \n>> ControlIdMap\n>> + * of all libcamera controls is provided by controls::controls.\n>\n> This sentence seems to be very similar to the sentence two rows above.\n> How about:\n> ```\n>  * For ControlList containing libcamera controls, a global map of all \n> libcamera\n>  * controls is provided by controls::controls and can be used as the \n> \\a idmap\n>  * argument.\n>  * In order to identify a single control from the ControlList, one \n> needs to find\n>  * the ControlId using the contains method in the numerical control \n> IDs of that\n>  * map.\n> ```\n>\n\nNot sure, why you would recommend contains() here, what are you trying \nto achieve? contains() won't give me any information about the mapped ID \nin any direction, right?\n\nAs an example, I'd write a snippet to merely iterate over all items in \nControlList and use each numerical ID with controls::controls.at(). This \nwas for me my first approach when I wanted to print the names of all my \ncontrols.\n\nRegards,\nMarian\n\n> I feel like when we are at it, just mention how you are supposed to do\n> it.\n>\n>>  */\n>> ControlList::ControlList(const ControlIdMap &idmap, ControlValidator \n>> *validator)\n>>     : validator_(validator), idmap_(&idmap), infoMap_(nullptr)\n>> -- \n>> 2.29.2\n>\n> Greetings,\n> Sebastian\n>>\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\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 85899C32E1\n\tfor ;\n\tMon, 22 Mar 2021 14:06:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D8D9B68D62;\n\tMon, 22 Mar 2021 15:06:05 +0100 (CET)","from metis.ext.pengutronix.de (metis.ext.pengutronix.de\n\t[IPv6:2001:67c:670:201:290:27ff:fe1d:cc33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DD00468D50\n\tfor ;\n\tMon, 22 Mar 2021 15:06:03 +0100 (CET)","from erdnuss.hi.pengutronix.de\n\t([2001:67c:670:100:2e4d:54ff:fe9d:849c])\n\tby metis.ext.pengutronix.de with esmtp (Exim 4.92)\n\t(envelope-from )\n\tid 1lOLBz-0001k6-7B; Mon, 22 Mar 2021 15:06:03 +0100"],"To":"Sebastian Fricke ,\n\tMarian Cichy ","References":"<20210319164046.5626-1-m.cichy@pengutronix.de>\n\t<20210319170357.vvgniin54dmduw3z@basti-TUXEDO-Book-XA1510>","From":"Marian Cichy ","Message-ID":"<5dad517a-3754-7400-0c44-df6758673f75@pengutronix.de>","Date":"Mon, 22 Mar 2021 15:06:02 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.8.0","MIME-Version":"1.0","In-Reply-To":"<20210319170357.vvgniin54dmduw3z@basti-TUXEDO-Book-XA1510>","Content-Language":"en-US","X-SA-Exim-Connect-IP":"2001:67c:670:100:2e4d:54ff:fe9d:849c","X-SA-Exim-Mail-From":"mci@pengutronix.de","X-SA-Exim-Scanned":"No (on metis.ext.pengutronix.de);\n\tSAEximRunCond expanded to false","X-PTX-Original-Recipient":"libcamera-devel@lists.libcamera.org","Subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Extend docs how\n\tto identify controls from ControlList","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, graphics@pengutronix.de","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]