[libcamera-devel,v1,1/1] libcamera: Fix ipu3's issue of first black frame
diff mbox series

Message ID 20230918085222.500697-2-chenghaoyang@chromium.org
State New
Headers show
Series
  • Fix ipu3 pipeline handler's first frame
Related show

Commit Message

Harvey Yang Sept. 18, 2023, 8:49 a.m. UTC
As ipu3 ISP requires two frames in the pipeline to generate non-delayed
output, this CL fixes the problem of the first frame (preview and
capture) being black by queueing the first buffer twice.

Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 50 ++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 7 deletions(-)

Comments

Harvey Yang Nov. 15, 2023, 8:26 a.m. UTC | #1
Hi folks,

Could I request a round of review? Thanks!

On Mon, Sep 18, 2023 at 4:52 PM Harvey Yang <chenghaoyang@chromium.org>
wrote:

> As ipu3 ISP requires two frames in the pipeline to generate non-delayed
> output, this CL fixes the problem of the first frame (preview and
> capture) being black by queueing the first buffer twice.
>
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 50 ++++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp
> b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index a81c817a..fa4a8a10 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -86,11 +86,15 @@ public:
>
>         ControlInfoMap ipaControls_;
>
> +       bool firstRequest_ = true;
> +
>  private:
>         void metadataReady(unsigned int id, const ControlList &metadata);
>         void paramsBufferReady(unsigned int id);
>         void setSensorControls(unsigned int id, const ControlList
> &sensorControls,
>                                const ControlList &lensControls);
> +
> +       std::map<FrameBuffer *, int> bufferReturnCounters_;
>  };
>
>  class IPU3CameraConfiguration : public CameraConfiguration
> @@ -721,6 +725,8 @@ int PipelineHandlerIPU3::start(Camera *camera,
> [[maybe_unused]] const ControlLis
>         ImgUDevice *imgu = data->imgu_;
>         int ret;
>
> +       data->firstRequest_ = true;
> +
>         /* Disable test pattern mode on the sensor, if any. */
>         ret = cio2->sensor()->setTestPatternMode(
>                 controls::draft::TestPatternModeEnum::TestPatternModeOff);
> @@ -1224,22 +1230,37 @@ void IPU3CameraData::paramsBufferReady(unsigned
> int id)
>         if (!info)
>                 return;
>
> +       const int yuvCount = firstRequest_ ? 2 : 1;
> +       firstRequest_ = false;
>         /* Queue all buffers from the request aimed for the ImgU. */
>         for (auto it : info->request->buffers()) {
>                 const Stream *stream = it.first;
>                 FrameBuffer *outbuffer = it.second;
>
> -               if (stream == &outStream_)
> -                       imgu_->output_->queueBuffer(outbuffer);
> -               else if (stream == &vfStream_)
> -                       imgu_->viewfinder_->queueBuffer(outbuffer);
> +               if (stream == &outStream_) {
> +                       for (int i = 0; i < yuvCount; ++i) {
> +                               bufferReturnCounters_[outbuffer] += 1;
> +                               imgu_->output_->queueBuffer(outbuffer);
> +                       }
> +               } else if (stream == &vfStream_) {
> +                       for (int i = 0; i < yuvCount; ++i) {
> +                               bufferReturnCounters_[outbuffer] += 1;
> +                               imgu_->viewfinder_->queueBuffer(outbuffer);
> +                       }
> +               }
>         }
>
>         info->paramBuffer->_d()->metadata().planes()[0].bytesused =
>                 sizeof(struct ipu3_uapi_params);
> -       imgu_->param_->queueBuffer(info->paramBuffer);
> -       imgu_->stat_->queueBuffer(info->statBuffer);
> -       imgu_->input_->queueBuffer(info->rawBuffer);
> +       for (int i = 0; i < yuvCount; ++i) {
> +               bufferReturnCounters_[info->paramBuffer] += 1;
> +               bufferReturnCounters_[info->statBuffer] += 1;
> +               bufferReturnCounters_[info->rawBuffer] += 1;
> +
> +               imgu_->param_->queueBuffer(info->paramBuffer);
> +               imgu_->stat_->queueBuffer(info->statBuffer);
> +               imgu_->input_->queueBuffer(info->rawBuffer);
> +       }
>  }
>
>  void IPU3CameraData::metadataReady(unsigned int id, const ControlList
> &metadata)
> @@ -1268,6 +1289,11 @@ void IPU3CameraData::metadataReady(unsigned int id,
> const ControlList &metadata)
>   */
>  void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
>  {
> +       if (--bufferReturnCounters_[buffer] > 0)
> +               return;
> +
> +       bufferReturnCounters_.erase(buffer);
> +
>         IPU3Frames::Info *info = frameInfos_.find(buffer);
>         if (!info)
>                 return;
> @@ -1334,6 +1360,11 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer
> *buffer)
>
>  void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)
>  {
> +       if (--bufferReturnCounters_[buffer] > 0)
> +               return;
> +
> +       bufferReturnCounters_.erase(buffer);
> +
>         IPU3Frames::Info *info = frameInfos_.find(buffer);
>         if (!info)
>                 return;
> @@ -1354,6 +1385,11 @@ void IPU3CameraData::paramBufferReady(FrameBuffer
> *buffer)
>
>  void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
>  {
> +       if (--bufferReturnCounters_[buffer] > 0)
> +               return;
> +
> +       bufferReturnCounters_.erase(buffer);
> +
>         IPU3Frames::Info *info = frameInfos_.find(buffer);
>         if (!info)
>                 return;
> --
> 2.42.0.459.ge4e396fd5e-goog
>
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index a81c817a..fa4a8a10 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -86,11 +86,15 @@  public:
 
 	ControlInfoMap ipaControls_;
 
+	bool firstRequest_ = true;
+
 private:
 	void metadataReady(unsigned int id, const ControlList &metadata);
 	void paramsBufferReady(unsigned int id);
 	void setSensorControls(unsigned int id, const ControlList &sensorControls,
 			       const ControlList &lensControls);
+
+	std::map<FrameBuffer *, int> bufferReturnCounters_;
 };
 
 class IPU3CameraConfiguration : public CameraConfiguration
@@ -721,6 +725,8 @@  int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis
 	ImgUDevice *imgu = data->imgu_;
 	int ret;
 
+	data->firstRequest_ = true;
+
 	/* Disable test pattern mode on the sensor, if any. */
 	ret = cio2->sensor()->setTestPatternMode(
 		controls::draft::TestPatternModeEnum::TestPatternModeOff);
@@ -1224,22 +1230,37 @@  void IPU3CameraData::paramsBufferReady(unsigned int id)
 	if (!info)
 		return;
 
+	const int yuvCount = firstRequest_ ? 2 : 1;
+	firstRequest_ = false;
 	/* Queue all buffers from the request aimed for the ImgU. */
 	for (auto it : info->request->buffers()) {
 		const Stream *stream = it.first;
 		FrameBuffer *outbuffer = it.second;
 
-		if (stream == &outStream_)
-			imgu_->output_->queueBuffer(outbuffer);
-		else if (stream == &vfStream_)
-			imgu_->viewfinder_->queueBuffer(outbuffer);
+		if (stream == &outStream_) {
+			for (int i = 0; i < yuvCount; ++i) {
+				bufferReturnCounters_[outbuffer] += 1;
+				imgu_->output_->queueBuffer(outbuffer);
+			}
+		} else if (stream == &vfStream_) {
+			for (int i = 0; i < yuvCount; ++i) {
+				bufferReturnCounters_[outbuffer] += 1;
+				imgu_->viewfinder_->queueBuffer(outbuffer);
+			}
+		}
 	}
 
 	info->paramBuffer->_d()->metadata().planes()[0].bytesused =
 		sizeof(struct ipu3_uapi_params);
-	imgu_->param_->queueBuffer(info->paramBuffer);
-	imgu_->stat_->queueBuffer(info->statBuffer);
-	imgu_->input_->queueBuffer(info->rawBuffer);
+	for (int i = 0; i < yuvCount; ++i) {
+		bufferReturnCounters_[info->paramBuffer] += 1;
+		bufferReturnCounters_[info->statBuffer] += 1;
+		bufferReturnCounters_[info->rawBuffer] += 1;
+
+		imgu_->param_->queueBuffer(info->paramBuffer);
+		imgu_->stat_->queueBuffer(info->statBuffer);
+		imgu_->input_->queueBuffer(info->rawBuffer);
+	}
 }
 
 void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)
@@ -1268,6 +1289,11 @@  void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)
  */
 void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
 {
+	if (--bufferReturnCounters_[buffer] > 0)
+		return;
+
+	bufferReturnCounters_.erase(buffer);
+
 	IPU3Frames::Info *info = frameInfos_.find(buffer);
 	if (!info)
 		return;
@@ -1334,6 +1360,11 @@  void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
 
 void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)
 {
+	if (--bufferReturnCounters_[buffer] > 0)
+		return;
+
+	bufferReturnCounters_.erase(buffer);
+
 	IPU3Frames::Info *info = frameInfos_.find(buffer);
 	if (!info)
 		return;
@@ -1354,6 +1385,11 @@  void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)
 
 void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
 {
+	if (--bufferReturnCounters_[buffer] > 0)
+		return;
+
+	bufferReturnCounters_.erase(buffer);
+
 	IPU3Frames::Info *info = frameInfos_.find(buffer);
 	if (!info)
 		return;