[libcamera-devel] [PATCH v3 1/9] ipa: move libipa::Algorithm to ipa/ipu3/algorithms
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Wed Aug 18 21:09:48 CEST 2021
On 18/08/2021 20:52, Laurent Pinchart wrote:
> On Wed, Aug 18, 2021 at 05:53:55PM +0200, Jean-Michel Hautbois wrote:
>> The abstract Algorithm class was originally placed in libipa as an attempt to
>> define a generic algorithm container.
>> This was a little optimistic and pushed a bit far too early.
>>
>> Move the Algorithm class into the IPU3 which is the only user of the class,
>> as we adapt it to support modular algorithm components for the IPU3.
>>
>> Not documenting the namespace may cause issues with Doxygen in libipa.
>> The file libipa.cpp is thus created as an empty file for now, but we can
>> leverage it in the future to add more global libipa documentation, and possibly
>> code too.
>
> Please reflow the commit message to 72 columns. Isn't that the default
> btw, or do I have a local configuration that I have forgotten about ?
It is the default, but I noticed that when I go inside a line to modify
it it may not wrap properly, while typing the sentence entirely works
fine...
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> Reviewed-by: Kieran Bingham kieran.bingham at ideasonboard.com
>> ---
>> .../{libipa => ipu3/algorithms}/algorithm.cpp | 17 ++++---------
>> src/ipa/ipu3/algorithms/algorithm.h | 24 +++++++++++++++++++
>> src/ipa/ipu3/algorithms/meson.build | 5 ++++
>> src/ipa/ipu3/ipu3_agc.h | 2 +-
>> src/ipa/ipu3/ipu3_awb.h | 2 +-
>> src/ipa/ipu3/meson.build | 4 ++++
>> src/ipa/libipa/algorithm.h | 24 -------------------
>> src/ipa/libipa/libipa.cpp | 22 +++++++++++++++++
>> src/ipa/libipa/meson.build | 5 ++--
>> 9 files changed, 63 insertions(+), 42 deletions(-)
>> rename src/ipa/{libipa => ipu3/algorithms}/algorithm.cpp (55%)
>> create mode 100644 src/ipa/ipu3/algorithms/algorithm.h
>> create mode 100644 src/ipa/ipu3/algorithms/meson.build
>> delete mode 100644 src/ipa/libipa/algorithm.h
>> create mode 100644 src/ipa/libipa/libipa.cpp
>>
>> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/ipu3/algorithms/algorithm.cpp
>> similarity index 55%
>> rename from src/ipa/libipa/algorithm.cpp
>> rename to src/ipa/ipu3/algorithms/algorithm.cpp
>> index 930f9353..325d34f0 100644
>> --- a/src/ipa/libipa/algorithm.cpp
>> +++ b/src/ipa/ipu3/algorithms/algorithm.cpp
>> @@ -2,7 +2,7 @@
>> /*
>> * Copyright (C) 2021, Ideas On Board
>> *
>> - * algorithm.cpp - ISP control algorithms
>> + * algorithm.h - IPU3 control algorithm interface
>
> s/algorithm.h/algorithm.cpp/
>
Oh, how did I let that one !
>> */
>>
>> #include "algorithm.h"
>> @@ -14,26 +14,17 @@
>>
>> namespace libcamera {
>>
>> -/**
>> - * \brief The IPA namespace
>> - *
>> - * The IPA namespace groups all types specific to IPA modules. It serves as the
>> - * top-level namespace for the IPA library libipa, and also contains
>> - * module-specific namespaces for IPA modules.
>> - */
>> -namespace ipa {
>> +namespace ipa::ipu3 {
>>
>> /**
>> * \class Algorithm
>> - * \brief The base class for all IPA algorithms
>> + * \brief The base class for all IPU3 algorithms
>> *
>> * The Algorithm class defines a standard interface for IPA algorithms. By
>> * abstracting algorithms, it makes possible the implementation of generic code
>> * to manage algorithms regardless of their specific type.
>> */
>>
>> -Algorithm::~Algorithm() = default;
>> -
>> -} /* namespace ipa */
>> +} /* namespace ipa::ipu3 */
>>
>> } /* namespace libcamera */
>> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h
>> new file mode 100644
>> index 00000000..072f01c4
>> --- /dev/null
>> +++ b/src/ipa/ipu3/algorithms/algorithm.h
>> @@ -0,0 +1,24 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2021, Ideas On Board
>> + *
>> + * algorithm.h - IPU3 control algorithm interface
>> + */
>> +#ifndef __LIBCAMERA_IPA_IPU3_ALGORITHM_H__
>> +#define __LIBCAMERA_IPA_IPU3_ALGORITHM_H__
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa::ipu3 {
>> +
>> +class Algorithm
>> +{
>> +public:
>> + virtual ~Algorithm() {}
>> +};
>> +
>> +} /* namespace ipa::ipu3 */
>> +
>> +} /* namespace libcamera */
>> +
>> +#endif /* __LIBCAMERA_IPA_IPU3_ALGORITHM_H__ */
>> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
>> new file mode 100644
>> index 00000000..dc538b79
>> --- /dev/null
>> +++ b/src/ipa/ipu3/algorithms/meson.build
>> @@ -0,0 +1,5 @@
>> +# SPDX-License-Identifier: CC0-1.0
>> +
>> +ipu3_ipa_algorithms = files([
>> + 'algorithm.cpp',
>> +])
>
> We may need to also track headers, in order to rebuild the documentation
> when they change (doxygen doesn't need an explicit list of headers, but
> meson needs to know about the dependency of the documentation to the
> headers). This can be handled later, when plumbing this with doxygen.
>
Yes, I hesitated, but could not foresee a need for it now.
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Thx :-)
>
>> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h
>> index 9f3d4257..f00b98d6 100644
>> --- a/src/ipa/ipu3/ipu3_agc.h
>> +++ b/src/ipa/ipu3/ipu3_agc.h
>> @@ -13,7 +13,7 @@
>>
>> #include <libcamera/geometry.h>
>>
>> -#include "libipa/algorithm.h"
>> +#include "algorithms/algorithm.h"
>>
>> namespace libcamera {
>>
>> diff --git a/src/ipa/ipu3/ipu3_awb.h b/src/ipa/ipu3/ipu3_awb.h
>> index 122cf68c..ea2d4320 100644
>> --- a/src/ipa/ipu3/ipu3_awb.h
>> +++ b/src/ipa/ipu3/ipu3_awb.h
>> @@ -13,7 +13,7 @@
>>
>> #include <libcamera/geometry.h>
>>
>> -#include "libipa/algorithm.h"
>> +#include "algorithms/algorithm.h"
>>
>> namespace libcamera {
>>
>> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build
>> index b6364190..fcb27d68 100644
>> --- a/src/ipa/ipu3/meson.build
>> +++ b/src/ipa/ipu3/meson.build
>> @@ -1,5 +1,7 @@
>> # SPDX-License-Identifier: CC0-1.0
>>
>> +subdir('algorithms')
>> +
>> ipa_name = 'ipa_ipu3'
>>
>> ipu3_ipa_sources = files([
>> @@ -8,6 +10,8 @@ ipu3_ipa_sources = files([
>> 'ipu3_awb.cpp',
>> ])
>>
>> +ipu3_ipa_sources += ipu3_ipa_algorithms
>> +
>> mod = shared_module(ipa_name,
>> [ipu3_ipa_sources, libcamera_generated_ipa_headers],
>> name_prefix : '',
>> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h
>> deleted file mode 100644
>> index 89cee4c4..00000000
>> --- a/src/ipa/libipa/algorithm.h
>> +++ /dev/null
>> @@ -1,24 +0,0 @@
>> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> -/*
>> - * Copyright (C) 2021, Ideas On Board
>> - *
>> - * algorithm.h - ISP control algorithm interface
>> - */
>> -#ifndef __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__
>> -#define __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__
>> -
>> -namespace libcamera {
>> -
>> -namespace ipa {
>> -
>> -class Algorithm
>> -{
>> -public:
>> - virtual ~Algorithm();
>> -};
>> -
>> -} /* namespace ipa */
>> -
>> -} /* namespace libcamera */
>> -
>> -#endif /* __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ */
>> diff --git a/src/ipa/libipa/libipa.cpp b/src/ipa/libipa/libipa.cpp
>> new file mode 100644
>> index 00000000..08bc3541
>> --- /dev/null
>> +++ b/src/ipa/libipa/libipa.cpp
>> @@ -0,0 +1,22 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2021, Ideas On Board
>> + *
>> + * libipa.cpp - libipa interface
>> + */
>> +
>> +namespace libcamera {
>> +
>> +/**
>> + * \brief The IPA namespace
>> + *
>> + * The IPA namespace groups all types specific to IPA modules. It serves as the
>> + * top-level namespace for the IPA library libipa, and also contains
>> + * module-specific namespaces for IPA modules.
>> + */
>> +namespace ipa {
>> +
>> +} /* namespace ipa */
>> +
>> +} /* namespace libcamera */
>> +
>> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
>> index 3fda7c00..4d073a03 100644
>> --- a/src/ipa/libipa/meson.build
>> +++ b/src/ipa/libipa/meson.build
>> @@ -1,15 +1,14 @@
>> # SPDX-License-Identifier: CC0-1.0
>>
>> libipa_headers = files([
>> - 'algorithm.h',
>> 'camera_sensor_helper.h',
>> 'histogram.h'
>> ])
>>
>> libipa_sources = files([
>> - 'algorithm.cpp',
>> 'camera_sensor_helper.cpp',
>> - 'histogram.cpp'
>> + 'histogram.cpp',
>> + 'libipa.cpp',
>> ])
>>
>> libipa_includes = include_directories('..')
>
More information about the libcamera-devel
mailing list