[PATCH v4 2/2] apps: lc-compliance: Add multi-stream tests

[Barnabás Pőcze]

Hi


2025. 03. 18. 9:49 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Fri, Mar 14, 2025 at 06:42:19PM +0100, Barnabás Pőcze wrote:
>> Rename the `SingleStream` test to `SimpleCapture`, and extend it
>> to support using multiple roles. And instantiate another test suite
>> from the `SimpleCapture` test that tests multiple streams in one
>> capture session.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
>> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
>> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> 
> My tag stands.
> 
> On top of this we can probably
> 1) Rename SimpleCapture as "Simple" doesn't tell much

Yes, but it can't be called "SingleCapture" anymore, and the
"Capture" name is already taken by the test.


> 2) Rework the number of requests that we iterate on as this seems
> rather random

I preserved the earlier values; by the way, it's the Fibonacci sequence.
But maybe they should indeed be more targeted.


Regards,
Barnabás Pőcze

> 
> But for now, feel free to push these changes!
> 
>> ---
>>   src/apps/lc-compliance/tests/capture_test.cpp | 88 +++++++++++--------
>>   1 file changed, 51 insertions(+), 37 deletions(-)
>>
>> diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp
>> index 06f15bdb4..d02caa8a1 100644
>> --- a/src/apps/lc-compliance/tests/capture_test.cpp
>> +++ b/src/apps/lc-compliance/tests/capture_test.cpp
>> @@ -8,7 +8,10 @@
>>
>>   #include "capture.h"
>>
>> -#include <iostream>
>> +#include <sstream>
>> +#include <string>
>> +#include <tuple>
>> +#include <vector>
>>
>>   #include <gtest/gtest.h>
>>
>> @@ -18,19 +21,10 @@ namespace {
>>
>>   using namespace libcamera;
>>
>> -const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
>> -
>> -const StreamRole ROLES[] = {
>> -	StreamRole::Raw,
>> -	StreamRole::StillCapture,
>> -	StreamRole::VideoRecording,
>> -	StreamRole::Viewfinder
>> -};
>> -
>> -class SingleStream : public testing::TestWithParam<std::tuple<StreamRole, int>>
>> +class SimpleCapture : public testing::TestWithParam<std::tuple<std::vector<StreamRole>, int>>
>>   {
>>   public:
>> -	static std::string nameParameters(const testing::TestParamInfo<SingleStream::ParamType> &info);
>> +	static std::string nameParameters(const testing::TestParamInfo<SimpleCapture::ParamType> &info);
>>
>>   protected:
>>   	void SetUp() override;
>> @@ -43,7 +37,7 @@ protected:
>>    * We use gtest's SetUp() and TearDown() instead of constructor and destructor
>>    * in order to be able to assert on them.
>>    */
>> -void SingleStream::SetUp()
>> +void SimpleCapture::SetUp()
>>   {
>>   	Environment *env = Environment::get();
>>
>> @@ -52,7 +46,7 @@ void SingleStream::SetUp()
>>   	ASSERT_EQ(camera_->acquire(), 0);
>>   }
>>
>> -void SingleStream::TearDown()
>> +void SimpleCapture::TearDown()
>>   {
>>   	if (!camera_)
>>   		return;
>> @@ -61,19 +55,17 @@ void SingleStream::TearDown()
>>   	camera_.reset();
>>   }
>>
>> -std::string SingleStream::nameParameters(const testing::TestParamInfo<SingleStream::ParamType> &info)
>> +std::string SimpleCapture::nameParameters(const testing::TestParamInfo<SimpleCapture::ParamType> &info)
>>   {
>> -	std::map<StreamRole, std::string> rolesMap = {
>> -		{ StreamRole::Raw, "Raw" },
>> -		{ StreamRole::StillCapture, "StillCapture" },
>> -		{ StreamRole::VideoRecording, "VideoRecording" },
>> -		{ StreamRole::Viewfinder, "Viewfinder" }
>> -	};
>> +	const auto &[roles, numRequests] = info.param;
>> +	std::ostringstream ss;
>>
>> -	std::string roleName = rolesMap[std::get<0>(info.param)];
>> -	std::string numRequestsName = std::to_string(std::get<1>(info.param));
>> +	for (StreamRole r : roles)
>> +		ss << r << '_';
>>
>> -	return roleName + "_" + numRequestsName;
>> +	ss << '_' << numRequests;
>> +
>> +	return ss.str();
>>   }
>>
>>   /*
>> @@ -83,13 +75,13 @@ std::string SingleStream::nameParameters(const testing::TestParamInfo<SingleStre
>>    * failure is a camera that completes less requests than the number of requests
>>    * queued.
>>    */
>> -TEST_P(SingleStream, Capture)
>> +TEST_P(SimpleCapture, Capture)
>>   {
>> -	auto [role, numRequests] = GetParam();
>> +	const auto &[roles, numRequests] = GetParam();
>>
>>   	Capture capture(camera_);
>>
>> -	capture.configure({ { role } });
>> +	capture.configure(roles);
>>
>>   	capture.run(numRequests, numRequests);
>>   }
>> @@ -101,14 +93,14 @@ TEST_P(SingleStream, Capture)
>>    * a camera that does not clean up correctly in its error path but is only
>>    * tested by single-capture applications.
>>    */
>> -TEST_P(SingleStream, CaptureStartStop)
>> +TEST_P(SimpleCapture, CaptureStartStop)
>>   {
>> -	auto [role, numRequests] = GetParam();
>> +	const auto &[roles, numRequests] = GetParam();
>>   	unsigned int numRepeats = 3;
>>
>>   	Capture capture(camera_);
>>
>> -	capture.configure({ { role } });
>> +	capture.configure(roles);
>>
>>   	for (unsigned int starts = 0; starts < numRepeats; starts++)
>>   		capture.run(numRequests, numRequests);
>> @@ -121,21 +113,43 @@ TEST_P(SingleStream, CaptureStartStop)
>>    * is a camera that does not handle cancelation of buffers coming back from the
>>    * video device while stopping.
>>    */
>> -TEST_P(SingleStream, UnbalancedStop)
>> +TEST_P(SimpleCapture, UnbalancedStop)
>>   {
>> -	auto [role, numRequests] = GetParam();
>> +	const auto &[roles, numRequests] = GetParam();
>>
>>   	Capture capture(camera_);
>>
>> -	capture.configure({ { role } });
>> +	capture.configure(roles);
>>
>>   	capture.run(numRequests);
>>   }
>>
>> -INSTANTIATE_TEST_SUITE_P(CaptureTests,
>> -			 SingleStream,
>> -			 testing::Combine(testing::ValuesIn(ROLES),
>> +const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
>> +
>> +const std::vector<StreamRole> SINGLEROLES[] = {
>> +	{ StreamRole::Raw, },
>> +	{ StreamRole::StillCapture, },
>> +	{ StreamRole::VideoRecording, },
>> +	{ StreamRole::Viewfinder, },
>> +};
>> +
>> +const std::vector<StreamRole> MULTIROLES[] = {
>> +	{ StreamRole::Raw, StreamRole::StillCapture },
>> +	{ StreamRole::Raw, StreamRole::VideoRecording },
>> +	{ StreamRole::StillCapture, StreamRole::VideoRecording },
>> +	{ StreamRole::VideoRecording, StreamRole::VideoRecording },
>> +};
>> +
>> +INSTANTIATE_TEST_SUITE_P(SingleStream,
>> +			 SimpleCapture,
>> +			 testing::Combine(testing::ValuesIn(SINGLEROLES),
>> +					  testing::ValuesIn(NUMREQUESTS)),
>> +			 SimpleCapture::nameParameters);
>> +
>> +INSTANTIATE_TEST_SUITE_P(MultiStream,
>> +			 SimpleCapture,
>> +			 testing::Combine(testing::ValuesIn(MULTIROLES),
>>   					  testing::ValuesIn(NUMREQUESTS)),
>> -			 SingleStream::nameParameters);
>> +			 SimpleCapture::nameParameters);
>>
>>   } /* namespace */
>> --
>> 2.48.1
>>