[libcamera-devel,v1,6/8] libcamera: apps: lcc: Add multi-stream capture test framework

18251 diff mbox series

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@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

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@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
>