[libcamera-devel,RFC] lc-compliance: Add test to set test pattern mode
diff mbox series

Message ID 20211004102308.3726725-1-hiroh@chromium.org
State New
Headers show
Series
  • [libcamera-devel,RFC] lc-compliance: Add test to set test pattern mode
Related show

Commit Message

Hirokazu Honda Oct. 4, 2021, 10:23 a.m. UTC
This adds the test to verify the requested test pattern mode is
correctly applied in request metadata.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
---
 src/lc-compliance/capture_test.cpp   |  15 ++++
 src/lc-compliance/simple_capture.cpp | 101 +++++++++++++++++++++++++++
 src/lc-compliance/simple_capture.h   |  16 +++++
 3 files changed, 132 insertions(+)

Comments

Laurent Pinchart Oct. 4, 2021, 11:22 p.m. UTC | #1
Hi Hiro,

Thank you for the patch.

On Mon, Oct 04, 2021 at 07:23:08PM +0900, Hirokazu Honda wrote:
> This adds the test to verify the requested test pattern mode is
> correctly applied in request metadata.
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> ---
>  src/lc-compliance/capture_test.cpp   |  15 ++++
>  src/lc-compliance/simple_capture.cpp | 101 +++++++++++++++++++++++++++
>  src/lc-compliance/simple_capture.h   |  16 +++++
>  3 files changed, 132 insertions(+)
> 
> diff --git a/src/lc-compliance/capture_test.cpp b/src/lc-compliance/capture_test.cpp
> index 52578207..fc0b5322 100644
> --- a/src/lc-compliance/capture_test.cpp
> +++ b/src/lc-compliance/capture_test.cpp
> @@ -121,6 +121,21 @@ TEST_P(SingleStream, UnbalancedStop)
>  	capture.capture(numRequests);
>  }
>  
> +TEST_P(SingleStream, TestPatternMode)
> +{
> +	auto [role, numRequests] = GetParam();

I don't think this test should be parametrized by role and number of
requests, those shouldn't matter for the feature being tested. You can
use a fixed role and a fixed number of requests instead. You can't use
SingleStream, as the test would still be parametrized even if you ignore
the value of the parameters. I'd thus create a new test suite related to
controls in a separate file, to host the TestPatternMode test.

> +
> +	SimpleCaptureTestPatternMode capture(camera_);
> +	capture.configure(role);
> +
> +	auto testPatternModes = capture.availableTestPatternModes();
> +	if (testPatternModes.empty())
> +		return;

The test should be reported as skipped when this happens, will it be the
case ?

> +
> +	for (const int32_t testPatternMode : testPatternModes)
> +		capture.capture(testPatternMode, numRequests);
> +}
> +
>  INSTANTIATE_TEST_SUITE_P(CaptureTests,
>  			 SingleStream,
>  			 testing::Combine(testing::ValuesIn(ROLES),
> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> index ab5cb35c..f279b6a7 100644
> --- a/src/lc-compliance/simple_capture.cpp
> +++ b/src/lc-compliance/simple_capture.cpp
> @@ -189,3 +189,104 @@ void SimpleCaptureUnbalanced::requestComplete(Request *request)
>  	if (camera_->queueRequest(request))
>  		loop_->exit(-EINVAL);
>  }
> +
> +/* SimpleCaptureTestPatternMode */

I would also move this to the new source file, it's specific to the test
pattern test, not a generic helper.

Could you expand this command a little bit to explain what the test does
? A \todo to indicate that the contents of the buffer should be checked
would also be useful.

> +
> +SimpleCaptureTestPatternMode::SimpleCaptureTestPatternMode(std::shared_ptr<Camera> camera)
> +	: SimpleCapture(camera)
> +{
> +}
> +
> +std::vector<int32_t> SimpleCaptureTestPatternMode::availableTestPatternModes() const
> +{
> +	const auto &controls = camera_->controls();
> +	const auto it = controls.find(&controls::draft::TestPatternMode);
> +	if (it == controls.end())
> +		return {};
> +
> +	std::vector<int32_t> testPatternModes;
> +	const std::vector<ControlValue> &values = it->second.values();
> +	for (const auto &value : values)
> +		testPatternModes.push_back(value.get<int32_t>());
> +
> +	return testPatternModes;
> +}
> +
> +void SimpleCaptureTestPatternMode::capture(int32_t testPatternMode,
> +					   unsigned int numRequests)
> +{
> +	start();
> +
> +	Stream *stream = config_->at(0).stream();
> +	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
> +	/* No point in testing less requests then the camera depth. */
> +	if (buffers.size() > numRequests) {
> +		std::cout << "Camera needs " << buffers.size()
> +			  << " requests, can't test only "
> +			  << numRequests << std::endl;
> +		GTEST_SKIP();
> +	}
> +
> +	captureCount_ = 0;
> +	captureLimit_ = numRequests;
> +	testPatternMode_ = testPatternMode;
> +
> +	/* Queue the recommended number of reqeuests. */
> +	std::vector<std::unique_ptr<libcamera::Request>> requests;
> +	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> +		std::unique_ptr<Request> request = camera_->createRequest();
> +		ASSERT_TRUE(request) << "Can't create request";
> +
> +		request->controls().set(controls::draft::TestPatternMode,
> +					testPatternMode);
> +
> +		ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request";
> +
> +		ASSERT_EQ(camera_->queueRequest(request.get()), 0) << "Failed to queue request";
> +
> +		requests.push_back(std::move(request));
> +	}
> +
> +	/* Run capture session. */
> +	loop_ = new EventLoop();
> +	int status = loop_->exec();
> +	stop();
> +	delete loop_;
> +
> +	ASSERT_EQ(status, 0);
> +}
> +
> +void SimpleCaptureTestPatternMode::requestComplete(Request *request)
> +{
> +	captureCount_++;
> +	if (captureCount_ >= captureLimit_) {
> +		loop_->exit(0);
> +		return;
> +	}
> +
> +	if (!request->metadata().contains(controls::draft::TestPatternMode)) {
> +		if (testPatternMode_ != controls::draft::TestPatternModeOff) {
> +			std::cerr << "no test pattern mode mismatch: expected="
> +				  << testPatternMode_ << std::endl;
> +			loop_->exit(-EINVAL);
> +			return;
> +		}
> +	} else {
> +		const int32_t testPatternMode =
> +			request->metadata().get(controls::draft::TestPatternMode);
> +		if (testPatternMode_ != testPatternMode) {
> +			std::cerr << "test pattern mode mismatch: expected="
> +				  << testPatternMode_ << ", actual="
> +				  << testPatternMode << std::endl;
> +
> +			loop_->exit(-EINVAL);
> +			return;
> +		}
> +	}
> +
> +	request->reuse(Request::ReuseBuffers);
> +	request->controls().set(controls::draft::TestPatternMode,
> +				testPatternMode_);
> +	if (camera_->queueRequest(request))
> +		loop_->exit(-EINVAL);
> +}
> diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
> index 100ffd66..7989a308 100644
> --- a/src/lc-compliance/simple_capture.h
> +++ b/src/lc-compliance/simple_capture.h
> @@ -64,4 +64,20 @@ private:
>  	unsigned int captureLimit_;
>  };
>  
> +class SimpleCaptureTestPatternMode : public SimpleCapture
> +{
> +public:
> +	SimpleCaptureTestPatternMode(std::shared_ptr<libcamera::Camera> camera);
> +
> +	std::vector<int32_t> availableTestPatternModes() const;
> +	void capture(int32_t testPatternMode, unsigned int numRequests);
> +
> +private:
> +	void requestComplete(libcamera::Request *request) override;
> +
> +	unsigned int captureCount_;
> +	unsigned int captureLimit_;
> +	int32_t testPatternMode_;
> +};
> +
>  #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */

