[libcamera-devel,4/5] test: camera: Add capture test

708 diff mbox series

Add positive capture test. Correctly configure the camera using the
default format and run a capture session for 100 milliseconds, which is
plenty of time, in tests over 600 requests completed using the vimc
pipeline.

The test passes if at least one requests completes.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 test/camera/capture.cpp | 130 ++++++++++++++++++++++++++++++++++++++++
 test/camera/meson.build |   1 +
 2 files changed, 131 insertions(+)
 create mode 100644 test/camera/capture.cpp

Hi Niklas,

Thank you for the patch.

On Wed, Mar 06, 2019 at 03:47:54AM +0100, Niklas Söderlund wrote:
> Add positive capture test. Correctly configure the camera using the

What's a positive test ?

> default format and run a capture session for 100 milliseconds, which is
> plenty of time, in tests over 600 requests completed using the vimc
> pipeline.
> 
> The test passes if at least one requests completes.

How about making that 2, or better, the number of buffers * 2, to make
sure we can cycle through all buffers ?

> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  test/camera/capture.cpp | 130 ++++++++++++++++++++++++++++++++++++++++
>  test/camera/meson.build |   1 +
>  2 files changed, 131 insertions(+)
>  create mode 100644 test/camera/capture.cpp
> 
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> new file mode 100644
> index 0000000000000000..133b38318e471f3f
> --- /dev/null
> +++ b/test/camera/capture.cpp
> @@ -0,0 +1,130 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * libcamera Camera API tests
> + */
> +
> +#include <iostream>
> +
> +#include "camera_test.h"
> +
> +using namespace std;
> +
> +namespace {
> +
> +class Capture : public CameraTest
> +{
> +protected:
> +	unsigned int count_buffers_;
> +	unsigned int count_requests_;

countBuffers_, countRequests_. Or possible better buffersCount_,
requestsCount_, or numBuffers_, numRequests_. Or to show this counts the
number of completed buffers and requests, completeBuffersCount_ and
completeRequestsCount_ ?

> +
> +	void bufferComplete(Request *request, Buffer *buffer)
> +	{
> +		count_buffers_++;
> +	}
> +
> +	void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)
> +	{
> +		if (request->status() == Request::RequestCancelled)
> +			return;
> +
> +		count_requests_++;
> +
> +		/* Reuse the buffers for a new request. */
> +		request = camera_->createRequest();
> +		request->setBuffers(buffers);
> +		camera_->queueRequest(request);
> +	}
> +
> +	int run()
> +	{
> +		if (camera_->acquire()) {
> +			cout << "Acquiring the camera failed" << endl;
> +			return TestFail;
> +		}
> +
> +		Stream *stream = *camera_->streams().begin();
> +		std::set<Stream *> streams = { stream };
> +		std::map<Stream *, StreamConfiguration> conf =
> +			camera_->streamConfiguration(streams);
> +		if (conf.size() != 1) {
> +			cout << "Reading default format failed" << endl;
> +			return TestFail;
> +		}

Using the base method from patch 2/5 here too ?

> +
> +		if (camera_->configureStreams(conf)) {
> +			cout << "Setting valid format failed" << endl;
> +			return TestFail;
> +		}
> +
> +		if (camera_->allocateBuffers()) {
> +			cout << "Allocating buffers failed" << endl;
> +			return TestFail;
> +		}
> +
> +		BufferPool &pool = stream->bufferPool();
> +		std::vector<Request *> requests;
> +		for (Buffer &buffer : pool.buffers()) {
> +			Request *request = camera_->createRequest();
> +			if (!request) {
> +				cout << "Creating request failed" << endl;
> +				return TestFail;
> +			}
> +
> +			std::map<Stream *, Buffer *> map = { { stream, &buffer } };
> +			if (request->setBuffers(map)) {
> +				cout << "Associating buffer with request failed" << endl;
> +				return TestFail;
> +			}
> +
> +			requests.push_back(request);
> +		}
> +
> +		if (camera_->start()) {
> +			cout << "Starting camera failed" << endl;
> +			return TestFail;
> +		}
> +
> +		for (Request *request : requests) {
> +			if (camera_->queueRequest(request)) {
> +				cout << "Queueing request failed" << endl;
> +				return TestFail;
> +			}
> +		}
> +
> +		camera_->bufferCompleted.connect(this, &Capture::bufferComplete);
> +		camera_->requestCompleted.connect(this, &Capture::requestComplete);

You should connect the slots before starting the camera. We're
single-threaded at the moment so it shouldn't make a big different, but
let's be prepared.

> +
> +		count_requests_ = 0;
> +		count_buffers_ = 0;

This should also be initialized before starting the camera.

> +
> +		EventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();
> +
> +		Timer timer;
> +		timer.start(100);
> +		while (timer.isRunning())
> +			dispatcher->processEvents();
> +
> +		if (!count_requests_ || !count_buffers_) {
> +			cout << "Capture failed" << endl;
> +			return TestFail;
> +		}

Should you also test that the two counters are identical ? You may then
need to skip increasing the buffers counter if the buffer status reports
an error.

> +
> +		if (camera_->stop()) {
> +			cout << "Stopping camera failed" << endl;
> +			return TestFail;
> +		}
> +
> +		if (camera_->freeBuffers()) {
> +			cout << "Freeing buffers failed" << endl;
> +			return TestFail;
> +		}
> +
> +		return TestPass;
> +	}
> +};
> +
> +} /* namespace */
> +
> +TEST_REGISTER(Capture);
> diff --git a/test/camera/meson.build b/test/camera/meson.build
> index f5f27c4229ac307f..6da297714f34a4e3 100644
> --- a/test/camera/meson.build
> +++ b/test/camera/meson.build
> @@ -3,6 +3,7 @@
>  camera_tests = [
>    [ 'format_default',        'format_default.cpp' ],
>    [ 'format_set',            'format_set.cpp' ],
> +  [ 'capture',               'capture.cpp' ],
>  ]
>  
>  foreach t : camera_tests

