[libcamera-devel] [PATCH v1 1/1] libcamera: Fix ipu3's issue of first black frame

[Cheng-Hao Yang]

Hi folks,

Could I request a round of review? Thanks!

On Mon, Sep 18, 2023 at 4:52 PM Harvey Yang <chenghaoyang at 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 at 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20231115/2b5e9e89/attachment.htm>