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

[Kieran Bingham]

Quoting David Plowman (2021-11-30 11:22:58)
> 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>
> ---
>  .../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;

Ouch, yes I see how this would have bitten.


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

>                 } else {
>                         /*
>                          * Since the ISP runs synchronous with the IPA and requests,
> -- 
> 2.30.2
>