[libcamera-devel] [PATCH v2 05/11] test: media_device: Create a common MediaDeviceTest base class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat May 11 14:46:21 CEST 2019
Hi Niklas,
Thank you for the patch.
On Wed, May 08, 2019 at 05:18:25PM +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 | 73 +++++---------------
> test/media_device/media_device_test.cpp | 41 +++++++++++
> test/media_device/media_device_test.h | 35 ++++++++++
> test/media_device/meson.build | 2 +-
> 4 files changed, 96 insertions(+), 55 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..26aedf0ea23ec709 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;
> + }
Shouldn't this go to he init() method of the base class, given that 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;
> }
> @@ -229,15 +203,6 @@ class MediaDeviceLinkTest : public Test
>
> return 0;
> }
> -
> - void cleanup()
> - {
> - dev_->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..710613eeb80b6de1
> --- /dev/null
> +++ b/test/media_device/media_device_test.cpp
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * libcamera MediaDevice API tests
> + */
> +
> +#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;
> +}
> +
> +void MediaDeviceTest::cleanup()
> +{
> + media_->release();
> +}
> diff --git a/test/media_device/media_device_test.h b/test/media_device/media_device_test.h
> new file mode 100644
> index 0000000000000000..af5c89f4fff2f63e
> --- /dev/null
> +++ b/test/media_device/media_device_test.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * media_device_test.h - libcamera media devie base class
s/devie/device test/
> + */
> +#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();
> + void cleanup();
> +
> + 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..1005685409d99aa7 100644
> --- a/test/media_device/meson.build
> +++ b/test/media_device/meson.build
> @@ -4,7 +4,7 @@ media_device_tests = [
> ]
>
> foreach t : media_device_tests
> - exe = executable(t[0], t[1],
> + exe = executable(t[0], [t[1], 'media_device_test.cpp'],
This is a bit of a hack. How about creating a static library for this
file, the same way we do with libtest ? It can live in this directory,
there's no need for a subdirectory or a separate meson.build file.
Alternatively you could include media_device_test.cpp in libtest, but
that seems to be a bit of a hack too.
> link_with : test_libraries,
> include_directories : test_includes_internal)
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list