[libcamera-devel] [RFC PATCH 7/8] android: camera_device: Query plane length
Tomasz Figa
tfiga at chromium.org
Mon Jul 27 15:45:17 CEST 2020
On Fri, Jul 24, 2020 at 7:16 PM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Kieran,
>
> (CC'ing Tomasz to get a review from a Chrome OS point of view)
>
> Thank you for the patch.
>
> On Mon, Jul 20, 2020 at 11:42:31PM +0100, Kieran Bingham wrote:
> > Use lseek to query the length of planes where possible rather than leaving
> > the plane.length as zero, which prevents mapping buffers for software
> > processing.
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >
> > I like this one ;-) We need to know the plane lengths for software
> > streams now, but the correct way to get this would be to talk to the
> > gralloc allocator. This would mean pulling in various extra
> > cros-specific libraries, where instead we can query the size of the
> > dmabuf by getting the offset when we SEEK_END.
> >
> >
> > src/android/camera_device.cpp | 18 ++++++++++++------
> > 1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 538b8ab5da03..6212ccdd61ec 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -1045,12 +1045,18 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer
> > continue;
> > }
> >
> > - /*
> > - * Setting length to zero here is OK as the length is only used
> > - * to map the memory of the plane. Libcamera do not need to poke
> > - * at the memory content queued by the HAL.
> > - */
> > - plane.length = 0;
> > + off_t length = lseek(plane.fd.fd(), 0, SEEK_END);
> > + if (length == -1) {
> > + /*
> > + * A zero length plane is not fatal unless the
> > + * FrameBuffer is used for a software stream, libcamera
> > + * itself will not access the internal frame content.
> > + */
> > + LOG(HAL, Error) << "Failed to query plane length";
> > + length = 0;
>
> Do you expect this to happen ? If not, I'd return an error. If yes, an
> error message would be printed in a loop, which isn't very nice. My
> preference would be to return an error if possible.
>
> Otherwise, nice hack :-)
A zero-length plane would be strange and could suggest that the
backing file is not a DMA-buf, so it could indeed make sense to error
out.
Otherwise, we tend to do this in a number of places in Chrome OS and
seems to be the official way to query a DMA-buf for its size. However
I wonder if we shouldn't restore the original position after the seek?
Best regards,
Tomasz
More information about the libcamera-devel
mailing list