[libcamera-devel] [PATCH v2] libcamera: ipa: raspberrypi: Enable focus measure without recompile

[David Plowman]

Hi Jacopo

Thanks for the review!

On Wed, 1 Jul 2020 at 17:09, Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hi David,
>
> On Wed, Jul 01, 2020 at 01:50:21PM +0100, David Plowman wrote:
> > Previously, output of the focus measure could not be enabled without
> > recompiling (because of the RPI_LOGGING_ENABLE macro). This uses the
> > libcamera logging mechanism instead, so can be enabled/disabled at
> > runtime.
> >
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  src/ipa/raspberrypi/controller/rpi/focus.cpp | 16 ++++++----------
> >  src/ipa/raspberrypi/controller/rpi/focus.hpp |  3 ---
> >  src/ipa/raspberrypi/data/imx477.json         |  2 +-
> >  3 files changed, 7 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp
> > index 1e2b649..573831b 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp
> > @@ -6,11 +6,15 @@
> >   */
> >  #include <stdint.h>
> >
> > +#include "libcamera/internal/log.h"
> > +
> >  #include "../focus_status.h"
> > -#include "../logging.hpp"
> >  #include "focus.hpp"
> >
> >  using namespace RPi;
> > +using namespace libcamera;
> > +
>
> OCD says 'l' comes before 'R'

No problem with that!

>
> > +LOG_DEFINE_CATEGORY(RPI_FOCUS)
> >
> >  #define NAME "rpi.focus"
> >
> > @@ -24,11 +28,6 @@ char const *Focus::Name() const
> >       return NAME;
> >  }
> >
> > -void Focus::Read(boost::property_tree::ptree const &params)
> > -{
> > -     print_ = params.get<int>("print", 0);
> > -}
> > -
> >  void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)
> >  {
> >       FocusStatus status;
> > @@ -37,10 +36,7 @@ void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)
> >               status.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1] / 1000;
> >       status.num = i;
> >       image_metadata->Set("focus.status", status);
> > -     if (print_) {
> > -             uint32_t value = (status.focus_measures[5] + status.focus_measures[6]) / 10;
> > -             RPI_LOG("Focus contrast measure: " << value);
> > -     }
> > +     LOG(RPI_FOCUS, Debug) << "Focus contrast measure: " << (status.focus_measures[5] + status.focus_measures[6]) / 10;
>
> maybe break this line

Pretty sure I had it like that first but checkstyle complained, and
then it was OK with this. But happy if we prefer to change it...

>
> >  }
> >
> >  /* Register algorithm with the system. */
> > diff --git a/src/ipa/raspberrypi/controller/rpi/focus.hpp b/src/ipa/raspberrypi/controller/rpi/focus.hpp
> > index d53401f..a9756ea 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/focus.hpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/focus.hpp
> > @@ -22,10 +22,7 @@ class Focus : public Algorithm
> >  public:
> >       Focus(Controller *controller);
> >       char const *Name() const override;
> > -     void Read(boost::property_tree::ptree const &params) override;
> >       void Process(StatisticsPtr &stats, Metadata *image_metadata) override;
> > -private:
> > -     bool print_;
> >  };
> >
> >  } /* namespace RPi */
> > diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json
> > index 389e8ce..747cd8d 100644
> > --- a/src/ipa/raspberrypi/data/imx477.json
> > +++ b/src/ipa/raspberrypi/data/imx477.json
> > @@ -415,6 +415,6 @@
> >      },
> >      "rpi.focus":
> >      {
> > -     "print": 1
> > +
>
> Is the empty line intentional ?

Well, for reasons I'm not totally clear about, our Python tuning tool
tends to produce a blank line when there's nothing between the curlies
- so I just tend to copy it like that. Mostly. But not always. So I'm
fine to delete it too...

Best regards
David

>
> Minors apart, which can be fixed while applying:
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> Thanks
>    j
>
> >      }
> >  }
> > --
> > 2.20.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel