From patchwork Thu Jul 22 23:28:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13074 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 8910AC0109 for ; Thu, 22 Jul 2021 23:29:26 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DE1D6687A4; Fri, 23 Jul 2021 01:29:25 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0267F60276 for ; Fri, 23 Jul 2021 01:29:23 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2434:a4c5:aa5e:1d8c:5e2c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 9AA521F44760; Fri, 23 Jul 2021 00:29:21 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 22 Jul 2021 20:28:40 -0300 Message-Id: <20210722232851.747614-1-nfraprado@collabora.com> X-Mailer: git-send-email 2.32.0 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 00/11] lc-compliance: Add test to queue more requests than hardware depth X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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. 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(-)