[libcamera-devel] [PATCH v4 9/9] ipu3: Fixes frame delay

[Cheng-Hao Yang]

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 at 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 at 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220803/3eb6afc3/attachment.htm>