[libcamera-devel,v3,04/22] v4l2: v4l2_camera_proxy: Free old buffers on reqbufs > 0

Message ID 20200623190836.53446-5-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • Support v4l2-compliance
Related show

Commit Message

Paul Elder June 23, 2020, 7:08 p.m. UTC
Free buffers if any were previously allocated, at VIDIOC_REQBUFS with
count > 0.

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

---
New in v3
- split from... a conglomerate of v4l2-compliance fixes patch
---
 src/v4l2/v4l2_camera_proxy.cpp | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Laurent Pinchart June 23, 2020, 10:25 p.m. UTC | #1
Hi Paul,

Thank you for the patch.

On Wed, Jun 24, 2020 at 04:08:18AM +0900, Paul Elder wrote:
> Free buffers if any were previously allocated, at VIDIOC_REQBUFS with

s/buffers/buffers,/ ?

> count > 0.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> New in v3
> - split from... a conglomerate of v4l2-compliance fixes patch
> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index eb59d49..7262453 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -361,6 +361,15 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
>  		return freeBuffers();
>  	}
>  
> +	if (bufferCount_ > 0) {
> +		int ret = freeBuffers();
> +		if (ret < 0) {
> +			LOG(V4L2Compat, Error)
> +				<< "Failed to free libcamera buffers for re-reqbufs";

			release(file);

?

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

> +			return ret;
> +		}
> +	}
> +
>  	Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);
>  	ret = vcam_->configure(&streamConfig_, size,
>  			       v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat),
Laurent Pinchart June 23, 2020, 10:26 p.m. UTC | #2
On Wed, Jun 24, 2020 at 01:25:41AM +0300, Laurent Pinchart wrote:
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Wed, Jun 24, 2020 at 04:08:18AM +0900, Paul Elder wrote:
> > Free buffers if any were previously allocated, at VIDIOC_REQBUFS with
> 
> s/buffers/buffers,/ ?
> 
> > count > 0.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > 
> > ---
> > New in v3
> > - split from... a conglomerate of v4l2-compliance fixes patch
> > ---
> >  src/v4l2/v4l2_camera_proxy.cpp | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> > index eb59d49..7262453 100644
> > --- a/src/v4l2/v4l2_camera_proxy.cpp
> > +++ b/src/v4l2/v4l2_camera_proxy.cpp
> > @@ -361,6 +361,15 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
> >  		return freeBuffers();
> >  	}
> >  
> > +	if (bufferCount_ > 0) {
> > +		int ret = freeBuffers();
> > +		if (ret < 0) {
> > +			LOG(V4L2Compat, Error)
> > +				<< "Failed to free libcamera buffers for re-reqbufs";
> 
> 			release(file);
> 
> ?

Actually likely not, as the result is that buffers are not freed, so
ownership should not be released. I'll let you decide which of my two
comments is the right one :-)

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > +			return ret;
> > +		}
> > +	}
> > +
> >  	Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);
> >  	ret = vcam_->configure(&streamConfig_, size,

Patch

diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
index eb59d49..7262453 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -361,6 +361,15 @@  int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
 		return freeBuffers();
 	}
 
+	if (bufferCount_ > 0) {
+		int ret = freeBuffers();
+		if (ret < 0) {
+			LOG(V4L2Compat, Error)
+				<< "Failed to free libcamera buffers for re-reqbufs";
+			return ret;
+		}
+	}
+
 	Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);
 	ret = vcam_->configure(&streamConfig_, size,
 			       v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat),