[libcamera-devel] [PATCH] ipa: rkisp1: ipa_context: Add camera data to IPAContext

[Laurent Pinchart]

Hi Kieran,

On Sat, Apr 29, 2023 at 10:42:20PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart via libcamera-devel (2023-04-26 02:10:37)
> > The IPARkISP1 class caches data that is immutable over the life time of
> > the camera in private class members at init() time. This covers the
> > current needs to the IPA module, but doesn't provide a mechanism to
> > access that data from IPA algorithms. This will be needed for instance
> > by the upcoming auto-focus algorithm.
> > 
> > To prepare for such use cases, create a new IPACameraData structure that
> > stores immutable camera data, and instantiate it in the IPAContext
> > structure. The IPACameraData structure is currently empty, new members
> > will be added as needed.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > 
> > Bikeshedding on the IPACameraData and IPAContext::data names is welcome
> > :-)
> 
> No specific worries about the name ... but given how this doesn't
> actually add the data, but just the structure and 'design' - should we
> add it to the IPU3 as well?

It's a good idea, yes.

> There aren't (currently) specific calls there for init() but perhaps
> that should be improved as well?
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > ---
> >  src/ipa/rkisp1/ipa_context.cpp | 13 +++++++++++++
> >  src/ipa/rkisp1/ipa_context.h   |  4 ++++
> >  src/ipa/rkisp1/rkisp1.cpp      |  2 +-
> >  3 files changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > index 9bbf368432fa..986b955ea29d 100644
> > --- a/src/ipa/rkisp1/ipa_context.cpp
> > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > @@ -14,6 +14,15 @@
> >  
> >  namespace libcamera::ipa::rkisp1 {
> >  
> > +/**
> > + * \struct IPACameraData
> > + * \brief Camera data for the IPA module
> > + *
> > + * The camera data contains all information about the camera that remains
> > + * constant for the lifetime of the IPA context. It is typically set during the
> > + * init() operation of the IPA module.
> > + */
> > +
> >  /**
> >   * \struct IPASessionConfiguration
> >   * \brief Session configuration for the IPA module
> > @@ -337,6 +346,10 @@ namespace libcamera::ipa::rkisp1 {
> >   * \struct IPAContext
> >   * \brief Global IPA context data shared between all algorithms
> >   *
> > + * \var IPAContext::data
> > + * \brief The IPA camera data, initialized at init time and immutable for the
> > + * lifetime of the context
> > + *
> >   * \var IPAContext::configuration
> >   * \brief The IPA session configuration, immutable during the session
> >   *
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index b9b2065328d6..53b79110c932 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -20,6 +20,9 @@ namespace libcamera {
> >  
> >  namespace ipa::rkisp1 {
> >  
> > +struct IPACameraData {
> > +};
> > +
> >  struct IPASessionConfiguration {
> >         struct {
> >                 struct rkisp1_cif_isp_window measureWindow;
> > @@ -143,6 +146,7 @@ struct IPAFrameContext : public FrameContext {
> >  };
> >  
> >  struct IPAContext {
> > +       IPACameraData data;
> >         IPASessionConfiguration configuration;
> >         IPAActiveState activeState;
> >  
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index 6544c925ba25..2fe66d34b6d4 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -111,7 +111,7 @@ const ControlInfoMap::Map rkisp1Controls{
> >  } /* namespace */
> >  
> >  IPARkISP1::IPARkISP1()
> > -       : context_({ {}, {}, { kMaxFrameContexts } })
> > +       : context_({ {}, {}, {}, { kMaxFrameContexts } })
> >  {
> >  }
> >  
> > 
> > base-commit: 683c6da83f078d09fc020d2b48a4abe471853b2b

-- 
Regards,

Laurent Pinchart