[libcamera-devel,2/3] v4l2: camera_proxy: Store v4l2_pix_format instead of v4l2_format
diff mbox series

Message ID 20200822134605.17666-3-laurent.pinchart@ideasonboard.com
State Accepted
Commit 852b9d0a7636fd7fa34cb424a9992642af4826b9
Headers show
Series
  • v4l2: Miscellaneous cleanups
Related show

Commit Message

Laurent Pinchart Aug. 22, 2020, 1:46 p.m. UTC 
The V4L2 compatibility layer only uses the fmt.pix field of
curV4L2Format_. There's no need to cache the full v4l2_format, store
v4l2_pix_format only and rename the member variable from curV4L2Format_
to v4l2PixFormat_. While at it, group the V4L2-related member variables
together in the V4L2CameraProxy class.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/v4l2/v4l2_camera_proxy.cpp | 32 ++++++++++++++++----------------
 src/v4l2/v4l2_camera_proxy.h   |  5 +++--
 2 files changed, 19 insertions(+), 18 deletions(-)

Comments

Kieran Bingham Aug. 24, 2020, 9:35 a.m. UTC | #1 
Hi Laurent,

On 22/08/2020 14:46, Laurent Pinchart wrote:
> The V4L2 compatibility layer only uses the fmt.pix field of
> curV4L2Format_. There's no need to cache the full v4l2_format, store
> v4l2_pix_format only and rename the member variable from curV4L2Format_
> to v4l2PixFormat_. While at it, group the V4L2-related member variables
> together in the V4L2CameraProxy class.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 32 ++++++++++++++++----------------
>  src/v4l2/v4l2_camera_proxy.h   |  5 +++--
>  2 files changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 079257f4bf22..80287d8f164e 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -167,17 +167,17 @@ void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)
>  	const PixelFormatInfo &info = PixelFormatInfo::info(streamConfig.pixelFormat);
>  	const Size &size = streamConfig.size;
>  
> -	curV4L2Format_.fmt.pix.width        = size.width;
> -	curV4L2Format_.fmt.pix.height       = size.height;
> -	curV4L2Format_.fmt.pix.pixelformat  = info.v4l2Format;
> -	curV4L2Format_.fmt.pix.field        = V4L2_FIELD_NONE;
> -	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;
> +	v4l2PixFormat_.width        = size.width;
> +	v4l2PixFormat_.height       = size.height;
> +	v4l2PixFormat_.pixelformat  = info.v4l2Format;
> +	v4l2PixFormat_.field        = V4L2_FIELD_NONE;
> +	v4l2PixFormat_.bytesperline = streamConfig.stride;
> +	v4l2PixFormat_.sizeimage    = streamConfig.frameSize;
> +	v4l2PixFormat_.colorspace   = V4L2_COLORSPACE_SRGB;
> +	v4l2PixFormat_.priv         = V4L2_PIX_FMT_PRIV_MAGIC;
> +	v4l2PixFormat_.ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;
> +	v4l2PixFormat_.quantization = V4L2_QUANTIZATION_DEFAULT;
> +	v4l2PixFormat_.xfer_func    = V4L2_XFER_FUNC_DEFAULT;
>  
>  	sizeimage_ = streamConfig.frameSize;
>  }
> @@ -291,7 +291,7 @@ int V4L2CameraProxy::vidioc_g_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
>  		return -EINVAL;
>  
>  	memset(&arg->fmt, 0, sizeof(arg->fmt));
> -	arg->fmt.pix = curV4L2Format_.fmt.pix;
> +	arg->fmt.pix = v4l2PixFormat_;
>  
>  	return 0;
>  }
> @@ -488,8 +488,8 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
>  	if (bufferCount_ > 0)
>  		freeBuffers();
>  
> -	Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);
> -	V4L2PixelFormat v4l2Format = V4L2PixelFormat(curV4L2Format_.fmt.pix.pixelformat);
> +	Size size(v4l2PixFormat_.width, v4l2PixFormat_.height);
> +	V4L2PixelFormat v4l2Format = V4L2PixelFormat(v4l2PixFormat_.pixelformat);
>  	int ret = vcam_->configure(&streamConfig_, size,
>  				   PixelFormatInfo::info(v4l2Format).format,
>  				   arg->count);
> @@ -511,9 +511,9 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
>  	for (unsigned int i = 0; i < arg->count; i++) {
>  		struct v4l2_buffer buf = {};
>  		buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> -		buf.length = curV4L2Format_.fmt.pix.sizeimage;
> +		buf.length = v4l2PixFormat_.sizeimage;
>  		buf.memory = V4L2_MEMORY_MMAP;
> -		buf.m.offset = i * curV4L2Format_.fmt.pix.sizeimage;
> +		buf.m.offset = i * v4l2PixFormat_.sizeimage;
>  		buf.index = i;
>  		buf.flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index 79eb87bcea51..92a79abb9e21 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -73,13 +73,14 @@ private:
>  	unsigned int refcount_;
>  	unsigned int index_;
>  
> -	struct v4l2_format curV4L2Format_;
>  	StreamConfiguration streamConfig_;
> -	struct v4l2_capability capabilities_;
>  	unsigned int bufferCount_;
>  	unsigned int currentBuf_;
>  	unsigned int sizeimage_;
>  
> +	struct v4l2_capability capabilities_;
> +	struct v4l2_pix_format v4l2PixFormat_;
> +
>  	std::vector<struct v4l2_buffer> buffers_;
>  	std::map<void *, unsigned int> mmaps_;
>  
>
Paul Elder Aug. 25, 2020, 3:20 a.m. UTC | #2 
Hi Laurent,

On Sat, Aug 22, 2020 at 04:46:04PM +0300, Laurent Pinchart wrote:
> The V4L2 compatibility layer only uses the fmt.pix field of
> curV4L2Format_. There's no need to cache the full v4l2_format, store
> v4l2_pix_format only and rename the member variable from curV4L2Format_
> to v4l2PixFormat_. While at it, group the V4L2-related member variables
> together in the V4L2CameraProxy class.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 32 ++++++++++++++++----------------
>  src/v4l2/v4l2_camera_proxy.h   |  5 +++--
>  2 files changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 079257f4bf22..80287d8f164e 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -167,17 +167,17 @@ void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)
>  	const PixelFormatInfo &info = PixelFormatInfo::info(streamConfig.pixelFormat);
>  	const Size &size = streamConfig.size;
>  
> -	curV4L2Format_.fmt.pix.width        = size.width;
> -	curV4L2Format_.fmt.pix.height       = size.height;
> -	curV4L2Format_.fmt.pix.pixelformat  = info.v4l2Format;
> -	curV4L2Format_.fmt.pix.field        = V4L2_FIELD_NONE;
> -	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;
> +	v4l2PixFormat_.width        = size.width;
> +	v4l2PixFormat_.height       = size.height;
> +	v4l2PixFormat_.pixelformat  = info.v4l2Format;
> +	v4l2PixFormat_.field        = V4L2_FIELD_NONE;
> +	v4l2PixFormat_.bytesperline = streamConfig.stride;
> +	v4l2PixFormat_.sizeimage    = streamConfig.frameSize;
> +	v4l2PixFormat_.colorspace   = V4L2_COLORSPACE_SRGB;
> +	v4l2PixFormat_.priv         = V4L2_PIX_FMT_PRIV_MAGIC;
> +	v4l2PixFormat_.ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;
> +	v4l2PixFormat_.quantization = V4L2_QUANTIZATION_DEFAULT;
> +	v4l2PixFormat_.xfer_func    = V4L2_XFER_FUNC_DEFAULT;
>  
>  	sizeimage_ = streamConfig.frameSize;
>  }
> @@ -291,7 +291,7 @@ int V4L2CameraProxy::vidioc_g_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
>  		return -EINVAL;
>  
>  	memset(&arg->fmt, 0, sizeof(arg->fmt));
> -	arg->fmt.pix = curV4L2Format_.fmt.pix;
> +	arg->fmt.pix = v4l2PixFormat_;
>  
>  	return 0;
>  }
> @@ -488,8 +488,8 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
>  	if (bufferCount_ > 0)
>  		freeBuffers();
>  
> -	Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);
> -	V4L2PixelFormat v4l2Format = V4L2PixelFormat(curV4L2Format_.fmt.pix.pixelformat);
> +	Size size(v4l2PixFormat_.width, v4l2PixFormat_.height);
> +	V4L2PixelFormat v4l2Format = V4L2PixelFormat(v4l2PixFormat_.pixelformat);
>  	int ret = vcam_->configure(&streamConfig_, size,
>  				   PixelFormatInfo::info(v4l2Format).format,
>  				   arg->count);
> @@ -511,9 +511,9 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
>  	for (unsigned int i = 0; i < arg->count; i++) {
>  		struct v4l2_buffer buf = {};
>  		buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> -		buf.length = curV4L2Format_.fmt.pix.sizeimage;
> +		buf.length = v4l2PixFormat_.sizeimage;
>  		buf.memory = V4L2_MEMORY_MMAP;
> -		buf.m.offset = i * curV4L2Format_.fmt.pix.sizeimage;
> +		buf.m.offset = i * v4l2PixFormat_.sizeimage;
>  		buf.index = i;
>  		buf.flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index 79eb87bcea51..92a79abb9e21 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -73,13 +73,14 @@ private:
>  	unsigned int refcount_;
>  	unsigned int index_;
>  
> -	struct v4l2_format curV4L2Format_;
>  	StreamConfiguration streamConfig_;
> -	struct v4l2_capability capabilities_;
>  	unsigned int bufferCount_;
>  	unsigned int currentBuf_;
>  	unsigned int sizeimage_;
>  
> +	struct v4l2_capability capabilities_;
> +	struct v4l2_pix_format v4l2PixFormat_;
> +
>  	std::vector<struct v4l2_buffer> buffers_;
>  	std::map<void *, unsigned int> mmaps_;
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
index 079257f4bf22..80287d8f164e 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -167,17 +167,17 @@  void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)
 	const PixelFormatInfo &info = PixelFormatInfo::info(streamConfig.pixelFormat);
 	const Size &size = streamConfig.size;
 
