[libcamera-devel,v4,8/8] libcamera: Switch to CameraConfiguration

Message ID 20190408134817.15247-9-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: stream: Add basic stream usages
Related show

Commit Message

Niklas Söderlund April 8, 2019, 1:48 p.m. UTC
Implement the camera configuration thru out the library, tests, cam and
qcam tools.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/camera.h               |  4 ++--
 src/cam/main.cpp                         |  8 ++++----
 src/libcamera/camera.cpp                 | 25 ++++++++++++------------
 src/libcamera/include/pipeline_handler.h |  6 +++---
 src/libcamera/pipeline/ipu3/ipu3.cpp     | 10 +++++-----
 src/libcamera/pipeline/uvcvideo.cpp      | 12 ++++++------
 src/libcamera/pipeline/vimc.cpp          | 12 ++++++------
 src/libcamera/pipeline_handler.cpp       |  6 +++---
 src/qcam/main_window.cpp                 |  2 +-
 src/qcam/main_window.h                   |  2 +-
 test/camera/camera_test.cpp              | 18 -----------------
 test/camera/camera_test.h                |  2 --
 test/camera/capture.cpp                  |  8 ++++----
 test/camera/configuration_default.cpp    | 12 +++---------
 test/camera/configuration_set.cpp        |  6 +++---
 test/camera/statemachine.cpp             |  2 +-
 16 files changed, 54 insertions(+), 81 deletions(-)

Patch

diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
index 6038da63e9c5c67f..b2dafda342fe6244 100644
--- a/include/libcamera/camera.h
+++ b/include/libcamera/camera.h
@@ -73,9 +73,9 @@  public:
 	int release();
 
 	const std::set<Stream *> &streams() const;
-	std::map<Stream *, StreamConfiguration>
+	CameraConfiguration
 	streamConfiguration(const std::vector<StreamUsage> &usage);
-	int configureStreams(std::map<Stream *, StreamConfiguration> &config);
+	int configureStreams(const CameraConfiguration &config);
 
 	int allocateBuffers();
 	int freeBuffers();
diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index d45ffd372d932d76..99ce564afd695b07 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -78,10 +78,10 @@  static int parseOptions(int argc, char *argv[])
 	return 0;
 }
 
