[libcamera-devel] [PATCH/RFC 05/12] libcamera: camera: Return a pointer from generateConfiguration()

Laurent Pinchart laurent.pinchart at ideasonboard.com
Sat May 18 01:06:14 CEST 2019


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 <laurent.pinchart at ideasonboard.com>
---
 include/libcamera/camera.h               |  4 +--
 src/cam/main.cpp                         | 38 ++++++++++----------
 src/libcamera/camera.cpp                 | 31 +++++++++--------
 src/libcamera/include/pipeline_handler.h |  6 ++--
 src/libcamera/pipeline/ipu3/ipu3.cpp     | 31 +++++++++--------
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 ++++++++------
 src/libcamera/pipeline/uvcvideo.cpp      | 30 ++++++++--------
 src/libcamera/pipeline/vimc.cpp          | 30 ++++++++--------
 src/libcamera/pipeline_handler.cpp       |  3 +-
 src/qcam/main_window.cpp                 |  5 ++-
 src/qcam/main_window.h                   |  2 +-
 test/camera/capture.cpp                  | 38 +++++++++++++++-----
 test/camera/configuration_default.cpp    | 14 +++++---
 test/camera/configuration_set.cpp        | 44 +++++++++++++++++-------
 test/camera/statemachine.cpp             | 22 ++++++++++--
 15 files changed, 201 insertions(+), 122 deletions(-)

diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
index e2759405f497..841e8fc505b9 100644
--- a/include/libcamera/camera.h
+++ b/include/libcamera/camera.h
@@ -68,8 +68,8 @@ public:
 	int release();
 
 	const std::set<Stream *> &streams() const;
-	CameraConfiguration generateConfiguration(const StreamRoles &roles);
-	int configure(CameraConfiguration &config);
+	CameraConfiguration *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..7a1b332f68c7 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 CameraConfiguration *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<OptionValue> &streamOptions =
 		options[OptStream].toArray();
@@ -113,16 +111,16 @@ 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()) {
+	CameraConfiguration *config = camera->generateConfiguration(roles);
+	if (!config || !config->isValid()) {
 		std::cerr << "Failed to get default stream configuration"
 			  << std::endl;
-		return -EINVAL;
+		delete config;
+		return nullptr;
 	}
 
 	/* Apply configuration explicitly requested. */
@@ -142,7 +140,7 @@ static int prepareCameraConfig(CameraConfiguration *config)
 			cfg.pixelFormat = conf["pixelformat"];
 	}
 
-	return 0;
+	return config;
 }
 
 static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)
@@ -191,25 +189,25 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *>
 
 static int capture()
 {
-	CameraConfiguration config;
 	int ret;
 
-	ret = prepareCameraConfig(&config);
-	if (ret) {
+	CameraConfiguration *config = prepareCameraConfig();
+	if (!config) {
 		std::cout << "Failed to prepare camera configuration" << std::endl;
-		return ret;
+		return -EINVAL;
 	}
 
 	ret = camera->configure(config);
 	if (ret < 0) {
 		std::cout << "Failed to configure camera" << std::endl;
+		delete config;
 		return ret;
 	}
 
 	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)[index];
 		streamInfo[cfg.stream()] = "stream" + std::to_string(index);
 	}
 
@@ -217,6 +215,7 @@ static int capture()
 	if (ret) {
 		std::cerr << "Failed to allocate buffers"
 			  << std::endl;
+		delete config;
 		return ret;
 	}
 
@@ -224,7 +223,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 +243,7 @@ static int capture()
 		}
 
 		std::map<Stream *, Buffer *> map;
-		for (StreamConfiguration &cfg : config) {
+		for (StreamConfiguration &cfg : *config) {
 			Stream *stream = cfg.stream();
 			map[stream] = &stream->bufferPool().buffers()[i];
 		}
@@ -280,6 +279,7 @@ static int capture()
 		std::cout << "Failed to stop capture" << std::endl;
 out:
 	camera->freeBuffers();
+	delete config;
 
 	return ret;
 }
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index f8a4946b4a6a..115cdb1c024b 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -518,21 +518,24 @@ const std::set<Stream *> &Camera::streams() const
  * specifies a list of stream roles and the camera returns a configuration
  * containing suitable streams and their suggested default configurations.
  *
- * \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)
+CameraConfiguration *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->isEmpty())
+		msg << " empty";
+
+	for (unsigned int index = 0; index < config->size(); ++index)
+		msg << " (" << index << ") " << (*config)[index].toString();
 
 	LOG(Camera, Debug) << msg.str();
 
@@ -566,7 +569,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;
 
@@ -576,7 +579,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;
@@ -584,8 +587,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)[index];
 		cfg.setStream(nullptr);
 		msg << " (" << index << ") " << cfg.toString();
 	}
@@ -597,7 +600,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<Stream *> &streams) = 0;
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index ed0ef69de1d1..7c6f2d4a23be 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<Stream *> &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<IPU3Stream *> 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<IPU3Stream *>(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)[0]);
 		if (ret)
 			return ret;
 	}
 
 	if (!vfStream->active_) {
-		ret = imgu->configureOutput(vfStream->device_, config[0]);
+		ret = imgu->configureOutput(vfStream->device_, (*config)[0]);
 		if (ret)
 			return ret;
 	}
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 6b35aa3fe7c3..c3b3912c96f3 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<Stream *> &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;
-	StreamConfiguration cfg{};
+	CameraConfiguration *config = new CameraConfiguration();
 
