[libcamera-devel] [PATCH] libcamera: pipeline: raspberrypi: Add StreamFormats to StreamConfiguration

[Jacopo Mondi]

Hi Naush,
   sorry for the delay, this fell through the cracks

On Wed, Jun 10, 2020 at 03:26:40PM +0100, Naushir Patuck wrote:
> In generateConfiguration(), add the device node specific formats to the
> StreamConfiguration for each StreamRole requested.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 52 +++++++++++++------
>  1 file changed, 36 insertions(+), 16 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index e16a9c7f..03a1e641 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -518,41 +518,45 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>  	RPiCameraData *data = cameraData(camera);
>  	CameraConfiguration *config = new RPiCameraConfiguration(data);
>  	V4L2DeviceFormat sensorFormat;
> +	unsigned int bufferCount;
> +	PixelFormat pixelFormat;
>  	V4L2PixFmtMap fmts;
> +	Size size;
>
>  	if (roles.empty())
>  		return config;
>
>  	for (const StreamRole role : roles) {
> -		StreamConfiguration cfg{};
> -
>  		switch (role) {
>  		case StreamRole::StillCaptureRaw:
> -			cfg.size = data->sensor_->resolution();
> +			size = data->sensor_->resolution();
>  			fmts = data->unicam_[Unicam::Image].dev()->formats();
> -			sensorFormat = findBestMode(fmts, cfg.size);
> -			cfg.pixelFormat = sensorFormat.fourcc.toPixelFormat();
> -			ASSERT(cfg.pixelFormat.isValid());
> -			cfg.bufferCount = 1;
> +			sensorFormat = findBestMode(fmts, size);
> +			pixelFormat = sensorFormat.fourcc.toPixelFormat();
> +			ASSERT(pixelFormat.isValid());
> +			bufferCount = 1;
>  			break;
>
>  		case StreamRole::StillCapture:
> -			cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);
> +			fmts = data->isp_[Isp::Output0].dev()->formats();
> +			pixelFormat = PixelFormat(DRM_FORMAT_NV12);
>  			/* Return the largest sensor resolution. */
> -			cfg.size = data->sensor_->resolution();
> -			cfg.bufferCount = 1;
> +			size = data->sensor_->resolution();
> +			bufferCount = 1;
>  			break;
>
>  		case StreamRole::VideoRecording:
> -			cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);
> -			cfg.size = { 1920, 1080 };
> -			cfg.bufferCount = 4;
> +			fmts = data->isp_[Isp::Output0].dev()->formats();
> +			pixelFormat = PixelFormat(DRM_FORMAT_NV12);
> +			size = { 1920, 1080 };
> +			bufferCount = 4;
>  			break;
>
>  		case StreamRole::Viewfinder:
> -			cfg.pixelFormat = PixelFormat(DRM_FORMAT_ARGB8888);
> -			cfg.size = { 800, 600 };
> -			cfg.bufferCount = 4;
> +			fmts = data->isp_[Isp::Output0].dev()->formats();
> +			pixelFormat = PixelFormat(DRM_FORMAT_ARGB8888);
> +			size = { 800, 600 };
> +			bufferCount = 4;
>  			break;
>
>  		default:
> @@ -561,6 +565,22 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>  			break;
>  		}
>
> +		/* Translate the V4L2PixelFormat to PixelFormat. */
> +		std::map<PixelFormat, std::vector<SizeRange>> deviceFormats;
> +		std::transform(fmts.begin(), fmts.end(), std::inserter(deviceFormats, deviceFormats.begin()),
> +			       [&](const decltype(fmts)::value_type &format) {
> +					return decltype(deviceFormats)::value_type{
> +						format.first.toPixelFormat(),
> +						format.second
> +					};
> +			       });

Really took me a while to parse this, as I was not expecting to see
std::inserter() but just deviceFormats.begin() there, but then I
learned about inserter, and indeed if I remove std::inserter() I get
an error due to the fact the copy operator of std::pair is deleted.

> +
> +		/* Add the stream format based on the device node used for the use case. */
> +		StreamFormats formats(deviceFormats);
> +		StreamConfiguration cfg(formats);
> +		cfg.size = size;
> +		cfg.pixelFormat = pixelFormat;
> +		cfg.bufferCount = bufferCount;

Patch looks good to me

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>  		config->addConfiguration(cfg);
>  	}
>
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel