[libcamera-devel] [PATCH v2 18/27] android: camera_device: Don't assume all planes use the same fd

[Hirokazu Honda]

Hi Laurent,

On Mon, Sep 6, 2021 at 4:38 PM Jean-Michel Hautbois
<jeanmichel.hautbois at ideasonboard.com> wrote:
>
> Hi Laurent,
>
> On 06/09/2021 04:00, Laurent Pinchart wrote:
> > Now that libcamera correctly supports frame buffers with different
> > dmabuf for each plane, remove the assumption that a single dmabuf is
> > used.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> > ---
> >  src/android/camera_device.cpp | 25 ++++++-------------------
> >  1 file changed, 6 insertions(+), 19 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 8ca76719a50f..c64064106ccc 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -749,25 +749,6 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer
> >                                            libcamera::PixelFormat pixelFormat,
> >                                            const libcamera::Size &size)
> >  {
> > -     FileDescriptor fd;
> > -     /*
> > -      * This assumes all the planes are in the same dmabuf.
> > -      *
> > -      * \todo Verify that this assumption holds, fstat() can be used to check
> > -      * if two fds refer to the same dmabuf.
> > -      */
> > -     for (int i = 0; i < camera3buffer->numFds; i++) {
> > -             if (camera3buffer->data[i] != -1) {
> > -                     fd = FileDescriptor(camera3buffer->data[i]);
> > -                     break;
> > -             }
> > -     }
> > -
> > -     if (!fd.isValid()) {
> > -             LOG(HAL, Fatal) << "No valid fd";
> > -             return nullptr;
> > -     }
> > -
> >       CameraBuffer buf(camera3buffer, pixelFormat, size, PROT_READ);
> >       if (!buf.isValid()) {
> >               LOG(HAL, Fatal) << "Failed to create CameraBuffer";
> > @@ -776,6 +757,12 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer
> >
> >       std::vector<FrameBuffer::Plane> planes(buf.numPlanes());
> >       for (size_t i = 0; i < buf.numPlanes(); ++i) {

I think it is not guaranteed that numFds is necessarily the same as
buf.numPlanes().

-Hiro
> > +             FileDescriptor fd{ camera3buffer->data[i] };
> > +             if (!fd.isValid()) {
> > +                     LOG(HAL, Fatal) << "No valid fd";
> > +                     return nullptr;
> > +             }
> > +
> >               planes[i].fd = fd;
> >               planes[i].offset = buf.offset(i);
> >               planes[i].length = buf.size(i);
> >