[libcamera-devel] [PATCH 2/2] pipeline: raspberrypi: Use a default format for ISP::Output0

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Sat, Mar 13, 2021 at 10:27:41AM +0000, Naushir Patuck wrote:
> If the ISP::Output1 stream has not been configured, we must enable it

Did you mean Output0 ?

> with a default format and resolution for internal use. This is to allow
> the pipeline handler data flow to be consistent, and allow the IPA to
> receive statistics for the frame.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 30 ++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 9847e926b048..5cc04ce4eb92 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -625,7 +625,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>  	 * StreamConfiguration appropriately.
>  	 */
>  	V4L2DeviceFormat format;
> -	bool output1Set = false;
> +	bool output0Set = false, output1Set = false;
>  	for (unsigned i = 0; i < config->size(); i++) {
>  		StreamConfiguration &cfg = config->at(i);
>  
> @@ -662,6 +662,34 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>  
>  		if (i != maxIndex)
>  			output1Set = true;
> +		else
> +			output0Set = true;
> +	}
> +
> +	/*
> +	 * If ISP::Output0 stream has not been configured by the application,
> +	 * we must allow the hardware to generate an output so that the data
> +	 * flow in the pipeline handler remains consistent, and we still generate
> +	 * statistics for the IPA to use. So enable the output at a very low
> +	 * resolution for internal use.
> +	 *
> +	 * \todo Allow the pipeline to work correctly without Output0 and only
> +	 * statistics coming from the hardware.
> +	 */
> +	if (!output0Set) {
> +		maxSize = Size(320, 240);
> +		format = {};
> +		format.size = maxSize;
> +		format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420, false);
> +		ret = data->isp_[Isp::Output0].dev()->setFormat(&format);
> +		if (ret) {
> +			LOG(RPI, Error) << "Failed to set default format on ISP Output0: "
> +					<< ret;

Minor comment, to shorten the lines,

			LOG(RPI, Error)
				<< "Failed to set default format on ISP Output0: "
				<< ret;

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I'll apply this after a confirmation that the commit message should
mention Ouput0.

> +			return -EINVAL;
> +		}
> +
> +		LOG(RPI, Debug) << "Defaulting ISP Output0 format to "
> +				<< format.toString();
>  	}
>  
>  	/*

-- 
Regards,

Laurent Pinchart