[libcamera-devel] [PATCH v3 5/9] ipu3: Update IPAIPU3Interface::fillParamsBuffer with captureBufferId

[Umang Jain]

Hi Harvey,

On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:
> From: Harvey Yang <chenghaoyang at chromium.org>
>
> This patch updates the ipa interface |IPAIPU3Interface::fillParamsBuffer|
> with additional |captureBufferId| to fill the param buffer for the
> StillCapture stream as well.


Whenever the IPAIPU3 Interface is updated,

     https://git.libcamera.org/libcamera/ipu3-ipa.git/tree/ (closed 
source IPA IPU3 wrapper)

will need corresponding updates as well (Just mentioning for the records)

>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> ---
>   include/libcamera/ipa/ipu3.mojom     |  2 +-
>   src/ipa/ipu3/ipu3.cpp                | 21 ++++++++++++++++++---
>   src/libcamera/pipeline/ipu3/ipu3.cpp |  3 ++-
>   3 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
> index d1b1c6b8..d94c344e 100644
> --- a/include/libcamera/ipa/ipu3.mojom
> +++ b/include/libcamera/ipa/ipu3.mojom
> @@ -31,7 +31,7 @@ interface IPAIPU3Interface {
>   	unmapBuffers(array<uint32> ids);
>   
>   	[async] queueRequest(uint32 frame, libcamera.ControlList controls);
> -	[async] fillParamsBuffer(uint32 frame, uint32 bufferId);
> +	[async] fillParamsBuffer(uint32 frame, uint32 bufferId, uint32 captureBufferId);
>   	[async] processStatsBuffer(uint32 frame, int64 frameTimestamp,
>   				   uint32 bufferId, libcamera.ControlList sensorControls);
>   };
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index dd6cfd79..149a3958 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -146,7 +146,8 @@ public:
>   	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>   
>   	void queueRequest(const uint32_t frame, const ControlList &controls) override;
> -	void fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) override;
> +	void fillParamsBuffer(const uint32_t frame, const uint32_t bufferId,
> +			      const uint32_t captureBufferId) override;
>   	void processStatsBuffer(const uint32_t frame, const int64_t frameTimestamp,
>   				const uint32_t bufferId,
>   				const ControlList &sensorControls) override;
> @@ -508,12 +509,14 @@ void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)
>   /**
>    * \brief Fill and return a buffer with ISP processing parameters for a frame
>    * \param[in] frame The frame number
> - * \param[in] bufferId ID of the parameter buffer to fill
> + * \param[in] bufferId ID of the video parameter buffer to fill
> + * \param[in] captureBufferId ID of the capture parameter buffer to fill
>    *
>    * Algorithms are expected to fill the IPU3 parameter buffer for the next
>    * frame given their most recent processing of the ImgU statistics.
>    */
> -void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
> +void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId,


Since now there are couple of parameter buffers to fill, this can be 
renamed to depict plural buffers:

     IPAIPU3::fillParamsBuffers(const uint32_t frame, const uint32_t 
bufferId,

> +			       const uint32_t captureBufferId)
>   {
>   	auto it = buffers_.find(bufferId);
>   	if (it == buffers_.end()) {
> @@ -536,6 +539,18 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
>   	 */
>   	params->use = {};
>   
> +	for (auto const &algo : algorithms_)
> +		algo->prepare(context_, params);
> +
> +	// TODO: use different algorithms to set StillCapture params.


Can you provide a brief summary here, specific to algorithms targetting 
StillCapture use-case? How would they differ (and I wonder if they 
work/need statistics from Imgu1).

> +	it = buffers_.find(captureBufferId);
> +	if (it == buffers_.end()) {
> +		LOG(IPAIPU3, Error) << "Could not find capture param buffer!";
> +		return;
> +	}
> +	mem = it->second.planes()[0];
> +	params = reinterpret_cast<ipu3_uapi_params *>(mem.data());
> +	params->use = {};
>   	for (auto const &algo : algorithms_)
>   		algo->prepare(context_, params);
>   
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index a201c5ca..a13fb881 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -1424,7 +1424,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
>   	if (request->findBuffer(&rawStream_))
>   		pipe()->completeBuffer(request, buffer);
>   
> -	ipa_->fillParamsBuffer(info->id, info->paramBuffer->cookie());
> +	ipa_->fillParamsBuffer(info->id, info->paramBuffer->cookie(),
> +			       info->captureParamBuffer->cookie());
>   }
>   
>   void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)