[RFC,v2,14/16] apps: lc-compliance: Support multiple streams in helpers
diff mbox series

Message ID 20250114182143.1773762-15-pobrn@protonmail.com
State Superseded
Headers show
Series
  • apps: lc-compliance: Multi-stream tests
Related show

Commit Message

Barnabás Pőcze Jan. 14, 2025, 6:22 p.m. UTC
Prepare to add a test suite for capture operations with multiple
streams.

Modify the Capture helper class to support multiple roles and streams
in the configure() and capture() operations.

Multi-stream support will be added in next patches.

Co-developed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
---
 src/apps/lc-compliance/helpers/capture.cpp    | 77 +++++++++++++++----
 src/apps/lc-compliance/helpers/capture.h      |  2 +-
 src/apps/lc-compliance/tests/capture_test.cpp |  6 +-
 3 files changed, 66 insertions(+), 19 deletions(-)

Comments

Paul Elder Jan. 23, 2025, 1:57 a.m. UTC | #1
On Tue, Jan 14, 2025 at 06:22:56PM +0000, Barnabás Pőcze wrote:
> Prepare to add a test suite for capture operations with multiple
> streams.
> 
> Modify the Capture helper class to support multiple roles and streams
> in the configure() and capture() operations.
> 
> Multi-stream support will be added in next patches.
> 
> Co-developed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/apps/lc-compliance/helpers/capture.cpp    | 77 +++++++++++++++----
>  src/apps/lc-compliance/helpers/capture.h      |  2 +-
>  src/apps/lc-compliance/tests/capture_test.cpp |  6 +-
>  3 files changed, 66 insertions(+), 19 deletions(-)
> 
> diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp
> index 77e87c9e4..dbaff4138 100644
> --- a/src/apps/lc-compliance/helpers/capture.cpp
> +++ b/src/apps/lc-compliance/helpers/capture.cpp
> @@ -24,13 +24,31 @@ Capture::~Capture()
>  	stop();
>  }
>  
> -void Capture::configure(StreamRole role)
> +void Capture::configure(libcamera::Span<const libcamera::StreamRole> roles)
>  {
> -	config_ = camera_->generateConfiguration({ role });
> +	assert(!roles.empty());
> +
> +	config_ = camera_->generateConfiguration(roles);
>  
>  	if (!config_)
>  		GTEST_SKIP() << "Role not supported by camera";
>  
> +	ASSERT_EQ(config_->size(), roles.size()) << "Unexpected number of streams in configuration";
> +
> +	/*
> +	 * Set the buffers count to the largest value across all streams.
> +	 * \todo: Should all streams from a Camera have the same buffer count ?
> +	 */
> +	auto largest =
> +		std::max_element(config_->begin(), config_->end(),
> +				 [](const StreamConfiguration &l, const StreamConfiguration &r)
> +				 { return l.bufferCount < r.bufferCount; });
> +
> +	assert(largest != config_->end());
> +
> +	for (auto &cfg : *config_)
> +		cfg.bufferCount = largest->bufferCount;
> +
>  	if (config_->validate() != CameraConfiguration::Valid) {
>  		config_.reset();
>  		FAIL() << "Configuration not valid";
> @@ -76,20 +94,36 @@ void Capture::prepareRequests(std::optional<unsigned int> queueLimit)
>  	assert(config_);
>  	assert(requests_.empty());
>  
> -	Stream *stream = config_->at(0).stream();
> -	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_.buffers(stream);
> +	std::size_t maxBuffers = 0;
> +
> +	for (const auto &cfg : *config_) {
> +		const auto &buffers = allocator_.buffers(cfg.stream());
> +		ASSERT_FALSE(buffers.empty()) << "Zero buffers allocated for stream";
> +
> +		maxBuffers = std::max(maxBuffers, buffers.size());
> +	}
>  
>  	/* No point in testing less requests then the camera depth. */
> -	if (queueLimit && *queueLimit < buffers.size()) {
> -		GTEST_SKIP() << "Camera needs " << buffers.size()
> +	if (queueLimit && *queueLimit < maxBuffers) {
> +		GTEST_SKIP() << "Camera needs " << maxBuffers
>  			     << " requests, can't test only " << *queueLimit;
>  	}
>  
> -	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> -		std::unique_ptr<Request> request = camera_->createRequest();
> +	for (std::size_t i = 0; i < maxBuffers; i++) {
> +		std::unique_ptr<Request> request = camera_->createRequest(i);
>  		ASSERT_TRUE(request) << "Can't create request";
>  
> -		ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request";
> +		for (const auto &cfg : *config_) {
> +			Stream *stream = cfg.stream();
> +			const auto &buffers = allocator_.buffers(stream);
> +			assert(!buffers.empty());
> +
> +			if (i >= buffers.size())
> +				continue;
> +
> +			ASSERT_EQ(request->addBuffer(stream, buffers[i].get()), 0)
> +				<< "Can't add buffer to request";
> +		}
>  
>  		requests_.push_back(std::move(request));
>  	}
> @@ -125,11 +159,19 @@ void Capture::requestComplete(Request *request)
>  
>  void Capture::start()
>  {
> -	Stream *stream = config_->at(0).stream();
> -	int count = allocator_.allocate(stream);
> +	assert(config_);
> +	assert(!config_->empty());
> +	assert(!allocator_.allocated());
> +
> +	for (const auto &cfg : *config_) {
> +		Stream *stream = cfg.stream();
> +		int count = allocator_.allocate(stream);
> +
> +		ASSERT_GE(count, 0) << "Failed to allocate buffers";
> +		EXPECT_EQ(count, cfg.bufferCount) << "Allocated less buffers than expected";
> +	}
>  
> -	ASSERT_GE(count, 0) << "Failed to allocate buffers";
> -	EXPECT_EQ(count, config_->at(0).bufferCount) << "Allocated less buffers than expected";
> +	ASSERT_TRUE(allocator_.allocated());
>  
>  	camera_->requestCompleted.connect(this, &Capture::requestComplete);
>  
> @@ -145,7 +187,12 @@ void Capture::stop()
>  
>  	camera_->requestCompleted.disconnect(this);
>  
> -	Stream *stream = config_->at(0).stream();
>  	requests_.clear();
> -	allocator_.free(stream);
> +
> +	for (const auto &cfg : *config_) {
> +		int ret = allocator_.free(cfg.stream());
> +		EXPECT_EQ(ret, 0) << "Failed to free buffers associated with stream";
> +	}
> +
> +	EXPECT_FALSE(allocator_.allocated());
>  }
> diff --git a/src/apps/lc-compliance/helpers/capture.h b/src/apps/lc-compliance/helpers/capture.h
> index 173421fd2..391184ad6 100644
> --- a/src/apps/lc-compliance/helpers/capture.h
> +++ b/src/apps/lc-compliance/helpers/capture.h
> @@ -20,7 +20,7 @@ public:
>  	Capture(std::shared_ptr<libcamera::Camera> camera);
>  	~Capture();
>  
> -	void configure(libcamera::StreamRole role);
> +	void configure(libcamera::Span<const libcamera::StreamRole> roles);
>  	void run(unsigned int captureLimit, std::optional<unsigned int> queueLimit = {});
>  
>  private:
> diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp
> index 93bed48f0..147e17019 100644
> --- a/src/apps/lc-compliance/tests/capture_test.cpp
> +++ b/src/apps/lc-compliance/tests/capture_test.cpp
> @@ -89,7 +89,7 @@ TEST_P(SingleStream, Capture)
>  
>  	Capture capture(camera_);
>  
> -	capture.configure(role);
> +	capture.configure(std::array{ role });
>  
>  	capture.run(numRequests, numRequests);
>  }
> @@ -108,7 +108,7 @@ TEST_P(SingleStream, CaptureStartStop)
>  
>  	Capture capture(camera_);
>  
> -	capture.configure(role);
> +	capture.configure(std::array{ role });
>  
>  	for (unsigned int starts = 0; starts < numRepeats; starts++)
>  		capture.run(numRequests, numRequests);
> @@ -127,7 +127,7 @@ TEST_P(SingleStream, UnbalancedStop)
>  
>  	Capture capture(camera_);
>  
> -	capture.configure(role);
> +	capture.configure(std::array{ role });
>  
>  	capture.run(numRequests);
>  }
> -- 
> 2.48.0
> 
>

Patch
diff mbox series

diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp
index 77e87c9e4..dbaff4138 100644
--- a/src/apps/lc-compliance/helpers/capture.cpp
+++ b/src/apps/lc-compliance/helpers/capture.cpp
@@ -24,13 +24,31 @@  Capture::~Capture()
 	stop();
 }
 
