[{"id":3888,"web_url":"https://patchwork.libcamera.org/comment/3888/","msgid":"<20200229161640.GE18738@pendragon.ideasonboard.com>","date":"2020-02-29T16:16:40","subject":"Re: [libcamera-devel] [PATCH v2 3/4] test: v4l2_videodevice: Add\n\ttest for V4L2BufferCache","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Mon, Feb 24, 2020 at 08:36:00PM +0100, Niklas Söderlund wrote:\n> Add test to test the different modes and situations the V4L2BufferCache\n> can be put in. The tests verify that a FrameBuffer used with the cache\n> results in a V4L2 video device index, and that the cache implementation\n> is capable of keeping buffers in a hot state.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> * Changes since v1\n> - Update comments in code.\n> - Use std::mt19937 PRNG instead of rand()\n> - Print randomize and print std::mt19937 initial seed to be able to\n> reproduce a test run with the same random sequences.\n> - Add const std::vector> &buffers \"alias\"\n> for source.buffers() to make code more readable.\n> - Make use of libtest common implementation of BufferSource.\n> ---\n> test/v4l2_videodevice/buffer_cache.cpp | 215 +++++++++++++++++++++++++\n> test/v4l2_videodevice/meson.build | 1 +\n> 2 files changed, 216 insertions(+)\n> create mode 100644 test/v4l2_videodevice/buffer_cache.cpp\n> \n> diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp\n> new file mode 100644\n> index 0000000000000000..0a8cb0d28ca9b204\n> --- /dev/null\n> +++ b/test/v4l2_videodevice/buffer_cache.cpp\n> @@ -0,0 +1,215 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * Test the buffer cache different operation modes\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"buffer_source.h\"\n> +\n> +#include \"test.h\"\n> +\n> +using namespace libcamera;\n> +\n> +namespace {\n> +\n> +class BufferCacheTest : public Test\n> +{\n> +public:\n> +\t/*\n> +\t * Test that a cache with the same size as there are buffers results in\n> +\t * a sequential run over; 0, 1, 2, 3, 4, 0, 1, 2, 3, 4, ...\n> +\t *\n> +\t * The test is only valid when the cache size is as least as big as the\n> +\t * number of buffers.\n> +\t */\n> +\tint testSequential(V4L2BufferCache *cache,\n> +\t\t\t const std::vector> &buffers)\n> +\t{\n> +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> +\t\t\tint nBuffer = i % buffers.size();\n> +\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> +\n> +\t\t\tif (index != nBuffer) {\n> +\t\t\t\tstd::cout << \"Expected index \" << nBuffer\n> +\t\t\t\t\t << \" got \" << index << std::endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\n> +\t\t\tcache->put(index);\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\t/*\n> +\t * Test that randomly putting buffers to the cache always results in a\n> +\t * valid index.\n> +\t */\n> +\tint testRandom(V4L2BufferCache *cache,\n> +\t\t const std::vector> &buffers)\n> +\t{\n> +\t\tstd::uniform_int_distribution<> dist(0, buffers.size() - 1);\n> +\n> +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> +\t\t\tint nBuffer = dist(generator_);\n> +\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> +\n> +\t\t\tif (index < 0) {\n> +\t\t\t\tstd::cout << \"Failed lookup from cache\"\n> +\t\t\t\t\t << std::endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\n> +\t\t\tcache->put(index);\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\t/*\n> +\t * Test that using a buffer more frequently keeps it hot in the cache at\n> +\t * all times.\n> +\t */\n> +\tint testHot(V4L2BufferCache *cache,\n> +\t\t const std::vector> &buffers,\n> +\t\t unsigned int hotFrequency)\n> +\t{\n> +\t\t/* Run the random test on the cache to make it messy. */\n> +\t\tif (testRandom(cache, buffers) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tstd::uniform_int_distribution<> dist(0, buffers.size() - 1);\n> +\n> +\t\t/* Pick a hot buffer at random and store its index. */\n> +\t\tint hotBuffer = dist(generator_);\n> +\t\tint hotIndex = cache->get(*buffers[hotBuffer].get());\n> +\t\tcache->put(hotIndex);\n> +\n> +\t\t/*\n> +\t\t * Queue hot buffer at the requested frequency and make sure\n> +\t\t * it stays hot.\n> +\t\t */\n> +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> +\t\t\tint nBuffer, index;\n> +\t\t\tbool hotQueue = i % hotFrequency == 0;\n> +\n> +\t\t\tif (hotQueue)\n> +\t\t\t\tnBuffer = hotBuffer;\n> +\t\t\telse\n> +\t\t\t\tnBuffer = dist(generator_);\n> +\n> +\t\t\tindex = cache->get(*buffers[nBuffer].get());\n> +\n> +\t\t\tif (index < 0) {\n> +\t\t\t\tstd::cout << \"Failed lookup from cache\"\n> +\t\t\t\t\t << std::endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\n> +\t\t\tif (hotQueue && index != hotIndex) {\n> +\t\t\t\tstd::cout << \"Hot buffer got cold\"\n> +\t\t\t\t\t << std::endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\n> +\t\t\tcache->put(index);\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint init() override\n> +\t{\n> +\t\tstd::random_device rd;\n> +\t\tunsigned int seed = rd();\n> +\n> +\t\tstd::cout << \"Random seed is \" << seed << std::endl;\n> +\n> +\t\tgenerator_.seed(seed);\n\nAs reported by Kieran, using real randomness in our tests makes them\nnon-reproducible. Should we leave the generate unseeded ?\n\nReviewed-by: Laurent Pinchart \n\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint run() override\n> +\t{\n> +\t\tconst unsigned int numBuffers = 8;\n> +\n> +\t\tStreamConfiguration cfg;\n> +\t\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> +\t\tcfg.size = Size(600, 800);\n> +\t\tcfg.bufferCount = numBuffers;\n> +\n> +\t\tBufferSource source;\n> +\t\tint ret = source.allocate(cfg);\n> +\t\tif (ret != TestPass)\n> +\t\t\treturn ret;\n> +\n> +\t\tconst std::vector> &buffers =\n> +\t\t\tsource.buffers();\n> +\n> +\t\tif (buffers.size() != numBuffers) {\n> +\t\t\tstd::cout << \"Got \" << buffers.size()\n> +\t\t\t\t << \" buffers, expected \" << numBuffers\n> +\t\t\t\t << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/*\n> +\t\t * Test cache of same size as there are buffers, the cache is\n> +\t\t * created from a list of buffers and will be pre-populated.\n> +\t\t */\n> +\t\tV4L2BufferCache cacheFromBuffers(buffers);\n> +\n> +\t\tif (testSequential(&cacheFromBuffers, buffers) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testRandom(&cacheFromBuffers, buffers) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testHot(&cacheFromBuffers, buffers, numBuffers) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/*\n> +\t\t * Test cache of same size as there are buffers, the cache is\n> +\t\t * not pre-populated.\n> +\t\t */\n> +\t\tV4L2BufferCache cacheFromNumbers(numBuffers);\n> +\n> +\t\tif (testSequential(&cacheFromNumbers, buffers) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testRandom(&cacheFromNumbers, buffers) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testHot(&cacheFromNumbers, buffers, numBuffers) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/*\n> +\t\t * Test cache half the size of number of buffers used, the cache\n> +\t\t * is not pre-populated.\n> +\t\t */\n> +\t\tV4L2BufferCache cacheHalf(numBuffers / 2);\n> +\n> +\t\tif (testRandom(&cacheHalf, buffers) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testHot(&cacheHalf, buffers, numBuffers / 2) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +private:\n> +\tstd::mt19937 generator_;\n> +};\n> +\n> +} /* namespace */\n> +\n> +TEST_REGISTER(BufferCacheTest);\n> diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build\n> index 5c52da7219c21cc3..685fcf6d16d72182 100644\n> --- a/test/v4l2_videodevice/meson.build\n> +++ b/test/v4l2_videodevice/meson.build\n> @@ -5,6 +5,7 @@ v4l2_videodevice_tests = [\n> [ 'controls', 'controls.cpp' ],\n> [ 'formats', 'formats.cpp' ],\n> [ 'request_buffers', 'request_buffers.cpp' ],\n> + [ 'buffer_cache', 'buffer_cache.cpp' ],\n> [ 'stream_on_off', 'stream_on_off.cpp' ],\n> [ 'capture_async', 'capture_async.cpp' ],\n> [ 'buffer_sharing', 'buffer_sharing.cpp' ],","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 40B9E62689\n\tfor ;\n\tSat, 29 Feb 2020 17:17:04 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A67AF33E;\n\tSat, 29 Feb 2020 17:17:03 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582993023;\n\tbh=csiYuF9gIseFD7DKB0ekJ7oUxPnAd92G2h4avXCKRf4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=quUENCtAWtd/QNfzgMH8a2P8rTNRdWUtFSOTG1b/IsHpkrpdUYb/9N7y60pBf4oL1\n\tKk4GEMEsmoJjSv2f6lFC05UVkL31pwvn521Izvgb2n2iCGPKGu857nR5rVRsr1u4bV\n\tQlqV3kunv6cwYFaNoBb7RUJtBfn/6VlGmvgBjBX0=","Date":"Sat, 29 Feb 2020 18:16:40 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200229161640.GE18738@pendragon.ideasonboard.com>","References":"<20200224193601.1040770-1-niklas.soderlund@ragnatech.se>\n\t<20200224193601.1040770-4-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200224193601.1040770-4-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] test: v4l2_videodevice: Add\n\ttest for V4L2BufferCache","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 29 Feb 2020 16:17:04 -0000"}},{"id":3930,"web_url":"https://patchwork.libcamera.org/comment/3930/","msgid":"<20200304214603.GE1791497@oden.dyn.berto.se>","date":"2020-03-04T21:46:03","subject":"Re: [libcamera-devel] [PATCH v2 3/4] test: v4l2_videodevice: Add\n\ttest for V4L2BufferCache","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2020-02-29 18:16:40 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Mon, Feb 24, 2020 at 08:36:00PM +0100, Niklas Söderlund wrote:\n> > Add test to test the different modes and situations the V4L2BufferCache\n> > can be put in. The tests verify that a FrameBuffer used with the cache\n> > results in a V4L2 video device index, and that the cache implementation\n> > is capable of keeping buffers in a hot state.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > * Changes since v1\n> > - Update comments in code.\n> > - Use std::mt19937 PRNG instead of rand()\n> > - Print randomize and print std::mt19937 initial seed to be able to\n> > reproduce a test run with the same random sequences.\n> > - Add const std::vector> &buffers \"alias\"\n> > for source.buffers() to make code more readable.\n> > - Make use of libtest common implementation of BufferSource.\n> > ---\n> > test/v4l2_videodevice/buffer_cache.cpp | 215 +++++++++++++++++++++++++\n> > test/v4l2_videodevice/meson.build | 1 +\n> > 2 files changed, 216 insertions(+)\n> > create mode 100644 test/v4l2_videodevice/buffer_cache.cpp\n> > \n> > diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp\n> > new file mode 100644\n> > index 0000000000000000..0a8cb0d28ca9b204\n> > --- /dev/null\n> > +++ b/test/v4l2_videodevice/buffer_cache.cpp\n> > @@ -0,0 +1,215 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * Test the buffer cache different operation modes\n> > + */\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"buffer_source.h\"\n> > +\n> > +#include \"test.h\"\n> > +\n> > +using namespace libcamera;\n> > +\n> > +namespace {\n> > +\n> > +class BufferCacheTest : public Test\n> > +{\n> > +public:\n> > +\t/*\n> > +\t * Test that a cache with the same size as there are buffers results in\n> > +\t * a sequential run over; 0, 1, 2, 3, 4, 0, 1, 2, 3, 4, ...\n> > +\t *\n> > +\t * The test is only valid when the cache size is as least as big as the\n> > +\t * number of buffers.\n> > +\t */\n> > +\tint testSequential(V4L2BufferCache *cache,\n> > +\t\t\t const std::vector> &buffers)\n> > +\t{\n> > +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> > +\t\t\tint nBuffer = i % buffers.size();\n> > +\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> > +\n> > +\t\t\tif (index != nBuffer) {\n> > +\t\t\t\tstd::cout << \"Expected index \" << nBuffer\n> > +\t\t\t\t\t << \" got \" << index << std::endl;\n> > +\t\t\t\treturn TestFail;\n> > +\t\t\t}\n> > +\n> > +\t\t\tcache->put(index);\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\t/*\n> > +\t * Test that randomly putting buffers to the cache always results in a\n> > +\t * valid index.\n> > +\t */\n> > +\tint testRandom(V4L2BufferCache *cache,\n> > +\t\t const std::vector> &buffers)\n> > +\t{\n> > +\t\tstd::uniform_int_distribution<> dist(0, buffers.size() - 1);\n> > +\n> > +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> > +\t\t\tint nBuffer = dist(generator_);\n> > +\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> > +\n> > +\t\t\tif (index < 0) {\n> > +\t\t\t\tstd::cout << \"Failed lookup from cache\"\n> > +\t\t\t\t\t << std::endl;\n> > +\t\t\t\treturn TestFail;\n> > +\t\t\t}\n> > +\n> > +\t\t\tcache->put(index);\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\t/*\n> > +\t * Test that using a buffer more frequently keeps it hot in the cache at\n> > +\t * all times.\n> > +\t */\n> > +\tint testHot(V4L2BufferCache *cache,\n> > +\t\t const std::vector> &buffers,\n> > +\t\t unsigned int hotFrequency)\n> > +\t{\n> > +\t\t/* Run the random test on the cache to make it messy. */\n> > +\t\tif (testRandom(cache, buffers) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tstd::uniform_int_distribution<> dist(0, buffers.size() - 1);\n> > +\n> > +\t\t/* Pick a hot buffer at random and store its index. */\n> > +\t\tint hotBuffer = dist(generator_);\n> > +\t\tint hotIndex = cache->get(*buffers[hotBuffer].get());\n> > +\t\tcache->put(hotIndex);\n> > +\n> > +\t\t/*\n> > +\t\t * Queue hot buffer at the requested frequency and make sure\n> > +\t\t * it stays hot.\n> > +\t\t */\n> > +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> > +\t\t\tint nBuffer, index;\n> > +\t\t\tbool hotQueue = i % hotFrequency == 0;\n> > +\n> > +\t\t\tif (hotQueue)\n> > +\t\t\t\tnBuffer = hotBuffer;\n> > +\t\t\telse\n> > +\t\t\t\tnBuffer = dist(generator_);\n> > +\n> > +\t\t\tindex = cache->get(*buffers[nBuffer].get());\n> > +\n> > +\t\t\tif (index < 0) {\n> > +\t\t\t\tstd::cout << \"Failed lookup from cache\"\n> > +\t\t\t\t\t << std::endl;\n> > +\t\t\t\treturn TestFail;\n> > +\t\t\t}\n> > +\n> > +\t\t\tif (hotQueue && index != hotIndex) {\n> > +\t\t\t\tstd::cout << \"Hot buffer got cold\"\n> > +\t\t\t\t\t << std::endl;\n> > +\t\t\t\treturn TestFail;\n> > +\t\t\t}\n> > +\n> > +\t\t\tcache->put(index);\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tint init() override\n> > +\t{\n> > +\t\tstd::random_device rd;\n> > +\t\tunsigned int seed = rd();\n> > +\n> > +\t\tstd::cout << \"Random seed is \" << seed << std::endl;\n> > +\n> > +\t\tgenerator_.seed(seed);\n> \n> As reported by Kieran, using real randomness in our tests makes them\n> non-reproducible. Should we leave the generate unseeded ?\n\nI like to keep them random, at least this one as I think it adds value. \n\nHowever they are reproducible, I log the seed value and if we wish to \nreproduce a test that fails we can inject the seed from the failed run \ninto generator_.seed() and it will generate the \nsame sequence as for the failed test.\n\nOn a side topic, if we want truly reproducible tests we should also log \nkernel version with checksum of build commit as subtle driver changes \nmight be the reason a test fails/pass.\n\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tint run() override\n> > +\t{\n> > +\t\tconst unsigned int numBuffers = 8;\n> > +\n> > +\t\tStreamConfiguration cfg;\n> > +\t\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> > +\t\tcfg.size = Size(600, 800);\n> > +\t\tcfg.bufferCount = numBuffers;\n> > +\n> > +\t\tBufferSource source;\n> > +\t\tint ret = source.allocate(cfg);\n> > +\t\tif (ret != TestPass)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\tconst std::vector> &buffers =\n> > +\t\t\tsource.buffers();\n> > +\n> > +\t\tif (buffers.size() != numBuffers) {\n> > +\t\t\tstd::cout << \"Got \" << buffers.size()\n> > +\t\t\t\t << \" buffers, expected \" << numBuffers\n> > +\t\t\t\t << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/*\n> > +\t\t * Test cache of same size as there are buffers, the cache is\n> > +\t\t * created from a list of buffers and will be pre-populated.\n> > +\t\t */\n> > +\t\tV4L2BufferCache cacheFromBuffers(buffers);\n> > +\n> > +\t\tif (testSequential(&cacheFromBuffers, buffers) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testRandom(&cacheFromBuffers, buffers) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testHot(&cacheFromBuffers, buffers, numBuffers) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/*\n> > +\t\t * Test cache of same size as there are buffers, the cache is\n> > +\t\t * not pre-populated.\n> > +\t\t */\n> > +\t\tV4L2BufferCache cacheFromNumbers(numBuffers);\n> > +\n> > +\t\tif (testSequential(&cacheFromNumbers, buffers) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testRandom(&cacheFromNumbers, buffers) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testHot(&cacheFromNumbers, buffers, numBuffers) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/*\n> > +\t\t * Test cache half the size of number of buffers used, the cache\n> > +\t\t * is not pre-populated.\n> > +\t\t */\n> > +\t\tV4L2BufferCache cacheHalf(numBuffers / 2);\n> > +\n> > +\t\tif (testRandom(&cacheHalf, buffers) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testHot(&cacheHalf, buffers, numBuffers / 2) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +private:\n> > +\tstd::mt19937 generator_;\n> > +};\n> > +\n> > +} /* namespace */\n> > +\n> > +TEST_REGISTER(BufferCacheTest);\n> > diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build\n> > index 5c52da7219c21cc3..685fcf6d16d72182 100644\n> > --- a/test/v4l2_videodevice/meson.build\n> > +++ b/test/v4l2_videodevice/meson.build\n> > @@ -5,6 +5,7 @@ v4l2_videodevice_tests = [\n> > [ 'controls', 'controls.cpp' ],\n> > [ 'formats', 'formats.cpp' ],\n> > [ 'request_buffers', 'request_buffers.cpp' ],\n> > + [ 'buffer_cache', 'buffer_cache.cpp' ],\n> > [ 'stream_on_off', 'stream_on_off.cpp' ],\n> > [ 'capture_async', 'capture_async.cpp' ],\n> > [ 'buffer_sharing', 'buffer_sharing.cpp' ],\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3822A60426\n\tfor ;\n\tWed, 4 Mar 2020 22:46:06 +0100 (CET)","by mail-lf1-x142.google.com with SMTP id b13so2760615lfb.12\n\tfor ;\n\tWed, 04 Mar 2020 13:46:06 -0800 (PST)","from localhost (h-200-138.A463.priv.bahnhof.se. [176.10.200.138])\n\tby smtp.gmail.com with ESMTPSA id\n\tj6sm14123162lfk.88.2020.03.04.13.46.03\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 04 Mar 2020 13:46:04 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=+HPh5q0f4iapNjuOmbJJ4WA1YOkXYDcWGfOhrJliYsg=;\n\tb=khZmyK73rqGe4S/8JPz92y1knTbnYVVSuV0H69cAUj7tgpW3YitYKvlDN1aWFLQfU+\n\tPwzhwODaUX7bJulY3cpfCM9wWSD/vlCDUDwoFGKAhTB/fSKFNTvmJT2NGbBmPFLzx4Gf\n\tSWHevC1UUmC7jtToSF0zHX5g1TzjtqtmF3PONzUoRzeFTZ9pzSII8sOuL81ebdLp25N7\n\tYvKZi21k86vT7ON0nXOjgAopQv27RJ7rtZWab6Le7NmjSze6+zC8v9tMqOFIo7XZty5/\n\tcxJbguxY8PpWcehEoQwqANQ7dW77nWMeOmI5AaWHKulL/emmnnoyKHQzPp40OGoRxZFg\n\t50VA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=+HPh5q0f4iapNjuOmbJJ4WA1YOkXYDcWGfOhrJliYsg=;\n\tb=NSeWjB6s9zPKWEcoSGpScOTZi/RZJVc+ltPc1E8s0Io71+gvD7IOlWcM6YjAhCIkfv\n\tl419rPUIm7zbSeY5cTf2HaoHPrpp4V8nr3gYGOiEhMbT6MFUXvVvcj9jy6cs+w2KjkJ6\n\tfYrnRWuZ7XdN5zXaBN0vPJPIBqDw4RkceC1ZvV3tfxuxAu+6vUbgbnQDAUXb64drbuZo\n\tuV9ln3aIe2T0RT0Z8zA5E71IKgMGPgyRXsmwpqRjOQYjXzhj86QuTgQ395X3xuyLlI0B\n\tADxSZuAiw258YhjkTnF7JfLJX3/nasHTvw5vSxq91r1qBReObiiI+1u52d5UeUXAdpwO\n\tEZ4A==","X-Gm-Message-State":"ANhLgQ2JVcisJfQSx6BErlpMBRx5Pv5gMcp6RrY+2LXhvWzoHySBHFcF\n\tkBoPc7/PHqEGM3NBr2lqrQRDZA==","X-Google-Smtp-Source":"ADFU+vtQeE5AItUbmuvyKbk7QsInKVHy2NQYKX/JlZkQJJJ6Pj8MBjh7ZyfXdJHJOctKVC26/q+Hnw==","X-Received":"by 2002:a19:2396:: with SMTP id\n\tj144mr3160134lfj.113.1583358365113; \n\tWed, 04 Mar 2020 13:46:05 -0800 (PST)","Date":"Wed, 4 Mar 2020 22:46:03 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200304214603.GE1791497@oden.dyn.berto.se>","References":"<20200224193601.1040770-1-niklas.soderlund@ragnatech.se>\n\t<20200224193601.1040770-4-niklas.soderlund@ragnatech.se>\n\t<20200229161640.GE18738@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200229161640.GE18738@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] test: v4l2_videodevice: Add\n\ttest for V4L2BufferCache","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 04 Mar 2020 21:46:06 -0000"}},{"id":3931,"web_url":"https://patchwork.libcamera.org/comment/3931/","msgid":"<20200304223939.GB28814@pendragon.ideasonboard.com>","date":"2020-03-04T22:39:39","subject":"Re: [libcamera-devel] [PATCH v2 3/4] test: v4l2_videodevice: Add\n\ttest for V4L2BufferCache","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Wed, Mar 04, 2020 at 10:46:03PM +0100, Niklas Söderlund wrote:\n> On 2020-02-29 18:16:40 +0200, Laurent Pinchart wrote:\n> > On Mon, Feb 24, 2020 at 08:36:00PM +0100, Niklas Söderlund wrote:\n> >> Add test to test the different modes and situations the V4L2BufferCache\n> >> can be put in. The tests verify that a FrameBuffer used with the cache\n> >> results in a V4L2 video device index, and that the cache implementation\n> >> is capable of keeping buffers in a hot state.\n> >> \n> >> Signed-off-by: Niklas Söderlund \n> >> ---\n> >> * Changes since v1\n> >> - Update comments in code.\n> >> - Use std::mt19937 PRNG instead of rand()\n> >> - Print randomize and print std::mt19937 initial seed to be able to\n> >> reproduce a test run with the same random sequences.\n> >> - Add const std::vector> &buffers \"alias\"\n> >> for source.buffers() to make code more readable.\n> >> - Make use of libtest common implementation of BufferSource.\n> >> ---\n> >> test/v4l2_videodevice/buffer_cache.cpp | 215 +++++++++++++++++++++++++\n> >> test/v4l2_videodevice/meson.build | 1 +\n> >> 2 files changed, 216 insertions(+)\n> >> create mode 100644 test/v4l2_videodevice/buffer_cache.cpp\n> >> \n> >> diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp\n> >> new file mode 100644\n> >> index 0000000000000000..0a8cb0d28ca9b204\n> >> --- /dev/null\n> >> +++ b/test/v4l2_videodevice/buffer_cache.cpp\n> >> @@ -0,0 +1,215 @@\n> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> >> +/*\n> >> + * Copyright (C) 2020, Google Inc.\n> >> + *\n> >> + * Test the buffer cache different operation modes\n> >> + */\n> >> +\n> >> +#include \n> >> +#include \n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \"buffer_source.h\"\n> >> +\n> >> +#include \"test.h\"\n> >> +\n> >> +using namespace libcamera;\n> >> +\n> >> +namespace {\n> >> +\n> >> +class BufferCacheTest : public Test\n> >> +{\n> >> +public:\n> >> +\t/*\n> >> +\t * Test that a cache with the same size as there are buffers results in\n> >> +\t * a sequential run over; 0, 1, 2, 3, 4, 0, 1, 2, 3, 4, ...\n> >> +\t *\n> >> +\t * The test is only valid when the cache size is as least as big as the\n> >> +\t * number of buffers.\n> >> +\t */\n> >> +\tint testSequential(V4L2BufferCache *cache,\n> >> +\t\t\t const std::vector> &buffers)\n> >> +\t{\n> >> +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> >> +\t\t\tint nBuffer = i % buffers.size();\n> >> +\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> >> +\n> >> +\t\t\tif (index != nBuffer) {\n> >> +\t\t\t\tstd::cout << \"Expected index \" << nBuffer\n> >> +\t\t\t\t\t << \" got \" << index << std::endl;\n> >> +\t\t\t\treturn TestFail;\n> >> +\t\t\t}\n> >> +\n> >> +\t\t\tcache->put(index);\n> >> +\t\t}\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +\t/*\n> >> +\t * Test that randomly putting buffers to the cache always results in a\n> >> +\t * valid index.\n> >> +\t */\n> >> +\tint testRandom(V4L2BufferCache *cache,\n> >> +\t\t const std::vector> &buffers)\n> >> +\t{\n> >> +\t\tstd::uniform_int_distribution<> dist(0, buffers.size() - 1);\n> >> +\n> >> +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> >> +\t\t\tint nBuffer = dist(generator_);\n> >> +\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> >> +\n> >> +\t\t\tif (index < 0) {\n> >> +\t\t\t\tstd::cout << \"Failed lookup from cache\"\n> >> +\t\t\t\t\t << std::endl;\n> >> +\t\t\t\treturn TestFail;\n> >> +\t\t\t}\n> >> +\n> >> +\t\t\tcache->put(index);\n> >> +\t\t}\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +\t/*\n> >> +\t * Test that using a buffer more frequently keeps it hot in the cache at\n> >> +\t * all times.\n> >> +\t */\n> >> +\tint testHot(V4L2BufferCache *cache,\n> >> +\t\t const std::vector> &buffers,\n> >> +\t\t unsigned int hotFrequency)\n> >> +\t{\n> >> +\t\t/* Run the random test on the cache to make it messy. */\n> >> +\t\tif (testRandom(cache, buffers) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tstd::uniform_int_distribution<> dist(0, buffers.size() - 1);\n> >> +\n> >> +\t\t/* Pick a hot buffer at random and store its index. */\n> >> +\t\tint hotBuffer = dist(generator_);\n> >> +\t\tint hotIndex = cache->get(*buffers[hotBuffer].get());\n> >> +\t\tcache->put(hotIndex);\n> >> +\n> >> +\t\t/*\n> >> +\t\t * Queue hot buffer at the requested frequency and make sure\n> >> +\t\t * it stays hot.\n> >> +\t\t */\n> >> +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> >> +\t\t\tint nBuffer, index;\n> >> +\t\t\tbool hotQueue = i % hotFrequency == 0;\n> >> +\n> >> +\t\t\tif (hotQueue)\n> >> +\t\t\t\tnBuffer = hotBuffer;\n> >> +\t\t\telse\n> >> +\t\t\t\tnBuffer = dist(generator_);\n> >> +\n> >> +\t\t\tindex = cache->get(*buffers[nBuffer].get());\n> >> +\n> >> +\t\t\tif (index < 0) {\n> >> +\t\t\t\tstd::cout << \"Failed lookup from cache\"\n> >> +\t\t\t\t\t << std::endl;\n> >> +\t\t\t\treturn TestFail;\n> >> +\t\t\t}\n> >> +\n> >> +\t\t\tif (hotQueue && index != hotIndex) {\n> >> +\t\t\t\tstd::cout << \"Hot buffer got cold\"\n> >> +\t\t\t\t\t << std::endl;\n> >> +\t\t\t\treturn TestFail;\n> >> +\t\t\t}\n> >> +\n> >> +\t\t\tcache->put(index);\n> >> +\t\t}\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +\tint init() override\n> >> +\t{\n> >> +\t\tstd::random_device rd;\n> >> +\t\tunsigned int seed = rd();\n> >> +\n> >> +\t\tstd::cout << \"Random seed is \" << seed << std::endl;\n> >> +\n> >> +\t\tgenerator_.seed(seed);\n> > \n> > As reported by Kieran, using real randomness in our tests makes them\n> > non-reproducible. Should we leave the generate unseeded ?\n> \n> I like to keep them random, at least this one as I think it adds value. \n\nIf we really think it does, then I'd say that increasing the number of\nsamples would probably add more value :-)\n\n> However they are reproducible, I log the seed value and if we wish to \n> reproduce a test that fails we can inject the seed from the failed run \n> into generator_.seed() and it will generate the \n> same sequence as for the failed test.\n> \n> On a side topic, if we want truly reproducible tests we should also log \n> kernel version with checksum of build commit as subtle driver changes \n> might be the reason a test fails/pass.\n\nReproducible tests mean that the same test should produce the same\nresult for the same build of libcamera on the same system. If you run\nthe rkisp1 test on a RK3399 and on an Intel platform, I expected\ndifferent results :-)\n\nThis being said, I think it makes sense to log platform information,\nyes.\n\n> > Reviewed-by: Laurent Pinchart \n> > \n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +\tint run() override\n> >> +\t{\n> >> +\t\tconst unsigned int numBuffers = 8;\n> >> +\n> >> +\t\tStreamConfiguration cfg;\n> >> +\t\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> >> +\t\tcfg.size = Size(600, 800);\n> >> +\t\tcfg.bufferCount = numBuffers;\n> >> +\n> >> +\t\tBufferSource source;\n> >> +\t\tint ret = source.allocate(cfg);\n> >> +\t\tif (ret != TestPass)\n> >> +\t\t\treturn ret;\n> >> +\n> >> +\t\tconst std::vector> &buffers =\n> >> +\t\t\tsource.buffers();\n> >> +\n> >> +\t\tif (buffers.size() != numBuffers) {\n> >> +\t\t\tstd::cout << \"Got \" << buffers.size()\n> >> +\t\t\t\t << \" buffers, expected \" << numBuffers\n> >> +\t\t\t\t << std::endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\t/*\n> >> +\t\t * Test cache of same size as there are buffers, the cache is\n> >> +\t\t * created from a list of buffers and will be pre-populated.\n> >> +\t\t */\n> >> +\t\tV4L2BufferCache cacheFromBuffers(buffers);\n> >> +\n> >> +\t\tif (testSequential(&cacheFromBuffers, buffers) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (testRandom(&cacheFromBuffers, buffers) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (testHot(&cacheFromBuffers, buffers, numBuffers) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\t/*\n> >> +\t\t * Test cache of same size as there are buffers, the cache is\n> >> +\t\t * not pre-populated.\n> >> +\t\t */\n> >> +\t\tV4L2BufferCache cacheFromNumbers(numBuffers);\n> >> +\n> >> +\t\tif (testSequential(&cacheFromNumbers, buffers) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (testRandom(&cacheFromNumbers, buffers) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (testHot(&cacheFromNumbers, buffers, numBuffers) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\t/*\n> >> +\t\t * Test cache half the size of number of buffers used, the cache\n> >> +\t\t * is not pre-populated.\n> >> +\t\t */\n> >> +\t\tV4L2BufferCache cacheHalf(numBuffers / 2);\n> >> +\n> >> +\t\tif (testRandom(&cacheHalf, buffers) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (testHot(&cacheHalf, buffers, numBuffers / 2) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +private:\n> >> +\tstd::mt19937 generator_;\n> >> +};\n> >> +\n> >> +} /* namespace */\n> >> +\n> >> +TEST_REGISTER(BufferCacheTest);\n> >> diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build\n> >> index 5c52da7219c21cc3..685fcf6d16d72182 100644\n> >> --- a/test/v4l2_videodevice/meson.build\n> >> +++ b/test/v4l2_videodevice/meson.build\n> >> @@ -5,6 +5,7 @@ v4l2_videodevice_tests = [\n> >> [ 'controls', 'controls.cpp' ],\n> >> [ 'formats', 'formats.cpp' ],\n> >> [ 'request_buffers', 'request_buffers.cpp' ],\n> >> + [ 'buffer_cache', 'buffer_cache.cpp' ],\n> >> [ 'stream_on_off', 'stream_on_off.cpp' ],\n> >> [ 'capture_async', 'capture_async.cpp' ],\n> >> [ 'buffer_sharing', 'buffer_sharing.cpp' ],","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D0D6E60426\n\tfor ;\n\tWed, 4 Mar 2020 23:39:42 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 47CDE33E;\n\tWed, 4 Mar 2020 23:39:42 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1583361582;\n\tbh=leyWEJpQsITJxj4IZ85VeclG3w/lzUgkLOJ2AEmK0kg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hyhgVQuF299PtWuwIgfh2HHqgOCe8SYI5SDJLZ1BYL0MLCOMr4aiOnR84FbHVL2fW\n\ti2WGst692Rg9Z5x3q1tb2wj8plonXd0Zmyyy1q1sh/nRgP6cbTr3aToL4t/nzBvBYS\n\tT1y/I6/8ZIfC22BzSB61A3akTcHi42296rQ7nWeg=","Date":"Thu, 5 Mar 2020 00:39:39 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200304223939.GB28814@pendragon.ideasonboard.com>","References":"<20200224193601.1040770-1-niklas.soderlund@ragnatech.se>\n\t<20200224193601.1040770-4-niklas.soderlund@ragnatech.se>\n\t<20200229161640.GE18738@pendragon.ideasonboard.com>\n\t<20200304214603.GE1791497@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200304214603.GE1791497@oden.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] test: v4l2_videodevice: Add\n\ttest for V4L2BufferCache","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 04 Mar 2020 22:39:43 -0000"}}]