Cover Letter Detail
Show a cover letter.
GET /api/1.1/covers/23272/?format=api
{ "id": 23272, "url": "https://patchwork.libcamera.org/api/1.1/covers/23272/?format=api", "web_url": "https://patchwork.libcamera.org/cover/23272/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20250428090413.38234-1-s.pueschel@pengutronix.de>", "date": "2025-04-28T09:02:25", "name": "[v11,00/19] lc-compliance: Add test to queue more requests than hardware depth", "submitter": { "id": 225, "url": "https://patchwork.libcamera.org/api/1.1/people/225/?format=api", "name": "Sven Püschel", "email": "s.pueschel@pengutronix.de" }, "mbox": "https://patchwork.libcamera.org/cover/23272/mbox/", "series": [ { "id": 5148, "url": "https://patchwork.libcamera.org/api/1.1/series/5148/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5148", "date": "2025-04-28T09:02:25", "name": "lc-compliance: Add test to queue more requests than hardware depth", "version": 11, "mbox": "https://patchwork.libcamera.org/series/5148/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/covers/23272/comments/", "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 120FDC327D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Apr 2025 09:04:54 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0C11768AD0;\n\tMon, 28 Apr 2025 11:04:53 +0200 (CEST)", "from metis.whiteo.stw.pengutronix.de\n\t(metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 67B49617DC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Apr 2025 11:04:51 +0200 (CEST)", "from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77]\n\thelo=peter.guest.stw.pengutronix.de)\n\tby metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92)\n\t(envelope-from <s.pueschel@pengutronix.de>)\n\tid 1u9KPy-0001au-Tr; Mon, 28 Apr 2025 11:04:50 +0200" ], "From": "=?utf-8?q?Sven_P=C3=BCschel?= <s.pueschel@pengutronix.de>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "=?utf-8?q?Sven_P=C3=BCschel?= <s.pueschel@pengutronix.de>", "Subject": "[PATCH v11 00/19] lc-compliance: Add test to queue more requests\n\tthan hardware depth", "Date": "Mon, 28 Apr 2025 11:02:25 +0200", "Message-ID": "<20250428090413.38234-1-s.pueschel@pengutronix.de>", "X-Mailer": "git-send-email 2.49.0", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "X-SA-Exim-Connect-IP": "2a0a:edc0:0:900:1d::77", "X-SA-Exim-Mail-From": "s.pueschel@pengutronix.de", "X-SA-Exim-Scanned": "No (on metis.whiteo.stw.pengutronix.de);\n\tSAEximRunCond expanded to false", "X-PTX-Original-Recipient": "libcamera-devel@lists.libcamera.org", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The purpose of this series is to add a new test to lc-compliance that tests\nqueuing a lot of requests at once in order to ensure that pipeline handlers are\nable to handle more requests than they have resources for (like internal buffers\nand V4L2 buffer slots) [1].\n\n[1] https://bugs.libcamera.org/show_bug.cgi?id=24\n\nIn order to achieve this, the FrameBufferAllocator had to be adapted to allow an\narbitrary number of buffers to be allocated. But there's also the issue of\nreporting the minimum number of requests required by the pipeline handler, which\nwas solved by creating a new MinimumRequests property.\n\nBriefly, patches 1 through 11 rework the core and pipeline handlers to use\nMinimumRequests and remove bufferCount. Patches 12 through 15 rework\nlc-compliance to add the new test and an additional test for MinimumRequests.\nPatches 16 through 19 integrate the request queue patches for v4l2 and rkisp1.\n\nPatch 1 adds the new MinimumRequests property to report the minimum number of\nrequests needed by the pipeline handler.\n\nPatch 2 adds a count argument to allocate() so that the number of buffers to\nallocate needs to be specified, as it is no longer assumed through bufferCount.\n\nPatches 3-9 decouple the number of internal buffers and V4L2 buffer slots from\nbufferCount in each pipeline handler.\n\nPatch 10 reworks the V4L2 compatibility layer to not depend on bufferCount.\n\nPatch 11 removes bufferCount from the StreamConfiguration and everywhere it was\nstill used, as it is no longer needed.\n\nPatches 12-13 does some refactoring in lc-compliance in order to reduce code\nduplication.\n\nPatch 14 adds the test to lc-compliance. The number of buffers to allocate was\nhardcoded to 8 since more than that would cause errors when allocating.\n\nPatch 15 adds another, very short, test to lc-compliance to make sure that the\nMinimumRequests property is set in the pipeline handler.\n\nPatches 16 and 17 add request queues to the rkisp1 and uvcvideo handler.\nThese were originally submitted in a separate patchset [2], but linked to this\npatchset, as they avoid canceling requests when too many buffers are queued.\nAs these are not strictly necessary for this patchset, I've only ported\nthe rkisp1 and uvcvideo patches, as I was able to test them\n(rkisp1 only against libcamera 0.3.2).\n\n[2] https://lists.libcamera.org/pipermail/libcamera-devel/2021-September/024121.html\n\nPatch 18 documents the request queue pattern.\n\nPatch 19 improves the v4l2 error to also include the amount of buffers\nrequested and how many were actually allocated by v4l2.\n\n\nGiven that the allocation function requires an unsigned amount of buffers to\nallocate, the MinimumRequests property was changed to an unsigned integer\nto avoid potential signedness warnings in applications when they adapt to the\nMinimumRequests property.\n\nv10: https://lists.libcamera.org/pipermail/libcamera-devel/2022-December/036255.html\nv9: https://lists.libcamera.org/pipermail/libcamera-devel/2022-December/036104.html\nv8: https://lists.libcamera.org/pipermail/libcamera-devel/2021-August/023773.html\nv7: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022577.html\nv6: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022356.html\nv5: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022098.html\nv4: https://lists.libcamera.org/pipermail/libcamera-devel/2021-May/020150.html\nv3: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019613.html\nv2: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019398.html\nv1: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019139.html\n\nChanges in v11:\n- picked up and rebased by Sven\n- add mali-c55, dcmipp, virtual, j721e, intel-ipu6 and mtk-seninf suppport\n- Changed MinimumRequests property type from int32_t -> uint32_t\n- Integrated internal request queue patches for v4l2 and rkisp1\n\nChanges in v10:\n- add imx8-isi, pycamera, and android\n- update MinimumRequests description and pipeline hander guide\n- clean up raspberry pi and ipu3 (compared to the last version)\n- rename variables in simple\n\nChanges in v9:\n- picked up and rebased by Paul\n\nChanges in v8:\n(thanks to Laurent and Kieran)\n- Changed internal buffer count constants for pipeline handlers to better values\n- Reordered patches to group non-lc-compliance changes together\n- Split buffer allocation changes into separate commits for each pipeline\n handler (patches 3-7)\n- Changed the MinimumRequests property meaning to require that frames aren't\n dropped\n- Set MinimumRequests on SimplePipeline depending on device and usage of\n converter\n- Undid definition of default MinimumRequests on CameraSensor constructor\n- Updated application-developer and pipeline-handler guides with new allocate()\n API and MinimumRequests property\n- Added handling for when allocate() returns less buffers than needed in cam and\n the capture unit test\n- Reworked buffer allocation handling in the raspberrypi pipeline handler\n- Moved V4L2 compatibility layer changes to separate commit\n- Added patch 10 to fix wrong file order in lc-compliance's meson.build\n- Added requests_ member to SimpleCapture to hold ownership of queued\n requests during capture\n- Moved CameraHolder to new test_base.{cpp,h} files\n- Fixed issue in UnbalancedStop test where requests cancelled due to stop() call\n were failing the test\n- Moved RequiredProperties test to property_test.cpp\n- Moved CameraTests to new test_base.{cpp,h} files\n\nChanges in v7:\n(thanks to Kieran and Jacopo)\n- Renamed property from MinNumRequests to MinimumRequests\n- Changed MinimumRequests property's description\n- Added patch 11 to test if MinimumRequests is valid\n\nChanges in v6:\n(thanks to Naushir)\n- Fixed style issues\n- Changed static_cast to unsigned int when comparing buffer count in\n lc-compliance\n- Added pipeline prefix to INTERNAL_BUFFER_COUNT and BUFFER_SLOT_COUNT constants\n- Removed comment from Raspberrypi MinNumRequests setting\n- Switched queueRequests()'s 'buffers' and 'requests' parameters order, since\n 'requests' is an output variable\n- Added comment to runCaptureSession()\n\nChanges in v5:\n- Rebased on master (now that lc-compliance was refactored to use Googletest)\n- Added patches 3, 5, 6 and 8\n- Fixed qcam to use at least two buffers\n- Made sure that qcam allocates at least 2 buffers\n\nChanges in v4:\n(thanks to Laurent and Niklas)\n- Renamed QueueDepth property to MinNumRequests and better documented it\n- Changed patch 6 to also remove bufferCount from android\n- Added patch 3 to factor common code in lc-compliance\n- Added patch 5 to remove pipeline dependency on bufferCount\n\nChanges in v3:\n- Added patches 1 and 4 to add the QueueDepth property and remove bufferCount\n- Made the count argument required in patch 2\n- Added previously missing changes to the gstreamer and V4L2 compatibility\n layers\n\nChanges in v2:\n- Renamed and reworded commits and series\n- Dropped patches 2 and 3, which were hacks to test, and added patch 1 to add\n count to FrameBufferAllocator\n- Thanks to Niklas:\n - Created new standalone test instead of looping over the other tests\n\nNícolas F. R. A. Prado (16):\n libcamera: property: Add MinimumRequests property\n libcamera: framebuffer_allocator: Make allocate() require count\n libcamera: pipeline: rpi: Don't rely on bufferCount\n libcamera: pipeline: ipu3: Don't rely on bufferCount\n libcamera: pipeline: rkisp1: Don't rely on bufferCount\n libcamera: pipeline: simple: Don't rely on bufferCount\n libcamera: pipeline: vimc, uvcvideo: Don't rely on bufferCount\n v4l2: Allocate buffers based on requested count and MinimumRequests\n libcamera: stream: Remove bufferCount\n lc-compliance: Move buffer allocation to separate function\n lc-compliance: Move camera setup to CameraHolder class\n lc-compliance: Add test to queue more requests than hardware depth\n lc-compliance: Add test to ensure MinimumRequests is valid\n libcamera: pipeline: uvcvideo: Add internal request queue\n libcamera: pipeline: rkisp1: Add internal request queue\n Documentation: guides: pipeline-handler: Document internal queue\n pattern\n\nPaul Elder (1):\n libcamera: pipeline: imx8-isi: Don't rely on bufferCount\n\nSven Püschel (2):\n libcamera: pipeline: mali-c55: Don't rely on bufferCount\n libcamera: v4l2_videodevice: Log buffer count on error\n\n .../guides/application-developer.rst | 9 +-\n Documentation/guides/pipeline-handler.rst | 167 +++++++++++++-----\n include/libcamera/camera.h | 2 +-\n include/libcamera/framebuffer_allocator.h | 2 +-\n include/libcamera/internal/converter.h | 2 +-\n .../internal/converter/converter_v4l2_m2m.h | 7 +-\n include/libcamera/internal/pipeline_handler.h | 2 +-\n include/libcamera/stream.h | 2 -\n src/android/camera_stream.cpp | 5 +-\n src/apps/cam/camera_session.cpp | 13 +-\n src/apps/lc-compliance/helpers/capture.cpp | 49 +++--\n src/apps/lc-compliance/helpers/capture.h | 2 +\n src/apps/lc-compliance/meson.build | 2 +\n src/apps/lc-compliance/property_test.cpp | 24 +++\n src/apps/lc-compliance/test_base.cpp | 38 ++++\n src/apps/lc-compliance/test_base.h | 31 ++++\n src/apps/lc-compliance/tests/capture_test.cpp | 81 +++++++--\n src/apps/qcam/main_window.cpp | 10 +-\n src/gstreamer/gstlibcameraallocator.cpp | 6 +-\n src/libcamera/camera.cpp | 4 +-\n src/libcamera/converter.cpp | 2 +\n .../converter/converter_v4l2_m2m.cpp | 15 +-\n src/libcamera/framebuffer_allocator.cpp | 9 +-\n src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 14 +-\n src/libcamera/pipeline/ipu3/cio2.cpp | 8 +-\n src/libcamera/pipeline/ipu3/cio2.h | 4 +-\n src/libcamera/pipeline/ipu3/imgu.cpp | 13 +-\n src/libcamera/pipeline/ipu3/imgu.h | 3 +-\n src/libcamera/pipeline/ipu3/ipu3.cpp | 59 ++++---\n src/libcamera/pipeline/mali-c55/mali-c55.cpp | 27 +--\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 109 ++++++++----\n src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 5 +-\n src/libcamera/pipeline/rkisp1/rkisp1_path.h | 4 +-\n .../pipeline/rpi/common/pipeline_base.cpp | 10 +-\n .../pipeline/rpi/common/pipeline_base.h | 2 +-\n src/libcamera/pipeline/rpi/pisp/pisp.cpp | 2 +-\n src/libcamera/pipeline/rpi/vc4/vc4.cpp | 2 +-\n src/libcamera/pipeline/simple/simple.cpp | 72 +++++---\n src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 120 ++++++++++---\n src/libcamera/pipeline/vimc/vimc.cpp | 17 +-\n src/libcamera/pipeline/virtual/virtual.cpp | 11 +-\n src/libcamera/pipeline_handler.cpp | 1 +\n src/libcamera/property_ids_core.yaml | 22 +++\n src/libcamera/stream.cpp | 12 +-\n src/libcamera/v4l2_videodevice.cpp | 3 +-\n src/py/libcamera/py_main.cpp | 5 +-\n src/v4l2/v4l2_camera.cpp | 21 ++-\n src/v4l2/v4l2_camera.h | 5 +-\n src/v4l2/v4l2_camera_proxy.cpp | 10 +-\n test/camera/buffer_import.cpp | 9 +-\n test/camera/camera_reconfigure.cpp | 5 +-\n test/camera/capture.cpp | 11 +-\n test/camera/statemachine.cpp | 5 +-\n test/fence.cpp | 6 +-\n test/libtest/buffer_source.cpp | 4 +-\n test/libtest/buffer_source.h | 2 +-\n test/mapped-buffer.cpp | 5 +-\n test/v4l2_videodevice/buffer_cache.cpp | 3 +-\n 58 files changed, 771 insertions(+), 324 deletions(-)\n create mode 100644 src/apps/lc-compliance/property_test.cpp\n create mode 100644 src/apps/lc-compliance/test_base.cpp\n create mode 100644 src/apps/lc-compliance/test_base.h" }