[libcamera-devel] [PATCH] pipeline: raspberrypi: Improve image/embedded buffer matching logic

[Naushir Patuck]

Hi,

Any takers for another review on this change please?

Regards,
Naush

On Wed, 13 Jul 2022 at 13:00, Naushir Patuck <naush at raspberrypi.com> wrote:

> The logic used to match asynchronous image and embedded buffers was being
> overly
> aggressive by possibly allowing an unmatched image buffer to be sent to
> the IPA
> if the matching embedded buffer had not yet been dequeued. This condition
> only
> occurs when the system is heavily loaded and dropping frames.
>
> Fix this by holding image buffer in the queue during these conditions
> until the
> next embedded buffer dequeue event.
>
> Reported-by: Nick Hollinghurst <nick.hollinghurst at raspberrypi.com>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 66a84b1dfb97..ef3c2d11d253 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -2159,16 +2159,12 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame
> &bayerFrame, FrameBuffer *&em
>         if (bayerQueue_.empty())
>                 return false;
>
> -       /* Start with the front of the bayer queue. */
> -       bayerFrame = std::move(bayerQueue_.front());
> -       bayerQueue_.pop();
> -
>         /*
>          * Find the embedded data buffer with a matching timestamp to pass
> to
>          * the IPA. Any embedded buffers with a timestamp lower than the
>          * current bayer buffer will be removed and re-queued to the
> driver.
>          */
> -       uint64_t ts = bayerFrame.buffer->metadata().timestamp;
> +       uint64_t ts = bayerQueue_.front().buffer->metadata().timestamp;
>         embeddedBuffer = nullptr;
>         while (!embeddedQueue_.empty()) {
>                 FrameBuffer *b = embeddedQueue_.front();
> @@ -2188,10 +2184,23 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame
> &bayerFrame, FrameBuffer *&em
>         }
>
>         if (!embeddedBuffer && sensorMetadata_) {
> +               if (embeddedQueue_.empty()) {
> +                       /*
> +                        * If the embedded buffer queue is empty, wait for
> the next
> +                        * buffer to arrive - dequeue ordering may send
> the image
> +                        * buffer first.
> +                        */
> +                       LOG(RPI, Debug) << "Waiting for next embedded
> buffer.";
> +                       return false;
> +               }
> +
>                 /* Log if there is no matching embedded data buffer found.
> */
>                 LOG(RPI, Debug) << "Returning bayer frame without a
> matching embedded buffer.";
>         }
>
> +       bayerFrame = std::move(bayerQueue_.front());
> +       bayerQueue_.pop();
> +
>         return true;
>  }
>
> --
> 2.25.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220721/05d0daad/attachment.htm>