Message ID | 20200804214711.177645-5-kieran.bingham@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
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'],
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'], >
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'],
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