[{"id":3179,"web_url":"https://patchwork.libcamera.org/comment/3179/","msgid":"<20191204162037.GG7811@pendragon.ideasonboard.com>","date":"2019-12-04T16:20:37","subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: camera: Add Camera\n\tproperties","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, Dec 04, 2019 at 02:21:04PM +0100, Jacopo Mondi wrote:\n> Add an operation to the Camera class to retrive the Camera properties\n\ns/an operation/a method/\ns/retrive/retrieve/\n\n> registered by the pipeline handler.\n> \n> While at it, rewords the Camera::controls() operation documentation to\n\ns/rewords/reword/\n\n> specify that the camera control information are constant during the\n> camera lifetime not their value, while the camera properties value are\n> the actually static information.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/camera.h | 1 +\n> src/libcamera/camera.cpp | 16 +++++++++++++++-\n> 2 files changed, 16 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index ef6a37bb142c..72d5d62cc902 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -85,6 +85,7 @@ public:\n> \tint release();\n> \n> \tconst ControlInfoMap &controls();\n> +\tconst ControlList &properties();\n> \n> \tconst std::set &streams() const;\n> \tstd::unique_ptr generateConfiguration(const StreamRoles &roles);\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index e810fb725d81..44031ffdb4b3 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -551,7 +551,8 @@ int Camera::release()\n> /**\n> * \\brief Retrieve the list of controls supported by the camera\n> *\n> - * Camera controls remain constant through the lifetime of the camera.\n> + * The list of controls supported by the camera and their associated\n> + * constraints remain constant through the lifetime of the camera.\n\nMaybe s/camera/Camera object/ ?\n\n> *\n> * \\return A ControlInfoMap listing the controls supported by the camera\n> */\n> @@ -560,6 +561,19 @@ const ControlInfoMap &Camera::controls()\n> \treturn pipe_->controls(this);\n> }\n> \n> +/**\n> + * \\brief Retrieve the list of properties of the camera\n> + *\n> + * Camera properties are static information that remain constant through the\n> + * lifetime of the camera.\n\nHow about\n\n * Camera properties are static information that describe the capabilities of\n * the camera. They remain constant through the lifetime of the Camera object.\n\n> + *\n> + * \\return A ControlList of properties supported by the camera\n> + */\n> +const ControlList &Camera::properties()\n> +{\n> +\treturn pipe_->properties(this);\n\nI think it would make sense at some point to let the Camera access\nCameraData, at which point we can short-circuit the pipeline handler for\nthis. It's out of scope for this patch series anyway.\n\nReviewed-by: Laurent Pinchart \n\n> +}\n> +\n> /**\n> * \\brief Retrieve all the camera's stream information\n> *","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8585860BFF\n\tfor ;\n\tWed, 4 Dec 2019 17:20:44 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E59132E5;\n\tWed, 4 Dec 2019 17:20:43 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1575476444;\n\tbh=UHJJMQslYfx42WcxN9FdjZcq1VNiiCV5fT188LElS10=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LtTmar5puGLPM3B3gttNkX4+77xtZUuAsE4vNbonyC1mgryEmeIARKxIX+LBQMlap\n\tnV3RvKleRI+/rTTKd6GpdevjxTNIMflQDqKe4faA2qvWRpbih8Ymyd0Mx19ZIdZAFH\n\tapNhk2HriDxqerB2EbbpU+ZCadkRRnyjk3QkedQE=","Date":"Wed, 4 Dec 2019 18:20:37 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191204162037.GG7811@pendragon.ideasonboard.com>","References":"<20191204132106.21582-1-jacopo@jmondi.org>\n\t<20191204132106.21582-9-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191204132106.21582-9-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: camera: Add Camera\n\tproperties","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","X-List-Received-Date":"Wed, 04 Dec 2019 16:20:44 -0000"}},{"id":3187,"web_url":"https://patchwork.libcamera.org/comment/3187/","msgid":"<20191205135513.pwzjv5flo5ktfml2@uno.localdomain>","date":"2019-12-05T13:55:59","subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: camera: Add Camera\n\tproperties","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n two minor notes on wording\n\nOn Wed, Dec 04, 2019 at 06:20:37PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Wed, Dec 04, 2019 at 02:21:04PM +0100, Jacopo Mondi wrote:\n> > Add an operation to the Camera class to retrive the Camera properties\n>\n> s/an operation/a method/\n\nI thought we stabilizd on 'operation' instead of 'method'\n\n> s/retrive/retrieve/\n>\n> > registered by the pipeline handler.\n> >\n> > While at it, rewords the Camera::controls() operation documentation to\n>\n> s/rewords/reword/\n>\n> > specify that the camera control information are constant during the\n> > camera lifetime not their value, while the camera properties value are\n> > the actually static information.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > include/libcamera/camera.h | 1 +\n> > src/libcamera/camera.cpp | 16 +++++++++++++++-\n> > 2 files changed, 16 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index ef6a37bb142c..72d5d62cc902 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -85,6 +85,7 @@ public:\n> > \tint release();\n> >\n> > \tconst ControlInfoMap &controls();\n> > +\tconst ControlList &properties();\n> >\n> > \tconst std::set &streams() const;\n> > \tstd::unique_ptr generateConfiguration(const StreamRoles &roles);\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index e810fb725d81..44031ffdb4b3 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -551,7 +551,8 @@ int Camera::release()\n> > /**\n> > * \\brief Retrieve the list of controls supported by the camera\n> > *\n> > - * Camera controls remain constant through the lifetime of the camera.\n> > + * The list of controls supported by the camera and their associated\n> > + * constraints remain constant through the lifetime of the camera.\n>\n> Maybe s/camera/Camera object/ ?\n>\n> > *\n> > * \\return A ControlInfoMap listing the controls supported by the camera\n> > */\n> > @@ -560,6 +561,19 @@ const ControlInfoMap &Camera::controls()\n> > \treturn pipe_->controls(this);\n> > }\n> >\n> > +/**\n> > + * \\brief Retrieve the list of properties of the camera\n> > + *\n> > + * Camera properties are static information that remain constant through the\n> > + * lifetime of the camera.\n>\n> How about\n>\n> * Camera properties are static information that describe the capabilities of\n> * the camera. They remain constant through the lifetime of the Camera object.\n\nI thought we use '$class instance' instead of 'object' as object is a\nJava terminology\n\nThanks\n j\n\n>\n> > + *\n> > + * \\return A ControlList of properties supported by the camera\n> > + */\n> > +const ControlList &Camera::properties()\n> > +{\n> > +\treturn pipe_->properties(this);\n>\n> I think it would make sense at some point to let the Camera access\n> CameraData, at which point we can short-circuit the pipeline handler for\n> this. It's out of scope for this patch series anyway.\n>\n> Reviewed-by: Laurent Pinchart \n>\n> > +}\n> > +\n> > /**\n> > * \\brief Retrieve all the camera's stream information\n> > *\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E437860BCA\n\tfor ;\n\tThu, 5 Dec 2019 14:53:49 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 4D8B61C000E;\n\tThu, 5 Dec 2019 13:53:49 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Thu, 5 Dec 2019 14:55:59 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191205135513.pwzjv5flo5ktfml2@uno.localdomain>","References":"<20191204132106.21582-1-jacopo@jmondi.org>\n\t<20191204132106.21582-9-jacopo@jmondi.org>\n\t<20191204162037.GG7811@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"vq3pw3omsfibzwng\"","Content-Disposition":"inline","In-Reply-To":"<20191204162037.GG7811@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: camera: Add Camera\n\tproperties","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","X-List-Received-Date":"Thu, 05 Dec 2019 13:53:50 -0000"}},{"id":3189,"web_url":"https://patchwork.libcamera.org/comment/3189/","msgid":"<20191205144551.GI4734@pendragon.ideasonboard.com>","date":"2019-12-05T14:45:51","subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: camera: Add Camera\n\tproperties","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Dec 05, 2019 at 02:55:59PM +0100, Jacopo Mondi wrote:\n> On Wed, Dec 04, 2019 at 06:20:37PM +0200, Laurent Pinchart wrote:\n> > On Wed, Dec 04, 2019 at 02:21:04PM +0100, Jacopo Mondi wrote:\n> > > Add an operation to the Camera class to retrive the Camera properties\n> >\n> > s/an operation/a method/\n> \n> I thought we stabilizd on 'operation' instead of 'method'\n\nIf we have I haven't noticed :-) We use the word 'operation' extensively\nfor IPA operations, but when it comes to C++ member methods, they're\njust called methods as far as I know.\n\n> > s/retrive/retrieve/\n> >\n> > > registered by the pipeline handler.\n> > >\n> > > While at it, rewords the Camera::controls() operation documentation to\n> >\n> > s/rewords/reword/\n> >\n> > > specify that the camera control information are constant during the\n> > > camera lifetime not their value, while the camera properties value are\n> > > the actually static information.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > include/libcamera/camera.h | 1 +\n> > > src/libcamera/camera.cpp | 16 +++++++++++++++-\n> > > 2 files changed, 16 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > index ef6a37bb142c..72d5d62cc902 100644\n> > > --- a/include/libcamera/camera.h\n> > > +++ b/include/libcamera/camera.h\n> > > @@ -85,6 +85,7 @@ public:\n> > > \tint release();\n> > >\n> > > \tconst ControlInfoMap &controls();\n> > > +\tconst ControlList &properties();\n> > >\n> > > \tconst std::set &streams() const;\n> > > \tstd::unique_ptr generateConfiguration(const StreamRoles &roles);\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index e810fb725d81..44031ffdb4b3 100644\n> > > --- a/src/libcamera/camera.cpp\n> > > +++ b/src/libcamera/camera.cpp\n> > > @@ -551,7 +551,8 @@ int Camera::release()\n> > > /**\n> > > * \\brief Retrieve the list of controls supported by the camera\n> > > *\n> > > - * Camera controls remain constant through the lifetime of the camera.\n> > > + * The list of controls supported by the camera and their associated\n> > > + * constraints remain constant through the lifetime of the camera.\n> >\n> > Maybe s/camera/Camera object/ ?\n> >\n> > > *\n> > > * \\return A ControlInfoMap listing the controls supported by the camera\n> > > */\n> > > @@ -560,6 +561,19 @@ const ControlInfoMap &Camera::controls()\n> > > \treturn pipe_->controls(this);\n> > > }\n> > >\n> > > +/**\n> > > + * \\brief Retrieve the list of properties of the camera\n> > > + *\n> > > + * Camera properties are static information that remain constant through the\n> > > + * lifetime of the camera.\n> >\n> > How about\n> >\n> > * Camera properties are static information that describe the capabilities of\n> > * the camera. They remain constant through the lifetime of the Camera object.\n> \n> I thought we use '$class instance' instead of 'object' as object is a\n> Java terminology\n\n'instance' works too, I don't care much, I think 'object' applies\nequally well to C++.\n\n> > > + *\n> > > + * \\return A ControlList of properties supported by the camera\n> > > + */\n> > > +const ControlList &Camera::properties()\n> > > +{\n> > > +\treturn pipe_->properties(this);\n> >\n> > I think it would make sense at some point to let the Camera access\n> > CameraData, at which point we can short-circuit the pipeline handler for\n> > this. It's out of scope for this patch series anyway.\n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > +}\n> > > +\n> > > /**\n> > > * \\brief Retrieve all the camera's stream information\n> > > *","headers":{"Return-Path":"","Received":["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 B9D0460BCA\n\tfor ;\n\tThu, 5 Dec 2019 15:45:58 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 209F02E5;\n\tThu, 5 Dec 2019 15:45:58 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1575557158;\n\tbh=huA4ElsBmyOEoWXOYOOcQ9ag82j7YAyOAQUDEHGO2BM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QWIuH7gGShSwx3hfjPD/Y5T6gkVQ1weHfnmsj9HAEsJV7Cogi6H0O2EjwrfUZQOtg\n\tDKVgoi5s/zLTYX80J4rqcckXxoKCLuw3Z1Ti0rexscI/6tYMdjnAWv9daLrTxSHJKZ\n\tsH7rwD+J5O7q7lv66fg4c7BZo6IXtiuV+OZ7+qqU=","Date":"Thu, 5 Dec 2019 16:45:51 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191205144551.GI4734@pendragon.ideasonboard.com>","References":"<20191204132106.21582-1-jacopo@jmondi.org>\n\t<20191204132106.21582-9-jacopo@jmondi.org>\n\t<20191204162037.GG7811@pendragon.ideasonboard.com>\n\t<20191205135513.pwzjv5flo5ktfml2@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191205135513.pwzjv5flo5ktfml2@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: camera: Add Camera\n\tproperties","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","X-List-Received-Date":"Thu, 05 Dec 2019 14:45:59 -0000"}}]