{"id":23273,"url":"https://patchwork.libcamera.org/api/patches/23273/?format=json","web_url":"https://patchwork.libcamera.org/patch/23273/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20250428090413.38234-2-s.pueschel@pengutronix.de>","date":"2025-04-28T09:02:26","name":"[v11,01/19] libcamera: property: Add MinimumRequests property","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"15d62258e7e5833dbef4157169d1284c4dda0213","submitter":{"id":225,"url":"https://patchwork.libcamera.org/api/people/225/?format=json","name":"Sven Püschel","email":"s.pueschel@pengutronix.de"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/23273/mbox/","series":[{"id":5148,"url":"https://patchwork.libcamera.org/api/series/5148/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=5148","date":"2025-04-28T09:02:25","name":"lc-compliance: Add test to queue more requests than hardware depth","version":11,"mbox":"https://patchwork.libcamera.org/series/5148/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/23273/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/23273/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CC48CC327D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Apr 2025 09:05:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5CEBB68B26;\n\tMon, 28 Apr 2025 11:05:08 +0200 (CEST)","from metis.whiteo.stw.pengutronix.de\n\t(metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D527668ACF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Apr 2025 11:05:05 +0200 (CEST)","from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77]\n\thelo=peter.guest.stw.pengutronix.de)\n\tby metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92)\n\t(envelope-from <s.pueschel@pengutronix.de>)\n\tid 1u9KQD-0001au-Hp; Mon, 28 Apr 2025 11:05:05 +0200"],"From":"=?utf-8?q?Sven_P=C3=BCschel?= <s.pueschel@pengutronix.de>","To":"libcamera-devel@lists.libcamera.org","Cc":"=?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= <nfraprado@collabora.com>,\n\tPaul Elder <paul.elder@ideasonboard.com>, \n\tUmang Jain <umang.jain@ideasonboard.com>, =?utf-8?q?Sven_P=C3=BCschel?=\n\t<s.pueschel@pengutronix.de>","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","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","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);\n\tSAEximRunCond 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"From: Nícolas F. R. A. Prado <nfraprado@collabora.com>\n\nThe MinimumRequests property reports the minimum number of capture\nrequests that the camera pipeline requires to have queued in order to\nsustain capture operations without frame drops. At this quantity,\nthere's no guarantee that manual per-frame controls will apply\ncorrectly. The quantity needed for that may be added as a separate\nproperty in the future.\n\nThe mali-c55 driver defines min_queued_buffers = 1 [1]. Therefore set\nset the minimum requests to 2 to account one request in the userspace.\n\nThe dcmipp and j721e drivers both defines min_queued_buffers = 1\nin the kernel under\ndrivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-bytecap.c\nand drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c .\nTherefore use these values, as they are added 1 more.\n\nFor the intel-ipu6, mtk-seninf simple devices and the virtual pipeline\nuse a conservative value of 3 minimumBuffers, as no further requirements\nare known about them.\n\n[1] https://lore.kernel.org/linux-media/20240131174355.GB20792@pendragon.ideasonboard.com/T/#t\n\nSigned-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\nAcked-by: Umang Jain <umang.jain@ideasonboard.com>\nSigned-off-by: Sven Püschel <s.pueschel@pengutronix.de>\n\n---\nChanges in v11\n- Add mali-c55, dcmipp, virtual, j721e, intel-ipu6 and mtk-seninf\n- Hold only the minimum buffers instead of the whole deviceInfo pointer in SimpleCameraData\n- Adjusted min_buffers_needed property to min_reqbufs_allocation in docs\n- Relax property description from no frame drops -> only minimal frame\n  drops, based on the comment from Naush [10]\n- Changed property type from int32_t -> uint32_t to match all of the\n  indirect casts to an unsigned value throughout the existing patchset.\n- Removed Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n- Removed Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n[10] https://lists.libcamera.org/pipermail/libcamera-devel/2022-December/035872.html\n\nChanges in v10:\n- ipu3: add a constant to populate MinimumRequests, as we'll also use it\n  elsewhere\n- update pipeline handler guide to set vivid' MinimumRequests to 2\n- expand the MinimumRequests property description to include a line\n  about startup\n- add isi\n\nChanges in v9:\n- rebased\n\nChanges in v8:\n- Changed the MinimumRequests property meaning to require that frames aren't\n  dropped\n- Set MinimumRequests on SimplePipeline depending on device and usage of\n  converter\n- Undid definition of default MinimumRequests on CameraSensor constructor\n- Updated pipeline-handler guides with new MinimumRequests property\n\nChanges in v7:\n- Renamed property from MinNumRequests to MinimumRequests\n- Changed MinimumRequests property's description\n\nChanges in v6:\n- Removed comment from Raspberrypi MinNumRequests setting\n- Removed an extra blank line\n---\n Documentation/guides/pipeline-handler.rst     | 20 ++++++---\n src/libcamera/pipeline/imx8-isi/imx8-isi.cpp  |  2 +\n src/libcamera/pipeline/ipu3/ipu3.cpp          |  4 ++\n src/libcamera/pipeline/mali-c55/mali-c55.cpp  |  1 +\n src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  1 +\n .../pipeline/rpi/common/pipeline_base.cpp     |  2 +\n src/libcamera/pipeline/simple/simple.cpp      | 41 +++++++++++++++----\n src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  2 +\n src/libcamera/pipeline/vimc/vimc.cpp          |  2 +\n src/libcamera/pipeline/virtual/virtual.cpp    |  1 +\n src/libcamera/property_ids_core.yaml          | 22 ++++++++++\n 11 files changed, 85 insertions(+), 13 deletions(-)","diff":"diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\nindex 9a15c20a..1079d998 100644\n--- a/Documentation/guides/pipeline-handler.rst\n+++ b/Documentation/guides/pipeline-handler.rst\n@@ -663,19 +663,29 @@ associated with immutable values, which represent static characteristics that ca\n be used by applications to identify camera devices in the system. Properties can be\n registered by inspecting the values of V4L2 controls from the video devices and\n camera sensor (for example to retrieve the position and orientation of a camera)\n-or to express other immutable characteristics. The example pipeline handler does\n-not register any property, but examples are available in the libcamera code\n-base.\n+or to express other immutable characteristics.\n \n-.. TODO: Add a property example to the pipeline handler. At least the model.\n+A required property is ``MinimumRequests``, which indicates how many requests\n+need to be queued in the pipeline for capture without frame drops to be\n+possible.\n+\n+In our case, the vivid driver requires two buffers before it'll start streaming\n+(can be seen in the ``min_reqbufs_allocation`` property for the ``vid_cap`` queue in\n+vivid's driver code). Therefore we will set our ``MinimumRequests`` to two.\n+Append the following line to ``init()``:\n+\n+.. code-block:: cpp\n+\n+   properties_.set(properties::MinimumRequests, 2);\n \n At this point you need to add the following includes to the top of the file for\n-handling controls:\n+handling controls and properties:\n \n .. code-block:: cpp\n \n    #include <libcamera/controls.h>\n    #include <libcamera/control_ids.h>\n+   #include <libcamera/property_ids.h>\n \n Vendor-specific controls and properties\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\ndiff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\nindex ecda426a..2b8a583e 100644\n--- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n+++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n@@ -18,6 +18,7 @@\n #include <libcamera/camera_manager.h>\n #include <libcamera/formats.h>\n #include <libcamera/geometry.h>\n+#include <libcamera/property_ids.h>\n #include <libcamera/stream.h>\n \n #include \"libcamera/internal/bayer_format.h\"\n@@ -165,6 +166,7 @@ int ISICameraData::init()\n \t\treturn ret;\n \n \tproperties_ = sensor_->properties();\n+\tproperties_.set(properties::MinimumRequests, 2);\n \n \treturn 0;\n }\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex e31e3879..356ca2e1 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -168,6 +168,8 @@ private:\n \tMediaDevice *imguMediaDev_;\n \n \tstd::vector<IPABuffer> ipaBuffers_;\n+\n+\tstatic constexpr unsigned int kMinimumRequests = 3;\n };\n \n IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data)\n@@ -1074,6 +1076,8 @@ int PipelineHandlerIPU3::registerCameras()\n \t\t/* Initialize the camera properties. */\n \t\tdata->properties_ = cio2->sensor()->properties();\n \n+\t\tdata->properties_.set(properties::MinimumRequests, kMinimumRequests);\n+\n \t\tret = initControls(data.get());\n \t\tif (ret)\n \t\t\tcontinue;\ndiff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\nindex a05e11fc..f0ab3d2e 100644\n--- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n+++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n@@ -1602,6 +1602,7 @@ bool PipelineHandlerMaliC55::registerSensorCamera(MediaLink *ispLink)\n \t\t\treturn false;\n \n \t\tdata->properties_ = data->sensor_->properties();\n+\t\tdata->properties_.set(properties::MinimumRequests, 2);\n \n \t\tconst CameraSensorProperties::SensorDelays &delays = data->sensor_->sensorDelays();\n \t\tstd::unordered_map<uint32_t, DelayedControls::ControlParams> params = {\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 194dfce7..5e3a4dba 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -1318,6 +1318,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n \n \t/* Initialize the camera properties. */\n \tdata->properties_ = data->sensor_->properties();\n+\tdata->properties_.set(properties::MinimumRequests, 3);\n \n \tscalerMaxCrop_ = Rectangle(data->sensor_->resolution());\n \ndiff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\nindex 1f13e523..ef51d530 100644\n--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n@@ -848,6 +848,8 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera\n \t */\n \tdata->properties_.set(properties::ScalerCropMaximum, Rectangle{});\n \n+\tdata->properties_.set(properties::MinimumRequests, 3);\n+\n \tret = platformRegister(cameraData, frontend, backend);\n \tif (ret)\n \t\treturn ret;\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex efb07051..7abf0fc9 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -26,6 +26,7 @@\n \n #include <libcamera/camera.h>\n #include <libcamera/control_ids.h>\n+#include <libcamera/property_ids.h>\n #include <libcamera/request.h>\n #include <libcamera/stream.h>\n \n@@ -233,6 +234,10 @@ SimpleFrameInfo *SimpleFrames::find(uint32_t frame)\n \n struct SimplePipelineInfo {\n \tconst char *driver;\n+\t/*\n+\t * Minimum number of buffers required by the driver to start streaming.\n+\t */\n+\tunsigned int minimumBuffers;\n \t/*\n \t * Each converter in the list contains the name\n \t * and the number of streams it supports.\n@@ -249,14 +254,14 @@ struct SimplePipelineInfo {\n namespace {\n \n static const SimplePipelineInfo supportedDevices[] = {\n-\t{ \"dcmipp\", {}, false },\n-\t{ \"imx7-csi\", { { \"pxp\", 1 } }, false },\n-\t{ \"intel-ipu6\", {}, true },\n-\t{ \"j721e-csi2rx\", {}, true },\n-\t{ \"mtk-seninf\", { { \"mtk-mdp\", 3 } }, false },\n-\t{ \"mxc-isi\", {}, false },\n-\t{ \"qcom-camss\", {}, true },\n-\t{ \"sun6i-csi\", {}, false },\n+\t{ \"dcmipp\", 1, {}, false },\n+\t{ \"imx7-csi\", 2, { { \"pxp\", 1 } }, false },\n+\t{ \"intel-ipu6\", 3, {}, true }, // \\todo Check if the minimumBuffers can be lowered\n+\t{ \"j721e-csi2rx\", 1, {}, true },\n+\t{ \"mtk-seninf\", 3, { { \"mtk-mdp\", 3 } }, false }, // \\todo Check if the minimumBuffers can be lowered\n+\t{ \"mxc-isi\", 3, {}, false },\n+\t{ \"qcom-camss\", 1, {}, true },\n+\t{ \"sun6i-csi\", 3, {}, false },\n };\n \n } /* namespace */\n@@ -346,6 +351,8 @@ public:\n \tstd::unique_ptr<SoftwareIsp> swIsp_;\n \tSimpleFrames frameInfo_;\n \n+\tunsigned int deviceInfoMinimumBuffers;\n+\n private:\n \tvoid tryPipeline(unsigned int code, const Size &size);\n \tstatic std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);\n@@ -1402,8 +1409,24 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n \tinputCfg.bufferCount = kNumInternalBuffers;\n \n \tif (data->converter_) {\n+\t\t/*\n+\t\t * The application will interact only with the capture node of\n+\t\t * the converter. Require two buffers for a frame drop free\n+\t\t * conversion, plus one extra to account for requeue delays.\n+\t\t */\n+\t\tdata->properties_.set(properties::MinimumRequests, 3);\n+\n \t\treturn data->converter_->configure(inputCfg, outputCfgs);\n \t} else {\n+\t\t/*\n+\t\t * The application will interact directly with the video capture\n+\t\t * device. Require the minimum required by the driver, plus one\n+\t\t * extra to account for requeue delays. Force at least three\n+\t\t * buffers in order to not drop frames.\n+\t\t */\n+\t\tdata->properties_.set(properties::MinimumRequests, std::max(data->deviceInfoMinimumBuffers + 1,\n+\t\t\t\t\t\t\t\t\t    3U));\n+\n \t\tipa::soft::IPAConfigInfo configInfo;\n \t\tconfigInfo.sensorControls = data->sensor_->controls();\n \t\treturn data->swIsp_->configure(inputCfg, outputCfgs, configInfo);\n@@ -1787,6 +1810,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n \tbool registered = false;\n \n \tfor (std::unique_ptr<SimpleCameraData> &data : pipelines) {\n+\t\tdata->deviceInfoMinimumBuffers = info->minimumBuffers;\n+\n \t\tint ret = data->init();\n \t\tif (ret < 0)\n \t\t\tcontinue;\ndiff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\nindex 586e932d..31951a12 100644\n--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n@@ -586,6 +586,8 @@ int UVCCameraData::init(MediaDevice *media)\n \tproperties_.set(properties::PixelArraySize, resolution);\n \tproperties_.set(properties::PixelArrayActiveAreas, { Rectangle(resolution) });\n \n+\tproperties_.set(properties::MinimumRequests, 3);\n+\n \t/* Initialise the supported controls. */\n \tControlInfoMap::Map ctrls;\n \ndiff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\nindex 07273bd2..01685a64 100644\n--- a/src/libcamera/pipeline/vimc/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc/vimc.cpp\n@@ -24,6 +24,7 @@\n #include <libcamera/formats.h>\n #include <libcamera/framebuffer.h>\n #include <libcamera/geometry.h>\n+#include <libcamera/property_ids.h>\n #include <libcamera/request.h>\n #include <libcamera/stream.h>\n \n@@ -592,6 +593,7 @@ int VimcCameraData::init()\n \n \t/* Initialize the camera properties. */\n \tproperties_ = sensor_->properties();\n+\tproperties_.set(properties::MinimumRequests, 3);\n \n \treturn 0;\n }\ndiff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\nindex 049ebcba..c33f1a5e 100644\n--- a/src/libcamera/pipeline/virtual/virtual.cpp\n+++ b/src/libcamera/pipeline/virtual/virtual.cpp\n@@ -126,6 +126,7 @@ VirtualCameraData::VirtualCameraData(PipelineHandler *pipe,\n \n \tproperties_.set(properties::PixelArrayActiveAreas,\n \t\t\t{ Rectangle(config_.maxResolutionSize) });\n+\tproperties_.set(properties::MinimumRequests, 3);\n \n \t/* \\todo Support multiple streams and pass multi_stream_test */\n \tstreamConfigs_.resize(kMaxStream);\ndiff --git a/src/libcamera/property_ids_core.yaml b/src/libcamera/property_ids_core.yaml\nindex 834454a4..cc28d677 100644\n--- a/src/libcamera/property_ids_core.yaml\n+++ b/src/libcamera/property_ids_core.yaml\n@@ -701,4 +701,26 @@ controls:\n \n         Different cameras may report identical devices.\n \n+  - MinimumRequests:\n+      type: uint32_t\n+      description: |\n+        The minimum number of capture requests that the camera pipeline requires\n+        to have queued in order to sustain capture operations with only minimal\n+        frame drops. At this quantity, there's no guarantee that manual per\n+        frame controls will apply correctly. This is also the minimum number of\n+        requests that must be queued before capture starts.\n+\n+        This property is based on the minimum number of memory buffers\n+        needed to fill the capture pipeline composed of hardware processing\n+        blocks plus as many buffers as needed to take into account propagation\n+        delays and avoid dropping frames.\n+\n+        \\todo Should this be a per-stream property?\n+\n+        \\todo Extend this property to expose the different minimum buffer and\n+        request counts (the minimum number of buffers to be able to capture at\n+        all, the minimum number of buffers to sustain capture without frame\n+        drop, and the minimum number of requests to ensure proper operation of\n+        per-frame controls).\n+\n ...\n","prefixes":["v11","01/19"]}