Patch Detail
Show a patch.
GET /api/patches/8953/?format=api
{ "id": 8953, "url": "https://patchwork.libcamera.org/api/patches/8953/?format=api", "web_url": "https://patchwork.libcamera.org/patch/8953/", "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": "<20200724072218.943245-10-naush@raspberrypi.com>", "date": "2020-07-24T07:22:17", "name": "[libcamera-devel,v5,09/10] libcamera: pipeline: raspberrypi: Add more robust stream buffer logic", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "f66117af5e1b433d8268e59bd8524c44331f4646", "submitter": { "id": 34, "url": "https://patchwork.libcamera.org/api/people/34/?format=api", "name": "Naushir Patuck", "email": "naush@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/8953/mbox/", "series": [ { "id": 1132, "url": "https://patchwork.libcamera.org/api/series/1132/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1132", "date": "2020-07-24T07:22:08", "name": "Zero-copy RAW stream work", "version": 5, "mbox": "https://patchwork.libcamera.org/series/1132/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/8953/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/8953/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 D83FABD86F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Jul 2020 07:23:00 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A5FCB611B7;\n\tFri, 24 Jul 2020 09:23:00 +0200 (CEST)", "from mail-wr1-x431.google.com (mail-wr1-x431.google.com\n\t[IPv6:2a00:1450:4864:20::431])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 546C86118D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Jul 2020 09:22:58 +0200 (CEST)", "by mail-wr1-x431.google.com with SMTP id f2so7333986wrp.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Jul 2020 00:22:58 -0700 (PDT)", "from naushir-VirtualBox.patuck.local ([88.97.76.4])\n\tby smtp.gmail.com with ESMTPSA id\n\t32sm246595wrn.86.2020.07.24.00.22.56\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 24 Jul 2020 00:22:56 -0700 (PDT)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"qS7BniEW\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=pO9m/3WEaO+YsGGYT5406UzcJktNRpMQ9mP+ggh1N14=;\n\tb=qS7BniEWl5QAagYEotazXNgtEkvBiKrK7/iojmM1krxyVvcUvk5hncPH66kizDcQGi\n\tlR4Cp0JTBnRYx2Uhr/SkMXDDb5OtFdawzswfVtHCzICcfjGm372V/P1Mw6CMCzNTd+UY\n\t2lJuFQ/NmmQIyOHNYo0nURow96bg4VwXD81H0fu2ig7hpnTpNgIS5CypGhndsOgM0owf\n\tmgnVrJ0qvRAxSRpDcbsLImYFwQ5TfprcoKA9KervnFSsCMtLWu6kYD6toKfQB9U4QBQ/\n\tLRmkw+GzpqBi0MSrKSV5h0S5ZBcMNwwfPdYkjKhIEOv/ymyB6INV7CWW27Ordh4HPK3x\n\tNVxw==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=pO9m/3WEaO+YsGGYT5406UzcJktNRpMQ9mP+ggh1N14=;\n\tb=Q2IiE9tx8JrAI8l2UxO+FEO6u0KfLpCOj2tp7e4rS/8GfAVxBsX8d2g+wTwS1Sd9Bh\n\toL06WJR/Mc0/lrO0YBOB6IrGE2AFW9+hrQXEzCgorvhsf58ZEyo+0iUSWB9LOZIy3kGF\n\t95XHxRTatTiR2hu+jcRnEkmRIM/geG4mkbFQQTOfDVgmKPyPKsvwvPDeDAouAvmiFJg8\n\tfdIEroNeaaH0UH7hCw+w09FVCJSG6EmP0y9s53Qx5j6ycJVLJmHedBRrjhEjSmYHKMKI\n\tVF7VS5Ev0wQWCJseLU5Lysi8kP4oUlI4X4SrRrdAUFtBfzM2E7oojIzkZY7mG+SwlUw3\n\tOyuw==", "X-Gm-Message-State": "AOAM532tshBqkUuXFuebojH6sL2sNuTCLHbd6QibQUrL7Ea9qchU42I8\n\t9zoudDJg3pFKKS61cuvUquwCHneVEjoZSw==", "X-Google-Smtp-Source": "ABdhPJyCMVPXl8bfkxujTQPN5kA6Y+qaECzfeN1QwBmuEth23ixlxTSjzyq7s8GngdBQTDKLhEJdjA==", "X-Received": "by 2002:a5d:6a8d:: with SMTP id\n\ts13mr7879529wru.201.1595575377411; \n\tFri, 24 Jul 2020 00:22:57 -0700 (PDT)", "From": "Naushir Patuck <naush@raspberrypi.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Fri, 24 Jul 2020 08:22:17 +0100", "Message-Id": "<20200724072218.943245-10-naush@raspberrypi.com>", "X-Mailer": "git-send-email 2.25.1", "In-Reply-To": "<20200724072218.943245-1-naush@raspberrypi.com>", "References": "<20200724072218.943245-1-naush@raspberrypi.com>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH v5 09/10] libcamera: pipeline:\n\traspberrypi: Add more robust stream buffer logic", "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": "Add further queueing into the RPiStream object to ensure that we always\nfollow the buffer ordering (be it internal or external) given by\nincoming Requests.\n\nThis is essential, otherwise we risk dropping frames that are meant to\nbe part of a Request, and can cause the pipeline to stall indefinitely.\nThis also prevents any possibility of mismatched frame buffers going\nthrough the pipeline and out to the application.\n\nSigned-off-by: Naushir Patuck <naush@raspberrypi.com>\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\nTested-by: David Plowman <david.plowman@raspberrypi.com>\n---\n .../pipeline/raspberrypi/rpi_stream.cpp | 76 +++++++++++++++----\n .../pipeline/raspberrypi/rpi_stream.h | 13 +++-\n 2 files changed, 75 insertions(+), 14 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\nindex b4d737db..b2a5dfa7 100644\n--- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n@@ -94,38 +94,75 @@ int RPiStream::queueBuffer(FrameBuffer *buffer)\n {\n \t/*\n \t * A nullptr buffer implies an external stream, but no external\n-\t * buffer has been supplied. So, pick one from the availableBuffers_\n-\t * queue.\n+\t * buffer has been supplied in the Request. So, pick one from the\n+\t * availableBuffers_ queue.\n \t */\n \tif (!buffer) {\n \t\tif (availableBuffers_.empty()) {\n-\t\t\tLOG(RPISTREAM, Warning) << \"No buffers available for \"\n+\t\t\tLOG(RPISTREAM, Info) << \"No buffers available for \"\n \t\t\t\t\t\t<< name_;\n-\t\t\treturn -EINVAL;\n+\t\t\t/*\n+\t\t\t * Note that we need to queue an internal buffer as soon\n+\t\t\t * as one becomes available.\n+\t\t\t */\n+\t\t\trequestBuffers_.push(nullptr);\n+\t\t\treturn 0;\n \t\t}\n \n \t\tbuffer = availableBuffers_.front();\n \t\tavailableBuffers_.pop();\n \t}\n \n-\tLOG(RPISTREAM, Debug) << \"Queuing buffer \" << buffer->cookie()\n-\t\t\t << \" for \" << name_;\n+\t/*\n+\t * If no earlier requests are pending to be queued we can go ahead and\n+\t * queue this buffer into the device.\n+\t */\n+\tif (requestBuffers_.empty())\n+\t\treturn queueToDevice(buffer);\n \n-\tint ret = dev_->queueBuffer(buffer);\n-\tif (ret) {\n-\t\tLOG(RPISTREAM, Error) << \"Failed to queue buffer for \"\n-\t\t\t\t << name_;\n-\t}\n+\t/*\n+\t * There are earlier Request buffers to be queued, so this buffer must go\n+\t * on the waiting list.\n+\t */\n+\trequestBuffers_.push(buffer);\n \n-\treturn ret;\n+\treturn 0;\n }\n \n void RPiStream::returnBuffer(FrameBuffer *buffer)\n {\n \t/* This can only be called for external streams. */\n-\tassert(external_);\n+\tASSERT(external_);\n \n+\t/* Push this buffer back into the queue to be used again. */\n \tavailableBuffers_.push(buffer);\n+\n+\t/*\n+\t * Do we have any Request buffers that are waiting to be queued?\n+\t * If so, do it now as availableBuffers_ will not be empty.\n+\t */\n+\twhile (!requestBuffers_.empty()) {\n+\t\tFrameBuffer *buffer = requestBuffers_.front();\n+\n+\t\tif (!buffer) {\n+\t\t\t/*\n+\t\t\t * We want to queue an internal buffer, but none\n+\t\t\t * are available. Can't do anything, quit the loop.\n+\t\t\t */\n+\t\t\tif (availableBuffers_.empty())\n+\t\t\t\tbreak;\n+\n+\t\t\t/*\n+\t\t\t * We want to queue an internal buffer, and at least one\n+\t\t\t * is available.\n+\t\t\t */\n+\t\t\tbuffer = availableBuffers_.front();\n+\t\t\tavailableBuffers_.pop();\n+\t\t}\n+\n+\t\trequestBuffers_.pop();\n+\t\tqueueToDevice(buffer);\n+\t}\n }\n \n int RPiStream::queueAllBuffers()\n@@ -155,10 +192,23 @@ void RPiStream::releaseBuffers()\n void RPiStream::clearBuffers()\n {\n \tavailableBuffers_ = std::queue<FrameBuffer *>{};\n+\trequestBuffers_ = std::queue<FrameBuffer *>{};\n \tinternalBuffers_.clear();\n \tbufferList_.clear();\n }\n \n+int RPiStream::queueToDevice(FrameBuffer *buffer)\n+{\n+\tLOG(RPISTREAM, Debug) << \"Queuing buffer \" << buffer->cookie()\n+\t\t\t << \" for \" << name_;\n+\n+\tint ret = dev_->queueBuffer(buffer);\n+\tif (ret)\n+\t\tLOG(RPISTREAM, Error) << \"Failed to queue buffer for \"\n+\t\t\t\t << name_;\n+\treturn ret;\n+}\n+\n } /* namespace RPi */\n \n } /* namespace libcamera */\ndiff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\nindex 73954ec7..f42e25b9 100644\n--- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n+++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n@@ -57,6 +57,7 @@ public:\n \n private:\n \tvoid clearBuffers();\n+\tint queueToDevice(FrameBuffer *buffer);\n \n \t/*\n \t * Indicates that this stream is active externally, i.e. the buffers\n@@ -73,7 +74,7 @@ private:\n \t/* The actual device stream. */\n \tstd::unique_ptr<V4L2VideoDevice> dev_;\n \n-\t/* All framebuffers associated with this device stream. */\n+\t/* All frame buffers associated with this device stream. */\n \tstd::vector<FrameBuffer *> bufferList_;\n \n \t/*\n@@ -83,6 +84,16 @@ private:\n \t */\n \tstd::queue<FrameBuffer *> availableBuffers_;\n \n+\t/*\n+\t * List of frame buffers that are to be queued into the device from a Request.\n+\t * A nullptr indicates any internal buffer can be used (from availableBuffers_),\n+\t * whereas a valid pointer indicates an external buffer to be queued.\n+\t *\n+\t * Ordering buffers to be queued is important here as it must match the\n+\t * requests coming from the application.\n+\t */\n+\tstd::queue<FrameBuffer *> requestBuffers_;\n+\n \t/*\n \t * This is a list of buffers exported internally. Need to keep this around\n \t * as the stream needs to maintain ownership of these buffers.\n", "prefixes": [ "libcamera-devel", "v5", "09/10" ] }