Patch Detail
Show a patch.
GET /api/patches/18016/?format=api
{ "id": 18016, "url": "https://patchwork.libcamera.org/api/patches/18016/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18016/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20221216122939.256534-3-paul.elder@ideasonboard.com>", "date": "2022-12-16T12:29:23", "name": "[libcamera-devel,v9,02/18] libcamera: property: Add MinimumRequests property", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "5245805309d05410cd548302887b108587e03b8c", "submitter": { "id": 17, "url": "https://patchwork.libcamera.org/api/people/17/?format=api", "name": "Paul Elder", "email": "paul.elder@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/18016/mbox/", "series": [ { "id": 3675, "url": "https://patchwork.libcamera.org/api/series/3675/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3675", "date": "2022-12-16T12:29:21", "name": "lc-compliance: Add test to queue more requests than hardware depth", "version": 9, "mbox": "https://patchwork.libcamera.org/series/3675/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18016/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18016/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 2EF6CC31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 16 Dec 2022 12:30:07 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 37FC86339E;\n\tFri, 16 Dec 2022 13:30:06 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C602E63360\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 16 Dec 2022 13:30:04 +0100 (CET)", "from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 28055A31;\n\tFri, 16 Dec 2022 13:30:02 +0100 (CET)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1671193806;\n\tbh=35LwYXm0BAOIYIDBVnWhJMZneP3HtGkAGqX3vIFrXH4=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=rsJsspHIZLtjsq5dLisQCXmEyOwyNEH/K/0Sbl1hfYgmqKs/wE7icEcdJ+RgVUCk+\n\tJ8WTZSuK16QP9fdExfnTzlj3UQr/QphlGeOOkknJ4fZoiDnYlO5qy1Zdhcur11bkmI\n\t1KN7rIjc/5t9087YdUxwYUY3PTDumv+v8MzhexGIvt2Tr9cJIHRYYXzbr3Cz0A3Uwp\n\tEni/nnCPMR7jb6afaDPM6rUcFSk4/UkOJ2ubv/o/UhmFR3KoYnWWEym3YNzxdCrv2q\n\tLqa6AdDXxVAnW6815AkxpkcqNaY0mcBcEVzXBJlJkWeTqXoWlkRpE7/gUq3KeG1Ep3\n\tH/7RoFMqgPiHw==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1671193804;\n\tbh=35LwYXm0BAOIYIDBVnWhJMZneP3HtGkAGqX3vIFrXH4=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=Tya1LJnQ2EGT9J+0KPfxNnZwSLwEcSxF64zWlhzge8p/xuUumaFpnicmKeW575wXI\n\tyhguPtWI30/TJ2/KVlOHMPWZU7HIck/1UvW/6HU1eJwmvOujHfElJzY7lc+PONWayL\n\t4/yWT5Z/AAbIPg3/oETaPGqUSMJygtj7edrFgqJM=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Tya1LJnQ\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Fri, 16 Dec 2022 21:29:23 +0900", "Message-Id": "<20221216122939.256534-3-paul.elder@ideasonboard.com>", "X-Mailer": "git-send-email 2.35.1", "In-Reply-To": "<20221216122939.256534-1-paul.elder@ideasonboard.com>", "References": "<20221216122939.256534-1-paul.elder@ideasonboard.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v9 02/18] libcamera: property: Add\n\tMinimumRequests property", "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>", "From": "Paul Elder via libcamera-devel <libcamera-devel@lists.libcamera.org>", "Reply-To": "Paul Elder <paul.elder@ideasonboard.com>", "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\nSigned-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\n\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 | 22 +++++++---\n src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +\n .../pipeline/raspberrypi/raspberrypi.cpp | 2 +\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +\n src/libcamera/pipeline/simple/simple.cpp | 40 +++++++++++++++++--\n src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +\n src/libcamera/pipeline/vimc/vimc.cpp | 2 +\n src/libcamera/property_ids.yaml | 21 ++++++++++\n 8 files changed, 84 insertions(+), 9 deletions(-)", "diff": "diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\nindex e1930fdf..a7356e4a 100644\n--- a/Documentation/guides/pipeline-handler.rst\n+++ b/Documentation/guides/pipeline-handler.rst\n@@ -658,19 +658,31 @@ 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_buffers_needed`` property for the ``vid_cap`` queue in\n+vivid's driver code). In order to not drop frames we should have one extra\n+buffer queued to the driver so that it is used as soon as the previous buffer\n+completes. Therefore we will set our ``MinimumRequests`` to three. Append the\n+following line to ``init()``:\n+\n+.. code-block:: cpp\n+\n+ properties_.set(properties::MinimumRequests, 3);\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 Generating a default configuration\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex e4d79ea4..98a4a3e5 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -1126,6 +1126,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, 3);\n+\n \t\tret = initControls(data.get());\n \t\tif (ret)\n \t\t\tcontinue;\ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 8569df17..4a08d01e 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -1063,6 +1063,8 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n \t/* Enable SOF event generation. */\n \tdata->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true);\n \n+\tdata->properties_.set(properties::MinimumRequests, 3);\n+\n \t/*\n \t * Reset the delayed controls with the gain and exposure values set by\n \t * the IPA.\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex f658d75e..45b6beaf 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -23,6 +23,7 @@\n #include <libcamera/control_ids.h>\n #include <libcamera/formats.h>\n #include <libcamera/framebuffer.h>\n+#include <libcamera/property_ids.h>\n #include <libcamera/request.h>\n #include <libcamera/stream.h>\n #include <libcamera/ipa/core_ipa_interface.h>\n@@ -1102,6 +1103,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 \t/*\n \t * \\todo Read dealy values from the sensor itself or from a\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 24ded4db..8ed983fe 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -25,6 +25,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@@ -180,6 +181,10 @@ class SimplePipelineHandler;\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@@ -190,10 +195,10 @@ struct SimplePipelineInfo {\n namespace {\n \n static const SimplePipelineInfo supportedDevices[] = {\n-\t{ \"imx7-csi\", { { \"pxp\", 1 } } },\n-\t{ \"mxc-isi\", {} },\n-\t{ \"qcom-camss\", {} },\n-\t{ \"sun6i-csi\", {} },\n+\t{ \"imx7-csi\", 2, { { \"pxp\", 1 } } },\n+\t{ \"mxc-isi\", 3, {} },\n+\t{ \"qcom-camss\", 1, {} },\n+\t{ \"sun6i-csi\", 3, {} },\n };\n \n } /* namespace */\n@@ -271,6 +276,8 @@ public:\n \tbool useConverter_;\n \tstd::queue<std::map<unsigned int, FrameBuffer *>> converterQueue_;\n \n+\tconst SimplePipelineInfo *deviceInfo;\n+\n private:\n \tvoid tryPipeline(unsigned int code, const Size &size);\n \tstatic std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);\n@@ -1168,6 +1175,29 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n \tinputCfg.stride = captureFormat.planes[0].bpl;\n \tinputCfg.bufferCount = kNumInternalBuffers;\n \n+\t/* Set the MinimumRequests property. */\n+\tunsigned int minimumRequests;\n+\n+\tif (data->useConverter_) {\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\tminimumRequests = 3;\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\tminimumRequests = std::max(data->deviceInfo->minimumBuffers + 1,\n+\t\t\t\t\t 3U);\n+\t}\n+\n+\tdata->properties_.set(properties::MinimumRequests, minimumRequests);\n+\n \treturn data->converter_->configure(inputCfg, outputCfgs);\n }\n \n@@ -1506,6 +1536,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n \tbool registered = false;\n \n \tfor (std::unique_ptr<SimpleCameraData> &data : pipelines) {\n+\t\tdata->deviceInfo = info;\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 277465b7..7f580955 100644\n--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n@@ -500,6 +500,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 204f5ad7..d2633be4 100644\n--- a/src/libcamera/pipeline/vimc/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc/vimc.cpp\n@@ -21,6 +21,7 @@\n #include <libcamera/control_ids.h>\n #include <libcamera/controls.h>\n #include <libcamera/formats.h>\n+#include <libcamera/property_ids.h>\n #include <libcamera/request.h>\n #include <libcamera/stream.h>\n \n@@ -571,6 +572,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/property_ids.yaml b/src/libcamera/property_ids.yaml\nindex cb55e0ed..c82ac17e 100644\n--- a/src/libcamera/property_ids.yaml\n+++ b/src/libcamera/property_ids.yaml\n@@ -690,6 +690,27 @@ controls:\n that is twice that of the full resolution mode. This value will be valid\n after the configure method has returned successfully.\n \n+ - MinimumRequests:\n+ type: int32_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 without frame\n+ drops. At this quantity, there's no guarantee that manual per-frame\n+ controls will apply correctly.\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 # Draft properties section\n \n", "prefixes": [ "libcamera-devel", "v9", "02/18" ] }