Message ID | 20250114182143.1773762-15-pobrn@protonmail.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
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 > >
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); }