-	cfg.pixelFormat = V4L2_PIX_FMT_NV12;
-	cfg.size = data->sensor_->resolution();
-	cfg.bufferCount = RKISP1_BUFFER_COUNT;
+	if (!roles.empty()) {
+		StreamConfiguration cfg{};
 
-	config.addConfiguration(cfg);
+		cfg.pixelFormat = V4L2_PIX_FMT_NV12;
+		cfg.size = data->sensor_->resolution();
+		cfg.bufferCount = RKISP1_BUFFER_COUNT;
+
+		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)[0];
 	CameraSensor *sensor = data->sensor_;
 	int ret;
 
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index 5dcc868b2fc9..5c061ca61016 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<Stream *> &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();
 
-	cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
-	cfg.size = { 640, 480 };
-	cfg.bufferCount = 4;
+	if (!roles.empty()) {
+		StreamConfiguration cfg;
 
-	config.addConfiguration(cfg);
+		cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
+		cfg.size = { 640, 480 };
+		cfg.bufferCount = 4;
+
+		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)[0];
 	int ret;
 
 	V4L2DeviceFormat format = {};
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index af6b6f21e3c5..0ece97f09e7e 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<Stream *> &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();
 
-	cfg.pixelFormat = V4L2_PIX_FMT_RGB24;
-	cfg.size = { 640, 480 };
-	cfg.bufferCount = 4;
+	if (!roles.empty()) {
+		StreamConfiguration cfg;
 
-	config.addConfiguration(cfg);
+		cfg.pixelFormat = V4L2_PIX_FMT_RGB24;
+		cfg.size = { 640, 480 };
+		cfg.bufferCount = 4;
+
+		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)[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..9f0454b7a5f3 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -104,7 +104,7 @@ int MainWindow::startCapture()
 		return ret;
 	}
 
-	const StreamConfiguration &cfg = config_[0];
+	const StreamConfiguration &cfg = (*config_)[0];
 	Stream *stream = cfg.stream();
 	ret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width,
 				     cfg.size.height);
@@ -180,6 +180,9 @@ void MainWindow::stopCapture()
 
 	camera_->freeBuffers();
 	isCapturing_ = false;
+
+	delete config_;
+	config_ = nullptr;
 }
 
 void MainWindow::requestComplete(Request *request,
diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
index 143b5b08a5a0..866866e93d23 100644
--- a/src/qcam/main_window.h
+++ b/src/qcam/main_window.h
@@ -45,7 +45,7 @@ private:
 
 	std::shared_ptr<Camera> camera_;
 	bool isCapturing_;
-	CameraConfiguration config_;
+	CameraConfiguration *config_;
 
 	ViewFinder *viewfinder_;
 };
diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
index bfd11eefedcf..f640e80fbaf3 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_)[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_)) {
 			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<Request *> 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,14 @@ protected:
 
 		return TestPass;
 	}
+
+	void cleanup() override
+	{
+		delete config_;
+		CameraTest::cleanup();
+	}
+
+	CameraConfiguration *config_;
 };
 
 } /* namespace */
diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp
index 8c4a03db498a..d5cefc1127c9 100644
--- a/test/camera/configuration_default.cpp
+++ b/test/camera/configuration_default.cpp
@@ -18,26 +18,32 @@ class ConfigurationDefault : public CameraTest
 protected:
 	int run()
 	{
-		CameraConfiguration config;
+		CameraConfiguration *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;
+			delete config;
 			return TestFail;
 		}
 
+		delete config;
+
 		/*
 		 * 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;
+			delete config;
 			return TestFail;
 		}
 
+		delete config;
+
 		return TestPass;
 	}
 };
diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
index 25b5db67103a..ca911f459c32 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_)[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_)) {
 			cout << "Failed to set default configuration" << endl;
 			return TestFail;
 		}
@@ -48,7 +60,7 @@ protected:
 			return TestFail;
 		}
 
-		if (!camera_->configure(config)) {
+		if (!camera_->configure(config_)) {
 			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_)) {
 			cout << "Failed to set modified configuration" << endl;
 			return TestFail;
 		}
@@ -74,14 +86,22 @@ protected:
 		/*
 		 * Test that setting an invalid configuration fails.
 		 */
-		cfg->size = { 0, 0 };
-		if (!camera_->configure(config)) {
+		cfg.size = { 0, 0 };
+		if (!camera_->configure(config_)) {
 			cout << "Invalid configuration incorrectly accepted" << endl;
 			return TestFail;
 		}
 
 		return TestPass;
 	}
+
+	void cleanup() override
+	{
+		delete config_;
+		CameraTest::cleanup();
+	}
+
+	CameraConfiguration *config_;
 };
 
 } /* namespace */
diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
index 7a74cd85a37a..a662e869fadf 100644
--- a/test/camera/statemachine.cpp
+++ b/test/camera/statemachine.cpp
@@ -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,13 @@ protected:
 		return TestPass;
 	}
 
-	CameraConfiguration defconf_;
+	void cleanup() override
+	{
+		delete defconf_;
+		CameraTest::cleanup();
+	}
+
+	CameraConfiguration *defconf_;
 };
 
 } /* namespace */
-- 
Regards,

Laurent Pinchart



More information about the libcamera-devel mailing list