[libcamera-devel,3/5] test: v4l2_device: Provide buffer sharing test

546 diff mbox series

Obtain two V4L2Devices and use one to obtain a BufferPool.

Propogate the formats from the first to the second device and then commence
sending buffers between the two devices in a ping-pong fashion.

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

Hi Kieran,

Thank you for the patch.

On Thu, Feb 07, 2019 at 09:21:17PM +0000, Kieran Bingham wrote:
> Obtain two V4L2Devices and use one to obtain a BufferPool.
> 
> Propogate the formats from the first to the second device and then commence

s/Propogate/Propagate/

> sending buffers between the two devices in a ping-pong fashion.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  test/v4l2_device/buffer_sharing.cpp | 178 ++++++++++++++++++++++++++++
>  test/v4l2_device/meson.build        |   1 +
>  2 files changed, 179 insertions(+)
>  create mode 100644 test/v4l2_device/buffer_sharing.cpp
> 
> diff --git a/test/v4l2_device/buffer_sharing.cpp b/test/v4l2_device/buffer_sharing.cpp
> new file mode 100644
> index 000000000000..0e96f7b894bd
> --- /dev/null
> +++ b/test/v4l2_device/buffer_sharing.cpp
> @@ -0,0 +1,178 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * libcamera V4L2 API tests

Should this be updated ?

> + */
> +
> +#include <iostream>
> +
> +#include <libcamera/buffer.h>
> +#include <libcamera/camera_manager.h>
> +#include <libcamera/event_dispatcher.h>
> +#include <libcamera/timer.h>
> +
> +#include "v4l2_device_test.h"
> +
> +#include "log.h"
> +
> +LOG_DEFINE_CATEGORY(Test)

The logger is internal to libcamera, let's not use it for tests. We
should define a test-specific logging infrastructure instead, to suit
the specific needs of tests.

> +class BufferSharingTest : public V4L2DeviceTest
> +{
> +public:
> +	BufferSharingTest()
> +		: output_(nullptr), framesCapture(0), framesOutput(0){};
> +
> +private:
> +	const unsigned int bufferCount = 4;
> +
> +	V4L2Device *output_;
> +	std::shared_ptr<MediaDevice> secondMedia_;
> +
> +	unsigned int framesCapture;
> +	unsigned int framesOutput;
> +
> +protected:
> +	int init()
> +	{
> +		int ret = V4L2DeviceTest::init();
> +		if (ret)
> +			return ret;
> +
> +		DeviceMatch uvcvideo("uvcvideo");
> +		uvcvideo.add("Logitech BRIO");

This is *very* specific to your setup, we need a more generic solution.
One option is to switch to vivid, as it has both a video capture and a
video output device.

> +		secondMedia_ = std::move(enumerator_->search(uvcvideo));
> +		if (!secondMedia_) {
> +			LOG(Test, Info) << "No Brio found";
> +			return TestSkip;
> +		}
> +
> +		secondMedia_->acquire();
> +
> +		MediaEntity *entity = secondMedia_->defaultEntity();
> +		if (!entity)
> +			return TestFail;
> +
> +		output_ = new V4L2Device(entity);
> +		if (!output_)
> +			return TestFail;
> +
> +		ret = output_->open();
> +		if (ret)
> +			return TestFail;

How about adding an openDevice(const DeviceMatch &dm, ...) function to
V4L2DeviceTest to do all this, and call it both from
V4L2DeviceTest::init() and here ?

> +		V4L2DeviceFormat format;
> +
> +		ret = dev_->getFormat(&format);
> +		if (ret) {
> +			return TestFail;
> +		}
> +
> +		LOG(Test, Info) << "Successfully obtained format from source";
> +
> +		ret = output_->setFormat(&format);
> +		if (ret)
> +			return TestFail;
> +
> +		LOG(Test, Info) << "Successfully set format to output";
> +
> +		pool_.createBuffers(bufferCount);
> +
> +		ret = dev_->exportBuffers(bufferCount, &pool_);
> +		if (ret)
> +			return TestFail;
> +
> +		ret = output_->importBuffers(&pool_);
> +		if (ret) {
> +			std::cerr << "Failed to import buffers" << std::endl;
> +			return TestFail;
> +		}
> +
> +		return 0;
> +	}
> +
> +	void receiveSourceBuffer(Buffer *buffer)
> +	{
> +		std::cout << "Received source buffer: " << buffer->index()
> +			  << " sequence " << buffer->sequence() << std::endl;
> +
> +		output_->queueBuffer(buffer);
> +		framesCapture++;
> +	}
> +
> +	void receiveDestinationBuffer(Buffer *buffer)
> +	{
> +		std::cout << "Received destination buffer: " << buffer->index()
> +			  << " sequence " << buffer->sequence() << std::endl;
> +
> +		dev_->queueBuffer(buffer);
> +		framesOutput++;
> +	}
> +
> +	int run()
> +	{
> +		EventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();
> +		Timer timeout;
> +		int ret;
> +
> +		dev_->bufferReady.connect(this, &BufferSharingTest::receiveSourceBuffer);
> +		output_->bufferReady.connect(this, &BufferSharingTest::receiveDestinationBuffer);
> +
> +		/* Queue all the buffers to the device. */
> +		for (Buffer &b : pool_.buffers()) {
> +			if (dev_->queueBuffer(&b))
> +				return TestFail;
> +		}
> +
> +		ret = dev_->streamOn();
> +		if (ret)
> +			return TestFail;
> +
> +		ret = output_->streamOn();
> +		if (ret)
> +			return TestFail;
> +
> +		timeout.start(5000);
> +		while (timeout.isRunning())
> +			dispatcher->processEvents();
> +
> +		if ((framesCapture < 1) || (framesOutput < 1)) {
> +			std::cout << "Failed to process any frames within timeout." << std::endl;
> +			return TestFail;
> +		}
> +
> +		if ((framesCapture < 30) || (framesOutput < 30)) {
> +			std::cout << "Failed to process 30 frames within timeout." << std::endl;
> +			return TestFail;
> +		}
> +
> +		std::cout
> +			<< "Processed " << framesCapture << " capture frames"
> +			<< " and " << framesOutput << " output frames"
> +			<< std::endl;
> +
> +		ret = dev_->streamOff();
> +		if (ret)
> +			return TestFail;
> +
> +		ret = output_->streamOff();
> +		if (ret)
> +			return TestFail;
> +
> +		return TestPass;
> +	}
> +
> +	void cleanup()
> +	{
> +		if (secondMedia_)
> +			secondMedia_->release();
> +
> +		delete output_;
> +
> +		V4L2DeviceTest::cleanup();
> +	}
> +};
> +
> +TEST_REGISTER(BufferSharingTest);
> diff --git a/test/v4l2_device/meson.build b/test/v4l2_device/meson.build
> index ec2c7f9f11ff..9f7a7545ac9b 100644
> --- a/test/v4l2_device/meson.build
> +++ b/test/v4l2_device/meson.build
> @@ -5,6 +5,7 @@ v4l2_device_tests = [
>    [ 'request_buffers',    'request_buffers.cpp' ],
>    [ 'stream_on_off',      'stream_on_off.cpp' ],
>    [ 'capture_async',      'capture_async.cpp' ],
> +  [ 'buffer_sharing',     'buffer_sharing.cpp' ],
>  ]
>  
>  foreach t : v4l2_device_tests

Hi Laurent,

On 08/02/2019 17:28, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thu, Feb 07, 2019 at 09:21:17PM +0000, Kieran Bingham wrote:
>> Obtain two V4L2Devices and use one to obtain a BufferPool.
>>
>> Propogate the formats from the first to the second device and then commence
> 
> s/Propogate/Propagate/

