[libcamera-devel,v3,04/13] test: mapped-buffers: Provide MappedBuffer test
diff mbox series

Message ID 20200804214711.177645-5-kieran.bingham@ideasonboard.com
State Superseded
Headers show
Series
  • android: JPEG support
Related show

Commit Message

Kieran Bingham Aug. 4, 2020, 9:47 p.m. UTC 
Provide initial testing framework for the MappedBuffer component.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 test/mapped-buffers.cpp | 113 ++++++++++++++++++++++++++++++++++++++++
 test/meson.build        |   1 +
 2 files changed, 114 insertions(+)
 create mode 100644 test/mapped-buffers.cpp

Comments

Laurent Pinchart Aug. 5, 2020, 12:43 a.m. UTC | #1 
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@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@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'],
Kieran Bingham Aug. 5, 2020, 2:28 p.m. UTC | #2 
Hi Laurent,

On 05/08/2020 01:43, Laurent Pinchart wrote:
> 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@ideasonboard.com>
>> ---
>>  test/mapped-buffers.cpp | 113 ++++++++++++++++++++++++++++++++++++++++
> 
> We usually don't pluralize the test file name.

Depluralised.

> 
>>  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 ?

Ah yes,


>> +
>> +using namespace std;
>> +
>> +namespace {
>> +
>> +class MappedBuffers : public CameraTest, public Test
> 
> We usually name the test class with a Test suffix (MappedBufferTest).

Fixed.


> 
>> +{
>> +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.
> 

In this case I don't care about the source, just that I can get an
allocation for a 'real' buffer.



>> +	{
>> +	}
>> +
>> +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:

Fixed.

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

Thanks.



> 
>> +	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'],
>

Patch
diff mbox series 

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"
+
+using namespace std;
+
+namespace {
+
+class MappedBuffers : public CameraTest, public Test
+{
+public:
+	MappedBuffers()
+		: CameraTest("VIMC Sensor B")
+	{
+	}
+
+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;
+	}
+
+	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'],