[libcamera-devel] [PATCH v3 4/9] ipu3: Add StillCapture stream and imgu1 param buffers

Umang Jain umang.jain at ideasonboard.com
Wed Jul 27 10:41:12 CEST 2022


Hi Harvey,

On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:
> From: Harvey Yang <chenghaoyang at chromium.org>
>
> This patch adds the StillCapture stream and imgu1 param buffers.


It might be worth to add that we will ignore statistics for still 
capture, as they aren't needed for 3A here, right?

> The following patches will enable imgu1 to handle StillCapture stream
> specifically, when the imgu0 is needed to handle video/preview streams.
>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> ---
>   src/libcamera/pipeline/ipu3/frames.cpp | 23 +++++++++++++++++++++--
>   src/libcamera/pipeline/ipu3/frames.h   |  8 +++++++-
>   src/libcamera/pipeline/ipu3/ipu3.cpp   | 20 +++++++++++++++++++-
>   3 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp
> index a4c3477c..f9b705a5 100644
> --- a/src/libcamera/pipeline/ipu3/frames.cpp
> +++ b/src/libcamera/pipeline/ipu3/frames.cpp
> @@ -23,7 +23,8 @@ IPU3Frames::IPU3Frames()
>   }
>   
>   void IPU3Frames::init(const std::vector<std::unique_ptr<FrameBuffer>> &paramBuffers,
> -		      const std::vector<std::unique_ptr<FrameBuffer>> &statBuffers)
> +		      const std::vector<std::unique_ptr<FrameBuffer>> &statBuffers,
> +		      const std::vector<std::unique_ptr<FrameBuffer>> &captureParamBuffers)
>   {
>   	for (const std::unique_ptr<FrameBuffer> &buffer : paramBuffers)
>   		availableParamBuffers_.push(buffer.get());
> @@ -31,6 +32,9 @@ void IPU3Frames::init(const std::vector<std::unique_ptr<FrameBuffer>> &paramBuff
>   	for (const std::unique_ptr<FrameBuffer> &buffer : statBuffers)
>   		availableStatBuffers_.push(buffer.get());
>   
> +	for (const std::unique_ptr<FrameBuffer> &buffer : captureParamBuffers)
> +		availableCaptureParamBuffers_.push(buffer.get());
> +
>   	frameInfo_.clear();
>   }
>   
> @@ -38,6 +42,7 @@ void IPU3Frames::clear()
>   {
>   	availableParamBuffers_ = {};


alternatively should we rename this to, availableVfParamBuffers_ to mark 
the distinction?

>   	availableStatBuffers_ = {};
> +	availableCaptureParamBuffers_ = {};
>   }
>   
>   IPU3Frames::Info *IPU3Frames::create(Request *request)
> @@ -54,14 +59,22 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)
>   		return nullptr;
>   	}
>   
> +	if (availableCaptureParamBuffers_.empty()) {
> +		LOG(IPU3, Debug) << "Capture parameters buffer underrun";
> +		return nullptr;
> +	}
> +
>   	FrameBuffer *paramBuffer = availableParamBuffers_.front();


Ties into the earlier comment, maybe vfParamBuffer ? ... and so on below..