-void Capture::configure(StreamRole role)
+void Capture::configure(libcamera::Span<const libcamera::StreamRole> roles)
 {
-	config_ = camera_->generateConfiguration({ role });
+	assert(!roles.empty());
+
+	config_ = camera_->generateConfiguration(roles);
 
 	if (!config_)
 		GTEST_SKIP() << "Role not supported by camera";
 
+	ASSERT_EQ(config_->size(), roles.size()) << "Unexpected number of streams in configuration";
+
+	/*
+	 * Set the buffers count to the largest value across all streams.
+	 * \todo: Should all streams from a Camera have the same buffer count ?
+	 */
+	auto largest =
+		std::max_element(config_->begin(), config_->end(),
+				 [](const StreamConfiguration &l, const StreamConfiguration &r)
+				 { return l.bufferCount < r.bufferCount; });
+
+	assert(largest != config_->end());
+
+	for (auto &cfg : *config_)
+		cfg.bufferCount = largest->bufferCount;
+
 	if (config_->validate() != CameraConfiguration::Valid) {
 		config_.reset();
 		FAIL() << "Configuration not valid";
@@ -76,20 +94,36 @@  void Capture::prepareRequests(std::optional<unsigned int> queueLimit)
 	assert(config_);
 	assert(requests_.empty());
 
-	Stream *stream = config_->at(0).stream();
-	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_.buffers(stream);
+	std::size_t maxBuffers = 0;
+
+	for (const auto &cfg : *config_) {
+		const auto &buffers = allocator_.buffers(cfg.stream());
+		ASSERT_FALSE(buffers.empty()) << "Zero buffers allocated for stream";
+
+		maxBuffers = std::max(maxBuffers, buffers.size());
+	}
 
 	/* No point in testing less requests then the camera depth. */
-	if (queueLimit && *queueLimit < buffers.size()) {
-		GTEST_SKIP() << "Camera needs " << buffers.size()
+	if (queueLimit && *queueLimit < maxBuffers) {
+		GTEST_SKIP() << "Camera needs " << maxBuffers
 			     << " requests, can't test only " << *queueLimit;
 	}
 
-	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
-		std::unique_ptr<Request> request = camera_->createRequest();
+	for (std::size_t i = 0; i < maxBuffers; i++) {
+		std::unique_ptr<Request> request = camera_->createRequest(i);
 		ASSERT_TRUE(request) << "Can't create request";
 
-		ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request";
+		for (const auto &cfg : *config_) {
+			Stream *stream = cfg.stream();
+			const auto &buffers = allocator_.buffers(stream);
+			assert(!buffers.empty());
+
+			if (i >= buffers.size())
+				continue;
+
+			ASSERT_EQ(request->addBuffer(stream, buffers[i].get()), 0)
+				<< "Can't add buffer to request";
+		}
 
 		requests_.push_back(std::move(request));
 	}
@@ -125,11 +159,19 @@  void Capture::requestComplete(Request *request)
 
 void Capture::start()
 {
-	Stream *stream = config_->at(0).stream();
-	int count = allocator_.allocate(stream);
+	assert(config_);
+	assert(!config_->empty());
+	assert(!allocator_.allocated());
+
+	for (const auto &cfg : *config_) {
+		Stream *stream = cfg.stream();
+		int count = allocator_.allocate(stream);
+
+		ASSERT_GE(count, 0) << "Failed to allocate buffers";
+		EXPECT_EQ(count, cfg.bufferCount) << "Allocated less buffers than expected";
+	}
 
-	ASSERT_GE(count, 0) << "Failed to allocate buffers";
-	EXPECT_EQ(count, config_->at(0).bufferCount) << "Allocated less buffers than expected";
+	ASSERT_TRUE(allocator_.allocated());
 
 	camera_->requestCompleted.connect(this, &Capture::requestComplete);
 
@@ -145,7 +187,12 @@  void Capture::stop()
 
 	camera_->requestCompleted.disconnect(this);
 
-	Stream *stream = config_->at(0).stream();
 	requests_.clear();
-	allocator_.free(stream);
+
+	for (const auto &cfg : *config_) {
+		int ret = allocator_.free(cfg.stream());
+		EXPECT_EQ(ret, 0) << "Failed to free buffers associated with stream";
+	}
+
+	EXPECT_FALSE(allocator_.allocated());
 }
diff --git a/src/apps/lc-compliance/helpers/capture.h b/src/apps/lc-compliance/helpers/capture.h
index 173421fd2..391184ad6 100644
--- a/src/apps/lc-compliance/helpers/capture.h
+++ b/src/apps/lc-compliance/helpers/capture.h
@@ -20,7 +20,7 @@  public:
 	Capture(std::shared_ptr<libcamera::Camera> camera);
 	~Capture();
 
-	void configure(libcamera::StreamRole role);
+	void configure(libcamera::Span<const libcamera::StreamRole> roles);
 	void run(unsigned int captureLimit, std::optional<unsigned int> queueLimit = {});
 
 private:
diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp
index 93bed48f0..147e17019 100644
--- a/src/apps/lc-compliance/tests/capture_test.cpp
+++ b/src/apps/lc-compliance/tests/capture_test.cpp
@@ -89,7 +89,7 @@  TEST_P(SingleStream, Capture)
 
 	Capture capture(camera_);
 
-	capture.configure(role);
+	capture.configure(std::array{ role });
 
 	capture.run(numRequests, numRequests);
 }
@@ -108,7 +108,7 @@  TEST_P(SingleStream, CaptureStartStop)
 
 	Capture capture(camera_);
 
-	capture.configure(role);
+	capture.configure(std::array{ role });
 
 	for (unsigned int starts = 0; starts < numRepeats; starts++)
 		capture.run(numRequests, numRequests);
@@ -127,7 +127,7 @@  TEST_P(SingleStream, UnbalancedStop)
 
 	Capture capture(camera_);
 
-	capture.configure(role);
+	capture.configure(std::array{ role });
 
 	capture.run(numRequests);
 }