[libcamera-devel,v4,2/3] lc-compliance: Add a libcamera compliance tool

11772 diff mbox series

Add a compliance tool to ease testing of cameras. In contrast to the
unit-tests under test/ that aims to test the internal components of
libcamera the compliance tool aims to test application use-cases and to
some extent the public API.

This change adds the boilerplate code of a simple framework for the
creation of tests. The tests aim both to demonstrate the tool and to
catch real problems. The tests added are:

 - Test that if one queues exactly N requests to a camera exactly N
   requests are eventually completed.

 - Test that a configured camera can be started and stopped multiple
   times in an attempt to exercise cleanup code paths otherwise not
   often tested with 'cam' for example.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
* Changes since v1
- Improve language in commit message and comments.
- Test all roles as they may exercise different code paths in the
  pipeline.
- Move SimpleCapture to its own .cpp/.h files.

* Changes since v2
- Fold in a use-after-free bug fix from Kieran, thanks!

* Changes since v3
- Update commit message.
- Moved command line option parsing to main() instead of in Harness.
- Return exit code 0 when display help due to -h.
- Rework to allow for a listCameras() function.
- Drop options empty check so available cameras are listed if no option
  is given.
- Use EXIT_SUCCESS instead of 0.
- Style logging as "PASS: foo" instead of "PASS - foo".
- Include utility for std::pair.
- Add todo to not forget to check if result aggregator can be shared
  with test/.
- Do not keep Result construction as one lines.
- Make const arrays static const.
- Remove unneeded camera exists check.
---
 src/lc-compliance/main.cpp           | 148 ++++++++++++++++++++++++++
 src/lc-compliance/meson.build        |  23 ++++
 src/lc-compliance/results.cpp        |  75 +++++++++++++
 src/lc-compliance/results.h          |  47 +++++++++
 src/lc-compliance/simple_capture.cpp | 151 +++++++++++++++++++++++++++
 src/lc-compliance/simple_capture.h   |  54 ++++++++++
 src/lc-compliance/single_stream.cpp  |  74 +++++++++++++
 src/lc-compliance/tests.h            |  16 +++
 src/meson.build                      |   2 +
 9 files changed, 590 insertions(+)
 create mode 100644 src/lc-compliance/main.cpp
 create mode 100644 src/lc-compliance/meson.build
 create mode 100644 src/lc-compliance/results.cpp
 create mode 100644 src/lc-compliance/results.h
 create mode 100644 src/lc-compliance/simple_capture.cpp
 create mode 100644 src/lc-compliance/simple_capture.h
 create mode 100644 src/lc-compliance/single_stream.cpp
 create mode 100644 src/lc-compliance/tests.h

Hi Niklas,

Thank you for the patch.

On Mon, Mar 29, 2021 at 07:02:49PM +0200, Niklas Söderlund wrote:
> Add a compliance tool to ease testing of cameras. In contrast to the
> unit-tests under test/ that aims to test the internal components of
> libcamera the compliance tool aims to test application use-cases and to
> some extent the public API.
> 
> This change adds the boilerplate code of a simple framework for the
> creation of tests. The tests aim both to demonstrate the tool and to
> catch real problems. The tests added are:
> 
>  - Test that if one queues exactly N requests to a camera exactly N
>    requests are eventually completed.
> 
>  - Test that a configured camera can be started and stopped multiple
>    times in an attempt to exercise cleanup code paths otherwise not
>    often tested with 'cam' for example.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
> * Changes since v1
> - Improve language in commit message and comments.
> - Test all roles as they may exercise different code paths in the
>   pipeline.
> - Move SimpleCapture to its own .cpp/.h files.
> 
> * Changes since v2
> - Fold in a use-after-free bug fix from Kieran, thanks!
> 
> * Changes since v3
> - Update commit message.
> - Moved command line option parsing to main() instead of in Harness.
> - Return exit code 0 when display help due to -h.
> - Rework to allow for a listCameras() function.
> - Drop options empty check so available cameras are listed if no option
>   is given.
> - Use EXIT_SUCCESS instead of 0.
> - Style logging as "PASS: foo" instead of "PASS - foo".
> - Include utility for std::pair.
> - Add todo to not forget to check if result aggregator can be shared
>   with test/.
> - Do not keep Result construction as one lines.
> - Make const arrays static const.
> - Remove unneeded camera exists check.
> ---
>  src/lc-compliance/main.cpp           | 148 ++++++++++++++++++++++++++
>  src/lc-compliance/meson.build        |  23 ++++
>  src/lc-compliance/results.cpp        |  75 +++++++++++++
>  src/lc-compliance/results.h          |  47 +++++++++
>  src/lc-compliance/simple_capture.cpp | 151 +++++++++++++++++++++++++++
>  src/lc-compliance/simple_capture.h   |  54 ++++++++++
>  src/lc-compliance/single_stream.cpp  |  74 +++++++++++++
>  src/lc-compliance/tests.h            |  16 +++
>  src/meson.build                      |   2 +
>  9 files changed, 590 insertions(+)
>  create mode 100644 src/lc-compliance/main.cpp
>  create mode 100644 src/lc-compliance/meson.build
>  create mode 100644 src/lc-compliance/results.cpp
>  create mode 100644 src/lc-compliance/results.h
>  create mode 100644 src/lc-compliance/simple_capture.cpp
>  create mode 100644 src/lc-compliance/simple_capture.h
>  create mode 100644 src/lc-compliance/single_stream.cpp
>  create mode 100644 src/lc-compliance/tests.h
> 
> diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp
> new file mode 100644
> index 0000000000000000..d42be412528a0439
> --- /dev/null
> +++ b/src/lc-compliance/main.cpp
> @@ -0,0 +1,148 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * main.cpp - lc-compliance - The libcamera compliance tool
> + */
> +
> +#include <iomanip>
> +#include <iostream>
> +#include <string.h>
> +
> +#include <libcamera/libcamera.h>
> +
> +#include "../cam/options.h"
> +#include "tests.h"
> +
> +using namespace libcamera;
> +
> +enum {
> +	OptCamera = 'c',
> +	OptHelp = 'h',
> +};
> +
> +class Harness
> +{
> +public:
> +	Harness(const OptionsParser::Options &options);
> +	~Harness();
> +
> +	int exec();
> +
> +private:
> +	int init();
> +	void listCameras();
> +
> +	OptionsParser::Options options_;
> +	std::unique_ptr<CameraManager> cm_;
> +	std::shared_ptr<Camera> camera_;
> +};
> +
> +Harness::Harness(const OptionsParser::Options &options)
> +	: options_(options)
> +{
> +	cm_ = std::make_unique<CameraManager>();
> +}
> +
> +Harness::~Harness()
> +{
> +	if (camera_) {
> +		camera_->release();
> +		camera_.reset();
> +	}
> +
> +	cm_->stop();
> +}
> +
> +int Harness::exec()
> +{
> +	int ret = init();
> +	if (ret)
> +		return ret;
> +
> +	std::vector<Results> results;
> +
> +	results.push_back(testSingleStream(camera_));
> +
> +	for (const Results &result : results) {
> +		ret = result.summary();
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int Harness::init()
> +{
> +	int ret = cm_->start();
> +	if (ret) {
> +		std::cout << "Failed to start camera manager: "
> +			  << strerror(-ret) << std::endl;
> +		return ret;
> +	}
> +
> +	if (!options_.isSet(OptCamera)) {
> +		std::cout << "No camera specified, available cameras:" << std::endl;
> +		listCameras();
> +		return -ENODEV;
> +	}
> +
> +	const std::string &cameraId = options_[OptCamera];
> +	camera_ = cm_->get(cameraId);
> +	if (!camera_) {
> +		std::cout << "Camera " << cameraId << " not found, available cameras:" << std::endl;
> +		listCameras();
> +		return -ENODEV;
> +	}
> +
> +	if (camera_->acquire()) {
> +		std::cout << "Failed to acquire camera" << std::endl;
> +		return -EINVAL;
> +	}
> +
> +	std::cout << "Using camera " << cameraId << std::endl;
> +
> +	return 0;
> +}
> +
> +void Harness::listCameras()
> +{
> +	for (const std::shared_ptr<Camera> &cam : cm_->cameras())
> +		std::cout << "- " << cam.get()->id() << std::endl;
> +}
> +
> +int parseOptions(int argc, char **argv, OptionsParser::Options *options)

This function should be static.

> +{
> +	OptionsParser parser;
> +	parser.addOption(OptCamera, OptionString,
> +			 "Specify which camera to operate on, by id", "camera",
> +			 ArgumentRequired, "camera");
> +	parser.addOption(OptHelp, OptionNone, "Display this help message",
> +			 "help");
> +
> +	*options = parser.parse(argc, argv);
> +	if (!options->valid())
> +		return -EINVAL;
> +
> +	if (options->isSet(OptHelp)) {
> +		parser.usage();
> +		return options->empty() ? -EINVAL : -EINTR;

What's the use case for returning -EINVAL, shouldn't main() return
EXIT_SUCCESS unconditionally when -h/--help is specified ?

> +	}
> +
> +	return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	OptionsParser::Options options;
> +	int ret = parseOptions(argc, argv, &options);
> +	if (ret == -EINTR)
> +		return EXIT_SUCCESS;
> +	if (ret < 0)
> +		return ret;
> +
> +	Harness harness(options);
> +
> +	return harness.exec() ? EXIT_FAILURE : EXIT_SUCCESS;
> +}
> diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build
> new file mode 100644
> index 0000000000000000..1bff3ccf5c543dea
> --- /dev/null
> +++ b/src/lc-compliance/meson.build
> @@ -0,0 +1,23 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +if not libevent.found()
> +    warning('libevent_pthreads not found, \'lc-compliance\' application will not be compiled')

The 'cam' application being automatically compiled was an issue on
Chrome OS, as they wanted to only compiled libcamera in production
environment. We could add a new lc-compliance meson option, similarly to
the newly added cam option. Or maybe reuse the existing test option, I
think bundling the two together makes sense, although it should then be
turned into a feature option.

> +    subdir_done()
> +endif
> +
> +lc_compliance_sources = files([
> +    '../cam/event_loop.cpp',
> +    '../cam/options.cpp',
> +    'main.cpp',
> +    'results.cpp',
> +    'simple_capture.cpp',
> +    'single_stream.cpp',
> +])
> +
> +lc_compliance  = executable('lc-compliance', lc_compliance_sources,
> +                  dependencies : [
> +                      libatomic,
> +                      libcamera_dep,
> +                      libevent,
> +                  ],
> +                  install : true)

Indentation is a bit weird.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> diff --git a/src/lc-compliance/results.cpp b/src/lc-compliance/results.cpp
> new file mode 100644
> index 0000000000000000..f149f7859e286b8f
> --- /dev/null
> +++ b/src/lc-compliance/results.cpp
> @@ -0,0 +1,75 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * results.cpp - Test result aggregator
> + */
> +
> +#include "results.h"
> +
> +#include <iostream>
> +
> +void Results::add(const Result &result)
> +{
> +	if (result.first == Pass)
> +		passed_++;
> +	else if (result.first == Fail)
> +		failed_++;
> +	else if (result.first == Skip)
> +		skipped_++;
> +
> +	printResult(result);
> +}
> +
> +void Results::add(Status status, const std::string &message)
> +{
> +	add({ status, message });
> +}
> +
> +void Results::fail(const std::string &message)
> +{
> +	add(Fail, message);
> +}
> +
> +void Results::pass(const std::string &message)
> +{
> +	add(Pass, message);
> +}
> +
> +void Results::skip(const std::string &message)
> +{
> +	add(Skip, message);
> +}
> +
> +int Results::summary() const
> +{
> +	if (failed_ + passed_ + skipped_ != planned_) {
> +		std::cout << "Planned and executed number of tests differ "
> +			  << failed_ + passed_ + skipped_ << " executed "
> +			  << planned_ << " planned" << std::endl;
> +
> +		return -EINVAL;
> +	}
> +
> +	std::cout << planned_ << " tests executed, "
> +		  << passed_ << " tests passed, "
> +		  << skipped_ << " tests skipped and "
> +		  << failed_ << " tests failed " << std::endl;
> +
> +	return 0;
> +}
> +
> +void Results::printResult(const Result &result)
> +{
> +	std::string prefix;
> +
> +	/* \todo Make parsable as TAP. */
> +	if (result.first == Pass)
> +		prefix = "PASS";
> +	else if (result.first == Fail)
> +		prefix = "FAIL";
> +	else if (result.first == Skip)
> +		prefix = "SKIP";
> +
> +	std::cout << "- " << prefix << ": " << result.second << std::endl;
> +}
> diff --git a/src/lc-compliance/results.h b/src/lc-compliance/results.h
> new file mode 100644
> index 0000000000000000..2a3722b841a6410a
> --- /dev/null
> +++ b/src/lc-compliance/results.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * results.h - Test result aggregator
> + */
> +#ifndef __LC_COMPLIANCE_RESULTS_H__
> +#define __LC_COMPLIANCE_RESULTS_H__
> +
> +#include <string>
> +#include <utility>
> +
> +/* \todo Check if result aggregator can be shared with self tests in test/ */
> +class Results
> +{
> +public:
> +	enum Status {
> +		Fail,
> +		Pass,
> +		Skip,
> +	};
> +
> +	using Result = std::pair<Status, std::string>;
> +
> +	Results(unsigned int planned)
> +		: planned_(planned), passed_(0), failed_(0), skipped_(0)
> +	{
> +	}
> +
> +	void add(const Result &result);
> +	void add(Status status, const std::string &message);
> +	void fail(const std::string &message);
> +	void pass(const std::string &message);
> +	void skip(const std::string &message);
> +
> +	int summary() const;
> +
> +private:
> +	void printResult(const Result &result);
> +
> +	unsigned int planned_;
> +	unsigned int passed_;
> +	unsigned int failed_;
> +	unsigned int skipped_;
> +};
> +
> +#endif /* __LC_COMPLIANCE_RESULTS_H__ */
> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> new file mode 100644
> index 0000000000000000..389dc11fc60225c5
> --- /dev/null
> +++ b/src/lc-compliance/simple_capture.cpp
> @@ -0,0 +1,151 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * simple_capture.cpp - Simple capture helper
> + */
> +
> +#include "simple_capture.h"
> +
> +using namespace libcamera;
> +
> +SimpleCapture::SimpleCapture(std::shared_ptr<Camera> camera)
> +	: camera_(camera), allocator_(std::make_unique<FrameBufferAllocator>(camera))
> +{
> +}
> +
> +SimpleCapture::~SimpleCapture()
> +{
> +}
> +
> +Results::Result SimpleCapture::configure(StreamRole role)
> +{
> +	config_ = camera_->generateConfiguration({ role });
> +
> +	if (config_->validate() != CameraConfiguration::Valid) {
> +		config_.reset();
> +		return { Results::Fail, "Configuration not valid" };
> +	}
> +
> +	if (camera_->configure(config_.get())) {
> +		config_.reset();
> +		return { Results::Fail, "Failed to configure camera" };
> +	}
> +
> +	return { Results::Pass, "Configure camera" };
> +}
> +
> +Results::Result SimpleCapture::start()
> +{
> +	Stream *stream = config_->at(0).stream();
> +	if (allocator_->allocate(stream) < 0)
> +		return { Results::Fail, "Failed to allocate buffers" };
> +
> +	if (camera_->start())
> +		return { Results::Fail, "Failed to start camera" };
> +
> +	camera_->requestCompleted.connect(this, &SimpleCapture::requestComplete);
> +
> +	return { Results::Pass, "Started camera" };
> +}
> +
> +Results::Result SimpleCapture::stop()
> +{
> +	Stream *stream = config_->at(0).stream();
> +
> +	camera_->stop();
> +
> +	camera_->requestCompleted.disconnect(this, &SimpleCapture::requestComplete);
> +
> +	allocator_->free(stream);
> +
> +	return { Results::Pass, "Stopped camera" };
> +}
> +
> +/* SimpleCaptureBalanced */
> +
> +SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr<Camera> camera)
> +	: SimpleCapture(camera)
> +{
> +}
> +
> +Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)
> +{
> +	Results::Result ret = start();
> +	if (ret.first != Results::Pass)
> +		return ret;
> +
> +	Stream *stream = config_->at(0).stream();
> +	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
> +
> +	/* No point in testing less requests then the camera depth. */
> +	if (buffers.size() > numRequests) {
> +		/* Cache buffers.size() before we destroy it in stop() */
> +		int buffers_size = buffers.size();
> +		stop();
> +
> +		return { Results::Skip, "Camera needs " + std::to_string(buffers_size)
> +			+ " requests, can't test only " + std::to_string(numRequests) };
> +	}
> +
> +	queueCount_ = 0;
> +	captureCount_ = 0;
> +	captureLimit_ = numRequests;
> +
> +	/* Queue the recommended number of reqeuests. */
> +	std::vector<std::unique_ptr<libcamera::Request>> requests;
> +	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> +		std::unique_ptr<Request> request = camera_->createRequest();
> +		if (!request) {
> +			stop();
> +			return { Results::Fail, "Can't create request" };
> +		}
> +
> +		if (request->addBuffer(stream, buffer.get())) {
> +			stop();
> +			return { Results::Fail, "Can't set buffer for request" };
> +		}
> +
> +		if (queueRequest(request.get()) < 0) {
> +			stop();
> +			return { Results::Fail, "Failed to queue request" };
> +		}
> +
> +		requests.push_back(std::move(request));
> +	}
> +
> +	/* Run capture session. */
> +	loop_ = new EventLoop();
> +	loop_->exec();
> +	stop();
> +	delete loop_;
> +
> +	if (captureCount_ != captureLimit_)
> +		return { Results::Fail, "Got " + std::to_string(captureCount_) +
> +			" request, wanted " + std::to_string(captureLimit_) };
> +
> +	return { Results::Pass, "Balanced capture of " +
> +		std::to_string(numRequests) + " requests" };
> +}
> +
> +int SimpleCaptureBalanced::queueRequest(Request *request)
> +{
> +	queueCount_++;
> +	if (queueCount_ > captureLimit_)
> +		return 0;
> +
> +	return camera_->queueRequest(request);
> +}
> +
> +void SimpleCaptureBalanced::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/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
> new file mode 100644
> index 0000000000000000..3a6afc538c623050
> --- /dev/null
> +++ b/src/lc-compliance/simple_capture.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * simple_capture.h - Simple capture helper
> + */
> +#ifndef __LC_COMPLIANCE_SIMPLE_CAPTURE_H__
> +#define __LC_COMPLIANCE_SIMPLE_CAPTURE_H__
> +
> +#include <memory>
> +
> +#include <libcamera/libcamera.h>
> +
> +#include "../cam/event_loop.h"
> +#include "results.h"
> +
> +class SimpleCapture
> +{
> +public:
> +	Results::Result configure(libcamera::StreamRole role);
> +
> +protected:
> +	SimpleCapture(std::shared_ptr<libcamera::Camera> camera);
> +	virtual ~SimpleCapture();
> +
> +	Results::Result start();
> +	Results::Result stop();
> +
> +	virtual void requestComplete(libcamera::Request *request) = 0;
> +
> +	EventLoop *loop_;
> +
> +	std::shared_ptr<libcamera::Camera> camera_;
> +	std::unique_ptr<libcamera::FrameBufferAllocator> allocator_;
> +	std::unique_ptr<libcamera::CameraConfiguration> config_;
> +};
> +
> +class SimpleCaptureBalanced : public SimpleCapture
> +{
> +public:
> +	SimpleCaptureBalanced(std::shared_ptr<libcamera::Camera> camera);
> +
> +	Results::Result capture(unsigned int numRequests);
> +
> +private:
> +	int queueRequest(libcamera::Request *request);
> +	void requestComplete(libcamera::Request *request) override;
> +
> +	unsigned int queueCount_;
> +	unsigned int captureCount_;
> +	unsigned int captureLimit_;
> +};
> +
> +#endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */
> diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp
> new file mode 100644
> index 0000000000000000..e9ca1d58ecb959cd
> --- /dev/null
> +++ b/src/lc-compliance/single_stream.cpp
> @@ -0,0 +1,74 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * single_stream.cpp - Test a single camera stream
> + */
> +
> +#include <iostream>
> +
> +#include "simple_capture.h"
> +#include "tests.h"
> +
> +using namespace libcamera;
> +
> +Results::Result testRequestBalance(std::shared_ptr<Camera> camera,
> +				   StreamRole role, unsigned int startCycles,
> +				   unsigned int numRequests)
> +{
> +	SimpleCaptureBalanced capture(camera);
> +
> +	Results::Result ret = capture.configure(role);
> +	if (ret.first != Results::Pass)
> +		return ret;
> +
> +	for (unsigned int starts = 0; starts < startCycles; starts++) {
> +		ret = capture.capture(numRequests);
> +		if (ret.first != Results::Pass)
> +			return ret;
> +	}
> +
> +	return { Results::Pass, "Balanced capture of " +
> +		std::to_string(numRequests) + " requests with " +
> +		std::to_string(startCycles) + " start cycles" };
> +}
> +
> +Results testSingleStream(std::shared_ptr<Camera> camera)
> +{
> +	static const std::vector<std::pair<std::string, StreamRole>> roles = {
> +		{ "raw", Raw },
> +		{ "still", StillCapture },
> +		{ "video", VideoRecording },
> +		{ "viewfinder", Viewfinder },
> +	};
> +	static const std::vector<unsigned int> numRequests = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
> +
> +	Results results(numRequests.size() * roles.size() * 2);
> +
> +	for (const auto &role : roles) {
> +		std::cout << "= Test role " << role.first << std::endl;
> +		/*
> +		 * Test single capture cycles
> +		 *
> +		 * Makes sure the camera completes the exact number of requests queued.
> +		 * Example failure is a camera that needs N+M requests queued to
> +		 * complete N requests to the application.
> +		 */
> +		std::cout << "* Test single capture cycles" << std::endl;
> +		for (unsigned int num : numRequests)
> +			results.add(testRequestBalance(camera, role.second, 1, num));
> +
> +		/*
> +		 * Test multiple start/stop cycles
> +		 *
> +		 * Makes sure the camera supports multiple start/stop cycles.
> +		 * Example failure is a camera that does not clean up correctly in its
> +		 * error path but is only tested by single-capture applications.
> +		 */
> +		std::cout << "* Test multiple start/stop cycles" << std::endl;
> +		for (unsigned int num : numRequests)
> +			results.add(testRequestBalance(camera, role.second, 3, num));
> +	}
> +
> +	return results;
> +}
> diff --git a/src/lc-compliance/tests.h b/src/lc-compliance/tests.h
> new file mode 100644
> index 0000000000000000..396605214e4b8980
> --- /dev/null
> +++ b/src/lc-compliance/tests.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * tests.h - Test modules
> + */
> +#ifndef __LC_COMPLIANCE_TESTS_H__
> +#define __LC_COMPLIANCE_TESTS_H__
> +
> +#include <libcamera/libcamera.h>
> +
> +#include "results.h"
> +
> +Results testSingleStream(std::shared_ptr<libcamera::Camera> camera);
> +
> +#endif /* __LC_COMPLIANCE_TESTS_H__ */
> diff --git a/src/meson.build b/src/meson.build
> index 14c49f6eeb1f5a01..0145e4f86033648d 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -22,6 +22,8 @@ subdir('android')
>  subdir('libcamera')
>  subdir('ipa')
>  
> +subdir('lc-compliance')
> +
>  subdir('cam')
>  subdir('qcam')
>

