[libcamera-devel,v3,10/10] ipa: raspberrypi: Generalise the focus reporting code
diff mbox series

Message ID 20230331072341.24851-11-naush@raspberrypi.com
State Accepted
Headers show
Series
  • Raspberry Pi: Generalised algorithms
Related show

Commit Message

Naushir Patuck March 31, 2023, 7:23 a.m. UTC 
Use the generalised focus statistics structure to compute the centre
window focus FoM value. This avoids needed to hard-code a specific
grid size.

Remove the focus reporting algorithm as the functionality is duplicated
by this bit of IPA code. Remove focus_status.h as it is no longer needed.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/ipa/raspberrypi/controller/focus_status.h | 20 --------
 src/ipa/raspberrypi/controller/rpi/af.h       |  1 -
 src/ipa/raspberrypi/controller/rpi/focus.cpp  | 49 -------------------
 src/ipa/raspberrypi/data/imx290.json          |  3 --
 src/ipa/raspberrypi/data/imx296.json          |  3 --
 src/ipa/raspberrypi/data/imx296_mono.json     |  3 --
 src/ipa/raspberrypi/data/imx477.json          |  3 --
 src/ipa/raspberrypi/data/imx477_noir.json     |  3 --
 .../raspberrypi/data/imx477_scientific.json   |  3 --
 src/ipa/raspberrypi/data/imx477_v1.json       |  3 --
 src/ipa/raspberrypi/data/imx708.json          |  3 --
 src/ipa/raspberrypi/data/imx708_noir.json     |  3 --
 src/ipa/raspberrypi/data/imx708_wide.json     |  3 --
 .../raspberrypi/data/imx708_wide_noir.json    |  3 --
 src/ipa/raspberrypi/meson.build               |  1 -
 src/ipa/raspberrypi/raspberrypi.cpp           | 32 +++++++++---
 16 files changed, 24 insertions(+), 112 deletions(-)
 delete mode 100644 src/ipa/raspberrypi/controller/focus_status.h
 delete mode 100644 src/ipa/raspberrypi/controller/rpi/focus.cpp

Comments

Jacopo Mondi March 31, 2023, 7:58 a.m. UTC | #1 
Hi Naush

On Fri, Mar 31, 2023 at 08:23:41AM +0100, Naushir Patuck via libcamera-devel wrote:
> Use the generalised focus statistics structure to compute the centre
> window focus FoM value. This avoids needed to hard-code a specific
> grid size.
>
> Remove the focus reporting algorithm as the functionality is duplicated
> by this bit of IPA code. Remove focus_status.h as it is no longer needed.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

Thanks for explaining why we can't populate focus.status in the AF
algorithm itself.

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

