Patch Detail
Show a patch.
GET /api/1.1/patches/10407/?format=api
{ "id": 10407, "url": "https://patchwork.libcamera.org/api/1.1/patches/10407/?format=api", "web_url": "https://patchwork.libcamera.org/patch/10407/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20201110002710.3233696-8-niklas.soderlund@ragnatech.se>", "date": "2020-11-10T00:27:09", "name": "[libcamera-devel,v2,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/1.1/people/5/?format=api", "name": "Niklas Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/10407/mbox/", "series": [ { "id": 1454, "url": "https://patchwork.libcamera.org/api/1.1/series/1454/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1454", "date": "2020-11-10T00:27:02", "name": "Add helper for controls that take effect with a delay", "version": 2, "mbox": "https://patchwork.libcamera.org/series/1454/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/10407/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/10407/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 64D3FBE084\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 10 Nov 2020 00:28:05 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 40C9D630C2;\n\tTue, 10 Nov 2020 01:28:05 +0100 (CET)", "from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net\n\t[195.74.38.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 69E39630C0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 10 Nov 2020 01:28:02 +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 c6e6ca33-22ea-11eb-bcc0-005056917f90;\n\tTue, 10 Nov 2020 01:22:12 +0100 (CET)" ], "X-Halon-ID": "c6e6ca33-22ea-11eb-bcc0-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": "Tue, 10 Nov 2020 01:27:09 +0100", "Message-Id": "<20201110002710.3233696-8-niklas.soderlund@ragnatech.se>", "X-Mailer": "git-send-email 2.29.2", "In-Reply-To": "<20201110002710.3233696-1-niklas.soderlund@ragnatech.se>", "References": "<20201110002710.3233696-1-niklas.soderlund@ragnatech.se>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH v2 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 6b71fdd1db00460b..0879644af69fe7de 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@@ -948,9 +900,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@@ -1088,6 +1045,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@@ -1167,8 +1125,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@@ -1178,6 +1134,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", "v2", "7/8" ] }