[libcamera-devel] [PATCH v7 00/11] lc-compliance: Add test to queue more requests than hardware depth
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Aug 1 23:23:09 CEST 2021
Hi Nícolas,
Thank you for the patch.
On Thu, Jul 22, 2021 at 08:28:40PM -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].
>
> [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.
It's now called MinimumRequests :-)
> 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.
Would it be possible to move patches 8 and 9 right after 2, to separate
the libcamera changes from the lc-compliance changes ? It doesn't seem
to cause any conflict.
> Patch 11 adds a test to verify that the MinimumRequests property added is set
> and valid 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.
>
> [2] https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022508.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 v7:
> - Thanks to Kieran:
> - Renamed property from MinNumRequests to MinimumRequests (patches 1, 2, 3, 10)
> - Added patch 11 to test if MinimumRequests is valid
> - Thanks to Jacopo:
> - Changed description of MinimumRequests
>
> v6: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022356.html
>
> 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 (11):
> libcamera: property: Add MinimumRequests 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
> lc-compliance: Add test to ensure MinimumRequests is valid
>
> 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 | 138 +++++++++++++---
> src/lc-compliance/simple_capture.cpp | 150 +++++++++++++-----
> src/lc-compliance/simple_capture.h | 27 +++-
> src/libcamera/camera.cpp | 4 +-
> src/libcamera/camera_sensor.cpp | 1 +
> 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 | 21 +--
> .../pipeline/raspberrypi/raspberrypi.cpp | 25 +--
> 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 | 10 ++
> 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 +-
> 40 files changed, 389 insertions(+), 206 deletions(-)
>
> --
> 2.32.0
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list