> ---
>  src/ipa/raspberrypi/controller/focus_status.h | 20 --------
>  src/ipa/raspberrypi/controller/rpi/af.h       |  1 -
>  src/ipa/raspberrypi/controller/rpi/focus.cpp  | 49 -------------------
>  src/ipa/raspberrypi/data/imx290.json          |  3 --
>  src/ipa/raspberrypi/data/imx296.json          |  3 --
>  src/ipa/raspberrypi/data/imx296_mono.json     |  3 --
>  src/ipa/raspberrypi/data/imx477.json          |  3 --
>  src/ipa/raspberrypi/data/imx477_noir.json     |  3 --
>  .../raspberrypi/data/imx477_scientific.json   |  3 --
>  src/ipa/raspberrypi/data/imx477_v1.json       |  3 --
>  src/ipa/raspberrypi/data/imx708.json          |  3 --
>  src/ipa/raspberrypi/data/imx708_noir.json     |  3 --
>  src/ipa/raspberrypi/data/imx708_wide.json     |  3 --
>  .../raspberrypi/data/imx708_wide_noir.json    |  3 --
>  src/ipa/raspberrypi/meson.build               |  1 -
>  src/ipa/raspberrypi/raspberrypi.cpp           | 32 +++++++++---
>  16 files changed, 24 insertions(+), 112 deletions(-)
>  delete mode 100644 src/ipa/raspberrypi/controller/focus_status.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/focus.cpp
>
> diff --git a/src/ipa/raspberrypi/controller/focus_status.h b/src/ipa/raspberrypi/controller/focus_status.h
> deleted file mode 100644
> index 8b74e59840c1..000000000000
> --- a/src/ipa/raspberrypi/controller/focus_status.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* SPDX-License-Identifier: BSD-2-Clause */
> -/*
> - * Copyright (C) 2020, Raspberry Pi Ltd
> - *
> - * focus_status.h - focus measurement status
> - */
> -#pragma once
> -
> -#include <linux/bcm2835-isp.h>
> -
> -/*
> - * The focus algorithm should post the following structure into the image's
> - * "focus.status" metadata. Recall that it's only reporting focus (contrast)
> - * measurements, it's not driving any kind of auto-focus algorithm!
> - */
> -
> -struct FocusStatus {
> -	unsigned int num;
> -	uint32_t focusMeasures[FOCUS_REGIONS];
> -};
> diff --git a/src/ipa/raspberrypi/controller/rpi/af.h b/src/ipa/raspberrypi/controller/rpi/af.h
> index b479feb88c39..6d2bae671a22 100644
> --- a/src/ipa/raspberrypi/controller/rpi/af.h
> +++ b/src/ipa/raspberrypi/controller/rpi/af.h
> @@ -28,7 +28,6 @@
>   * "nuisance" scans. During each interval where PDAF is not working, only
>   * ONE scan will be performed; CAF cannot track objects using CDAF alone.
>   *
> - * This algorithm is unrelated to "rpi.focus" which merely reports CDAF FoM.
>   */
>
>  namespace RPiController {
> diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp
> deleted file mode 100644
> index ea3cc00e42c3..000000000000
> --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -/* SPDX-License-Identifier: BSD-2-Clause */
> -/*
> - * Copyright (C) 2020, Raspberry Pi Ltd
> - *
> - * focus.cpp - focus algorithm
> - */
> -#include <stdint.h>
> -
> -#include <libcamera/base/log.h>
> -
> -#include "../focus_status.h"
> -#include "focus.h"
> -
> -using namespace RPiController;
> -using namespace libcamera;
> -
> -LOG_DEFINE_CATEGORY(RPiFocus)
> -
> -#define NAME "rpi.focus"
> -
> -Focus::Focus(Controller *controller)
> -	: Algorithm(controller)
> -{
> -}
> -
> -char const *Focus::name() const
> -{
> -	return NAME;
> -}
> -
> -void Focus::process(StatisticsPtr &stats, Metadata *imageMetadata)
> -{
> -	FocusStatus status;
> -	for (unsigned int i = 0; i < stats->focusRegions.numRegions(); i++)
> -		status.focusMeasures[i] = stats->focusRegions.get(i).val;
> -	status.num = stats->focusRegions.numRegions();
> -	imageMetadata->set("focus.status", status);
> -
> -	LOG(RPiFocus, Debug)
> -		<< "Focus contrast measure: "
> -		<< (status.focusMeasures[5] + status.focusMeasures[6]) / 10;
> -}
> -
> -/* Register algorithm with the system. */
> -static Algorithm *create(Controller *controller)
> -{
> -	return new Focus(controller);
> -}
> -static RegisterAlgorithm reg(NAME, &create);
> diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json
> index bfb9c6093d26..ace68d0ebf1c 100644
> --- a/src/ipa/raspberrypi/data/imx290.json
> +++ b/src/ipa/raspberrypi/data/imx290.json
> @@ -195,9 +195,6 @@
>                      }
>                  ]
>              }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> \ No newline at end of file
> diff --git a/src/ipa/raspberrypi/data/imx296.json b/src/ipa/raspberrypi/data/imx296.json
> index 346f5b658957..ae8722c46a85 100644
> --- a/src/ipa/raspberrypi/data/imx296.json
> +++ b/src/ipa/raspberrypi/data/imx296.json
> @@ -532,9 +532,6 @@
>                  "strength": 1.0,
>                  "limit": 0.18
>              }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> diff --git a/src/ipa/raspberrypi/data/imx296_mono.json b/src/ipa/raspberrypi/data/imx296_mono.json
> index e9fa30c6a08d..30965b4b8d61 100644
> --- a/src/ipa/raspberrypi/data/imx296_mono.json
> +++ b/src/ipa/raspberrypi/data/imx296_mono.json
> @@ -228,9 +228,6 @@
>                  "strength": 1.0,
>                  "limit": 0.18
>              }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json
> index bfc0774ffdfc..daffc268ff43 100644
> --- a/src/ipa/raspberrypi/data/imx477.json
> +++ b/src/ipa/raspberrypi/data/imx477.json
> @@ -513,9 +513,6 @@
>          },
>          {
>              "rpi.sharpen": { }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> \ No newline at end of file
> diff --git a/src/ipa/raspberrypi/data/imx477_noir.json b/src/ipa/raspberrypi/data/imx477_noir.json
> index dadd97bc3d78..52d7f072d0ce 100644
> --- a/src/ipa/raspberrypi/data/imx477_noir.json
> +++ b/src/ipa/raspberrypi/data/imx477_noir.json
> @@ -424,9 +424,6 @@
>          },
>          {
>              "rpi.sharpen": { }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> \ No newline at end of file
> diff --git a/src/ipa/raspberrypi/data/imx477_scientific.json b/src/ipa/raspberrypi/data/imx477_scientific.json
> index 17c4ed0a5e74..26c692fdbab9 100644
> --- a/src/ipa/raspberrypi/data/imx477_scientific.json
> +++ b/src/ipa/raspberrypi/data/imx477_scientific.json
> @@ -474,9 +474,6 @@
>          },
>          {
>              "rpi.sharpen": { }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> \ No newline at end of file
> diff --git a/src/ipa/raspberrypi/data/imx477_v1.json b/src/ipa/raspberrypi/data/imx477_v1.json
> index 5bcaac67069b..d64020091efa 100644
> --- a/src/ipa/raspberrypi/data/imx477_v1.json
> +++ b/src/ipa/raspberrypi/data/imx477_v1.json
> @@ -511,9 +511,6 @@
>          },
>          {
>              "rpi.sharpen": { }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> \ No newline at end of file
> diff --git a/src/ipa/raspberrypi/data/imx708.json b/src/ipa/raspberrypi/data/imx708.json
> index c38b2d4cf256..b9830a3bf692 100644
> --- a/src/ipa/raspberrypi/data/imx708.json
> +++ b/src/ipa/raspberrypi/data/imx708.json
> @@ -512,9 +512,6 @@
>          {
>              "rpi.sharpen": { }
>          },
> -        {
> -            "rpi.focus": { }
> -        },
>          {
>              "rpi.af":
>              {
> diff --git a/src/ipa/raspberrypi/data/imx708_noir.json b/src/ipa/raspberrypi/data/imx708_noir.json
> index 082274e34c53..075f70355cec 100644
> --- a/src/ipa/raspberrypi/data/imx708_noir.json
> +++ b/src/ipa/raspberrypi/data/imx708_noir.json
> @@ -512,9 +512,6 @@
>          {
>              "rpi.sharpen": { }
>          },
> -        {
> -            "rpi.focus": { }
> -        },
>          {
>              "rpi.af":
>              {
> diff --git a/src/ipa/raspberrypi/data/imx708_wide.json b/src/ipa/raspberrypi/data/imx708_wide.json
> index cdc61436995d..b772efee3b96 100644
> --- a/src/ipa/raspberrypi/data/imx708_wide.json
> +++ b/src/ipa/raspberrypi/data/imx708_wide.json
> @@ -403,9 +403,6 @@
>          {
>              "rpi.sharpen": { }
>          },
> -        {
> -            "rpi.focus": { }
> -        },
>          {
>              "rpi.af":
>              {
> diff --git a/src/ipa/raspberrypi/data/imx708_wide_noir.json b/src/ipa/raspberrypi/data/imx708_wide_noir.json
> index 8a7f59910833..c5f6b53dca7a 100644
> --- a/src/ipa/raspberrypi/data/imx708_wide_noir.json
> +++ b/src/ipa/raspberrypi/data/imx708_wide_noir.json
> @@ -403,9 +403,6 @@
>          {
>              "rpi.sharpen": { }
>          },
> -        {
> -            "rpi.focus": { }
> -        },
>          {
>              "rpi.af":
>              {
> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> index 9230e17bca22..de78cbd80f9c 100644
> --- a/src/ipa/raspberrypi/meson.build
> +++ b/src/ipa/raspberrypi/meson.build
> @@ -33,7 +33,6 @@ rpi_ipa_sources = files([
>      'controller/rpi/awb.cpp',
>      'controller/rpi/sharpen.cpp',
>      'controller/rpi/black_level.cpp',
> -    'controller/rpi/focus.cpp',
>      'controller/rpi/geq.cpp',
>      'controller/rpi/noise.cpp',
>      'controller/rpi/lux.cpp',
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index e2662938c9d3..9f3f3ac3b15d 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -50,7 +50,6 @@
>  #include "denoise_algorithm.h"
>  #include "denoise_status.h"
>  #include "dpc_status.h"
> -#include "focus_status.h"
>  #include "geq_status.h"
>  #include "lux_status.h"
>  #include "metadata.h"
> @@ -642,14 +641,28 @@ void IPARPi::reportMetadata(unsigned int ipaContext)
>  					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
>  					 static_cast<int32_t>(blackLevelStatus->blackLevelB) });
>
> -	FocusStatus *focusStatus = rpiMetadata.getLocked<FocusStatus>("focus.status");
> -	if (focusStatus && focusStatus->num == 12) {
> +	RPiController::FocusRegions *focusStatus =
> +		rpiMetadata.getLocked<RPiController::FocusRegions>("focus.status");
> +	if (focusStatus) {
>  		/*
> -		 * We get a 4x3 grid of regions by default. Calculate the average
> -		 * FoM over the central two positions to give an overall scene FoM.
> -		 * This can change later if it is not deemed suitable.
> +		 * Calculate the average FoM over the central (symmetric) positions
> +		 * to give an overall scene FoM. This can change later if it is
> +		 * not deemed suitable.
>  		 */
> -		int32_t focusFoM = (focusStatus->focusMeasures[5] + focusStatus->focusMeasures[6]) / 2;
> +		libcamera::Size size = focusStatus->size();
> +		unsigned rows = size.height;
> +		unsigned cols = size.width;
> +
> +		uint64_t sum = 0;
> +		unsigned int numRegions = 0;
> +		for (unsigned r = rows / 3; r < rows - rows / 3; ++r) {
> +			for (unsigned c = cols / 4; c < cols - cols / 4; ++c) {
> +				sum += focusStatus->get({ (int)c, (int)r }).val;
> +				numRegions++;
> +			}
> +		}
> +
> +		uint32_t focusFoM = (sum / numRegions) >> 16;
>  		libcameraMetadata_.set(controls::FocusFoM, focusFoM);
>  	}
>
> @@ -1428,7 +1441,6 @@ RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
>  		statistics->focusRegions.set(i, { stats->focus_stats[i].contrast_val[1][1] / 1000,
>  						  stats->focus_stats[i].contrast_val_num[1][1],
>  						  stats->focus_stats[i].contrast_val_num[1][0] });
> -
>  	return statistics;
>  }
>
> @@ -1445,6 +1457,10 @@ void IPARPi::processStats(unsigned int bufferId, unsigned int ipaContext)
>  	Span<uint8_t> mem = it->second.planes()[0];
>  	bcm2835_isp_stats *stats = reinterpret_cast<bcm2835_isp_stats *>(mem.data());
>  	RPiController::StatisticsPtr statistics = fillStatistics(stats);
> +
> +	/* Save the focus stats in the metadata structure to report out later. */
> +	rpiMetadata_[ipaContext].set("focus.status", statistics->focusRegions);
> +
>  	helper_->process(statistics, rpiMetadata);
>  	controller_.process(statistics, &rpiMetadata);
>
> --
> 2.34.1
>
David Plowman March 31, 2023, 8:18 a.m. UTC | #2 
Hi Naush

Thanks for the patch.

On Fri, 31 Mar 2023 at 08:41, Naushir Patuck via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> Use the generalised focus statistics structure to compute the centre
> window focus FoM value. This avoids needed to hard-code a specific
> grid size.
>
> Remove the focus reporting algorithm as the functionality is duplicated
> by this bit of IPA code. Remove focus_status.h as it is no longer needed.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

I think this is all fine. My only very slight misgiving is that
getting the focus FOM out (for manual lens adjustment) is ever so
slightly harder now for folks who are running without X Windows, but
hopefully writing 5 lines of Python is not beyond too many people...

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks!
David

> ---
>  src/ipa/raspberrypi/controller/focus_status.h | 20 --------
>  src/ipa/raspberrypi/controller/rpi/af.h       |  1 -
>  src/ipa/raspberrypi/controller/rpi/focus.cpp  | 49 -------------------
>  src/ipa/raspberrypi/data/imx290.json          |  3 --
>  src/ipa/raspberrypi/data/imx296.json          |  3 --
>  src/ipa/raspberrypi/data/imx296_mono.json     |  3 --
>  src/ipa/raspberrypi/data/imx477.json          |  3 --
>  src/ipa/raspberrypi/data/imx477_noir.json     |  3 --
>  .../raspberrypi/data/imx477_scientific.json   |  3 --
>  src/ipa/raspberrypi/data/imx477_v1.json       |  3 --
>  src/ipa/raspberrypi/data/imx708.json          |  3 --
>  src/ipa/raspberrypi/data/imx708_noir.json     |  3 --
>  src/ipa/raspberrypi/data/imx708_wide.json     |  3 --
>  .../raspberrypi/data/imx708_wide_noir.json    |  3 --
>  src/ipa/raspberrypi/meson.build               |  1 -
>  src/ipa/raspberrypi/raspberrypi.cpp           | 32 +++++++++---
>  16 files changed, 24 insertions(+), 112 deletions(-)
>  delete mode 100644 src/ipa/raspberrypi/controller/focus_status.h
>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/focus.cpp
>
> diff --git a/src/ipa/raspberrypi/controller/focus_status.h b/src/ipa/raspberrypi/controller/focus_status.h
> deleted file mode 100644
> index 8b74e59840c1..000000000000
> --- a/src/ipa/raspberrypi/controller/focus_status.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* SPDX-License-Identifier: BSD-2-Clause */
> -/*
> - * Copyright (C) 2020, Raspberry Pi Ltd
> - *
> - * focus_status.h - focus measurement status
> - */
> -#pragma once
> -
> -#include <linux/bcm2835-isp.h>
> -
> -/*
> - * The focus algorithm should post the following structure into the image's
> - * "focus.status" metadata. Recall that it's only reporting focus (contrast)
> - * measurements, it's not driving any kind of auto-focus algorithm!
> - */
> -
> -struct FocusStatus {
> -       unsigned int num;
> -       uint32_t focusMeasures[FOCUS_REGIONS];
> -};
> diff --git a/src/ipa/raspberrypi/controller/rpi/af.h b/src/ipa/raspberrypi/controller/rpi/af.h
> index b479feb88c39..6d2bae671a22 100644
> --- a/src/ipa/raspberrypi/controller/rpi/af.h
> +++ b/src/ipa/raspberrypi/controller/rpi/af.h
> @@ -28,7 +28,6 @@
>   * "nuisance" scans. During each interval where PDAF is not working, only
>   * ONE scan will be performed; CAF cannot track objects using CDAF alone.
>   *
> - * This algorithm is unrelated to "rpi.focus" which merely reports CDAF FoM.
>   */
>
>  namespace RPiController {
> diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp
> deleted file mode 100644
> index ea3cc00e42c3..000000000000
> --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -/* SPDX-License-Identifier: BSD-2-Clause */
> -/*
> - * Copyright (C) 2020, Raspberry Pi Ltd
> - *
> - * focus.cpp - focus algorithm
> - */
> -#include <stdint.h>
> -
> -#include <libcamera/base/log.h>
> -
> -#include "../focus_status.h"
> -#include "focus.h"
> -
> -using namespace RPiController;
> -using namespace libcamera;
> -
> -LOG_DEFINE_CATEGORY(RPiFocus)
> -
> -#define NAME "rpi.focus"
> -
> -Focus::Focus(Controller *controller)
> -       : Algorithm(controller)
> -{
> -}
> -
> -char const *Focus::name() const
> -{
> -       return NAME;
> -}
> -
> -void Focus::process(StatisticsPtr &stats, Metadata *imageMetadata)
> -{
> -       FocusStatus status;
> -       for (unsigned int i = 0; i < stats->focusRegions.numRegions(); i++)
> -               status.focusMeasures[i] = stats->focusRegions.get(i).val;
> -       status.num = stats->focusRegions.numRegions();
> -       imageMetadata->set("focus.status", status);
> -
> -       LOG(RPiFocus, Debug)
> -               << "Focus contrast measure: "
> -               << (status.focusMeasures[5] + status.focusMeasures[6]) / 10;
> -}
> -
> -/* Register algorithm with the system. */
> -static Algorithm *create(Controller *controller)
> -{
> -       return new Focus(controller);
> -}
> -static RegisterAlgorithm reg(NAME, &create);
> diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json
> index bfb9c6093d26..ace68d0ebf1c 100644
> --- a/src/ipa/raspberrypi/data/imx290.json
> +++ b/src/ipa/raspberrypi/data/imx290.json
> @@ -195,9 +195,6 @@
>                      }
>                  ]
>              }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> \ No newline at end of file
> diff --git a/src/ipa/raspberrypi/data/imx296.json b/src/ipa/raspberrypi/data/imx296.json
> index 346f5b658957..ae8722c46a85 100644
> --- a/src/ipa/raspberrypi/data/imx296.json
> +++ b/src/ipa/raspberrypi/data/imx296.json
> @@ -532,9 +532,6 @@
>                  "strength": 1.0,
>                  "limit": 0.18
>              }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> diff --git a/src/ipa/raspberrypi/data/imx296_mono.json b/src/ipa/raspberrypi/data/imx296_mono.json
> index e9fa30c6a08d..30965b4b8d61 100644
> --- a/src/ipa/raspberrypi/data/imx296_mono.json
> +++ b/src/ipa/raspberrypi/data/imx296_mono.json
> @@ -228,9 +228,6 @@
>                  "strength": 1.0,
>                  "limit": 0.18
>              }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json
> index bfc0774ffdfc..daffc268ff43 100644
> --- a/src/ipa/raspberrypi/data/imx477.json
> +++ b/src/ipa/raspberrypi/data/imx477.json
> @@ -513,9 +513,6 @@
>          },
>          {
>              "rpi.sharpen": { }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> \ No newline at end of file
> diff --git a/src/ipa/raspberrypi/data/imx477_noir.json b/src/ipa/raspberrypi/data/imx477_noir.json
> index dadd97bc3d78..52d7f072d0ce 100644
> --- a/src/ipa/raspberrypi/data/imx477_noir.json
> +++ b/src/ipa/raspberrypi/data/imx477_noir.json
> @@ -424,9 +424,6 @@
>          },
>          {
>              "rpi.sharpen": { }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> \ No newline at end of file
> diff --git a/src/ipa/raspberrypi/data/imx477_scientific.json b/src/ipa/raspberrypi/data/imx477_scientific.json
> index 17c4ed0a5e74..26c692fdbab9 100644
> --- a/src/ipa/raspberrypi/data/imx477_scientific.json
> +++ b/src/ipa/raspberrypi/data/imx477_scientific.json
> @@ -474,9 +474,6 @@
>          },
>          {
>              "rpi.sharpen": { }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> \ No newline at end of file
> diff --git a/src/ipa/raspberrypi/data/imx477_v1.json b/src/ipa/raspberrypi/data/imx477_v1.json
> index 5bcaac67069b..d64020091efa 100644
> --- a/src/ipa/raspberrypi/data/imx477_v1.json
> +++ b/src/ipa/raspberrypi/data/imx477_v1.json
> @@ -511,9 +511,6 @@
>          },
>          {
>              "rpi.sharpen": { }
> -        },
> -        {
> -            "rpi.focus": { }
>          }
>      ]
>  }
> \ No newline at end of file
> diff --git a/src/ipa/raspberrypi/data/imx708.json b/src/ipa/raspberrypi/data/imx708.json
> index c38b2d4cf256..b9830a3bf692 100644
> --- a/src/ipa/raspberrypi/data/imx708.json
> +++ b/src/ipa/raspberrypi/data/imx708.json
> @@ -512,9 +512,6 @@
>          {
>              "rpi.sharpen": { }
>          },
> -        {
> -            "rpi.focus": { }
> -        },
>          {
>              "rpi.af":
>              {
> diff --git a/src/ipa/raspberrypi/data/imx708_noir.json b/src/ipa/raspberrypi/data/imx708_noir.json
> index 082274e34c53..075f70355cec 100644
> --- a/src/ipa/raspberrypi/data/imx708_noir.json
> +++ b/src/ipa/raspberrypi/data/imx708_noir.json
> @@ -512,9 +512,6 @@
>          {
>              "rpi.sharpen": { }
>          },
> -        {
> -            "rpi.focus": { }
> -        },
>          {
>              "rpi.af":
>              {
> diff --git a/src/ipa/raspberrypi/data/imx708_wide.json b/src/ipa/raspberrypi/data/imx708_wide.json
> index cdc61436995d..b772efee3b96 100644
> --- a/src/ipa/raspberrypi/data/imx708_wide.json
> +++ b/src/ipa/raspberrypi/data/imx708_wide.json
> @@ -403,9 +403,6 @@
>          {
>              "rpi.sharpen": { }
>          },
> -        {
> -            "rpi.focus": { }
> -        },
>          {
>              "rpi.af":
>              {
> diff --git a/src/ipa/raspberrypi/data/imx708_wide_noir.json b/src/ipa/raspberrypi/data/imx708_wide_noir.json
> index 8a7f59910833..c5f6b53dca7a 100644
> --- a/src/ipa/raspberrypi/data/imx708_wide_noir.json
> +++ b/src/ipa/raspberrypi/data/imx708_wide_noir.json
> @@ -403,9 +403,6 @@
>          {
>              "rpi.sharpen": { }
>          },
> -        {
> -            "rpi.focus": { }
> -        },
>          {
>              "rpi.af":
>              {
> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> index 9230e17bca22..de78cbd80f9c 100644
> --- a/src/ipa/raspberrypi/meson.build
> +++ b/src/ipa/raspberrypi/meson.build
> @@ -33,7 +33,6 @@ rpi_ipa_sources = files([
>      'controller/rpi/awb.cpp',
>      'controller/rpi/sharpen.cpp',
>      'controller/rpi/black_level.cpp',
> -    'controller/rpi/focus.cpp',
>      'controller/rpi/geq.cpp',
>      'controller/rpi/noise.cpp',
>      'controller/rpi/lux.cpp',
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index e2662938c9d3..9f3f3ac3b15d 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -50,7 +50,6 @@
>  #include "denoise_algorithm.h"
>  #include "denoise_status.h"
>  #include "dpc_status.h"
> -#include "focus_status.h"
>  #include "geq_status.h"
>  #include "lux_status.h"
>  #include "metadata.h"
> @@ -642,14 +641,28 @@ void IPARPi::reportMetadata(unsigned int ipaContext)
>                                          static_cast<int32_t>(blackLevelStatus->blackLevelG),
>                                          static_cast<int32_t>(blackLevelStatus->blackLevelB) });
>
> -       FocusStatus *focusStatus = rpiMetadata.getLocked<FocusStatus>("focus.status");
> -       if (focusStatus && focusStatus->num == 12) {
> +       RPiController::FocusRegions *focusStatus =
> +               rpiMetadata.getLocked<RPiController::FocusRegions>("focus.status");
> +       if (focusStatus) {
>                 /*
> -                * We get a 4x3 grid of regions by default. Calculate the average
> -                * FoM over the central two positions to give an overall scene FoM.
> -                * This can change later if it is not deemed suitable.
> +                * Calculate the average FoM over the central (symmetric) positions
> +                * to give an overall scene FoM. This can change later if it is
> +                * not deemed suitable.
>                  */
> -               int32_t focusFoM = (focusStatus->focusMeasures[5] + focusStatus->focusMeasures[6]) / 2;
> +               libcamera::Size size = focusStatus->size();
> +               unsigned rows = size.height;
> +               unsigned cols = size.width;
> +
> +               uint64_t sum = 0;
> +               unsigned int numRegions = 0;
> +               for (unsigned r = rows / 3; r < rows - rows / 3; ++r) {
> +                       for (unsigned c = cols / 4; c < cols - cols / 4; ++c) {
> +                               sum += focusStatus->get({ (int)c, (int)r }).val;
> +                               numRegions++;
> +                       }
> +               }
> +
> +               uint32_t focusFoM = (sum / numRegions) >> 16;
>                 libcameraMetadata_.set(controls::FocusFoM, focusFoM);
>         }
>
> @@ -1428,7 +1441,6 @@ RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
>                 statistics->focusRegions.set(i, { stats->focus_stats[i].contrast_val[1][1] / 1000,
>                                                   stats->focus_stats[i].contrast_val_num[1][1],
>                                                   stats->focus_stats[i].contrast_val_num[1][0] });
> -
>         return statistics;
>  }
>
> @@ -1445,6 +1457,10 @@ void IPARPi::processStats(unsigned int bufferId, unsigned int ipaContext)
>         Span<uint8_t> mem = it->second.planes()[0];
>         bcm2835_isp_stats *stats = reinterpret_cast<bcm2835_isp_stats *>(mem.data());
>         RPiController::StatisticsPtr statistics = fillStatistics(stats);
> +
> +       /* Save the focus stats in the metadata structure to report out later. */
> +       rpiMetadata_[ipaContext].set("focus.status", statistics->focusRegions);
> +
>         helper_->process(statistics, rpiMetadata);
>         controller_.process(statistics, &rpiMetadata);
>
> --
> 2.34.1
>

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/controller/focus_status.h b/src/ipa/raspberrypi/controller/focus_status.h
deleted file mode 100644
index 8b74e59840c1..000000000000
--- a/src/ipa/raspberrypi/controller/focus_status.h
+++ /dev/null
@@ -1,20 +0,0 @@ 
-/* SPDX-License-Identifier: BSD-2-Clause */
-/*
- * Copyright (C) 2020, Raspberry Pi Ltd
- *
- * focus_status.h - focus measurement status
- */
-#pragma once
-
-#include <linux/bcm2835-isp.h>
-
-/*
- * The focus algorithm should post the following structure into the image's
- * "focus.status" metadata. Recall that it's only reporting focus (contrast)
- * measurements, it's not driving any kind of auto-focus algorithm!
- */
-
-struct FocusStatus {
-	unsigned int num;
-	uint32_t focusMeasures[FOCUS_REGIONS];
-};
diff --git a/src/ipa/raspberrypi/controller/rpi/af.h b/src/ipa/raspberrypi/controller/rpi/af.h
index b479feb88c39..6d2bae671a22 100644
--- a/src/ipa/raspberrypi/controller/rpi/af.h
+++ b/src/ipa/raspberrypi/controller/rpi/af.h
@@ -28,7 +28,6 @@ 
  * "nuisance" scans. During each interval where PDAF is not working, only
  * ONE scan will be performed; CAF cannot track objects using CDAF alone.
  *
- * This algorithm is unrelated to "rpi.focus" which merely reports CDAF FoM.
  */
 
 namespace RPiController {
diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp
deleted file mode 100644
index ea3cc00e42c3..000000000000
--- a/src/ipa/raspberrypi/controller/rpi/focus.cpp
+++ /dev/null
@@ -1,49 +0,0 @@ 
-/* SPDX-License-Identifier: BSD-2-Clause */
-/*
- * Copyright (C) 2020, Raspberry Pi Ltd
- *
- * focus.cpp - focus algorithm
- */
-#include <stdint.h>
-
-#include <libcamera/base/log.h>
-
-#include "../focus_status.h"
-#include "focus.h"
-
-using namespace RPiController;
-using namespace libcamera;
-
-LOG_DEFINE_CATEGORY(RPiFocus)
-
-#define NAME "rpi.focus"
-
-Focus::Focus(Controller *controller)
-	: Algorithm(controller)
-{
-}
-
-char const *Focus::name() const
-{
-	return NAME;
-}
-
-void Focus::process(StatisticsPtr &stats, Metadata *imageMetadata)
-{
-	FocusStatus status;
-	for (unsigned int i = 0; i < stats->focusRegions.numRegions(); i++)
-		status.focusMeasures[i] = stats->focusRegions.get(i).val;
-	status.num = stats->focusRegions.numRegions();
-	imageMetadata->set("focus.status", status);
-
-	LOG(RPiFocus, Debug)
-		<< "Focus contrast measure: "
-		<< (status.focusMeasures[5] + status.focusMeasures[6]) / 10;
-}
-
-/* Register algorithm with the system. */
-static Algorithm *create(Controller *controller)
-{
-	return new Focus(controller);
-}
-static RegisterAlgorithm reg(NAME, &create);
diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json
index bfb9c6093d26..ace68d0ebf1c 100644
--- a/src/ipa/raspberrypi/data/imx290.json
+++ b/src/ipa/raspberrypi/data/imx290.json
@@ -195,9 +195,6 @@ 
                     }
                 ]
             }
-        },
-        {
-            "rpi.focus": { }
         }
     ]
 }
\ No newline at end of file
diff --git a/src/ipa/raspberrypi/data/imx296.json b/src/ipa/raspberrypi/data/imx296.json
index 346f5b658957..ae8722c46a85 100644
--- a/src/ipa/raspberrypi/data/imx296.json
+++ b/src/ipa/raspberrypi/data/imx296.json
@@ -532,9 +532,6 @@ 
                 "strength": 1.0,
                 "limit": 0.18
             }
-        },
-        {
-            "rpi.focus": { }
         }
     ]
 }
diff --git a/src/ipa/raspberrypi/data/imx296_mono.json b/src/ipa/raspberrypi/data/imx296_mono.json
index e9fa30c6a08d..30965b4b8d61 100644
--- a/src/ipa/raspberrypi/data/imx296_mono.json
+++ b/src/ipa/raspberrypi/data/imx296_mono.json
@@ -228,9 +228,6 @@ 
                 "strength": 1.0,
                 "limit": 0.18
             }
