[libcamera-devel,1/8] libcamera: ipa_interface: add header
diff mbox series

Message ID 20190527223540.21855-2-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • Add IPAManager and IPAInterface
Related show

Commit Message

Paul Elder May 27, 2019, 10:35 p.m. UTC 
Define an IPAInterface class which will contain an IPA implementation.
The methods that the IPAInterface exposes form the interface to the IPA
implementation, hence the name. IPA module shared objects will implement
this class.

This also means that IPA module shared objects must be implemented in
C++, so remove the C test IPA module.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 include/libcamera/ipa/ipa_interface.h | 22 ++++++++++++++++++++++
 include/libcamera/meson.build         |  1 +
 src/libcamera/ipa_interface.cpp       | 27 +++++++++++++++++++++++++++
 src/libcamera/meson.build             |  1 +
 test/ipa/ipa_test.cpp                 |  2 --
 test/ipa/meson.build                  |  1 -
 test/ipa/shared_test.c                |  6 ------
 7 files changed, 51 insertions(+), 9 deletions(-)
 create mode 100644 include/libcamera/ipa/ipa_interface.h
 create mode 100644 src/libcamera/ipa_interface.cpp
 delete mode 100644 test/ipa/shared_test.c

Comments

Laurent Pinchart May 28, 2019, 2:58 p.m. UTC | #1 
Hi Paul,

Thank you for the patch.

On Mon, May 27, 2019 at 06:35:33PM -0400, Paul Elder wrote:
> Define an IPAInterface class which will contain an IPA implementation.
> The methods that the IPAInterface exposes form the interface to the IPA
> implementation, hence the name. IPA module shared objects will implement
> this class.
> 
> This also means that IPA module shared objects must be implemented in
> C++, so remove the C test IPA module.

You could have split it in two patches, but it's ok.

> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  include/libcamera/ipa/ipa_interface.h | 22 ++++++++++++++++++++++
>  include/libcamera/meson.build         |  1 +
>  src/libcamera/ipa_interface.cpp       | 27 +++++++++++++++++++++++++++
>  src/libcamera/meson.build             |  1 +
>  test/ipa/ipa_test.cpp                 |  2 --
>  test/ipa/meson.build                  |  1 -
>  test/ipa/shared_test.c                |  6 ------
>  7 files changed, 51 insertions(+), 9 deletions(-)
>  create mode 100644 include/libcamera/ipa/ipa_interface.h
>  create mode 100644 src/libcamera/ipa_interface.cpp
>  delete mode 100644 test/ipa/shared_test.c
> 
> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
> new file mode 100644
> index 0000000..2c5eb1f
> --- /dev/null
> +++ b/include/libcamera/ipa/ipa_interface.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_interface.h - Image Processing Algorithm interface
> + */
> +#ifndef __LIBCAMERA_IPA_INTERFACE_H__
> +#define __LIBCAMERA_IPA_INTERFACE_H__
> +
> +namespace libcamera {
> +
> +class IPAInterface
> +{
> +public:
> +	virtual ~IPAInterface() {}
> +
> +	virtual int init() = 0;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPA_INTERFACE_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 1fcf6b5..1b86fdc 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -5,6 +5,7 @@ libcamera_api = files([
>      'event_dispatcher.h',
>      'event_notifier.h',
>      'geometry.h',
> +    'ipa/ipa_interface.h',
>      'ipa/ipa_module_info.h',
>      'object.h',
>      'request.h',
> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
> new file mode 100644
> index 0000000..9d30da2
> --- /dev/null
> +++ b/src/libcamera/ipa_interface.cpp
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_interface.cpp - Image Processing Algorithm interface
> + */
> +
> +#include <libcamera/ipa/ipa_interface.h>
> +
> +/**
> + * \file ipa_interface.h
> + * \brief Image Processing Algorithm interface
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \class IPAInterface
> + * \brief Interface for IPA implementation
> + */
> +
> +/**
> + * \fn IPAInterface::init()
> + * \brief Initialise the IPAInterface
> + */

As this is just a skeleton we'll fix it later, but in general
documentation should be more detailed than this for public APIs.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 6a73580..07335e5 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -10,6 +10,7 @@ libcamera_sources = files([
>      'event_notifier.cpp',
>      'formats.cpp',
>      'geometry.cpp',
> +    'ipa_interface.cpp',
>      'ipa_module.cpp',
>      'log.cpp',
>      'media_device.cpp',
> diff --git a/test/ipa/ipa_test.cpp b/test/ipa/ipa_test.cpp
> index 9861ee2..2dbc702 100644
> --- a/test/ipa/ipa_test.cpp
> +++ b/test/ipa/ipa_test.cpp
> @@ -60,8 +60,6 @@ protected:
>  			9001,
>  		};
>  
> -		count += runTest("test/ipa/ipa-dummy-c.so", testInfo);
> -
>  		count += runTest("test/ipa/ipa-dummy-cpp.so", testInfo);
>  
>  		if (count < 0)
> diff --git a/test/ipa/meson.build b/test/ipa/meson.build
> index ecde313..08ee95c 100644
> --- a/test/ipa/meson.build
> +++ b/test/ipa/meson.build
> @@ -1,5 +1,4 @@
>  ipa_modules_sources = [
> -    ['ipa-dummy-c',   'shared_test.c'],
>      ['ipa-dummy-cpp', 'shared_test.cpp'],
>  ]
>  
> diff --git a/test/ipa/shared_test.c b/test/ipa/shared_test.c
> deleted file mode 100644
> index 87d182b..0000000
> --- a/test/ipa/shared_test.c
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -#include <libcamera/ipa/ipa_module_info.h>
> -
> -const struct IPAModuleInfo ipaModuleInfo = {
> -	.name = "It's over nine thousand!",
> -	.version = 9001,
> -};
Niklas Söderlund June 3, 2019, 9:54 p.m. UTC | #2 
Hi Paul,

Thanks for your work.

On 2019-05-27 18:35:33 -0400, Paul Elder wrote:
> Define an IPAInterface class which will contain an IPA implementation.

s/will contain/describes/

> The methods that the IPAInterface exposes form the interface to the IPA
> implementation, hence the name. IPA module shared objects will implement
> this class.

How about,

The IPAInterface acts as a base for IPA implementations and describes 
its interface. This commit only describes a skeleton interface with a 
single init() method.

> 
> This also means that IPA module shared objects must be implemented in
> C++, so remove the C test IPA module.

I think you should move this to a separate patch.

> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  include/libcamera/ipa/ipa_interface.h | 22 ++++++++++++++++++++++
>  include/libcamera/meson.build         |  1 +
>  src/libcamera/ipa_interface.cpp       | 27 +++++++++++++++++++++++++++
>  src/libcamera/meson.build             |  1 +
>  test/ipa/ipa_test.cpp                 |  2 --
>  test/ipa/meson.build                  |  1 -
>  test/ipa/shared_test.c                |  6 ------
>  7 files changed, 51 insertions(+), 9 deletions(-)
>  create mode 100644 include/libcamera/ipa/ipa_interface.h
>  create mode 100644 src/libcamera/ipa_interface.cpp
>  delete mode 100644 test/ipa/shared_test.c
> 
> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
> new file mode 100644
> index 0000000..2c5eb1f
> --- /dev/null
> +++ b/include/libcamera/ipa/ipa_interface.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_interface.h - Image Processing Algorithm interface
> + */
> +#ifndef __LIBCAMERA_IPA_INTERFACE_H__
> +#define __LIBCAMERA_IPA_INTERFACE_H__
> +
> +namespace libcamera {
> +
> +class IPAInterface
> +{
> +public:
> +	virtual ~IPAInterface() {}
> +
> +	virtual int init() = 0;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPA_INTERFACE_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 1fcf6b5..1b86fdc 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -5,6 +5,7 @@ libcamera_api = files([
>      'event_dispatcher.h',
>      'event_notifier.h',
>      'geometry.h',
> +    'ipa/ipa_interface.h',
>      'ipa/ipa_module_info.h',
>      'object.h',
>      'request.h',
> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
> new file mode 100644
> index 0000000..9d30da2
> --- /dev/null
> +++ b/src/libcamera/ipa_interface.cpp
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_interface.cpp - Image Processing Algorithm interface
> + */
> +
> +#include <libcamera/ipa/ipa_interface.h>
> +
> +/**
> + * \file ipa_interface.h
> + * \brief Image Processing Algorithm interface
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \class IPAInterface
> + * \brief Interface for IPA implementation

s/IPA implementation/IPA implementations/

or

s/IPA implementation/an IPA implementation/

> + */
> +
> +/**
> + * \fn IPAInterface::init()
> + * \brief Initialise the IPAInterface
> + */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 6a73580..07335e5 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -10,6 +10,7 @@ libcamera_sources = files([
>      'event_notifier.cpp',
>      'formats.cpp',
>      'geometry.cpp',
> +    'ipa_interface.cpp',
>      'ipa_module.cpp',
>      'log.cpp',
>      'media_device.cpp',
> diff --git a/test/ipa/ipa_test.cpp b/test/ipa/ipa_test.cpp
> index 9861ee2..2dbc702 100644
> --- a/test/ipa/ipa_test.cpp
> +++ b/test/ipa/ipa_test.cpp
> @@ -60,8 +60,6 @@ protected:
>  			9001,
>  		};
>  
> -		count += runTest("test/ipa/ipa-dummy-c.so", testInfo);
> -
>  		count += runTest("test/ipa/ipa-dummy-cpp.so", testInfo);
>  
>  		if (count < 0)
> diff --git a/test/ipa/meson.build b/test/ipa/meson.build
> index ecde313..08ee95c 100644
> --- a/test/ipa/meson.build
> +++ b/test/ipa/meson.build
> @@ -1,5 +1,4 @@
>  ipa_modules_sources = [
> -    ['ipa-dummy-c',   'shared_test.c'],
>      ['ipa-dummy-cpp', 'shared_test.cpp'],
>  ]
>  
> diff --git a/test/ipa/shared_test.c b/test/ipa/shared_test.c
> deleted file mode 100644
> index 87d182b..0000000
> --- a/test/ipa/shared_test.c
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -#include <libcamera/ipa/ipa_module_info.h>
> -
> -const struct IPAModuleInfo ipaModuleInfo = {
> -	.name = "It's over nine thousand!",
> -	.version = 9001,
> -};
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
new file mode 100644
index 0000000..2c5eb1f
--- /dev/null
+++ b/include/libcamera/ipa/ipa_interface.h
@@ -0,0 +1,22 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2019, Google Inc.
+ *
+ * ipa_interface.h - Image Processing Algorithm interface
+ */
+#ifndef __LIBCAMERA_IPA_INTERFACE_H__
+#define __LIBCAMERA_IPA_INTERFACE_H__
+
+namespace libcamera {
+
+class IPAInterface
+{
+public:
+	virtual ~IPAInterface() {}
+
+	virtual int init() = 0;
+};
+
+} /* namespace libcamera */
+
+#endif /* __LIBCAMERA_IPA_INTERFACE_H__ */
diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
index 1fcf6b5..1b86fdc 100644
--- a/include/libcamera/meson.build
+++ b/include/libcamera/meson.build
@@ -5,6 +5,7 @@  libcamera_api = files([
     'event_dispatcher.h',
     'event_notifier.h',
     'geometry.h',
+    'ipa/ipa_interface.h',
     'ipa/ipa_module_info.h',
     'object.h',
     'request.h',
diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
new file mode 100644
index 0000000..9d30da2
--- /dev/null
+++ b/src/libcamera/ipa_interface.cpp
@@ -0,0 +1,27 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2019, Google Inc.
+ *
+ * ipa_interface.cpp - Image Processing Algorithm interface
+ */
+
+#include <libcamera/ipa/ipa_interface.h>
+
+/**
+ * \file ipa_interface.h
+ * \brief Image Processing Algorithm interface
+ */
+
+namespace libcamera {
+
+/**
+ * \class IPAInterface
+ * \brief Interface for IPA implementation
+ */
+
+/**
+ * \fn IPAInterface::init()
+ * \brief Initialise the IPAInterface
+ */
+
+} /* namespace libcamera */
diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
index 6a73580..07335e5 100644
--- a/src/libcamera/meson.build
+++ b/src/libcamera/meson.build
@@ -10,6 +10,7 @@  libcamera_sources = files([
     'event_notifier.cpp',
     'formats.cpp',
     'geometry.cpp',
+    'ipa_interface.cpp',
     'ipa_module.cpp',
     'log.cpp',
     'media_device.cpp',
diff --git a/test/ipa/ipa_test.cpp b/test/ipa/ipa_test.cpp
index 9861ee2..2dbc702 100644
--- a/test/ipa/ipa_test.cpp
+++ b/test/ipa/ipa_test.cpp
@@ -60,8 +60,6 @@  protected:
 			9001,
 		};
 
-		count += runTest("test/ipa/ipa-dummy-c.so", testInfo);
-
 		count += runTest("test/ipa/ipa-dummy-cpp.so", testInfo);
 
 		if (count < 0)
diff --git a/test/ipa/meson.build b/test/ipa/meson.build
index ecde313..08ee95c 100644
--- a/test/ipa/meson.build
+++ b/test/ipa/meson.build
@@ -1,5 +1,4 @@ 
 ipa_modules_sources = [
-    ['ipa-dummy-c',   'shared_test.c'],
     ['ipa-dummy-cpp', 'shared_test.cpp'],
 ]
 
diff --git a/test/ipa/shared_test.c b/test/ipa/shared_test.c
deleted file mode 100644
index 87d182b..0000000
--- a/test/ipa/shared_test.c
+++ /dev/null
@@ -1,6 +0,0 @@ 
-#include <libcamera/ipa/ipa_module_info.h>
-
-const struct IPAModuleInfo ipaModuleInfo = {
-	.name = "It's over nine thousand!",
-	.version = 9001,
-};