>   	FrameBuffer *statBuffer = availableStatBuffers_.front();
> +	FrameBuffer *captureParamBuffer = availableCaptureParamBuffers_.front();
>   
>   	paramBuffer->_d()->setRequest(request);
>   	statBuffer->_d()->setRequest(request);
> +	captureParamBuffer->_d()->setRequest(request);
>   
>   	availableParamBuffers_.pop();
>   	availableStatBuffers_.pop();
> +	availableCaptureParamBuffers_.pop();
>   
>   	/* \todo Remove the dynamic allocation of Info */
>   	std::unique_ptr<Info> info = std::make_unique<Info>();
> @@ -71,7 +84,9 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)
>   	info->rawBuffer = nullptr;
>   	info->paramBuffer = paramBuffer;
>   	info->statBuffer = statBuffer;
> +	info->captureParamBuffer = captureParamBuffer;
>   	info->paramDequeued = false;
> +	info->captureParamDequeued = false;
>   	info->metadataProcessed = false;
>   
>   	frameInfo_[id] = std::move(info);
> @@ -84,6 +99,7 @@ void IPU3Frames::remove(IPU3Frames::Info *info)
>   	/* Return params and stat buffer for reuse. */
>   	availableParamBuffers_.push(info->paramBuffer);
>   	availableStatBuffers_.push(info->statBuffer);
> +	availableCaptureParamBuffers_.push(info->captureParamBuffer);
>   
>   	/* Delete the extended frame information. */
>   	frameInfo_.erase(info->id);
> @@ -102,6 +118,9 @@ bool IPU3Frames::tryComplete(IPU3Frames::Info *info)
>   	if (!info->paramDequeued)
>   		return false;
>   
> +	if (!info->captureParamDequeued)
> +		return false;
> +
>   	remove(info);
>   
>   	bufferAvailable.emit();
> @@ -131,7 +150,7 @@ IPU3Frames::Info *IPU3Frames::find(FrameBuffer *buffer)
>   				return info;
>   
>   		if (info->rawBuffer == buffer || info->paramBuffer == buffer ||
> -		    info->statBuffer == buffer)
> +		    info->statBuffer == buffer || info->captureParamBuffer == buffer)
>   			return info;
>   	}
>   
> diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h
> index 6e3cb915..8fcb8a14 100644
> --- a/src/libcamera/pipeline/ipu3/frames.h
> +++ b/src/libcamera/pipeline/ipu3/frames.h
> @@ -36,16 +36,20 @@ public:
>   		FrameBuffer *paramBuffer;
>   		FrameBuffer *statBuffer;
>   
> +		FrameBuffer *captureParamBuffer;
> +
>   		ControlList effectiveSensorControls;
>   
>   		bool paramDequeued;
> +		bool captureParamDequeued;
>   		bool metadataProcessed;
>   	};
>   
>   	IPU3Frames();
>   
>   	void init(const std::vector<std::unique_ptr<FrameBuffer>> &paramBuffers,
> -		  const std::vector<std::unique_ptr<FrameBuffer>> &statBuffers);
> +		  const std::vector<std::unique_ptr<FrameBuffer>> &statBuffers,
> +		  const std::vector<std::unique_ptr<FrameBuffer>> &captureParamBuffers);
>   	void clear();
>   
>   	Info *create(Request *request);
> @@ -61,6 +65,8 @@ private:
>   	std::queue<FrameBuffer *> availableParamBuffers_;
>   	std::queue<FrameBuffer *> availableStatBuffers_;
>   
> +	std::queue<FrameBuffer *> availableCaptureParamBuffers_;
> +
>   	std::map<unsigned int, std::unique_ptr<Info>> frameInfo_;
>   };
>   
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index e219f704..a201c5ca 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -70,6 +70,7 @@ public:
>   	Stream outStream_;
>   	Stream vfStream_;
>   	Stream rawStream_;
> +	Stream outCaptureStream_;


If there is a outCaptureStream_ now, should we be dropping outStream_ 
and associated buffer allocations for outStream_ ?

My guess is yes, let's see what happens in subsequent patches.

For this one,

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

>   
>   	Rectangle cropRegion_;
>   	bool supportsFlips_;
> @@ -696,6 +697,8 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
>   		return imgu0_.viewfinder_->exportBuffers(count, buffers);
>   	else if (stream == &data->rawStream_)
>   		return data->cio2_.exportBuffers(count, buffers);
> +	else if (stream == &data->outCaptureStream_)
> +		return imgu1_.output_->exportBuffers(count, buffers);
>   
>   	return -EINVAL;
>   }
> @@ -718,11 +721,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)
>   		data->outStream_.configuration().bufferCount,
>   		data->vfStream_.configuration().bufferCount,
>   		data->rawStream_.configuration().bufferCount,
> +		data->outCaptureStream_.configuration().bufferCount,
>   	});
>   
>   	ret = imgu0_.allocateBuffers(bufferCount);
>   	if (ret < 0)
>   		return ret;
> +	ret = imgu1_.allocateBuffers(bufferCount);
> +	if (ret < 0) {
> +		imgu0_.freeBuffers();
> +		return ret;
> +	}
>   
>   	/* Map buffers to the IPA. */
>   	unsigned int ipaBufferId = 1;
> @@ -737,9 +746,14 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)
>   		ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());
>   	}
>   
> +	for (const std::unique_ptr<FrameBuffer> &buffer : imgu1_.paramBuffers_) {
> +		buffer->setCookie(ipaBufferId++);
> +		ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());
> +	}
> +
>   	data->ipa_->mapBuffers(ipaBuffers_);
>   
> -	data->frameInfos_.init(imgu0_.paramBuffers_, imgu0_.statBuffers_);
> +	data->frameInfos_.init(imgu0_.paramBuffers_, imgu0_.statBuffers_, imgu1_.paramBuffers_);
>   	data->frameInfos_.bufferAvailable.connect(
>   		data, &IPU3CameraData::queuePendingRequests);
>   
> @@ -760,6 +774,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)
>   	ipaBuffers_.clear();
>   
>   	imgu0_.freeBuffers();
> +	imgu1_.freeBuffers();
>   
>   	return 0;
>   }
> @@ -1143,6 +1158,7 @@ int PipelineHandlerIPU3::registerCameras()
>   			&data->outStream_,
>   			&data->vfStream_,
>   			&data->rawStream_,
> +			&data->outCaptureStream_,
>   		};
>   		CIO2Device *cio2 = &data->cio2_;
>   
> @@ -1318,6 +1334,8 @@ void IPU3CameraData::paramsBufferReady(unsigned int id)
>   	imgu0_->param_->queueBuffer(info->paramBuffer);
>   	imgu0_->stat_->queueBuffer(info->statBuffer);
>   	imgu0_->input_->queueBuffer(info->rawBuffer);
> +
> +	info->captureParamDequeued = true;
>   }
>   
>   void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)


More information about the libcamera-devel mailing list