[{"id":16030,"web_url":"https://patchwork.libcamera.org/comment/16030/","msgid":"","date":"2021-03-29T23:07:27","subject":"Re: [libcamera-devel] [PATCH v4 2/3] lc-compliance: Add a libcamera\n\tcompliance tool","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Mon, Mar 29, 2021 at 07:02:49PM +0200, Niklas Söderlund wrote:\n> Add a compliance tool to ease testing of cameras. In contrast to the\n> unit-tests under test/ that aims to test the internal components of\n> libcamera the compliance tool aims to test application use-cases and to\n> some extent the public API.\n> \n> This change adds the boilerplate code of a simple framework for the\n> creation of tests. The tests aim both to demonstrate the tool and to\n> catch real problems. The tests added are:\n> \n> - Test that if one queues exactly N requests to a camera exactly N\n> requests are eventually completed.\n> \n> - Test that a configured camera can be started and stopped multiple\n> times in an attempt to exercise cleanup code paths otherwise not\n> often tested with 'cam' for example.\n> \n> Signed-off-by: Niklas Söderlund \n> Acked-by: Kieran Bingham \n> Tested-by: Kieran Bingham \n> ---\n> * Changes since v1\n> - Improve language in commit message and comments.\n> - Test all roles as they may exercise different code paths in the\n> pipeline.\n> - Move SimpleCapture to its own .cpp/.h files.\n> \n> * Changes since v2\n> - Fold in a use-after-free bug fix from Kieran, thanks!\n> \n> * Changes since v3\n> - Update commit message.\n> - Moved command line option parsing to main() instead of in Harness.\n> - Return exit code 0 when display help due to -h.\n> - Rework to allow for a listCameras() function.\n> - Drop options empty check so available cameras are listed if no option\n> is given.\n> - Use EXIT_SUCCESS instead of 0.\n> - Style logging as \"PASS: foo\" instead of \"PASS - foo\".\n> - Include utility for std::pair.\n> - Add todo to not forget to check if result aggregator can be shared\n> with test/.\n> - Do not keep Result construction as one lines.\n> - Make const arrays static const.\n> - Remove unneeded camera exists check.\n> ---\n> src/lc-compliance/main.cpp | 148 ++++++++++++++++++++++++++\n> src/lc-compliance/meson.build | 23 ++++\n> src/lc-compliance/results.cpp | 75 +++++++++++++\n> src/lc-compliance/results.h | 47 +++++++++\n> src/lc-compliance/simple_capture.cpp | 151 +++++++++++++++++++++++++++\n> src/lc-compliance/simple_capture.h | 54 ++++++++++\n> src/lc-compliance/single_stream.cpp | 74 +++++++++++++\n> src/lc-compliance/tests.h | 16 +++\n> src/meson.build | 2 +\n> 9 files changed, 590 insertions(+)\n> create mode 100644 src/lc-compliance/main.cpp\n> create mode 100644 src/lc-compliance/meson.build\n> create mode 100644 src/lc-compliance/results.cpp\n> create mode 100644 src/lc-compliance/results.h\n> create mode 100644 src/lc-compliance/simple_capture.cpp\n> create mode 100644 src/lc-compliance/simple_capture.h\n> create mode 100644 src/lc-compliance/single_stream.cpp\n> create mode 100644 src/lc-compliance/tests.h\n> \n> diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp\n> new file mode 100644\n> index 0000000000000000..d42be412528a0439\n> --- /dev/null\n> +++ b/src/lc-compliance/main.cpp\n> @@ -0,0 +1,148 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * main.cpp - lc-compliance - The libcamera compliance tool\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"../cam/options.h\"\n> +#include \"tests.h\"\n> +\n> +using namespace libcamera;\n> +\n> +enum {\n> +\tOptCamera = 'c',\n> +\tOptHelp = 'h',\n> +};\n> +\n> +class Harness\n> +{\n> +public:\n> +\tHarness(const OptionsParser::Options &options);\n> +\t~Harness();\n> +\n> +\tint exec();\n> +\n> +private:\n> +\tint init();\n> +\tvoid listCameras();\n> +\n> +\tOptionsParser::Options options_;\n> +\tstd::unique_ptr cm_;\n> +\tstd::shared_ptr camera_;\n> +};\n> +\n> +Harness::Harness(const OptionsParser::Options &options)\n> +\t: options_(options)\n> +{\n> +\tcm_ = std::make_unique();\n> +}\n> +\n> +Harness::~Harness()\n> +{\n> +\tif (camera_) {\n> +\t\tcamera_->release();\n> +\t\tcamera_.reset();\n> +\t}\n> +\n> +\tcm_->stop();\n> +}\n> +\n> +int Harness::exec()\n> +{\n> +\tint ret = init();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tstd::vector results;\n> +\n> +\tresults.push_back(testSingleStream(camera_));\n> +\n> +\tfor (const Results &result : results) {\n> +\t\tret = result.summary();\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int Harness::init()\n> +{\n> +\tint ret = cm_->start();\n> +\tif (ret) {\n> +\t\tstd::cout << \"Failed to start camera manager: \"\n> +\t\t\t << strerror(-ret) << std::endl;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tif (!options_.isSet(OptCamera)) {\n> +\t\tstd::cout << \"No camera specified, available cameras:\" << std::endl;\n> +\t\tlistCameras();\n> +\t\treturn -ENODEV;\n> +\t}\n> +\n> +\tconst std::string &cameraId = options_[OptCamera];\n> +\tcamera_ = cm_->get(cameraId);\n> +\tif (!camera_) {\n> +\t\tstd::cout << \"Camera \" << cameraId << \" not found, available cameras:\" << std::endl;\n> +\t\tlistCameras();\n> +\t\treturn -ENODEV;\n> +\t}\n> +\n> +\tif (camera_->acquire()) {\n> +\t\tstd::cout << \"Failed to acquire camera\" << std::endl;\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tstd::cout << \"Using camera \" << cameraId << std::endl;\n> +\n> +\treturn 0;\n> +}\n> +\n> +void Harness::listCameras()\n> +{\n> +\tfor (const std::shared_ptr &cam : cm_->cameras())\n> +\t\tstd::cout << \"- \" << cam.get()->id() << std::endl;\n> +}\n> +\n> +int parseOptions(int argc, char **argv, OptionsParser::Options *options)\n\nThis function should be static.\n\n> +{\n> +\tOptionsParser parser;\n> +\tparser.addOption(OptCamera, OptionString,\n> +\t\t\t \"Specify which camera to operate on, by id\", \"camera\",\n> +\t\t\t ArgumentRequired, \"camera\");\n> +\tparser.addOption(OptHelp, OptionNone, \"Display this help message\",\n> +\t\t\t \"help\");\n> +\n> +\t*options = parser.parse(argc, argv);\n> +\tif (!options->valid())\n> +\t\treturn -EINVAL;\n> +\n> +\tif (options->isSet(OptHelp)) {\n> +\t\tparser.usage();\n> +\t\treturn options->empty() ? -EINVAL : -EINTR;\n\nWhat's the use case for returning -EINVAL, shouldn't main() return\nEXIT_SUCCESS unconditionally when -h/--help is specified ?\n\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int main(int argc, char **argv)\n> +{\n> +\tOptionsParser::Options options;\n> +\tint ret = parseOptions(argc, argv, &options);\n> +\tif (ret == -EINTR)\n> +\t\treturn EXIT_SUCCESS;\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tHarness harness(options);\n> +\n> +\treturn harness.exec() ? EXIT_FAILURE : EXIT_SUCCESS;\n> +}\n> diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build\n> new file mode 100644\n> index 0000000000000000..1bff3ccf5c543dea\n> --- /dev/null\n> +++ b/src/lc-compliance/meson.build\n> @@ -0,0 +1,23 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +if not libevent.found()\n> + warning('libevent_pthreads not found, \\'lc-compliance\\' application will not be compiled')\n\nThe 'cam' application being automatically compiled was an issue on\nChrome OS, as they wanted to only compiled libcamera in production\nenvironment. We could add a new lc-compliance meson option, similarly to\nthe newly added cam option. Or maybe reuse the existing test option, I\nthink bundling the two together makes sense, although it should then be\nturned into a feature option.\n\n> + subdir_done()\n> +endif\n> +\n> +lc_compliance_sources = files([\n> + '../cam/event_loop.cpp',\n> + '../cam/options.cpp',\n> + 'main.cpp',\n> + 'results.cpp',\n> + 'simple_capture.cpp',\n> + 'single_stream.cpp',\n> +])\n> +\n> +lc_compliance = executable('lc-compliance', lc_compliance_sources,\n> + dependencies : [\n> + libatomic,\n> + libcamera_dep,\n> + libevent,\n> + ],\n> + install : true)\n\nIndentation is a bit weird.\n\nReviewed-by: Laurent Pinchart \n\n> diff --git a/src/lc-compliance/results.cpp b/src/lc-compliance/results.cpp\n> new file mode 100644\n> index 0000000000000000..f149f7859e286b8f\n> --- /dev/null\n> +++ b/src/lc-compliance/results.cpp\n> @@ -0,0 +1,75 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * results.cpp - Test result aggregator\n> + */\n> +\n> +#include \"results.h\"\n> +\n> +#include \n> +\n> +void Results::add(const Result &result)\n> +{\n> +\tif (result.first == Pass)\n> +\t\tpassed_++;\n> +\telse if (result.first == Fail)\n> +\t\tfailed_++;\n> +\telse if (result.first == Skip)\n> +\t\tskipped_++;\n> +\n> +\tprintResult(result);\n> +}\n> +\n> +void Results::add(Status status, const std::string &message)\n> +{\n> +\tadd({ status, message });\n> +}\n> +\n> +void Results::fail(const std::string &message)\n> +{\n> +\tadd(Fail, message);\n> +}\n> +\n> +void Results::pass(const std::string &message)\n> +{\n> +\tadd(Pass, message);\n> +}\n> +\n> +void Results::skip(const std::string &message)\n> +{\n> +\tadd(Skip, message);\n> +}\n> +\n> +int Results::summary() const\n> +{\n> +\tif (failed_ + passed_ + skipped_ != planned_) {\n> +\t\tstd::cout << \"Planned and executed number of tests differ \"\n> +\t\t\t << failed_ + passed_ + skipped_ << \" executed \"\n> +\t\t\t << planned_ << \" planned\" << std::endl;\n> +\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tstd::cout << planned_ << \" tests executed, \"\n> +\t\t << passed_ << \" tests passed, \"\n> +\t\t << skipped_ << \" tests skipped and \"\n> +\t\t << failed_ << \" tests failed \" << std::endl;\n> +\n> +\treturn 0;\n> +}\n> +\n> +void Results::printResult(const Result &result)\n> +{\n> +\tstd::string prefix;\n> +\n> +\t/* \\todo Make parsable as TAP. */\n> +\tif (result.first == Pass)\n> +\t\tprefix = \"PASS\";\n> +\telse if (result.first == Fail)\n> +\t\tprefix = \"FAIL\";\n> +\telse if (result.first == Skip)\n> +\t\tprefix = \"SKIP\";\n> +\n> +\tstd::cout << \"- \" << prefix << \": \" << result.second << std::endl;\n> +}\n> diff --git a/src/lc-compliance/results.h b/src/lc-compliance/results.h\n> new file mode 100644\n> index 0000000000000000..2a3722b841a6410a\n> --- /dev/null\n> +++ b/src/lc-compliance/results.h\n> @@ -0,0 +1,47 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * results.h - Test result aggregator\n> + */\n> +#ifndef __LC_COMPLIANCE_RESULTS_H__\n> +#define __LC_COMPLIANCE_RESULTS_H__\n> +\n> +#include \n> +#include \n> +\n> +/* \\todo Check if result aggregator can be shared with self tests in test/ */\n> +class Results\n> +{\n> +public:\n> +\tenum Status {\n> +\t\tFail,\n> +\t\tPass,\n> +\t\tSkip,\n> +\t};\n> +\n> +\tusing Result = std::pair;\n> +\n> +\tResults(unsigned int planned)\n> +\t\t: planned_(planned), passed_(0), failed_(0), skipped_(0)\n> +\t{\n> +\t}\n> +\n> +\tvoid add(const Result &result);\n> +\tvoid add(Status status, const std::string &message);\n> +\tvoid fail(const std::string &message);\n> +\tvoid pass(const std::string &message);\n> +\tvoid skip(const std::string &message);\n> +\n> +\tint summary() const;\n> +\n> +private:\n> +\tvoid printResult(const Result &result);\n> +\n> +\tunsigned int planned_;\n> +\tunsigned int passed_;\n> +\tunsigned int failed_;\n> +\tunsigned int skipped_;\n> +};\n> +\n> +#endif /* __LC_COMPLIANCE_RESULTS_H__ */\n> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp\n> new file mode 100644\n> index 0000000000000000..389dc11fc60225c5\n> --- /dev/null\n> +++ b/src/lc-compliance/simple_capture.cpp\n> @@ -0,0 +1,151 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * simple_capture.cpp - Simple capture helper\n> + */\n> +\n> +#include \"simple_capture.h\"\n> +\n> +using namespace libcamera;\n> +\n> +SimpleCapture::SimpleCapture(std::shared_ptr camera)\n> +\t: camera_(camera), allocator_(std::make_unique(camera))\n> +{\n> +}\n> +\n> +SimpleCapture::~SimpleCapture()\n> +{\n> +}\n> +\n> +Results::Result SimpleCapture::configure(StreamRole role)\n> +{\n> +\tconfig_ = camera_->generateConfiguration({ role });\n> +\n> +\tif (config_->validate() != CameraConfiguration::Valid) {\n> +\t\tconfig_.reset();\n> +\t\treturn { Results::Fail, \"Configuration not valid\" };\n> +\t}\n> +\n> +\tif (camera_->configure(config_.get())) {\n> +\t\tconfig_.reset();\n> +\t\treturn { Results::Fail, \"Failed to configure camera\" };\n> +\t}\n> +\n> +\treturn { Results::Pass, \"Configure camera\" };\n> +}\n> +\n> +Results::Result SimpleCapture::start()\n> +{\n> +\tStream *stream = config_->at(0).stream();\n> +\tif (allocator_->allocate(stream) < 0)\n> +\t\treturn { Results::Fail, \"Failed to allocate buffers\" };\n> +\n> +\tif (camera_->start())\n> +\t\treturn { Results::Fail, \"Failed to start camera\" };\n> +\n> +\tcamera_->requestCompleted.connect(this, &SimpleCapture::requestComplete);\n> +\n> +\treturn { Results::Pass, \"Started camera\" };\n> +}\n> +\n> +Results::Result SimpleCapture::stop()\n> +{\n> +\tStream *stream = config_->at(0).stream();\n> +\n> +\tcamera_->stop();\n> +\n> +\tcamera_->requestCompleted.disconnect(this, &SimpleCapture::requestComplete);\n> +\n> +\tallocator_->free(stream);\n> +\n> +\treturn { Results::Pass, \"Stopped camera\" };\n> +}\n> +\n> +/* SimpleCaptureBalanced */\n> +\n> +SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr camera)\n> +\t: SimpleCapture(camera)\n> +{\n> +}\n> +\n> +Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)\n> +{\n> +\tResults::Result ret = start();\n> +\tif (ret.first != Results::Pass)\n> +\t\treturn ret;\n> +\n> +\tStream *stream = config_->at(0).stream();\n> +\tconst std::vector> &buffers = allocator_->buffers(stream);\n> +\n> +\t/* No point in testing less requests then the camera depth. */\n> +\tif (buffers.size() > numRequests) {\n> +\t\t/* Cache buffers.size() before we destroy it in stop() */\n> +\t\tint buffers_size = buffers.size();\n> +\t\tstop();\n> +\n> +\t\treturn { Results::Skip, \"Camera needs \" + std::to_string(buffers_size)\n> +\t\t\t+ \" requests, can't test only \" + std::to_string(numRequests) };\n> +\t}\n> +\n> +\tqueueCount_ = 0;\n> +\tcaptureCount_ = 0;\n> +\tcaptureLimit_ = numRequests;\n> +\n> +\t/* Queue the recommended number of reqeuests. */\n> +\tstd::vector> requests;\n> +\tfor (const std::unique_ptr &buffer : buffers) {\n> +\t\tstd::unique_ptr request = camera_->createRequest();\n> +\t\tif (!request) {\n> +\t\t\tstop();\n> +\t\t\treturn { Results::Fail, \"Can't create request\" };\n> +\t\t}\n> +\n> +\t\tif (request->addBuffer(stream, buffer.get())) {\n> +\t\t\tstop();\n> +\t\t\treturn { Results::Fail, \"Can't set buffer for request\" };\n> +\t\t}\n> +\n> +\t\tif (queueRequest(request.get()) < 0) {\n> +\t\t\tstop();\n> +\t\t\treturn { Results::Fail, \"Failed to queue request\" };\n> +\t\t}\n> +\n> +\t\trequests.push_back(std::move(request));\n> +\t}\n> +\n> +\t/* Run capture session. */\n> +\tloop_ = new EventLoop();\n> +\tloop_->exec();\n> +\tstop();\n> +\tdelete loop_;\n> +\n> +\tif (captureCount_ != captureLimit_)\n> +\t\treturn { Results::Fail, \"Got \" + std::to_string(captureCount_) +\n> +\t\t\t\" request, wanted \" + std::to_string(captureLimit_) };\n> +\n> +\treturn { Results::Pass, \"Balanced capture of \" +\n> +\t\tstd::to_string(numRequests) + \" requests\" };\n> +}\n> +\n> +int SimpleCaptureBalanced::queueRequest(Request *request)\n> +{\n> +\tqueueCount_++;\n> +\tif (queueCount_ > captureLimit_)\n> +\t\treturn 0;\n> +\n> +\treturn camera_->queueRequest(request);\n> +}\n> +\n> +void SimpleCaptureBalanced::requestComplete(Request *request)\n> +{\n> +\tcaptureCount_++;\n> +\tif (captureCount_ >= captureLimit_) {\n> +\t\tloop_->exit(0);\n> +\t\treturn;\n> +\t}\n> +\n> +\trequest->reuse(Request::ReuseBuffers);\n> +\tif (queueRequest(request))\n> +\t\tloop_->exit(-EINVAL);\n> +}\n> diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h\n> new file mode 100644\n> index 0000000000000000..3a6afc538c623050\n> --- /dev/null\n> +++ b/src/lc-compliance/simple_capture.h\n> @@ -0,0 +1,54 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * simple_capture.h - Simple capture helper\n> + */\n> +#ifndef __LC_COMPLIANCE_SIMPLE_CAPTURE_H__\n> +#define __LC_COMPLIANCE_SIMPLE_CAPTURE_H__\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"../cam/event_loop.h\"\n> +#include \"results.h\"\n> +\n> +class SimpleCapture\n> +{\n> +public:\n> +\tResults::Result configure(libcamera::StreamRole role);\n> +\n> +protected:\n> +\tSimpleCapture(std::shared_ptr camera);\n> +\tvirtual ~SimpleCapture();\n> +\n> +\tResults::Result start();\n> +\tResults::Result stop();\n> +\n> +\tvirtual void requestComplete(libcamera::Request *request) = 0;\n> +\n> +\tEventLoop *loop_;\n> +\n> +\tstd::shared_ptr camera_;\n> +\tstd::unique_ptr allocator_;\n> +\tstd::unique_ptr config_;\n> +};\n> +\n> +class SimpleCaptureBalanced : public SimpleCapture\n> +{\n> +public:\n> +\tSimpleCaptureBalanced(std::shared_ptr camera);\n> +\n> +\tResults::Result capture(unsigned int numRequests);\n> +\n> +private:\n> +\tint queueRequest(libcamera::Request *request);\n> +\tvoid requestComplete(libcamera::Request *request) override;\n> +\n> +\tunsigned int queueCount_;\n> +\tunsigned int captureCount_;\n> +\tunsigned int captureLimit_;\n> +};\n> +\n> +#endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */\n> diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp\n> new file mode 100644\n> index 0000000000000000..e9ca1d58ecb959cd\n> --- /dev/null\n> +++ b/src/lc-compliance/single_stream.cpp\n> @@ -0,0 +1,74 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * single_stream.cpp - Test a single camera stream\n> + */\n> +\n> +#include \n> +\n> +#include \"simple_capture.h\"\n> +#include \"tests.h\"\n> +\n> +using namespace libcamera;\n> +\n> +Results::Result testRequestBalance(std::shared_ptr camera,\n> +\t\t\t\t StreamRole role, unsigned int startCycles,\n> +\t\t\t\t unsigned int numRequests)\n> +{\n> +\tSimpleCaptureBalanced capture(camera);\n> +\n> +\tResults::Result ret = capture.configure(role);\n> +\tif (ret.first != Results::Pass)\n> +\t\treturn ret;\n> +\n> +\tfor (unsigned int starts = 0; starts < startCycles; starts++) {\n> +\t\tret = capture.capture(numRequests);\n> +\t\tif (ret.first != Results::Pass)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\treturn { Results::Pass, \"Balanced capture of \" +\n> +\t\tstd::to_string(numRequests) + \" requests with \" +\n> +\t\tstd::to_string(startCycles) + \" start cycles\" };\n> +}\n> +\n> +Results testSingleStream(std::shared_ptr camera)\n> +{\n> +\tstatic const std::vector> roles = {\n> +\t\t{ \"raw\", Raw },\n> +\t\t{ \"still\", StillCapture },\n> +\t\t{ \"video\", VideoRecording },\n> +\t\t{ \"viewfinder\", Viewfinder },\n> +\t};\n> +\tstatic const std::vector numRequests = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };\n> +\n> +\tResults results(numRequests.size() * roles.size() * 2);\n> +\n> +\tfor (const auto &role : roles) {\n> +\t\tstd::cout << \"= Test role \" << role.first << std::endl;\n> +\t\t/*\n> +\t\t * Test single capture cycles\n> +\t\t *\n> +\t\t * Makes sure the camera completes the exact number of requests queued.\n> +\t\t * Example failure is a camera that needs N+M requests queued to\n> +\t\t * complete N requests to the application.\n> +\t\t */\n> +\t\tstd::cout << \"* Test single capture cycles\" << std::endl;\n> +\t\tfor (unsigned int num : numRequests)\n> +\t\t\tresults.add(testRequestBalance(camera, role.second, 1, num));\n> +\n> +\t\t/*\n> +\t\t * Test multiple start/stop cycles\n> +\t\t *\n> +\t\t * Makes sure the camera supports multiple start/stop cycles.\n> +\t\t * Example failure is a camera that does not clean up correctly in its\n> +\t\t * error path but is only tested by single-capture applications.\n> +\t\t */\n> +\t\tstd::cout << \"* Test multiple start/stop cycles\" << std::endl;\n> +\t\tfor (unsigned int num : numRequests)\n> +\t\t\tresults.add(testRequestBalance(camera, role.second, 3, num));\n> +\t}\n> +\n> +\treturn results;\n> +}\n> diff --git a/src/lc-compliance/tests.h b/src/lc-compliance/tests.h\n> new file mode 100644\n> index 0000000000000000..396605214e4b8980\n> --- /dev/null\n> +++ b/src/lc-compliance/tests.h\n> @@ -0,0 +1,16 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * tests.h - Test modules\n> + */\n> +#ifndef __LC_COMPLIANCE_TESTS_H__\n> +#define __LC_COMPLIANCE_TESTS_H__\n> +\n> +#include \n> +\n> +#include \"results.h\"\n> +\n> +Results testSingleStream(std::shared_ptr camera);\n> +\n> +#endif /* __LC_COMPLIANCE_TESTS_H__ */\n> diff --git a/src/meson.build b/src/meson.build\n> index 14c49f6eeb1f5a01..0145e4f86033648d 100644\n> --- a/src/meson.build\n> +++ b/src/meson.build\n> @@ -22,6 +22,8 @@ subdir('android')\n> subdir('libcamera')\n> subdir('ipa')\n> \n> +subdir('lc-compliance')\n> +\n> subdir('cam')\n> subdir('qcam')\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2211CC32EA\n\tfor ;\n\tMon, 29 Mar 2021 23:08:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8018E602D1;\n\tTue, 30 Mar 2021 01:08:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 323B8602D1\n\tfor ;\n\tTue, 30 Mar 2021 01:08:12 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8ADAB292;\n\tTue, 30 Mar 2021 01:08:11 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"K6kh8SBr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1617059291;\n\tbh=6shExCwbO5iojkwz0Dt4z5H/8KqkslBgCFH1AEiyGBc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=K6kh8SBrz3D/mnWrFCShdfAYMOg4RgegiGp+FMe8a4Vvxi7FGe/wENJdQtMN7S4L3\n\ta7ADu9F73KnE09diZIZQaeHUBSovtqdgtWT9s1xclhzlzInQd1rW7qI5tlLIqvw5ts\n\thi/P460Y3CCSqoAsGmoPj7e4XRoNj7h48y+I79do=","Date":"Tue, 30 Mar 2021 02:07:27 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"","References":"<20210329170250.937120-1-niklas.soderlund@ragnatech.se>\n\t<20210329170250.937120-3-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210329170250.937120-3-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v4 2/3] lc-compliance: Add a libcamera\n\tcompliance tool","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16164,"web_url":"https://patchwork.libcamera.org/comment/16164/","msgid":"","date":"2021-04-09T14:47:56","subject":"Re: [libcamera-devel] [PATCH v4 2/3] lc-compliance: Add a libcamera\n\tcompliance tool","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your comments.\n\nOn 2021-03-30 02:07:27 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Mon, Mar 29, 2021 at 07:02:49PM +0200, Niklas Söderlund wrote:\n> > Add a compliance tool to ease testing of cameras. In contrast to the\n> > unit-tests under test/ that aims to test the internal components of\n> > libcamera the compliance tool aims to test application use-cases and to\n> > some extent the public API.\n> > \n> > This change adds the boilerplate code of a simple framework for the\n> > creation of tests. The tests aim both to demonstrate the tool and to\n> > catch real problems. The tests added are:\n> > \n> > - Test that if one queues exactly N requests to a camera exactly N\n> > requests are eventually completed.\n> > \n> > - Test that a configured camera can be started and stopped multiple\n> > times in an attempt to exercise cleanup code paths otherwise not\n> > often tested with 'cam' for example.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > Acked-by: Kieran Bingham \n> > Tested-by: Kieran Bingham \n> > ---\n> > * Changes since v1\n> > - Improve language in commit message and comments.\n> > - Test all roles as they may exercise different code paths in the\n> > pipeline.\n> > - Move SimpleCapture to its own .cpp/.h files.\n> > \n> > * Changes since v2\n> > - Fold in a use-after-free bug fix from Kieran, thanks!\n> > \n> > * Changes since v3\n> > - Update commit message.\n> > - Moved command line option parsing to main() instead of in Harness.\n> > - Return exit code 0 when display help due to -h.\n> > - Rework to allow for a listCameras() function.\n> > - Drop options empty check so available cameras are listed if no option\n> > is given.\n> > - Use EXIT_SUCCESS instead of 0.\n> > - Style logging as \"PASS: foo\" instead of \"PASS - foo\".\n> > - Include utility for std::pair.\n> > - Add todo to not forget to check if result aggregator can be shared\n> > with test/.\n> > - Do not keep Result construction as one lines.\n> > - Make const arrays static const.\n> > - Remove unneeded camera exists check.\n> > ---\n> > src/lc-compliance/main.cpp | 148 ++++++++++++++++++++++++++\n> > src/lc-compliance/meson.build | 23 ++++\n> > src/lc-compliance/results.cpp | 75 +++++++++++++\n> > src/lc-compliance/results.h | 47 +++++++++\n> > src/lc-compliance/simple_capture.cpp | 151 +++++++++++++++++++++++++++\n> > src/lc-compliance/simple_capture.h | 54 ++++++++++\n> > src/lc-compliance/single_stream.cpp | 74 +++++++++++++\n> > src/lc-compliance/tests.h | 16 +++\n> > src/meson.build | 2 +\n> > 9 files changed, 590 insertions(+)\n> > create mode 100644 src/lc-compliance/main.cpp\n> > create mode 100644 src/lc-compliance/meson.build\n> > create mode 100644 src/lc-compliance/results.cpp\n> > create mode 100644 src/lc-compliance/results.h\n> > create mode 100644 src/lc-compliance/simple_capture.cpp\n> > create mode 100644 src/lc-compliance/simple_capture.h\n> > create mode 100644 src/lc-compliance/single_stream.cpp\n> > create mode 100644 src/lc-compliance/tests.h\n> > \n> > diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp\n> > new file mode 100644\n> > index 0000000000000000..d42be412528a0439\n> > --- /dev/null\n> > +++ b/src/lc-compliance/main.cpp\n> > @@ -0,0 +1,148 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * main.cpp - lc-compliance - The libcamera compliance tool\n> > + */\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"../cam/options.h\"\n> > +#include \"tests.h\"\n> > +\n> > +using namespace libcamera;\n> > +\n> > +enum {\n> > +\tOptCamera = 'c',\n> > +\tOptHelp = 'h',\n> > +};\n> > +\n> > +class Harness\n> > +{\n> > +public:\n> > +\tHarness(const OptionsParser::Options &options);\n> > +\t~Harness();\n> > +\n> > +\tint exec();\n> > +\n> > +private:\n> > +\tint init();\n> > +\tvoid listCameras();\n> > +\n> > +\tOptionsParser::Options options_;\n> > +\tstd::unique_ptr cm_;\n> > +\tstd::shared_ptr camera_;\n> > +};\n> > +\n> > +Harness::Harness(const OptionsParser::Options &options)\n> > +\t: options_(options)\n> > +{\n> > +\tcm_ = std::make_unique();\n> > +}\n> > +\n> > +Harness::~Harness()\n> > +{\n> > +\tif (camera_) {\n> > +\t\tcamera_->release();\n> > +\t\tcamera_.reset();\n> > +\t}\n> > +\n> > +\tcm_->stop();\n> > +}\n> > +\n> > +int Harness::exec()\n> > +{\n> > +\tint ret = init();\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tstd::vector results;\n> > +\n> > +\tresults.push_back(testSingleStream(camera_));\n> > +\n> > +\tfor (const Results &result : results) {\n> > +\t\tret = result.summary();\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int Harness::init()\n> > +{\n> > +\tint ret = cm_->start();\n> > +\tif (ret) {\n> > +\t\tstd::cout << \"Failed to start camera manager: \"\n> > +\t\t\t << strerror(-ret) << std::endl;\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tif (!options_.isSet(OptCamera)) {\n> > +\t\tstd::cout << \"No camera specified, available cameras:\" << std::endl;\n> > +\t\tlistCameras();\n> > +\t\treturn -ENODEV;\n> > +\t}\n> > +\n> > +\tconst std::string &cameraId = options_[OptCamera];\n> > +\tcamera_ = cm_->get(cameraId);\n> > +\tif (!camera_) {\n> > +\t\tstd::cout << \"Camera \" << cameraId << \" not found, available cameras:\" << std::endl;\n> > +\t\tlistCameras();\n> > +\t\treturn -ENODEV;\n> > +\t}\n> > +\n> > +\tif (camera_->acquire()) {\n> > +\t\tstd::cout << \"Failed to acquire camera\" << std::endl;\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tstd::cout << \"Using camera \" << cameraId << std::endl;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +void Harness::listCameras()\n> > +{\n> > +\tfor (const std::shared_ptr &cam : cm_->cameras())\n> > +\t\tstd::cout << \"- \" << cam.get()->id() << std::endl;\n> > +}\n> > +\n> > +int parseOptions(int argc, char **argv, OptionsParser::Options *options)\n> \n> This function should be static.\n\nThanks.\n\n> \n> > +{\n> > +\tOptionsParser parser;\n> > +\tparser.addOption(OptCamera, OptionString,\n> > +\t\t\t \"Specify which camera to operate on, by id\", \"camera\",\n> > +\t\t\t ArgumentRequired, \"camera\");\n> > +\tparser.addOption(OptHelp, OptionNone, \"Display this help message\",\n> > +\t\t\t \"help\");\n> > +\n> > +\t*options = parser.parse(argc, argv);\n> > +\tif (!options->valid())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tif (options->isSet(OptHelp)) {\n> > +\t\tparser.usage();\n> > +\t\treturn options->empty() ? -EINVAL : -EINTR;\n> \n> What's the use case for returning -EINVAL, shouldn't main() return\n> EXIT_SUCCESS unconditionally when -h/--help is specified ?\n\nGood point, this is a left over from something else.\n\n> \n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int main(int argc, char **argv)\n> > +{\n> > +\tOptionsParser::Options options;\n> > +\tint ret = parseOptions(argc, argv, &options);\n> > +\tif (ret == -EINTR)\n> > +\t\treturn EXIT_SUCCESS;\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\n> > +\tHarness harness(options);\n> > +\n> > +\treturn harness.exec() ? EXIT_FAILURE : EXIT_SUCCESS;\n> > +}\n> > diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build\n> > new file mode 100644\n> > index 0000000000000000..1bff3ccf5c543dea\n> > --- /dev/null\n> > +++ b/src/lc-compliance/meson.build\n> > @@ -0,0 +1,23 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +\n> > +if not libevent.found()\n> > + warning('libevent_pthreads not found, \\'lc-compliance\\' application will not be compiled')\n> \n> The 'cam' application being automatically compiled was an issue on\n> Chrome OS, as they wanted to only compiled libcamera in production\n> environment. We could add a new lc-compliance meson option, similarly to\n> the newly added cam option. Or maybe reuse the existing test option, I\n> think bundling the two together makes sense, although it should then be\n> turned into a feature option.\n\nI think I will go for a new feature option that go in parallel with the \nnew 'cam' option.\n\n> \n> > + subdir_done()\n> > +endif\n> > +\n> > +lc_compliance_sources = files([\n> > + '../cam/event_loop.cpp',\n> > + '../cam/options.cpp',\n> > + 'main.cpp',\n> > + 'results.cpp',\n> > + 'simple_capture.cpp',\n> > + 'single_stream.cpp',\n> > +])\n> > +\n> > +lc_compliance = executable('lc-compliance', lc_compliance_sources,\n> > + dependencies : [\n> > + libatomic,\n> > + libcamera_dep,\n> > + libevent,\n> > + ],\n> > + install : true)\n> \n> Indentation is a bit weird.\n\nWops.\n\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > diff --git a/src/lc-compliance/results.cpp b/src/lc-compliance/results.cpp\n> > new file mode 100644\n> > index 0000000000000000..f149f7859e286b8f\n> > --- /dev/null\n> > +++ b/src/lc-compliance/results.cpp\n> > @@ -0,0 +1,75 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * results.cpp - Test result aggregator\n> > + */\n> > +\n> > +#include \"results.h\"\n> > +\n> > +#include \n> > +\n> > +void Results::add(const Result &result)\n> > +{\n> > +\tif (result.first == Pass)\n> > +\t\tpassed_++;\n> > +\telse if (result.first == Fail)\n> > +\t\tfailed_++;\n> > +\telse if (result.first == Skip)\n> > +\t\tskipped_++;\n> > +\n> > +\tprintResult(result);\n> > +}\n> > +\n> > +void Results::add(Status status, const std::string &message)\n> > +{\n> > +\tadd({ status, message });\n> > +}\n> > +\n> > +void Results::fail(const std::string &message)\n> > +{\n> > +\tadd(Fail, message);\n> > +}\n> > +\n> > +void Results::pass(const std::string &message)\n> > +{\n> > +\tadd(Pass, message);\n> > +}\n> > +\n> > +void Results::skip(const std::string &message)\n> > +{\n> > +\tadd(Skip, message);\n> > +}\n> > +\n> > +int Results::summary() const\n> > +{\n> > +\tif (failed_ + passed_ + skipped_ != planned_) {\n> > +\t\tstd::cout << \"Planned and executed number of tests differ \"\n> > +\t\t\t << failed_ + passed_ + skipped_ << \" executed \"\n> > +\t\t\t << planned_ << \" planned\" << std::endl;\n> > +\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tstd::cout << planned_ << \" tests executed, \"\n> > +\t\t << passed_ << \" tests passed, \"\n> > +\t\t << skipped_ << \" tests skipped and \"\n> > +\t\t << failed_ << \" tests failed \" << std::endl;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +void Results::printResult(const Result &result)\n> > +{\n> > +\tstd::string prefix;\n> > +\n> > +\t/* \\todo Make parsable as TAP. */\n> > +\tif (result.first == Pass)\n> > +\t\tprefix = \"PASS\";\n> > +\telse if (result.first == Fail)\n> > +\t\tprefix = \"FAIL\";\n> > +\telse if (result.first == Skip)\n> > +\t\tprefix = \"SKIP\";\n> > +\n> > +\tstd::cout << \"- \" << prefix << \": \" << result.second << std::endl;\n> > +}\n> > diff --git a/src/lc-compliance/results.h b/src/lc-compliance/results.h\n> > new file mode 100644\n> > index 0000000000000000..2a3722b841a6410a\n> > --- /dev/null\n> > +++ b/src/lc-compliance/results.h\n> > @@ -0,0 +1,47 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * results.h - Test result aggregator\n> > + */\n> > +#ifndef __LC_COMPLIANCE_RESULTS_H__\n> > +#define __LC_COMPLIANCE_RESULTS_H__\n> > +\n> > +#include \n> > +#include \n> > +\n> > +/* \\todo Check if result aggregator can be shared with self tests in test/ */\n> > +class Results\n> > +{\n> > +public:\n> > +\tenum Status {\n> > +\t\tFail,\n> > +\t\tPass,\n> > +\t\tSkip,\n> > +\t};\n> > +\n> > +\tusing Result = std::pair;\n> > +\n> > +\tResults(unsigned int planned)\n> > +\t\t: planned_(planned), passed_(0), failed_(0), skipped_(0)\n> > +\t{\n> > +\t}\n> > +\n> > +\tvoid add(const Result &result);\n> > +\tvoid add(Status status, const std::string &message);\n> > +\tvoid fail(const std::string &message);\n> > +\tvoid pass(const std::string &message);\n> > +\tvoid skip(const std::string &message);\n> > +\n> > +\tint summary() const;\n> > +\n> > +private:\n> > +\tvoid printResult(const Result &result);\n> > +\n> > +\tunsigned int planned_;\n> > +\tunsigned int passed_;\n> > +\tunsigned int failed_;\n> > +\tunsigned int skipped_;\n> > +};\n> > +\n> > +#endif /* __LC_COMPLIANCE_RESULTS_H__ */\n> > diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp\n> > new file mode 100644\n> > index 0000000000000000..389dc11fc60225c5\n> > --- /dev/null\n> > +++ b/src/lc-compliance/simple_capture.cpp\n> > @@ -0,0 +1,151 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * simple_capture.cpp - Simple capture helper\n> > + */\n> > +\n> > +#include \"simple_capture.h\"\n> > +\n> > +using namespace libcamera;\n> > +\n> > +SimpleCapture::SimpleCapture(std::shared_ptr camera)\n> > +\t: camera_(camera), allocator_(std::make_unique(camera))\n> > +{\n> > +}\n> > +\n> > +SimpleCapture::~SimpleCapture()\n> > +{\n> > +}\n> > +\n> > +Results::Result SimpleCapture::configure(StreamRole role)\n> > +{\n> > +\tconfig_ = camera_->generateConfiguration({ role });\n> > +\n> > +\tif (config_->validate() != CameraConfiguration::Valid) {\n> > +\t\tconfig_.reset();\n> > +\t\treturn { Results::Fail, \"Configuration not valid\" };\n> > +\t}\n> > +\n> > +\tif (camera_->configure(config_.get())) {\n> > +\t\tconfig_.reset();\n> > +\t\treturn { Results::Fail, \"Failed to configure camera\" };\n> > +\t}\n> > +\n> > +\treturn { Results::Pass, \"Configure camera\" };\n> > +}\n> > +\n> > +Results::Result SimpleCapture::start()\n> > +{\n> > +\tStream *stream = config_->at(0).stream();\n> > +\tif (allocator_->allocate(stream) < 0)\n> > +\t\treturn { Results::Fail, \"Failed to allocate buffers\" };\n> > +\n> > +\tif (camera_->start())\n> > +\t\treturn { Results::Fail, \"Failed to start camera\" };\n> > +\n> > +\tcamera_->requestCompleted.connect(this, &SimpleCapture::requestComplete);\n> > +\n> > +\treturn { Results::Pass, \"Started camera\" };\n> > +}\n> > +\n> > +Results::Result SimpleCapture::stop()\n> > +{\n> > +\tStream *stream = config_->at(0).stream();\n> > +\n> > +\tcamera_->stop();\n> > +\n> > +\tcamera_->requestCompleted.disconnect(this, &SimpleCapture::requestComplete);\n> > +\n> > +\tallocator_->free(stream);\n> > +\n> > +\treturn { Results::Pass, \"Stopped camera\" };\n> > +}\n> > +\n> > +/* SimpleCaptureBalanced */\n> > +\n> > +SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr camera)\n> > +\t: SimpleCapture(camera)\n> > +{\n> > +}\n> > +\n> > +Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)\n> > +{\n> > +\tResults::Result ret = start();\n> > +\tif (ret.first != Results::Pass)\n> > +\t\treturn ret;\n> > +\n> > +\tStream *stream = config_->at(0).stream();\n> > +\tconst std::vector> &buffers = allocator_->buffers(stream);\n> > +\n> > +\t/* No point in testing less requests then the camera depth. */\n> > +\tif (buffers.size() > numRequests) {\n> > +\t\t/* Cache buffers.size() before we destroy it in stop() */\n> > +\t\tint buffers_size = buffers.size();\n> > +\t\tstop();\n> > +\n> > +\t\treturn { Results::Skip, \"Camera needs \" + std::to_string(buffers_size)\n> > +\t\t\t+ \" requests, can't test only \" + std::to_string(numRequests) };\n> > +\t}\n> > +\n> > +\tqueueCount_ = 0;\n> > +\tcaptureCount_ = 0;\n> > +\tcaptureLimit_ = numRequests;\n> > +\n> > +\t/* Queue the recommended number of reqeuests. */\n> > +\tstd::vector> requests;\n> > +\tfor (const std::unique_ptr &buffer : buffers) {\n> > +\t\tstd::unique_ptr request = camera_->createRequest();\n> > +\t\tif (!request) {\n> > +\t\t\tstop();\n> > +\t\t\treturn { Results::Fail, \"Can't create request\" };\n> > +\t\t}\n> > +\n> > +\t\tif (request->addBuffer(stream, buffer.get())) {\n> > +\t\t\tstop();\n> > +\t\t\treturn { Results::Fail, \"Can't set buffer for request\" };\n> > +\t\t}\n> > +\n> > +\t\tif (queueRequest(request.get()) < 0) {\n> > +\t\t\tstop();\n> > +\t\t\treturn { Results::Fail, \"Failed to queue request\" };\n> > +\t\t}\n> > +\n> > +\t\trequests.push_back(std::move(request));\n> > +\t}\n> > +\n> > +\t/* Run capture session. */\n> > +\tloop_ = new EventLoop();\n> > +\tloop_->exec();\n> > +\tstop();\n> > +\tdelete loop_;\n> > +\n> > +\tif (captureCount_ != captureLimit_)\n> > +\t\treturn { Results::Fail, \"Got \" + std::to_string(captureCount_) +\n> > +\t\t\t\" request, wanted \" + std::to_string(captureLimit_) };\n> > +\n> > +\treturn { Results::Pass, \"Balanced capture of \" +\n> > +\t\tstd::to_string(numRequests) + \" requests\" };\n> > +}\n> > +\n> > +int SimpleCaptureBalanced::queueRequest(Request *request)\n> > +{\n> > +\tqueueCount_++;\n> > +\tif (queueCount_ > captureLimit_)\n> > +\t\treturn 0;\n> > +\n> > +\treturn camera_->queueRequest(request);\n> > +}\n> > +\n> > +void SimpleCaptureBalanced::requestComplete(Request *request)\n> > +{\n> > +\tcaptureCount_++;\n> > +\tif (captureCount_ >= captureLimit_) {\n> > +\t\tloop_->exit(0);\n> > +\t\treturn;\n> > +\t}\n> > +\n> > +\trequest->reuse(Request::ReuseBuffers);\n> > +\tif (queueRequest(request))\n> > +\t\tloop_->exit(-EINVAL);\n> > +}\n> > diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h\n> > new file mode 100644\n> > index 0000000000000000..3a6afc538c623050\n> > --- /dev/null\n> > +++ b/src/lc-compliance/simple_capture.h\n> > @@ -0,0 +1,54 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * simple_capture.h - Simple capture helper\n> > + */\n> > +#ifndef __LC_COMPLIANCE_SIMPLE_CAPTURE_H__\n> > +#define __LC_COMPLIANCE_SIMPLE_CAPTURE_H__\n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"../cam/event_loop.h\"\n> > +#include \"results.h\"\n> > +\n> > +class SimpleCapture\n> > +{\n> > +public:\n> > +\tResults::Result configure(libcamera::StreamRole role);\n> > +\n> > +protected:\n> > +\tSimpleCapture(std::shared_ptr camera);\n> > +\tvirtual ~SimpleCapture();\n> > +\n> > +\tResults::Result start();\n> > +\tResults::Result stop();\n> > +\n> > +\tvirtual void requestComplete(libcamera::Request *request) = 0;\n> > +\n> > +\tEventLoop *loop_;\n> > +\n> > +\tstd::shared_ptr camera_;\n> > +\tstd::unique_ptr allocator_;\n> > +\tstd::unique_ptr config_;\n> > +};\n> > +\n> > +class SimpleCaptureBalanced : public SimpleCapture\n> > +{\n> > +public:\n> > +\tSimpleCaptureBalanced(std::shared_ptr camera);\n> > +\n> > +\tResults::Result capture(unsigned int numRequests);\n> > +\n> > +private:\n> > +\tint queueRequest(libcamera::Request *request);\n> > +\tvoid requestComplete(libcamera::Request *request) override;\n> > +\n> > +\tunsigned int queueCount_;\n> > +\tunsigned int captureCount_;\n> > +\tunsigned int captureLimit_;\n> > +};\n> > +\n> > +#endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */\n> > diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp\n> > new file mode 100644\n> > index 0000000000000000..e9ca1d58ecb959cd\n> > --- /dev/null\n> > +++ b/src/lc-compliance/single_stream.cpp\n> > @@ -0,0 +1,74 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * single_stream.cpp - Test a single camera stream\n> > + */\n> > +\n> > +#include \n> > +\n> > +#include \"simple_capture.h\"\n> > +#include \"tests.h\"\n> > +\n> > +using namespace libcamera;\n> > +\n> > +Results::Result testRequestBalance(std::shared_ptr camera,\n> > +\t\t\t\t StreamRole role, unsigned int startCycles,\n> > +\t\t\t\t unsigned int numRequests)\n> > +{\n> > +\tSimpleCaptureBalanced capture(camera);\n> > +\n> > +\tResults::Result ret = capture.configure(role);\n> > +\tif (ret.first != Results::Pass)\n> > +\t\treturn ret;\n> > +\n> > +\tfor (unsigned int starts = 0; starts < startCycles; starts++) {\n> > +\t\tret = capture.capture(numRequests);\n> > +\t\tif (ret.first != Results::Pass)\n> > +\t\t\treturn ret;\n> > +\t}\n> > +\n> > +\treturn { Results::Pass, \"Balanced capture of \" +\n> > +\t\tstd::to_string(numRequests) + \" requests with \" +\n> > +\t\tstd::to_string(startCycles) + \" start cycles\" };\n> > +}\n> > +\n> > +Results testSingleStream(std::shared_ptr camera)\n> > +{\n> > +\tstatic const std::vector> roles = {\n> > +\t\t{ \"raw\", Raw },\n> > +\t\t{ \"still\", StillCapture },\n> > +\t\t{ \"video\", VideoRecording },\n> > +\t\t{ \"viewfinder\", Viewfinder },\n> > +\t};\n> > +\tstatic const std::vector numRequests = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };\n> > +\n> > +\tResults results(numRequests.size() * roles.size() * 2);\n> > +\n> > +\tfor (const auto &role : roles) {\n> > +\t\tstd::cout << \"= Test role \" << role.first << std::endl;\n> > +\t\t/*\n> > +\t\t * Test single capture cycles\n> > +\t\t *\n> > +\t\t * Makes sure the camera completes the exact number of requests queued.\n> > +\t\t * Example failure is a camera that needs N+M requests queued to\n> > +\t\t * complete N requests to the application.\n> > +\t\t */\n> > +\t\tstd::cout << \"* Test single capture cycles\" << std::endl;\n> > +\t\tfor (unsigned int num : numRequests)\n> > +\t\t\tresults.add(testRequestBalance(camera, role.second, 1, num));\n> > +\n> > +\t\t/*\n> > +\t\t * Test multiple start/stop cycles\n> > +\t\t *\n> > +\t\t * Makes sure the camera supports multiple start/stop cycles.\n> > +\t\t * Example failure is a camera that does not clean up correctly in its\n> > +\t\t * error path but is only tested by single-capture applications.\n> > +\t\t */\n> > +\t\tstd::cout << \"* Test multiple start/stop cycles\" << std::endl;\n> > +\t\tfor (unsigned int num : numRequests)\n> > +\t\t\tresults.add(testRequestBalance(camera, role.second, 3, num));\n> > +\t}\n> > +\n> > +\treturn results;\n> > +}\n> > diff --git a/src/lc-compliance/tests.h b/src/lc-compliance/tests.h\n> > new file mode 100644\n> > index 0000000000000000..396605214e4b8980\n> > --- /dev/null\n> > +++ b/src/lc-compliance/tests.h\n> > @@ -0,0 +1,16 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * tests.h - Test modules\n> > + */\n> > +#ifndef __LC_COMPLIANCE_TESTS_H__\n> > +#define __LC_COMPLIANCE_TESTS_H__\n> > +\n> > +#include \n> > +\n> > +#include \"results.h\"\n> > +\n> > +Results testSingleStream(std::shared_ptr camera);\n> > +\n> > +#endif /* __LC_COMPLIANCE_TESTS_H__ */\n> > diff --git a/src/meson.build b/src/meson.build\n> > index 14c49f6eeb1f5a01..0145e4f86033648d 100644\n> > --- a/src/meson.build\n> > +++ b/src/meson.build\n> > @@ -22,6 +22,8 @@ subdir('android')\n> > subdir('libcamera')\n> > subdir('ipa')\n> > \n> > +subdir('lc-compliance')\n> > +\n> > subdir('cam')\n> > subdir('qcam')\n> > \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 66F04BD16B\n\tfor ;\n\tFri, 9 Apr 2021 14:48:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 78CA5687F8;\n\tFri, 9 Apr 2021 16:48:00 +0200 (CEST)","from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com\n\t[IPv6:2a00:1450:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9B105687EF\n\tfor ;\n\tFri, 9 Apr 2021 16:47:58 +0200 (CEST)","by mail-lf1-x12e.google.com with SMTP id r8so10045291lfp.10\n\tfor ;\n\tFri, 09 Apr 2021 07:47:58 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tz28sm297328ljn.117.2021.04.09.07.47.56\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 09 Apr 2021 07:47:56 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"Y6zPrTtL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=97qDg97kBPONPp08Zay5DDqI8ZBS92Rgk57LV/n2hls=;\n\tb=Y6zPrTtLY8LWvdo34UlngxwLFQGzwLHDGW3UsHdJfKa+/HpztybBLoOjvUXXpSc3cs\n\tTJjjhizpVrtG6RDKsJihaYZn4qE1itj4ny9NJXZ8kHcBXuq+YWpWQoRCO/ixBpF6w1IO\n\t1xBOrhKQZiBhHvpECDVWIZ3bkgWZl0icTPSlsWwfJyuobi3PkdmOII2GWTIoYNctoFxV\n\tB5rLrz91e430paimVkIqFwt+AGEgShfOyPx3u/z944oLLJVvfS3KFjm8Lrq6gNE8gUYd\n\tjO5uhUcaiuP5lx7WiNdjzoH8RyYCtx3nJV/ENsi2Zl21Jl7GARDZohvAVujDa/nfWYTp\n\t9akg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=97qDg97kBPONPp08Zay5DDqI8ZBS92Rgk57LV/n2hls=;\n\tb=jQ6nqaqhl1M90tk/oMCOhjZbLA+VL9Y/4BFMppqr7Wg6hGsVT/34UC02ylG9aG2qQW\n\tR6giq9anSaYeGNdzwMh6RCyRzQ/t4M+PTuSoxBJZzSUZ/F4Bcs7YJ9CkU0TVQn7BGvnT\n\t5H65TgO1biXY2jhQRV/tUxr58yrDjkZ3Z0gcPz3enZg3MrkKm8P/Ni2JIpLS5MPrA4/y\n\tjFH58xJYLK1N3xIJNxzrmBKfkdo9tQefxjNegsxdx7ElJs54DN/8S+E/sj+1m4yeALxG\n\t1qSxufeRqGxhShjRVx/MD81DWw+1v3d443f6Oj8agleDXY1D5IyFe5kMWsSA6CQMcxTm\n\t3jVA==","X-Gm-Message-State":"AOAM532fUZS3q1iZsVHtxqdtO3Y4GdZSQK2n4LGXDfT/3PN+xO9x2+jZ\n\tlawg6RBg78A0Dvj8R6ZJ5+vnYA==","X-Google-Smtp-Source":"ABdhPJz6bTvdJm+pRh/rfS/ptcKqzOYTGBhXPkk9JaIQwQbZVZfCefeJssG3O5tGzBm7GTrMLVW8bQ==","X-Received":"by 2002:ac2:5592:: with SMTP id\n\tv18mr7305413lfg.561.1617979677571; \n\tFri, 09 Apr 2021 07:47:57 -0700 (PDT)","Date":"Fri, 9 Apr 2021 16:47:56 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"","References":"<20210329170250.937120-1-niklas.soderlund@ragnatech.se>\n\t<20210329170250.937120-3-niklas.soderlund@ragnatech.se>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 2/3] lc-compliance: Add a libcamera\n\tcompliance tool","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]