[libcamera-devel] [PATCH v5 08/12] pipeline: raspberrypi: Handle OptionalStream hints for ISP Output0

[Kieran Bingham]

Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:49)
> Look for OptionalStream flag in the hints field of the ISP Output0
> StreamConfiguration structure. If this flag is not set, it guarantees that the
> application will provide buffers for the ISP, do not allocate any internal
> buffers for the device.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>

If this is the chosen orientation of the hint, then:

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

> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 29 ++++++++++++++-----
>  1 file changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 2b396f1db9b6..13d0ab4c4968 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1453,7 +1453,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
>         RPiCameraData *data = cameraData(camera);
>         unsigned int minUnicamBuffers = data->config_.minUnicamBuffers,
>                      minTotalUnicamBuffers = data->config_.minTotalUnicamBuffers,
> -                    numRawBuffers = 0;
> +                    numRawBuffers = 0, minIspBuffers = 1;
>         int ret;
>  
>         for (Stream *s : camera->streams()) {
> @@ -1477,8 +1477,21 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
>                                         minTotalUnicamBuffers = 0;
>                                 }
>                         }
> -
> -                       break;
> +               } else if (s == &data->isp_[Isp::Output0]) {
> +                       /*
> +                        * Since the ISP runs synchronous with the IPA and requests,
> +                        * we only ever need a maximum of one internal buffer. Any
> +                        * buffers the application wants to hold onto will already
> +                        * be exported through PipelineHandlerRPi::exportFrameBuffers().
> +                        *
> +                        * However, as above, if the application provides a guarantee
> +                        * that the buffer will always be provided for the ISP Output0
> +                        * stream in a Request, we don't need any internal buffers
> +                        * allocated.
> +                        */
> +                       if (!(s->configuration().hints & StreamConfiguration::Hint::OptionalStream) &&
> +                           !data->dropFrameCount_)
> +                               minIspBuffers = 0;
>                 }
>         }
>  
> @@ -1514,12 +1527,14 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
>                          * so allocate the minimum required to avoid frame drops.
>                          */
>                         numBuffers = data->config_.minTotalUnicamBuffers;
> +               } else if (stream == &data->isp_[Isp::Output0]) {
> +                       /* Buffer count for this is handled in the earlier loop above. */
> +                       numBuffers = minIspBuffers;
>                 } else {
>                         /*
> -                        * Since the ISP runs synchronous with the IPA and requests,
> -                        * we only ever need one set of internal buffers. Any buffers
> -                        * the application wants to hold onto will already be exported
> -                        * through PipelineHandlerRPi::exportFrameBuffers().
> +                        * Same reasoning as for ISP Output 0, we only ever need
> +                        * a maximum of one internal buffer for Output1 (required
> +                        * for colour denoise) and ISP statistics.
>                          */
>                         numBuffers = 1;
>                 }
> -- 
> 2.25.1
>