[{"id":3771,"web_url":"https://patchwork.libcamera.org/comment/3771/","msgid":"<20200216221835.GG28645@pendragon.ideasonboard.com>","date":"2020-02-16T22:18:35","subject":"Re: [libcamera-devel] [PATCH 2/2] test: v4l2_videodevice: Add test\n\tfor 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 Sun, Feb 16, 2020 at 05:16:42PM +0100, Niklas Söderlund wrote:\n> Add test to test the different modes and situations the V4L2BufferCache\n> can be put to. The tests verify that a FrameBuffer used with the cache\n\ns/put to/put in/ ?\n\n> results in a V4L2 video device index can be resolved, and that the cache\n\ns/can be resolved/that can be resolved/ ? Or did you mean something else\n? I'm not sure what you mean by resolved here to be honest.\n\n> implementation is capable of keeping buffers in a hot state.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\nCould you move this before patch 1/2 to see that it fails, and that the\nfix actually fixes the problem ?\n\n> ---\n>  test/v4l2_videodevice/buffer_cache.cpp | 286 +++++++++++++++++++++++++\n>  test/v4l2_videodevice/meson.build      |   1 +\n>  2 files changed, 287 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..6244361130525d29\n> --- /dev/null\n> +++ b/test/v4l2_videodevice/buffer_cache.cpp\n> @@ -0,0 +1,286 @@\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 <iostream>\n> +#include <random>\n> +#include <vector>\n> +\n> +#include <libcamera/stream.h>\n> +\n> +#include \"device_enumerator.h\"\n> +#include \"media_device.h\"\n> +#include \"v4l2_videodevice.h\"\n> +\n> +#include \"test.h\"\n> +\n> +using namespace libcamera;\n> +\n> +namespace {\n> +\n> +/* A provider of source of external buffers, suitable for to use by V4L2BufferCache. */\n\nLine wrap ?\n\nA provider or a source ?\n\ns/for to use/for use/\n\n> +class BufferSource\n> +{\n> +public:\n> +\tBufferSource()\n> +\t\t: video_(nullptr)\n> +\t{\n> +\t}\n> +\n> +\t~BufferSource()\n> +\t{\n> +\t\tif (video_) {\n> +\t\t\tvideo_->releaseBuffers();\n> +\t\t\tvideo_->close();\n> +\t\t}\n> +\n> +\t\tdelete video_;\n> +\t\tvideo_ = nullptr;\n> +\n> +\t\tif (media_)\n> +\t\t\tmedia_->release();\n> +\t}\n> +\n> +\tint allocate(const StreamConfiguration &config)\n> +\t{\n> +\t\t/* Locate and open the video device. */\n> +\t\tstd::string videoDeviceName = \"vivid-000-vid-out\";\n> +\n> +\t\tstd::unique_ptr<DeviceEnumerator> enumerator =\n> +\t\t\tDeviceEnumerator::create();\n> +\t\tif (!enumerator) {\n> +\t\t\tstd::cout << \"Failed to create device enumerator\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (enumerator->enumerate()) {\n> +\t\t\tstd::cout << \"Failed to enumerate media devices\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tDeviceMatch dm(\"vivid\");\n> +\t\tdm.add(videoDeviceName);\n> +\n> +\t\tmedia_ = enumerator->search(dm);\n> +\t\tif (!media_) {\n> +\t\t\tstd::cout << \"No vivid output device available\" << std::endl;\n> +\t\t\treturn TestSkip;\n> +\t\t}\n> +\n> +\t\tvideo_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName);\n> +\t\tif (!video_) {\n> +\t\t\tstd::cout << \"Unable to open \" << videoDeviceName << std::endl;\n\nI think this error message is for the open() failure.\n\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (video_->open())\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Configure the format. */\n> +\t\tV4L2DeviceFormat format;\n> +\t\tif (video_->getFormat(&format)) {\n> +\t\t\tstd::cout << \"Failed to get format on output device\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tformat.size = config.size;\n> +\t\tformat.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false);\n> +\t\tif (video_->setFormat(&format)) {\n> +\t\t\tstd::cout << \"Failed to set format on output device\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (video_->exportBuffers(config.bufferCount, &buffers_) < 0) {\n> +\t\t\tstd::cout << \"Failed to export buffers\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n\nWe clearly need a better way to get hold of dmabufs from the kernel :-)\n\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers()\n> +\t{\n> +\t\treturn buffers_;\n> +\t}\n> +\n> +private:\n> +\tstd::shared_ptr<MediaDevice> media_;\n> +\tV4L2VideoDevice *video_;\n> +\tstd::vector<std::unique_ptr<FrameBuffer>> buffers_;\n> +};\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 * Test only valid when the cache size is as least as big as there are\n\ns/Test/The test is/\n\nYou won't be charged per word.\n\n> +\t * number of buffers.\n\n\"... as the number of buffers.\"\n\n> +\t */\n> +\tint testSequential(V4L2BufferCache *cache,\n> +\t\t\t   const std::vector<std::unique_ptr<FrameBuffer>> &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<std::unique_ptr<FrameBuffer>> &buffers)\n> +\t{\n\n\t\tstd::random_device rd;\n\t\tstd::mt19937 gen(rd());\n\t\tstd::uniform_int_distribution<> dis(0, buffers.size());\n\n(rd and gen should likely be class members as you need random number\ngeneration in testHot() too, dis can stay local and be duplicated in\ntestHot()).\n\n> +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> +\t\t\tint nBuffer = rand() % buffers.size();\n\n\t\t\tint nBuffer = dis(gen);\n\nSlightly longer to write, but will give you a real uniform distribution.\nIt's certainly overkill here but I think it's useful to get used to the\nbest practices as it won't hurt.\n\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<std::unique_ptr<FrameBuffer>> &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\t/* Pick a hot buffer at random and store its index */\n\ns/index/index./\n\n> +\t\tint hotBuffer = rand() % buffers.size();\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 = rand() % buffers.size();\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 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\nI would add\n\n\t\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffer = source.buffers();\n\nas you use source.buffers() all the time.\n\n> +\t\tif (source.buffers().size() != numBuffers) {\n> +\t\t\tstd::cout << \"Got \" << source.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, cache is\n\ns/cache is/the cache is/\n\nSame below.\n\n> +\t\t * created from a list of buffers and will be pre-populated.\n> +\t\t */\n> +\t\tV4L2BufferCache cacheFromBuffers(source.buffers());\n> +\n> +\t\tif (testSequential(&cacheFromBuffers, source.buffers()) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testRandom(&cacheFromBuffers, source.buffers()) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testHot(&cacheFromBuffers, source.buffers(), numBuffers - 1) != TestPass)\n\nIs the -1 needed (here and below) ?\n\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/*\n> +\t\t * Test cache of same size as there are buffers, cache is not\n> +\t\t * pre-populated.\n> +\t\t */\n> +\t\tV4L2BufferCache cacheFromNumbers(numBuffers);\n> +\n> +\t\tif (testSequential(&cacheFromNumbers, source.buffers()) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testRandom(&cacheFromNumbers, source.buffers()) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testHot(&cacheFromNumbers, source.buffers(), numBuffers - 1) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/*\n> +\t\t * Test cache half the size of number of buffers used, cache is\n> +\t\t * not pre-populated.\n> +\t\t */\n> +\t\tV4L2BufferCache cacheHalf(numBuffers / 2);\n> +\n> +\t\tif (testRandom(&cacheHalf, source.buffers()) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testHot(&cacheHalf, source.buffers(), numBuffers / 2 - 1) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn TestPass;\n> +\t}\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":"<laurent.pinchart@ideasonboard.com>","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 E3FA460438\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 16 Feb 2020 23:18:53 +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 453F62AF;\n\tSun, 16 Feb 2020 23:18:53 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1581891533;\n\tbh=Ea69Rdm1GCwOSzEprbDOP1mpSwSTBB9jZpCw6DcSmHw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jehCiKz3pNwzZYnugnQ3sxXPu4MRhqzja9vylJDekz6gxkSS1BT1GEzAnAIYsJPd/\n\tvrf/8fzVWs19BCATkoQzt7RkaQg+5/sxNHK36MCKRnir5kghnZf1AYNZEhQp+CgsHa\n\t7hXeBEKgB31KZ+3OpQydNWuEB2f065NMDzWLc1+U=","Date":"Mon, 17 Feb 2020 00:18:35 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200216221835.GG28645@pendragon.ideasonboard.com>","References":"<20200216161642.152263-1-niklas.soderlund@ragnatech.se>\n\t<20200216161642.152263-3-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":"<20200216161642.152263-3-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/2] test: v4l2_videodevice: Add test\n\tfor V4L2BufferCache","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sun, 16 Feb 2020 22:18:54 -0000"}},{"id":3775,"web_url":"https://patchwork.libcamera.org/comment/3775/","msgid":"<25107fb5-54c0-aa50-7a6e-c0e5403b7de2@ideasonboard.com>","date":"2020-02-17T10:12:30","subject":"Re: [libcamera-devel] [PATCH 2/2] test: v4l2_videodevice: Add test\n\tfor V4L2BufferCache","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 16/02/2020 16:16, Niklas Söderlund wrote:\n> Add test to test the different modes and situations the V4L2BufferCache\n> can be put to. The tests verify that a FrameBuffer used with the cache\n> results in a V4L2 video device index can be resolved, and that the cache\n> implementation is capable of keeping buffers in a hot state.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  test/v4l2_videodevice/buffer_cache.cpp | 286 +++++++++++++++++++++++++\n>  test/v4l2_videodevice/meson.build      |   1 +\n>  2 files changed, 287 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..6244361130525d29\n> --- /dev/null\n> +++ b/test/v4l2_videodevice/buffer_cache.cpp\n> @@ -0,0 +1,286 @@\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 <iostream>\n> +#include <random>\n> +#include <vector>\n> +\n> +#include <libcamera/stream.h>\n> +\n> +#include \"device_enumerator.h\"\n> +#include \"media_device.h\"\n> +#include \"v4l2_videodevice.h\"\n> +\n> +#include \"test.h\"\n> +\n> +using namespace libcamera;\n> +\n> +namespace {\n> +\n> +/* A provider of source of external buffers, suitable for to use by V4L2BufferCache. */\n> +class BufferSource\n> +{\n> +public:\n> +\tBufferSource()\n> +\t\t: video_(nullptr)\n> +\t{\n> +\t}\n> +\n> +\t~BufferSource()\n> +\t{\n> +\t\tif (video_) {\n> +\t\t\tvideo_->releaseBuffers();\n> +\t\t\tvideo_->close();\n> +\t\t}\n> +\n> +\t\tdelete video_;\n> +\t\tvideo_ = nullptr;\n> +\n> +\t\tif (media_)\n> +\t\t\tmedia_->release();\n> +\t}\n> +\n> +\tint allocate(const StreamConfiguration &config)\n> +\t{\n> +\t\t/* Locate and open the video device. */\n> +\t\tstd::string videoDeviceName = \"vivid-000-vid-out\";\n> +\n> +\t\tstd::unique_ptr<DeviceEnumerator> enumerator =\n> +\t\t\tDeviceEnumerator::create();\n> +\t\tif (!enumerator) {\n> +\t\t\tstd::cout << \"Failed to create device enumerator\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (enumerator->enumerate()) {\n> +\t\t\tstd::cout << \"Failed to enumerate media devices\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tDeviceMatch dm(\"vivid\");\n> +\t\tdm.add(videoDeviceName);\n> +\n> +\t\tmedia_ = enumerator->search(dm);\n> +\t\tif (!media_) {\n> +\t\t\tstd::cout << \"No vivid output device available\" << std::endl;\n> +\t\t\treturn TestSkip;\n> +\t\t}\n> +\n> +\t\tvideo_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName);\n> +\t\tif (!video_) {\n> +\t\t\tstd::cout << \"Unable to open \" << videoDeviceName << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (video_->open())\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Configure the format. */\n> +\t\tV4L2DeviceFormat format;\n> +\t\tif (video_->getFormat(&format)) {\n> +\t\t\tstd::cout << \"Failed to get format on output device\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tformat.size = config.size;\n> +\t\tformat.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false);\n> +\t\tif (video_->setFormat(&format)) {\n> +\t\t\tstd::cout << \"Failed to set format on output device\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (video_->exportBuffers(config.bufferCount, &buffers_) < 0) {\n> +\t\t\tstd::cout << \"Failed to export buffers\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers()\n> +\t{\n> +\t\treturn buffers_;\n> +\t}\n> +\n> +private:\n> +\tstd::shared_ptr<MediaDevice> media_;\n> +\tV4L2VideoDevice *video_;\n> +\tstd::vector<std::unique_ptr<FrameBuffer>> buffers_;\n> +};\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 * Test only valid when the cache size is as least as big as there are\n> +\t * number of buffers.\n> +\t */\n> +\tint testSequential(V4L2BufferCache *cache,\n> +\t\t\t   const std::vector<std::unique_ptr<FrameBuffer>> &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<std::unique_ptr<FrameBuffer>> &buffers)\n> +\t{\n> +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> +\t\t\tint nBuffer = rand() % buffers.size();\n\nIs rand() a prng? Or does it get arbitrary random data?\n\nIt would be useful if this was based around a prng (which is perhaps\nseeded randomly) so that we can ensure we can reliably repeat a failing\ntest....\n\nThen in any event of failure (and providing that the failure test case\nprints it's random seed in the logs) we can hardcode the seed into the\nbinary and repeat the failing case to identify the root-cause.\n\nThat's mostly a more general comment about seeing random usage in tests\nthan this specific use-case of course, but I fear it may still be\napplicable in some aspect.\n\n--\nKieran\n\n\n\n\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<std::unique_ptr<FrameBuffer>> &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\t/* Pick a hot buffer at random and store its index */\n> +\t\tint hotBuffer = rand() % buffers.size();\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 = rand() % buffers.size();\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 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\tif (source.buffers().size() != numBuffers) {\n> +\t\t\tstd::cout << \"Got \" << source.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, cache is\n> +\t\t * created from a list of buffers and will be pre-populated.\n> +\t\t */\n> +\t\tV4L2BufferCache cacheFromBuffers(source.buffers());\n> +\n> +\t\tif (testSequential(&cacheFromBuffers, source.buffers()) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testRandom(&cacheFromBuffers, source.buffers()) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testHot(&cacheFromBuffers, source.buffers(), numBuffers - 1) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/*\n> +\t\t * Test cache of same size as there are buffers, cache is not\n> +\t\t * pre-populated.\n> +\t\t */\n> +\t\tV4L2BufferCache cacheFromNumbers(numBuffers);\n> +\n> +\t\tif (testSequential(&cacheFromNumbers, source.buffers()) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testRandom(&cacheFromNumbers, source.buffers()) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testHot(&cacheFromNumbers, source.buffers(), numBuffers - 1) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/*\n> +\t\t * Test cache half the size of number of buffers used, cache is\n> +\t\t * not pre-populated.\n> +\t\t */\n> +\t\tV4L2BufferCache cacheHalf(numBuffers / 2);\n> +\n> +\t\tif (testRandom(&cacheHalf, source.buffers()) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (testHot(&cacheHalf, source.buffers(), numBuffers / 2 - 1) != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn TestPass;\n> +\t}\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>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","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 E840160436\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Feb 2020 11:12:34 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 10B86563;\n\tMon, 17 Feb 2020 11:12:33 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1581934354;\n\tbh=gZRYGKjjv45mPYXkXKHM+jnuCah7plRjr6I0WwZdPK0=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=GXCFikIKekbIfkjqHrUsKaraW2aVt027zxn9HxM5HZFLXfih0xmbNJ1YiP6yX6whK\n\t2xXXMXssoJbtFAibz7hI5KS7CDti3qT/ibCUM4Zar09kRvAAE9Ynlo4pSxksU50yeh\n\t48rUeJM2/fDPBDMoGhzRBzov7p8/TAyK4pPfc27Y=","Reply-To":"kieran.bingham@ideasonboard.com","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200216161642.152263-1-niklas.soderlund@ragnatech.se>\n\t<20200216161642.152263-3-niklas.soderlund@ragnatech.se>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<25107fb5-54c0-aa50-7a6e-c0e5403b7de2@ideasonboard.com>","Date":"Mon, 17 Feb 2020 10:12:30 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.1","MIME-Version":"1.0","In-Reply-To":"<20200216161642.152263-3-niklas.soderlund@ragnatech.se>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 2/2] test: v4l2_videodevice: Add test\n\tfor V4L2BufferCache","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 17 Feb 2020 10:12:35 -0000"}},{"id":3784,"web_url":"https://patchwork.libcamera.org/comment/3784/","msgid":"<20200217124614.GU3013231@oden.dyn.berto.se>","date":"2020-02-17T12:46:14","subject":"Re: [libcamera-devel] [PATCH 2/2] test: v4l2_videodevice: Add test\n\tfor V4L2BufferCache","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran,\n\nThanks for your feedback.\n\nOn 2020-02-17 10:12:30 +0000, Kieran Bingham wrote:\n> Hi Niklas,\n> \n> On 16/02/2020 16:16, Niklas Söderlund wrote:\n> > Add test to test the different modes and situations the V4L2BufferCache\n> > can be put to. The tests verify that a FrameBuffer used with the cache\n> > results in a V4L2 video device index can be resolved, and that the cache\n> > implementation is capable of keeping buffers in a hot state.\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  test/v4l2_videodevice/buffer_cache.cpp | 286 +++++++++++++++++++++++++\n> >  test/v4l2_videodevice/meson.build      |   1 +\n> >  2 files changed, 287 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..6244361130525d29\n> > --- /dev/null\n> > +++ b/test/v4l2_videodevice/buffer_cache.cpp\n> > @@ -0,0 +1,286 @@\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 <iostream>\n> > +#include <random>\n> > +#include <vector>\n> > +\n> > +#include <libcamera/stream.h>\n> > +\n> > +#include \"device_enumerator.h\"\n> > +#include \"media_device.h\"\n> > +#include \"v4l2_videodevice.h\"\n> > +\n> > +#include \"test.h\"\n> > +\n> > +using namespace libcamera;\n> > +\n> > +namespace {\n> > +\n> > +/* A provider of source of external buffers, suitable for to use by V4L2BufferCache. */\n> > +class BufferSource\n> > +{\n> > +public:\n> > +\tBufferSource()\n> > +\t\t: video_(nullptr)\n> > +\t{\n> > +\t}\n> > +\n> > +\t~BufferSource()\n> > +\t{\n> > +\t\tif (video_) {\n> > +\t\t\tvideo_->releaseBuffers();\n> > +\t\t\tvideo_->close();\n> > +\t\t}\n> > +\n> > +\t\tdelete video_;\n> > +\t\tvideo_ = nullptr;\n> > +\n> > +\t\tif (media_)\n> > +\t\t\tmedia_->release();\n> > +\t}\n> > +\n> > +\tint allocate(const StreamConfiguration &config)\n> > +\t{\n> > +\t\t/* Locate and open the video device. */\n> > +\t\tstd::string videoDeviceName = \"vivid-000-vid-out\";\n> > +\n> > +\t\tstd::unique_ptr<DeviceEnumerator> enumerator =\n> > +\t\t\tDeviceEnumerator::create();\n> > +\t\tif (!enumerator) {\n> > +\t\t\tstd::cout << \"Failed to create device enumerator\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (enumerator->enumerate()) {\n> > +\t\t\tstd::cout << \"Failed to enumerate media devices\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tDeviceMatch dm(\"vivid\");\n> > +\t\tdm.add(videoDeviceName);\n> > +\n> > +\t\tmedia_ = enumerator->search(dm);\n> > +\t\tif (!media_) {\n> > +\t\t\tstd::cout << \"No vivid output device available\" << std::endl;\n> > +\t\t\treturn TestSkip;\n> > +\t\t}\n> > +\n> > +\t\tvideo_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName);\n> > +\t\tif (!video_) {\n> > +\t\t\tstd::cout << \"Unable to open \" << videoDeviceName << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (video_->open())\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/* Configure the format. */\n> > +\t\tV4L2DeviceFormat format;\n> > +\t\tif (video_->getFormat(&format)) {\n> > +\t\t\tstd::cout << \"Failed to get format on output device\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tformat.size = config.size;\n> > +\t\tformat.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false);\n> > +\t\tif (video_->setFormat(&format)) {\n> > +\t\t\tstd::cout << \"Failed to set format on output device\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (video_->exportBuffers(config.bufferCount, &buffers_) < 0) {\n> > +\t\t\tstd::cout << \"Failed to export buffers\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers()\n> > +\t{\n> > +\t\treturn buffers_;\n> > +\t}\n> > +\n> > +private:\n> > +\tstd::shared_ptr<MediaDevice> media_;\n> > +\tV4L2VideoDevice *video_;\n> > +\tstd::vector<std::unique_ptr<FrameBuffer>> buffers_;\n> > +};\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 * Test only valid when the cache size is as least as big as there are\n> > +\t * number of buffers.\n> > +\t */\n> > +\tint testSequential(V4L2BufferCache *cache,\n> > +\t\t\t   const std::vector<std::unique_ptr<FrameBuffer>> &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<std::unique_ptr<FrameBuffer>> &buffers)\n> > +\t{\n> > +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> > +\t\t\tint nBuffer = rand() % buffers.size();\n> \n> Is rand() a prng? Or does it get arbitrary random data?\n\nTo be fully correct I think one need to seed rand() with srand().\n\n> \n> It would be useful if this was based around a prng (which is perhaps\n> seeded randomly) so that we can ensure we can reliably repeat a failing\n> test....\n> \n> Then in any event of failure (and providing that the failure test case\n> prints it's random seed in the logs) we can hardcode the seed into the\n> binary and repeat the failing case to identify the root-cause.\n> \n> That's mostly a more general comment about seeing random usage in tests\n> than this specific use-case of course, but I fear it may still be\n> applicable in some aspect.\n\nI agree there could be tests where this level of reproducibility would \nbe highly desirable. I'm not so concerned about this particular test \ncase tho.\n\nIf you feel strongly about it I'm not against it but then I think we \nshall provide a helper class that can be reused for all our testcases in \ncase they need random numbers.\n\n> \n> --\n> Kieran\n> \n> \n> \n> \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<std::unique_ptr<FrameBuffer>> &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\t/* Pick a hot buffer at random and store its index */\n> > +\t\tint hotBuffer = rand() % buffers.size();\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 = rand() % buffers.size();\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 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\tif (source.buffers().size() != numBuffers) {\n> > +\t\t\tstd::cout << \"Got \" << source.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, cache is\n> > +\t\t * created from a list of buffers and will be pre-populated.\n> > +\t\t */\n> > +\t\tV4L2BufferCache cacheFromBuffers(source.buffers());\n> > +\n> > +\t\tif (testSequential(&cacheFromBuffers, source.buffers()) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testRandom(&cacheFromBuffers, source.buffers()) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testHot(&cacheFromBuffers, source.buffers(), numBuffers - 1) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/*\n> > +\t\t * Test cache of same size as there are buffers, cache is not\n> > +\t\t * pre-populated.\n> > +\t\t */\n> > +\t\tV4L2BufferCache cacheFromNumbers(numBuffers);\n> > +\n> > +\t\tif (testSequential(&cacheFromNumbers, source.buffers()) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testRandom(&cacheFromNumbers, source.buffers()) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testHot(&cacheFromNumbers, source.buffers(), numBuffers - 1) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/*\n> > +\t\t * Test cache half the size of number of buffers used, cache is\n> > +\t\t * not pre-populated.\n> > +\t\t */\n> > +\t\tV4L2BufferCache cacheHalf(numBuffers / 2);\n> > +\n> > +\t\tif (testRandom(&cacheHalf, source.buffers()) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testHot(&cacheHalf, source.buffers(), numBuffers / 2 - 1) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\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> -- \n> Regards\n> --\n> Kieran","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5AEAA61935\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Feb 2020 13:46:16 +0100 (CET)","by mail-lj1-x243.google.com with SMTP id r19so18701885ljg.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Feb 2020 04:46:16 -0800 (PST)","from localhost (h-200-138.A463.priv.bahnhof.se. [176.10.200.138])\n\tby smtp.gmail.com with ESMTPSA id\n\tn3sm335113ljc.100.2020.02.17.04.46.14\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 17 Feb 2020 04:46:14 -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=lyX2X3+PQNnjAQyoZfacxo1p3k+bZJcnLCHiH5dLfLw=;\n\tb=NAKMDwkedi7XEoqbTTCWt9OXuDqMnIafJlkbU9I+mO/YX3Ar4NEsB6oREGED/bARM/\n\tbblbhgC0yHaaiSdujc2gf/bTvpAOoOVuKk9Mg5tAwiiBPDJmk7GSpY/w16pzlYwE6Vgr\n\tLfM3tNl/CBxBmdZc4tjpMAuUjNUowUAP9+YdOB2s5CXenvwbZm38HMrMzn/XwwGh4D5d\n\tbSfB4j2iThGdyLcYPvzLlabMbXivFVWTjI7G9idSKl406sm1f2hyr7e2RdyLZBXScwzT\n\t0lQw08NgNiueUod9+d6nJ5zrGeA+0LzOaXAWMyLWnNmvdV/oewiA7aKnRnOScT80jYPs\n\thUdg==","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=lyX2X3+PQNnjAQyoZfacxo1p3k+bZJcnLCHiH5dLfLw=;\n\tb=TKM86p0VYh0qWxLMM0fmo3b3Xt85Fpf4iNaESPa+iANIKEczmwSaZOaDeeWKDeYO0O\n\t01C1lVgerOqfoiu7Rh+kOJdWeefP3jcg+3ec75tX/ckG33dzbb6bT4do9u/5USEHci2k\n\tGLLFCAXzG60zVMlozmM2EIi7XA6SrclCiQG0ogdjFx0VVnQEcEYjtgPK6tw2rOYqzSv+\n\tYdWISD/Uekj1Y877Gqv20LtKOcwWyCGsl9ERuzhh5srS0uo8mo85xNpJwndXBdE2intn\n\tDYW/dEyLDiswjqnADZfoz7ZfgkrIqqAY3C3rKksvnaZeqqmSOpWM+4ls075hhVUmtyKI\n\tEzGg==","X-Gm-Message-State":"APjAAAUGUGKsxJ3CAnPYqe5iw8mX0N92CwEqWYiNXvax1uIpqhmMpn8d\n\t8ScO2CbQusEHh82+dZHPCA23dg==","X-Google-Smtp-Source":"APXvYqx9akU6uBpS05gEIyKlA5pDcOpyFK5vMSr92I4WXl6RfOQfLjIog/JBnDoJ3be2djEvKP5sYQ==","X-Received":"by 2002:a2e:89cd:: with SMTP id\n\tc13mr9666077ljk.139.1581943575620; \n\tMon, 17 Feb 2020 04:46:15 -0800 (PST)","Date":"Mon, 17 Feb 2020 13:46:14 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200217124614.GU3013231@oden.dyn.berto.se>","References":"<20200216161642.152263-1-niklas.soderlund@ragnatech.se>\n\t<20200216161642.152263-3-niklas.soderlund@ragnatech.se>\n\t<25107fb5-54c0-aa50-7a6e-c0e5403b7de2@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<25107fb5-54c0-aa50-7a6e-c0e5403b7de2@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] test: v4l2_videodevice: Add test\n\tfor V4L2BufferCache","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 17 Feb 2020 12:46:16 -0000"}},{"id":3785,"web_url":"https://patchwork.libcamera.org/comment/3785/","msgid":"<20200217125007.GV3013231@oden.dyn.berto.se>","date":"2020-02-17T12:50:07","subject":"Re: [libcamera-devel] [PATCH 2/2] test: v4l2_videodevice: Add test\n\tfor 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-17 00:18:35 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Sun, Feb 16, 2020 at 05:16:42PM +0100, Niklas Söderlund wrote:\n> > Add test to test the different modes and situations the V4L2BufferCache\n> > can be put to. The tests verify that a FrameBuffer used with the cache\n> \n> s/put to/put in/ ?\n> \n> > results in a V4L2 video device index can be resolved, and that the cache\n> \n> s/can be resolved/that can be resolved/ ? Or did you mean something else\n> ? I'm not sure what you mean by resolved here to be honest.\n\nMaybe s/resolved/produced/ ?\n\n> \n> > implementation is capable of keeping buffers in a hot state.\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> \n> Could you move this before patch 1/2 to see that it fails, and that the\n> fix actually fixes the problem ?\n\nSure.\n\n> \n> > ---\n> >  test/v4l2_videodevice/buffer_cache.cpp | 286 +++++++++++++++++++++++++\n> >  test/v4l2_videodevice/meson.build      |   1 +\n> >  2 files changed, 287 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..6244361130525d29\n> > --- /dev/null\n> > +++ b/test/v4l2_videodevice/buffer_cache.cpp\n> > @@ -0,0 +1,286 @@\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 <iostream>\n> > +#include <random>\n> > +#include <vector>\n> > +\n> > +#include <libcamera/stream.h>\n> > +\n> > +#include \"device_enumerator.h\"\n> > +#include \"media_device.h\"\n> > +#include \"v4l2_videodevice.h\"\n> > +\n> > +#include \"test.h\"\n> > +\n> > +using namespace libcamera;\n> > +\n> > +namespace {\n> > +\n> > +/* A provider of source of external buffers, suitable for to use by V4L2BufferCache. */\n> \n> Line wrap ?\n> \n> A provider or a source ?\n> \n> s/for to use/for use/\n> \n> > +class BufferSource\n> > +{\n> > +public:\n> > +\tBufferSource()\n> > +\t\t: video_(nullptr)\n> > +\t{\n> > +\t}\n> > +\n> > +\t~BufferSource()\n> > +\t{\n> > +\t\tif (video_) {\n> > +\t\t\tvideo_->releaseBuffers();\n> > +\t\t\tvideo_->close();\n> > +\t\t}\n> > +\n> > +\t\tdelete video_;\n> > +\t\tvideo_ = nullptr;\n> > +\n> > +\t\tif (media_)\n> > +\t\t\tmedia_->release();\n> > +\t}\n> > +\n> > +\tint allocate(const StreamConfiguration &config)\n> > +\t{\n> > +\t\t/* Locate and open the video device. */\n> > +\t\tstd::string videoDeviceName = \"vivid-000-vid-out\";\n> > +\n> > +\t\tstd::unique_ptr<DeviceEnumerator> enumerator =\n> > +\t\t\tDeviceEnumerator::create();\n> > +\t\tif (!enumerator) {\n> > +\t\t\tstd::cout << \"Failed to create device enumerator\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (enumerator->enumerate()) {\n> > +\t\t\tstd::cout << \"Failed to enumerate media devices\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tDeviceMatch dm(\"vivid\");\n> > +\t\tdm.add(videoDeviceName);\n> > +\n> > +\t\tmedia_ = enumerator->search(dm);\n> > +\t\tif (!media_) {\n> > +\t\t\tstd::cout << \"No vivid output device available\" << std::endl;\n> > +\t\t\treturn TestSkip;\n> > +\t\t}\n> > +\n> > +\t\tvideo_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName);\n> > +\t\tif (!video_) {\n> > +\t\t\tstd::cout << \"Unable to open \" << videoDeviceName << std::endl;\n> \n> I think this error message is for the open() failure.\n> \n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (video_->open())\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/* Configure the format. */\n> > +\t\tV4L2DeviceFormat format;\n> > +\t\tif (video_->getFormat(&format)) {\n> > +\t\t\tstd::cout << \"Failed to get format on output device\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tformat.size = config.size;\n> > +\t\tformat.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false);\n> > +\t\tif (video_->setFormat(&format)) {\n> > +\t\t\tstd::cout << \"Failed to set format on output device\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (video_->exportBuffers(config.bufferCount, &buffers_) < 0) {\n> > +\t\t\tstd::cout << \"Failed to export buffers\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> \n> We clearly need a better way to get hold of dmabufs from the kernel :-)\n\n:-)\n\n> \n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers()\n> > +\t{\n> > +\t\treturn buffers_;\n> > +\t}\n> > +\n> > +private:\n> > +\tstd::shared_ptr<MediaDevice> media_;\n> > +\tV4L2VideoDevice *video_;\n> > +\tstd::vector<std::unique_ptr<FrameBuffer>> buffers_;\n> > +};\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 * Test only valid when the cache size is as least as big as there are\n> \n> s/Test/The test is/\n> \n> You won't be charged per word.\n> \n> > +\t * number of buffers.\n> \n> \"... as the number of buffers.\"\n> \n> > +\t */\n> > +\tint testSequential(V4L2BufferCache *cache,\n> > +\t\t\t   const std::vector<std::unique_ptr<FrameBuffer>> &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<std::unique_ptr<FrameBuffer>> &buffers)\n> > +\t{\n> \n> \t\tstd::random_device rd;\n> \t\tstd::mt19937 gen(rd());\n> \t\tstd::uniform_int_distribution<> dis(0, buffers.size());\n> \n> (rd and gen should likely be class members as you need random number\n> generation in testHot() too, dis can stay local and be duplicated in\n> testHot()).\n> \n> > +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> > +\t\t\tint nBuffer = rand() % buffers.size();\n> \n> \t\t\tint nBuffer = dis(gen);\n> \n> Slightly longer to write, but will give you a real uniform distribution.\n> It's certainly overkill here but I think it's useful to get used to the\n> best practices as it won't hurt.\n\nI will see how the discussion about reproducible random numbers end and \neither to this or add a helper random number class which uses the std \nrandom numbers if they can be made reproducible.\n\n> \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<std::unique_ptr<FrameBuffer>> &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\t/* Pick a hot buffer at random and store its index */\n> \n> s/index/index./\n> \n> > +\t\tint hotBuffer = rand() % buffers.size();\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 = rand() % buffers.size();\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 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> \n> I would add\n> \n> \t\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffer = source.buffers();\n> \n> as you use source.buffers() all the time.\n> \n> > +\t\tif (source.buffers().size() != numBuffers) {\n> > +\t\t\tstd::cout << \"Got \" << source.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, cache is\n> \n> s/cache is/the cache is/\n> \n> Same below.\n> \n> > +\t\t * created from a list of buffers and will be pre-populated.\n> > +\t\t */\n> > +\t\tV4L2BufferCache cacheFromBuffers(source.buffers());\n> > +\n> > +\t\tif (testSequential(&cacheFromBuffers, source.buffers()) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testRandom(&cacheFromBuffers, source.buffers()) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testHot(&cacheFromBuffers, source.buffers(), numBuffers - 1) != TestPass)\n> \n> Is the -1 needed (here and below) ?\n\nNo, good catch. They are a left over from an earlier state of \ndevelopment.\n\n> \n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/*\n> > +\t\t * Test cache of same size as there are buffers, cache is not\n> > +\t\t * pre-populated.\n> > +\t\t */\n> > +\t\tV4L2BufferCache cacheFromNumbers(numBuffers);\n> > +\n> > +\t\tif (testSequential(&cacheFromNumbers, source.buffers()) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testRandom(&cacheFromNumbers, source.buffers()) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testHot(&cacheFromNumbers, source.buffers(), numBuffers - 1) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/*\n> > +\t\t * Test cache half the size of number of buffers used, cache is\n> > +\t\t * not pre-populated.\n> > +\t\t */\n> > +\t\tV4L2BufferCache cacheHalf(numBuffers / 2);\n> > +\n> > +\t\tif (testRandom(&cacheHalf, source.buffers()) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (testHot(&cacheHalf, source.buffers(), numBuffers / 2 - 1) != TestPass)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\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":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 39EBF61935\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Feb 2020 13:50:10 +0100 (CET)","by mail-lf1-x144.google.com with SMTP id l18so11819434lfc.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Feb 2020 04:50:10 -0800 (PST)","from localhost (h-200-138.A463.priv.bahnhof.se. [176.10.200.138])\n\tby smtp.gmail.com with ESMTPSA id\n\tb13sm280506lfi.77.2020.02.17.04.50.08\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 17 Feb 2020 04:50:08 -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=TIWH+4066AoausdqNnEuCi50MRR5YBbAaqPXt3VQ68E=;\n\tb=ICWePP+R9gTYOiI/+C9UWlTKB1nP5nP0Evm0SxhDjHa+ke5zHAry9aA2vN3f//bQAz\n\tOEOleByyOVZBFp5DhVT1d0yg+AYjz/WYKdkAaygejS163fMWebV8BQQvjdu7iG6pt+4z\n\tH6cncFY7vW3ris+sr72+ksweVgK1P0PD1bJCalgB/jRxijpPxsFnvhI1QM9SKcXL56Ci\n\timJgBrHLLKRMEWtow654zbxJpiOmg0WmDdFC8Ivwt2xg8fEfeSYO8DVejlnnekQOxFIF\n\tvzTf/rKpjEpUVOSJ+NhqkrLQAnRBo5uG3EH0Ej/uNZjPNzN0CgXjLUSLLpfTsGNlR2rn\n\tG5UQ==","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=TIWH+4066AoausdqNnEuCi50MRR5YBbAaqPXt3VQ68E=;\n\tb=YbqrhHxqqTwtOV2hyt3ybZWv2GdYase0wVaYmW8fhkvEo5btXnvly3G490kZWAB5BH\n\tdRNyJBs2Qg5rOnuigKFyBfUxmp1LVoUDk8dKR4JKGGgtu9XSJ7b+XiBpGR/j5kS9antW\n\tLB4v+HFdSZatEfwZzHR+C1HqpJLFxnWcuspK/yXUm34L1K9DBNKBBnGgA/Stv4UxPYMP\n\tP+WKkKT83GjpePohoHatunAUfUCPielmlmo/u8vYk1nVSxMgDC7hDAZDrGO/BQR32lba\n\tRLq/OVq7dVTvBgUEkMl1pAnKUy0CWJp4VjWkRtDV5I2MYMF5ctoAVlrjcVQko4J9XUPU\n\td45g==","X-Gm-Message-State":"APjAAAV00LTWjVSaV/53WDCjBQdU8wPUicE4zr6ep/NonR7DZZHe2wmY\n\tb4feR/QPTL1lRv3YzlBo8UpAzLhKNiA=","X-Google-Smtp-Source":"APXvYqz2qTcsnGISfD79BspfLDhj9nt13STxe+MnwpbMwzrApb5PnsS/0Q9VE2jbbuSJzqz43m/zFw==","X-Received":"by 2002:a05:6512:687:: with SMTP id\n\tt7mr8221962lfe.30.1581943809365; \n\tMon, 17 Feb 2020 04:50:09 -0800 (PST)","Date":"Mon, 17 Feb 2020 13:50:07 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200217125007.GV3013231@oden.dyn.berto.se>","References":"<20200216161642.152263-1-niklas.soderlund@ragnatech.se>\n\t<20200216161642.152263-3-niklas.soderlund@ragnatech.se>\n\t<20200216221835.GG28645@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":"<20200216221835.GG28645@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] test: v4l2_videodevice: Add test\n\tfor V4L2BufferCache","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 17 Feb 2020 12:50:10 -0000"}},{"id":3789,"web_url":"https://patchwork.libcamera.org/comment/3789/","msgid":"<20200217233851.GH4830@pendragon.ideasonboard.com>","date":"2020-02-17T23:38:51","subject":"Re: [libcamera-devel] [PATCH 2/2] test: v4l2_videodevice: Add test\n\tfor V4L2BufferCache","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Mon, Feb 17, 2020 at 01:46:14PM +0100, Niklas Söderlund wrote:\n> On 2020-02-17 10:12:30 +0000, Kieran Bingham wrote:\n> > On 16/02/2020 16:16, Niklas Söderlund wrote:\n> >> Add test to test the different modes and situations the V4L2BufferCache\n> >> can be put to. The tests verify that a FrameBuffer used with the cache\n> >> results in a V4L2 video device index can be resolved, and that the cache\n> >> implementation is capable of keeping buffers in a hot state.\n> >> \n> >> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> >> ---\n> >>  test/v4l2_videodevice/buffer_cache.cpp | 286 +++++++++++++++++++++++++\n> >>  test/v4l2_videodevice/meson.build      |   1 +\n> >>  2 files changed, 287 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..6244361130525d29\n> >> --- /dev/null\n> >> +++ b/test/v4l2_videodevice/buffer_cache.cpp\n> >> @@ -0,0 +1,286 @@\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 <iostream>\n> >> +#include <random>\n> >> +#include <vector>\n> >> +\n> >> +#include <libcamera/stream.h>\n> >> +\n> >> +#include \"device_enumerator.h\"\n> >> +#include \"media_device.h\"\n> >> +#include \"v4l2_videodevice.h\"\n> >> +\n> >> +#include \"test.h\"\n> >> +\n> >> +using namespace libcamera;\n> >> +\n> >> +namespace {\n> >> +\n> >> +/* A provider of source of external buffers, suitable for to use by V4L2BufferCache. */\n> >> +class BufferSource\n> >> +{\n> >> +public:\n> >> +\tBufferSource()\n> >> +\t\t: video_(nullptr)\n> >> +\t{\n> >> +\t}\n> >> +\n> >> +\t~BufferSource()\n> >> +\t{\n> >> +\t\tif (video_) {\n> >> +\t\t\tvideo_->releaseBuffers();\n> >> +\t\t\tvideo_->close();\n> >> +\t\t}\n> >> +\n> >> +\t\tdelete video_;\n> >> +\t\tvideo_ = nullptr;\n> >> +\n> >> +\t\tif (media_)\n> >> +\t\t\tmedia_->release();\n> >> +\t}\n> >> +\n> >> +\tint allocate(const StreamConfiguration &config)\n> >> +\t{\n> >> +\t\t/* Locate and open the video device. */\n> >> +\t\tstd::string videoDeviceName = \"vivid-000-vid-out\";\n> >> +\n> >> +\t\tstd::unique_ptr<DeviceEnumerator> enumerator =\n> >> +\t\t\tDeviceEnumerator::create();\n> >> +\t\tif (!enumerator) {\n> >> +\t\t\tstd::cout << \"Failed to create device enumerator\" << std::endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\tif (enumerator->enumerate()) {\n> >> +\t\t\tstd::cout << \"Failed to enumerate media devices\" << std::endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\tDeviceMatch dm(\"vivid\");\n> >> +\t\tdm.add(videoDeviceName);\n> >> +\n> >> +\t\tmedia_ = enumerator->search(dm);\n> >> +\t\tif (!media_) {\n> >> +\t\t\tstd::cout << \"No vivid output device available\" << std::endl;\n> >> +\t\t\treturn TestSkip;\n> >> +\t\t}\n> >> +\n> >> +\t\tvideo_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName);\n> >> +\t\tif (!video_) {\n> >> +\t\t\tstd::cout << \"Unable to open \" << videoDeviceName << std::endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\tif (video_->open())\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\t/* Configure the format. */\n> >> +\t\tV4L2DeviceFormat format;\n> >> +\t\tif (video_->getFormat(&format)) {\n> >> +\t\t\tstd::cout << \"Failed to get format on output device\" << std::endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\tformat.size = config.size;\n> >> +\t\tformat.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false);\n> >> +\t\tif (video_->setFormat(&format)) {\n> >> +\t\t\tstd::cout << \"Failed to set format on output device\" << std::endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\tif (video_->exportBuffers(config.bufferCount, &buffers_) < 0) {\n> >> +\t\t\tstd::cout << \"Failed to export buffers\" << std::endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers()\n> >> +\t{\n> >> +\t\treturn buffers_;\n> >> +\t}\n> >> +\n> >> +private:\n> >> +\tstd::shared_ptr<MediaDevice> media_;\n> >> +\tV4L2VideoDevice *video_;\n> >> +\tstd::vector<std::unique_ptr<FrameBuffer>> buffers_;\n> >> +};\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 * Test only valid when the cache size is as least as big as there are\n> >> +\t * number of buffers.\n> >> +\t */\n> >> +\tint testSequential(V4L2BufferCache *cache,\n> >> +\t\t\t   const std::vector<std::unique_ptr<FrameBuffer>> &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<std::unique_ptr<FrameBuffer>> &buffers)\n> >> +\t{\n> >> +\t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> >> +\t\t\tint nBuffer = rand() % buffers.size();\n> > \n> > Is rand() a prng? Or does it get arbitrary random data?\n> \n> To be fully correct I think one need to seed rand() with srand().\n> \n> > It would be useful if this was based around a prng (which is perhaps\n> > seeded randomly) so that we can ensure we can reliably repeat a failing\n> > test....\n> > \n> > Then in any event of failure (and providing that the failure test case\n> > prints it's random seed in the logs) we can hardcode the seed into the\n> > binary and repeat the failing case to identify the root-cause.\n> > \n> > That's mostly a more general comment about seeing random usage in tests\n> > than this specific use-case of course, but I fear it may still be\n> > applicable in some aspect.\n> \n> I agree there could be tests where this level of reproducibility would \n> be highly desirable. I'm not so concerned about this particular test \n> case tho.\n> \n> If you feel strongly about it I'm not against it but then I think we \n> shall provide a helper class that can be reused for all our testcases in \n> case they need random numbers.\n\nI agree with Kieran that tests should be reproducible. How about\n\n\tstd::mt19937 gen;\n\tstd::uniform_int_distribution<> dist(0, buffers.size() - 1);\n\n\t...\n\t\tint nBuffer = dist(gen);\n\nThis will be a PRNG and should always produce the same results. We can\nspecify a seed when constructing gen, otherwise the value 5489 is used.\n\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<std::unique_ptr<FrameBuffer>> &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\t/* Pick a hot buffer at random and store its index */\n> >> +\t\tint hotBuffer = rand() % buffers.size();\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 = rand() % buffers.size();\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 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\tif (source.buffers().size() != numBuffers) {\n> >> +\t\t\tstd::cout << \"Got \" << source.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, cache is\n> >> +\t\t * created from a list of buffers and will be pre-populated.\n> >> +\t\t */\n> >> +\t\tV4L2BufferCache cacheFromBuffers(source.buffers());\n> >> +\n> >> +\t\tif (testSequential(&cacheFromBuffers, source.buffers()) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (testRandom(&cacheFromBuffers, source.buffers()) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (testHot(&cacheFromBuffers, source.buffers(), numBuffers - 1) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\t/*\n> >> +\t\t * Test cache of same size as there are buffers, cache is not\n> >> +\t\t * pre-populated.\n> >> +\t\t */\n> >> +\t\tV4L2BufferCache cacheFromNumbers(numBuffers);\n> >> +\n> >> +\t\tif (testSequential(&cacheFromNumbers, source.buffers()) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (testRandom(&cacheFromNumbers, source.buffers()) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (testHot(&cacheFromNumbers, source.buffers(), numBuffers - 1) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\t/*\n> >> +\t\t * Test cache half the size of number of buffers used, cache is\n> >> +\t\t * not pre-populated.\n> >> +\t\t */\n> >> +\t\tV4L2BufferCache cacheHalf(numBuffers / 2);\n> >> +\n> >> +\t\tif (testRandom(&cacheHalf, source.buffers()) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (testHot(&cacheHalf, source.buffers(), numBuffers / 2 - 1) != TestPass)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\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":"<laurent.pinchart@ideasonboard.com>","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 D2CEE60438\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 Feb 2020 00:39:09 +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 3787A1220;\n\tTue, 18 Feb 2020 00:39:09 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1581982749;\n\tbh=R1NBcuXZRn0VkMdkwP7X+hIQ8MY1ZOqZgTxkQ/PQsL8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nHYRAeMtn4cEl0bwy7OH5Y3rgrYbfe7V0O3uUx+BSpdCLNmHtEckeLh2NwPQ1057u\n\tBAboUuXAeWxVqNGvoI6IreGi8LdFa7oaghzbHC1LtJxSJqf6oPh0nR0CyFOW4nKrwy\n\tp2faHB/A8t6aHSznR5AJm4NfB/bV0zRC2OqXJT/c=","Date":"Tue, 18 Feb 2020 01:38:51 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200217233851.GH4830@pendragon.ideasonboard.com>","References":"<20200216161642.152263-1-niklas.soderlund@ragnatech.se>\n\t<20200216161642.152263-3-niklas.soderlund@ragnatech.se>\n\t<25107fb5-54c0-aa50-7a6e-c0e5403b7de2@ideasonboard.com>\n\t<20200217124614.GU3013231@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":"<20200217124614.GU3013231@oden.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/2] test: v4l2_videodevice: Add test\n\tfor V4L2BufferCache","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 17 Feb 2020 23:39:10 -0000"}}]