[libcamera-devel] [PATCH v3 0/9] Add offset to FrameBuffer::Plane

[Hirokazu Honda]

Hi Laurent,

On Thu, Aug 26, 2021 at 10:11 PM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Hiro,
>
> On Thu, Aug 26, 2021 at 08:25:30PM +0900, Hirokazu Honda wrote:
> > Since FrameBuffer::Plane doesn't have an offset variable, it is
> > impossible to map the FrameBuffer::Plane correctly. This series fixes
> > the issue by adding an offset and aligns some code of mapping and
> > creating FrameBuffer::Plane with the new offset.
> >
> > One of the pateches fixes V4L2Device to make a returned FrameBuffer
> > color format planes from v4l2 format planes. However, V4L2 API doesn't
> > provide a way of getting a dmabuf plane offset, so a multi planar
> > format e.g. V4L2_PIX_FMT_NV12M is not supported with this patch series.
> >
> > This series must be applied on top of the pacth series whose id is 2384.
> > cf.) https://patchwork.libcamera.org/project/libcamera/list/?series=2384
>
> Very nice series. I've replied with a few minor comments on patches 1/9
> to 6/9, and 8/9 to 9/9. I can handle those when pushing the series.
>
> Patch 7/9 is still problematic, as it breaks a unit test. If it's the
> only patch that needs a new version, you can resubmit that one only (for
> instance as v3.1, in a reply to v3) if it's easier for you. If you
> prefer sending a v4 of the whole series, please address the minor
> comments on the other patches as well.
>

Thanks for testing. I uploaded 7/9 patch only in v4.

-Hiro


> > Change in v3:
> > - address review comments to v2
> > - squash 01/10 and 02/10 (serialization change) of v2
> >
> > Change in v2:
> > - adress review comments to v1
> >
> > Hirokazu Honda (9):
> >   libcamera: framebuffer: Add offset to FrameBuffer::Plane
> >   libcamera: mapped_framebuffer: Return plane begin address by
> >     MappedBuffer::maps()
> >   cam: file_sink: Use offset in mapping FrameBuffer
> >   ipa: rkisp1: Use offset in mapping IPABuffer
> >   qcam: main_window: Use offset mapping FrameBuffer
> >   gstreamer: gstlibcameraallocator: Use offset in creating a buffer
> >   V4L2VideDevice::createBuffer() creates the same number of
> >     FrameBuffer::Planes as V4L2 format planes. Therefore, if the v4l2
> >     format single is single-planar format, the created number of
> >     FrameBuffer::Planes is 1. It should rather create the same number of
> >     FrameBuffer::Planes as the color format planes.
> >   android: camera_device: Fill offset and right length in
> >     CreateFrameBuffer()
> >   libcamera: framebuffer: Add assertion to detect offset is unfilled
> >
> >  include/libcamera/framebuffer.h               |  12 +-
> >  .../libcamera/internal/mapped_framebuffer.h   |   4 +-
> >  include/libcamera/internal/v4l2_videodevice.h |  10 +-
> >  src/android/camera_device.cpp                 |  52 ++++---
> >  src/android/camera_device.h                   |   6 +-
> >  src/android/mm/generic_camera_buffer.cpp      |   2 -
> >  src/cam/file_sink.cpp                         |  20 ++-
> >  src/cam/file_sink.h                           |   1 +
> >  src/gstreamer/gstlibcameraallocator.cpp       |   4 +-
> >  src/ipa/rkisp1/rkisp1.cpp                     |  33 ++--
> >  src/libcamera/framebuffer.cpp                 |  29 +++-
> >  src/libcamera/ipa_data_serializer.cpp         |   5 +-
> >  src/libcamera/mapped_framebuffer.cpp          |  71 +++++++--
> >  src/libcamera/v4l2_videodevice.cpp            | 147 ++++++++++++++----
> >  src/qcam/main_window.cpp                      |  15 +-
> >  src/qcam/main_window.h                        |   1 +
> >  test/v4l2_videodevice/buffer_cache.cpp        |   6 +-
> >  17 files changed, 311 insertions(+), 107 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart