[libcamera-devel] [PATCH v1 6/8] libcamera: apps: lcc: Add multi-stream capture test framework
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Thu Mar 2 11:31:52 CET 2023
Hi Naush
thanks for adding this, it's a lot of work!
On Fri, Feb 03, 2023 at 09:44:22AM +0000, Naushir Patuck via libcamera-devel wrote:
> Add a framework for testing multi-stream captures with lcc. To start
> with, a single test class, MultiStream, has been implemented. This class
> is based off the SimpleCaptureBalanced behavior, and tests that the
> pipeline handler completes the exact number of requests queued to it.
>
> Add a test to capture_test.cpp using the MultiStream class to test the
> pipeline handler running with two simultaneous streams. The test
> framework permutate across a list of named stream pairs.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> src/apps/lc-compliance/capture_test.cpp | 77 ++++++++++
> src/apps/lc-compliance/meson.build | 1 +
> src/apps/lc-compliance/multi_capture.cpp | 187 +++++++++++++++++++++++
> src/apps/lc-compliance/multi_capture.h | 61 ++++++++
> 4 files changed, 326 insertions(+)
> create mode 100644 src/apps/lc-compliance/multi_capture.cpp
> create mode 100644 src/apps/lc-compliance/multi_capture.h
>
> diff --git a/src/apps/lc-compliance/capture_test.cpp b/src/apps/lc-compliance/capture_test.cpp
> index 37138dfb3d2e..8d534161e985 100644
> --- a/src/apps/lc-compliance/capture_test.cpp
> +++ b/src/apps/lc-compliance/capture_test.cpp
> @@ -11,6 +11,7 @@
> #include <gtest/gtest.h>
>
> #include "environment.h"
> +#include "multi_capture.h"
> #include "simple_capture.h"
>
> using namespace libcamera;
> @@ -32,6 +33,13 @@ const std::vector<StreamRole> ROLES = {
> StreamRole::Viewfinder
> };
>
> +const std::vector<std::pair<StreamRole, StreamRole>> MULTIROLES = {
> + { StreamRole::Raw, StreamRole::StillCapture },
> + { StreamRole::Raw, StreamRole::VideoRecording },
> + { StreamRole::StillCapture, StreamRole::VideoRecording },
> + { StreamRole::VideoRecording, StreamRole::VideoRecording },
> +};
> +
> } /* namespace */
>
> class SingleStream : public testing::TestWithParam<std::tuple<StreamRole, int>>
> @@ -137,3 +145,72 @@ INSTANTIATE_TEST_SUITE_P(CaptureTests,
> testing::Combine(testing::ValuesIn(ROLES),
> testing::ValuesIn(NUMREQUESTS)),
> SingleStream::nameParameters);
> +
> +class MultiStream : public testing::TestWithParam<std::tuple<std::pair<StreamRole, StreamRole>, int>>
> +{
> +public:
> + static std::string nameParameters(const testing::TestParamInfo<MultiStream::ParamType> &info);
> +
> +protected:
> + void SetUp() override;
> + void TearDown() override;
> +
> + std::shared_ptr<Camera> camera_;
> +};
> +
> +/*
> + * We use gtest's SetUp() and TearDown() instead of constructor and destructor
> + * in order to be able to assert on them.
> + */
> +void MultiStream::SetUp()
> +{
> + Environment *env = Environment::get();
> +
> + camera_ = env->cm()->get(env->cameraId());
> +
> + ASSERT_EQ(camera_->acquire(), 0);
> +}
> +
> +void MultiStream::TearDown()
> +{
> + if (!camera_)
> + return;
> +
> + camera_->release();
> + camera_.reset();
> +}
> +
> +std::string MultiStream::nameParameters(const testing::TestParamInfo<MultiStream::ParamType> &info)
> +{
> + std::string roleName = rolesMap[std::get<0>(info.param).first] + "_" +
> + rolesMap[std::get<0>(info.param).second];
> + std::string numRequestsName = std::to_string(std::get<1>(info.param));
> +
> + return roleName + "_" + numRequestsName;
> +}
> +
> +/*
> + * Test multi-stream capture cycles
> + *
> + * Makes sure the camera completes the exact number of requests queued. Example
> + * failure is a camera that completes less requests than the number of requests
> + * queued.
> + */
> +TEST_P(MultiStream, Capture)
> +{
> + constexpr unsigned int NumStreams = 2;
> +
> + auto [roles, numRequests] = GetParam();
> +
> + MultiCapture capture(camera_);
> +
> + capture.configure({ roles.first, roles.second });
> +
> + capture.capture(numRequests, NumStreams);
> +}
> +
> +INSTANTIATE_TEST_SUITE_P(MultiCaptureTests,
> + MultiStream,
> + testing::Combine(testing::ValuesIn(MULTIROLES),
> + testing::ValuesIn(NUMREQUESTS)),
> + MultiStream::nameParameters);
> diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build
> index 51d9075ac30b..0357cda2f301 100644
> --- a/src/apps/lc-compliance/meson.build
> +++ b/src/apps/lc-compliance/meson.build
> @@ -13,6 +13,7 @@ lc_compliance_enabled = true
> lc_compliance_sources = files([
> 'environment.cpp',
> 'main.cpp',
> + 'multi_capture.cpp',
> 'simple_capture.cpp',
> 'capture_test.cpp',
> ])
> diff --git a/src/apps/lc-compliance/multi_capture.cpp b/src/apps/lc-compliance/multi_capture.cpp
> new file mode 100644
> index 000000000000..23becf964fd7
> --- /dev/null
> +++ b/src/apps/lc-compliance/multi_capture.cpp
> @@ -0,0 +1,187 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2023, Raspberry Pi Ltd
> + *
> + * multi_capture.cpp - Multi-stream capture helper
> + */
> +#include "multi_capture.h"
> +
> +#include <algorithm>
> +
> +#include <gtest/gtest.h>
> +
> +using namespace libcamera;
> +
> +MultiCaptureBase::MultiCaptureBase(std::shared_ptr<Camera> camera)
> + : loop_(nullptr), camera_(camera),
> + allocator_(std::make_unique<FrameBufferAllocator>(camera))
> +{
> +}
> +
> +MultiCaptureBase::~MultiCaptureBase()
> +{
> + stop();
> +}
> +
> +void MultiCaptureBase::configure(const libcamera::StreamRoles &roles)
> +{
> + config_ = camera_->generateConfiguration(roles);
> +
> + if (!config_) {
> + std::cout << "Roles not supported by camera" << std::endl;
> + GTEST_SKIP();
> + }
> +
> + /* Set the buffer counts to the largest value across all streams */
> + auto largest =
> + std::max_element(config_->begin(), config_->end(),
> + [](libcamera::StreamConfiguration &l, libcamera::StreamConfiguration &r)
> + { return l.bufferCount < r.bufferCount; });
> +
> + for (auto &stream : *config_)
> + stream.bufferCount = largest->bufferCount;
> +
> + updateConfig();
> +
> + if (config_->validate() != CameraConfiguration::Valid) {
> + config_.reset();
> + FAIL() << "Configuration not valid";
> + }
> +
> + if (camera_->configure(config_.get())) {
> + config_.reset();
> + FAIL() << "Failed to configure camera";
> + }
> +}
> +
> +void MultiCaptureBase::start()
> +{
> + unsigned int i = 0;
> +
> + for (auto const &config : *config_) {
> + Stream *stream = config.stream();
> + int count = allocator_->allocate(stream);
> +
> + ASSERT_GE(count, 0) << "Failed to allocate buffers for stream "
> + << i;
> + EXPECT_EQ(count, config.bufferCount)
> + << "Alocated less buffers than expected for stream "
> + << i;
> + i++;
> + }
> +
> + camera_->requestCompleted.connect(this, &MultiCaptureBase::requestComplete);
> +
> + ASSERT_EQ(camera_->start(), 0) << "Failed to start camera";
> +}
> +
> +void MultiCaptureBase::stop()
> +{
> + if (!config_ || !allocator_->allocated())
> + return;
> +
> + camera_->stop();
> +
> + camera_->requestCompleted.disconnect(this);
> +
> + requests_.clear();
> +
> + for (auto const &config : *config_) {
> + Stream *stream = config.stream();
> + allocator_->free(stream);
> + }
> +}
While I understand the need to differentiate the capture operations,
I wonder if you have considered augmenting the SimpleCapture to
operate on a vector of roles instead than on a single one. The
implementation of these base methods seems very similar
> +
> +std::vector<const FrameBufferList *>
> +MultiCaptureBase::prepareBuffers(unsigned int numRequests, unsigned int numStreams)
> +{
> + std::vector<const FrameBufferList *> buffers;
> +
> + for (unsigned int i = 0; i < numStreams; i++) {
> + Stream *stream = config_->at(i).stream();
> +
> + buffers.emplace_back(&allocator_->buffers(stream));
> +
> + /* No point in testing less requests then the camera depth. */
> + if (buffers.back()->size() > numRequests) {
> + std::cout << "Stream " << i
> + << " needs " << std::to_string(buffers.back()->size())
> + << " requests, can't test only "
> + << std::to_string(numRequests) << std::endl;
> + return {};
> + }
> + }
> +
> + return buffers;
> +}
> +
> +/* MultiCapture */
> +
> +MultiCapture::MultiCapture(std::shared_ptr<Camera> camera)
> + : MultiCaptureBase(camera)
> +{
> +}
> +
> +void MultiCapture::capture(unsigned int numRequests, unsigned int numStreams)
> +{
> + start();
> +
> + queueCount_ = 0;
> + captureCount_ = 0;
> + captureLimit_ = numRequests;
> +
> + std::vector<const FrameBufferList *>
> + buffers = prepareBuffers(numRequests, numStreams);
> +
> + if (!buffers.size())
> + GTEST_SKIP();
> +
> + /* Queue the recommended number of requests. */
> + const unsigned int inFlightRequests = config_->at(0).bufferCount;
> + for (unsigned int i = 0; i < inFlightRequests; i++) {
> + std::unique_ptr<Request> request = camera_->createRequest();
> + ASSERT_TRUE(request) << "Can't create request";
> +
> + for (unsigned int j = 0; j < numStreams; j++) {
> + const FrameBufferList *bufferList = buffers[j];
> + Stream *stream = config_->at(j).stream();
> +
> + ASSERT_EQ(request->addBuffer(stream, (*bufferList)[i].get()), 0)
> + << "Can't set buffer for request";
> + }
> +
> + ASSERT_EQ(queueRequest(request.get()), 0)
> + << "Failed to queue request";
> + requests_.push_back(std::move(request));
> + }
> +
> + /* Run capture session. */
> + loop_ = new EventLoop();
> + loop_->exec();
> + stop();
> + delete loop_;
> +
> + ASSERT_EQ(captureCount_, captureLimit_);
> +}
> +
> +int MultiCapture::queueRequest(Request *request)
> +{
> + queueCount_++;
> + if (queueCount_ > captureLimit_)
> + return 0;
> +
> + return camera_->queueRequest(request);
> +}
> +
> +void MultiCapture::requestComplete(Request *request)
> +{
> + captureCount_++;
> + if (captureCount_ >= captureLimit_) {
> + loop_->exit(0);
> + return;
> + }
> +
> + request->reuse(Request::ReuseBuffers);
> + if (queueRequest(request))
> + loop_->exit(-EINVAL);
> +}
> diff --git a/src/apps/lc-compliance/multi_capture.h b/src/apps/lc-compliance/multi_capture.h
> new file mode 100644
> index 000000000000..9037099988e5
> --- /dev/null
> +++ b/src/apps/lc-compliance/multi_capture.h
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2023, Raspberry Pi Ltd
> + *
> + * multi_capture.h - Multi-stream capture helper
> + */
> +
> +#pragma once
> +
> +#include <memory>
> +#include <vector>
> +
> +#include <libcamera/libcamera.h>
> +
> +#include "../common/event_loop.h"
> +
> +using FrameBufferList = std::vector<std::unique_ptr<libcamera::FrameBuffer>>;
> +
> +class MultiCaptureBase
> +{
> +public:
> + void configure(const libcamera::StreamRoles &roles);
> +
> +protected:
> + MultiCaptureBase(std::shared_ptr<libcamera::Camera> camera);
> + virtual ~MultiCaptureBase();
> +
> + void start();
> + void stop();
> +
> + std::vector<const FrameBufferList *>
> + prepareBuffers(unsigned int numRequests, unsigned int numStreams);
> +
> + virtual void requestComplete(libcamera::Request *request) = 0;
> + virtual void updateConfig() = 0;
> +
> + EventLoop *loop_;
> +
> + std::shared_ptr<libcamera::Camera> camera_;
> + std::unique_ptr<libcamera::FrameBufferAllocator> allocator_;
> + std::unique_ptr<libcamera::CameraConfiguration> config_;
> + std::vector<std::unique_ptr<libcamera::Request>> requests_;
> +};
> +
> +class MultiCapture : public MultiCaptureBase
> +{
> +public:
> + MultiCapture(std::shared_ptr<libcamera::Camera> camera);
> +
> + void capture(unsigned int numRequests, unsigned int numStreams);
> +
> +protected:
> + int queueRequest(libcamera::Request *request);
> + void requestComplete(libcamera::Request *request) override;
> +
> + void updateConfig() override {}
I wonder if it wouldn't be best to implement this inline in MultiCaptureHints
instead of making it a part of the class hierarcy
> +
> + unsigned int queueCount_;
> + unsigned int captureCount_;
> + unsigned int captureLimit_;
> +};
> --
> 2.25.1
>
More information about the libcamera-devel
mailing list