[libcamera-devel] [PATCH v2 07/10] libcamera: ipa: add dummy IPA implementation

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Mon, Jun 03, 2019 at 07:16:34PM -0400, Paul Elder wrote:
> Add a dummy IPA module.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> Changes in v2:
> - use macros for defining the version fields in ipaModuleInfo
> 
>  src/ipa/ipa_dummy.cpp | 45 +++++++++++++++++++++++++++++++++++++++++++
>  src/ipa/meson.build   | 11 +++++++++++
>  2 files changed, 56 insertions(+)
>  create mode 100644 src/ipa/ipa_dummy.cpp
> 
> diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp
> new file mode 100644
> index 0000000..99a74ff
> --- /dev/null
> +++ b/src/ipa/ipa_dummy.cpp
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_dummy.cpp - Dummy Image Processing Algorithm module
> + */
> +
> +#include <iostream>
> +
> +#include <libcamera/ipa/ipa_interface.h>
> +#include <libcamera/ipa/ipa_module_info.h>
> +
> +namespace libcamera {
> +
> +class IPADummy : public IPAInterface
> +{
> +public:
> +	int init();
> +};
> +
> +int IPADummy::init()
> +{
> +	std::cout << "initializing dummy IPA!" << std::endl;
> +	return 0;
> +}
> +
> +/*
> + * External IPA module interface
> + */
> +
> +extern "C" {
> +const struct libcamera::IPAModuleInfo ipaModuleInfo = {

You're in the libcamera namespace, so there's no need for libcamera::.

> +	IPA_MODULE_API_VERSION,
> +	PIPELINE_VERSION(0, 1),
> +	"PipelineHandlerVimc",
> +	"Dummy IPA for Vimc",
> +};
> +
> +IPAInterface *ipaCreate()
> +{
> +	return new IPADummy();
> +}
> +};
> +
> +}; /* namespace libcamera */
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index be4f954..33d734d 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -1,2 +1,13 @@
> +ipa_dummy_sources = files([
> +    'ipa_dummy.cpp',
> +])
> +
> +ipa_dummy = shared_library('ipa_dummy',
> +                           ipa_dummy_sources,
> +                           name_prefix : '',
> +                           include_directories : libcamera_includes,
> +                           install : true,
> +                           install_dir : join_paths(get_option('libdir'), 'libcamera'))

I expect other IPA modules to need the same install_dir. How about

ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')

ipa_dummy = shared_library('ipa_dummy',
...
                           install_dir : ipa_install_dir)

> +
>  config_h.set('IPA_MODULE_DIR',
>               '"' + join_paths(get_option('prefix'), get_option('libdir'), 'libcamera') + '"')

And here

             '"' + join_paths(get_option('prefix'), ipa_install_dir) +'"')

With this fixed,

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

-- 
Regards,

Laurent Pinchart