[libcamera-devel] test: Use float values for brightness, contrast and saturation

Message ID 20200427181034.10388-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit 256d0a4098aa4c4e9f87db46cecbd66f693dd9bf
Headers show
Series
  • [libcamera-devel] test: Use float values for brightness, contrast and saturation
Related show

Commit Message

Laurent Pinchart April 27, 2020, 6:10 p.m. UTC
Two tests use the brightness, contrast and saturation controls with
integer failures. They were not updated by commit eff4b1aa01c1 which
turned those controls into floats. This doesn't cause test failures as
the control API converts the value types. For correctness, update the
tests to use float values.

Fixes: eff4b1aa01c1 ("libcamera: controls: Reorder and update description of existing controls")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/controls/control_list.cpp               | 20 ++++++++++----------
 test/serialization/control_serialization.cpp |  6 +++---
 2 files changed, 13 insertions(+), 13 deletions(-)

Comments

Jacopo Mondi April 27, 2020, 7:26 p.m. UTC | #1
Hi Laurent,

On Mon, Apr 27, 2020 at 09:10:34PM +0300, Laurent Pinchart wrote:
> Two tests use the brightness, contrast and saturation controls with
> integer failures. They were not updated by commit eff4b1aa01c1 which
> turned those controls into floats. This doesn't cause test failures as
> the control API converts the value types. For correctness, update the
> tests to use float values.
>
> Fixes: eff4b1aa01c1 ("libcamera: controls: Reorder and update description of existing controls")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

Thanks
  j

> ---
>  test/controls/control_list.cpp               | 20 ++++++++++----------
>  test/serialization/control_serialization.cpp |  6 +++---
>  2 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
> index 5374c6f99f80..d51ec47d61d3 100644
> --- a/test/controls/control_list.cpp
> +++ b/test/controls/control_list.cpp
> @@ -68,7 +68,7 @@ protected:
>  		 * Set a control, and verify that the list now contains it, and
>  		 * nothing else.
>  		 */
> -		list.set(controls::Brightness, 255);
> +		list.set(controls::Brightness, -0.5f);
>
>  		if (list.empty()) {
>  			cout << "List should not be empty" << endl;
> @@ -94,7 +94,7 @@ protected:
>  			return TestFail;
>  		}
>
> -		if (list.get(controls::Brightness) != 255) {
> +		if (list.get(controls::Brightness) != -0.5f) {
>  			cout << "Incorrest Brightness control value" << endl;
>  			return TestFail;
>  		}
> @@ -105,8 +105,8 @@ protected:
>  		}
>
>  		/* Update the first control and set a second one. */
> -		list.set(controls::Brightness, 64);
> -		list.set(controls::Contrast, 128);
> +		list.set(controls::Brightness, 0.0f);
> +		list.set(controls::Contrast, 1.5f);
>
>  		if (!list.contains(controls::Contrast) ||
>  		    !list.contains(controls::Contrast)) {
> @@ -114,8 +114,8 @@ protected:
>  			return TestFail;
>  		}
>
> -		if (list.get(controls::Brightness) != 64 ||
> -		    list.get(controls::Contrast) != 128) {
> +		if (list.get(controls::Brightness) != 0.0f ||
> +		    list.get(controls::Contrast) != 1.5f) {
>  			cout << "Failed to retrieve control value" << endl;
>  			return TestFail;
>  		}
> @@ -124,11 +124,11 @@ protected:
>  		 * Update both controls and verify that the container doesn't
>  		 * grow.
>  		 */
> -		list.set(controls::Brightness, 10);
> -		list.set(controls::Contrast, 20);
> +		list.set(controls::Brightness, 0.5f);
> +		list.set(controls::Contrast, 1.1f);
>
> -		if (list.get(controls::Brightness) != 10 ||
> -		    list.get(controls::Contrast) != 20) {
> +		if (list.get(controls::Brightness) != 0.5f ||
> +		    list.get(controls::Contrast) != 1.1f) {
>  			cout << "Failed to update control value" << endl;
>  			return TestFail;
>  		}
> diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp
> index 2989b52774fb..111365241eb8 100644
> --- a/test/serialization/control_serialization.cpp
> +++ b/test/serialization/control_serialization.cpp
> @@ -42,9 +42,9 @@ protected:
>  		const ControlInfoMap &infoMap = camera_->controls();
>  		ControlList list(infoMap);
>
> -		list.set(controls::Brightness, 255);
> -		list.set(controls::Contrast, 128);
> -		list.set(controls::Saturation, 50);
> +		list.set(controls::Brightness, 0.5f);
> +		list.set(controls::Contrast, 1.2f);
> +		list.set(controls::Saturation, 0.2f);
>
>  		/*
>  		 * Serialize the control list, this should fail as the control
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
index 5374c6f99f80..d51ec47d61d3 100644
--- a/test/controls/control_list.cpp
+++ b/test/controls/control_list.cpp
@@ -68,7 +68,7 @@  protected:
 		 * Set a control, and verify that the list now contains it, and
 		 * nothing else.
 		 */
-		list.set(controls::Brightness, 255);
+		list.set(controls::Brightness, -0.5f);
 
 		if (list.empty()) {
 			cout << "List should not be empty" << endl;
@@ -94,7 +94,7 @@  protected:
 			return TestFail;
 		}
 
-		if (list.get(controls::Brightness) != 255) {
+		if (list.get(controls::Brightness) != -0.5f) {
 			cout << "Incorrest Brightness control value" << endl;
 			return TestFail;
 		}
@@ -105,8 +105,8 @@  protected:
 		}
 
 		/* Update the first control and set a second one. */
-		list.set(controls::Brightness, 64);
-		list.set(controls::Contrast, 128);
+		list.set(controls::Brightness, 0.0f);
+		list.set(controls::Contrast, 1.5f);
 
 		if (!list.contains(controls::Contrast) ||
 		    !list.contains(controls::Contrast)) {
@@ -114,8 +114,8 @@  protected:
 			return TestFail;
 		}
 
-		if (list.get(controls::Brightness) != 64 ||
-		    list.get(controls::Contrast) != 128) {
+		if (list.get(controls::Brightness) != 0.0f ||
+		    list.get(controls::Contrast) != 1.5f) {
 			cout << "Failed to retrieve control value" << endl;
 			return TestFail;
 		}
@@ -124,11 +124,11 @@  protected:
 		 * Update both controls and verify that the container doesn't
 		 * grow.
 		 */
-		list.set(controls::Brightness, 10);
-		list.set(controls::Contrast, 20);
+		list.set(controls::Brightness, 0.5f);
+		list.set(controls::Contrast, 1.1f);
 
-		if (list.get(controls::Brightness) != 10 ||
-		    list.get(controls::Contrast) != 20) {
+		if (list.get(controls::Brightness) != 0.5f ||
+		    list.get(controls::Contrast) != 1.1f) {
 			cout << "Failed to update control value" << endl;
 			return TestFail;
 		}
diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp
index 2989b52774fb..111365241eb8 100644
--- a/test/serialization/control_serialization.cpp
+++ b/test/serialization/control_serialization.cpp
@@ -42,9 +42,9 @@  protected:
 		const ControlInfoMap &infoMap = camera_->controls();
 		ControlList list(infoMap);
 
-		list.set(controls::Brightness, 255);
-		list.set(controls::Contrast, 128);
-		list.set(controls::Saturation, 50);
+		list.set(controls::Brightness, 0.5f);
+		list.set(controls::Contrast, 1.2f);
+		list.set(controls::Saturation, 0.2f);
 
 		/*
 		 * Serialize the control list, this should fail as the control