-        },
-        {
-            "rpi.focus": { }
         }
     ]
 }
diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json
index bfc0774ffdfc..daffc268ff43 100644
--- a/src/ipa/raspberrypi/data/imx477.json
+++ b/src/ipa/raspberrypi/data/imx477.json
@@ -513,9 +513,6 @@ 
         },
         {
             "rpi.sharpen": { }
-        },
-        {
-            "rpi.focus": { }
         }
     ]
 }
\ No newline at end of file
diff --git a/src/ipa/raspberrypi/data/imx477_noir.json b/src/ipa/raspberrypi/data/imx477_noir.json
index dadd97bc3d78..52d7f072d0ce 100644
--- a/src/ipa/raspberrypi/data/imx477_noir.json
+++ b/src/ipa/raspberrypi/data/imx477_noir.json
@@ -424,9 +424,6 @@ 
         },
         {
             "rpi.sharpen": { }
-        },
-        {
-            "rpi.focus": { }
         }
     ]
 }
\ No newline at end of file
diff --git a/src/ipa/raspberrypi/data/imx477_scientific.json b/src/ipa/raspberrypi/data/imx477_scientific.json
index 17c4ed0a5e74..26c692fdbab9 100644
--- a/src/ipa/raspberrypi/data/imx477_scientific.json
+++ b/src/ipa/raspberrypi/data/imx477_scientific.json
@@ -474,9 +474,6 @@ 
         },
         {
             "rpi.sharpen": { }
-        },
-        {
-            "rpi.focus": { }
         }
     ]
 }
