[{"id":16580,"web_url":"https://patchwork.libcamera.org/comment/16580/","msgid":"<YIaKdRbZU/wjcmEd@oden.dyn.berto.se>","date":"2021-04-26T09:40:05","subject":"Re: [libcamera-devel] [PATCH v3 3/4] lc-compliance: Add test to\n\tqueue more requests than hardware depth","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-04-21 13:51:38 -0300, Nícolas F. R. A. Prado wrote:\n> A pipeline handler should be able to handle an arbitrary amount of\n> simultaneous requests by submitting what it can to the video device and\n> queuing the rest internally until resources are available. This isn't\n> currently done by some pipeline handlers however [1].\n> \n> Add a new test to lc-compliance that submits a lot of requests at once\n> to check if the pipeline handler is behaving well in this situation.\n> \n> [1] https://bugs.libcamera.org/show_bug.cgi?id=24\n> \n> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>\n> ---\n>  src/lc-compliance/simple_capture.cpp | 70 ++++++++++++++++++++++++++++\n>  src/lc-compliance/simple_capture.h   | 15 ++++++\n>  src/lc-compliance/single_stream.cpp  | 31 +++++++++++-\n>  3 files changed, 115 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp\n> index 875772a80c27..01d76380e998 100644\n> --- a/src/lc-compliance/simple_capture.cpp\n> +++ b/src/lc-compliance/simple_capture.cpp\n> @@ -223,3 +223,73 @@ void SimpleCaptureUnbalanced::requestComplete(Request *request)\n>  \tif (camera_->queueRequest(request))\n>  \t\tloop_->exit(-EINVAL);\n>  }\n> +\n> +SimpleCaptureOverflow::SimpleCaptureOverflow(std::shared_ptr<Camera> camera)\n> +\t: SimpleCapture(camera)\n> +{\n> +}\n> +\n> +Results::Result SimpleCaptureOverflow::capture()\n> +{\n> +\tResults::Result ret = start();\n> +\tif (ret.first != Results::Pass)\n> +\t\treturn ret;\n> +\n> +\tStream *stream = config_->at(0).stream();\n> +\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);\n> +\n> +\tcaptureCount_ = 0;\n> +\tcaptureLimit_ = buffers.size();\n> +\n> +\tstd::vector<std::unique_ptr<libcamera::Request>> requests;\n> +\tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers) {\n> +\t\tstd::unique_ptr<Request> request = camera_->createRequest();\n> +\t\tif (!request) {\n> +\t\t\tstop();\n> +\t\t\treturn { Results::Fail, \"Can't create request\" };\n> +\t\t}\n> +\n> +\t\tif (request->addBuffer(stream, buffer.get())) {\n> +\t\t\tstop();\n> +\t\t\treturn { Results::Fail, \"Can't set buffer for request\" };\n> +\t\t}\n> +\n> +\t\tif (camera_->queueRequest(request.get()) < 0) {\n> +\t\t\tstop();\n> +\t\t\treturn { Results::Fail, \"Failed to queue request\" };\n> +\t\t}\n> +\n> +\t\trequests.push_back(std::move(request));\n> +\t}\n> +\n> +\t/* Run capture session. */\n> +\tloop_ = new EventLoop();\n> +\tloop_->exec();\n> +\tstop();\n> +\tdelete loop_;\n> +\n> +\tif (captureCount_ != captureLimit_)\n> +\t\treturn { Results::Fail, \"Got \" + std::to_string(captureCount_) +\n> +\t\t\t\" request, wanted \" + std::to_string(captureLimit_) };\n> +\n> +\treturn { Results::Pass, \"Overflow capture of \" +\n> +\t\tstd::to_string(captureLimit_) + \" requests\" };\n> +}\n\nReading this I now see there is quiet a bit of duplication between \nSimpleCapture*::capture() implementations. This existed before this \npatch and can be addressed on-top.\n\n> +\n> +void SimpleCaptureOverflow::requestComplete([[maybe_unused]]Request *request)\n> +{\n> +\tcaptureCount_++;\n> +\tif (captureCount_ >= captureLimit_) {\n> +\t\tloop_->exit(0);\n> +\t\treturn;\n> +\t}\n> +}\n\nLike above I now see this is could be moved to SimpleCapture without \nmuch work and the code can then be shared between all SimpleCapture* \ntests. Maybe this one is so straight forward you could do it as a pre \npatch to this one? If you like it can also be done on-top.\n\n> +\n> +Results::Result SimpleCaptureOverflow::allocateBuffers(unsigned int count)\n> +{\n> +\tStream *stream = config_->at(0).stream();\n> +\tif (allocator_->allocate(stream, count) < 0)\n> +\t\treturn { Results::Fail, \"Failed to allocate buffers\" };\n> +\n> +\treturn { Results::Pass, \"Allocated buffers\" };\n> +}\n> diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h\n> index 82e2c56a55f1..cafcdafe10c2 100644\n> --- a/src/lc-compliance/simple_capture.h\n> +++ b/src/lc-compliance/simple_capture.h\n> @@ -66,4 +66,19 @@ private:\n>  \tunsigned int captureLimit_;\n>  };\n>  \n> +class SimpleCaptureOverflow : public SimpleCapture\n> +{\n> +public:\n> +\tSimpleCaptureOverflow(std::shared_ptr<libcamera::Camera> camera);\n> +\n> +\tResults::Result allocateBuffers(unsigned int count);\n> +\tResults::Result capture();\n> +\n> +private:\n> +\tvoid requestComplete(libcamera::Request *request) override;\n> +\n> +\tunsigned int captureCount_;\n> +\tunsigned int captureLimit_;\n> +};\n> +\n>  #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */\n> diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp\n> index 649291c7bb73..61979f66d92d 100644\n> --- a/src/lc-compliance/single_stream.cpp\n> +++ b/src/lc-compliance/single_stream.cpp\n> @@ -12,6 +12,23 @@\n>  \n>  using namespace libcamera;\n>  \n> +static const unsigned int MAX_SIMULTANEOUS_REQUESTS = 8;\n> +\n> +Results::Result testRequestOverflow(std::shared_ptr<Camera> camera,\n> +\t\t\t\t   StreamRole role, unsigned int numRequests)\n> +{\n> +\tSimpleCaptureOverflow capture(camera);\n> +\n> +\tResults::Result ret = capture.configure(role);\n> +\tif (ret.first != Results::Pass)\n> +\t\treturn ret;\n> +\tcapture.allocateBuffers(numRequests);\n> +\tif (ret.first != Results::Pass)\n> +\t\treturn ret;\n> +\n> +\treturn capture.capture();\n> +}\n> +\n>  Results::Result testRequestBalance(std::shared_ptr<Camera> camera,\n>  \t\t\t\t   StreamRole role, unsigned int startCycles,\n>  \t\t\t\t   unsigned int numRequests)\n> @@ -61,7 +78,7 @@ Results testSingleStream(std::shared_ptr<Camera> camera)\n>  \t};\n>  \tstatic const std::vector<unsigned int> 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() * 1);\n\nI think you should drop the '* 1' at the end.\n\n>  \n>  \tfor (const auto &role : roles) {\n>  \t\tstd::cout << \"= Test role \" << role.first << std::endl;\n> @@ -97,6 +114,18 @@ Results testSingleStream(std::shared_ptr<Camera> 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 overflowing pipeline with requests\n> +\t\t *\n> +\t\t * Makes sure that the camera supports receiving a large amount\n> +\t\t * of requests at once. Example failure is a camera that doesn't\n> +\t\t * check if there are available resources (free internal\n> +\t\t * buffers, free buffers in the video devices) before handling a\n> +\t\t * request.\n> +\t\t */\n> +\t\tstd::cout << \"* Test overflowing requests\" << std::endl;\n> +\t\tresults.add(testRequestOverflow(camera, role.second, MAX_SIMULTANEOUS_REQUESTS));\n>  \t}\n>  \n>  \treturn results;\n> -- \n> 2.31.1\n>","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 CE511BDC9A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 26 Apr 2021 09:40:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BB01E688B2;\n\tMon, 26 Apr 2021 11:40:09 +0200 (CEST)","from mail-lj1-x22c.google.com (mail-lj1-x22c.google.com\n\t[IPv6:2a00:1450:4864:20::22c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B42A568882\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 26 Apr 2021 11:40:07 +0200 (CEST)","by mail-lj1-x22c.google.com with SMTP id d15so12032289ljo.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 26 Apr 2021 02:40:07 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ta4sm1392544lfs.130.2021.04.26.02.40.06\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 26 Apr 2021 02:40:06 -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=\"hEZ4QEQV\"; 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=meuEIJPNviUiY2iGoEjGkRtGtxmmIM6ZH61RaaDUk5U=;\n\tb=hEZ4QEQVKDDexPYp/UcvEIdSQ7LqxNbx08BSVrUiHOmQNGl7iJswE8HXFJcI2upFIe\n\t2vVU1KxhlfWI4lq3oIumP60jWP4r+ExPCtAVjt5gFfRETknfHWa6z113M4tmyiKwF1k0\n\tDsBXO/XB78w+iV4YVNvp2BvcbKBqreL6fjNkOyO5BX4ujSlUerGUbMwGvNeC2E/Y/ye9\n\totZYFWNlYK+ly5YVs567FzopWuYtgvk190TiAd+EWbXRlKIMSXBEkLTz0on8W/3JvAVA\n\tGpHzRrAfH775HI5cdSf825Ll6/uevPnzsXX4+WnQHYVbDIHe7DX/7JTQ/Bz2nEVVkCXR\n\t5hpg==","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=meuEIJPNviUiY2iGoEjGkRtGtxmmIM6ZH61RaaDUk5U=;\n\tb=pnZ3EF0qWHocLhxPXW/+15X9ycoB1SCsjeaN8mf2Do8JTG+NvcYY9By4NNd106rvHr\n\tisx5zu+tE4T2m/H06AZRBoqkldng0Dd0e92yFpcYxR96WEF4T2XTZYRruKMBxqxO4LrA\n\tF0WOJwymxdWn01/IRIhGTt+iSBZ6mwW7chZXFmYYGb8Rmry+nvz+qclmW2MpwFRBuT+w\n\tBSRSFmQkPg9IKAzJeSmu0Qzoox+1PF2svsBfS3VOebN7vD0kEUzSEOzb4BHv0njSMn0D\n\t+KQgfhdkpJzuo1zAPkrDOkbhDnijFZUk87pbuqSYmF6y2n6QigIsazUBf9h9wr5JEy+R\n\t4cVw==","X-Gm-Message-State":"AOAM530OqRLLX0dLxbtzfhdoVNa93+sRMK2LH19bVoWKzL/WtzVtyCHJ\n\tz3Gz+EC6EmJHbUGwZEIqL+gRBA==","X-Google-Smtp-Source":"ABdhPJzS2CMlqZzcPZkYFESZYoyHyeGzxZDyxDRcMYu0quJZVJxGZPwiTduaU4NZQkgs3zM89l6BzA==","X-Received":"by 2002:a2e:8182:: with SMTP id\n\te2mr12726933ljg.238.1619430007117; \n\tMon, 26 Apr 2021 02:40:07 -0700 (PDT)","Date":"Mon, 26 Apr 2021 11:40:05 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado <nfraprado@collabora.com>","Message-ID":"<YIaKdRbZU/wjcmEd@oden.dyn.berto.se>","References":"<20210421165139.318432-1-nfraprado@collabora.com>\n\t<20210421165139.318432-4-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210421165139.318432-4-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v3 3/4] lc-compliance: Add test to\n\tqueue more requests than hardware depth","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, =?iso-8859-1?q?Andr=E9?=\n\tAlmeida <andrealmeid@collabora.com>,  kernel@collabora.com","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":16676,"web_url":"https://patchwork.libcamera.org/comment/16676/","msgid":"<CAZEFVDKB9AQ.2CQTDQOY1HI90@notapiano>","date":"2021-04-28T13:42:48","subject":"Re: [libcamera-devel] [PATCH v3 3/4] lc-compliance: Add test to\n\tqueue more requests than hardware depth","submitter":{"id":84,"url":"https://patchwork.libcamera.org/api/people/84/","name":"Nícolas F. R. A. Prado","email":"nfraprado@collabora.com"},"content":"Hi Niklas,\n\nthank you for the feedback.\n\nEm 2021-04-26 06:40, Niklas Söderlund escreveu:\n\n> Hi \"Nícolas,\n>\n> Thanks for your work.\n>\n> On 2021-04-21 13:51:38 -0300, Nícolas F. R. A. Prado wrote:\n> > A pipeline handler should be able to handle an arbitrary amount of\n> > simultaneous requests by submitting what it can to the video device and\n> > queuing the rest internally until resources are available. This isn't\n> > currently done by some pipeline handlers however [1].\n> > \n> > Add a new test to lc-compliance that submits a lot of requests at once\n> > to check if the pipeline handler is behaving well in this situation.\n> > \n> > [1] https://bugs.libcamera.org/show_bug.cgi?id=24\n> > \n> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>\n> > ---\n> >  src/lc-compliance/simple_capture.cpp | 70 ++++++++++++++++++++++++++++\n> >  src/lc-compliance/simple_capture.h   | 15 ++++++\n> >  src/lc-compliance/single_stream.cpp  | 31 +++++++++++-\n> >  3 files changed, 115 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp\n> > index 875772a80c27..01d76380e998 100644\n> > --- a/src/lc-compliance/simple_capture.cpp\n> > +++ b/src/lc-compliance/simple_capture.cpp\n> > @@ -223,3 +223,73 @@ void SimpleCaptureUnbalanced::requestComplete(Request *request)\n> >  \tif (camera_->queueRequest(request))\n> >  \t\tloop_->exit(-EINVAL);\n> >  }\n> > +\n> > +SimpleCaptureOverflow::SimpleCaptureOverflow(std::shared_ptr<Camera> camera)\n> > +\t: SimpleCapture(camera)\n> > +{\n> > +}\n> > +\n> > +Results::Result SimpleCaptureOverflow::capture()\n> > +{\n> > +\tResults::Result ret = start();\n> > +\tif (ret.first != Results::Pass)\n> > +\t\treturn ret;\n> > +\n> > +\tStream *stream = config_->at(0).stream();\n> > +\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);\n> > +\n> > +\tcaptureCount_ = 0;\n> > +\tcaptureLimit_ = buffers.size();\n> > +\n> > +\tstd::vector<std::unique_ptr<libcamera::Request>> requests;\n> > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers) {\n> > +\t\tstd::unique_ptr<Request> request = camera_->createRequest();\n> > +\t\tif (!request) {\n> > +\t\t\tstop();\n> > +\t\t\treturn { Results::Fail, \"Can't create request\" };\n> > +\t\t}\n> > +\n> > +\t\tif (request->addBuffer(stream, buffer.get())) {\n> > +\t\t\tstop();\n> > +\t\t\treturn { Results::Fail, \"Can't set buffer for request\" };\n> > +\t\t}\n> > +\n> > +\t\tif (camera_->queueRequest(request.get()) < 0) {\n> > +\t\t\tstop();\n> > +\t\t\treturn { Results::Fail, \"Failed to queue request\" };\n> > +\t\t}\n> > +\n> > +\t\trequests.push_back(std::move(request));\n> > +\t}\n> > +\n> > +\t/* Run capture session. */\n> > +\tloop_ = new EventLoop();\n> > +\tloop_->exec();\n> > +\tstop();\n> > +\tdelete loop_;\n> > +\n> > +\tif (captureCount_ != captureLimit_)\n> > +\t\treturn { Results::Fail, \"Got \" + std::to_string(captureCount_) +\n> > +\t\t\t\" request, wanted \" + std::to_string(captureLimit_) };\n> > +\n> > +\treturn { Results::Pass, \"Overflow capture of \" +\n> > +\t\tstd::to_string(captureLimit_) + \" requests\" };\n> > +}\n>\n> Reading this I now see there is quiet a bit of duplication between\n> SimpleCapture*::capture() implementations. This existed before this\n> patch and can be addressed on-top.\n>\n> > +\n> > +void SimpleCaptureOverflow::requestComplete([[maybe_unused]]Request *request)\n> > +{\n> > +\tcaptureCount_++;\n> > +\tif (captureCount_ >= captureLimit_) {\n> > +\t\tloop_->exit(0);\n> > +\t\treturn;\n> > +\t}\n> > +}\n>\n> Like above I now see this is could be moved to SimpleCapture without\n> much work and the code can then be shared between all SimpleCapture*\n> tests. Maybe this one is so straight forward you could do it as a pre\n> patch to this one? If you like it can also be done on-top.\n\nI agree that there's duplicated code, but each class does things a little\ndifferently, just enough that we can't have a single common function for all of\nthem, from what I can tell.\n\nFor example, in SimpleCaptureOverflow::requestComplete() I don't need to requeue\nbuffers as they complete since I queue them all at once, so even though part of\nit is similar to other requestComplete()'s they aren't the same.\n\nFor SimpleCapture*::capture() I also don't see how they could all be shared,\nsince they have different variables to initialize and different success asserts\nat the end. But at least the requests queueing loop could be shared, if we can\nhave a default queueRequest() that is overrided by SimpleCaptureBalanced.\n\nPlease see below my attempt at removing code duplication. Note that it doesn't\nwork because SimpleCapture::queueRequest() gets called instead of\nSimpleCaptureBalanced::queueRequest() even on the SimpleCaptureBalanced\ninstance. Some OOP concept that I'm missing... Any pointers on how to fix it or\non a better way to reduce the duplication would be great :).\n\nThanks,\nNícolas\n\nFrom 4166a9b1b0a3005ef5a077a4e2f10267d89bf375 Mon Sep 17 00:00:00 2001\nFrom: =?UTF-8?q?N=C3=ADcolas=20F=2E=20R=2E=20A=2E=20Prado?=\n <nfraprado@collabora.com>\nDate: Tue, 27 Apr 2021 18:10:03 -0300\nSubject: [PATCH] tmp2\n\n---\n src/lc-compliance/simple_capture.cpp | 126 ++++++++++-----------------\n src/lc-compliance/simple_capture.h   |  23 +++--\n 2 files changed, 57 insertions(+), 92 deletions(-)\n\ndiff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp\nindex e4f6c6723f4d..34e116852d0b 100644\n--- a/src/lc-compliance/simple_capture.cpp\n+++ b/src/lc-compliance/simple_capture.cpp\n@@ -73,6 +73,24 @@ void SimpleCapture::stop()\n \tallocator_->free(stream);\n }\n \n+int SimpleCapture::queueRequest(Request *request)\n+{\n+\treturn camera_->queueRequest(request);\n+}\n+\n+void SimpleCapture::requestComplete(Request *request)\n+{\n+\tcaptureCount_++;\n+\tif (captureCount_ >= captureLimit_) {\n+\t\tloop_->exit(0);\n+\t\treturn;\n+\t}\n+\n+\trequest->reuse(Request::ReuseBuffers);\n+\tif (queueRequest(request))\n+\t\tloop_->exit(-EINVAL);\n+}\n+\n /* SimpleCaptureBalanced */\n \n SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr<Camera> camera)\n@@ -103,27 +121,10 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)\n \tcaptureCount_ = 0;\n \tcaptureLimit_ = numRequests;\n \n-\t/* Queue the recommended number of reqeuests. */\n \tstd::vector<std::unique_ptr<libcamera::Request>> requests;\n-\tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers) {\n-\t\tstd::unique_ptr<Request> request = camera_->createRequest();\n-\t\tif (!request) {\n-\t\t\tstop();\n-\t\t\treturn { Results::Fail, \"Can't create request\" };\n-\t\t}\n-\n-\t\tif (request->addBuffer(stream, buffer.get())) {\n-\t\t\tstop();\n-\t\t\treturn { Results::Fail, \"Can't set buffer for request\" };\n-\t\t}\n-\n-\t\tif (queueRequest(request.get()) < 0) {\n-\t\t\tstop();\n-\t\t\treturn { Results::Fail, \"Failed to queue request\" };\n-\t\t}\n-\n-\t\trequests.push_back(std::move(request));\n-\t}\n+\tret = queueRequests(stream, requests, buffers);\n+\tif (ret.first != Results::Pass)\n+\t\treturn ret;\n \n \t/* Run capture session. */\n \tloop_ = new EventLoop();\n@@ -148,19 +149,6 @@ int SimpleCaptureBalanced::queueRequest(Request *request)\n \treturn camera_->queueRequest(request);\n }\n \n-void SimpleCaptureBalanced::requestComplete(Request *request)\n-{\n-\tcaptureCount_++;\n-\tif (captureCount_ >= captureLimit_) {\n-\t\tloop_->exit(0);\n-\t\treturn;\n-\t}\n-\n-\trequest->reuse(Request::ReuseBuffers);\n-\tif (queueRequest(request))\n-\t\tloop_->exit(-EINVAL);\n-}\n-\n /* SimpleCaptureUnbalanced */\n \n SimpleCaptureUnbalanced::SimpleCaptureUnbalanced(std::shared_ptr<Camera> camera)\n@@ -180,27 +168,10 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)\n \tcaptureCount_ = 0;\n \tcaptureLimit_ = numRequests;\n \n-\t/* Queue the recommended number of reqeuests. */\n \tstd::vector<std::unique_ptr<libcamera::Request>> requests;\n-\tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers) {\n-\t\tstd::unique_ptr<Request> request = camera_->createRequest();\n-\t\tif (!request) {\n-\t\t\tstop();\n-\t\t\treturn { Results::Fail, \"Can't create request\" };\n-\t\t}\n-\n-\t\tif (request->addBuffer(stream, buffer.get())) {\n-\t\t\tstop();\n-\t\t\treturn { Results::Fail, \"Can't set buffer for request\" };\n-\t\t}\n-\n-\t\tif (camera_->queueRequest(request.get()) < 0) {\n-\t\t\tstop();\n-\t\t\treturn { Results::Fail, \"Failed to queue request\" };\n-\t\t}\n-\n-\t\trequests.push_back(std::move(request));\n-\t}\n+\tret = queueRequests(stream, requests, buffers);\n+\tif (ret.first != Results::Pass)\n+\t\treturn ret;\n \n \t/* Run capture session. */\n \tloop_ = new EventLoop();\n@@ -211,37 +182,14 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)\n \treturn { status ? Results::Fail : Results::Pass, \"Unbalanced capture of \" + std::to_string(numRequests) + \" requests\" };\n }\n \n-void SimpleCaptureUnbalanced::requestComplete(Request *request)\n-{\n-\tcaptureCount_++;\n-\tif (captureCount_ >= captureLimit_) {\n-\t\tloop_->exit(0);\n-\t\treturn;\n-\t}\n-\n-\trequest->reuse(Request::ReuseBuffers);\n-\tif (camera_->queueRequest(request))\n-\t\tloop_->exit(-EINVAL);\n-}\n-\n SimpleCaptureOverflow::SimpleCaptureOverflow(std::shared_ptr<Camera> camera)\n \t: SimpleCapture(camera)\n {\n }\n \n-Results::Result SimpleCaptureOverflow::capture()\n+Results::Result SimpleCapture::queueRequests(Stream *stream, std::vector<std::unique_ptr<libcamera::Request>> &requests,\n+\t\t\t\t\t     const std::vector<std::unique_ptr<FrameBuffer>> &buffers)\n {\n-\tResults::Result ret = start();\n-\tif (ret.first != Results::Pass)\n-\t\treturn ret;\n-\n-\tStream *stream = config_->at(0).stream();\n-\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);\n-\n-\tcaptureCount_ = 0;\n-\tcaptureLimit_ = buffers.size();\n-\n-\tstd::vector<std::unique_ptr<libcamera::Request>> requests;\n \tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers) {\n \t\tstd::unique_ptr<Request> request = camera_->createRequest();\n \t\tif (!request) {\n@@ -254,7 +202,7 @@ Results::Result SimpleCaptureOverflow::capture()\n \t\t\treturn { Results::Fail, \"Can't set buffer for request\" };\n \t\t}\n \n-\t\tif (camera_->queueRequest(request.get()) < 0) {\n+\t\tif (queueRequest(request.get()) < 0) {\n \t\t\tstop();\n \t\t\treturn { Results::Fail, \"Failed to queue request\" };\n \t\t}\n@@ -262,6 +210,26 @@ Results::Result SimpleCaptureOverflow::capture()\n \t\trequests.push_back(std::move(request));\n \t}\n \n+\treturn { Results::Pass, \"Succesfully queued requests\" };\n+}\n+\n+Results::Result SimpleCaptureOverflow::capture()\n+{\n+\tResults::Result ret = start();\n+\tif (ret.first != Results::Pass)\n+\t\treturn ret;\n+\n+\tStream *stream = config_->at(0).stream();\n+\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);\n+\n+\tcaptureCount_ = 0;\n+\tcaptureLimit_ = buffers.size();\n+\n+\tstd::vector<std::unique_ptr<libcamera::Request>> requests;\n+\tret = queueRequests(stream, requests, buffers);\n+\tif (ret.first != Results::Pass)\n+\t\treturn ret;\n+\n \t/* Run capture session. */\n \tloop_ = new EventLoop();\n \tloop_->exec();\ndiff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h\nindex cafcdafe10c2..754dc6ea0db8 100644\n--- a/src/lc-compliance/simple_capture.h\n+++ b/src/lc-compliance/simple_capture.h\n@@ -26,11 +26,18 @@ protected:\n \n \tResults::Result start();\n \tvoid stop();\n+\tint queueRequest(libcamera::Request *request);\n+\tResults::Result queueRequests(libcamera::Stream *stream,\n+\t\t\t\t      std::vector<std::unique_ptr<libcamera::Request>> &requests,\n+\t\t\t\t      const std::vector<std::unique_ptr<libcamera::FrameBuffer>> &buffers);\n \n-\tvirtual void requestComplete(libcamera::Request *request) = 0;\n+\tvoid requestComplete(libcamera::Request *request);\n \n \tEventLoop *loop_;\n \n+\tunsigned int captureCount_;\n+\tunsigned int captureLimit_;\n+\n \tstd::shared_ptr<libcamera::Camera> camera_;\n \tstd::unique_ptr<libcamera::FrameBufferAllocator> allocator_;\n \tstd::unique_ptr<libcamera::CameraConfiguration> config_;\n@@ -45,11 +52,9 @@ public:\n \n private:\n \tint queueRequest(libcamera::Request *request);\n-\tvoid requestComplete(libcamera::Request *request) override;\n+\tvoid requestComplete(libcamera::Request *request);\n \n \tunsigned int queueCount_;\n-\tunsigned int captureCount_;\n-\tunsigned int captureLimit_;\n };\n \n class SimpleCaptureUnbalanced : public SimpleCapture\n@@ -59,11 +64,6 @@ public:\n \n \tResults::Result capture(unsigned int numRequests);\n \n-private:\n-\tvoid requestComplete(libcamera::Request *request) override;\n-\n-\tunsigned int captureCount_;\n-\tunsigned int captureLimit_;\n };\n \n class SimpleCaptureOverflow : public SimpleCapture\n@@ -75,10 +75,7 @@ public:\n \tResults::Result capture();\n \n private:\n-\tvoid requestComplete(libcamera::Request *request) override;\n-\n-\tunsigned int captureCount_;\n-\tunsigned int captureLimit_;\n+\tvoid requestComplete(libcamera::Request *request);\n };\n \n #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */","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 7A502BDE41\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 28 Apr 2021 14:07:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4563A688E4;\n\tWed, 28 Apr 2021 16:07:06 +0200 (CEST)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 661AA688DC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 28 Apr 2021 16:07:05 +0200 (CEST)","from localhost (unknown\n\t[IPv6:2804:14c:1a9:2978:c5f3:918d:a027:70d9])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits))\n\t(No client certificate requested) (Authenticated sender: nfraprado)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id 126151F42AFA;\n\tWed, 28 Apr 2021 15:07:02 +0100 (BST)"],"Mime-Version":"1.0","From":"=?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= <nfraprado@collabora.com>","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Date":"Wed, 28 Apr 2021 10:42:48 -0300","Message-Id":"<CAZEFVDKB9AQ.2CQTDQOY1HI90@notapiano>","In-Reply-To":"<YIaKdRbZU/wjcmEd@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v3 3/4] lc-compliance: Add test to\n\tqueue more requests than hardware depth","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, =?utf-8?q?Andr=C3=A9_Almeida?=\n\t<andrealmeid@collabora.com>,  kernel@collabora.com","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":16687,"web_url":"https://patchwork.libcamera.org/comment/16687/","msgid":"<YIqXP8NbruaN8osS@oden.dyn.berto.se>","date":"2021-04-29T11:23:43","subject":"Re: [libcamera-devel] [PATCH v3 3/4] lc-compliance: Add test to\n\tqueue more requests than hardware depth","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hello Nícolas,\n\nOn 2021-04-28 10:42:48 -0300, Nícolas F. R. A. Prado wrote:\n> Hi Niklas,\n> \n> thank you for the feedback.\n> \n> Em 2021-04-26 06:40, Niklas Söderlund escreveu:\n> \n> > Hi \"Nícolas,\n> >\n> > Thanks for your work.\n> >\n> > On 2021-04-21 13:51:38 -0300, Nícolas F. R. A. Prado wrote:\n> > > A pipeline handler should be able to handle an arbitrary amount of\n> > > simultaneous requests by submitting what it can to the video device and\n> > > queuing the rest internally until resources are available. This isn't\n> > > currently done by some pipeline handlers however [1].\n> > > \n> > > Add a new test to lc-compliance that submits a lot of requests at once\n> > > to check if the pipeline handler is behaving well in this situation.\n> > > \n> > > [1] https://bugs.libcamera.org/show_bug.cgi?id=24\n> > > \n> > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>\n> > > ---\n> > >  src/lc-compliance/simple_capture.cpp | 70 ++++++++++++++++++++++++++++\n> > >  src/lc-compliance/simple_capture.h   | 15 ++++++\n> > >  src/lc-compliance/single_stream.cpp  | 31 +++++++++++-\n> > >  3 files changed, 115 insertions(+), 1 deletion(-)\n> > > \n> > > diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp\n> > > index 875772a80c27..01d76380e998 100644\n> > > --- a/src/lc-compliance/simple_capture.cpp\n> > > +++ b/src/lc-compliance/simple_capture.cpp\n> > > @@ -223,3 +223,73 @@ void SimpleCaptureUnbalanced::requestComplete(Request *request)\n> > >  \tif (camera_->queueRequest(request))\n> > >  \t\tloop_->exit(-EINVAL);\n> > >  }\n> > > +\n> > > +SimpleCaptureOverflow::SimpleCaptureOverflow(std::shared_ptr<Camera> camera)\n> > > +\t: SimpleCapture(camera)\n> > > +{\n> > > +}\n> > > +\n> > > +Results::Result SimpleCaptureOverflow::capture()\n> > > +{\n> > > +\tResults::Result ret = start();\n> > > +\tif (ret.first != Results::Pass)\n> > > +\t\treturn ret;\n> > > +\n> > > +\tStream *stream = config_->at(0).stream();\n> > > +\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);\n> > > +\n> > > +\tcaptureCount_ = 0;\n> > > +\tcaptureLimit_ = buffers.size();\n> > > +\n> > > +\tstd::vector<std::unique_ptr<libcamera::Request>> requests;\n> > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers) {\n> > > +\t\tstd::unique_ptr<Request> request = camera_->createRequest();\n> > > +\t\tif (!request) {\n> > > +\t\t\tstop();\n> > > +\t\t\treturn { Results::Fail, \"Can't create request\" };\n> > > +\t\t}\n> > > +\n> > > +\t\tif (request->addBuffer(stream, buffer.get())) {\n> > > +\t\t\tstop();\n> > > +\t\t\treturn { Results::Fail, \"Can't set buffer for request\" };\n> > > +\t\t}\n> > > +\n> > > +\t\tif (camera_->queueRequest(request.get()) < 0) {\n> > > +\t\t\tstop();\n> > > +\t\t\treturn { Results::Fail, \"Failed to queue request\" };\n> > > +\t\t}\n> > > +\n> > > +\t\trequests.push_back(std::move(request));\n> > > +\t}\n> > > +\n> > > +\t/* Run capture session. */\n> > > +\tloop_ = new EventLoop();\n> > > +\tloop_->exec();\n> > > +\tstop();\n> > > +\tdelete loop_;\n> > > +\n> > > +\tif (captureCount_ != captureLimit_)\n> > > +\t\treturn { Results::Fail, \"Got \" + std::to_string(captureCount_) +\n> > > +\t\t\t\" request, wanted \" + std::to_string(captureLimit_) };\n> > > +\n> > > +\treturn { Results::Pass, \"Overflow capture of \" +\n> > > +\t\tstd::to_string(captureLimit_) + \" requests\" };\n> > > +}\n> >\n> > Reading this I now see there is quiet a bit of duplication between\n> > SimpleCapture*::capture() implementations. This existed before this\n> > patch and can be addressed on-top.\n> >\n> > > +\n> > > +void SimpleCaptureOverflow::requestComplete([[maybe_unused]]Request *request)\n> > > +{\n> > > +\tcaptureCount_++;\n> > > +\tif (captureCount_ >= captureLimit_) {\n> > > +\t\tloop_->exit(0);\n> > > +\t\treturn;\n> > > +\t}\n> > > +}\n> >\n> > Like above I now see this is could be moved to SimpleCapture without\n> > much work and the code can then be shared between all SimpleCapture*\n> > tests. Maybe this one is so straight forward you could do it as a pre\n> > patch to this one? If you like it can also be done on-top.\n> \n> I agree that there's duplicated code, but each class does things a little\n> differently, just enough that we can't have a single common function for all of\n> them, from what I can tell.\n\nAs always the devils in the details :-) I still think we can improve the \ncode reuse here. But please only consider my suggestions as possible \nways forward, they could very well turn out to be bad once tried.\n\n> \n> For example, in SimpleCaptureOverflow::requestComplete() I don't need to requeue\n> buffers as they complete since I queue them all at once, so even though part of\n> it is similar to other requestComplete()'s they aren't the same.\n\nMaybe a flag could be added to control if requests should be request or \nnot?\n\n> \n> For SimpleCapture*::capture() I also don't see how they could all be shared,\n> since they have different variables to initialize and different success asserts\n> at the end. But at least the requests queueing loop could be shared, if we can\n> have a default queueRequest() that is overrided by SimpleCaptureBalanced.\n\nMaybe the bulk of the code that can be reused can be broken out to a \npossible parameterized helper functions (or functions) in the base class \nthat is then called from the specialized case?\n\n> \n> Please see below my attempt at removing code duplication. Note that it doesn't\n> work because SimpleCapture::queueRequest() gets called instead of\n> SimpleCaptureBalanced::queueRequest() even on the SimpleCaptureBalanced\n> instance. Some OOP concept that I'm missing... Any pointers on how to fix it or\n> on a better way to reduce the duplication would be great :).\n> \n> Thanks,\n> Nícolas\n> \n> From 4166a9b1b0a3005ef5a077a4e2f10267d89bf375 Mon Sep 17 00:00:00 2001\n> From: =?UTF-8?q?N=C3=ADcolas=20F=2E=20R=2E=20A=2E=20Prado?=\n>  <nfraprado@collabora.com>\n> Date: Tue, 27 Apr 2021 18:10:03 -0300\n> Subject: [PATCH] tmp2\n> \n> ---\n>  src/lc-compliance/simple_capture.cpp | 126 ++++++++++-----------------\n>  src/lc-compliance/simple_capture.h   |  23 +++--\n>  2 files changed, 57 insertions(+), 92 deletions(-)\n> \n> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp\n> index e4f6c6723f4d..34e116852d0b 100644\n> --- a/src/lc-compliance/simple_capture.cpp\n> +++ b/src/lc-compliance/simple_capture.cpp\n> @@ -73,6 +73,24 @@ void SimpleCapture::stop()\n>  \tallocator_->free(stream);\n>  }\n>  \n> +int SimpleCapture::queueRequest(Request *request)\n> +{\n> +\treturn camera_->queueRequest(request);\n> +}\n> +\n> +void SimpleCapture::requestComplete(Request *request)\n> +{\n> +\tcaptureCount_++;\n> +\tif (captureCount_ >= captureLimit_) {\n> +\t\tloop_->exit(0);\n> +\t\treturn;\n> +\t}\n> +\n> +\trequest->reuse(Request::ReuseBuffers);\n> +\tif (queueRequest(request))\n> +\t\tloop_->exit(-EINVAL);\n> +}\n> +\n>  /* SimpleCaptureBalanced */\n>  \n>  SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr<Camera> camera)\n> @@ -103,27 +121,10 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)\n>  \tcaptureCount_ = 0;\n>  \tcaptureLimit_ = numRequests;\n>  \n> -\t/* Queue the recommended number of reqeuests. */\n>  \tstd::vector<std::unique_ptr<libcamera::Request>> requests;\n> -\tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers) {\n> -\t\tstd::unique_ptr<Request> request = camera_->createRequest();\n> -\t\tif (!request) {\n> -\t\t\tstop();\n> -\t\t\treturn { Results::Fail, \"Can't create request\" };\n> -\t\t}\n> -\n> -\t\tif (request->addBuffer(stream, buffer.get())) {\n> -\t\t\tstop();\n> -\t\t\treturn { Results::Fail, \"Can't set buffer for request\" };\n> -\t\t}\n> -\n> -\t\tif (queueRequest(request.get()) < 0) {\n> -\t\t\tstop();\n> -\t\t\treturn { Results::Fail, \"Failed to queue request\" };\n> -\t\t}\n> -\n> -\t\trequests.push_back(std::move(request));\n> -\t}\n> +\tret = queueRequests(stream, requests, buffers);\n> +\tif (ret.first != Results::Pass)\n> +\t\treturn ret;\n>  \n>  \t/* Run capture session. */\n>  \tloop_ = new EventLoop();\n> @@ -148,19 +149,6 @@ int SimpleCaptureBalanced::queueRequest(Request *request)\n>  \treturn camera_->queueRequest(request);\n>  }\n>  \n> -void SimpleCaptureBalanced::requestComplete(Request *request)\n> -{\n> -\tcaptureCount_++;\n> -\tif (captureCount_ >= captureLimit_) {\n> -\t\tloop_->exit(0);\n> -\t\treturn;\n> -\t}\n> -\n> -\trequest->reuse(Request::ReuseBuffers);\n> -\tif (queueRequest(request))\n> -\t\tloop_->exit(-EINVAL);\n> -}\n> -\n>  /* SimpleCaptureUnbalanced */\n>  \n>  SimpleCaptureUnbalanced::SimpleCaptureUnbalanced(std::shared_ptr<Camera> camera)\n> @@ -180,27 +168,10 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)\n>  \tcaptureCount_ = 0;\n>  \tcaptureLimit_ = numRequests;\n>  \n> -\t/* Queue the recommended number of reqeuests. */\n>  \tstd::vector<std::unique_ptr<libcamera::Request>> requests;\n> -\tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers) {\n> -\t\tstd::unique_ptr<Request> request = camera_->createRequest();\n> -\t\tif (!request) {\n> -\t\t\tstop();\n> -\t\t\treturn { Results::Fail, \"Can't create request\" };\n> -\t\t}\n> -\n> -\t\tif (request->addBuffer(stream, buffer.get())) {\n> -\t\t\tstop();\n> -\t\t\treturn { Results::Fail, \"Can't set buffer for request\" };\n> -\t\t}\n> -\n> -\t\tif (camera_->queueRequest(request.get()) < 0) {\n> -\t\t\tstop();\n> -\t\t\treturn { Results::Fail, \"Failed to queue request\" };\n> -\t\t}\n> -\n> -\t\trequests.push_back(std::move(request));\n> -\t}\n> +\tret = queueRequests(stream, requests, buffers);\n> +\tif (ret.first != Results::Pass)\n> +\t\treturn ret;\n>  \n>  \t/* Run capture session. */\n>  \tloop_ = new EventLoop();\n> @@ -211,37 +182,14 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)\n>  \treturn { status ? Results::Fail : Results::Pass, \"Unbalanced capture of \" + std::to_string(numRequests) + \" requests\" };\n>  }\n>  \n> -void SimpleCaptureUnbalanced::requestComplete(Request *request)\n> -{\n> -\tcaptureCount_++;\n> -\tif (captureCount_ >= captureLimit_) {\n> -\t\tloop_->exit(0);\n> -\t\treturn;\n> -\t}\n> -\n> -\trequest->reuse(Request::ReuseBuffers);\n> -\tif (camera_->queueRequest(request))\n> -\t\tloop_->exit(-EINVAL);\n> -}\n> -\n>  SimpleCaptureOverflow::SimpleCaptureOverflow(std::shared_ptr<Camera> camera)\n>  \t: SimpleCapture(camera)\n>  {\n>  }\n>  \n> -Results::Result SimpleCaptureOverflow::capture()\n> +Results::Result SimpleCapture::queueRequests(Stream *stream, std::vector<std::unique_ptr<libcamera::Request>> &requests,\n> +\t\t\t\t\t     const std::vector<std::unique_ptr<FrameBuffer>> &buffers)\n>  {\n> -\tResults::Result ret = start();\n> -\tif (ret.first != Results::Pass)\n> -\t\treturn ret;\n> -\n> -\tStream *stream = config_->at(0).stream();\n> -\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);\n> -\n> -\tcaptureCount_ = 0;\n> -\tcaptureLimit_ = buffers.size();\n> -\n> -\tstd::vector<std::unique_ptr<libcamera::Request>> requests;\n>  \tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers) {\n>  \t\tstd::unique_ptr<Request> request = camera_->createRequest();\n>  \t\tif (!request) {\n> @@ -254,7 +202,7 @@ Results::Result SimpleCaptureOverflow::capture()\n>  \t\t\treturn { Results::Fail, \"Can't set buffer for request\" };\n>  \t\t}\n>  \n> -\t\tif (camera_->queueRequest(request.get()) < 0) {\n> +\t\tif (queueRequest(request.get()) < 0) {\n>  \t\t\tstop();\n>  \t\t\treturn { Results::Fail, \"Failed to queue request\" };\n>  \t\t}\n> @@ -262,6 +210,26 @@ Results::Result SimpleCaptureOverflow::capture()\n>  \t\trequests.push_back(std::move(request));\n>  \t}\n>  \n> +\treturn { Results::Pass, \"Succesfully queued requests\" };\n> +}\n> +\n> +Results::Result SimpleCaptureOverflow::capture()\n> +{\n> +\tResults::Result ret = start();\n> +\tif (ret.first != Results::Pass)\n> +\t\treturn ret;\n> +\n> +\tStream *stream = config_->at(0).stream();\n> +\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);\n> +\n> +\tcaptureCount_ = 0;\n> +\tcaptureLimit_ = buffers.size();\n> +\n> +\tstd::vector<std::unique_ptr<libcamera::Request>> requests;\n> +\tret = queueRequests(stream, requests, buffers);\n> +\tif (ret.first != Results::Pass)\n> +\t\treturn ret;\n> +\n>  \t/* Run capture session. */\n>  \tloop_ = new EventLoop();\n>  \tloop_->exec();\n> diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h\n> index cafcdafe10c2..754dc6ea0db8 100644\n> --- a/src/lc-compliance/simple_capture.h\n> +++ b/src/lc-compliance/simple_capture.h\n> @@ -26,11 +26,18 @@ protected:\n>  \n>  \tResults::Result start();\n>  \tvoid stop();\n> +\tint queueRequest(libcamera::Request *request);\n> +\tResults::Result queueRequests(libcamera::Stream *stream,\n> +\t\t\t\t      std::vector<std::unique_ptr<libcamera::Request>> &requests,\n> +\t\t\t\t      const std::vector<std::unique_ptr<libcamera::FrameBuffer>> &buffers);\n>  \n> -\tvirtual void requestComplete(libcamera::Request *request) = 0;\n> +\tvoid requestComplete(libcamera::Request *request);\n>  \n>  \tEventLoop *loop_;\n>  \n> +\tunsigned int captureCount_;\n> +\tunsigned int captureLimit_;\n> +\n>  \tstd::shared_ptr<libcamera::Camera> camera_;\n>  \tstd::unique_ptr<libcamera::FrameBufferAllocator> allocator_;\n>  \tstd::unique_ptr<libcamera::CameraConfiguration> config_;\n> @@ -45,11 +52,9 @@ public:\n>  \n>  private:\n>  \tint queueRequest(libcamera::Request *request);\n> -\tvoid requestComplete(libcamera::Request *request) override;\n> +\tvoid requestComplete(libcamera::Request *request);\n>  \n>  \tunsigned int queueCount_;\n> -\tunsigned int captureCount_;\n> -\tunsigned int captureLimit_;\n>  };\n>  \n>  class SimpleCaptureUnbalanced : public SimpleCapture\n> @@ -59,11 +64,6 @@ public:\n>  \n>  \tResults::Result capture(unsigned int numRequests);\n>  \n> -private:\n> -\tvoid requestComplete(libcamera::Request *request) override;\n> -\n> -\tunsigned int captureCount_;\n> -\tunsigned int captureLimit_;\n>  };\n>  \n>  class SimpleCaptureOverflow : public SimpleCapture\n> @@ -75,10 +75,7 @@ public:\n>  \tResults::Result capture();\n>  \n>  private:\n> -\tvoid requestComplete(libcamera::Request *request) override;\n> -\n> -\tunsigned int captureCount_;\n> -\tunsigned int captureLimit_;\n> +\tvoid requestComplete(libcamera::Request *request);\n>  };\n>  \n>  #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */\n> -- \n> 2.31.1\n>","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 BDC9EBDE45\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 29 Apr 2021 11:23:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0176668909;\n\tThu, 29 Apr 2021 13:23:46 +0200 (CEST)","from mail-lj1-x232.google.com (mail-lj1-x232.google.com\n\t[IPv6:2a00:1450:4864:20::232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CE67C60511\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 29 Apr 2021 13:23:45 +0200 (CEST)","by mail-lj1-x232.google.com with SMTP id s9so18097249ljj.6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 29 Apr 2021 04:23:45 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tl11sm511879ljj.9.2021.04.29.04.23.44\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 29 Apr 2021 04:23:44 -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=\"EuLVKVXJ\"; 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=XlKrMHV/bN4YeL2LZqD7WZhpu0W9z/95GSkaU+82ThI=;\n\tb=EuLVKVXJSTUG3/3PcuMHj1Rw1ZqHQfBcb1+8o/jrXFh+WhEdefadCOlYFqY8X78C6D\n\t4wVKvP80Q0SVqcnevqjPlnbNNSHuhwinOUHLW37sMIWV7nxpt9cuMUZgszH40VuE0mqv\n\t7G4iTs61QlDzp8curtBtRz+xOcPvGOhVQPeop+j76E0XQviPnH0qupucILcco7gJOn4O\n\tWazQgm46oywdPg47vIA5rrJ4/wWwLLqPkONq7I4hluzuEGYmrWRIxWpyxhuDyZXbc/V4\n\thLCvCeAzGA0uFebgczq6oWBCbPdrC4qoRzAUkFzDObQ3C9v6581aIGH0lsQEBQLv6SL6\n\tYMIQ==","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=XlKrMHV/bN4YeL2LZqD7WZhpu0W9z/95GSkaU+82ThI=;\n\tb=G0FQ5PclLQ2MRC5/u9cfd+2dkFoC+6C8+ML1BJ4HtrIfaDknAqwjClYfP/bq7rnfHI\n\tDGNOpJa+lmOgUV+f5Exs3ghNgLWRDSEuddzkBS0nIUSf0uwYBjXJWuVDuYPJL1FYs7K5\n\tgxeoLPsj4YXL4sZ08n3LgaHysQe2tXxGJ7lHTsYvpKX+G2AKg/6s5DWpnycKdIclR5oZ\n\t5lFBzKgBYCGLZpbG80R+T3qDFD1+TULafgxUyI2yiw3tmRE04VC8iUEOWt7h6IJNHIK3\n\t5jRZi6bi8E3f04IfMnqd19KKwWcnLGDGxLjPiZxWxr60SVFRaLl6y6ZGxJI7liWHEP0h\n\t91Ng==","X-Gm-Message-State":"AOAM531BTUWKcqu6JfS0g/N6wZcuVEAaPRK1wQW6kAXeJfxvnbWo56r5\n\tTKp/v8K3XKC0gWYV6gpcML2ANw==","X-Google-Smtp-Source":"ABdhPJxTzfmuNlxXRD6c5tcni9FkkzDKXrR794NNZNkH9tM4cZeVo1++2g1/HK1tHC1h6Hc9Yv6Uiw==","X-Received":"by 2002:a2e:a7d1:: with SMTP id\n\tx17mr23979160ljp.384.1619695425069; \n\tThu, 29 Apr 2021 04:23:45 -0700 (PDT)","Date":"Thu, 29 Apr 2021 13:23:43 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado <nfraprado@collabora.com>","Message-ID":"<YIqXP8NbruaN8osS@oden.dyn.berto.se>","References":"<YIaKdRbZU/wjcmEd@oden.dyn.berto.se>\n\t<CAZEFVDKB9AQ.2CQTDQOY1HI90@notapiano>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<CAZEFVDKB9AQ.2CQTDQOY1HI90@notapiano>","Subject":"Re: [libcamera-devel] [PATCH v3 3/4] lc-compliance: Add test to\n\tqueue more requests than hardware depth","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, =?iso-8859-1?q?Andr=E9?=\n\tAlmeida <andrealmeid@collabora.com>,  kernel@collabora.com","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>"}}]