[libcamera-devel] [PATCH v2 3/4] test: v4l2_videodevice: Add test for V4L2BufferCache
Niklas Söderlund
niklas.soderlund at ragnatech.se
Wed Mar 4 22:46:03 CET 2020
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 at 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 at 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
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list