[libcamera-devel] [PATCH 1/2] pipeline: raspberrypi: Fix under-allocation of embedded data buffers

[Naushir Patuck]

Hi David,

Thank you for your patch.

On Tue, 30 Nov 2021 at 11:23, David Plowman <david.plowman at raspberrypi.com>
wrote:

> The code was reducing the number of raw stream buffers allocated when
> the application is providing some of its own. However, it was not
> taking account of the fact that the application cannot supply embedded
> data buffers, so it must always allocate a reasonable minimum number
> of these buffers (possibly more than the number of raw stream buffers)
> to prevent frame drops.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
>

Reviewed-by: Naushir Patuck <naush at raspberrypi.com>

---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 25 ++++++++++++-------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index e31fa3b2..045725dd 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1248,18 +1248,25 @@ int PipelineHandlerRPi::prepareBuffers(Camera
> *camera)
>         /* Decide how many internal buffers to allocate. */
>         for (auto const stream : data->streams_) {
>                 unsigned int numBuffers;
> -
> -               if (stream == &data->unicam_[Unicam::Image] ||
> -                   stream == &data->unicam_[Unicam::Embedded]) {
> +               /*
> +                * For Unicam, allocate a minimum of 4 buffers as we want
> +                * to avoid any frame drops.
> +                */
> +               constexpr unsigned int minBuffers = 4;
> +               if (stream == &data->unicam_[Unicam::Image]) {
>                         /*
> -                        * For Unicam, allocate a minimum of 4 buffers as
> we want
> -                        * to avoid any frame drops. If an application has
> configured
> -                        * a RAW stream, allocate additional buffers to
> make up the
> -                        * minimum, but ensure we have at least 2 sets of
> internal
> -                        * buffers to use to minimise frame drops.
> +                        * If an application has configured a RAW stream,
> allocate
> +                        * additional buffers to make up the minimum, but
> ensure
> +                        * we have at least 2 sets of internal buffers to
> use to
> +                        * minimise frame drops.
>                          */
> -                       constexpr unsigned int minBuffers = 4;
>                         numBuffers = std::max<int>(2, minBuffers -
> numRawBuffers);
> +               } else if (stream == &data->unicam_[Unicam::Embedded]) {
> +                       /*
> +                        * Embedded data buffers are (currently) for
> internal use,
> +                        * so allocate the minimum required to avoid frame
> drops.
> +                        */
> +                       numBuffers = minBuffers;
>                 } else {
>                         /*
>                          * Since the ISP runs synchronous with the IPA and
> requests,
> --
> 2.30.2
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211201/323f51f1/attachment-0001.htm>