[PATCH 1/4] libcamera: rkisp1: Drop base IPA headers inclusion
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jul 1 15:38:32 CEST 2024
On Mon, Jul 01, 2024 at 10:46:45AM +0100, Kieran Bingham wrote:
> Quoting Umang Jain (2024-07-01 08:57:17)
> > The IPA headers ipa_interface.h and core_ipa_interface.h are
> > included as part of the rkisp1_ipa_interface.h generated from
> > module_ipa_interface.h.tmpl. Drop them as deemed redundant.
>
> I wonder why these were put in in the first place. Are there any
> artificats that would mean we should include them becuase of 'include
> what you use' ? But that said - I really can't see a reason to inlcude
> the base interface when we include a targetted/specific interface..
libcamera/ipa/ipa_interface.h defines the interface exposed by the IPA
module binary. That's the top-level ipaCreate() function, and the base
IPAInterface class.
The file is included in rkisp1_ipa_interface.h for the definition of the
IPAInterface class, as IPARkISP1Interface derives from it. rkisp1.cpp
doesn't make use of the base IPAInterface class, so there's no need to
include ipa_interface.h for that.
The ipaCreate() function declaration, however, is needed by rkisp1.cpp.
As it's declared by ipa_interface.h, we include the header in the
top-level .cpp file of each IPA module, and I think it should stay
there, following the "include what you use" principle, to avoid
depending on indirect includes.
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> > ---
> > src/ipa/rkisp1/rkisp1.cpp | 1 -
> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 1 -
> > 2 files changed, 2 deletions(-)
> >
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index d31cdbab..a43116d7 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -19,7 +19,6 @@
> >
> > #include <libcamera/control_ids.h>
> > #include <libcamera/framebuffer.h>
> > -#include <libcamera/ipa/ipa_interface.h>
> > #include <libcamera/ipa/ipa_module_info.h>
> > #include <libcamera/ipa/rkisp1_ipa_interface.h>
> > #include <libcamera/request.h>
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 4cbf105d..97cd78a7 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -27,7 +27,6 @@
> > #include <libcamera/stream.h>
> > #include <libcamera/transform.h>
> >
> > -#include <libcamera/ipa/core_ipa_interface.h>
> > #include <libcamera/ipa/rkisp1_ipa_interface.h>
> > #include <libcamera/ipa/rkisp1_ipa_proxy.h>
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list