[libcamera-devel,v2,3/4] test: v4l2_videodevice: Add test for V4L2BufferCache

2877 diff mbox series

Add test to test the different modes and situations the V4L2BufferCache
can be put in. The tests verify that a FrameBuffer used with the cache
results in a V4L2 video device index, and that the cache implementation
is capable of keeping buffers in a hot state.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
* Changes since v1
- Update comments in code.
- Use std::mt19937 PRNG instead of rand()
- Print randomize and print std::mt19937 initial seed to be able to
  reproduce a test run with the same random sequences.
- Add const std::vector<std::unique_ptr<FrameBuffer>> &buffers "alias"
  for source.buffers() to make code more readable.
- Make use of libtest common implementation of BufferSource.
---
 test/v4l2_videodevice/buffer_cache.cpp | 215 +++++++++++++++++++++++++
 test/v4l2_videodevice/meson.build      |   1 +
 2 files changed, 216 insertions(+)
 create mode 100644 test/v4l2_videodevice/buffer_cache.cpp

Hi Niklas,

Thank you for the patch.

On Mon, Feb 24, 2020 at 08:36:00PM +0100, Niklas Söderlund wrote:
> Add test to test the different modes and situations the V4L2BufferCache
> can be put in. The tests verify that a FrameBuffer used with the cache
> results in a V4L2 video device index, and that the cache implementation
> is capable of keeping buffers in a hot state.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
> * Changes since v1
> - Update comments in code.
> - Use std::mt19937 PRNG instead of rand()
> - Print randomize and print std::mt19937 initial seed to be able to
>   reproduce a test run with the same random sequences.
> - Add const std::vector<std::unique_ptr<FrameBuffer>> &buffers "alias"
>   for source.buffers() to make code more readable.
> - Make use of libtest common implementation of BufferSource.
> ---
>  test/v4l2_videodevice/buffer_cache.cpp | 215 +++++++++++++++++++++++++
>  test/v4l2_videodevice/meson.build      |   1 +
>  2 files changed, 216 insertions(+)
>  create mode 100644 test/v4l2_videodevice/buffer_cache.cpp
> 
> diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp
> new file mode 100644
> index 0000000000000000..0a8cb0d28ca9b204
> --- /dev/null
> +++ b/test/v4l2_videodevice/buffer_cache.cpp
> @@ -0,0 +1,215 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * Test the buffer cache different operation modes
> + */
> +
> +#include <iostream>
> +#include <random>
> +#include <vector>
> +
> +#include <libcamera/stream.h>
> +
> +#include "buffer_source.h"
> +
> +#include "test.h"
> +
> +using namespace libcamera;
> +
> +namespace {
> +
> +class BufferCacheTest : public Test
> +{
> +public:
> +	/*
> +	 * Test that a cache with the same size as there are buffers results in
> +	 * a sequential run over; 0, 1, 2, 3, 4, 0, 1, 2, 3, 4, ...
> +	 *
> +	 * The test is only valid when the cache size is as least as big as the
> +	 * number of buffers.
> +	 */
> +	int testSequential(V4L2BufferCache *cache,
> +			   const std::vector<std::unique_ptr<FrameBuffer>> &buffers)
> +	{
> +		for (unsigned int i = 0; i < buffers.size() * 100; i++) {
> +			int nBuffer = i % buffers.size();
> +			int index = cache->get(*buffers[nBuffer].get());
> +
> +			if (index != nBuffer) {
> +				std::cout << "Expected index " << nBuffer
> +					  << " got " << index << std::endl;
> +				return TestFail;
> +			}
> +
> +			cache->put(index);
> +		}
> +
> +		return TestPass;
> +	}
> +
> +	/*
> +	 * Test that randomly putting buffers to the cache always results in a
> +	 * valid index.
> +	 */
> +	int testRandom(V4L2BufferCache *cache,
> +		       const std::vector<std::unique_ptr<FrameBuffer>> &buffers)
> +	{
> +		std::uniform_int_distribution<> dist(0, buffers.size() - 1);
> +
> +		for (unsigned int i = 0; i < buffers.size() * 100; i++) {
> +			int nBuffer = dist(generator_);
> +			int index = cache->get(*buffers[nBuffer].get());
> +
> +			if (index < 0) {
> +				std::cout << "Failed lookup from cache"
> +					  << std::endl;
> +				return TestFail;
> +			}
> +
> +			cache->put(index);
> +		}
> +
> +		return TestPass;
> +	}
> +
> +	/*
> +	 * Test that using a buffer more frequently keeps it hot in the cache at
> +	 * all times.
> +	 */
> +	int testHot(V4L2BufferCache *cache,
> +		    const std::vector<std::unique_ptr<FrameBuffer>> &buffers,
> +		    unsigned int hotFrequency)
> +	{
> +		/* Run the random test on the cache to make it messy. */
> +		if (testRandom(cache, buffers) != TestPass)
> +			return TestFail;
> +
> +		std::uniform_int_distribution<> dist(0, buffers.size() - 1);
> +
> +		/* Pick a hot buffer at random and store its index. */
> +		int hotBuffer = dist(generator_);
> +		int hotIndex = cache->get(*buffers[hotBuffer].get());
> +		cache->put(hotIndex);
> +
> +		/*
> +		 * Queue hot buffer at the requested frequency and make sure
> +		 * it stays hot.
> +		 */
> +		for (unsigned int i = 0; i < buffers.size() * 100; i++) {
> +			int nBuffer, index;
> +			bool hotQueue = i % hotFrequency == 0;
> +
> +			if (hotQueue)
> +				nBuffer = hotBuffer;
> +			else
> +				nBuffer = dist(generator_);
> +
> +			index = cache->get(*buffers[nBuffer].get());
> +
> +			if (index < 0) {
> +				std::cout << "Failed lookup from cache"
> +					  << std::endl;
> +				return TestFail;
> +			}
> +
> +			if (hotQueue && index != hotIndex) {
> +				std::cout << "Hot buffer got cold"
> +					  << std::endl;
> +				return TestFail;
> +			}
> +
> +			cache->put(index);
> +		}
> +
> +		return TestPass;
> +	}
> +
> +	int init() override
> +	{
> +		std::random_device rd;
> +		unsigned int seed = rd();
> +
> +		std::cout << "Random seed is " << seed << std::endl;
> +
> +		generator_.seed(seed);

As reported by Kieran, using real randomness in our tests makes them
non-reproducible. Should we leave the generate unseeded ?

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

> +
> +		return TestPass;
> +	}
> +
> +	int run() override
> +	{
> +		const unsigned int numBuffers = 8;
> +
> +		StreamConfiguration cfg;
> +		cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> +		cfg.size = Size(600, 800);
> +		cfg.bufferCount = numBuffers;
> +
> +		BufferSource source;
> +		int ret = source.allocate(cfg);
> +		if (ret != TestPass)
> +			return ret;
> +
> +		const std::vector<std::unique_ptr<FrameBuffer>> &buffers =
> +			source.buffers();
> +
> +		if (buffers.size() != numBuffers) {
> +			std::cout << "Got " << buffers.size()
> +				  << " buffers, expected " << numBuffers
> +				  << std::endl;
> +			return TestFail;
> +		}
> +
> +		/*
> +		 * Test cache of same size as there are buffers, the cache is
> +		 * created from a list of buffers and will be pre-populated.
> +		 */
> +		V4L2BufferCache cacheFromBuffers(buffers);
> +
> +		if (testSequential(&cacheFromBuffers, buffers) != TestPass)
> +			return TestFail;
> +
> +		if (testRandom(&cacheFromBuffers, buffers) != TestPass)
> +			return TestFail;
> +
> +		if (testHot(&cacheFromBuffers, buffers, numBuffers) != TestPass)
> +			return TestFail;
> +
> +		/*
> +		 * Test cache of same size as there are buffers, the cache is
> +		 * not pre-populated.
> +		 */
> +		V4L2BufferCache cacheFromNumbers(numBuffers);
> +
> +		if (testSequential(&cacheFromNumbers, buffers) != TestPass)
> +			return TestFail;
> +
> +		if (testRandom(&cacheFromNumbers, buffers) != TestPass)
> +			return TestFail;
> +
> +		if (testHot(&cacheFromNumbers, buffers, numBuffers) != TestPass)
> +			return TestFail;
> +
> +		/*
> +		 * Test cache half the size of number of buffers used, the cache
> +		 * is not pre-populated.
> +		 */
> +		V4L2BufferCache cacheHalf(numBuffers / 2);
> +
> +		if (testRandom(&cacheHalf, buffers) != TestPass)
> +			return TestFail;
> +
> +		if (testHot(&cacheHalf, buffers, numBuffers / 2) != TestPass)
> +			return TestFail;
> +
> +		return TestPass;
> +	}
> +
> +private:
> +	std::mt19937 generator_;
> +};
> +
> +} /* namespace */
> +
> +TEST_REGISTER(BufferCacheTest);
> diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build
> index 5c52da7219c21cc3..685fcf6d16d72182 100644
> --- a/test/v4l2_videodevice/meson.build
> +++ b/test/v4l2_videodevice/meson.build
> @@ -5,6 +5,7 @@ v4l2_videodevice_tests = [
>      [ 'controls',           'controls.cpp' ],
>      [ 'formats',            'formats.cpp' ],
>      [ 'request_buffers',    'request_buffers.cpp' ],
> +    [ 'buffer_cache',       'buffer_cache.cpp' ],
>      [ 'stream_on_off',      'stream_on_off.cpp' ],
>      [ 'capture_async',      'capture_async.cpp' ],
>      [ 'buffer_sharing',     'buffer_sharing.cpp' ],

Hi Laurent,

Thanks for your feedback.

On 2020-02-29 18:16:40 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Mon, Feb 24, 2020 at 08:36:00PM +0100, Niklas Söderlund wrote:
> > Add test to test the different modes and situations the V4L2BufferCache
> > can be put in. The tests verify that a FrameBuffer used with the cache
> > results in a V4L2 video device index, and that the cache implementation
> > is capable of keeping buffers in a hot state.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> > * Changes since v1
> > - Update comments in code.
> > - Use std::mt19937 PRNG instead of rand()
> > - Print randomize and print std::mt19937 initial seed to be able to
> >   reproduce a test run with the same random sequences.
> > - Add const std::vector<std::unique_ptr<FrameBuffer>> &buffers "alias"
> >   for source.buffers() to make code more readable.
> > - Make use of libtest common implementation of BufferSource.
> > ---
> >  test/v4l2_videodevice/buffer_cache.cpp | 215 +++++++++++++++++++++++++
> >  test/v4l2_videodevice/meson.build      |   1 +
> >  2 files changed, 216 insertions(+)
> >  create mode 100644 test/v4l2_videodevice/buffer_cache.cpp
> > 
> > diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp
> > new file mode 100644
> > index 0000000000000000..0a8cb0d28ca9b204
> > --- /dev/null
> > +++ b/test/v4l2_videodevice/buffer_cache.cpp
> > @@ -0,0 +1,215 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * Test the buffer cache different operation modes
> > + */
> > +
> > +#include <iostream>
> > +#include <random>
> > +#include <vector>
> > +
> > +#include <libcamera/stream.h>
> > +
> > +#include "buffer_source.h"
> > +
> > +#include "test.h"
> > +
> > +using namespace libcamera;
> > +
> > +namespace {
> > +
> > +class BufferCacheTest : public Test
> > +{
> > +public:
> > +	/*
> > +	 * Test that a cache with the same size as there are buffers results in
> > +	 * a sequential run over; 0, 1, 2, 3, 4, 0, 1, 2, 3, 4, ...
> > +	 *
> > +	 * The test is only valid when the cache size is as least as big as the
> > +	 * number of buffers.
> > +	 */
> > +	int testSequential(V4L2BufferCache *cache,
> > +			   const std::vector<std::unique_ptr<FrameBuffer>> &buffers)
> > +	{
> > +		for (unsigned int i = 0; i < buffers.size() * 100; i++) {
> > +			int nBuffer = i % buffers.size();
> > +			int index = cache->get(*buffers[nBuffer].get());
> > +
> > +			if (index != nBuffer) {
> > +				std::cout << "Expected index " << nBuffer
> > +					  << " got " << index << std::endl;
> > +				return TestFail;
> > +			}
> > +
> > +			cache->put(index);
> > +		}
> > +
> > +		return TestPass;
> > +	}
> > +
> > +	/*
> > +	 * Test that randomly putting buffers to the cache always results in a
> > +	 * valid index.
> > +	 */
> > +	int testRandom(V4L2BufferCache *cache,
> > +		       const std::vector<std::unique_ptr<FrameBuffer>> &buffers)
> > +	{
> > +		std::uniform_int_distribution<> dist(0, buffers.size() - 1);
> > +
> > +		for (unsigned int i = 0; i < buffers.size() * 100; i++) {
> > +			int nBuffer = dist(generator_);
> > +			int index = cache->get(*buffers[nBuffer].get());
> > +
> > +			if (index < 0) {
> > +				std::cout << "Failed lookup from cache"
> > +					  << std::endl;
> > +				return TestFail;
> > +			}
> > +
> > +			cache->put(index);
> > +		}
> > +
> > +		return TestPass;
> > +	}
> > +
> > +	/*
> > +	 * Test that using a buffer more frequently keeps it hot in the cache at
> > +	 * all times.
> > +	 */
> > +	int testHot(V4L2BufferCache *cache,
> > +		    const std::vector<std::unique_ptr<FrameBuffer>> &buffers,
> > +		    unsigned int hotFrequency)
> > +	{
> > +		/* Run the random test on the cache to make it messy. */
> > +		if (testRandom(cache, buffers) != TestPass)
> > +			return TestFail;
> > +
> > +		std::uniform_int_distribution<> dist(0, buffers.size() - 1);
> > +
> > +		/* Pick a hot buffer at random and store its index. */
> > +		int hotBuffer = dist(generator_);
> > +		int hotIndex = cache->get(*buffers[hotBuffer].get());
> > +		cache->put(hotIndex);
> > +
> > +		/*
> > +		 * Queue hot buffer at the requested frequency and make sure
> > +		 * it stays hot.
> > +		 */
> > +		for (unsigned int i = 0; i < buffers.size() * 100; i++) {
> > +			int nBuffer, index;
> > +			bool hotQueue = i % hotFrequency == 0;
> > +
> > +			if (hotQueue)
> > +				nBuffer = hotBuffer;
> > +			else
> > +				nBuffer = dist(generator_);
> > +
> > +			index = cache->get(*buffers[nBuffer].get());
> > +
> > +			if (index < 0) {
> > +				std::cout << "Failed lookup from cache"
> > +					  << std::endl;
> > +				return TestFail;
> > +			}
> > +
> > +			if (hotQueue && index != hotIndex) {
> > +				std::cout << "Hot buffer got cold"
> > +					  << std::endl;
> > +				return TestFail;
> > +			}
> > +
> > +			cache->put(index);
> > +		}
> > +
> > +		return TestPass;
> > +	}
> > +
> > +	int init() override
> > +	{
> > +		std::random_device rd;
> > +		unsigned int seed = rd();
> > +
> > +		std::cout << "Random seed is " << seed << std::endl;
> > +
> > +		generator_.seed(seed);
> 
> As reported by Kieran, using real randomness in our tests makes them
> non-reproducible. Should we leave the generate unseeded ?

I like to keep them random, at least this one as I think it adds value.  

However they are reproducible, I log the seed value and if we wish to 
reproduce a test that fails we can inject the seed from the failed run 
into generator_.seed(<seed value from log>) and it will generate the 
same sequence as for the failed test.

On a side topic, if we want truly reproducible tests we should also log 
kernel version with checksum of build commit as subtle driver changes 
might be the reason a test fails/pass.

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > +
> > +		return TestPass;
> > +	}
> > +
> > +	int run() override
> > +	{
> > +		const unsigned int numBuffers = 8;
> > +
> > +		StreamConfiguration cfg;
> > +		cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> > +		cfg.size = Size(600, 800);
> > +		cfg.bufferCount = numBuffers;
> > +
> > +		BufferSource source;
> > +		int ret = source.allocate(cfg);
> > +		if (ret != TestPass)
> > +			return ret;
> > +
> > +		const std::vector<std::unique_ptr<FrameBuffer>> &buffers =
> > +			source.buffers();
> > +
> > +		if (buffers.size() != numBuffers) {
> > +			std::cout << "Got " << buffers.size()
> > +				  << " buffers, expected " << numBuffers
> > +				  << std::endl;
> > +			return TestFail;
> > +		}
> > +
> > +		/*
> > +		 * Test cache of same size as there are buffers, the cache is
> > +		 * created from a list of buffers and will be pre-populated.
> > +		 */
> > +		V4L2BufferCache cacheFromBuffers(buffers);
> > +
> > +		if (testSequential(&cacheFromBuffers, buffers) != TestPass)
> > +			return TestFail;
> > +
> > +		if (testRandom(&cacheFromBuffers, buffers) != TestPass)
> > +			return TestFail;
> > +
> > +		if (testHot(&cacheFromBuffers, buffers, numBuffers) != TestPass)
> > +			return TestFail;
> > +
> > +		/*
> > +		 * Test cache of same size as there are buffers, the cache is
> > +		 * not pre-populated.
> > +		 */
> > +		V4L2BufferCache cacheFromNumbers(numBuffers);
> > +
> > +		if (testSequential(&cacheFromNumbers, buffers) != TestPass)
> > +			return TestFail;
> > +
> > +		if (testRandom(&cacheFromNumbers, buffers) != TestPass)
> > +			return TestFail;
> > +
> > +		if (testHot(&cacheFromNumbers, buffers, numBuffers) != TestPass)
> > +			return TestFail;
> > +
> > +		/*
> > +		 * Test cache half the size of number of buffers used, the cache
> > +		 * is not pre-populated.
> > +		 */
> > +		V4L2BufferCache cacheHalf(numBuffers / 2);
> > +
> > +		if (testRandom(&cacheHalf, buffers) != TestPass)
> > +			return TestFail;
> > +
> > +		if (testHot(&cacheHalf, buffers, numBuffers / 2) != TestPass)
> > +			return TestFail;
> > +
> > +		return TestPass;
> > +	}
> > +
> > +private:
> > +	std::mt19937 generator_;
> > +};
> > +
> > +} /* namespace */
> > +
> > +TEST_REGISTER(BufferCacheTest);
> > diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build
> > index 5c52da7219c21cc3..685fcf6d16d72182 100644
> > --- a/test/v4l2_videodevice/meson.build
> > +++ b/test/v4l2_videodevice/meson.build
> > @@ -5,6 +5,7 @@ v4l2_videodevice_tests = [
> >      [ 'controls',           'controls.cpp' ],
> >      [ 'formats',            'formats.cpp' ],
> >      [ 'request_buffers',    'request_buffers.cpp' ],
> > +    [ 'buffer_cache',       'buffer_cache.cpp' ],
> >      [ 'stream_on_off',      'stream_on_off.cpp' ],
> >      [ 'capture_async',      'capture_async.cpp' ],
> >      [ 'buffer_sharing',     'buffer_sharing.cpp' ],
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Hi Niklas,

On Wed, Mar 04, 2020 at 10:46:03PM +0100, Niklas Söderlund wrote:
> On 2020-02-29 18:16:40 +0200, Laurent Pinchart wrote:
> > On Mon, Feb 24, 2020 at 08:36:00PM +0100, Niklas Söderlund wrote:
> >> Add test to test the different modes and situations the V4L2BufferCache
> >> can be put in. The tests verify that a FrameBuffer used with the cache
> >> results in a V4L2 video device index, and that the cache implementation
> >> is capable of keeping buffers in a hot state.
> >> 
> >> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> >> ---
> >> * Changes since v1
> >> - Update comments in code.
> >> - Use std::mt19937 PRNG instead of rand()
> >> - Print randomize and print std::mt19937 initial seed to be able to
> >>   reproduce a test run with the same random sequences.
> >> - Add const std::vector<std::unique_ptr<FrameBuffer>> &buffers "alias"
> >>   for source.buffers() to make code more readable.
> >> - Make use of libtest common implementation of BufferSource.
> >> ---
> >>  test/v4l2_videodevice/buffer_cache.cpp | 215 +++++++++++++++++++++++++
> >>  test/v4l2_videodevice/meson.build      |   1 +
> >>  2 files changed, 216 insertions(+)
> >>  create mode 100644 test/v4l2_videodevice/buffer_cache.cpp
> >> 
> >> diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp
> >> new file mode 100644
> >> index 0000000000000000..0a8cb0d28ca9b204
> >> --- /dev/null
> >> +++ b/test/v4l2_videodevice/buffer_cache.cpp
> >> @@ -0,0 +1,215 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >> +/*
> >> + * Copyright (C) 2020, Google Inc.
> >> + *
> >> + * Test the buffer cache different operation modes
> >> + */
> >> +
> >> +#include <iostream>
> >> +#include <random>
> >> +#include <vector>
> >> +
> >> +#include <libcamera/stream.h>
> >> +
> >> +#include "buffer_source.h"
> >> +
> >> +#include "test.h"
> >> +
> >> +using namespace libcamera;
> >> +
> >> +namespace {
> >> +
> >> +class BufferCacheTest : public Test
> >> +{
> >> +public:
> >> +	/*
> >> +	 * Test that a cache with the same size as there are buffers results in
> >> +	 * a sequential run over; 0, 1, 2, 3, 4, 0, 1, 2, 3, 4, ...
> >> +	 *
> >> +	 * The test is only valid when the cache size is as least as big as the
> >> +	 * number of buffers.
> >> +	 */
> >> +	int testSequential(V4L2BufferCache *cache,
> >> +			   const std::vector<std::unique_ptr<FrameBuffer>> &buffers)
> >> +	{
> >> +		for (unsigned int i = 0; i < buffers.size() * 100; i++) {
> >> +			int nBuffer = i % buffers.size();
> >> +			int index = cache->get(*buffers[nBuffer].get());
> >> +
> >> +			if (index != nBuffer) {
> >> +				std::cout << "Expected index " << nBuffer
> >> +					  << " got " << index << std::endl;
> >> +				return TestFail;
> >> +			}
> >> +
> >> +			cache->put(index);
> >> +		}
> >> +
> >> +		return TestPass;
> >> +	}
> >> +
> >> +	/*
> >> +	 * Test that randomly putting buffers to the cache always results in a
> >> +	 * valid index.
> >> +	 */
> >> +	int testRandom(V4L2BufferCache *cache,
> >> +		       const std::vector<std::unique_ptr<FrameBuffer>> &buffers)
> >> +	{
> >> +		std::uniform_int_distribution<> dist(0, buffers.size() - 1);
> >> +
> >> +		for (unsigned int i = 0; i < buffers.size() * 100; i++) {
> >> +			int nBuffer = dist(generator_);
> >> +			int index = cache->get(*buffers[nBuffer].get());
> >> +
> >> +			if (index < 0) {
> >> +				std::cout << "Failed lookup from cache"
> >> +					  << std::endl;
> >> +				return TestFail;
> >> +			}
> >> +
> >> +			cache->put(index);
> >> +		}
> >> +
> >> +		return TestPass;
> >> +	}
> >> +
> >> +	/*
> >> +	 * Test that using a buffer more frequently keeps it hot in the cache at
> >> +	 * all times.
> >> +	 */
> >> +	int testHot(V4L2BufferCache *cache,
> >> +		    const std::vector<std::unique_ptr<FrameBuffer>> &buffers,
> >> +		    unsigned int hotFrequency)
> >> +	{
> >> +		/* Run the random test on the cache to make it messy. */
> >> +		if (testRandom(cache, buffers) != TestPass)
> >> +			return TestFail;
> >> +
> >> +		std::uniform_int_distribution<> dist(0, buffers.size() - 1);
> >> +
> >> +		/* Pick a hot buffer at random and store its index. */
> >> +		int hotBuffer = dist(generator_);
> >> +		int hotIndex = cache->get(*buffers[hotBuffer].get());
> >> +		cache->put(hotIndex);
> >> +
> >> +		/*
> >> +		 * Queue hot buffer at the requested frequency and make sure
> >> +		 * it stays hot.
> >> +		 */
> >> +		for (unsigned int i = 0; i < buffers.size() * 100; i++) {
> >> +			int nBuffer, index;
> >> +			bool hotQueue = i % hotFrequency == 0;
> >> +
> >> +			if (hotQueue)
> >> +				nBuffer = hotBuffer;
> >> +			else
> >> +				nBuffer = dist(generator_);
> >> +
> >> +			index = cache->get(*buffers[nBuffer].get());
> >> +
> >> +			if (index < 0) {
> >> +				std::cout << "Failed lookup from cache"
> >> +					  << std::endl;
> >> +				return TestFail;
> >> +			}
> >> +
> >> +			if (hotQueue && index != hotIndex) {
> >> +				std::cout << "Hot buffer got cold"
> >> +					  << std::endl;
> >> +				return TestFail;
> >> +			}
> >> +
> >> +			cache->put(index);
> >> +		}
> >> +
> >> +		return TestPass;
> >> +	}
> >> +
> >> +	int init() override
> >> +	{
> >> +		std::random_device rd;
> >> +		unsigned int seed = rd();
> >> +
> >> +		std::cout << "Random seed is " << seed << std::endl;
> >> +
> >> +		generator_.seed(seed);
> > 
> > As reported by Kieran, using real randomness in our tests makes them
> > non-reproducible. Should we leave the generate unseeded ?
> 
> I like to keep them random, at least this one as I think it adds value.  

If we really think it does, then I'd say that increasing the number of
samples would probably add more value :-)

> However they are reproducible, I log the seed value and if we wish to 
> reproduce a test that fails we can inject the seed from the failed run 
> into generator_.seed(<seed value from log>) and it will generate the 
> same sequence as for the failed test.
> 
> On a side topic, if we want truly reproducible tests we should also log 
> kernel version with checksum of build commit as subtle driver changes 
> might be the reason a test fails/pass.

Reproducible tests mean that the same test should produce the same
result for the same build of libcamera on the same system. If you run
the rkisp1 test on a RK3399 and on an Intel platform, I expected
different results :-)

This being said, I think it makes sense to log platform information,
yes.

> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> >> +
> >> +		return TestPass;
> >> +	}
> >> +
> >> +	int run() override
> >> +	{
> >> +		const unsigned int numBuffers = 8;
> >> +
> >> +		StreamConfiguration cfg;
> >> +		cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> >> +		cfg.size = Size(600, 800);
> >> +		cfg.bufferCount = numBuffers;
> >> +
> >> +		BufferSource source;
> >> +		int ret = source.allocate(cfg);
> >> +		if (ret != TestPass)
> >> +			return ret;
> >> +
> >> +		const std::vector<std::unique_ptr<FrameBuffer>> &buffers =
> >> +			source.buffers();
> >> +
> >> +		if (buffers.size() != numBuffers) {
> >> +			std::cout << "Got " << buffers.size()
> >> +				  << " buffers, expected " << numBuffers
> >> +				  << std::endl;
> >> +			return TestFail;
> >> +		}
> >> +
> >> +		/*
> >> +		 * Test cache of same size as there are buffers, the cache is
> >> +		 * created from a list of buffers and will be pre-populated.
> >> +		 */
> >> +		V4L2BufferCache cacheFromBuffers(buffers);
> >> +
> >> +		if (testSequential(&cacheFromBuffers, buffers) != TestPass)
> >> +			return TestFail;
> >> +
> >> +		if (testRandom(&cacheFromBuffers, buffers) != TestPass)
> >> +			return TestFail;
> >> +
> >> +		if (testHot(&cacheFromBuffers, buffers, numBuffers) != TestPass)
> >> +			return TestFail;
> >> +
> >> +		/*
> >> +		 * Test cache of same size as there are buffers, the cache is
> >> +		 * not pre-populated.
> >> +		 */
> >> +		V4L2BufferCache cacheFromNumbers(numBuffers);
> >> +
> >> +		if (testSequential(&cacheFromNumbers, buffers) != TestPass)
> >> +			return TestFail;
> >> +
> >> +		if (testRandom(&cacheFromNumbers, buffers) != TestPass)
> >> +			return TestFail;
> >> +
> >> +		if (testHot(&cacheFromNumbers, buffers, numBuffers) != TestPass)
> >> +			return TestFail;
> >> +
> >> +		/*
> >> +		 * Test cache half the size of number of buffers used, the cache
> >> +		 * is not pre-populated.
> >> +		 */
> >> +		V4L2BufferCache cacheHalf(numBuffers / 2);
> >> +
> >> +		if (testRandom(&cacheHalf, buffers) != TestPass)
> >> +			return TestFail;
> >> +
> >> +		if (testHot(&cacheHalf, buffers, numBuffers / 2) != TestPass)
> >> +			return TestFail;
> >> +
> >> +		return TestPass;
> >> +	}
> >> +
> >> +private:
> >> +	std::mt19937 generator_;
> >> +};
> >> +
> >> +} /* namespace */
> >> +
> >> +TEST_REGISTER(BufferCacheTest);
> >> diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build
> >> index 5c52da7219c21cc3..685fcf6d16d72182 100644
> >> --- a/test/v4l2_videodevice/meson.build
> >> +++ b/test/v4l2_videodevice/meson.build
> >> @@ -5,6 +5,7 @@ v4l2_videodevice_tests = [
> >>      [ 'controls',           'controls.cpp' ],
> >>      [ 'formats',            'formats.cpp' ],
> >>      [ 'request_buffers',    'request_buffers.cpp' ],
> >> +    [ 'buffer_cache',       'buffer_cache.cpp' ],
> >>      [ 'stream_on_off',      'stream_on_off.cpp' ],
> >>      [ 'capture_async',      'capture_async.cpp' ],
> >>      [ 'buffer_sharing',     'buffer_sharing.cpp' ],