[{"id":2843,"web_url":"https://patchwork.libcamera.org/comment/2843/","msgid":"<20191009092739.hwndzn6zyi24fwb5@uno.localdomain>","date":"2019-10-09T09:27:39","subject":"Re: [libcamera-devel] [PATCH 9/9] libcamera: v4l2_controls: Add\n\tcontrol validator","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Oct 08, 2019 at 01:46:42AM +0300, Laurent Pinchart wrote:\n> Add a ControlValidator specialisation that validates controls based on a\n> V4L2 device.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/include/v4l2_controls.h | 14 ++++++++++++\n> src/libcamera/v4l2_controls.cpp | 33 +++++++++++++++++++++++++++\n> 2 files changed, 47 insertions(+)\n>\n> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> index 133ab9ec208b..5fd92e3ac4e5 100644\n> --- a/src/libcamera/include/v4l2_controls.h\n> +++ b/src/libcamera/include/v4l2_controls.h\n> @@ -17,6 +17,8 @@\n>\n> #include \n>\n> +#include \"control_validator.h\"\n> +\n> namespace libcamera {\n>\n> class V4L2ControlId : public ControlId\n> @@ -57,6 +59,18 @@ private:\n> \tconst V4L2ControlInfoMap &controls_;\n> };\n>\n> +class V4L2ControlValidator final : public ControlValidator\n> +{\n> +public:\n> +\tV4L2ControlValidator(V4L2Device *device);\n> +\n> +\tconst std::string &name() const override;\n> +\tbool validate(const ControlId &id) const override;\n> +\n> +private:\n> +\tV4L2Device *dev_;\n> +};\n> +\n> } /* namespace libcamera */\n>\n> #endif /* __LIBCAMERA_V4L2_CONTROLS_H__ */\n> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> index d3f94709e821..498567ee0bf0 100644\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ b/src/libcamera/v4l2_controls.cpp\n> @@ -239,4 +239,37 @@ void V4L2ControlList::set(unsigned int id, const ControlValue &value)\n> \tControlList::set(ctrl->second.id(), value);\n> }\n>\n> +/**\n> + * \\class V4L2ControlValidator\n> + * \\brief A control validator for V4L2Devoe instances\n\nV4L2Device\n\n> + *\n> + * This ControlValidator specialisation validates that controls exist in the\n> + * V4L2Device associated with the validator.\n> + */\n> +\n> +/**\n> + * \\brief Construst a V4L2ControlValidator for the V4L2 \\a device\n> + * \\param[in] device The V4L2 device\n> + */\n> +V4L2ControlValidator::V4L2ControlValidator(V4L2Device *device)\n> +\t: dev_(device)\n> +{\n> +}\n> +\n> +const std::string &V4L2ControlValidator::name() const\n> +{\n> +\treturn dev_->deviceNode();\n> +}\n> +\n> +/**\n> + * \\brief Validate a control\n> + * \\param[in] id The control ID\n> + * \\return True if the control is valid, false otherwise\n\nI would expand what \"valid\" means in this context.\nAll validators 'validates' controls, this one does it against the list\nof controls supported by the video device.\n\nThanks\n j\n\n\n> + */\n> +bool V4L2ControlValidator::validate(const ControlId &id) const\n> +{\n> +\tconst V4L2ControlInfoMap &controls = dev_->controls();\n> +\treturn controls.find(id.id()) != controls.end();\n\nThe fact std::map::contains(T key) has only been introduced in C++ is\nreally a shame, we have this find() == end() pattern everywhere :(\n\n> +}\n> +\n> } /* namespace libcamera */\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2CBE66157B\n\tfor ;\n\tWed, 9 Oct 2019 11:25:54 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id B402D240010;\n\tWed, 9 Oct 2019 09:25:53 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 9 Oct 2019 11:27:39 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191009092739.hwndzn6zyi24fwb5@uno.localdomain>","References":"<20191007224642.6597-1-laurent.pinchart@ideasonboard.com>\n\t<20191007224642.6597-10-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"yb6ncldfh4m3d3ti\"","Content-Disposition":"inline","In-Reply-To":"<20191007224642.6597-10-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 9/9] libcamera: v4l2_controls: Add\n\tcontrol validator","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, 09 Oct 2019 09:25:54 -0000"}},{"id":2846,"web_url":"https://patchwork.libcamera.org/comment/2846/","msgid":"<20191009094836.GK22998@pendragon.ideasonboard.com>","date":"2019-10-09T09:48:36","subject":"Re: [libcamera-devel] [PATCH 9/9] libcamera: v4l2_controls: Add\n\tcontrol validator","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Oct 09, 2019 at 11:27:39AM +0200, Jacopo Mondi wrote:\n> On Tue, Oct 08, 2019 at 01:46:42AM +0300, Laurent Pinchart wrote:\n> > Add a ControlValidator specialisation that validates controls based on a\n> > V4L2 device.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/include/v4l2_controls.h | 14 ++++++++++++\n> > src/libcamera/v4l2_controls.cpp | 33 +++++++++++++++++++++++++++\n> > 2 files changed, 47 insertions(+)\n> >\n> > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> > index 133ab9ec208b..5fd92e3ac4e5 100644\n> > --- a/src/libcamera/include/v4l2_controls.h\n> > +++ b/src/libcamera/include/v4l2_controls.h\n> > @@ -17,6 +17,8 @@\n> >\n> > #include \n> >\n> > +#include \"control_validator.h\"\n> > +\n> > namespace libcamera {\n> >\n> > class V4L2ControlId : public ControlId\n> > @@ -57,6 +59,18 @@ private:\n> > \tconst V4L2ControlInfoMap &controls_;\n> > };\n> >\n> > +class V4L2ControlValidator final : public ControlValidator\n> > +{\n> > +public:\n> > +\tV4L2ControlValidator(V4L2Device *device);\n> > +\n> > +\tconst std::string &name() const override;\n> > +\tbool validate(const ControlId &id) const override;\n> > +\n> > +private:\n> > +\tV4L2Device *dev_;\n> > +};\n> > +\n> > } /* namespace libcamera */\n> >\n> > #endif /* __LIBCAMERA_V4L2_CONTROLS_H__ */\n> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> > index d3f94709e821..498567ee0bf0 100644\n> > --- a/src/libcamera/v4l2_controls.cpp\n> > +++ b/src/libcamera/v4l2_controls.cpp\n> > @@ -239,4 +239,37 @@ void V4L2ControlList::set(unsigned int id, const ControlValue &value)\n> > \tControlList::set(ctrl->second.id(), value);\n> > }\n> >\n> > +/**\n> > + * \\class V4L2ControlValidator\n> > + * \\brief A control validator for V4L2Devoe instances\n> \n> V4L2Device\n\nProbably better than introducing a V4L2Devoe class, yes.\n\n> > + *\n> > + * This ControlValidator specialisation validates that controls exist in the\n> > + * V4L2Device associated with the validator.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Construst a V4L2ControlValidator for the V4L2 \\a device\n> > + * \\param[in] device The V4L2 device\n> > + */\n> > +V4L2ControlValidator::V4L2ControlValidator(V4L2Device *device)\n> > +\t: dev_(device)\n> > +{\n> > +}\n> > +\n> > +const std::string &V4L2ControlValidator::name() const\n> > +{\n> > +\treturn dev_->deviceNode();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Validate a control\n> > + * \\param[in] id The control ID\n> > + * \\return True if the control is valid, false otherwise\n> \n> I would expand what \"valid\" means in this context.\n> All validators 'validates' controls, this one does it against the list\n> of controls supported by the video device.\n\n\"A control is considered valid if it is supported by the V4L2Device\nassociated with the validator.\" ?\n\n> > + */\n> > +bool V4L2ControlValidator::validate(const ControlId &id) const\n> > +{\n> > +\tconst V4L2ControlInfoMap &controls = dev_->controls();\n> > +\treturn controls.find(id.id()) != controls.end();\n> \n> The fact std::map::contains(T key) has only been introduced in C++ is\n> really a shame, we have this find() == end() pattern everywhere :(\n\nI know :-( At some point we could bump our minimal C++ version, but\nC++20 is definitely too recent.\n\n> > +}\n> > +\n> > } /* namespace libcamera */","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 A4B326157B\n\tfor ;\n\tWed, 9 Oct 2019 11:48:37 +0200 (CEST)","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 28E174FF;\n\tWed, 9 Oct 2019 11:48:37 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1570614517;\n\tbh=i35cCnKWFcbQXM23fuo68+RxJykWGICg3wg/IimGkmA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OM9D/YB1MMTC+qZzfjjBLtZxS9wqw+/wTVjiCyWThIwn+l4M1TIcj4CVRQQjyaFTx\n\teHcMpipm0cg95K2TJ7FgfGcUm3RiYLdIzTxZ0EPeWeHVQOEy4RV7uBBYgeNG2Orz5p\n\tn88G+huZFMXQmbKgYXYzP+o1O2Uz13bNz+YjjEfg=","Date":"Wed, 9 Oct 2019 12:48:36 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191009094836.GK22998@pendragon.ideasonboard.com>","References":"<20191007224642.6597-1-laurent.pinchart@ideasonboard.com>\n\t<20191007224642.6597-10-laurent.pinchart@ideasonboard.com>\n\t<20191009092739.hwndzn6zyi24fwb5@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191009092739.hwndzn6zyi24fwb5@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 9/9] libcamera: v4l2_controls: Add\n\tcontrol validator","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, 09 Oct 2019 09:48:37 -0000"}},{"id":2848,"web_url":"https://patchwork.libcamera.org/comment/2848/","msgid":"<20191009095923.7rrhge4sz5qao3jx@uno.localdomain>","date":"2019-10-09T09:59:23","subject":"Re: [libcamera-devel] [PATCH 9/9] libcamera: v4l2_controls: Add\n\tcontrol validator","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Wed, Oct 09, 2019 at 12:48:36PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Wed, Oct 09, 2019 at 11:27:39AM +0200, Jacopo Mondi wrote:\n> > On Tue, Oct 08, 2019 at 01:46:42AM +0300, Laurent Pinchart wrote:\n> > > Add a ControlValidator specialisation that validates controls based on a\n> > > V4L2 device.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/libcamera/include/v4l2_controls.h | 14 ++++++++++++\n> > > src/libcamera/v4l2_controls.cpp | 33 +++++++++++++++++++++++++++\n> > > 2 files changed, 47 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> > > index 133ab9ec208b..5fd92e3ac4e5 100644\n> > > --- a/src/libcamera/include/v4l2_controls.h\n> > > +++ b/src/libcamera/include/v4l2_controls.h\n> > > @@ -17,6 +17,8 @@\n> > >\n> > > #include \n> > >\n> > > +#include \"control_validator.h\"\n> > > +\n> > > namespace libcamera {\n> > >\n> > > class V4L2ControlId : public ControlId\n> > > @@ -57,6 +59,18 @@ private:\n> > > \tconst V4L2ControlInfoMap &controls_;\n> > > };\n> > >\n> > > +class V4L2ControlValidator final : public ControlValidator\n> > > +{\n> > > +public:\n> > > +\tV4L2ControlValidator(V4L2Device *device);\n> > > +\n> > > +\tconst std::string &name() const override;\n> > > +\tbool validate(const ControlId &id) const override;\n> > > +\n> > > +private:\n> > > +\tV4L2Device *dev_;\n> > > +};\n> > > +\n> > > } /* namespace libcamera */\n> > >\n> > > #endif /* __LIBCAMERA_V4L2_CONTROLS_H__ */\n> > > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> > > index d3f94709e821..498567ee0bf0 100644\n> > > --- a/src/libcamera/v4l2_controls.cpp\n> > > +++ b/src/libcamera/v4l2_controls.cpp\n> > > @@ -239,4 +239,37 @@ void V4L2ControlList::set(unsigned int id, const ControlValue &value)\n> > > \tControlList::set(ctrl->second.id(), value);\n> > > }\n> > >\n> > > +/**\n> > > + * \\class V4L2ControlValidator\n> > > + * \\brief A control validator for V4L2Devoe instances\n> >\n> > V4L2Device\n>\n> Probably better than introducing a V4L2Devoe class, yes.\n>\n> > > + *\n> > > + * This ControlValidator specialisation validates that controls exist in the\n> > > + * V4L2Device associated with the validator.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Construst a V4L2ControlValidator for the V4L2 \\a device\n> > > + * \\param[in] device The V4L2 device\n> > > + */\n> > > +V4L2ControlValidator::V4L2ControlValidator(V4L2Device *device)\n> > > +\t: dev_(device)\n> > > +{\n> > > +}\n> > > +\n> > > +const std::string &V4L2ControlValidator::name() const\n> > > +{\n> > > +\treturn dev_->deviceNode();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Validate a control\n> > > + * \\param[in] id The control ID\n> > > + * \\return True if the control is valid, false otherwise\n> >\n> > I would expand what \"valid\" means in this context.\n> > All validators 'validates' controls, this one does it against the list\n> > of controls supported by the video device.\n>\n> \"A control is considered valid if it is supported by the V4L2Device\n> associated with the validator.\" ?\n>\n\nBetter, thanks!\n\n> > > + */\n> > > +bool V4L2ControlValidator::validate(const ControlId &id) const\n> > > +{\n> > > +\tconst V4L2ControlInfoMap &controls = dev_->controls();\n> > > +\treturn controls.find(id.id()) != controls.end();\n> >\n> > The fact std::map::contains(T key) has only been introduced in C++ is\n> > really a shame, we have this find() == end() pattern everywhere :(\n>\n> I know :-( At some point we could bump our minimal C++ version, but\n> C++20 is definitely too recent.\n>\n\nNot just too recent, it's in the future!1!1\n\nThanks\n j\n\n> > > +}\n> > > +\n> > > } /* namespace libcamera */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3CB7E6157B\n\tfor ;\n\tWed, 9 Oct 2019 11:57:38 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id B7C924001C;\n\tWed, 9 Oct 2019 09:57:37 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 9 Oct 2019 11:59:23 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191009095923.7rrhge4sz5qao3jx@uno.localdomain>","References":"<20191007224642.6597-1-laurent.pinchart@ideasonboard.com>\n\t<20191007224642.6597-10-laurent.pinchart@ideasonboard.com>\n\t<20191009092739.hwndzn6zyi24fwb5@uno.localdomain>\n\t<20191009094836.GK22998@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"xrppzmofann34mcj\"","Content-Disposition":"inline","In-Reply-To":"<20191009094836.GK22998@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 9/9] libcamera: v4l2_controls: Add\n\tcontrol validator","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, 09 Oct 2019 09:57:38 -0000"}}]