[{"id":13786,"web_url":"https://patchwork.libcamera.org/comment/13786/","msgid":"<20201118141253.6edepzajgokp5d5w@uno.localdomain>","date":"2020-11-18T14:12:53","subject":"Re: [libcamera-devel] [PATCH v2 2/8] test: delayed_controls: Add\n\ttest case for DelayedControls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Nov 10, 2020 at 01:27:04AM +0100, Niklas Söderlund wrote:\n> Add a test-case for DelayedControls that exercise the setting of\n> controls and reading back what controls where used for a particular\n> frame. Also exercise corner case such as a V4L2 devices that do not\n> reset it sequence number to 0 at stream on and sequence number wrapping\n\ns/it/its\n\n> around the uint32_t value space.\n>\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  test/delayed_contols.cpp | 307 +++++++++++++++++++++++++++++++++++++++\n>  test/meson.build         |   1 +\n>  2 files changed, 308 insertions(+)\n>  create mode 100644 test/delayed_contols.cpp\n>\n> diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp\n> new file mode 100644\n> index 0000000000000000..e71da6662c30bbdc\n> --- /dev/null\n> +++ b/test/delayed_contols.cpp\n> @@ -0,0 +1,307 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * libcamera delayed controls tests\n> + */\n> +\n> +#include <iostream>\n> +\n> +#include \"libcamera/internal/delayed_controls.h\"\n> +#include \"libcamera/internal/device_enumerator.h\"\n> +#include \"libcamera/internal/media_device.h\"\n> +#include \"libcamera/internal/v4l2_videodevice.h\"\n> +\n> +#include \"test.h\"\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +class DelayedControlsTest : public Test\n> +{\n> +public:\n> +\tDelayedControlsTest()\n> +\t\t: dev_(nullptr)\n> +\t{\n> +\t}\n> +\n> +\tint init()\n> +\t{\n> +\t\tenumerator_ = DeviceEnumerator::create();\n> +\t\tif (!enumerator_) {\n> +\t\t\tcerr << \"Failed to create device enumerator\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (enumerator_->enumerate()) {\n> +\t\t\tcerr << \"Failed to enumerate media devices\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tDeviceMatch dm(\"vivid\");\n> +\t\tdm.add(\"vivid-000-vid-cap\");\n> +\n> +\t\tmedia_ = enumerator_->search(dm);\n> +\t\tif (!media_) {\n> +\t\t\tcerr << \"vivid video device found\" << endl;\n> +\t\t\treturn TestSkip;\n> +\t\t}\n> +\n> +\t\tMediaEntity *entity = media_->getEntityByName(\"vivid-000-vid-cap\");\n> +\t\tdev_ = new V4L2VideoDevice(entity->deviceNode());\n> +\t\tif (dev_->open()) {\n> +\t\t\tcerr << \"Failed to open video device\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tconst ControlInfoMap &infoMap = dev_->controls();\n> +\n> +\t\t/* Test control enumeration. */\n> +\t\tif (infoMap.empty()) {\n> +\t\t\tcerr << \"Failed to enumerate controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (infoMap.find(V4L2_CID_BRIGHTNESS) == infoMap.end() ||\n> +\t\t    infoMap.find(V4L2_CID_CONTRAST) == infoMap.end()) {\n> +\t\t\tcerr << \"Missing controls\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tControlList ctrls = dev_->getControls({ V4L2_CID_BRIGHTNESS });\n\nWhat's ctrls for here ?\n\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint singleControlNoDelay()\n> +\t{\n> +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, 0 },\n> +\t\t};\n> +\t\tstd::unique_ptr<DelayedControls> delayed =\n> +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n\nShould this just be\n                DelayedControls delayed(dev_, delays) ?\nit's not used outside of the function\n\n> +\n\nAdditional empty line\n\n> +\t\tControlList ctrls;\n\nDo we want to create the ControlList with the device info map for\nadditional validation ?\n                ControlList ctrl(dev_->controls());\n\n> +\n> +\t\t/* Reset control to value not used in test. */\n> +\t\tctrls.set(V4L2_CID_BRIGHTNESS, 1);\n> +\t\tdelayed->reset(&ctrls);\n\nI would use the ControlInfo.min()\n\nDo you expect after reset() that BRIGHTNESS is equal to 1 ?\n\n> +\n> +\t\t/* Test control without delay are set at once. */\n> +\t\tfor (int32_t i = 0; i < 10; i++) {\n> +\t\t\tint32_t value = 100 + i;\n> +\n> +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> +\t\t\tdelayed->push(ctrls);\n> +\n> +\t\t\tdelayed->frameStart(i);\n> +\n> +\t\t\tControlList result = delayed->get(i);\n> +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> +\t\t\tif (brightness != value) {\n> +\t\t\t\tcerr << \"Failed single control without delay\"\n> +\t\t\t\t     << \" frame \" << i\n> +\t\t\t\t     << \" expected \" << value\n> +\t\t\t\t     << \" got \" << brightness\n> +\t\t\t\t     << endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint singleControlWithDelay()\n> +\t{\n> +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> +\t\t};\n> +\t\tstd::unique_ptr<DelayedControls> delayed =\n> +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> +\n> +\t\tControlList ctrls;\n\nSame questions\n\n> +\n> +\t\t/* Reset control to value that will be first in test. */\n> +\t\tint32_t expected = 4;\n> +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> +\t\tdelayed->reset(&ctrls);\n> +\n> +\t\t/* Test single control with delay. */\n> +\t\tfor (int32_t i = 0; i < 100; i++) {\n> +\t\t\tint32_t value = 10 + i;\n\nIs swapping 10 and 100 in the for loop compared to the previous test\nintended ?\n\n> +\n> +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> +\t\t\tdelayed->push(ctrls);\n> +\n> +\t\t\tdelayed->frameStart(i);\n> +\n> +\t\t\tControlList result = delayed->get(i);\n> +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> +\t\t\tif (brightness != expected) {\n\nAre we're lucky and BRIGHTNESS.min() >= 4 ?\n\n> +\t\t\t\tcerr << \"Failed single control with delay\"\n> +\t\t\t\t     << \" frame \" << i\n> +\t\t\t\t     << \" expected \" << expected\n> +\t\t\t\t     << \" got \" << brightness\n> +\t\t\t\t     << endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\n> +\t\t\texpected = value;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint dualControlsWithDelay(uint32_t startOffset)\n> +\t{\n> +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> +\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> +\t\t};\n> +\t\tstd::unique_ptr<DelayedControls> delayed =\n> +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> +\n> +\t\tControlList ctrls;\n> +\n\nSame :)\n\n> +\t\t/* Reset control to value that will be first two frames in test. */\n> +\t\tint32_t expected = 200;\n> +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> +\t\tctrls.set(V4L2_CID_CONTRAST, expected);\n> +\t\tdelayed->reset(&ctrls);\n> +\n> +\t\t/* Test dual control with delay. */\n> +\t\tfor (int32_t i = 0; i < 100; i++) {\n> +\t\t\tuint32_t frame = startOffset + i;\n> +\n\nAdditional empty line\n\n> +\t\t\tint32_t value = 10 + i;\n> +\n> +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> +\t\t\tctrls.set(V4L2_CID_CONTRAST, value);\n> +\t\t\tdelayed->push(ctrls);\n> +\n> +\t\t\tdelayed->frameStart(frame);\n> +\n> +\t\t\tControlList result = delayed->get(frame);\n> +\n\nIn the previous functions you don't have an empty line here\n\n> +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> +\t\t\tint32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();\n> +\t\t\tif (brightness != expected || contrast != expected) {\n> +\t\t\t\tcerr << \"Failed dual controls\"\n> +\t\t\t\t     << \" frame \" << frame\n> +\t\t\t\t     << \" brightness \" << brightness\n> +\t\t\t\t     << \" contrast \" << contrast\n> +\t\t\t\t     << \" expected \" << expected\n> +\t\t\t\t     << endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\n> +\t\t\texpected = i < 1 ? expected : value - 1;\n\nWhy do I expect this to fail ?\n\nexpected = 200\n0:\n        value = 10;\n        set(BRIGHTNESS, 10);\n        set(CONTRAST, 10);\n\n        frameStart(0);\n        brightness = 200;\n        contrast = 200;\n        (200 == 200; 200 == 200);\n        expected = 200;\n\n1:\n        value = 11;\n        set(BRIGHTNESS, 11);\n        set(CONTRAST, 11);\n\n        frameStart(1);\n        brightness = 10; <- delay was 1\n        contrast = 200;  <- delay was 2\n        (10 != 200) <- Shouldn't this fail ?\n\n        expected = 10;\n\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint dualControlsMultiQueue()\n> +\t{\n> +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> +\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> +\t\t};\n> +\t\tstd::unique_ptr<DelayedControls> delayed =\n> +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> +\n> +\t\tControlList ctrls;\n> +\n> +\t\t/* Reset control to value that will be first two frames in test. */\n> +\t\tint32_t expected = 100;\n> +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> +\t\tctrls.set(V4L2_CID_CONTRAST, expected);\n> +\t\tdelayed->reset(&ctrls);\n> +\n> +\t\t/*\n> +\t\t * Queue all controls before any fake frame start. Note we\n> +\t\t * can't queue up more then the delayed controls history size\n> +\t\t * which is 16. Where one spot is used by the reset control.\n> +\t\t */\n> +\t\tfor (int32_t i = 0; i < 15; i++) {\n> +\t\t\tint32_t value = 10 + i;\n> +\n> +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> +\t\t\tctrls.set(V4L2_CID_CONTRAST, value);\n> +\t\t\tdelayed->push(ctrls);\n> +\t\t}\n> +\n> +\t\t/* Process all queued controls. */\n> +\t\tfor (int32_t i = 0; i < 16; i++) {\n> +\t\t\tint32_t value = 10 + i;\n> +\n> +\t\t\tdelayed->frameStart(i);\n> +\n> +\t\t\tControlList result = delayed->get(i);\n> +\n> +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> +\t\t\tint32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();\n> +\t\t\tif (brightness != expected || contrast != expected) {\n> +\t\t\t\tcerr << \"Failed multi queue\"\n> +\t\t\t\t     << \" frame \" << i\n> +\t\t\t\t     << \" brightness \" << brightness\n> +\t\t\t\t     << \" contrast \" << contrast\n> +\t\t\t\t     << \" expected \" << expected\n> +\t\t\t\t     << endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\n> +\t\t\texpected = i < 1 ? expected : value - 1;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint run()\n> +\t{\n> +\t\tint ret;\n> +\n> +\t\t/* Test single control without delay. */\n> +\t\tret = singleControlNoDelay();\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\n> +\t\t/* Test single control with delay. */\n> +\t\tret = singleControlWithDelay();\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\n> +\t\t/* Test dual controls with different delays. */\n> +\t\tret = dualControlsWithDelay(0);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\n> +\t\t/* Test dual controls with non-zero sequence start. */\n> +\t\tret = dualControlsWithDelay(10000);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\n> +\t\t/* Test dual controls with sequence number wraparound. */\n> +\t\tret = dualControlsWithDelay(UINT32_MAX - 50);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\n> +\t\t/* Test control values produced faster then consumed. */\n> +\t\tret = dualControlsMultiQueue();\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n\nMaybe an empty line ?\n\nThanks\n  j\n\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tvoid cleanup()\n> +\t{\n> +\t\tdelete dev_;\n> +\t}\n> +\n> +private:\n> +\tstd::unique_ptr<DeviceEnumerator> enumerator_;\n> +\tstd::shared_ptr<MediaDevice> media_;\n> +\tV4L2VideoDevice *dev_;\n> +};\n> +\n> +TEST_REGISTER(DelayedControlsTest)\n> diff --git a/test/meson.build b/test/meson.build\n> index 0a1d434e399641bb..a683a657a439b4ff 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -25,6 +25,7 @@ public_tests = [\n>  internal_tests = [\n>      ['byte-stream-buffer',              'byte-stream-buffer.cpp'],\n>      ['camera-sensor',                   'camera-sensor.cpp'],\n> +    ['delayed_contols',                 'delayed_contols.cpp'],\n>      ['event',                           'event.cpp'],\n>      ['event-dispatcher',                'event-dispatcher.cpp'],\n>      ['event-thread',                    'event-thread.cpp'],\n> --\n> 2.29.2\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8F3EDBE176\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 18 Nov 2020 14:12:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B91D77E9B1;\n\tWed, 18 Nov 2020 15:12:54 +0100 (CET)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B60EE7E9AE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 18 Nov 2020 15:12:51 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id C02E760006;\n\tWed, 18 Nov 2020 14:12:50 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 18 Nov 2020 15:12:53 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20201118141253.6edepzajgokp5d5w@uno.localdomain>","References":"<20201110002710.3233696-1-niklas.soderlund@ragnatech.se>\n\t<20201110002710.3233696-3-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201110002710.3233696-3-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v2 2/8] test: delayed_controls: Add\n\ttest case for DelayedControls","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13834,"web_url":"https://patchwork.libcamera.org/comment/13834/","msgid":"<20201123211725.GF1773213@oden.dyn.berto.se>","date":"2020-11-23T21:17:25","subject":"Re: [libcamera-devel] [PATCH v2 2/8] test: delayed_controls: Add\n\ttest case for DelayedControls","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your feedback.\n\nOn 2020-11-18 15:12:53 +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Tue, Nov 10, 2020 at 01:27:04AM +0100, Niklas Söderlund wrote:\n> > Add a test-case for DelayedControls that exercise the setting of\n> > controls and reading back what controls where used for a particular\n> > frame. Also exercise corner case such as a V4L2 devices that do not\n> > reset it sequence number to 0 at stream on and sequence number wrapping\n> \n> s/it/its\n> \n> > around the uint32_t value space.\n> >\n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  test/delayed_contols.cpp | 307 +++++++++++++++++++++++++++++++++++++++\n> >  test/meson.build         |   1 +\n> >  2 files changed, 308 insertions(+)\n> >  create mode 100644 test/delayed_contols.cpp\n> >\n> > diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp\n> > new file mode 100644\n> > index 0000000000000000..e71da6662c30bbdc\n> > --- /dev/null\n> > +++ b/test/delayed_contols.cpp\n> > @@ -0,0 +1,307 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * libcamera delayed controls tests\n> > + */\n> > +\n> > +#include <iostream>\n> > +\n> > +#include \"libcamera/internal/delayed_controls.h\"\n> > +#include \"libcamera/internal/device_enumerator.h\"\n> > +#include \"libcamera/internal/media_device.h\"\n> > +#include \"libcamera/internal/v4l2_videodevice.h\"\n> > +\n> > +#include \"test.h\"\n> > +\n> > +using namespace std;\n> > +using namespace libcamera;\n> > +\n> > +class DelayedControlsTest : public Test\n> > +{\n> > +public:\n> > +\tDelayedControlsTest()\n> > +\t\t: dev_(nullptr)\n> > +\t{\n> > +\t}\n> > +\n> > +\tint init()\n> > +\t{\n> > +\t\tenumerator_ = DeviceEnumerator::create();\n> > +\t\tif (!enumerator_) {\n> > +\t\t\tcerr << \"Failed to create device enumerator\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (enumerator_->enumerate()) {\n> > +\t\t\tcerr << \"Failed to enumerate media devices\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tDeviceMatch dm(\"vivid\");\n> > +\t\tdm.add(\"vivid-000-vid-cap\");\n> > +\n> > +\t\tmedia_ = enumerator_->search(dm);\n> > +\t\tif (!media_) {\n> > +\t\t\tcerr << \"vivid video device found\" << endl;\n> > +\t\t\treturn TestSkip;\n> > +\t\t}\n> > +\n> > +\t\tMediaEntity *entity = media_->getEntityByName(\"vivid-000-vid-cap\");\n> > +\t\tdev_ = new V4L2VideoDevice(entity->deviceNode());\n> > +\t\tif (dev_->open()) {\n> > +\t\t\tcerr << \"Failed to open video device\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tconst ControlInfoMap &infoMap = dev_->controls();\n> > +\n> > +\t\t/* Test control enumeration. */\n> > +\t\tif (infoMap.empty()) {\n> > +\t\t\tcerr << \"Failed to enumerate controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (infoMap.find(V4L2_CID_BRIGHTNESS) == infoMap.end() ||\n> > +\t\t    infoMap.find(V4L2_CID_CONTRAST) == infoMap.end()) {\n> > +\t\t\tcerr << \"Missing controls\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tControlList ctrls = dev_->getControls({ V4L2_CID_BRIGHTNESS });\n> \n> What's ctrls for here ?\n\nGood catch, I will remove it for next version.\n\n> \n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tint singleControlNoDelay()\n> > +\t{\n> > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > +\t\t\t{ V4L2_CID_BRIGHTNESS, 0 },\n> > +\t\t};\n> > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> \n> Should this just be\n>                 DelayedControls delayed(dev_, delays) ?\n> it's not used outside of the function\n\nIt could, I have no real preference. I picked this as I wanted to test \nsome things during development and then I left it as such. Do you see an \nparticular gain in reworking this?\n\n> \n> > +\n> \n> Additional empty line\n\nThanks.\n\n> \n> > +\t\tControlList ctrls;\n> \n> Do we want to create the ControlList with the device info map for\n> additional validation ?\n>                 ControlList ctrl(dev_->controls());\n\nWe could, but I don't see the value in it as the goal is not to test \nControlList. The controls are carefully selected to have a value range \nof [min=0 max=255 step=1]. If this ever changes this test will fail in \nother ways.\n\n> \n> > +\n> > +\t\t/* Reset control to value not used in test. */\n> > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, 1);\n> > +\t\tdelayed->reset(&ctrls);\n> \n> I would use the ControlInfo.min()\n> \n> Do you expect after reset() that BRIGHTNESS is equal to 1 ?\n\nYes.\n\n> \n> > +\n> > +\t\t/* Test control without delay are set at once. */\n> > +\t\tfor (int32_t i = 0; i < 10; i++) {\n> > +\t\t\tint32_t value = 100 + i;\n> > +\n> > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > +\t\t\tdelayed->push(ctrls);\n> > +\n> > +\t\t\tdelayed->frameStart(i);\n> > +\n> > +\t\t\tControlList result = delayed->get(i);\n> > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > +\t\t\tif (brightness != value) {\n> > +\t\t\t\tcerr << \"Failed single control without delay\"\n> > +\t\t\t\t     << \" frame \" << i\n> > +\t\t\t\t     << \" expected \" << value\n> > +\t\t\t\t     << \" got \" << brightness\n> > +\t\t\t\t     << endl;\n> > +\t\t\t\treturn TestFail;\n> > +\t\t\t}\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tint singleControlWithDelay()\n> > +\t{\n> > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> > +\t\t};\n> > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> > +\n> > +\t\tControlList ctrls;\n> \n> Same questions\n> \n> > +\n> > +\t\t/* Reset control to value that will be first in test. */\n> > +\t\tint32_t expected = 4;\n> > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> > +\t\tdelayed->reset(&ctrls);\n> > +\n> > +\t\t/* Test single control with delay. */\n> > +\t\tfor (int32_t i = 0; i < 100; i++) {\n> > +\t\t\tint32_t value = 10 + i;\n> \n> Is swapping 10 and 100 in the for loop compared to the previous test\n> intended ?\n\nNo it should be 100 in the previous test.\n\n> \n> > +\n> > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > +\t\t\tdelayed->push(ctrls);\n> > +\n> > +\t\t\tdelayed->frameStart(i);\n> > +\n> > +\t\t\tControlList result = delayed->get(i);\n> > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > +\t\t\tif (brightness != expected) {\n> \n> Are we're lucky and BRIGHTNESS.min() >= 4 ?\n\nI'm sorry I don't think I understand this question fully I think. We are \nnot lucky here, before the loop we set the control to 4 and this is \nwhats tested here in the first iteration. But the vale 4 is arbitrary \nand we can set it to any value between 0 and 255 before the loop and the \ntest works.\n\n> \n> > +\t\t\t\tcerr << \"Failed single control with delay\"\n> > +\t\t\t\t     << \" frame \" << i\n> > +\t\t\t\t     << \" expected \" << expected\n> > +\t\t\t\t     << \" got \" << brightness\n> > +\t\t\t\t     << endl;\n> > +\t\t\t\treturn TestFail;\n> > +\t\t\t}\n> > +\n> > +\t\t\texpected = value;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tint dualControlsWithDelay(uint32_t startOffset)\n> > +\t{\n> > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> > +\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> > +\t\t};\n> > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> > +\n> > +\t\tControlList ctrls;\n> > +\n> \n> Same :)\n> \n> > +\t\t/* Reset control to value that will be first two frames in test. */\n> > +\t\tint32_t expected = 200;\n> > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> > +\t\tctrls.set(V4L2_CID_CONTRAST, expected);\n> > +\t\tdelayed->reset(&ctrls);\n> > +\n> > +\t\t/* Test dual control with delay. */\n> > +\t\tfor (int32_t i = 0; i < 100; i++) {\n> > +\t\t\tuint32_t frame = startOffset + i;\n> > +\n> \n> Additional empty line\n> \n> > +\t\t\tint32_t value = 10 + i;\n> > +\n> > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > +\t\t\tctrls.set(V4L2_CID_CONTRAST, value);\n> > +\t\t\tdelayed->push(ctrls);\n> > +\n> > +\t\t\tdelayed->frameStart(frame);\n> > +\n> > +\t\t\tControlList result = delayed->get(frame);\n> > +\n> \n> In the previous functions you don't have an empty line here\n> \n> > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > +\t\t\tint32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();\n> > +\t\t\tif (brightness != expected || contrast != expected) {\n> > +\t\t\t\tcerr << \"Failed dual controls\"\n> > +\t\t\t\t     << \" frame \" << frame\n> > +\t\t\t\t     << \" brightness \" << brightness\n> > +\t\t\t\t     << \" contrast \" << contrast\n> > +\t\t\t\t     << \" expected \" << expected\n> > +\t\t\t\t     << endl;\n> > +\t\t\t\treturn TestFail;\n> > +\t\t\t}\n> > +\n> > +\t\t\texpected = i < 1 ? expected : value - 1;\n> \n> Why do I expect this to fail ?\n> \n> expected = 200\n> 0:\n>         value = 10;\n>         set(BRIGHTNESS, 10);\n>         set(CONTRAST, 10);\n> \n>         frameStart(0);\n>         brightness = 200;\n>         contrast = 200;\n>         (200 == 200; 200 == 200);\n>         expected = 200;\n> \n> 1:\n>         value = 11;\n>         set(BRIGHTNESS, 11);\n>         set(CONTRAST, 11);\n> \n>         frameStart(1);\n>         brightness = 10; <- delay was 1\n>         contrast = 200;  <- delay was 2\n>         (10 != 200) <- Shouldn't this fail ?\n> \n>         expected = 10;\n\nI agree it's confusing :-) The idea of DelayedControls is not to apply \nthe controls as soon as possible but to sync the setting of all controls \nof a group to the worst delay.\n\nThe flow is:\n\nexpected = 200\n0:\n        value = 10;\n        set(BRIGHTNESS, 10);\n        set(CONTRAST, 10);\n\n        frameStart(0);\n        brightness = 200;\n        contrast = 200;\n        (200 == 200; 200 == 200);\n        expected = 200;\n\n1:\n        value = 11;\n        set(BRIGHTNESS, 11);\n        set(CONTRAST, 11);\n\n        frameStart(1);\n        brightness = 200; <- brightness delay is 1 but for the group 2\n        contrast = 200;  <- delay was 2\n        (200 == 200; 200 == 200);\n        expected = 10;\n\n2:\n        value = 12;\n        set(BRIGHTNESS, 12);\n        set(CONTRAST, 12);\n\n        frameStart(2);\n        brightness = 10;\n        contrast = 10;\n        (10 == 10; 10 == 10);\n        expected = 11;\n\n3:\n        value = 13;\n        set(BRIGHTNESS, 13);\n        set(CONTRAST, 13);\n\n        frameStart(3);\n        brightness = 11;\n        contrast = 11;\n        (11 == 11; 11 == 11);\n        expected = 12;\n\n> \n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tint dualControlsMultiQueue()\n> > +\t{\n> > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> > +\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> > +\t\t};\n> > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> > +\n> > +\t\tControlList ctrls;\n> > +\n> > +\t\t/* Reset control to value that will be first two frames in test. */\n> > +\t\tint32_t expected = 100;\n> > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> > +\t\tctrls.set(V4L2_CID_CONTRAST, expected);\n> > +\t\tdelayed->reset(&ctrls);\n> > +\n> > +\t\t/*\n> > +\t\t * Queue all controls before any fake frame start. Note we\n> > +\t\t * can't queue up more then the delayed controls history size\n> > +\t\t * which is 16. Where one spot is used by the reset control.\n> > +\t\t */\n> > +\t\tfor (int32_t i = 0; i < 15; i++) {\n> > +\t\t\tint32_t value = 10 + i;\n> > +\n> > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > +\t\t\tctrls.set(V4L2_CID_CONTRAST, value);\n> > +\t\t\tdelayed->push(ctrls);\n> > +\t\t}\n> > +\n> > +\t\t/* Process all queued controls. */\n> > +\t\tfor (int32_t i = 0; i < 16; i++) {\n> > +\t\t\tint32_t value = 10 + i;\n> > +\n> > +\t\t\tdelayed->frameStart(i);\n> > +\n> > +\t\t\tControlList result = delayed->get(i);\n> > +\n> > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > +\t\t\tint32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();\n> > +\t\t\tif (brightness != expected || contrast != expected) {\n> > +\t\t\t\tcerr << \"Failed multi queue\"\n> > +\t\t\t\t     << \" frame \" << i\n> > +\t\t\t\t     << \" brightness \" << brightness\n> > +\t\t\t\t     << \" contrast \" << contrast\n> > +\t\t\t\t     << \" expected \" << expected\n> > +\t\t\t\t     << endl;\n> > +\t\t\t\treturn TestFail;\n> > +\t\t\t}\n> > +\n> > +\t\t\texpected = i < 1 ? expected : value - 1;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tint run()\n> > +\t{\n> > +\t\tint ret;\n> > +\n> > +\t\t/* Test single control without delay. */\n> > +\t\tret = singleControlNoDelay();\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\t/* Test single control with delay. */\n> > +\t\tret = singleControlWithDelay();\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\t/* Test dual controls with different delays. */\n> > +\t\tret = dualControlsWithDelay(0);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\t/* Test dual controls with non-zero sequence start. */\n> > +\t\tret = dualControlsWithDelay(10000);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\t/* Test dual controls with sequence number wraparound. */\n> > +\t\tret = dualControlsWithDelay(UINT32_MAX - 50);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\t/* Test control values produced faster then consumed. */\n> > +\t\tret = dualControlsMultiQueue();\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> \n> Maybe an empty line ?\n\nYes :-)\n\n> \n> Thanks\n>   j\n> \n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tvoid cleanup()\n> > +\t{\n> > +\t\tdelete dev_;\n> > +\t}\n> > +\n> > +private:\n> > +\tstd::unique_ptr<DeviceEnumerator> enumerator_;\n> > +\tstd::shared_ptr<MediaDevice> media_;\n> > +\tV4L2VideoDevice *dev_;\n> > +};\n> > +\n> > +TEST_REGISTER(DelayedControlsTest)\n> > diff --git a/test/meson.build b/test/meson.build\n> > index 0a1d434e399641bb..a683a657a439b4ff 100644\n> > --- a/test/meson.build\n> > +++ b/test/meson.build\n> > @@ -25,6 +25,7 @@ public_tests = [\n> >  internal_tests = [\n> >      ['byte-stream-buffer',              'byte-stream-buffer.cpp'],\n> >      ['camera-sensor',                   'camera-sensor.cpp'],\n> > +    ['delayed_contols',                 'delayed_contols.cpp'],\n> >      ['event',                           'event.cpp'],\n> >      ['event-dispatcher',                'event-dispatcher.cpp'],\n> >      ['event-thread',                    'event-thread.cpp'],\n> > --\n> > 2.29.2\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BFDA6BE08A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 23 Nov 2020 21:17:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D2CC633F4;\n\tMon, 23 Nov 2020 22:17:29 +0100 (CET)","from mail-lf1-x135.google.com (mail-lf1-x135.google.com\n\t[IPv6:2a00:1450:4864:20::135])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 384ED631D0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Nov 2020 22:17:28 +0100 (CET)","by mail-lf1-x135.google.com with SMTP id u18so25781923lfd.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Nov 2020 13:17:28 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tf25sm1499822lfc.234.2020.11.23.13.17.26\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 23 Nov 2020 13:17:26 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"uOWSmTz6\"; dkim-atps=neutral","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=Gc61FjRmTlgudbwMy3cVhnrMVltvFiF/6My9lL6pcis=;\n\tb=uOWSmTz6ynWlHGWz9N6luqW3dJ7D7TD4xvlz7Aon58F0WRzLqhtlD0mE5f6dP9bwVP\n\tIbCGPFOuhBm38KZYjSlQNDEcs2iiImvS1un4hbcOlHbK8Jr80bq6e1ndNIhlI6+kwwT2\n\t/O2o2hYSstBZOWppPsJiF0PCXInyuQkO7T/n1QSyFVsRrvvmCR5jbQ/gx63ZsenGZ1u6\n\tiiNzsUTtLU25fMAvAiDKgtCpzwuJ5ThTSQIS8svMv6JdONM4f+KyMJSYwpr2zeH4WeSe\n\t4KlR7R4cWND8j/OTuRIGwy450dDri02fuZTT0CqwIISza8gf9u4HoDK7dxpsdJ1B5z87\n\thtBQ==","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=Gc61FjRmTlgudbwMy3cVhnrMVltvFiF/6My9lL6pcis=;\n\tb=F4/a/go4nQCQAPlON3vX6Tk9pQELmS//T65/71fMOwKXffSNxoeATtKvwTUH9CMxbl\n\tIjEg0+qzJ5sqUvWIbujnr9H81HWvq46wvo4gpLhHI5223fRSeI9p6EWaIb2qWofTnJma\n\tKmAdATTo1QWlD7oCptPJ2JY+EOVTxcDiVO7p0ZMQq+03fFmHkPdfAvk3jlJWRbQzvdxd\n\tU7IvihA8O+B/4M2an8wv9CDq6mfJ/lXQUVRV+u5d+2bOWXeIQm5KXbTEhrpfvyQssR/V\n\tG4RsCktlvaJV7lejOiYHGgLItKIXTj5ciawRPJWITvDFi68zw+K1/sfEg02c39WRiFz6\n\tdTxA==","X-Gm-Message-State":"AOAM533LzqTU0crFgUJbQRKI94Syr8qmQmyxThorpiy2IXmXc3qno4K+\n\tdg/FjjRXaAWFyXc+d3y3gBApTg==","X-Google-Smtp-Source":"ABdhPJz4ngaXGKCMLVIEuVqUekh4IAAJNIWzZqIwWhK33JI/zJQEBuBGDTMusjK57U23DLDE+1agZg==","X-Received":"by 2002:ac2:5466:: with SMTP id e6mr468941lfn.17.1606166247353; \n\tMon, 23 Nov 2020 13:17:27 -0800 (PST)","Date":"Mon, 23 Nov 2020 22:17:25 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20201123211725.GF1773213@oden.dyn.berto.se>","References":"<20201110002710.3233696-1-niklas.soderlund@ragnatech.se>\n\t<20201110002710.3233696-3-niklas.soderlund@ragnatech.se>\n\t<20201118141253.6edepzajgokp5d5w@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201118141253.6edepzajgokp5d5w@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 2/8] test: delayed_controls: Add\n\ttest case for DelayedControls","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13844,"web_url":"https://patchwork.libcamera.org/comment/13844/","msgid":"<20201124113118.ko77inlt4hhwcexu@uno.localdomain>","date":"2020-11-24T11:31:18","subject":"Re: [libcamera-devel] [PATCH v2 2/8] test: delayed_controls: Add\n\ttest case for DelayedControls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Mon, Nov 23, 2020 at 10:17:25PM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your feedback.\n>\n> On 2020-11-18 15:12:53 +0100, Jacopo Mondi wrote:\n> > Hi Niklas,\n> >\n> > On Tue, Nov 10, 2020 at 01:27:04AM +0100, Niklas Söderlund wrote:\n> > > Add a test-case for DelayedControls that exercise the setting of\n> > > controls and reading back what controls where used for a particular\n> > > frame. Also exercise corner case such as a V4L2 devices that do not\n> > > reset it sequence number to 0 at stream on and sequence number wrapping\n> >\n> > s/it/its\n> >\n> > > around the uint32_t value space.\n> > >\n> > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > ---\n> > >  test/delayed_contols.cpp | 307 +++++++++++++++++++++++++++++++++++++++\n> > >  test/meson.build         |   1 +\n> > >  2 files changed, 308 insertions(+)\n> > >  create mode 100644 test/delayed_contols.cpp\n> > >\n> > > diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp\n> > > new file mode 100644\n> > > index 0000000000000000..e71da6662c30bbdc\n> > > --- /dev/null\n> > > +++ b/test/delayed_contols.cpp\n> > > @@ -0,0 +1,307 @@\n> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > > +/*\n> > > + * Copyright (C) 2020, Google Inc.\n> > > + *\n> > > + * libcamera delayed controls tests\n> > > + */\n> > > +\n> > > +#include <iostream>\n> > > +\n> > > +#include \"libcamera/internal/delayed_controls.h\"\n> > > +#include \"libcamera/internal/device_enumerator.h\"\n> > > +#include \"libcamera/internal/media_device.h\"\n> > > +#include \"libcamera/internal/v4l2_videodevice.h\"\n> > > +\n> > > +#include \"test.h\"\n> > > +\n> > > +using namespace std;\n> > > +using namespace libcamera;\n> > > +\n> > > +class DelayedControlsTest : public Test\n> > > +{\n> > > +public:\n> > > +\tDelayedControlsTest()\n> > > +\t\t: dev_(nullptr)\n> > > +\t{\n> > > +\t}\n> > > +\n> > > +\tint init()\n> > > +\t{\n> > > +\t\tenumerator_ = DeviceEnumerator::create();\n> > > +\t\tif (!enumerator_) {\n> > > +\t\t\tcerr << \"Failed to create device enumerator\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\tif (enumerator_->enumerate()) {\n> > > +\t\t\tcerr << \"Failed to enumerate media devices\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\tDeviceMatch dm(\"vivid\");\n> > > +\t\tdm.add(\"vivid-000-vid-cap\");\n> > > +\n> > > +\t\tmedia_ = enumerator_->search(dm);\n> > > +\t\tif (!media_) {\n> > > +\t\t\tcerr << \"vivid video device found\" << endl;\n> > > +\t\t\treturn TestSkip;\n> > > +\t\t}\n> > > +\n> > > +\t\tMediaEntity *entity = media_->getEntityByName(\"vivid-000-vid-cap\");\n> > > +\t\tdev_ = new V4L2VideoDevice(entity->deviceNode());\n> > > +\t\tif (dev_->open()) {\n> > > +\t\t\tcerr << \"Failed to open video device\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\tconst ControlInfoMap &infoMap = dev_->controls();\n> > > +\n> > > +\t\t/* Test control enumeration. */\n> > > +\t\tif (infoMap.empty()) {\n> > > +\t\t\tcerr << \"Failed to enumerate controls\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\tif (infoMap.find(V4L2_CID_BRIGHTNESS) == infoMap.end() ||\n> > > +\t\t    infoMap.find(V4L2_CID_CONTRAST) == infoMap.end()) {\n> > > +\t\t\tcerr << \"Missing controls\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\tControlList ctrls = dev_->getControls({ V4L2_CID_BRIGHTNESS });\n> >\n> > What's ctrls for here ?\n>\n> Good catch, I will remove it for next version.\n>\n> >\n> > > +\n> > > +\t\treturn TestPass;\n> > > +\t}\n> > > +\n> > > +\tint singleControlNoDelay()\n> > > +\t{\n> > > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > > +\t\t\t{ V4L2_CID_BRIGHTNESS, 0 },\n> > > +\t\t};\n> > > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> >\n> > Should this just be\n> >                 DelayedControls delayed(dev_, delays) ?\n> > it's not used outside of the function\n>\n> It could, I have no real preference. I picked this as I wanted to test\n> some things during development and then I left it as such. Do you see an\n> particular gain in reworking this?\n>\n\nIt is more that I don't see any gain in using a unique_ptr. The scope\nof the variable is local to the function, there's no lifetime\nmanagement at play, so allocating it on the stack has the same exact\neffect (I won't go and mention the overhead related to using smart\npointer in term of performaces/memory occupation, as I cannot quantify\nthem for real and this is a test, so it's not a big deal at all).\n\n> >\n> > > +\n> >\n> > Additional empty line\n>\n> Thanks.\n>\n> >\n> > > +\t\tControlList ctrls;\n> >\n> > Do we want to create the ControlList with the device info map for\n> > additional validation ?\n> >                 ControlList ctrl(dev_->controls());\n>\n> We could, but I don't see the value in it as the goal is not to test\n> ControlList. The controls are carefully selected to have a value range\n> of [min=0 max=255 step=1]. If this ever changes this test will fail in\n> other ways.\n\nWell, it will fail as you use hardcoded 1 and 255. If they're replaced\nby min() and max() the test should be able to pass even if the\nunderlying driver implementation changes.\n\n>\n> >\n> > > +\n> > > +\t\t/* Reset control to value not used in test. */\n> > > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, 1);\n> > > +\t\tdelayed->reset(&ctrls);\n> >\n> > I would use the ControlInfo.min()\n> >\n> > Do you expect after reset() that BRIGHTNESS is equal to 1 ?\n>\n> Yes.\n>\n\nUnlikely to change, but I would avoid using hardcoded value.\nAlso setting it to min() makes the code more explicit.\n\n> >\n> > > +\n> > > +\t\t/* Test control without delay are set at once. */\n> > > +\t\tfor (int32_t i = 0; i < 10; i++) {\n> > > +\t\t\tint32_t value = 100 + i;\n> > > +\n> > > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > > +\t\t\tdelayed->push(ctrls);\n> > > +\n> > > +\t\t\tdelayed->frameStart(i);\n> > > +\n> > > +\t\t\tControlList result = delayed->get(i);\n> > > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > > +\t\t\tif (brightness != value) {\n> > > +\t\t\t\tcerr << \"Failed single control without delay\"\n> > > +\t\t\t\t     << \" frame \" << i\n> > > +\t\t\t\t     << \" expected \" << value\n> > > +\t\t\t\t     << \" got \" << brightness\n> > > +\t\t\t\t     << endl;\n> > > +\t\t\t\treturn TestFail;\n> > > +\t\t\t}\n> > > +\t\t}\n> > > +\n> > > +\t\treturn TestPass;\n> > > +\t}\n> > > +\n> > > +\tint singleControlWithDelay()\n> > > +\t{\n> > > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > > +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> > > +\t\t};\n> > > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> > > +\n> > > +\t\tControlList ctrls;\n> >\n> > Same questions\n> >\n> > > +\n> > > +\t\t/* Reset control to value that will be first in test. */\n> > > +\t\tint32_t expected = 4;\n> > > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> > > +\t\tdelayed->reset(&ctrls);\n> > > +\n> > > +\t\t/* Test single control with delay. */\n> > > +\t\tfor (int32_t i = 0; i < 100; i++) {\n> > > +\t\t\tint32_t value = 10 + i;\n> >\n> > Is swapping 10 and 100 in the for loop compared to the previous test\n> > intended ?\n>\n> No it should be 100 in the previous test.\n>\n> >\n> > > +\n> > > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > > +\t\t\tdelayed->push(ctrls);\n> > > +\n> > > +\t\t\tdelayed->frameStart(i);\n> > > +\n> > > +\t\t\tControlList result = delayed->get(i);\n> > > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > > +\t\t\tif (brightness != expected) {\n> >\n> > Are we're lucky and BRIGHTNESS.min() >= 4 ?\n>\n> I'm sorry I don't think I understand this question fully I think. We are\n> not lucky here, before the loop we set the control to 4 and this is\n> whats tested here in the first iteration. But the vale 4 is arbitrary\n> and we can set it to any value between 0 and 255 before the loop and the\n> test works.\n>\n\nWhat I mean was probably \"are we lucky and BRIGHTNESS.min() != 4\".\nI would use a generic min() + 10 to make sure the test adapts if the\ndriver changes.\n\n> >\n> > > +\t\t\t\tcerr << \"Failed single control with delay\"\n> > > +\t\t\t\t     << \" frame \" << i\n> > > +\t\t\t\t     << \" expected \" << expected\n> > > +\t\t\t\t     << \" got \" << brightness\n> > > +\t\t\t\t     << endl;\n> > > +\t\t\t\treturn TestFail;\n> > > +\t\t\t}\n> > > +\n> > > +\t\t\texpected = value;\n> > > +\t\t}\n> > > +\n> > > +\t\treturn TestPass;\n> > > +\t}\n> > > +\n> > > +\tint dualControlsWithDelay(uint32_t startOffset)\n> > > +\t{\n> > > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > > +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> > > +\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> > > +\t\t};\n> > > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> > > +\n> > > +\t\tControlList ctrls;\n> > > +\n> >\n> > Same :)\n> >\n> > > +\t\t/* Reset control to value that will be first two frames in test. */\n> > > +\t\tint32_t expected = 200;\n> > > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> > > +\t\tctrls.set(V4L2_CID_CONTRAST, expected);\n> > > +\t\tdelayed->reset(&ctrls);\n> > > +\n> > > +\t\t/* Test dual control with delay. */\n> > > +\t\tfor (int32_t i = 0; i < 100; i++) {\n> > > +\t\t\tuint32_t frame = startOffset + i;\n> > > +\n> >\n> > Additional empty line\n> >\n> > > +\t\t\tint32_t value = 10 + i;\n> > > +\n> > > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > > +\t\t\tctrls.set(V4L2_CID_CONTRAST, value);\n> > > +\t\t\tdelayed->push(ctrls);\n> > > +\n> > > +\t\t\tdelayed->frameStart(frame);\n> > > +\n> > > +\t\t\tControlList result = delayed->get(frame);\n> > > +\n> >\n> > In the previous functions you don't have an empty line here\n> >\n> > > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > > +\t\t\tint32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();\n> > > +\t\t\tif (brightness != expected || contrast != expected) {\n> > > +\t\t\t\tcerr << \"Failed dual controls\"\n> > > +\t\t\t\t     << \" frame \" << frame\n> > > +\t\t\t\t     << \" brightness \" << brightness\n> > > +\t\t\t\t     << \" contrast \" << contrast\n> > > +\t\t\t\t     << \" expected \" << expected\n> > > +\t\t\t\t     << endl;\n> > > +\t\t\t\treturn TestFail;\n> > > +\t\t\t}\n> > > +\n> > > +\t\t\texpected = i < 1 ? expected : value - 1;\n> >\n> > Why do I expect this to fail ?\n> >\n> > expected = 200\n> > 0:\n> >         value = 10;\n> >         set(BRIGHTNESS, 10);\n> >         set(CONTRAST, 10);\n> >\n> >         frameStart(0);\n> >         brightness = 200;\n> >         contrast = 200;\n> >         (200 == 200; 200 == 200);\n> >         expected = 200;\n> >\n> > 1:\n> >         value = 11;\n> >         set(BRIGHTNESS, 11);\n> >         set(CONTRAST, 11);\n> >\n> >         frameStart(1);\n> >         brightness = 10; <- delay was 1\n> >         contrast = 200;  <- delay was 2\n> >         (10 != 200) <- Shouldn't this fail ?\n> >\n> >         expected = 10;\n>\n> I agree it's confusing :-) The idea of DelayedControls is not to apply\n> the controls as soon as possible but to sync the setting of all controls\n> of a group to the worst delay.\n>\n> The flow is:\n>\n> expected = 200\n> 0:\n>         value = 10;\n>         set(BRIGHTNESS, 10);\n>         set(CONTRAST, 10);\n>\n>         frameStart(0);\n>         brightness = 200;\n>         contrast = 200;\n>         (200 == 200; 200 == 200);\n>         expected = 200;\n>\n> 1:\n>         value = 11;\n>         set(BRIGHTNESS, 11);\n>         set(CONTRAST, 11);\n>\n>         frameStart(1);\n>         brightness = 200; <- brightness delay is 1 but for the group 2\n>         contrast = 200;  <- delay was 2\n>         (200 == 200; 200 == 200);\n>         expected = 10;\n>\n> 2:\n>         value = 12;\n>         set(BRIGHTNESS, 12);\n>         set(CONTRAST, 12);\n>\n>         frameStart(2);\n>         brightness = 10;\n>         contrast = 10;\n>         (10 == 10; 10 == 10);\n>         expected = 11;\n>\n> 3:\n>         value = 13;\n>         set(BRIGHTNESS, 13);\n>         set(CONTRAST, 13);\n>\n>         frameStart(3);\n>         brightness = 11;\n>         contrast = 11;\n>         (11 == 11; 11 == 11);\n>         expected = 12;\n>\n\nThanks, I will run a few iterations and better understand this.\n\nThanks\n  j\n\n> >\n> > > +\t\t}\n> > > +\n> > > +\t\treturn TestPass;\n> > > +\t}\n> > > +\n> > > +\tint dualControlsMultiQueue()\n> > > +\t{\n> > > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > > +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> > > +\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> > > +\t\t};\n> > > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> > > +\n> > > +\t\tControlList ctrls;\n> > > +\n> > > +\t\t/* Reset control to value that will be first two frames in test. */\n> > > +\t\tint32_t expected = 100;\n> > > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> > > +\t\tctrls.set(V4L2_CID_CONTRAST, expected);\n> > > +\t\tdelayed->reset(&ctrls);\n> > > +\n> > > +\t\t/*\n> > > +\t\t * Queue all controls before any fake frame start. Note we\n> > > +\t\t * can't queue up more then the delayed controls history size\n> > > +\t\t * which is 16. Where one spot is used by the reset control.\n> > > +\t\t */\n> > > +\t\tfor (int32_t i = 0; i < 15; i++) {\n> > > +\t\t\tint32_t value = 10 + i;\n> > > +\n> > > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > > +\t\t\tctrls.set(V4L2_CID_CONTRAST, value);\n> > > +\t\t\tdelayed->push(ctrls);\n> > > +\t\t}\n> > > +\n> > > +\t\t/* Process all queued controls. */\n> > > +\t\tfor (int32_t i = 0; i < 16; i++) {\n> > > +\t\t\tint32_t value = 10 + i;\n> > > +\n> > > +\t\t\tdelayed->frameStart(i);\n> > > +\n> > > +\t\t\tControlList result = delayed->get(i);\n> > > +\n> > > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > > +\t\t\tint32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();\n> > > +\t\t\tif (brightness != expected || contrast != expected) {\n> > > +\t\t\t\tcerr << \"Failed multi queue\"\n> > > +\t\t\t\t     << \" frame \" << i\n> > > +\t\t\t\t     << \" brightness \" << brightness\n> > > +\t\t\t\t     << \" contrast \" << contrast\n> > > +\t\t\t\t     << \" expected \" << expected\n> > > +\t\t\t\t     << endl;\n> > > +\t\t\t\treturn TestFail;\n> > > +\t\t\t}\n> > > +\n> > > +\t\t\texpected = i < 1 ? expected : value - 1;\n> > > +\t\t}\n> > > +\n> > > +\t\treturn TestPass;\n> > > +\t}\n> > > +\n> > > +\tint run()\n> > > +\t{\n> > > +\t\tint ret;\n> > > +\n> > > +\t\t/* Test single control without delay. */\n> > > +\t\tret = singleControlNoDelay();\n> > > +\t\tif (ret)\n> > > +\t\t\treturn ret;\n> > > +\n> > > +\t\t/* Test single control with delay. */\n> > > +\t\tret = singleControlWithDelay();\n> > > +\t\tif (ret)\n> > > +\t\t\treturn ret;\n> > > +\n> > > +\t\t/* Test dual controls with different delays. */\n> > > +\t\tret = dualControlsWithDelay(0);\n> > > +\t\tif (ret)\n> > > +\t\t\treturn ret;\n> > > +\n> > > +\t\t/* Test dual controls with non-zero sequence start. */\n> > > +\t\tret = dualControlsWithDelay(10000);\n> > > +\t\tif (ret)\n> > > +\t\t\treturn ret;\n> > > +\n> > > +\t\t/* Test dual controls with sequence number wraparound. */\n> > > +\t\tret = dualControlsWithDelay(UINT32_MAX - 50);\n> > > +\t\tif (ret)\n> > > +\t\t\treturn ret;\n> > > +\n> > > +\t\t/* Test control values produced faster then consumed. */\n> > > +\t\tret = dualControlsMultiQueue();\n> > > +\t\tif (ret)\n> > > +\t\t\treturn ret;\n> >\n> > Maybe an empty line ?\n>\n> Yes :-)\n>\n> >\n> > Thanks\n> >   j\n> >\n> > > +\t\treturn TestPass;\n> > > +\t}\n> > > +\n> > > +\tvoid cleanup()\n> > > +\t{\n> > > +\t\tdelete dev_;\n> > > +\t}\n> > > +\n> > > +private:\n> > > +\tstd::unique_ptr<DeviceEnumerator> enumerator_;\n> > > +\tstd::shared_ptr<MediaDevice> media_;\n> > > +\tV4L2VideoDevice *dev_;\n> > > +};\n> > > +\n> > > +TEST_REGISTER(DelayedControlsTest)\n> > > diff --git a/test/meson.build b/test/meson.build\n> > > index 0a1d434e399641bb..a683a657a439b4ff 100644\n> > > --- a/test/meson.build\n> > > +++ b/test/meson.build\n> > > @@ -25,6 +25,7 @@ public_tests = [\n> > >  internal_tests = [\n> > >      ['byte-stream-buffer',              'byte-stream-buffer.cpp'],\n> > >      ['camera-sensor',                   'camera-sensor.cpp'],\n> > > +    ['delayed_contols',                 'delayed_contols.cpp'],\n> > >      ['event',                           'event.cpp'],\n> > >      ['event-dispatcher',                'event-dispatcher.cpp'],\n> > >      ['event-thread',                    'event-thread.cpp'],\n> > > --\n> > > 2.29.2\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 318F1BE176\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 24 Nov 2020 11:31:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B834A633F8;\n\tTue, 24 Nov 2020 12:31:17 +0100 (CET)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C239260335\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 24 Nov 2020 12:31:15 +0100 (CET)","from uno.localdomain (host-79-25-179-116.retail.telecomitalia.it\n\t[79.25.179.116]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 16E78C0009;\n\tTue, 24 Nov 2020 11:31:13 +0000 (UTC)"],"X-Originating-IP":"79.25.179.116","Date":"Tue, 24 Nov 2020 12:31:18 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20201124113118.ko77inlt4hhwcexu@uno.localdomain>","References":"<20201110002710.3233696-1-niklas.soderlund@ragnatech.se>\n\t<20201110002710.3233696-3-niklas.soderlund@ragnatech.se>\n\t<20201118141253.6edepzajgokp5d5w@uno.localdomain>\n\t<20201123211725.GF1773213@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201123211725.GF1773213@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v2 2/8] test: delayed_controls: Add\n\ttest case for DelayedControls","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13846,"web_url":"https://patchwork.libcamera.org/comment/13846/","msgid":"<20201124114630.GA2162566@oden.dyn.berto.se>","date":"2020-11-24T11:46:30","subject":"Re: [libcamera-devel] [PATCH v2 2/8] test: delayed_controls: Add\n\ttest case for DelayedControls","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nOn 2020-11-24 12:31:18 +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Mon, Nov 23, 2020 at 10:17:25PM +0100, Niklas Söderlund wrote:\n> > Hi Jacopo,\n> >\n> > Thanks for your feedback.\n> >\n> > On 2020-11-18 15:12:53 +0100, Jacopo Mondi wrote:\n> > > Hi Niklas,\n> > >\n> > > On Tue, Nov 10, 2020 at 01:27:04AM +0100, Niklas Söderlund wrote:\n> > > > Add a test-case for DelayedControls that exercise the setting of\n> > > > controls and reading back what controls where used for a particular\n> > > > frame. Also exercise corner case such as a V4L2 devices that do not\n> > > > reset it sequence number to 0 at stream on and sequence number wrapping\n> > >\n> > > s/it/its\n> > >\n> > > > around the uint32_t value space.\n> > > >\n> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > > ---\n> > > >  test/delayed_contols.cpp | 307 +++++++++++++++++++++++++++++++++++++++\n> > > >  test/meson.build         |   1 +\n> > > >  2 files changed, 308 insertions(+)\n> > > >  create mode 100644 test/delayed_contols.cpp\n> > > >\n> > > > diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp\n> > > > new file mode 100644\n> > > > index 0000000000000000..e71da6662c30bbdc\n> > > > --- /dev/null\n> > > > +++ b/test/delayed_contols.cpp\n> > > > @@ -0,0 +1,307 @@\n> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2020, Google Inc.\n> > > > + *\n> > > > + * libcamera delayed controls tests\n> > > > + */\n> > > > +\n> > > > +#include <iostream>\n> > > > +\n> > > > +#include \"libcamera/internal/delayed_controls.h\"\n> > > > +#include \"libcamera/internal/device_enumerator.h\"\n> > > > +#include \"libcamera/internal/media_device.h\"\n> > > > +#include \"libcamera/internal/v4l2_videodevice.h\"\n> > > > +\n> > > > +#include \"test.h\"\n> > > > +\n> > > > +using namespace std;\n> > > > +using namespace libcamera;\n> > > > +\n> > > > +class DelayedControlsTest : public Test\n> > > > +{\n> > > > +public:\n> > > > +\tDelayedControlsTest()\n> > > > +\t\t: dev_(nullptr)\n> > > > +\t{\n> > > > +\t}\n> > > > +\n> > > > +\tint init()\n> > > > +\t{\n> > > > +\t\tenumerator_ = DeviceEnumerator::create();\n> > > > +\t\tif (!enumerator_) {\n> > > > +\t\t\tcerr << \"Failed to create device enumerator\" << endl;\n> > > > +\t\t\treturn TestFail;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tif (enumerator_->enumerate()) {\n> > > > +\t\t\tcerr << \"Failed to enumerate media devices\" << endl;\n> > > > +\t\t\treturn TestFail;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tDeviceMatch dm(\"vivid\");\n> > > > +\t\tdm.add(\"vivid-000-vid-cap\");\n> > > > +\n> > > > +\t\tmedia_ = enumerator_->search(dm);\n> > > > +\t\tif (!media_) {\n> > > > +\t\t\tcerr << \"vivid video device found\" << endl;\n> > > > +\t\t\treturn TestSkip;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tMediaEntity *entity = media_->getEntityByName(\"vivid-000-vid-cap\");\n> > > > +\t\tdev_ = new V4L2VideoDevice(entity->deviceNode());\n> > > > +\t\tif (dev_->open()) {\n> > > > +\t\t\tcerr << \"Failed to open video device\" << endl;\n> > > > +\t\t\treturn TestFail;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tconst ControlInfoMap &infoMap = dev_->controls();\n> > > > +\n> > > > +\t\t/* Test control enumeration. */\n> > > > +\t\tif (infoMap.empty()) {\n> > > > +\t\t\tcerr << \"Failed to enumerate controls\" << endl;\n> > > > +\t\t\treturn TestFail;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tif (infoMap.find(V4L2_CID_BRIGHTNESS) == infoMap.end() ||\n> > > > +\t\t    infoMap.find(V4L2_CID_CONTRAST) == infoMap.end()) {\n> > > > +\t\t\tcerr << \"Missing controls\" << endl;\n> > > > +\t\t\treturn TestFail;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tControlList ctrls = dev_->getControls({ V4L2_CID_BRIGHTNESS });\n> > >\n> > > What's ctrls for here ?\n> >\n> > Good catch, I will remove it for next version.\n> >\n> > >\n> > > > +\n> > > > +\t\treturn TestPass;\n> > > > +\t}\n> > > > +\n> > > > +\tint singleControlNoDelay()\n> > > > +\t{\n> > > > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > > > +\t\t\t{ V4L2_CID_BRIGHTNESS, 0 },\n> > > > +\t\t};\n> > > > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > > > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> > >\n> > > Should this just be\n> > >                 DelayedControls delayed(dev_, delays) ?\n> > > it's not used outside of the function\n> >\n> > It could, I have no real preference. I picked this as I wanted to test\n> > some things during development and then I left it as such. Do you see an\n> > particular gain in reworking this?\n> >\n> \n> It is more that I don't see any gain in using a unique_ptr. The scope\n> of the variable is local to the function, there's no lifetime\n> management at play, so allocating it on the stack has the same exact\n> effect (I won't go and mention the overhead related to using smart\n> pointer in term of performaces/memory occupation, as I cannot quantify\n> them for real and this is a test, so it's not a big deal at all).\n\nI can rework it for v4 if it bothers you :-)\n\n> \n> > >\n> > > > +\n> > >\n> > > Additional empty line\n> >\n> > Thanks.\n> >\n> > >\n> > > > +\t\tControlList ctrls;\n> > >\n> > > Do we want to create the ControlList with the device info map for\n> > > additional validation ?\n> > >                 ControlList ctrl(dev_->controls());\n> >\n> > We could, but I don't see the value in it as the goal is not to test\n> > ControlList. The controls are carefully selected to have a value range\n> > of [min=0 max=255 step=1]. If this ever changes this test will fail in\n> > other ways.\n> \n> Well, it will fail as you use hardcoded 1 and 255. If they're replaced\n> by min() and max() the test should be able to pass even if the\n> underlying driver implementation changes.\n\nYes and No ;-)\n\nIt will fail if the value space is not 0 - 255 as a lot of the loops \nused in this whole file depends on it to not go out of bounds. I agree \nif it was only here and in the example below I would switch to min() but \nas the whole file depends on [min=0 max=255 step=1] I think this is \neasier to read then mixing the two styles.\n\n> \n> >\n> > >\n> > > > +\n> > > > +\t\t/* Reset control to value not used in test. */\n> > > > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, 1);\n> > > > +\t\tdelayed->reset(&ctrls);\n> > >\n> > > I would use the ControlInfo.min()\n> > >\n> > > Do you expect after reset() that BRIGHTNESS is equal to 1 ?\n> >\n> > Yes.\n> >\n> \n> Unlikely to change, but I would avoid using hardcoded value.\n> Also setting it to min() makes the code more explicit.\n> \n> > >\n> > > > +\n> > > > +\t\t/* Test control without delay are set at once. */\n> > > > +\t\tfor (int32_t i = 0; i < 10; i++) {\n> > > > +\t\t\tint32_t value = 100 + i;\n> > > > +\n> > > > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > > > +\t\t\tdelayed->push(ctrls);\n> > > > +\n> > > > +\t\t\tdelayed->frameStart(i);\n> > > > +\n> > > > +\t\t\tControlList result = delayed->get(i);\n> > > > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > > > +\t\t\tif (brightness != value) {\n> > > > +\t\t\t\tcerr << \"Failed single control without delay\"\n> > > > +\t\t\t\t     << \" frame \" << i\n> > > > +\t\t\t\t     << \" expected \" << value\n> > > > +\t\t\t\t     << \" got \" << brightness\n> > > > +\t\t\t\t     << endl;\n> > > > +\t\t\t\treturn TestFail;\n> > > > +\t\t\t}\n> > > > +\t\t}\n> > > > +\n> > > > +\t\treturn TestPass;\n> > > > +\t}\n> > > > +\n> > > > +\tint singleControlWithDelay()\n> > > > +\t{\n> > > > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > > > +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> > > > +\t\t};\n> > > > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > > > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> > > > +\n> > > > +\t\tControlList ctrls;\n> > >\n> > > Same questions\n> > >\n> > > > +\n> > > > +\t\t/* Reset control to value that will be first in test. */\n> > > > +\t\tint32_t expected = 4;\n> > > > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> > > > +\t\tdelayed->reset(&ctrls);\n> > > > +\n> > > > +\t\t/* Test single control with delay. */\n> > > > +\t\tfor (int32_t i = 0; i < 100; i++) {\n> > > > +\t\t\tint32_t value = 10 + i;\n> > >\n> > > Is swapping 10 and 100 in the for loop compared to the previous test\n> > > intended ?\n> >\n> > No it should be 100 in the previous test.\n> >\n> > >\n> > > > +\n> > > > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > > > +\t\t\tdelayed->push(ctrls);\n> > > > +\n> > > > +\t\t\tdelayed->frameStart(i);\n> > > > +\n> > > > +\t\t\tControlList result = delayed->get(i);\n> > > > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > > > +\t\t\tif (brightness != expected) {\n> > >\n> > > Are we're lucky and BRIGHTNESS.min() >= 4 ?\n> >\n> > I'm sorry I don't think I understand this question fully I think. We are\n> > not lucky here, before the loop we set the control to 4 and this is\n> > whats tested here in the first iteration. But the vale 4 is arbitrary\n> > and we can set it to any value between 0 and 255 before the loop and the\n> > test works.\n> >\n> \n> What I mean was probably \"are we lucky and BRIGHTNESS.min() != 4\".\n> I would use a generic min() + 10 to make sure the test adapts if the\n> driver changes.\n> \n> > >\n> > > > +\t\t\t\tcerr << \"Failed single control with delay\"\n> > > > +\t\t\t\t     << \" frame \" << i\n> > > > +\t\t\t\t     << \" expected \" << expected\n> > > > +\t\t\t\t     << \" got \" << brightness\n> > > > +\t\t\t\t     << endl;\n> > > > +\t\t\t\treturn TestFail;\n> > > > +\t\t\t}\n> > > > +\n> > > > +\t\t\texpected = value;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\treturn TestPass;\n> > > > +\t}\n> > > > +\n> > > > +\tint dualControlsWithDelay(uint32_t startOffset)\n> > > > +\t{\n> > > > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > > > +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> > > > +\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> > > > +\t\t};\n> > > > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > > > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> > > > +\n> > > > +\t\tControlList ctrls;\n> > > > +\n> > >\n> > > Same :)\n> > >\n> > > > +\t\t/* Reset control to value that will be first two frames in test. */\n> > > > +\t\tint32_t expected = 200;\n> > > > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> > > > +\t\tctrls.set(V4L2_CID_CONTRAST, expected);\n> > > > +\t\tdelayed->reset(&ctrls);\n> > > > +\n> > > > +\t\t/* Test dual control with delay. */\n> > > > +\t\tfor (int32_t i = 0; i < 100; i++) {\n> > > > +\t\t\tuint32_t frame = startOffset + i;\n> > > > +\n> > >\n> > > Additional empty line\n> > >\n> > > > +\t\t\tint32_t value = 10 + i;\n> > > > +\n> > > > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > > > +\t\t\tctrls.set(V4L2_CID_CONTRAST, value);\n> > > > +\t\t\tdelayed->push(ctrls);\n> > > > +\n> > > > +\t\t\tdelayed->frameStart(frame);\n> > > > +\n> > > > +\t\t\tControlList result = delayed->get(frame);\n> > > > +\n> > >\n> > > In the previous functions you don't have an empty line here\n> > >\n> > > > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > > > +\t\t\tint32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();\n> > > > +\t\t\tif (brightness != expected || contrast != expected) {\n> > > > +\t\t\t\tcerr << \"Failed dual controls\"\n> > > > +\t\t\t\t     << \" frame \" << frame\n> > > > +\t\t\t\t     << \" brightness \" << brightness\n> > > > +\t\t\t\t     << \" contrast \" << contrast\n> > > > +\t\t\t\t     << \" expected \" << expected\n> > > > +\t\t\t\t     << endl;\n> > > > +\t\t\t\treturn TestFail;\n> > > > +\t\t\t}\n> > > > +\n> > > > +\t\t\texpected = i < 1 ? expected : value - 1;\n> > >\n> > > Why do I expect this to fail ?\n> > >\n> > > expected = 200\n> > > 0:\n> > >         value = 10;\n> > >         set(BRIGHTNESS, 10);\n> > >         set(CONTRAST, 10);\n> > >\n> > >         frameStart(0);\n> > >         brightness = 200;\n> > >         contrast = 200;\n> > >         (200 == 200; 200 == 200);\n> > >         expected = 200;\n> > >\n> > > 1:\n> > >         value = 11;\n> > >         set(BRIGHTNESS, 11);\n> > >         set(CONTRAST, 11);\n> > >\n> > >         frameStart(1);\n> > >         brightness = 10; <- delay was 1\n> > >         contrast = 200;  <- delay was 2\n> > >         (10 != 200) <- Shouldn't this fail ?\n> > >\n> > >         expected = 10;\n> >\n> > I agree it's confusing :-) The idea of DelayedControls is not to apply\n> > the controls as soon as possible but to sync the setting of all controls\n> > of a group to the worst delay.\n> >\n> > The flow is:\n> >\n> > expected = 200\n> > 0:\n> >         value = 10;\n> >         set(BRIGHTNESS, 10);\n> >         set(CONTRAST, 10);\n> >\n> >         frameStart(0);\n> >         brightness = 200;\n> >         contrast = 200;\n> >         (200 == 200; 200 == 200);\n> >         expected = 200;\n> >\n> > 1:\n> >         value = 11;\n> >         set(BRIGHTNESS, 11);\n> >         set(CONTRAST, 11);\n> >\n> >         frameStart(1);\n> >         brightness = 200; <- brightness delay is 1 but for the group 2\n> >         contrast = 200;  <- delay was 2\n> >         (200 == 200; 200 == 200);\n> >         expected = 10;\n> >\n> > 2:\n> >         value = 12;\n> >         set(BRIGHTNESS, 12);\n> >         set(CONTRAST, 12);\n> >\n> >         frameStart(2);\n> >         brightness = 10;\n> >         contrast = 10;\n> >         (10 == 10; 10 == 10);\n> >         expected = 11;\n> >\n> > 3:\n> >         value = 13;\n> >         set(BRIGHTNESS, 13);\n> >         set(CONTRAST, 13);\n> >\n> >         frameStart(3);\n> >         brightness = 11;\n> >         contrast = 11;\n> >         (11 == 11; 11 == 11);\n> >         expected = 12;\n> >\n> \n> Thanks, I will run a few iterations and better understand this.\n> \n> Thanks\n>   j\n> \n> > >\n> > > > +\t\t}\n> > > > +\n> > > > +\t\treturn TestPass;\n> > > > +\t}\n> > > > +\n> > > > +\tint dualControlsMultiQueue()\n> > > > +\t{\n> > > > +\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n> > > > +\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> > > > +\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> > > > +\t\t};\n> > > > +\t\tstd::unique_ptr<DelayedControls> delayed =\n> > > > +\t\t\tstd::make_unique<DelayedControls>(dev_, delays);\n> > > > +\n> > > > +\t\tControlList ctrls;\n> > > > +\n> > > > +\t\t/* Reset control to value that will be first two frames in test. */\n> > > > +\t\tint32_t expected = 100;\n> > > > +\t\tctrls.set(V4L2_CID_BRIGHTNESS, expected);\n> > > > +\t\tctrls.set(V4L2_CID_CONTRAST, expected);\n> > > > +\t\tdelayed->reset(&ctrls);\n> > > > +\n> > > > +\t\t/*\n> > > > +\t\t * Queue all controls before any fake frame start. Note we\n> > > > +\t\t * can't queue up more then the delayed controls history size\n> > > > +\t\t * which is 16. Where one spot is used by the reset control.\n> > > > +\t\t */\n> > > > +\t\tfor (int32_t i = 0; i < 15; i++) {\n> > > > +\t\t\tint32_t value = 10 + i;\n> > > > +\n> > > > +\t\t\tctrls.set(V4L2_CID_BRIGHTNESS, value);\n> > > > +\t\t\tctrls.set(V4L2_CID_CONTRAST, value);\n> > > > +\t\t\tdelayed->push(ctrls);\n> > > > +\t\t}\n> > > > +\n> > > > +\t\t/* Process all queued controls. */\n> > > > +\t\tfor (int32_t i = 0; i < 16; i++) {\n> > > > +\t\t\tint32_t value = 10 + i;\n> > > > +\n> > > > +\t\t\tdelayed->frameStart(i);\n> > > > +\n> > > > +\t\t\tControlList result = delayed->get(i);\n> > > > +\n> > > > +\t\t\tint32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();\n> > > > +\t\t\tint32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();\n> > > > +\t\t\tif (brightness != expected || contrast != expected) {\n> > > > +\t\t\t\tcerr << \"Failed multi queue\"\n> > > > +\t\t\t\t     << \" frame \" << i\n> > > > +\t\t\t\t     << \" brightness \" << brightness\n> > > > +\t\t\t\t     << \" contrast \" << contrast\n> > > > +\t\t\t\t     << \" expected \" << expected\n> > > > +\t\t\t\t     << endl;\n> > > > +\t\t\t\treturn TestFail;\n> > > > +\t\t\t}\n> > > > +\n> > > > +\t\t\texpected = i < 1 ? expected : value - 1;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\treturn TestPass;\n> > > > +\t}\n> > > > +\n> > > > +\tint run()\n> > > > +\t{\n> > > > +\t\tint ret;\n> > > > +\n> > > > +\t\t/* Test single control without delay. */\n> > > > +\t\tret = singleControlNoDelay();\n> > > > +\t\tif (ret)\n> > > > +\t\t\treturn ret;\n> > > > +\n> > > > +\t\t/* Test single control with delay. */\n> > > > +\t\tret = singleControlWithDelay();\n> > > > +\t\tif (ret)\n> > > > +\t\t\treturn ret;\n> > > > +\n> > > > +\t\t/* Test dual controls with different delays. */\n> > > > +\t\tret = dualControlsWithDelay(0);\n> > > > +\t\tif (ret)\n> > > > +\t\t\treturn ret;\n> > > > +\n> > > > +\t\t/* Test dual controls with non-zero sequence start. */\n> > > > +\t\tret = dualControlsWithDelay(10000);\n> > > > +\t\tif (ret)\n> > > > +\t\t\treturn ret;\n> > > > +\n> > > > +\t\t/* Test dual controls with sequence number wraparound. */\n> > > > +\t\tret = dualControlsWithDelay(UINT32_MAX - 50);\n> > > > +\t\tif (ret)\n> > > > +\t\t\treturn ret;\n> > > > +\n> > > > +\t\t/* Test control values produced faster then consumed. */\n> > > > +\t\tret = dualControlsMultiQueue();\n> > > > +\t\tif (ret)\n> > > > +\t\t\treturn ret;\n> > >\n> > > Maybe an empty line ?\n> >\n> > Yes :-)\n> >\n> > >\n> > > Thanks\n> > >   j\n> > >\n> > > > +\t\treturn TestPass;\n> > > > +\t}\n> > > > +\n> > > > +\tvoid cleanup()\n> > > > +\t{\n> > > > +\t\tdelete dev_;\n> > > > +\t}\n> > > > +\n> > > > +private:\n> > > > +\tstd::unique_ptr<DeviceEnumerator> enumerator_;\n> > > > +\tstd::shared_ptr<MediaDevice> media_;\n> > > > +\tV4L2VideoDevice *dev_;\n> > > > +};\n> > > > +\n> > > > +TEST_REGISTER(DelayedControlsTest)\n> > > > diff --git a/test/meson.build b/test/meson.build\n> > > > index 0a1d434e399641bb..a683a657a439b4ff 100644\n> > > > --- a/test/meson.build\n> > > > +++ b/test/meson.build\n> > > > @@ -25,6 +25,7 @@ public_tests = [\n> > > >  internal_tests = [\n> > > >      ['byte-stream-buffer',              'byte-stream-buffer.cpp'],\n> > > >      ['camera-sensor',                   'camera-sensor.cpp'],\n> > > > +    ['delayed_contols',                 'delayed_contols.cpp'],\n> > > >      ['event',                           'event.cpp'],\n> > > >      ['event-dispatcher',                'event-dispatcher.cpp'],\n> > > >      ['event-thread',                    'event-thread.cpp'],\n> > > > --\n> > > > 2.29.2\n> > > >\n> > > > _______________________________________________\n> > > > libcamera-devel mailing list\n> > > > libcamera-devel@lists.libcamera.org\n> > > > https://lists.libcamera.org/listinfo/libcamera-devel\n> >\n> > --\n> > Regards,\n> > Niklas Söderlund","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 46F26BE176\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 24 Nov 2020 11:46:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B9BA2633F6;\n\tTue, 24 Nov 2020 12:46:33 +0100 (CET)","from mail-lj1-x236.google.com (mail-lj1-x236.google.com\n\t[IPv6:2a00:1450:4864:20::236])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 758A960332\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 24 Nov 2020 12:46:32 +0100 (CET)","by mail-lj1-x236.google.com with SMTP id s9so21599653ljo.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 24 Nov 2020 03:46:32 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\t17sm1713192lfr.52.2020.11.24.03.46.30\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 24 Nov 2020 03:46:31 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"PWVGs5D8\"; dkim-atps=neutral","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=A3JxcgwYpCNP1UjGUHBX0IRuKX/0kYFXWueB3V9cTKQ=;\n\tb=PWVGs5D85oqcRFqWRlx8FxXA/la3zugX4j72ynDbkuLnGuzlDAFkwO/ZfVSrDH5/F3\n\twr7r/RsYsEa0fzwjeI7UIfA+XFM3FZ4cIt5Wh5Pd9/GiZiqh5WxHbFL5xpplu0YCLONB\n\t98hOf1IbjcXXVTjtDoHxl493zqllAy0xG3U//VqgHbhqP7C+qPA5MynGNei/FjSX/ucf\n\tQ4ZEXC/h/wmT+Yt1Ig1Sd1N4aBF8OIcpZiwXX/XAo6Toy6uIkZxy702u+2cRJMcP3RKa\n\tbXefLG3JU+u+QQb+LhPFawgzbkxKajz64gnIx0wDxvwmXH0w9I3bj8ImltbNqbD9/9tq\n\tXNkg==","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=A3JxcgwYpCNP1UjGUHBX0IRuKX/0kYFXWueB3V9cTKQ=;\n\tb=MfLdP7hgW/3/fnl5W1960WfjcesYxHVdSdy7QYcYN3AAqqJYVdDeRQVJHCETbAc/5Y\n\t/l0ByKRqCIfajyNupAdviMLOUICJOcijXPPVavgLQQIwIvdFcPkU7KfQ9wOzw6glqp4U\n\tU6DRDqvwByZHNkdtUy2YqqBxxrKdc3jjZ6H+Ff6E6q9FrP2AT9SmFziPilQy5deYXsgF\n\toZxgVxgDGKGnR0S04sd0QlFBFlezw7powp8XMISUGn43bwu+p+aEbbcdc1s0/GBPmZ3n\n\tzG79Czt/lzHYSVA47/y3Y2mCAfeaB+GwJoA5yYMCdRRdhLW6GplZ1ydMVrYPk7eXMQFq\n\tfd9Q==","X-Gm-Message-State":"AOAM530ownemuMysEKZUriIeuBzpMWjqbT2KZ1l7yKdbtnFFpfP6v/3L\n\tMVHCBAUKvBvhlCLC6WCtwmPNRw==","X-Google-Smtp-Source":"ABdhPJwjoSuZBEWEeH/Qe+hFTZqqQ7HuW977l4LjOAPNljBnHbsIGW5RdJlVwL6NJ8vjVzE2A26pZA==","X-Received":"by 2002:a2e:1606:: with SMTP id w6mr1698675ljd.57.1606218391690; \n\tTue, 24 Nov 2020 03:46:31 -0800 (PST)","Date":"Tue, 24 Nov 2020 12:46:30 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20201124114630.GA2162566@oden.dyn.berto.se>","References":"<20201110002710.3233696-1-niklas.soderlund@ragnatech.se>\n\t<20201110002710.3233696-3-niklas.soderlund@ragnatech.se>\n\t<20201118141253.6edepzajgokp5d5w@uno.localdomain>\n\t<20201123211725.GF1773213@oden.dyn.berto.se>\n\t<20201124113118.ko77inlt4hhwcexu@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201124113118.ko77inlt4hhwcexu@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 2/8] test: delayed_controls: Add\n\ttest case for DelayedControls","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]