[11/19] libcamera: software_isp: Call Algorithm::prepare

20403 diff mbox series

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

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 include/libcamera/ipa/soft.mojom            | 1 +
 src/ipa/simple/soft_simple.cpp              | 8 ++++++++
 src/libcamera/software_isp/software_isp.cpp | 1 +
 3 files changed, 10 insertions(+)

Hi Milan

On 26/06/24 12:50 pm, Milan Zamazal wrote:
> This patch adds Algorithm::prepare call for the defined algorithms.
> This is preparation only since there are currently no Algorithm based
> algorithms defined.
>
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>   include/libcamera/ipa/soft.mojom            | 1 +
>   src/ipa/simple/soft_simple.cpp              | 8 ++++++++
>   src/libcamera/software_isp/software_isp.cpp | 1 +
>   3 files changed, 10 insertions(+)
>
> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
> index bd48ece9..4975b251 100644
> --- a/include/libcamera/ipa/soft.mojom
> +++ b/include/libcamera/ipa/soft.mojom
> @@ -19,6 +19,7 @@ interface IPASoftInterface {
>   	configure(libcamera.ControlInfoMap sensorCtrlInfoMap)
>   		=> (int32 ret);
>   
> +        prepare(uint32 frame);

misaligned
>   	[async] processStats(uint32 frame, uint32 bufferId, libcamera.ControlList sensorControls);
>   };
>   
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index c0cb6769..9387508e 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -78,6 +78,7 @@ public:
>   	int start() override;
>   	void stop() override;
>   
> +	void prepare(const uint32_t frame) override;
>   	void processStats(const uint32_t frame, const uint32_t bufferId,
>   			  const ControlList &sensorControls) override;
>   
> @@ -264,6 +265,13 @@ void IPASoftSimple::stop()
>   {
>   }
>   
> +void IPASoftSimple::prepare(const uint32_t frame)
> +{
> +	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
> +	for (auto const &algo : algorithms())
> +		algo->prepare(context_, frame, frameContext, params_);
> +}
> +
>   void IPASoftSimple::processStats(
>   	const uint32_t frame,
>   	[[maybe_unused]] const uint32_t bufferId,
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index aa60fb5f..812bc910 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -347,6 +347,7 @@ void SoftwareIsp::stop()
>    */
>   void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output)
>   {
> +	ipa_->prepare(frame);

The algo->prepare() loop can be in-lined here. I don't (yet) the benefit 
of having and calling a separate function as ipa_->prepare().

>   	debayer_->invokeMethod(&DebayerCpu::process,
>   			       ConnectionTypeQueued, frame, input, output, debayerParams_);
>   }

Hi Umang,

thank you for review.

Umang Jain <umang.jain@ideasonboard.com> writes:

> Hi Milan
>
> On 26/06/24 12:50 pm, Milan Zamazal wrote:
>> This patch adds Algorithm::prepare call for the defined algorithms.
>> This is preparation only since there are currently no Algorithm based
>> algorithms defined.
>>
>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
>> ---
>>   include/libcamera/ipa/soft.mojom            | 1 +
>>   src/ipa/simple/soft_simple.cpp              | 8 ++++++++
>>   src/libcamera/software_isp/software_isp.cpp | 1 +
>>   3 files changed, 10 insertions(+)
>>
>> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
>> index bd48ece9..4975b251 100644
>> --- a/include/libcamera/ipa/soft.mojom
>> +++ b/include/libcamera/ipa/soft.mojom
>> @@ -19,6 +19,7 @@ interface IPASoftInterface {
>>   	configure(libcamera.ControlInfoMap sensorCtrlInfoMap)
>>   		=> (int32 ret);
>>   +        prepare(uint32 frame);
>
> misaligned
>>   	[async] processStats(uint32 frame, uint32 bufferId, libcamera.ControlList sensorControls);
>>   };
>>   diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
>> index c0cb6769..9387508e 100644
>> --- a/src/ipa/simple/soft_simple.cpp
>> +++ b/src/ipa/simple/soft_simple.cpp
>> @@ -78,6 +78,7 @@ public:
>>   	int start() override;
>>   	void stop() override;
>>   +	void prepare(const uint32_t frame) override;
>>   	void processStats(const uint32_t frame, const uint32_t bufferId,
>>   			  const ControlList &sensorControls) override;
>>   @@ -264,6 +265,13 @@ void IPASoftSimple::stop()
>>   {
>>   }
>>   +void IPASoftSimple::prepare(const uint32_t frame)
>> +{
>> +	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
>> +	for (auto const &algo : algorithms())
>> +		algo->prepare(context_, frame, frameContext, params_);
>> +}
>> +
>>   void IPASoftSimple::processStats(
>>   	const uint32_t frame,
>>   	[[maybe_unused]] const uint32_t bufferId,
>> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
>> index aa60fb5f..812bc910 100644
>> --- a/src/libcamera/software_isp/software_isp.cpp
>> +++ b/src/libcamera/software_isp/software_isp.cpp
>> @@ -347,6 +347,7 @@ void SoftwareIsp::stop()
>>    */
>>   void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output)
>>   {
>> +	ipa_->prepare(frame);
>
> The algo->prepare() loop can be in-lined here. I don't (yet) the benefit of having
> and calling a separate function as ipa_->prepare().

I'm not sure I understand correctly what you mean here.  algo->prepare()
is an IPA thing, i.e. belonging to ipa_ conceptually, and it requires
the IPA context, which is not available here.

>>   	debayer_->invokeMethod(&DebayerCpu::process,
>>   			       ConnectionTypeQueued, frame, input, output, debayerParams_);
>>   }