<div dir="ltr"><div dir="ltr">Hi Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 4 Oct 2022 at 23:29, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello,<br>
<br>
This patch series fixes a warning printed by videobuf2.<br>
<br>
The V4L2 API specification indicates that, for an output video device,<br>
the driver will interpret a zero bytesused value as the buffer length<br>
(for v4l2_buffer) or the plane length (for v4l2_plane). The videobuf2<br>
framework implements this behaviour, but also considers this case as<br>
deprecated and prints a warning:<br>
<br>
[   54.375534] use of bytesused == 0 is deprecated and will be removed in the future,<br>
[   54.388026] use the actual size instead.<br>
<br>
This is triggered by the RkISP1 and IPU3 pipeline handlers, which don't<br>
set bytesused for the parameters buffers.<br>
<br>
As the FrameBuffer metadata is private and only accessible in const form<br>
outside of the class (with the exception of V4L2VideoDevice that is<br>
listed as a friend), patch 1/4 first makes mutable access to the<br>
metadata possible. In order to avoid exposing mutable access to<br>
applications, it moves the metadata (and the other remaining private<br>
members of the FrameBuffer class) to the FrameBuffer::Private class, and<br>
exposes a mutable accessor there.<br>
<br>
Patches 2/4 and 3/4 then use this new metadata accessor to set the<br>
bytesused values in the RkISP1 and IPU3 pipeline handlers. Finally,<br>
patch 4/4 prints a warning message in V4L2VideoDevice::queueBuffer() to<br>
catch callers that still use the deprecated API.<br>
<br>
One nice side effect of patch 1/4 is that the V4L2VideoDevice class<br>
doesn't need to be listed as a friend of the FrameBuffer class anymore.<br>
<br>
2/4 and 3/4 currently hardcode the bytesused value to the size of the<br>
parameters buffer structure. This works fine with the RkISP1 and IPU3<br>
pipeline handlers as the parameters buffers have a fixed size, but other<br>
devices in the future may use a variable-size buffer which would require<br>
the IPA module to pass the data size to the pipeline handler. I was<br>
tempted to do the same for RkISP1 and IPU3 for correctness, but decided<br>
it was probably not worth it.<br>
<br>
As far as I can tell, the Raspberry Pi pipeline handler doesn't need any<br>
change. Naush, would you be able to test this series to confirm that it<br>
doesn't introduce any regression for you ?<br></blockquote><div><br></div><div>Done some testing on this series, and I cannot see any obvious regressions</div><div>for our platform.  So...</div><div><br></div><div>Tested-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Laurent Pinchart (4):<br>
  libcamera: framebuffer: Move remaining private data to Private class<br>
  pipeline: ipu3: Set bytesused before queuing parameters buffer<br>
  pipeline: rkisp1: Set bytesused before queuing parameters buffer<br>
  libcamera: v4l2_videodevice: Warn if bytesused == 0 when queuing<br>
    output buffer<br>
<br>
 include/libcamera/framebuffer.h               | 19 ++----<br>
 include/libcamera/internal/framebuffer.h      | 10 +++-<br>
 .../mm/cros_frame_buffer_allocator.cpp        |  8 +--<br>
 .../mm/generic_frame_buffer_allocator.cpp     |  9 +--<br>
 src/libcamera/framebuffer.cpp                 | 58 ++++++++++++-------<br>
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  4 ++<br>
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  4 ++<br>
 src/libcamera/v4l2_videodevice.cpp            | 25 +++++---<br>
 8 files changed, 84 insertions(+), 53 deletions(-)<br>
<br>
<br>
base-commit: 12f4708e35cde15ff9607d59eb053ece1b2bd081<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div>