[libcamera-devel,2/5] test: camera: Remove streams argument from configurationValid()
diff mbox series

Message ID 20190403011221.12711-3-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: camera: Add support for stream roles
Related show

Commit Message

Niklas Söderlund April 3, 2019, 1:12 a.m. UTC 
In preparation of reworking how a default configuration is retrieved
from a camera remove the streams and validation using the stream when
judging if a camera configuration is valid. This is needed as once
stream usage hints are added applications will no longer fetch default
configuration based on Stream IDs so using them to verify the returned
format is not useful.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/camera/camera_test.cpp           | 25 ++++++++-----------------
 test/camera/camera_test.h             |  3 +--
 test/camera/capture.cpp               |  2 +-
 test/camera/configuration_default.cpp |  2 +-
 test/camera/configuration_set.cpp     |  2 +-
 5 files changed, 12 insertions(+), 22 deletions(-)

Comments

Jacopo Mondi April 3, 2019, 1:20 p.m. UTC | #1 
Hi Niklas,

On Wed, Apr 03, 2019 at 03:12:18AM +0200, Niklas Söderlund wrote:
> In preparation of reworking how a default configuration is retrieved
> from a camera remove the streams and validation using the stream when
> judging if a camera configuration is valid. This is needed as once
> stream usage hints are added applications will no longer fetch default
> configuration based on Stream IDs so using them to verify the returned
> format is not useful.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  test/camera/camera_test.cpp           | 25 ++++++++-----------------
>  test/camera/camera_test.h             |  3 +--
>  test/camera/capture.cpp               |  2 +-
>  test/camera/configuration_default.cpp |  2 +-
>  test/camera/configuration_set.cpp     |  2 +-
>  5 files changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/test/camera/camera_test.cpp b/test/camera/camera_test.cpp
> index a92f2165bf3a53c1..1609c4b02842186a 100644
> --- a/test/camera/camera_test.cpp
> +++ b/test/camera/camera_test.cpp
> @@ -46,27 +46,18 @@ void CameraTest::cleanup()
>  	cm_->stop();
>  };
>
> -bool CameraTest::configurationValid(const std::set<Stream *> &streams,
> -				    const std::map<Stream *, StreamConfiguration> &conf) const
> +bool CameraTest::configurationValid(const std::map<Stream *, StreamConfiguration> &config) const
>  {
> -	/* Test that the numbers of streams matches that of configuration. */
> -	if (streams.size() != conf.size())
> +	/* Test that the configuration is not empty. */
> +	if (config.empty())
>  		return false;
>
> -	/*
> -	 * Test that stream can be found in configuration and that the
> -	 * configuration is valid.
> -	 */
> -	for (Stream *stream : streams) {
> -		std::map<Stream *, StreamConfiguration>::const_iterator it =
> -			conf.find(stream);
> +	/* Test that configuration is valid. */
> +	for (auto const &it : config) {
> +		const StreamConfiguration &conf = it.second;
>
> -		if (it == conf.end())
> -			return false;
> -
> -		const StreamConfiguration *sconf = &it->second;
> -		if (sconf->width == 0 || sconf->height == 0 ||
> -		    sconf->pixelFormat == 0 || sconf->bufferCount == 0)
> +		if (conf.width == 0 || conf.height == 0 ||
> +		    conf.pixelFormat == 0 || conf.bufferCount == 0)
>  			return false;
>  	}
>
> diff --git a/test/camera/camera_test.h b/test/camera/camera_test.h
> index 48fb47a23fe8f49c..5801fad3281e1653 100644
> --- a/test/camera/camera_test.h
> +++ b/test/camera/camera_test.h
> @@ -23,8 +23,7 @@ protected:
>  	int init();
>  	void cleanup();
>
> -	bool configurationValid(const std::set<Stream *> &streams,
> -				const std::map<Stream *, StreamConfiguration> &conf) const;
> +	bool configurationValid(const std::map<Stream *, StreamConfiguration> &config) const;
>
>  	std::shared_ptr<Camera> camera_;
>
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index 28eb61405d90a4c7..f6932b7505571712 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -48,7 +48,7 @@ protected:
>  			camera_->streamConfiguration(streams);
>  		StreamConfiguration *sconf = &conf.begin()->second;
>
> -		if (!configurationValid(streams, conf)) {
> +		if (!configurationValid(conf)) {
>  			cout << "Failed to read default configuration" << endl;
>  			return TestFail;
>  		}
> diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp
> index 71e79844667591b2..53ee021d33ca39b1 100644
> --- a/test/camera/configuration_default.cpp
> +++ b/test/camera/configuration_default.cpp
> @@ -32,7 +32,7 @@ protected:
>  			return TestFail;
>  		}
>
> -		if (!configurationValid(streams, conf)) {
> +		if (!configurationValid(conf)) {
>  			cout << "Default configuration invalid" << endl;
>  			return TestFail;
>  		}
> diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
> index dedb85009335aa46..cac1da959f241bc5 100644
> --- a/test/camera/configuration_set.cpp
> +++ b/test/camera/configuration_set.cpp
> @@ -23,7 +23,7 @@ protected:
>  			camera_->streamConfiguration(streams);
>  		StreamConfiguration *sconf = &conf.begin()->second;
>
> -		if (!configurationValid(streams, conf)) {
> +		if (!configurationValid(conf)) {
>  			cout << "Failed to read default configuration" << endl;
>  			return TestFail;
>  		}
> --
> 2.21.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/test/camera/camera_test.cpp b/test/camera/camera_test.cpp
index a92f2165bf3a53c1..1609c4b02842186a 100644
--- a/test/camera/camera_test.cpp
+++ b/test/camera/camera_test.cpp
@@ -46,27 +46,18 @@  void CameraTest::cleanup()
 	cm_->stop();
 };
 
-bool CameraTest::configurationValid(const std::set<Stream *> &streams,
-				    const std::map<Stream *, StreamConfiguration> &conf) const
+bool CameraTest::configurationValid(const std::map<Stream *, StreamConfiguration> &config) const
 {
-	/* Test that the numbers of streams matches that of configuration. */
-	if (streams.size() != conf.size())
+	/* Test that the configuration is not empty. */
+	if (config.empty())
 		return false;
 
-	/*
-	 * Test that stream can be found in configuration and that the
-	 * configuration is valid.
-	 */
-	for (Stream *stream : streams) {
-		std::map<Stream *, StreamConfiguration>::const_iterator it =
-			conf.find(stream);
+	/* Test that configuration is valid. */
+	for (auto const &it : config) {
+		const StreamConfiguration &conf = it.second;
 
-		if (it == conf.end())
-			return false;
-
-		const StreamConfiguration *sconf = &it->second;
-		if (sconf->width == 0 || sconf->height == 0 ||
-		    sconf->pixelFormat == 0 || sconf->bufferCount == 0)
+		if (conf.width == 0 || conf.height == 0 ||
+		    conf.pixelFormat == 0 || conf.bufferCount == 0)
 			return false;
 	}
 
diff --git a/test/camera/camera_test.h b/test/camera/camera_test.h
index 48fb47a23fe8f49c..5801fad3281e1653 100644
--- a/test/camera/camera_test.h
+++ b/test/camera/camera_test.h
@@ -23,8 +23,7 @@  protected:
 	int init();
 	void cleanup();
 
-	bool configurationValid(const std::set<Stream *> &streams,
-				const std::map<Stream *, StreamConfiguration> &conf) const;
+	bool configurationValid(const std::map<Stream *, StreamConfiguration> &config) const;
 
 	std::shared_ptr<Camera> camera_;
 
diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
index 28eb61405d90a4c7..f6932b7505571712 100644
--- a/test/camera/capture.cpp
+++ b/test/camera/capture.cpp
@@ -48,7 +48,7 @@  protected:
 			camera_->streamConfiguration(streams);
 		StreamConfiguration *sconf = &conf.begin()->second;
 
-		if (!configurationValid(streams, conf)) {
+		if (!configurationValid(conf)) {
 			cout << "Failed to read default configuration" << endl;
 			return TestFail;
 		}
diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp
index 71e79844667591b2..53ee021d33ca39b1 100644
--- a/test/camera/configuration_default.cpp
+++ b/test/camera/configuration_default.cpp
@@ -32,7 +32,7 @@  protected:
 			return TestFail;
 		}
 
-		if (!configurationValid(streams, conf)) {
+		if (!configurationValid(conf)) {
 			cout << "Default configuration invalid" << endl;
 			return TestFail;
 		}
diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
index dedb85009335aa46..cac1da959f241bc5 100644
--- a/test/camera/configuration_set.cpp
+++ b/test/camera/configuration_set.cpp
@@ -23,7 +23,7 @@  protected:
 			camera_->streamConfiguration(streams);
 		StreamConfiguration *sconf = &conf.begin()->second;
 
-		if (!configurationValid(streams, conf)) {
+		if (!configurationValid(conf)) {
 			cout << "Failed to read default configuration" << endl;
 			return TestFail;
 		}