[libcamera-devel] [RFC PATCH v2] lc-compliance: Add test to set test pattern mode

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Tue, Oct 05, 2021 at 02:40:27PM +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 at chromium.org>
> ---
>  src/lc-compliance/meson.build                |   1 +
>  src/lc-compliance/test_pattern_mode_test.cpp | 157 +++++++++++++++++++
>  2 files changed, 158 insertions(+)
>  create mode 100644 src/lc-compliance/test_pattern_mode_test.cpp
> 
> diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build
> index aa5852f6..0fd280df 100644
> --- a/src/lc-compliance/meson.build
> +++ b/src/lc-compliance/meson.build
> @@ -17,6 +17,7 @@ lc_compliance_sources = files([
>      'main.cpp',
>      'simple_capture.cpp',
>      'capture_test.cpp',
> +    'test_pattern_mode_test.cpp',
>  ])
>  
>  lc_compliance  = executable('lc-compliance', lc_compliance_sources,
> diff --git a/src/lc-compliance/test_pattern_mode_test.cpp b/src/lc-compliance/test_pattern_mode_test.cpp
> new file mode 100644
> index 00000000..d8599b64
> --- /dev/null
> +++ b/src/lc-compliance/test_pattern_mode_test.cpp
> @@ -0,0 +1,157 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + * Copyright (C) 2021, Collabora Ltd.
> + *
> + * test_pattern_mode_test.cpp - Test a capture with a test pattern mode
> + */
> +
> +#include <libcamera/libcamera.h>
> +
> +#include <gtest/gtest.h>
> +
> +#include "environment.h"
> +#include "simple_capture.h"
> +
> +using namespace libcamera;
> +
> +namespace {

You can add a blank line here.

> +/*
> + * SimpleCaptureTestPatternMode verifies captures with a test pattern mode. It
> + * checks the request has the specified test pattern mode in the metadata, but
> + * doesn't verify the capture content.
> + */
> +class SimpleCaptureTestPatternMode : public SimpleCapture
> +{
> +public:
> +	SimpleCaptureTestPatternMode(std::shared_ptr<Camera> camera);
> +
> +	std::vector<int32_t> availableTestPatternModes() const;
> +	void capture(int32_t testPatternMode);
> +
> +private:
> +	void requestComplete(Request *request) override;
> +
> +	unsigned int captureCount_;
> +	unsigned int captureLimit_;
> +	int32_t testPatternMode_;
> +};
> +
> +/* 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)
> +{
> +	start();
> +
> +	Stream *stream = config_->at(0).stream();
> +	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
> +	ASSERT_FALSE(buffers.empty());
> +
> +	captureCount_ = 0;
> +	captureLimit_ = buffers.size();
> +	testPatternMode_ = testPatternMode;
> +
> +	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);

Would it be useful to start without a test pattern and only enable it
after a few frames, to test that it gets enabled exactly when requested
?

> +
> +		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="

s/mismatch/reported/

> +				  << testPatternMode_ << std::endl;
> +			loop_->exit(-EINVAL);

Is there a reason why we can't use ASSERT here, and have to delay it
until the status check in SimpleCaptureTestPatternMode::capture() ?

> +			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);
> +}
> +} /* namespace */
> +
> +TEST(TestPatternModeTest, SetTestPatternMode)
> +{
> +	Environment *env = Environment::get();
> +
> +	auto camera = env->cm()->get(env->cameraId());
> +	ASSERT_TRUE(camera);
> +	ASSERT_EQ(camera->acquire(), 0);
> +
> +	SimpleCaptureTestPatternMode capture(camera);
> +	capture.configure(StreamRole::Viewfinder);
> +
> +	auto testPatternModes = capture.availableTestPatternModes();
> +	if (testPatternModes.empty()) {
> +		camera->release();
> +		GTEST_SKIP() << "No test pattern is available";
> +	}
> +
> +	for (const int32_t testPatternMode : testPatternModes)
> +		capture.capture(testPatternMode);
> +
> +	camera->release();
> +}

-- 
Regards,

Laurent Pinchart