Patch Detail
Show a patch.
GET /api/patches/11087/?format=api
{ "id": 11087, "url": "https://patchwork.libcamera.org/api/patches/11087/?format=api", "web_url": "https://patchwork.libcamera.org/patch/11087/", "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": "<20210131224702.8838-9-laurent.pinchart@ideasonboard.com>", "date": "2021-01-31T22:46:50", "name": "[libcamera-devel,08/20] libcamera: pipeline: simple: converter: Decouple input and output completion", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "fda2cf8fae8e41a2ace1b587e6ad168dfbbb08be", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": { "id": 14, "url": "https://patchwork.libcamera.org/api/users/14/?format=api", "username": "pinchartl", "first_name": "Laurent", "last_name": "Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "mbox": "https://patchwork.libcamera.org/patch/11087/mbox/", "series": [ { "id": 1633, "url": "https://patchwork.libcamera.org/api/series/1633/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1633", "date": "2021-01-31T22:46:42", "name": "[libcamera-devel,01/20] libcamera: pipeline: simple: Manage converter with std::unique_ptr<>", "version": 1, "mbox": "https://patchwork.libcamera.org/series/1633/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/11087/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/11087/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 3635EBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 31 Jan 2021 22:47:39 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ECE46683F9;\n\tSun, 31 Jan 2021 23:47:38 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E5E2F683DD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 31 Jan 2021 23:47:30 +0100 (CET)", "from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6DA07145C; \n\tSun, 31 Jan 2021 23:47:30 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kJ5KPWd0\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612133250;\n\tbh=u+pC3266rubsngdrw/0GegTHaDvdoizTRyIRuXLKRt8=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=kJ5KPWd0R1zq4f78HOA4lZ13q9kERZZLjQ4APuxgxUbtr3pUuVo/2YNo8axKXRJZR\n\t/vyP4GXSHtlTwxYQPsKBoWfmP/RZf/UzH0kZBSBMxbFs7VTiZRKhQ1kWf3ycg4CN6S\n\tEtu/mX0LSfHdFzr25bWewTC0xaqQ9CZ9WZ+uUzWI=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 1 Feb 2021 00:46:50 +0200", "Message-Id": "<20210131224702.8838-9-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.28.0", "In-Reply-To": "<20210131224702.8838-1-laurent.pinchart@ideasonboard.com>", "References": "<20210131224702.8838-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH 08/20] libcamera: pipeline: simple:\n\tconverter: Decouple input and output completion", "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>", "Cc": "Phi-Bang Nguyen <pnguyen@baylibre.com>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The SimpleConverter API signals completion of input and output buffer\npairs. This unnecessarily delays requeueing the input buffer to the\nvideo capture queue until the output buffer completes, and also delays\nsignalling request completion until the input buffer completes. While\nthis shouldn't cause large delays in practice, it will also not scale\nwhen multi-stream support will be added to the converter class.\n\nTo address the current issue and prepare for the future, decouple\nsignalling of input and output buffers completion.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/libcamera/pipeline/simple/converter.cpp | 24 +++++--------------\n src/libcamera/pipeline/simple/converter.h | 11 ++++-----\n src/libcamera/pipeline/simple/simple.cpp | 26 +++++++++++++--------\n 3 files changed, 26 insertions(+), 35 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp\nindex f782fbc63b09..8324baedc198 100644\n--- a/src/libcamera/pipeline/simple/converter.cpp\n+++ b/src/libcamera/pipeline/simple/converter.cpp\n@@ -45,8 +45,8 @@ SimpleConverter::SimpleConverter(MediaDevice *media)\n \t\treturn;\n \t}\n \n-\tm2m_->output()->bufferReady.connect(this, &SimpleConverter::outputBufferReady);\n-\tm2m_->capture()->bufferReady.connect(this, &SimpleConverter::captureBufferReady);\n+\tm2m_->output()->bufferReady.connect(this, &SimpleConverter::m2mInputBufferReady);\n+\tm2m_->capture()->bufferReady.connect(this, &SimpleConverter::m2mOutputBufferReady);\n }\n \n std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input)\n@@ -247,26 +247,14 @@ int SimpleConverter::queueBuffers(FrameBuffer *input, FrameBuffer *output)\n \treturn 0;\n }\n \n-void SimpleConverter::captureBufferReady(FrameBuffer *buffer)\n+void SimpleConverter::m2mInputBufferReady(FrameBuffer *buffer)\n {\n-\tif (!outputDoneQueue_.empty()) {\n-\t\tFrameBuffer *other = outputDoneQueue_.front();\n-\t\toutputDoneQueue_.pop();\n-\t\tbufferReady.emit(other, buffer);\n-\t} else {\n-\t\tcaptureDoneQueue_.push(buffer);\n-\t}\n+\tinputBufferReady.emit(buffer);\n }\n \n-void SimpleConverter::outputBufferReady(FrameBuffer *buffer)\n+void SimpleConverter::m2mOutputBufferReady(FrameBuffer *buffer)\n {\n-\tif (!captureDoneQueue_.empty()) {\n-\t\tFrameBuffer *other = captureDoneQueue_.front();\n-\t\tcaptureDoneQueue_.pop();\n-\t\tbufferReady.emit(buffer, other);\n-\t} else {\n-\t\toutputDoneQueue_.push(buffer);\n-\t}\n+\toutputBufferReady.emit(buffer);\n }\n \n } /* namespace libcamera */\ndiff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h\nindex 780bfa8f7832..739b24df0200 100644\n--- a/src/libcamera/pipeline/simple/converter.h\n+++ b/src/libcamera/pipeline/simple/converter.h\n@@ -9,7 +9,6 @@\n #define __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__\n \n #include <memory>\n-#include <queue>\n #include <tuple>\n #include <vector>\n \n@@ -47,17 +46,15 @@ public:\n \n \tint queueBuffers(FrameBuffer *input, FrameBuffer *output);\n \n-\tSignal<FrameBuffer *, FrameBuffer *> bufferReady;\n+\tSignal<FrameBuffer *> inputBufferReady;\n+\tSignal<FrameBuffer *> outputBufferReady;\n \n private:\n-\tvoid captureBufferReady(FrameBuffer *buffer);\n-\tvoid outputBufferReady(FrameBuffer *buffer);\n+\tvoid m2mInputBufferReady(FrameBuffer *buffer);\n+\tvoid m2mOutputBufferReady(FrameBuffer *buffer);\n \n \tstd::unique_ptr<V4L2M2MDevice> m2m_;\n \n-\tstd::queue<FrameBuffer *> captureDoneQueue_;\n-\tstd::queue<FrameBuffer *> outputDoneQueue_;\n-\n \tunsigned int inputBufferCount_;\n \tunsigned int outputBufferCount_;\n };\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 20a4ebca94fd..7f9c57234256 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -144,7 +144,8 @@ private:\n \t}\n \n \tvoid bufferReady(FrameBuffer *buffer);\n-\tvoid converterDone(FrameBuffer *input, FrameBuffer *output);\n+\tvoid converterInputDone(FrameBuffer *buffer);\n+\tvoid converterOutputDone(FrameBuffer *buffer);\n \n \tMediaDevice *media_;\n \tstd::map<const MediaEntity *, std::unique_ptr<V4L2VideoDevice>> videos_;\n@@ -768,7 +769,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n \t\t\t\t<< \"Failed to create converter, disabling format conversion\";\n \t\t\tconverter_.reset();\n \t\t} else {\n-\t\t\tconverter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone);\n+\t\t\tconverter_->inputBufferReady.connect(this, &SimplePipelineHandler::converterInputDone);\n+\t\t\tconverter_->outputBufferReady.connect(this, &SimplePipelineHandler::converterOutputDone);\n \t\t}\n \t}\n \n@@ -925,19 +927,23 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)\n \tcompleteRequest(request);\n }\n \n-void SimplePipelineHandler::converterDone(FrameBuffer *input,\n-\t\t\t\t\t FrameBuffer *output)\n+void SimplePipelineHandler::converterInputDone(FrameBuffer *buffer)\n {\n \tASSERT(activeCamera_);\n \tSimpleCameraData *data = cameraData(activeCamera_);\n \n-\t/* Complete the request. */\n-\tRequest *request = output->request();\n-\tcompleteBuffer(request, output);\n-\tcompleteRequest(request);\n-\n \t/* Queue the input buffer back for capture. */\n-\tdata->video_->queueBuffer(input);\n+\tdata->video_->queueBuffer(buffer);\n+}\n+\n+void SimplePipelineHandler::converterOutputDone(FrameBuffer *buffer)\n+{\n+\tASSERT(activeCamera_);\n+\n+\t/* Complete the request. */\n+\tRequest *request = buffer->request();\n+\tcompleteBuffer(request, buffer);\n+\tcompleteRequest(request);\n }\n \n REGISTER_PIPELINE_HANDLER(SimplePipelineHandler)\n", "prefixes": [ "libcamera-devel", "08/20" ] }