[{"id":2870,"web_url":"https://patchwork.libcamera.org/comment/2870/","msgid":"<20191013140759.f5fdaqbif477na7e@uno.localdomain>","date":"2019-10-13T14:07:59","subject":"Re: [libcamera-devel] [PATCH v2 08/14] test: v4l2_videodevice: Add\n\tV4L2 control test","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Sat, Oct 12, 2019 at 09:44:01PM +0300, Laurent Pinchart wrote:\n> Add a test that exercises the control enumeration, get and set APIs on a\n> V4L2Device.\n>\n\nThis test was really needed, thanks.\nI would have however moved it after the V4L2ControlList API rework to\navoid having it patch it later in the series.\n\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  test/v4l2_videodevice/controls.cpp | 98 ++++++++++++++++++++++++++++++\n>  test/v4l2_videodevice/meson.build  |  1 +\n>  2 files changed, 99 insertions(+)\n>  create mode 100644 test/v4l2_videodevice/controls.cpp\n>\n> diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp\n> new file mode 100644\n> index 000000000000..4a7535245c00\n> --- /dev/null\n> +++ b/test/v4l2_videodevice/controls.cpp\n> @@ -0,0 +1,98 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * controls.cpp - V4L2 device controls handling test\n> + */\n> +\n> +#include <climits>\n> +#include <iostream>\n> +\n> +#include \"v4l2_videodevice.h\"\n> +\n> +#include \"v4l2_videodevice_test.h\"\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +class V4L2ControlTest : public V4L2VideoDeviceTest\n> +{\n> +public:\n> +\tV4L2ControlTest()\n> +\t\t: V4L2VideoDeviceTest(\"vivid\", \"vivid-000-vid-cap\") {}\n\nI still don't get what is the preferred way of doing this\nif {} on the same line\nor\n        {\n        }\n\n> +\n> +protected:\n> +\tint run()\n> +\t{\n> +\t\tconst V4L2ControlInfoMap &info = capture_->controls();\n> +\n> +\t\t/* Test control enumeration. */\n> +\t\tif (info.empty()) {\n> +\t\t\tcerr << \"Failed to enumerate controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (info.find(V4L2_CID_BRIGHTNESS) == info.end() ||\n> +\t\t    info.find(V4L2_CID_CONTRAST) == info.end() ||\n> +\t\t    info.find(V4L2_CID_SATURATION) == info.end()) {\n> +\t\t\tcerr << \"Missing controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tconst V4L2ControlInfo &brightness = info.find(V4L2_CID_BRIGHTNESS)->second;\n> +\t\tconst V4L2ControlInfo &contrast = info.find(V4L2_CID_CONTRAST)->second;\n> +\t\tconst V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second;\n> +\n> +\t\t/* Test getting controls. */\n> +\t\tV4L2ControlList ctrls;\n> +\t\tctrls.add(V4L2_CID_BRIGHTNESS);\n> +\t\tctrls.add(V4L2_CID_CONTRAST);\n> +\t\tctrls.add(V4L2_CID_SATURATION);\n> +\n> +\t\tint ret = capture_->getControls(&ctrls);\n> +\t\tif (ret != 0) {\n\nJust if (ret) ?\n\n> +\t\t\tcerr << \"Failed to get controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value().get<int32_t>() == -1 ||\n> +\t\t    ctrls[V4L2_CID_CONTRAST]->value().get<int32_t>() == -1 ||\n> +\t\t    ctrls[V4L2_CID_SATURATION]->value().get<int32_t>() == -1) {\n\nI'm missing where they're set to -1\n\n> +\t\t\tcerr << \"Incorrect value for retrieved controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Test setting controls. */\n> +\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min();\n> +\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max();\n> +\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min();\n> +\n> +\t\tret = capture_->setControls(&ctrls);\n> +\t\tif (ret != 0) {\n\nsame comment: just if (ret) ?\n\n> +\t\t\tcerr << \"Failed to set controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Test setting controls outside of range. */\n> +\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get<int32_t>() - 1;\n> +\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get<int32_t>() + 1;\n> +\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get<int32_t>() + 1;\n> +\n> +\t\tret = capture_->setControls(&ctrls);\n> +\t\tif (ret != 0) {\n\nShouldn't setting controls out of range fail?\n\nAnyway, with these clarified\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n> +\t\t\tcerr << \"Failed to set controls (out of range)\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() ||\n> +\t\t    ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() ||\n> +\t\t    ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get<int32_t>() + 1) {\n> +\t\t\tcerr << \"Controls not updated when set\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +};\n> +\n> +TEST_REGISTER(V4L2ControlTest);\n> diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build\n> index ad41898b5f8b..5c52da7219c2 100644\n> --- a/test/v4l2_videodevice/meson.build\n> +++ b/test/v4l2_videodevice/meson.build\n> @@ -2,6 +2,7 @@\n>  # They are not alphabetically sorted.\n>  v4l2_videodevice_tests = [\n>      [ 'double_open',        'double_open.cpp' ],\n> +    [ 'controls',           'controls.cpp' ],\n>      [ 'formats',            'formats.cpp' ],\n>      [ 'request_buffers',    'request_buffers.cpp' ],\n>      [ 'stream_on_off',      'stream_on_off.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 relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3D2A461562\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 13 Oct 2019 16:06:12 +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 relay6-d.mail.gandi.net (Postfix) with ESMTPSA id D5F80C000B;\n\tSun, 13 Oct 2019 14:06:11 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Sun, 13 Oct 2019 16:07:59 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191013140759.f5fdaqbif477na7e@uno.localdomain>","References":"<20191012184407.31684-1-laurent.pinchart@ideasonboard.com>\n\t<20191012184407.31684-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"hs5lxssno6gvkgj3\"","Content-Disposition":"inline","In-Reply-To":"<20191012184407.31684-9-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 08/14] test: v4l2_videodevice: Add\n\tV4L2 control test","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, 13 Oct 2019 14:06:12 -0000"}},{"id":2871,"web_url":"https://patchwork.libcamera.org/comment/2871/","msgid":"<20191013141135.GC4886@pendragon.ideasonboard.com>","date":"2019-10-13T14:11:35","subject":"Re: [libcamera-devel] [PATCH v2 08/14] test: v4l2_videodevice: Add\n\tV4L2 control test","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, Oct 13, 2019 at 04:07:59PM +0200, Jacopo Mondi wrote:\n> On Sat, Oct 12, 2019 at 09:44:01PM +0300, Laurent Pinchart wrote:\n> > Add a test that exercises the control enumeration, get and set APIs on a\n> > V4L2Device.\n> \n> This test was really needed, thanks.\n> I would have however moved it after the V4L2ControlList API rework to\n> avoid having it patch it later in the series.\n\nI specifically added it before the rework, to test for regressions :-)\n\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  test/v4l2_videodevice/controls.cpp | 98 ++++++++++++++++++++++++++++++\n> >  test/v4l2_videodevice/meson.build  |  1 +\n> >  2 files changed, 99 insertions(+)\n> >  create mode 100644 test/v4l2_videodevice/controls.cpp\n> >\n> > diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp\n> > new file mode 100644\n> > index 000000000000..4a7535245c00\n> > --- /dev/null\n> > +++ b/test/v4l2_videodevice/controls.cpp\n> > @@ -0,0 +1,98 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * controls.cpp - V4L2 device controls handling test\n> > + */\n> > +\n> > +#include <climits>\n> > +#include <iostream>\n> > +\n> > +#include \"v4l2_videodevice.h\"\n> > +\n> > +#include \"v4l2_videodevice_test.h\"\n> > +\n> > +using namespace std;\n> > +using namespace libcamera;\n> > +\n> > +class V4L2ControlTest : public V4L2VideoDeviceTest\n> > +{\n> > +public:\n> > +\tV4L2ControlTest()\n> > +\t\t: V4L2VideoDeviceTest(\"vivid\", \"vivid-000-vid-cap\") {}\n> \n> I still don't get what is the preferred way of doing this\n> if {} on the same line\n> or\n>         {\n>         }\n\nThe second is preferred, this comes from a copy&paste, I'll fix it.\n\n> > +\n> > +protected:\n> > +\tint run()\n> > +\t{\n> > +\t\tconst V4L2ControlInfoMap &info = capture_->controls();\n> > +\n> > +\t\t/* Test control enumeration. */\n> > +\t\tif (info.empty()) {\n> > +\t\t\tcerr << \"Failed to enumerate controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (info.find(V4L2_CID_BRIGHTNESS) == info.end() ||\n> > +\t\t    info.find(V4L2_CID_CONTRAST) == info.end() ||\n> > +\t\t    info.find(V4L2_CID_SATURATION) == info.end()) {\n> > +\t\t\tcerr << \"Missing controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tconst V4L2ControlInfo &brightness = info.find(V4L2_CID_BRIGHTNESS)->second;\n> > +\t\tconst V4L2ControlInfo &contrast = info.find(V4L2_CID_CONTRAST)->second;\n> > +\t\tconst V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second;\n> > +\n> > +\t\t/* Test getting controls. */\n> > +\t\tV4L2ControlList ctrls;\n> > +\t\tctrls.add(V4L2_CID_BRIGHTNESS);\n> > +\t\tctrls.add(V4L2_CID_CONTRAST);\n> > +\t\tctrls.add(V4L2_CID_SATURATION);\n> > +\n> > +\t\tint ret = capture_->getControls(&ctrls);\n> > +\t\tif (ret != 0) {\n> \n> Just if (ret) ?\n\nWill fix.\n\n> > +\t\t\tcerr << \"Failed to get controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value().get<int32_t>() == -1 ||\n> > +\t\t    ctrls[V4L2_CID_CONTRAST]->value().get<int32_t>() == -1 ||\n> > +\t\t    ctrls[V4L2_CID_SATURATION]->value().get<int32_t>() == -1) {\n> \n> I'm missing where they're set to -1\n\nThey're not, they're set to 0. And I can't test for != 0 as 0 is a valid\nvalue. I'll remove this.\n\n> > +\t\t\tcerr << \"Incorrect value for retrieved controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Test setting controls. */\n> > +\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min();\n> > +\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max();\n> > +\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min();\n> > +\n> > +\t\tret = capture_->setControls(&ctrls);\n> > +\t\tif (ret != 0) {\n> \n> same comment: just if (ret) ?\n\nWill fix too.\n\n> > +\t\t\tcerr << \"Failed to set controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Test setting controls outside of range. */\n> > +\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get<int32_t>() - 1;\n> > +\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get<int32_t>() + 1;\n> > +\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get<int32_t>() + 1;\n> > +\n> > +\t\tret = capture_->setControls(&ctrls);\n> > +\t\tif (ret != 0) {\n> \n> Shouldn't setting controls out of range fail?\n\nNo, V4L2 controls are adjusted by the kernel. We may later perform the\nadjustement in libcamera, or even fail, in which case I'll adapt the\ntest, but at the moment the test matches the implementation.\n\n> Anyway, with these clarified\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> > +\t\t\tcerr << \"Failed to set controls (out of range)\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() ||\n> > +\t\t    ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() ||\n> > +\t\t    ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get<int32_t>() + 1) {\n> > +\t\t\tcerr << \"Controls not updated when set\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +};\n> > +\n> > +TEST_REGISTER(V4L2ControlTest);\n> > diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build\n> > index ad41898b5f8b..5c52da7219c2 100644\n> > --- a/test/v4l2_videodevice/meson.build\n> > +++ b/test/v4l2_videodevice/meson.build\n> > @@ -2,6 +2,7 @@\n> >  # They are not alphabetically sorted.\n> >  v4l2_videodevice_tests = [\n> >      [ 'double_open',        'double_open.cpp' ],\n> > +    [ 'controls',           'controls.cpp' ],\n> >      [ 'formats',            'formats.cpp' ],\n> >      [ 'request_buffers',    'request_buffers.cpp' ],\n> >      [ 'stream_on_off',      'stream_on_off.cpp' ],","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1ECB361562\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 13 Oct 2019 16:11:38 +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 95966A46;\n\tSun, 13 Oct 2019 16:11:37 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1570975897;\n\tbh=6KzMN72LFCPVbWQaqYS8y9DoTQvo5K5OrC7oqjR5XXQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=s3CfaBqSeP9X7CrCfxohK61MCva3racgEfHrdsI8bjyrnom3sjYQiq2v0DtFA+x2e\n\tBi4OCbM7p7/R552N7JE/PMCY7P9AhqTqZgiqWUE3sjFSbsVbYiClJtMOq8e27F0oBQ\n\tE91TYrRsuAGOnt0xRgUhe+s03KJcSBGh3bZENp8s=","Date":"Sun, 13 Oct 2019 17:11:35 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191013141135.GC4886@pendragon.ideasonboard.com>","References":"<20191012184407.31684-1-laurent.pinchart@ideasonboard.com>\n\t<20191012184407.31684-9-laurent.pinchart@ideasonboard.com>\n\t<20191013140759.f5fdaqbif477na7e@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191013140759.f5fdaqbif477na7e@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 08/14] test: v4l2_videodevice: Add\n\tV4L2 control test","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, 13 Oct 2019 14:11:38 -0000"}},{"id":2885,"web_url":"https://patchwork.libcamera.org/comment/2885/","msgid":"<20191013155554.GQ23166@bigcity.dyn.berto.se>","date":"2019-10-13T15:55:54","subject":"Re: [libcamera-devel] [PATCH v2 08/14] test: v4l2_videodevice: Add\n\tV4L2 control test","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2019-10-12 21:44:01 +0300, Laurent Pinchart wrote:\n> Add a test that exercises the control enumeration, get and set APIs on a\n> V4L2Device.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  test/v4l2_videodevice/controls.cpp | 98 ++++++++++++++++++++++++++++++\n>  test/v4l2_videodevice/meson.build  |  1 +\n>  2 files changed, 99 insertions(+)\n>  create mode 100644 test/v4l2_videodevice/controls.cpp\n> \n> diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp\n> new file mode 100644\n> index 000000000000..4a7535245c00\n> --- /dev/null\n> +++ b/test/v4l2_videodevice/controls.cpp\n> @@ -0,0 +1,98 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * controls.cpp - V4L2 device controls handling test\n> + */\n> +\n> +#include <climits>\n> +#include <iostream>\n> +\n> +#include \"v4l2_videodevice.h\"\n> +\n> +#include \"v4l2_videodevice_test.h\"\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +class V4L2ControlTest : public V4L2VideoDeviceTest\n> +{\n> +public:\n> +\tV4L2ControlTest()\n> +\t\t: V4L2VideoDeviceTest(\"vivid\", \"vivid-000-vid-cap\") {}\n> +\n> +protected:\n> +\tint run()\n> +\t{\n> +\t\tconst V4L2ControlInfoMap &info = capture_->controls();\n> +\n> +\t\t/* Test control enumeration. */\n> +\t\tif (info.empty()) {\n> +\t\t\tcerr << \"Failed to enumerate controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (info.find(V4L2_CID_BRIGHTNESS) == info.end() ||\n> +\t\t    info.find(V4L2_CID_CONTRAST) == info.end() ||\n> +\t\t    info.find(V4L2_CID_SATURATION) == info.end()) {\n> +\t\t\tcerr << \"Missing controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tconst V4L2ControlInfo &brightness = info.find(V4L2_CID_BRIGHTNESS)->second;\n> +\t\tconst V4L2ControlInfo &contrast = info.find(V4L2_CID_CONTRAST)->second;\n> +\t\tconst V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second;\n> +\n> +\t\t/* Test getting controls. */\n> +\t\tV4L2ControlList ctrls;\n> +\t\tctrls.add(V4L2_CID_BRIGHTNESS);\n> +\t\tctrls.add(V4L2_CID_CONTRAST);\n> +\t\tctrls.add(V4L2_CID_SATURATION);\n> +\n> +\t\tint ret = capture_->getControls(&ctrls);\n> +\t\tif (ret != 0) {\n> +\t\t\tcerr << \"Failed to get controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value().get<int32_t>() == -1 ||\n> +\t\t    ctrls[V4L2_CID_CONTRAST]->value().get<int32_t>() == -1 ||\n> +\t\t    ctrls[V4L2_CID_SATURATION]->value().get<int32_t>() == -1) {\n> +\t\t\tcerr << \"Incorrect value for retrieved controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Test setting controls. */\n> +\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min();\n> +\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max();\n> +\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min();\n> +\n> +\t\tret = capture_->setControls(&ctrls);\n> +\t\tif (ret != 0) {\n> +\t\t\tcerr << \"Failed to set controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Test setting controls outside of range. */\n> +\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get<int32_t>() - 1;\n> +\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get<int32_t>() + 1;\n> +\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get<int32_t>() + 1;\n> +\n> +\t\tret = capture_->setControls(&ctrls);\n> +\t\tif (ret != 0) {\n> +\t\t\tcerr << \"Failed to set controls (out of range)\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() ||\n> +\t\t    ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() ||\n> +\t\t    ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get<int32_t>() + 1) {\n> +\t\t\tcerr << \"Controls not updated when set\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +};\n> +\n> +TEST_REGISTER(V4L2ControlTest);\n> diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build\n> index ad41898b5f8b..5c52da7219c2 100644\n> --- a/test/v4l2_videodevice/meson.build\n> +++ b/test/v4l2_videodevice/meson.build\n> @@ -2,6 +2,7 @@\n>  # They are not alphabetically sorted.\n>  v4l2_videodevice_tests = [\n>      [ 'double_open',        'double_open.cpp' ],\n> +    [ 'controls',           'controls.cpp' ],\n\nDo we aim for this to be in alphabetical order?\n\nWith this and Jacopo's comments addressed,\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n>      [ 'formats',            'formats.cpp' ],\n>      [ 'request_buffers',    'request_buffers.cpp' ],\n>      [ 'stream_on_off',      'stream_on_off.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":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 230D261562\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 13 Oct 2019 17:55:56 +0200 (CEST)","by mail-lf1-x143.google.com with SMTP id c195so10099351lfg.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 13 Oct 2019 08:55:56 -0700 (PDT)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tp27sm3579642lfo.95.2019.10.13.08.55.54\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 13 Oct 2019 08:55:55 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=+naR3yQZ32hd+R9T5QfH9jvbQdYFpsx4RKmnbm3k2yQ=;\n\tb=fq+gxoo+u2SEyniwglUukJO925CJ+rS9q5QkkxmwMbQ6ppRcb6PHasp0Kf/ayh/iAt\n\tTE94/MNjB7tuRzT0THI/2UTi6rgfTR37v+pdWS/9GbfegSTahd8TxqC+PzBoxMCQo+3W\n\tJIKX9CfD6owBh/z4UWi2ryjBKUID7HftyHckF0QuaG07iAu/gwy1iHHO/z12vqUbK2dd\n\tCchi3y/uLpe2CtGBXnRvEDvJ6UCr6FgrX0stJ1mmmRHOG3gfperkj7zJlZINRzZdysW5\n\tyqcOtOLYir+VeO0S8neX1tfHjzyInWyD4cIPiLVLa2ioJ2EuR/MdUWPPZe506Tghi0Bi\n\tMADA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=+naR3yQZ32hd+R9T5QfH9jvbQdYFpsx4RKmnbm3k2yQ=;\n\tb=A/cdCfrl8msyn0WqXUVG0H69rIUHtqqS3rIMclcYJK22LU0Y02u752jGuhNgkOodzj\n\tcJZAgeqBcDtTrc1R2HlvhQanAV4bcdxz8I/exgHnqi0fcIS8qBmD45gbqPdsyTMOpJwj\n\tq6Oaic0jZaMjSv9hfZEv8PxC6dp8LtOCIXchqIfN4IXy6vvXiS371m+S0LwVettYCKUU\n\t2XEpumXwycGFIk81R2RujcurYwgiaZwoeUTo/UwOQkJd1MFuXA/Yr4G5vUHX6+yrQ4ZT\n\tuZyCIRUgZdaM/BDV9HgR+peQWc2QvrKesem04BaUTbp0Mk+llLjo7NNd9CHcna6bokow\n\tn1nQ==","X-Gm-Message-State":"APjAAAXkpxagho6dPKtjCDZ+MUh8MFFc6PC0DB7IRB2qEqYU5doS46oK\n\t8YrMjNJrzzUPc0Cxy0d+PwUIlG04KFA=","X-Google-Smtp-Source":"APXvYqxBKZ9EEF2RboNxMfS1kvQbNAcR4+ooA8egb5Dsv0woR3gBxORruzSXxFireyKWjuV7dJ7hgw==","X-Received":"by 2002:a19:84c:: with SMTP id 73mr14575979lfi.180.1570982155551;\n\tSun, 13 Oct 2019 08:55:55 -0700 (PDT)","Date":"Sun, 13 Oct 2019 17:55:54 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191013155554.GQ23166@bigcity.dyn.berto.se>","References":"<20191012184407.31684-1-laurent.pinchart@ideasonboard.com>\n\t<20191012184407.31684-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191012184407.31684-9-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH v2 08/14] test: v4l2_videodevice: Add\n\tV4L2 control test","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, 13 Oct 2019 15:55:56 -0000"}},{"id":2888,"web_url":"https://patchwork.libcamera.org/comment/2888/","msgid":"<20191013161159.GE4886@pendragon.ideasonboard.com>","date":"2019-10-13T16:11:59","subject":"Re: [libcamera-devel] [PATCH v2 08/14] test: v4l2_videodevice: Add\n\tV4L2 control test","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Sun, Oct 13, 2019 at 05:55:54PM +0200, Niklas Söderlund wrote:\n> On 2019-10-12 21:44:01 +0300, Laurent Pinchart wrote:\n> > Add a test that exercises the control enumeration, get and set APIs on a\n> > V4L2Device.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  test/v4l2_videodevice/controls.cpp | 98 ++++++++++++++++++++++++++++++\n> >  test/v4l2_videodevice/meson.build  |  1 +\n> >  2 files changed, 99 insertions(+)\n> >  create mode 100644 test/v4l2_videodevice/controls.cpp\n> > \n> > diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp\n> > new file mode 100644\n> > index 000000000000..4a7535245c00\n> > --- /dev/null\n> > +++ b/test/v4l2_videodevice/controls.cpp\n> > @@ -0,0 +1,98 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * controls.cpp - V4L2 device controls handling test\n> > + */\n> > +\n> > +#include <climits>\n> > +#include <iostream>\n> > +\n> > +#include \"v4l2_videodevice.h\"\n> > +\n> > +#include \"v4l2_videodevice_test.h\"\n> > +\n> > +using namespace std;\n> > +using namespace libcamera;\n> > +\n> > +class V4L2ControlTest : public V4L2VideoDeviceTest\n> > +{\n> > +public:\n> > +\tV4L2ControlTest()\n> > +\t\t: V4L2VideoDeviceTest(\"vivid\", \"vivid-000-vid-cap\") {}\n> > +\n> > +protected:\n> > +\tint run()\n> > +\t{\n> > +\t\tconst V4L2ControlInfoMap &info = capture_->controls();\n> > +\n> > +\t\t/* Test control enumeration. */\n> > +\t\tif (info.empty()) {\n> > +\t\t\tcerr << \"Failed to enumerate controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (info.find(V4L2_CID_BRIGHTNESS) == info.end() ||\n> > +\t\t    info.find(V4L2_CID_CONTRAST) == info.end() ||\n> > +\t\t    info.find(V4L2_CID_SATURATION) == info.end()) {\n> > +\t\t\tcerr << \"Missing controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tconst V4L2ControlInfo &brightness = info.find(V4L2_CID_BRIGHTNESS)->second;\n> > +\t\tconst V4L2ControlInfo &contrast = info.find(V4L2_CID_CONTRAST)->second;\n> > +\t\tconst V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second;\n> > +\n> > +\t\t/* Test getting controls. */\n> > +\t\tV4L2ControlList ctrls;\n> > +\t\tctrls.add(V4L2_CID_BRIGHTNESS);\n> > +\t\tctrls.add(V4L2_CID_CONTRAST);\n> > +\t\tctrls.add(V4L2_CID_SATURATION);\n> > +\n> > +\t\tint ret = capture_->getControls(&ctrls);\n> > +\t\tif (ret != 0) {\n> > +\t\t\tcerr << \"Failed to get controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value().get<int32_t>() == -1 ||\n> > +\t\t    ctrls[V4L2_CID_CONTRAST]->value().get<int32_t>() == -1 ||\n> > +\t\t    ctrls[V4L2_CID_SATURATION]->value().get<int32_t>() == -1) {\n> > +\t\t\tcerr << \"Incorrect value for retrieved controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Test setting controls. */\n> > +\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min();\n> > +\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max();\n> > +\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min();\n> > +\n> > +\t\tret = capture_->setControls(&ctrls);\n> > +\t\tif (ret != 0) {\n> > +\t\t\tcerr << \"Failed to set controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Test setting controls outside of range. */\n> > +\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get<int32_t>() - 1;\n> > +\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get<int32_t>() + 1;\n> > +\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get<int32_t>() + 1;\n> > +\n> > +\t\tret = capture_->setControls(&ctrls);\n> > +\t\tif (ret != 0) {\n> > +\t\t\tcerr << \"Failed to set controls (out of range)\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() ||\n> > +\t\t    ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() ||\n> > +\t\t    ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get<int32_t>() + 1) {\n> > +\t\t\tcerr << \"Controls not updated when set\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +};\n> > +\n> > +TEST_REGISTER(V4L2ControlTest);\n> > diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build\n> > index ad41898b5f8b..5c52da7219c2 100644\n> > --- a/test/v4l2_videodevice/meson.build\n> > +++ b/test/v4l2_videodevice/meson.build\n> > @@ -2,6 +2,7 @@\n> >  # They are not alphabetically sorted.\n\nSee this :-)\n\n> >  v4l2_videodevice_tests = [\n> >      [ 'double_open',        'double_open.cpp' ],\n> > +    [ 'controls',           'controls.cpp' ],\n> \n> Do we aim for this to be in alphabetical order?\n> \n> With this and Jacopo's comments addressed,\n> \n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> \n> >      [ 'formats',            'formats.cpp' ],\n> >      [ 'request_buffers',    'request_buffers.cpp' ],\n> >      [ 'stream_on_off',      'stream_on_off.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 52E0D61562\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 13 Oct 2019 18:12:02 +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 C9886A46;\n\tSun, 13 Oct 2019 18:12:01 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1570983122;\n\tbh=7sSX3Qk1xOU3HDQ7kcEdj8t23duJyDDvcq4BT9MSS34=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HEuHo0MGG6/bGSXXCNMoeVS21DQEe5CznEKp9ZBsezf8hhuon5uAh5h/AnOl89TGv\n\tQWsLboyznEmY9QExKIKtuxwB59yUHrmni+ZCpLf2WBie4YKkSAtrNvNCF4g2rSsTV0\n\thVqOF4xRBfq4wMnlMbpdg0d2g+lgRqb7KmqzuVLI=","Date":"Sun, 13 Oct 2019 19:11:59 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191013161159.GE4886@pendragon.ideasonboard.com>","References":"<20191012184407.31684-1-laurent.pinchart@ideasonboard.com>\n\t<20191012184407.31684-9-laurent.pinchart@ideasonboard.com>\n\t<20191013155554.GQ23166@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191013155554.GQ23166@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 08/14] test: v4l2_videodevice: Add\n\tV4L2 control test","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, 13 Oct 2019 16:12:02 -0000"}}]