[libcamera-devel] [PATCH 3/4] libcamera: pipeline: vimc: enforce stream configuration

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Thu, Feb 21, 2019 at 12:59:38AM +0100, Niklas Söderlund wrote:
> The format requested by configureStreams() should exactly match what
> programmed on the video device it self. If they do not match the call
> should fail as the application in that case will not know what
> configuration actually was programmed.

Same comments as for 2/4, but in any case,

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

for 2/4 and 3/4 (I forgot to add it to my reply to 2/4)

> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/vimc.cpp | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index a93a7589643834c6..812777cffbb3e618 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -83,6 +83,7 @@ int PipelineHandlerVimc::configureStreams(Camera *camera,
>  				      std::map<Stream *, StreamConfiguration> &config)
>  {
>  	StreamConfiguration *cfg = &config[&stream_];
> +	int ret;
>  
>  	LOG(VIMC, Debug) << "Configure the camera for resolution "
>  			 << cfg->width << "x" << cfg->height;
> @@ -92,7 +93,16 @@ int PipelineHandlerVimc::configureStreams(Camera *camera,
>  	format.height = cfg->height;
>  	format.fourcc = cfg->pixelFormat;
>  
> -	return video_->setFormat(&format);
> +	ret = video_->setFormat(&format);
> +	if (ret)
> +		return ret;
> +
> +	if (format.width != cfg->width ||
> +	    format.height != cfg->height ||
> +	    format.fourcc != cfg->pixelFormat)
> +		return -EINVAL;
> +
> +	return 0;
>  }
>  
>  int PipelineHandlerVimc::allocateBuffers(Camera *camera, Stream *stream)

-- 
Regards,

Laurent Pinchart