[libcamera-devel] [PATCH v3 0/4] lc-compliance: Add test to queue more requests than hardware depth

[Laurent Pinchart]

Hi Nícolas,

Thank you for the patches.

On Wed, Apr 21, 2021 at 01:51:35PM -0300, Nícolas F. R. A. Prado wrote:
> The purpose of this series is to add a new test to lc-compliance that tests
> queuing a lot of requests at once in order to ensure that pipeline handlers are
> able to handle more requests than they have resources for (like internal buffers
> and buffers in the videodev) [1].
> 
> In order to achieve this, the FrameBufferAllocator had to be adapted in order to
> allow an arbitrary amount of buffers to be allocated. But there's also the issue
> of reporting the minimum amount of requests required by the pipeline handler,
> which was solved by creating a new QueueDepth property.
> 
> So patch 1 adds the new QueueDepth property to report the minimum amount of
> requests needed by the pipeline handler.
> 
> Patch 2 adds a count argument to allocate() so that the amount of buffers to
> allocate needs to be specified, as it is no longer assumed through bufferCount.
> 
> Patch 3 adds the test to lc-compliance.
> 
> Patch 4 removes bufferCount from the StreamConfiguration as it is no longer
> needed.
> 
> The amount of buffers to allocate in the lc-compliance test (patch 3) was
> hardcoded to 8 since more than that would cause errors when allocating.

We could probably go for less than 8, I'll comment on that in patch 3/4.

> Hirokazu,
> since you have a patch series addressing this issue on the IPU3
> pipeline [2], could you test this series? If both my and your patch series are
> working, lc-compliance should crash before applying your series but run through
> completion after applying it.

That's a perfect use of tests :-)

> Changes in v3:
> - Added patches 1 and 4 to add the QueueDepth property and remove bufferCount
> - Made the count argument required in patch 2
> - Added previously missing changes to the gstreamer and v4l2 compatibility
>   layers
> 
> Changes in v2:
> - Renamed and reworded commits and series
> - Dropped patches 2 and 3, which were hacks to test, and added patch 1 to add
>   count to FrameBufferAllocator
> - Thanks to Niklas:
>   - Created new standalone test instead of looping over the other tests
> 
> [1] https://bugs.libcamera.org/show_bug.cgi?id=24
> [2] https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019108.html
> 
> v1: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019139.html
> v2: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019398.html
> 
> Nícolas F. R. A. Prado (4):
>   libcamera: property: Add QueueDepth property
>   libcamera: framebuffer_allocator: Make allocate() require count
>   lc-compliance: Add test to queue more requests than hardware depth
>   libcamera: stream: Remove bufferCount
> 
>  include/libcamera/camera.h                    |  2 +-
>  include/libcamera/framebuffer_allocator.h     |  2 +-
>  include/libcamera/internal/pipeline_handler.h |  2 +-
>  include/libcamera/stream.h                    |  2 -
>  src/cam/capture.cpp                           | 10 +--
>  src/gstreamer/gstlibcameraallocator.cpp       |  4 +-
>  src/lc-compliance/simple_capture.cpp          | 83 ++++++++++++++++++-
>  src/lc-compliance/simple_capture.h            | 16 ++++
>  src/lc-compliance/single_stream.cpp           | 37 ++++++++-
>  src/libcamera/camera.cpp                      |  4 +-
>  src/libcamera/framebuffer_allocator.cpp       |  5 +-
>  src/libcamera/pipeline/ipu3/cio2.cpp          |  1 -
>  src/libcamera/pipeline/ipu3/ipu3.cpp          | 23 ++---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 17 ++--
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 15 ++--
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp |  2 -
>  src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  4 +-
>  src/libcamera/pipeline/simple/converter.cpp   |  7 +-
>  src/libcamera/pipeline/simple/converter.h     |  5 +-
>  src/libcamera/pipeline/simple/simple.cpp      | 19 ++---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  | 14 ++--
>  src/libcamera/pipeline/vimc/vimc.cpp          | 15 ++--
>  src/libcamera/pipeline_handler.cpp            |  1 +
>  src/libcamera/property_ids.yaml               |  5 ++
>  src/libcamera/stream.cpp                      |  9 +-
>  src/qcam/main_window.cpp                      |  4 +-
>  src/v4l2/v4l2_camera.cpp                      | 16 ++--
>  src/v4l2/v4l2_camera.h                        |  5 +-
>  src/v4l2/v4l2_camera_proxy.cpp                |  8 +-
>  test/camera/buffer_import.cpp                 | 10 ++-
>  test/camera/capture.cpp                       |  4 +-
>  test/camera/statemachine.cpp                  |  4 +-
>  test/libtest/buffer_source.cpp                |  4 +-
>  test/libtest/buffer_source.h                  |  2 +-
>  test/mapped-buffer.cpp                        |  4 +-
>  test/v4l2_videodevice/buffer_cache.cpp        |  4 +-
>  36 files changed, 246 insertions(+), 123 deletions(-)

-- 
Regards,

Laurent Pinchart