[libcamera-devel,v6,00/10] lc-compliance: Add test to queue more requests than hardware depth

12946 mbox series

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 v4l2 buffer slots) [1].

[1] https://bugs.libcamera.org/show_bug.cgi?id=24

In order to achieve this, the FrameBufferAllocator had to be adapted 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 MinNumRequests property.

So patch 1 adds the new MinNumRequests 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.

Patches 3-6 does some refactoring in lc-compliance in order to reduce code
duplication.

Patch 7 adds the test to lc-compliance.

Patch 8 adds checks to ensure that requests which failed to be enqueued are
reported as test failure.

Patch 9 replaces all usage of bufferCount for allocateBuffers() and
importBuffers() in pipeline handlers to constants, as bufferCount wasn't a good
fit and to allow its removal. This is intended as a temporary measure, and
pipeline handlers will probably want to improve the logic of those values.

Patch 10 removes bufferCount from the StreamConfiguration as it is no longer
needed.

I've run this new lc-compliance test on the rkisp1, raspberrypi, uvcvideo and
vimc pipelines.
* raspberrypi already handles it well.
* uvcvideo runs successfully after applying the patch in [2]
* rkisp1 runs successfully after applying the patch in [3]
* vimc fails, but should be easily fixed with a patch similar to the one for
  uvcvideo.

[2] https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022029.html
[3] https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022135.html

The amount of buffers to allocate in the lc-compliance test (patch 7) was
hardcoded to 8 since more than that would cause errors when allocating.

Changes in v6:
- Thanks to Naushir:
  - Removed unneeded comment on MinNumRequests property of RPi
- Fixed style issues
- Changed static_cast to unsigned int when comparing buffer count in
  lc-compliance
- Changed parameter order of queueRequests()
- Added comment on runCaptureSession()
- Added pipeline prefix to INTERNAL_BUFFER_COUNT and BUFFER_SLOT_COUNT constants
- Removed unused variable in the IPU3 pipeline

v5: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022098.html

Changes in v5:
- Rebased on master (now that lc-compliance was refactored to use Googletest)
- Added patches 3, 5, 6 and 8
- Fixed qcam to use at least two buffers

v4: https://lists.libcamera.org/pipermail/libcamera-devel/2021-May/020150.html

Changes in v4:
- Thanks to Laurent:
  - Renamed QueueDepth property to MinNumRequests and better documented it
  - Changed patch 6 to also remove bufferCount from android
- Thanks to Niklas:
  - Added patch 3 to factor common code in lc-compliance
- Added patch 5 to remove pipeline dependency on bufferCount

v3: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019613.html

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

v2: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019398.html

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

v1: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019139.html

Nícolas F. R. A. Prado (10):
  libcamera: property: Add MinNumRequests property
  libcamera: framebuffer_allocator: Make allocate() require count
  lc-compliance: Move buffer allocation to separate function
  lc-compliance: Factor common capture code into SimpleCapture
  lc-compliance: Move camera setup to CameraHolder class
  lc-compliance: Move role to string conversion to its own function
  lc-compliance: Add test to queue more requests than hardware depth
  lc-compliance: Check that requests complete successfully
  libcamera: pipeline: Don't rely on bufferCount
  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/android/camera_stream.cpp                 |   7 +-
 src/cam/capture.cpp                           |   9 +-
 src/gstreamer/gstlibcameraallocator.cpp       |   4 +-
 src/lc-compliance/capture_test.cpp            | 111 ++++++++++---
 src/lc-compliance/simple_capture.cpp          | 150 +++++++++++++-----
 src/lc-compliance/simple_capture.h            |  27 +++-
 src/libcamera/camera.cpp                      |   4 +-
 src/libcamera/framebuffer_allocator.cpp       |   5 +-
 src/libcamera/pipeline/ipu3/cio2.cpp          |   1 -
 src/libcamera/pipeline/ipu3/imgu.cpp          |  12 +-
 src/libcamera/pipeline/ipu3/imgu.h            |   5 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  27 ++--
 .../pipeline/raspberrypi/raspberrypi.cpp      |  27 ++--
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  15 +-
 src/libcamera/pipeline/rkisp1/rkisp1_path.cpp |   4 +-
 src/libcamera/pipeline/rkisp1/rkisp1_path.h   |   3 +
 src/libcamera/pipeline/simple/converter.cpp   |   7 +-
 src/libcamera/pipeline/simple/converter.h     |   6 +-
 src/libcamera/pipeline/simple/simple.cpp      |  17 +-
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  17 +-
 src/libcamera/pipeline/vimc/vimc.cpp          |  17 +-
 src/libcamera/pipeline_handler.cpp            |   1 +
 src/libcamera/property_ids.yaml               |   8 +
 src/libcamera/stream.cpp                      |   9 +-
 src/qcam/main_window.cpp                      |  11 +-
 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 +-
 39 files changed, 367 insertions(+), 206 deletions(-)