Ack.

> 
>> sending buffers between the two devices in a ping-pong fashion.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> ---
>>  test/v4l2_device/buffer_sharing.cpp | 178 ++++++++++++++++++++++++++++
>>  test/v4l2_device/meson.build        |   1 +
>>  2 files changed, 179 insertions(+)
>>  create mode 100644 test/v4l2_device/buffer_sharing.cpp
>>
>> diff --git a/test/v4l2_device/buffer_sharing.cpp b/test/v4l2_device/buffer_sharing.cpp
>> new file mode 100644
>> index 000000000000..0e96f7b894bd
>> --- /dev/null
>> +++ b/test/v4l2_device/buffer_sharing.cpp
>> @@ -0,0 +1,178 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (C) 2019, Google Inc.
>> + *
>> + * libcamera V4L2 API tests
> 
> Should this be updated ?

It's part of the V4L2 API tests, no?


> 
>> + */
>> +
>> +#include <iostream>
>> +
>> +#include <libcamera/buffer.h>
>> +#include <libcamera/camera_manager.h>
>> +#include <libcamera/event_dispatcher.h>
>> +#include <libcamera/timer.h>
>> +
>> +#include "v4l2_device_test.h"
>> +
>> +#include "log.h"
>> +
>> +LOG_DEFINE_CATEGORY(Test)
> 
> The logger is internal to libcamera, let's not use it for tests. We
> should define a test-specific logging infrastructure instead, to suit
> the specific needs of tests.
> 

Ah yes - I should have dropped this - That part was just me playing
around with the code.

It works in this instance because this code is an 'internal' test and
thus has access to all internals including the logging mechanism - but I
agree the Tests need their own.

Care to write a TestLoggger sometime so we can actually fix all this up?



>> +class BufferSharingTest : public V4L2DeviceTest
>> +{
>> +public:
>> +	BufferSharingTest()
>> +		: output_(nullptr), framesCapture(0), framesOutput(0){};
>> +
>> +private:
>> +	const unsigned int bufferCount = 4;
>> +
>> +	V4L2Device *output_;
>> +	std::shared_ptr<MediaDevice> secondMedia_;
>> +
>> +	unsigned int framesCapture;
>> +	unsigned int framesOutput;
>> +
>> +protected:
>> +	int init()
>> +	{
>> +		int ret = V4L2DeviceTest::init();
>> +		if (ret)
>> +			return ret;
>> +
>> +		DeviceMatch uvcvideo("uvcvideo");
>> +		uvcvideo.add("Logitech BRIO");
> 
> This is *very* specific to your setup, we need a more generic solution.
> One option is to switch to vivid, as it has both a video capture and a
> video output device.


Yes, of course it is - as was detailed in the cover letter.

The main purpose of this posting is to support Jacopo in his continued
development on the IPU3 which needs buffer imports.



> 
>> +		secondMedia_ = std::move(enumerator_->search(uvcvideo));
>> +		if (!secondMedia_) {
>> +			LOG(Test, Info) << "No Brio found";
>> +			return TestSkip;
>> +		}
>> +
>> +		secondMedia_->acquire();
>> +
>> +		MediaEntity *entity = secondMedia_->defaultEntity();
>> +		if (!entity)
>> +			return TestFail;
>> +
>> +		output_ = new V4L2Device(entity);
>> +		if (!output_)
>> +			return TestFail;
>> +
>> +		ret = output_->open();
>> +		if (ret)
>> +			return TestFail;
> 
> How about adding an openDevice(const DeviceMatch &dm, ...) function to
> V4L2DeviceTest to do all this, and call it both from
> V4L2DeviceTest::init() and here ?


This function certainly got long fast. A helper would be useful yes.


> 
>> +		V4L2DeviceFormat format;
>> +
>> +		ret = dev_->getFormat(&format);
>> +		if (ret) {
>> +			return TestFail;
>> +		}
>> +
>> +		LOG(Test, Info) << "Successfully obtained format from source";
>> +
>> +		ret = output_->setFormat(&format);
>> +		if (ret)
>> +			return TestFail;
>> +
>> +		LOG(Test, Info) << "Successfully set format to output";
>> +
>> +		pool_.createBuffers(bufferCount);
>> +
>> +		ret = dev_->exportBuffers(bufferCount, &pool_);
>> +		if (ret)
>> +			return TestFail;
>> +
>> +		ret = output_->importBuffers(&pool_);
>> +		if (ret) {
>> +			std::cerr << "Failed to import buffers" << std::endl;
>> +			return TestFail;
>> +		}
>> +
>> +		return 0;
>> +	}
>> +
>> +	void receiveSourceBuffer(Buffer *buffer)
>> +	{
>> +		std::cout << "Received source buffer: " << buffer->index()
>> +			  << " sequence " << buffer->sequence() << std::endl;
>> +
>> +		output_->queueBuffer(buffer);
>> +		framesCapture++;
>> +	}
>> +
>> +	void receiveDestinationBuffer(Buffer *buffer)
>> +	{
>> +		std::cout << "Received destination buffer: " << buffer->index()
>> +			  << " sequence " << buffer->sequence() << std::endl;
>> +
>> +		dev_->queueBuffer(buffer);
>> +		framesOutput++;
>> +	}
>> +
>> +	int run()
>> +	{
>> +		EventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();
>> +		Timer timeout;
>> +		int ret;
>> +
>> +		dev_->bufferReady.connect(this, &BufferSharingTest::receiveSourceBuffer);
>> +		output_->bufferReady.connect(this, &BufferSharingTest::receiveDestinationBuffer);
>> +
>> +		/* Queue all the buffers to the device. */
>> +		for (Buffer &b : pool_.buffers()) {
>> +			if (dev_->queueBuffer(&b))
>> +				return TestFail;
>> +		}
>> +
>> +		ret = dev_->streamOn();
>> +		if (ret)
>> +			return TestFail;
>> +
>> +		ret = output_->streamOn();
>> +		if (ret)
>> +			return TestFail;
>> +
>> +		timeout.start(5000);
>> +		while (timeout.isRunning())
>> +			dispatcher->processEvents();
>> +
>> +		if ((framesCapture < 1) || (framesOutput < 1)) {
>> +			std::cout << "Failed to process any frames within timeout." << std::endl;
>> +			return TestFail;
>> +		}
>> +
>> +		if ((framesCapture < 30) || (framesOutput < 30)) {
>> +			std::cout << "Failed to process 30 frames within timeout." << std::endl;
>> +			return TestFail;
>> +		}
>> +
>> +		std::cout
>> +			<< "Processed " << framesCapture << " capture frames"
>> +			<< " and " << framesOutput << " output frames"
>> +			<< std::endl;
>> +
>> +		ret = dev_->streamOff();
>> +		if (ret)
>> +			return TestFail;
>> +
>> +		ret = output_->streamOff();
>> +		if (ret)
>> +			return TestFail;
>> +
>> +		return TestPass;
>> +	}
>> +
>> +	void cleanup()
>> +	{
>> +		if (secondMedia_)
>> +			secondMedia_->release();
>> +
>> +		delete output_;
>> +
>> +		V4L2DeviceTest::cleanup();
>> +	}
>> +};
>> +
>> +TEST_REGISTER(BufferSharingTest);
>> diff --git a/test/v4l2_device/meson.build b/test/v4l2_device/meson.build
>> index ec2c7f9f11ff..9f7a7545ac9b 100644
>> --- a/test/v4l2_device/meson.build
>> +++ b/test/v4l2_device/meson.build
>> @@ -5,6 +5,7 @@ v4l2_device_tests = [
>>    [ 'request_buffers',    'request_buffers.cpp' ],
>>    [ 'stream_on_off',      'stream_on_off.cpp' ],
>>    [ 'capture_async',      'capture_async.cpp' ],
>> +  [ 'buffer_sharing',     'buffer_sharing.cpp' ],
>>  ]
>>  
>>  foreach t : v4l2_device_tests
>

Hi Kieran,

On Mon, Feb 11, 2019 at 11:52:35AM +0000, Kieran Bingham wrote:
> On 08/02/2019 17:28, Laurent Pinchart wrote:
> > On Thu, Feb 07, 2019 at 09:21:17PM +0000, Kieran Bingham wrote:
> >> Obtain two V4L2Devices and use one to obtain a BufferPool.
> >>
> >> Propogate the formats from the first to the second device and then commence
> > 
> > s/Propogate/Propagate/
> 
> Ack.
> 
> >> sending buffers between the two devices in a ping-pong fashion.
> >>
> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >> ---
> >>  test/v4l2_device/buffer_sharing.cpp | 178 ++++++++++++++++++++++++++++
> >>  test/v4l2_device/meson.build        |   1 +
> >>  2 files changed, 179 insertions(+)
> >>  create mode 100644 test/v4l2_device/buffer_sharing.cpp
> >>
> >> diff --git a/test/v4l2_device/buffer_sharing.cpp b/test/v4l2_device/buffer_sharing.cpp
> >> new file mode 100644
> >> index 000000000000..0e96f7b894bd
> >> --- /dev/null
> >> +++ b/test/v4l2_device/buffer_sharing.cpp
> >> @@ -0,0 +1,178 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >> +/*
> >> + * Copyright (C) 2019, Google Inc.
> >> + *
> >> + * libcamera V4L2 API tests
> > 
> > Should this be updated ?
> 
> It's part of the V4L2 API tests, no?

Sure, but it wouldn't hurt to be a tad more specific :-)

> >> + */
> >> +
> >> +#include <iostream>
> >> +
> >> +#include <libcamera/buffer.h>
> >> +#include <libcamera/camera_manager.h>
> >> +#include <libcamera/event_dispatcher.h>
> >> +#include <libcamera/timer.h>
> >> +
> >> +#include "v4l2_device_test.h"
> >> +
> >> +#include "log.h"
> >> +
> >> +LOG_DEFINE_CATEGORY(Test)
> > 
> > The logger is internal to libcamera, let's not use it for tests. We
> > should define a test-specific logging infrastructure instead, to suit
> > the specific needs of tests.
> 
> Ah yes - I should have dropped this - That part was just me playing
> around with the code.
> 
> It works in this instance because this code is an 'internal' test and
> thus has access to all internals including the logging mechanism - but I
> agree the Tests need their own.
> 
> Care to write a TestLoggger sometime so we can actually fix all this up?

I've added a task for that, let's see who can tackle it first :-)

> >> +class BufferSharingTest : public V4L2DeviceTest
> >> +{
> >> +public:
> >> +	BufferSharingTest()
> >> +		: output_(nullptr), framesCapture(0), framesOutput(0){};
> >> +
> >> +private:
> >> +	const unsigned int bufferCount = 4;
> >> +
> >> +	V4L2Device *output_;
> >> +	std::shared_ptr<MediaDevice> secondMedia_;
> >> +
> >> +	unsigned int framesCapture;
> >> +	unsigned int framesOutput;
> >> +
> >> +protected:
> >> +	int init()
> >> +	{
> >> +		int ret = V4L2DeviceTest::init();
> >> +		if (ret)
> >> +			return ret;
> >> +
> >> +		DeviceMatch uvcvideo("uvcvideo");
> >> +		uvcvideo.add("Logitech BRIO");
> > 
> > This is *very* specific to your setup, we need a more generic solution.
> > One option is to switch to vivid, as it has both a video capture and a
> > video output device.
> 
> Yes, of course it is - as was detailed in the cover letter.
> 
> The main purpose of this posting is to support Jacopo in his continued
> development on the IPU3 which needs buffer imports.

Do you think it would be lots of work to move this to vivid ?

> >> +		secondMedia_ = std::move(enumerator_->search(uvcvideo));
> >> +		if (!secondMedia_) {
> >> +			LOG(Test, Info) << "No Brio found";
> >> +			return TestSkip;
> >> +		}
> >> +
> >> +		secondMedia_->acquire();
> >> +
> >> +		MediaEntity *entity = secondMedia_->defaultEntity();
> >> +		if (!entity)
> >> +			return TestFail;
> >> +
> >> +		output_ = new V4L2Device(entity);
> >> +		if (!output_)
> >> +			return TestFail;
> >> +
> >> +		ret = output_->open();
> >> +		if (ret)
> >> +			return TestFail;
> > 
> > How about adding an openDevice(const DeviceMatch &dm, ...) function to
> > V4L2DeviceTest to do all this, and call it both from
> > V4L2DeviceTest::init() and here ?
> 
> This function certainly got long fast. A helper would be useful yes.
> 
> >> +		V4L2DeviceFormat format;
> >> +
> >> +		ret = dev_->getFormat(&format);
> >> +		if (ret) {
> >> +			return TestFail;
> >> +		}
> >> +
> >> +		LOG(Test, Info) << "Successfully obtained format from source";
> >> +
> >> +		ret = output_->setFormat(&format);
> >> +		if (ret)
> >> +			return TestFail;
> >> +
> >> +		LOG(Test, Info) << "Successfully set format to output";
> >> +
> >> +		pool_.createBuffers(bufferCount);
> >> +
> >> +		ret = dev_->exportBuffers(bufferCount, &pool_);
> >> +		if (ret)
> >> +			return TestFail;
> >> +
> >> +		ret = output_->importBuffers(&pool_);
> >> +		if (ret) {
> >> +			std::cerr << "Failed to import buffers" << std::endl;
> >> +			return TestFail;
> >> +		}
> >> +
> >> +		return 0;
> >> +	}
> >> +
> >> +	void receiveSourceBuffer(Buffer *buffer)
> >> +	{
> >> +		std::cout << "Received source buffer: " << buffer->index()
> >> +			  << " sequence " << buffer->sequence() << std::endl;
> >> +
> >> +		output_->queueBuffer(buffer);
> >> +		framesCapture++;
> >> +	}
> >> +
> >> +	void receiveDestinationBuffer(Buffer *buffer)
> >> +	{
> >> +		std::cout << "Received destination buffer: " << buffer->index()
> >> +			  << " sequence " << buffer->sequence() << std::endl;
> >> +
> >> +		dev_->queueBuffer(buffer);
> >> +		framesOutput++;
> >> +	}
> >> +
> >> +	int run()
> >> +	{
> >> +		EventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();
> >> +		Timer timeout;
> >> +		int ret;
> >> +
> >> +		dev_->bufferReady.connect(this, &BufferSharingTest::receiveSourceBuffer);
> >> +		output_->bufferReady.connect(this, &BufferSharingTest::receiveDestinationBuffer);
> >> +
> >> +		/* Queue all the buffers to the device. */
> >> +		for (Buffer &b : pool_.buffers()) {
> >> +			if (dev_->queueBuffer(&b))
> >> +				return TestFail;
> >> +		}
> >> +
> >> +		ret = dev_->streamOn();
> >> +		if (ret)
> >> +			return TestFail;
> >> +
> >> +		ret = output_->streamOn();
> >> +		if (ret)
> >> +			return TestFail;
> >> +
> >> +		timeout.start(5000);
> >> +		while (timeout.isRunning())
> >> +			dispatcher->processEvents();
> >> +
> >> +		if ((framesCapture < 1) || (framesOutput < 1)) {
> >> +			std::cout << "Failed to process any frames within timeout." << std::endl;
> >> +			return TestFail;
> >> +		}
> >> +
> >> +		if ((framesCapture < 30) || (framesOutput < 30)) {
> >> +			std::cout << "Failed to process 30 frames within timeout." << std::endl;
> >> +			return TestFail;
> >> +		}
> >> +
> >> +		std::cout
> >> +			<< "Processed " << framesCapture << " capture frames"
> >> +			<< " and " << framesOutput << " output frames"
> >> +			<< std::endl;
> >> +
> >> +		ret = dev_->streamOff();
> >> +		if (ret)
> >> +			return TestFail;
> >> +
> >> +		ret = output_->streamOff();
> >> +		if (ret)
> >> +			return TestFail;
> >> +
> >> +		return TestPass;
> >> +	}
> >> +
> >> +	void cleanup()
> >> +	{
> >> +		if (secondMedia_)
> >> +			secondMedia_->release();
> >> +
> >> +		delete output_;
> >> +
> >> +		V4L2DeviceTest::cleanup();
> >> +	}
> >> +};
> >> +
> >> +TEST_REGISTER(BufferSharingTest);
> >> diff --git a/test/v4l2_device/meson.build b/test/v4l2_device/meson.build
> >> index ec2c7f9f11ff..9f7a7545ac9b 100644
> >> --- a/test/v4l2_device/meson.build
> >> +++ b/test/v4l2_device/meson.build
> >> @@ -5,6 +5,7 @@ v4l2_device_tests = [
> >>    [ 'request_buffers',    'request_buffers.cpp' ],
> >>    [ 'stream_on_off',      'stream_on_off.cpp' ],
> >>    [ 'capture_async',      'capture_async.cpp' ],
> >> +  [ 'buffer_sharing',     'buffer_sharing.cpp' ],
> >>  ]
> >>  
> >>  foreach t : v4l2_device_tests

