[libcamera-devel] [PATCH 10/11] android: camera_stream: Don't close fence if wait times out

[Laurent Pinchart]

Hi Jacopo,

On Mon, Oct 18, 2021 at 06:38:05PM +0200, Jacopo Mondi wrote:
> On Mon, Oct 18, 2021 at 06:59:22PM +0530, Umang Jain wrote:
> > From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > The camera HAL APIs requires that any acquire fence that hasn't been
> > waited on to be sent back to the framework as a release fence. The
> > CameraDevice already copies the acquire fence to the release fence when
> > signaling request completion, but the CameraStream incorrectly closes
> > the fence when a wait times out and sets it to -1. Fix it.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/android/camera_stream.cpp | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> > index 9b5cd0c4..8e6ccb83 100644
> > --- a/src/android/camera_stream.cpp
> > +++ b/src/android/camera_stream.cpp
> > @@ -147,16 +147,16 @@ int CameraStream::process(const FrameBuffer &source,
> >  			  Camera3RequestDescriptor *request)
> >  {
> >  	/* Handle waiting on fences on the destination buffer. */
> > -	int fence = dest.fence;
> > -	if (fence != -1) {
> > -		int ret = waitFence(fence);
> > -		::close(fence);
> > -		dest.fence = -1;
> > +	if (dest.fence != -1) {
> > +		int ret = waitFence(dest.fence);
> >  		if (ret < 0) {
> 
> If the only reason why a fence should not be closed is a timeout,
> shouldn't we inspect the return error code then ?

Or we should update the commit message. If we don't successfully wait on
a fence, I think it should be passed back to the camera service (I'm not
sure what other conditions could arise though, it's probably theoretical
only).

s/times out/fails/

in the subject and commit message.

> >  			LOG(HAL, Error) << "Failed waiting for fence: "
> > -					<< fence << ": " << strerror(-ret);
> > +					<< dest.fence << ": " << strerror(-ret);
> >  			return ret;
> >  		}
> > +
> > +		::close(dest.fence);
> > +		dest.fence = -1;
> >  	}
> >
> >  	if (!postProcessor_)

-- 
Regards,

Laurent Pinchart