[libcamera-devel] [PATCH v4 05/11] test: media_device: Create a common MediaDeviceTest base class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri May 17 11:02:12 CEST 2019
Hi Niklas,
Thank you for the patch.
On Fri, May 17, 2019 at 02:54:41AM +0200, Niklas Söderlund wrote:
> Before adding more tests which will act on the vimc pipeline break out a
> common base from media_device_link_test.cpp which already acts on vimc.
> The new common base class will help reduce code duplication.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> test/media_device/media_device_link_test.cpp | 71 ++++++--------------
> test/media_device/media_device_test.cpp | 36 ++++++++++
> test/media_device/media_device_test.h | 34 ++++++++++
> test/media_device/meson.build | 9 ++-
> 4 files changed, 99 insertions(+), 51 deletions(-)
> create mode 100644 test/media_device/media_device_test.cpp
> create mode 100644 test/media_device/media_device_test.h
>
> diff --git a/test/media_device/media_device_link_test.cpp b/test/media_device/media_device_link_test.cpp
> index 334bb44a6fc14371..60e502df8043f979 100644
> --- a/test/media_device/media_device_link_test.cpp
> +++ b/test/media_device/media_device_link_test.cpp
> @@ -4,13 +4,10 @@
> *
> * media_device_link_test.cpp - Tests link handling on VIMC media device
> */
> +
> #include <iostream>
> -#include <memory>
>
> -#include "device_enumerator.h"
> -#include "media_device.h"
> -
> -#include "test.h"
> +#include "media_device_test.h"
>
> using namespace libcamera;
> using namespace std;
> @@ -29,44 +26,21 @@ using namespace std;
> * loaded) the test is skipped.
> */
>
> -class MediaDeviceLinkTest : public Test
> +class MediaDeviceLinkTest : public MediaDeviceTest
> {
> - int init()
> - {
> - enumerator = unique_ptr<DeviceEnumerator>(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");
> - dev_ = enumerator->search(dm);
> - if (!dev_) {
> - cerr << "No VIMC media device found: skip test" << endl;
> - return TestSkip;
> - }
> -
> - if (!dev_->acquire()) {
> - cerr << "Unable to acquire media device "
> - << dev_->deviceNode() << endl;
> - return TestSkip;
> - }
> -
> - return 0;
> - }
> -
> int run()
> {
> + if (!media_->acquire()) {
> + cerr << "Unable to acquire media device "
> + << media_->deviceNode() << endl;
> + return TestFail;
> + }
This should be in MediaDeviceLinkTest::init() (which should call
MediaDeviceTest::init()) as the release call is in cleanup().
> +
> /*
> * First of all disable all links in the media graph to
> * ensure we start from a known state.
> */
> - if (dev_->disableLinks()) {
> + if (media_->disableLinks()) {
> cerr << "Failed to disable all links in the media graph";
> return TestFail;
> }
> @@ -76,26 +50,26 @@ class MediaDeviceLinkTest : public Test
> * different methods the media device offers.
> */
> string linkName("'Debayer A':[1] -> 'Scaler':[0]'");
> - MediaLink *link = dev_->link("Debayer A", 1, "Scaler", 0);
> + MediaLink *link = media_->link("Debayer A", 1, "Scaler", 0);
> if (!link) {
> cerr << "Unable to find link: " << linkName
> << " using lookup by name" << endl;
> return TestFail;
> }
>
> - MediaEntity *source = dev_->getEntityByName("Debayer A");
> + MediaEntity *source = media_->getEntityByName("Debayer A");
> if (!source) {
> cerr << "Unable to find entity: 'Debayer A'" << endl;
> return TestFail;
> }
>
> - MediaEntity *sink = dev_->getEntityByName("Scaler");
> + MediaEntity *sink = media_->getEntityByName("Scaler");
> if (!sink) {
> cerr << "Unable to find entity: 'Scaler'" << endl;
> return TestFail;
> }
>
> - MediaLink *link2 = dev_->link(source, 1, sink, 0);
> + MediaLink *link2 = media_->link(source, 1, sink, 0);
> if (!link2) {
> cerr << "Unable to find link: " << linkName
> << " using lookup by entity" << endl;
> @@ -108,7 +82,7 @@ class MediaDeviceLinkTest : public Test
> return TestFail;
> }
>
> - link2 = dev_->link(source->getPadByIndex(1),
> + link2 = media_->link(source->getPadByIndex(1),
> sink->getPadByIndex(0));
> if (!link2) {
> cerr << "Unable to find link: " << linkName
> @@ -160,7 +134,7 @@ class MediaDeviceLinkTest : public Test
>
> /* Try to get a non existing link. */
> linkName = "'Sensor A':[1] -> 'Scaler':[0]";
> - link = dev_->link("Sensor A", 1, "Scaler", 0);
> + link = media_->link("Sensor A", 1, "Scaler", 0);
> if (link) {
> cerr << "Link lookup for " << linkName
> << " succeeded but link does not exist"
> @@ -170,7 +144,7 @@ class MediaDeviceLinkTest : public Test
>
> /* Now get an immutable link and try to disable it. */
> linkName = "'Sensor A':[0] -> 'Raw Capture 0':[0]";
> - link = dev_->link("Sensor A", 0, "Raw Capture 0", 0);
> + link = media_->link("Sensor A", 0, "Raw Capture 0", 0);
> if (!link) {
> cerr << "Unable to find link: " << linkName
> << " using lookup by name" << endl;
> @@ -195,7 +169,7 @@ class MediaDeviceLinkTest : public Test
> * after disabling all links in the media graph.
> */
> linkName = "'Debayer B':[1] -> 'Scaler':[0]'";
> - link = dev_->link("Debayer B", 1, "Scaler", 0);
> + link = media_->link("Debayer B", 1, "Scaler", 0);
> if (!link) {
> cerr << "Unable to find link: " << linkName
> << " using lookup by name" << endl;
> @@ -215,7 +189,7 @@ class MediaDeviceLinkTest : public Test
> return TestFail;
> }
>
> - if (dev_->disableLinks()) {
> + if (media_->disableLinks()) {
> cerr << "Failed to disable all links in the media graph";
> return TestFail;
> }
> @@ -227,17 +201,14 @@ class MediaDeviceLinkTest : public Test
> return TestFail;
> }
>
> +
Unneeded blank line.
> return 0;
> }
>
> void cleanup()
> {
> - dev_->release();
> + media_->release();
> }
> -
> -private:
> - unique_ptr<DeviceEnumerator> enumerator;
> - shared_ptr<MediaDevice> dev_;
> };
>
> TEST_REGISTER(MediaDeviceLinkTest);
> diff --git a/test/media_device/media_device_test.cpp b/test/media_device/media_device_test.cpp
> new file mode 100644
> index 0000000000000000..055069ad1c331cbb
> --- /dev/null
> +++ b/test/media_device/media_device_test.cpp
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * libcamera MediaDevice API tests
* media_device_test.cpp - libcamera media device test base class
> + */
> +
> +#include <iostream>
> +
> +#include "media_device_test.h"
> +
> +using namespace libcamera;
> +using namespace std;
> +
> +int MediaDeviceTest::init()
> +{
> + enumerator_ = unique_ptr<DeviceEnumerator>(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_ = enumerator_->search(dm);
> + if (!media_) {
> + cerr << "No VIMC media device found: skip test" << endl;
> + return TestSkip;
> + }
> +
> + return TestPass;
> +}
> diff --git a/test/media_device/media_device_test.h b/test/media_device/media_device_test.h
> new file mode 100644
> index 0000000000000000..cdbd14841d5ccca2
> --- /dev/null
> +++ b/test/media_device/media_device_test.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * media_device_test.h - libcamera media device test base class
> + */
> +#ifndef __LIBCAMERA_MEDIADEVICE_TEST_H__
> +#define __LIBCAMERA_MEDIADEVICE_TEST_H__
> +
> +#include <memory>
> +
> +#include "device_enumerator.h"
> +#include "media_device.h"
> +
> +#include "test.h"
> +
> +using namespace libcamera;
> +
> +class MediaDeviceTest : public Test
> +{
> +public:
> + MediaDeviceTest()
> + : media_(nullptr), enumerator_(nullptr) {}
> +
> +protected:
> + int init();
> +
> + std::shared_ptr<MediaDevice> media_;
> +
> +private:
> + std::unique_ptr<DeviceEnumerator> enumerator_;
> +};
> +
> +#endif /* __LIBCAMERA_MEDIADEVICE_TEST_H__ */
> diff --git a/test/media_device/meson.build b/test/media_device/meson.build
> index d91a022fab190abf..364b4ecf662077ac 100644
> --- a/test/media_device/meson.build
> +++ b/test/media_device/meson.build
> @@ -1,11 +1,18 @@
> +libmediadevicetest_sources = files([
> + 'media_device_test.cpp',
> +])
> +
> media_device_tests = [
> ['media_device_print_test', 'media_device_print_test.cpp'],
> ['media_device_link_test', 'media_device_link_test.cpp'],
> ]
>
> +libmediadevicetest = static_library('libmediadevicetest', libmediadevicetest_sources,
> + include_directories : test_includes_internal)
That's a long name, do you think lib_mdev_test would be better ? Up to
you.
With these small issues fixed,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> +
> foreach t : media_device_tests
> exe = executable(t[0], t[1],
> - link_with : test_libraries,
> + link_with : [test_libraries, libmediadevicetest],
> include_directories : test_includes_internal)
>
> test(t[0], exe, suite: 'media_device', is_parallel: false)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list