Patch
diff mbox series

diff --git a/src/lc-compliance/capture_test.cpp b/src/lc-compliance/capture_test.cpp
index 52578207..fc0b5322 100644
--- a/src/lc-compliance/capture_test.cpp
+++ b/src/lc-compliance/capture_test.cpp
@@ -121,6 +121,21 @@  TEST_P(SingleStream, UnbalancedStop)
 	capture.capture(numRequests);
 }
 
+TEST_P(SingleStream, TestPatternMode)
+{
+	auto [role, numRequests] = GetParam();
+
+	SimpleCaptureTestPatternMode capture(camera_);
+	capture.configure(role);
+
+	auto testPatternModes = capture.availableTestPatternModes();
+	if (testPatternModes.empty())
+		return;
+
+	for (const int32_t testPatternMode : testPatternModes)
+		capture.capture(testPatternMode, numRequests);
+}
+
 INSTANTIATE_TEST_SUITE_P(CaptureTests,
 			 SingleStream,
 			 testing::Combine(testing::ValuesIn(ROLES),
diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
index ab5cb35c..f279b6a7 100644
--- a/src/lc-compliance/simple_capture.cpp
+++ b/src/lc-compliance/simple_capture.cpp
@@ -189,3 +189,104 @@  void SimpleCaptureUnbalanced::requestComplete(Request *request)
 	if (camera_->queueRequest(request))
 		loop_->exit(-EINVAL);
 }
+
+/* SimpleCaptureTestPatternMode */
+
+SimpleCaptureTestPatternMode::SimpleCaptureTestPatternMode(std::shared_ptr<Camera> camera)
+	: SimpleCapture(camera)
+{
+}
+
+std::vector<int32_t> SimpleCaptureTestPatternMode::availableTestPatternModes() const
+{
+	const auto &controls = camera_->controls();
+	const auto it = controls.find(&controls::draft::TestPatternMode);
+	if (it == controls.end())
+		return {};
+
+	std::vector<int32_t> testPatternModes;
+	const std::vector<ControlValue> &values = it->second.values();
+	for (const auto &value : values)
+		testPatternModes.push_back(value.get<int32_t>());
+
+	return testPatternModes;
+}
+
+void SimpleCaptureTestPatternMode::capture(int32_t testPatternMode,
+					   unsigned int numRequests)
+{
+	start();
+
+	Stream *stream = config_->at(0).stream();
+	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
+	/* No point in testing less requests then the camera depth. */
+	if (buffers.size() > numRequests) {
+		std::cout << "Camera needs " << buffers.size()
+			  << " requests, can't test only "
+			  << numRequests << std::endl;
+		GTEST_SKIP();
+	}
+
+	captureCount_ = 0;
+	captureLimit_ = numRequests;
+	testPatternMode_ = testPatternMode;
+
+	/* Queue the recommended number of reqeuests. */
+	std::vector<std::unique_ptr<libcamera::Request>> requests;
+	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
+		std::unique_ptr<Request> request = camera_->createRequest();
+		ASSERT_TRUE(request) << "Can't create request";
+
+		request->controls().set(controls::draft::TestPatternMode,
+					testPatternMode);
+
+		ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request";
+
+		ASSERT_EQ(camera_->queueRequest(request.get()), 0) << "Failed to queue request";
+
+		requests.push_back(std::move(request));
+	}
+
+	/* Run capture session. */
+	loop_ = new EventLoop();
+	int status = loop_->exec();
+	stop();
+	delete loop_;
+
+	ASSERT_EQ(status, 0);
+}
+
+void SimpleCaptureTestPatternMode::requestComplete(Request *request)
+{
+	captureCount_++;
+	if (captureCount_ >= captureLimit_) {
+		loop_->exit(0);
+		return;
+	}
+
+	if (!request->metadata().contains(controls::draft::TestPatternMode)) {
+		if (testPatternMode_ != controls::draft::TestPatternModeOff) {
+			std::cerr << "no test pattern mode mismatch: expected="
+				  << testPatternMode_ << std::endl;
+			loop_->exit(-EINVAL);
+			return;
+		}
+	} else {
+		const int32_t testPatternMode =
+			request->metadata().get(controls::draft::TestPatternMode);
+		if (testPatternMode_ != testPatternMode) {
+			std::cerr << "test pattern mode mismatch: expected="
+				  << testPatternMode_ << ", actual="
+				  << testPatternMode << std::endl;
+
+			loop_->exit(-EINVAL);
+			return;
+		}
+	}
+
+	request->reuse(Request::ReuseBuffers);
+	request->controls().set(controls::draft::TestPatternMode,
+				testPatternMode_);
+	if (camera_->queueRequest(request))
+		loop_->exit(-EINVAL);
+}
diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
index 100ffd66..7989a308 100644
--- a/src/lc-compliance/simple_capture.h
+++ b/src/lc-compliance/simple_capture.h
@@ -64,4 +64,20 @@  private:
 	unsigned int captureLimit_;
 };
 
+class SimpleCaptureTestPatternMode : public SimpleCapture
+{
+public:
+	SimpleCaptureTestPatternMode(std::shared_ptr<libcamera::Camera> camera);
+
+	std::vector<int32_t> availableTestPatternModes() const;
+	void capture(int32_t testPatternMode, unsigned int numRequests);
+
+private:
+	void requestComplete(libcamera::Request *request) override;
+
+	unsigned int captureCount_;
+	unsigned int captureLimit_;
+	int32_t testPatternMode_;
+};
+
 #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */