From patchwork Fri Dec 16 12:29:22 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18015 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id BB65CC31E9 for ; Fri, 16 Dec 2022 12:30:05 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7C8716339D; Fri, 16 Dec 2022 13:30:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193805; bh=J+Dh7BQdJU0XP23221A3LpBpTQDVMagTTDvFDJONVy0=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=RJSy89iXZm52GinAcfrbVwujRtNsb3eTgsmcGOewnAzZxNaGD+MSs9EZnWlY6vXpc vy62myk9OPS0KcTM1q63mAVGkd+M1fVvKwLEtM8FFSdhLSa8R/IKGjxxsXGoxamwn/ szC62e2IxF9tDlriVgyFqGQ7xGfBa6yLN7/EyqOHt10FRTpUc1odAgxHBjwb+iAseC LbIwbxrXVnKaGetzEKUAgX/2QiRwGA2hCMhQX5WG33zCL1OsYvF8K89RhutfEFAlPQ /TZS1Bu+3uqITX/uBFnUPusWAZSqJY6w7iaPrH7+MpN0C2WYfAUuIMqBeN1IPKc4n5 1+jb9LGzrGYBg== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C9E0F6339D for ; Fri, 16 Dec 2022 13:30:02 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ZSFno2sI"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9F0D0110E; Fri, 16 Dec 2022 13:30:01 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193802; bh=J+Dh7BQdJU0XP23221A3LpBpTQDVMagTTDvFDJONVy0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZSFno2sIZ2lfeBn2c7BswbOMUZhX3ZoW4PJSMFSsDOgo+p7ULemD2xB8Abn4FxWRl HMkRSdffudqf5Xe1A3+V5yTHHuNGv6bnEglt4rmJVoQHu1zxwYO8ZpB6aJFA1zZXTf 5tcZiqbdDYZoH70dtMX0yL4zg5lhOCr0zay2niB4= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:22 +0900 Message-Id: <20221216122939.256534-2-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 01/18] pipeline: rkisp1: Reorder headers to appease the linter X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Re-order the header includes to appease the style-checker, so that when we add headers later (specifically, property_ids.h), it won't complain. Signed-off-by: Paul Elder Reviewed-by: Jacopo Mondi Reviewed-by: Umang Jain --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index eb9ad65c..f658d75e 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -23,11 +23,11 @@ #include #include #include +#include +#include #include #include #include -#include -#include #include "libcamera/internal/camera.h" #include "libcamera/internal/camera_sensor.h" From patchwork Fri Dec 16 12:29:23 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18016 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 2EF6CC31E9 for ; Fri, 16 Dec 2022 12:30:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 37FC86339E; Fri, 16 Dec 2022 13:30:06 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193806; bh=35LwYXm0BAOIYIDBVnWhJMZneP3HtGkAGqX3vIFrXH4=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=rsJsspHIZLtjsq5dLisQCXmEyOwyNEH/K/0Sbl1hfYgmqKs/wE7icEcdJ+RgVUCk+ J8WTZSuK16QP9fdExfnTzlj3UQr/QphlGeOOkknJ4fZoiDnYlO5qy1Zdhcur11bkmI 1KN7rIjc/5t9087YdUxwYUY3PTDumv+v8MzhexGIvt2Tr9cJIHRYYXzbr3Cz0A3Uwp Eni/nnCPMR7jb6afaDPM6rUcFSk4/UkOJ2ubv/o/UhmFR3KoYnWWEym3YNzxdCrv2q Lqa6AdDXxVAnW6815AkxpkcqNaY0mcBcEVzXBJlJkWeTqXoWlkRpE7/gUq3KeG1Ep3 H/7RoFMqgPiHw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C602E63360 for ; Fri, 16 Dec 2022 13:30:04 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Tya1LJnQ"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 28055A31; Fri, 16 Dec 2022 13:30:02 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193804; bh=35LwYXm0BAOIYIDBVnWhJMZneP3HtGkAGqX3vIFrXH4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Tya1LJnQ2EGT9J+0KPfxNnZwSLwEcSxF64zWlhzge8p/xuUumaFpnicmKeW575wXI yhguPtWI30/TJ2/KVlOHMPWZU7HIck/1UvW/6HU1eJwmvOujHfElJzY7lc+PONWayL 4/yWT5Z/AAbIPg3/oETaPGqUSMJygtj7edrFgqJM= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:23 +0900 Message-Id: <20221216122939.256534-3-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 02/18] libcamera: property: Add MinimumRequests property X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado The MinimumRequests property reports the minimum number of capture requests that the camera pipeline requires to have queued in order to sustain capture operations without frame drops. At this quantity, there's no guarantee that manual per-frame controls will apply correctly. The quantity needed for that may be added as a separate property in the future. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Laurent Pinchart Reviewed-by: Paul Elder Signed-off-by: Paul Elder Acked-by: Umang Jain --- Changes in v9: - rebased Changes in v8: - Changed the MinimumRequests property meaning to require that frames aren't dropped - Set MinimumRequests on SimplePipeline depending on device and usage of converter - Undid definition of default MinimumRequests on CameraSensor constructor - Updated pipeline-handler guides with new MinimumRequests property Changes in v7: - Renamed property from MinNumRequests to MinimumRequests - Changed MinimumRequests property's description Changes in v6: - Removed comment from Raspberrypi MinNumRequests setting - Removed an extra blank line --- Documentation/guides/pipeline-handler.rst | 22 +++++++--- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 + .../pipeline/raspberrypi/raspberrypi.cpp | 2 + src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 + src/libcamera/pipeline/simple/simple.cpp | 40 +++++++++++++++++-- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 + src/libcamera/pipeline/vimc/vimc.cpp | 2 + src/libcamera/property_ids.yaml | 21 ++++++++++ 8 files changed, 84 insertions(+), 9 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index e1930fdf..a7356e4a 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -658,19 +658,31 @@ associated with immutable values, which represent static characteristics that ca be used by applications to identify camera devices in the system. Properties can be registered by inspecting the values of V4L2 controls from the video devices and camera sensor (for example to retrieve the position and orientation of a camera) -or to express other immutable characteristics. The example pipeline handler does -not register any property, but examples are available in the libcamera code -base. +or to express other immutable characteristics. -.. TODO: Add a property example to the pipeline handler. At least the model. +A required property is ``MinimumRequests``, which indicates how many requests +need to be queued in the pipeline for capture without frame drops to be +possible. + +In our case, the vivid driver requires two buffers before it'll start streaming +(can be seen in the ``min_buffers_needed`` property for the ``vid_cap`` queue in +vivid's driver code). In order to not drop frames we should have one extra +buffer queued to the driver so that it is used as soon as the previous buffer +completes. Therefore we will set our ``MinimumRequests`` to three. Append the +following line to ``init()``: + +.. code-block:: cpp + + properties_.set(properties::MinimumRequests, 3); At this point you need to add the following includes to the top of the file for -handling controls: +handling controls and properties: .. code-block:: cpp #include #include + #include Generating a default configuration ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index e4d79ea4..98a4a3e5 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -1126,6 +1126,8 @@ int PipelineHandlerIPU3::registerCameras() /* Initialize the camera properties. */ data->properties_ = cio2->sensor()->properties(); + data->properties_.set(properties::MinimumRequests, 3); + ret = initControls(data.get()); if (ret) continue; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 8569df17..4a08d01e 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1063,6 +1063,8 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls) /* Enable SOF event generation. */ data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true); + data->properties_.set(properties::MinimumRequests, 3); + /* * Reset the delayed controls with the gain and exposure values set by * the IPA. diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index f658d75e..45b6beaf 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -1102,6 +1103,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) /* Initialize the camera properties. */ data->properties_ = data->sensor_->properties(); + data->properties_.set(properties::MinimumRequests, 3); /* * \todo Read dealy values from the sensor itself or from a diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 24ded4db..8ed983fe 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -180,6 +181,10 @@ class SimplePipelineHandler; struct SimplePipelineInfo { const char *driver; + /* + * Minimum number of buffers required by the driver to start streaming. + */ + unsigned int minimumBuffers; /* * Each converter in the list contains the name * and the number of streams it supports. @@ -190,10 +195,10 @@ struct SimplePipelineInfo { namespace { static const SimplePipelineInfo supportedDevices[] = { - { "imx7-csi", { { "pxp", 1 } } }, - { "mxc-isi", {} }, - { "qcom-camss", {} }, - { "sun6i-csi", {} }, + { "imx7-csi", 2, { { "pxp", 1 } } }, + { "mxc-isi", 3, {} }, + { "qcom-camss", 1, {} }, + { "sun6i-csi", 3, {} }, }; } /* namespace */ @@ -271,6 +276,8 @@ public: bool useConverter_; std::queue> converterQueue_; + const SimplePipelineInfo *deviceInfo; + private: void tryPipeline(unsigned int code, const Size &size); static std::vector routedSourcePads(MediaPad *sink); @@ -1168,6 +1175,29 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) inputCfg.stride = captureFormat.planes[0].bpl; inputCfg.bufferCount = kNumInternalBuffers; + /* Set the MinimumRequests property. */ + unsigned int minimumRequests; + + if (data->useConverter_) { + /* + * The application will interact only with the capture node of + * the converter. Require two buffers for a frame drop free + * conversion, plus one extra to account for requeue delays. + */ + minimumRequests = 3; + } else { + /* + * The application will interact directly with the video capture + * device. Require the minimum required by the driver, plus one + * extra to account for requeue delays. Force at least three + * buffers in order to not drop frames. + */ + minimumRequests = std::max(data->deviceInfo->minimumBuffers + 1, + 3U); + } + + data->properties_.set(properties::MinimumRequests, minimumRequests); + return data->converter_->configure(inputCfg, outputCfgs); } @@ -1506,6 +1536,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) bool registered = false; for (std::unique_ptr &data : pipelines) { + data->deviceInfo = info; + int ret = data->init(); if (ret < 0) continue; diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 277465b7..7f580955 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -500,6 +500,8 @@ int UVCCameraData::init(MediaDevice *media) properties_.set(properties::PixelArraySize, resolution); properties_.set(properties::PixelArrayActiveAreas, { Rectangle(resolution) }); + properties_.set(properties::MinimumRequests, 3); + /* Initialise the supported controls. */ ControlInfoMap::Map ctrls; diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 204f5ad7..d2633be4 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -571,6 +572,7 @@ int VimcCameraData::init() /* Initialize the camera properties. */ properties_ = sensor_->properties(); + properties_.set(properties::MinimumRequests, 3); return 0; } diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml index cb55e0ed..c82ac17e 100644 --- a/src/libcamera/property_ids.yaml +++ b/src/libcamera/property_ids.yaml @@ -690,6 +690,27 @@ controls: that is twice that of the full resolution mode. This value will be valid after the configure method has returned successfully. + - MinimumRequests: + type: int32_t + description: | + The minimum number of capture requests that the camera pipeline requires + to have queued in order to sustain capture operations without frame + drops. At this quantity, there's no guarantee that manual per-frame + controls will apply correctly. + + This property is based on the minimum number of memory buffers + needed to fill the capture pipeline composed of hardware processing + blocks plus as many buffers as needed to take into account propagation + delays and avoid dropping frames. + + \todo Should this be a per-stream property? + + \todo Extend this property to expose the different minimum buffer and + request counts (the minimum number of buffers to be able to capture at + all, the minimum number of buffers to sustain capture without frame + drop, and the minimum number of requests to ensure proper operation of + per-frame controls). + # ---------------------------------------------------------------------------- # Draft properties section From patchwork Fri Dec 16 12:29:24 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18017 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 7CB1EC31E9 for ; Fri, 16 Dec 2022 12:30:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 33E1E633A4; Fri, 16 Dec 2022 13:30:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193809; bh=C1lOOwQ/7O2Co/57KOtHQ6+8s9cqLHzcW8b4zm1YOVE=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=3iWI+9H2/oZcMk0JV62y+gjB+HAyJV3UMH9XqxF0uDQyU5Z8c70xUxbVS/DJRzhbX QIcSJsdS5rvTvmaBpL3tgnJWo4nXVdkShVo3pea86n0T0Uurs5RMmE5juqzoNAoPTu 3q0KK8yRx+O4BajNhvByw4ZRDLjt9MXETStdcWmuUtZhixflWpbK1EmB8+J3lRYPpv 1L1Ihhnul+ZOVRhz6C7v004Td9wbHNkJiXZH1h2XgXveL4uoBeaaKDgopw2JBjoyas rN/jisWA29rxKtVE6CsyiTt1HNFKJHt82HzZA0Oo+55imKCwxhdltN/j6xb6BQ/erv afu4uXW6XPbAA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 45AA4633A2 for ; Fri, 16 Dec 2022 13:30:07 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="f/xdFFMp"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 20F50128D; Fri, 16 Dec 2022 13:30:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193807; bh=C1lOOwQ/7O2Co/57KOtHQ6+8s9cqLHzcW8b4zm1YOVE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=f/xdFFMp3YckcXhwCed3SIwDK/DteUZeFdqBjPvB7w1O/zXwJcQiO/QmCZFazxZUN cAuvfyKqiuX7QB8iw+QbMXw9G9vAmBHgLWCZ3EJ0qXd6BF219PnHH5m87BxCLGgWL4 kzz1Q1pev8KWgkDEiNSkK478w3Rh/EmRX2SftcBE= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:24 +0900 Message-Id: <20221216122939.256534-4-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 03/18] libcamera: framebuffer_allocator: Make allocate() require count X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Make FrameBufferAllocator::allocate() require a 'count' argument for the number of buffers to be allocated. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart Reviewed-by: Paul Elder Signed-off-by: Paul Elder Reviewed-by: Jacopo Mondi --- Changes in v9: - rebased - notably, ControlList::get() returns a std::optional, and since the MinimumRequests property is a required property, we can simply get it via .value() - this will be enforced by lc-compliance in a later patch Changes in v8: - Updated application-developer and pipeline-handler guides with new allocate() API and MinimumRequests property - Added handling for when allocate() returns less buffers than needed in cam and the capture unit test - Improved FrameBufferAllocator::allocate() description Changes in v6: - Changed static_cast to convert 'count' to unsigned int instead of 'bufferCount' to int when comparing Changes in v5: - Made sure that qcam allocates at least 2 buffers --- Documentation/guides/application-developer.rst | 9 +++++++-- Documentation/guides/pipeline-handler.rst | 3 +-- include/libcamera/camera.h | 2 +- include/libcamera/framebuffer_allocator.h | 2 +- include/libcamera/internal/pipeline_handler.h | 2 +- src/apps/cam/camera_session.cpp | 13 +++++++++---- src/apps/lc-compliance/simple_capture.cpp | 8 ++++++-- src/apps/qcam/main_window.cpp | 10 +++++++++- src/gstreamer/gstlibcameraallocator.cpp | 5 ++++- src/libcamera/camera.cpp | 4 ++-- src/libcamera/framebuffer_allocator.cpp | 9 +++++++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++-- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 ++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- src/libcamera/pipeline/simple/simple.cpp | 4 ++-- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 7 ++++--- src/libcamera/pipeline/vimc/vimc.cpp | 7 ++++--- src/libcamera/pipeline_handler.cpp | 1 + src/v4l2/v4l2_camera.cpp | 2 +- test/camera/camera_reconfigure.cpp | 5 ++++- test/camera/capture.cpp | 11 +++++++---- test/camera/statemachine.cpp | 5 ++++- test/fence.cpp | 6 +++++- test/mapped-buffer.cpp | 5 ++++- 24 files changed, 90 insertions(+), 42 deletions(-) diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst index 1b2d7727..40bcf67a 100644 --- a/Documentation/guides/application-developer.rst +++ b/Documentation/guides/application-developer.rst @@ -30,6 +30,7 @@ defined names and types without the need of prefixing them. #include #include + #include using namespace libcamera; using namespace std::chrono_literals; @@ -272,7 +273,10 @@ Using the libcamera ``FrameBufferAllocator`` Applications create a ``FrameBufferAllocator`` for a Camera and use it to allocate buffers for streams of a ``CameraConfiguration`` with the -``allocate()`` function. +``allocate()`` function. The number of buffers to be allocated needs to be +specified, and should be at least equal to the value of the ``MinimumRequests`` +property in order for the pipeline to have enough requests to be able to +capture without frame drops. The list of allocated buffers can be retrieved using the ``Stream`` instance as the parameter of the ``FrameBufferAllocator::buffers()`` function. @@ -280,9 +284,10 @@ as the parameter of the ``FrameBufferAllocator::buffers()`` function. .. code:: cpp FrameBufferAllocator *allocator = new FrameBufferAllocator(camera); + unsigned int bufferCount = camera->properties().get(properties::MinimumRequests); for (StreamConfiguration &cfg : *config) { - int ret = allocator->allocate(cfg.stream()); + int ret = allocator->allocate(cfg.stream(), bufferCount); if (ret < 0) { std::cerr << "Can't allocate buffers" << std::endl; return -ENOMEM; diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index a7356e4a..70de82af 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -206,7 +206,7 @@ implementations for the overridden class members. const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int exportFrameBuffers(Camera *camera, Stream *stream, + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count, std::vector> *buffers) override; int start(Camera *camera, const ControlList *controls) override; @@ -1187,7 +1187,6 @@ handle this: .. code-block:: cpp - unsigned int count = stream->configuration().bufferCount; VividCameraData *data = cameraData(camera); return data->video_->exportBuffers(count, buffers); diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 5bb06584..4d32d16b 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -126,7 +126,7 @@ private: void requestComplete(Request *request); friend class FrameBufferAllocator; - int exportFrameBuffers(Stream *stream, + int exportFrameBuffers(Stream *stream, unsigned int count, std::vector> *buffers); }; diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h index 45ff232b..7a0bf1a0 100644 --- a/include/libcamera/framebuffer_allocator.h +++ b/include/libcamera/framebuffer_allocator.h @@ -25,7 +25,7 @@ public: FrameBufferAllocator(std::shared_ptr camera); ~FrameBufferAllocator(); - int allocate(Stream *stream); + int allocate(Stream *stream, unsigned int count); int free(Stream *stream); bool allocated() const { return !buffers_.empty(); } diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index ec4f662d..8af165a3 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -52,7 +52,7 @@ public: const StreamRoles &roles) = 0; virtual int configure(Camera *camera, CameraConfiguration *config) = 0; - virtual int exportFrameBuffers(Camera *camera, Stream *stream, + virtual int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count, std::vector> *buffers) = 0; virtual int start(Camera *camera, const ControlList *controls) = 0; diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp index 8fcec630..b2f64e39 100644 --- a/src/apps/cam/camera_session.cpp +++ b/src/apps/cam/camera_session.cpp @@ -253,17 +253,22 @@ int CameraSession::startCapture() { int ret; - /* Identify the stream with the least number of buffers. */ - unsigned int nbuffers = UINT_MAX; + unsigned int nbuffers = + camera_->properties().get(properties::MinimumRequests).value(); + for (StreamConfiguration &cfg : *config_) { - ret = allocator_->allocate(cfg.stream()); + ret = allocator_->allocate(cfg.stream(), nbuffers); if (ret < 0) { std::cerr << "Can't allocate buffers" << std::endl; return -ENOMEM; } unsigned int allocated = allocator_->buffers(cfg.stream()).size(); - nbuffers = std::min(nbuffers, allocated); + if (allocated < nbuffers) { + std::cerr << "Unable to allocate enough buffers" + << std::endl; + return -ENOMEM; + } } /* diff --git a/src/apps/lc-compliance/simple_capture.cpp b/src/apps/lc-compliance/simple_capture.cpp index cf4d7cf3..be495986 100644 --- a/src/apps/lc-compliance/simple_capture.cpp +++ b/src/apps/lc-compliance/simple_capture.cpp @@ -7,6 +7,8 @@ #include +#include + #include "simple_capture.h" using namespace libcamera; @@ -44,11 +46,13 @@ void SimpleCapture::configure(StreamRole role) void SimpleCapture::start() { + unsigned int bufferCount = + camera_->properties().get(properties::MinimumRequests).value(); Stream *stream = config_->at(0).stream(); - int count = allocator_->allocate(stream); + int count = allocator_->allocate(stream, bufferCount); ASSERT_GE(count, 0) << "Failed to allocate buffers"; - EXPECT_EQ(count, config_->at(0).bufferCount) << "Allocated less buffers than expected"; + EXPECT_EQ(count, bufferCount) << "Allocated less buffers than expected"; camera_->requestCompleted.connect(this, &SimpleCapture::requestComplete); diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp index fb2db4aa..85b696fe 100644 --- a/src/apps/qcam/main_window.cpp +++ b/src/apps/qcam/main_window.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include @@ -464,7 +465,14 @@ int MainWindow::startCapture() for (StreamConfiguration &config : *config_) { Stream *stream = config.stream(); - ret = allocator_->allocate(stream); + /* + * We hold on to a buffer for display, so need one extra from + * the minimum required for capture. + */ + unsigned int bufferCount = + camera_->properties().get(properties::MinimumRequests).value() + 1; + + ret = allocator_->allocate(stream, bufferCount); if (ret < 0) { qWarning() << "Failed to allocate capture buffers"; goto error; diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp index c740b8fc..5f9b96ad 100644 --- a/src/gstreamer/gstlibcameraallocator.cpp +++ b/src/gstreamer/gstlibcameraallocator.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include "gstlibcamera-utils.h" @@ -193,13 +194,15 @@ gst_libcamera_allocator_new(std::shared_ptr camera, { auto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR, nullptr)); + unsigned int bufferCount = + camera->properties().get(properties::MinimumRequests).value(); self->fb_allocator = new FrameBufferAllocator(camera); for (StreamConfiguration &streamCfg : *config_) { Stream *stream = streamCfg.stream(); gint ret; - ret = self->fb_allocator->allocate(stream); + ret = self->fb_allocator->allocate(stream, bufferCount); if (ret == 0) return nullptr; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 2d947a44..ad289b28 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -779,7 +779,7 @@ void Camera::disconnect() disconnected.emit(); } -int Camera::exportFrameBuffers(Stream *stream, +int Camera::exportFrameBuffers(Stream *stream, unsigned int count, std::vector> *buffers) { Private *const d = _d(); @@ -796,7 +796,7 @@ int Camera::exportFrameBuffers(Stream *stream, return d->pipe_->invokeMethod(&PipelineHandler::exportFrameBuffers, ConnectionTypeBlocking, this, stream, - buffers); + count, buffers); } /** diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp index dabd9219..6a0bb8df 100644 --- a/src/libcamera/framebuffer_allocator.cpp +++ b/src/libcamera/framebuffer_allocator.cpp @@ -71,6 +71,7 @@ FrameBufferAllocator::~FrameBufferAllocator() /** * \brief Allocate buffers for a configured stream * \param[in] stream The stream to allocate buffers for + * \param[in] count The number of buffers to allocate * * Allocate buffers suitable for capturing frames from the \a stream. The Camera * shall have been previously configured with Camera::configure() and shall be @@ -79,6 +80,10 @@ FrameBufferAllocator::~FrameBufferAllocator() * Upon successful allocation, the allocated buffers can be retrieved with the * buffers() function. * + * This function may allocate less buffers than requested, due to memory and + * other system constraints. The caller shall always check the return value to + * verify if the number of allocate buffers matches its needs. + * * \return The number of allocated buffers on success or a negative error code * otherwise * \retval -EACCES The camera is not in a state where buffers can be allocated @@ -86,7 +91,7 @@ FrameBufferAllocator::~FrameBufferAllocator() * not part of the active camera configuration * \retval -EBUSY Buffers are already allocated for the \a stream */ -int FrameBufferAllocator::allocate(Stream *stream) +int FrameBufferAllocator::allocate(Stream *stream, unsigned int count) { const auto &[it, inserted] = buffers_.try_emplace(stream); @@ -95,7 +100,7 @@ int FrameBufferAllocator::allocate(Stream *stream) return -EBUSY; } - int ret = camera_->exportFrameBuffers(stream, &it->second); + int ret = camera_->exportFrameBuffers(stream, count, &it->second); if (ret == -EINVAL) LOG(Allocator, Error) << "Stream is not part of " << camera_->id() diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 98a4a3e5..bab2db65 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -140,7 +140,7 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int exportFrameBuffers(Camera *camera, Stream *stream, + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count, std::vector> *buffers) override; int start(Camera *camera, const ControlList *controls) override; @@ -687,10 +687,10 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) } int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, std::vector> *buffers) { IPU3CameraData *data = cameraData(camera); - unsigned int count = stream->configuration().bufferCount; if (stream == &data->outStream_) return data->imgu_->output_->exportBuffers(count, buffers); diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 4a08d01e..4641c76f 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -328,7 +328,7 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int exportFrameBuffers(Camera *camera, Stream *stream, + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count, std::vector> *buffers) override; int start(Camera *camera, const ControlList *controls) override; @@ -1005,10 +1005,10 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) } int PipelineHandlerRPi::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream, + unsigned int count, std::vector> *buffers) { RPi::Stream *s = static_cast(stream); - unsigned int count = stream->configuration().bufferCount; int ret = s->dev()->exportBuffers(count, buffers); s->setExportedBuffers(buffers); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 45b6beaf..8fe37c4f 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -148,7 +148,7 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int exportFrameBuffers(Camera *camera, Stream *stream, + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count, std::vector> *buffers) override; int start(Camera *camera, const ControlList *controls) override; @@ -816,10 +816,10 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) } int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream, + unsigned int count, std::vector> *buffers) { RkISP1CameraData *data = cameraData(camera); - unsigned int count = stream->configuration().bufferCount; if (stream == &data->mainPathStream_) return mainPath_.exportBuffers(count, buffers); diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 8ed983fe..6b7c6d5c 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -322,7 +322,7 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int exportFrameBuffers(Camera *camera, Stream *stream, + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count, std::vector> *buffers) override; int start(Camera *camera, const ControlList *controls) override; @@ -1202,10 +1202,10 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) } int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, std::vector> *buffers) { SimpleCameraData *data = cameraData(camera); - unsigned int count = stream->configuration().bufferCount; /* * Export buffers on the converter or capture video node, depending on diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 7f580955..4ce240a4 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -78,7 +78,7 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int exportFrameBuffers(Camera *camera, Stream *stream, + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count, std::vector> *buffers) override; int start(Camera *camera, const ControlList *controls) override; @@ -226,11 +226,12 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) return 0; } -int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, +int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, + [[maybe_unused]] Stream *stream, + unsigned int count, std::vector> *buffers) { UVCCameraData *data = cameraData(camera); - unsigned int count = stream->configuration().bufferCount; return data->video_->exportBuffers(count, buffers); } diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index d2633be4..e58caf99 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -89,7 +89,7 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int exportFrameBuffers(Camera *camera, Stream *stream, + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count, std::vector> *buffers) override; int start(Camera *camera, const ControlList *controls) override; @@ -321,11 +321,12 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) return 0; } -int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream, +int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, + [[maybe_unused]] Stream *stream, + unsigned int count, std::vector> *buffers) { VimcCameraData *data = cameraData(camera); - unsigned int count = stream->configuration().bufferCount; return data->video_->exportBuffers(count, buffers); } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index cfade490..fffbd51b 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -280,6 +280,7 @@ void PipelineHandler::unlockMediaDevices() * \brief Allocate and export buffers for \a stream * \param[in] camera The camera * \param[in] stream The stream to allocate buffers for + * \param[in] count The number of buffers to allocate * \param[out] buffers Array of buffers successfully allocated * * This function allocates buffers for the \a stream from the devices associated diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 7b97c2d5..1c5fab64 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -160,7 +160,7 @@ int V4L2Camera::allocBuffers(unsigned int count) { Stream *stream = config_->at(0).stream(); - int ret = bufferAllocator_->allocate(stream); + int ret = bufferAllocator_->allocate(stream, count); if (ret < 0) return ret; diff --git a/test/camera/camera_reconfigure.cpp b/test/camera/camera_reconfigure.cpp index 06c87730..63a97863 100644 --- a/test/camera/camera_reconfigure.cpp +++ b/test/camera/camera_reconfigure.cpp @@ -17,6 +17,7 @@ #include #include +#include #include "camera_test.h" #include "test.h" @@ -78,7 +79,9 @@ private: * same buffer allocation for each run. */ if (!allocated_) { - int ret = allocator_->allocate(stream); + unsigned int bufferCount = + camera_->properties().get(properties::MinimumRequests).value(); + int ret = allocator_->allocate(stream, bufferCount); if (ret < 0) { cerr << "Failed to allocate buffers" << endl; return TestFail; diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index de824083..64d3a8e7 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -7,12 +7,13 @@ #include -#include - #include #include #include +#include +#include + #include "camera_test.h" #include "test.h" @@ -98,8 +99,10 @@ protected: Stream *stream = cfg.stream(); - int ret = allocator_->allocate(stream); - if (ret < 0) + unsigned int bufferCount = + camera_->properties().get(properties::MinimumRequests).value(); + int ret = allocator_->allocate(stream, bufferCount); + if (ret < static_cast(bufferCount)) return TestFail; for (const std::unique_ptr &buffer : allocator_->buffers(stream)) { diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index 9c2b0c6a..a9ddb323 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -8,6 +8,7 @@ #include #include +#include #include "camera_test.h" #include "test.h" @@ -120,7 +121,9 @@ protected: /* Use internally allocated buffers. */ allocator_ = new FrameBufferAllocator(camera_); Stream *stream = *camera_->streams().begin(); - if (allocator_->allocate(stream) < 0) + unsigned int bufferCount = + camera_->properties().get(properties::MinimumRequests).value(); + if (allocator_->allocate(stream, bufferCount) < 0) return TestFail; if (camera_->start()) diff --git a/test/fence.cpp b/test/fence.cpp index 1e38bc2f..88ce2857 100644 --- a/test/fence.cpp +++ b/test/fence.cpp @@ -18,6 +18,7 @@ #include #include +#include #include "camera_test.h" #include "test.h" @@ -117,8 +118,11 @@ int FenceTest::init() StreamConfiguration &cfg = config_->at(0); stream_ = cfg.stream(); + unsigned int bufferCount = + camera_->properties().get(properties::MinimumRequests).value(); + allocator_ = std::make_unique(camera_); - if (allocator_->allocate(stream_) < 0) + if (allocator_->allocate(stream_, bufferCount) < 0) return TestFail; nbuffers_ = allocator_->buffers(stream_).size(); diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp index b4422f7d..1b98feea 100644 --- a/test/mapped-buffer.cpp +++ b/test/mapped-buffer.cpp @@ -8,6 +8,7 @@ #include #include +#include #include "libcamera/internal/mapped_framebuffer.h" @@ -55,7 +56,9 @@ protected: stream_ = cfg.stream(); - int ret = allocator_->allocate(stream_); + unsigned int bufferCount = + camera_->properties().get(properties::MinimumRequests).value(); + int ret = allocator_->allocate(stream_, bufferCount); if (ret < 0) return TestFail; From patchwork Fri Dec 16 12:29:25 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18018 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id D811EC31E9 for ; Fri, 16 Dec 2022 12:30:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9CC82633A4; Fri, 16 Dec 2022 13:30:11 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193811; bh=+VGjpxv0w1TaA3+XrC+I8rEgzQ9lwcHkDQKe06UxPO8=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=q1XYAPa7ZhqJn5Uj4YaBUyn4TP6LAy28ksL0Rl3ekGwYJWv4JFdgPZlDUA3+k9/La 1xQe4OIqX7t/vY59TY9lu/zYTypZzyvosg4gCWCgsBM7EeI8f4c4y1fSYb6q2N/opS 3rGhD7oNi5BIhFbvTVi+BTYKyiureLMvedzFpWonxe2K0Wato6N4euV0Fn3D1F7WnQ 4LGapyoI2OCrGJhmJiMXbO/6nMzkAEmffFMDj5azFkQzPtZW1pglhW0j/3h2riUto4 7/POg9zNm01hPEOXgjSqZKi4oHFNh1n3UNRqJWQVxUFWihlXuteUL+jxwNbDwQBXvb fq6v1UXFDwwCw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 065436339D for ; Fri, 16 Dec 2022 13:30:09 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="nT/0Iop3"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 93F80110E; Fri, 16 Dec 2022 13:30:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193808; bh=+VGjpxv0w1TaA3+XrC+I8rEgzQ9lwcHkDQKe06UxPO8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nT/0Iop3tMgQDi7LT51f6+6+GFCkatz84fOgoWZkRLt2z2T/dn96EJaUOWY6bVN+Q wS60brAuqj/CrIk3RqtsM7baO7C+qqbzp+OoBgOH1btqVyzYo+/PSLEzeg/rdWWtB8 +5ChNbX1L53eoUhLyOS31KTeOxjecFQOfZmEVCrY= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:25 +0900 Message-Id: <20221216122939.256534-5-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 04/18] libcamera: pipeline: raspberrypi: Don't rely on bufferCount X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Currently the raspberrypi pipeline handler relies on bufferCount to decide on the number of buffers to allocate internally and for the number of V4L2 buffer slots to reserve. Instead, the number of internal buffers should be the minimum required by the pipeline to keep the requests flowing, in order to avoid wasting memory, while the number of V4L2 buffer slots should be greater than the expected number of requests queued, in order to avoid thrashing dmabuf mappings, which would degrade performance. Extend the Stream class to receive the number of internal buffers that should be allocated, and optionally the number of buffer slots to reserve. Use these new member variables to specify better suited numbers for internal buffers and buffer slots for each stream individually, which also allows us to remove bufferCount. Signed-off-by: Nícolas F. R. A. Prado Signed-off-by: Paul Elder --- Changes in v9: - rebased - I've decided that the buffer allocation decisions that Nícolas implemented covered the same cases that were added in PipelineHandlerRPi::prepareBuffers(), but in a slightly nicer way, especially considering that bufferCount is to be removed from StreamConfiguration in this series. Comments welcome, of course. Changes in v8: - Reworked buffer allocation handling in the raspberrypi pipeline handler - New --- .../pipeline/raspberrypi/raspberrypi.cpp | 83 +++++++------------ .../pipeline/raspberrypi/rpi_stream.cpp | 39 +++++---- .../pipeline/raspberrypi/rpi_stream.h | 24 +++++- 3 files changed, 71 insertions(+), 75 deletions(-) diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 4641c76f..72502c36 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -341,6 +341,25 @@ public: void releaseDevice(Camera *camera) override; private: + /* + * The number of buffers to allocate internally for transferring raw + * frames from the Unicam Image stream to the ISP Input stream. It is + * defined such that: + * - one buffer is being written to in Unicam Image + * - one buffer is being processed in ISP Input + * - one extra buffer is queued to Unicam Image + */ + static constexpr unsigned int kInternalRawBufferCount = 3; + + /* + * The number of buffers to allocate internally for the external + * streams. They're used to drop the first few frames while the IPA + * algorithms converge. It is defined such that: + * - one buffer is being used on the stream + * - one extra buffer is queued + */ + static constexpr unsigned int kInternalDropoutBufferCount = 2; + RPiCameraData *cameraData(Camera *camera) { return static_cast(camera->_d()); @@ -1221,21 +1240,23 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me return -ENOENT; /* Locate and open the unicam video streams. */ - data->unicam_[Unicam::Image] = RPi::Stream("Unicam Image", unicamImage); + data->unicam_[Unicam::Image] = RPi::Stream("Unicam Image", unicamImage, + kInternalRawBufferCount); /* An embedded data node will not be present if the sensor does not support it. */ MediaEntity *unicamEmbedded = unicam->getEntityByName("unicam-embedded"); if (unicamEmbedded) { - data->unicam_[Unicam::Embedded] = RPi::Stream("Unicam Embedded", unicamEmbedded); + data->unicam_[Unicam::Embedded] = + RPi::Stream("Unicam Embedded", unicamEmbedded, kInternalRawBufferCount); data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(), &RPiCameraData::unicamBufferDequeue); } /* Tag the ISP input stream as an import stream. */ - data->isp_[Isp::Input] = RPi::Stream("ISP Input", ispOutput0, true); - data->isp_[Isp::Output0] = RPi::Stream("ISP Output0", ispCapture1); - data->isp_[Isp::Output1] = RPi::Stream("ISP Output1", ispCapture2); - data->isp_[Isp::Stats] = RPi::Stream("ISP Stats", ispCapture3); + data->isp_[Isp::Input] = RPi::Stream("ISP Input", ispOutput0, 0, kInternalRawBufferCount, true); + data->isp_[Isp::Output0] = RPi::Stream("ISP Output0", ispCapture1, kInternalDropoutBufferCount); + data->isp_[Isp::Output1] = RPi::Stream("ISP Output1", ispCapture2, kInternalDropoutBufferCount); + data->isp_[Isp::Stats] = RPi::Stream("ISP Stats", ispCapture3, kInternalDropoutBufferCount); /* Wire up all the buffer connections. */ data->unicam_[Unicam::Image].dev()->dequeueTimeout.connect(data.get(), &RPiCameraData::unicamTimeout); @@ -1428,57 +1449,11 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera) int PipelineHandlerRPi::prepareBuffers(Camera *camera) { RPiCameraData *data = cameraData(camera); - unsigned int numRawBuffers = 0; int ret; - for (Stream *s : camera->streams()) { - if (isRaw(s->configuration().pixelFormat)) { - numRawBuffers = s->configuration().bufferCount; - break; - } - } - - /* Decide how many internal buffers to allocate. */ + /* Allocate internal buffers. */ for (auto const stream : data->streams_) { - unsigned int numBuffers; - /* - * For Unicam, allocate a minimum of 4 buffers as we want - * to avoid any frame drops. - */ - constexpr unsigned int minBuffers = 4; - if (stream == &data->unicam_[Unicam::Image]) { - /* - * If an application has configured a RAW stream, allocate - * additional buffers to make up the minimum, but ensure - * we have at least 2 sets of internal buffers to use to - * minimise frame drops. - */ - numBuffers = std::max(2, minBuffers - numRawBuffers); - } else if (stream == &data->isp_[Isp::Input]) { - /* - * ISP input buffers are imported from Unicam, so follow - * similar logic as above to count all the RAW buffers - * available. - */ - numBuffers = numRawBuffers + std::max(2, minBuffers - numRawBuffers); - - } else if (stream == &data->unicam_[Unicam::Embedded]) { - /* - * Embedded data buffers are (currently) for internal use, - * so allocate the minimum required to avoid frame drops. - */ - numBuffers = minBuffers; - } else { - /* - * Since the ISP runs synchronous with the IPA and requests, - * we only ever need one set of internal buffers. Any buffers - * the application wants to hold onto will already be exported - * through PipelineHandlerRPi::exportFrameBuffers(). - */ - numBuffers = 1; - } - - ret = stream->prepareBuffers(numBuffers); + ret = stream->prepareBuffers(); if (ret < 0) return ret; } diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp index 2bb10f25..cde04efa 100644 --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp @@ -84,14 +84,24 @@ void Stream::removeExternalBuffer(FrameBuffer *buffer) bufferMap_.erase(id); } -int Stream::prepareBuffers(unsigned int count) +int Stream::prepareBuffers() { + unsigned int slotCount; int ret; if (!importOnly_) { - if (count) { + /* + * External streams overallocate buffer slots in order to handle + * the buffers queued from applications without degrading + * performance. Internal streams only need to have enough buffer + * slots to have the internal buffers queued. + */ + slotCount = isExternal() ? kRPIExternalBufferSlotCount + : internalBufferCount_; + + if (internalBufferCount_) { /* Export some frame buffers for internal use. */ - ret = dev_->exportBuffers(count, &internalBuffers_); + ret = dev_->exportBuffers(internalBufferCount_, &internalBuffers_); if (ret < 0) return ret; @@ -100,23 +110,16 @@ int Stream::prepareBuffers(unsigned int count) resetBuffers(); } - /* We must import all internal/external exported buffers. */ - count = bufferMap_.size(); + } else { + /* + * An importOnly stream doesn't have its own internal buffers, + * so it needs to have the number of buffer slots to reserve + * explicitly declared. + */ + slotCount = bufferSlotCount_; } - /* - * If this is an external stream, we must allocate slots for buffers that - * might be externally allocated. We have no indication of how many buffers - * may be used, so this might overallocate slots in the buffer cache. - * Similarly, if this stream is only importing buffers, we do the same. - * - * \todo Find a better heuristic, or, even better, an exact solution to - * this issue. - */ - if (isExternal() || importOnly_) - count = count * 2; - - return dev_->importBuffers(count); + return dev_->importBuffers(slotCount); } int Stream::queueBuffer(FrameBuffer *buffer) diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h index b8bd79cf..e63bf02b 100644 --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h @@ -42,9 +42,13 @@ public: { } - Stream(const char *name, MediaEntity *dev, bool importOnly = false) + Stream(const char *name, MediaEntity *dev, + unsigned int internalBufferCount, + unsigned int bufferSlotCount = 0, bool importOnly = false) : external_(false), importOnly_(importOnly), name_(name), - dev_(std::make_unique(dev)), id_(BufferMask::MaskID) + dev_(std::make_unique(dev)), id_(BufferMask::MaskID), + internalBufferCount_(internalBufferCount), + bufferSlotCount_(bufferSlotCount) { } @@ -63,7 +67,7 @@ public: void setExternalBuffer(FrameBuffer *buffer); void removeExternalBuffer(FrameBuffer *buffer); - int prepareBuffers(unsigned int count); + int prepareBuffers(); int queueBuffer(FrameBuffer *buffer); void returnBuffer(FrameBuffer *buffer); @@ -71,6 +75,8 @@ public: void releaseBuffers(); private: + static constexpr unsigned int kRPIExternalBufferSlotCount = 16; + class IdGenerator { public: @@ -133,6 +139,18 @@ private: /* All frame buffers associated with this device stream. */ BufferMap bufferMap_; + /* + * The number of buffers that should be allocated internally for this + * stream. + */ + unsigned int internalBufferCount_; + + /* + * The number of buffer slots that should be reserved for this stream. + * Only relevant for importOnly streams. + */ + unsigned int bufferSlotCount_; + /* * List of frame buffers that we can use if none have been provided by * the application for external streams. This is populated by the From patchwork Fri Dec 16 12:29:26 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18019 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 53E1EC328E for ; Fri, 16 Dec 2022 12:30:12 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 187216339E; Fri, 16 Dec 2022 13:30:12 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193812; bh=CuCRJbuND3NwTG+nvCb+uGmvkczxn9s47Jjyw2RC8/Y=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=IRoC3HsITPVWFv/bn9BA1xKxbH+cagChn2ZechSfTEjdrhmA/a/u0iTmCVvG0WuUa 4xN+9ez5n7uYCLVieL2dUebLptETqkZntUwvKTWT2qGX5vs1chUk0zznRZ9mtzlMlZ +zO9KFu+K6lfSEafI1cQfYmPnhgcg/1d5yd1zMWmyREKrIp10nr2gqWWOlhE5z0fuR Z329sDFfxVwE8/sgyEzrcE5rUmTMHjBvxNpxm/4r3x4TKV7Vl03BhR3uU8ZiUpnZWP Sg+JWXBnZT4Fp3FgEhKetXZpW2ZOTcN9U+qPfblVTnYaafm8Uuv/aNvjePrNFzHNcc jWxh58whbta/g== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B6464633A2 for ; Fri, 16 Dec 2022 13:30:10 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="L7dH6d7C"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 526E6A31; Fri, 16 Dec 2022 13:30:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193810; bh=CuCRJbuND3NwTG+nvCb+uGmvkczxn9s47Jjyw2RC8/Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=L7dH6d7CF76U4pPXowR+agKwQI2CK28ljgKFCxFR9Iif8JG7dms70pNsXTbUcgdQT KU80maH70dvMY+8Rn5IBfPwWRgg67AHB4l1ksP67fZVDufuOCj8tim/BRTouOV9lcp jcemO6wou7KQ459qqPJ2FrdUfDSAS2Exy/tXq5xc= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:26 +0900 Message-Id: <20221216122939.256534-6-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 05/18] libcamera: pipeline: ipu3: Don't rely on bufferCount X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Currently the ipu3 pipeline handler relies on bufferCount to decide on the number of V4L2 buffer slots to reserve as well as for the number of buffers to allocate internally for the CIO2 raw buffers and parameter-statistics ImgU buffer pairs. Instead, the number of internal buffers should be the minimum required by the pipeline to keep the requests flowing without frame drops, in order to avoid wasting memory, while the number of V4L2 buffer slots should be greater than the expected number of requests queued by the application, in order to avoid thrashing dmabuf mappings, which would degrade performance. Stop relying on bufferCount for these numbers and instead set them to appropriate, and independent, constants. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder --- Changes in v9: - rebase Changes in v8: - New --- src/libcamera/pipeline/ipu3/cio2.cpp | 4 ++-- src/libcamera/pipeline/ipu3/cio2.h | 16 +++++++++++++++- src/libcamera/pipeline/ipu3/imgu.cpp | 12 ++++++------ src/libcamera/pipeline/ipu3/imgu.h | 15 ++++++++++++++- src/libcamera/pipeline/ipu3/ipu3.cpp | 20 ++++++++------------ 5 files changed, 45 insertions(+), 22 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index d4e523af..feb69991 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -335,13 +335,13 @@ int CIO2Device::exportBuffers(unsigned int count, return output_->exportBuffers(count, buffers); } -int CIO2Device::start() +int CIO2Device::start(unsigned int bufferSlotCount) { int ret = output_->exportBuffers(kBufferCount, &buffers_); if (ret < 0) return ret; - ret = output_->importBuffers(kBufferCount); + ret = output_->importBuffers(bufferSlotCount); if (ret) LOG(IPU3, Error) << "Failed to import CIO2 buffers"; diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h index 68504a2d..8ed208d3 100644 --- a/src/libcamera/pipeline/ipu3/cio2.h +++ b/src/libcamera/pipeline/ipu3/cio2.h @@ -30,6 +30,20 @@ struct StreamConfiguration; class CIO2Device { public: + /* + * This many internal buffers for the CIO2 ensures that the pipeline + * runs smoothly, without frame drops. This number considers: + * - one buffer being DMA'ed to in CIO2 + * - one buffer programmed by the CIO2 as the next buffer + * - one buffer under processing in ImgU + * - one extra idle buffer queued to CIO2, to account for possible + * delays in requeuing the buffer from ImgU back to CIO2 + * + * Transient situations can arise when one of the parts, CIO2 or ImgU, + * finishes its processing first and experiences a lack of buffers, but + * they will shortly after return to the state described above as the + * other part catches up. + */ static constexpr unsigned int kBufferCount = 4; CIO2Device(); @@ -48,7 +62,7 @@ public: V4L2SubdeviceFormat getSensorFormat(const std::vector &mbusCodes, const Size &size) const; - int start(); + int start(unsigned int bufferSlotCount); int stop(); CameraSensor *sensor() { return sensor_.get(); } diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp index 531879f1..fa920d87 100644 --- a/src/libcamera/pipeline/ipu3/imgu.cpp +++ b/src/libcamera/pipeline/ipu3/imgu.cpp @@ -576,22 +576,22 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, /** * \brief Allocate buffers for all the ImgU video devices */ -int ImgUDevice::allocateBuffers(unsigned int bufferCount) +int ImgUDevice::allocateBuffers(unsigned int bufferSlotCount) { /* Share buffers between CIO2 output and ImgU input. */ - int ret = input_->importBuffers(bufferCount); + int ret = input_->importBuffers(bufferSlotCount); if (ret) { LOG(IPU3, Error) << "Failed to import ImgU input buffers"; return ret; } - ret = param_->allocateBuffers(bufferCount, ¶mBuffers_); + ret = param_->allocateBuffers(kImgUInternalBufferCount, ¶mBuffers_); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU param buffers"; goto error; } - ret = stat_->allocateBuffers(bufferCount, &statBuffers_); + ret = stat_->allocateBuffers(kImgUInternalBufferCount, &statBuffers_); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; goto error; @@ -602,13 +602,13 @@ int ImgUDevice::allocateBuffers(unsigned int bufferCount) * corresponding stream is active or inactive, as the driver needs * buffers to be requested on the V4L2 devices in order to operate. */ - ret = output_->importBuffers(bufferCount); + ret = output_->importBuffers(bufferSlotCount); if (ret < 0) { LOG(IPU3, Error) << "Failed to import ImgU output buffers"; goto error; } - ret = viewfinder_->importBuffers(bufferCount); + ret = viewfinder_->importBuffers(bufferSlotCount); if (ret < 0) { LOG(IPU3, Error) << "Failed to import ImgU viewfinder buffers"; goto error; diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h index 0af4dd8a..85873961 100644 --- a/src/libcamera/pipeline/ipu3/imgu.h +++ b/src/libcamera/pipeline/ipu3/imgu.h @@ -84,7 +84,7 @@ public: outputFormat); } - int allocateBuffers(unsigned int bufferCount); + int allocateBuffers(unsigned int bufferSlotCount); void freeBuffers(); int start(); @@ -119,6 +119,19 @@ private: std::string name_; MediaDevice *media_; + + /* + * This many internal buffers (or rather parameter and statistics buffer + * pairs) for the ImgU ensures that the pipeline runs smoothly, without + * frame drops. This number considers: + * - three buffers queued to the CIO2 (Since these buffers are bound to + * CIO2 buffers before queuing to the CIO2) + * - one buffer under processing in ImgU + * + * \todo Update this number when we make these buffers only get added to + * the FrameInfo after the raw buffers are dequeued from CIO2. + */ + static constexpr unsigned int kImgUInternalBufferCount = 4; }; } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index bab2db65..4d8fcfeb 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -160,7 +160,7 @@ private: int updateControls(IPU3CameraData *data); int registerCameras(); - int allocateBuffers(Camera *camera); + int allocateBuffers(Camera *camera, unsigned int bufferSlotCount); int freeBuffers(Camera *camera); ImgUDevice imgu0_; @@ -169,6 +169,8 @@ private: MediaDevice *imguMediaDev_; std::vector ipaBuffers_; + + static constexpr unsigned int kIPU3BufferSlotCount = 16; }; IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data) @@ -710,20 +712,14 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, * In order to be able to start the 'viewfinder' and 'stat' nodes, we need * memory to be reserved. */ -int PipelineHandlerIPU3::allocateBuffers(Camera *camera) +int PipelineHandlerIPU3::allocateBuffers(Camera *camera, + unsigned int bufferSlotCount) { IPU3CameraData *data = cameraData(camera); ImgUDevice *imgu = data->imgu_; - unsigned int bufferCount; int ret; - bufferCount = std::max({ - data->outStream_.configuration().bufferCount, - data->vfStream_.configuration().bufferCount, - data->rawStream_.configuration().bufferCount, - }); - - ret = imgu->allocateBuffers(bufferCount); + ret = imgu->allocateBuffers(bufferSlotCount); if (ret < 0) return ret; @@ -781,7 +777,7 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis return ret; /* Allocate buffers for internal pipeline usage. */ - ret = allocateBuffers(camera); + ret = allocateBuffers(camera, kIPU3BufferSlotCount); if (ret) return ret; @@ -795,7 +791,7 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis * Start the ImgU video devices, buffers will be queued to the * ImgU output and viewfinder when requests will be queued. */ - ret = cio2->start(); + ret = cio2->start(kIPU3BufferSlotCount); if (ret) goto error; From patchwork Fri Dec 16 12:29:27 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18020 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 0062CC31E9 for ; Fri, 16 Dec 2022 12:30:14 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B4DBE633A2; Fri, 16 Dec 2022 13:30:14 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193814; bh=+AxFQm3WbHzlPk12pkAGGEBjxtMNKSxUJxxbSLm0+5M=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=sZYYBw/m1jnDBM9kHz72r2/t3Uoo7F0mLOF3VXQpclqzZ6FqXuOJdIIoUK0xlkGf5 C5ZWm1frT8gfZ5qJJNSe5XpCVkwv/SXiNmQcJ3iyARNnKpqdAW/yJ9fN0iyACpbd33 SM1C06msnVkeT40LxskJsdB6OI96UJj7KVZakdPV/Ep/dGfrP6UWl0YSIzwXeUn8gR AHR00g8yxUGmAvSvH8CKeLv37IkDfiBDgHycC/4oWzWl0l1CvNAWb6UYnSGPmf2Zf1 Q8OXLx7slySicLqheGN7l/ybUbcTFhbTsrOn4u3O5DuOt4sYkD8kCg3fH44nN+DBXg +gpb5h+RXyxVA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 903FE633A8 for ; Fri, 16 Dec 2022 13:30:12 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="i4WfC2Ao"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 137D5110E; Fri, 16 Dec 2022 13:30:10 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193812; bh=+AxFQm3WbHzlPk12pkAGGEBjxtMNKSxUJxxbSLm0+5M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=i4WfC2AoLOlZNVsnLlptzYp4rQMrnF2XN/IoYboC+tsVn/6Ba0jNOePkoOIN3s/EB tXixz3ez/ZFEjj4bcPTQMVSlKFH7tQUHkgCwtvPbPkcOWQIud8EM9WMr9hP/2kJZh9 wWarMcPIs2490KTSHxn0ZC+DnYsfiDxYkFEEXznI= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:27 +0900 Message-Id: <20221216122939.256534-7-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 06/18] libcamera: pipeline: simple: Don't rely on bufferCount X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Currently the simple pipeline handler relies on bufferCount to decide on the number of buffers to allocate internally when a converter is in use and for the number of V4L2 buffer slots to reserve. Instead, the number of internal buffers should be the minimum required by the pipeline to keep the requests flowing, in order to avoid wasting memory, while the number of V4L2 buffer slots should be greater than the expected number of requests queued by the application, in order to avoid thrashing dmabuf mappings, which would degrade performance. Stop relying on bufferCount for these numbers and instead set them to appropriate, and independent, constants. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder --- Changes in v9: - rebased Changes in v8: - New --- include/libcamera/internal/converter.h | 3 ++- .../internal/converter/converter_v4l2_m2m.h | 6 +++-- .../converter/converter_v4l2_m2m.cpp | 12 +++++----- src/libcamera/pipeline/simple/simple.cpp | 22 ++++++++++++++----- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h index 834ec5ab..32490fe0 100644 --- a/include/libcamera/internal/converter.h +++ b/include/libcamera/internal/converter.h @@ -49,7 +49,8 @@ public: virtual int exportBuffers(unsigned int output, unsigned int count, std::vector> *buffers) = 0; - virtual int start() = 0; + virtual int start(unsigned int internalBufferCount, + unsigned int bufferSlotCount) = 0; virtual void stop() = 0; virtual int queueBuffers(FrameBuffer *input, diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h index 815916d0..1f471071 100644 --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h @@ -50,7 +50,8 @@ public: int exportBuffers(unsigned int ouput, unsigned int count, std::vector> *buffers); - int start(); + int start(unsigned int internalBufferCount, + unsigned int bufferSlotCount); void stop(); int queueBuffers(FrameBuffer *input, @@ -69,7 +70,8 @@ private: int exportBuffers(unsigned int count, std::vector> *buffers); - int start(); + int start(unsigned int internalBufferCount, + unsigned int bufferSlotCount); void stop(); int queueBuffers(FrameBuffer *input, FrameBuffer *output); diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp index 2a4d1d99..9d25f25a 100644 --- a/src/libcamera/converter/converter_v4l2_m2m.cpp +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp @@ -107,13 +107,14 @@ int V4L2M2MConverter::Stream::exportBuffers(unsigned int count, return m2m_->capture()->exportBuffers(count, buffers); } -int V4L2M2MConverter::Stream::start() +int V4L2M2MConverter::Stream::start(unsigned int internalBufferCount, + unsigned int bufferSlotCount) { - int ret = m2m_->output()->importBuffers(inputBufferCount_); + int ret = m2m_->output()->importBuffers(internalBufferCount); if (ret < 0) return ret; - ret = m2m_->capture()->importBuffers(outputBufferCount_); + ret = m2m_->capture()->importBuffers(bufferSlotCount); if (ret < 0) { stop(); return ret; @@ -373,12 +374,13 @@ int V4L2M2MConverter::exportBuffers(unsigned int output, unsigned int count, /** * \copydoc libcamera::Converter::start */ -int V4L2M2MConverter::start() +int V4L2M2MConverter::start(unsigned int internalBufferCount, + unsigned int bufferSlotCount) { int ret; for (Stream &stream : streams_) { - ret = stream.start(); + ret = stream.start(internalBufferCount, bufferSlotCount); if (ret < 0) { stop(); return ret; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 6b7c6d5c..196e5252 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -339,6 +339,7 @@ protected: private: static constexpr unsigned int kNumInternalBuffers = 3; + static constexpr unsigned int kSimpleBufferSlotCount = 16; struct EntityData { std::unique_ptr video; @@ -1232,17 +1233,27 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL return -EBUSY; } + /* + * Number of internal buffers that will be used to move video capture + * device frames to the converter. + * + * \todo Make this depend on the driver in use instead of being + * hardcoded. In order to not drop frames, the realtime requirements for + * each device should be checked, so it may not be as simple as just + * using the minimum number of buffers required by the driver. + */ + static constexpr unsigned int internalBufferCount = 3; + if (data->useConverter_) { /* * When using the converter allocate a fixed number of internal * buffers. */ - ret = video->allocateBuffers(kNumInternalBuffers, + ret = video->allocateBuffers(internalBufferCount, &data->converterBuffers_); } else { - /* Otherwise, prepare for using buffers from the only stream. */ - Stream *stream = &data->streams_[0]; - ret = video->importBuffers(stream->configuration().bufferCount); + /* Otherwise, prepare for using buffers. */ + ret = video->importBuffers(kSimpleBufferSlotCount); } if (ret < 0) { releasePipeline(data); @@ -1258,7 +1269,8 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL } if (data->useConverter_) { - ret = data->converter_->start(); + ret = data->converter_->start(internalBufferCount, + kSimpleBufferSlotCount); if (ret < 0) { stop(camera); return ret; From patchwork Fri Dec 16 12:29:28 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18021 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 86D7DC328E for ; Fri, 16 Dec 2022 12:30:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3657E6339E; Fri, 16 Dec 2022 13:30:15 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193815; bh=Tfmed31QJsiSFUJTl8LXBMnkbz7VKjAsoTW99wZxYjM=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=DiuvFDfX+l78FI03rbM8jco9Qg++vFZYuOF3fYZjak2ESql9j7N0U2gsYstjKJlhr YV61CXb36wi4a9Re3fvp0COqzdEUmpyyRUHmTWDaUmW11CPCoaVDQn6av+Th+6owp3 GQzTjR1jB0heExwCtuO5I2LvE+dYPafyvzX7HeaFPPseUFu2VMlFSQNPiKI8myVlCe QHFLFx88c5LE69eA+J1Gg7WlKYsWSznmHBck+PX/SeLDG3fIw00cloTAC7y6Iw05Ss FCNeFA1+ETZGPHReD83s6y5pfRabpNcClSf2jWCloSKUt87TU/PFLG1efxDauQRfV8 d14YfTThSpH8g== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 43F1B633A2 for ; Fri, 16 Dec 2022 13:30:14 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="k7h53e2o"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CE075158D; Fri, 16 Dec 2022 13:30:12 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193814; bh=Tfmed31QJsiSFUJTl8LXBMnkbz7VKjAsoTW99wZxYjM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=k7h53e2o31XciafZbNahfKuPibwGv6CnhjGZYie/Jfu/8r5fNVmB6+4GElntJBts0 yMmoaW5jDbllxwuPRiLkAQSBcwPXEFHdSesWw10kffBua5koJkdfzEHTpDaQgYcZSZ XPr8YUziOK6WDmv/A/STUaOQZa9y3kG83hTMw5+s= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:28 +0900 Message-Id: <20221216122939.256534-8-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 07/18] libcamera: pipeline: rkisp1: Don't rely on bufferCount X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Currently the rkisp1 pipeline handler relies on bufferCount to decide on the number of parameter and statistics buffers to allocate internally and for the number of V4L2 buffer slots to reserve. Instead, the number of internal buffers should be the minimum required by the pipeline to keep the requests flowing, in order to avoid wasting memory, while the number of V4L2 buffer slots should be greater than the expected number of requests queued by the application, in order to avoid thrashing dmabuf mappings, which would degrade performance. Stop relying on bufferCount for these numbers and instead set them to appropriate, and independent, constants. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder Reviewed-by: Jacopo Mondi --- Changes in v9: - rebased Changes in v8: - New --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 ++++++++++++------- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 3 +-- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 ++ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 8fe37c4f..5028ef1a 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -199,6 +199,16 @@ private: Camera *activeCamera_; const MediaPad *ispSink_; + + /* + * This many internal buffers (or rather parameter and statistics buffer + * pairs) ensures that the pipeline runs smoothly, without frame drops. + * This number considers: + * - three buffers queued to the driver, which is also the minimum + * required to start streaming + * - one buffer being processed on the IPA + */ + static constexpr unsigned int kRkISP1InternalBufferCount = 4; }; RkISP1Frames::RkISP1Frames(PipelineHandler *pipe) @@ -835,17 +845,12 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) unsigned int ipaBufferId = 1; int ret; - unsigned int maxCount = std::max({ - data->mainPathStream_.configuration().bufferCount, - data->selfPathStream_.configuration().bufferCount, - }); - if (!isRaw_) { - ret = param_->allocateBuffers(maxCount, ¶mBuffers_); + ret = param_->allocateBuffers(kRkISP1InternalBufferCount, ¶mBuffers_); if (ret < 0) goto error; - ret = stat_->allocateBuffers(maxCount, &statBuffers_); + ret = stat_->allocateBuffers(kRkISP1InternalBufferCount, &statBuffers_); if (ret < 0) goto error; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index 5079b268..a168e0ad 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -370,8 +370,7 @@ int RkISP1Path::start() if (running_) return -EBUSY; - /* \todo Make buffer count user configurable. */ - ret = video_->importBuffers(RKISP1_BUFFER_COUNT); + ret = video_->importBuffers(kRkISP1BufferSlotCount); if (ret) return ret; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h index bdf3f95b..5b53783c 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -76,6 +76,8 @@ private: std::unique_ptr resizer_; std::unique_ptr video_; MediaLink *link_; + + static constexpr unsigned int kRkISP1BufferSlotCount = 16; }; class RkISP1MainPath : public RkISP1Path From patchwork Fri Dec 16 12:29:29 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18022 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 0112AC31E9 for ; Fri, 16 Dec 2022 12:30:18 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BF79B633A9; Fri, 16 Dec 2022 13:30:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193817; bh=tWJBA3ZzmkGxJdFje1EEjYVPBGmeYjY0ojq/kBPOLBY=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=p4LGgptsXzIUHzAvmuoDbInrRyeF2+y8rQpAm7JoJmTnFb9dOBFkNFH9NjZ/2EKqt TmfLFRIdd7MHthTzdncu12vhqKnmoWt+eImzQIYO/Zu7mUuN8UKIsZAkSnQLZK9gvD WoE8DrQpJuWDlJRv2jNzM3a9riDxL10z74idBxvOSpdVQNoffc3/UBdoiC5/lds+lB LIP++So1Glo8gdEC+ImC1NpziUAt0YbKHm2gljxA6spjDegMUkvxzOZHo5VwDyESIP hqd3JtNAdP1/aiOjV1I8Febvm0NDiftrk6LNHZXLwb3yKZDQUoRg8A5Islf24+/IWh t+YfrN1CqzX/w== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 02484633A5 for ; Fri, 16 Dec 2022 13:30:16 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="f/4vqUtU"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8F07BA31; Fri, 16 Dec 2022 13:30:14 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193815; bh=tWJBA3ZzmkGxJdFje1EEjYVPBGmeYjY0ojq/kBPOLBY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=f/4vqUtUusR3QI8wuBxJiZA8c9hPEYCrJFyWLBsaxVQLPVIDrK+7FKp3QvQ/1OvQh OZg/fOhty4W797OYRq0ED7iRo2gdV9ZSeRAGWe1raUDXYkM3yDxA4XFgekjBRMXcDM x1jQCsUGmfwEUW57l2aTnOJUXBdhi9q7CnoZbiDs= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:29 +0900 Message-Id: <20221216122939.256534-9-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 08/18] libcamera: pipeline: vimc, uvcvideo: Don't rely on bufferCount X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Instead of using bufferCount as the number of V4L2 buffer slots to reserve in the vimc and uvcvideo pipeline handlers, use a reasonably high constant: 16. Overallocating isn't a problem as buffer slots are cheap. Having too few, on the other hand, could degrade performance. It is expected that this number will be more than enough for most, if not all, use cases. This makes way for removing bufferCount. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder Reviewed-by: Jacopo Mondi --- Changes in v9: - rebased Changes in v8: - New --- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 5 +++-- src/libcamera/pipeline/vimc/vimc.cpp | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 4ce240a4..18966d01 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -97,6 +97,8 @@ private: { return static_cast(camera->_d()); } + + static constexpr unsigned int kUVCBufferSlotCount = 16; }; UVCCameraConfiguration::UVCCameraConfiguration(UVCCameraData *data) @@ -239,9 +241,8 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls) { UVCCameraData *data = cameraData(camera); - unsigned int count = data->stream_.configuration().bufferCount; - int ret = data->video_->importBuffers(count); + int ret = data->video_->importBuffers(kUVCBufferSlotCount); if (ret < 0) return ret; diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index e58caf99..eaa6ebf7 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -106,6 +106,8 @@ private: { return static_cast(camera->_d()); } + + static constexpr unsigned int kVimcBufferSlotCount = 16; }; namespace { @@ -334,9 +336,8 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] const ControlList *controls) { VimcCameraData *data = cameraData(camera); - unsigned int count = data->stream_.configuration().bufferCount; - int ret = data->video_->importBuffers(count); + int ret = data->video_->importBuffers(kVimcBufferSlotCount); if (ret < 0) return ret; From patchwork Fri Dec 16 12:29:30 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18023 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 7F083C31E9 for ; Fri, 16 Dec 2022 12:30:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 47D5E633B0; Fri, 16 Dec 2022 13:30:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193819; bh=CgTVHMsudzEX7Z7J8lk8mQNzHP/BX6KslEspC+7juCw=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=QF98e3TdHIvk0Gei2cRbD+bpAAW8tpmJkB4XsJgbXWIX722qMZsXzv5JH/HoR3N72 Pkryyo0ya6T6lQsaQ/QKUV8El1JTbGenuNmfbKHdMhsoq+s2geqL870b4DyO3/wHTo N0YMSyZvUPaJtlbdqpZB47DdHUACELfrr5SwEzfj0PF7qczAi6aCBUq6ALgPP6DXPu vfqLl1X5rTK8Ixesan3xEJZgZ4t20EpJSF6A5ZL6smAVD5nM5yT1rdf0HtF0wGXJp0 LFeods0a/87MWshLp0cq9fCdmIWRxhphFzRGeY2xX3mtv+W86I45HHQXzbpdh01P7Y NQpohl/qa+TJQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B4BD5633A5 for ; Fri, 16 Dec 2022 13:30:17 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="t541L2bo"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5013A110E; Fri, 16 Dec 2022 13:30:16 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193817; bh=CgTVHMsudzEX7Z7J8lk8mQNzHP/BX6KslEspC+7juCw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=t541L2boKWQm6xn/It1XqhJK+h5u/zehWt8FYNGWqukR0juiTnMncGvhu5wXSCJwK /NEoYGaOND4E2bZjA8lpjsVVRTEssk25afSzywgg9gXCZ85jOmH322ZmvRYiUu4v5O KooQ9vBxtr2DZ89O6WFb1Qu/kl8ujltms42Wq2gM= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:30 +0900 Message-Id: <20221216122939.256534-10-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 09/18] v4l2: Allocate buffers based on requested count and MinimumRequests X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Currently the number of buffers allocated is based on bufferCount, which is hardcoded to 1. Instead allocate buffers based on the requested count and on the MinimumRequests property for the camera, which is accessed through a newly added getter. This allows the removal of bufferCount. While at it, fix a typo: s/interval/internal/. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder --- Changes in v9: - rebased Changes in v8: - New - Fixed typo: s/interval/internal/ --- src/v4l2/v4l2_camera.cpp | 20 +++++++++++++------- src/v4l2/v4l2_camera.h | 5 +++-- src/v4l2/v4l2_camera_proxy.cpp | 10 +++++----- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 1c5fab64..eb0fd239 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -12,6 +12,8 @@ #include +#include + using namespace libcamera; LOG_DECLARE_CATEGORY(V4L2Compat) @@ -106,15 +108,13 @@ void V4L2Camera::requestComplete(Request *request) } int V4L2Camera::configure(StreamConfiguration *streamConfigOut, - const Size &size, const PixelFormat &pixelformat, - unsigned int bufferCount) + const Size &size, const PixelFormat &pixelformat) { StreamConfiguration &streamConfig = config_->at(0); streamConfig.size.width = size.width; streamConfig.size.height = size.height; streamConfig.pixelFormat = pixelformat; - streamConfig.bufferCount = bufferCount; - /* \todo memoryType (interval vs external) */ + /* \todo memoryType (internal vs external) */ CameraConfiguration::Status validation = config_->validate(); if (validation == CameraConfiguration::Invalid) { @@ -145,7 +145,6 @@ int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat, StreamConfiguration &cfg = config->at(0); cfg.size = size; cfg.pixelFormat = pixelFormat; - cfg.bufferCount = 1; CameraConfiguration::Status validation = config->validate(); if (validation == CameraConfiguration::Invalid) @@ -164,7 +163,9 @@ int V4L2Camera::allocBuffers(unsigned int count) if (ret < 0) return ret; - for (unsigned int i = 0; i < count; i++) { + unsigned int allocatedCount = ret; + + for (unsigned int i = 0; i < allocatedCount; i++) { std::unique_ptr request = camera_->createRequest(i); if (!request) { requestPool_.clear(); @@ -173,7 +174,7 @@ int V4L2Camera::allocBuffers(unsigned int count) requestPool_.push_back(std::move(request)); } - return ret; + return allocatedCount; } void V4L2Camera::freeBuffers() @@ -298,3 +299,8 @@ bool V4L2Camera::isRunning() { return isRunning_; } + +unsigned int V4L2Camera::minimumRequests() +{ + return camera_->properties().get(properties::MinimumRequests).value(); +} diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h index d3483444..ada66421 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -43,8 +43,7 @@ public: int configure(libcamera::StreamConfiguration *streamConfigOut, const libcamera::Size &size, - const libcamera::PixelFormat &pixelformat, - unsigned int bufferCount); + const libcamera::PixelFormat &pixelformat); int validateConfiguration(const libcamera::PixelFormat &pixelformat, const libcamera::Size &size, libcamera::StreamConfiguration *streamConfigOut); @@ -63,6 +62,8 @@ public: bool isRunning(); + unsigned int minimumRequests(); + private: void requestComplete(libcamera::Request *request) LIBCAMERA_TSA_EXCLUDES(bufferLock_); diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index 55ff62cd..bcf9c2ab 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -367,8 +367,7 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg) Size size(arg->fmt.pix.width, arg->fmt.pix.height); V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat); - ret = vcam_->configure(&streamConfig_, size, v4l2Format.toPixelFormat(), - bufferCount_); + ret = vcam_->configure(&streamConfig_, size, v4l2Format.toPixelFormat()); if (ret < 0) return -EINVAL; @@ -514,14 +513,13 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf Size size(v4l2PixFormat_.width, v4l2PixFormat_.height); V4L2PixelFormat v4l2Format = V4L2PixelFormat(v4l2PixFormat_.pixelformat); int ret = vcam_->configure(&streamConfig_, size, - v4l2Format.toPixelFormat(), arg->count); + v4l2Format.toPixelFormat()); if (ret < 0) return -EINVAL; setFmtFromConfig(streamConfig_); - arg->count = streamConfig_.bufferCount; - bufferCount_ = arg->count; + arg->count = std::max(arg->count, vcam_->minimumRequests()); ret = vcam_->allocBuffers(arg->count); if (ret < 0) { @@ -529,6 +527,8 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf return ret; } + bufferCount_ = arg->count = ret; + buffers_.resize(arg->count); for (unsigned int i = 0; i < arg->count; i++) { struct v4l2_buffer buf = {}; From patchwork Fri Dec 16 12:29:31 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18024 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 1A5FBC31E9 for ; Fri, 16 Dec 2022 12:30:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DBE1F633AF; Fri, 16 Dec 2022 13:30:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193821; bh=5rn9glm+9fKtsqHbxX63cFSTkAujYcQgl49gioiLolo=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Bis2gH9T32lzobJpTi/XG5e0tcGHyfmrCSzHgaUrWhtKqwCm2ROtr2BcanlmjFSi2 9rgqas8mxxA10qEmE/BW8+0DNKY/SFMKmIyVBKny8UUrAhFWxZnWcYImtwXbCdw2AT MBogy9+Xe1RKdTQU/N3c+CK58uZThEPJDPCId3pwZPMMkTCuYcO6/NN+y+gFjsV+Vz R+eNAJfC6EGDdVQbldIFhlqqBl3l1LIFUKxXEeRDZE1CN5niVG+00tyqvx43htrzso d9nUgdDUJx2ftk0CDNg5NtmrdmcJpqPZQDrANdTccu0mKOcA2IXQ0q68R9+NBoEbvH zdp8G2ZUNM0Ag== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 74D79633A6 for ; Fri, 16 Dec 2022 13:30:19 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="UiZm1M28"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 10FE2A31; Fri, 16 Dec 2022 13:30:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193819; bh=5rn9glm+9fKtsqHbxX63cFSTkAujYcQgl49gioiLolo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=UiZm1M28w3NV2Llaqe18ULdAekx+W0unOm89NgJWQeI4NzLSap/UTt3D1+zfz+mgx yG7z9YzSQcq+LMTdAJDeBqrRF5XUCiwJ3m3CeegkatFSTDzgYqawgaMjzPbzQL2F72 1KJGVnukKTiFqbqqh99ZyiZaV6igtuio7WRv531c= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:31 +0900 Message-Id: <20221216122939.256534-11-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 10/18] libcamera: stream: Remove bufferCount X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Now that the number of buffers allocated by the FrameBufferAllocator helper is passed through FrameBufferAllocator::allocate() and the pipelines no longer use bufferCount for internal buffer or V4L2 buffer slots allocation, we no longer need to have bufferCount in the StreamConfiguration, so remove it. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder --- Changes in v9: - rebased Changes in v8: - Updated the pipeline-handler guide to use MinimumRequests instead of bufferCount - Removed kNumInternalBuffers as it was unused Changes in v6: - Removed IPU3_BUFFER_COUNT as it was unused --- Documentation/guides/pipeline-handler.rst | 15 +++++++++------ .../internal/converter/converter_v4l2_m2m.h | 3 --- include/libcamera/stream.h | 2 -- src/android/camera_stream.cpp | 2 +- src/libcamera/converter/converter_v4l2_m2m.cpp | 3 --- src/libcamera/pipeline/ipu3/cio2.cpp | 1 - src/libcamera/pipeline/ipu3/ipu3.cpp | 8 -------- .../pipeline/raspberrypi/raspberrypi.cpp | 6 ------ src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 -- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 -- src/libcamera/pipeline/simple/simple.cpp | 6 +----- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 --- src/libcamera/pipeline/vimc/vimc.cpp | 3 --- src/libcamera/stream.cpp | 12 +++--------- test/camera/buffer_import.cpp | 11 ++++++++--- test/libtest/buffer_source.cpp | 4 ++-- test/libtest/buffer_source.h | 2 +- test/v4l2_videodevice/buffer_cache.cpp | 3 +-- 18 files changed, 26 insertions(+), 62 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 70de82af..1d4314bb 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -827,14 +827,12 @@ As well as a list of supported StreamFormats, the StreamConfiguration is also expected to provide an initialised default configuration. This may be arbitrary, but depending on use case you may wish to select an output that matches the Sensor output, or prefer a pixelformat which might provide higher performance on -the hardware. The bufferCount represents the number of buffers required to -support functional continuous processing on this stream. +the hardware. .. code-block:: cpp cfg.pixelFormat = formats::BGR888; cfg.size = { 1280, 720 }; - cfg.bufferCount = 4; Finally add each ``StreamConfiguration`` generated to the ``CameraConfiguration``, and ensure that it has been validated before returning @@ -900,8 +898,6 @@ Add the following function implementation to your file: status = Adjusted; } - cfg.bufferCount = 4; - return status; } @@ -1145,13 +1141,20 @@ is performed by using the ``V4L2VideoDevice`` API, which provides an .. _FrameBuffer: https://libcamera.org/api-html/classlibcamera_1_1FrameBuffer.html +The number passed to ``importBuffers()`` should be at least equal to the value +of the ``MinimumRequests`` property in order to be possible to queue enough +buffers to the video device that frames won't be dropped during capture. A +bigger value can be advantageous to reduce the thrashing of dma-buf file +descriptor mappings in case the application queues more requests and therefore +improve performance, but for simplicity we'll just use ``MinimumRequests``. + Implement the pipeline handler ``start()`` function by replacing the stub version with the following code: .. code-block:: c++ VividCameraData *data = cameraData(camera); - unsigned int count = data->stream_.configuration().bufferCount; + unsigned int count = camera->properties().get(properties::MinimumRequests); int ret = data->video_->importBuffers(count); if (ret < 0) diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h index 1f471071..bcc03347 100644 --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h @@ -86,9 +86,6 @@ private: V4L2M2MConverter *converter_; unsigned int index_; std::unique_ptr m2m_; - - unsigned int inputBufferCount_; - unsigned int outputBufferCount_; }; std::unique_ptr m2m_; diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 29235ddf..dba9b453 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -47,8 +47,6 @@ struct StreamConfiguration { unsigned int stride; unsigned int frameSize; - unsigned int bufferCount; - std::optional colorSpace; Stream *stream() const { return stream_; } diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 045e6006..f51f0c3a 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -131,7 +131,7 @@ int CameraStream::configure() allocator_ = std::make_unique(cameraDevice_); mutex_ = std::make_unique(); - camera3Stream_->max_buffers = configuration().bufferCount; + camera3Stream_->max_buffers = bufferCount; return 0; } diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp index 9d25f25a..b9372804 100644 --- a/src/libcamera/converter/converter_v4l2_m2m.cpp +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp @@ -95,9 +95,6 @@ int V4L2M2MConverter::Stream::configure(const StreamConfiguration &inputCfg, return -EINVAL; } - inputBufferCount_ = inputCfg.bufferCount; - outputBufferCount_ = outputCfg.bufferCount; - return 0; } diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index feb69991..02dc9406 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -234,7 +234,6 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const cfg.size = sensorFormat.size; cfg.pixelFormat = mbusCodesToPixelFormat.at(sensorFormat.mbus_code); - cfg.bufferCount = kBufferCount; return cfg; } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 4d8fcfeb..4511adde 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -98,7 +98,6 @@ private: class IPU3CameraConfiguration : public CameraConfiguration { public: - static constexpr unsigned int kBufferCount = 4; static constexpr unsigned int kMaxStreams = 3; IPU3CameraConfiguration(IPU3CameraData *data); @@ -326,7 +325,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() /* Initialize the RAW stream with the CIO2 configuration. */ cfg->size = cio2Configuration_.size; cfg->pixelFormat = cio2Configuration_.pixelFormat; - cfg->bufferCount = cio2Configuration_.bufferCount; cfg->stride = info.stride(cfg->size.width, 0, 64); cfg->frameSize = info.frameSize(cfg->size, 64); cfg->setStream(const_cast(&data_->rawStream_)); @@ -370,7 +368,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() ImgUDevice::kOutputAlignHeight); cfg->pixelFormat = formats::NV12; - cfg->bufferCount = kBufferCount; cfg->stride = info.stride(cfg->size.width, 0, 1); cfg->frameSize = info.frameSize(cfg->size, 1); @@ -439,7 +436,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro Size sensorResolution = data->cio2_.sensor()->resolution(); for (const StreamRole role : roles) { std::map> streamFormats; - unsigned int bufferCount; PixelFormat pixelFormat; Size size; @@ -459,7 +455,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro .alignedDownTo(ImgUDevice::kOutputMarginWidth, ImgUDevice::kOutputMarginHeight); pixelFormat = formats::NV12; - bufferCount = IPU3CameraConfiguration::kBufferCount; streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, size } }; break; @@ -469,7 +464,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro data->cio2_.generateConfiguration(sensorResolution); pixelFormat = cio2Config.pixelFormat; size = cio2Config.size; - bufferCount = cio2Config.bufferCount; for (const PixelFormat &format : data->cio2_.formats()) streamFormats[format] = data->cio2_.sizes(format); @@ -488,7 +482,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro .alignedDownTo(ImgUDevice::kOutputAlignWidth, ImgUDevice::kOutputAlignHeight); pixelFormat = formats::NV12; - bufferCount = IPU3CameraConfiguration::kBufferCount; streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, size } }; break; @@ -504,7 +497,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro StreamConfiguration cfg(formats); cfg.size = size; cfg.pixelFormat = pixelFormat; - cfg.bufferCount = bufferCount; config->addConfiguration(cfg); } diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 72502c36..c0d96024 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -591,7 +591,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol std::unique_ptr config = std::make_unique(data); V4L2SubdeviceFormat sensorFormat; - unsigned int bufferCount; PixelFormat pixelFormat; V4L2VideoDevice::Formats fmts; Size size; @@ -612,7 +611,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol BayerFormat::Packing::CSI2); ASSERT(pixelFormat.isValid()); colorSpace = ColorSpace::Raw; - bufferCount = 2; rawCount++; break; @@ -627,7 +625,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol colorSpace = ColorSpace::Sycc; /* Return the largest sensor resolution. */ size = sensorSize; - bufferCount = 1; outCount++; break; @@ -648,7 +645,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol */ colorSpace = ColorSpace::Rec709; size = { 1920, 1080 }; - bufferCount = 4; outCount++; break; @@ -657,7 +653,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol pixelFormat = formats::ARGB8888; colorSpace = ColorSpace::Sycc; size = { 800, 600 }; - bufferCount = 4; outCount++; break; @@ -704,7 +699,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol cfg.size = size; cfg.pixelFormat = pixelFormat; cfg.colorSpace = colorSpace; - cfg.bufferCount = bufferCount; config->addConfiguration(cfg); } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index a168e0ad..9ef9c3e2 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -190,7 +190,6 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, StreamRole role) StreamConfiguration cfg(formats); cfg.pixelFormat = format; cfg.size = maxResolution; - cfg.bufferCount = RKISP1_BUFFER_COUNT; return cfg; } @@ -280,7 +279,6 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, cfg->size.boundTo(maxResolution); cfg->size.expandTo(minResolution); - cfg->bufferCount = RKISP1_BUFFER_COUNT; V4L2DeviceFormat format; format.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h index 5b53783c..366720de 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -63,8 +63,6 @@ public: private: void populateFormats(); - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; - const char *name_; bool running_; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 196e5252..5d309777 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -338,7 +338,6 @@ protected: int queueRequestDevice(Camera *camera, Request *request) override; private: - static constexpr unsigned int kNumInternalBuffers = 3; static constexpr unsigned int kSimpleBufferSlotCount = 16; struct EntityData { @@ -1010,7 +1009,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() cfg.size != pipeConfig_->captureSize) needConversion_ = true; - /* Set the stride, frameSize and bufferCount. */ + /* Set the stride and frameSize. */ if (needConversion_) { std::tie(cfg.stride, cfg.frameSize) = data_->converter_->strideAndFrameSize(cfg.pixelFormat, @@ -1029,8 +1028,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() cfg.stride = format.planes[0].bpl; cfg.frameSize = format.planes[0].size; } - - cfg.bufferCount = 3; } return status; @@ -1174,7 +1171,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) inputCfg.pixelFormat = pipeConfig->captureFormat; inputCfg.size = pipeConfig->captureSize; inputCfg.stride = captureFormat.planes[0].bpl; - inputCfg.bufferCount = kNumInternalBuffers; /* Set the MinimumRequests property. */ unsigned int minimumRequests; diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 18966d01..5d88e6f3 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -154,8 +154,6 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -196,7 +194,6 @@ PipelineHandlerUVC::generateConfiguration(Camera *camera, cfg.pixelFormat = formats.pixelformats().front(); cfg.size = formats.sizes(cfg.pixelFormat).back(); - cfg.bufferCount = 4; config->addConfiguration(cfg); diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index eaa6ebf7..ef5146f9 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -171,8 +171,6 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -230,7 +228,6 @@ PipelineHandlerVimc::generateConfiguration(Camera *camera, cfg.pixelFormat = formats::BGR888; cfg.size = { 1920, 1080 }; - cfg.bufferCount = 4; config->addConfiguration(cfg); diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 67f30815..aa61c855 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -280,8 +280,7 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const * handlers provide StreamFormats. */ StreamConfiguration::StreamConfiguration() - : pixelFormat(0), stride(0), frameSize(0), bufferCount(0), - stream_(nullptr) + : pixelFormat(0), stride(0), frameSize(0), stream_(nullptr) { } @@ -289,8 +288,8 @@ StreamConfiguration::StreamConfiguration() * \brief Construct a configuration with stream formats */ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) - : pixelFormat(0), stride(0), frameSize(0), bufferCount(0), - stream_(nullptr), formats_(formats) + : pixelFormat(0), stride(0), frameSize(0), stream_(nullptr), + formats_(formats) { } @@ -324,11 +323,6 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) * validating the configuration with a call to CameraConfiguration::validate(). */ -/** - * \var StreamConfiguration::bufferCount - * \brief Requested number of buffers to allocate for the stream - */ - /** * \var StreamConfiguration::colorSpace * \brief The ColorSpace for this stream diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 92884004..cb7a0cef 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -16,6 +16,8 @@ #include #include +#include + #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/v4l2_videodevice.h" @@ -93,10 +95,13 @@ protected: return TestFail; } + unsigned int bufferCount = + camera_->properties().get(properties::MinimumRequests).value(); + Stream *stream = cfg.stream(); BufferSource source; - int ret = source.allocate(cfg); + int ret = source.allocate(cfg, bufferCount); if (ret != TestPass) return ret; @@ -140,10 +145,10 @@ protected: while (timer.isRunning()) dispatcher->processEvents(); - if (completeRequestsCount_ < cfg.bufferCount * 2) { + if (completeRequestsCount_ < bufferCount * 2) { std::cout << "Failed to capture enough frames (got " << completeRequestsCount_ << " expected at least " - << cfg.bufferCount * 2 << ")" << std::endl; + << bufferCount * 2 << ")" << std::endl; return TestFail; } diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp index dde11f36..c1bc45db 100644 --- a/test/libtest/buffer_source.cpp +++ b/test/libtest/buffer_source.cpp @@ -26,7 +26,7 @@ BufferSource::~BufferSource() media_->release(); } -int BufferSource::allocate(const StreamConfiguration &config) +int BufferSource::allocate(const StreamConfiguration &config, unsigned int count) { /* Locate and open the video device. */ std::string videoDeviceName = "vivid-000-vid-out"; @@ -78,7 +78,7 @@ int BufferSource::allocate(const StreamConfiguration &config) return TestFail; } - if (video->allocateBuffers(config.bufferCount, &buffers_) < 0) { + if (video->allocateBuffers(count, &buffers_) < 0) { std::cout << "Failed to allocate buffers" << std::endl; return TestFail; } diff --git a/test/libtest/buffer_source.h b/test/libtest/buffer_source.h index 0cc71aa5..f430a7f3 100644 --- a/test/libtest/buffer_source.h +++ b/test/libtest/buffer_source.h @@ -18,7 +18,7 @@ public: BufferSource(); ~BufferSource(); - int allocate(const libcamera::StreamConfiguration &config); + int allocate(const libcamera::StreamConfiguration &config, unsigned int count); const std::vector> &buffers(); private: diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp index 5a9aa219..8d2cf6d1 100644 --- a/test/v4l2_videodevice/buffer_cache.cpp +++ b/test/v4l2_videodevice/buffer_cache.cpp @@ -174,10 +174,9 @@ public: StreamConfiguration cfg; cfg.pixelFormat = formats::YUYV; cfg.size = Size(600, 800); - cfg.bufferCount = numBuffers; BufferSource source; - int ret = source.allocate(cfg); + int ret = source.allocate(cfg, numBuffers); if (ret != TestPass) return ret; From patchwork Fri Dec 16 12:29:32 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18025 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id AF5EEC328E for ; Fri, 16 Dec 2022 12:30:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 720AB63360; Fri, 16 Dec 2022 13:30:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193822; bh=2aDr6B6L5+L0eZxbwtrNI5HCG8OB2SMdQmCtFUg4iiw=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=TJ6IDHzXcapU9BFXrPjBR3eekdIhGFosQScQ/BIWEiBg8xNEytXxFd9xQ8E6T+1/S YPnTjbqwhPhKPtvbvrNKjNf7yyezHMQJM06zDdOjV45uPFxWov1WVH6UpUwv8c5f9J dZp+LXYxxa2sSsJA2+6Xz/0ONQii1yHh4i8ggZCG5yAta48umqCf8eSlvZ4wjlqLn4 2l9hVZteUVCbpH6vCa/+ZidWeo1KODsjnhRlB8a39xV7PXkgujNyYh+Qqqo91MXtUC dOQGm8KQ9I4NjdXFzGEtB59bJanRZ+lOhLPc1v2m7sZ6fXg85ex0LBc6D3+O95dcfA yRNAsqbW3VVZA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 39B57633A6 for ; Fri, 16 Dec 2022 13:30:21 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="oi7raAL9"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C28A3110E; Fri, 16 Dec 2022 13:30:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193821; bh=2aDr6B6L5+L0eZxbwtrNI5HCG8OB2SMdQmCtFUg4iiw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=oi7raAL9e16JR9viteBGEWitXTVbWoZ3v0KUloocUAQ+ZTdsf/xqnSH4BnXIaOg/s p1n6vPWHLB0tvFxU6trRD8zIHA7VQ/yHzSB3Vko+5gjyzsAUIpRU093oE/Wq67QVUG M7bL9IZ+oYmQEwgJFn2Jsd3hrwCYw/kCZKux5jXs= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:32 +0900 Message-Id: <20221216122939.256534-12-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 11/18] lc-compliance: Fix source file ordering in meson.build X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado The capture_test.cpp file was added in the source list of meson in the wrong place. Fix it so the list is alphabetically sorted. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder Reviewed-by: Umang Jain --- Changes in v9: - rebased Changes in v8: - New --- src/apps/lc-compliance/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build index 51d9075a..3b1927fd 100644 --- a/src/apps/lc-compliance/meson.build +++ b/src/apps/lc-compliance/meson.build @@ -11,10 +11,10 @@ endif lc_compliance_enabled = true lc_compliance_sources = files([ + 'capture_test.cpp', 'environment.cpp', 'main.cpp', 'simple_capture.cpp', - 'capture_test.cpp', ]) lc_compliance = executable('lc-compliance', lc_compliance_sources, From patchwork Fri Dec 16 12:29:33 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18026 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 62E38C31E9 for ; Fri, 16 Dec 2022 12:30:25 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 23AAA633AE; Fri, 16 Dec 2022 13:30:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193825; bh=0Ty1h03QfLcxojEN+CbLhHIvt/SjBcyEsZ0W/uCgrXs=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=k75Idlki8MsdhBwYgOS95vPP0AL/ldPW5v2c/kdqEbPRn30QAzlxIb6md/Zlg4iT9 d+Pi6PXiwXriqpyuNT5WXpMaPL6auI9voS5k+UDZuX2fDWviY28ofRaI9z03A+dsfT 7lPW1GOhxYk4Zr36qm+q1cKijAPPDRX0no1qATt18tz8BgbvfSXQ90FPWCdQatJ62F 3naWwtK0q2bAWLdjsayOAM1inz//xRkegDW9hT+bJVYt3lqIENB4OSI2Uo9idnQWa7 20R+xn9z9MypxSxRqL3WQzoYS1Iuom2R2K8+Q10kY+u9kATbNL1BkTExAKeZL8KRtj C0XzftGsbwGgg== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id ECD7C633A3 for ; Fri, 16 Dec 2022 13:30:22 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="roU6xCpS"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8361DA31; Fri, 16 Dec 2022 13:30:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193822; bh=0Ty1h03QfLcxojEN+CbLhHIvt/SjBcyEsZ0W/uCgrXs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=roU6xCpSa43zNDirhoWadzjVKra42t+f7A/3SXlONZ+mkprDpxxK+ALp5agb0m1wn MJufmp7fxgpBoPfq2BOBeOGwO7w63hqpEueq2YM+hGxaEEocX7MX/83CvGF7err2BZ 4dbSFTROF4k8LOJIJY4g7fGX+GMOkrrapqTsYP0k= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:33 +0900 Message-Id: <20221216122939.256534-13-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 12/18] lc-compliance: Move buffer allocation to separate function X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Move buffer allocation to its own function and with an optional count argument so tests can specify how many buffers to allocate. When count is omitted, allocate MinimumRequests buffers. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder --- Changes in v9: - rebased Changes in v8: - Made SimpleCapture::allocateBuffers() a single function with an optional parameter Changes in v5: - New --- src/apps/lc-compliance/capture_test.cpp | 8 +++++++- src/apps/lc-compliance/simple_capture.cpp | 16 ++++++++++------ src/apps/lc-compliance/simple_capture.h | 1 + 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/apps/lc-compliance/capture_test.cpp b/src/apps/lc-compliance/capture_test.cpp index 1dcfcf92..923395cf 100644 --- a/src/apps/lc-compliance/capture_test.cpp +++ b/src/apps/lc-compliance/capture_test.cpp @@ -87,6 +87,8 @@ TEST_P(SingleStream, Capture) capture.configure(role); + capture.allocateBuffers(); + capture.capture(numRequests); } @@ -106,8 +108,10 @@ TEST_P(SingleStream, CaptureStartStop) capture.configure(role); - for (unsigned int starts = 0; starts < numRepeats; starts++) + for (unsigned int starts = 0; starts < numRepeats; starts++) { + capture.allocateBuffers(); capture.capture(numRequests); + } } /* @@ -125,6 +129,8 @@ TEST_P(SingleStream, UnbalancedStop) capture.configure(role); + capture.allocateBuffers(); + capture.capture(numRequests); } diff --git a/src/apps/lc-compliance/simple_capture.cpp b/src/apps/lc-compliance/simple_capture.cpp index be495986..1e3e816a 100644 --- a/src/apps/lc-compliance/simple_capture.cpp +++ b/src/apps/lc-compliance/simple_capture.cpp @@ -44,16 +44,20 @@ void SimpleCapture::configure(StreamRole role) } } -void SimpleCapture::start() +void SimpleCapture::allocateBuffers(unsigned int count) { - unsigned int bufferCount = - camera_->properties().get(properties::MinimumRequests).value(); + if (!count) + count = camera_->properties().get(properties::MinimumRequests).value(); + Stream *stream = config_->at(0).stream(); - int count = allocator_->allocate(stream, bufferCount); + int allocatedCount = allocator_->allocate(stream, count); - ASSERT_GE(count, 0) << "Failed to allocate buffers"; - EXPECT_EQ(count, bufferCount) << "Allocated less buffers than expected"; + ASSERT_GE(allocatedCount, 0) << "Failed to allocate buffers"; + EXPECT_EQ(allocatedCount, count) << "Allocated less buffers than expected"; +} +void SimpleCapture::start() +{ camera_->requestCompleted.connect(this, &SimpleCapture::requestComplete); ASSERT_EQ(camera_->start(), 0) << "Failed to start camera"; diff --git a/src/apps/lc-compliance/simple_capture.h b/src/apps/lc-compliance/simple_capture.h index 2911d601..b3091547 100644 --- a/src/apps/lc-compliance/simple_capture.h +++ b/src/apps/lc-compliance/simple_capture.h @@ -17,6 +17,7 @@ class SimpleCapture { public: void configure(libcamera::StreamRole role); + void allocateBuffers(unsigned int count = 0); protected: SimpleCapture(std::shared_ptr camera); From patchwork Fri Dec 16 12:29:34 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18027 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id D8347C31E9 for ; Fri, 16 Dec 2022 12:30:26 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8C008633AF; Fri, 16 Dec 2022 13:30:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193826; bh=VPXe7AwBiDzM/pQmANFAF8RGe93JNSnljJB4v1hFYsg=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=ZvBMJ01KJO0y9pp59xLTG3N9R6GegS99gaRB0ueo2twyvcmAOFq0X8u9T+C0inxJx ri7j4cvJFiAPvCIqxA/Ta4/eS9BAXgTKFC+ZtiKHyC5R50z1FCcUsQK6E/vz1EiSeT yKs1AXZLwNdw25qzO8sZmheil1VLs2U3VGB8fd3/Vih2XI6s78GIxU6RyvXQ8t0yuQ yLjZjkGjQOvD/j/sW0AqZoMwlqYRZiM3vGIO6yEfgDtWHsKtp3Ak1Ghm8shi50fs8S WCbE+Bfr6jEDWAxX4nIXjfSYubNrN/oA7UT5MRf/BhQN+6vIM0JbpP6HAt4wHBWozi 6HrIzBbkdsacA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AB101633A4 for ; Fri, 16 Dec 2022 13:30:24 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="sRlWnkfx"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4638E128D; Fri, 16 Dec 2022 13:30:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193824; bh=VPXe7AwBiDzM/pQmANFAF8RGe93JNSnljJB4v1hFYsg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sRlWnkfxNwXPj/7jx3JnNFptoqg2Tp+rumnK6vhYAwebCpYFNvvsXTfntWktffEHv BeatAxQIzL8UU99I5balh10gnROAHAGiUn4HlgLGAt5t4QacESuhU/pKr/Pj0EdBUr n9IpdPNhK3nRW229qvRDq3m9pP/dGvkq5ApuoK80= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:34 +0900 Message-Id: <20221216122939.256534-14-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 13/18] lc-compliance: Factor common capture code into SimpleCapture X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Factor common code from SimpleCapture* subclasses into the SimpleCapture parent class in order to avoid duplicated code. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder --- Changes in v9: - rebased Changes in v8: - Added requests_ member to SimpleCapture to hold ownership of queued requests during capture - Made queueRequest() virtual and removed SimpleCaptureBalanced::queueRequests() Changes in v6: - Switched queueRequests()'s 'buffers' and 'requests' parameters order, since 'requests' is an output variable - Added comment to runCaptureSession() - Added missing blank line before return in captureCompleted() - Added blank line to runCaptureSession() --- src/apps/lc-compliance/simple_capture.cpp | 89 +++++++++++++---------- src/apps/lc-compliance/simple_capture.h | 15 ++-- 2 files changed, 58 insertions(+), 46 deletions(-) diff --git a/src/apps/lc-compliance/simple_capture.cpp b/src/apps/lc-compliance/simple_capture.cpp index 1e3e816a..6dd5c7af 100644 --- a/src/apps/lc-compliance/simple_capture.cpp +++ b/src/apps/lc-compliance/simple_capture.cpp @@ -77,6 +77,49 @@ void SimpleCapture::stop() allocator_->free(stream); } +bool SimpleCapture::captureCompleted() +{ + captureCount_++; + if (captureCount_ >= captureLimit_) { + loop_->exit(0); + return true; + } + + return false; +} + +int SimpleCapture::queueRequest(Request *request) +{ + return camera_->queueRequest(request); +} + +void SimpleCapture::queueRequests(Stream *stream, + const std::vector> &buffers) +{ + for (const std::unique_ptr &buffer : buffers) { + std::unique_ptr request = camera_->createRequest(); + ASSERT_TRUE(request) << "Can't create request"; + + ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request"; + + ASSERT_EQ(queueRequest(request.get()), 0) << "Failed to queue request"; + + requests_.push_back(std::move(request)); + } +} + +int SimpleCapture::runCaptureSession() +{ + loop_ = new EventLoop(); + int status = loop_->exec(); + + /* After session ended */ + stop(); + delete loop_; + + return status; +} + /* SimpleCaptureBalanced */ SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr camera) @@ -103,23 +146,9 @@ void SimpleCaptureBalanced::capture(unsigned int numRequests) captureCount_ = 0; captureLimit_ = numRequests; - /* Queue the recommended number of reqeuests. */ - for (const std::unique_ptr &buffer : buffers) { - std::unique_ptr request = camera_->createRequest(); - ASSERT_TRUE(request) << "Can't create request"; - - ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request"; + queueRequests(stream, buffers); - ASSERT_EQ(queueRequest(request.get()), 0) << "Failed to queue request"; - - requests_.push_back(std::move(request)); - } - - /* Run capture session. */ - loop_ = new EventLoop(); - loop_->exec(); - stop(); - delete loop_; + runCaptureSession(); ASSERT_EQ(captureCount_, captureLimit_); } @@ -135,11 +164,8 @@ int SimpleCaptureBalanced::queueRequest(Request *request) void SimpleCaptureBalanced::requestComplete(Request *request) { - captureCount_++; - if (captureCount_ >= captureLimit_) { - loop_->exit(0); + if (captureCompleted()) return; - } request->reuse(Request::ReuseBuffers); if (queueRequest(request)) @@ -163,34 +189,17 @@ void SimpleCaptureUnbalanced::capture(unsigned int numRequests) captureCount_ = 0; captureLimit_ = numRequests; - /* Queue the recommended number of reqeuests. */ - for (const std::unique_ptr &buffer : buffers) { - std::unique_ptr request = camera_->createRequest(); - ASSERT_TRUE(request) << "Can't create request"; - - ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request"; + queueRequests(stream, buffers); - ASSERT_EQ(camera_->queueRequest(request.get()), 0) << "Failed to queue request"; - - requests_.push_back(std::move(request)); - } - - /* Run capture session. */ - loop_ = new EventLoop(); - int status = loop_->exec(); - stop(); - delete loop_; + int status = runCaptureSession(); ASSERT_EQ(status, 0); } void SimpleCaptureUnbalanced::requestComplete(Request *request) { - captureCount_++; - if (captureCount_ >= captureLimit_) { - loop_->exit(0); + if (captureCompleted()) return; - } request->reuse(Request::ReuseBuffers); if (camera_->queueRequest(request)) diff --git a/src/apps/lc-compliance/simple_capture.h b/src/apps/lc-compliance/simple_capture.h index b3091547..89ecc71c 100644 --- a/src/apps/lc-compliance/simple_capture.h +++ b/src/apps/lc-compliance/simple_capture.h @@ -25,11 +25,19 @@ protected: void start(); void stop(); + virtual int queueRequest(libcamera::Request *request); + void queueRequests(libcamera::Stream *stream, + const std::vector> &buffers); + bool captureCompleted(); + int runCaptureSession(); virtual void requestComplete(libcamera::Request *request) = 0; EventLoop *loop_; + unsigned int captureCount_; + unsigned int captureLimit_; + std::shared_ptr camera_; std::unique_ptr allocator_; std::unique_ptr config_; @@ -44,12 +52,10 @@ public: void capture(unsigned int numRequests); private: - int queueRequest(libcamera::Request *request); + int queueRequest(libcamera::Request *request) override; void requestComplete(libcamera::Request *request) override; unsigned int queueCount_; - unsigned int captureCount_; - unsigned int captureLimit_; }; class SimpleCaptureUnbalanced : public SimpleCapture @@ -61,7 +67,4 @@ public: private: void requestComplete(libcamera::Request *request) override; - - unsigned int captureCount_; - unsigned int captureLimit_; }; From patchwork Fri Dec 16 12:29:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18028 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 5D875C31E9 for ; Fri, 16 Dec 2022 12:30:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 17E48633AB; Fri, 16 Dec 2022 13:30:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193829; bh=WoN22LIegFrp62olPQq8QlNLCc0xK4dHM9kOmMmOYSw=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=G2wC3P4LKX+4Z43DENuvfl1sGU41bWi3L8GHA9+m3xRlGIsOCClbknB7MNcJ+quGB rUo9zobFBlumohT9D+WaexNF6lnPLBjPzpx9oIRqCjilMPY72nrN08k+AccpI8rWV1 pGaaapzwVUiYlGsGYTRy60HmroZ6LyWBhb1T1cFFh3F4i1t3RkyLgsE5RTO3yMgLxs r64KjBHG/2fLiwsLSLAaTSfJKsIZtKrW9T/i/OzCDku/sccfwHBlIx1OCAfbcJ3voZ yyrz+raatPDToPQvIRLfV+Ork8A+qiTddwzYjAz9RTKnz070/zwYNHpRGd99GQouNq YSgTIfyvMDiqQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B3066633B3 for ; Fri, 16 Dec 2022 13:30:26 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="uPsnrku0"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 08371A31; Fri, 16 Dec 2022 13:30:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193826; bh=WoN22LIegFrp62olPQq8QlNLCc0xK4dHM9kOmMmOYSw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uPsnrku0KHrkMptsBp6ZF5tnO9KBaQTHxD7Ooux6vDRVUJ6mBc+GukxRWF2cnvPzA dV3WE6SMoXoinq2M2b9tqz6BPmIgZ4xq1GyWiweTbaniI+C6c0QR6Nk6O/N6GlAglV QWY8F37ad5kcGHvqVJRfOJDjGqfMUQ8U+Hdlpw3Q= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:35 +0900 Message-Id: <20221216122939.256534-15-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 14/18] lc-compliance: Move camera setup to CameraHolder class X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Different base classes can be used for different setups on tests, but all of them will need to setup the camera for the test. To reuse that code, move it to a separate CameraHolder class that is inherited by test classes. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Laurent Pinchart Reviewed-by: Paul Elder Signed-off-by: Paul Elder --- Changes in v9: - rebased Changes in v8: - Moved CameraHolder to new test_base.{cpp,h} files Changes in v5: - New --- src/apps/lc-compliance/capture_test.cpp | 18 ++++------------ src/apps/lc-compliance/meson.build | 1 + src/apps/lc-compliance/test_base.cpp | 28 +++++++++++++++++++++++++ src/apps/lc-compliance/test_base.h | 24 +++++++++++++++++++++ 4 files changed, 57 insertions(+), 14 deletions(-) create mode 100644 src/apps/lc-compliance/test_base.cpp create mode 100644 src/apps/lc-compliance/test_base.h diff --git a/src/apps/lc-compliance/capture_test.cpp b/src/apps/lc-compliance/capture_test.cpp index 923395cf..2b82443a 100644 --- a/src/apps/lc-compliance/capture_test.cpp +++ b/src/apps/lc-compliance/capture_test.cpp @@ -10,8 +10,8 @@ #include -#include "environment.h" #include "simple_capture.h" +#include "test_base.h" using namespace libcamera; @@ -23,7 +23,7 @@ const std::vector ROLES = { StreamRole::Viewfinder }; -class SingleStream : public testing::TestWithParam> +class SingleStream : public testing::TestWithParam>, public CameraHolder { public: static std::string nameParameters(const testing::TestParamInfo &info); @@ -31,8 +31,6 @@ public: protected: void SetUp() override; void TearDown() override; - - std::shared_ptr camera_; }; /* @@ -41,20 +39,12 @@ protected: */ void SingleStream::SetUp() { - Environment *env = Environment::get(); - - camera_ = env->cm()->get(env->cameraId()); - - ASSERT_EQ(camera_->acquire(), 0); + acquireCamera(); } void SingleStream::TearDown() { - if (!camera_) - return; - - camera_->release(); - camera_.reset(); + releaseCamera(); } std::string SingleStream::nameParameters(const testing::TestParamInfo &info) diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build index 3b1927fd..fae290bb 100644 --- a/src/apps/lc-compliance/meson.build +++ b/src/apps/lc-compliance/meson.build @@ -15,6 +15,7 @@ lc_compliance_sources = files([ 'environment.cpp', 'main.cpp', 'simple_capture.cpp', + 'test_base.cpp', ]) lc_compliance = executable('lc-compliance', lc_compliance_sources, diff --git a/src/apps/lc-compliance/test_base.cpp b/src/apps/lc-compliance/test_base.cpp new file mode 100644 index 00000000..c9957b9e --- /dev/null +++ b/src/apps/lc-compliance/test_base.cpp @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2021, Collabora Ltd. + * + * test_base.cpp - Base definitions for tests + */ + +#include "test_base.h" + +#include "environment.h" + +void CameraHolder::acquireCamera() +{ + Environment *env = Environment::get(); + + camera_ = env->cm()->get(env->cameraId()); + + ASSERT_EQ(camera_->acquire(), 0); +} + +void CameraHolder::releaseCamera() +{ + if (!camera_) + return; + + camera_->release(); + camera_.reset(); +} diff --git a/src/apps/lc-compliance/test_base.h b/src/apps/lc-compliance/test_base.h new file mode 100644 index 00000000..52347749 --- /dev/null +++ b/src/apps/lc-compliance/test_base.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2021, Collabora Ltd. + * + * test_base.h - Base definitions for tests + */ + +#ifndef __LC_COMPLIANCE_TEST_BASE_H__ +#define __LC_COMPLIANCE_TEST_BASE_H__ + +#include + +#include + +class CameraHolder +{ +protected: + void acquireCamera(); + void releaseCamera(); + + std::shared_ptr camera_; +}; + +#endif /* __LC_COMPLIANCE_TEST_BASE_H__ */ From patchwork Fri Dec 16 12:29:36 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18029 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id B79B8C328E for ; Fri, 16 Dec 2022 12:30:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7C3AE6339F; Fri, 16 Dec 2022 13:30:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193829; bh=01pZHYVZngDe+B2HzCbcASuxSqUGF6xdmRpG+MH2Sh8=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Kr9d0zngmA9qwUyJpkyV5v994Jz5Rxr84Hhq+HeH9cJ8Q0G4SDCct4Vy+JkiC4Aph 9xJTkH4Uj3ZK/FJ89KFpxALY2dXTR2RHpyaJq07+cbCvk78pCTA/CdEyN6mVM2BW8Z H9KfLrQ3mQWrssCJoHP2YDEjFHKVqOwoH+jd2cJy/QlUoM8zaU5skoprqHfXuJNZyO yp7JWQKMlDIC9gPO0PteNNBVnPVVqIjBR85EK6x3trW++n758+Jn0BA4RfpH9M4VTn yEiJQfq9xnLI88LAs1qrV0mXuKmhQt6cm4S97a/CCcd7GhzuK4coAC/VkXTlt39t9c pJAmVBgEyaW1Q== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A3DAA6339F for ; Fri, 16 Dec 2022 13:30:28 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="rlewFkH0"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 00817A31; Fri, 16 Dec 2022 13:30:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193828; bh=01pZHYVZngDe+B2HzCbcASuxSqUGF6xdmRpG+MH2Sh8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=rlewFkH0fokTo2PLvCU+pKJLJgvBNrsVSzANxXihXsxlKj1SaTqMgWY7+j1e1FKOV npDWppFdxdBeN/WWbRIPSiidGQ35d3H7YCvtF3n2SYmPdLPlONQkVOq/u23+KIJbMA 3DZutr5z+XKv+IBE2mFACIvIcNe6F3e60UB7FP1M= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:36 +0900 Message-Id: <20221216122939.256534-16-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 15/18] lc-compliance: Move role to string conversion to its own function X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado The functions that generate the test name based on the parameters need to convert a StreamRole to a string. Move this to a separate function to avoid redundancy. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Laurent Pinchart Reviewed-by: Paul Elder Signed-off-by: Paul Elder --- Changes in v8: - Made roleToString()'s map const and changed usage from [] operator to at() Changes in v5: - New --- src/apps/lc-compliance/capture_test.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/apps/lc-compliance/capture_test.cpp b/src/apps/lc-compliance/capture_test.cpp index 2b82443a..d62f7692 100644 --- a/src/apps/lc-compliance/capture_test.cpp +++ b/src/apps/lc-compliance/capture_test.cpp @@ -23,6 +23,18 @@ const std::vector ROLES = { StreamRole::Viewfinder }; +static const std::string &roleToString(const StreamRole &role) +{ + static const std::map rolesMap = { + { StreamRole::Raw, "Raw" }, + { StreamRole::StillCapture, "StillCapture" }, + { StreamRole::VideoRecording, "VideoRecording" }, + { StreamRole::Viewfinder, "Viewfinder" } + }; + + return rolesMap.at(role); +} + class SingleStream : public testing::TestWithParam>, public CameraHolder { public: @@ -49,17 +61,8 @@ void SingleStream::TearDown() std::string SingleStream::nameParameters(const testing::TestParamInfo &info) { - std::map rolesMap = { - { StreamRole::Raw, "Raw" }, - { StreamRole::StillCapture, "StillCapture" }, - { StreamRole::VideoRecording, "VideoRecording" }, - { StreamRole::Viewfinder, "Viewfinder" } - }; - - std::string roleName = rolesMap[std::get<0>(info.param)]; - std::string numRequestsName = std::to_string(std::get<1>(info.param)); - - return roleName + "_" + numRequestsName; + return roleToString(std::get<0>(info.param)) + "_" + + std::to_string(std::get<1>(info.param)); } /* From patchwork Fri Dec 16 12:29:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18030 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 814DFC31E9 for ; Fri, 16 Dec 2022 12:30:32 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 42064633B1; Fri, 16 Dec 2022 13:30:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193832; bh=OEmstN1ZafXMAjzzMaEtkY6Zm9yHYv3lOWuomsxExj8=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=M+miiVUMsid7jK2/KJkdgmM0OmhPVRL+TD2rWJQp3qZt9caXXw7BKVB73IdpB8ArZ gYUvg1bOpxUDTQbs2qsBzVyhuc0hEAlEcoLwDtPlOh3nTVTQG1vqI21Mr5b45cIatn MFb9fsVbq1n5FwG1mS0ltwxVqPlTsC9/OLyBGDGOuwfBqZYoh97h4PluXXezoHQJjg uyJmHdnAqtnWNWMw/8UTdaTKLhFHu22vQLJqsuWDfWT7k0KyCAiWXUOPLBowBQVYXJ Nnj1rQlwRuHfkqChuYcp/noXzS/0am9UHilPU3REcpb1gw/Sp6q+tMM3d8A1H2yUt0 x+TkZGE1DS1xA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 639EF63360 for ; Fri, 16 Dec 2022 13:30:30 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="fbWu1yI3"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F29FAA31; Fri, 16 Dec 2022 13:30:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193830; bh=OEmstN1ZafXMAjzzMaEtkY6Zm9yHYv3lOWuomsxExj8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fbWu1yI3U+wGKAH6Hd5Wqab8wO3D+L6u7/kFbHZdSUFG2aACZK0Qmdr3FV17ZcEu9 jLu80dgjYKQHcO5hPYPlo3adQst/LQ5JKpDpDjx49ymYWioDfHHiGaUZmIQIVfBk0M 5iAZfEOVp2botTByKXf3ZBHxcY0mLSDxENclhy3o= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:37 +0900 Message-Id: <20221216122939.256534-17-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 16/18] lc-compliance: Add test to queue more requests than hardware depth X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado A pipeline handler should be able to handle an arbitrary number of simultaneous requests by submitting what it can to the video device and queuing the rest internally until resources are available. This isn't currently done by some pipeline handlers however [1]. Add a new test to lc-compliance that submits a lot of requests at once to check if the pipeline handler is behaving well in this situation. [1] https://bugs.libcamera.org/show_bug.cgi?id=24 Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder --- Changes in v9: - rebased (no changes since v6) Changes in v6: - Made MAX_SIMULTANEOUS_REQUESTS a constexpr Changes in v5: - Updated to use googletest, and CameraHolder and roleToString from previous patches --- src/apps/lc-compliance/capture_test.cpp | 45 +++++++++++++++++++++++ src/apps/lc-compliance/simple_capture.cpp | 29 +++++++++++++++ src/apps/lc-compliance/simple_capture.h | 11 ++++++ 3 files changed, 85 insertions(+) diff --git a/src/apps/lc-compliance/capture_test.cpp b/src/apps/lc-compliance/capture_test.cpp index d62f7692..8dd26120 100644 --- a/src/apps/lc-compliance/capture_test.cpp +++ b/src/apps/lc-compliance/capture_test.cpp @@ -23,6 +23,8 @@ const std::vector ROLES = { StreamRole::Viewfinder }; +static constexpr unsigned int MAX_SIMULTANEOUS_REQUESTS = 8; + static const std::string &roleToString(const StreamRole &role) { static const std::map rolesMap = { @@ -59,12 +61,37 @@ void SingleStream::TearDown() releaseCamera(); } +class RoleParametrizedTest : public testing::TestWithParam, public CameraHolder +{ +public: + static std::string nameParameters(const testing::TestParamInfo &info); + +protected: + void SetUp() override; + void TearDown() override; +}; + +void RoleParametrizedTest::SetUp() +{ + acquireCamera(); +} + +void RoleParametrizedTest::TearDown() +{ + releaseCamera(); +} + std::string SingleStream::nameParameters(const testing::TestParamInfo &info) { return roleToString(std::get<0>(info.param)) + "_" + std::to_string(std::get<1>(info.param)); } +std::string RoleParametrizedTest::nameParameters(const testing::TestParamInfo &info) +{ + return roleToString(info.param); +} + /* * Test single capture cycles * @@ -127,8 +154,26 @@ TEST_P(SingleStream, UnbalancedStop) capture.capture(numRequests); } +TEST_P(RoleParametrizedTest, Overflow) +{ + auto role = GetParam(); + + SimpleCaptureOverflow capture(camera_); + + capture.configure(role); + + capture.allocateBuffers(MAX_SIMULTANEOUS_REQUESTS); + + capture.capture(); +} + INSTANTIATE_TEST_SUITE_P(CaptureTests, SingleStream, testing::Combine(testing::ValuesIn(ROLES), testing::ValuesIn(NUMREQUESTS)), SingleStream::nameParameters); + +INSTANTIATE_TEST_SUITE_P(CaptureTests, + RoleParametrizedTest, + testing::ValuesIn(ROLES), + RoleParametrizedTest::nameParameters); diff --git a/src/apps/lc-compliance/simple_capture.cpp b/src/apps/lc-compliance/simple_capture.cpp index 6dd5c7af..a37a98ac 100644 --- a/src/apps/lc-compliance/simple_capture.cpp +++ b/src/apps/lc-compliance/simple_capture.cpp @@ -205,3 +205,32 @@ void SimpleCaptureUnbalanced::requestComplete(Request *request) if (camera_->queueRequest(request)) loop_->exit(-EINVAL); } + +/* SimpleCaptureOverflow */ + +SimpleCaptureOverflow::SimpleCaptureOverflow(std::shared_ptr camera) + : SimpleCapture(camera) +{ +} + +void SimpleCaptureOverflow::capture() +{ + start(); + + Stream *stream = config_->at(0).stream(); + const std::vector> &buffers = allocator_->buffers(stream); + + captureCount_ = 0; + captureLimit_ = buffers.size(); + + queueRequests(stream, buffers); + + runCaptureSession(); + + ASSERT_EQ(captureCount_, captureLimit_); +} + +void SimpleCaptureOverflow::requestComplete([[maybe_unused]] Request *request) +{ + captureCompleted(); +} diff --git a/src/apps/lc-compliance/simple_capture.h b/src/apps/lc-compliance/simple_capture.h index 89ecc71c..18be8a01 100644 --- a/src/apps/lc-compliance/simple_capture.h +++ b/src/apps/lc-compliance/simple_capture.h @@ -68,3 +68,14 @@ public: private: void requestComplete(libcamera::Request *request) override; }; + +class SimpleCaptureOverflow : public SimpleCapture +{ +public: + SimpleCaptureOverflow(std::shared_ptr camera); + + void capture(); + +private: + void requestComplete(libcamera::Request *request) override; +}; From patchwork Fri Dec 16 12:29:38 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18031 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id F117BC31E9 for ; Fri, 16 Dec 2022 12:30:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id ABEEA633B2; Fri, 16 Dec 2022 13:30:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193833; bh=mf9lLD2HWERLMm1bvaPiXYY+T6Bud9wv1LwIPxTz7ag=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=ynYB5TmvdGasThu/YHb5ib11+82QUx2DVUipuuVKpLpHbsYEgAZ6vOcDEmt0Q2j0R VWvY2swdCliP+HYTOPmMb7HdEG9tDYcK4ib+IUFqXXZUgVZKYVGwcsJ8ohm2XyB/bf dW4aGWsFtxi+zKcFoXE4Fh1fIFxv45rQnoWceKWPxWAyM+i0u2G3AgH2bInobLT9P6 Uk4+QIZoVl4wT5GDza6Usa7zrtS+7VgGtqukYyuqcRX4M/7HDFCiYW3aAXorlT3Mxa TuwqmNnro2VvL7GaGQafgHpOfIi/dM/wMm6wRBQV8cXX7n/hWaRyPla5xcB/i5LJCg L7qEZwKnTEeWg== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 25C1063360 for ; Fri, 16 Dec 2022 13:30:32 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="p/O7DIst"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id B391C128D; Fri, 16 Dec 2022 13:30:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193831; bh=mf9lLD2HWERLMm1bvaPiXYY+T6Bud9wv1LwIPxTz7ag=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=p/O7DIst/tq5cAAfS+tiLcSX6wOFEUsHkqR9Hjhg0X6B3kA935KyNayEBBeUYCOBo kfbmwX4ZjJ/WtqsVXnltYFlTOzjjHMMONWsQwGP2hN5wAlu9g0o6c0Oo1DZxFBp/LS P7yS7OkoCPxbFmvZnvQb41E4NZXAdDuF1s9x7DHs= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:38 +0900 Message-Id: <20221216122939.256534-18-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 17/18] lc-compliance: Check that requests complete successfully X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado When a request fails to queue it is completed but with its status set to RequestCancelled. Add a check in the requestComplete callback to make sure that the request was completed successfully. For the SimpleCaptureUnbalanced test we need to do this check only if the capture isn't over yet, otherwise the few extra requests that get cancelled at the end, which is the normal behavior, will make the test fail. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder --- Changes in v9: - rebased Changes in v8: - Fixed issue in UnbalancedStop test where requests cancelled due to stop() call were failing the test - Fixed formatting Changes in v5: - New --- src/apps/lc-compliance/simple_capture.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/apps/lc-compliance/simple_capture.cpp b/src/apps/lc-compliance/simple_capture.cpp index a37a98ac..02b00d9a 100644 --- a/src/apps/lc-compliance/simple_capture.cpp +++ b/src/apps/lc-compliance/simple_capture.cpp @@ -164,6 +164,9 @@ int SimpleCaptureBalanced::queueRequest(Request *request) void SimpleCaptureBalanced::requestComplete(Request *request) { + EXPECT_EQ(request->status(), Request::Status::RequestComplete) + << "Request didn't complete successfully"; + if (captureCompleted()) return; @@ -201,6 +204,9 @@ void SimpleCaptureUnbalanced::requestComplete(Request *request) if (captureCompleted()) return; + EXPECT_EQ(request->status(), Request::Status::RequestComplete) + << "Request didn't complete successfully"; + request->reuse(Request::ReuseBuffers); if (camera_->queueRequest(request)) loop_->exit(-EINVAL); @@ -232,5 +238,8 @@ void SimpleCaptureOverflow::capture() void SimpleCaptureOverflow::requestComplete([[maybe_unused]] Request *request) { + EXPECT_EQ(request->status(), Request::Status::RequestComplete) + << "Request didn't complete successfully"; + captureCompleted(); } From patchwork Fri Dec 16 12:29:39 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18032 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 6DF81C31E9 for ; Fri, 16 Dec 2022 12:30:36 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2DE19633A4; Fri, 16 Dec 2022 13:30:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1671193836; bh=/M26qPJ7k6mqHPhoPV+Oh3tT8sQFGR1h1qUekrIScDE=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=hhidQWKhYWoqnYIxe6DXLx3LzNUCxfcacSHxi3QNgaOFUmpsHykNIRVBOA5La3nRP ETxjxLHpbTorgPiCl96OCA3jYEVK5aRaxDJvGFqDU49JaO2qNc0RgKRRk5ERfrPJem mbG5vRVLq763QgwaX/tdUsrI33QqHwzJl2LpYlcJ9xq0+WSbBYEQPFcoRTITpEc512 t+mE1sBRq3BQaHKW6NM+VXqLpXoZu8rwkA3zOZAadV1qxTV+zOkQ10pSkF8L350bAP CDdfd6/H2IPMueihgSkCe4vhMonHOkjC7OXLgsKbpB1OM9rMagN+/sSWJ/NjbBIw4A 17795kNce5hWA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D93BA633AF for ; Fri, 16 Dec 2022 13:30:33 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Yba/TGYu"; dkim-atps=neutral Received: from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp [175.177.42.159]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 77718A31; Fri, 16 Dec 2022 13:30:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1671193833; bh=/M26qPJ7k6mqHPhoPV+Oh3tT8sQFGR1h1qUekrIScDE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Yba/TGYuM8fbKgH7JlFFdK9OMv9tpzHF0sFSveucdU5yTB9x+WJFHK/hym3dUbJ7U SEABzLvsTkfnBEPXDFbDs2lJDJ1QerjX6TWikuB1dVtIva2xkdSkzPT8WKSiDHp5fX NNTMKA54KfXApJp/mvxk62IEiSM6MwXYGf3uZd6c= To: libcamera-devel@lists.libcamera.org Date: Fri, 16 Dec 2022 21:29:39 +0900 Message-Id: <20221216122939.256534-19-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221216122939.256534-1-paul.elder@ideasonboard.com> References: <20221216122939.256534-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v9 18/18] lc-compliance: Add test to ensure MinimumRequests is valid X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Add a test in lc-compliance to check that the MinimumRequests property is set and valid, that is, greater than 0. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Paul Elder Signed-off-by: Paul Elder Reviewed-by: Umang Jain --- Changes in v9: - rebased Changes in v8: - Moved RequiredProperties test to property_test.cpp - Moved CameraTests to new test_base.{cpp,h} files Changes in v7: - New --- src/apps/lc-compliance/meson.build | 1 + src/apps/lc-compliance/property_test.cpp | 24 ++++++++++++++++++++++++ src/apps/lc-compliance/test_base.cpp | 10 ++++++++++ src/apps/lc-compliance/test_base.h | 7 +++++++ 4 files changed, 42 insertions(+) create mode 100644 src/apps/lc-compliance/property_test.cpp diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build index fae290bb..a10a3cc2 100644 --- a/src/apps/lc-compliance/meson.build +++ b/src/apps/lc-compliance/meson.build @@ -14,6 +14,7 @@ lc_compliance_sources = files([ 'capture_test.cpp', 'environment.cpp', 'main.cpp', + 'property_test.cpp', 'simple_capture.cpp', 'test_base.cpp', ]) diff --git a/src/apps/lc-compliance/property_test.cpp b/src/apps/lc-compliance/property_test.cpp new file mode 100644 index 00000000..6a0951b2 --- /dev/null +++ b/src/apps/lc-compliance/property_test.cpp @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2021, Collabora Ltd. + * + * property_test.cpp - Test camera properties + */ + +#include + +#include + +#include "test_base.h" + +using namespace libcamera; + +TEST_F(CameraTests, RequiredProperties) +{ + const ControlList &properties = camera_->properties(); + + using namespace properties; + + EXPECT_GT(properties.get(MinimumRequests), 0) + << "Camera should have a positive value for MinimumRequests property"; +} diff --git a/src/apps/lc-compliance/test_base.cpp b/src/apps/lc-compliance/test_base.cpp index c9957b9e..a5e64c8b 100644 --- a/src/apps/lc-compliance/test_base.cpp +++ b/src/apps/lc-compliance/test_base.cpp @@ -26,3 +26,13 @@ void CameraHolder::releaseCamera() camera_->release(); camera_.reset(); } + +void CameraTests::SetUp() +{ + acquireCamera(); +} + +void CameraTests::TearDown() +{ + releaseCamera(); +} diff --git a/src/apps/lc-compliance/test_base.h b/src/apps/lc-compliance/test_base.h index 52347749..8125e1c6 100644 --- a/src/apps/lc-compliance/test_base.h +++ b/src/apps/lc-compliance/test_base.h @@ -21,4 +21,11 @@ protected: std::shared_ptr camera_; }; +class CameraTests : public ::testing::Test, public CameraHolder +{ +protected: + void SetUp() override; + void TearDown() override; +}; + #endif /* __LC_COMPLIANCE_TEST_BASE_H__ */