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

[Laurent Pinchart]

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 at 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 at ideasonboard.com>
> 
> > +			return ret;
> > +		}
> > +	}
> > +
> >  	Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);
> >  	ret = vcam_->configure(&streamConfig_, size,
-- 
Regards,

Laurent Pinchart