[{"id":25956,"web_url":"https://patchwork.libcamera.org/comment/25956/","msgid":"<Y4iQhGx2K0n6CLWm@pyrite.rasen.tech>","date":"2022-12-01T11:31:16","subject":"Re: [libcamera-devel] [PATCH v8 15/17] lc-compliance: Add test to\n\tqueue more requests than hardware depth","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Aug 24, 2021 at 04:56:34PM -0300, Nícolas F. R. A. Prado wrote:\n> A pipeline handler should be able to handle an arbitrary number of\n> simultaneous requests by submitting what it can to the video device and\n> queuing the rest internally until resources are available. This isn't\n> currently done by some pipeline handlers however [1].\n> \n> Add a new test to lc-compliance that submits a lot of requests at once\n> to check if the pipeline handler is behaving well in this situation.\n> \n> [1] https://bugs.libcamera.org/show_bug.cgi?id=24\n> \n> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> \n> ---\n> \n> (no changes since v6)\n> \n> Changes in v6:\n> - Made MAX_SIMULTANEOUS_REQUESTS a constexpr\n> \n> Changes in v5:\n> - Updated to use googletest, and CameraHolder and roleToString from previous\n>   patches\n> \n>  src/lc-compliance/capture_test.cpp   | 45 ++++++++++++++++++++++++++++\n>  src/lc-compliance/simple_capture.cpp | 29 ++++++++++++++++++\n>  src/lc-compliance/simple_capture.h   | 11 +++++++\n>  3 files changed, 85 insertions(+)\n> \n> diff --git a/src/lc-compliance/capture_test.cpp b/src/lc-compliance/capture_test.cpp\n> index 40242fbbc0ba..2e495810e40f 100644\n> --- a/src/lc-compliance/capture_test.cpp\n> +++ b/src/lc-compliance/capture_test.cpp\n> @@ -18,6 +18,8 @@ using namespace libcamera;\n>  const std::vector<int> NUMREQUESTS = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };\n>  const std::vector<StreamRole> ROLES = { Raw, StillCapture, VideoRecording, Viewfinder };\n>  \n> +static constexpr unsigned int MAX_SIMULTANEOUS_REQUESTS = 8;\n> +\n>  static const std::string &roleToString(const StreamRole &role)\n>  {\n>  \tstatic const std::map<StreamRole, std::string> rolesMap = {\n> @@ -54,12 +56,37 @@ void SingleStream::TearDown()\n>  \treleaseCamera();\n>  }\n>  \n> +class RoleParametrizedTest : public testing::TestWithParam<StreamRole>, public CameraHolder\n> +{\n> +public:\n> +\tstatic std::string nameParameters(const testing::TestParamInfo<RoleParametrizedTest::ParamType> &info);\n> +\n> +protected:\n> +\tvoid SetUp() override;\n> +\tvoid TearDown() override;\n> +};\n> +\n> +void RoleParametrizedTest::SetUp()\n> +{\n> +\tacquireCamera();\n> +}\n> +\n> +void RoleParametrizedTest::TearDown()\n> +{\n> +\treleaseCamera();\n> +}\n> +\n>  std::string SingleStream::nameParameters(const testing::TestParamInfo<SingleStream::ParamType> &info)\n>  {\n>  \treturn roleToString(std::get<0>(info.param)) + \"_\" +\n>  \t       std::to_string(std::get<1>(info.param));\n>  }\n>  \n> +std::string RoleParametrizedTest::nameParameters(const testing::TestParamInfo<RoleParametrizedTest::ParamType> &info)\n> +{\n> +\treturn roleToString(info.param);\n> +}\n> +\n>  /*\n>   * Test single capture cycles\n>   *\n> @@ -122,8 +149,26 @@ TEST_P(SingleStream, UnbalancedStop)\n>  \tcapture.capture(numRequests);\n>  }\n>  \n> +TEST_P(RoleParametrizedTest, Overflow)\n> +{\n> +\tauto role = GetParam();\n> +\n> +\tSimpleCaptureOverflow capture(camera_);\n> +\n> +\tcapture.configure(role);\n> +\n> +\tcapture.allocateBuffers(MAX_SIMULTANEOUS_REQUESTS);\n> +\n> +\tcapture.capture();\n> +}\n> +\n>  INSTANTIATE_TEST_SUITE_P(CaptureTests,\n>  \t\t\t SingleStream,\n>  \t\t\t testing::Combine(testing::ValuesIn(ROLES),\n>  \t\t\t\t\t  testing::ValuesIn(NUMREQUESTS)),\n>  \t\t\t SingleStream::nameParameters);\n> +\n> +INSTANTIATE_TEST_SUITE_P(CaptureTests,\n> +\t\t\t RoleParametrizedTest,\n> +\t\t\t testing::ValuesIn(ROLES),\n> +\t\t\t RoleParametrizedTest::nameParameters);\n> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp\n> index e4f70974dd2c..d4805ff3403c 100644\n> --- a/src/lc-compliance/simple_capture.cpp\n> +++ b/src/lc-compliance/simple_capture.cpp\n> @@ -206,3 +206,32 @@ void SimpleCaptureUnbalanced::requestComplete(Request *request)\n>  \tif (camera_->queueRequest(request))\n>  \t\tloop_->exit(-EINVAL);\n>  }\n> +\n> +/* SimpleCaptureOverflow */\n> +\n> +SimpleCaptureOverflow::SimpleCaptureOverflow(std::shared_ptr<Camera> camera)\n> +\t: SimpleCapture(camera)\n> +{\n> +}\n> +\n> +void SimpleCaptureOverflow::capture()\n> +{\n> +\tstart();\n> +\n> +\tStream *stream = config_->at(0).stream();\n> +\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);\n> +\n> +\tcaptureCount_ = 0;\n> +\tcaptureLimit_ = buffers.size();\n> +\n> +\tqueueRequests(stream, buffers);\n> +\n> +\trunCaptureSession();\n> +\n> +\tASSERT_EQ(captureCount_, captureLimit_);\n> +}\n> +\n> +void SimpleCaptureOverflow::requestComplete([[maybe_unused]] Request *request)\n> +{\n> +\tcaptureCompleted();\n> +}\n> diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h\n> index 8a6db2a355f6..9d86fb32e9e0 100644\n> --- a/src/lc-compliance/simple_capture.h\n> +++ b/src/lc-compliance/simple_capture.h\n> @@ -69,4 +69,15 @@ private:\n>  \tvoid requestComplete(libcamera::Request *request) override;\n>  };\n>  \n> +class SimpleCaptureOverflow : public SimpleCapture\n> +{\n> +public:\n> +\tSimpleCaptureOverflow(std::shared_ptr<libcamera::Camera> camera);\n> +\n> +\tvoid capture();\n> +\n> +private:\n> +\tvoid requestComplete(libcamera::Request *request) override;\n> +};\n> +\n>  #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */\n> -- \n> 2.33.0\n>","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 016D4BE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  1 Dec 2022 11:31:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AE8606333F;\n\tThu,  1 Dec 2022 12:31:25 +0100 (CET)","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 075CF63335\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  1 Dec 2022 12:31:24 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4A3A633F;\n\tThu,  1 Dec 2022 12:31:22 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669894285;\n\tbh=V+n90mZD4CTesFamMkuMUlgkSLmo7l80zj0WAvL0Ygg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=v9IVlYtQUxaNmr4xgAxozjcwxAzTK91zjiZyOtEsfMyg6Ne07hIizkfaaQSzrRZYr\n\tdLQrNnBvVUP7Ban874JwnToioE/6sjj0r9ycwgx66y0BGMg7ZZ2qSkYtG3E9IZdMOo\n\tVL40gdw/JXnDw094D4NZaPd0jKYMglW+RjhAo6I73R9DiZOOcdg/QT9nCOxWRL1iOm\n\tMIE9YfCJRoxpkV8qSvhhBmwEa0G/Z+bNnwUPrmcWpJKVaFoc6VhgScRz6UCRHyXsY+\n\tNG1s+MuaKAa0tcv8sNgOonIzLsXDRnWMkavzmFWCDifTDNXsKpBXhttTb3ys9Mxgas\n\tUjKXKTm4H3S4w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669894283;\n\tbh=V+n90mZD4CTesFamMkuMUlgkSLmo7l80zj0WAvL0Ygg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Jm7TbQFa3ze1Xe2uLrLEQLaqfwy4tXki6wU38cmBl1SzwebhcXr6q43bNcurWYjQH\n\tttpcdsQaaaUyXXCP+CNgSv8u7YJIz6tDsSEicTIXzGf/RsVc/F2orms+mGA3qw9ln6\n\t68M5TCgYfAYlt4pV/zzQoNwYesLUSa9bwtBFHdQU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Jm7TbQFa\"; dkim-atps=neutral","Date":"Thu, 1 Dec 2022 20:31:16 +0900","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado <nfraprado@collabora.com>","Message-ID":"<Y4iQhGx2K0n6CLWm@pyrite.rasen.tech>","References":"<20210824195636.1110845-1-nfraprado@collabora.com>\n\t<20210824195636.1110845-16-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210824195636.1110845-16-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v8 15/17] 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":"<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>","From":"Paul Elder via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida <andrealmeid@collabora.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]