[libcamera-devel,v4,05/11] test: media_device: Create a common MediaDeviceTest base class
diff mbox series

Message ID 20190517005447.27171-6-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamerea: Add support for exclusive access to cameras between processes
Related show

Commit Message

Niklas Söderlund May 17, 2019, 12:54 a.m. UTC 
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@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

Comments

Laurent Pinchart May 17, 2019, 9:02 a.m. UTC | #1 
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@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@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)

Patch
diff mbox series 

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;
+		}
+
 		/*
 		 * 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;
 		}
 
+
 		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
+ */
+
+#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)
+
 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)