-	curV4L2Format_.fmt.pix.width        = size.width;
-	curV4L2Format_.fmt.pix.height       = size.height;
-	curV4L2Format_.fmt.pix.pixelformat  = info.v4l2Format;
-	curV4L2Format_.fmt.pix.field        = V4L2_FIELD_NONE;
-	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;
+	v4l2PixFormat_.width        = size.width;
+	v4l2PixFormat_.height       = size.height;
+	v4l2PixFormat_.pixelformat  = info.v4l2Format;
+	v4l2PixFormat_.field        = V4L2_FIELD_NONE;
+	v4l2PixFormat_.bytesperline = streamConfig.stride;
+	v4l2PixFormat_.sizeimage    = streamConfig.frameSize;
+	v4l2PixFormat_.colorspace   = V4L2_COLORSPACE_SRGB;
+	v4l2PixFormat_.priv         = V4L2_PIX_FMT_PRIV_MAGIC;
+	v4l2PixFormat_.ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;
+	v4l2PixFormat_.quantization = V4L2_QUANTIZATION_DEFAULT;
+	v4l2PixFormat_.xfer_func    = V4L2_XFER_FUNC_DEFAULT;
 
 	sizeimage_ = streamConfig.frameSize;
 }
@@ -291,7 +291,7 @@  int V4L2CameraProxy::vidioc_g_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
 		return -EINVAL;
 
 	memset(&arg->fmt, 0, sizeof(arg->fmt));
