[libcamera-devel] [PATCH v2 13/13] libcamera: pipeline: rkisp1: Expose self path stream

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Sep 14, 2020 at 04:21:49PM +0200, Niklas Söderlund wrote:
> Expose the self stream to applications and prefers it for the viewfinder
> and video roles as it can produce RGB. Keep preferring the main path for
> still capture as it could be extended to support RAW formats which makes
> most sens for still capture.

s/sens/sense/

> With this change the self path becomes available to applications and a
> camera backed by this pipeline can produce two streams simultaneously.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 44 ++++++++++++++++++------
>  1 file changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 851ff68f138b98dd..1260ed947d385ca9 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -729,17 +729,38 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera
>  	if (roles.empty())
>  		return config;
>  
> -	std::map<PixelFormat, std::vector<SizeRange>> streamFormats;
> -	for (const PixelFormat &format : RKISP1_RSZ_MP_FORMATS)
> -		streamFormats[format] =
> -			{ { RKISP1_RSZ_MP_SRC_MIN, data->sensor_->resolution() } };
> +	for (const StreamRole role : roles) {
> +		std::map<PixelFormat, std::vector<SizeRange>> streamFormats;
>  
> -	StreamFormats formats(streamFormats);
> -	StreamConfiguration cfg(formats);
> -	cfg.pixelFormat = formats::NV12;
> -	cfg.size = data->sensor_->resolution();
> +		switch (role) {
> +		case StreamRole::StillCapture: {
> +			for (const PixelFormat &format : RKISP1_RSZ_MP_FORMATS)
> +				streamFormats[format] =
> +					{ { RKISP1_RSZ_MP_SRC_MIN, data->sensor_->resolution() } };
> +			break;
> +		}
> +		case StreamRole::Viewfinder:
> +		case StreamRole::VideoRecording: {
> +			for (const PixelFormat &format : RKISP1_RSZ_SP_FORMATS)
> +				streamFormats[format] =
> +					{ { RKISP1_RSZ_SP_SRC_MIN, data->sensor_->resolution() } };
> +			break;
> +		}

If we get viewfinder + video we'll generate a configuration with stream
formats that won't be possible to produce simultaneously.

> +		default:
> +			LOG(RkISP1, Warning)
> +				<< "Requested stream role not supported: " << role;
> +			delete config;
> +			return nullptr;
> +		}
>  
> -	config->addConfiguration(cfg);
> +		StreamFormats formats(streamFormats);
> +		StreamConfiguration cfg(formats);
> +		cfg.pixelFormat = formats::NV12;
> +		cfg.size = data->sensor_->resolution();
> +		cfg.bufferCount = 4;
> +
> +		config->addConfiguration(cfg);
> +	}
>  
>  	config->validate();
>  
> @@ -1216,7 +1237,10 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  	if (ret)
>  		return ret;
>  
> -	std::set<Stream *> streams{ &data->mainPathStream_ };
> +	std::set<Stream *> streams{
> +		&data->mainPathStream_,
> +		&data->selfPathStream_,
> +	};
>  	std::shared_ptr<Camera> camera =
>  		Camera::create(this, data->sensor_->id(), streams);
>  	registerCamera(std::move(camera), std::move(data));

-- 
Regards,

Laurent Pinchart