From patchwork Thu Jun 10 18:37:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 12555 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 90E6EC320B for ; Thu, 10 Jun 2021 18:38:32 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5DA7968933; Thu, 10 Jun 2021 20:38:32 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E3816029C for ; Thu, 10 Jun 2021 20:38:31 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:fdf4:5e57:8aee:5d72]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id D24DB1F441F2; Thu, 10 Jun 2021 19:38:29 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 10 Jun 2021 15:37:28 -0300 Message-Id: <20210610183732.174697-2-nfraprado@collabora.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210610183732.174697-1-nfraprado@collabora.com> References: <20210610183732.174697-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 1/5] libcamera: camera: Make stop() idempotent X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Make Camera::stop() idempotent so that it can be called in any state and consecutive times. When called in any state other than CameraRunning, it is a no-op. This simplifies the cleanup path for applications. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- No changes in v7 No changes in v6 Changes in v5: - Thanks to Laurent: - Simplified isRunning() Changes in v4: - Thanks to Jacopo: - Added \todo in Camera::stop() No changes in v3 Changes in v2: - Suggested by Kieran: - Add new isRunning() function instead of relying on isAccessAllowed() - Suggested by Laurent: - Make stop()'s description clearer src/libcamera/camera.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 1340c266cc5f..a13a15dd1fc9 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -347,6 +347,7 @@ public: const std::set &streams); ~Private(); + bool isRunning() const; int isAccessAllowed(State state, bool allowDisconnected = false, const char *from = __builtin_FUNCTION()) const; int isAccessAllowed(State low, State high, @@ -388,6 +389,11 @@ static const char *const camera_state_names[] = { "Running", }; +bool Camera::Private::isRunning() const +{ + return state_.load(std::memory_order_acquire) == CameraRunning; +} + int Camera::Private::isAccessAllowed(State state, bool allowDisconnected, const char *from) const { @@ -1061,9 +1067,10 @@ int Camera::start(const ControlList *controls) * This method stops capturing and processing requests immediately. All pending * requests are cancelled and complete synchronously in an error state. * - * \context This function may only be called when the camera is in the Running - * state as defined in \ref camera_operation, and shall be synchronized by the - * caller with other functions that affect the camera state. + * \context This function may be called in any camera state as defined in \ref + * camera_operation, and shall be synchronized by the caller with other + * functions that affect the camera state. If called when the camera isn't + * running, it is a no-op. * * \return 0 on success or a negative error code otherwise * \retval -ENODEV The camera has been disconnected from the system @@ -1073,6 +1080,13 @@ int Camera::stop() { Private *const d = LIBCAMERA_D_PTR(); + /* + * \todo Make calling stop() when not in 'Running' part of the state + * machine rather than take this shortcut + */ + if (!d->isRunning()) + return 0; + int ret = d->isAccessAllowed(Private::CameraRunning); if (ret < 0) return ret; From patchwork Thu Jun 10 18:37:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 12556 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id EDA96C320B for ; Thu, 10 Jun 2021 18:38:35 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BBC236029C; Thu, 10 Jun 2021 20:38:35 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 11D0F6029B for ; Thu, 10 Jun 2021 20:38:34 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:fdf4:5e57:8aee:5d72]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 4CB731F441F3; Thu, 10 Jun 2021 19:38:31 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 10 Jun 2021 15:37:29 -0300 Message-Id: <20210610183732.174697-3-nfraprado@collabora.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210610183732.174697-1-nfraprado@collabora.com> References: <20210610183732.174697-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 2/5] lc-compliance: Make SimpleCapture::stop() idempotent X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Make SimpleCapture::stop() be able to be called multiple times and at any point so that it can be called from the destructor and an assert failure can return immediately. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- Changes in v7: - Thanks to Jacopo: - Moved the check for buffers allocated to the beginning of SimpleCapture::stop() No changes in v6 No changes in v5 No changes in v4 No changes in v3 Changes in v2: - Suggested by Laurent: - Fixed code style src/lc-compliance/simple_capture.cpp | 30 ++++++++++------------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp index 64e862a08e3a..bfc91b26444e 100644 --- a/src/lc-compliance/simple_capture.cpp +++ b/src/lc-compliance/simple_capture.cpp @@ -17,6 +17,7 @@ SimpleCapture::SimpleCapture(std::shared_ptr camera) SimpleCapture::~SimpleCapture() { + stop(); } Results::Result SimpleCapture::configure(StreamRole role) @@ -55,12 +56,14 @@ Results::Result SimpleCapture::start() void SimpleCapture::stop() { - Stream *stream = config_->at(0).stream(); + if (!config_ || !allocator_->allocated()) + return; camera_->stop(); camera_->requestCompleted.disconnect(this, &SimpleCapture::requestComplete); + Stream *stream = config_->at(0).stream(); allocator_->free(stream); } @@ -84,7 +87,6 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) 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) }; @@ -98,20 +100,14 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) std::vector> requests; for (const std::unique_ptr &buffer : buffers) { std::unique_ptr request = camera_->createRequest(); - if (!request) { - stop(); + if (!request) return { Results::Fail, "Can't create request" }; - } - if (request->addBuffer(stream, buffer.get())) { - stop(); + if (request->addBuffer(stream, buffer.get())) return { Results::Fail, "Can't set buffer for request" }; - } - if (queueRequest(request.get()) < 0) { - stop(); + if (queueRequest(request.get()) < 0) return { Results::Fail, "Failed to queue request" }; - } requests.push_back(std::move(request)); } @@ -175,20 +171,14 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests) std::vector> requests; for (const std::unique_ptr &buffer : buffers) { std::unique_ptr request = camera_->createRequest(); - if (!request) { - stop(); + if (!request) return { Results::Fail, "Can't create request" }; - } - if (request->addBuffer(stream, buffer.get())) { - stop(); + if (request->addBuffer(stream, buffer.get())) return { Results::Fail, "Can't set buffer for request" }; - } - if (camera_->queueRequest(request.get()) < 0) { - stop(); + if (camera_->queueRequest(request.get()) < 0) return { Results::Fail, "Failed to queue request" }; - } requests.push_back(std::move(request)); } From patchwork Thu Jun 10 18:37:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 12557 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 26911C320B for ; Thu, 10 Jun 2021 18:38:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E729D6892C; Thu, 10 Jun 2021 20:38:37 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1CAD86892C for ; Thu, 10 Jun 2021 20:38:36 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:fdf4:5e57:8aee:5d72]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 5952F1F441F2; Thu, 10 Jun 2021 19:38:34 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 10 Jun 2021 15:37:30 -0300 Message-Id: <20210610183732.174697-4-nfraprado@collabora.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210610183732.174697-1-nfraprado@collabora.com> References: <20210610183732.174697-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 3/5] lc-compliance: Add Environment singleton X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add a singleton Environment class in order to make the camera available inside all tests. This is needed for the Googletest refactor, otherwise the tests, which are statically declared, won't be able to access the camera. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- Changes in v7: - Thanks to Jacopo: - Fixed style issue - Made CameraManager a unique_ptr and passed to Environment as raw pointer Changes in v6: - Thanks to Niklas: - Made cameraId() return const& Changes in v5: - Thanks to Laurent: - Stored CameraManager and cameraId instead of Camera in Environment - Thanks to Laurent & Niklas: - Improved Environment singleton class instantiation and destruction Added in v4 src/lc-compliance/environment.cpp | 20 ++++++++++++++++++++ src/lc-compliance/environment.h | 31 +++++++++++++++++++++++++++++++ src/lc-compliance/meson.build | 1 + 3 files changed, 52 insertions(+) create mode 100644 src/lc-compliance/environment.cpp create mode 100644 src/lc-compliance/environment.h diff --git a/src/lc-compliance/environment.cpp b/src/lc-compliance/environment.cpp new file mode 100644 index 000000000000..3a539848362f --- /dev/null +++ b/src/lc-compliance/environment.cpp @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2021, Collabora Ltd. + * + * environment.cpp - Common environment for tests + */ + +#include "environment.h" + +Environment *Environment::get() +{ + static Environment instance; + return &instance; +} + +void Environment::setup(CameraManager* cm, std::string cameraId) +{ + cm_ = cm; + cameraId_ = cameraId; +} diff --git a/src/lc-compliance/environment.h b/src/lc-compliance/environment.h new file mode 100644 index 000000000000..614eb2777800 --- /dev/null +++ b/src/lc-compliance/environment.h @@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2021, Collabora Ltd. + * + * environment.h - Common environment for tests + */ +#ifndef __LC_COMPLIANCE_ENVIRONMENT_H__ +#define __LC_COMPLIANCE_ENVIRONMENT_H__ + +#include + +using namespace libcamera; + +class Environment +{ +public: + static Environment *get(); + + void setup(CameraManager* cm, std::string cameraId); + + const std::string &cameraId() const { return cameraId_; } + CameraManager* cm() const { return cm_; } + +private: + Environment() {}; + + std::string cameraId_; + CameraManager* cm_; +}; + +#endif /* __LC_COMPLIANCE_ENVIRONMENT_H__ */ diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build index a2bfcceb1259..6dd49085569f 100644 --- a/src/lc-compliance/meson.build +++ b/src/lc-compliance/meson.build @@ -12,6 +12,7 @@ lc_compliance_enabled = true lc_compliance_sources = files([ '../cam/event_loop.cpp', '../cam/options.cpp', + 'environment.cpp', 'main.cpp', 'results.cpp', 'simple_capture.cpp', From patchwork Thu Jun 10 18:37:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 12558 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 52704C320B for ; Thu, 10 Jun 2021 18:38:40 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 20A8068930; Thu, 10 Jun 2021 20:38:40 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5FCB16029C for ; Thu, 10 Jun 2021 20:38:38 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:fdf4:5e57:8aee:5d72]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 649161F441F5; Thu, 10 Jun 2021 19:38:36 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 10 Jun 2021 15:37:31 -0300 Message-Id: <20210610183732.174697-5-nfraprado@collabora.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210610183732.174697-1-nfraprado@collabora.com> References: <20210610183732.174697-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 4/5] lc-compliance: Refactor using Googletest X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Refactor lc-compliance using Googletest as the test framework. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Niklas Söderlund --- Changes in v7: - Thanks to Jacopo: - Made CameraManager a unique_ptr and passed to Environment as raw pointer No changes in v6 Changes in v5: - Thanks to Laurent: - Fixed style issues - Added SetUp and TearDown functions to acquire and release the camera for each test Changes in v4: - Removed old lc-compliance results classes - Thanks to Niklas: - Improved naming of tests Changes in v3: - Thanks to Niklas: - Went back to static test registration, and created a Singleton Environment class to store the camera global variable - Added a nameParameters() function to give meaningful names to the test parameters, removing the need to register each test suite for every role src/lc-compliance/main.cpp | 73 +++++------ src/lc-compliance/meson.build | 4 +- src/lc-compliance/results.cpp | 75 ------------ src/lc-compliance/results.h | 47 -------- src/lc-compliance/simple_capture.cpp | 74 +++++------- src/lc-compliance/simple_capture.h | 9 +- src/lc-compliance/single_stream.cpp | 174 ++++++++++++++++----------- src/lc-compliance/tests.h | 16 --- 8 files changed, 175 insertions(+), 297 deletions(-) delete mode 100644 src/lc-compliance/results.cpp delete mode 100644 src/lc-compliance/results.h delete mode 100644 src/lc-compliance/tests.h diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp index 54cee54aa978..8c18845141de 100644 --- a/src/lc-compliance/main.cpp +++ b/src/lc-compliance/main.cpp @@ -9,10 +9,12 @@ #include #include +#include + #include +#include "environment.h" #include "../cam/options.h" -#include "tests.h" using namespace libcamera; @@ -21,21 +23,35 @@ enum { OptHelp = 'h', }; +/* + * Make asserts act like exceptions, otherwise they only fail (or skip) the + * current function. From gtest documentation: + * https://google.github.io/googletest/advanced.html#asserting-on-subroutines-with-an-exception + */ +class ThrowListener : public testing::EmptyTestEventListener +{ + void OnTestPartResult(const testing::TestPartResult &result) override + { + if (result.type() == testing::TestPartResult::kFatalFailure || + result.type() == testing::TestPartResult::kSkip) + throw testing::AssertionException(result); + } +}; + class Harness { public: Harness(const OptionsParser::Options &options); ~Harness(); - int exec(); + int init(); + int run(int argc, char **argv); private: - int init(); void listCameras(); OptionsParser::Options options_; std::unique_ptr cm_; - std::shared_ptr camera_; }; Harness::Harness(const OptionsParser::Options &options) @@ -46,35 +62,13 @@ Harness::Harness(const OptionsParser::Options &options) Harness::~Harness() { - if (camera_) { - camera_->release(); - camera_.reset(); - } - cm_->stop(); } -int Harness::exec() -{ - int ret = init(); - if (ret) - return ret; - - std::vector results; - - results.push_back(testSingleStream(camera_)); - - for (const Results &result : results) { - ret = result.summary(); - if (ret) - return ret; - } - - return 0; -} - int Harness::init() { + std::shared_ptr camera; + int ret = cm_->start(); if (ret) { std::cout << "Failed to start camera manager: " @@ -89,23 +83,29 @@ int Harness::init() } const std::string &cameraId = options_[OptCamera]; - camera_ = cm_->get(cameraId); - if (!camera_) { + 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; - } + Environment::get()->setup(cm_.get(), cameraId); std::cout << "Using camera " << cameraId << std::endl; return 0; } +int Harness::run(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + + testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener); + + return RUN_ALL_TESTS(); +} + void Harness::listCameras() { for (const std::shared_ptr &cam : cm_->cameras()) @@ -143,6 +143,9 @@ int main(int argc, char **argv) return EXIT_FAILURE; Harness harness(options); + ret = harness.init(); + if (ret) + return ret; - return harness.exec() ? EXIT_FAILURE : EXIT_SUCCESS; + return harness.run(argc, argv); } diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build index 6dd49085569f..156b938ee9ad 100644 --- a/src/lc-compliance/meson.build +++ b/src/lc-compliance/meson.build @@ -14,15 +14,17 @@ lc_compliance_sources = files([ '../cam/options.cpp', 'environment.cpp', 'main.cpp', - 'results.cpp', 'simple_capture.cpp', 'single_stream.cpp', ]) +libgtest = dependency('gtest') + lc_compliance = executable('lc-compliance', lc_compliance_sources, dependencies : [ libatomic, libcamera_dep, libevent, + libgtest, ], install : true) diff --git a/src/lc-compliance/results.cpp b/src/lc-compliance/results.cpp deleted file mode 100644 index f149f7859e28..000000000000 --- a/src/lc-compliance/results.cpp +++ /dev/null @@ -1,75 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ -/* - * Copyright (C) 2020, Google Inc. - * - * results.cpp - Test result aggregator - */ - -#include "results.h" - -#include - -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 deleted file mode 100644 index 2a3722b841a6..000000000000 --- a/src/lc-compliance/results.h +++ /dev/null @@ -1,47 +0,0 @@ -/* 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 -#include - -/* \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; - - 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 index bfc91b26444e..d5a9d9e8cebe 100644 --- a/src/lc-compliance/simple_capture.cpp +++ b/src/lc-compliance/simple_capture.cpp @@ -5,6 +5,8 @@ * simple_capture.cpp - Simple capture helper */ +#include + #include "simple_capture.h" using namespace libcamera; @@ -20,38 +22,34 @@ SimpleCapture::~SimpleCapture() stop(); } -Results::Result SimpleCapture::configure(StreamRole role) +void SimpleCapture::configure(StreamRole role) { config_ = camera_->generateConfiguration({ role }); - if (!config_) - return { Results::Skip, "Role not supported by camera" }; + if (!config_) { + std::cout << "Role not supported by camera" << std::endl; + GTEST_SKIP(); + } if (config_->validate() != CameraConfiguration::Valid) { config_.reset(); - return { Results::Fail, "Configuration not valid" }; + FAIL() << "Configuration not valid"; } if (camera_->configure(config_.get())) { config_.reset(); - return { Results::Fail, "Failed to configure camera" }; + FAIL() << "Failed to configure camera"; } - - return { Results::Pass, "Configure camera" }; } -Results::Result SimpleCapture::start() +void SimpleCapture::start() { Stream *stream = config_->at(0).stream(); - if (allocator_->allocate(stream) < 0) - return { Results::Fail, "Failed to allocate buffers" }; + ASSERT_GE(allocator_->allocate(stream), 0) << "Failed to allocate buffers"; - if (camera_->start()) - return { Results::Fail, "Failed to start camera" }; + ASSERT_TRUE(!camera_->start()) << "Failed to start camera"; camera_->requestCompleted.connect(this, &SimpleCapture::requestComplete); - - return { Results::Pass, "Started camera" }; } void SimpleCapture::stop() @@ -74,22 +72,19 @@ SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr camera) { } -Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) +void SimpleCaptureBalanced::capture(unsigned int numRequests) { - Results::Result ret = start(); - if (ret.first != Results::Pass) - return ret; + start(); Stream *stream = config_->at(0).stream(); const std::vector> &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(); - - return { Results::Skip, "Camera needs " + std::to_string(buffers_size) - + " requests, can't test only " + std::to_string(numRequests) }; + std::cout << "Camera needs " + std::to_string(buffers.size()) + + " requests, can't test only " + + std::to_string(numRequests) << std::endl; + GTEST_SKIP(); } queueCount_ = 0; @@ -100,14 +95,11 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) std::vector> requests; for (const std::unique_ptr &buffer : buffers) { std::unique_ptr request = camera_->createRequest(); - if (!request) - return { Results::Fail, "Can't create request" }; + ASSERT_TRUE(request) << "Can't create request"; - if (request->addBuffer(stream, buffer.get())) - return { Results::Fail, "Can't set buffer for request" }; + ASSERT_FALSE(request->addBuffer(stream, buffer.get())) << "Can't set buffer for request"; - if (queueRequest(request.get()) < 0) - return { Results::Fail, "Failed to queue request" }; + ASSERT_GE(queueRequest(request.get()), 0) << "Failed to queue request"; requests.push_back(std::move(request)); } @@ -118,12 +110,7 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) 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" }; + ASSERT_EQ(captureCount_, captureLimit_); } int SimpleCaptureBalanced::queueRequest(Request *request) @@ -155,11 +142,9 @@ SimpleCaptureUnbalanced::SimpleCaptureUnbalanced(std::shared_ptr camera) { } -Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests) +void SimpleCaptureUnbalanced::capture(unsigned int numRequests) { - Results::Result ret = start(); - if (ret.first != Results::Pass) - return ret; + start(); Stream *stream = config_->at(0).stream(); const std::vector> &buffers = allocator_->buffers(stream); @@ -171,14 +156,11 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests) std::vector> requests; for (const std::unique_ptr &buffer : buffers) { std::unique_ptr request = camera_->createRequest(); - if (!request) - return { Results::Fail, "Can't create request" }; + ASSERT_TRUE(request) << "Can't create request"; - if (request->addBuffer(stream, buffer.get())) - return { Results::Fail, "Can't set buffer for request" }; + ASSERT_FALSE(request->addBuffer(stream, buffer.get())) << "Can't set buffer for request"; - if (camera_->queueRequest(request.get()) < 0) - return { Results::Fail, "Failed to queue request" }; + ASSERT_GE(camera_->queueRequest(request.get()), 0) << "Failed to queue request"; requests.push_back(std::move(request)); } @@ -189,7 +171,7 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests) stop(); delete loop_; - return { status ? Results::Fail : Results::Pass, "Unbalanced capture of " + std::to_string(numRequests) + " requests" }; + ASSERT_FALSE(status); } void SimpleCaptureUnbalanced::requestComplete(Request *request) diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h index d9de53fb63a3..62984243a1fa 100644 --- a/src/lc-compliance/simple_capture.h +++ b/src/lc-compliance/simple_capture.h @@ -12,18 +12,17 @@ #include #include "../cam/event_loop.h" -#include "results.h" class SimpleCapture { public: - Results::Result configure(libcamera::StreamRole role); + void configure(libcamera::StreamRole role); protected: SimpleCapture(std::shared_ptr camera); virtual ~SimpleCapture(); - Results::Result start(); + void start(); void stop(); virtual void requestComplete(libcamera::Request *request) = 0; @@ -40,7 +39,7 @@ class SimpleCaptureBalanced : public SimpleCapture public: SimpleCaptureBalanced(std::shared_ptr camera); - Results::Result capture(unsigned int numRequests); + void capture(unsigned int numRequests); private: int queueRequest(libcamera::Request *request); @@ -56,7 +55,7 @@ class SimpleCaptureUnbalanced : public SimpleCapture public: SimpleCaptureUnbalanced(std::shared_ptr camera); - Results::Result capture(unsigned int numRequests); + void capture(unsigned int numRequests); private: void requestComplete(libcamera::Request *request) override; diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp index 8318b42f42d6..837e17a1c189 100644 --- a/src/lc-compliance/single_stream.cpp +++ b/src/lc-compliance/single_stream.cpp @@ -7,91 +7,121 @@ #include +#include + +#include "environment.h" #include "simple_capture.h" -#include "tests.h" using namespace libcamera; -Results::Result testRequestBalance(std::shared_ptr camera, - StreamRole role, unsigned int startCycles, - unsigned int numRequests) +const std::vector NUMREQUESTS = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 }; +const std::vector ROLES = { Raw, StillCapture, VideoRecording, Viewfinder }; + +class FixedRequestsTest : public testing::TestWithParam> { - SimpleCaptureBalanced capture(camera); +public: + static std::string nameParameters(const testing::TestParamInfo &info); - Results::Result ret = capture.configure(role); - if (ret.first != Results::Pass) - return ret; +protected: + void SetUp() override; + void TearDown() override; - for (unsigned int starts = 0; starts < startCycles; starts++) { - ret = capture.capture(numRequests); - if (ret.first != Results::Pass) - return ret; - } + std::shared_ptr camera_; +}; - return { Results::Pass, "Balanced capture of " + - std::to_string(numRequests) + " requests with " + - std::to_string(startCycles) + " start cycles" }; +/* + * We use gtest's SetUp() and TearDown() instead of constructor and destructor + * in order to be able to assert on them. + */ +void FixedRequestsTest::SetUp() +{ + Environment *env = Environment::get(); + + camera_ = env->cm()->get(env->cameraId()); + + ASSERT_FALSE(camera_->acquire()); } -Results::Result testRequestUnbalance(std::shared_ptr camera, - StreamRole role, unsigned int numRequests) +void FixedRequestsTest::TearDown() { - SimpleCaptureUnbalanced capture(camera); + if (!camera_) + return; - Results::Result ret = capture.configure(role); - if (ret.first != Results::Pass) - return ret; - - return capture.capture(numRequests); + camera_->release(); + camera_.reset(); } -Results testSingleStream(std::shared_ptr camera) +std::string FixedRequestsTest::nameParameters(const testing::TestParamInfo &info) { - static const std::vector> roles = { - { "raw", Raw }, - { "still", StillCapture }, - { "video", VideoRecording }, - { "viewfinder", Viewfinder }, - }; - static const std::vector numRequests = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 }; - - Results results(numRequests.size() * roles.size() * 3); - - 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)); - - /* - * Test unbalanced stop - * - * Makes sure the camera supports a stop with requests queued. - * Example failure is a camera that does not handle cancelation - * of buffers coming back from the video device while stopping. - */ - std::cout << "* Test unbalanced stop" << std::endl; - for (unsigned int num : numRequests) - results.add(testRequestUnbalance(camera, role.second, num)); - } - - return results; + std::map rolesMap = { { Raw, "Raw" }, + { StillCapture, "StillCapture" }, + { VideoRecording, "VideoRecording" }, + { Viewfinder, "Viewfinder" } }; + + std::string roleName = rolesMap[std::get<0>(info.param)]; + std::string numRequestsName = std::to_string(std::get<1>(info.param)); + + return roleName + "_" + numRequestsName; } + +/* + * 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. + */ +TEST_P(FixedRequestsTest, BalancedSingleCycle) +{ + auto [role, numRequests] = GetParam(); + + SimpleCaptureBalanced capture(camera_); + + capture.configure(role); + + capture.capture(numRequests); +}; + +/* + * 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. + */ +TEST_P(FixedRequestsTest, BalancedMultiCycle) +{ + auto [role, numRequests] = GetParam(); + unsigned int numRepeats = 3; + + SimpleCaptureBalanced capture(camera_); + + capture.configure(role); + + for (unsigned int starts = 0; starts < numRepeats; starts++) + capture.capture(numRequests); +}; + +/* + * Test unbalanced stop + * + * Makes sure the camera supports a stop with requests queued. Example failure + * is a camera that does not handle cancelation of buffers coming back from the + * video device while stopping. + */ +TEST_P(FixedRequestsTest, Unbalanced) +{ + auto [role, numRequests] = GetParam(); + + SimpleCaptureUnbalanced capture(camera_); + + capture.configure(role); + + capture.capture(numRequests); +}; + +INSTANTIATE_TEST_SUITE_P(RolesAndRequests, + FixedRequestsTest, + testing::Combine(testing::ValuesIn(ROLES), + testing::ValuesIn(NUMREQUESTS)), + FixedRequestsTest::nameParameters); diff --git a/src/lc-compliance/tests.h b/src/lc-compliance/tests.h deleted file mode 100644 index 396605214e4b..000000000000 --- a/src/lc-compliance/tests.h +++ /dev/null @@ -1,16 +0,0 @@ -/* 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 - -#include "results.h" - -Results testSingleStream(std::shared_ptr camera); - -#endif /* __LC_COMPLIANCE_TESTS_H__ */ From patchwork Thu Jun 10 18:37:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 12559 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id D452FC320B for ; Thu, 10 Jun 2021 18:38:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A153E6892C; Thu, 10 Jun 2021 20:38:41 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7629C68937 for ; Thu, 10 Jun 2021 20:38:40 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:fdf4:5e57:8aee:5d72]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id A31EF1F441FA; Thu, 10 Jun 2021 19:38:38 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Thu, 10 Jun 2021 15:37:32 -0300 Message-Id: <20210610183732.174697-6-nfraprado@collabora.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210610183732.174697-1-nfraprado@collabora.com> References: <20210610183732.174697-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 5/5] lc-compliance: Add list and filter parameters X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add a --list parameter that lists all current tests (by mapping to googletest's --gtest_list_tests). Add a --filter 'filterString' parameter that filters the tests to run (by mapping to googletest's --gtest_filter). Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Niklas Söderlund --- Changes in v7: - Thanks to Niklas: - Removed intermediary filter string variable in Harness::initGtestParameters() Changes in v6: - Thanks to Niklas: - Changed buildGtestParameters() into initGtestParameters() and removed the need for Harness::gtestArgc_ and Harness::gtestArgv_ Changes in v5: - Thanks to Niklas: - Moved buildGtestParameters() inside run() No changes in v4 src/lc-compliance/main.cpp | 69 +++++++++++++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 4 deletions(-) diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp index 8c18845141de..e0bd5892b8e4 100644 --- a/src/lc-compliance/main.cpp +++ b/src/lc-compliance/main.cpp @@ -20,6 +20,8 @@ using namespace libcamera; enum { OptCamera = 'c', + OptList = 'l', + OptFilter = 'f', OptHelp = 'h', }; @@ -45,13 +47,17 @@ public: ~Harness(); int init(); - int run(int argc, char **argv); + int run(char *arg0); private: + int initGtestParameters(char *arg0); void listCameras(); OptionsParser::Options options_; std::unique_ptr cm_; + + const std::map gtestFlag_ = { { "list", "--gtest_list_tests" }, + { "filter", "--gtest_filter" } }; }; Harness::Harness(const OptionsParser::Options &options) @@ -76,6 +82,12 @@ int Harness::init() return ret; } + /* + * No need to initialize the camera if we'll just list tests + */ + if (options_.isSet(OptList)) + return 0; + if (!options_.isSet(OptCamera)) { std::cout << "No camera specified, available cameras:" << std::endl; listCameras(); @@ -97,9 +109,11 @@ int Harness::init() return 0; } -int Harness::run(int argc, char **argv) +int Harness::run(char *arg0) { - ::testing::InitGoogleTest(&argc, argv); + int ret = initGtestParameters(arg0); + if (ret) + return ret; testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener); @@ -112,12 +126,59 @@ void Harness::listCameras() std::cout << "- " << cam.get()->id() << std::endl; } +int Harness::initGtestParameters(char *arg0) +{ + int argc = 0; + char **argv; + std::string filterParam; + + /* + * +2 to have space for both the 0th argument that is needed but not + * used and the null at the end. + */ + argv = (char **) malloc((gtestFlag_.size() + 2) * sizeof(char *)); + if (!argv) + return -ENOMEM; + + argv[argc] = arg0; + argc++; + + if (options_.isSet(OptList)) { + argv[argc] = const_cast(gtestFlag_.at("list").c_str()); + argc++; + } + + if (options_.isSet(OptFilter)) { + /* + * The filter flag needs to be passed as a single parameter, in + * the format --gtest_filter=filterStr + */ + filterParam = gtestFlag_.at("filter") + "=" + + (const std::string&) options_[OptFilter]; + + argv[argc] = const_cast(filterParam.c_str()); + argc++; + } + + argv[argc] = 0; + + ::testing::InitGoogleTest(&argc, argv); + + free(argv); + + return 0; +} + static int parseOptions(int argc, char **argv, OptionsParser::Options *options) { OptionsParser parser; parser.addOption(OptCamera, OptionString, "Specify which camera to operate on, by id", "camera", ArgumentRequired, "camera"); + parser.addOption(OptList, OptionNone, "List all tests and exit", "list"); + parser.addOption(OptFilter, OptionString, + "Specify which tests to run", "filter", + ArgumentRequired, "filter"); parser.addOption(OptHelp, OptionNone, "Display this help message", "help"); @@ -147,5 +208,5 @@ int main(int argc, char **argv) if (ret) return ret; - return harness.run(argc, argv); + return harness.run(argv[0]); }