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

[Jacopo Mondi]

Hi Laurent,

On Wed, Feb 27, 2019 at 08:44:02PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Tue, Feb 26, 2019 at 05:26:40PM +0100, Jacopo Mondi wrote:
> > Add test for V4L2Device set and get format methods.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  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' ],
>
> V4L2Device tests are soretd by order of increasing complexity, I think
> think this one should be last in the list.
>

May I say this is not the most intuitive ordering method?
last in the complexity list == first in the array?

> >  ]
> >
> >  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();
>
> override
>
> > +};
> > +
> > +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 "
> > +		     << "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 "
> > +		     << "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;
>
> With Kieran's comment addressed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> (I however wonder if we need both the 0 and UINT_MAX tests. Testing that
> the setFormat() function correctly updates the passed format is useful
> to test our implementation, but testing both 0 and UINT_MAX feels like
> testing the kernel instead, which is a bit out of scope)

Ah, I re-wrote this stupid tests a million times, as everytime I did
something a bit more complex I was actually testing the kernel driver
instead of the library. This seemed to me as a compromise between
going for testing the driver and a test with no actual value, but I
can drop one of the cases.

Thanks
   j

>
> > +}
> > +
> > +TEST_REGISTER(Format);
>
> --
> Regards,
>
> Laurent Pinchart
-------------- 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/20190228/35ae5051/attachment.sig>