[v5,12/18] libcamera: software_isp: Call Algorithm::process

21068 diff mbox series

This patch adds Algorithm::process call for the defined algorithms.
This is preparation only since there are currently no Algorithm based
algorithms defined.

As software ISP currently doesn't produce any metadata, a dummy and
unused metadata instance is created to satisfy Algorithm::process API.
This should be changed in future.

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/soft_simple.cpp | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Hi Milan,

Thank you for the patch.

On Fri, Aug 30, 2024 at 09:25:48AM +0200, Milan Zamazal wrote:
> This patch adds Algorithm::process call for the defined algorithms.
> This is preparation only since there are currently no Algorithm based
> algorithms defined.
> 
> As software ISP currently doesn't produce any metadata, a dummy and
> unused metadata instance is created to satisfy Algorithm::process API.
> This should be changed in future.

s/should/will/ as I suppose it's addressed later in the series ?

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> 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/soft_simple.cpp | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index d52242be..4b9201dc 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -287,10 +287,21 @@ void IPASoftSimple::prepare(const uint32_t frame)
>  		algo->prepare(context_, frame, frameContext, params_);
>  }
>  
> -void IPASoftSimple::processStats([[maybe_unused]] const uint32_t frame,
> +void IPASoftSimple::processStats(const uint32_t frame,
>  				 [[maybe_unused]] const uint32_t bufferId,
>  				 const ControlList &sensorControls)
>  {
> +	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
> +	/*
> +	 * Software ISP currently does not produce any metadata. Use an empty
> +	 * ControlList for now.
> +	 *
> +	 * \todo Implement proper metadata handling
> +	 */
> +	ControlList metadata(controls::controls);
> +	for (auto const &algo : algorithms())
> +		algo->process(context_, frame, frameContext, stats_, metadata);
> +
>  	SwIspStats::Histogram histogram = stats_->yHistogram;
>  	if (ignoreUpdates_ > 0)
>  		blackLevel_.update(histogram);

Hi Laurent,

thank you for reviews.

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Hi Milan,
>
> Thank you for the patch.
>
> On Fri, Aug 30, 2024 at 09:25:48AM +0200, Milan Zamazal wrote:
>> This patch adds Algorithm::process call for the defined algorithms.
>> This is preparation only since there are currently no Algorithm based
>> algorithms defined.
>> 
>> As software ISP currently doesn't produce any metadata, a dummy and
>> unused metadata instance is created to satisfy Algorithm::process API.
>> This should be changed in future.
>
> s/should/will/ as I suppose it's addressed later in the series ?

It is not.  There is
https://patchwork.libcamera.org/project/libcamera/list/?series=4405 by
Kieran that introduces metadata to the simple pipeline.  Which would be
worth to revive at some point once the basic refactoring is finished.

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
>> 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/soft_simple.cpp | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
>> index d52242be..4b9201dc 100644
>> --- a/src/ipa/simple/soft_simple.cpp
>> +++ b/src/ipa/simple/soft_simple.cpp
>> @@ -287,10 +287,21 @@ void IPASoftSimple::prepare(const uint32_t frame)
>>  		algo->prepare(context_, frame, frameContext, params_);
>>  }
>>  
>> -void IPASoftSimple::processStats([[maybe_unused]] const uint32_t frame,
>> +void IPASoftSimple::processStats(const uint32_t frame,
>>  				 [[maybe_unused]] const uint32_t bufferId,
>>  				 const ControlList &sensorControls)
>>  {
>> +	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
>> +	/*
>> +	 * Software ISP currently does not produce any metadata. Use an empty
>> +	 * ControlList for now.
>> +	 *
>> +	 * \todo Implement proper metadata handling
>> +	 */
>> +	ControlList metadata(controls::controls);
>> +	for (auto const &algo : algorithms())
>> +		algo->process(context_, frame, frameContext, stats_, metadata);
>> +
>>  	SwIspStats::Histogram histogram = stats_->yHistogram;
>>  	if (ignoreUpdates_ > 0)
>>  		blackLevel_.update(histogram);