[{"id":33142,"web_url":"https://patchwork.libcamera.org/comment/33142/","msgid":"","date":"2025-01-23T01:57:25","subject":"Re: [RFC PATCH v2 14/16] apps: lc-compliance: Support multiple\n\tstreams in helpers","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Jan 14, 2025 at 06:22:56PM +0000, Barnabás Pőcze wrote:\n> Prepare to add a test suite for capture operations with multiple\n> streams.\n> \n> Modify the Capture helper class to support multiple roles and streams\n> in the configure() and capture() operations.\n> \n> Multi-stream support will be added in next patches.\n> \n> Co-developed-by: Jacopo Mondi \n> Signed-off-by: Jacopo Mondi \n> Signed-off-by: Barnabás Pőcze \n\nReviewed-by: Paul Elder \n\n> ---\n> src/apps/lc-compliance/helpers/capture.cpp | 77 +++++++++++++++----\n> src/apps/lc-compliance/helpers/capture.h | 2 +-\n> src/apps/lc-compliance/tests/capture_test.cpp | 6 +-\n> 3 files changed, 66 insertions(+), 19 deletions(-)\n> \n> diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp\n> index 77e87c9e4..dbaff4138 100644\n> --- a/src/apps/lc-compliance/helpers/capture.cpp\n> +++ b/src/apps/lc-compliance/helpers/capture.cpp\n> @@ -24,13 +24,31 @@ Capture::~Capture()\n> \tstop();\n> }\n> \n> -void Capture::configure(StreamRole role)\n> +void Capture::configure(libcamera::Span roles)\n> {\n> -\tconfig_ = camera_->generateConfiguration({ role });\n> +\tassert(!roles.empty());\n> +\n> +\tconfig_ = camera_->generateConfiguration(roles);\n> \n> \tif (!config_)\n> \t\tGTEST_SKIP() << \"Role not supported by camera\";\n> \n> +\tASSERT_EQ(config_->size(), roles.size()) << \"Unexpected number of streams in configuration\";\n> +\n> +\t/*\n> +\t * Set the buffers count to the largest value across all streams.\n> +\t * \\todo: Should all streams from a Camera have the same buffer count ?\n> +\t */\n> +\tauto largest =\n> +\t\tstd::max_element(config_->begin(), config_->end(),\n> +\t\t\t\t [](const StreamConfiguration &l, const StreamConfiguration &r)\n> +\t\t\t\t { return l.bufferCount < r.bufferCount; });\n> +\n> +\tassert(largest != config_->end());\n> +\n> +\tfor (auto &cfg : *config_)\n> +\t\tcfg.bufferCount = largest->bufferCount;\n> +\n> \tif (config_->validate() != CameraConfiguration::Valid) {\n> \t\tconfig_.reset();\n> \t\tFAIL() << \"Configuration not valid\";\n> @@ -76,20 +94,36 @@ void Capture::prepareRequests(std::optional queueLimit)\n> \tassert(config_);\n> \tassert(requests_.empty());\n> \n> -\tStream *stream = config_->at(0).stream();\n> -\tconst std::vector> &buffers = allocator_.buffers(stream);\n> +\tstd::size_t maxBuffers = 0;\n> +\n> +\tfor (const auto &cfg : *config_) {\n> +\t\tconst auto &buffers = allocator_.buffers(cfg.stream());\n> +\t\tASSERT_FALSE(buffers.empty()) << \"Zero buffers allocated for stream\";\n> +\n> +\t\tmaxBuffers = std::max(maxBuffers, buffers.size());\n> +\t}\n> \n> \t/* No point in testing less requests then the camera depth. */\n> -\tif (queueLimit && *queueLimit < buffers.size()) {\n> -\t\tGTEST_SKIP() << \"Camera needs \" << buffers.size()\n> +\tif (queueLimit && *queueLimit < maxBuffers) {\n> +\t\tGTEST_SKIP() << \"Camera needs \" << maxBuffers\n> \t\t\t << \" requests, can't test only \" << *queueLimit;\n> \t}\n> \n> -\tfor (const std::unique_ptr &buffer : buffers) {\n> -\t\tstd::unique_ptr request = camera_->createRequest();\n> +\tfor (std::size_t i = 0; i < maxBuffers; i++) {\n> +\t\tstd::unique_ptr request = camera_->createRequest(i);\n> \t\tASSERT_TRUE(request) << \"Can't create request\";\n> \n> -\t\tASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << \"Can't set buffer for request\";\n> +\t\tfor (const auto &cfg : *config_) {\n> +\t\t\tStream *stream = cfg.stream();\n> +\t\t\tconst auto &buffers = allocator_.buffers(stream);\n> +\t\t\tassert(!buffers.empty());\n> +\n> +\t\t\tif (i >= buffers.size())\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tASSERT_EQ(request->addBuffer(stream, buffers[i].get()), 0)\n> +\t\t\t\t<< \"Can't add buffer to request\";\n> +\t\t}\n> \n> \t\trequests_.push_back(std::move(request));\n> \t}\n> @@ -125,11 +159,19 @@ void Capture::requestComplete(Request *request)\n> \n> void Capture::start()\n> {\n> -\tStream *stream = config_->at(0).stream();\n> -\tint count = allocator_.allocate(stream);\n> +\tassert(config_);\n> +\tassert(!config_->empty());\n> +\tassert(!allocator_.allocated());\n> +\n> +\tfor (const auto &cfg : *config_) {\n> +\t\tStream *stream = cfg.stream();\n> +\t\tint count = allocator_.allocate(stream);\n> +\n> +\t\tASSERT_GE(count, 0) << \"Failed to allocate buffers\";\n> +\t\tEXPECT_EQ(count, cfg.bufferCount) << \"Allocated less buffers than expected\";\n> +\t}\n> \n> -\tASSERT_GE(count, 0) << \"Failed to allocate buffers\";\n> -\tEXPECT_EQ(count, config_->at(0).bufferCount) << \"Allocated less buffers than expected\";\n> +\tASSERT_TRUE(allocator_.allocated());\n> \n> \tcamera_->requestCompleted.connect(this, &Capture::requestComplete);\n> \n> @@ -145,7 +187,12 @@ void Capture::stop()\n> \n> \tcamera_->requestCompleted.disconnect(this);\n> \n> -\tStream *stream = config_->at(0).stream();\n> \trequests_.clear();\n> -\tallocator_.free(stream);\n> +\n> +\tfor (const auto &cfg : *config_) {\n> +\t\tint ret = allocator_.free(cfg.stream());\n> +\t\tEXPECT_EQ(ret, 0) << \"Failed to free buffers associated with stream\";\n> +\t}\n> +\n> +\tEXPECT_FALSE(allocator_.allocated());\n> }\n> diff --git a/src/apps/lc-compliance/helpers/capture.h b/src/apps/lc-compliance/helpers/capture.h\n> index 173421fd2..391184ad6 100644\n> --- a/src/apps/lc-compliance/helpers/capture.h\n> +++ b/src/apps/lc-compliance/helpers/capture.h\n> @@ -20,7 +20,7 @@ public:\n> \tCapture(std::shared_ptr camera);\n> \t~Capture();\n> \n> -\tvoid configure(libcamera::StreamRole role);\n> +\tvoid configure(libcamera::Span roles);\n> \tvoid run(unsigned int captureLimit, std::optional queueLimit = {});\n> \n> private:\n> diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp\n> index 93bed48f0..147e17019 100644\n> --- a/src/apps/lc-compliance/tests/capture_test.cpp\n> +++ b/src/apps/lc-compliance/tests/capture_test.cpp\n> @@ -89,7 +89,7 @@ TEST_P(SingleStream, Capture)\n> \n> \tCapture capture(camera_);\n> \n> -\tcapture.configure(role);\n> +\tcapture.configure(std::array{ role });\n> \n> \tcapture.run(numRequests, numRequests);\n> }\n> @@ -108,7 +108,7 @@ TEST_P(SingleStream, CaptureStartStop)\n> \n> \tCapture capture(camera_);\n> \n> -\tcapture.configure(role);\n> +\tcapture.configure(std::array{ role });\n> \n> \tfor (unsigned int starts = 0; starts < numRepeats; starts++)\n> \t\tcapture.run(numRequests, numRequests);\n> @@ -127,7 +127,7 @@ TEST_P(SingleStream, UnbalancedStop)\n> \n> \tCapture capture(camera_);\n> \n> -\tcapture.configure(role);\n> +\tcapture.configure(std::array{ role });\n> \n> \tcapture.run(numRequests);\n> }\n> -- \n> 2.48.0\n> \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 42111BE08B\n\tfor ;\n\tThu, 23 Jan 2025 01:57:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE3DC68558;\n\tThu, 23 Jan 2025 02:57:30 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1342261878\n\tfor ;\n\tThu, 23 Jan 2025 02:57:30 +0100 (CET)","from pyrite.rasen.tech (unknown [IPv6:2603:6081:63f0:60f0::17f2])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4009213C;\n\tThu, 23 Jan 2025 02:56:26 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"q28yAtb6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737597387;\n\tbh=ySA63RjAtn7YZtzjxkPQl3uoLejbda+032iUj12rg3k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=q28yAtb6f4A2wprUypWV0O698mo5z8ucqxHQVMwvCqFSNZPhE9GkcE996faSQehUc\n\tRb6gw+2mcfjDDcH6LUkRJiTw2Bu1ytw/Lvb4dFkUKgSyA1h0u1hYBsadYpX9J8Q62H\n\tdNQZo9G05YzsxyaVF4qn3BPD9tU+g2CmPkalobBs=","Date":"Wed, 22 Jan 2025 20:57:25 -0500","From":"Paul Elder ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org,\n\tJacopo Mondi ","Subject":"Re: [RFC PATCH v2 14/16] apps: lc-compliance: Support multiple\n\tstreams in helpers","Message-ID":"","References":"<20250114182143.1773762-1-pobrn@protonmail.com>\n\t<20250114182143.1773762-15-pobrn@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250114182143.1773762-15-pobrn@protonmail.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]