[libcamera-devel,v4,9/9] ipu3: Fixes frame delay
diff mbox series

Message ID 20220802102943.3221109-10-chenghaoyang@google.com
State New
Headers show
Series
  • Use two imgus in ipu3 pipeline handler
Related show

Commit Message

Cheng-Hao Yang Aug. 2, 2022, 10:29 a.m. UTC 
Like the shipped ipu3 HAL [1], handle the frame delay with one extra input,
needed for the first frame and StillCapture's non-sequential frame
requests.

[1]:
https://source.corp.google.com/chromeos_public/src/platform/camera/hal/intel/ipu3/psl/ipu3/ImguUnit.cpp;l=773

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

Comments

Cheng-Hao Yang Aug. 3, 2022, 8:44 a.m. UTC | #1 
Sorry I forgot to include the comment that explains how |stillCount| is
calculated and what it does.
To avoid the spam, I'll put the comment here and upload it with PATCH v5:


/*

* If the StillCapture request is the first request or isn't

* consequential, we need to requeue the same buffers to ImgU1 to

* mitigate the frame delay (i.e. |stillCount| being 2). Otherwise,

* |stillCount| would be 1, which means the buffers are queued once as

* usual.

*/



On Tue, Aug 2, 2022 at 6:30 PM Harvey Yang <chenghaoyang@chromium.org>
wrote:

