Patch Detail
Show a patch.
GET /api/patches/3285/?format=api
{ "id": 3285, "url": "https://patchwork.libcamera.org/api/patches/3285/?format=api", "web_url": "https://patchwork.libcamera.org/patch/3285/", "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": "<20200323173559.21109-20-laurent.pinchart@ideasonboard.com>", "date": "2020-03-23T17:35:57", "name": "[libcamera-devel,v2,19/21] qcam: viewfinder: Avoid memory copy when conversion isn't needed", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "0b4b3ff202147f60a0d841c05f8b34bc62922a1f", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/3285/mbox/", "series": [ { "id": 762, "url": "https://patchwork.libcamera.org/api/series/762/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=762", "date": "2020-03-23T17:35:38", "name": "qcam: Bypass format conversion when not required", "version": 2, "mbox": "https://patchwork.libcamera.org/series/762/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/3285/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/3285/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D408F62CD4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Mar 2020 18:36:22 +0100 (CET)", "from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 75C76308\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Mar 2020 18:36:22 +0100 (CET)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584984982;\n\tbh=XBJTDLeBpsITfQTnntj/V4e5CGymnErEuB9yFtPeiFc=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=pPE93g7DNlJBOCzVKz0OHRSepm/q1LkF+VPnLcAxEqJ4YRdY5gc9xlZU2AUuDRM6i\n\tyjSw//Vg0xRlDdiWAGZS+043IdpUye8rXWw7kpgm8M6kqsbZq8EeNk501taxjm41qY\n\tfFkQ28MEQPn0suHtgeacX/IUbmCkYP+w2yoQqNno=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 23 Mar 2020 19:35:57 +0200", "Message-Id": "<20200323173559.21109-20-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.24.1", "In-Reply-To": "<20200323173559.21109-1-laurent.pinchart@ideasonboard.com>", "References": "<20200323173559.21109-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 19/21] qcam: viewfinder: Avoid memory\n\tcopy when conversion isn't needed", "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>", "X-List-Received-Date": "Mon, 23 Mar 2020 17:36:26 -0000" }, "content": "If the frame buffer format is identical to the display format, the\nviewfinder still invokes the converter to perform what is essentially a\nslow memcpy(). Make it possible to skip that operation by creating a\nQImage referencing the buffer memory instead. A reference to the frame\nbuffer is kept internally, and released when the next buffer is queued,\npushing the current one out.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n---\nChanges since v1:\n\n- Add stop() function to release the buffer\n---\n src/qcam/main_window.cpp | 2 ++\n src/qcam/viewfinder.cpp | 74 +++++++++++++++++++++++++++++++---------\n src/qcam/viewfinder.h | 2 ++\n 3 files changed, 61 insertions(+), 17 deletions(-)", "diff": "diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\nindex e872bdd7ba86..b13660bccb25 100644\n--- a/src/qcam/main_window.cpp\n+++ b/src/qcam/main_window.cpp\n@@ -416,6 +416,8 @@ void MainWindow::stopCapture()\n \tif (!isCapturing_)\n \t\treturn;\n \n+\tviewfinder_->stop();\n+\n \tint ret = camera_->stop();\n \tif (ret)\n \t\tqInfo() << \"Failed to stop capture\";\ndiff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp\nindex 31b358da47dc..3f984efbf434 100644\n--- a/src/qcam/viewfinder.cpp\n+++ b/src/qcam/viewfinder.cpp\n@@ -7,6 +7,8 @@\n \n #include \"viewfinder.h\"\n \n+#include <utility>\n+\n #include <QImage>\n #include <QImageWriter>\n #include <QMutexLocker>\n@@ -16,7 +18,7 @@\n #include \"format_converter.h\"\n \n ViewFinder::ViewFinder(QWidget *parent)\n-\t: QWidget(parent)\n+\t: QWidget(parent), buffer_(nullptr)\n {\n }\n \n@@ -27,17 +29,23 @@ ViewFinder::~ViewFinder()\n int ViewFinder::setFormat(const libcamera::PixelFormat &format,\n \t\t\t const QSize &size)\n {\n-\tint ret;\n+\timage_ = QImage();\n \n-\tret = converter_.configure(format, size);\n-\tif (ret < 0)\n-\t\treturn ret;\n+\t/*\n+\t * If format conversion is needed, configure the converter and allocate\n+\t * the destination image.\n+\t */\n+\tif (format != DRM_FORMAT_ARGB8888) {\n+\t\tint ret = converter_.configure(format, size);\n+\t\tif (ret < 0)\n+\t\t\treturn ret;\n+\n+\t\timage_ = QImage(size, QImage::Format_RGB32);\n+\t}\n \n \tformat_ = format;\n \tsize_ = size;\n \n-\timage_ = QImage(size_, QImage::Format_RGB32);\n-\n \tupdateGeometry();\n \treturn 0;\n }\n@@ -49,19 +57,51 @@ void ViewFinder::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n \t\treturn;\n \t}\n \n-\tQMutexLocker locker(&mutex_);\n-\n-\t/*\n-\t * \\todo We're not supposed to block the pipeline handler thread\n-\t * for long, implement a better way to save images without\n-\t * impacting performances.\n-\t */\n+\tunsigned char *memory = static_cast<unsigned char *>(map->memory);\n+\tsize_t size = buffer->metadata().planes[0].bytesused;\n+\n+\t{\n+\t\tQMutexLocker locker(&mutex_);\n+\n+\t\tif (format_ == DRM_FORMAT_ARGB8888) {\n+\t\t\t/*\n+\t\t\t * If the frame format is identical to the display\n+\t\t\t * format, create a QImage that references the frame\n+\t\t\t * and store a reference to the frame buffer. The\n+\t\t\t * previously stored frame buffer, if any, will be\n+\t\t\t * released.\n+\t\t\t *\n+\t\t\t * \\todo Get the stride from the buffer instead of\n+\t\t\t * computing it naively\n+\t\t\t */\n+\t\t\timage_ = QImage(memory, size_.width(), size_.height(),\n+\t\t\t\t\tsize / size_.height(), QImage::Format_RGB32);\n+\t\t\tstd::swap(buffer, buffer_);\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * Otherwise, convert the format and release the frame\n+\t\t\t * buffer immediately.\n+\t\t\t */\n+\t\t\tconverter_.convert(memory, size, &image_);\n+\t\t}\n+\t}\n \n-\tconverter_.convert(static_cast<unsigned char *>(map->memory),\n-\t\t\t buffer->metadata().planes[0].bytesused, &image_);\n \tupdate();\n \n-\trenderComplete(buffer);\n+\tif (buffer)\n+\t\trenderComplete(buffer);\n+}\n+\n+void ViewFinder::stop()\n+{\n+\timage_ = QImage();\n+\n+\tif (buffer_) {\n+\t\trenderComplete(buffer_);\n+\t\tbuffer_ = nullptr;\n+\t}\n+\n+\tupdate();\n }\n \n QImage ViewFinder::getCurrentImage()\ndiff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h\nindex 54c0fa9dd7a0..4d0622a8e4ea 100644\n--- a/src/qcam/viewfinder.h\n+++ b/src/qcam/viewfinder.h\n@@ -36,6 +36,7 @@ public:\n \n \tint setFormat(const libcamera::PixelFormat &format, const QSize &size);\n \tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map);\n+\tvoid stop();\n \n \tQImage getCurrentImage();\n \n@@ -52,6 +53,7 @@ private:\n \tlibcamera::PixelFormat format_;\n \tQSize size_;\n \n+\tlibcamera::FrameBuffer *buffer_;\n \tQImage image_;\n \tQMutex mutex_; /* Prevent concurrent access to image_ */\n };\n", "prefixes": [ "libcamera-devel", "v2", "19/21" ] }