[libcamera-devel,3/5] test: camera: Add setting of format test
diff mbox series

Message ID 20190306024755.28726-4-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • test: camera: Add basic tests for the camera
Related show

Commit Message

Niklas Söderlund March 6, 2019, 2:47 a.m. UTC 
Try to set the default format, a modified valid format and an invalid
format.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 test/camera/format_set.cpp | 88 ++++++++++++++++++++++++++++++++++++++
 test/camera/meson.build    |  1 +
 2 files changed, 89 insertions(+)
 create mode 100644 test/camera/format_set.cpp

Comments

Laurent Pinchart March 10, 2019, 1:30 p.m. UTC | #1 
Hi Niklas,

Thank you for the patch.

On Wed, Mar 06, 2019 at 03:47:53AM +0100, Niklas Söderlund wrote:
> Try to set the default format, a modified valid format and an invalid
> format.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  test/camera/format_set.cpp | 88 ++++++++++++++++++++++++++++++++++++++
>  test/camera/meson.build    |  1 +
>  2 files changed, 89 insertions(+)
>  create mode 100644 test/camera/format_set.cpp
> 
> diff --git a/test/camera/format_set.cpp b/test/camera/format_set.cpp
> new file mode 100644
> index 0000000000000000..948428ca6b00d2d0
> --- /dev/null
> +++ b/test/camera/format_set.cpp
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * libcamera Camera API tests
> + */
> +
> +#include <iostream>
> +
> +#include "camera_test.h"
> +
> +using namespace std;
> +
> +namespace {
> +
> +class FormatSet : public CameraTest
> +{
> +protected:
> +	int run()
> +	{
> +		if (camera_->acquire()) {
> +			cout << "Acquiring the camera failed" << endl;
> +			return TestFail;
> +		}
> +
> +		std::set<Stream *> streams = { *camera_->streams().begin() };
> +		std::map<Stream *, StreamConfiguration> conf =
> +			camera_->streamConfiguration(streams);
> +		StreamConfiguration *sconf = &conf.begin()->second;
> +		if (conf.size() != 1) {
> +			cout << "Reading default format failed" << endl;
> +			return TestFail;
> +		}

This could use the validation function I mentioned in patch 2/5. Or you
could rely on the test from patch 2/5 performing this check, and skip it
here, up to you.

> +
> +		/*
> +		 * Test setting the default format works.
s/Test/Test that/

> +		 */

This holds on a single line.

> +		if (camera_->configureStreams(conf)) {
> +			cout << "Setting valid format test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		/*
> +		 * Test setting the default format fails the camera is not

s/Test/Test that/
s/the camera/if the camera/

> +		 * acquired.
> +		 */
> +		if (camera_->release()) {
> +			cout << "Releasing the camera failed" << endl;
> +			return TestFail;
> +		}
> +
> +		if (!camera_->configureStreams(conf)) {
> +			cout << "Setting valid format on a camera not acquired failed" << endl;
> +			return TestFail;
> +		}
> +
> +		if (camera_->acquire()) {
> +			cout << "Acquiring the camera failed" << endl;
> +			return TestFail;
> +		}

If you moved this before the previous test you could drop the
releast/acquire. On the other hand there's value in testing
release/acquire. If you want to keep them I'd mention that in the
comment above.

> +
> +		/*
> +		 * Test doubling the resolution works.

s/Test/Test that/

You should expand this comment and mention that VIMC supports it,
otherwise it may not be clear why a double resolution is expected to be
valid.

> +		 */
> +		sconf->width *= 2;
> +		sconf->height *= 2;
> +		if (camera_->configureStreams(conf)) {
> +			cout << "Setting modified valid format test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		/*
> +		 * Test Setting an invalid format fails.
> +		 */
> +		sconf->width = 0;
> +		sconf->height = 0;
> +		if (!camera_->configureStreams(conf)) {
> +			cout << "Setting invalid format test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		return TestPass;
> +	}
> +};
> +
> +} /* namespace */
> +
> +TEST_REGISTER(FormatSet);
> diff --git a/test/camera/meson.build b/test/camera/meson.build
> index 4f2ed244a9240512..f5f27c4229ac307f 100644
> --- a/test/camera/meson.build
> +++ b/test/camera/meson.build
> @@ -2,6 +2,7 @@
>  # They are not alphabetically sorted.
>  camera_tests = [
>    [ 'format_default',        'format_default.cpp' ],
> +  [ 'format_set',            'format_set.cpp' ],
>  ]
>  
>  foreach t : camera_tests
Niklas Söderlund March 11, 2019, 1:26 a.m. UTC | #2 
Hi Laurent,

Thanks for your feedback.

On 2019-03-10 15:30:39 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Wed, Mar 06, 2019 at 03:47:53AM +0100, Niklas Söderlund wrote:
> > Try to set the default format, a modified valid format and an invalid
> > format.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  test/camera/format_set.cpp | 88 ++++++++++++++++++++++++++++++++++++++
> >  test/camera/meson.build    |  1 +
> >  2 files changed, 89 insertions(+)
> >  create mode 100644 test/camera/format_set.cpp
> > 
> > diff --git a/test/camera/format_set.cpp b/test/camera/format_set.cpp
> > new file mode 100644
> > index 0000000000000000..948428ca6b00d2d0
> > --- /dev/null
> > +++ b/test/camera/format_set.cpp
> > @@ -0,0 +1,88 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * libcamera Camera API tests
> > + */
> > +
> > +#include <iostream>
> > +
> > +#include "camera_test.h"
> > +
> > +using namespace std;
> > +
> > +namespace {
> > +
> > +class FormatSet : public CameraTest
> > +{
> > +protected:
> > +	int run()
> > +	{
> > +		if (camera_->acquire()) {
> > +			cout << "Acquiring the camera failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		std::set<Stream *> streams = { *camera_->streams().begin() };
> > +		std::map<Stream *, StreamConfiguration> conf =
> > +			camera_->streamConfiguration(streams);
> > +		StreamConfiguration *sconf = &conf.begin()->second;
> > +		if (conf.size() != 1) {
> > +			cout << "Reading default format failed" << endl;
> > +			return TestFail;
> > +		}
> 
> This could use the validation function I mentioned in patch 2/5. Or you
> could rely on the test from patch 2/5 performing this check, and skip it
> here, up to you.

Great idea, I think I will keep the new check here to catch potential 
errors early.

> 
> > +
> > +		/*
> > +		 * Test setting the default format works.
> s/Test/Test that/
> 
> > +		 */
> 
> This holds on a single line.

Thanks.

> 
> > +		if (camera_->configureStreams(conf)) {
> > +			cout << "Setting valid format test failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/*
> > +		 * Test setting the default format fails the camera is not
> 
> s/Test/Test that/
> s/the camera/if the camera/

Thanks.

> 
> > +		 * acquired.
> > +		 */
> > +		if (camera_->release()) {
> > +			cout << "Releasing the camera failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (!camera_->configureStreams(conf)) {
> > +			cout << "Setting valid format on a camera not acquired failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (camera_->acquire()) {
> > +			cout << "Acquiring the camera failed" << endl;
> > +			return TestFail;
> > +		}
> 
> If you moved this before the previous test you could drop the
> releast/acquire. On the other hand there's value in testing
> release/acquire. If you want to keep them I'd mention that in the
> comment above.

It was my intention to also test the release/acquire, I will add it to 
the comment.

> 
> > +
> > +		/*
> > +		 * Test doubling the resolution works.
> 
> s/Test/Test that/
> 
> You should expand this comment and mention that VIMC supports it,
> otherwise it may not be clear why a double resolution is expected to be
> valid.

Good point.

> 
> > +		 */
> > +		sconf->width *= 2;
> > +		sconf->height *= 2;
> > +		if (camera_->configureStreams(conf)) {
> > +			cout << "Setting modified valid format test failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/*
> > +		 * Test Setting an invalid format fails.
> > +		 */
> > +		sconf->width = 0;
> > +		sconf->height = 0;
> > +		if (!camera_->configureStreams(conf)) {
> > +			cout << "Setting invalid format test failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		return TestPass;
> > +	}
> > +};
> > +
> > +} /* namespace */
> > +
> > +TEST_REGISTER(FormatSet);
> > diff --git a/test/camera/meson.build b/test/camera/meson.build
> > index 4f2ed244a9240512..f5f27c4229ac307f 100644
> > --- a/test/camera/meson.build
> > +++ b/test/camera/meson.build
> > @@ -2,6 +2,7 @@
> >  # They are not alphabetically sorted.
> >  camera_tests = [
> >    [ 'format_default',        'format_default.cpp' ],
> > +  [ 'format_set',            'format_set.cpp' ],
> >  ]
> >  
> >  foreach t : camera_tests
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/test/camera/format_set.cpp b/test/camera/format_set.cpp
new file mode 100644
index 0000000000000000..948428ca6b00d2d0
--- /dev/null
+++ b/test/camera/format_set.cpp
@@ -0,0 +1,88 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2019, Google Inc.
+ *
+ * libcamera Camera API tests
+ */
+
+#include <iostream>
+
+#include "camera_test.h"
+
+using namespace std;
+
+namespace {
+
+class FormatSet : public CameraTest
+{
+protected:
+	int run()
+	{
+		if (camera_->acquire()) {
+			cout << "Acquiring the camera failed" << endl;
+			return TestFail;
+		}
+
+		std::set<Stream *> streams = { *camera_->streams().begin() };
+		std::map<Stream *, StreamConfiguration> conf =
+			camera_->streamConfiguration(streams);
+		StreamConfiguration *sconf = &conf.begin()->second;
+		if (conf.size() != 1) {
+			cout << "Reading default format failed" << endl;
+			return TestFail;
+		}
+
+		/*
+		 * Test setting the default format works.
+		 */
+		if (camera_->configureStreams(conf)) {
+			cout << "Setting valid format test failed" << endl;
+			return TestFail;
+		}
+
+		/*
+		 * Test setting the default format fails the camera is not
+		 * acquired.
+		 */
+		if (camera_->release()) {
+			cout << "Releasing the camera failed" << endl;
+			return TestFail;
+		}
+
+		if (!camera_->configureStreams(conf)) {
+			cout << "Setting valid format on a camera not acquired failed" << endl;
+			return TestFail;
+		}
+
+		if (camera_->acquire()) {
+			cout << "Acquiring the camera failed" << endl;
+			return TestFail;
+		}
+
+		/*
+		 * Test doubling the resolution works.
+		 */
+		sconf->width *= 2;
+		sconf->height *= 2;
+		if (camera_->configureStreams(conf)) {
+			cout << "Setting modified valid format test failed" << endl;
+			return TestFail;
+		}
+
+		/*
+		 * Test Setting an invalid format fails.
+		 */
+		sconf->width = 0;
+		sconf->height = 0;
+		if (!camera_->configureStreams(conf)) {
+			cout << "Setting invalid format test failed" << endl;
+			return TestFail;
+		}
+
+		return TestPass;
+	}
+};
+
+} /* namespace */
+
+TEST_REGISTER(FormatSet);
diff --git a/test/camera/meson.build b/test/camera/meson.build
index 4f2ed244a9240512..f5f27c4229ac307f 100644
--- a/test/camera/meson.build
+++ b/test/camera/meson.build
@@ -2,6 +2,7 @@ 
 # They are not alphabetically sorted.
 camera_tests = [
   [ 'format_default',        'format_default.cpp' ],
+  [ 'format_set',            'format_set.cpp' ],
 ]
 
 foreach t : camera_tests