| Message ID | 20240717085444.289997-7-mzamazal@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Milan, Thank you for the patch. On Wed, Jul 17, 2024 at 10:54:27AM +0200, Milan Zamazal wrote: > To be in the same namespace as the other software ISP IPA stuff. The commit message body should be readable by itself, not as a continuation of the subject line. You could write IPA modules use custom namespaces for all their internal components to avoid namespace clashes. The simple IPA module for the software ISP uses libcamera::ipa::soft for this purpose. It however defines an internal class named BlackLevel in the root of the libcamera namespace, making it prone to clashes. Move it to the ipa::soft namespace along with the rest of the code. There's no need to resubmit just for this, and I would be fine merging the patch even without the updated commit message, but let's keep this in mind for future patches. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/ipa/simple/black_level.cpp | 5 +++++ > src/ipa/simple/black_level.h | 4 ++++ > 2 files changed, 9 insertions(+) > > diff --git a/src/ipa/simple/black_level.cpp b/src/ipa/simple/black_level.cpp > index cc490eb5..37e0109c 100644 > --- a/src/ipa/simple/black_level.cpp > +++ b/src/ipa/simple/black_level.cpp > @@ -15,6 +15,8 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(IPASoftBL) > > +namespace ipa::soft { > + > /** > * \class BlackLevel > * \brief Object providing black point level for software ISP > @@ -85,4 +87,7 @@ void BlackLevel::update(SwIspStats::Histogram &yHistogram) > } > }; > } > + > +} /* namespace ipa::soft */ > + > } /* namespace libcamera */ > diff --git a/src/ipa/simple/black_level.h b/src/ipa/simple/black_level.h > index 5e032f9f..a04230c9 100644 > --- a/src/ipa/simple/black_level.h > +++ b/src/ipa/simple/black_level.h > @@ -14,6 +14,8 @@ > > namespace libcamera { > > +namespace ipa::soft { > + > class BlackLevel > { > public: > @@ -26,4 +28,6 @@ private: > bool blackLevelSet_; > }; > > +} /* namespace ipa::soft */ > + > } /* namespace libcamera */
On 17/07/2024 09:54, Milan Zamazal wrote: > To be in the same namespace as the other software ISP IPA stuff. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > src/ipa/simple/black_level.cpp | 5 +++++ > src/ipa/simple/black_level.h | 4 ++++ > 2 files changed, 9 insertions(+) > > diff --git a/src/ipa/simple/black_level.cpp b/src/ipa/simple/black_level.cpp > index cc490eb5..37e0109c 100644 > --- a/src/ipa/simple/black_level.cpp > +++ b/src/ipa/simple/black_level.cpp > @@ -15,6 +15,8 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(IPASoftBL) > > +namespace ipa::soft { > + > /** > * \class BlackLevel > * \brief Object providing black point level for software ISP > @@ -85,4 +87,7 @@ void BlackLevel::update(SwIspStats::Histogram &yHistogram) > } > }; > } > + > +} /* namespace ipa::soft */ > + > } /* namespace libcamera */ > diff --git a/src/ipa/simple/black_level.h b/src/ipa/simple/black_level.h > index 5e032f9f..a04230c9 100644 > --- a/src/ipa/simple/black_level.h > +++ b/src/ipa/simple/black_level.h > @@ -14,6 +14,8 @@ > > namespace libcamera { > > +namespace ipa::soft { > + > class BlackLevel > { > public: > @@ -26,4 +28,6 @@ private: > bool blackLevelSet_; > }; > > +} /* namespace ipa::soft */ > + > } /* namespace libcamera */
diff --git a/src/ipa/simple/black_level.cpp b/src/ipa/simple/black_level.cpp index cc490eb5..37e0109c 100644 --- a/src/ipa/simple/black_level.cpp +++ b/src/ipa/simple/black_level.cpp @@ -15,6 +15,8 @@ namespace libcamera { LOG_DEFINE_CATEGORY(IPASoftBL) +namespace ipa::soft { + /** * \class BlackLevel * \brief Object providing black point level for software ISP @@ -85,4 +87,7 @@ void BlackLevel::update(SwIspStats::Histogram &yHistogram) } }; } + +} /* namespace ipa::soft */ + } /* namespace libcamera */ diff --git a/src/ipa/simple/black_level.h b/src/ipa/simple/black_level.h index 5e032f9f..a04230c9 100644 --- a/src/ipa/simple/black_level.h +++ b/src/ipa/simple/black_level.h @@ -14,6 +14,8 @@ namespace libcamera { +namespace ipa::soft { + class BlackLevel { public: @@ -26,4 +28,6 @@ private: bool blackLevelSet_; }; +} /* namespace ipa::soft */ + } /* namespace libcamera */