Hi Laurent,

On 12/02/2019 09:20, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Mon, Feb 11, 2019 at 11:52:35AM +0000, Kieran Bingham wrote:
>> On 08/02/2019 17:28, Laurent Pinchart wrote:
>>> On Thu, Feb 07, 2019 at 09:21:17PM +0000, Kieran Bingham wrote:
>>>> Obtain two V4L2Devices and use one to obtain a BufferPool.
>>>>
>>>> Propogate the formats from the first to the second device and then commence
>>>
>>> s/Propogate/Propagate/
>>
>> Ack.
>>
>>>> sending buffers between the two devices in a ping-pong fashion.
>>>>
>>>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>>> ---
>>>>  test/v4l2_device/buffer_sharing.cpp | 178 ++++++++++++++++++++++++++++
>>>>  test/v4l2_device/meson.build        |   1 +
>>>>  2 files changed, 179 insertions(+)
>>>>  create mode 100644 test/v4l2_device/buffer_sharing.cpp
>>>>
>>>> diff --git a/test/v4l2_device/buffer_sharing.cpp b/test/v4l2_device/buffer_sharing.cpp
>>>> new file mode 100644
>>>> index 000000000000..0e96f7b894bd
>>>> --- /dev/null
>>>> +++ b/test/v4l2_device/buffer_sharing.cpp
>>>> @@ -0,0 +1,178 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>>> +/*
>>>> + * Copyright (C) 2019, Google Inc.
>>>> + *
>>>> + * libcamera V4L2 API tests
>>>
>>> Should this be updated ?
>>
>> It's part of the V4L2 API tests, no?
> 
> Sure, but it wouldn't hurt to be a tad more specific :-)
> 
>>>> + */
>>>> +
>>>> +#include <iostream>
>>>> +
>>>> +#include <libcamera/buffer.h>
>>>> +#include <libcamera/camera_manager.h>
>>>> +#include <libcamera/event_dispatcher.h>
>>>> +#include <libcamera/timer.h>
>>>> +
>>>> +#include "v4l2_device_test.h"
>>>> +
>>>> +#include "log.h"
>>>> +
>>>> +LOG_DEFINE_CATEGORY(Test)
>>>
>>> The logger is internal to libcamera, let's not use it for tests. We
>>> should define a test-specific logging infrastructure instead, to suit
>>> the specific needs of tests.
>>
>> Ah yes - I should have dropped this - That part was just me playing
>> around with the code.
>>
>> It works in this instance because this code is an 'internal' test and
>> thus has access to all internals including the logging mechanism - but I
>> agree the Tests need their own.
>>
>> Care to write a TestLoggger sometime so we can actually fix all this up?
> 
> I've added a task for that, let's see who can tackle it first :-)
> 
>>>> +class BufferSharingTest : public V4L2DeviceTest
>>>> +{
>>>> +public:
>>>> +	BufferSharingTest()
>>>> +		: output_(nullptr), framesCapture(0), framesOutput(0){};
>>>> +
>>>> +private:
>>>> +	const unsigned int bufferCount = 4;
>>>> +
>>>> +	V4L2Device *output_;
>>>> +	std::shared_ptr<MediaDevice> secondMedia_;
>>>> +
>>>> +	unsigned int framesCapture;
>>>> +	unsigned int framesOutput;
>>>> +
>>>> +protected:
>>>> +	int init()
>>>> +	{
>>>> +		int ret = V4L2DeviceTest::init();
>>>> +		if (ret)
>>>> +			return ret;
>>>> +
>>>> +		DeviceMatch uvcvideo("uvcvideo");
>>>> +		uvcvideo.add("Logitech BRIO");
>>>
>>> This is *very* specific to your setup, we need a more generic solution.
>>> One option is to switch to vivid, as it has both a video capture and a
>>> video output device.
>>
>> Yes, of course it is - as was detailed in the cover letter.
>>
>> The main purpose of this posting is to support Jacopo in his continued
>> development on the IPU3 which needs buffer imports.
> 
> Do you think it would be lots of work to move this to vivid ?

You know my initial blocker was my inaccurate association of VIVID/VIMC.
and VIVID != VIMC which is not packaged by my distro :S
 (I should file a bug on Ubuntu)

kbingham@Q:~/projects$ sudo modprobe vimc
modprobe: FATAL: Module vimc not found in directory
/lib/modules/4.18.0-14-generic
kbingham@Q:~/projects$ sudo modprobe vivid
kbingham@Q:~/projects$ lsv4l2
video0: HP Wide Vision FHD Camera: HP W
video1: HP Wide Vision FHD Camera: HP W
video2: HP Wide Vision FHD Camera: HP I
video3: HP Wide Vision FHD Camera: HP I
video4: vivid-000-vid-cap
video5: vivid-000-vid-out


So I have vivid - but it does not provide a media device. So we can't
enumerate it with our current DeviceMatch...

I hear rumours that Linux v4.20 now provides a media device for VIVID
... should we restrict our tests to v4.20+ or add support for
non-media-controller devices?



>>>> +		secondMedia_ = std::move(enumerator_->search(uvcvideo));
>>>> +		if (!secondMedia_) {
>>>> +			LOG(Test, Info) << "No Brio found";
>>>> +			return TestSkip;
>>>> +		}
>>>> +
>>>> +		secondMedia_->acquire();
>>>> +
>>>> +		MediaEntity *entity = secondMedia_->defaultEntity();
>>>> +		if (!entity)
>>>> +			return TestFail;
>>>> +
>>>> +		output_ = new V4L2Device(entity);
>>>> +		if (!output_)
>>>> +			return TestFail;
>>>> +
>>>> +		ret = output_->open();
>>>> +		if (ret)
>>>> +			return TestFail;
>>>
>>> How about adding an openDevice(const DeviceMatch &dm, ...) function to
>>> V4L2DeviceTest to do all this, and call it both from
>>> V4L2DeviceTest::init() and here ?
>>
>> This function certainly got long fast. A helper would be useful yes.
>>
>>>> +		V4L2DeviceFormat format;
>>>> +
>>>> +		ret = dev_->getFormat(&format);
>>>> +		if (ret) {
>>>> +			return TestFail;
>>>> +		}
>>>> +
>>>> +		LOG(Test, Info) << "Successfully obtained format from source";
>>>> +
>>>> +		ret = output_->setFormat(&format);
>>>> +		if (ret)
>>>> +			return TestFail;
>>>> +
>>>> +		LOG(Test, Info) << "Successfully set format to output";
>>>> +
>>>> +		pool_.createBuffers(bufferCount);
>>>> +
>>>> +		ret = dev_->exportBuffers(bufferCount, &pool_);
>>>> +		if (ret)
>>>> +			return TestFail;
>>>> +
>>>> +		ret = output_->importBuffers(&pool_);
>>>> +		if (ret) {
>>>> +			std::cerr << "Failed to import buffers" << std::endl;
>>>> +			return TestFail;
>>>> +		}
>>>> +
>>>> +		return 0;
>>>> +	}
>>>> +
>>>> +	void receiveSourceBuffer(Buffer *buffer)
>>>> +	{
>>>> +		std::cout << "Received source buffer: " << buffer->index()
>>>> +			  << " sequence " << buffer->sequence() << std::endl;
>>>> +
>>>> +		output_->queueBuffer(buffer);
>>>> +		framesCapture++;
>>>> +	}
>>>> +
>>>> +	void receiveDestinationBuffer(Buffer *buffer)
>>>> +	{
>>>> +		std::cout << "Received destination buffer: " << buffer->index()
>>>> +			  << " sequence " << buffer->sequence() << std::endl;
>>>> +
>>>> +		dev_->queueBuffer(buffer);
>>>> +		framesOutput++;
>>>> +	}
>>>> +
>>>> +	int run()
>>>> +	{
>>>> +		EventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();
>>>> +		Timer timeout;
>>>> +		int ret;
>>>> +
>>>> +		dev_->bufferReady.connect(this, &BufferSharingTest::receiveSourceBuffer);
>>>> +		output_->bufferReady.connect(this, &BufferSharingTest::receiveDestinationBuffer);
>>>> +
>>>> +		/* Queue all the buffers to the device. */
>>>> +		for (Buffer &b : pool_.buffers()) {
>>>> +			if (dev_->queueBuffer(&b))
>>>> +				return TestFail;
>>>> +		}
>>>> +
>>>> +		ret = dev_->streamOn();
>>>> +		if (ret)
>>>> +			return TestFail;
>>>> +
>>>> +		ret = output_->streamOn();
>>>> +		if (ret)
>>>> +			return TestFail;
>>>> +
>>>> +		timeout.start(5000);
>>>> +		while (timeout.isRunning())
>>>> +			dispatcher->processEvents();
>>>> +
>>>> +		if ((framesCapture < 1) || (framesOutput < 1)) {
>>>> +			std::cout << "Failed to process any frames within timeout." << std::endl;
>>>> +			return TestFail;
>>>> +		}
>>>> +
>>>> +		if ((framesCapture < 30) || (framesOutput < 30)) {
>>>> +			std::cout << "Failed to process 30 frames within timeout." << std::endl;
>>>> +			return TestFail;
>>>> +		}
>>>> +
>>>> +		std::cout
>>>> +			<< "Processed " << framesCapture << " capture frames"
>>>> +			<< " and " << framesOutput << " output frames"
>>>> +			<< std::endl;
>>>> +
>>>> +		ret = dev_->streamOff();
>>>> +		if (ret)
>>>> +			return TestFail;
>>>> +
>>>> +		ret = output_->streamOff();
>>>> +		if (ret)
>>>> +			return TestFail;
>>>> +
>>>> +		return TestPass;
>>>> +	}
>>>> +
>>>> +	void cleanup()
>>>> +	{
>>>> +		if (secondMedia_)
>>>> +			secondMedia_->release();
>>>> +
>>>> +		delete output_;
>>>> +
>>>> +		V4L2DeviceTest::cleanup();
>>>> +	}
>>>> +};
>>>> +
>>>> +TEST_REGISTER(BufferSharingTest);
>>>> diff --git a/test/v4l2_device/meson.build b/test/v4l2_device/meson.build
>>>> index ec2c7f9f11ff..9f7a7545ac9b 100644
>>>> --- a/test/v4l2_device/meson.build
>>>> +++ b/test/v4l2_device/meson.build
>>>> @@ -5,6 +5,7 @@ v4l2_device_tests = [
>>>>    [ 'request_buffers',    'request_buffers.cpp' ],
>>>>    [ 'stream_on_off',      'stream_on_off.cpp' ],
>>>>    [ 'capture_async',      'capture_async.cpp' ],
>>>> +  [ 'buffer_sharing',     'buffer_sharing.cpp' ],
>>>>  ]
>>>>  
>>>>  foreach t : v4l2_device_tests
>

