[{"id":21011,"web_url":"https://patchwork.libcamera.org/comment/21011/","msgid":"","date":"2021-11-18T19:50:10","subject":"Re: [libcamera-devel] [PATCH] libcamera: media_device:\n\tDocumentation fix","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Thu, Nov 18, 2021 at 11:37:07PM +0530, Umang Jain wrote:\n> Fix a couple of sentence formations to clarify the intent.\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/libcamera/media_device.cpp | 12 ++++++------\n> 1 file changed, 6 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> index aa93da75..dd5def20 100644\n> --- a/src/libcamera/media_device.cpp\n> +++ b/src/libcamera/media_device.cpp\n> @@ -40,15 +40,15 @@ LOG_DEFINE_CATEGORY(MediaDevice)\n> * instance.\n> *\n> * The instance is created with an empty media graph. Before performing any\n> - * other operation, it must be populate by calling populate(). Instances of\n> + * other operation, it must be populated by calling populate(). Instances of\n\nOops. Good catch.\n\n> * MediaEntity, MediaPad and MediaLink are created to model the media graph,\n> * and stored in a map indexed by object id.\n> *\n> - * The graph is valid once successfully populated, as reported by the isValid()\n> - * function. It can be queried to list all entities(), or entities can be\n> - * looked up by name with getEntityByName(). The graph can be traversed from\n> - * entity to entity through pads and links as exposed by the corresponding\n> - * classes.\n> + * The graph is valid once it has been successfully populated,\n\nI think the sentence is grammatically correct already. Is it confusing ?\n\n> and is reported\n> + * as valid by the isValid() function. It can be queried to list all entities\n\nThat doesn't mean the same. The current text meant to say that the\nsuccessful population of the graph is reported by isValid().\n\n> + * with entities(), or entities can be looked up by name with getEntityByName()\n> + * function. The graph can be traversed from entity to entity through pads and\n> + * links as exposed by the corresponding classes.\n> *\n> * Media devices can be claimed for exclusive use with acquire(), released with\n> * release() and tested with busy(). This mechanism is aimed at pipeline","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 EC1C4BDB1C\n\tfor ;\n\tThu, 18 Nov 2021 19:50:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C26860371;\n\tThu, 18 Nov 2021 20:50:35 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E02C360231\n\tfor ;\n\tThu, 18 Nov 2021 20:50:33 +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 5103DE7;\n\tThu, 18 Nov 2021 20:50:33 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"LT1/9eij\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637265033;\n\tbh=ShaME50LU1YcbFgceJWThpV7/F9wnBQVUalG/tm5KhM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LT1/9eij7o4yrUemirqpzvx/FXfZKWYh8MwLocHZ2RMCw4eN8HOW7N7Mf64+zpDiS\n\tRLvzXq4wUpEB2JIsS4OwuyMcn0HzXsmstv3SGVy3iRD4wgEzyc7KmoE4N1mwS4lJ8+\n\txMiUXZRjyBABPKPYu0RknuKdarlWSzZXdAEqSVuM=","Date":"Thu, 18 Nov 2021 21:50:10 +0200","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20211118180707.348256-1-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211118180707.348256-1-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: media_device:\n\tDocumentation fix","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\" "}},{"id":21013,"web_url":"https://patchwork.libcamera.org/comment/21013/","msgid":"<0121a334-2e72-9eff-ae84-bfa387329145@ideasonboard.com>","date":"2021-11-19T05:21:13","subject":"Re: [libcamera-devel] [PATCH] libcamera: media_device:\n\tDocumentation fix","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 11/19/21 1:20 AM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Thu, Nov 18, 2021 at 11:37:07PM +0530, Umang Jain wrote:\n>> Fix a couple of sentence formations to clarify the intent.\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> src/libcamera/media_device.cpp | 12 ++++++------\n>> 1 file changed, 6 insertions(+), 6 deletions(-)\n>>\n>> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n>> index aa93da75..dd5def20 100644\n>> --- a/src/libcamera/media_device.cpp\n>> +++ b/src/libcamera/media_device.cpp\n>> @@ -40,15 +40,15 @@ LOG_DEFINE_CATEGORY(MediaDevice)\n>> * instance.\n>> *\n>> * The instance is created with an empty media graph. Before performing any\n>> - * other operation, it must be populate by calling populate(). Instances of\n>> + * other operation, it must be populated by calling populate(). Instances of\n> Oops. Good catch.\n>\n>> * MediaEntity, MediaPad and MediaLink are created to model the media graph,\n>> * and stored in a map indexed by object id.\n>> *\n>> - * The graph is valid once successfully populated, as reported by the isValid()\n>> - * function. It can be queried to list all entities(), or entities can be\n>> - * looked up by name with getEntityByName(). The graph can be traversed from\n>> - * entity to entity through pads and links as exposed by the corresponding\n>> - * classes.\n>> + * The graph is valid once it has been successfully populated,\n> I think the sentence is grammatically correct already. Is it confusing ?\n\n\nIt seems confusing to me: Other options were:\n\n     * The graph is valid once populated successfully, as reported by \nthe isValid() ...\n\nBut I don't think it goes with the explanation you gave below about \nisValid()\n>\n>> and is reported\n>> + * as valid by the isValid() function. It can be queried to list all entities\n> That doesn't mean the same. The current text meant to say that the\n> successful population of the graph is reported by isValid().\n\n\nOh. So I re-read the isValid() to clarify this -\n\n         /**\n          * \\fn MediaDevice::isValid()\n          * \\brief Query whether the media graph has been populated and \nis valid\n          * \\return true if the media graph is valid, false otherwise\n          */\n\nIt makes sense to me now, but the line above in the para, still seems a \nbit implicit to bring this meaning there.\n>\n>> + * with entities(), or entities can be looked up by name with getEntityByName()\n>> + * function. The graph can be traversed from entity to entity through pads and\n>> + * links as exposed by the corresponding classes.\n>> *\n>> * Media devices can be claimed for exclusive use with acquire(), released with\n>> * release() and tested with busy(). This mechanism is aimed at pipeline","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 513EABF415\n\tfor ;\n\tFri, 19 Nov 2021 05:21:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7BA7C60371;\n\tFri, 19 Nov 2021 06:21: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 527EF60121\n\tfor ;\n\tFri, 19 Nov 2021 06:21:19 +0100 (CET)","from [192.168.1.106] (unknown [103.251.226.87])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0DA4F5A7;\n\tFri, 19 Nov 2021 06:21:17 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MYlHms+O\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637299278;\n\tbh=UpV12ZoSxvB2qvoHcGLKyrKbgf24ULxE9NE4ILhxdys=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=MYlHms+OFKYDcFWo/8zT2eFnQQVr+zkt9LFSsomoc+HWqczxsPNEUoA9P2kTVUIjC\n\teXgkec14FH5KqqCf9KGl9pCEysgNmxrcM9EClGi9H09qIvTqf9ufZ5w/HVS1pdj3g8\n\td4cHW1fSEjSczYuoUW6vPXoppHaiGuXEz1g+wJxw=","To":"Laurent Pinchart ","References":"<20211118180707.348256-1-umang.jain@ideasonboard.com>\n\t","From":"Umang Jain ","Message-ID":"<0121a334-2e72-9eff-ae84-bfa387329145@ideasonboard.com>","Date":"Fri, 19 Nov 2021 10:51:13 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] libcamera: media_device:\n\tDocumentation fix","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\" "}},{"id":21048,"web_url":"https://patchwork.libcamera.org/comment/21048/","msgid":"<163733109574.1448748.7195271750401253580@Monstersaurus>","date":"2021-11-19T14:11:35","subject":"Re: [libcamera-devel] [PATCH] libcamera: media_device:\n\tDocumentation fix","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain (2021-11-19 05:21:13)\n> Hi Laurent,\n> \n> On 11/19/21 1:20 AM, Laurent Pinchart wrote:\n> > Hi Umang,\n> >\n> > Thank you for the patch.\n> >\n> > On Thu, Nov 18, 2021 at 11:37:07PM +0530, Umang Jain wrote:\n> >> Fix a couple of sentence formations to clarify the intent.\n> >>\n> >> Signed-off-by: Umang Jain \n> >> ---\n> >> src/libcamera/media_device.cpp | 12 ++++++------\n> >> 1 file changed, 6 insertions(+), 6 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> >> index aa93da75..dd5def20 100644\n> >> --- a/src/libcamera/media_device.cpp\n> >> +++ b/src/libcamera/media_device.cpp\n> >> @@ -40,15 +40,15 @@ LOG_DEFINE_CATEGORY(MediaDevice)\n> >> * instance.\n> >> *\n> >> * The instance is created with an empty media graph. Before performing any\n> >> - * other operation, it must be populate by calling populate(). Instances of\n> >> + * other operation, it must be populated by calling populate(). Instances of\n> > Oops. Good catch.\n> >\n> >> * MediaEntity, MediaPad and MediaLink are created to model the media graph,\n> >> * and stored in a map indexed by object id.\n> >> *\n> >> - * The graph is valid once successfully populated, as reported by the isValid()\n> >> - * function. It can be queried to list all entities(), or entities can be\n> >> - * looked up by name with getEntityByName(). The graph can be traversed from\n> >> - * entity to entity through pads and links as exposed by the corresponding\n> >> - * classes.\n> >> + * The graph is valid once it has been successfully populated,\n> > I think the sentence is grammatically correct already. Is it confusing ?\n> \n> \n\nAyyee wrapped lines are making that really hard to interpret.\n\nAs far as I can tell:\n\nOriginal:\n\n The graph is valid once successfully populated, as reported by the\n isValid() function.\n\nSuggested:\n\n The graph is valid once it has been successfully populated,\n and is reported as valid by the isValid() function.\n\ndiff:\n\nA) s/once/once it has been/\nB) s/as reported/and is reported/\n\n\nDiff-A certainly works for me.\n\nDiff-B makes it sound like calling isValid() is a requirement to\nmaking it valid it, which it isn't.\n\nTo me a clearer diff would be:\n\nB s/as reported/which is reported/\n\n\n\n> It seems confusing to me: Other options were:\n> \n>     * The graph is valid once populated successfully, as reported by \n> the isValid() ...\n> \n> But I don't think it goes with the explanation you gave below about \n> isValid()\n> >\n> >> and is reported\n> >> + * as valid by the isValid() function. It can be queried to list all entities\n> > That doesn't mean the same. The current text meant to say that the\n> > successful population of the graph is reported by isValid().\n> \n> \n> Oh. So I re-read the isValid() to clarify this -\n> \n>         /**\n>          * \\fn MediaDevice::isValid()\n>          * \\brief Query whether the media graph has been populated and \n> is valid\n>          * \\return true if the media graph is valid, false otherwise\n>          */\n> \n> It makes sense to me now, but the line above in the para, still seems a \n> bit implicit to bring this meaning there.\n> >\n> >> + * with entities(), or entities can be looked up by name with getEntityByName()\n> >> + * function. The graph can be traversed from entity to entity through pads and\n> >> + * links as exposed by the corresponding classes.\n> >> *\n> >> * Media devices can be claimed for exclusive use with acquire(), released with\n> >> * release() and tested with busy(). This mechanism is aimed at pipeline","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 2DB52BDB13\n\tfor ;\n\tFri, 19 Nov 2021 14:11:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2997760371;\n\tFri, 19 Nov 2021 15:11:40 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DBAE3600B5\n\tfor ;\n\tFri, 19 Nov 2021 15:11:38 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7601A1959;\n\tFri, 19 Nov 2021 15:11:38 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"heMqa8j+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637331098;\n\tbh=nPeKKAl71XBDlKMV2rPhye6y7x/1hKRbG7Xyk/jpXcg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=heMqa8j+4eDZ+unWnCVqdibrdSHtEzgHdzaK+1wS6xvj1gxnxIjDEdcXyYfAiFaig\n\tn/fYI2nGuJqSGWe2QxpbsZdapLg1sFnOX/iyP6ZHs6Vo/cv1LNZoM72h9Z7R2mkmYK\n\td3Yquku25S8S/JsdxMq3nZpukDJ3HHRuig+KiLQM=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<0121a334-2e72-9eff-ae84-bfa387329145@ideasonboard.com>","References":"<20211118180707.348256-1-umang.jain@ideasonboard.com>\n\t\n\t<0121a334-2e72-9eff-ae84-bfa387329145@ideasonboard.com>","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tUmang Jain ","Date":"Fri, 19 Nov 2021 14:11:35 +0000","Message-ID":"<163733109574.1448748.7195271750401253580@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] libcamera: media_device:\n\tDocumentation fix","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\" "}}]