[libcamera-devel] [PATCH v3 04/13] test: mapped-buffers: Provide MappedBuffer test

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tue, Aug 04, 2020 at 10:47:02PM +0100, Kieran Bingham wrote:
> Provide initial testing framework for the MappedBuffer component.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  test/mapped-buffers.cpp | 113 ++++++++++++++++++++++++++++++++++++++++

We usually don't pluralize the test file name.

>  test/meson.build        |   1 +
>  2 files changed, 114 insertions(+)
>  create mode 100644 test/mapped-buffers.cpp
> 
> diff --git a/test/mapped-buffers.cpp b/test/mapped-buffers.cpp
> new file mode 100644
> index 000000000000..4c0557070ca3
> --- /dev/null
> +++ b/test/mapped-buffers.cpp
> @@ -0,0 +1,113 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * libcamera internal MappedBuffer tests
> + */
> +
> +#include <iostream>
> +
> +#include "camera_test.h"
> +#include "test.h"
> +
> +#include "libcamera/internal/buffer.h"

Should this go between iostream and camera_test.h ?

> +
> +using namespace std;
> +
> +namespace {
> +
> +class MappedBuffers : public CameraTest, public Test

We usually name the test class with a Test suffix (MappedBufferTest).

> +{
> +public:
> +	MappedBuffers()
> +		: CameraTest("VIMC Sensor B")

Either you or Niklas will have to solve this merge conflict :-) I wonder
if CameraTest shouldn't default to VIMC Sensor, but that's for another
patch.

> +	{
> +	}
> +
> +protected:
> +	int init() override
> +	{
> +		if (status_ != TestPass)
> +			return status_;
> +
> +		config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
> +		if (!config_ || config_->size() != 1) {
> +			cout << "Failed to generate default configuration" << endl;
> +			return TestFail;
> +		}
> +
> +		allocator_ = new FrameBufferAllocator(camera_);
> +
> +		StreamConfiguration &cfg = config_->at(0);
> +
> +		if (camera_->acquire()) {
> +			cout << "Failed to acquire the camera" << endl;
> +			return TestFail;
> +		}
> +
> +		if (camera_->configure(config_.get())) {
> +			cout << "Failed to set default configuration" << endl;
> +			return TestFail;
> +		}
> +
> +		stream_ = cfg.stream();
> +
> +		int ret = allocator_->allocate(stream_);
> +		if (ret < 0)
> +			return TestFail;
> +
> +		return TestPass;
> +	}
> +
> +	void cleanup() override
> +	{
> +		delete allocator_;
> +	}
> +
> +	int run() override
> +	{
> +		const std::unique_ptr<FrameBuffer> &buffer = allocator_->buffers(stream_).front();
> +		std::vector<MappedBuffer> maps;
> +
> +		MappedFrameBuffer map(buffer.get(), PROT_READ);
> +		if (!map.isValid()) {
> +			cout << "Failed to successfully map buffer" << endl;
> +			return TestFail;
> +		}
> +
> +		/* Make sure we can move it. */
> +		maps.emplace_back(std::move(map));
> +
> +		/* But copying is prevented, it would cause double-unmap. */
> +		// MappedFrameBuffer map_copy = map;
> +
> +		/* Local map should be invalid (after move). */
> +		if (map.isValid()) {
> +			cout << "Post-move map should not be valid" << endl;
> +			return TestFail;
> +		}
> +
> +		/* Test for multiple successful maps on the same buffer. */
> +		MappedFrameBuffer write_map(buffer.get(), PROT_WRITE);
> +		if (!write_map.isValid()) {
> +			cout << "Failed to map write buffer" << endl;
> +			return TestFail;
> +		}
> +
> +		MappedFrameBuffer rw_map(buffer.get(), PROT_READ | PROT_WRITE);
> +		if (!rw_map.isValid()) {
> +			cout << "Failed to map RW buffer" << endl;
> +			return TestFail;
> +		}
> +
> +		return TestPass;
> +	}
> +

private:

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +	std::unique_ptr<CameraConfiguration> config_;
> +	FrameBufferAllocator *allocator_;
> +	Stream *stream_;
> +};
> +
> +} /* namespace */
> +
> +TEST_REGISTER(MappedBuffers);
> diff --git a/test/meson.build b/test/meson.build
> index 775187159dec..376ee6cee175 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -31,6 +31,7 @@ internal_tests = [
>      ['file',                            'file.cpp'],
>      ['file-descriptor',                 'file-descriptor.cpp'],
>      ['hotplug-cameras',                 'hotplug-cameras.cpp'],
> +    ['mapped-buffers',                  'mapped-buffers.cpp'],
>      ['message',                         'message.cpp'],
>      ['object',                          'object.cpp'],
>      ['object-delete',                   'object-delete.cpp'],

-- 
Regards,

Laurent Pinchart