[PATCH 04/19] libcamera: software_isp: Define skeletons for IPA refactoring

[Umang Jain]

Hi Milan,

Thank you for the patch.

On 26/06/24 12:50 pm, Milan Zamazal wrote:
> Software ISP image processing algorithms are currently defined in a
> simplified way, different from other libcamera pipelines.  This is not
> good for several reasons:
>
> - It makes the software ISP code harder to understand due to its
>    different structuring.
> - Adding more algorithms may make the code harder to understand
>    generally.
> - Mass libcamera code changes may not be easily applicable to software
>    ISP.
> - Algorithm sharing with other pipelines is not easily possible.
>
> This patch introduces basic software ISP IPA skeletons structured
> similarly to the other pipelines.  The newly added files are currently
> not used or compiled and the general skeleton structures don't contain
> anything particular.  It is just a preparation step for a larger
> refactoring and the code will be actually used and extended as needed in
> followup patches.
>
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>

I think you also need a ipa_context.cpp here, to document(doxygen 
generate) the structures in ipa_context.h, like done in other IPAs.
(and obviously it should be compiled).

> ---
>   src/ipa/simple/algorithms/algorithm.h | 22 +++++++++++++++++
>   src/ipa/simple/ipa_context.h          | 34 +++++++++++++++++++++++++++
>   src/ipa/simple/module.h               | 30 +++++++++++++++++++++++
>   3 files changed, 86 insertions(+)
>   create mode 100644 src/ipa/simple/algorithms/algorithm.h
>   create mode 100644 src/ipa/simple/ipa_context.h
>   create mode 100644 src/ipa/simple/module.h
>
> diff --git a/src/ipa/simple/algorithms/algorithm.h b/src/ipa/simple/algorithms/algorithm.h
> new file mode 100644
> index 00000000..41f63170
> --- /dev/null
> +++ b/src/ipa/simple/algorithms/algorithm.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024 Red Hat, Inc.
> + *
> + * Software ISP control algorithm interface
> + */
> +
> +#pragma once
> +
> +#include <libipa/algorithm.h>
> +
> +#include "module.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::soft {
> +
> +using Algorithm = libcamera::ipa::Algorithm<Module>;
> +
> +} /* namespace ipa::soft */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
> new file mode 100644
> index 00000000..bc1235b6
> --- /dev/null
> +++ b/src/ipa/simple/ipa_context.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024 Red Hat, Inc.
> + *
> + * Simple pipeline IPA Context
> + *
> + */
> +
> +#pragma once
> +
> +#include <libipa/fc_queue.h>
> +
> +namespace libcamera {
> +
> +namespace ipa::soft {
> +
> +struct IPASessionConfiguration {
> +};
> +
> +struct IPAActiveState {
> +};
> +
> +struct IPAFrameContext : public FrameContext {
> +};
> +
> +struct IPAContext {
> +	IPASessionConfiguration configuration;
> +	IPAActiveState activeState;
> +	FCQueue<IPAFrameContext> frameContexts;
> +};
> +
> +} /* namespace ipa::soft */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/simple/module.h b/src/ipa/simple/module.h
> new file mode 100644
> index 00000000..21328ecd
> --- /dev/null
> +++ b/src/ipa/simple/module.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024 Red Hat, Inc.
> + *
> + * Software ISP IPA Module
> + */
> +
> +#pragma once
> +
> +#include <libcamera/controls.h>
> +
> +#include <libcamera/ipa/core_ipa_interface.h>
> +
> +#include "libcamera/internal/software_isp/debayer_params.h"
> +#include "libcamera/internal/software_isp/swisp_stats.h"
> +
> +#include <libipa/module.h>
> +
> +#include "ipa_context.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::soft {
> +
> +using Module = ipa::Module<IPAContext, IPAFrameContext, ControlInfoMap,
> +			   DebayerParams, SwIspStats>;
> +
> +} /* namespace ipa::soft */
> +
> +} /* namespace libcamera */