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

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Wed, Jan 18, 2023 at 08:59:49AM +0000, Naushir Patuck via libcamera-devel wrote:
> 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>
> ---
>  .../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;

One variable per line.

>  	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;

Do you plan to also optimize the buffer allocation based on hints for
Output1 ?

>  		}

-- 
Regards,

Laurent Pinchart