[libcamera-devel] [PATCH v4 17/21] libcamera: simple: Fill stride and frameSize at config validation

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Wed, Jul 08, 2020 at 10:44:13PM +0900, Paul Elder wrote:
> Fill the stride and frameSize fields of the StreamConfiguration at
> configuration validation time instead of at camera configuration time.
> This allows applications to get the stride when trying a configuration
> without modifying the active configuration of the camera.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Changes in v4:
> - fix converter's stride and frameSize (get it via tryFormat instead of
>   calculation)
> - merge converter's stride and frameSize to one function
> - return error if tryFormat fails
> - mention motivation in commit message
> 
> New in v3
> ---
>  src/libcamera/pipeline/simple/converter.cpp | 19 +++++++++++++++
>  src/libcamera/pipeline/simple/converter.h   |  4 +++
>  src/libcamera/pipeline/simple/simple.cpp    | 27 +++++++++++++++++++--
>  3 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> index e5e2f0f..cef1503 100644
> --- a/src/libcamera/pipeline/simple/converter.cpp
> +++ b/src/libcamera/pipeline/simple/converter.cpp
> @@ -261,4 +261,23 @@ void SimpleConverter::outputBufferReady(FrameBuffer *buffer)
>  	}
>  }
>  
> +int SimpleConverter::strideAndFrameSize(const Size &size,
> +					const PixelFormat &pixelFormat,
> +					unsigned int *strideOut,
> +					unsigned int *frameSizeOut)
> +{
> +	V4L2DeviceFormat format = {};
> +	format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat);
> +	format.size = size;
> +
> +	int ret = m2m_->capture()->tryFormat(&format);
> +	if (ret < 0)
> +		return -1;
> +
> +	*strideOut = format.planes[0].bpl;
> +	*frameSizeOut = format.planes[0].size;
> +
> +	return 0;
> +}
> +
>  } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
> index ef18cf7..3e46988 100644
> --- a/src/libcamera/pipeline/simple/converter.h
> +++ b/src/libcamera/pipeline/simple/converter.h
> @@ -46,6 +46,10 @@ public:
>  
>  	int queueBuffers(FrameBuffer *input, FrameBuffer *output);
>  
> +	int strideAndFrameSize(const Size &size, const PixelFormat &pixelFormat,
> +			       unsigned int *strideOut,
> +			       unsigned int *frameSizeOut);
> +
>  	Signal<FrameBuffer *, FrameBuffer *> bufferReady;
>  
>  private:
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 1ec8d0f..0c52b47 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -457,6 +457,31 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
>  
>  	cfg.bufferCount = 3;
>  
> +

One blank line is enough.

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

> +	/* Set the stride and frameSize. */
> +	if (!needConversion_) {
> +		V4L2DeviceFormat format = {};
> +		format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
> +		format.size = cfg.size;
> +
> +		int ret = data_->video_->tryFormat(&format);
> +		if (ret < 0)
> +			return Invalid;
> +
> +		cfg.stride = format.planes[0].bpl;
> +		cfg.frameSize = format.planes[0].size;
> +
> +		return status;
> +	}
> +
> +	SimplePipelineHandler *pipe = static_cast<SimplePipelineHandler *>(data_->pipe_);
> +	SimpleConverter *converter = pipe->converter();
> +
> +	int ret = converter->strideAndFrameSize(cfg.size, cfg.pixelFormat,
> +						&cfg.stride, &cfg.frameSize);
> +	if (ret < 0)
> +		return Invalid;
> +
>  	return status;
>  }
>  
> @@ -557,8 +582,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  		return -EINVAL;
>  	}
>  
> -	cfg.stride = captureFormat.planes[0].bpl;
> -
>  	/* Configure the converter if required. */
>  	useConverter_ = config->needConversion();
>  

-- 
Regards,

Laurent Pinchart