[libcamera-devel] [PATCH 14/20] libcamera: pipeline: simple: Cache pipeline config in SimpleCameraConfiguration

[Kieran Bingham]

On 31/01/2021 22:46, Laurent Pinchart wrote:
> As the pipeline configuration is selected in
> SimpleCameraConfiguration::validate() already, cache it in the
> SimpleCameraConfiguration instead of looking it up in
> SimplePipelineHandler::configure(). This makes little difference at the
> moment, but will save duplication of more complex logic between
> validate() and configure() when adding support for multiple streams.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  src/libcamera/pipeline/simple/simple.cpp | 38 +++++++++++++-----------
>  1 file changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 72a18cc0b97f..c44fa9ee28ed 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -177,6 +177,11 @@ public:
>  
>  	Status validate() override;
>  
> +	const SimpleCameraData::Configuration *pipeConfig() const
> +	{
> +		return pipeConfig_;
> +	}
> +
>  	bool needConversion() const { return needConversion_; }
>  
>  private:
> @@ -188,6 +193,7 @@ private:
>  	std::shared_ptr<Camera> camera_;
>  	const SimpleCameraData *data_;
>  
> +	const SimpleCameraData::Configuration *pipeConfig_;
>  	bool needConversion_;
>  };
>  
> @@ -506,7 +512,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
>  SimpleCameraConfiguration::SimpleCameraConfiguration(Camera *camera,
>  						     SimpleCameraData *data)
>  	: CameraConfiguration(), camera_(camera->shared_from_this()),
> -	  data_(data)
> +	  data_(data), pipeConfig_(nullptr)
>  {
>  }
>  
> @@ -542,17 +548,17 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
>  		status = Adjusted;
>  	}
>  
> -	const SimpleCameraData::Configuration &pipeConfig = it->second;
> -	if (!pipeConfig.outputSizes.contains(cfg.size)) {
> +	pipeConfig_ = &it->second;
> +	if (!pipeConfig_->outputSizes.contains(cfg.size)) {
>  		LOG(SimplePipeline, Debug)
>  			<< "Adjusting size from " << cfg.size.toString()
> -			<< " to " << pipeConfig.captureSize.toString();
> -		cfg.size = pipeConfig.captureSize;
> +			<< " to " << pipeConfig_->captureSize.toString();
> +		cfg.size = pipeConfig_->captureSize;
>  		status = Adjusted;
>  	}
>  
> -	needConversion_ = cfg.pixelFormat != pipeConfig.captureFormat
> -			|| cfg.size != pipeConfig.captureSize;
> +	needConversion_ = cfg.pixelFormat != pipeConfig_->captureFormat
> +			|| cfg.size != pipeConfig_->captureSize;
>  
>  	cfg.bufferCount = 3;
>  
> @@ -646,21 +652,19 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  	if (ret < 0)
>  		return ret;
>  
> -	const SimpleCameraData::Configuration &pipeConfig =
> -		data->formats_[cfg.pixelFormat];
> -
> -	V4L2SubdeviceFormat format{ pipeConfig.code, data->sensor_->resolution() };
> +	const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig();
> +	V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() };
>  
>  	ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);
>  	if (ret < 0)
>  		return ret;
>  
>  	/* Configure the video node. */
> -	V4L2PixelFormat videoFormat = video->toV4L2PixelFormat(pipeConfig.captureFormat);
> +	V4L2PixelFormat videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat);
>  
>  	V4L2DeviceFormat captureFormat;
>  	captureFormat.fourcc = videoFormat;
> -	captureFormat.size = pipeConfig.captureSize;
> +	captureFormat.size = pipeConfig->captureSize;
>  
>  	ret = video->setFormat(&captureFormat);
>  	if (ret)
> @@ -673,10 +677,10 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  	}
>  
>  	if (captureFormat.fourcc != videoFormat ||
> -	    captureFormat.size != pipeConfig.captureSize) {
> +	    captureFormat.size != pipeConfig->captureSize) {
>  		LOG(SimplePipeline, Error)
>  			<< "Unable to configure capture in "
> -			<< pipeConfig.captureSize.toString() << "-"
> +			<< pipeConfig->captureSize.toString() << "-"
>  			<< videoFormat.toString();
>  		return -EINVAL;
>  	}
> @@ -686,8 +690,8 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  
>  	if (useConverter_) {
>  		StreamConfiguration inputCfg;
> -		inputCfg.pixelFormat = pipeConfig.captureFormat;
> -		inputCfg.size = pipeConfig.captureSize;
> +		inputCfg.pixelFormat = pipeConfig->captureFormat;
> +		inputCfg.size = pipeConfig->captureSize;
>  		inputCfg.stride = captureFormat.planes[0].bpl;
>  		inputCfg.bufferCount = cfg.bufferCount;
>  
> 

-- 
Regards
--
Kieran