Message ID | 20240717085444.289997-9-mzamazal@redhat.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Milan, what a cool set - thanks - I think it's great to move the Software ISP to the same model as the other IPAs. On 17/07/2024 09:54, 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@redhat.com> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/ipa/simple/algorithms/algorithm.h | 22 +++++++++++ > src/ipa/simple/ipa_context.cpp | 53 +++++++++++++++++++++++++++ > src/ipa/simple/ipa_context.h | 34 +++++++++++++++++ > src/ipa/simple/meson.build | 1 + > src/ipa/simple/module.h | 28 ++++++++++++++ > 5 files changed, 138 insertions(+) > create mode 100644 src/ipa/simple/algorithms/algorithm.h > create mode 100644 src/ipa/simple/ipa_context.cpp > 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.cpp b/src/ipa/simple/ipa_context.cpp > new file mode 100644 > index 00000000..3c1c7262 > --- /dev/null > +++ b/src/ipa/simple/ipa_context.cpp > @@ -0,0 +1,53 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2021, Google Inc. > + * Copyright (C) 2024 Red Hat Inc. > + * > + * Software ISP IPA Context > + */ > + > +#include "ipa_context.h" > + > +/** > + * \file ipa_context.h > + * \brief Context and state information shared between the algorithms > + */ > + > +namespace libcamera::ipa::soft { > + > +/** > + * \struct IPASessionConfiguration > + * \brief Session configuration for the IPA module > + * > + * The session configuration contains all IPA configuration parameters that > + * remain constant during the capture session, from IPA module start to stop. > + * It is typically set during the configure() operation of the IPA module, but > + * may also be updated in the start() operation. > + */ > + > +/** > + * \struct IPAActiveState > + * \brief The active state of the IPA algorithms > + * > + * The IPA is fed with the statistics generated from the latest frame processed. > + * The statistics are then processed by the IPA algorithms to compute parameters > + * required for the next frame capture and processing. The current state of the > + * algorithms is reflected through the IPAActiveState to store the values most > + * recently computed by the IPA algorithms. > + */ > + > +/** > + * \struct IPAContext > + * \brief Global IPA context data shared between all algorithms > + * > + * \var IPAContext::configuration > + * \brief The IPA session configuration, immutable during the session > + * > + * \var IPAContext::frameContexts > + * \brief Ring buffer of the IPAFrameContext(s) > + * > + * \var IPAContext::activeState > + * \brief The current state of IPA algorithms > + */ > + > +} /* namespace libcamera::ipa::soft */ > 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/meson.build b/src/ipa/simple/meson.build > index 33d1c96a..363251fb 100644 > --- a/src/ipa/simple/meson.build > +++ b/src/ipa/simple/meson.build > @@ -3,6 +3,7 @@ > ipa_name = 'ipa_soft_simple' > > soft_simple_sources = files([ > + 'ipa_context.cpp', > 'soft_simple.cpp', > 'black_level.cpp', > ]) > diff --git a/src/ipa/simple/module.h b/src/ipa/simple/module.h > new file mode 100644 > index 00000000..33a7d1db > --- /dev/null > +++ b/src/ipa/simple/module.h > @@ -0,0 +1,28 @@ > +/* 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/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, The ControlInfoMap here rather than some mojom generated struct surprises me a little, but the rest is fine and I assume that will become clear later: Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > + DebayerParams, SwIspStats>; > + > +} /* namespace ipa::soft */ > + > +} /* namespace libcamera */
On Mon, Aug 12, 2024 at 02:15:29PM +0100, Daniel Scally wrote: > Hi Milan, what a cool set - thanks - I think it's great to move the > Software ISP to the same model as the other IPAs. > > On 17/07/2024 09:54, 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 s/don't/doesn't/ > > 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@redhat.com> > > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > > --- > > src/ipa/simple/algorithms/algorithm.h | 22 +++++++++++ > > src/ipa/simple/ipa_context.cpp | 53 +++++++++++++++++++++++++++ > > src/ipa/simple/ipa_context.h | 34 +++++++++++++++++ > > src/ipa/simple/meson.build | 1 + > > src/ipa/simple/module.h | 28 ++++++++++++++ > > 5 files changed, 138 insertions(+) > > create mode 100644 src/ipa/simple/algorithms/algorithm.h > > create mode 100644 src/ipa/simple/ipa_context.cpp > > 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.cpp b/src/ipa/simple/ipa_context.cpp > > new file mode 100644 > > index 00000000..3c1c7262 > > --- /dev/null > > +++ b/src/ipa/simple/ipa_context.cpp > > @@ -0,0 +1,53 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2021, Google Inc. > > + * Copyright (C) 2024 Red Hat Inc. > > + * > > + * Software ISP IPA Context > > + */ > > + > > +#include "ipa_context.h" > > + > > +/** > > + * \file ipa_context.h > > + * \brief Context and state information shared between the algorithms > > + */ > > + > > +namespace libcamera::ipa::soft { > > + > > +/** > > + * \struct IPASessionConfiguration > > + * \brief Session configuration for the IPA module > > + * > > + * The session configuration contains all IPA configuration parameters that > > + * remain constant during the capture session, from IPA module start to stop. > > + * It is typically set during the configure() operation of the IPA module, but > > + * may also be updated in the start() operation. > > + */ > > + > > +/** > > + * \struct IPAActiveState > > + * \brief The active state of the IPA algorithms > > + * > > + * The IPA is fed with the statistics generated from the latest frame processed. > > + * The statistics are then processed by the IPA algorithms to compute parameters > > + * required for the next frame capture and processing. The current state of the > > + * algorithms is reflected through the IPAActiveState to store the values most > > + * recently computed by the IPA algorithms. > > + */ > > + > > +/** > > + * \struct IPAContext > > + * \brief Global IPA context data shared between all algorithms > > + * > > + * \var IPAContext::configuration > > + * \brief The IPA session configuration, immutable during the session > > + * > > + * \var IPAContext::frameContexts > > + * \brief Ring buffer of the IPAFrameContext(s) > > + * > > + * \var IPAContext::activeState > > + * \brief The current state of IPA algorithms > > + */ > > + > > +} /* namespace libcamera::ipa::soft */ > > 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 > > + * Extra blank line. > > + */ > > + > > +#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/meson.build b/src/ipa/simple/meson.build > > index 33d1c96a..363251fb 100644 > > --- a/src/ipa/simple/meson.build > > +++ b/src/ipa/simple/meson.build > > @@ -3,6 +3,7 @@ > > ipa_name = 'ipa_soft_simple' > > > > soft_simple_sources = files([ > > + 'ipa_context.cpp', > > 'soft_simple.cpp', > > 'black_level.cpp', > > ]) > > diff --git a/src/ipa/simple/module.h b/src/ipa/simple/module.h > > new file mode 100644 > > index 00000000..33a7d1db > > --- /dev/null > > +++ b/src/ipa/simple/module.h > > @@ -0,0 +1,28 @@ > > +/* 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/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, > > The ControlInfoMap here rather than some mojom generated struct surprises me a little, but the rest > is fine and I assume that will become clear later: > > > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > + DebayerParams, SwIspStats>; > > + > > +} /* namespace ipa::soft */ > > + > > +} /* namespace libcamera */
Hi Laurent, thank you for review. Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > On Mon, Aug 12, 2024 at 02:15:29PM +0100, Daniel Scally wrote: >> Hi Milan, what a cool set - thanks - I think it's great to move the >> Software ISP to the same model as the other IPAs. > >> >> On 17/07/2024 09:54, 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 > > s/don't/doesn't/ ? "structures" is plural. >> > 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@redhat.com> >> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> >> > --- >> > src/ipa/simple/algorithms/algorithm.h | 22 +++++++++++ >> > src/ipa/simple/ipa_context.cpp | 53 +++++++++++++++++++++++++++ >> > src/ipa/simple/ipa_context.h | 34 +++++++++++++++++ >> > src/ipa/simple/meson.build | 1 + >> > src/ipa/simple/module.h | 28 ++++++++++++++ >> > 5 files changed, 138 insertions(+) >> > create mode 100644 src/ipa/simple/algorithms/algorithm.h >> > create mode 100644 src/ipa/simple/ipa_context.cpp >> > 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.cpp b/src/ipa/simple/ipa_context.cpp >> > new file mode 100644 >> > index 00000000..3c1c7262 >> > --- /dev/null >> > +++ b/src/ipa/simple/ipa_context.cpp >> > @@ -0,0 +1,53 @@ >> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >> > +/* >> > + * Copyright (C) 2021, Google Inc. >> > + * Copyright (C) 2024 Red Hat Inc. >> > + * >> > + * Software ISP IPA Context >> > + */ >> > + >> > +#include "ipa_context.h" >> > + >> > +/** >> > + * \file ipa_context.h >> > + * \brief Context and state information shared between the algorithms >> > + */ >> > + >> > +namespace libcamera::ipa::soft { >> > + >> > +/** >> > + * \struct IPASessionConfiguration >> > + * \brief Session configuration for the IPA module >> > + * >> > + * The session configuration contains all IPA configuration parameters that >> > + * remain constant during the capture session, from IPA module start to stop. >> > + * It is typically set during the configure() operation of the IPA module, but >> > + * may also be updated in the start() operation. >> > + */ >> > + >> > +/** >> > + * \struct IPAActiveState >> > + * \brief The active state of the IPA algorithms >> > + * >> > + * The IPA is fed with the statistics generated from the latest frame processed. >> > + * The statistics are then processed by the IPA algorithms to compute parameters >> > + * required for the next frame capture and processing. The current state of the >> > + * algorithms is reflected through the IPAActiveState to store the values most >> > + * recently computed by the IPA algorithms. >> > + */ >> > + >> > +/** >> > + * \struct IPAContext >> > + * \brief Global IPA context data shared between all algorithms >> > + * >> > + * \var IPAContext::configuration >> > + * \brief The IPA session configuration, immutable during the session >> > + * >> > + * \var IPAContext::frameContexts >> > + * \brief Ring buffer of the IPAFrameContext(s) >> > + * >> > + * \var IPAContext::activeState >> > + * \brief The current state of IPA algorithms >> > + */ >> > + >> > +} /* namespace libcamera::ipa::soft */ >> > 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 >> > + * > > Extra blank line. OK, I'll remove it. >> > + */ >> > + >> > +#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/meson.build b/src/ipa/simple/meson.build >> > index 33d1c96a..363251fb 100644 >> > --- a/src/ipa/simple/meson.build >> > +++ b/src/ipa/simple/meson.build >> > @@ -3,6 +3,7 @@ >> > ipa_name = 'ipa_soft_simple' >> > >> > soft_simple_sources = files([ >> > + 'ipa_context.cpp', >> > 'soft_simple.cpp', >> > 'black_level.cpp', >> > ]) >> > diff --git a/src/ipa/simple/module.h b/src/ipa/simple/module.h >> > new file mode 100644 >> > index 00000000..33a7d1db >> > --- /dev/null >> > +++ b/src/ipa/simple/module.h >> > @@ -0,0 +1,28 @@ >> > +/* 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/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, >> >> The ControlInfoMap here rather than some mojom generated struct surprises me a little, but the rest >> is fine and I assume that will become clear later: >> >> >> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> > + DebayerParams, SwIspStats>; >> > + >> > +} /* namespace ipa::soft */ >> > + >> > +} /* namespace libcamera */
On Tue, Aug 13, 2024 at 10:26:41AM +0200, Milan Zamazal wrote: > Hi Laurent, > > thank you for review. > > Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > > > On Mon, Aug 12, 2024 at 02:15:29PM +0100, Daniel Scally wrote: > >> Hi Milan, what a cool set - thanks - I think it's great to move the > >> Software ISP to the same model as the other IPAs. > > > >> > >> On 17/07/2024 09:54, 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 > > > > s/don't/doesn't/ > > ? "structures" is plural. I'm not sure what I misread. Please ignore my comment. > >> > 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@redhat.com> > >> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > >> > --- > >> > src/ipa/simple/algorithms/algorithm.h | 22 +++++++++++ > >> > src/ipa/simple/ipa_context.cpp | 53 +++++++++++++++++++++++++++ > >> > src/ipa/simple/ipa_context.h | 34 +++++++++++++++++ > >> > src/ipa/simple/meson.build | 1 + > >> > src/ipa/simple/module.h | 28 ++++++++++++++ > >> > 5 files changed, 138 insertions(+) > >> > create mode 100644 src/ipa/simple/algorithms/algorithm.h > >> > create mode 100644 src/ipa/simple/ipa_context.cpp > >> > 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.cpp b/src/ipa/simple/ipa_context.cpp > >> > new file mode 100644 > >> > index 00000000..3c1c7262 > >> > --- /dev/null > >> > +++ b/src/ipa/simple/ipa_context.cpp > >> > @@ -0,0 +1,53 @@ > >> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > >> > +/* > >> > + * Copyright (C) 2021, Google Inc. > >> > + * Copyright (C) 2024 Red Hat Inc. > >> > + * > >> > + * Software ISP IPA Context > >> > + */ > >> > + > >> > +#include "ipa_context.h" > >> > + > >> > +/** > >> > + * \file ipa_context.h > >> > + * \brief Context and state information shared between the algorithms > >> > + */ > >> > + > >> > +namespace libcamera::ipa::soft { > >> > + > >> > +/** > >> > + * \struct IPASessionConfiguration > >> > + * \brief Session configuration for the IPA module > >> > + * > >> > + * The session configuration contains all IPA configuration parameters that > >> > + * remain constant during the capture session, from IPA module start to stop. > >> > + * It is typically set during the configure() operation of the IPA module, but > >> > + * may also be updated in the start() operation. > >> > + */ > >> > + > >> > +/** > >> > + * \struct IPAActiveState > >> > + * \brief The active state of the IPA algorithms > >> > + * > >> > + * The IPA is fed with the statistics generated from the latest frame processed. > >> > + * The statistics are then processed by the IPA algorithms to compute parameters > >> > + * required for the next frame capture and processing. The current state of the > >> > + * algorithms is reflected through the IPAActiveState to store the values most > >> > + * recently computed by the IPA algorithms. > >> > + */ > >> > + > >> > +/** > >> > + * \struct IPAContext > >> > + * \brief Global IPA context data shared between all algorithms > >> > + * > >> > + * \var IPAContext::configuration > >> > + * \brief The IPA session configuration, immutable during the session > >> > + * > >> > + * \var IPAContext::frameContexts > >> > + * \brief Ring buffer of the IPAFrameContext(s) > >> > + * > >> > + * \var IPAContext::activeState > >> > + * \brief The current state of IPA algorithms > >> > + */ > >> > + > >> > +} /* namespace libcamera::ipa::soft */ > >> > 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 > >> > + * > > > > Extra blank line. > > OK, I'll remove it. > > >> > + */ > >> > + > >> > +#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/meson.build b/src/ipa/simple/meson.build > >> > index 33d1c96a..363251fb 100644 > >> > --- a/src/ipa/simple/meson.build > >> > +++ b/src/ipa/simple/meson.build > >> > @@ -3,6 +3,7 @@ > >> > ipa_name = 'ipa_soft_simple' > >> > > >> > soft_simple_sources = files([ > >> > + 'ipa_context.cpp', > >> > 'soft_simple.cpp', > >> > 'black_level.cpp', > >> > ]) > >> > diff --git a/src/ipa/simple/module.h b/src/ipa/simple/module.h > >> > new file mode 100644 > >> > index 00000000..33a7d1db > >> > --- /dev/null > >> > +++ b/src/ipa/simple/module.h > >> > @@ -0,0 +1,28 @@ > >> > +/* 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/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, > >> > >> The ControlInfoMap here rather than some mojom generated struct surprises me a little, but the rest > >> is fine and I assume that will become clear later: > >> > >> > >> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > >> > + DebayerParams, SwIspStats>; > >> > + > >> > +} /* namespace ipa::soft */ > >> > + > >> > +} /* namespace libcamera */ >
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.cpp b/src/ipa/simple/ipa_context.cpp new file mode 100644 index 00000000..3c1c7262 --- /dev/null +++ b/src/ipa/simple/ipa_context.cpp @@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021, Google Inc. + * Copyright (C) 2024 Red Hat Inc. + * + * Software ISP IPA Context + */ + +#include "ipa_context.h" + +/** + * \file ipa_context.h + * \brief Context and state information shared between the algorithms + */ + +namespace libcamera::ipa::soft { + +/** + * \struct IPASessionConfiguration + * \brief Session configuration for the IPA module + * + * The session configuration contains all IPA configuration parameters that + * remain constant during the capture session, from IPA module start to stop. + * It is typically set during the configure() operation of the IPA module, but + * may also be updated in the start() operation. + */ + +/** + * \struct IPAActiveState + * \brief The active state of the IPA algorithms + * + * The IPA is fed with the statistics generated from the latest frame processed. + * The statistics are then processed by the IPA algorithms to compute parameters + * required for the next frame capture and processing. The current state of the + * algorithms is reflected through the IPAActiveState to store the values most + * recently computed by the IPA algorithms. + */ + +/** + * \struct IPAContext + * \brief Global IPA context data shared between all algorithms + * + * \var IPAContext::configuration + * \brief The IPA session configuration, immutable during the session + * + * \var IPAContext::frameContexts + * \brief Ring buffer of the IPAFrameContext(s) + * + * \var IPAContext::activeState + * \brief The current state of IPA algorithms + */ + +} /* namespace libcamera::ipa::soft */ 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/meson.build b/src/ipa/simple/meson.build index 33d1c96a..363251fb 100644 --- a/src/ipa/simple/meson.build +++ b/src/ipa/simple/meson.build @@ -3,6 +3,7 @@ ipa_name = 'ipa_soft_simple' soft_simple_sources = files([ + 'ipa_context.cpp', 'soft_simple.cpp', 'black_level.cpp', ]) diff --git a/src/ipa/simple/module.h b/src/ipa/simple/module.h new file mode 100644 index 00000000..33a7d1db --- /dev/null +++ b/src/ipa/simple/module.h @@ -0,0 +1,28 @@ +/* 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/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 */