[libcamera-devel] [PATCH v2 3/4] test: v4l2_videodevice: Add test for V4L2BufferCache
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Mar 4 23:39:39 CET 2020
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 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.
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 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
More information about the libcamera-devel
mailing list