[libcamera-devel] [PATCH v8 00/17] lc-compliance: Add test to queue more requests than hardware depth
Paul Elder
paul.elder at ideasonboard.com
Thu Dec 1 13:32:02 CET 2022
Hi Nícolas,
On Tue, Aug 24, 2021 at 04:56:19PM -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 V4L2 buffer slots) [1].
Sorry for getting to this so late. I was considering that I would rebase
this and send it again, since it's been so long; is that fine with you?
Thanks,
Paul
>
> [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 number of buffers to be allocated. But there's also the issue of
> reporting the minimum number of requests required by the pipeline handler, which
> was solved by creating a new MinimumRequests property.
>
> Briefly, patches 1 through 9 rework the core and pipeline handlers to use
> MinimumRequests and remove bufferCount, while patches 10 through 17 rework
> lc-compliance to add the new test and an additional test for MinimumRequests.
>
> Patch 1 adds the new MinimumRequests property to report the minimum number of
> requests needed by the pipeline handler.
>
> Patch 2 adds a count argument to allocate() so that the number of buffers to
> allocate needs to be specified, as it is no longer assumed through bufferCount.
>
> Patches 3-7 decouple the number of internal buffers and V4L2 buffer slots from
> bufferCount in each pipeline handler.
>
> Patch 8 reworks the V4L2 compatibility layer to not depend on bufferCount.
>
> Patch 9 removes bufferCount from the StreamConfiguration and everywhere it was
> still used, as it is no longer needed.
>
> Patch 10 fixes a file ordering issue in lc-compliance's meson.build.
>
> Patches 11-14 does some refactoring in lc-compliance in order to reduce code
> duplication.
>
> Patch 15 adds the test to lc-compliance.
>
> Patch 16 adds checks in lc-compliance to ensure that requests which failed to be
> enqueued are reported as test failure.
>
> Patch 17 adds another, very short, test to lc-compliance to make sure that the
> MinimumRequests property is set in the pipeline handler.
>
> I've run this new lc-compliance test on the raspberrypi, rkisp1, uvcvideo and
> vimc pipelines. raspberrypi already handles it well, while the other three run
> successfully after applying the series in [2]. The ipu3 should run fine as well
> since the series in [2] was based on the internal queue already present there.
> A patch for the simple pipeline is still pending.
>
> [2] https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022508.html
>
> The number of buffers to allocate in the lc-compliance test (patch 15) was
> hardcoded to 8 since more than that would cause errors when allocating.
>
> v7: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022577.html
> v6: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022356.html
> v5: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022098.html
> v4: https://lists.libcamera.org/pipermail/libcamera-devel/2021-May/020150.html
> v3: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019613.html
> v2: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019398.html
> v1: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019139.html
>
> Changes in v8:
> (thanks to Laurent and Kieran)
> - Changed internal buffer count constants for pipeline handlers to better values
> - Reordered patches to group non-lc-compliance changes together
> - Split buffer allocation changes into separate commits for each pipeline
> handler (patches 3-7)
> - Changed the MinimumRequests property meaning to require that frames aren't
> dropped
> - Set MinimumRequests on SimplePipeline depending on device and usage of
> converter
> - Undid definition of default MinimumRequests on CameraSensor constructor
> - Updated application-developer and pipeline-handler guides with new allocate()
> API and MinimumRequests property
> - Added handling for when allocate() returns less buffers than needed in cam and
> the capture unit test
> - Reworked buffer allocation handling in the raspberrypi pipeline handler
> - Moved V4L2 compatibility layer changes to separate commit
> - Added patch 10 to fix wrong file order in lc-compliance's meson.build
> - Added requests_ member to SimpleCapture to hold ownership of queued
> requests during capture
> - Moved CameraHolder to new test_base.{cpp,h} files
> - Fixed issue in UnbalancedStop test where requests cancelled due to stop() call
> were failing the test
> - Moved RequiredProperties test to property_test.cpp
> - Moved CameraTests to new test_base.{cpp,h} files
>
> Changes in v7:
> (thanks to Kieran and Jacopo)
> - Renamed property from MinNumRequests to MinimumRequests
> - Changed MinimumRequests property's description
> - Added patch 11 to test if MinimumRequests is valid
>
> Changes in v6:
> (thanks to Naushir)
> - Fixed style issues
> - Changed static_cast to unsigned int when comparing buffer count in
> lc-compliance
> - Added pipeline prefix to INTERNAL_BUFFER_COUNT and BUFFER_SLOT_COUNT constants
> - Removed comment from Raspberrypi MinNumRequests setting
> - Switched queueRequests()'s 'buffers' and 'requests' parameters order, since
> 'requests' is an output variable
> - Added comment to runCaptureSession()
>
> 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
> - Made sure that qcam allocates at least 2 buffers
>
> Changes in v4:
> (thanks to Laurent and Niklas)
> - Renamed QueueDepth property to MinNumRequests and better documented it
> - Changed patch 6 to also remove bufferCount from android
> - Added patch 3 to factor common code in lc-compliance
> - Added patch 5 to remove pipeline dependency on bufferCount
>
> 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
>
> Nícolas F. R. A. Prado (17):
> libcamera: property: Add MinimumRequests property
> libcamera: framebuffer_allocator: Make allocate() require count
> libcamera: pipeline: raspberrypi: Don't rely on bufferCount
> libcamera: pipeline: ipu3: Don't rely on bufferCount
> libcamera: pipeline: simple: Don't rely on bufferCount
> libcamera: pipeline: rkisp1: Don't rely on bufferCount
> libcamera: pipeline: vimc, uvcvideo: Don't rely on bufferCount
> v4l2: Allocate buffers based on requested count and MinimumRequests
> libcamera: stream: Remove bufferCount
> lc-compliance: Fix source file ordering in meson.build
> 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
> lc-compliance: Add test to ensure MinimumRequests is valid
>
> .../guides/application-developer.rst | 9 +-
> Documentation/guides/pipeline-handler.rst | 40 +++--
> 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/camera_session.cpp | 12 +-
> src/gstreamer/gstlibcameraallocator.cpp | 4 +-
> src/lc-compliance/capture_test.cpp | 90 ++++++++---
> src/lc-compliance/meson.build | 4 +-
> src/lc-compliance/property_test.cpp | 24 +++
> src/lc-compliance/simple_capture.cpp | 147 ++++++++++++------
> src/lc-compliance/simple_capture.h | 26 +++-
> src/lc-compliance/test_base.cpp | 38 +++++
> src/lc-compliance/test_base.h | 31 ++++
> src/libcamera/camera.cpp | 4 +-
> src/libcamera/framebuffer_allocator.cpp | 9 +-
> src/libcamera/pipeline/ipu3/cio2.cpp | 7 +-
> src/libcamera/pipeline/ipu3/cio2.h | 20 ++-
> src/libcamera/pipeline/ipu3/imgu.cpp | 12 +-
> src/libcamera/pipeline/ipu3/imgu.h | 15 +-
> src/libcamera/pipeline/ipu3/ipu3.cpp | 34 ++--
> .../pipeline/raspberrypi/raspberrypi.cpp | 57 +++----
> .../pipeline/raspberrypi/rpi_stream.cpp | 28 +++-
> .../pipeline/raspberrypi/rpi_stream.h | 24 ++-
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 +--
> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 5 +-
> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 4 +-
> src/libcamera/pipeline/simple/converter.cpp | 15 +-
> src/libcamera/pipeline/simple/converter.h | 9 +-
> src/libcamera/pipeline/simple/simple.cpp | 70 +++++++--
> 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 | 21 +++
> src/libcamera/stream.cpp | 12 +-
> src/qcam/main_window.cpp | 9 +-
> src/v4l2/v4l2_camera.cpp | 22 ++-
> src/v4l2/v4l2_camera.h | 5 +-
> src/v4l2/v4l2_camera_proxy.cpp | 11 +-
> test/camera/buffer_import.cpp | 10 +-
> test/camera/camera_reconfigure.cpp | 4 +-
> test/camera/capture.cpp | 6 +-
> 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 | 3 +-
> 49 files changed, 655 insertions(+), 275 deletions(-)
> create mode 100644 src/lc-compliance/property_test.cpp
> create mode 100644 src/lc-compliance/test_base.cpp
> create mode 100644 src/lc-compliance/test_base.h
>
> --
> 2.33.0
>
More information about the libcamera-devel
mailing list