-	arg->fmt.pix = curV4L2Format_.fmt.pix;
+	arg->fmt.pix = v4l2PixFormat_;
 
 	return 0;
 }
@@ -488,8 +488,8 @@  int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
 	if (bufferCount_ > 0)
 		freeBuffers();
 
-	Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);
-	V4L2PixelFormat v4l2Format = V4L2PixelFormat(curV4L2Format_.fmt.pix.pixelformat);
+	Size size(v4l2PixFormat_.width, v4l2PixFormat_.height);
+	V4L2PixelFormat v4l2Format = V4L2PixelFormat(v4l2PixFormat_.pixelformat);
 	int ret = vcam_->configure(&streamConfig_, size,
 				   PixelFormatInfo::info(v4l2Format).format,
 				   arg->count);
@@ -511,9 +511,9 @@  int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
 	for (unsigned int i = 0; i < arg->count; i++) {
 		struct v4l2_buffer buf = {};
 		buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-		buf.length = curV4L2Format_.fmt.pix.sizeimage;
+		buf.length = v4l2PixFormat_.sizeimage;
 		buf.memory = V4L2_MEMORY_MMAP;
-		buf.m.offset = i * curV4L2Format_.fmt.pix.sizeimage;
+		buf.m.offset = i * v4l2PixFormat_.sizeimage;
 		buf.index = i;
 		buf.flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
index 79eb87bcea51..92a79abb9e21 100644
--- a/src/v4l2/v4l2_camera_proxy.h
+++ b/src/v4l2/v4l2_camera_proxy.h
@@ -73,13 +73,14 @@  private:
 	unsigned int refcount_;
 	unsigned int index_;
 
-	struct v4l2_format curV4L2Format_;
 	StreamConfiguration streamConfig_;
-	struct v4l2_capability capabilities_;
 	unsigned int bufferCount_;
 	unsigned int currentBuf_;
 	unsigned int sizeimage_;
 
+	struct v4l2_capability capabilities_;
+	struct v4l2_pix_format v4l2PixFormat_;
+
 	std::vector<struct v4l2_buffer> buffers_;
 	std::map<void *, unsigned int> mmaps_;