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

8630 diff mbox series

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@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(-)

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@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@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();