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

[Naushir Patuck]

Hi Laurent,

On Tue, 4 Oct 2022 at 23:29, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> 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.
>
> 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 1/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 2/4 and 3/4 then use this new metadata accessor to set the
> bytesused values in the RkISP1 and IPU3 pipeline handlers. Finally,
> patch 4/4 prints a warning message in V4L2VideoDevice::queueBuffer() to
> catch callers that still use the deprecated API.
>
> One nice side effect of patch 1/4 is that the V4L2VideoDevice class
> doesn't need to be listed as a friend of the FrameBuffer class anymore.
>
> 2/4 and 3/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.
>
> As far as I can tell, the Raspberry Pi pipeline handler doesn't need any
> change. Naush, would you be able to test this series to confirm that it
> doesn't introduce any regression for you ?
>

Done some testing on this series, and I cannot see any obvious regressions
for our platform.  So...

Tested-by: Naushir Patuck <naush at raspberrypi.com>



> Laurent Pinchart (4):
>   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
>   libcamera: v4l2_videodevice: Warn if bytesused == 0 when queuing
>     output 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            | 25 +++++---
>  8 files changed, 84 insertions(+), 53 deletions(-)
>
>
> base-commit: 12f4708e35cde15ff9607d59eb053ece1b2bd081
> --
> Regards,
>
> Laurent Pinchart
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221005/37d3f394/attachment.htm>