Hi Laurent,

Thanks for your comments.

On 2021-03-30 02:07:27 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Mon, Mar 29, 2021 at 07:02:49PM +0200, Niklas Söderlund wrote:
> > Add a compliance tool to ease testing of cameras. In contrast to the
> > unit-tests under test/ that aims to test the internal components of
> > libcamera the compliance tool aims to test application use-cases and to
> > some extent the public API.
> > 
> > This change adds the boilerplate code of a simple framework for the
> > creation of tests. The tests aim both to demonstrate the tool and to
> > catch real problems. The tests added are:
> > 
> >  - Test that if one queues exactly N requests to a camera exactly N
> >    requests are eventually completed.
> > 
> >  - Test that a configured camera can be started and stopped multiple
> >    times in an attempt to exercise cleanup code paths otherwise not
> >    often tested with 'cam' for example.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> > * Changes since v1
> > - Improve language in commit message and comments.
> > - Test all roles as they may exercise different code paths in the
> >   pipeline.
> > - Move SimpleCapture to its own .cpp/.h files.
> > 
> > * Changes since v2
> > - Fold in a use-after-free bug fix from Kieran, thanks!
> > 
> > * Changes since v3
> > - Update commit message.
> > - Moved command line option parsing to main() instead of in Harness.
> > - Return exit code 0 when display help due to -h.
> > - Rework to allow for a listCameras() function.
> > - Drop options empty check so available cameras are listed if no option
> >   is given.
> > - Use EXIT_SUCCESS instead of 0.
> > - Style logging as "PASS: foo" instead of "PASS - foo".
> > - Include utility for std::pair.
> > - Add todo to not forget to check if result aggregator can be shared
> >   with test/.
> > - Do not keep Result construction as one lines.
> > - Make const arrays static const.
> > - Remove unneeded camera exists check.
> > ---
> >  src/lc-compliance/main.cpp           | 148 ++++++++++++++++++++++++++
> >  src/lc-compliance/meson.build        |  23 ++++
> >  src/lc-compliance/results.cpp        |  75 +++++++++++++
> >  src/lc-compliance/results.h          |  47 +++++++++
> >  src/lc-compliance/simple_capture.cpp | 151 +++++++++++++++++++++++++++
> >  src/lc-compliance/simple_capture.h   |  54 ++++++++++
> >  src/lc-compliance/single_stream.cpp  |  74 +++++++++++++
> >  src/lc-compliance/tests.h            |  16 +++
> >  src/meson.build                      |   2 +
> >  9 files changed, 590 insertions(+)
> >  create mode 100644 src/lc-compliance/main.cpp
> >  create mode 100644 src/lc-compliance/meson.build
> >  create mode 100644 src/lc-compliance/results.cpp
> >  create mode 100644 src/lc-compliance/results.h
> >  create mode 100644 src/lc-compliance/simple_capture.cpp
> >  create mode 100644 src/lc-compliance/simple_capture.h
> >  create mode 100644 src/lc-compliance/single_stream.cpp
> >  create mode 100644 src/lc-compliance/tests.h
> > 
> > diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp
> > new file mode 100644
> > index 0000000000000000..d42be412528a0439
> > --- /dev/null
> > +++ b/src/lc-compliance/main.cpp
> > @@ -0,0 +1,148 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * main.cpp - lc-compliance - The libcamera compliance tool
> > + */
> > +
> > +#include <iomanip>
> > +#include <iostream>
> > +#include <string.h>
> > +
> > +#include <libcamera/libcamera.h>
> > +
> > +#include "../cam/options.h"
> > +#include "tests.h"
> > +
> > +using namespace libcamera;
> > +
> > +enum {
> > +	OptCamera = 'c',
> > +	OptHelp = 'h',
> > +};
> > +
> > +class Harness
> > +{
> > +public:
> > +	Harness(const OptionsParser::Options &options);
> > +	~Harness();
> > +
> > +	int exec();
> > +
> > +private:
> > +	int init();
> > +	void listCameras();
> > +
> > +	OptionsParser::Options options_;
> > +	std::unique_ptr<CameraManager> cm_;
> > +	std::shared_ptr<Camera> camera_;
> > +};
> > +
> > +Harness::Harness(const OptionsParser::Options &options)
> > +	: options_(options)
> > +{
> > +	cm_ = std::make_unique<CameraManager>();
> > +}
> > +
> > +Harness::~Harness()
> > +{
> > +	if (camera_) {
> > +		camera_->release();
> > +		camera_.reset();
> > +	}
> > +
> > +	cm_->stop();
> > +}
> > +
> > +int Harness::exec()
> > +{
> > +	int ret = init();
> > +	if (ret)
> > +		return ret;
> > +
> > +	std::vector<Results> results;
> > +
> > +	results.push_back(testSingleStream(camera_));
> > +
> > +	for (const Results &result : results) {
> > +		ret = result.summary();
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +int Harness::init()
> > +{
> > +	int ret = cm_->start();
> > +	if (ret) {
> > +		std::cout << "Failed to start camera manager: "
> > +			  << strerror(-ret) << std::endl;
> > +		return ret;
> > +	}
> > +
> > +	if (!options_.isSet(OptCamera)) {
> > +		std::cout << "No camera specified, available cameras:" << std::endl;
> > +		listCameras();
> > +		return -ENODEV;
> > +	}
> > +
> > +	const std::string &cameraId = options_[OptCamera];
> > +	camera_ = cm_->get(cameraId);
> > +	if (!camera_) {
> > +		std::cout << "Camera " << cameraId << " not found, available cameras:" << std::endl;
> > +		listCameras();
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (camera_->acquire()) {
> > +		std::cout << "Failed to acquire camera" << std::endl;
> > +		return -EINVAL;
> > +	}
> > +
> > +	std::cout << "Using camera " << cameraId << std::endl;
> > +
> > +	return 0;
> > +}
> > +
> > +void Harness::listCameras()
> > +{
> > +	for (const std::shared_ptr<Camera> &cam : cm_->cameras())
> > +		std::cout << "- " << cam.get()->id() << std::endl;
> > +}
> > +
> > +int parseOptions(int argc, char **argv, OptionsParser::Options *options)
> 
> This function should be static.

Thanks.

> 
> > +{
> > +	OptionsParser parser;
> > +	parser.addOption(OptCamera, OptionString,
> > +			 "Specify which camera to operate on, by id", "camera",
> > +			 ArgumentRequired, "camera");
> > +	parser.addOption(OptHelp, OptionNone, "Display this help message",
> > +			 "help");
> > +
> > +	*options = parser.parse(argc, argv);
> > +	if (!options->valid())
> > +		return -EINVAL;
> > +
> > +	if (options->isSet(OptHelp)) {
> > +		parser.usage();
> > +		return options->empty() ? -EINVAL : -EINTR;
> 
> What's the use case for returning -EINVAL, shouldn't main() return
> EXIT_SUCCESS unconditionally when -h/--help is specified ?

Good point, this is a left over from something else.

> 
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +	OptionsParser::Options options;
> > +	int ret = parseOptions(argc, argv, &options);
> > +	if (ret == -EINTR)
> > +		return EXIT_SUCCESS;
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	Harness harness(options);
> > +
> > +	return harness.exec() ? EXIT_FAILURE : EXIT_SUCCESS;
> > +}
> > diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build
> > new file mode 100644
> > index 0000000000000000..1bff3ccf5c543dea
> > --- /dev/null
> > +++ b/src/lc-compliance/meson.build
> > @@ -0,0 +1,23 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +if not libevent.found()
> > +    warning('libevent_pthreads not found, \'lc-compliance\' application will not be compiled')
> 
> The 'cam' application being automatically compiled was an issue on
> Chrome OS, as they wanted to only compiled libcamera in production
> environment. We could add a new lc-compliance meson option, similarly to
> the newly added cam option. Or maybe reuse the existing test option, I
> think bundling the two together makes sense, although it should then be
> turned into a feature option.

I think I will go for a new feature option that go in parallel with the 
new 'cam' option.

> 
> > +    subdir_done()
> > +endif
> > +
> > +lc_compliance_sources = files([
> > +    '../cam/event_loop.cpp',
> > +    '../cam/options.cpp',
> > +    'main.cpp',
> > +    'results.cpp',
> > +    'simple_capture.cpp',
> > +    'single_stream.cpp',
> > +])
> > +
> > +lc_compliance  = executable('lc-compliance', lc_compliance_sources,
> > +                  dependencies : [
> > +                      libatomic,
> > +                      libcamera_dep,
> > +                      libevent,
> > +                  ],
> > +                  install : true)
> 
> Indentation is a bit weird.

Wops.

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > diff --git a/src/lc-compliance/results.cpp b/src/lc-compliance/results.cpp
> > new file mode 100644
> > index 0000000000000000..f149f7859e286b8f
> > --- /dev/null
> > +++ b/src/lc-compliance/results.cpp
> > @@ -0,0 +1,75 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * results.cpp - Test result aggregator
> > + */
> > +
> > +#include "results.h"
> > +
> > +#include <iostream>
> > +
> > +void Results::add(const Result &result)
> > +{
> > +	if (result.first == Pass)
> > +		passed_++;
> > +	else if (result.first == Fail)
> > +		failed_++;
> > +	else if (result.first == Skip)
> > +		skipped_++;
> > +
> > +	printResult(result);
> > +}
> > +
> > +void Results::add(Status status, const std::string &message)
> > +{
> > +	add({ status, message });
> > +}
> > +
> > +void Results::fail(const std::string &message)
> > +{
> > +	add(Fail, message);
> > +}
> > +
> > +void Results::pass(const std::string &message)
> > +{
> > +	add(Pass, message);
> > +}
> > +
> > +void Results::skip(const std::string &message)
> > +{
> > +	add(Skip, message);
> > +}
> > +
> > +int Results::summary() const
> > +{
> > +	if (failed_ + passed_ + skipped_ != planned_) {
> > +		std::cout << "Planned and executed number of tests differ "
> > +			  << failed_ + passed_ + skipped_ << " executed "
> > +			  << planned_ << " planned" << std::endl;
> > +
> > +		return -EINVAL;
> > +	}
> > +
> > +	std::cout << planned_ << " tests executed, "
> > +		  << passed_ << " tests passed, "
> > +		  << skipped_ << " tests skipped and "
> > +		  << failed_ << " tests failed " << std::endl;
> > +
> > +	return 0;
> > +}
> > +
> > +void Results::printResult(const Result &result)
> > +{
> > +	std::string prefix;
> > +
> > +	/* \todo Make parsable as TAP. */
> > +	if (result.first == Pass)
> > +		prefix = "PASS";
> > +	else if (result.first == Fail)
> > +		prefix = "FAIL";
> > +	else if (result.first == Skip)
> > +		prefix = "SKIP";
> > +
> > +	std::cout << "- " << prefix << ": " << result.second << std::endl;
> > +}
> > diff --git a/src/lc-compliance/results.h b/src/lc-compliance/results.h
> > new file mode 100644
> > index 0000000000000000..2a3722b841a6410a
> > --- /dev/null
> > +++ b/src/lc-compliance/results.h
> > @@ -0,0 +1,47 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * results.h - Test result aggregator
> > + */
> > +#ifndef __LC_COMPLIANCE_RESULTS_H__
> > +#define __LC_COMPLIANCE_RESULTS_H__
> > +
> > +#include <string>
> > +#include <utility>
> > +
> > +/* \todo Check if result aggregator can be shared with self tests in test/ */
> > +class Results
> > +{
> > +public:
> > +	enum Status {
> > +		Fail,
> > +		Pass,
> > +		Skip,
> > +	};
> > +
> > +	using Result = std::pair<Status, std::string>;
> > +
> > +	Results(unsigned int planned)
> > +		: planned_(planned), passed_(0), failed_(0), skipped_(0)
> > +	{
> > +	}
> > +
> > +	void add(const Result &result);
> > +	void add(Status status, const std::string &message);
> > +	void fail(const std::string &message);
> > +	void pass(const std::string &message);
> > +	void skip(const std::string &message);
> > +
> > +	int summary() const;
> > +
> > +private:
> > +	void printResult(const Result &result);
> > +
> > +	unsigned int planned_;
> > +	unsigned int passed_;
> > +	unsigned int failed_;
> > +	unsigned int skipped_;
> > +};
> > +
> > +#endif /* __LC_COMPLIANCE_RESULTS_H__ */
> > diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> > new file mode 100644
> > index 0000000000000000..389dc11fc60225c5
> > --- /dev/null
> > +++ b/src/lc-compliance/simple_capture.cpp
> > @@ -0,0 +1,151 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * simple_capture.cpp - Simple capture helper
> > + */
> > +
> > +#include "simple_capture.h"
> > +
> > +using namespace libcamera;
> > +
> > +SimpleCapture::SimpleCapture(std::shared_ptr<Camera> camera)
> > +	: camera_(camera), allocator_(std::make_unique<FrameBufferAllocator>(camera))
> > +{
> > +}
> > +
> > +SimpleCapture::~SimpleCapture()
> > +{
> > +}
> > +
> > +Results::Result SimpleCapture::configure(StreamRole role)
> > +{
> > +	config_ = camera_->generateConfiguration({ role });
> > +
> > +	if (config_->validate() != CameraConfiguration::Valid) {
> > +		config_.reset();
> > +		return { Results::Fail, "Configuration not valid" };
> > +	}
> > +
> > +	if (camera_->configure(config_.get())) {
> > +		config_.reset();
> > +		return { Results::Fail, "Failed to configure camera" };
> > +	}
> > +
> > +	return { Results::Pass, "Configure camera" };
> > +}
> > +
> > +Results::Result SimpleCapture::start()
> > +{
> > +	Stream *stream = config_->at(0).stream();
> > +	if (allocator_->allocate(stream) < 0)
> > +		return { Results::Fail, "Failed to allocate buffers" };
> > +
> > +	if (camera_->start())
> > +		return { Results::Fail, "Failed to start camera" };
> > +
> > +	camera_->requestCompleted.connect(this, &SimpleCapture::requestComplete);
> > +
> > +	return { Results::Pass, "Started camera" };
> > +}
> > +
> > +Results::Result SimpleCapture::stop()
> > +{
> > +	Stream *stream = config_->at(0).stream();
> > +
> > +	camera_->stop();
> > +
> > +	camera_->requestCompleted.disconnect(this, &SimpleCapture::requestComplete);
> > +
> > +	allocator_->free(stream);
> > +
> > +	return { Results::Pass, "Stopped camera" };
> > +}
> > +
> > +/* SimpleCaptureBalanced */
> > +
> > +SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr<Camera> camera)
> > +	: SimpleCapture(camera)
> > +{
> > +}
> > +
> > +Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)
> > +{
> > +	Results::Result ret = start();
> > +	if (ret.first != Results::Pass)
> > +		return ret;
> > +
> > +	Stream *stream = config_->at(0).stream();
> > +	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
> > +
> > +	/* No point in testing less requests then the camera depth. */
> > +	if (buffers.size() > numRequests) {
> > +		/* Cache buffers.size() before we destroy it in stop() */
> > +		int buffers_size = buffers.size();
> > +		stop();
> > +
> > +		return { Results::Skip, "Camera needs " + std::to_string(buffers_size)
> > +			+ " requests, can't test only " + std::to_string(numRequests) };
> > +	}
> > +
> > +	queueCount_ = 0;
> > +	captureCount_ = 0;
> > +	captureLimit_ = numRequests;
> > +
> > +	/* Queue the recommended number of reqeuests. */
> > +	std::vector<std::unique_ptr<libcamera::Request>> requests;
> > +	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> > +		std::unique_ptr<Request> request = camera_->createRequest();
> > +		if (!request) {
> > +			stop();
> > +			return { Results::Fail, "Can't create request" };
> > +		}
> > +
> > +		if (request->addBuffer(stream, buffer.get())) {
> > +			stop();
> > +			return { Results::Fail, "Can't set buffer for request" };
> > +		}
> > +
> > +		if (queueRequest(request.get()) < 0) {
> > +			stop();
> > +			return { Results::Fail, "Failed to queue request" };
> > +		}
> > +
> > +		requests.push_back(std::move(request));
> > +	}
> > +
> > +	/* Run capture session. */
> > +	loop_ = new EventLoop();
> > +	loop_->exec();
> > +	stop();
> > +	delete loop_;
> > +
> > +	if (captureCount_ != captureLimit_)
> > +		return { Results::Fail, "Got " + std::to_string(captureCount_) +
> > +			" request, wanted " + std::to_string(captureLimit_) };
> > +
> > +	return { Results::Pass, "Balanced capture of " +
> > +		std::to_string(numRequests) + " requests" };
> > +}
> > +
> > +int SimpleCaptureBalanced::queueRequest(Request *request)
> > +{
> > +	queueCount_++;
> > +	if (queueCount_ > captureLimit_)
> > +		return 0;
> > +
> > +	return camera_->queueRequest(request);
> > +}
> > +
> > +void SimpleCaptureBalanced::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/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
> > new file mode 100644
> > index 0000000000000000..3a6afc538c623050
> > --- /dev/null
> > +++ b/src/lc-compliance/simple_capture.h
> > @@ -0,0 +1,54 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * simple_capture.h - Simple capture helper
> > + */
> > +#ifndef __LC_COMPLIANCE_SIMPLE_CAPTURE_H__
> > +#define __LC_COMPLIANCE_SIMPLE_CAPTURE_H__
> > +
> > +#include <memory>
> > +
> > +#include <libcamera/libcamera.h>
> > +
> > +#include "../cam/event_loop.h"
> > +#include "results.h"
> > +
> > +class SimpleCapture
> > +{
> > +public:
> > +	Results::Result configure(libcamera::StreamRole role);
> > +
> > +protected:
> > +	SimpleCapture(std::shared_ptr<libcamera::Camera> camera);
> > +	virtual ~SimpleCapture();
> > +
> > +	Results::Result start();
> > +	Results::Result stop();
> > +
> > +	virtual void requestComplete(libcamera::Request *request) = 0;
> > +
> > +	EventLoop *loop_;
> > +
> > +	std::shared_ptr<libcamera::Camera> camera_;
> > +	std::unique_ptr<libcamera::FrameBufferAllocator> allocator_;
> > +	std::unique_ptr<libcamera::CameraConfiguration> config_;
> > +};
> > +
> > +class SimpleCaptureBalanced : public SimpleCapture
> > +{
> > +public:
> > +	SimpleCaptureBalanced(std::shared_ptr<libcamera::Camera> camera);
> > +
> > +	Results::Result capture(unsigned int numRequests);
> > +
> > +private:
> > +	int queueRequest(libcamera::Request *request);
> > +	void requestComplete(libcamera::Request *request) override;
> > +
> > +	unsigned int queueCount_;
> > +	unsigned int captureCount_;
> > +	unsigned int captureLimit_;
> > +};
> > +
> > +#endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */
> > diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp
> > new file mode 100644
> > index 0000000000000000..e9ca1d58ecb959cd
> > --- /dev/null
> > +++ b/src/lc-compliance/single_stream.cpp
> > @@ -0,0 +1,74 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * single_stream.cpp - Test a single camera stream
> > + */
> > +
> > +#include <iostream>
> > +
> > +#include "simple_capture.h"
> > +#include "tests.h"
> > +
> > +using namespace libcamera;
> > +
> > +Results::Result testRequestBalance(std::shared_ptr<Camera> camera,
> > +				   StreamRole role, unsigned int startCycles,
> > +				   unsigned int numRequests)
> > +{
> > +	SimpleCaptureBalanced capture(camera);
> > +
> > +	Results::Result ret = capture.configure(role);
> > +	if (ret.first != Results::Pass)
> > +		return ret;
> > +
> > +	for (unsigned int starts = 0; starts < startCycles; starts++) {
> > +		ret = capture.capture(numRequests);
> > +		if (ret.first != Results::Pass)
> > +			return ret;
> > +	}
> > +
> > +	return { Results::Pass, "Balanced capture of " +
> > +		std::to_string(numRequests) + " requests with " +
> > +		std::to_string(startCycles) + " start cycles" };
> > +}
> > +
> > +Results testSingleStream(std::shared_ptr<Camera> camera)
> > +{
> > +	static const std::vector<std::pair<std::string, StreamRole>> roles = {
> > +		{ "raw", Raw },
> > +		{ "still", StillCapture },
> > +		{ "video", VideoRecording },
> > +		{ "viewfinder", Viewfinder },
> > +	};
> > +	static const std::vector<unsigned int> numRequests = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
> > +
> > +	Results results(numRequests.size() * roles.size() * 2);
> > +
> > +	for (const auto &role : roles) {
> > +		std::cout << "= Test role " << role.first << std::endl;
> > +		/*
> > +		 * Test single capture cycles
> > +		 *
> > +		 * Makes sure the camera completes the exact number of requests queued.
> > +		 * Example failure is a camera that needs N+M requests queued to
> > +		 * complete N requests to the application.
> > +		 */
> > +		std::cout << "* Test single capture cycles" << std::endl;
> > +		for (unsigned int num : numRequests)
> > +			results.add(testRequestBalance(camera, role.second, 1, num));
> > +
> > +		/*
> > +		 * Test multiple start/stop cycles
> > +		 *
> > +		 * Makes sure the camera supports multiple start/stop cycles.
> > +		 * Example failure is a camera that does not clean up correctly in its
> > +		 * error path but is only tested by single-capture applications.
> > +		 */
> > +		std::cout << "* Test multiple start/stop cycles" << std::endl;
> > +		for (unsigned int num : numRequests)
> > +			results.add(testRequestBalance(camera, role.second, 3, num));
> > +	}
> > +
> > +	return results;
> > +}
> > diff --git a/src/lc-compliance/tests.h b/src/lc-compliance/tests.h
> > new file mode 100644
> > index 0000000000000000..396605214e4b8980
> > --- /dev/null
> > +++ b/src/lc-compliance/tests.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * tests.h - Test modules
> > + */
> > +#ifndef __LC_COMPLIANCE_TESTS_H__
> > +#define __LC_COMPLIANCE_TESTS_H__
> > +
> > +#include <libcamera/libcamera.h>
> > +
> > +#include "results.h"
> > +
> > +Results testSingleStream(std::shared_ptr<libcamera::Camera> camera);
> > +
> > +#endif /* __LC_COMPLIANCE_TESTS_H__ */
> > diff --git a/src/meson.build b/src/meson.build
> > index 14c49f6eeb1f5a01..0145e4f86033648d 100644
> > --- a/src/meson.build
> > +++ b/src/meson.build
> > @@ -22,6 +22,8 @@ subdir('android')
> >  subdir('libcamera')
> >  subdir('ipa')
> >  
> > +subdir('lc-compliance')
> > +
> >  subdir('cam')
> >  subdir('qcam')
> >  
> -- 
> Regards,
> 
> Laurent Pinchart