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

[Laurent Pinchart]

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.

> 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