[libcamera-devel,v2,08/14] test: v4l2_videodevice: Add V4L2 control test

Message ID 20191012184407.31684-9-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Use ControlList for both libcamera and V4L2 controls
Related show

Commit Message

Laurent Pinchart Oct. 12, 2019, 6:44 p.m. UTC
Add a test that exercises the control enumeration, get and set APIs on a
V4L2Device.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/v4l2_videodevice/controls.cpp | 98 ++++++++++++++++++++++++++++++
 test/v4l2_videodevice/meson.build  |  1 +
 2 files changed, 99 insertions(+)
 create mode 100644 test/v4l2_videodevice/controls.cpp

Comments

Jacopo Mondi Oct. 13, 2019, 2:07 p.m. UTC | #1
Hi Laurent,

On Sat, Oct 12, 2019 at 09:44:01PM +0300, Laurent Pinchart wrote:
> Add a test that exercises the control enumeration, get and set APIs on a
> V4L2Device.
>

This test was really needed, thanks.
I would have however moved it after the V4L2ControlList API rework to
avoid having it patch it later in the series.

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  test/v4l2_videodevice/controls.cpp | 98 ++++++++++++++++++++++++++++++
>  test/v4l2_videodevice/meson.build  |  1 +
>  2 files changed, 99 insertions(+)
>  create mode 100644 test/v4l2_videodevice/controls.cpp
>
> diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
> new file mode 100644
> index 000000000000..4a7535245c00
> --- /dev/null
> +++ b/test/v4l2_videodevice/controls.cpp
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * controls.cpp - V4L2 device controls handling test
> + */
> +
> +#include <climits>
> +#include <iostream>
> +
> +#include "v4l2_videodevice.h"
> +
> +#include "v4l2_videodevice_test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +class V4L2ControlTest : public V4L2VideoDeviceTest
> +{
> +public:
> +	V4L2ControlTest()
> +		: V4L2VideoDeviceTest("vivid", "vivid-000-vid-cap") {}

I still don't get what is the preferred way of doing this
if {} on the same line
or
        {
        }

> +
> +protected:
> +	int run()
> +	{
> +		const V4L2ControlInfoMap &info = capture_->controls();
> +
> +		/* Test control enumeration. */
> +		if (info.empty()) {
> +			cerr << "Failed to enumerate controls" << endl;
> +			return TestFail;
> +		}
> +
> +		if (info.find(V4L2_CID_BRIGHTNESS) == info.end() ||
> +		    info.find(V4L2_CID_CONTRAST) == info.end() ||
> +		    info.find(V4L2_CID_SATURATION) == info.end()) {
> +			cerr << "Missing controls" << endl;
> +			return TestFail;
> +		}
> +
> +		const V4L2ControlInfo &brightness = info.find(V4L2_CID_BRIGHTNESS)->second;
> +		const V4L2ControlInfo &contrast = info.find(V4L2_CID_CONTRAST)->second;
> +		const V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second;
> +
> +		/* Test getting controls. */
> +		V4L2ControlList ctrls;
> +		ctrls.add(V4L2_CID_BRIGHTNESS);
> +		ctrls.add(V4L2_CID_CONTRAST);
> +		ctrls.add(V4L2_CID_SATURATION);
> +
> +		int ret = capture_->getControls(&ctrls);
> +		if (ret != 0) {

Just if (ret) ?

> +			cerr << "Failed to get controls" << endl;
> +			return TestFail;
> +		}
> +
> +		if (ctrls[V4L2_CID_BRIGHTNESS]->value().get<int32_t>() == -1 ||
> +		    ctrls[V4L2_CID_CONTRAST]->value().get<int32_t>() == -1 ||
> +		    ctrls[V4L2_CID_SATURATION]->value().get<int32_t>() == -1) {

I'm missing where they're set to -1

> +			cerr << "Incorrect value for retrieved controls" << endl;
> +			return TestFail;
> +		}
> +
> +		/* Test setting controls. */
> +		ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min();
> +		ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max();
> +		ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min();
> +
> +		ret = capture_->setControls(&ctrls);
> +		if (ret != 0) {

same comment: just if (ret) ?

> +			cerr << "Failed to set controls" << endl;
> +			return TestFail;
> +		}
> +
> +		/* Test setting controls outside of range. */
> +		ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get<int32_t>() - 1;
> +		ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get<int32_t>() + 1;
> +		ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get<int32_t>() + 1;
> +
> +		ret = capture_->setControls(&ctrls);
> +		if (ret != 0) {

Shouldn't setting controls out of range fail?

Anyway, with these clarified
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

> +			cerr << "Failed to set controls (out of range)" << endl;
> +			return TestFail;
> +		}
> +
> +		if (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() ||
> +		    ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() ||
> +		    ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get<int32_t>() + 1) {
> +			cerr << "Controls not updated when set" << endl;
> +			return TestFail;
> +		}
> +
> +		return TestPass;
> +	}
> +};
> +
> +TEST_REGISTER(V4L2ControlTest);
> diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build
> index ad41898b5f8b..5c52da7219c2 100644
> --- a/test/v4l2_videodevice/meson.build
> +++ b/test/v4l2_videodevice/meson.build
> @@ -2,6 +2,7 @@
>  # They are not alphabetically sorted.
>  v4l2_videodevice_tests = [
>      [ 'double_open',        'double_open.cpp' ],
> +    [ 'controls',           'controls.cpp' ],
>      [ 'formats',            'formats.cpp' ],
>      [ 'request_buffers',    'request_buffers.cpp' ],
>      [ 'stream_on_off',      'stream_on_off.cpp' ],
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Oct. 13, 2019, 2:11 p.m. UTC | #2
Hi Jacopo,

On Sun, Oct 13, 2019 at 04:07:59PM +0200, Jacopo Mondi wrote:
> On Sat, Oct 12, 2019 at 09:44:01PM +0300, Laurent Pinchart wrote:
> > Add a test that exercises the control enumeration, get and set APIs on a
> > V4L2Device.
> 
> This test was really needed, thanks.
> I would have however moved it after the V4L2ControlList API rework to
> avoid having it patch it later in the series.

I specifically added it before the rework, to test for regressions :-)

> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  test/v4l2_videodevice/controls.cpp | 98 ++++++++++++++++++++++++++++++
> >  test/v4l2_videodevice/meson.build  |  1 +
> >  2 files changed, 99 insertions(+)
> >  create mode 100644 test/v4l2_videodevice/controls.cpp
> >
> > diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
> > new file mode 100644
> > index 000000000000..4a7535245c00
> > --- /dev/null
> > +++ b/test/v4l2_videodevice/controls.cpp
> > @@ -0,0 +1,98 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * controls.cpp - V4L2 device controls handling test
> > + */
> > +
> > +#include <climits>
> > +#include <iostream>
> > +
> > +#include "v4l2_videodevice.h"
> > +
> > +#include "v4l2_videodevice_test.h"
> > +
> > +using namespace std;
> > +using namespace libcamera;
> > +
> > +class V4L2ControlTest : public V4L2VideoDeviceTest
> > +{
> > +public:
> > +	V4L2ControlTest()
> > +		: V4L2VideoDeviceTest("vivid", "vivid-000-vid-cap") {}
> 
> I still don't get what is the preferred way of doing this
> if {} on the same line
> or
>         {
>         }

The second is preferred, this comes from a copy&paste, I'll fix it.

> > +
> > +protected:
> > +	int run()
> > +	{
> > +		const V4L2ControlInfoMap &info = capture_->controls();
> > +
> > +		/* Test control enumeration. */
> > +		if (info.empty()) {
> > +			cerr << "Failed to enumerate controls" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (info.find(V4L2_CID_BRIGHTNESS) == info.end() ||
> > +		    info.find(V4L2_CID_CONTRAST) == info.end() ||
> > +		    info.find(V4L2_CID_SATURATION) == info.end()) {
> > +			cerr << "Missing controls" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		const V4L2ControlInfo &brightness = info.find(V4L2_CID_BRIGHTNESS)->second;
> > +		const V4L2ControlInfo &contrast = info.find(V4L2_CID_CONTRAST)->second;
> > +		const V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second;
> > +
> > +		/* Test getting controls. */
> > +		V4L2ControlList ctrls;
> > +		ctrls.add(V4L2_CID_BRIGHTNESS);
> > +		ctrls.add(V4L2_CID_CONTRAST);
> > +		ctrls.add(V4L2_CID_SATURATION);
> > +
> > +		int ret = capture_->getControls(&ctrls);
> > +		if (ret != 0) {
> 
> Just if (ret) ?

Will fix.

> > +			cerr << "Failed to get controls" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (ctrls[V4L2_CID_BRIGHTNESS]->value().get<int32_t>() == -1 ||
> > +		    ctrls[V4L2_CID_CONTRAST]->value().get<int32_t>() == -1 ||
> > +		    ctrls[V4L2_CID_SATURATION]->value().get<int32_t>() == -1) {
> 
> I'm missing where they're set to -1

They're not, they're set to 0. And I can't test for != 0 as 0 is a valid
value. I'll remove this.

> > +			cerr << "Incorrect value for retrieved controls" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test setting controls. */
> > +		ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min();
> > +		ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max();
> > +		ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min();
> > +
> > +		ret = capture_->setControls(&ctrls);
> > +		if (ret != 0) {
> 
> same comment: just if (ret) ?

Will fix too.

> > +			cerr << "Failed to set controls" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test setting controls outside of range. */
> > +		ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get<int32_t>() - 1;
> > +		ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get<int32_t>() + 1;
> > +		ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get<int32_t>() + 1;
> > +
> > +		ret = capture_->setControls(&ctrls);
> > +		if (ret != 0) {
> 
> Shouldn't setting controls out of range fail?

No, V4L2 controls are adjusted by the kernel. We may later perform the
adjustement in libcamera, or even fail, in which case I'll adapt the
test, but at the moment the test matches the implementation.

> Anyway, with these clarified
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > +			cerr << "Failed to set controls (out of range)" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() ||
> > +		    ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() ||
> > +		    ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get<int32_t>() + 1) {
> > +			cerr << "Controls not updated when set" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		return TestPass;
> > +	}
> > +};
> > +
> > +TEST_REGISTER(V4L2ControlTest);
> > diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build
> > index ad41898b5f8b..5c52da7219c2 100644
> > --- a/test/v4l2_videodevice/meson.build
> > +++ b/test/v4l2_videodevice/meson.build
> > @@ -2,6 +2,7 @@
> >  # They are not alphabetically sorted.
> >  v4l2_videodevice_tests = [
> >      [ 'double_open',        'double_open.cpp' ],
> > +    [ 'controls',           'controls.cpp' ],
> >      [ 'formats',            'formats.cpp' ],
> >      [ 'request_buffers',    'request_buffers.cpp' ],
> >      [ 'stream_on_off',      'stream_on_off.cpp' ],
Niklas Söderlund Oct. 13, 2019, 3:55 p.m. UTC | #3
Hi Laurent,

Thanks for your work.

On 2019-10-12 21:44:01 +0300, Laurent Pinchart wrote:
> Add a test that exercises the control enumeration, get and set APIs on a
> V4L2Device.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  test/v4l2_videodevice/controls.cpp | 98 ++++++++++++++++++++++++++++++
>  test/v4l2_videodevice/meson.build  |  1 +
>  2 files changed, 99 insertions(+)
>  create mode 100644 test/v4l2_videodevice/controls.cpp
> 
> diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
> new file mode 100644
> index 000000000000..4a7535245c00
> --- /dev/null
> +++ b/test/v4l2_videodevice/controls.cpp
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * controls.cpp - V4L2 device controls handling test
> + */
> +
> +#include <climits>
> +#include <iostream>
> +
> +#include "v4l2_videodevice.h"
> +
> +#include "v4l2_videodevice_test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +class V4L2ControlTest : public V4L2VideoDeviceTest
> +{
> +public:
> +	V4L2ControlTest()
> +		: V4L2VideoDeviceTest("vivid", "vivid-000-vid-cap") {}
> +
> +protected:
> +	int run()
> +	{
> +		const V4L2ControlInfoMap &info = capture_->controls();
> +
> +		/* Test control enumeration. */
> +		if (info.empty()) {
> +			cerr << "Failed to enumerate controls" << endl;
> +			return TestFail;
> +		}
> +
> +		if (info.find(V4L2_CID_BRIGHTNESS) == info.end() ||
> +		    info.find(V4L2_CID_CONTRAST) == info.end() ||
> +		    info.find(V4L2_CID_SATURATION) == info.end()) {
> +			cerr << "Missing controls" << endl;
> +			return TestFail;
> +		}
> +
> +		const V4L2ControlInfo &brightness = info.find(V4L2_CID_BRIGHTNESS)->second;
> +		const V4L2ControlInfo &contrast = info.find(V4L2_CID_CONTRAST)->second;
> +		const V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second;
> +
> +		/* Test getting controls. */
> +		V4L2ControlList ctrls;
> +		ctrls.add(V4L2_CID_BRIGHTNESS);
> +		ctrls.add(V4L2_CID_CONTRAST);
> +		ctrls.add(V4L2_CID_SATURATION);
> +
> +		int ret = capture_->getControls(&ctrls);
> +		if (ret != 0) {
> +			cerr << "Failed to get controls" << endl;
> +			return TestFail;
> +		}
> +
> +		if (ctrls[V4L2_CID_BRIGHTNESS]->value().get<int32_t>() == -1 ||
> +		    ctrls[V4L2_CID_CONTRAST]->value().get<int32_t>() == -1 ||
> +		    ctrls[V4L2_CID_SATURATION]->value().get<int32_t>() == -1) {
> +			cerr << "Incorrect value for retrieved controls" << endl;
> +			return TestFail;
> +		}
> +
> +		/* Test setting controls. */
> +		ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min();
> +		ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max();
> +		ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min();
> +
> +		ret = capture_->setControls(&ctrls);
> +		if (ret != 0) {
> +			cerr << "Failed to set controls" << endl;
> +			return TestFail;
> +		}
> +
> +		/* Test setting controls outside of range. */
> +		ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get<int32_t>() - 1;
> +		ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get<int32_t>() + 1;
> +		ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get<int32_t>() + 1;
> +
> +		ret = capture_->setControls(&ctrls);
> +		if (ret != 0) {
> +			cerr << "Failed to set controls (out of range)" << endl;
> +			return TestFail;
> +		}
> +
> +		if (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() ||
> +		    ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() ||
> +		    ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get<int32_t>() + 1) {
> +			cerr << "Controls not updated when set" << endl;
> +			return TestFail;
> +		}
> +
> +		return TestPass;
> +	}
> +};
> +
> +TEST_REGISTER(V4L2ControlTest);
> diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build
> index ad41898b5f8b..5c52da7219c2 100644
> --- a/test/v4l2_videodevice/meson.build
> +++ b/test/v4l2_videodevice/meson.build
> @@ -2,6 +2,7 @@
>  # They are not alphabetically sorted.
>  v4l2_videodevice_tests = [
>      [ 'double_open',        'double_open.cpp' ],
> +    [ 'controls',           'controls.cpp' ],

Do we aim for this to be in alphabetical order?

With this and Jacopo's comments addressed,

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

>      [ 'formats',            'formats.cpp' ],
>      [ 'request_buffers',    'request_buffers.cpp' ],
>      [ 'stream_on_off',      'stream_on_off.cpp' ],
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Oct. 13, 2019, 4:11 p.m. UTC | #4
Hi Niklas,

On Sun, Oct 13, 2019 at 05:55:54PM +0200, Niklas Söderlund wrote:
> On 2019-10-12 21:44:01 +0300, Laurent Pinchart wrote:
> > Add a test that exercises the control enumeration, get and set APIs on a
> > V4L2Device.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  test/v4l2_videodevice/controls.cpp | 98 ++++++++++++++++++++++++++++++
> >  test/v4l2_videodevice/meson.build  |  1 +
> >  2 files changed, 99 insertions(+)
> >  create mode 100644 test/v4l2_videodevice/controls.cpp
> > 
> > diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
> > new file mode 100644
> > index 000000000000..4a7535245c00
> > --- /dev/null
> > +++ b/test/v4l2_videodevice/controls.cpp
> > @@ -0,0 +1,98 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * controls.cpp - V4L2 device controls handling test
> > + */
> > +
> > +#include <climits>
> > +#include <iostream>
> > +
> > +#include "v4l2_videodevice.h"
> > +
> > +#include "v4l2_videodevice_test.h"
> > +
> > +using namespace std;
> > +using namespace libcamera;
> > +
> > +class V4L2ControlTest : public V4L2VideoDeviceTest
> > +{
> > +public:
> > +	V4L2ControlTest()
> > +		: V4L2VideoDeviceTest("vivid", "vivid-000-vid-cap") {}
> > +
> > +protected:
> > +	int run()
> > +	{
> > +		const V4L2ControlInfoMap &info = capture_->controls();
> > +
> > +		/* Test control enumeration. */
> > +		if (info.empty()) {
> > +			cerr << "Failed to enumerate controls" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (info.find(V4L2_CID_BRIGHTNESS) == info.end() ||
> > +		    info.find(V4L2_CID_CONTRAST) == info.end() ||
> > +		    info.find(V4L2_CID_SATURATION) == info.end()) {
> > +			cerr << "Missing controls" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		const V4L2ControlInfo &brightness = info.find(V4L2_CID_BRIGHTNESS)->second;
> > +		const V4L2ControlInfo &contrast = info.find(V4L2_CID_CONTRAST)->second;
> > +		const V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second;
> > +
> > +		/* Test getting controls. */
> > +		V4L2ControlList ctrls;
> > +		ctrls.add(V4L2_CID_BRIGHTNESS);
> > +		ctrls.add(V4L2_CID_CONTRAST);
> > +		ctrls.add(V4L2_CID_SATURATION);
> > +
> > +		int ret = capture_->getControls(&ctrls);
> > +		if (ret != 0) {
> > +			cerr << "Failed to get controls" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (ctrls[V4L2_CID_BRIGHTNESS]->value().get<int32_t>() == -1 ||
> > +		    ctrls[V4L2_CID_CONTRAST]->value().get<int32_t>() == -1 ||
> > +		    ctrls[V4L2_CID_SATURATION]->value().get<int32_t>() == -1) {
> > +			cerr << "Incorrect value for retrieved controls" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test setting controls. */
> > +		ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min();
> > +		ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max();
> > +		ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min();
> > +
> > +		ret = capture_->setControls(&ctrls);
> > +		if (ret != 0) {
> > +			cerr << "Failed to set controls" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/* Test setting controls outside of range. */
> > +		ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get<int32_t>() - 1;
> > +		ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get<int32_t>() + 1;
> > +		ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get<int32_t>() + 1;
> > +
> > +		ret = capture_->setControls(&ctrls);
> > +		if (ret != 0) {
> > +			cerr << "Failed to set controls (out of range)" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() ||
> > +		    ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() ||
> > +		    ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get<int32_t>() + 1) {
> > +			cerr << "Controls not updated when set" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		return TestPass;
> > +	}
> > +};
> > +
> > +TEST_REGISTER(V4L2ControlTest);
> > diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build
> > index ad41898b5f8b..5c52da7219c2 100644
> > --- a/test/v4l2_videodevice/meson.build
> > +++ b/test/v4l2_videodevice/meson.build
> > @@ -2,6 +2,7 @@
> >  # They are not alphabetically sorted.

See this :-)

> >  v4l2_videodevice_tests = [
> >      [ 'double_open',        'double_open.cpp' ],
> > +    [ 'controls',           'controls.cpp' ],
> 
> Do we aim for this to be in alphabetical order?
> 
> With this and Jacopo's comments addressed,
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
> >      [ 'formats',            'formats.cpp' ],
> >      [ 'request_buffers',    'request_buffers.cpp' ],
> >      [ 'stream_on_off',      'stream_on_off.cpp' ],

Patch

diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
new file mode 100644
index 000000000000..4a7535245c00
--- /dev/null
+++ b/test/v4l2_videodevice/controls.cpp
@@ -0,0 +1,98 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2019, Google Inc.
+ *
+ * controls.cpp - V4L2 device controls handling test
+ */
+
+#include <climits>
+#include <iostream>
+
+#include "v4l2_videodevice.h"
+
+#include "v4l2_videodevice_test.h"
+
+using namespace std;
+using namespace libcamera;
+
+class V4L2ControlTest : public V4L2VideoDeviceTest
+{
+public:
+	V4L2ControlTest()
+		: V4L2VideoDeviceTest("vivid", "vivid-000-vid-cap") {}
+
+protected:
+	int run()
+	{
+		const V4L2ControlInfoMap &info = capture_->controls();
+
+		/* Test control enumeration. */
+		if (info.empty()) {
+			cerr << "Failed to enumerate controls" << endl;
+			return TestFail;
+		}
+
+		if (info.find(V4L2_CID_BRIGHTNESS) == info.end() ||
+		    info.find(V4L2_CID_CONTRAST) == info.end() ||
+		    info.find(V4L2_CID_SATURATION) == info.end()) {
+			cerr << "Missing controls" << endl;
+			return TestFail;
+		}
+
+		const V4L2ControlInfo &brightness = info.find(V4L2_CID_BRIGHTNESS)->second;
+		const V4L2ControlInfo &contrast = info.find(V4L2_CID_CONTRAST)->second;
+		const V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second;
+
+		/* Test getting controls. */
+		V4L2ControlList ctrls;
+		ctrls.add(V4L2_CID_BRIGHTNESS);
+		ctrls.add(V4L2_CID_CONTRAST);
+		ctrls.add(V4L2_CID_SATURATION);
+
+		int ret = capture_->getControls(&ctrls);
+		if (ret != 0) {
+			cerr << "Failed to get controls" << endl;
+			return TestFail;
+		}
+
+		if (ctrls[V4L2_CID_BRIGHTNESS]->value().get<int32_t>() == -1 ||
+		    ctrls[V4L2_CID_CONTRAST]->value().get<int32_t>() == -1 ||
+		    ctrls[V4L2_CID_SATURATION]->value().get<int32_t>() == -1) {
+			cerr << "Incorrect value for retrieved controls" << endl;
+			return TestFail;
+		}
+
+		/* Test setting controls. */
+		ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min();
+		ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max();
+		ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min();
+
+		ret = capture_->setControls(&ctrls);
+		if (ret != 0) {
+			cerr << "Failed to set controls" << endl;
+			return TestFail;
+		}
+
+		/* Test setting controls outside of range. */
+		ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get<int32_t>() - 1;
+		ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get<int32_t>() + 1;
+		ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get<int32_t>() + 1;
+
+		ret = capture_->setControls(&ctrls);
+		if (ret != 0) {
+			cerr << "Failed to set controls (out of range)" << endl;
+			return TestFail;
+		}
+
+		if (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() ||
+		    ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() ||
+		    ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get<int32_t>() + 1) {
+			cerr << "Controls not updated when set" << endl;
+			return TestFail;
+		}
+
+		return TestPass;
+	}
+};
+
+TEST_REGISTER(V4L2ControlTest);
diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build
index ad41898b5f8b..5c52da7219c2 100644
--- a/test/v4l2_videodevice/meson.build
+++ b/test/v4l2_videodevice/meson.build
@@ -2,6 +2,7 @@ 
 # They are not alphabetically sorted.
 v4l2_videodevice_tests = [
     [ 'double_open',        'double_open.cpp' ],
+    [ 'controls',           'controls.cpp' ],
     [ 'formats',            'formats.cpp' ],
     [ 'request_buffers',    'request_buffers.cpp' ],
     [ 'stream_on_off',      'stream_on_off.cpp' ],