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

[Jacopo Mondi]

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'

> +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

>  }
>
>  /* 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 ?

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