Hi Kieran,

On Tue, Feb 12, 2019 at 09:47:45AM +0000, Kieran Bingham wrote:
> On 12/02/2019 09:20, Laurent Pinchart wrote:
> > On Mon, Feb 11, 2019 at 11:52:35AM +0000, Kieran Bingham wrote:
> >> On 08/02/2019 17:28, Laurent Pinchart wrote:
> >>> On Thu, Feb 07, 2019 at 09:21:17PM +0000, Kieran Bingham wrote:
> >>>> Obtain two V4L2Devices and use one to obtain a BufferPool.
> >>>>
> >>>> Propogate the formats from the first to the second device and then commence
> >>>
> >>> s/Propogate/Propagate/
> >>
> >> Ack.
> >>
> >>>> sending buffers between the two devices in a ping-pong fashion.
> >>>>
> >>>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >>>> ---
> >>>>  test/v4l2_device/buffer_sharing.cpp | 178 ++++++++++++++++++++++++++++
> >>>>  test/v4l2_device/meson.build        |   1 +
> >>>>  2 files changed, 179 insertions(+)
> >>>>  create mode 100644 test/v4l2_device/buffer_sharing.cpp
> >>>>
> >>>> diff --git a/test/v4l2_device/buffer_sharing.cpp b/test/v4l2_device/buffer_sharing.cpp
> >>>> new file mode 100644
> >>>> index 000000000000..0e96f7b894bd
> >>>> --- /dev/null
> >>>> +++ b/test/v4l2_device/buffer_sharing.cpp
> >>>> @@ -0,0 +1,178 @@
> >>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >>>> +/*
> >>>> + * Copyright (C) 2019, Google Inc.
> >>>> + *
> >>>> + * libcamera V4L2 API tests
> >>>
> >>> Should this be updated ?
> >>
> >> It's part of the V4L2 API tests, no?
> > 
> > Sure, but it wouldn't hurt to be a tad more specific :-)
> > 
> >>>> + */
> >>>> +
> >>>> +#include <iostream>
> >>>> +
> >>>> +#include <libcamera/buffer.h>
> >>>> +#include <libcamera/camera_manager.h>
> >>>> +#include <libcamera/event_dispatcher.h>
> >>>> +#include <libcamera/timer.h>
> >>>> +
> >>>> +#include "v4l2_device_test.h"
> >>>> +
> >>>> +#include "log.h"
> >>>> +
> >>>> +LOG_DEFINE_CATEGORY(Test)
> >>>
> >>> The logger is internal to libcamera, let's not use it for tests. We
> >>> should define a test-specific logging infrastructure instead, to suit
> >>> the specific needs of tests.
> >>
> >> Ah yes - I should have dropped this - That part was just me playing
> >> around with the code.
> >>
> >> It works in this instance because this code is an 'internal' test and
> >> thus has access to all internals including the logging mechanism - but I
> >> agree the Tests need their own.
> >>
> >> Care to write a TestLoggger sometime so we can actually fix all this up?
> > 
> > I've added a task for that, let's see who can tackle it first :-)
> > 
> >>>> +class BufferSharingTest : public V4L2DeviceTest
> >>>> +{
> >>>> +public:
> >>>> +	BufferSharingTest()
> >>>> +		: output_(nullptr), framesCapture(0), framesOutput(0){};
> >>>> +
> >>>> +private:
> >>>> +	const unsigned int bufferCount = 4;
> >>>> +
> >>>> +	V4L2Device *output_;
> >>>> +	std::shared_ptr<MediaDevice> secondMedia_;
> >>>> +
> >>>> +	unsigned int framesCapture;
> >>>> +	unsigned int framesOutput;
> >>>> +
> >>>> +protected:
> >>>> +	int init()
> >>>> +	{
> >>>> +		int ret = V4L2DeviceTest::init();
> >>>> +		if (ret)
> >>>> +			return ret;
> >>>> +
> >>>> +		DeviceMatch uvcvideo("uvcvideo");
> >>>> +		uvcvideo.add("Logitech BRIO");
> >>>
> >>> This is *very* specific to your setup, we need a more generic solution.
> >>> One option is to switch to vivid, as it has both a video capture and a
> >>> video output device.
> >>
> >> Yes, of course it is - as was detailed in the cover letter.
> >>
> >> The main purpose of this posting is to support Jacopo in his continued
> >> development on the IPU3 which needs buffer imports.
> > 
> > Do you think it would be lots of work to move this to vivid ?
> 
> You know my initial blocker was my inaccurate association of VIVID/VIMC.
> and VIVID != VIMC which is not packaged by my distro :S
>  (I should file a bug on Ubuntu)
> 
> kbingham@Q:~/projects$ sudo modprobe vimc
> modprobe: FATAL: Module vimc not found in directory

:-/ A bug report is indeed needed.

> /lib/modules/4.18.0-14-generic
> kbingham@Q:~/projects$ sudo modprobe vivid
> kbingham@Q:~/projects$ lsv4l2
> video0: HP Wide Vision FHD Camera: HP W
> video1: HP Wide Vision FHD Camera: HP W
> video2: HP Wide Vision FHD Camera: HP I
> video3: HP Wide Vision FHD Camera: HP I
> video4: vivid-000-vid-cap
> video5: vivid-000-vid-out
> 
> 
> So I have vivid - but it does not provide a media device. So we can't
> enumerate it with our current DeviceMatch...
> 
> I hear rumours that Linux v4.20 now provides a media device for VIVID
> ... should we restrict our tests to v4.20+ or add support for
> non-media-controller devices?

I think we shouldn't relax the media controller requirement. The
vivid-based tests would then be restricted to v4.20+.

> >>>> +		secondMedia_ = std::move(enumerator_->search(uvcvideo));
> >>>> +		if (!secondMedia_) {
> >>>> +			LOG(Test, Info) << "No Brio found";
> >>>> +			return TestSkip;
> >>>> +		}
> >>>> +
> >>>> +		secondMedia_->acquire();
> >>>> +
> >>>> +		MediaEntity *entity = secondMedia_->defaultEntity();
> >>>> +		if (!entity)
> >>>> +			return TestFail;
> >>>> +
> >>>> +		output_ = new V4L2Device(entity);
> >>>> +		if (!output_)
> >>>> +			return TestFail;
> >>>> +
> >>>> +		ret = output_->open();
> >>>> +		if (ret)
> >>>> +			return TestFail;
> >>>
> >>> How about adding an openDevice(const DeviceMatch &dm, ...) function to
> >>> V4L2DeviceTest to do all this, and call it both from
> >>> V4L2DeviceTest::init() and here ?
> >>
> >> This function certainly got long fast. A helper would be useful yes.
> >>
> >>>> +		V4L2DeviceFormat format;
> >>>> +
> >>>> +		ret = dev_->getFormat(&format);
> >>>> +		if (ret) {
> >>>> +			return TestFail;
> >>>> +		}
> >>>> +
> >>>> +		LOG(Test, Info) << "Successfully obtained format from source";
> >>>> +
> >>>> +		ret = output_->setFormat(&format);
> >>>> +		if (ret)
> >>>> +			return TestFail;
> >>>> +
> >>>> +		LOG(Test, Info) << "Successfully set format to output";
> >>>> +
> >>>> +		pool_.createBuffers(bufferCount);
> >>>> +
> >>>> +		ret = dev_->exportBuffers(bufferCount, &pool_);
> >>>> +		if (ret)
> >>>> +			return TestFail;
> >>>> +
> >>>> +		ret = output_->importBuffers(&pool_);
> >>>> +		if (ret) {
> >>>> +			std::cerr << "Failed to import buffers" << std::endl;
> >>>> +			return TestFail;
> >>>> +		}
> >>>> +
> >>>> +		return 0;
> >>>> +	}
> >>>> +
> >>>> +	void receiveSourceBuffer(Buffer *buffer)
> >>>> +	{
> >>>> +		std::cout << "Received source buffer: " << buffer->index()
> >>>> +			  << " sequence " << buffer->sequence() << std::endl;
> >>>> +
> >>>> +		output_->queueBuffer(buffer);
> >>>> +		framesCapture++;
> >>>> +	}
> >>>> +
> >>>> +	void receiveDestinationBuffer(Buffer *buffer)
> >>>> +	{
> >>>> +		std::cout << "Received destination buffer: " << buffer->index()
> >>>> +			  << " sequence " << buffer->sequence() << std::endl;
> >>>> +
> >>>> +		dev_->queueBuffer(buffer);
> >>>> +		framesOutput++;
> >>>> +	}
> >>>> +
> >>>> +	int run()
> >>>> +	{
> >>>> +		EventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();
> >>>> +		Timer timeout;
> >>>> +		int ret;
> >>>> +
> >>>> +		dev_->bufferReady.connect(this, &BufferSharingTest::receiveSourceBuffer);
> >>>> +		output_->bufferReady.connect(this, &BufferSharingTest::receiveDestinationBuffer);
> >>>> +
> >>>> +		/* Queue all the buffers to the device. */
> >>>> +		for (Buffer &b : pool_.buffers()) {
> >>>> +			if (dev_->queueBuffer(&b))
> >>>> +				return TestFail;
> >>>> +		}
> >>>> +
> >>>> +		ret = dev_->streamOn();
> >>>> +		if (ret)
> >>>> +			return TestFail;
> >>>> +
> >>>> +		ret = output_->streamOn();
> >>>> +		if (ret)
> >>>> +			return TestFail;
> >>>> +
> >>>> +		timeout.start(5000);
> >>>> +		while (timeout.isRunning())
> >>>> +			dispatcher->processEvents();
> >>>> +
> >>>> +		if ((framesCapture < 1) || (framesOutput < 1)) {
> >>>> +			std::cout << "Failed to process any frames within timeout." << std::endl;
> >>>> +			return TestFail;
> >>>> +		}
> >>>> +
> >>>> +		if ((framesCapture < 30) || (framesOutput < 30)) {
> >>>> +			std::cout << "Failed to process 30 frames within timeout." << std::endl;
> >>>> +			return TestFail;
> >>>> +		}
> >>>> +
> >>>> +		std::cout
> >>>> +			<< "Processed " << framesCapture << " capture frames"
> >>>> +			<< " and " << framesOutput << " output frames"
> >>>> +			<< std::endl;
> >>>> +
> >>>> +		ret = dev_->streamOff();
> >>>> +		if (ret)
> >>>> +			return TestFail;
> >>>> +
> >>>> +		ret = output_->streamOff();
> >>>> +		if (ret)
> >>>> +			return TestFail;
> >>>> +
> >>>> +		return TestPass;
> >>>> +	}
> >>>> +
> >>>> +	void cleanup()
> >>>> +	{
> >>>> +		if (secondMedia_)
> >>>> +			secondMedia_->release();
> >>>> +
> >>>> +		delete output_;
> >>>> +
> >>>> +		V4L2DeviceTest::cleanup();
> >>>> +	}
> >>>> +};
> >>>> +
> >>>> +TEST_REGISTER(BufferSharingTest);
> >>>> diff --git a/test/v4l2_device/meson.build b/test/v4l2_device/meson.build
> >>>> index ec2c7f9f11ff..9f7a7545ac9b 100644
> >>>> --- a/test/v4l2_device/meson.build
> >>>> +++ b/test/v4l2_device/meson.build
> >>>> @@ -5,6 +5,7 @@ v4l2_device_tests = [
> >>>>    [ 'request_buffers',    'request_buffers.cpp' ],
> >>>>    [ 'stream_on_off',      'stream_on_off.cpp' ],
> >>>>    [ 'capture_async',      'capture_async.cpp' ],
> >>>> +  [ 'buffer_sharing',     'buffer_sharing.cpp' ],
> >>>>  ]
> >>>>  
> >>>>  foreach t : v4l2_device_tests

