[{"id":2728,"web_url":"https://patchwork.libcamera.org/comment/2728/","msgid":"<20190929101939.wqj5juxch3ava6kx@uno.localdomain>","date":"2019-09-29T10:19:39","subject":"Re: [libcamera-devel] [PATCH 11/12] libcamera: Add ControlValidator\n\timplementation for Camera","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Sat, Sep 28, 2019 at 06:22:37PM +0300, Laurent Pinchart wrote:\n> Add a new CameraControlValidator class that implements the\n> ControlValidator interface for a Camera object.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/libcamera/camera_controls.cpp       | 53 +++++++++++++++++++++++++\n>  src/libcamera/include/camera_controls.h | 30 ++++++++++++++\n>  src/libcamera/include/meson.build       |  1 +\n>  src/libcamera/meson.build               |  1 +\n>  4 files changed, 85 insertions(+)\n>  create mode 100644 src/libcamera/camera_controls.cpp\n>  create mode 100644 src/libcamera/include/camera_controls.h\n>\n> diff --git a/src/libcamera/camera_controls.cpp b/src/libcamera/camera_controls.cpp\n> new file mode 100644\n> index 000000000000..341da56019f7\n> --- /dev/null\n> +++ b/src/libcamera/camera_controls.cpp\n> @@ -0,0 +1,53 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * camera_controls.cpp - Camera controls\n> + */\n> +\n> +#include \"camera_controls.h\"\n> +\n> +#include <libcamera/camera.h>\n> +#include <libcamera/controls.h>\n> +\n> +/**\n> + * \\file camera_controls.h\n> + * \\brief Controls for Camera instances\n> + */\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class CameraControlValidator\n> + * \\brief A control validator for Camera instances\n> + *\n> + * This ControlValidator specialisation validates that controls exist in the\n> + * Camera associated with the validator.\n> + */\n> +\n> +/**\n> + * \\brief Construst a CameraControlValidator for the \\a camera\n> + * \\param[in] camera The camera\n> + */\n> +CameraControlValidator::CameraControlValidator(Camera *camera)\n> +\t: camera_(camera)\n> +{\n> +}\n\nI'm not sure how we're supposed to serialize this, considering\nControlList depends on ControlValidator, and ControlValidator depends\non Camera.\n\nAlso, I'm not sure what is best in hainvg a separate validation object\nthat basically checks if the control is part of the ControlInfoMap of\nthe Camera.\n\nI would keep using the ControlInfoMap in the ControlList, as they're\neasily serializable.\n\nFor V4L2Controls I assume you want to do the same and go directly to\nthe video device when adding controls to a list. I would rather\nretrieve the V4L2ControlnfoMap from the video device and use that to\nconstruct the V4L2ControlList.\n\n> +\n> +const std::string &CameraControlValidator::name() const\n> +{\n> +\treturn camera_->name();\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> +bool CameraControlValidator::validate(const ControlId &id) const\n> +{\n> +\tconst ControlInfoMap &controls = camera_->controls();\n> +\treturn controls.find(&id) != controls.end();\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/include/camera_controls.h b/src/libcamera/include/camera_controls.h\n> new file mode 100644\n> index 000000000000..998a2d155a44\n> --- /dev/null\n> +++ b/src/libcamera/include/camera_controls.h\n> @@ -0,0 +1,30 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * camera_controls.h - Camera controls\n> + */\n> +#ifndef __LIBCAMERA_CAMERA_CONTROLS_H__\n> +#define __LIBCAMERA_CAMERA_CONTROLS_H__\n> +\n> +#include \"control_validator.h\"\n> +\n> +namespace libcamera {\n> +\n> +class Camera;\n> +\n> +class CameraControlValidator final : public ControlValidator\n> +{\n> +public:\n> +\tCameraControlValidator(Camera *camera);\n> +\n> +\tconst std::string &name() const override;\n> +\tbool validate(const ControlId &id) const override;\n> +\n> +private:\n> +\tCamera *camera_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_CAMERA_CONTROLS_H__ */\n> diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build\n> index 1cf47204f2b5..2c74d29bd925 100644\n> --- a/src/libcamera/include/meson.build\n> +++ b/src/libcamera/include/meson.build\n> @@ -1,4 +1,5 @@\n>  libcamera_headers = files([\n> +    'camera_controls.h',\n>      'camera_sensor.h',\n>      'control_validator.h',\n>      'device_enumerator.h',\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 984cb9985f49..fbfa11d469e4 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -2,6 +2,7 @@ libcamera_sources = files([\n>      'bound_method.cpp',\n>      'buffer.cpp',\n>      'camera.cpp',\n> +    'camera_controls.cpp',\n>      'camera_manager.cpp',\n>      'camera_sensor.cpp',\n>      'controls.cpp',\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":"<jacopo@jmondi.org>","Received":["from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A550261654\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 29 Sep 2019 12:17:58 +0200 (CEST)","from uno.localdomain\n\t(host7-199-dynamic.171-212-r.retail.telecomitalia.it [212.171.199.7])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id B70B7E0002;\n\tSun, 29 Sep 2019 10:17:57 +0000 (UTC)"],"X-Originating-IP":"212.171.199.7","Date":"Sun, 29 Sep 2019 12:19:39 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190929101939.wqj5juxch3ava6kx@uno.localdomain>","References":"<20190928152238.23752-1-laurent.pinchart@ideasonboard.com>\n\t<20190928152238.23752-12-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"fyta54h2wewcqqc7\"","Content-Disposition":"inline","In-Reply-To":"<20190928152238.23752-12-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 11/12] libcamera: Add ControlValidator\n\timplementation for Camera","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>","X-List-Received-Date":"Sun, 29 Sep 2019 10:17:58 -0000"}},{"id":2736,"web_url":"https://patchwork.libcamera.org/comment/2736/","msgid":"<20190929133700.GH4827@pendragon.ideasonboard.com>","date":"2019-09-29T13:37:00","subject":"Re: [libcamera-devel] [PATCH 11/12] libcamera: Add ControlValidator\n\timplementation for Camera","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Sun, Sep 29, 2019 at 12:19:39PM +0200, Jacopo Mondi wrote:\n> On Sat, Sep 28, 2019 at 06:22:37PM +0300, Laurent Pinchart wrote:\n> > Add a new CameraControlValidator class that implements the\n> > ControlValidator interface for a Camera object.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/libcamera/camera_controls.cpp       | 53 +++++++++++++++++++++++++\n> >  src/libcamera/include/camera_controls.h | 30 ++++++++++++++\n> >  src/libcamera/include/meson.build       |  1 +\n> >  src/libcamera/meson.build               |  1 +\n> >  4 files changed, 85 insertions(+)\n> >  create mode 100644 src/libcamera/camera_controls.cpp\n> >  create mode 100644 src/libcamera/include/camera_controls.h\n> >\n> > diff --git a/src/libcamera/camera_controls.cpp b/src/libcamera/camera_controls.cpp\n> > new file mode 100644\n> > index 000000000000..341da56019f7\n> > --- /dev/null\n> > +++ b/src/libcamera/camera_controls.cpp\n> > @@ -0,0 +1,53 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * camera_controls.cpp - Camera controls\n> > + */\n> > +\n> > +#include \"camera_controls.h\"\n> > +\n> > +#include <libcamera/camera.h>\n> > +#include <libcamera/controls.h>\n> > +\n> > +/**\n> > + * \\file camera_controls.h\n> > + * \\brief Controls for Camera instances\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +/**\n> > + * \\class CameraControlValidator\n> > + * \\brief A control validator for Camera instances\n> > + *\n> > + * This ControlValidator specialisation validates that controls exist in the\n> > + * Camera associated with the validator.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Construst a CameraControlValidator for the \\a camera\n> > + * \\param[in] camera The camera\n> > + */\n> > +CameraControlValidator::CameraControlValidator(Camera *camera)\n> > +\t: camera_(camera)\n> > +{\n> > +}\n> \n> I'm not sure how we're supposed to serialize this, considering\n> ControlList depends on ControlValidator, and ControlValidator depends\n> on Camera.\n\nI think the validator can easily be modified to take a ControlInfoMap\ninstead of a Camera. This series tries not to introduce regressions when\nit comes to serialisation, but it also doesn't provide everything you'll\nneed for serialisation :-) My goal here was to remove the ControlList\ndependency on Camera, more can be built on top.\n\n> Also, I'm not sure what is best in hainvg a separate validation object\n> that basically checks if the control is part of the ControlInfoMap of\n> the Camera.\n> \n> I would keep using the ControlInfoMap in the ControlList, as they're\n> easily serializable.\n> \n> For V4L2Controls I assume you want to do the same and go directly to\n> the video device when adding controls to a list. I would rather\n> retrieve the V4L2ControlnfoMap from the video device and use that to\n> construct the V4L2ControlList.\n\nYes, it should be based on V4L2ControlnfoMap for V4L2 controls.\n\n> > +\n> > +const std::string &CameraControlValidator::name() const\n> > +{\n> > +\treturn camera_->name();\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> > +bool CameraControlValidator::validate(const ControlId &id) const\n> > +{\n> > +\tconst ControlInfoMap &controls = camera_->controls();\n> > +\treturn controls.find(&id) != controls.end();\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/include/camera_controls.h b/src/libcamera/include/camera_controls.h\n> > new file mode 100644\n> > index 000000000000..998a2d155a44\n> > --- /dev/null\n> > +++ b/src/libcamera/include/camera_controls.h\n> > @@ -0,0 +1,30 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * camera_controls.h - Camera controls\n> > + */\n> > +#ifndef __LIBCAMERA_CAMERA_CONTROLS_H__\n> > +#define __LIBCAMERA_CAMERA_CONTROLS_H__\n> > +\n> > +#include \"control_validator.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +class Camera;\n> > +\n> > +class CameraControlValidator final : public ControlValidator\n> > +{\n> > +public:\n> > +\tCameraControlValidator(Camera *camera);\n> > +\n> > +\tconst std::string &name() const override;\n> > +\tbool validate(const ControlId &id) const override;\n> > +\n> > +private:\n> > +\tCamera *camera_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_CAMERA_CONTROLS_H__ */\n> > diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build\n> > index 1cf47204f2b5..2c74d29bd925 100644\n> > --- a/src/libcamera/include/meson.build\n> > +++ b/src/libcamera/include/meson.build\n> > @@ -1,4 +1,5 @@\n> >  libcamera_headers = files([\n> > +    'camera_controls.h',\n> >      'camera_sensor.h',\n> >      'control_validator.h',\n> >      'device_enumerator.h',\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 984cb9985f49..fbfa11d469e4 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -2,6 +2,7 @@ libcamera_sources = files([\n> >      'bound_method.cpp',\n> >      'buffer.cpp',\n> >      'camera.cpp',\n> > +    'camera_controls.cpp',\n> >      'camera_manager.cpp',\n> >      'camera_sensor.cpp',\n> >      'controls.cpp',","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","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 5FC0A61654\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 29 Sep 2019 15:37:11 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C4131320;\n\tSun, 29 Sep 2019 15:37:10 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1569764231;\n\tbh=M5YK2najrA4huf9q5AhwQ4q/IiBuOY0QuprFFrLunEk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mIG0p6AAnUJiMgmvtwMXYKU9b5zoKjw3aNX/wBGwyEGrkGMP2oRf7Y6e1xWujjXqm\n\tEidRRp/XrvJkhMY0v9m2T3nZXJDCECliM8x+y1n+wZAdm5bMYfZF9NbjmAsUr15aGy\n\t+9FwzdeITjKQQj+v1tbkYQ2ga3PeNxgOFmIcsZDg=","Date":"Sun, 29 Sep 2019 16:37:00 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190929133700.GH4827@pendragon.ideasonboard.com>","References":"<20190928152238.23752-1-laurent.pinchart@ideasonboard.com>\n\t<20190928152238.23752-12-laurent.pinchart@ideasonboard.com>\n\t<20190929101939.wqj5juxch3ava6kx@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190929101939.wqj5juxch3ava6kx@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 11/12] libcamera: Add ControlValidator\n\timplementation for Camera","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>","X-List-Received-Date":"Sun, 29 Sep 2019 13:37:11 -0000"}}]