[libcamera-devel] [PATCH v2 00/27] libcamera: Handle fallout of FrameBuffer offset support
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Sep 6 14:14:09 CEST 2021
Hi David,
On Mon, Sep 06, 2021 at 12:22:25PM +0100, David Plowman wrote:
> Hi Laurent, everyone
>
> Thanks for all the efforts to get this working! I had just a little
> question or two...
>
> 1. Is it easy to tell if a FrameBuffer is actually single plane or
> multi plane? If not, could we add a public API function that would
> tell us?
You can use FrameBuffer::planes().size() to get the number of planes. Or
did you mean checking if the different planes in a multi-planar frame
buffer are stored contiguously in the same dmabuf ? There's a private
helper for that (FrameBuffer::Private::isContiguous()), I haven't made
it public yet as I wanted to evaluate the use cases first.
> 2. Is it easy to get the full size of the buffer for the single plane
> case (rather than having to add all the bits up)? And again, if the
> answer is no, could we add such a thing?
>
> I'm thinking of trying to make life easy for applications that might
> want to pass these buffers to codecs where the driver might only
> support single planes. Not thinking of any platform in particular...
> :)
It again depends what you mean :-) If the FrameBuffer has a single
plane, FrameBuffer::planes()[0].length (and
FrameMetaData::planes()[0].bytesused) will give you what you need. I
suspect you're however consider the case of a multi-planar FrameBuffer
with planes stored contiguously in memory, using the single-planar V4L2
formats (e.g. V4L2_PIX_FMT_NV12, as opposed to V4L2_PIX_FMT_NV12M). I'm
a bit worried that a helper function in that case would be used by
applications to ignore that the buffer can be truly multi-planar.
> On Mon, 6 Sept 2021 at 03:01, Laurent Pinchart wrote:
> >
> > Hello everybody,
> >
> > This patch series started as an investigation of a qcam failure with
> > IPU3 after the merge of the FrameBuffer offset support. While a hack in
> > qcam would be possible, I decided to instead address the core issue and
> > fix it in V4L2VideoDevice.
> >
> > Compared to v1, the series now includes fixes for cam and qcam in
> > addition to the changes needed in the libcamera core. They have been
> > tested with the Raspberry Pi, IPU3, VIMC and UVC pipeline handlers.
> >
> > The GStreamer element seems to work fine without any change required.
> > The V4L2 compatibility layer is still broken, and I haven't tested the
> > Android HAL yet (any volunteer ?).
> >
> > The most important change is in patches 13/27 and 14/27, which translate
> > between V4L2 buffers and libcamera FrameBuffer to handle the case where
> > a multi-planar frame buffer is used with the V4L2 single-planar API.
> > It's working more or less by chance at the moment (except in qcam where
> > it's broken, and possibly in other places I haven't tested). Patches
> > 01/27 to 12/27 are cleanups and additions to prepare for the work in
> > V4L2VideoDevice, and patch 15/27 is a small cleanup on top. Patches
> > 16/27 and 17/27 then improve the FrameBuffer class API as a cleanup.
> >
> > Patches 18/27 to 27/27 fix the cam and qcam applications, as well as an
> > issue in the Android HAL. Worth being noted is patch 19/27 that
> > introduces an Image class shared by cam and qcam. The class duplicates
> > the MappedFrameBuffer implementation private to libcamera. I've tried to
> > rework MappedFrameBuffer into something I would be happy to see in the
> > public API, but failed to do so in a reasonable amount of time, and I
> > didn't want to delay this important regression fix.
> >
> > This series doesn't break any unit test, as vimc doesn't support NV12.
> > Addition of NV12 support to the vimc kernel driver would be very nice,
> > in order to test multi-planar support in our unit tests. Volunteers are
> > welcome ;-)
> >
> > Laurent Pinchart (27):
> > libcamera: base: utils: Use size_t for index in utils::enumerate()
> > libcamera: file_descriptor: Add a function to retrieve the inode
> > libcamera: v4l2_videodevice: Drop toV4L2PixelFormat()
> > libcamera: Use V4L2PixelFormat::fromPixelFormat()
> > libcamera: formats: Move plane info structure to PixelFormatInfo
> > libcamera: formats: Add planeSize() helpers to PixelFormatInfo
> > libcamera: formats: Support V4L2 non-contiguous formats
> > libcamera: framebuffer: Move planes check to constructor
> > libcamera: framebuffer: Add a function to check if planes are
> > contiguous
> > libcamera: v4l2_videodevice: Cache PixelFormatInfo
> > libcamera: v4l2_videodevice: Document plane handling in createBuffer()
> > libcamera: v4l2_videodevice: Take stride into account to compute
> > offsets
> > libcamera: v4l2_videodevice: Coalesce planes when queuing buffer
> > libcamera: v4l2_videodevice: Split planes when dequeuing buffer
> > libcamera: v4l2_videodevice: Use utils::enumerate()
> > libcamera: framebuffer: Allocate metadata planes at construction time
> > libcamera: framebuffer: Prevent modifying the number of metadata
> > planes
> > android: camera_device: Don't assume all planes use the same fd
> > cam: Add Image class
> > cam: file_sink: Use Image class to access pixel data
> > cam: drm: Support per-plane stride values
> > cam: drm: Set per-plane offsets when creating DRM frame buffer
> > cam: drm: Avoid importing the same dmabuf multiple times
> > qcam: Print bytesused for all planes
> > qcam: Use Image class to access pixel data
> > qcam: viewfinder_gl: Support multi-planar buffers
> > qcam: viewfinder_qt: Support multi-planar buffers
> >
> > include/libcamera/base/utils.h | 4 +-
> > include/libcamera/file_descriptor.h | 3 +
> > include/libcamera/framebuffer.h | 19 +-
> > include/libcamera/internal/formats.h | 22 +-
> > include/libcamera/internal/framebuffer.h | 2 +
> > include/libcamera/internal/v4l2_pixelformat.h | 2 +-
> > include/libcamera/internal/v4l2_videodevice.h | 3 +-
> > src/android/camera_device.cpp | 25 +-
> > src/android/mm/generic_camera_buffer.cpp | 11 +-
> > src/android/yuv/post_processor_yuv.cpp | 10 +-
> > src/cam/camera_session.cpp | 4 +-
> > src/cam/drm.cpp | 38 +-
> > src/cam/drm.h | 7 +-
> > src/cam/file_sink.cpp | 44 +--
> > src/cam/file_sink.h | 6 +-
> > src/cam/image.cpp | 107 +++++
> > src/cam/image.h | 52 +++
> > src/cam/kms_sink.cpp | 28 +-
> > src/cam/meson.build | 1 +
> > src/libcamera/file_descriptor.cpp | 26 ++
> > src/libcamera/formats.cpp | 373 ++++++++++++++----
> > src/libcamera/framebuffer.cpp | 57 ++-
> > src/libcamera/pipeline/ipu3/cio2.cpp | 4 +-
> > src/libcamera/pipeline/ipu3/imgu.cpp | 2 +-
> > .../pipeline/raspberrypi/raspberrypi.cpp | 8 +-
> > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 +-
> > src/libcamera/pipeline/simple/converter.cpp | 8 +-
> > src/libcamera/pipeline/simple/simple.cpp | 4 +-
> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 6 +-
> > src/libcamera/pipeline/vimc/vimc.cpp | 8 +-
> > src/libcamera/v4l2_pixelformat.cpp | 11 +-
> > src/libcamera/v4l2_videodevice.cpp | 196 ++++++---
> > src/qcam/format_converter.cpp | 18 +-
> > src/qcam/format_converter.h | 9 +-
> > src/qcam/main_window.cpp | 38 +-
> > src/qcam/main_window.h | 4 +-
> > src/qcam/meson.build | 1 +
> > src/qcam/viewfinder.h | 6 +-
> > src/qcam/viewfinder_gl.cpp | 45 +--
> > src/qcam/viewfinder_gl.h | 4 +-
> > src/qcam/viewfinder_qt.cpp | 20 +-
> > src/qcam/viewfinder_qt.h | 2 +-
> > src/v4l2/v4l2_camera_proxy.cpp | 11 +-
> > test/libtest/buffer_source.cpp | 3 +-
> > test/utils.cpp | 10 +-
> > 45 files changed, 911 insertions(+), 357 deletions(-)
> > create mode 100644 src/cam/image.cpp
> > create mode 100644 src/cam/image.h
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list