On 2019-02-12 13:11:31 +0200, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Tue, Feb 12, 2019 at 09:47:45AM +0000, Kieran Bingham wrote:
> > On 12/02/2019 09:20, Laurent Pinchart wrote:
> > > On Mon, Feb 11, 2019 at 11:52:35AM +0000, Kieran Bingham wrote:
> > >> On 08/02/2019 17:28, Laurent Pinchart wrote:
> > >>> On Thu, Feb 07, 2019 at 09:21:17PM +0000, Kieran Bingham wrote:
> > >>>> Obtain two V4L2Devices and use one to obtain a BufferPool.
> > >>>>
> > >>>> Propogate the formats from the first to the second device and then commence
> > >>>
> > >>> s/Propogate/Propagate/
> > >>
> > >> Ack.
> > >>
> > >>>> sending buffers between the two devices in a ping-pong fashion.
> > >>>>
> > >>>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > >>>> ---
> > >>>>  test/v4l2_device/buffer_sharing.cpp | 178 ++++++++++++++++++++++++++++
> > >>>>  test/v4l2_device/meson.build        |   1 +
> > >>>>  2 files changed, 179 insertions(+)
> > >>>>  create mode 100644 test/v4l2_device/buffer_sharing.cpp
> > >>>>
> > >>>> diff --git a/test/v4l2_device/buffer_sharing.cpp b/test/v4l2_device/buffer_sharing.cpp
> > >>>> new file mode 100644
> > >>>> index 000000000000..0e96f7b894bd
> > >>>> --- /dev/null
> > >>>> +++ b/test/v4l2_device/buffer_sharing.cpp
> > >>>> @@ -0,0 +1,178 @@
> > >>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > >>>> +/*
> > >>>> + * Copyright (C) 2019, Google Inc.
> > >>>> + *
> > >>>> + * libcamera V4L2 API tests
> > >>>
> > >>> Should this be updated ?
> > >>
> > >> It's part of the V4L2 API tests, no?
> > > 
> > > Sure, but it wouldn't hurt to be a tad more specific :-)
> > > 
> > >>>> + */
> > >>>> +
> > >>>> +#include <iostream>
> > >>>> +
> > >>>> +#include <libcamera/buffer.h>
> > >>>> +#include <libcamera/camera_manager.h>
> > >>>> +#include <libcamera/event_dispatcher.h>
> > >>>> +#include <libcamera/timer.h>
> > >>>> +
> > >>>> +#include "v4l2_device_test.h"
> > >>>> +
> > >>>> +#include "log.h"
> > >>>> +
> > >>>> +LOG_DEFINE_CATEGORY(Test)
> > >>>
> > >>> The logger is internal to libcamera, let's not use it for tests. We
> > >>> should define a test-specific logging infrastructure instead, to suit
> > >>> the specific needs of tests.
> > >>
> > >> Ah yes - I should have dropped this - That part was just me playing
> > >> around with the code.
> > >>
> > >> It works in this instance because this code is an 'internal' test and
> > >> thus has access to all internals including the logging mechanism - but I
> > >> agree the Tests need their own.
> > >>
> > >> Care to write a TestLoggger sometime so we can actually fix all this up?
> > > 
> > > I've added a task for that, let's see who can tackle it first :-)
> > > 
> > >>>> +class BufferSharingTest : public V4L2DeviceTest
> > >>>> +{
> > >>>> +public:
> > >>>> +	BufferSharingTest()
> > >>>> +		: output_(nullptr), framesCapture(0), framesOutput(0){};
> > >>>> +
> > >>>> +private:
> > >>>> +	const unsigned int bufferCount = 4;
> > >>>> +
> > >>>> +	V4L2Device *output_;
> > >>>> +	std::shared_ptr<MediaDevice> secondMedia_;
> > >>>> +
> > >>>> +	unsigned int framesCapture;
> > >>>> +	unsigned int framesOutput;
> > >>>> +
> > >>>> +protected:
> > >>>> +	int init()
> > >>>> +	{
> > >>>> +		int ret = V4L2DeviceTest::init();
> > >>>> +		if (ret)
> > >>>> +			return ret;
> > >>>> +
> > >>>> +		DeviceMatch uvcvideo("uvcvideo");
> > >>>> +		uvcvideo.add("Logitech BRIO");
> > >>>
> > >>> This is *very* specific to your setup, we need a more generic solution.
> > >>> One option is to switch to vivid, as it has both a video capture and a
> > >>> video output device.
> > >>
> > >> Yes, of course it is - as was detailed in the cover letter.
> > >>
> > >> The main purpose of this posting is to support Jacopo in his continued
> > >> development on the IPU3 which needs buffer imports.
> > > 
> > > Do you think it would be lots of work to move this to vivid ?
> > 
> > You know my initial blocker was my inaccurate association of VIVID/VIMC.
> > and VIVID != VIMC which is not packaged by my distro :S
> >  (I should file a bug on Ubuntu)
> > 
> > kbingham@Q:~/projects$ sudo modprobe vimc
> > modprobe: FATAL: Module vimc not found in directory
> 
> :-/ A bug report is indeed needed.
> 
> > /lib/modules/4.18.0-14-generic
> > kbingham@Q:~/projects$ sudo modprobe vivid
> > kbingham@Q:~/projects$ lsv4l2
> > video0: HP Wide Vision FHD Camera: HP W
> > video1: HP Wide Vision FHD Camera: HP W
> > video2: HP Wide Vision FHD Camera: HP I
> > video3: HP Wide Vision FHD Camera: HP I
> > video4: vivid-000-vid-cap
> > video5: vivid-000-vid-out
> > 
> > 
> > So I have vivid - but it does not provide a media device. So we can't
> > enumerate it with our current DeviceMatch...
> > 
> > I hear rumours that Linux v4.20 now provides a media device for VIVID
> > ... should we restrict our tests to v4.20+ or add support for
> > non-media-controller devices?
> 
> I think we shouldn't relax the media controller requirement. The
> vivid-based tests would then be restricted to v4.20+.

I agree, we should not relax the media controller requirement. That 
would IMHO open a can of worms :-)

> 
> > >>>> +		secondMedia_ = std::move(enumerator_->search(uvcvideo));
> > >>>> +		if (!secondMedia_) {
> > >>>> +			LOG(Test, Info) << "No Brio found";
> > >>>> +			return TestSkip;
> > >>>> +		}
> > >>>> +
> > >>>> +		secondMedia_->acquire();
> > >>>> +
> > >>>> +		MediaEntity *entity = secondMedia_->defaultEntity();
> > >>>> +		if (!entity)
> > >>>> +			return TestFail;
> > >>>> +
> > >>>> +		output_ = new V4L2Device(entity);
> > >>>> +		if (!output_)
> > >>>> +			return TestFail;
> > >>>> +
> > >>>> +		ret = output_->open();
> > >>>> +		if (ret)
> > >>>> +			return TestFail;
> > >>>
> > >>> How about adding an openDevice(const DeviceMatch &dm, ...) function to
> > >>> V4L2DeviceTest to do all this, and call it both from
> > >>> V4L2DeviceTest::init() and here ?
> > >>
> > >> This function certainly got long fast. A helper would be useful yes.
> > >>
> > >>>> +		V4L2DeviceFormat format;
> > >>>> +
> > >>>> +		ret = dev_->getFormat(&format);
> > >>>> +		if (ret) {
> > >>>> +			return TestFail;
> > >>>> +		}
> > >>>> +
> > >>>> +		LOG(Test, Info) << "Successfully obtained format from source";
> > >>>> +
> > >>>> +		ret = output_->setFormat(&format);
> > >>>> +		if (ret)
> > >>>> +			return TestFail;
> > >>>> +
> > >>>> +		LOG(Test, Info) << "Successfully set format to output";
> > >>>> +
> > >>>> +		pool_.createBuffers(bufferCount);
> > >>>> +
> > >>>> +		ret = dev_->exportBuffers(bufferCount, &pool_);
> > >>>> +		if (ret)
> > >>>> +			return TestFail;
> > >>>> +
> > >>>> +		ret = output_->importBuffers(&pool_);
> > >>>> +		if (ret) {
> > >>>> +			std::cerr << "Failed to import buffers" << std::endl;
> > >>>> +			return TestFail;
> > >>>> +		}
> > >>>> +
> > >>>> +		return 0;
> > >>>> +	}
> > >>>> +
> > >>>> +	void receiveSourceBuffer(Buffer *buffer)
> > >>>> +	{
> > >>>> +		std::cout << "Received source buffer: " << buffer->index()
> > >>>> +			  << " sequence " << buffer->sequence() << std::endl;
> > >>>> +
> > >>>> +		output_->queueBuffer(buffer);
> > >>>> +		framesCapture++;
> > >>>> +	}
> > >>>> +
> > >>>> +	void receiveDestinationBuffer(Buffer *buffer)
> > >>>> +	{
> > >>>> +		std::cout << "Received destination buffer: " << buffer->index()
> > >>>> +			  << " sequence " << buffer->sequence() << std::endl;
> > >>>> +
> > >>>> +		dev_->queueBuffer(buffer);
> > >>>> +		framesOutput++;
> > >>>> +	}
> > >>>> +
> > >>>> +	int run()
> > >>>> +	{
> > >>>> +		EventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();
> > >>>> +		Timer timeout;
> > >>>> +		int ret;
> > >>>> +
> > >>>> +		dev_->bufferReady.connect(this, &BufferSharingTest::receiveSourceBuffer);
> > >>>> +		output_->bufferReady.connect(this, &BufferSharingTest::receiveDestinationBuffer);
> > >>>> +
> > >>>> +		/* Queue all the buffers to the device. */
> > >>>> +		for (Buffer &b : pool_.buffers()) {
> > >>>> +			if (dev_->queueBuffer(&b))
> > >>>> +				return TestFail;
> > >>>> +		}
> > >>>> +
> > >>>> +		ret = dev_->streamOn();
> > >>>> +		if (ret)
> > >>>> +			return TestFail;
> > >>>> +
> > >>>> +		ret = output_->streamOn();
> > >>>> +		if (ret)
> > >>>> +			return TestFail;
> > >>>> +
> > >>>> +		timeout.start(5000);
> > >>>> +		while (timeout.isRunning())
> > >>>> +			dispatcher->processEvents();
> > >>>> +
> > >>>> +		if ((framesCapture < 1) || (framesOutput < 1)) {
> > >>>> +			std::cout << "Failed to process any frames within timeout." << std::endl;
> > >>>> +			return TestFail;
> > >>>> +		}
> > >>>> +
> > >>>> +		if ((framesCapture < 30) || (framesOutput < 30)) {
> > >>>> +			std::cout << "Failed to process 30 frames within timeout." << std::endl;
> > >>>> +			return TestFail;
> > >>>> +		}
> > >>>> +
> > >>>> +		std::cout
> > >>>> +			<< "Processed " << framesCapture << " capture frames"
> > >>>> +			<< " and " << framesOutput << " output frames"
> > >>>> +			<< std::endl;
> > >>>> +
> > >>>> +		ret = dev_->streamOff();
> > >>>> +		if (ret)
> > >>>> +			return TestFail;
> > >>>> +
> > >>>> +		ret = output_->streamOff();
> > >>>> +		if (ret)
> > >>>> +			return TestFail;
> > >>>> +
> > >>>> +		return TestPass;
> > >>>> +	}
> > >>>> +
> > >>>> +	void cleanup()
> > >>>> +	{
> > >>>> +		if (secondMedia_)
> > >>>> +			secondMedia_->release();
> > >>>> +
> > >>>> +		delete output_;
> > >>>> +
> > >>>> +		V4L2DeviceTest::cleanup();
> > >>>> +	}
> > >>>> +};
> > >>>> +
> > >>>> +TEST_REGISTER(BufferSharingTest);
> > >>>> diff --git a/test/v4l2_device/meson.build b/test/v4l2_device/meson.build
> > >>>> index ec2c7f9f11ff..9f7a7545ac9b 100644
> > >>>> --- a/test/v4l2_device/meson.build
> > >>>> +++ b/test/v4l2_device/meson.build
> > >>>> @@ -5,6 +5,7 @@ v4l2_device_tests = [
> > >>>>    [ 'request_buffers',    'request_buffers.cpp' ],
> > >>>>    [ 'stream_on_off',      'stream_on_off.cpp' ],
> > >>>>    [ 'capture_async',      'capture_async.cpp' ],
> > >>>> +  [ 'buffer_sharing',     'buffer_sharing.cpp' ],
> > >>>>  ]
> > >>>>  
> > >>>>  foreach t : v4l2_device_tests
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel