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

Message ID 20200914142149.63857-14-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: pipeline: rkisp1: Extend to support two streams
Related show

Commit Message

Niklas Söderlund Sept. 14, 2020, 2:21 p.m. UTC
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.

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@ragnatech.se>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 44 ++++++++++++++++++------
 1 file changed, 34 insertions(+), 10 deletions(-)

Comments

Laurent Pinchart Sept. 15, 2020, 1:35 a.m. UTC | #1
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@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));

Patch

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;
+		}
+		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));