[libcamera-devel] [PATCH v3 7/8] test: v4l2_device: Add format handling test

[Kieran Bingham]

Hi Jacopo,

On 26/02/2019 16:26, Jacopo Mondi wrote:
> Add test for V4L2Device set and get format methods.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

With minor topics below tackled,

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  test/v4l2_device/meson.build      |  1 +
>  test/v4l2_device/test_formats.cpp | 65 +++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
>  create mode 100644 test/v4l2_device/test_formats.cpp
> 
> diff --git a/test/v4l2_device/meson.build b/test/v4l2_device/meson.build
> index 9f7a7545ac9b..e5e50faac282 100644
> --- a/test/v4l2_device/meson.build
> +++ b/test/v4l2_device/meson.build
> @@ -6,6 +6,7 @@ v4l2_device_tests = [
>    [ 'stream_on_off',      'stream_on_off.cpp' ],
>    [ 'capture_async',      'capture_async.cpp' ],
>    [ 'buffer_sharing',     'buffer_sharing.cpp' ],
> +  [ 'test_formats',	  'test_formats.cpp' ],

It looks like there might be a trivial whitespace issue in the table here.

>  ]
>  
>  foreach t : v4l2_device_tests
> diff --git a/test/v4l2_device/test_formats.cpp b/test/v4l2_device/test_formats.cpp
> new file mode 100644
> index 000000000000..dcb05a3904f7
> --- /dev/null
> +++ b/test/v4l2_device/test_formats.cpp
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * libcamera V4L2 device format handling test
> + */
> +
> +#include <climits>
> +#include <iostream>
> +
> +#include "v4l2_device.h"
> +
> +#include "v4l2_device_test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +class Format : public V4L2DeviceTest
> +{
> +protected:
> +	int run();
> +};
> +
> +int Format::run()
> +{
> +	V4L2DeviceFormat format = {};
> +
> +	int ret = capture_->getFormat(&format);
> +	if (ret) {
> +		cerr << "Failed to get format" << endl;
> +		return TestFail;
> +	}
> +
> +	format.width = UINT_MAX;
> +	format.height = UINT_MAX;
> +	ret = capture_->setFormat(&format);
> +	if (ret) {
> +		cerr << "Failed to set format: image resolution is wrong, but "

s/wrong/unsupported/ ? or (invalid, incompatible...)?



> +		     << "setFormat() should not fail." << endl;
> +		return TestFail;
> +	}
> +
> +	if (format.width == UINT_MAX || format.height == UINT_MAX) {
> +		cerr << "Failed to update image format" << endl;
> +		return TestFail;
> +	}
> +
> +	format.width = 0;
> +	format.height = 0;
> +	ret = capture_->setFormat(&format);
> +	if (ret) {
> +		cerr << "Failed to set format: image resolution is wrong, but "

Could we distinguish the error message between the minimum test and
maximum test failures so that if one fails it can be identified easily?

> +		     << "setFormat() should not fail." << endl;
> +		return TestFail;
> +	}
> +
> +	if (format.width == 0 || format.height == 0) {
> +		cerr << "Failed to update image format" << endl;
> +		return TestFail;
> +	}
> +
> +	return TestPass;
> +}
> +
> +TEST_REGISTER(Format);
> 

-- 
Regards
--
Kieran