[PATCH 12/19] libcamera: software_isp: Call Algorithm::queueRequest

[Milan Zamazal]

Hi Umang,

thank you for review.

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

> Hi Milan
>
> On 26/06/24 12:50 pm, Milan Zamazal wrote:
>> This patch adds Algorithm::queueRequest call for the defined algorithms.
>> This is preparation only since there are currently no Algorithm based
>> algorithms defined.
>>
>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>>   include/libcamera/internal/software_isp/software_isp.h |  1 +
>>   include/libcamera/ipa/soft.mojom                       |  1 +
>>   src/ipa/simple/soft_simple.cpp                         | 10 ++++++++++
>>   src/libcamera/pipeline/simple/simple.cpp               |  2 ++
>>   src/libcamera/software_isp/software_isp.cpp            | 10 ++++++++++
>>   5 files changed, 24 insertions(+)
>>
>> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
>> index 7365b49a..8753c2fd 100644
>> --- a/include/libcamera/internal/software_isp/software_isp.h
>> +++ b/include/libcamera/internal/software_isp/software_isp.h
>> @@ -72,6 +72,7 @@ public:
>>   	int start();
>>   	void stop();
>>   +	void queueRequest(const uint32_t frame, const ControlList &controls);
>>   	int queueBuffers(uint32_t frame, FrameBuffer *input,
>>   			 const std::map<unsigned int, FrameBuffer *> &outputs);
>>   diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
>> index 4975b251..dad352ba 100644
>> --- a/include/libcamera/ipa/soft.mojom
>> +++ b/include/libcamera/ipa/soft.mojom
>> @@ -20,6 +20,7 @@ interface IPASoftInterface {
>>   		=> (int32 ret);
>>             prepare(uint32 frame);
>> +        queueRequest(uint32 frame, libcamera.ControlList sensorControls);
>
> alignment needs to be fixed.

Yes, tabs x spaces mismatch. :-(

>>   	[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 9387508e..514a9db5 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 queueRequest(const uint32_t frame, const ControlList &controls) override;
>>   	void prepare(const uint32_t frame) override;
>>   	void processStats(const uint32_t frame, const uint32_t bufferId,
>>   			  const ControlList &sensorControls) override;
>> @@ -272,6 +273,15 @@ void IPASoftSimple::prepare(const uint32_t frame)
>>   		algo->prepare(context_, frame, frameContext, params_);
>>   }
>>   +void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)
>> +{
>> +	IPAFrameContext &frameContext = context_.frameContexts.alloc(frame);
>> +
>> +	for (auto const &algo : algorithms()) {
>> +		algo->queueRequest(context_, frame, frameContext, controls);
>> +	}
>> +}
>> +
>>   void IPASoftSimple::processStats(
>>   	const uint32_t frame,
>>   	[[maybe_unused]] const uint32_t bufferId,
>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
>> index 5cca94c3..0fa9db38 100644
>> --- a/src/libcamera/pipeline/simple/simple.cpp
>> +++ b/src/libcamera/pipeline/simple/simple.cpp
>> @@ -1421,6 +1421,8 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
>>   	if (data->useConversion_)
>>   		data->conversionQueue_.push(std::move(buffers));
>>   +	data->swIsp_->queueRequest(request->sequence(), request->controls());
>> +
>
> I think just passing request should be appropriate here
>
> +	data->swIsp_->queueRequest(request);
>
> would read better? and get frame sequence and control list inside the function ?

The other pipelines (rkisp1, ipu3) use the two arguments, because they
must.  Although it's not necessary here, I think it's better to be
consistent and not to move the logic down.  So I'll keep it as it is for
now, let's see what the other reviewers think.

>>   	return 0;
>>   }
>>   diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
>> index 812bc910..ba3f1bae 100644
>> --- a/src/libcamera/software_isp/software_isp.cpp
>> +++ b/src/libcamera/software_isp/software_isp.cpp
>> @@ -277,6 +277,16 @@ int SoftwareIsp::exportBuffers(unsigned int output, unsigned int count,
>>   	return count;
>>   }
>>   +/**
>> + * \brief Queue a request and process the control list from the application
>> + * \param[in] frame The number of the frame which will be processed next
>> + * \param[in] controls The controls for the \a frame
>> + */
>> +void SoftwareIsp::queueRequest(const uint32_t frame, const ControlList &controls)
>> +{
>> +	ipa_->queueRequest(frame, controls);
>> +}
>> +
>>   /**
>>    * \brief Queue buffers to Software ISP
>>    * \param[in] frame The frame number