From patchwork Tue Aug 24 19:56:20 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13464 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 A95D2C3241 for ; Tue, 24 Aug 2021 19:57:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6BD8E688D2; Tue, 24 Aug 2021 21:57:22 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A2EB768892 for ; Tue, 24 Aug 2021 21:57:19 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id 4360A1F432FF From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 24 Aug 2021 16:56:20 -0300 Message-Id: <20210824195636.1110845-2-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210824195636.1110845-1-nfraprado@collabora.com> References: <20210824195636.1110845-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 01/17] 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: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- Changes in v8: - Changed the MinimumRequests property meaning to require that frames aren't dropped - Set MinimumRequests on SimplePipeline depending on device and usage of converter - Undid definition of default MinimumRequests on CameraSensor constructor - Updated pipeline-handler guides with new MinimumRequests property Changes in v7: - Renamed property from MinNumRequests to MinimumRequests - Changed MinimumRequests property's description Changes in v6: - Removed comment from Raspberrypi MinNumRequests setting - Removed an extra blank line Documentation/guides/pipeline-handler.rst | 22 ++++++++--- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 + .../pipeline/raspberrypi/raspberrypi.cpp | 2 + src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 + src/libcamera/pipeline/simple/simple.cpp | 38 +++++++++++++++++-- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 + src/libcamera/pipeline/vimc/vimc.cpp | 2 + src/libcamera/property_ids.yaml | 21 ++++++++++ 8 files changed, 83 insertions(+), 8 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 54c8e7f1f553..32e3fc518d91 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -658,19 +658,31 @@ associated with immutable values, which represent static characteristics that ca be used by applications to identify camera devices in the system. Properties can be registered by inspecting the values of V4L2 controls from the video devices and camera sensor (for example to retrieve the position and orientation of a camera) -or to express other immutable characteristics. The example pipeline handler does -not register any property, but examples are available in the libcamera code -base. +or to express other immutable characteristics. -.. TODO: Add a property example to the pipeline handler. At least the model. +A required property is ``MinimumRequests``, which indicates how many requests +need to be queued in the pipeline for capture without frame drops to be +possible. + +In our case, the vivid driver requires two buffers before it'll start streaming +(can be seen in the ``min_buffers_needed`` property for the ``vid_cap`` queue in +vivid's driver code). In order to not drop frames we should have one extra +buffer queued to the driver so that it is used as soon as the previous buffer +completes. Therefore we will set our ``MinimumRequests`` to three. Append the +following line to ``init()``: + +.. code-block:: cpp + + properties_.set(properties::MinimumRequests, 3); At this point you need to add the following includes to the top of the file for -handling controls: +handling controls and properties: .. code-block:: cpp #include #include + #include Generating a default configuration ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index a98d7effc1bb..2309609093cc 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -1090,6 +1090,8 @@ int PipelineHandlerIPU3::registerCameras() /* Initialize the camera properties. */ data->properties_ = cio2->sensor()->properties(); + data->properties_.set(properties::MinimumRequests, 3); + ret = initControls(data.get()); if (ret) continue; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index b2674ac02109..d35b9714f0c3 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1046,6 +1046,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) /* Initialize the camera properties. */ data->properties_ = data->sensor_->properties(); + data->properties_.set(properties::MinimumRequests, 3); + /* * Set a default value for the ScalerCropMaximum property to show * that we support its use, however, initialise it to zero because diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 980088628523..cc663c983b3b 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -948,6 +949,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 8ff954722fed..26c59e601a76 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -130,6 +131,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. @@ -140,9 +145,9 @@ struct SimplePipelineInfo { namespace { static const SimplePipelineInfo supportedDevices[] = { - { "imx7-csi", { { "pxp", 1 } } }, - { "qcom-camss", {} }, - { "sun6i-csi", {} }, + { "imx7-csi", 2, { { "pxp", 1 } } }, + { "qcom-camss", 1, {} }, + { "sun6i-csi", 3, {} }, }; } /* namespace */ @@ -191,6 +196,8 @@ public: std::vector> converterBuffers_; bool useConverter_; std::queue> converterQueue_; + + const SimplePipelineInfo *deviceInfo; }; class SimpleCameraConfiguration : public CameraConfiguration @@ -774,6 +781,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 converter_->configure(inputCfg, outputCfgs); } @@ -1040,6 +1070,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 973ecd5b835e..75d57300555c 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -526,6 +526,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 baeb6a7e6fa6..a0d8fd6c48c3 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 @@ -560,6 +561,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 12ecbce5eed4..9df131265b9f 100644 --- a/src/libcamera/property_ids.yaml +++ b/src/libcamera/property_ids.yaml @@ -678,6 +678,27 @@ controls: \todo Turn this property into a "maximum control value" for the ScalerCrop control once "dynamic" controls have been implemented. + - MinimumRequests: + type: int32_t + description: | + The minimum number of capture requests that the camera pipeline requires + to have queued in order to sustain capture operations without frame + drops. At this quantity, there's no guarantee that manual per-frame + controls will apply correctly. + + This property is based on the minimum number of memory buffers + needed to fill the capture pipeline composed of hardware processing + blocks plus as many buffers as needed to take into account propagation + delays and avoid dropping frames. + + \todo Should this be a per-stream property? + + \todo Extend this property to expose the different minimum buffer and + request counts (the minimum number of buffers to be able to capture at + all, the minimum number of buffers to sustain capture without frame + drop, and the minimum number of requests to ensure proper operation of + per-frame controls). + # ---------------------------------------------------------------------------- # Draft properties section From patchwork Tue Aug 24 19:56:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13466 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 BFD33C3245 for ; Tue, 24 Aug 2021 19:57:32 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0E16468911; Tue, 24 Aug 2021 21:57:28 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3989268892 for ; Tue, 24 Aug 2021 21:57:27 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id C9F481F4339D From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 24 Aug 2021 16:56:24 -0300 Message-Id: <20210824195636.1110845-6-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210824195636.1110845-1-nfraprado@collabora.com> References: <20210824195636.1110845-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 05/17] 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: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- Changes in v8: - New src/libcamera/pipeline/simple/converter.cpp | 12 ++++++----- src/libcamera/pipeline/simple/converter.h | 6 ++++-- src/libcamera/pipeline/simple/simple.cpp | 22 ++++++++++++++++----- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp index b5e34c4cd0c5..3133b3dbda07 100644 --- a/src/libcamera/pipeline/simple/converter.cpp +++ b/src/libcamera/pipeline/simple/converter.cpp @@ -101,13 +101,14 @@ int SimpleConverter::Stream::exportBuffers(unsigned int count, return m2m_->capture()->exportBuffers(count, buffers); } -int SimpleConverter::Stream::start() +int SimpleConverter::Stream::start(unsigned int internalBufferCount, + unsigned int bufferSlotCount) { - int ret = m2m_->output()->importBuffers(inputBufferCount_); + int ret = m2m_->output()->importBuffers(internalBufferCount); if (ret < 0) return ret; - ret = m2m_->capture()->importBuffers(outputBufferCount_); + ret = m2m_->capture()->importBuffers(bufferSlotCount); if (ret < 0) { stop(); return ret; @@ -331,12 +332,13 @@ int SimpleConverter::exportBuffers(unsigned int output, unsigned int count, return streams_[output].exportBuffers(count, buffers); } -int SimpleConverter::start() +int SimpleConverter::start(unsigned int internalBufferCount, + unsigned int bufferSlotCount) { int ret; for (Stream &stream : streams_) { - ret = stream.start(); + ret = stream.start(internalBufferCount, bufferSlotCount); if (ret < 0) { stop(); return ret; diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h index 276a2a291c21..deb3df0d08df 100644 --- a/src/libcamera/pipeline/simple/converter.h +++ b/src/libcamera/pipeline/simple/converter.h @@ -47,7 +47,8 @@ public: int exportBuffers(unsigned int ouput, unsigned int count, std::vector> *buffers); - int start(); + int start(unsigned int internalBufferCount, + unsigned int bufferSlotCount); void stop(); int queueBuffers(FrameBuffer *input, @@ -69,7 +70,8 @@ private: int exportBuffers(unsigned int count, std::vector> *buffers); - int start(); + int start(unsigned int internalBufferCount, + unsigned int bufferSlotCount); void stop(); int queueBuffers(FrameBuffer *input, FrameBuffer *output); diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 737815452bae..d0a658a23be8 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -253,6 +253,7 @@ protected: private: static constexpr unsigned int kNumInternalBuffers = 3; + static constexpr unsigned int kSimpleBufferSlotCount = 16; SimpleCameraData *cameraData(Camera *camera) { @@ -830,17 +831,27 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL V4L2VideoDevice *video = data->video_; int ret; + /* + * 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) return ret; @@ -852,7 +863,8 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL } if (data->useConverter_) { - ret = converter_->start(); + ret = converter_->start(internalBufferCount, + kSimpleBufferSlotCount); if (ret < 0) { stop(camera); return ret; From patchwork Tue Aug 24 19:56:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13469 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 73442C3247 for ; Tue, 24 Aug 2021 19:57:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D44A8688AC; Tue, 24 Aug 2021 21:57:32 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C1253688CA for ; Tue, 24 Aug 2021 21:57:30 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id 6127D1F43362 From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 24 Aug 2021 16:56:26 -0300 Message-Id: <20210824195636.1110845-8-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210824195636.1110845-1-nfraprado@collabora.com> References: <20210824195636.1110845-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 07/17] 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: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= 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 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 --- 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 7821cacfa883..c210cf57750f 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -91,6 +91,8 @@ private: { return static_cast(camera->_d()); } + + static constexpr unsigned int kUVCBufferSlotCount = 16; }; UVCCameraConfiguration::UVCCameraConfiguration(UVCCameraData *data) @@ -236,9 +238,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 eebdfd1a4c01..d2943f61a745 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -105,6 +105,8 @@ private: { return static_cast(camera->_d()); } + + static constexpr unsigned int kVimcBufferSlotCount = 16; }; namespace { @@ -331,9 +333,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 Tue Aug 24 19:56:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13471 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 0457CC3248 for ; Tue, 24 Aug 2021 19:57:51 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2BD37688E5; Tue, 24 Aug 2021 21:57:34 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7BE3E68892 for ; Tue, 24 Aug 2021 21:57:32 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id 1C7461F43389 From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 24 Aug 2021 16:56:27 -0300 Message-Id: <20210824195636.1110845-9-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210824195636.1110845-1-nfraprado@collabora.com> References: <20210824195636.1110845-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 08/17] 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: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- 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 | 11 +++++------ 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index d01eacfa2b84..4db171a3c0d0 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) @@ -107,15 +109,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) { @@ -146,7 +146,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) @@ -165,7 +164,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(); @@ -174,7 +175,7 @@ int V4L2Camera::allocBuffers(unsigned int count) requestPool_.push_back(std::move(request)); } - return ret; + return allocatedCount; } void V4L2Camera::freeBuffers() @@ -299,3 +300,8 @@ bool V4L2Camera::isRunning() { return isRunning_; } + +unsigned int V4L2Camera::minimumRequests() +{ + return camera_->properties().get(properties::MinimumRequests); +} diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h index a095f4e27524..c0a122906f35 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -45,8 +45,7 @@ public: std::vector completedBuffers(); int configure(StreamConfiguration *streamConfigOut, - const Size &size, const PixelFormat &pixelformat, - unsigned int bufferCount); + const Size &size, const PixelFormat &pixelformat); int validateConfiguration(const PixelFormat &pixelformat, const Size &size, StreamConfiguration *streamConfigOut); @@ -65,6 +64,8 @@ public: bool isRunning(); + unsigned int minimumRequests(); + private: void requestComplete(Request *request); diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index 7682c4bddf90..424262dd205c 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -349,8 +349,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, - PixelFormatInfo::info(v4l2Format).format, - bufferCount_); + PixelFormatInfo::info(v4l2Format).format); if (ret < 0) return -EINVAL; @@ -491,15 +490,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, - PixelFormatInfo::info(v4l2Format).format, - arg->count); + PixelFormatInfo::info(v4l2Format).format); 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) { @@ -507,6 +504,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 Tue Aug 24 19:56:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13470 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 9BADDC3249 for ; Tue, 24 Aug 2021 19:57:56 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9A1E768918; Tue, 24 Aug 2021 21:57:37 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 426116890C for ; Tue, 24 Aug 2021 21:57:34 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id DBBA11F43378 From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 24 Aug 2021 16:56:28 -0300 Message-Id: <20210824195636.1110845-10-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210824195636.1110845-1-nfraprado@collabora.com> References: <20210824195636.1110845-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 09/17] 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: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- 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 +++++++++------ include/libcamera/stream.h | 2 -- src/android/camera_stream.cpp | 2 +- src/libcamera/pipeline/ipu3/cio2.cpp | 1 - src/libcamera/pipeline/ipu3/cio2.h | 2 -- src/libcamera/pipeline/ipu3/ipu3.cpp | 8 -------- .../pipeline/raspberrypi/raspberrypi.cpp | 6 ------ src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 -- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 -- src/libcamera/pipeline/simple/converter.cpp | 3 --- src/libcamera/pipeline/simple/converter.h | 3 --- src/libcamera/pipeline/simple/simple.cpp | 6 +----- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 --- src/libcamera/pipeline/vimc/vimc.cpp | 3 --- src/libcamera/stream.cpp | 12 +++--------- test/camera/buffer_import.cpp | 10 +++++++--- test/libtest/buffer_source.cpp | 4 ++-- test/libtest/buffer_source.h | 2 +- test/v4l2_videodevice/buffer_cache.cpp | 3 +-- 19 files changed, 25 insertions(+), 64 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 2a69ef7d7461..3ee79b98c4dc 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -826,14 +826,12 @@ As well as a list of supported StreamFormats, the StreamConfiguration is also expected to provide an initialsed default configuration. This may be arbitrary, but depending on use case you may which 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 @@ -899,8 +897,6 @@ Add the following function implementation to your file: status = Adjusted; } - cfg.bufferCount = 4; - return status; } @@ -1144,13 +1140,20 @@ is performed by using the ``V4L2VideoDevice`` API, which provides an .. _FrameBuffer: http://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/stream.h b/include/libcamera/stream.h index 0c55e7164592..b25f0059f2f1 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -45,8 +45,6 @@ struct StreamConfiguration { unsigned int stride; unsigned int frameSize; - unsigned int bufferCount; - Stream *stream() const { return stream_; } void setStream(Stream *stream) { stream_ = stream; } const StreamFormats &formats() const { return formats_; } diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 29be1ac5ca4f..d9ee8842938f 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -96,7 +96,7 @@ int CameraStream::configure() buffers_.push_back(frameBuffer.get()); } - camera3Stream_->max_buffers = configuration().bufferCount; + camera3Stream_->max_buffers = bufferCount; return 0; } diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index b940a0f6d7d6..c555ef5ffd27 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -214,7 +214,6 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const cfg.size = sensorFormat.size; cfg.pixelFormat = mbusCodesToPixelFormat.at(sensorFormat.mbus_code); - cfg.bufferCount = CIO2_BUFFER_COUNT; return cfg; } diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h index ab915b6a16fa..50ccb20765d3 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 CIO2_BUFFER_COUNT = 4; - CIO2Device(); std::vector formats() const; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index cc519ae6adbe..4a60f00ecf8b 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -39,7 +39,6 @@ namespace libcamera { LOG_DEFINE_CATEGORY(IPU3) -static constexpr unsigned int IPU3_BUFFER_COUNT = 4; static constexpr unsigned int IPU3_MAX_STREAMS = 3; static const Size IMGU_OUTPUT_MIN_SIZE = { 2, 2 }; static const Size IMGU_OUTPUT_MAX_SIZE = { 4480, 34004 }; @@ -313,7 +312,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_)); @@ -357,7 +355,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() IMGU_OUTPUT_HEIGHT_ALIGN); cfg->pixelFormat = formats::NV12; - cfg->bufferCount = IPU3_BUFFER_COUNT; cfg->stride = info.stride(cfg->size.width, 0, 1); cfg->frameSize = info.frameSize(cfg->size, 1); @@ -425,7 +422,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, Size sensorResolution = data->cio2_.sensor()->resolution(); for (const StreamRole role : roles) { std::map> streamFormats; - unsigned int bufferCount; PixelFormat pixelFormat; Size size; @@ -446,7 +442,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, size.height = utils::alignDown(size.height - 1, IMGU_OUTPUT_HEIGHT_MARGIN); pixelFormat = formats::NV12; - bufferCount = IPU3_BUFFER_COUNT; streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } }; break; @@ -456,7 +451,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, 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(); @@ -475,7 +469,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, .alignedDownTo(IMGU_OUTPUT_WIDTH_ALIGN, IMGU_OUTPUT_HEIGHT_ALIGN); pixelFormat = formats::NV12; - bufferCount = IPU3_BUFFER_COUNT; streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } }; break; @@ -492,7 +485,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, StreamConfiguration cfg(formats); cfg.size = size; cfg.pixelFormat = pixelFormat; - cfg.bufferCount = bufferCount; config->addConfiguration(cfg); } diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index a7c1cc1d5001..12d6729044e6 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -492,7 +492,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, RPiCameraData *data = cameraData(camera); CameraConfiguration *config = new RPiCameraConfiguration(data); V4L2DeviceFormat sensorFormat; - unsigned int bufferCount; PixelFormat pixelFormat; V4L2VideoDevice::Formats fmts; Size size; @@ -510,7 +509,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, sensorFormat = findBestMode(fmts, size); pixelFormat = sensorFormat.fourcc.toPixelFormat(); ASSERT(pixelFormat.isValid()); - bufferCount = 2; rawCount++; break; @@ -519,7 +517,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, pixelFormat = formats::NV12; /* Return the largest sensor resolution. */ size = data->sensor_->resolution(); - bufferCount = 1; outCount++; break; @@ -535,7 +532,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, fmts = data->isp_[Isp::Output0].dev()->formats(); pixelFormat = formats::YUV420; size = { 1920, 1080 }; - bufferCount = 4; outCount++; break; @@ -543,7 +539,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, fmts = data->isp_[Isp::Output0].dev()->formats(); pixelFormat = formats::ARGB8888; size = { 800, 600 }; - bufferCount = 4; outCount++; break; @@ -573,7 +568,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, 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 515f4be16d7e..9bbdf951d9b6 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -61,7 +61,6 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution) StreamConfiguration cfg(formats); cfg.pixelFormat = formats::NV12; cfg.size = maxResolution; - cfg.bufferCount = RKISP1_BUFFER_COUNT; return cfg; } @@ -77,7 +76,6 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg) 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 267d8f988ace..dd54fc609da6 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -57,8 +57,6 @@ public: Signal &bufferReady() { return video_->bufferReady; } private: - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; - const char *name_; bool running_; diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp index 3133b3dbda07..46ab503b7c38 100644 --- a/src/libcamera/pipeline/simple/converter.cpp +++ b/src/libcamera/pipeline/simple/converter.cpp @@ -89,9 +89,6 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, return -EINVAL; } - inputBufferCount_ = inputCfg.bufferCount; - outputBufferCount_ = outputCfg.bufferCount; - return 0; } diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h index deb3df0d08df..365b99e9853e 100644 --- a/src/libcamera/pipeline/simple/converter.h +++ b/src/libcamera/pipeline/simple/converter.h @@ -86,9 +86,6 @@ private: SimpleConverter *converter_; unsigned int index_; std::unique_ptr m2m_; - - unsigned int inputBufferCount_; - unsigned int outputBufferCount_; }; std::string deviceNode_; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index d0a658a23be8..6deba5d7dd61 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -252,7 +252,6 @@ protected: int queueRequestDevice(Camera *camera, Request *request) override; private: - static constexpr unsigned int kNumInternalBuffers = 3; static constexpr unsigned int kSimpleBufferSlotCount = 16; SimpleCameraData *cameraData(Camera *camera) @@ -638,7 +637,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) = converter->strideAndFrameSize(cfg.pixelFormat, cfg.size); @@ -656,8 +655,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() cfg.stride = format.planes[0].bpl; cfg.frameSize = format.planes[0].size; } - - cfg.bufferCount = 3; } return status; @@ -780,7 +777,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 c210cf57750f..5977312a795d 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -150,8 +150,6 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -193,7 +191,6 @@ CameraConfiguration *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 d2943f61a745..ad71bfc67228 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -170,8 +170,6 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -227,7 +225,6 @@ CameraConfiguration *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 b421e17ecb36..d4cc0fafb76f 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 - */ - /** * \fn StreamConfiguration::stream() * \brief Retrieve the stream associated with the configuration diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index c504ea09e64b..67ac0ad20e15 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" @@ -92,10 +94,12 @@ protected: return TestFail; } + unsigned int bufferCount = camera_->properties().get(properties::MinimumRequests); + Stream *stream = cfg.stream(); BufferSource source; - int ret = source.allocate(cfg); + int ret = source.allocate(cfg, bufferCount); if (ret != TestPass) return ret; @@ -139,10 +143,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 73563f2fc39d..c3d5286a2462 100644 --- a/test/libtest/buffer_source.cpp +++ b/test/libtest/buffer_source.cpp @@ -24,7 +24,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"; @@ -77,7 +77,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 14b4770e8d8a..6a18e269a575 100644 --- a/test/libtest/buffer_source.h +++ b/test/libtest/buffer_source.h @@ -20,7 +20,7 @@ public: BufferSource(); ~BufferSource(); - int allocate(const StreamConfiguration &config); + int allocate(const 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 b3f2bec11783..6c8183800d0b 100644 --- a/test/v4l2_videodevice/buffer_cache.cpp +++ b/test/v4l2_videodevice/buffer_cache.cpp @@ -145,10 +145,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 Tue Aug 24 19:56:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13473 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 7032EC324C for ; Tue, 24 Aug 2021 19:58:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A4907688A5; Tue, 24 Aug 2021 21:57:40 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C7CF26891A for ; Tue, 24 Aug 2021 21:57:37 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id 6C7041F43379 From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 24 Aug 2021 16:56:30 -0300 Message-Id: <20210824195636.1110845-12-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210824195636.1110845-1-nfraprado@collabora.com> References: <20210824195636.1110845-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 11/17] 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: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- Changes in v8: - Made SimpleCapture::allocateBuffers() a single function with an optional parameter Changes in v5: - New src/lc-compliance/capture_test.cpp | 8 +++++++- src/lc-compliance/simple_capture.cpp | 15 ++++++++++----- src/lc-compliance/simple_capture.h | 1 + 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/lc-compliance/capture_test.cpp b/src/lc-compliance/capture_test.cpp index 52578207c11f..6439cbd88f8e 100644 --- a/src/lc-compliance/capture_test.cpp +++ b/src/lc-compliance/capture_test.cpp @@ -80,6 +80,8 @@ TEST_P(SingleStream, Capture) capture.configure(role); + capture.allocateBuffers(); + capture.capture(numRequests); } @@ -99,8 +101,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); + } } /* @@ -118,6 +122,8 @@ TEST_P(SingleStream, UnbalancedStop) capture.configure(role); + capture.allocateBuffers(); + capture.capture(numRequests); } diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp index d87f30cbeb1b..041a2a4cbb5a 100644 --- a/src/lc-compliance/simple_capture.cpp +++ b/src/lc-compliance/simple_capture.cpp @@ -44,15 +44,20 @@ void SimpleCapture::configure(StreamRole role) } } -void SimpleCapture::start() +void SimpleCapture::allocateBuffers(unsigned int count) { - unsigned int bufferCount = camera_->properties().get(properties::MinimumRequests); + if (!count) + count = camera_->properties().get(properties::MinimumRequests); + 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/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h index 100ffd6637ad..401ba8273da8 100644 --- a/src/lc-compliance/simple_capture.h +++ b/src/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 Tue Aug 24 19:56:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13472 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 3FD63C324B for ; Tue, 24 Aug 2021 19:58:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E7B3C6891A; Tue, 24 Aug 2021 21:57:40 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 908FF688A3 for ; Tue, 24 Aug 2021 21:57:39 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id 2BF061F43379 From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 24 Aug 2021 16:56:31 -0300 Message-Id: <20210824195636.1110845-13-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210824195636.1110845-1-nfraprado@collabora.com> References: <20210824195636.1110845-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 12/17] 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: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- 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/lc-compliance/simple_capture.cpp | 93 +++++++++++++++------------- src/lc-compliance/simple_capture.h | 16 +++-- 2 files changed, 61 insertions(+), 48 deletions(-) diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp index 041a2a4cbb5a..e4f70974dd2c 100644 --- a/src/lc-compliance/simple_capture.cpp +++ b/src/lc-compliance/simple_capture.cpp @@ -70,12 +70,57 @@ void SimpleCapture::stop() camera_->stop(); + requests_.clear(); + camera_->requestCompleted.disconnect(this, &SimpleCapture::requestComplete); Stream *stream = config_->at(0).stream(); 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) @@ -102,24 +147,9 @@ void SimpleCaptureBalanced::capture(unsigned int numRequests) captureCount_ = 0; captureLimit_ = numRequests; - /* Queue the recommended number of reqeuests. */ - std::vector> requests; - for (const std::unique_ptr &buffer : buffers) { - std::unique_ptr request = camera_->createRequest(); - ASSERT_TRUE(request) << "Can't create request"; + queueRequests(stream, buffers); - 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)); - } - - /* Run capture session. */ - loop_ = new EventLoop(); - loop_->exec(); - stop(); - delete loop_; + runCaptureSession(); ASSERT_EQ(captureCount_, captureLimit_); } @@ -135,11 +165,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,35 +190,17 @@ void SimpleCaptureUnbalanced::capture(unsigned int numRequests) captureCount_ = 0; captureLimit_ = numRequests; - /* Queue the recommended number of reqeuests. */ - std::vector> requests; - 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(camera_->queueRequest(request.get()), 0) << "Failed to queue request"; - - requests.push_back(std::move(request)); - } + queueRequests(stream, buffers); - /* 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/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h index 401ba8273da8..8a6db2a355f6 100644 --- a/src/lc-compliance/simple_capture.h +++ b/src/lc-compliance/simple_capture.h @@ -25,14 +25,23 @@ 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_; + std::vector> requests_; }; class SimpleCaptureBalanced : public SimpleCapture @@ -43,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 @@ -60,9 +67,6 @@ public: private: void requestComplete(libcamera::Request *request) override; - - unsigned int captureCount_; - unsigned int captureLimit_; }; #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */ From patchwork Tue Aug 24 19:56:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13475 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 86255C324E for ; Tue, 24 Aug 2021 19:58:18 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 63DA668909; Tue, 24 Aug 2021 21:57:43 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 59B1A688A3 for ; Tue, 24 Aug 2021 21:57:41 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id EDCAD1F43379 From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 24 Aug 2021 16:56:32 -0300 Message-Id: <20210824195636.1110845-14-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210824195636.1110845-1-nfraprado@collabora.com> References: <20210824195636.1110845-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 13/17] 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: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- Changes in v8: - Moved CameraHolder to new test_base.{cpp,h} files Changes in v5: - New src/lc-compliance/capture_test.cpp | 18 ++++-------------- src/lc-compliance/meson.build | 1 + src/lc-compliance/test_base.cpp | 28 ++++++++++++++++++++++++++++ src/lc-compliance/test_base.h | 24 ++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 14 deletions(-) create mode 100644 src/lc-compliance/test_base.cpp create mode 100644 src/lc-compliance/test_base.h diff --git a/src/lc-compliance/capture_test.cpp b/src/lc-compliance/capture_test.cpp index 6439cbd88f8e..49012048729e 100644 --- a/src/lc-compliance/capture_test.cpp +++ b/src/lc-compliance/capture_test.cpp @@ -10,15 +10,15 @@ #include -#include "environment.h" #include "simple_capture.h" +#include "test_base.h" using namespace libcamera; const std::vector NUMREQUESTS = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 }; const std::vector ROLES = { Raw, StillCapture, VideoRecording, Viewfinder }; -class SingleStream : public testing::TestWithParam> +class SingleStream : public testing::TestWithParam>, public CameraHolder { public: static std::string nameParameters(const testing::TestParamInfo &info); @@ -26,8 +26,6 @@ public: protected: void SetUp() override; void TearDown() override; - - std::shared_ptr camera_; }; /* @@ -36,20 +34,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/lc-compliance/meson.build b/src/lc-compliance/meson.build index 4be14b694426..db593a6ec201 100644 --- a/src/lc-compliance/meson.build +++ b/src/lc-compliance/meson.build @@ -17,6 +17,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/lc-compliance/test_base.cpp b/src/lc-compliance/test_base.cpp new file mode 100644 index 000000000000..c9957b9efd36 --- /dev/null +++ b/src/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/lc-compliance/test_base.h b/src/lc-compliance/test_base.h new file mode 100644 index 000000000000..52347749ab10 --- /dev/null +++ b/src/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 Tue Aug 24 19:56:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13474 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 7D4A6C324D for ; Tue, 24 Aug 2021 19:58:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BB491688C4; Tue, 24 Aug 2021 21:57:45 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 167D068895 for ; Tue, 24 Aug 2021 21:57:43 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id B04EE1F43379 From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 24 Aug 2021 16:56:33 -0300 Message-Id: <20210824195636.1110845-15-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210824195636.1110845-1-nfraprado@collabora.com> References: <20210824195636.1110845-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 14/17] 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: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- Changes in v8: - Made roleToString()'s map const and changed usage from [] operator to at() Changes in v5: - New src/lc-compliance/capture_test.cpp | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/lc-compliance/capture_test.cpp b/src/lc-compliance/capture_test.cpp index 49012048729e..40242fbbc0ba 100644 --- a/src/lc-compliance/capture_test.cpp +++ b/src/lc-compliance/capture_test.cpp @@ -18,6 +18,18 @@ using namespace libcamera; const std::vector NUMREQUESTS = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 }; const std::vector ROLES = { Raw, StillCapture, VideoRecording, Viewfinder }; +static const std::string &roleToString(const StreamRole &role) +{ + static const std::map rolesMap = { + { Raw, "Raw" }, + { StillCapture, "StillCapture" }, + { VideoRecording, "VideoRecording" }, + { Viewfinder, "Viewfinder" } + }; + + return rolesMap.at(role); +} + class SingleStream : public testing::TestWithParam>, public CameraHolder { public: @@ -44,15 +56,8 @@ void SingleStream::TearDown() std::string SingleStream::nameParameters(const testing::TestParamInfo &info) { - std::map rolesMap = { { Raw, "Raw" }, - { StillCapture, "StillCapture" }, - { VideoRecording, "VideoRecording" }, - { 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 Tue Aug 24 19:56:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13476 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 1002AC324F for ; Tue, 24 Aug 2021 19:58:23 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3470C688A3; Tue, 24 Aug 2021 21:57:55 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C16CB60505 for ; Tue, 24 Aug 2021 21:57:54 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id 968AF1F43389 From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 24 Aug 2021 16:56:34 -0300 Message-Id: <20210824195636.1110845-16-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210824195636.1110845-1-nfraprado@collabora.com> References: <20210824195636.1110845-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 15/17] 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: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- (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/lc-compliance/capture_test.cpp | 45 ++++++++++++++++++++++++++++ src/lc-compliance/simple_capture.cpp | 29 ++++++++++++++++++ src/lc-compliance/simple_capture.h | 11 +++++++ 3 files changed, 85 insertions(+) diff --git a/src/lc-compliance/capture_test.cpp b/src/lc-compliance/capture_test.cpp index 40242fbbc0ba..2e495810e40f 100644 --- a/src/lc-compliance/capture_test.cpp +++ b/src/lc-compliance/capture_test.cpp @@ -18,6 +18,8 @@ using namespace libcamera; const std::vector NUMREQUESTS = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 }; const std::vector ROLES = { Raw, StillCapture, VideoRecording, Viewfinder }; +static constexpr unsigned int MAX_SIMULTANEOUS_REQUESTS = 8; + static const std::string &roleToString(const StreamRole &role) { static const std::map rolesMap = { @@ -54,12 +56,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 * @@ -122,8 +149,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/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp index e4f70974dd2c..d4805ff3403c 100644 --- a/src/lc-compliance/simple_capture.cpp +++ b/src/lc-compliance/simple_capture.cpp @@ -206,3 +206,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/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h index 8a6db2a355f6..9d86fb32e9e0 100644 --- a/src/lc-compliance/simple_capture.h +++ b/src/lc-compliance/simple_capture.h @@ -69,4 +69,15 @@ 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; +}; + #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */ From patchwork Tue Aug 24 19:56:35 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13477 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 7A095C3250 for ; Tue, 24 Aug 2021 19:58:23 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 64DD6688AE; Tue, 24 Aug 2021 21:57:57 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 85D3F60505 for ; Tue, 24 Aug 2021 21:57:56 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nfraprado) with ESMTPSA id 29BAF1F43387 From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 24 Aug 2021 16:56:35 -0300 Message-Id: <20210824195636.1110845-17-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210824195636.1110845-1-nfraprado@collabora.com> References: <20210824195636.1110845-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 16/17] 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: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- 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/lc-compliance/simple_capture.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp index d4805ff3403c..701d0feb70d1 100644 --- a/src/lc-compliance/simple_capture.cpp +++ b/src/lc-compliance/simple_capture.cpp @@ -165,6 +165,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; @@ -202,6 +205,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); @@ -233,5 +239,8 @@ void SimpleCaptureOverflow::capture() void SimpleCaptureOverflow::requestComplete([[maybe_unused]] Request *request) { + EXPECT_EQ(request->status(), Request::Status::RequestComplete) + << "Request didn't complete successfully"; + captureCompleted(); }