[PATCH v2 4/5] ipa: simple: Report black levels in metadata

[Kieran Bingham]

Hi Milan,

Quoting Milan Zamazal (2024-12-19 21:10:09)
> From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> Provide the determined black level values in the metadata
> to add to the completed requests.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  src/ipa/simple/algorithms/blc.cpp | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp
> index 1d7d370b..d5759f92 100644
> --- a/src/ipa/simple/algorithms/blc.cpp
> +++ b/src/ipa/simple/algorithms/blc.cpp
> @@ -11,6 +11,8 @@
>  
>  #include <libcamera/base/log.h>
>  
> +#include "control_ids.h"
> +
>  namespace libcamera {
>  
>  namespace ipa::soft::algorithms {
> @@ -49,8 +51,13 @@ void BlackLevel::process(IPAContext &context,
>                          [[maybe_unused]] const uint32_t frame,
>                          IPAFrameContext &frameContext,
>                          const SwIspStats *stats,
> -                        [[maybe_unused]] ControlList &metadata)
> +                        ControlList &metadata)
>  {
> +       /* Assign each of the R G G B channels as the same black level. */
> +       const int32_t blackLevel = context.activeState.blc.level * 256;

What are your thoughts on
https://patchwork.libcamera.org/project/libcamera/list/?series=4830?

This could then be:
	const int32_t blackLevel = context.activeState.blc.level.convert<16>();

or
	BitDepthValue<16> blackLevel = context.activeState.blc.level;

I'm curious if you think this provides a helpful / expressive way to
convey this? or if it obfuscates more ?

> +       const int32_t blackLevels[] = { blackLevel, blackLevel, blackLevel, blackLevel };
> +       metadata.set(controls::SensorBlackLevels, blackLevels);
> +
>         if (context.configuration.black.level.has_value())
>                 return;
>  
> -- 
> 2.44.2
>