[{"id":33630,"web_url":"https://patchwork.libcamera.org/comment/33630/","msgid":"","date":"2025-03-18T08:49:48","subject":"Re: [PATCH v4 2/2] apps: lc-compliance: Add multi-stream tests","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Fri, Mar 14, 2025 at 06:42:19PM +0100, Barnabás Pőcze wrote:\n> Rename the `SingleStream` test to `SimpleCapture`, and extend it\n> to support using multiple roles. And instantiate another test suite\n> from the `SimpleCapture` test that tests multiple streams in one\n> capture session.\n>\n> Signed-off-by: Barnabás Pőcze \n> Reviewed-by: Paul Elder \n> Reviewed-by: Jacopo Mondi \n\nMy tag stands.\n\nOn top of this we can probably\n1) Rename SimpleCapture as \"Simple\" doesn't tell much\n2) Rework the number of requests that we iterate on as this seems\nrather random\n\nBut for now, feel free to push these changes!\n\n> ---\n> src/apps/lc-compliance/tests/capture_test.cpp | 88 +++++++++++--------\n> 1 file changed, 51 insertions(+), 37 deletions(-)\n>\n> diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp\n> index 06f15bdb4..d02caa8a1 100644\n> --- a/src/apps/lc-compliance/tests/capture_test.cpp\n> +++ b/src/apps/lc-compliance/tests/capture_test.cpp\n> @@ -8,7 +8,10 @@\n>\n> #include \"capture.h\"\n>\n> -#include \n> +#include \n> +#include \n> +#include \n> +#include \n>\n> #include \n>\n> @@ -18,19 +21,10 @@ namespace {\n>\n> using namespace libcamera;\n>\n> -const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };\n> -\n> -const StreamRole ROLES[] = {\n> -\tStreamRole::Raw,\n> -\tStreamRole::StillCapture,\n> -\tStreamRole::VideoRecording,\n> -\tStreamRole::Viewfinder\n> -};\n> -\n> -class SingleStream : public testing::TestWithParam>\n> +class SimpleCapture : public testing::TestWithParam, int>>\n> {\n> public:\n> -\tstatic std::string nameParameters(const testing::TestParamInfo &info);\n> +\tstatic std::string nameParameters(const testing::TestParamInfo &info);\n>\n> protected:\n> \tvoid SetUp() override;\n> @@ -43,7 +37,7 @@ protected:\n> * We use gtest's SetUp() and TearDown() instead of constructor and destructor\n> * in order to be able to assert on them.\n> */\n> -void SingleStream::SetUp()\n> +void SimpleCapture::SetUp()\n> {\n> \tEnvironment *env = Environment::get();\n>\n> @@ -52,7 +46,7 @@ void SingleStream::SetUp()\n> \tASSERT_EQ(camera_->acquire(), 0);\n> }\n>\n> -void SingleStream::TearDown()\n> +void SimpleCapture::TearDown()\n> {\n> \tif (!camera_)\n> \t\treturn;\n> @@ -61,19 +55,17 @@ void SingleStream::TearDown()\n> \tcamera_.reset();\n> }\n>\n> -std::string SingleStream::nameParameters(const testing::TestParamInfo &info)\n> +std::string SimpleCapture::nameParameters(const testing::TestParamInfo &info)\n> {\n> -\tstd::map rolesMap = {\n> -\t\t{ StreamRole::Raw, \"Raw\" },\n> -\t\t{ StreamRole::StillCapture, \"StillCapture\" },\n> -\t\t{ StreamRole::VideoRecording, \"VideoRecording\" },\n> -\t\t{ StreamRole::Viewfinder, \"Viewfinder\" }\n> -\t};\n> +\tconst auto &[roles, numRequests] = info.param;\n> +\tstd::ostringstream ss;\n>\n> -\tstd::string roleName = rolesMap[std::get<0>(info.param)];\n> -\tstd::string numRequestsName = std::to_string(std::get<1>(info.param));\n> +\tfor (StreamRole r : roles)\n> +\t\tss << r << '_';\n>\n> -\treturn roleName + \"_\" + numRequestsName;\n> +\tss << '_' << numRequests;\n> +\n> +\treturn ss.str();\n> }\n>\n> /*\n> @@ -83,13 +75,13 @@ std::string SingleStream::nameParameters(const testing::TestParamInfo * failure is a camera that completes less requests than the number of requests\n> * queued.\n> */\n> -TEST_P(SingleStream, Capture)\n> +TEST_P(SimpleCapture, Capture)\n> {\n> -\tauto [role, numRequests] = GetParam();\n> +\tconst auto &[roles, numRequests] = GetParam();\n>\n> \tCapture capture(camera_);\n>\n> -\tcapture.configure({ { role } });\n> +\tcapture.configure(roles);\n>\n> \tcapture.run(numRequests, numRequests);\n> }\n> @@ -101,14 +93,14 @@ TEST_P(SingleStream, Capture)\n> * a camera that does not clean up correctly in its error path but is only\n> * tested by single-capture applications.\n> */\n> -TEST_P(SingleStream, CaptureStartStop)\n> +TEST_P(SimpleCapture, CaptureStartStop)\n> {\n> -\tauto [role, numRequests] = GetParam();\n> +\tconst auto &[roles, numRequests] = GetParam();\n> \tunsigned int numRepeats = 3;\n>\n> \tCapture capture(camera_);\n>\n> -\tcapture.configure({ { role } });\n> +\tcapture.configure(roles);\n>\n> \tfor (unsigned int starts = 0; starts < numRepeats; starts++)\n> \t\tcapture.run(numRequests, numRequests);\n> @@ -121,21 +113,43 @@ TEST_P(SingleStream, CaptureStartStop)\n> * is a camera that does not handle cancelation of buffers coming back from the\n> * video device while stopping.\n> */\n> -TEST_P(SingleStream, UnbalancedStop)\n> +TEST_P(SimpleCapture, UnbalancedStop)\n> {\n> -\tauto [role, numRequests] = GetParam();\n> +\tconst auto &[roles, numRequests] = GetParam();\n>\n> \tCapture capture(camera_);\n>\n> -\tcapture.configure({ { role } });\n> +\tcapture.configure(roles);\n>\n> \tcapture.run(numRequests);\n> }\n>\n> -INSTANTIATE_TEST_SUITE_P(CaptureTests,\n> -\t\t\t SingleStream,\n> -\t\t\t testing::Combine(testing::ValuesIn(ROLES),\n> +const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };\n> +\n> +const std::vector SINGLEROLES[] = {\n> +\t{ StreamRole::Raw, },\n> +\t{ StreamRole::StillCapture, },\n> +\t{ StreamRole::VideoRecording, },\n> +\t{ StreamRole::Viewfinder, },\n> +};\n> +\n> +const std::vector MULTIROLES[] = {\n> +\t{ StreamRole::Raw, StreamRole::StillCapture },\n> +\t{ StreamRole::Raw, StreamRole::VideoRecording },\n> +\t{ StreamRole::StillCapture, StreamRole::VideoRecording },\n> +\t{ StreamRole::VideoRecording, StreamRole::VideoRecording },\n> +};\n> +\n> +INSTANTIATE_TEST_SUITE_P(SingleStream,\n> +\t\t\t SimpleCapture,\n> +\t\t\t testing::Combine(testing::ValuesIn(SINGLEROLES),\n> +\t\t\t\t\t testing::ValuesIn(NUMREQUESTS)),\n> +\t\t\t SimpleCapture::nameParameters);\n> +\n> +INSTANTIATE_TEST_SUITE_P(MultiStream,\n> +\t\t\t SimpleCapture,\n> +\t\t\t testing::Combine(testing::ValuesIn(MULTIROLES),\n> \t\t\t\t\t testing::ValuesIn(NUMREQUESTS)),\n> -\t\t\t SingleStream::nameParameters);\n> +\t\t\t SimpleCapture::nameParameters);\n>\n> } /* namespace */\n> --\n> 2.48.1\n>","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 DB645BD1F1\n\tfor ;\n\tTue, 18 Mar 2025 08:49:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E109C68947;\n\tTue, 18 Mar 2025 09:49:53 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C60B4687FA\n\tfor ;\n\tTue, 18 Mar 2025 09:49:51 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8D60F109A;\n\tTue, 18 Mar 2025 09:48:09 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mB7N+9iy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742287689;\n\tbh=UvCU7hT041lArt8g0pOVwzpixhwSf88sTCvz/jbLN64=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mB7N+9iyDRy6hPCz2M/QYJk4mvlSktvfK8WiwRIF6LkLA0YeAaG15SSstTS3v6Ux0\n\tCZz8mXtWpAHfGWzmtjoPuBhsJr5AgVmcy46obUlrk1eQr2230HojiMhv2lICY290l/\n\tUmqK8e/dpJPaiHCKg+D2MQp+XOnTyfyhbAQ3oCC4=","Date":"Tue, 18 Mar 2025 09:49:48 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, \n\tPaul Elder ,\n\tJacopo Mondi ","Subject":"Re: [PATCH v4 2/2] apps: lc-compliance: Add multi-stream tests","Message-ID":"","References":"<20250314174220.1015597-1-barnabas.pocze@ideasonboard.com>\n\t<20250314174220.1015597-3-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250314174220.1015597-3-barnabas.pocze@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33675,"web_url":"https://patchwork.libcamera.org/comment/33675/","msgid":"","date":"2025-03-21T08:12:00","subject":"Re: [PATCH v4 2/2] apps: lc-compliance: Add multi-stream tests","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 03. 18. 9:49 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Fri, Mar 14, 2025 at 06:42:19PM +0100, Barnabás Pőcze wrote:\n>> Rename the `SingleStream` test to `SimpleCapture`, and extend it\n>> to support using multiple roles. And instantiate another test suite\n>> from the `SimpleCapture` test that tests multiple streams in one\n>> capture session.\n>>\n>> Signed-off-by: Barnabás Pőcze \n>> Reviewed-by: Paul Elder \n>> Reviewed-by: Jacopo Mondi \n> \n> My tag stands.\n> \n> On top of this we can probably\n> 1) Rename SimpleCapture as \"Simple\" doesn't tell much\n\nYes, but it can't be called \"SingleCapture\" anymore, and the\n\"Capture\" name is already taken by the test.\n\n\n> 2) Rework the number of requests that we iterate on as this seems\n> rather random\n\nI preserved the earlier values; by the way, it's the Fibonacci sequence.\nBut maybe they should indeed be more targeted.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> But for now, feel free to push these changes!\n> \n>> ---\n>> src/apps/lc-compliance/tests/capture_test.cpp | 88 +++++++++++--------\n>> 1 file changed, 51 insertions(+), 37 deletions(-)\n>>\n>> diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp\n>> index 06f15bdb4..d02caa8a1 100644\n>> --- a/src/apps/lc-compliance/tests/capture_test.cpp\n>> +++ b/src/apps/lc-compliance/tests/capture_test.cpp\n>> @@ -8,7 +8,10 @@\n>>\n>> #include \"capture.h\"\n>>\n>> -#include \n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>>\n>> #include \n>>\n>> @@ -18,19 +21,10 @@ namespace {\n>>\n>> using namespace libcamera;\n>>\n>> -const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };\n>> -\n>> -const StreamRole ROLES[] = {\n>> -\tStreamRole::Raw,\n>> -\tStreamRole::StillCapture,\n>> -\tStreamRole::VideoRecording,\n>> -\tStreamRole::Viewfinder\n>> -};\n>> -\n>> -class SingleStream : public testing::TestWithParam>\n>> +class SimpleCapture : public testing::TestWithParam, int>>\n>> {\n>> public:\n>> -\tstatic std::string nameParameters(const testing::TestParamInfo &info);\n>> +\tstatic std::string nameParameters(const testing::TestParamInfo &info);\n>>\n>> protected:\n>> \tvoid SetUp() override;\n>> @@ -43,7 +37,7 @@ protected:\n>> * We use gtest's SetUp() and TearDown() instead of constructor and destructor\n>> * in order to be able to assert on them.\n>> */\n>> -void SingleStream::SetUp()\n>> +void SimpleCapture::SetUp()\n>> {\n>> \tEnvironment *env = Environment::get();\n>>\n>> @@ -52,7 +46,7 @@ void SingleStream::SetUp()\n>> \tASSERT_EQ(camera_->acquire(), 0);\n>> }\n>>\n>> -void SingleStream::TearDown()\n>> +void SimpleCapture::TearDown()\n>> {\n>> \tif (!camera_)\n>> \t\treturn;\n>> @@ -61,19 +55,17 @@ void SingleStream::TearDown()\n>> \tcamera_.reset();\n>> }\n>>\n>> -std::string SingleStream::nameParameters(const testing::TestParamInfo &info)\n>> +std::string SimpleCapture::nameParameters(const testing::TestParamInfo &info)\n>> {\n>> -\tstd::map rolesMap = {\n>> -\t\t{ StreamRole::Raw, \"Raw\" },\n>> -\t\t{ StreamRole::StillCapture, \"StillCapture\" },\n>> -\t\t{ StreamRole::VideoRecording, \"VideoRecording\" },\n>> -\t\t{ StreamRole::Viewfinder, \"Viewfinder\" }\n>> -\t};\n>> +\tconst auto &[roles, numRequests] = info.param;\n>> +\tstd::ostringstream ss;\n>>\n>> -\tstd::string roleName = rolesMap[std::get<0>(info.param)];\n>> -\tstd::string numRequestsName = std::to_string(std::get<1>(info.param));\n>> +\tfor (StreamRole r : roles)\n>> +\t\tss << r << '_';\n>>\n>> -\treturn roleName + \"_\" + numRequestsName;\n>> +\tss << '_' << numRequests;\n>> +\n>> +\treturn ss.str();\n>> }\n>>\n>> /*\n>> @@ -83,13 +75,13 @@ std::string SingleStream::nameParameters(const testing::TestParamInfo> * failure is a camera that completes less requests than the number of requests\n>> * queued.\n>> */\n>> -TEST_P(SingleStream, Capture)\n>> +TEST_P(SimpleCapture, Capture)\n>> {\n>> -\tauto [role, numRequests] = GetParam();\n>> +\tconst auto &[roles, numRequests] = GetParam();\n>>\n>> \tCapture capture(camera_);\n>>\n>> -\tcapture.configure({ { role } });\n>> +\tcapture.configure(roles);\n>>\n>> \tcapture.run(numRequests, numRequests);\n>> }\n>> @@ -101,14 +93,14 @@ TEST_P(SingleStream, Capture)\n>> * a camera that does not clean up correctly in its error path but is only\n>> * tested by single-capture applications.\n>> */\n>> -TEST_P(SingleStream, CaptureStartStop)\n>> +TEST_P(SimpleCapture, CaptureStartStop)\n>> {\n>> -\tauto [role, numRequests] = GetParam();\n>> +\tconst auto &[roles, numRequests] = GetParam();\n>> \tunsigned int numRepeats = 3;\n>>\n>> \tCapture capture(camera_);\n>>\n>> -\tcapture.configure({ { role } });\n>> +\tcapture.configure(roles);\n>>\n>> \tfor (unsigned int starts = 0; starts < numRepeats; starts++)\n>> \t\tcapture.run(numRequests, numRequests);\n>> @@ -121,21 +113,43 @@ TEST_P(SingleStream, CaptureStartStop)\n>> * is a camera that does not handle cancelation of buffers coming back from the\n>> * video device while stopping.\n>> */\n>> -TEST_P(SingleStream, UnbalancedStop)\n>> +TEST_P(SimpleCapture, UnbalancedStop)\n>> {\n>> -\tauto [role, numRequests] = GetParam();\n>> +\tconst auto &[roles, numRequests] = GetParam();\n>>\n>> \tCapture capture(camera_);\n>>\n>> -\tcapture.configure({ { role } });\n>> +\tcapture.configure(roles);\n>>\n>> \tcapture.run(numRequests);\n>> }\n>>\n>> -INSTANTIATE_TEST_SUITE_P(CaptureTests,\n>> -\t\t\t SingleStream,\n>> -\t\t\t testing::Combine(testing::ValuesIn(ROLES),\n>> +const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };\n>> +\n>> +const std::vector SINGLEROLES[] = {\n>> +\t{ StreamRole::Raw, },\n>> +\t{ StreamRole::StillCapture, },\n>> +\t{ StreamRole::VideoRecording, },\n>> +\t{ StreamRole::Viewfinder, },\n>> +};\n>> +\n>> +const std::vector MULTIROLES[] = {\n>> +\t{ StreamRole::Raw, StreamRole::StillCapture },\n>> +\t{ StreamRole::Raw, StreamRole::VideoRecording },\n>> +\t{ StreamRole::StillCapture, StreamRole::VideoRecording },\n>> +\t{ StreamRole::VideoRecording, StreamRole::VideoRecording },\n>> +};\n>> +\n>> +INSTANTIATE_TEST_SUITE_P(SingleStream,\n>> +\t\t\t SimpleCapture,\n>> +\t\t\t testing::Combine(testing::ValuesIn(SINGLEROLES),\n>> +\t\t\t\t\t testing::ValuesIn(NUMREQUESTS)),\n>> +\t\t\t SimpleCapture::nameParameters);\n>> +\n>> +INSTANTIATE_TEST_SUITE_P(MultiStream,\n>> +\t\t\t SimpleCapture,\n>> +\t\t\t testing::Combine(testing::ValuesIn(MULTIROLES),\n>> \t\t\t\t\t testing::ValuesIn(NUMREQUESTS)),\n>> -\t\t\t SingleStream::nameParameters);\n>> +\t\t\t SimpleCapture::nameParameters);\n>>\n>> } /* namespace */\n>> --\n>> 2.48.1\n>>","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 03CF9C3272\n\tfor ;\n\tFri, 21 Mar 2025 08:12:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1316A68951;\n\tFri, 21 Mar 2025 09:12:06 +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 3EF71600EB\n\tfor ;\n\tFri, 21 Mar 2025 09:12:04 +0100 (CET)","from [192.168.33.18] (185.221.143.221.nat.pool.zt.hu\n\t[185.221.143.221])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D339D50A;\n\tFri, 21 Mar 2025 09:10:19 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"m4ASlq+9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742544620;\n\tbh=yhPuE0SXBSHJB7B4I4E1cWRx4nXZErFIfsQNS7890D8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=m4ASlq+9RhKxRriIFDfRRws/6Qk7yTTerOK8i/JbIOnYsU/enRzULIyTNh8liwBls\n\tZyfmy95pNDzNflCZgjY1VXqqiYpE47XiKLQmCzj+kVh3TyYLPywnrBPMSEBWHBL/OE\n\tTvzwWbNqnsoLl3dV2xJS1PQ4COctm7Mgz7m++DBQ=","Message-ID":"","Date":"Fri, 21 Mar 2025 09:12:00 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 2/2] apps: lc-compliance: Add multi-stream tests","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250314174220.1015597-1-barnabas.pocze@ideasonboard.com>\n\t<20250314174220.1015597-3-barnabas.pocze@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]