-static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config)
+static int prepareCameraConfig(CameraConfiguration *config)
 {
 	*config = camera->streamConfiguration({ Stream::VideoRecording() });
-	Stream *stream = config->begin()->first;
+	Stream *stream = config->front();
 
 	if (options.isSet(OptFormat)) {
 		KeyValueParser::Options format = options[OptFormat];
@@ -135,7 +135,7 @@  static void requestComplete(Request *request, const std::map<Stream *, Buffer *>
 
 static int capture()
 {
-	std::map<Stream *, StreamConfiguration> config;
+	CameraConfiguration config;
 	std::vector<Request *> requests;
 	int ret;
 
@@ -151,7 +151,7 @@  static int capture()
 		return ret;
 	}
 
-	Stream *stream = config.begin()->first;
+	Stream *stream = config.front();
 
 	ret = camera->allocateBuffers();
 	if (ret) {
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index c1b9561304b04ab5..e056a1232b1b1cd2 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -544,21 +544,21 @@  const std::set<Stream *> &Camera::streams() const
  * list of stream usages and the camera returns a map of suitable streams and
  * their suggested default configurations.
  *
- * \return A map of streams to configurations if the requested usages can be
- * satisfied, or an empty map otherwise
+ * \return A valid CameraConfiguration if the requested usages can be satisfied,
+ * or a invalid one otherwise
  */
-std::map<Stream *, StreamConfiguration>
+CameraConfiguration
 Camera::streamConfiguration(const std::vector<StreamUsage> &usages)
 {
 	if (disconnected_ || !usages.size() || usages.size() > streams_.size())
-		return std::map<Stream *, StreamConfiguration>{};
+		return CameraConfiguration();
 
 	return pipe_->streamConfiguration(this, usages);
 }
 
 /**
  * \brief Configure the camera's streams prior to capture
- * \param[in] config A map of stream IDs and configurations to setup
+ * \param[in] config The camera configurations to setup
  *
  * Prior to starting capture, the camera must be configured to select a
  * group of streams to be involved in the capture and their configuration.
@@ -580,7 +580,7 @@  Camera::streamConfiguration(const std::vector<StreamUsage> &usages)
  * \retval -EACCES The camera is not in a state where it can be configured
  * \retval -EINVAL The configuration is not valid
  */
-int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
+int Camera::configureStreams(const CameraConfiguration &config)
 {
 	int ret;
 
@@ -590,14 +590,14 @@  int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
 	if (!stateBetween(CameraAcquired, CameraConfigured))
 		return -EACCES;
 
-	if (!config.size()) {
+	if (!config.isValid()) {
 		LOG(Camera, Error)
-			<< "Can't configure streams without a configuration";
+			<< "Can't configure camera with invalid configuration";
 		return -EINVAL;
 	}
 
-	for (auto const &iter : config) {
-		if (streams_.find(iter.first) == streams_.end())
+	for (Stream *stream : config) {
+		if (streams_.find(stream) == streams_.end())
 			return -EINVAL;
 	}
 
@@ -606,9 +606,8 @@  int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
 		return ret;
 
 	activeStreams_.clear();
-	for (auto const &iter : config) {
-		Stream *stream = iter.first;
-		const StreamConfiguration &cfg = iter.second;
+	for (Stream *stream : config) {
+		const StreamConfiguration &cfg = config[stream];
 
 		stream->configuration_ = cfg;
 		activeStreams_.insert(stream);
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
index 8a1706fad5377bf4..b6cbd3bae51b08dc 100644
--- a/src/libcamera/include/pipeline_handler.h
+++ b/src/libcamera/include/pipeline_handler.h
@@ -18,6 +18,7 @@  namespace libcamera {
 class Buffer;
 class BufferPool;
 class Camera;
+class CameraConfiguration;
 class CameraManager;
 class DeviceEnumerator;
 class MediaDevice;
@@ -52,10 +53,9 @@  public:
 
 	virtual bool match(DeviceEnumerator *enumerator) = 0;
 
-	virtual std::map<Stream *, StreamConfiguration>
+	virtual CameraConfiguration
 	streamConfiguration(Camera *camera, const std::vector<StreamUsage> &usages) = 0;
-	virtual int configureStreams(Camera *camera,
-				     std::map<Stream *, StreamConfiguration> &config) = 0;
+	virtual int configureStreams(Camera *camera, const CameraConfiguration &config) = 0;
 
 	virtual int allocateBuffers(Camera *camera, Stream *stream) = 0;
 	virtual int freeBuffers(Camera *camera, Stream *stream) = 0;
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 5d01504e24471bcf..ca09da753b908448 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -139,11 +139,11 @@  public:
 	PipelineHandlerIPU3(CameraManager *manager);
 	~PipelineHandlerIPU3();
 
-	std::map<Stream *, StreamConfiguration>
+	CameraConfiguration
 	streamConfiguration(Camera *camera,
 			    const std::vector<StreamUsage> &usages) override;
 	int configureStreams(Camera *camera,
-			     std::map<Stream *, StreamConfiguration> &config) override;
+			     const CameraConfiguration &config) override;
 
 	int allocateBuffers(Camera *camera, Stream *stream) override;
 	int freeBuffers(Camera *camera, Stream *stream) override;
@@ -204,11 +204,11 @@  PipelineHandlerIPU3::~PipelineHandlerIPU3()
 		imguMediaDev_->release();
 }
 
-std::map<Stream *, StreamConfiguration>
+CameraConfiguration
 PipelineHandlerIPU3::streamConfiguration(Camera *camera,
 					 const std::vector<StreamUsage> &usages)
 {
-	std::map<Stream *, StreamConfiguration> configs;
+	CameraConfiguration configs;
 	IPU3CameraData *data = cameraData(camera);
 	StreamConfiguration *config = &configs[&data->stream_];
 
@@ -234,7 +234,7 @@  PipelineHandlerIPU3::streamConfiguration(Camera *camera,
 }
 
 int PipelineHandlerIPU3::configureStreams(Camera *camera,
-					  std::map<Stream *, StreamConfiguration> &config)
+					  const CameraConfiguration &config)
 {
 	IPU3CameraData *data = cameraData(camera);
 	const StreamConfiguration &cfg = config[&data->stream_];
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index dfff7116bc2aa5d3..cd472cfadd86b7ba 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -26,11 +26,11 @@  public:
 	PipelineHandlerUVC(CameraManager *manager);
 	~PipelineHandlerUVC();
 
-	std::map<Stream *, StreamConfiguration>
+	CameraConfiguration
 	streamConfiguration(Camera *camera,
 			    const std::vector<StreamUsage> &usages) override;
 	int configureStreams(Camera *camera,
-			     std::map<Stream *, StreamConfiguration> &config) override;
+			     const CameraConfiguration &config) override;
 
 	int allocateBuffers(Camera *camera, Stream *stream) override;
 	int freeBuffers(Camera *camera, Stream *stream) override;
@@ -82,12 +82,12 @@  PipelineHandlerUVC::~PipelineHandlerUVC()
 		media_->release();
 }
 
-std::map<Stream *, StreamConfiguration>
+CameraConfiguration
 PipelineHandlerUVC::streamConfiguration(Camera *camera,
 					const std::vector<StreamUsage> &usages)
 {
 	UVCCameraData *data = cameraData(camera);
-	std::map<Stream *, StreamConfiguration> configs;
+	CameraConfiguration configs;
 	StreamConfiguration config{};
 
 	config.width = 640;
@@ -101,10 +101,10 @@  PipelineHandlerUVC::streamConfiguration(Camera *camera,
 }
 
 int PipelineHandlerUVC::configureStreams(Camera *camera,
-					 std::map<Stream *, StreamConfiguration> &config)
+					 const CameraConfiguration &config)
 {
 	UVCCameraData *data = cameraData(camera);
-	StreamConfiguration *cfg = &config[&data->stream_];
+	const StreamConfiguration *cfg = &config[&data->stream_];
 	int ret;
 
 	LOG(UVC, Debug) << "Configure the camera for resolution "
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 4da4ca018d4b942a..c8bbe2a19847ba1e 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -26,11 +26,11 @@  public:
 	PipelineHandlerVimc(CameraManager *manager);
 	~PipelineHandlerVimc();
 
-	std::map<Stream *, StreamConfiguration>
+	CameraConfiguration
 	streamConfiguration(Camera *camera,
 			    const std::vector<StreamUsage> &usages) override;
 	int configureStreams(Camera *camera,
-			     std::map<Stream *, StreamConfiguration> &config) override;
+			     const CameraConfiguration &config) override;
 
 	int allocateBuffers(Camera *camera, Stream *stream) override;
 	int freeBuffers(Camera *camera, Stream *stream) override;
@@ -82,12 +82,12 @@  PipelineHandlerVimc::~PipelineHandlerVimc()
 		media_->release();
 }
 
-std::map<Stream *, StreamConfiguration>
+CameraConfiguration
 PipelineHandlerVimc::streamConfiguration(Camera *camera,
 					 const std::vector<StreamUsage> &usages)
 {
 	VimcCameraData *data = cameraData(camera);
-	std::map<Stream *, StreamConfiguration> configs;
+	CameraConfiguration configs;
 	StreamConfiguration config{};
 
 	config.width = 640;
@@ -101,10 +101,10 @@  PipelineHandlerVimc::streamConfiguration(Camera *camera,
 }
 
 int PipelineHandlerVimc::configureStreams(Camera *camera,
-				      std::map<Stream *, StreamConfiguration> &config)
+					  const CameraConfiguration &config)
 {
 	VimcCameraData *data = cameraData(camera);
-	StreamConfiguration *cfg = &config[&data->stream_];
+	const StreamConfiguration *cfg = &config[&data->stream_];
 	int ret;
 
 	LOG(VIMC, Debug) << "Configure the camera for resolution "
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 0574f9904367605c..43550c0e0210b7b3 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -163,15 +163,15 @@  PipelineHandler::~PipelineHandler()
  * The intended companion to this is \a configureStreams() which can be used to
  * change the group of streams parameters.
  *
- * \return A map of successfully retrieved streams and configurations or an
- * empty map on error.
+ * \return A valid CameraConfiguration if the requested usages can be satisfied,
+ * or a invalid configuration otherwise
  */
 
 /**
  * \fn PipelineHandler::configureStreams()
  * \brief Configure a group of streams for capture
  * \param[in] camera The camera to configure
- * \param[in] config A map of stream configurations to apply
+ * \param[in] config The camera configurations to setup
  *
  * Configure the specified group of streams for \a camera according to the
  * configuration specified in \a configs. The intended caller of this interface
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index faa3bc5739dd8453..4bc0440370410525 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -98,7 +98,7 @@  int MainWindow::startCapture()
 	int ret;
 
 	config_ = camera_->streamConfiguration({ Stream::VideoRecording() });
-	Stream *stream = config_.begin()->first;
+	Stream *stream = config_.front();
 	ret = camera_->configureStreams(config_);
 	if (ret < 0) {
 		std::cout << "Failed to configure camera" << std::endl;
diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
index 5e27a8fd6b4eb8f3..9b31da2bf4b75a03 100644
--- a/src/qcam/main_window.h
+++ b/src/qcam/main_window.h
@@ -46,7 +46,7 @@  private:
 
 	std::shared_ptr<Camera> camera_;
 	bool isCapturing_;
-	std::map<Stream *, StreamConfiguration> config_;
+	CameraConfiguration config_;
 
 	ViewFinder *viewfinder_;
 };
diff --git a/test/camera/camera_test.cpp b/test/camera/camera_test.cpp
index 1609c4b02842186a..24ff5fe0c64d331e 100644
--- a/test/camera/camera_test.cpp
+++ b/test/camera/camera_test.cpp
@@ -45,21 +45,3 @@  void CameraTest::cleanup()
 
 	cm_->stop();
 };
-
-bool CameraTest::configurationValid(const std::map<Stream *, StreamConfiguration> &config) const
-{
-	/* Test that the configuration is not empty. */
-	if (config.empty())
-		return false;
-
-	/* Test that configuration is valid. */
-	for (auto const &it : config) {
-		const StreamConfiguration &conf = it.second;
-
-		if (conf.width == 0 || conf.height == 0 ||
-		    conf.pixelFormat == 0 || conf.bufferCount == 0)
-			return false;
-	}
-
-	return true;
-}
diff --git a/test/camera/camera_test.h b/test/camera/camera_test.h
index 5801fad3281e1653..ffc8a485bfaff836 100644
--- a/test/camera/camera_test.h
+++ b/test/camera/camera_test.h
@@ -23,8 +23,6 @@  protected:
 	int init();
 	void cleanup();
 
-	bool configurationValid(const std::map<Stream *, StreamConfiguration> &config) const;
-
 	std::shared_ptr<Camera> camera_;
 
 private:
diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
index b8dbdb62f9a50a33..4e75a75c5b714c58 100644
--- a/test/camera/capture.cpp
+++ b/test/camera/capture.cpp
@@ -42,12 +42,12 @@  protected:
 
 	int run()
 	{
-		std::map<Stream *, StreamConfiguration> conf =
+		CameraConfiguration conf =
 			camera_->streamConfiguration({ Stream::VideoRecording() });
-		Stream *stream = conf.begin()->first;
-		StreamConfiguration *sconf = &conf.begin()->second;
+		Stream *stream = conf.front();
+		StreamConfiguration *sconf = &conf[stream];
 
-		if (!configurationValid(conf)) {
+		if (!conf.isValid()) {
 			cout << "Failed to read default configuration" << endl;
 			return TestFail;
 		}
diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp
index 09861716973d752c..dd5106689f090921 100644
--- a/test/camera/configuration_default.cpp
+++ b/test/camera/configuration_default.cpp
@@ -18,17 +18,11 @@  class ConfigurationDefault : public CameraTest
 protected:
 	int run()
 	{
-		std::map<Stream *, StreamConfiguration> conf;
+		CameraConfiguration conf;
 
 		/* Test asking for configuration for a video stream. */
 		conf = camera_->streamConfiguration({ Stream::VideoRecording() });
-		if (conf.empty()) {
-			cout << "Failed to retrieve configuration for video streams"
-			     << endl;
-			return TestFail;
-		}
-
-		if (!configurationValid(conf)) {
+		if (!conf.isValid()) {
 			cout << "Default configuration invalid" << endl;
 			return TestFail;
 		}
@@ -38,7 +32,7 @@  protected:
 		 * stream usages returns an empty list of configurations.
 		 */
 		conf = camera_->streamConfiguration({});
-		if (!conf.empty()) {
+		if (conf.isValid()) {
 			cout << "Failed to retrieve configuration for empty usage list"
 			     << endl;
 			return TestFail;
diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
index 1bc01e66625eedf0..5ac2a7a908c8903d 100644
--- a/test/camera/configuration_set.cpp
+++ b/test/camera/configuration_set.cpp
@@ -18,11 +18,11 @@  class ConfigurationSet : public CameraTest
 protected:
 	int run()
 	{
-		std::map<Stream *, StreamConfiguration> conf =
+		CameraConfiguration conf =
 			camera_->streamConfiguration({ Stream::VideoRecording() });
-		StreamConfiguration *sconf = &conf.begin()->second;
+		StreamConfiguration *sconf = &conf[conf.front()];
 
-		if (!configurationValid(conf)) {
+		if (!conf.isValid()) {
 			cout << "Failed to read default configuration" << endl;
 			return TestFail;
 		}
diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
index ab3c6fb5bea38c36..8ae93bee7ca178d4 100644
--- a/test/camera/statemachine.cpp
+++ b/test/camera/statemachine.cpp
@@ -265,7 +265,7 @@  protected:
 		return TestPass;
 	}
 
-	std::map<Stream *, StreamConfiguration> defconf_;
+	CameraConfiguration defconf_;
 };
 
 } /* namespace */