[{"id":16543,"web_url":"https://patchwork.libcamera.org/comment/16543/","msgid":"","date":"2021-04-26T01:07:43","subject":"Re: [libcamera-devel] [PATCH v3 0/4] lc-compliance: Add test to\n\tqueue more requests than hardware depth","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nícolas,\n\nThank you for the patches.\n\nOn Wed, Apr 21, 2021 at 01:51:35PM -0300, Nícolas F. R. A. Prado wrote:\n> The purpose of this series is to add a new test to lc-compliance that tests\n> queuing a lot of requests at once in order to ensure that pipeline handlers are\n> able to handle more requests than they have resources for (like internal buffers\n> and buffers in the videodev) [1].\n> \n> In order to achieve this, the FrameBufferAllocator had to be adapted in order to\n> allow an arbitrary amount of buffers to be allocated. But there's also the issue\n> of reporting the minimum amount of requests required by the pipeline handler,\n> which was solved by creating a new QueueDepth property.\n> \n> So patch 1 adds the new QueueDepth property to report the minimum amount of\n> requests needed by the pipeline handler.\n> \n> Patch 2 adds a count argument to allocate() so that the amount of buffers to\n> allocate needs to be specified, as it is no longer assumed through bufferCount.\n> \n> Patch 3 adds the test to lc-compliance.\n> \n> Patch 4 removes bufferCount from the StreamConfiguration as it is no longer\n> needed.\n> \n> The amount of buffers to allocate in the lc-compliance test (patch 3) was\n> hardcoded to 8 since more than that would cause errors when allocating.\n\nWe could probably go for less than 8, I'll comment on that in patch 3/4.\n\n> Hirokazu,\n> since you have a patch series addressing this issue on the IPU3\n> pipeline [2], could you test this series? If both my and your patch series are\n> working, lc-compliance should crash before applying your series but run through\n> completion after applying it.\n\nThat's a perfect use of tests :-)\n\n> Changes 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> \n> Changes 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> \n> [1] https://bugs.libcamera.org/show_bug.cgi?id=24\n> [2] https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019108.html\n> \n> v1: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019139.html\n> v2: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019398.html\n> \n> Nícolas F. R. A. Prado (4):\n> libcamera: property: Add QueueDepth property\n> libcamera: framebuffer_allocator: Make allocate() require count\n> lc-compliance: Add test to queue more requests than hardware depth\n> libcamera: stream: Remove bufferCount\n> \n> include/libcamera/camera.h | 2 +-\n> include/libcamera/framebuffer_allocator.h | 2 +-\n> include/libcamera/internal/pipeline_handler.h | 2 +-\n> include/libcamera/stream.h | 2 -\n> src/cam/capture.cpp | 10 +--\n> src/gstreamer/gstlibcameraallocator.cpp | 4 +-\n> src/lc-compliance/simple_capture.cpp | 83 ++++++++++++++++++-\n> src/lc-compliance/simple_capture.h | 16 ++++\n> src/lc-compliance/single_stream.cpp | 37 ++++++++-\n> src/libcamera/camera.cpp | 4 +-\n> src/libcamera/framebuffer_allocator.cpp | 5 +-\n> src/libcamera/pipeline/ipu3/cio2.cpp | 1 -\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 23 ++---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 17 ++--\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 15 ++--\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 -\n> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 4 +-\n> src/libcamera/pipeline/simple/converter.cpp | 7 +-\n> src/libcamera/pipeline/simple/converter.h | 5 +-\n> src/libcamera/pipeline/simple/simple.cpp | 19 ++---\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 14 ++--\n> src/libcamera/pipeline/vimc/vimc.cpp | 15 ++--\n> src/libcamera/pipeline_handler.cpp | 1 +\n> src/libcamera/property_ids.yaml | 5 ++\n> src/libcamera/stream.cpp | 9 +-\n> src/qcam/main_window.cpp | 4 +-\n> src/v4l2/v4l2_camera.cpp | 16 ++--\n> src/v4l2/v4l2_camera.h | 5 +-\n> src/v4l2/v4l2_camera_proxy.cpp | 8 +-\n> test/camera/buffer_import.cpp | 10 ++-\n> test/camera/capture.cpp | 4 +-\n> test/camera/statemachine.cpp | 4 +-\n> test/libtest/buffer_source.cpp | 4 +-\n> test/libtest/buffer_source.h | 2 +-\n> test/mapped-buffer.cpp | 4 +-\n> test/v4l2_videodevice/buffer_cache.cpp | 4 +-\n> 36 files changed, 246 insertions(+), 123 deletions(-)","headers":{"Return-Path":"","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 3139FBDC91\n\tfor ;\n\tMon, 26 Apr 2021 01:07:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8B2BB6887D;\n\tMon, 26 Apr 2021 03:07:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 098E6602D1\n\tfor ;\n\tMon, 26 Apr 2021 03:07:49 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5DAF74FB;\n\tMon, 26 Apr 2021 03:07:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qmqje3IP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619399268;\n\tbh=u0MeFmNmz+xDRXOzKtY30vPFlV+mI2gWpdOMlKpyueo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qmqje3IPwZlZNxZjDW3OkDatQbEjMfcC/yfOIAYw9cAai3WAzKaZhH3C6oc8RtynE\n\tEWwwvSKa7foufqmE3yx2RSAhsWuItfU/R93Ia/KmrMvUA3d4mA0Gnx8haxmYFgfb8O\n\t3Jj5DZMmmSYhB6ddGt0arwLeiu8Z0Ku5V+Ci/vJ8=","Date":"Mon, 26 Apr 2021 04:07:43 +0300","From":"Laurent Pinchart ","To":"=?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4=?= Prado ","Message-ID":"","References":"<20210421165139.318432-1-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210421165139.318432-1-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v3 0/4] lc-compliance: Add test to\n\tqueue more requests than hardware depth","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, =?utf-8?b?QW5kcsOp?=\n\tAlmeida , kernel@collabora.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]