From patchwork Tue May 21 19:27:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1247 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E315B60E4A for ; Tue, 21 May 2019 21:28:04 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1C72F52C for ; Tue, 21 May 2019 21:28:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558466884; bh=QW83BWw3knb8zhdnVSNSlcZ4egd8D/TFBO33GoK2YPw=; h=From:To:Subject:Date:In-Reply-To:References:From; b=JJYg6fGHhVtmTwN+5ucpN+1jQB3VID+b9eZAMJEavL9wr5+QtnMROpQJMyY+5PLih BcWaWh7QRX1JdCckvOVNiAZHNABxL/OG6zfIuthrpFoUz2sCWnROPJ2y3c+fHtd5L6 YO5M73REpFAqz7sbXw9MMtqmhnyEsT4rohF9KBz4= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 21 May 2019 22:27:38 +0300 Message-Id: <20190521192740.28112-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190521192740.28112-1-laurent.pinchart@ideasonboard.com> References: <20190521192740.28112-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 4/6] libcamera: camera: Return a pointer from generateConfiguration() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 May 2019 19:28:05 -0000 To prepare for specialising the CameraConfiguration class in pipeline handlers, return a pointer to a camera configuration instead of a reference from Camera::generateConfiguration(). The camera configuration always needs to be allocated from the pipeline handler, and its ownership is passed to the application. For symmetry, change Camera::configure() to take a CameraConfiguration pointer instead of a reference. This aligns with our coding practice of passing parameters that are modified by the callee by pointer. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- Changes since v2: - Return an std::unique_ptr<> from Camera::generateConfiguration() - Document that Camera::generateConfiguration() can take an empty list of roles --- include/libcamera/camera.h | 4 +-- src/cam/main.cpp | 38 +++++++++++------------- src/libcamera/camera.cpp | 37 +++++++++++++---------- src/libcamera/include/pipeline_handler.h | 6 ++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 31 +++++++++---------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 +++++++----- src/libcamera/pipeline/uvcvideo.cpp | 24 ++++++++------- src/libcamera/pipeline/vimc.cpp | 24 ++++++++------- src/libcamera/pipeline_handler.cpp | 3 +- src/qcam/main_window.cpp | 6 ++-- src/qcam/main_window.h | 3 +- test/camera/capture.cpp | 32 ++++++++++++++------ test/camera/configuration_default.cpp | 8 ++--- test/camera/configuration_set.cpp | 38 ++++++++++++++++-------- test/camera/statemachine.cpp | 30 +++++++++++++------ 15 files changed, 178 insertions(+), 125 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 284e5276a055..a3a7289a7aa7 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -77,8 +77,8 @@ public: int release(); const std::set &streams() const; - CameraConfiguration generateConfiguration(const StreamRoles &roles); - int configure(CameraConfiguration &config); + std::unique_ptr generateConfiguration(const StreamRoles &roles); + int configure(CameraConfiguration *config); int allocateBuffers(); int freeBuffers(); diff --git a/src/cam/main.cpp b/src/cam/main.cpp index cd165bea34cd..535c2420893e 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -85,15 +85,13 @@ static int parseOptions(int argc, char *argv[]) return 0; } -static int prepareCameraConfig(CameraConfiguration *config) +static std::unique_ptr prepareCameraConfig() { StreamRoles roles; /* If no configuration is provided assume a single video stream. */ - if (!options.isSet(OptStream)) { - *config = camera->generateConfiguration({ StreamRole::VideoRecording }); - return 0; - } + if (!options.isSet(OptStream)) + return camera->generateConfiguration({ StreamRole::VideoRecording }); const std::vector &streamOptions = options[OptStream].toArray(); @@ -113,23 +111,22 @@ static int prepareCameraConfig(CameraConfiguration *config) } else { std::cerr << "Unknown stream role " << conf["role"].toString() << std::endl; - return -EINVAL; + return nullptr; } } - *config = camera->generateConfiguration(roles); - - if (!config->isValid()) { + std::unique_ptr config = camera->generateConfiguration(roles); + if (!config || !config->isValid()) { std::cerr << "Failed to get default stream configuration" << std::endl; - return -EINVAL; + return nullptr; } /* Apply configuration explicitly requested. */ unsigned int i = 0; for (auto const &value : streamOptions) { KeyValueParser::Options conf = value.toKeyValues(); - StreamConfiguration &cfg = (*config)[i++]; + StreamConfiguration &cfg = config->at(i++); if (conf.isSet("width")) cfg.size.width = conf["width"]; @@ -142,7 +139,7 @@ static int prepareCameraConfig(CameraConfiguration *config) cfg.pixelFormat = conf["pixelformat"]; } - return 0; + return config; } static void requestComplete(Request *request, const std::map &buffers) @@ -191,16 +188,15 @@ static void requestComplete(Request *request, const std::map static int capture() { - CameraConfiguration config; int ret; - ret = prepareCameraConfig(&config); - if (ret) { + std::unique_ptr config = prepareCameraConfig(); + if (!config) { std::cout << "Failed to prepare camera configuration" << std::endl; - return ret; + return -EINVAL; } - ret = camera->configure(config); + ret = camera->configure(config.get()); if (ret < 0) { std::cout << "Failed to configure camera" << std::endl; return ret; @@ -208,8 +204,8 @@ static int capture() streamInfo.clear(); - for (unsigned int index = 0; index < config.size(); ++index) { - StreamConfiguration &cfg = config[index]; + for (unsigned int index = 0; index < config->size(); ++index) { + StreamConfiguration &cfg = config->at(index); streamInfo[cfg.stream()] = "stream" + std::to_string(index); } @@ -224,7 +220,7 @@ static int capture() /* Identify the stream with the least number of buffers. */ unsigned int nbuffers = UINT_MAX; - for (StreamConfiguration &cfg : config) { + for (StreamConfiguration &cfg : *config) { Stream *stream = cfg.stream(); nbuffers = std::min(nbuffers, stream->bufferPool().count()); } @@ -244,7 +240,7 @@ static int capture() } std::map map; - for (StreamConfiguration &cfg : config) { + for (StreamConfiguration &cfg : *config) { Stream *stream = cfg.stream(); map[stream] = &stream->bufferPool().buffers()[i]; } diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 5848330f639a..0e5fd7f57271 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -540,27 +540,32 @@ const std::set &Camera::streams() const * * Generate a camera configuration for a set of desired stream roles. The caller * specifies a list of stream roles and the camera returns a configuration - * containing suitable streams and their suggested default configurations. + * containing suitable streams and their suggested default configurations. An + * empty list of roles is valid, and will generate an empty configuration that + * can be filled by the caller. * - * \return A valid CameraConfiguration if the requested roles can be satisfied, - * or a invalid one otherwise + * \return A CameraConfiguration if the requested roles can be satisfied, or a + * null pointer otherwise. The ownership of the returned configuration is + * passed to the caller. */ -CameraConfiguration -Camera::generateConfiguration(const StreamRoles &roles) +std::unique_ptr Camera::generateConfiguration(const StreamRoles &roles) { - if (disconnected_ || !roles.size() || roles.size() > streams_.size()) - return CameraConfiguration(); + if (disconnected_ || roles.size() > streams_.size()) + return nullptr; - CameraConfiguration config = pipe_->generateConfiguration(this, roles); + CameraConfiguration *config = pipe_->generateConfiguration(this, roles); std::ostringstream msg("streams configuration:", std::ios_base::ate); - for (unsigned int index = 0; index < config.size(); ++index) - msg << " (" << index << ") " << config[index].toString(); + if (config->empty()) + msg << " empty"; + + for (unsigned int index = 0; index < config->size(); ++index) + msg << " (" << index << ") " << config->at(index).toString(); LOG(Camera, Debug) << msg.str(); - return config; + return std::unique_ptr(config); } /** @@ -590,7 +595,7 @@ Camera::generateConfiguration(const StreamRoles &roles) * \retval -EACCES The camera is not in a state where it can be configured * \retval -EINVAL The configuration is not valid */ -int Camera::configure(CameraConfiguration &config) +int Camera::configure(CameraConfiguration *config) { int ret; @@ -600,7 +605,7 @@ int Camera::configure(CameraConfiguration &config) if (!stateBetween(CameraAcquired, CameraConfigured)) return -EACCES; - if (!config.isValid()) { + if (!config->isValid()) { LOG(Camera, Error) << "Can't configure camera with invalid configuration"; return -EINVAL; @@ -608,8 +613,8 @@ int Camera::configure(CameraConfiguration &config) std::ostringstream msg("configuring streams:", std::ios_base::ate); - for (unsigned int index = 0; index < config.size(); ++index) { - StreamConfiguration &cfg = config[index]; + for (unsigned int index = 0; index < config->size(); ++index) { + StreamConfiguration &cfg = config->at(index); cfg.setStream(nullptr); msg << " (" << index << ") " << cfg.toString(); } @@ -621,7 +626,7 @@ int Camera::configure(CameraConfiguration &config) return ret; activeStreams_.clear(); - for (const StreamConfiguration &cfg : config) { + for (const StreamConfiguration &cfg : *config) { Stream *stream = cfg.stream(); if (!stream) LOG(Camera, Fatal) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index a025905ab68f..7da6df1ab2a0 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -60,9 +60,9 @@ public: bool lock(); void unlock(); - virtual CameraConfiguration - generateConfiguration(Camera *camera, const StreamRoles &roles) = 0; - virtual int configure(Camera *camera, CameraConfiguration &config) = 0; + virtual CameraConfiguration *generateConfiguration(Camera *camera, + const StreamRoles &roles) = 0; + virtual int configure(Camera *camera, CameraConfiguration *config) = 0; virtual int allocateBuffers(Camera *camera, const std::set &streams) = 0; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ed0ef69de1d1..3acf82ff4665 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -150,9 +150,9 @@ class PipelineHandlerIPU3 : public PipelineHandler public: PipelineHandlerIPU3(CameraManager *manager); - CameraConfiguration - generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, CameraConfiguration &config) override; + CameraConfiguration *generateConfiguration(Camera *camera, + const StreamRoles &roles) override; + int configure(Camera *camera, CameraConfiguration *config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -207,12 +207,11 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) { } -CameraConfiguration -PipelineHandlerIPU3::generateConfiguration(Camera *camera, - const StreamRoles &roles) +CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, + const StreamRoles &roles) { IPU3CameraData *data = cameraData(camera); - CameraConfiguration config = {}; + CameraConfiguration *config = new CameraConfiguration(); std::set streams = { &data->outStream_, &data->vfStream_, @@ -290,21 +289,23 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, break; } - if (!stream) - return CameraConfiguration{}; + if (!stream) { + delete config; + return nullptr; + } streams.erase(stream); cfg.pixelFormat = V4L2_PIX_FMT_NV12; cfg.bufferCount = IPU3_BUFFER_COUNT; - config.addConfiguration(cfg); + config->addConfiguration(cfg); } return config; } -int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config) +int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *config) { IPU3CameraData *data = cameraData(camera); IPU3Stream *outStream = &data->outStream_; @@ -316,7 +317,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config) outStream->active_ = false; vfStream->active_ = false; - for (StreamConfiguration &cfg : config) { + for (StreamConfiguration &cfg : *config) { /* * Pick a stream for the configuration entry. * \todo: This is a naive temporary implementation that will be @@ -382,7 +383,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config) return ret; /* Apply the format to the configured streams output devices. */ - for (StreamConfiguration &cfg : config) { + for (StreamConfiguration &cfg : *config) { IPU3Stream *stream = static_cast(cfg.stream()); ret = imgu->configureOutput(stream->device_, cfg); if (ret) @@ -395,13 +396,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config) * be at least one active stream in the configuration request). */ if (!outStream->active_) { - ret = imgu->configureOutput(outStream->device_, config[0]); + ret = imgu->configureOutput(outStream->device_, config->at(0)); if (ret) return ret; } if (!vfStream->active_) { - ret = imgu->configureOutput(vfStream->device_, config[0]); + ret = imgu->configureOutput(vfStream->device_, config->at(0)); if (ret) return ret; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index ec6980b0943a..c8d217abdca8 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -34,9 +34,9 @@ public: PipelineHandlerRkISP1(CameraManager *manager); ~PipelineHandlerRkISP1(); - CameraConfiguration generateConfiguration(Camera *camera, + CameraConfiguration *generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, CameraConfiguration &config) override; + int configure(Camera *camera, CameraConfiguration *config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -105,26 +105,29 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1() * Pipeline Operations */ -CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera, +CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera, const StreamRoles &roles) { RkISP1CameraData *data = cameraData(camera); - CameraConfiguration config; + CameraConfiguration *config = new CameraConfiguration(); + + if (roles.empty()) + return config; + StreamConfiguration cfg{}; - cfg.pixelFormat = V4L2_PIX_FMT_NV12; cfg.size = data->sensor_->resolution(); cfg.bufferCount = RKISP1_BUFFER_COUNT; - config.addConfiguration(cfg); + config->addConfiguration(cfg); return config; } -int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration &config) +int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *config) { RkISP1CameraData *data = cameraData(camera); - StreamConfiguration &cfg = config[0]; + StreamConfiguration &cfg = config->at(0); CameraSensor *sensor = data->sensor_; int ret; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 5dcc868b2fc9..120d8d3a1522 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -25,9 +25,9 @@ class PipelineHandlerUVC : public PipelineHandler public: PipelineHandlerUVC(CameraManager *manager); - CameraConfiguration - generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, CameraConfiguration &config) override; + CameraConfiguration *generateConfiguration(Camera *camera, + const StreamRoles &roles) override; + int configure(Camera *camera, CameraConfiguration *config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -73,26 +73,28 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) { } -CameraConfiguration -PipelineHandlerUVC::generateConfiguration(Camera *camera, - const StreamRoles &roles) +CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera, + const StreamRoles &roles) { - CameraConfiguration config; - StreamConfiguration cfg; + CameraConfiguration *config = new CameraConfiguration(); + if (roles.empty()) + return config; + + StreamConfiguration cfg{}; cfg.pixelFormat = V4L2_PIX_FMT_YUYV; cfg.size = { 640, 480 }; cfg.bufferCount = 4; - config.addConfiguration(cfg); + config->addConfiguration(cfg); return config; } -int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration &config) +int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) { UVCCameraData *data = cameraData(camera); - StreamConfiguration &cfg = config[0]; + StreamConfiguration &cfg = config->at(0); int ret; V4L2DeviceFormat format = {}; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index af6b6f21e3c5..f6aa32683e5e 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -25,9 +25,9 @@ class PipelineHandlerVimc : public PipelineHandler public: PipelineHandlerVimc(CameraManager *manager); - CameraConfiguration - generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, CameraConfiguration &config) override; + CameraConfiguration *generateConfiguration(Camera *camera, + const StreamRoles &roles) override; + int configure(Camera *camera, CameraConfiguration *config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -73,26 +73,28 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) { } -CameraConfiguration -PipelineHandlerVimc::generateConfiguration(Camera *camera, - const StreamRoles &roles) +CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera, + const StreamRoles &roles) { - CameraConfiguration config; - StreamConfiguration cfg; + CameraConfiguration *config = new CameraConfiguration(); + if (roles.empty()) + return config; + + StreamConfiguration cfg{}; cfg.pixelFormat = V4L2_PIX_FMT_RGB24; cfg.size = { 640, 480 }; cfg.bufferCount = 4; - config.addConfiguration(cfg); + config->addConfiguration(cfg); return config; } -int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration &config) +int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) { VimcCameraData *data = cameraData(camera); - StreamConfiguration &cfg = config[0]; + StreamConfiguration &cfg = config->at(0); int ret; V4L2DeviceFormat format = {}; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 4185e3557dcb..de46e98880a2 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -233,7 +233,8 @@ void PipelineHandler::unlock() * the group of streams parameters. * * \return A valid CameraConfiguration if the requested roles can be satisfied, - * or a invalid configuration otherwise + * or a null pointer otherwise. The ownership of the returned configuration is + * passed to the caller. */ /** diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 06ae2985f80d..16b123132dd9 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -98,13 +98,13 @@ int MainWindow::startCapture() int ret; config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); - ret = camera_->configure(config_); + ret = camera_->configure(config_.get()); if (ret < 0) { std::cout << "Failed to configure camera" << std::endl; return ret; } - const StreamConfiguration &cfg = config_[0]; + const StreamConfiguration &cfg = config_->at(0); Stream *stream = cfg.stream(); ret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width, cfg.size.height); @@ -180,6 +180,8 @@ void MainWindow::stopCapture() camera_->freeBuffers(); isCapturing_ = false; + + config_.reset(); } void MainWindow::requestComplete(Request *request, diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 143b5b08a5a0..fe565cbcb460 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -8,6 +8,7 @@ #define __QCAM_MAIN_WINDOW_H__ #include +#include #include @@ -45,7 +46,7 @@ private: std::shared_ptr camera_; bool isCapturing_; - CameraConfiguration config_; + std::unique_ptr config_; ViewFinder *viewfinder_; }; diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index bfd11eefedcf..bb7d380cdc1a 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -40,13 +40,25 @@ protected: camera_->queueRequest(request); } - int run() + int init() override { - CameraConfiguration config = - camera_->generateConfiguration({ StreamRole::VideoRecording }); - StreamConfiguration *cfg = &config[0]; + CameraTest::init(); - if (!config.isValid()) { + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); + if (!config_) { + cout << "Failed to generate default configuration" << endl; + CameraTest::cleanup(); + return TestFail; + } + + return TestPass; + } + + int run() override + { + StreamConfiguration &cfg = config_->at(0); + + if (!config_->isValid()) { cout << "Failed to read default configuration" << endl; return TestFail; } @@ -56,7 +68,7 @@ protected: return TestFail; } - if (camera_->configure(config)) { + if (camera_->configure(config_.get())) { cout << "Failed to set default configuration" << endl; return TestFail; } @@ -66,7 +78,7 @@ protected: return TestFail; } - Stream *stream = cfg->stream(); + Stream *stream = cfg.stream(); BufferPool &pool = stream->bufferPool(); std::vector requests; for (Buffer &buffer : pool.buffers()) { @@ -110,10 +122,10 @@ protected: while (timer.isRunning()) dispatcher->processEvents(); - if (completeRequestsCount_ <= cfg->bufferCount * 2) { + if (completeRequestsCount_ <= cfg.bufferCount * 2) { cout << "Failed to capture enough frames (got " << completeRequestsCount_ << " expected at least " - << cfg->bufferCount * 2 << ")" << endl; + << cfg.bufferCount * 2 << ")" << endl; return TestFail; } @@ -134,6 +146,8 @@ protected: return TestPass; } + + std::unique_ptr config_; }; } /* namespace */ diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp index 8c4a03db498a..8a767d2321e0 100644 --- a/test/camera/configuration_default.cpp +++ b/test/camera/configuration_default.cpp @@ -18,21 +18,21 @@ class ConfigurationDefault : public CameraTest protected: int run() { - CameraConfiguration config; + std::unique_ptr config; /* Test asking for configuration for a video stream. */ config = camera_->generateConfiguration({ StreamRole::VideoRecording }); - if (!config.isValid()) { + if (!config || !config->isValid()) { cout << "Default configuration invalid" << endl; return TestFail; } /* * Test that asking for configuration for an empty array of - * stream roles returns an empty list of configurations. + * stream roles returns an empty camera configuration. */ config = camera_->generateConfiguration({}); - if (config.isValid()) { + if (!config || config->isValid()) { cout << "Failed to retrieve configuration for empty roles list" << endl; return TestFail; diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp index 25b5db67103a..ece987c7752a 100644 --- a/test/camera/configuration_set.cpp +++ b/test/camera/configuration_set.cpp @@ -16,13 +16,25 @@ namespace { class ConfigurationSet : public CameraTest { protected: - int run() + int init() override { - CameraConfiguration config = - camera_->generateConfiguration({ StreamRole::VideoRecording }); - StreamConfiguration *cfg = &config[0]; + CameraTest::init(); - if (!config.isValid()) { + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); + if (!config_) { + cout << "Failed to generate default configuration" << endl; + CameraTest::cleanup(); + return TestFail; + } + + return TestPass; + } + + int run() override + { + StreamConfiguration &cfg = config_->at(0); + + if (!config_->isValid()) { cout << "Failed to read default configuration" << endl; return TestFail; } @@ -33,7 +45,7 @@ protected: } /* Test that setting the default configuration works. */ - if (camera_->configure(config)) { + if (camera_->configure(config_.get())) { cout << "Failed to set default configuration" << endl; return TestFail; } @@ -48,7 +60,7 @@ protected: return TestFail; } - if (!camera_->configure(config)) { + if (!camera_->configure(config_.get())) { cout << "Setting configuration on a camera not acquired succeeded when it should have failed" << endl; return TestFail; @@ -64,9 +76,9 @@ protected: * the default configuration of the VIMC camera is known to * work. */ - cfg->size.width *= 2; - cfg->size.height *= 2; - if (camera_->configure(config)) { + cfg.size.width *= 2; + cfg.size.height *= 2; + if (camera_->configure(config_.get())) { cout << "Failed to set modified configuration" << endl; return TestFail; } @@ -74,14 +86,16 @@ protected: /* * Test that setting an invalid configuration fails. */ - cfg->size = { 0, 0 }; - if (!camera_->configure(config)) { + cfg.size = { 0, 0 }; + if (!camera_->configure(config_.get())) { cout << "Invalid configuration incorrectly accepted" << endl; return TestFail; } return TestPass; } + + std::unique_ptr config_; }; } /* namespace */ diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index 7a74cd85a37a..d489f197e402 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -19,7 +19,7 @@ protected: int testAvailable() { /* Test operations which should fail. */ - if (camera_->configure(defconf_) != -EACCES) + if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; if (camera_->allocateBuffers() != -EACCES) @@ -84,7 +84,7 @@ protected: if (camera_->acquire()) return TestFail; - if (camera_->configure(defconf_)) + if (camera_->configure(defconf_.get())) return TestFail; return TestPass; @@ -113,7 +113,7 @@ protected: return TestFail; /* Test operations which should pass. */ - if (camera_->configure(defconf_)) + if (camera_->configure(defconf_.get())) return TestFail; /* Test valid state transitions, end in Prepared state. */ @@ -123,7 +123,7 @@ protected: if (camera_->acquire()) return TestFail; - if (camera_->configure(defconf_)) + if (camera_->configure(defconf_.get())) return TestFail; if (camera_->allocateBuffers()) @@ -141,7 +141,7 @@ protected: if (camera_->release() != -EBUSY) return TestFail; - if (camera_->configure(defconf_) != -EACCES) + if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; if (camera_->allocateBuffers() != -EACCES) @@ -172,7 +172,7 @@ protected: if (camera_->acquire()) return TestFail; - if (camera_->configure(defconf_)) + if (camera_->configure(defconf_.get())) return TestFail; if (camera_->allocateBuffers()) @@ -193,7 +193,7 @@ protected: if (camera_->release() != -EBUSY) return TestFail; - if (camera_->configure(defconf_) != -EACCES) + if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; if (camera_->allocateBuffers() != -EACCES) @@ -233,10 +233,22 @@ protected: return TestPass; } - int run() + int init() override { + CameraTest::init(); + defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); + if (!defconf_) { + cout << "Failed to generate default configuration" << endl; + CameraTest::cleanup(); + return TestFail; + } + return TestPass; + } + + int run() override + { if (testAvailable() != TestPass) { cout << "State machine in Available state failed" << endl; return TestFail; @@ -265,7 +277,7 @@ protected: return TestPass; } - CameraConfiguration defconf_; + std::unique_ptr defconf_; }; } /* namespace */