Patch Detail
Show a patch.
GET /api/patches/10487/?format=api
{ "id": 10487, "url": "https://patchwork.libcamera.org/api/patches/10487/?format=api", "web_url": "https://patchwork.libcamera.org/patch/10487/", "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": "<20201123221234.485933-8-niklas.soderlund@ragnatech.se>", "date": "2020-11-23T22:12:33", "name": "[libcamera-devel,v3,7/8] libcamera: pipeline: rkisp1: Use SOF event to warn about late parameters", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "fcfd42df7e51ce779a99c4338ff6e10cb1b2821f", "submitter": { "id": 5, "url": "https://patchwork.libcamera.org/api/people/5/?format=api", "name": "Niklas Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "delegate": { "id": 16, "url": "https://patchwork.libcamera.org/api/users/16/?format=api", "username": "neg", "first_name": "Niklas", "last_name": "Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "mbox": "https://patchwork.libcamera.org/patch/10487/mbox/", "series": [ { "id": 1478, "url": "https://patchwork.libcamera.org/api/series/1478/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1478", "date": "2020-11-23T22:12:26", "name": "libcamera: Add helper for controls that take effect with a delay", "version": 3, "mbox": "https://patchwork.libcamera.org/series/1478/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/10487/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/10487/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 8F67DBE178\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 23 Nov 2020 22:12:55 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 696AB633FB;\n\tMon, 23 Nov 2020 23:12:55 +0100 (CET)", "from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net\n\t[195.74.38.229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1DD8363401\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Nov 2020 23:12:52 +0100 (CET)", "from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de\n\t[79.202.36.88])\n\tby bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid 28b5035f-2dd8-11eb-a076-005056917f90;\n\tMon, 23 Nov 2020 23:06:38 +0100 (CET)" ], "X-Halon-ID": "28b5035f-2dd8-11eb-a076-005056917f90", "Authorized-sender": "niklas.soderlund@fsdn.se", "From": "=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>", "To": "libcamera-devel@lists.libcamera.org,\n\tnaush@raspberrypi.com", "Date": "Mon, 23 Nov 2020 23:12:33 +0100", "Message-Id": "<20201123221234.485933-8-niklas.soderlund@ragnatech.se>", "X-Mailer": "git-send-email 2.29.2", "In-Reply-To": "<20201123221234.485933-1-niklas.soderlund@ragnatech.se>", "References": "<20201123221234.485933-1-niklas.soderlund@ragnatech.se>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH v3 7/8] libcamera: pipeline: rkisp1: Use\n\tSOF event to warn about late parameters", "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>", "Content-Type": "text/plain; charset=\"utf-8\"", "Content-Transfer-Encoding": "base64", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "In the Timeline approach the idea was to delay queuing buffers to the\ndevice until the IPA had a chance to prepare the parameters buffer. A\ncheck was still added to warn if the IPA queued buffers before the\nparameters buffer was filled in.\n\nThis check happened much sooner then needed as the parameter buffers\ndoes not have to be ready when the buffer is queued but just before its\nconsumed. As the pipeline now has true knowledge of each SOF we can move\nthe check there and remove the delaying of queuing of buffers.\n\nThis change really speeds up the IPA reactions as the delays used in the\nTimeline where approximated while with this change they are driven by\nevents reported by the device.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 78 ++++++++----------------\n 1 file changed, 24 insertions(+), 54 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex c3c4b5a65e3d9afe..3662a53ac4a43fcd 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -40,7 +40,6 @@ namespace libcamera {\n LOG_DEFINE_CATEGORY(RkISP1)\n \n class PipelineHandlerRkISP1;\n-class RkISP1ActionQueueBuffers;\n class RkISP1CameraData;\n \n enum RkISP1ActionType {\n@@ -203,7 +202,6 @@ private:\n \t\t\tPipelineHandler::cameraData(camera));\n \t}\n \n-\tfriend RkISP1ActionQueueBuffers;\n \tfriend RkISP1CameraData;\n \tfriend RkISP1Frames;\n \n@@ -214,6 +212,7 @@ private:\n \tvoid bufferReady(FrameBuffer *buffer);\n \tvoid paramReady(FrameBuffer *buffer);\n \tvoid statReady(FrameBuffer *buffer);\n+\tvoid frameStart(uint32_t sequence);\n \n \tint allocateBuffers(Camera *camera);\n \tint freeBuffers(Camera *camera);\n@@ -348,53 +347,6 @@ RkISP1FrameInfo *RkISP1Frames::find(Request *request)\n \treturn nullptr;\n }\n \n-class RkISP1ActionQueueBuffers : public FrameAction\n-{\n-public:\n-\tRkISP1ActionQueueBuffers(unsigned int frame, RkISP1CameraData *data,\n-\t\t\t\t PipelineHandlerRkISP1 *pipe)\n-\t\t: FrameAction(frame, QueueBuffers), data_(data), pipe_(pipe)\n-\t{\n-\t}\n-\n-protected:\n-\tvoid run() override\n-\t{\n-\t\tRkISP1FrameInfo *info = data_->frameInfo_.find(frame());\n-\t\tif (!info)\n-\t\t\tLOG(RkISP1, Fatal) << \"Frame not known\";\n-\n-\t\t/*\n-\t\t * \\todo: If parameters are not filled a better method to handle\n-\t\t * the situation than queuing a buffer with unknown content\n-\t\t * should be used.\n-\t\t *\n-\t\t * It seems excessive to keep an internal zeroed scratch\n-\t\t * parameters buffer around as this should not happen unless the\n-\t\t * devices is under too much load. Perhaps failing the request\n-\t\t * and returning it to the application with an error code is\n-\t\t * better than queue it to hardware?\n-\t\t */\n-\t\tif (!info->paramFilled)\n-\t\t\tLOG(RkISP1, Error)\n-\t\t\t\t<< \"Parameters not ready on time for frame \"\n-\t\t\t\t<< frame();\n-\n-\t\tpipe_->param_->queueBuffer(info->paramBuffer);\n-\t\tpipe_->stat_->queueBuffer(info->statBuffer);\n-\n-\t\tif (info->mainPathBuffer)\n-\t\t\tpipe_->mainPath_.queueBuffer(info->mainPathBuffer);\n-\n-\t\tif (info->selfPathBuffer)\n-\t\t\tpipe_->selfPath_.queueBuffer(info->selfPathBuffer);\n-\t}\n-\n-private:\n-\tRkISP1CameraData *data_;\n-\tPipelineHandlerRkISP1 *pipe_;\n-};\n-\n int RkISP1CameraData::loadIPA()\n {\n \tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n@@ -958,9 +910,14 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n \top.controls = { request->controls() };\n \tdata->ipa_->processEvent(op);\n \n-\tdata->timeline_.scheduleAction(std::make_unique<RkISP1ActionQueueBuffers>(data->frame_,\n-\t\t\t\t\t\t\t\t\t\t data,\n-\t\t\t\t\t\t\t\t\t\t this));\n+\tparam_->queueBuffer(info->paramBuffer);\n+\tstat_->queueBuffer(info->statBuffer);\n+\n+\tif (info->mainPathBuffer)\n+\t\tmainPath_.queueBuffer(info->mainPathBuffer);\n+\n+\tif (info->selfPathBuffer)\n+\t\tselfPath_.queueBuffer(info->selfPathBuffer);\n \n \tdata->frame_++;\n \n@@ -1098,6 +1055,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n \tmainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);\n \tselfPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);\n \tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n+\tisp_->frameStart.connect(this, &PipelineHandlerRkISP1::frameStart);\n \tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n \n \t/*\n@@ -1177,8 +1135,6 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n \tif (!info)\n \t\treturn;\n \n-\tdata->timeline_.bufferReady(buffer);\n-\n \tif (data->frame_ <= buffer->metadata().sequence)\n \t\tdata->frame_ = buffer->metadata().sequence + 1;\n \n@@ -1188,6 +1144,20 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n \tdata->ipa_->processEvent(op);\n }\n \n+void PipelineHandlerRkISP1::frameStart(uint32_t sequence)\n+{\n+\tif (!activeCamera_)\n+\t\treturn;\n+\n+\tRkISP1CameraData *data = cameraData(activeCamera_);\n+\n+\tRkISP1FrameInfo *info = data->frameInfo_.find(sequence);\n+\tif (!info || !info->paramFilled)\n+\t\tLOG(RkISP1, Info)\n+\t\t\t<< \"Parameters not ready on time for frame \"\n+\t\t\t<< sequence;\n+}\n+\n REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)\n \n } /* namespace libcamera */\n", "prefixes": [ "libcamera-devel", "v3", "7/8" ] }