[libcamera-devel] [PATCH v5 7/9] test: v4l2_subdevice: Add format handling test

Jacopo Mondi jacopo at jmondi.org
Fri Mar 1 10:25:41 CET 2019


Hi Laurent,

On Fri, Mar 01, 2019 at 12:09:20AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Thu, Feb 28, 2019 at 09:01:49PM +0100, Jacopo Mondi wrote:
> > Add test for video format get and set operations on V4L2Subdevice class.
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  test/meson.build                            |  1 +
> >  test/v4l2_subdevice/meson.build             | 10 +++
> >  test/v4l2_subdevice/test_formats.cpp        | 78 ++++++++++++++++++
> >  test/v4l2_subdevice/v4l2_subdevice_test.cpp | 89 +++++++++++++++++++++
> >  test/v4l2_subdevice/v4l2_subdevice_test.h   | 36 +++++++++
> >  5 files changed, 214 insertions(+)
> >  create mode 100644 test/v4l2_subdevice/meson.build
> >  create mode 100644 test/v4l2_subdevice/test_formats.cpp
> >  create mode 100644 test/v4l2_subdevice/v4l2_subdevice_test.cpp
> >  create mode 100644 test/v4l2_subdevice/v4l2_subdevice_test.h
> >
> > diff --git a/test/meson.build b/test/meson.build
> > index d515a716207e..5fb16fa6afb6 100644
> > --- a/test/meson.build
> > +++ b/test/meson.build
> > @@ -3,6 +3,7 @@ subdir('libtest')
> >  subdir('media_device')
> >  subdir('pipeline')
> >  subdir('v4l2_device')
> > +subdir('v4l2_subdevice')
> >
> >  public_tests = [
> >      ['event',                           'event.cpp'],
> > diff --git a/test/v4l2_subdevice/meson.build b/test/v4l2_subdevice/meson.build
> > new file mode 100644
> > index 000000000000..f45dca0d23d7
> > --- /dev/null
> > +++ b/test/v4l2_subdevice/meson.build
> > @@ -0,0 +1,10 @@
> > +v4l2_subdevice_tests = [
> > +  [ 'test_formats',             'test_formats.cpp'],
> > +]
> > +
> > +foreach t : v4l2_subdevice_tests
> > +    exe = executable(t[0], [t[1], 'v4l2_subdevice_test.cpp'],
> > +        link_with : test_libraries,
> > +        include_directories : test_includes_internal)
> > +    test(t[0], exe, suite: 'v4l2_subdevice', is_parallel: false)
> > +endforeach
> > diff --git a/test/v4l2_subdevice/test_formats.cpp b/test/v4l2_subdevice/test_formats.cpp
> > new file mode 100644
> > index 000000000000..af954459a3d8
> > --- /dev/null
> > +++ b/test/v4l2_subdevice/test_formats.cpp
> > @@ -0,0 +1,78 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * libcamera V4L2 Subdevice format handling test
> > + */
> > +
> > +#include <climits>
> > +#include <iostream>
> > +
> > +#include "v4l2_subdevice.h"
> > +#include "v4l2_subdevice_test.h"
> > +
> > +using namespace std;
> > +using namespace libcamera;
> > +
> > +/* Test format handling on the "Scaler" subdevice of vimc media device.  */
> > +
> > +class FormatHandlingTest : public V4L2SubdeviceTest
> > +{
> > +protected:
> > +	int run() override;
> > +};
> > +
> > +int FormatHandlingTest::run()
> > +{
> > +	V4L2SubdeviceFormat format = {};
> > +
> > +	/*
> > +	 * Get format on a non-existing Scaler pad: expect failure.
> > +	 */
> > +	int ret = scaler_->getFormat(2, &format);
> > +	if (!ret) {
> > +		cerr << "Getting format on a non existing pad should fail" << endl;
> > +		return TestFail;
> > +	}
> > +
> > +	ret = scaler_->getFormat(0, &format);
> > +	if (ret) {
> > +		cerr << "Failed to get format" << endl;
> > +		return TestFail;
> > +	}
> > +
> > +	/*
> > +	 * Set unrealistic image resolutions and make sure it gets updated.
> > +	 */
> > +	format.width = UINT_MAX;
> > +	format.height = UINT_MAX;
> > +	ret = scaler_->setFormat(0, &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 = scaler_->setFormat(0, &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;
> > +}
> > +
> > +TEST_REGISTER(FormatHandlingTest);
> > diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp
> > new file mode 100644
> > index 000000000000..a110f05339a1
> > --- /dev/null
> > +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp
> > @@ -0,0 +1,89 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * v4l2_subdevice_test.cpp - VIMC-based V4L2 subdevice test
> > + */
> > +
> > +#include <iostream>
> > +#include <string.h>
> > +#include <sys/stat.h>
> > +
> > +#include "device_enumerator.h"
> > +#include "media_device.h"
> > +#include "v4l2_subdevice.h"
> > +#include "v4l2_subdevice_test.h"
> > +
> > +using namespace std;
> > +using namespace libcamera;
> > +
> > +/*
> > + * This test runs on vimc media device. For a description of vimc, in the
> > + * context of libcamera testing, please refer to
> > + * 'test/media_device/media_device_link_test.cpp' file.
> > + *
> > + * If the vimc module is not loaded, the test gets skipped.
> > + */
> > +
> > +int V4L2SubdeviceTest::init()
> > +{
> > +	enumerator_ = DeviceEnumerator::create();
> > +	if (!enumerator_) {
> > +		cerr << "Failed to create device enumerator" << endl;
> > +		return TestFail;
> > +	}
> > +
> > +	if (enumerator_->enumerate()) {
> > +		cerr << "Failed to enumerate media devices" << endl;
> > +		return TestFail;
> > +	}
> > +
> > +	DeviceMatch dm("vimc");
> > +	media_ = std::move(enumerator_->search(dm));
> > +	if (!media_) {
> > +		cerr << "Unable to find \'vimc\' media device node" << endl;
> > +		return TestSkip;
> > +	}
> > +
> > +	media_->acquire();
> > +
> > +	int ret = media_->open();
> > +	if (ret) {
> > +		cerr << "Unable to open media device: " << media_->deviceNode()
> > +		     << ": " << strerror(ret) << endl;
> > +		media_->release();
> > +		return TestSkip;
> > +	}
> > +
> > +	MediaEntity *videoEntity = media_->getEntityByName("Scaler");
> > +	if (!videoEntity) {
> > +		cerr << "Unable to find media entity 'Scaler'" << endl;
> > +		media_->release();
> > +		return TestFail;
> > +	}
> > +
> > +	scaler_ = new V4L2Subdevice(videoEntity);
> > +	if (!scaler_) {
>
> This can't happen, can it ?
>

Indeed. Well, in case the system runs out of memory blah blah... but
we'll have other issues bigger than this in that case.

> > +		cerr << "Unable to create media device from media entity: "
> > +		     << videoEntity->deviceNode();
> > +		media_->release();
> > +		return TestFail;
> > +	}
> > +
> > +	ret = scaler_->open();
> > +	if (ret) {
> > +		cerr << "Unable to open video subdevice "
> > +		     << scaler_->deviceNode() << endl;
> > +		media_->release();
> > +		return TestSkip;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +void V4L2SubdeviceTest::cleanup()
> > +{
> > +	media_->release();
> > +
> > +	delete scaler_;
> > +}
> > diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.h b/test/v4l2_subdevice/v4l2_subdevice_test.h
> > new file mode 100644
> > index 000000000000..1d8e08a9c2b9
> > --- /dev/null
> > +++ b/test/v4l2_subdevice/v4l2_subdevice_test.h
> > @@ -0,0 +1,36 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * v4l2_subdevice_test.h - VIMC-based V4L2 subdevice test
> > + */
> > +
> > +#ifndef __LIBCAMERA_V4L2_SUBDEVICE_TEST_H__
> > +#define __LIBCAMERA_V4L2_SUBDEVICE_TEST_H__
> > +
> > +#include <libcamera/buffer.h>
> > +
> > +#include "device_enumerator.h"
> > +#include "media_device.h"
> > +#include "test.h"
> > +#include "v4l2_subdevice.h"
> > +
> > +using namespace libcamera;
> > +
> > +class V4L2SubdeviceTest : public Test
> > +{
> > +public:
> > +	V4L2SubdeviceTest()
> > +		: scaler_(nullptr){};
> > +
> > +protected:
> > +	int init() override;
> > +	virtual int run() = 0;
>
> No need for this.

Oh, I had a linker error when I removed this which is now gone, so I
suppose it was something different. I'll remove it.
>
> With these small issues fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Thanks
  j

>
> > +	void cleanup() override;
> > +
> > +	std::unique_ptr<DeviceEnumerator> enumerator_;
> > +	std::shared_ptr<MediaDevice> media_;
> > +	V4L2Subdevice *scaler_;
> > +};
> > +
> > +#endif /* __LIBCAMERA_V4L2_SUBDEVICE_TEST_H__ */
>
> --
> 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/20190301/e911b4a9/attachment.sig>


More information about the libcamera-devel mailing list