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

[Jacopo Mondi]

Hi Laurent,

On Mon, Nov 18, 2019 at 03:09:19AM +0200, Laurent Pinchart wrote:
> 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

No no, more documentation it's fine, please take my tag in.
>
>  * 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191119/c052ba78/attachment.sig>