From patchwork Wed Dec 28 22:29:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18057 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 F0CA5C3220 for ; Wed, 28 Dec 2022 22:30:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A3983625CF; Wed, 28 Dec 2022 23:30:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266621; bh=6/8c2/5xyoWJaUWWdX2bCzUsfpvBhsrBcR1Ct107KEU=; 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=fglybPJBVz0oInFQVIwdhX9D2hjr55S/HzzwIT6J/mDEHXdLHOvkeDhveHZGjZq6p 8gthTVhB4zgnSQIkSQIe0abymLc3P0Qz7s3WV/2ddsCdhVq9LGHbWcxBxGzP7ROObL Wv7Grc+5PUMxu9nMKLhsFoRj+ZY4yuxpJ2YZss+Qe0k02x2PdQEQDFdyf2O9dvsahS BFQsrABfAEinZOfmROJzoaJt/Xo5u2Tqbg3SQTQiIZdEjgbOrb6tREpZGcmnbDnVKb 1CVb8Cu2V3ZYGErl6MrX6uslKaTpM0FWCx+7RDXZl9g5hU8XzF70/nLVEeuJalZgo8 U1EK7lWT1vimA== 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 821C361F13 for ; Wed, 28 Dec 2022 23: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="uKog8EEN"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5E09D25B; Wed, 28 Dec 2022 23:30:16 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266617; bh=6/8c2/5xyoWJaUWWdX2bCzUsfpvBhsrBcR1Ct107KEU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uKog8EENgOEmmfnuS78CLRzX6qeZ8iplyYpSo/12nMbQ8E0Lo1M/JDPykMwpUF6ge WQJtVF6OjOmK7vpBjLCCJ+IaqeiC9R6UtHfk+Z6EUeZeYoAyy1htmHWIUrbS4fA5go e3t5KjIFOvW9AMNHOx7LrpdlGSXy7rlnFEDvTEWE= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:45 -0600 Message-Id: <20221228223003.2265712-2-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 01/19] 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 --- No change in v10 New in v9 --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index c27b3ef9..5bdb4d25 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -23,11 +23,12 @@ #include #include #include +#include +#include + #include #include #include -#include -#include #include "libcamera/internal/camera.h" #include "libcamera/internal/camera_sensor.h" From patchwork Wed Dec 28 22:29:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18058 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 65213C3220 for ; Wed, 28 Dec 2022 22:30:23 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 79905625DA; Wed, 28 Dec 2022 23:30:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266622; bh=mdNLrM37PI0D7ObVE2l8aYpp1/ZKGb0nAzRZE/FtT/U=; 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=Rl6+nQgd2b9z240CPA2qrG9XJJ8Of81S0jNUJeYuUfQ3hM4CNxTd7lNn6Ogjw+8cq djb+m29fEP5l5XvSGvW1iFFOwM+amfHr9/wh6P/n9QtAq9Xtl/9/XBlNGrOb9pUsWp oDaVX7XD4Ikhocw7WsFEmB0KF3jJCzXbYS39AZwcsdQcyUqt4cFZPySvM8M21HCtHq IGgYcdHw5b7PHt4aWul3MLK9xdcTaIxudysy8utBA/7/dV25fxP8zb+jHiZAljyjLl 1qhXp62qTXCR5v2HhrSCM3LTO7EyaKglu8BVxPovyh+tjrJhqHA56T5BlLYyEhoKvU R/kbEsoh5zH2w== 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 E69F661F13 for ; Wed, 28 Dec 2022 23:30:18 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="wpbj1XKc"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7649F109; Wed, 28 Dec 2022 23:30:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266618; bh=mdNLrM37PI0D7ObVE2l8aYpp1/ZKGb0nAzRZE/FtT/U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=wpbj1XKc1XPnCDbfVo1NDPeWsyAC535yxfBpjM/+XOTF0QokjSoPY2sqUtDtLEBMG DZF+EaGiqb4VRExwCKFrSI8O10SOnT7PnqarmB8YuRbHvNnst/Z4eX9Rdf3btDZ3yP Ll1UFRY6Zu1/ICVcRolQ8OTCNYaYwKE8AylHXC/Y= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:46 -0600 Message-Id: <20221228223003.2265712-3-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 02/19] 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 v10: - ipu3: add a constant to populate MinimumRequests, as we'll also use it elsewhere - update pipeline handler guide to set vivid' MinimumRequests to 2 - expand the MinimumRequests property description to include a line about startup - add isi 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 | 20 +++++++--- src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 2 + src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++ .../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 | 22 ++++++++++ 9 files changed, 87 insertions(+), 9 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index e1930fdf..bfa87faa 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -658,19 +658,29 @@ 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). Therefore we will set our ``MinimumRequests`` to two. +Append the following line to ``init()``: + +.. code-block:: cpp + + properties_.set(properties::MinimumRequests, 2); 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/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index 0c67e35d..9e1063d0 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "libcamera/internal/bayer_format.h" @@ -170,6 +171,7 @@ int ISICameraData::init() return ret; properties_ = sensor_->properties(); + properties_.set(properties::MinimumRequests, 2); return 0; } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index e4d79ea4..dd7df136 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -169,6 +169,8 @@ private: MediaDevice *imguMediaDev_; std::vector ipaBuffers_; + + static constexpr unsigned int kMinimumRequests = 3; }; IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data) @@ -1126,6 +1128,8 @@ int PipelineHandlerIPU3::registerCameras() /* Initialize the camera properties. */ data->properties_ = cio2->sensor()->properties(); + data->properties_.set(properties::MinimumRequests, kMinimumRequests); + 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 5bdb4d25..76f59f2e 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 @@ -1103,6 +1104,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..cc0decdc 100644 --- a/src/libcamera/property_ids.yaml +++ b/src/libcamera/property_ids.yaml @@ -690,6 +690,28 @@ 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 is also the minimum number of + requests that must be queued before capture starts. + + 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 Wed Dec 28 22:29:47 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18059 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 72EF2C3226 for ; Wed, 28 Dec 2022 22:30:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 37424625D4; Wed, 28 Dec 2022 23:30:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266623; bh=U+/lU1UnnktP+kK6HCvIaqzQLxWxQ+b6CSm+Q/Np/1A=; 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=UX7vx873XTtPNrvpPixiR628LG4fiQlJFUr7WF5C/YpyrXK9rs/IxiSsdR2CKvgGU VvQ67jO70R1UmuA7qlXKecAreS+0RKzz+Lc9Or0Qa65VFo3xfy1B3BjdpqhZIVwBxG AQKW0DEwLkISRag4Qu5Kzd1ysuemnHMxL3ymDzZA6rhDkxvNSo9EhcV0VtAWq8mY56 nAYMCBEwLRrm9i5jAmbB7lc5wXTcFWCNBGwPctbol6+JhjFoILk4WHxe5yF8wp2hvU UYixOnn0WBoXjx8WNRTmhm2md/N6ZCld/BoVoIy6r4uu50sem1Neg96xtQZRHcrTTV SzVs1neVsZOSw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A4DBB61F13 for ; Wed, 28 Dec 2022 23:30:20 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="aZYB8qe5"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1142A25B; Wed, 28 Dec 2022 23:30:18 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266620; bh=U+/lU1UnnktP+kK6HCvIaqzQLxWxQ+b6CSm+Q/Np/1A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=aZYB8qe5pUM2YOZHnWbFPLeagxn67KKGN60hlYhHA6zPNpRiOSviLAE+qltCPGwNs ptXMy0vZgFwQx57sDvOwV+tTf1cFPnAL+aTOMAF6n3RczRHk3iWvShP0vo35+3bkc6 yp5HFq2qi5Vk1B1Nk+uVUKxqEIWlR0ESDCooaBQQ= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:47 -0600 Message-Id: <20221228223003.2265712-4-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 03/19] 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 v10: - add imx8-isi 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/imx8-isi/imx8-isi.cpp | 5 ++--- 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 ++++- 25 files changed, 92 insertions(+), 45 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 bfa87faa..c0890bc2 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; @@ -1185,7 +1185,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/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index 9e1063d0..434fbd63 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -116,7 +116,7 @@ public: generateConfiguration(Camera *camera, 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; @@ -805,10 +805,9 @@ int PipelineHandlerISI::configure(Camera *camera, CameraConfiguration *c) return 0; } -int PipelineHandlerISI::exportFrameBuffers(Camera *camera, Stream *stream, +int PipelineHandlerISI::exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count, std::vector> *buffers) { - unsigned int count = stream->configuration().bufferCount; Pipe *pipe = pipeFromStream(camera, stream); return pipe->capture->exportBuffers(count, buffers); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index dd7df136..f4cc2467 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; @@ -689,10 +689,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 76f59f2e..5994b974 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -149,7 +149,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; @@ -817,10 +817,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 Wed Dec 28 22:29:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18060 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 BED36C3220 for ; Wed, 28 Dec 2022 22:30:26 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5E3D2625E3; Wed, 28 Dec 2022 23:30:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266626; bh=CAXuEf1hBrJKDEj/grW5ODI+EwoIENTtnlZfVsD3a4o=; 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=d/A+rnaZMw9xCYLbaTBq/3GPFMZr6fHeRnw+8OSOC7S4FYCC/Z1qBvwffKskc5sXQ gszczQ0Z8rKEnNCFMJONdrbh52lsgBasbkpF4KZbXrAiYZq8SWinzP96vYOEjF+P0z 3+fBF5epXDmCjRtBZLCKG1rcv55Mn36MotFqG0/9xwXfLrd7lG9TOLCDIm7TWRKSoG +mKMYwyx2wjQOGMY8Po7t5AhJZPdnIGTwOObv2F6qzYY+yi6SLkLm403vYhvs02ZLM XUGaBrXIjPAAF8XRzxl0IKrPsz1NC5wQi+b5dH+eAa+JNREE99dz26s5yn1g/1gmc1 L5Y+zFTv+kXOQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 593BB625C8 for ; Wed, 28 Dec 2022 23: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="nFltqDLM"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D4AEA109; Wed, 28 Dec 2022 23:30:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266621; bh=CAXuEf1hBrJKDEj/grW5ODI+EwoIENTtnlZfVsD3a4o=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nFltqDLM8/7PXInfu2QC7QHfjXaKIG/p0xa3vbtSU2UETBeei/gY9bOaehrBlKZI/ 9JSjoJfHD+0JaMrFG+L1S/YPwJ4dkpFn4+dIbdsgOblnwLuCS/tcAi7M5THhsSjBq5 uB9GQM3KxSOhcaIzqTOuV4YKJnjEaj4AjnBT2Tks= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:48 -0600 Message-Id: <20221228223003.2265712-5-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 04/19] 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. There alrady exists a procedure for determining the number of buffers to reserve, so to remove reliance onf bufferCount we simply replace the one instance that it is used in, as well as remove populating it in generateConfiguration(). Signed-off-by: Nícolas F. R. A. Prado Signed-off-by: Paul Elder --- Changes in v10: - completely changed, to leverage the existing buffer count calculations - the refactoring that was in v9's rebase wasn't necessary, i think - this also makes it conflict less with [1] which ought to be coming soon [1] https://patchwork.libcamera.org/project/libcamera/list/?series=3663 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 --- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 4641c76f..15055d58 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -572,7 +572,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; @@ -593,7 +592,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol BayerFormat::Packing::CSI2); ASSERT(pixelFormat.isValid()); colorSpace = ColorSpace::Raw; - bufferCount = 2; rawCount++; break; @@ -608,7 +606,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol colorSpace = ColorSpace::Sycc; /* Return the largest sensor resolution. */ size = sensorSize; - bufferCount = 1; outCount++; break; @@ -629,7 +626,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol */ colorSpace = ColorSpace::Rec709; size = { 1920, 1080 }; - bufferCount = 4; outCount++; break; @@ -638,7 +634,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol pixelFormat = formats::ARGB8888; colorSpace = ColorSpace::Sycc; size = { 800, 600 }; - bufferCount = 4; outCount++; break; @@ -685,7 +680,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol cfg.size = size; cfg.pixelFormat = pixelFormat; cfg.colorSpace = colorSpace; - cfg.bufferCount = bufferCount; config->addConfiguration(cfg); } @@ -1433,7 +1427,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) for (Stream *s : camera->streams()) { if (isRaw(s->configuration().pixelFormat)) { - numRawBuffers = s->configuration().bufferCount; + numRawBuffers = data->unicam_[Unicam::Image].getBuffers().size(); break; } } From patchwork Wed Dec 28 22:29:49 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18061 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 0A2DEC3220 for ; Wed, 28 Dec 2022 22:30:28 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 856A0625D8; Wed, 28 Dec 2022 23:30:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266627; bh=UxTRy6b7TsWo1BB4KSjqmp/xXP73ssNrieC+W2NcEck=; 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=0lzX374wJMRdvP6C29Whoh2oILdlqg6XJ0RLZGkhqyUIIaTThfRVtEid7vsVisUeW myIFD4EFu2QzW2e+hHlBHiBfXknGg9i5/R5ipf+HnUZKH544vKkoAABK6VB+TTAghz DV6nA9PGUuk32zXtPAw53sClbiLY/6BxoLL075SM6qL/gHsunjAuGmjnXX4UOH8WIR 63c3yjmZuFiKEEBogGfA95kJgkvCgwx0wnWGgrAbNyBz94SyIYNaYn5TQKvK3MHTY7 VgUUh8sLTQngPE58DNFYVItvK4sNYaPstNHJMZebrfhxpxVFavDEIC0obo2KfyB+V8 /xxIcP+jsM9tw== 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 79328625DD for ; Wed, 28 Dec 2022 23:30:23 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="v7hlpfnS"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 32F1525B; Wed, 28 Dec 2022 23:30:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266623; bh=UxTRy6b7TsWo1BB4KSjqmp/xXP73ssNrieC+W2NcEck=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=v7hlpfnSNSbJH5z+jQKEdf0SA8eSqJNWfCm89tCIBZMvXUvxQlQIxQor94GkZNgho S0WjfvVik44SDaH6v7nq6geWiOAp1UK/Xvu6W0tDh4btXpQeEhW5ElaQ9VWWs01Oyi CKTDMDQs9eJfFQ94bwUDcK5hEO6lmafEbHm91CFE= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:49 -0600 Message-Id: <20221228223003.2265712-6-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 05/19] 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 v10: - make hardcoded internal buffer allocation counts for CIO2 and IMGU come from MinimumRequests Changes in v9: - rebase Changes in v8: - New --- src/libcamera/pipeline/ipu3/cio2.cpp | 8 +++--- src/libcamera/pipeline/ipu3/cio2.h | 4 +-- src/libcamera/pipeline/ipu3/imgu.cpp | 13 +++++---- src/libcamera/pipeline/ipu3/imgu.h | 3 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 43 ++++++++++++++++++++-------- 5 files changed, 45 insertions(+), 26 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index d4e523af..e999b7ae 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; } @@ -335,13 +334,14 @@ int CIO2Device::exportBuffers(unsigned int count, return output_->exportBuffers(count, buffers); } -int CIO2Device::start() +int CIO2Device::start(unsigned int internalBufferCount, + unsigned int bufferSlotCount) { - int ret = output_->exportBuffers(kBufferCount, &buffers_); + int ret = output_->exportBuffers(internalBufferCount, &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..14372710 100644 --- a/src/libcamera/pipeline/ipu3/cio2.h +++ b/src/libcamera/pipeline/ipu3/cio2.h @@ -30,8 +30,6 @@ struct StreamConfiguration; class CIO2Device { public: - static constexpr unsigned int kBufferCount = 4; - CIO2Device(); std::vector formats() const; @@ -48,7 +46,7 @@ public: V4L2SubdeviceFormat getSensorFormat(const std::vector &mbusCodes, const Size &size) const; - int start(); + int start(unsigned int internalBufferCount, 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..e5ec2cba 100644 --- a/src/libcamera/pipeline/ipu3/imgu.cpp +++ b/src/libcamera/pipeline/ipu3/imgu.cpp @@ -576,22 +576,23 @@ 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 internalBufferCount, + 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(internalBufferCount, ¶mBuffers_); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU param buffers"; goto error; } - ret = stat_->allocateBuffers(bufferCount, &statBuffers_); + ret = stat_->allocateBuffers(internalBufferCount, &statBuffers_); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; goto error; @@ -602,13 +603,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..668fa427 100644 --- a/src/libcamera/pipeline/ipu3/imgu.h +++ b/src/libcamera/pipeline/ipu3/imgu.h @@ -84,7 +84,8 @@ public: outputFormat); } - int allocateBuffers(unsigned int bufferCount); + int allocateBuffers(unsigned int internalBufferCount, + unsigned int bufferSlotCount); void freeBuffers(); int start(); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index f4cc2467..790b2665 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_; @@ -171,6 +171,7 @@ private: std::vector ipaBuffers_; static constexpr unsigned int kMinimumRequests = 3; + static constexpr unsigned int kIPU3BufferSlotCount = 16; }; IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data) @@ -712,20 +713,25 @@ 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); + /* + * 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. + */ + ret = imgu->allocateBuffers(kMinimumRequests + 1, bufferSlotCount); if (ret < 0) return ret; @@ -783,7 +789,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; @@ -796,8 +802,21 @@ 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. + * + * 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. */ - ret = cio2->start(); + ret = cio2->start(kMinimumRequests + 1, kIPU3BufferSlotCount); if (ret) goto error; From patchwork Wed Dec 28 22:29:50 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18062 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 23DD4C3226 for ; Wed, 28 Dec 2022 22:30:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C5E18625DF; Wed, 28 Dec 2022 23:30:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266628; bh=fm5mBlWrh1ps5b66vFDjfPs9kLP9NmvtJw6JPr2+r0g=; 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=DYCj6q3Pa9uKnWylWp70yEnDKXNeHzfmaQd4YsQiB7KqalOMeg2y5TxVJA45FD7pj VgllIw5CCb0aqdH0hiaUpM0kqkMERzutO8uRZ78cU+Qiqf2wFvdeRKzW/Bz/R+yivb 1WkpwZ328m+dsAnntvn3Y8AiDYmT51V7kCiSxZVmzZ8jLnfGk3xu8cRVuRosInhF5W eSZ/SloSNdd4VPJESM89NarlnHfO3LaBhzsk/TJdpK38JnsaGJUzbSqwU4oZDPGMZP KC4WChBq1uR6TcxuUm3QRDeEYKZQoVJOfSbTn/e3CchXvbpbfvw9ehzlXcFUqgFqGK gFrQ+2uhVBefA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 98A44625D7 for ; Wed, 28 Dec 2022 23: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="Yhxe+qvk"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9B674109; Wed, 28 Dec 2022 23:30:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266624; bh=fm5mBlWrh1ps5b66vFDjfPs9kLP9NmvtJw6JPr2+r0g=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Yhxe+qvkjFCuQK/UnfSk88dCPB9medHtXtZ7grNoUp9qJAcNHmPEQw+/65f8QRwmW MPEpYRofSufyCD5a4TDOgjex7/WTH1N2Y4vWNSKt8oLqq+1N2SKTWvXTBrKUEfLKIF 688OJw9haYrjTIwvbLnuFHbK9F674JqoROgLOuEw= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:50 -0600 Message-Id: <20221228223003.2265712-7-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 06/19] 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 v10: - rename internalBufferCount and bufferSlotCount to inputBufferCount and outputBufferCount, respectively - remove outputBufferCount from Converter::start() parameter - instead, each Converter will decide on their own (how do we like this?) Changes in v9: - rebased Changes in v8: - New --- include/libcamera/internal/converter.h | 2 +- .../internal/converter/converter_v4l2_m2m.h | 4 ++-- src/libcamera/converter.cpp | 2 ++ .../converter/converter_v4l2_m2m.cpp | 12 ++++++----- src/libcamera/pipeline/simple/simple.cpp | 21 ++++++++++++++----- 5 files changed, 28 insertions(+), 13 deletions(-) diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h index 834ec5ab..e8a5aeca 100644 --- a/include/libcamera/internal/converter.h +++ b/include/libcamera/internal/converter.h @@ -49,7 +49,7 @@ public: virtual int exportBuffers(unsigned int output, unsigned int count, std::vector> *buffers) = 0; - virtual int start() = 0; + virtual int start(unsigned int inputBufferCount) = 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..8aedb6df 100644 --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h @@ -50,7 +50,7 @@ public: int exportBuffers(unsigned int ouput, unsigned int count, std::vector> *buffers); - int start(); + int start(unsigned int inputBufferCount); void stop(); int queueBuffers(FrameBuffer *input, @@ -69,7 +69,7 @@ private: int exportBuffers(unsigned int count, std::vector> *buffers); - int start(); + int start(unsigned int inputBufferCount); void stop(); int queueBuffers(FrameBuffer *input, FrameBuffer *output); diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp index 3de39cff..2413424b 100644 --- a/src/libcamera/converter.cpp +++ b/src/libcamera/converter.cpp @@ -127,6 +127,8 @@ Converter::~Converter() /** * \fn Converter::start() * \brief Start the converter streaming operation + * \param[in] inputBufferCount Number of input buffers that will be used to + * move video capture device frames into the converter. * \return 0 on success or a negative error code otherwise */ diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp index 2a4d1d99..261c3329 100644 --- a/src/libcamera/converter/converter_v4l2_m2m.cpp +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp @@ -107,13 +107,15 @@ int V4L2M2MConverter::Stream::exportBuffers(unsigned int count, return m2m_->capture()->exportBuffers(count, buffers); } -int V4L2M2MConverter::Stream::start() +int V4L2M2MConverter::Stream::start(unsigned int inputBufferCount) { - int ret = m2m_->output()->importBuffers(inputBufferCount_); + static constexpr unsigned int kOutputBufferCount = 16; + + int ret = m2m_->output()->importBuffers(inputBufferCount); if (ret < 0) return ret; - ret = m2m_->capture()->importBuffers(outputBufferCount_); + ret = m2m_->capture()->importBuffers(kOutputBufferCount); if (ret < 0) { stop(); return ret; @@ -373,12 +375,12 @@ int V4L2M2MConverter::exportBuffers(unsigned int output, unsigned int count, /** * \copydoc libcamera::Converter::start */ -int V4L2M2MConverter::start() +int V4L2M2MConverter::start(unsigned int inputBufferCount) { int ret; for (Stream &stream : streams_) { - ret = stream.start(); + ret = stream.start(inputBufferCount); if (ret < 0) { stop(); return ret; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 6b7c6d5c..64f341e1 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,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL } if (data->useConverter_) { - ret = data->converter_->start(); + ret = data->converter_->start(internalBufferCount); if (ret < 0) { stop(camera); return ret; From patchwork Wed Dec 28 22:29:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18063 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 23D08C3220 for ; Wed, 28 Dec 2022 22:30:30 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7281E625C8; Wed, 28 Dec 2022 23:30:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266629; bh=n57lT7DNtJle+qcAg9+E5WWWv74y0tC+ehajux5sFhA=; 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=xUbPY7iRePqJCRTQx7IVe23Wdo/iqSW1hncOHigt0TDmoXCJugpm90FRYhGoD0BqK 7YiBj5Q/VbR8ZcJm3q56ienJyAim62CGzbqPgM/3mPWHL6OpNOoDAiRa2WPRyLMoIY 0wJE8emqvp+FXu7CY7dxWDQ1IPiWOGOEZXvFv5i5vbbLkW8W6b4cl1TOTYbU99Gldr A00Q6R+3j75OF4nOpm7uVAEStuff7d/zdaHhiMZlbjyi0xaeFrivmwv88WQeKR4rc6 Zo/ZD90h9PM9J8kcb7Nkha9/EZH9zTLLIojnPEIQkDXic/yYio99a0IPGen7HQtK39 +o68gAAodRXTA== 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 F2C8E625DD for ; Wed, 28 Dec 2022 23:30:25 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="hcdvECQX"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BAFD76D0; Wed, 28 Dec 2022 23:30:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266625; bh=n57lT7DNtJle+qcAg9+E5WWWv74y0tC+ehajux5sFhA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hcdvECQXLKrYs5Njvm+Z9zr8B4HdzauO9bvV5JVSfNY9l8rOzNYALEP01BUDZme+b LMD1oXBW7PBiq/m7xki8to+mnbaTcR3FGOQaFsc7RPb2Owcm08AHWyLfjY5mFNDjCh QE91A8cUhIPvLaEuontL3o4PInMKth+GHwJz+YEA= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:51 -0600 Message-Id: <20221228223003.2265712-8-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 07/19] 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 5994b974..27d70c6e 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -200,6 +200,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) @@ -836,17 +846,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 Wed Dec 28 22:29:52 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18064 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 D09A4C3220 for ; Wed, 28 Dec 2022 22:30:32 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4CA5F625EE; Wed, 28 Dec 2022 23:30:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266632; bh=A/v6s9S+6QPq7TXJyUJoD86lQzpWi2AxeuEf8n4vCv4=; 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=Yn5cmdmSYIej/tn0rUSxI3vTTITD4VXNfKOx3z5NDRw9eD+wDHCggJXtP6oi6fule Ud1QWN/kImg989r1mRgJDljqNgLrhiS70JH7mXBs/anKS9sMafqjrpVxRMGjHmuBky yDBxdSPowSKs7VgLF8TvLyguUQLssaoyIvaul0I5AAKVsPaaVxxfxeuW7nYflCXu0e lR6EXLnXGMywRlhEAS934Eo/50MekOmdEDfvKgeNCtg11nBuwHxtN8oPb6vtjKONlT vuHWOSSeWU+bzmaKFccv9LR/8zm53hL8zGPknGNmOws/FMap85NGrS+F8uUU+r8Pnj Kz9WEmmhP2Vdw== 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 5C6C7625DE for ; Wed, 28 Dec 2022 23:30:27 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Ik78jgyA"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0F492109; Wed, 28 Dec 2022 23:30:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266627; bh=A/v6s9S+6QPq7TXJyUJoD86lQzpWi2AxeuEf8n4vCv4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Ik78jgyAUuvDmycuCNKYeSPTvIocRNMjhM/YuWsDwQgG6yT94kLdDLzLVBxSrTrM3 Do8mf0pbmiDaf4EM1foGvJCAYbYVxX+I8NNq6B2dEEAJDn2DHeJAXyL0nPdeJz+h44 6huatWEt4K+wSyLPGFR+suX80eIOXMIyTn8yXyrY= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:52 -0600 Message-Id: <20221228223003.2265712-9-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 08/19] 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 Wed Dec 28 22:29:53 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18065 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 85B02C3220 for ; Wed, 28 Dec 2022 22:30:35 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 42A36625F1; Wed, 28 Dec 2022 23:30:35 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266635; bh=KwCPVGMhnKnk3gOlN8lsIY4y9ZgU1B0i6eBSxJaRGbg=; 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=dKabyulCys+M+J4JxDAXMTOy9lXMIwitQBS6JpP9SPpLF8hA+qNK9sk2LJecA4xe8 q1ADHjRpWvzImu52fthjIdHLWBmyTvzr0iQ15Sm+BfuoXZUEgH0tEOLt73PGLjjczQ m4fP6jzANDUE6qoNPYEIVUl6YjUKVAaggtWkqYQMNx/c3R5fseYbCqOEefNA1RbuI8 5m8dek/8+QSfJ0CDkRMv50ScmSymd83JIvBecwm4ouYqFKl+nUwmvcYXdQpZicECRC NnQI27yLxv2FCShNfcG1RaC4R6hrQwKIZI69B8bmajVFAjX0yOQ1uaJrGvW4YXboZf MvBMmR+3IYeEw== 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 3FF66625E2 for ; Wed, 28 Dec 2022 23: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="WWm0h8H0"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6C0926D0; Wed, 28 Dec 2022 23:30:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266628; bh=KwCPVGMhnKnk3gOlN8lsIY4y9ZgU1B0i6eBSxJaRGbg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WWm0h8H0ntMl8qRegeQPzMC0UznKS0Zz9ghdEUFvhOOrUpEdsJsRxnZZNhDzKbWcW z+P/TAZDDfdtvU59rH7cSlULW31ieG6fqTyFCfLhqsx8qfDLdo3vTCMRVnFrJ+a7O+ 4t0jEuCI9Wj7NEFFHSyvfIgY338tHWPRh+0dVDYY= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:53 -0600 Message-Id: <20221228223003.2265712-10-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 09/19] libcamera: pipeline: imx8-isi: 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" Instead of using bufferCount as the number of V4L2 buffer slots to reserve in the isi pipeline handler, 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. Signed-off-by: Paul Elder --- New in v10 --- src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index 434fbd63..73eeaf0e 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -148,6 +148,8 @@ private: std::unique_ptr crossbar_; std::vector pipes_; + + static constexpr unsigned int kBufferSlotCount = 16; }; /* ----------------------------------------------------------------------------- @@ -820,9 +822,8 @@ int PipelineHandlerISI::start(Camera *camera, for (const auto &stream : data->enabledStreams_) { Pipe *pipe = pipeFromStream(camera, stream); - const StreamConfiguration &config = stream->configuration(); - int ret = pipe->capture->importBuffers(config.bufferCount); + int ret = pipe->capture->importBuffers(kBufferSlotCount); if (ret) return ret; From patchwork Wed Dec 28 22:29:54 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18066 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 2F0AFC3220 for ; Wed, 28 Dec 2022 22:30:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id CBB86625FE; Wed, 28 Dec 2022 23:30:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266637; 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=U4vN2VmZEujhPe+1gjbY7R8Z0bd0vojns9l9j8BOCqLbw4fHgJmeHcxNB1iYvknjV 4bxNcKg95cjIlEof1b/GgyN/azuQ7qsHyL3q0axjQRNes3Zj9kIwAH4+4F276LUo4/ HxvrryQZl6c1tx6wJNjCCJMVZYEN7eEW7fxCwJGf/PMzvNR6IQV6nu6DJ32swRbStY Ee2Doany/rh5yPbd+ioK46RoDNHOvZmBaSeSD589YDj4pqC2ft8UortMfD9pq1jklw s9Y+Fb8ItnObI3SqZyUnl5sgeGYPMm6E3oxQ8Bktc9Yn6sUM1zFPDz3u692ytmnA1v I/g0tyJJqzS/Q== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E789625EB for ; Wed, 28 Dec 2022 23:30:29 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="eXj6PT2q"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6E121109; Wed, 28 Dec 2022 23:30:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266629; bh=CgTVHMsudzEX7Z7J8lk8mQNzHP/BX6KslEspC+7juCw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eXj6PT2qpMZr2WApM98NrqEvlZ3TxEcFd1n2rcth8yuMPfxE7Vm1VNhWqY63Bynf+ 2jE3Dtrh6ClXGSxhD0vl3L2/7b8PUGCRVg8ig8BavJZoxeFvsKgBlo1+X6ukafVePe keEKMhvp2MxPnruWy/TnoDHe7AdFaxxKOiKeodx4= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:54 -0600 Message-Id: <20221228223003.2265712-11-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 10/19] 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 Wed Dec 28 22:29:55 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18067 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 63E4EC3220 for ; Wed, 28 Dec 2022 22:30:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 063C6625F4; Wed, 28 Dec 2022 23:30:39 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266639; bh=eqR6low7l+DKLvlsUpwybkEnEc8rGsaRxOO/Ru9xt+s=; 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=K9WcqlnAL7ExlobBqPk3hAa8DM17apSQhwFKbCn6fBlyI2gxDH7nHsFuMxqIJEOsy YAJijnfxyULqt5LpJ+mK9a3A68g2pyfYwZGwLTjajPa3moZ9yZa+ErCn5vhQe3C3I9 /uzX76JLDmYohu+woEL2PkqfOBsNz8EAgRZAWbCD541g19H9di0+2A+FxIbPlFEdX/ KfbyXDVL7lV3bRE3IVTx4ccz4loxEXdVszCkuCI/Hl7ZjfBbcL6fGLnhECA37bXSv1 kmyC7sAT6gsmVbU6Abd3QWlOPvUbvhojlFONxzunlN5efzjyO2PZHlAGNYB4rcjrzp HPMWdFhBDYGfQ== 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 E3703625EC for ; Wed, 28 Dec 2022 23: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="tx7SRVvS"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BF08A6D0; Wed, 28 Dec 2022 23:30:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266630; bh=eqR6low7l+DKLvlsUpwybkEnEc8rGsaRxOO/Ru9xt+s=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=tx7SRVvS5Qbqk8VjYYSBrBNFMNQLPMmztghRJjSvN6VNk8CAl0Y63ATwfRNwj0PkW oYoKC9Wcl+ER8ynQze93fCRL/vEkTyYz2JuLICjK8z9/sFTQTigklpdlbxksAUz0eY GAMtd6s3vNEKXayFw2g1MIejWA7EyzZoRLWEaNsQ= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:55 -0600 Message-Id: <20221228223003.2265712-12-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 11/19] 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 v10: - fix compilation error (android and pycamera) - add isi 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 | 5 ++++- src/libcamera/converter/converter_v4l2_m2m.cpp | 3 --- src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 1 - src/libcamera/pipeline/ipu3/ipu3.cpp | 8 -------- 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 +++--------- src/py/libcamera/py_main.cpp | 1 - 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, 29 insertions(+), 57 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index c0890bc2..4f811f78 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -825,14 +825,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 @@ -898,8 +896,6 @@ Add the following function implementation to your file: status = Adjusted; } - cfg.bufferCount = 4; - return status; } @@ -1143,13 +1139,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 8aedb6df..1ffee7f0 100644 --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h @@ -84,9 +84,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..0abe6a43 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -14,6 +14,7 @@ #include #include +#include #include "jpeg/post_processor_jpeg.h" #include "yuv/post_processor_yuv.h" @@ -131,7 +132,9 @@ int CameraStream::configure() allocator_ = std::make_unique(cameraDevice_); mutex_ = std::make_unique(); - camera3Stream_->max_buffers = configuration().bufferCount; + unsigned int bufferCount = + cameraDevice_->camera()->properties().get(properties::MinimumRequests).value(); + 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 261c3329..41dc147a 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/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index 73eeaf0e..1ea27d14 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -690,7 +690,6 @@ PipelineHandlerISI::generateConfiguration(Camera *camera, StreamConfiguration cfg(formats); cfg.pixelFormat = pixelFormat; cfg.size = size; - cfg.bufferCount = 4; config->addConfiguration(cfg); } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 790b2665..17a381e9 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); @@ -327,7 +326,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_)); @@ -371,7 +369,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); @@ -440,7 +437,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; @@ -460,7 +456,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; @@ -470,7 +465,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); @@ -489,7 +483,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; @@ -505,7 +498,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/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 64f341e1..e5f97e9a 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/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp index d14e18e2..5de5f26c 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -241,7 +241,6 @@ PYBIND11_MODULE(_libcamera, m) .def_readwrite("pixel_format", &StreamConfiguration::pixelFormat) .def_readwrite("stride", &StreamConfiguration::stride) .def_readwrite("frame_size", &StreamConfiguration::frameSize) - .def_readwrite("buffer_count", &StreamConfiguration::bufferCount) .def_property_readonly("formats", &StreamConfiguration::formats, py::return_value_policy::reference_internal) .def_readwrite("color_space", &StreamConfiguration::colorSpace); 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 Wed Dec 28 22:29:56 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18068 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 EE030C3220 for ; Wed, 28 Dec 2022 22:30:40 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 68C16625F5; Wed, 28 Dec 2022 23:30:40 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266640; bh=eFujqOl0f5mY2JqTnwJ2EXmIUexGB2qWJM0VbIBOWro=; 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=s5JgVnJWxK+rHayMvNQziuI15nHBvB+upa6qmBc64IJuV4muypPQbEHUjHAIPuUfL N/wKKw04cOUR3WhY27Xwo4+07CF5rCttlMlnI9apkiOE3YA9NZJdvTS4/wMNBqnV+F Ve7lRzxZK6uSk9dYB+fx81gCgY5/CmMdiNp/jECkzqnQiF5U3jF/EYGAZjP8o+b7sZ jBUXRe79JvYXYfkk6nUM3eX37RG0APnJVgO8bFoC6PZn/aM5CjTe6ymtWem/aZ6GeA w2EUb3XALs9OSE31txof/mdUovTj51YpxFPhSULDAMgbJKBdTTHu3ibNUAnjGzCwhd OftYdbn3yl4fA== 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 45F9B625DE for ; Wed, 28 Dec 2022 23: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="orJhXZRv"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1AF9425B; Wed, 28 Dec 2022 23:30:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266632; bh=eFujqOl0f5mY2JqTnwJ2EXmIUexGB2qWJM0VbIBOWro=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=orJhXZRvtWB+YpycWETBcYvW00Ab/obsLGJgRZREjwNcSCHn4JzYgSl2iBESIGCqm LbKWgFLYyyhBi1kHmJPvgDHBWkv40GCEkcGYyUvJfpWIZS+rngcQPzVp1w3lQkpYG4 ocooJ9aKKzWpENMdFSuFB96Ihqs7RolyCbkcgBwg= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:56 -0600 Message-Id: <20221228223003.2265712-13-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 12/19] 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 --- No changes in v10 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 Wed Dec 28 22:29:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18069 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 AAF07C3226 for ; Wed, 28 Dec 2022 22:30:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 67222625EC; Wed, 28 Dec 2022 23:30:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266641; 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=lgUl7EbiMbbEMsdsK4Foi/2ql82qGLcMngwkNpdr4E222aAAz+wzl9XvpG22fI4aq Rm94lx1PbLt7QwzzjBSpy4j/4i1qeX5KUc4XjTvw7kpyVJxA7J/0qKMGGjfb0LBBXn le1tlx3J6J2AADKcCRh8BceNy8QYDYTlW1X7akhOSK5OOIJrCywbqrhCFxmFvGByy5 tVjiSAvvLho/6kw7dE8ihR6BNwMp1YxBJatEk20rK59KkV9IDSz2EmPk0c01Tyl2Hx eKR7RYQdiYVAzXMitM4KePvPNuh4ufwA+KUL06f0i9tav8xdKyJ5hUDc62JpVJtj1e umuDRQ8k8dU7g== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 907CD625ED for ; Wed, 28 Dec 2022 23: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="PbO5Vh58"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6BCE2109; Wed, 28 Dec 2022 23:30:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266633; bh=0Ty1h03QfLcxojEN+CbLhHIvt/SjBcyEsZ0W/uCgrXs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=PbO5Vh58kutGGrUTOQsGgtwrExIlDT+8RXva8TRwfan4fhorojLk6MBNMHCwITMPE NNpgI6WY2SbLHad7omrTCYKMbY845MqE47dLRugU3cNttFq1ptA+0fv+Mwbhn/MXoT aazdez6FqHIQbfWN11+cfVnbhFQWcYQTvkA2wGWc= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:57 -0600 Message-Id: <20221228223003.2265712-14-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 13/19] 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 Wed Dec 28 22:29:58 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18070 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 2F235C3220 for ; Wed, 28 Dec 2022 22:30:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id CE87462606; Wed, 28 Dec 2022 23:30:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266641; 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=W9YL0E9JTFZ0t5Uodbl/fN+qrXOI82dnLzIk5gFYgGXSwEL0WRFpHXf+HJ4gUuiwP S7NcmIf4cHbn8REpPi1ROKqmo1MCtMBOsWYn60oXx4IqKkKLFCqCBuJ3Me4wErBp1+ kJbI2NrZIc/et9ZYICf9Cxp4qyzYfTvoabMJFTvWpqJkzxW0pplJ0jSXPilOEZoeaO TeRFmecAsSoE0gI8dgD2jJAJVbyTbxAk1/DXRnNcA50ioKZyH1kqPDOaljUjqQYVpX 1LXb6Yxct9jsRCwSN+FE9QqB+s65lYrH8m/ILoQHcEV6wMsk6BAINhDN700U7uMjTf KwxdSZ188sNcQ== 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 CD14A625ED for ; Wed, 28 Dec 2022 23:30:34 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="XbVOl7Gm"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BCDB16D0; Wed, 28 Dec 2022 23:30:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266634; bh=VPXe7AwBiDzM/pQmANFAF8RGe93JNSnljJB4v1hFYsg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XbVOl7GmIH+rLzcz2b5ZNz1r5J9aRKcLjIwC24Z0Z5X99kK31HNlxewGTM6llnZP8 Nxdjn/0541nSXQEWWKzGBzdzz1T1uVZYrDF2xXyRxZ81bWmpHZvLzJReKWhQ25gXKQ a6WdvpT5RdjEOC8QBpPZSt5soSNXGL8uoRpRWKZM= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:58 -0600 Message-Id: <20221228223003.2265712-15-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 14/19] 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 Wed Dec 28 22:29:59 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18071 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 A8474C3291 for ; Wed, 28 Dec 2022 22:30:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 54E7562608; Wed, 28 Dec 2022 23:30:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266642; 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=3Y1rel1ZyxfE9fNwtNW/HB1aCvcmJcsi3u5XrcyFUGK0NnEEFTCs2SbUjnRoUzKbD Cv/4UvkXueonkeCX6A7IDzOehaWhxZhWUQ/U0tsQv5sD64WU4VTPXN/p9DgiGLa36N iyQ0kpgeveMsJt+oqlY1PDBP+knN2sgd38mbYm+fnNZQ+7neazztDy6BxgFj76i9gy //cmGDajDaa6BRv36yipEXFyaUiigo6BODS+4VL2YZZZjpSZSROkhPuJcC9ZZkDa9g 0CDHmYk51h1tiXpPDvQumrt7cSf5zM8HQA6GgKMlfGWcpDNPSoMT7DuV8Ol+6OCruw f62GhG+GZ2Xww== 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 6CF61625EF for ; Wed, 28 Dec 2022 23:30:36 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="nT3Vkg6s"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 02909109; Wed, 28 Dec 2022 23:30:34 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266636; bh=WoN22LIegFrp62olPQq8QlNLCc0xK4dHM9kOmMmOYSw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nT3Vkg6sZonncODwm62E9ouZTy+HT36k6xTIPajG8QRNUzieqHzzLM4yWSvpKyS9F yneCJQ8htGCk7L1anSlGdwQf2DVEDXVvaZG3mi0XrckxlFtkf+slwuQDUQoR/vXUZG J/2kwVFCu9pqRT67mCwWKnI420Vjx57b4+Krdpgo= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:59 -0600 Message-Id: <20221228223003.2265712-16-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 15/19] 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 Wed Dec 28 22:30:00 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18072 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 156AEC3292 for ; Wed, 28 Dec 2022 22:30:43 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B91A5625FD; Wed, 28 Dec 2022 23:30:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266642; 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=S95kk6NCKkontw8LPSFUFbDCzgnACYV9HYIRmaefTHbiZnuvZX4jO47yz9o1//3+w 9EcDr6DOBKGzW55d+ZtLfRebywfHhviGHo8pUVr66VuKT86Y8JP0bNsDj0lqZd9uUS JoRzCi7950XvvEWxLaCrtS9i3Btq1VhknMJB1e85y31KhNqwzxxxUSY5kx7fNjY4ua XfRamfACo9dzTm1pGHY/+pNGpPHDpGzIuF5PmKr+7AHcEsdWbaNIIWgb5oQk4oPRgM 5dtzjnkHlGzxJlgojm/fmMVx82ljcWZUc99Y17Cwi599xq2dv/VhVrxpozXp1SSjjx M0sE0ciS+kidg== 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 BCAB6625F6 for ; Wed, 28 Dec 2022 23:30:37 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="stEEz1x4"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9C63225B; Wed, 28 Dec 2022 23:30:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266637; bh=01pZHYVZngDe+B2HzCbcASuxSqUGF6xdmRpG+MH2Sh8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=stEEz1x4BtO2DLVbnQRPPYZYR72m00Yg0G4ViwNPT7v5Hi5ps9Ob9aChXNVIprg3O PUl0okS5bI/K72tRiiHIQTjsGqXzfB6s1RV9gRDhZuhqd3SwJTs886gbVh5VMBHtt5 8Yfg6n9yYn0Jci6SPmqxyJhJXEXxmbeXs9DUPNP4= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:30:00 -0600 Message-Id: <20221228223003.2265712-17-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 16/19] 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 Wed Dec 28 22:30:01 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18073 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 821D3C3226 for ; Wed, 28 Dec 2022 22:30:43 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3DB5D625E4; Wed, 28 Dec 2022 23:30:43 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266643; 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=0kV8fxGgcduh+feHaroXaQiXgWKBqRVFCkNwQOnABx0JHwkW2K+l1FVGwaa/m/Orb d0q8p3HLQRmfeIuSHhVyY01qEQXkJHPHU5UHvfOb0+yAmU50XrBoU3K2olhoqcC2ow FIGicdUmxA7tZ5RICZWSYURPobODZy8bsfC6pD/3QdCuPSPDuW9IjJGq4ihBf/mPi6 KmTW+k6m/YLvQT0dlaqFrRTMom8sWp1sYOnY34K3YSCGQ+AaXZo8Gy4NtoZ7cU64tn sLRlnKWuzZkABV46ImWLPl/9JDEK4oEJlIUsN5wrqJkpz4A2BQjIdJ91LTalzWGNEJ uaRqhZTKf2RxQ== 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 4204B625F6 for ; Wed, 28 Dec 2022 23:30:39 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="vMKfGjWi"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id ECE3F109; Wed, 28 Dec 2022 23:30:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266638; bh=OEmstN1ZafXMAjzzMaEtkY6Zm9yHYv3lOWuomsxExj8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vMKfGjWi55kFLml7SrKkz22GgHCQBzVr34u3vuR54xSb2AJqXVlOOAvyrq7Cr5kl1 4o3ej1Vo+QQ5BWRq9iSm2UaqstBjizgQ7aczG599XyjMmsyXnwxW3PQfOPVqxY5rsg Gq/O0VfmmjMb3ANvJaw1GxWZ7at5AfNciUZ/V08M= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:30:01 -0600 Message-Id: <20221228223003.2265712-18-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 17/19] 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 Wed Dec 28 22:30:02 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18074 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 0DC08C3220 for ; Wed, 28 Dec 2022 22:30:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BA02E625EA; Wed, 28 Dec 2022 23:30:45 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266645; 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=wYfW//2iW3qnmfKJXEVk8sJGGgWh1xlVEfsQoGhU6nuA4ESbiVoDpokrgBwiJtQlJ 3CEA58On132M5lxaYz5Rj1nH+iT2JtfhOFB/F6UV3LDo4aQiw5L8za5QCtw5K90FqH IZDP3lCkvVt6hBgQig11gJCv2XmDt2oodyfl6kWrGVta9d2MzKSSSnIkfCUY+KzSqB 1cEVjWFZjDJhEUG8gzyjED8UndRaQFHaOoyLP7THPEziukHZt4MD9PBoWirY6d9fiC dSsEkbNXtEpz9y4WJNsG+cDEOefmZ03Zpw7OY7uWC538IAOxdToZY7bfpNAber4Hgl d0zql4VSLkoow== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AE9E3625DC for ; Wed, 28 Dec 2022 23:30:40 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Cjlm91pd"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 62CD16D0; Wed, 28 Dec 2022 23:30:39 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266640; bh=mf9lLD2HWERLMm1bvaPiXYY+T6Bud9wv1LwIPxTz7ag=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Cjlm91pdcPEJajJ3jO3DKWwvIHKHr5hQjGnJcKYc/FXdeGVuSdKkPXJLb9V90785F stFWFYmc+WLum0nyFP/8UK1AVVEbsD2yiFx7tbyAQ1QeXJOLIGcb30d7EXtc/URQ8b +3CUH7YbOgGKatEX9vz7AY0gunAlq0NJOX2I1elk= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:30:02 -0600 Message-Id: <20221228223003.2265712-19-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 18/19] 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 Wed Dec 28 22:30:03 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18075 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 74EE3C3291 for ; Wed, 28 Dec 2022 22:30:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 21F43625CC; Wed, 28 Dec 2022 23:30:46 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266646; bh=Qmri63OtuQVbg9dIGqC4smNcZ7MRbgtnjv3mdBBOxek=; 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=U+dU2JTWpBLkvz6ZAGGkiWSQ0pdnrH/vBuN5XUlPGovymbSwAqAwW4cWlDNdv3jOG MDp4cvnUeXzGnStmp5ZXSYsm/4sgVdWKuwFdUk/iSJtgRzFG1rsSSfeJTs3wzbDgNU 4FPkfuGLfgYA4cw/KCS67zsovD2C4jg8ITl08uNMRgq0Ele/iTpAUa5QhRTN8bbRWf B1Gexhdsx2xrP7GOmWBBYztS4Sq5wYuWeogWbcost0HNBS61MojetBX6XWmZnQAKkm 8KMfidFZ3HP0jiHKqWbHRw5prLagt5NyzbWmqMIXVepffytRU+6lTsUHc/UPQWdKFq mNBj0vI7dwPIQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5644A62609 for ; Wed, 28 Dec 2022 23:30:42 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="qmzwQy+1"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CCBB0109; Wed, 28 Dec 2022 23:30:40 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266642; bh=Qmri63OtuQVbg9dIGqC4smNcZ7MRbgtnjv3mdBBOxek=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=qmzwQy+1HwBlXf3FZQi3CHPmW/NTLE15lpSKJRTYCCulM7aKz0sBWqRUfq7vzsLmw YswQPPW8/Z8RJmBCwvGxZPEsk5uZBKi6kd4Dk+guHBxhi8jbCmIbPXzLsCB198YZwe R8QSDd0Fjq0GWd4OyBHesJohoLjMXGiMDJU2M5lo= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:30:03 -0600 Message-Id: <20221228223003.2265712-20-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 19/19] 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 --- No change in v10 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__ */