[libcamera-devel] [PATCH v2 21/24] ipa: Declare the ipaCreate() function prototype

[Laurent Pinchart]

Hi Jacopo,

On Fri, Nov 15, 2019 at 06:05:21PM +0100, Jacopo Mondi wrote:
> On Fri, Nov 08, 2019 at 10:54:06PM +0200, Laurent Pinchart wrote:
> > IPA modules have to implement a public ipaCreate() function, but its
> > prototype isn't declared in any header file. This allows for modules to
> > get the prototype wrong without being warned by the compiler. Fix it.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  include/ipa/ipa_interface.h     |  2 ++
> >  src/libcamera/ipa_interface.cpp | 10 ++++++++++
> >  2 files changed, 12 insertions(+)
> >
> > diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
> > index 3a423e37671f..92f1aac50b85 100644
> > --- a/include/ipa/ipa_interface.h
> > +++ b/include/ipa/ipa_interface.h
> > @@ -80,6 +80,8 @@ struct ipa_context_ops {
> >  			      const struct ipa_operation_data *data);
> >  };
> >
> > +struct ipa_context *ipaCreate();
> > +
> >  #ifdef __cplusplus
> >  }
> >
> > diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
> > index 89fce0e8347f..cb2767a04711 100644
> > --- a/src/libcamera/ipa_interface.cpp
> > +++ b/src/libcamera/ipa_interface.cpp
> > @@ -261,6 +261,16 @@
> >   * \sa libcamera::IPAInterface::processEvent()
> >   */
> >
> > +/**
> > + * \fn ipaCreate()
> > + * \brief Entry point to the IPA modules
> > + *
> > + * This function is the entry point to the IPA modules. It is implemented by
> > + * every IPA module, and called by libcamera to create a new IPA context.
> > + *
> > + * \return A newly created IPA context
> > + */
> > +
> 
> Yout might want to remove the prototype defined in the \file comment
> block in ipa_interface.cpp and let Doxygen link ipaCreate() mentions
> with this

I like having it there, as it gives a nice overview without a need to
jump to the ipaCreate() documentation, but if you think it should be
removed, I can replace the two paragraphs with

 * IPA modules shall expose a public function named ipaCreate(). The function
 * creates an instance of an IPA context, which models a context of execution
 * for the IPA. IPA modules shall support creating one context per camera, as
 * required by their associated pipeline handler.

Please let me know what you think is best.

> Apart from that
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> >  namespace libcamera {
> >
> >  /**

-- 
Regards,

Laurent Pinchart