[libcamera-devel] [PATCH v2 08/14] test: v4l2_videodevice: Add V4L2 control test
Jacopo Mondi
jacopo at jmondi.org
Sun Oct 13 16:07:59 CEST 2019
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 at 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 at 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 at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191013/375365b9/attachment.sig>
More information about the libcamera-devel
mailing list