[{"id":16871,"web_url":"https://patchwork.libcamera.org/comment/16871/","msgid":"","date":"2021-05-10T20:45:05","subject":"Re: [libcamera-devel] [PATCH] lc-compliance: Add test to call\n\tmultiple configure()","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Nícolas,\n\nThanks for your work.\n\nOn 2021-05-10 14:43:04 -0300, Nícolas F. R. A. Prado wrote:\n> Add a test to lc-compliance that calls configure() multiple times on the\n> camera before starting to capture. This isn't a common pattern for\n> applications but is allowed and so should be tested by lc-compliance.\n> \n> Signed-off-by: Nícolas F. R. A. Prado \n> ---\n> \n> This tests multiple generateConfiguration() -> validate() -> configure()\n> sequences, but maybe it would be valuable to test repetitions of each one of\n> those steps in the test? Although I think repeating the whole sequence should\n> already catch any bug, it may be better for lc-compliance to test too much\n> rather than too little.\n> \n> Repeating each step individually would duplicate a bit of code, and require\n> SimpleCapture::config_ to be public, but that may be inevitable if we want to\n> inspect and fiddle with those properties during the tests.\n> \n> Also, this doesn't really check if the last configured role is the one in effect\n> during the capture. Is that even something verifiable? It doesn't seem to be\n> since each pipeline handler decides what each role means.\n> \n> src/lc-compliance/single_stream.cpp | 44 ++++++++++++++++++++++++-----\n> 1 file changed, 37 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp\n> index 8318b42f42d6..6f1c259547a7 100644\n> --- a/src/lc-compliance/single_stream.cpp\n> +++ b/src/lc-compliance/single_stream.cpp\n> @@ -12,6 +12,13 @@\n> \n> using namespace libcamera;\n> \n> +static const std::vector> roles = {\n> +\t{ \"raw\", Raw },\n> +\t{ \"still\", StillCapture },\n> +\t{ \"video\", VideoRecording },\n> +\t{ \"viewfinder\", Viewfinder },\n> +};\n\nI would keep this in testSingleStream() for now and create a new list in \ntestMultipleConfigures(). The smaller the diff the easier it will be \nonce the gtest series is merged :-)\n\nThe rest looks good to me.\n\n> +\n> Results::Result testRequestBalance(std::shared_ptr camera,\n> \t\t\t\t StreamRole role, unsigned int startCycles,\n> \t\t\t\t unsigned int numRequests)\n> @@ -33,6 +40,25 @@ Results::Result testRequestBalance(std::shared_ptr camera,\n> \t\tstd::to_string(startCycles) + \" start cycles\" };\n> }\n> \n> +Results::Result testMultipleConfigures(std::shared_ptr camera,\n> +\t\t\t\t StreamRole role, unsigned int numRequests)\n> +{\n> +\tResults::Result ret;\n> +\n> +\tSimpleCaptureBalanced capture(camera);\n> +\n> +\tfor (auto r: roles) {\n> +\t\tret = capture.configure(r.second);\n> +\t\tif (ret.first == Results::Fail)\n> +\t\t\treturn ret;\n> +\t}\n> +\tret = capture.configure(role);\n> +\tif (ret.first != Results::Pass)\n> +\t\treturn ret;\n> +\n> +\treturn capture.capture(numRequests);\n> +}\n> +\n> Results::Result testRequestUnbalance(std::shared_ptr camera,\n> \t\t\t\t StreamRole role, unsigned int numRequests)\n> {\n> @@ -47,15 +73,9 @@ Results::Result testRequestUnbalance(std::shared_ptr camera,\n> \n> Results testSingleStream(std::shared_ptr camera)\n> {\n> -\tstatic const std::vector> roles = {\n> -\t\t{ \"raw\", Raw },\n> -\t\t{ \"still\", StillCapture },\n> -\t\t{ \"video\", VideoRecording },\n> -\t\t{ \"viewfinder\", Viewfinder },\n> -\t};\n> \tstatic const std::vector numRequests = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };\n> \n> -\tResults results(numRequests.size() * roles.size() * 3);\n> +\tResults results(numRequests.size() * roles.size() * 3 + roles.size());\n> \n> \tfor (const auto &role : roles) {\n> \t\tstd::cout << \"= Test role \" << role.first << std::endl;\n> @@ -91,6 +111,16 @@ Results testSingleStream(std::shared_ptr camera)\n> \t\tstd::cout << \"* Test unbalanced stop\" << std::endl;\n> \t\tfor (unsigned int num : numRequests)\n> \t\t\tresults.add(testRequestUnbalance(camera, role.second, num));\n> +\n> +\t\t/*\n> +\t\t * Test multiple configure()\n> +\t\t *\n> +\t\t * Makes sure that the camera supports being configured multiple\n> +\t\t * times, with only the last one taking effect, before starting\n> +\t\t * to capture.\n> +\t\t */\n> +\t\tstd::cout << \"* Test multiple configure()\" << std::endl;\n> +\t\tresults.add(testMultipleConfigures(camera, role.second, numRequests.back()));\n> \t}\n> \n> \treturn results;\n> -- \n> 2.31.1\n>","headers":{"Return-Path":"","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 D5FF0BF829\n\tfor ;\n\tMon, 10 May 2021 20:45:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5DD4D602BE;\n\tMon, 10 May 2021 22:45:08 +0200 (CEST)","from mail-lf1-x133.google.com (mail-lf1-x133.google.com\n\t[IPv6:2a00:1450:4864:20::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0D0A1602BE\n\tfor ;\n\tMon, 10 May 2021 22:45:07 +0200 (CEST)","by mail-lf1-x133.google.com with SMTP id x20so25382526lfu.6\n\tfor ;\n\tMon, 10 May 2021 13:45:06 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tk1sm2402704lfg.140.2021.05.10.13.45.05\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 10 May 2021 13:45:05 -0700 (PDT)"],"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=\"HcFEqOi3\"; 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=uRGPIil2ZsrnEGX1EY0HwW+Z0WpZtf6a3FizWDLuIYE=;\n\tb=HcFEqOi3Evw51PXtJ8Im8xSR6hRuvkuffS+DFAya7aS2HfNcJ8boJJTQaPEcVesoso\n\tbqNW9QI15gXerekAs18OfbfTB/NkbP/RA9qPNWFb9dE0b2YpG7nbFc4XSRiR3OJNzYpU\n\tXrAnuWvtYkPeIfLxlmfE4SGjSop8YsfT7IGNGZr2Xicz5ZOfr03PyM3E12OohUGuSLJN\n\t1p1NFXg8KkZHBUX9NWYZfXdBkR4vXkRtM5OjfRyDpdw42bw02nk2zF/rlrkTSNwKDHFy\n\tG2ZWDjnkv3ITMbvLir6N4NAjIthDXoiVFzMlw+K3GPjBdUm4ukC3RRaFsQkUsItRkB6M\n\tw6ag==","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=uRGPIil2ZsrnEGX1EY0HwW+Z0WpZtf6a3FizWDLuIYE=;\n\tb=e2ieWIag73jPZJUW1I2OAp1+TSLiM+fCBt1iVeJU4ZEHioerYIzreYrtNBZnRwtwT0\n\tiDEvkZBgGfU1xSQwT86TkQU6AiR1t2qP4VazLhNfUwYADQ13knlapVgOWzAmd4hBfxEe\n\tlOwNZsoYI3u1fZdzCQAGHde5WueeZZxWvVDDP3uRm6MKaLXKTCJ3edH2nZVqhQWApHo9\n\twztcT+CKq4oAs2TjYXd9dXPBO7fSMTHzE2yM/CsHX4J0GmJyhrgn79HX2lHPUaFWcgZn\n\tjJWQo/utlPKmWXxrw5B6u3lm5GtmT44Tm6fjpyHzAUjIkyDMQjnR4DWB5AIyeIE/keQc\n\tyg0Q==","X-Gm-Message-State":"AOAM532SYu4zQ5dwYlSYbK2iluPMdQvV+jdOn8b6DJLc8GZaljy2oLCH\n\tUVGcFaXFJ0dwYdXpTvVXuCmGBhJnjJHg+P/p","X-Google-Smtp-Source":"ABdhPJy0kzjqu8rzoC6IZ5GOHAcX6MxgmBvfiejXiIc3rXsTIs+rJM1zk8oT9xaGZEv6REWJD/vt/Q==","X-Received":"by 2002:a05:6512:3619:: with SMTP id\n\tf25mr15766798lfs.399.1620679506386; \n\tMon, 10 May 2021 13:45:06 -0700 (PDT)","Date":"Mon, 10 May 2021 22:45:05 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado ","Message-ID":"","References":"<20210510174304.560185-1-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210510174304.560185-1-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH] lc-compliance: Add test to call\n\tmultiple configure()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida ","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]