Patch Detail
Show a patch.
GET /api/patches/10667/?format=api
{ "id": 10667, "url": "https://patchwork.libcamera.org/api/patches/10667/?format=api", "web_url": "https://patchwork.libcamera.org/patch/10667/", "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": "<20201215004811.602429-8-niklas.soderlund@ragnatech.se>", "date": "2020-12-15T00:48:10", "name": "[libcamera-devel,v4,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/10667/mbox/", "series": [ { "id": 1533, "url": "https://patchwork.libcamera.org/api/series/1533/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1533", "date": "2020-12-15T00:48:03", "name": "libcamera: Add helper for controls that take effect with a delay", "version": 4, "mbox": "https://patchwork.libcamera.org/series/1533/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/10667/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/10667/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 0F8E4BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 15 Dec 2020 00:48:32 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DB6E361592;\n\tTue, 15 Dec 2020 01:48:31 +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 1271961591\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Dec 2020 01:48:29 +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 3eab7125-3e6f-11eb-a076-005056917f90;\n\tTue, 15 Dec 2020 01:48:28 +0100 (CET)" ], "X-Halon-ID": "3eab7125-3e6f-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": "Tue, 15 Dec 2020 01:48:10 +0100", "Message-Id": "<20201215004811.602429-8-niklas.soderlund@ragnatech.se>", "X-Mailer": "git-send-email 2.29.2", "In-Reply-To": "<20201215004811.602429-1-niklas.soderlund@ragnatech.se>", "References": "<20201215004811.602429-1-niklas.soderlund@ragnatech.se>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH v4 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 it's\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 513a60b04e5f2e21..3851d94e7b133356 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -41,7 +41,6 @@ namespace libcamera {\n LOG_DEFINE_CATEGORY(RkISP1)\n \n class PipelineHandlerRkISP1;\n-class RkISP1ActionQueueBuffers;\n class RkISP1CameraData;\n \n enum RkISP1ActionType {\n@@ -202,7 +201,6 @@ private:\n \t\t\tPipelineHandler::cameraData(camera));\n \t}\n \n-\tfriend RkISP1ActionQueueBuffers;\n \tfriend RkISP1CameraData;\n \tfriend RkISP1Frames;\n \n@@ -213,6 +211,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@@ -347,53 +346,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@@ -950,9 +902,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@@ -1102,6 +1059,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@@ -1181,8 +1139,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@@ -1192,6 +1148,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", "v4", "7/8" ] }