[libcamera-devel] [PATCH v3 09/22] v4l2: v4l2_camera_proxy: Get stride and frameSize from stream config

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Sat, Jul 04, 2020 at 10:31:27PM +0900, Paul Elder wrote:
> The stride and frameSize should be obtained through StreamConfiguration
> rather than PixelFormatInfo, as pipeline handlers might have different
> values (eg. for alignment). Get the stride and frameSize values from
> StreamConfiguration instead of from PixelFormatInfo.
> 
> This also removes the need for V4L2CameraProxy::calculateSizeImage, so
> remove it.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>

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

> ---
> New in v3
> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 42 ++++------------------------------
>  src/v4l2/v4l2_camera_proxy.h   |  1 -
>  2 files changed, 5 insertions(+), 38 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 6a31415..9121d3d 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -71,7 +71,6 @@ int V4L2CameraProxy::open(V4L2CameraFile *file)
>  
>  	vcam_->getStreamConfig(&streamConfig_);
>  	setFmtFromConfig(streamConfig_);
> -	sizeimage_ = calculateSizeImage(streamConfig_);
>  
>  	files_.insert(file);
>  
> @@ -166,30 +165,20 @@ bool V4L2CameraProxy::validateMemoryType(uint32_t memory)
>  void V4L2CameraProxy::setFmtFromConfig(StreamConfiguration &streamConfig)
>  {
>  	const PixelFormatInfo &info = PixelFormatInfo::info(streamConfig.pixelFormat);
> -	Size size = streamConfig.size;
>  
> -	curV4L2Format_.fmt.pix.width        = size.width;
> -	curV4L2Format_.fmt.pix.height       = size.height;
> +	curV4L2Format_.fmt.pix.width        = streamConfig.size.width;
> +	curV4L2Format_.fmt.pix.height       = streamConfig.size.height;
>  	curV4L2Format_.fmt.pix.pixelformat  = info.v4l2Format;
>  	curV4L2Format_.fmt.pix.field        = V4L2_FIELD_NONE;
> -	curV4L2Format_.fmt.pix.bytesperline = info.stride(size.width, 0);
> -	curV4L2Format_.fmt.pix.sizeimage    = info.frameSize(size);
> +	curV4L2Format_.fmt.pix.bytesperline = streamConfig.stride;
> +	curV4L2Format_.fmt.pix.sizeimage    = streamConfig.frameSize;
>  	curV4L2Format_.fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;
>  	curV4L2Format_.fmt.pix.priv         = V4L2_PIX_FMT_PRIV_MAGIC;
>  	curV4L2Format_.fmt.pix.ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;
>  	curV4L2Format_.fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT;
>  	curV4L2Format_.fmt.pix.xfer_func    = V4L2_XFER_FUNC_DEFAULT;
> -}
> -
> -unsigned int V4L2CameraProxy::calculateSizeImage(StreamConfiguration &streamConfig)
> -{
> -	/*
> -	 * \todo Merge this method with setFmtFromConfig (need imageSize to
> -	 * support all libcamera formats first, or filter out MJPEG for now).
> -	 */
> -	const PixelFormatInfo &info = PixelFormatInfo::info(streamConfig.pixelFormat);
>  
> -	return info.frameSize(streamConfig.size);
> +	sizeimage_ = streamConfig.frameSize;
>  }
>  
>  void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera)
> @@ -359,12 +348,6 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
>  	if (ret < 0)
>  		return -EINVAL;
>  
> -	unsigned int sizeimage = calculateSizeImage(streamConfig_);
> -	if (sizeimage == 0)
> -		return -EINVAL;
> -
> -	sizeimage_ = sizeimage;
> -
>  	setFmtFromConfig(streamConfig_);
>  
>  	return 0;
> @@ -505,21 +488,6 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
>  	if (ret < 0)
>  		return -EINVAL;
>  
> -	sizeimage_ = calculateSizeImage(streamConfig_);
> -	/*
> -	 * If we return -EINVAL here then the application will think that we
> -	 * don't support streaming mmap. Since we don't support readwrite and
> -	 * userptr either, the application will get confused and think that
> -	 * we don't support anything.
> -	 * On the other hand, if the set format at the time of reqbufs has a
> -	 * zero sizeimage we'll get a floating point exception when we try to
> -	 * stream it.
> -	 */
> -	if (sizeimage_ == 0)
> -		LOG(V4L2Compat, Warning)
> -			<< "sizeimage of at least one format is zero. "
> -			<< "Streaming this format will cause a floating point exception.";
> -
>  	setFmtFromConfig(streamConfig_);
>  
>  	arg->count = streamConfig_.bufferCount;
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index 49184a1..e962694 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -40,7 +40,6 @@ private:
>  	bool validateBufferType(uint32_t type);
>  	bool validateMemoryType(uint32_t memory);
>  	void setFmtFromConfig(StreamConfiguration &streamConfig);
> -	unsigned int calculateSizeImage(StreamConfiguration &streamConfig);
>  	void querycap(std::shared_ptr<Camera> camera);
>  	void tryFormat(struct v4l2_format *arg);
>  	enum v4l2_priority maxPriority();

-- 
Regards,

Laurent Pinchart