[libcamera-devel] [PATCH v2 00/27] libcamera: Handle fallout of FrameBuffer offset support
David Plowman
david.plowman at raspberrypi.com
Mon Sep 6 15:13:58 CEST 2021
Hi Laurent
Thanks for the reply!
On Mon, 6 Sept 2021 at 13:14, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> 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.
Yes, isContiguous() sounds pretty much like what I want. I'm
interested in functions that make it more convenient for me to know
how I pass a buffer (for example) to my V4L2 h.264 encoder.
>
> > 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.
Perhaps we could define a "safe" version of the function that
complains if it was called on a truly multi planar buffer? It might
return zero, or print a warning to the console - would that help?
Best regards
David
>
> > 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