[libcamera-devel,3/3] v4l2: camera: Merge getStreamConfig() with open()
diff mbox series

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

Commit Message

Laurent Pinchart Aug. 22, 2020, 1:46 p.m. UTC 
The V4L2CameraProxy always calls V4L2Camera::getStreamConfig() right
after V4L2Camera::open(), and never afterwards. Simplify the code by
returning the initial configuration from V4L2Camera::open() and removing
V4L2Camera::getStreamConfig().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/v4l2/v4l2_camera.cpp       | 8 ++------
 src/v4l2/v4l2_camera.h         | 4 ++--
 src/v4l2/v4l2_camera_proxy.cpp | 3 +--
 3 files changed, 5 insertions(+), 10 deletions(-)

Comments

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

On 22/08/2020 14:46, Laurent Pinchart wrote:
> The V4L2CameraProxy always calls V4L2Camera::getStreamConfig() right
> after V4L2Camera::open(), and never afterwards. Simplify the code by
> returning the initial configuration from V4L2Camera::open() and removing
> V4L2Camera::getStreamConfig().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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


> ---
>  src/v4l2/v4l2_camera.cpp       | 8 ++------
>  src/v4l2/v4l2_camera.h         | 4 ++--
>  src/v4l2/v4l2_camera_proxy.cpp | 3 +--
>  3 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 61bca0732447..24805d5f3c3f 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -28,7 +28,7 @@ V4L2Camera::~V4L2Camera()
>  	close();
>  }
>  
> -int V4L2Camera::open()
> +int V4L2Camera::open(StreamConfiguration *streamConfig)
>  {
>  	if (camera_->acquire() < 0) {
>  		LOG(V4L2Compat, Error) << "Failed to acquire camera";
> @@ -43,6 +43,7 @@ int V4L2Camera::open()
>  
>  	bufferAllocator_ = new FrameBufferAllocator(camera_);
>  
> +	*streamConfig = config_->at(0);
>  	return 0;
>  }
>  
> @@ -64,11 +65,6 @@ void V4L2Camera::unbind()
>  	efd_ = -1;
>  }
>  
> -void V4L2Camera::getStreamConfig(StreamConfiguration *streamConfig)
> -{
> -	*streamConfig = config_->at(0);
> -}
> -
>  std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers()
>  {
>  	std::vector<Buffer> v;
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index be6c4e187363..1fc5ebefef6a 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -37,11 +37,11 @@ public:
>  	V4L2Camera(std::shared_ptr<Camera> camera);
>  	~V4L2Camera();
>  
> -	int open();
> +	int open(StreamConfiguration *streamConfig);
>  	void close();
>  	void bind(int efd);
>  	void unbind();
> -	void getStreamConfig(StreamConfiguration *streamConfig);
> +
>  	std::vector<Buffer> completedBuffers();
>  
>  	int configure(StreamConfiguration *streamConfigOut,
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 80287d8f164e..8ff990f68fcd 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -63,13 +63,12 @@ int V4L2CameraProxy::open(V4L2CameraFile *file)
>  	 * with count = 0.
>  	 */
>  
> -	int ret = vcam_->open();
> +	int ret = vcam_->open(&streamConfig_);
>  	if (ret < 0) {
>  		refcount_--;
>  		return ret;
>  	}
>  
> -	vcam_->getStreamConfig(&streamConfig_);
>  	setFmtFromConfig(streamConfig_);
>  
>  	files_.insert(file);
>
Paul Elder Aug. 25, 2020, 3:21 a.m. UTC | #2 
Hi Laurent,

On Sat, Aug 22, 2020 at 04:46:05PM +0300, Laurent Pinchart wrote:
> The V4L2CameraProxy always calls V4L2Camera::getStreamConfig() right
> after V4L2Camera::open(), and never afterwards. Simplify the code by
> returning the initial configuration from V4L2Camera::open() and removing
> V4L2Camera::getStreamConfig().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  src/v4l2/v4l2_camera.cpp       | 8 ++------
>  src/v4l2/v4l2_camera.h         | 4 ++--
>  src/v4l2/v4l2_camera_proxy.cpp | 3 +--
>  3 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 61bca0732447..24805d5f3c3f 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -28,7 +28,7 @@ V4L2Camera::~V4L2Camera()
>  	close();
>  }
>  
> -int V4L2Camera::open()
> +int V4L2Camera::open(StreamConfiguration *streamConfig)
>  {
>  	if (camera_->acquire() < 0) {
>  		LOG(V4L2Compat, Error) << "Failed to acquire camera";
> @@ -43,6 +43,7 @@ int V4L2Camera::open()
>  
>  	bufferAllocator_ = new FrameBufferAllocator(camera_);
>  
> +	*streamConfig = config_->at(0);
>  	return 0;
>  }
>  
> @@ -64,11 +65,6 @@ void V4L2Camera::unbind()
>  	efd_ = -1;
>  }
>  
> -void V4L2Camera::getStreamConfig(StreamConfiguration *streamConfig)
> -{
> -	*streamConfig = config_->at(0);
> -}
> -
>  std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers()
>  {
>  	std::vector<Buffer> v;
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index be6c4e187363..1fc5ebefef6a 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -37,11 +37,11 @@ public:
>  	V4L2Camera(std::shared_ptr<Camera> camera);
>  	~V4L2Camera();
>  
> -	int open();
> +	int open(StreamConfiguration *streamConfig);
>  	void close();
>  	void bind(int efd);
>  	void unbind();
> -	void getStreamConfig(StreamConfiguration *streamConfig);
> +
>  	std::vector<Buffer> completedBuffers();
>  
>  	int configure(StreamConfiguration *streamConfigOut,
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 80287d8f164e..8ff990f68fcd 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -63,13 +63,12 @@ int V4L2CameraProxy::open(V4L2CameraFile *file)
>  	 * with count = 0.
>  	 */
>  
> -	int ret = vcam_->open();
> +	int ret = vcam_->open(&streamConfig_);
>  	if (ret < 0) {
>  		refcount_--;
>  		return ret;
>  	}
>  
> -	vcam_->getStreamConfig(&streamConfig_);
>  	setFmtFromConfig(streamConfig_);
>  
>  	files_.insert(file);
> -- 
> 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.cpp b/src/v4l2/v4l2_camera.cpp
index 61bca0732447..24805d5f3c3f 100644
--- a/src/v4l2/v4l2_camera.cpp
+++ b/src/v4l2/v4l2_camera.cpp
@@ -28,7 +28,7 @@  V4L2Camera::~V4L2Camera()
 	close();
 }
 
-int V4L2Camera::open()
+int V4L2Camera::open(StreamConfiguration *streamConfig)
 {
 	if (camera_->acquire() < 0) {
 		LOG(V4L2Compat, Error) << "Failed to acquire camera";
@@ -43,6 +43,7 @@  int V4L2Camera::open()
 
 	bufferAllocator_ = new FrameBufferAllocator(camera_);
 
+	*streamConfig = config_->at(0);
 	return 0;
 }
 
@@ -64,11 +65,6 @@  void V4L2Camera::unbind()
 	efd_ = -1;
 }
 
-void V4L2Camera::getStreamConfig(StreamConfiguration *streamConfig)
-{
-	*streamConfig = config_->at(0);
-}
-
 std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers()
 {
 	std::vector<Buffer> v;
diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
index be6c4e187363..1fc5ebefef6a 100644
--- a/src/v4l2/v4l2_camera.h
+++ b/src/v4l2/v4l2_camera.h
@@ -37,11 +37,11 @@  public:
 	V4L2Camera(std::shared_ptr<Camera> camera);
 	~V4L2Camera();
 
-	int open();
+	int open(StreamConfiguration *streamConfig);
 	void close();
 	void bind(int efd);
 	void unbind();
-	void getStreamConfig(StreamConfiguration *streamConfig);
+
 	std::vector<Buffer> completedBuffers();
 
 	int configure(StreamConfiguration *streamConfigOut,
diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
index 80287d8f164e..8ff990f68fcd 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -63,13 +63,12 @@  int V4L2CameraProxy::open(V4L2CameraFile *file)
 	 * with count = 0.
 	 */
 
-	int ret = vcam_->open();
+	int ret = vcam_->open(&streamConfig_);
 	if (ret < 0) {
 		refcount_--;
 		return ret;
 	}
 
-	vcam_->getStreamConfig(&streamConfig_);
 	setFmtFromConfig(streamConfig_);
 
 	files_.insert(file);