\ No newline at end of file
diff --git a/src/ipa/raspberrypi/data/imx477_v1.json b/src/ipa/raspberrypi/data/imx477_v1.json
index 5bcaac67069b..d64020091efa 100644
--- a/src/ipa/raspberrypi/data/imx477_v1.json
+++ b/src/ipa/raspberrypi/data/imx477_v1.json
@@ -511,9 +511,6 @@ 
         },
         {
             "rpi.sharpen": { }
-        },
-        {
-            "rpi.focus": { }
         }
     ]
 }
\ No newline at end of file
diff --git a/src/ipa/raspberrypi/data/imx708.json b/src/ipa/raspberrypi/data/imx708.json
index c38b2d4cf256..b9830a3bf692 100644
--- a/src/ipa/raspberrypi/data/imx708.json
+++ b/src/ipa/raspberrypi/data/imx708.json
@@ -512,9 +512,6 @@ 
         {
             "rpi.sharpen": { }
         },
-        {
-            "rpi.focus": { }
-        },
         {
             "rpi.af":
             {
diff --git a/src/ipa/raspberrypi/data/imx708_noir.json b/src/ipa/raspberrypi/data/imx708_noir.json
index 082274e34c53..075f70355cec 100644
--- a/src/ipa/raspberrypi/data/imx708_noir.json
+++ b/src/ipa/raspberrypi/data/imx708_noir.json
@@ -512,9 +512,6 @@ 
         {
             "rpi.sharpen": { }
         },
-        {
-            "rpi.focus": { }
-        },
         {
             "rpi.af":
             {
diff --git a/src/ipa/raspberrypi/data/imx708_wide.json b/src/ipa/raspberrypi/data/imx708_wide.json
index cdc61436995d..b772efee3b96 100644
--- a/src/ipa/raspberrypi/data/imx708_wide.json
+++ b/src/ipa/raspberrypi/data/imx708_wide.json
@@ -403,9 +403,6 @@ 
         {
             "rpi.sharpen": { }
         },
-        {
-            "rpi.focus": { }
-        },
         {
             "rpi.af":
             {
diff --git a/src/ipa/raspberrypi/data/imx708_wide_noir.json b/src/ipa/raspberrypi/data/imx708_wide_noir.json
index 8a7f59910833..c5f6b53dca7a 100644
--- a/src/ipa/raspberrypi/data/imx708_wide_noir.json
+++ b/src/ipa/raspberrypi/data/imx708_wide_noir.json
@@ -403,9 +403,6 @@ 
         {
             "rpi.sharpen": { }
         },
-        {
-            "rpi.focus": { }
-        },
         {
             "rpi.af":
             {
diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
index 9230e17bca22..de78cbd80f9c 100644
--- a/src/ipa/raspberrypi/meson.build
+++ b/src/ipa/raspberrypi/meson.build
@@ -33,7 +33,6 @@  rpi_ipa_sources = files([
     'controller/rpi/awb.cpp',
     'controller/rpi/sharpen.cpp',
     'controller/rpi/black_level.cpp',
-    'controller/rpi/focus.cpp',
     'controller/rpi/geq.cpp',
     'controller/rpi/noise.cpp',
     'controller/rpi/lux.cpp',
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index e2662938c9d3..9f3f3ac3b15d 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -50,7 +50,6 @@ 
 #include "denoise_algorithm.h"
 #include "denoise_status.h"
 #include "dpc_status.h"
-#include "focus_status.h"
 #include "geq_status.h"
 #include "lux_status.h"
 #include "metadata.h"
@@ -642,14 +641,28 @@  void IPARPi::reportMetadata(unsigned int ipaContext)
 					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
 					 static_cast<int32_t>(blackLevelStatus->blackLevelB) });
 
-	FocusStatus *focusStatus = rpiMetadata.getLocked<FocusStatus>("focus.status");
-	if (focusStatus && focusStatus->num == 12) {
+	RPiController::FocusRegions *focusStatus =
+		rpiMetadata.getLocked<RPiController::FocusRegions>("focus.status");
+	if (focusStatus) {
 		/*
-		 * We get a 4x3 grid of regions by default. Calculate the average
-		 * FoM over the central two positions to give an overall scene FoM.
-		 * This can change later if it is not deemed suitable.
+		 * Calculate the average FoM over the central (symmetric) positions
+		 * to give an overall scene FoM. This can change later if it is
+		 * not deemed suitable.
 		 */
-		int32_t focusFoM = (focusStatus->focusMeasures[5] + focusStatus->focusMeasures[6]) / 2;
+		libcamera::Size size = focusStatus->size();
+		unsigned rows = size.height;
+		unsigned cols = size.width;
+
+		uint64_t sum = 0;
+		unsigned int numRegions = 0;
+		for (unsigned r = rows / 3; r < rows - rows / 3; ++r) {
+			for (unsigned c = cols / 4; c < cols - cols / 4; ++c) {
+				sum += focusStatus->get({ (int)c, (int)r }).val;
+				numRegions++;
+			}
+		}
+
+		uint32_t focusFoM = (sum / numRegions) >> 16;
 		libcameraMetadata_.set(controls::FocusFoM, focusFoM);
 	}
 
@@ -1428,7 +1441,6 @@  RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
 		statistics->focusRegions.set(i, { stats->focus_stats[i].contrast_val[1][1] / 1000,
 						  stats->focus_stats[i].contrast_val_num[1][1],
 						  stats->focus_stats[i].contrast_val_num[1][0] });
-
 	return statistics;
 }
 
@@ -1445,6 +1457,10 @@  void IPARPi::processStats(unsigned int bufferId, unsigned int ipaContext)
 	Span<uint8_t> mem = it->second.planes()[0];
 	bcm2835_isp_stats *stats = reinterpret_cast<bcm2835_isp_stats *>(mem.data());
 	RPiController::StatisticsPtr statistics = fillStatistics(stats);
+
+	/* Save the focus stats in the metadata structure to report out later. */
+	rpiMetadata_[ipaContext].set("focus.status", statistics->focusRegions);
+
 	helper_->process(statistics, rpiMetadata);
 	controller_.process(statistics, &rpiMetadata);