[{"id":25929,"web_url":"https://patchwork.libcamera.org/comment/25929/","msgid":"","date":"2022-11-30T11:18:16","subject":"Re: [libcamera-devel] [PATCH v8 08/17] v4l2: Allocate buffers based\n\ton requested count and MinimumRequests","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Aug 24, 2021 at 04:56:27PM -0300, Nícolas F. R. A. Prado wrote:\n> Currently the number of buffers allocated is based on bufferCount, which\n> is hardcoded to 1. Instead allocate buffers based on the requested count\n> and on the MinimumRequests property for the camera, which is accessed\n> through a newly added getter.\n> \n> This allows the removal of bufferCount.\n> \n> While at it, fix a typo: s/interval/internal/.\n> \n> Signed-off-by: Nícolas F. R. A. Prado \n\nReviewed-by: Paul Elder \n\n> \n> ---\n> \n> Changes in v8:\n> - New\n> - Fixed typo: s/interval/internal/\n> \n> src/v4l2/v4l2_camera.cpp | 20 +++++++++++++-------\n> src/v4l2/v4l2_camera.h | 5 +++--\n> src/v4l2/v4l2_camera_proxy.cpp | 11 +++++------\n> 3 files changed, 21 insertions(+), 15 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> index d01eacfa2b84..4db171a3c0d0 100644\n> --- a/src/v4l2/v4l2_camera.cpp\n> +++ b/src/v4l2/v4l2_camera.cpp\n> @@ -12,6 +12,8 @@\n> \n> #include \n> \n> +#include \n> +\n> using namespace libcamera;\n> \n> LOG_DECLARE_CATEGORY(V4L2Compat)\n> @@ -107,15 +109,13 @@ void V4L2Camera::requestComplete(Request *request)\n> }\n> \n> int V4L2Camera::configure(StreamConfiguration *streamConfigOut,\n> -\t\t\t const Size &size, const PixelFormat &pixelformat,\n> -\t\t\t unsigned int bufferCount)\n> +\t\t\t const Size &size, const PixelFormat &pixelformat)\n> {\n> \tStreamConfiguration &streamConfig = config_->at(0);\n> \tstreamConfig.size.width = size.width;\n> \tstreamConfig.size.height = size.height;\n> \tstreamConfig.pixelFormat = pixelformat;\n> -\tstreamConfig.bufferCount = bufferCount;\n> -\t/* \\todo memoryType (interval vs external) */\n> +\t/* \\todo memoryType (internal vs external) */\n> \n> \tCameraConfiguration::Status validation = config_->validate();\n> \tif (validation == CameraConfiguration::Invalid) {\n> @@ -146,7 +146,6 @@ int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,\n> \tStreamConfiguration &cfg = config->at(0);\n> \tcfg.size = size;\n> \tcfg.pixelFormat = pixelFormat;\n> -\tcfg.bufferCount = 1;\n> \n> \tCameraConfiguration::Status validation = config->validate();\n> \tif (validation == CameraConfiguration::Invalid)\n> @@ -165,7 +164,9 @@ int V4L2Camera::allocBuffers(unsigned int count)\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> -\tfor (unsigned int i = 0; i < count; i++) {\n> +\tunsigned int allocatedCount = ret;\n> +\n> +\tfor (unsigned int i = 0; i < allocatedCount; i++) {\n> \t\tstd::unique_ptr request = camera_->createRequest(i);\n> \t\tif (!request) {\n> \t\t\trequestPool_.clear();\n> @@ -174,7 +175,7 @@ int V4L2Camera::allocBuffers(unsigned int count)\n> \t\trequestPool_.push_back(std::move(request));\n> \t}\n> \n> -\treturn ret;\n> +\treturn allocatedCount;\n> }\n> \n> void V4L2Camera::freeBuffers()\n> @@ -299,3 +300,8 @@ bool V4L2Camera::isRunning()\n> {\n> \treturn isRunning_;\n> }\n> +\n> +unsigned int V4L2Camera::minimumRequests()\n> +{\n> +\treturn camera_->properties().get(properties::MinimumRequests);\n> +}\n> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h\n> index a095f4e27524..c0a122906f35 100644\n> --- a/src/v4l2/v4l2_camera.h\n> +++ b/src/v4l2/v4l2_camera.h\n> @@ -45,8 +45,7 @@ public:\n> \tstd::vector completedBuffers();\n> \n> \tint configure(StreamConfiguration *streamConfigOut,\n> -\t\t const Size &size, const PixelFormat &pixelformat,\n> -\t\t unsigned int bufferCount);\n> +\t\t const Size &size, const PixelFormat &pixelformat);\n> \tint validateConfiguration(const PixelFormat &pixelformat,\n> \t\t\t\t const Size &size,\n> \t\t\t\t StreamConfiguration *streamConfigOut);\n> @@ -65,6 +64,8 @@ public:\n> \n> \tbool isRunning();\n> \n> +\tunsigned int minimumRequests();\n> +\n> private:\n> \tvoid requestComplete(Request *request);\n> \n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 7682c4bddf90..424262dd205c 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -349,8 +349,7 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg)\n> \tSize size(arg->fmt.pix.width, arg->fmt.pix.height);\n> \tV4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);\n> \tret = vcam_->configure(&streamConfig_, size,\n> -\t\t\t PixelFormatInfo::info(v4l2Format).format,\n> -\t\t\t bufferCount_);\n> +\t\t\t PixelFormatInfo::info(v4l2Format).format);\n> \tif (ret < 0)\n> \t\treturn -EINVAL;\n> \n> @@ -491,15 +490,13 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf\n> \tSize size(v4l2PixFormat_.width, v4l2PixFormat_.height);\n> \tV4L2PixelFormat v4l2Format = V4L2PixelFormat(v4l2PixFormat_.pixelformat);\n> \tint ret = vcam_->configure(&streamConfig_, size,\n> -\t\t\t\t PixelFormatInfo::info(v4l2Format).format,\n> -\t\t\t\t arg->count);\n> +\t\t\t\t PixelFormatInfo::info(v4l2Format).format);\n> \tif (ret < 0)\n> \t\treturn -EINVAL;\n> \n> \tsetFmtFromConfig(streamConfig_);\n> \n> -\targ->count = streamConfig_.bufferCount;\n> -\tbufferCount_ = arg->count;\n> +\targ->count = std::max(arg->count, vcam_->minimumRequests());\n> \n> \tret = vcam_->allocBuffers(arg->count);\n> \tif (ret < 0) {\n> @@ -507,6 +504,8 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf\n> \t\treturn ret;\n> \t}\n> \n> +\tbufferCount_ = arg->count = ret;\n> +\n> \tbuffers_.resize(arg->count);\n> \tfor (unsigned int i = 0; i < arg->count; i++) {\n> \t\tstruct v4l2_buffer buf = {};\n> -- \n> 2.33.0\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 1FF6CBDE6B\n\tfor ;\n\tWed, 30 Nov 2022 11:18:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9092061F24;\n\tWed, 30 Nov 2022 12:18:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4332961F23\n\tfor ;\n\tWed, 30 Nov 2022 12:18:24 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 713F613EA;\n\tWed, 30 Nov 2022 12:18:22 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669807106;\n\tbh=dG9LD6hss1Dj9QDjvyxkw1wNGTbZH4zBZ2wrvFau6OI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=PbQ7xoFDrhR0F1S6rwFjyR/QnbPx1Il4N7JUSFm0zK3nrai6jvB1zR138GIwIekyl\n\tnJWTgD/z6utIdgmKLBwl1pHKKSGxXoiskMRi8GZac1rkUplxVvPeK6uSRjSLRuxaLa\n\tU21nWVqlGMrLbkCWym/Nq5yQc5Gi2ambTamREbslX/AguOyxZ20H07IdzshmPwqMDX\n\tVt89PQNfvBUpi8SNtvLTJ4M5Q/0TIa5mFt3P/3sOj+W+e2M7ROD4eZGV1K55lPSs7X\n\tW+YlCMhB525tACp6LVkfgr8NHfmf3k/3F6fyMfyAZxVG5UMu/Q8PDV07stqTC4ySnP\n\tOZfGvKR/NHxPw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669807103;\n\tbh=dG9LD6hss1Dj9QDjvyxkw1wNGTbZH4zBZ2wrvFau6OI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iiqS11tXZrfyF/tNggXiQfXwAURR2IuAi4wf86XkHjo9Me8j0/DJ4an0tX50ffsxw\n\tp7cOF1j926lJKT+2+4qgz1oBnRJsSS4bU90kpQT4DGBkPlmFNz6wMeJIz4ner5JqU0\n\t/vyi6HqYSO+xMjvk6dA+1Qhx50fi2uybO7K9HRaQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"iiqS11tX\"; dkim-atps=neutral","Date":"Wed, 30 Nov 2022 20:18:16 +0900","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado ","Message-ID":"","References":"<20210824195636.1110845-1-nfraprado@collabora.com>\n\t<20210824195636.1110845-9-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210824195636.1110845-9-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v8 08/17] v4l2: Allocate buffers based\n\ton requested count and MinimumRequests","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","From":"Paul Elder via libcamera-devel ","Reply-To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]