[libcamera-devel] [PATCH 0/4] libcamera: Fix kernel deprecation warning with output buffers

[Laurent Pinchart]

Hello,

This patch series fixes a warning printed by videobuf2.

The V4L2 API specification indicates that, for an output video device,
the driver will interpret a zero bytesused value as the buffer length
(for v4l2_buffer) or the plane length (for v4l2_plane). The videobuf2
framework implements this behaviour, but also considers this case as
deprecated and prints a warning:

[   54.375534] use of bytesused == 0 is deprecated and will be removed in the future,
[   54.388026] use the actual size instead.

This is triggered by the RkISP1 and IPU3 pipeline handlers, which don't
set bytesused for the parameters buffers.

There are multiple ways to fix this issue, and as I couldn't decide
which one is best, I've included two in this series.

Patch 1/4 is the simple fix, it sets bytesused to the buffer or plane
length before calling VIDIOC_QBUF, essentially replicating in userspace
the kernel behaviour that videobuf2 considers deprecated. It may however
not be considered right, one may argue that the user of the
V4L2VideoDevice should always set the bytesused values correctly.

Patches 2/4 to 4/4 are an attempt at setting bytesused correctly. As the
FrameBuffer metadata is private and only accessible in const form
outside of the class (with the exception of V4L2VideoDevice that is
listed as a friend), patch 2/4 first makes mutable access to the
metadata possible. In order to avoid exposing mutable access to
applications, it moves the metadata (and the other remaining private
members of the FrameBuffer class) to the FrameBuffer::Private class, and
exposes a mutable accessor there. Patches 3/4 and 4/4 then use that
accessor to set the bytesused values in the RkISP1 and IPU3 pipeline
handlers.

One nice side effect of patch 2/4 is that the V4L2VideoDevice class
doesn't need to be listed as a friend of the FrameBuffer class anymore.
If 1/4 is considered a good fix, I would thus be tempted to still merge
2/4 just to drop the friend statement.

3/4 and 4/4 currently hardcode the bytesused value to the size of the
parameters buffer structure. This works fine with the RkISP1 and IPU3
pipeline handlers as the parameters buffers have a fixed size, but other
devices in the future may use a variable-size buffer which would require
the IPA module to pass the data size to the pipeline handler. I was
tempted to do the same for RkISP1 and IPU3 for correctness, but decided
it was probably not worth it.

Note also that, if a future device requires variable-size buffers, the
approach in patch 1/4 won't work anymore, the more complex fix will then
be required.

Laurent Pinchart (4):
  libcamera: v4l2_videodevice: Ensure non-zero bytesused for output
    buffers
  libcamera: framebuffer: Move remaining private data to Private class
  pipeline: ipu3: Set bytesused before queuing parameters buffer
  pipeline: rkisp1: Set bytesused before queuing parameters buffer

 include/libcamera/framebuffer.h               | 19 ++----
 include/libcamera/internal/framebuffer.h      | 10 +++-
 .../mm/cros_frame_buffer_allocator.cpp        |  8 +--
 .../mm/generic_frame_buffer_allocator.cpp     |  9 +--
 src/libcamera/framebuffer.cpp                 | 58 ++++++++++++-------
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  4 ++
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  4 ++
 src/libcamera/v4l2_videodevice.cpp            | 30 +++++-----
 8 files changed, 85 insertions(+), 57 deletions(-)


base-commit: 79f0fc937d95cbf1bd39f04dfd8b83206bda5098
-- 
Regards,

Laurent Pinchart