Hi Laurent,

Thanks for your feedback.

On 2019-03-10 15:37:21 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Wed, Mar 06, 2019 at 03:47:54AM +0100, Niklas Söderlund wrote:
> > Add positive capture test. Correctly configure the camera using the
> 
> What's a positive test ?

A test that don't try to provoke and error but tests that the intended 
function works :-) I will drop 'positive' for v2.

> 
> > default format and run a capture session for 100 milliseconds, which is
> > plenty of time, in tests over 600 requests completed using the vimc
> > pipeline.
> > 
> > The test passes if at least one requests completes.
> 
> How about making that 2, or better, the number of buffers * 2, to make
> sure we can cycle through all buffers ?

Good idea, will make it so for v2.

> 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  test/camera/capture.cpp | 130 ++++++++++++++++++++++++++++++++++++++++
> >  test/camera/meson.build |   1 +
> >  2 files changed, 131 insertions(+)
> >  create mode 100644 test/camera/capture.cpp
> > 
> > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> > new file mode 100644
> > index 0000000000000000..133b38318e471f3f
> > --- /dev/null
> > +++ b/test/camera/capture.cpp
> > @@ -0,0 +1,130 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * libcamera Camera API tests
> > + */
> > +
> > +#include <iostream>
> > +
> > +#include "camera_test.h"
> > +
> > +using namespace std;
> > +
> > +namespace {
> > +
> > +class Capture : public CameraTest
> > +{
> > +protected:
> > +	unsigned int count_buffers_;
> > +	unsigned int count_requests_;
> 
> countBuffers_, countRequests_. Or possible better buffersCount_,
> requestsCount_, or numBuffers_, numRequests_. Or to show this counts the
> number of completed buffers and requests, completeBuffersCount_ and
> completeRequestsCount_ ?

I will go for completeBuffersCount_ and completeRequestsCount_.

> 
> > +
> > +	void bufferComplete(Request *request, Buffer *buffer)
> > +	{
> > +		count_buffers_++;
> > +	}
> > +
> > +	void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)
> > +	{
> > +		if (request->status() == Request::RequestCancelled)
> > +			return;
> > +
> > +		count_requests_++;
> > +
> > +		/* Reuse the buffers for a new request. */
> > +		request = camera_->createRequest();
> > +		request->setBuffers(buffers);
> > +		camera_->queueRequest(request);
> > +	}
> > +
> > +	int run()
> > +	{
> > +		if (camera_->acquire()) {
> > +			cout << "Acquiring the camera failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		Stream *stream = *camera_->streams().begin();
> > +		std::set<Stream *> streams = { stream };
> > +		std::map<Stream *, StreamConfiguration> conf =
> > +			camera_->streamConfiguration(streams);
> > +		if (conf.size() != 1) {
> > +			cout << "Reading default format failed" << endl;
> > +			return TestFail;
> > +		}
> 
> Using the base method from patch 2/5 here too ?

Yes :-)

> 
> > +
> > +		if (camera_->configureStreams(conf)) {
> > +			cout << "Setting valid format failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (camera_->allocateBuffers()) {
> > +			cout << "Allocating buffers failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		BufferPool &pool = stream->bufferPool();
> > +		std::vector<Request *> requests;
> > +		for (Buffer &buffer : pool.buffers()) {
> > +			Request *request = camera_->createRequest();
> > +			if (!request) {
> > +				cout << "Creating request failed" << endl;
> > +				return TestFail;
> > +			}
> > +
> > +			std::map<Stream *, Buffer *> map = { { stream, &buffer } };
> > +			if (request->setBuffers(map)) {
> > +				cout << "Associating buffer with request failed" << endl;
> > +				return TestFail;
> > +			}
> > +
> > +			requests.push_back(request);
> > +		}
> > +
> > +		if (camera_->start()) {
> > +			cout << "Starting camera failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		for (Request *request : requests) {
> > +			if (camera_->queueRequest(request)) {
> > +				cout << "Queueing request failed" << endl;
> > +				return TestFail;
> > +			}
> > +		}
> > +
> > +		camera_->bufferCompleted.connect(this, &Capture::bufferComplete);
> > +		camera_->requestCompleted.connect(this, &Capture::requestComplete);
> 
> You should connect the slots before starting the camera. We're
> single-threaded at the moment so it shouldn't make a big different, but
> let's be prepared.

Good point.

> 
> > +
> > +		count_requests_ = 0;
> > +		count_buffers_ = 0;
> 
> This should also be initialized before starting the camera.

Done.

> 
> > +
> > +		EventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();
> > +
> > +		Timer timer;
> > +		timer.start(100);
> > +		while (timer.isRunning())
> > +			dispatcher->processEvents();
> > +
> > +		if (!count_requests_ || !count_buffers_) {
> > +			cout << "Capture failed" << endl;
> > +			return TestFail;
> > +		}
> 
> Should you also test that the two counters are identical ? You may then
> need to skip increasing the buffers counter if the buffer status reports
> an error.

Will do so for v2.

> 
> > +
> > +		if (camera_->stop()) {
> > +			cout << "Stopping camera failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		if (camera_->freeBuffers()) {
> > +			cout << "Freeing buffers failed" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		return TestPass;
> > +	}
> > +};
> > +
> > +} /* namespace */
> > +
> > +TEST_REGISTER(Capture);
> > diff --git a/test/camera/meson.build b/test/camera/meson.build
> > index f5f27c4229ac307f..6da297714f34a4e3 100644
> > --- a/test/camera/meson.build
> > +++ b/test/camera/meson.build
> > @@ -3,6 +3,7 @@
> >  camera_tests = [
> >    [ 'format_default',        'format_default.cpp' ],
> >    [ 'format_set',            'format_set.cpp' ],
> > +  [ 'capture',               'capture.cpp' ],
> >  ]
> >  
> >  foreach t : camera_tests
> 
> -- 
> Regards,
> 
> Laurent Pinchart