From patchwork Mon Apr 28 09:02:26 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23273 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 CC48CC327D for ; Mon, 28 Apr 2025 09:05:08 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5CEBB68B26; Mon, 28 Apr 2025 11:05:08 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D527668ACF for ; Mon, 28 Apr 2025 11:05:05 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQD-0001au-Hp; Mon, 28 Apr 2025 11:05:05 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Paul Elder , Umang Jain , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 01/19] libcamera: property: Add MinimumRequests property Date: Mon, 28 Apr 2025 11:02:26 +0200 Message-ID: <20250428090413.38234-2-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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. The mali-c55 driver defines min_queued_buffers = 1 [1]. Therefore set set the minimum requests to 2 to account one request in the userspace. The dcmipp and j721e drivers both defines min_queued_buffers = 1 in the kernel under drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-bytecap.c and drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c . Therefore use these values, as they are added 1 more. For the intel-ipu6, mtk-seninf simple devices and the virtual pipeline use a conservative value of 3 minimumBuffers, as no further requirements are known about them. [1] https://lore.kernel.org/linux-media/20240131174355.GB20792@pendragon.ideasonboard.com/T/#t Signed-off-by: Nícolas F. R. A. Prado Signed-off-by: Paul Elder Acked-by: Umang Jain Signed-off-by: Sven Püschel --- Changes in v11 - Add mali-c55, dcmipp, virtual, j721e, intel-ipu6 and mtk-seninf - Hold only the minimum buffers instead of the whole deviceInfo pointer in SimpleCameraData - Adjusted min_buffers_needed property to min_reqbufs_allocation in docs - Relax property description from no frame drops -> only minimal frame drops, based on the comment from Naush [10] - Changed property type from int32_t -> uint32_t to match all of the indirect casts to an unsigned value throughout the existing patchset. - Removed Reviewed-by: Laurent Pinchart - Removed Reviewed-by: Paul Elder [10] https://lists.libcamera.org/pipermail/libcamera-devel/2022-December/035872.html 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 ++ src/libcamera/pipeline/mali-c55/mali-c55.cpp | 1 + src/libcamera/pipeline/rkisp1/rkisp1.cpp | 1 + .../pipeline/rpi/common/pipeline_base.cpp | 2 + src/libcamera/pipeline/simple/simple.cpp | 41 +++++++++++++++---- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 + src/libcamera/pipeline/vimc/vimc.cpp | 2 + src/libcamera/pipeline/virtual/virtual.cpp | 1 + src/libcamera/property_ids_core.yaml | 22 ++++++++++ 11 files changed, 85 insertions(+), 13 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 9a15c20a..1079d998 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -663,19 +663,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_reqbufs_allocation`` 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 Vendor-specific controls and properties ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index ecda426a..2b8a583e 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" @@ -165,6 +166,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 e31e3879..356ca2e1 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -168,6 +168,8 @@ private: MediaDevice *imguMediaDev_; std::vector ipaBuffers_; + + static constexpr unsigned int kMinimumRequests = 3; }; IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data) @@ -1074,6 +1076,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/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index a05e11fc..f0ab3d2e 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -1602,6 +1602,7 @@ bool PipelineHandlerMaliC55::registerSensorCamera(MediaLink *ispLink) return false; data->properties_ = data->sensor_->properties(); + data->properties_.set(properties::MinimumRequests, 2); const CameraSensorProperties::SensorDelays &delays = data->sensor_->sensorDelays(); std::unordered_map params = { diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 194dfce7..5e3a4dba 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1318,6 +1318,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) /* Initialize the camera properties. */ data->properties_ = data->sensor_->properties(); + data->properties_.set(properties::MinimumRequests, 3); scalerMaxCrop_ = Rectangle(data->sensor_->resolution()); diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index 1f13e523..ef51d530 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -848,6 +848,8 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr &camera */ data->properties_.set(properties::ScalerCropMaximum, Rectangle{}); + data->properties_.set(properties::MinimumRequests, 3); + ret = platformRegister(cameraData, frontend, backend); if (ret) return ret; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index efb07051..7abf0fc9 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include @@ -233,6 +234,10 @@ SimpleFrameInfo *SimpleFrames::find(uint32_t frame) 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. @@ -249,14 +254,14 @@ struct SimplePipelineInfo { namespace { static const SimplePipelineInfo supportedDevices[] = { - { "dcmipp", {}, false }, - { "imx7-csi", { { "pxp", 1 } }, false }, - { "intel-ipu6", {}, true }, - { "j721e-csi2rx", {}, true }, - { "mtk-seninf", { { "mtk-mdp", 3 } }, false }, - { "mxc-isi", {}, false }, - { "qcom-camss", {}, true }, - { "sun6i-csi", {}, false }, + { "dcmipp", 1, {}, false }, + { "imx7-csi", 2, { { "pxp", 1 } }, false }, + { "intel-ipu6", 3, {}, true }, // \todo Check if the minimumBuffers can be lowered + { "j721e-csi2rx", 1, {}, true }, + { "mtk-seninf", 3, { { "mtk-mdp", 3 } }, false }, // \todo Check if the minimumBuffers can be lowered + { "mxc-isi", 3, {}, false }, + { "qcom-camss", 1, {}, true }, + { "sun6i-csi", 3, {}, false }, }; } /* namespace */ @@ -346,6 +351,8 @@ public: std::unique_ptr swIsp_; SimpleFrames frameInfo_; + unsigned int deviceInfoMinimumBuffers; + private: void tryPipeline(unsigned int code, const Size &size); static std::vector routedSourcePads(MediaPad *sink); @@ -1402,8 +1409,24 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) inputCfg.bufferCount = kNumInternalBuffers; if (data->converter_) { + /* + * 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. + */ + data->properties_.set(properties::MinimumRequests, 3); + return data->converter_->configure(inputCfg, outputCfgs); } 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. + */ + data->properties_.set(properties::MinimumRequests, std::max(data->deviceInfoMinimumBuffers + 1, + 3U)); + ipa::soft::IPAConfigInfo configInfo; configInfo.sensorControls = data->sensor_->controls(); return data->swIsp_->configure(inputCfg, outputCfgs, configInfo); @@ -1787,6 +1810,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) bool registered = false; for (std::unique_ptr &data : pipelines) { + data->deviceInfoMinimumBuffers = info->minimumBuffers; + 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 586e932d..31951a12 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -586,6 +586,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 07273bd2..01685a64 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -592,6 +593,7 @@ int VimcCameraData::init() /* Initialize the camera properties. */ properties_ = sensor_->properties(); + properties_.set(properties::MinimumRequests, 3); return 0; } diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp index 049ebcba..c33f1a5e 100644 --- a/src/libcamera/pipeline/virtual/virtual.cpp +++ b/src/libcamera/pipeline/virtual/virtual.cpp @@ -126,6 +126,7 @@ VirtualCameraData::VirtualCameraData(PipelineHandler *pipe, properties_.set(properties::PixelArrayActiveAreas, { Rectangle(config_.maxResolutionSize) }); + properties_.set(properties::MinimumRequests, 3); /* \todo Support multiple streams and pass multi_stream_test */ streamConfigs_.resize(kMaxStream); diff --git a/src/libcamera/property_ids_core.yaml b/src/libcamera/property_ids_core.yaml index 834454a4..cc28d677 100644 --- a/src/libcamera/property_ids_core.yaml +++ b/src/libcamera/property_ids_core.yaml @@ -701,4 +701,26 @@ controls: Different cameras may report identical devices. + - MinimumRequests: + type: uint32_t + description: | + The minimum number of capture requests that the camera pipeline requires + to have queued in order to sustain capture operations with only minimal + 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). + ... From patchwork Mon Apr 28 09:02:27 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23274 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 89556C32A2 for ; Mon, 28 Apr 2025 09:05:13 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B7DB168AD5; Mon, 28 Apr 2025 11:05:12 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 45B9168AD0 for ; Mon, 28 Apr 2025 11:05:06 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQD-0001au-UX; Mon, 28 Apr 2025 11:05:06 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Kieran Bingham , Laurent Pinchart , Paul Elder , Jacopo Mondi , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 02/19] libcamera: framebuffer_allocator: Make allocate() require count Date: Mon, 28 Apr 2025 11:02:27 +0200 Message-ID: <20250428090413.38234-3-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 Signed-off-by: Sven Püschel --- Changes in v11: - add mali-c55 and virtual - adapt python binding 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/helpers/capture.cpp | 5 +++-- src/apps/qcam/main_window.cpp | 10 +++++++++- src/gstreamer/gstlibcameraallocator.cpp | 6 +++++- 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/mali-c55/mali-c55.cpp | 3 ++- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 2 +- src/libcamera/pipeline/rpi/common/pipeline_base.h | 2 +- src/libcamera/pipeline/simple/simple.cpp | 4 ++-- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 7 ++++--- src/libcamera/pipeline/vimc/vimc.cpp | 7 ++++--- src/libcamera/pipeline/virtual/virtual.cpp | 5 +++-- src/libcamera/pipeline_handler.cpp | 1 + src/py/libcamera/py_main.cpp | 4 ++-- 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 ++++- 29 files changed, 97 insertions(+), 50 deletions(-) diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst index f3798d17..dbd2bfc6 100644 --- a/Documentation/guides/application-developer.rst +++ b/Documentation/guides/application-developer.rst @@ -32,6 +32,7 @@ defined names and types without the need of prefixing them. #include #include + #include using namespace libcamera; using namespace std::chrono_literals; @@ -276,7 +277,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. @@ -284,9 +288,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 1079d998..2e9825dc 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -209,7 +209,7 @@ implementations for the overridden class members. Span 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; @@ -1242,7 +1242,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 94cee7bd..ce9d8812 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -163,7 +163,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 f3896bf2..1568c55d 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 972a2fa6..0e9fe23e 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -48,7 +48,7 @@ public: Span 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 97c1ae44..f50108f3 100644 --- a/src/apps/cam/camera_session.cpp +++ b/src/apps/cam/camera_session.cpp @@ -326,17 +326,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/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp index 2a3fa3b6..690de005 100644 --- a/src/apps/lc-compliance/helpers/capture.cpp +++ b/src/apps/lc-compliance/helpers/capture.cpp @@ -120,7 +120,8 @@ void Capture::start() assert(!allocator_.allocated()); assert(requests_.empty()); - const auto bufferCount = config_->at(0).bufferCount; + unsigned int bufferCount = + camera_->properties().get(properties::MinimumRequests).value(); /* No point in testing less requests then the camera depth. */ if (queueLimit_ && *queueLimit_ < bufferCount) { @@ -137,7 +138,7 @@ void Capture::start() for (const auto &cfg : *config_) { Stream *stream = cfg.stream(); - int count = allocator_.allocate(stream); + int count = allocator_.allocate(stream, bufferCount); ASSERT_GE(count, 0) << "Failed to allocate buffers"; const auto &buffers = allocator_.buffers(stream); diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp index d2ccbd23..9a9b7fa9 100644 --- a/src/apps/qcam/main_window.cpp +++ b/src/apps/qcam/main_window.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include @@ -447,7 +448,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 d4492d99..0d8e4c40 100644 --- a/src/gstreamer/gstlibcameraallocator.cpp +++ b/src/gstreamer/gstlibcameraallocator.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include "gstlibcamera-utils.h" @@ -209,11 +210,14 @@ gst_libcamera_allocator_new(std::shared_ptr camera, if (ret) return nullptr; + unsigned int bufferCount = + camera->properties().get(properties::MinimumRequests).value(); + self->fb_allocator = new FrameBufferAllocator(camera); for (StreamConfiguration &streamCfg : *config_) { Stream *stream = streamCfg.stream(); - 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 c180a5fd..33e928b9 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -950,7 +950,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(); @@ -967,7 +967,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 3d53bde2..c522f479 100644 --- a/src/libcamera/framebuffer_allocator.cpp +++ b/src/libcamera/framebuffer_allocator.cpp @@ -68,6 +68,7 @@ FrameBufferAllocator::~FrameBufferAllocator() = default; /** * \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 @@ -76,6 +77,10 @@ FrameBufferAllocator::~FrameBufferAllocator() = default; * 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 @@ -83,7 +88,7 @@ FrameBufferAllocator::~FrameBufferAllocator() = default; * 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); @@ -92,7 +97,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 2b8a583e..21f44424 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -108,7 +108,7 @@ public: generateConfiguration(Camera *camera, Span 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; @@ -908,10 +908,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 356ca2e1..30b74d4e 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -139,7 +139,7 @@ public: Span 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; @@ -637,10 +637,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/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index f0ab3d2e..9df1622e 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -608,6 +608,7 @@ public: int configure(Camera *camera, CameraConfiguration *config) override; int exportFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, std::vector> *buffers) override; int allocateBuffers(Camera *camera); void freeBuffers(Camera *camera); @@ -1087,10 +1088,10 @@ int PipelineHandlerMaliC55::configure(Camera *camera, } int PipelineHandlerMaliC55::exportFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, std::vector> *buffers) { MaliC55Pipe *pipe = pipeFromStream(cameraData(camera), stream); - unsigned int count = stream->configuration().bufferCount; return pipe->cap->exportBuffers(count, buffers); } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 5e3a4dba..4e8eff70 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -158,7 +158,7 @@ public: Span 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; @@ -953,10 +953,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_) { /* diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index ef51d530..dc1e0a2e 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -630,10 +630,10 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config) } int PipelineHandlerBase::exportFrameBuffers([[maybe_unused]] Camera *camera, libcamera::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/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h index aae0c2f3..e73d6f18 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h @@ -218,7 +218,7 @@ public: generateConfiguration(Camera *camera, Span roles) override; int configure(Camera *camera, CameraConfiguration *config) override; - int exportFrameBuffers(Camera *camera, libcamera::Stream *stream, + int exportFrameBuffers(Camera *camera, libcamera::Stream *stream, unsigned int count, std::vector> *buffers) override; int start(Camera *camera, const ControlList *controls) override; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 7abf0fc9..1e77ccdc 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -404,7 +404,7 @@ public: Span 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; @@ -1434,10 +1434,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 31951a12..60c82364 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -87,7 +87,7 @@ public: Span 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; @@ -278,11 +278,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 01685a64..8833647a 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -92,7 +92,7 @@ public: Span 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; @@ -343,11 +343,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/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp index c33f1a5e..ad2bdb57 100644 --- a/src/libcamera/pipeline/virtual/virtual.cpp +++ b/src/libcamera/pipeline/virtual/virtual.cpp @@ -87,7 +87,7 @@ public: Span 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; @@ -269,6 +269,7 @@ int PipelineHandlerVirtual::configure(Camera *camera, int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream, + unsigned int count, std::vector> *buffers) { if (!dmaBufAllocator_.isValid()) @@ -281,7 +282,7 @@ int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera *camera, for (size_t i = 0; i < info.numPlanes(); ++i) planeSizes.push_back(info.planeSize(config.size, i)); - return dmaBufAllocator_.exportBuffers(config.bufferCount, planeSizes, buffers); + return dmaBufAllocator_.exportBuffers(count, planeSizes, buffers); } int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera, diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index d84dff3c..3211256c 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -314,6 +314,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/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp index 441a70ab..f75450ee 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -349,8 +349,8 @@ PYBIND11_MODULE(_libcamera, m) pyFrameBufferAllocator .def(py::init>(), py::keep_alive<1, 2>()) - .def("allocate", [](FrameBufferAllocator &self, Stream *stream) { - int ret = self.allocate(stream); + .def("allocate", [](FrameBufferAllocator &self, Stream *stream, unsigned int count) { + int ret = self.allocate(stream, count); if (ret < 0) throw std::system_error(-ret, std::generic_category(), "Failed to allocate buffers"); diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 94d138cd..a4b0e562 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -162,7 +162,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 8766fb19..05f4a1e3 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" @@ -101,8 +102,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 8095b228..eb6c8b54 100644 --- a/test/fence.cpp +++ b/test/fence.cpp @@ -18,6 +18,7 @@ #include #include +#include #include "camera_test.h" #include "test.h" @@ -119,8 +120,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 Mon Apr 28 09:02:28 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23275 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 B24B7C327D for ; Mon, 28 Apr 2025 09:05:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E80B068AD5; Mon, 28 Apr 2025 11:05:14 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8484268AD4 for ; Mon, 28 Apr 2025 11:05:06 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQE-0001au-4G; Mon, 28 Apr 2025 11:05:06 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Paul Elder , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 03/19] libcamera: pipeline: rpi: Don't rely on bufferCount Date: Mon, 28 Apr 2025 11:02:28 +0200 Message-ID: <20250428090413.38234-4-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 already exists a procedure for determining the number of buffers to reserve, so to remove reliance on 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 Signed-off-by: Sven Püschel --- Changes in v11: - adapted pisp.cpp similar to vc4.cpp - corrected spelling errors in commit description 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/rpi/common/pipeline_base.cpp | 6 ------ src/libcamera/pipeline/rpi/pisp/pisp.cpp | 2 +- src/libcamera/pipeline/rpi/vc4/vc4.cpp | 2 +- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index dc1e0a2e..bcc74052 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -388,7 +388,6 @@ PipelineHandlerBase::generateConfiguration(Camera *camera, Span config = std::make_unique(data); V4L2SubdeviceFormat sensorFormat; - unsigned int bufferCount; PixelFormat pixelFormat; V4L2VideoDevice::Formats fmts; Size size; @@ -407,7 +406,6 @@ PipelineHandlerBase::generateConfiguration(Camera *camera, SpanaddConfiguration(cfg); } diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp index 91e7f4c9..0f607a64 100644 --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp @@ -965,7 +965,7 @@ int PipelineHandlerPiSP::prepareBuffers(Camera *camera) for (Stream *s : camera->streams()) { if (PipelineHandlerBase::isRaw(s->configuration().pixelFormat)) { - numRawBuffers = s->configuration().bufferCount; + numRawBuffers = data->cfe_[Cfe::Output0].getBuffers().size(); break; } } diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp index fe910bdf..97e7a6ff 100644 --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp @@ -224,7 +224,7 @@ int PipelineHandlerVc4::prepareBuffers(Camera *camera) for (Stream *s : camera->streams()) { if (BayerFormat::fromPixelFormat(s->configuration().pixelFormat).isValid()) { - numRawBuffers = s->configuration().bufferCount; + numRawBuffers = data->unicam_[Unicam::Image].getBuffers().size(); break; } } From patchwork Mon Apr 28 09:02:29 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23276 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 7935FC331E for ; Mon, 28 Apr 2025 09:05:18 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 38C0068B26; Mon, 28 Apr 2025 11:05:16 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AC1F568AD5 for ; Mon, 28 Apr 2025 11:05:06 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQE-0001au-AL; Mon, 28 Apr 2025 11:05:06 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Paul Elder , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 04/19] libcamera: pipeline: ipu3: Don't rely on bufferCount Date: Mon, 28 Apr 2025 11:02:29 +0200 Message-ID: <20250428090413.38234-5-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 Signed-off-by: Sven Püschel --- Changes in v11: - rebased 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 aa544d7b..5a2c5644 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -236,7 +236,6 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const cfg.size = sensorFormat.size; cfg.pixelFormat = mbusCodesToPixelFormat.at(sensorFormat.code); - cfg.bufferCount = kBufferCount; return cfg; } @@ -337,13 +336,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 963c2f6b..3c6cc8bc 100644 --- a/src/libcamera/pipeline/ipu3/cio2.h +++ b/src/libcamera/pipeline/ipu3/cio2.h @@ -31,8 +31,6 @@ enum class Transform; class CIO2Device { public: - static constexpr unsigned int kBufferCount = 4; - CIO2Device(); std::vector formats() const; @@ -50,7 +48,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 7be78091..50b981f2 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 fa508316..47d6ed0c 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 30b74d4e..665466c7 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -159,7 +159,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_; @@ -170,6 +170,7 @@ private: std::vector ipaBuffers_; static constexpr unsigned int kMinimumRequests = 3; + static constexpr unsigned int kIPU3BufferSlotCount = 16; }; IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data) @@ -660,20 +661,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; @@ -731,7 +737,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; @@ -744,8 +750,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 Mon Apr 28 09:02:30 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23277 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 4D489C331F for ; Mon, 28 Apr 2025 09:05:20 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A9EC568B32; Mon, 28 Apr 2025 11:05:17 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E06C368AD6 for ; Mon, 28 Apr 2025 11:05:06 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQE-0001au-H3; Mon, 28 Apr 2025 11:05:06 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Paul Elder , Jacopo Mondi , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 05/19] libcamera: pipeline: rkisp1: Don't rely on bufferCount Date: Mon, 28 Apr 2025 11:02:30 +0200 Message-ID: <20250428090413.38234-6-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 Signed-off-by: Sven Püschel --- Changes in v11: - rebased Changes in v9: - rebased Changes in v8: - New --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 21 ++++++++++++------- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 3 +-- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 ++ 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 4e8eff70..81bf1c55 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -225,6 +225,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) @@ -981,24 +991,19 @@ 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; } /* If the dewarper is being used, allocate internal buffers for ISP. */ if (useDewarper_) { - ret = mainPath_.exportBuffers(maxCount, &mainPathBuffers_); + ret = mainPath_.exportBuffers(kRkISP1InternalBufferCount, &mainPathBuffers_); if (ret < 0) goto error; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index 64018dc5..6a5b22ba 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -487,8 +487,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 430181d3..7365275d 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -88,6 +88,8 @@ private: * which are guaranteed to be supported by the pipeline. */ std::map> sensorSizesMap_; + + static constexpr unsigned int kRkISP1BufferSlotCount = 16; }; class RkISP1MainPath : public RkISP1Path From patchwork Mon Apr 28 09:02:31 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23278 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 702A3C3320 for ; Mon, 28 Apr 2025 09:05:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C34AF68B31; Mon, 28 Apr 2025 11:05:19 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 16FEF68AD7 for ; Mon, 28 Apr 2025 11:05:07 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQE-0001au-Mx; Mon, 28 Apr 2025 11:05:06 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Paul Elder , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 06/19] libcamera: pipeline: simple: Don't rely on bufferCount Date: Mon, 28 Apr 2025 11:02:31 +0200 Message-ID: <20250428090413.38234-7-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 Signed-off-by: Sven Püschel --- Changes in v11: - added rkisp1 adjustments for the dewarper 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/rkisp1/rkisp1.cpp | 2 +- src/libcamera/pipeline/simple/simple.cpp | 21 ++++++++++++++----- 6 files changed, 29 insertions(+), 14 deletions(-) diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h index 644ec429..1da51d1e 100644 --- a/include/libcamera/internal/converter.h +++ b/include/libcamera/internal/converter.h @@ -75,7 +75,7 @@ public: virtual int exportBuffers(const Stream *stream, 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 0ad7bf7f..24997c6f 100644 --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h @@ -59,7 +59,7 @@ public: int exportBuffers(const Stream *stream, unsigned int count, std::vector> *buffers) override; - int start() override; + int start(unsigned int inputBufferCount) override; void stop() override; int validateOutput(StreamConfiguration *cfg, bool *adjusted, @@ -85,7 +85,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 d551b908..2c6f1e4b 100644 --- a/src/libcamera/converter.cpp +++ b/src/libcamera/converter.cpp @@ -190,6 +190,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 ee05b798..242e0d85 100644 --- a/src/libcamera/converter/converter_v4l2_m2m.cpp +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp @@ -160,13 +160,15 @@ int V4L2M2MConverter::V4L2M2MStream::exportBuffers(unsigned int count, return m2m_->capture()->exportBuffers(count, buffers); } -int V4L2M2MConverter::V4L2M2MStream::start() +int V4L2M2MConverter::V4L2M2MStream::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; @@ -620,12 +622,12 @@ V4L2M2MConverter::inputCropBounds(const Stream *stream) /** * \copydoc libcamera::Converter::start */ -int V4L2M2MConverter::start() +int V4L2M2MConverter::start(unsigned int inputBufferCount) { int ret; for (auto &iter : streams_) { - ret = iter.second->start(); + ret = iter.second->start(inputBufferCount); if (ret < 0) { stop(); return ret; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 81bf1c55..623bcfe5 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1110,7 +1110,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL actions += [&]() { stat_->streamOff(); }; if (useDewarper_) { - ret = dewarper_->start(); + ret = dewarper_->start(kRkISP1InternalBufferCount); if (ret) { LOG(RkISP1, Error) << "Failed to start dewarper"; return ret; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 1e77ccdc..4f4cba06 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -422,6 +422,7 @@ protected: private: static constexpr unsigned int kNumInternalBuffers = 3; + static constexpr unsigned int kSimpleBufferSlotCount = 16; struct EntityData { std::unique_ptr video; @@ -1466,17 +1467,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->useConversion_) { /* * When using the converter allocate a fixed number of internal * buffers. */ - ret = video->allocateBuffers(kNumInternalBuffers, + ret = video->allocateBuffers(internalBufferCount, &data->conversionBuffers_); } 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); @@ -1504,7 +1515,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL if (data->useConversion_) { if (data->converter_) - ret = data->converter_->start(); + ret = data->converter_->start(internalBufferCount); else if (data->swIsp_) ret = data->swIsp_->start(); else From patchwork Mon Apr 28 09:02:32 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23279 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 6B1EFC3321 for ; Mon, 28 Apr 2025 09:05:23 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 148F468B38; Mon, 28 Apr 2025 11:05:21 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A71D68AD9 for ; Mon, 28 Apr 2025 11:05:07 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQE-0001au-TP; Mon, 28 Apr 2025 11:05:06 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Paul Elder , Jacopo Mondi , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 07/19] libcamera: pipeline: vimc, uvcvideo: Don't rely on bufferCount Date: Mon, 28 Apr 2025 11:02:32 +0200 Message-ID: <20250428090413.38234-8-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 Signed-off-by: Sven Püschel --- Changes in v11: - rebased 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 60c82364..e1259cf3 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -109,6 +109,8 @@ private: { return static_cast(camera->_d()); } + + static constexpr unsigned int kUVCBufferSlotCount = 16; }; namespace { @@ -291,9 +293,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 8833647a..e66dbd78 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -109,6 +109,8 @@ private: { return static_cast(camera->_d()); } + + static constexpr unsigned int kVimcBufferSlotCount = 16; }; namespace { @@ -356,9 +358,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 Mon Apr 28 09:02:33 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23280 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 2914BC3322 for ; Mon, 28 Apr 2025 09:05:26 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7381B68B38; Mon, 28 Apr 2025 11:05:25 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6271568ADA for ; Mon, 28 Apr 2025 11:05:07 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQF-0001au-2P; Mon, 28 Apr 2025 11:05:07 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: Paul Elder , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 08/19] libcamera: pipeline: imx8-isi: Don't rely on bufferCount Date: Mon, 28 Apr 2025 11:02:33 +0200 Message-ID: <20250428090413.38234-9-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Paul Elder 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 Signed-off-by: Sven Püschel --- Changes in v11: - rebased 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 21f44424..efb280d7 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -144,6 +144,8 @@ private: std::unique_ptr crossbar_; std::vector pipes_; + + static constexpr unsigned int kBufferSlotCount = 16; }; /* ----------------------------------------------------------------------------- @@ -923,9 +925,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 Mon Apr 28 09:02:34 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23281 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 C3B97C3323 for ; Mon, 28 Apr 2025 09:05:27 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 025DB68B36; Mon, 28 Apr 2025 11:05:26 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 93D7568ADB for ; Mon, 28 Apr 2025 11:05:07 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQF-0001au-77; Mon, 28 Apr 2025 11:05:07 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 09/19] libcamera: pipeline: mali-c55: Don't rely on bufferCount Date: Mon, 28 Apr 2025 11:02:34 +0200 Message-ID: <20250428090413.38234-10-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 mali-c55 pipeline handler. Instead of using the previous bufferCount value of 4 use the higher value of 16 like in other pipeline handlers to avoid trashing dmabuf mappings if the application cycles more than 4 buffers. This makes way for removing bufferCount. Signed-off-by: Sven Püschel --- Changes in v11: - Added --- src/libcamera/pipeline/mali-c55/mali-c55.cpp | 22 ++++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index 9df1622e..726d9780 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -701,6 +701,14 @@ private: std::array pipes_; bool dsFitted_; + + /* + * This many internal buffers (or rather parameter and statistics buffer + * pairs) ensures that the pipeline runs smoothly, without frame drops. + * \todo check if this can be lowered + */ + static constexpr unsigned int kMaliC55InternalBufferCount = 4; + static constexpr unsigned int kMaliC55BufferSlotCount = 16; }; PipelineHandlerMaliC55::PipelineHandlerMaliC55(CameraManager *manager) @@ -1128,15 +1136,9 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) { MaliC55CameraData *data = cameraData(camera); unsigned int ipaBufferId = 1; - unsigned int bufferCount; int ret; - bufferCount = std::max({ - data->frStream_.configuration().bufferCount, - data->dsStream_.configuration().bufferCount, - }); - - ret = stats_->allocateBuffers(bufferCount, &statsBuffers_); + ret = stats_->allocateBuffers(kMaliC55InternalBufferCount, &statsBuffers_); if (ret < 0) return ret; @@ -1147,7 +1149,7 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) availableStatsBuffers_.push(buffer.get()); } - ret = params_->allocateBuffers(bufferCount, ¶msBuffers_); + ret = params_->allocateBuffers(kMaliC55InternalBufferCount, ¶msBuffers_); if (ret < 0) return ret; @@ -1189,9 +1191,7 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control if (!pipe.stream) continue; - Stream *stream = pipe.stream; - - ret = pipe.cap->importBuffers(stream->configuration().bufferCount); + ret = pipe.cap->importBuffers(kMaliC55BufferSlotCount); if (ret) { LOG(MaliC55, Error) << "Failed to import buffers"; if (data->ipa_) From patchwork Mon Apr 28 09:02:35 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23282 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 08626C32A2 for ; Mon, 28 Apr 2025 09:05:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 034BB68B3B; Mon, 28 Apr 2025 11:05:28 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B20E968AD8 for ; Mon, 28 Apr 2025 11:05:07 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQF-0001au-DP; Mon, 28 Apr 2025 11:05:07 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Paul Elder , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 10/19] v4l2: Allocate buffers based on requested count and MinimumRequests Date: Mon, 28 Apr 2025 11:02:35 +0200 Message-ID: <20250428090413.38234-11-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 Signed-off-by: Sven Püschel --- Changes in v11: - rebased Changes in v9: - rebased Changes in v8: - New - Fixed typo: s/interval/internal/ --- src/v4l2/v4l2_camera.cpp | 19 ++++++++++++------- src/v4l2/v4l2_camera.h | 5 +++-- src/v4l2/v4l2_camera_proxy.cpp | 10 +++++----- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index a4b0e562..feb6d405 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -13,6 +13,7 @@ #include #include +#include using namespace libcamera; @@ -108,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) { @@ -147,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) @@ -166,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(); @@ -175,7 +175,7 @@ int V4L2Camera::allocBuffers(unsigned int count) requestPool_.push_back(std::move(request)); } - return ret; + return allocatedCount; } void V4L2Camera::freeBuffers() @@ -304,3 +304,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 9bd161b9..402a7a76 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -45,8 +45,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); @@ -68,6 +67,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 559ffc61..b12a59a0 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -392,8 +392,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; @@ -539,14 +538,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) { @@ -554,6 +552,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 Mon Apr 28 09:02:36 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23283 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 9E4FCC3324 for ; Mon, 28 Apr 2025 09:05:30 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C040868B46; Mon, 28 Apr 2025 11:05:29 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E006068ADE for ; Mon, 28 Apr 2025 11:05:07 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQF-0001au-JB; Mon, 28 Apr 2025 11:05:07 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Paul Elder , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 11/19] libcamera: stream: Remove bufferCount Date: Mon, 28 Apr 2025 11:02:36 +0200 Message-ID: <20250428090413.38234-12-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 Signed-off-by: Sven Püschel --- Changes in v11: - add mali-c55 and virtual 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/apps/lc-compliance/helpers/capture.cpp | 14 -------------- src/libcamera/converter/converter_v4l2_m2m.cpp | 3 --- src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 2 -- src/libcamera/pipeline/ipu3/ipu3.cpp | 8 -------- src/libcamera/pipeline/mali-c55/mali-c55.cpp | 1 - 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/pipeline/virtual/virtual.cpp | 5 ----- src/libcamera/stream.cpp | 12 +++--------- src/py/libcamera/py_main.cpp | 1 - test/camera/buffer_import.cpp | 9 +++++++-- test/libtest/buffer_source.cpp | 4 ++-- test/libtest/buffer_source.h | 2 +- test/v4l2_videodevice/buffer_cache.cpp | 3 +-- 21 files changed, 28 insertions(+), 77 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 2e9825dc..725e35e6 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -882,14 +882,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 @@ -955,8 +953,6 @@ Add the following function implementation to your file: status = Adjusted; } - cfg.bufferCount = 4; - return status; } @@ -1200,13 +1196,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 24997c6f..b32c338c 100644 --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h @@ -106,9 +106,6 @@ private: const Stream *stream_; std::unique_ptr m2m_; - unsigned int inputBufferCount_; - unsigned int outputBufferCount_; - std::pair inputCropBounds_; }; diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index b5e8f0a9..a75a82a0 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -46,8 +46,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 1d68540d..9b11c3fa 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/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp index 690de005..5e8b5df6 100644 --- a/src/apps/lc-compliance/helpers/capture.cpp +++ b/src/apps/lc-compliance/helpers/capture.cpp @@ -33,20 +33,6 @@ void Capture::configure(libcamera::Span roles) ASSERT_EQ(config_->size(), roles.size()) << "Unexpected number of streams in configuration"; - /* - * Set the buffers count to the largest value across all streams. - * \todo: Should all streams from a Camera have the same buffer count ? - */ - auto largest = - std::max_element(config_->begin(), config_->end(), - [](const StreamConfiguration &l, const StreamConfiguration &r) - { return l.bufferCount < r.bufferCount; }); - - assert(largest != config_->end()); - - for (auto &cfg : *config_) - cfg.bufferCount = largest->bufferCount; - if (config_->validate() != CameraConfiguration::Valid) { config_.reset(); FAIL() << "Configuration not valid"; diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp index 242e0d85..3efd1f76 100644 --- a/src/libcamera/converter/converter_v4l2_m2m.cpp +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp @@ -141,9 +141,6 @@ int V4L2M2MConverter::V4L2M2MStream::configure(const StreamConfiguration &inputC return -EINVAL; } - inputBufferCount_ = inputCfg.bufferCount; - outputBufferCount_ = outputCfg.bufferCount; - if (converter_->features() & Feature::InputCrop) { ret = getCropBounds(m2m_->output(), inputCropBounds_.first, inputCropBounds_.second); diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index efb280d7..3d2e6a3d 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -666,7 +666,6 @@ StreamConfiguration PipelineHandlerISI::generateYUVConfiguration(Camera *camera, StreamConfiguration cfg(formats); cfg.pixelFormat = pixelFormat; cfg.size = sensorSize; - cfg.bufferCount = 4; return cfg; } @@ -734,7 +733,6 @@ StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera) StreamConfiguration cfg(formats); cfg.size = sensor->resolution(); cfg.pixelFormat = pixelFormat; - cfg.bufferCount = 4; return cfg; } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 665466c7..c8b25913 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -97,7 +97,6 @@ private: class IPU3CameraConfiguration : public CameraConfiguration { public: - static constexpr unsigned int kBufferCount = 4; static constexpr unsigned int kMaxStreams = 3; IPU3CameraConfiguration(IPU3CameraData *data); @@ -293,7 +292,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_)); @@ -337,7 +335,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); @@ -406,7 +403,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, Spancio2_.sensor()->resolution(); for (const StreamRole role : roles) { std::map> streamFormats; - unsigned int bufferCount; PixelFormat pixelFormat; Size size; @@ -426,7 +422,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, Spancio2_.generateConfiguration(sensorResolution); pixelFormat = cio2Config.pixelFormat; size = cio2Config.size; - bufferCount = cio2Config.bufferCount; for (const PixelFormat &format : data->cio2_.formats()) streamFormats[format] = data->cio2_.sizes(format); @@ -455,7 +449,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, SpanaddConfiguration(cfg); } diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index 726d9780..0ce19b16 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -818,7 +818,6 @@ PipelineHandlerMaliC55::generateConfiguration(Camera *camera, StreamFormats streamFormats(formats); StreamConfiguration cfg(streamFormats); cfg.pixelFormat = pixelFormat; - cfg.bufferCount = 4; cfg.size = size; config->addConfiguration(cfg); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index 6a5b22ba..e07be15e 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -249,7 +249,6 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size, StreamConfiguration cfg(formats); cfg.pixelFormat = format; cfg.size = streamSize; - cfg.bufferCount = RKISP1_BUFFER_COUNT; return cfg; } @@ -383,7 +382,6 @@ 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 7365275d..d7c50f3a 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -69,8 +69,6 @@ private: void populateFormats(); Size filterSensorResolution(const CameraSensor *sensor); - 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 4f4cba06..c49ea6f6 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -421,7 +421,6 @@ protected: int queueRequestDevice(Camera *camera, Request *request) override; private: - static constexpr unsigned int kNumInternalBuffers = 3; static constexpr unsigned int kSimpleBufferSlotCount = 16; struct EntityData { @@ -1239,7 +1238,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_ @@ -1261,8 +1260,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() cfg.stride = format.planes[0].bpl; cfg.frameSize = format.planes[0].size; } - - cfg.bufferCount = 4; } return status; @@ -1407,7 +1404,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; if (data->converter_) { /* diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index e1259cf3..c3b718f9 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -186,8 +186,6 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -248,7 +246,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 e66dbd78..2632a1ab 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -183,8 +183,6 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -244,7 +242,6 @@ PipelineHandlerVimc::generateConfiguration(Camera *camera, cfg.pixelFormat = formats::BGR888; cfg.size = { 1920, 1080 }; - cfg.bufferCount = 4; config->addConfiguration(cfg); diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp index ad2bdb57..60c06d4e 100644 --- a/src/libcamera/pipeline/virtual/virtual.cpp +++ b/src/libcamera/pipeline/virtual/virtual.cpp @@ -67,8 +67,6 @@ overloaded(Ts...) -> overloaded; class VirtualCameraConfiguration : public CameraConfiguration { public: - static constexpr unsigned int kBufferCount = 4; - VirtualCameraConfiguration(VirtualCameraData *data); Status validate() override; @@ -188,8 +186,6 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate() const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat); cfg.stride = info.stride(cfg.size.width, 0, 1); cfg.frameSize = info.frameSize(cfg.size, 1); - - cfg.bufferCount = VirtualCameraConfiguration::kBufferCount; } return status; @@ -244,7 +240,6 @@ PipelineHandlerVirtual::generateConfiguration(Camera *camera, StreamConfiguration cfg(formats); cfg.pixelFormat = pixelFormat; cfg.size = data->config_.maxResolutionSize; - cfg.bufferCount = VirtualCameraConfiguration::kBufferCount; config->addConfiguration(cfg); } diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 978d7275..684bc209 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) { } @@ -325,11 +324,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 f75450ee..10e1d5cb 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -337,7 +337,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 815d1cae..47958898 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" @@ -97,10 +99,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; @@ -137,7 +142,7 @@ protected: } } - const unsigned int nFrames = cfg.bufferCount * 2; + const unsigned int nFrames = bufferCount * 2; Timer timer; timer.start(500ms * nFrames); 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 495da8a9..deca2aab 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 Mon Apr 28 09:02:37 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23284 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 055DCC3325 for ; Mon, 28 Apr 2025 09:05:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2037968B2A; Mon, 28 Apr 2025 11:05:31 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4D52F68B25 for ; Mon, 28 Apr 2025 11:05:08 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQF-0001au-Oz; Mon, 28 Apr 2025 11:05:07 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Paul Elder , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 12/19] lc-compliance: Move buffer allocation to separate function Date: Mon, 28 Apr 2025 11:02:37 +0200 Message-ID: <20250428090413.38234-13-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 Signed-off-by: Sven Püschel --- Changes in v11: - adapted to support multiple streams - added requestCount_ member to pass allocated count to start() - added assert to allocateBuffers 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/helpers/capture.cpp | 36 ++++++++++++------- src/apps/lc-compliance/helpers/capture.h | 2 ++ src/apps/lc-compliance/tests/capture_test.cpp | 8 ++++- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp index 5e8b5df6..1e232dd1 100644 --- a/src/apps/lc-compliance/helpers/capture.cpp +++ b/src/apps/lc-compliance/helpers/capture.cpp @@ -44,6 +44,24 @@ void Capture::configure(libcamera::Span roles) } } +void Capture::allocateBuffers(unsigned int count) +{ + assert(!allocator_.allocated()); + + if (!count) + count = camera_->properties().get(properties::MinimumRequests).value(); + + for (const auto &cfg : *config_) { + Stream *stream = cfg.stream(); + + int allocatedCount = allocator_.allocate(stream, count); + ASSERT_GE(allocatedCount, 0) << "Failed to allocate buffers"; + EXPECT_EQ(allocatedCount, count) << "Allocated less buffers than expected"; + } + + requestCount_ = count; +} + void Capture::run(unsigned int captureLimit, std::optional queueLimit) { assert(!queueLimit || captureLimit <= *queueLimit); @@ -103,19 +121,16 @@ void Capture::start() { assert(config_); assert(!config_->empty()); - assert(!allocator_.allocated()); + assert(allocator_.allocated()); assert(requests_.empty()); - unsigned int bufferCount = - camera_->properties().get(properties::MinimumRequests).value(); - /* No point in testing less requests then the camera depth. */ - if (queueLimit_ && *queueLimit_ < bufferCount) { - GTEST_SKIP() << "Camera needs " << bufferCount + if (queueLimit_ && *queueLimit_ < requestCount_) { + GTEST_SKIP() << "Camera needs " << requestCount_ << " requests, can't test only " << *queueLimit_; } - for (std::size_t i = 0; i < bufferCount; i++) { + for (std::size_t i = 0; i < requestCount_; i++) { std::unique_ptr request = camera_->createRequest(); ASSERT_TRUE(request) << "Can't create request"; requests_.push_back(std::move(request)); @@ -124,13 +139,10 @@ void Capture::start() for (const auto &cfg : *config_) { Stream *stream = cfg.stream(); - int count = allocator_.allocate(stream, bufferCount); - ASSERT_GE(count, 0) << "Failed to allocate buffers"; - const auto &buffers = allocator_.buffers(stream); - ASSERT_EQ(buffers.size(), bufferCount) << "Mismatching buffer count"; + ASSERT_EQ(buffers.size(), requestCount_) << "Mismatching buffer count"; - for (std::size_t i = 0; i < bufferCount; i++) { + for (std::size_t i = 0; i < requestCount_; i++) { ASSERT_EQ(requests_[i]->addBuffer(stream, buffers[i].get()), 0) << "Failed to add buffer to request"; } diff --git a/src/apps/lc-compliance/helpers/capture.h b/src/apps/lc-compliance/helpers/capture.h index ea01c11c..2a14a721 100644 --- a/src/apps/lc-compliance/helpers/capture.h +++ b/src/apps/lc-compliance/helpers/capture.h @@ -21,6 +21,7 @@ public: ~Capture(); void configure(libcamera::Span roles); + void allocateBuffers(unsigned int count = 0); void run(unsigned int captureLimit, std::optional queueLimit = {}); private: @@ -42,4 +43,5 @@ private: std::optional queueLimit_; unsigned int captureCount_ = 0; unsigned int queueCount_ = 0; + unsigned int requestCount_ = 0; }; diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp index d02caa8a..9fb82a2b 100644 --- a/src/apps/lc-compliance/tests/capture_test.cpp +++ b/src/apps/lc-compliance/tests/capture_test.cpp @@ -83,6 +83,8 @@ TEST_P(SimpleCapture, Capture) capture.configure(roles); + capture.allocateBuffers(); + capture.run(numRequests, numRequests); } @@ -102,8 +104,10 @@ TEST_P(SimpleCapture, CaptureStartStop) capture.configure(roles); - for (unsigned int starts = 0; starts < numRepeats; starts++) + for (unsigned int starts = 0; starts < numRepeats; starts++) { + capture.allocateBuffers(); capture.run(numRequests, numRequests); + } } /* @@ -121,6 +125,8 @@ TEST_P(SimpleCapture, UnbalancedStop) capture.configure(roles); + capture.allocateBuffers(); + capture.run(numRequests); } From patchwork Mon Apr 28 09:02:38 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23285 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 E3128C3326 for ; Mon, 28 Apr 2025 09:05:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1B4C068ADE; Mon, 28 Apr 2025 11:05:33 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6575168B27 for ; Mon, 28 Apr 2025 11:05:08 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQF-0001au-VH; Mon, 28 Apr 2025 11:05:08 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Laurent Pinchart , Paul Elder , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 13/19] lc-compliance: Move camera setup to CameraHolder class Date: Mon, 28 Apr 2025 11:02:38 +0200 Message-ID: <20250428090413.38234-14-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 Signed-off-by: Sven Püschel --- Changes in v11: - rebased Changes in v9: - rebased Changes in v8: - Moved CameraHolder to new test_base.{cpp,h} files Changes in v5: - New --- src/apps/lc-compliance/meson.build | 1 + src/apps/lc-compliance/test_base.cpp | 28 +++++++++++++++++++ src/apps/lc-compliance/test_base.h | 24 ++++++++++++++++ src/apps/lc-compliance/tests/capture_test.cpp | 18 +++--------- 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/meson.build b/src/apps/lc-compliance/meson.build index b1f605f3..80b9a160 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', 'helpers/capture.cpp', 'main.cpp', + 'test_base.cpp', 'tests/capture_test.cpp', ]) 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__ */ diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp index 9fb82a2b..24081fef 100644 --- a/src/apps/lc-compliance/tests/capture_test.cpp +++ b/src/apps/lc-compliance/tests/capture_test.cpp @@ -15,13 +15,13 @@ #include -#include "environment.h" +#include "test_base.h" namespace { using namespace libcamera; -class SimpleCapture : public testing::TestWithParam, int>> +class SimpleCapture : public testing::TestWithParam, int>>, public CameraHolder { public: static std::string nameParameters(const testing::TestParamInfo &info); @@ -29,8 +29,6 @@ public: protected: void SetUp() override; void TearDown() override; - - std::shared_ptr camera_; }; /* @@ -39,20 +37,12 @@ protected: */ void SimpleCapture::SetUp() { - Environment *env = Environment::get(); - - camera_ = env->cm()->get(env->cameraId()); - - ASSERT_EQ(camera_->acquire(), 0); + acquireCamera(); } void SimpleCapture::TearDown() { - if (!camera_) - return; - - camera_->release(); - camera_.reset(); + releaseCamera(); } std::string SimpleCapture::nameParameters(const testing::TestParamInfo &info) From patchwork Mon Apr 28 09:02:39 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23286 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 1171BC327D for ; Mon, 28 Apr 2025 09:05:35 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 73CB868ADE; Mon, 28 Apr 2025 11:05:35 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9EDBE68B29 for ; Mon, 28 Apr 2025 11:05:08 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQG-0001au-4A; Mon, 28 Apr 2025 11:05:08 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Paul Elder Subject: [PATCH v11 14/19] lc-compliance: Add test to queue more requests than hardware depth Date: Mon, 28 Apr 2025 11:02:39 +0200 Message-ID: <20250428090413.38234-15-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 v11: - Removed Capture class overloading - Adapted to also handle multiple roles 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/tests/capture_test.cpp | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp index 24081fef..3aa82858 100644 --- a/src/apps/lc-compliance/tests/capture_test.cpp +++ b/src/apps/lc-compliance/tests/capture_test.cpp @@ -21,6 +21,8 @@ namespace { using namespace libcamera; +static constexpr unsigned int MAX_SIMULTANEOUS_REQUESTS = 8; + class SimpleCapture : public testing::TestWithParam, int>>, public CameraHolder { public: @@ -58,6 +60,36 @@ std::string SimpleCapture::nameParameters(const testing::TestParamInfo>, 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 RoleParametrizedTest::nameParameters(const testing::TestParamInfo &info) +{ + std::ostringstream ss; + + for (StreamRole r : info.param) + ss << r << '_'; + + return ss.str(); +} + /* * Test single capture cycles * @@ -120,6 +152,19 @@ TEST_P(SimpleCapture, UnbalancedStop) capture.run(numRequests); } +TEST_P(RoleParametrizedTest, Overflow) +{ + auto roles = GetParam(); + + Capture capture(camera_); + + capture.configure(roles); + + capture.allocateBuffers(MAX_SIMULTANEOUS_REQUESTS); + + capture.run(MAX_SIMULTANEOUS_REQUESTS, MAX_SIMULTANEOUS_REQUESTS); +} + const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 }; const std::vector SINGLEROLES[] = { @@ -148,4 +193,14 @@ INSTANTIATE_TEST_SUITE_P(MultiStream, testing::ValuesIn(NUMREQUESTS)), SimpleCapture::nameParameters); +INSTANTIATE_TEST_SUITE_P(SingleStream, + RoleParametrizedTest, + testing::ValuesIn(SINGLEROLES), + RoleParametrizedTest::nameParameters); + +INSTANTIATE_TEST_SUITE_P(MultiStream, + RoleParametrizedTest, + testing::ValuesIn(MULTIROLES), + RoleParametrizedTest::nameParameters); + } /* namespace */ From patchwork Mon Apr 28 09:02:40 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23287 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 44B9CC3327 for ; Mon, 28 Apr 2025 09:05:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6BCDC68B27; Mon, 28 Apr 2025 11:05:37 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C56AE68B24 for ; Mon, 28 Apr 2025 11:05:08 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQG-0001au-Aa; Mon, 28 Apr 2025 11:05:08 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Paul Elder , Umang Jain , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 15/19] lc-compliance: Add test to ensure MinimumRequests is valid Date: Mon, 28 Apr 2025 11:02:40 +0200 Message-ID: <20250428090413.38234-16-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , 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 Signed-off-by: Sven Püschel --- Changes in v11: - rebased 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 80b9a160..fb020b48 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', 'helpers/capture.cpp', 'main.cpp', + 'property_test.cpp', 'test_base.cpp', 'tests/capture_test.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__ */ From patchwork Mon Apr 28 09:02:41 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23288 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 731EEC331E for ; Mon, 28 Apr 2025 09:05:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9752768B31; Mon, 28 Apr 2025 11:05:38 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E6F2568ACF for ; Mon, 28 Apr 2025 11:05:08 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQG-0001au-Fs; Mon, 28 Apr 2025 11:05:08 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 16/19] libcamera: pipeline: uvcvideo: Add internal request queue Date: Mon, 28 Apr 2025 11:02:41 +0200 Message-ID: <20250428090413.38234-17-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Add an internal queue that stores requests until there are V4L2 buffer slots available. This avoids the need to cancel requests when there is a shortage of said buffers. Signed-off-by: Nícolas F. R. A. Prado Signed-off-by: Sven Püschel --- Changes in v11: - Added from https://lists.libcamera.org/pipermail/libcamera-devel/2021-September/024121.html --- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 103 +++++++++++++++---- 1 file changed, 82 insertions(+), 21 deletions(-) diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index c3b718f9..52659fa2 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -29,6 +30,7 @@ #include "libcamera/internal/camera.h" #include "libcamera/internal/device_enumerator.h" +#include "libcamera/internal/framebuffer.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/sysfs.h" @@ -53,18 +55,28 @@ public: const std::string &id() const { return id_; } + void queuePendingRequests(); + void cancelPendingRequests(); + + void setAvailableBufferSlotCount(unsigned int count) { availableBufferSlotCount_ = count; } + Mutex openLock_; std::unique_ptr video_; Stream stream_; std::map> formats_; + std::queue pendingRequests_; std::optional autoExposureMode_; std::optional manualExposureMode_; private: bool generateId(); + int processControl(ControlList *controls, unsigned int id, + const ControlValue &value); + int processControls(Request *request); std::string id_; + unsigned int availableBufferSlotCount_; }; class UVCCameraConfiguration : public CameraConfiguration @@ -98,10 +110,6 @@ public: bool match(DeviceEnumerator *enumerator) override; private: - int processControl(const UVCCameraData *data, ControlList *controls, - unsigned int id, const ControlValue &value); - int processControls(UVCCameraData *data, Request *request); - bool acquireDevice(Camera *camera) override; void releaseDevice(Camera *camera) override; @@ -295,6 +303,8 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList if (ret < 0) return ret; + data->setAvailableBufferSlotCount(kUVCBufferSlotCount); + ret = data->video_->streamOn(); if (ret < 0) { data->video_->releaseBuffers(); @@ -308,11 +318,12 @@ void PipelineHandlerUVC::stopDevice(Camera *camera) { UVCCameraData *data = cameraData(camera); data->video_->streamOff(); + data->cancelPendingRequests(); data->video_->releaseBuffers(); } -int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *controls, - unsigned int id, const ControlValue &value) +int UVCCameraData::processControl(ControlList *controls, unsigned int id, + const ControlValue &value) { uint32_t cid; @@ -362,10 +373,10 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c switch (value.get()) { case controls::ExposureTimeModeAuto: - mode = data->autoExposureMode_; + mode = autoExposureMode_; break; case controls::ExposureTimeModeManual: - mode = data->manualExposureMode_; + mode = manualExposureMode_; break; } @@ -409,19 +420,19 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c return 0; } -int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) +int UVCCameraData::processControls(Request *request) { - ControlList controls(data->video_->controls()); + ControlList controls(video_->controls()); for (const auto &[id, value] : request->controls()) - processControl(data, &controls, id, value); + processControl(&controls, id, value); for (const auto &ctrl : controls) LOG(UVC, Debug) << "Setting control " << utils::hex(ctrl.first) << " to " << ctrl.second.toString(); - int ret = data->video_->setControls(&controls); + int ret = video_->setControls(&controls); if (ret) { LOG(UVC, Error) << "Failed to set controls: " << ret; return ret < 0 ? ret : -EINVAL; @@ -433,21 +444,16 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request) { UVCCameraData *data = cameraData(camera); - FrameBuffer *buffer = request->findBuffer(&data->stream_); - if (!buffer) { + + if (!request->findBuffer(&data->stream_)) { LOG(UVC, Error) << "Attempt to queue request with invalid stream"; return -ENOENT; } - int ret = processControls(data, request); - if (ret < 0) - return ret; - - ret = data->video_->queueBuffer(buffer); - if (ret < 0) - return ret; + data->pendingRequests_.push(request); + data->queuePendingRequests(); return 0; } @@ -882,6 +888,61 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer) pipe()->completeBuffer(request, buffer); pipe()->completeRequest(request); + + availableBufferSlotCount_++; + + queuePendingRequests(); +} + +void UVCCameraData::queuePendingRequests() +{ + while (!pendingRequests_.empty() && availableBufferSlotCount_) { + Request *request = pendingRequests_.front(); + FrameBuffer *buffer = request->findBuffer(&stream_); + + int ret = processControls(request); + if (ret < 0) { + LOG(UVC, Error) << "Failed to process controls with" + << " error " << ret << ". Cancelling" + << " buffer."; + buffer->_d()->cancel(); + pipe()->completeBuffer(request, buffer); + pipe()->completeRequest(request); + pendingRequests_.pop(); + + continue; + } + + ret = video_->queueBuffer(buffer); + if (ret < 0) { + LOG(UVC, Error) << "Failed to queue buffer with error " + << ret << ". Cancelling buffer."; + buffer->_d()->cancel(); + pipe()->completeBuffer(request, buffer); + pipe()->completeRequest(request); + pendingRequests_.pop(); + + continue; + } + + availableBufferSlotCount_--; + + pendingRequests_.pop(); + } +} + +void UVCCameraData::cancelPendingRequests() +{ + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); + FrameBuffer *buffer = request->findBuffer(&stream_); + + buffer->_d()->cancel(); + pipe()->completeBuffer(request, buffer); + pipe()->completeRequest(request); + + pendingRequests_.pop(); + } } REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC, "uvcvideo") From patchwork Mon Apr 28 09:02:42 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23290 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 95B74C3329 for ; Mon, 28 Apr 2025 09:05:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9E39768B41; Mon, 28 Apr 2025 11:05:40 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 021B668B2A for ; Mon, 28 Apr 2025 11:05:08 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQG-0001au-Lm; Mon, 28 Apr 2025 11:05:08 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Laurent Pinchart , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 17/19] libcamera: pipeline: rkisp1: Add internal request queue Date: Mon, 28 Apr 2025 11:02:42 +0200 Message-ID: <20250428090413.38234-18-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Add an internal queue that stores requests until there are internal buffers and V4L2 buffer slots available. This avoids the need to cancel requests when there is a shortage of said buffers. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Laurent Pinchart Signed-off-by: Sven Püschel --- Changes in v11: - Added from https://lists.libcamera.org/pipermail/libcamera-devel/2021-September/024121.html --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 81 ++++++++++++++++++------ 1 file changed, 62 insertions(+), 19 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 623bcfe5..d48da4e2 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -101,6 +101,9 @@ public: const PipelineHandlerRkISP1 *pipe() const; int loadIPA(unsigned int hwRevision); + void queuePendingRequests(); + void cancelPendingRequests(); + Stream mainPathStream_; Stream selfPathStream_; std::unique_ptr sensor_; @@ -115,6 +118,7 @@ public: std::unique_ptr ipa_; ControlInfoMap ipaControls_; + std::queue pendingRequests_; private: void paramsComputed(unsigned int frame, unsigned int bytesused); @@ -254,12 +258,12 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req if (!isRaw) { if (pipe_->availableParamBuffers_.empty()) { - LOG(RkISP1, Error) << "Parameters buffer underrun"; + LOG(RkISP1, Debug) << "Parameters buffer underrun"; return nullptr; } if (pipe_->availableStatBuffers_.empty()) { - LOG(RkISP1, Error) << "Statistic buffer underrun"; + LOG(RkISP1, Debug) << "Statistic buffer underrun"; return nullptr; } @@ -447,6 +451,56 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta pipe()->tryCompleteRequest(info); } +void RkISP1CameraData::queuePendingRequests() +{ + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); + + /* + * If there aren't internal buffers available, we break and try + * again later. If there are, we're guaranteed to also have V4L2 + * buffer slots available to queue the request, since we should + * always have more (or equal) buffer slots than internal + * buffers. + */ + RkISP1FrameInfo *info = frameInfo_.create(this, request, pipe()->isRaw_); + if (!info) + break; + + ipa_->queueRequest(frame_, request->controls()); + if (pipe()->isRaw_) { + if (info->mainPathBuffer) + mainPath_->queueBuffer(info->mainPathBuffer); + + if (selfPath_ && info->selfPathBuffer) + selfPath_->queueBuffer(info->selfPathBuffer); + } else { + ipa_->computeParams(frame_, info->paramBuffer->cookie()); + } + + frame_++; + + pendingRequests_.pop(); + } +} + +void RkISP1CameraData::cancelPendingRequests() +{ + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); + + for (auto it : request->buffers()) { + FrameBuffer *buffer = it.second; + buffer->_d()->cancel(); + pipe()->completeBuffer(request, buffer); + } + + pipe()->completeRequest(request); + + pendingRequests_.pop(); + } +} + /* ----------------------------------------------------------------------------- * Camera Configuration */ @@ -1168,6 +1222,8 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera) dewarper_->stop(); } + data->cancelPendingRequests(); + ASSERT(data->queuedRequests_.empty()); data->frameInfo_.clear(); @@ -1180,23 +1236,8 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request) { RkISP1CameraData *data = cameraData(camera); - RkISP1FrameInfo *info = data->frameInfo_.create(data, request, isRaw_); - if (!info) - return -ENOENT; - - data->ipa_->queueRequest(data->frame_, request->controls()); - if (isRaw_) { - if (info->mainPathBuffer) - data->mainPath_->queueBuffer(info->mainPathBuffer); - - if (data->selfPath_ && info->selfPathBuffer) - data->selfPath_->queueBuffer(info->selfPathBuffer); - } else { - data->ipa_->computeParams(data->frame_, - info->paramBuffer->cookie()); - } - - data->frame_++; + data->pendingRequests_.push(request); + data->queuePendingRequests(); return 0; } @@ -1481,6 +1522,8 @@ void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info) data->frameInfo_.destroy(info->frame); completeRequest(request); + + data->queuePendingRequests(); } void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) From patchwork Mon Apr 28 09:02:43 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23289 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 9004AC3328 for ; Mon, 28 Apr 2025 09:05:40 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 885EC68B3E; Mon, 28 Apr 2025 11:05:39 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 354A068B2B for ; Mon, 28 Apr 2025 11:05:09 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQG-0001au-Qr; Mon, 28 Apr 2025 11:05:08 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 18/19] Documentation: guides: pipeline-handler: Document internal queue pattern Date: Mon, 28 Apr 2025 11:02:43 +0200 Message-ID: <20250428090413.38234-19-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Pipeline handlers need to implement a common pattern of adding queued requests to an internal queue and queuing them to the video devices as the buffer slots there become available. Add this pattern to the vivid example in the pipeline-handler guide so it's clear that new pipeline handlers should also implement it. Signed-off-by: Nícolas F. R. A. Prado Signed-off-by: Sven Püschel --- Changes in v11: - Added from https://lists.libcamera.org/pipermail/libcamera-devel/2021-September/024121.html --- Documentation/guides/pipeline-handler.rst | 129 +++++++++++++++++----- 1 file changed, 99 insertions(+), 30 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 725e35e6..a42d1aca 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -452,11 +452,27 @@ it will be used: int init(); void bufferReady(FrameBuffer *buffer); + void queuePendingRequests(); + void cancelPendingRequests(); + + void setAvailableBufferSlotCount(unsigned int count) { availableBufferSlotCount_ = count; } + MediaDevice *media_; V4L2VideoDevice *video_; Stream stream_; + + std::queue pendingRequests_; + + private: + unsigned int availableBufferSlotCount_; }; +And the following include needs to be added: + +.. code-block:: cpp + + #include + This example pipeline handler handles a single video device and supports a single stream, represented by the ``VividCameraData`` class members. More complex pipeline handlers might register cameras composed of several video @@ -1274,7 +1290,8 @@ algorithms, or other devices you should also stop them. Of course we also need to handle the corresponding actions to stop streaming on a device, Add the following to the ``stop`` function, to stop the stream with -the `streamOff`_ function and release all buffers. +the `streamOff`_ function and release all buffers. Additionally we need to +cancel pending requests, the reason will become clearer in the next section. .. _streamOff: https://libcamera.org/api-html/classlibcamera_1_1V4L2VideoDevice.html#a61998710615bdf7aa25a046c8565ed66 @@ -1282,6 +1299,7 @@ the `streamOff`_ function and release all buffers. VividCameraData *data = cameraData(camera); data->video_->streamOff(); + data->cancelPendingRequests(); data->video_->releaseBuffers(); Queuing requests between applications and hardware @@ -1302,25 +1320,76 @@ directly with the `queueBuffer`_ function provided by the V4L2VideoDevice. .. _findBuffer: https://libcamera.org/api-html/classlibcamera_1_1Request.html#ac66050aeb9b92c64218945158559c4d4 .. _queueBuffer: https://libcamera.org/api-html/classlibcamera_1_1V4L2VideoDevice.html#a594cd594686a8c1cf9ae8dba0b2a8a75 +Since the pipeline handler has a finite number of buffer slots allocated through +importBuffers(), in order to not drop frames when more requests than that are +queued by the application, we need to implement a queue to hold the requests and +queue them to the devices as slots become available. + Replace the stubbed contents of ``queueRequestDevice`` with the following: .. code-block:: cpp VividCameraData *data = cameraData(camera); - FrameBuffer *buffer = request->findBuffer(&data->stream_); - if (!buffer) { + if (!request->findBuffer(&data->stream_)) { LOG(VIVID, Error) << "Attempt to queue request with invalid stream"; return -ENOENT; } - int ret = data->video_->queueBuffer(buffer); - if (ret < 0) - return ret; + data->pendingRequests_.push(request); + data->queuePendingRequests(); return 0; +Now create the ``queuePendingRequests`` function for ``VividCameraData`` and +inside it add: + +.. code-block:: cpp + + while (!pendingRequests_.empty() && availableBufferSlotCount_) { + Request *request = pendingRequests_.front(); + FrameBuffer *buffer = request->findBuffer(&stream_); + + ret = video_->queueBuffer(buffer); + if (ret < 0) { + LOG(UVC, Error) << "Failed to queue buffer with error " + << ret << ". Cancelling buffer."; + buffer->_d()->cancel(); + pipe()->completeBuffer(request, buffer); + pipe()->completeRequest(request); + pendingRequests_.pop(); + + continue; + } + + availableBufferSlotCount_--; + + pendingRequests_.pop(); + } + +Create ``cancelPendingRequests`` as well with: + +.. code-block:: cpp + + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); + FrameBuffer *buffer = request->findBuffer(&stream_); + + buffer->_d()->cancel(); + pipe()->completeBuffer(request, buffer); + pipe()->completeRequest(request); + + pendingRequests_.pop(); + } + +Finally we need to initialize the buffer slot count in ``start()``. Add the +following line there: + +.. code-block:: cpp + + data->setAvailableBufferSlotCount(count); + Processing controls ~~~~~~~~~~~~~~~~~~~ @@ -1395,31 +1464,27 @@ where appropriate by setting controls on V4L2Subdevices directly. Each pipeline handler is responsible for understanding the correct procedure for applying controls to the device they support. -This example pipeline handler applies controls during the `queueRequestDevice`_ -function for each request, and applies them to the capture device through the -capture node. - -.. _queueRequestDevice: https://libcamera.org/api-html/classlibcamera_1_1PipelineHandler.html#a106914cca210640c9da9ee1f0419e83c +This example pipeline handler applies controls as soon as the request is queued +to the device from ``queuePendingRequests``, and applies them to the capture +device through the capture node. -In the ``queueRequestDevice`` function, replace the following: +In the ``queuePendingRequests`` function, before the line ``ret = +video_->queueBuffer(buffer);``, add the following: .. code-block:: cpp - int ret = data->video_->queueBuffer(buffer); - if (ret < 0) - return ret; - -With the following code: - -.. code-block:: cpp - - int ret = processControls(data, request); - if (ret < 0) - return ret; - - ret = data->video_->queueBuffer(buffer); - if (ret < 0) - return ret; + int ret = processControls(request); + if (ret < 0) { + LOG(UVC, Error) << "Failed to process controls with" + << " error " << ret << ". Cancelling" + << " buffer."; + buffer->_d()->cancel(); + pipe()->completeBuffer(request, buffer); + pipe()->completeRequest(request); + pendingRequests_.pop(); + + continue; + } We also need to add the following include directive to support the control value translation operations: @@ -1483,9 +1548,10 @@ VividCameradata::init() implementation. The ``bufferReady`` function obtains the request from the buffer using the ``request`` function, and notifies the ``Camera`` that the buffer and request are completed. In this simpler pipeline handler, there is only one -stream, so it completes the request immediately. You can find a more complex -example of event handling with supporting multiple streams in the libcamera -code-base. +stream, so it completes the request immediately. It also updates the number of +available buffer slots and tries to queue any pending requests. You can find a +more complex example of event handling with support to multiple streams in the +libcamera code-base. .. TODO: Add link @@ -1497,6 +1563,9 @@ code-base. pipe_->completeBuffer(request, buffer); pipe_->completeRequest(request); + + availableBufferSlotCount_++; + queuePendingRequests(); } Testing a pipeline handler From patchwork Mon Apr 28 09:02:44 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Sven_P=C3=BCschel?= X-Patchwork-Id: 23291 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 A5F62C332A for ; Mon, 28 Apr 2025 09:05:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C94CB68B3E; Mon, 28 Apr 2025 11:05:41 +0200 (CEST) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4995868B2D for ; Mon, 28 Apr 2025 11:05:09 +0200 (CEST) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=peter.guest.stw.pengutronix.de) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1u9KQG-0001au-VT; Mon, 28 Apr 2025 11:05:09 +0200 From: =?utf-8?q?Sven_P=C3=BCschel?= To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v11 19/19] libcamera: v4l2_videodevice: Log buffer count on error Date: Mon, 28 Apr 2025 11:02:44 +0200 Message-ID: <20250428090413.38234-20-s.pueschel@pengutronix.de> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250428090413.38234-1-s.pueschel@pengutronix.de> References: <20250428090413.38234-1-s.pueschel@pengutronix.de> MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: s.pueschel@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Log the actual and requested buffer count in the v4l2 error, when not the requested buffer count could was allocated by V4L2. Signed-off-by: Sven Püschel Reviewed-by: Barnabás Pőcze --- Changes in v11: - Added --- src/libcamera/v4l2_videodevice.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index f5b3fa09..d6f8c3cd 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1326,7 +1326,8 @@ int V4L2VideoDevice::requestBuffers(unsigned int count, if (rb.count < count) { LOG(V4L2, Error) - << "Not enough buffers provided by V4L2VideoDevice"; + << "Not enough buffers provided by V4L2VideoDevice. Wanted: " + << count << ", got: " << rb.count; requestBuffers(0, memoryType); return -ENOMEM; }