> Like the shipped ipu3 HAL [1], handle the frame delay with one extra input,
> needed for the first frame and StillCapture's non-sequential frame
> requests.
>
> [1]:
>
> https://source.corp.google.com/chromeos_public/src/platform/camera/hal/intel/ipu3/psl/ipu3/ImguUnit.cpp;l=773
>
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 73 +++++++++++++++++++++++-----
>  1 file changed, 62 insertions(+), 11 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp
> b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index f8101c6c..36ca47c2 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -90,6 +90,9 @@ public:
>
>         ControlInfoMap ipaControls_;
>
> +       bool firstRequest_ = true;
> +       int lastCaptureRequestSeq_ = -1;
> +
>  private:
>         void metadataReady(unsigned int id, const ControlList &metadata);
>         void paramsBufferReady(unsigned int id);
> @@ -884,6 +887,9 @@ int PipelineHandlerIPU3::start(Camera *camera,
> [[maybe_unused]] const ControlLis
>         CIO2Device *cio2 = &data->cio2_;
>         int ret;
>
> +       data->firstRequest_ = true;
> +       data->lastCaptureRequestSeq_ = -1;
> +
>         imgu0_.input_->bufferReady.connect(data,
>
>  &IPU3CameraData::tryReturnBuffer);
>         imgu0_.output_->bufferReady.connect(data,
> @@ -1422,6 +1428,11 @@ void IPU3CameraData::paramsBufferReady(unsigned int
> id)
>         if (!info)
>                 return;
>
> +       int yuvCount = firstRequest_ ? 2 : 1;
> +       int stillCount = firstRequest_ || (lastCaptureRequestSeq_ + 1) !=
> static_cast<int>(info->request->sequence()) ? 2 : 1;
> +
> +       firstRequest_ = false;
> +
>         bool hasYuv = false;
>         bool hasCapture = false;
>         /* Queue all buffers from the request aimed for the ImgU. */
> @@ -1431,33 +1442,53 @@ void IPU3CameraData::paramsBufferReady(unsigned
> int id)
>
>                 if (stream == &outStream_) {
>                         hasYuv = true;
> -                       imgu0_->output_->queueBuffer(outbuffer);
> +
> +                       for (int i = 0; i < yuvCount; ++i) {
> +                               bufferReturnCounters[outbuffer] += 1;
> +                               imgu0_->output_->queueBuffer(outbuffer);
> +                       }
>                 } else if (stream == &vfStream_) {
>                         hasYuv = true;
> -                       imgu0_->viewfinder_->queueBuffer(outbuffer);
> +
> +                       for (int i = 0; i < yuvCount; ++i) {
> +                               bufferReturnCounters[outbuffer] += 1;
> +
>  imgu0_->viewfinder_->queueBuffer(outbuffer);
> +                       }
>                 } else if (stream == &outCaptureStream_) {
>                         hasCapture = true;
>
> -                       imgu1_->output_->queueBuffer(outbuffer);
> +                       for (int i = 0; i < stillCount; ++i) {
> +                               bufferReturnCounters[outbuffer] += 1;
> +                               imgu1_->output_->queueBuffer(outbuffer);
> +                       }
>                 }
>         }
>
>         if (hasYuv) {
> -               bufferReturnCounters[info->rawBuffer] += 1;
> -
> -               imgu0_->param_->queueBuffer(info->paramBuffer);
> -               imgu0_->stat_->queueBuffer(info->statBuffer);
> -               imgu0_->input_->queueBuffer(info->rawBuffer);
> +               for (int i = 0; i < yuvCount; ++i) {
> +                       bufferReturnCounters[info->paramBuffer] += 1;
> +                       bufferReturnCounters[info->statBuffer] += 1;
> +                       bufferReturnCounters[info->rawBuffer] += 1;
> +
> +                       imgu0_->param_->queueBuffer(info->paramBuffer);
> +                       imgu0_->stat_->queueBuffer(info->statBuffer);
> +                       imgu0_->input_->queueBuffer(info->rawBuffer);
> +               }
>         } else {
>                 info->paramDequeued = true;
>                 info->metadataProcessed = true;
>         }
>
>         if (hasCapture) {
> -               bufferReturnCounters[info->rawBuffer] += 1;
> +               lastCaptureRequestSeq_ = info->request->sequence();
>
> -               imgu1_->param_->queueBuffer(info->captureParamBuffer);
> -               imgu1_->input_->queueBuffer(info->rawBuffer);
> +               for (int i = 0; i < stillCount; ++i) {
> +                       bufferReturnCounters[info->captureParamBuffer] +=
> 1;
> +                       bufferReturnCounters[info->rawBuffer] += 1;
> +
> +
>  imgu1_->param_->queueBuffer(info->captureParamBuffer);
> +                       imgu1_->input_->queueBuffer(info->rawBuffer);
> +               }
>         } else {
>                 info->captureParamDequeued = true;
>         }
> @@ -1498,6 +1529,11 @@ void IPU3CameraData::tryReturnBuffer(FrameBuffer
> *buffer)
>   */
>  void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
>  {
> +       if (--bufferReturnCounters[buffer] > 0)
> +               return;
> +
> +       bufferReturnCounters.erase(buffer);
> +
>         IPU3Frames::Info *info = frameInfos_.find(buffer);
>         if (!info)
>                 return;
> @@ -1564,6 +1600,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;
> @@ -1584,6 +1625,11 @@ void IPU3CameraData::paramBufferReady(FrameBuffer
> *buffer)
>
>  void IPU3CameraData::captureParamBufferReady(FrameBuffer *buffer)
>  {
> +       if (--bufferReturnCounters[buffer] > 0)
> +               return;
> +
> +       bufferReturnCounters.erase(buffer);
> +
>         IPU3Frames::Info *info = frameInfos_.find(buffer);
>         if (!info)
>                 return;
> @@ -1604,6 +1650,11 @@ void
> IPU3CameraData::captureParamBufferReady(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.37.1.455.g008518b4e5-goog
>
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index f8101c6c..36ca47c2 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -90,6 +90,9 @@  public:
 
 	ControlInfoMap ipaControls_;
 
+	bool firstRequest_ = true;
+	int lastCaptureRequestSeq_ = -1;
+
 private:
 	void metadataReady(unsigned int id, const ControlList &metadata);
 	void paramsBufferReady(unsigned int id);
@@ -884,6 +887,9 @@  int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis
 	CIO2Device *cio2 = &data->cio2_;
 	int ret;
 
+	data->firstRequest_ = true;
+	data->lastCaptureRequestSeq_ = -1;
+
 	imgu0_.input_->bufferReady.connect(data,
 					   &IPU3CameraData::tryReturnBuffer);
 	imgu0_.output_->bufferReady.connect(data,
@@ -1422,6 +1428,11 @@  void IPU3CameraData::paramsBufferReady(unsigned int id)
 	if (!info)
 		return;
 
+	int yuvCount = firstRequest_ ? 2 : 1;
+	int stillCount = firstRequest_ || (lastCaptureRequestSeq_ + 1) != static_cast<int>(info->request->sequence()) ? 2 : 1;
+
+	firstRequest_ = false;
+
 	bool hasYuv = false;
 	bool hasCapture = false;
 	/* Queue all buffers from the request aimed for the ImgU. */
@@ -1431,33 +1442,53 @@  void IPU3CameraData::paramsBufferReady(unsigned int id)
 
 		if (stream == &outStream_) {
 			hasYuv = true;
-			imgu0_->output_->queueBuffer(outbuffer);
+
+			for (int i = 0; i < yuvCount; ++i) {
+				bufferReturnCounters[outbuffer] += 1;
+				imgu0_->output_->queueBuffer(outbuffer);
+			}
 		} else if (stream == &vfStream_) {
 			hasYuv = true;
-			imgu0_->viewfinder_->queueBuffer(outbuffer);
+
+			for (int i = 0; i < yuvCount; ++i) {
+				bufferReturnCounters[outbuffer] += 1;
+				imgu0_->viewfinder_->queueBuffer(outbuffer);
+			}
 		} else if (stream == &outCaptureStream_) {
 			hasCapture = true;
 
-			imgu1_->output_->queueBuffer(outbuffer);
+			for (int i = 0; i < stillCount; ++i) {
+				bufferReturnCounters[outbuffer] += 1;
+				imgu1_->output_->queueBuffer(outbuffer);
+			}
 		}
 	}
 
 	if (hasYuv) {
-		bufferReturnCounters[info->rawBuffer] += 1;
-
-		imgu0_->param_->queueBuffer(info->paramBuffer);
-		imgu0_->stat_->queueBuffer(info->statBuffer);
-		imgu0_->input_->queueBuffer(info->rawBuffer);
+		for (int i = 0; i < yuvCount; ++i) {
+			bufferReturnCounters[info->paramBuffer] += 1;
+			bufferReturnCounters[info->statBuffer] += 1;
+			bufferReturnCounters[info->rawBuffer] += 1;
+
+			imgu0_->param_->queueBuffer(info->paramBuffer);
+			imgu0_->stat_->queueBuffer(info->statBuffer);
+			imgu0_->input_->queueBuffer(info->rawBuffer);
+		}
 	} else {
 		info->paramDequeued = true;
 		info->metadataProcessed = true;
 	}
 
 	if (hasCapture) {
-		bufferReturnCounters[info->rawBuffer] += 1;
+		lastCaptureRequestSeq_ = info->request->sequence();
 
-		imgu1_->param_->queueBuffer(info->captureParamBuffer);
-		imgu1_->input_->queueBuffer(info->rawBuffer);
+		for (int i = 0; i < stillCount; ++i) {
+			bufferReturnCounters[info->captureParamBuffer] += 1;
+			bufferReturnCounters[info->rawBuffer] += 1;
+
+			imgu1_->param_->queueBuffer(info->captureParamBuffer);
+			imgu1_->input_->queueBuffer(info->rawBuffer);
+		}
 	} else {
 		info->captureParamDequeued = true;
 	}
@@ -1498,6 +1529,11 @@  void IPU3CameraData::tryReturnBuffer(FrameBuffer *buffer)
  */
 void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
 {
+	if (--bufferReturnCounters[buffer] > 0)
+		return;
+
+	bufferReturnCounters.erase(buffer);
+
 	IPU3Frames::Info *info = frameInfos_.find(buffer);
 	if (!info)
 		return;
@@ -1564,6 +1600,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;
@@ -1584,6 +1625,11 @@  void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)
 
 void IPU3CameraData::captureParamBufferReady(FrameBuffer *buffer)
 {
+	if (--bufferReturnCounters[buffer] > 0)
+		return;
+
+	bufferReturnCounters.erase(buffer);
+
 	IPU3Frames::Info *info = frameInfos_.find(buffer);
 	if (!info)
 		return;
@@ -1604,6 +1650,11 @@  void IPU3CameraData::captureParamBufferReady(FrameBuffer *buffer)
 
 void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
 {
+	if (--bufferReturnCounters[buffer] > 0)
+		return;
+
+	bufferReturnCounters.erase(buffer);
+
 	IPU3Frames::Info *info = frameInfos_.find(buffer);
 	if (!info)
 		return;