{"id":13624,"url":"https://patchwork.libcamera.org/api/1.1/patches/13624/?format=json","web_url":"https://patchwork.libcamera.org/patch/13624/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210906020100.14430-18-laurent.pinchart@ideasonboard.com>","date":"2021-09-06T02:00:50","name":"[libcamera-devel,v2,17/27] libcamera: framebuffer: Prevent modifying the number of metadata planes","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"67c5c799a46fae151a3acddc4a304928a17b8a42","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/13624/mbox/","series":[{"id":2446,"url":"https://patchwork.libcamera.org/api/1.1/series/2446/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2446","date":"2021-09-06T02:00:33","name":"libcamera: Handle fallout of FrameBuffer offset support","version":2,"mbox":"https://patchwork.libcamera.org/series/2446/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/13624/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/13624/checks/","tags":{},"headers":{"Return-Path":"","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 A6B89C324C\n\tfor ;\n\tMon, 6 Sep 2021 02:01:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 04C976916C;\n\tMon, 6 Sep 2021 04:01:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B24056916F\n\tfor ;\n\tMon, 6 Sep 2021 04:01:27 +0200 (CEST)","from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 403458D7;\n\tMon, 6 Sep 2021 04:01:27 +0200 (CEST)"],"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=\"aENYqNOE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630893687;\n\tbh=3qDqvTFoKWq0JsFPCuhidIF49wrJmvmJs7RUtDS73gU=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=aENYqNOEJl2n2H+Mwn4vHCnQQkjZjmrWSGnHX+M8NNRrRuJK+s16o/dSDePLrGJWV\n\tYZ+7cO90RIDvRMCrbyRkQGg4eTxQ76lnQqIUhYTFHyPv8AtwY6FdYNbrkbrdAmudDo\n\t/bDvvmfS+wiNzw3tB6mapz0NPj4FPY6S6jiq27FM=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 6 Sep 2021 05:00:50 +0300","Message-Id":"<20210906020100.14430-18-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.32.0","In-Reply-To":"<20210906020100.14430-1-laurent.pinchart@ideasonboard.com>","References":"<20210906020100.14430-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v2 17/27] libcamera: framebuffer: Prevent\n\tmodifying the number of metadata planes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The number of metadata planes should always match the number of frame\nbuffer planes. Enforce this by making the vector private and providing\naccessor functions.\n\nAs this changes the public API, update all in-tree users accordingly.\n\nSigned-off-by: Laurent Pinchart \nReviewed-by: Kieran Bingham \n---\n include/libcamera/framebuffer.h | 10 +++++++++-\n src/cam/camera_session.cpp | 4 ++--\n src/cam/file_sink.cpp | 2 +-\n src/libcamera/framebuffer.cpp | 12 +++++++++---\n src/libcamera/v4l2_videodevice.cpp | 14 +++++++-------\n src/qcam/main_window.cpp | 2 +-\n src/qcam/viewfinder_gl.cpp | 2 +-\n src/qcam/viewfinder_qt.cpp | 2 +-\n src/v4l2/v4l2_camera_proxy.cpp | 2 +-\n 9 files changed, 32 insertions(+), 18 deletions(-)","diff":"diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h\nindex fd68ed0a139d..7f2f176af691 100644\n--- a/include/libcamera/framebuffer.h\n+++ b/include/libcamera/framebuffer.h\n@@ -13,6 +13,7 @@\n #include \n \n #include \n+#include \n \n #include \n \n@@ -34,7 +35,14 @@ struct FrameMetadata {\n \tStatus status;\n \tunsigned int sequence;\n \tuint64_t timestamp;\n-\tstd::vector planes;\n+\n+\tSpan planes() { return planes_; }\n+\tSpan planes() const { return planes_; }\n+\n+private:\n+\tfriend class FrameBuffer;\n+\n+\tstd::vector planes_;\n };\n \n class FrameBuffer final : public Extensible\ndiff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp\nindex 60d640f2b15c..32a373a99b72 100644\n--- a/src/cam/camera_session.cpp\n+++ b/src/cam/camera_session.cpp\n@@ -374,9 +374,9 @@ void CameraSession::processRequest(Request *request)\n \t\t << \" bytesused: \";\n \n \t\tunsigned int nplane = 0;\n-\t\tfor (const FrameMetadata::Plane &plane : metadata.planes) {\n+\t\tfor (const FrameMetadata::Plane &plane : metadata.planes()) {\n \t\t\tinfo << plane.bytesused;\n-\t\t\tif (++nplane < metadata.planes.size())\n+\t\t\tif (++nplane < metadata.planes().size())\n \t\t\t\tinfo << \"/\";\n \t\t}\n \t}\ndiff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp\nindex 0b529e3eb767..0fc7d621f50b 100644\n--- a/src/cam/file_sink.cpp\n+++ b/src/cam/file_sink.cpp\n@@ -110,7 +110,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer)\n \n \tfor (unsigned int i = 0; i < buffer->planes().size(); ++i) {\n \t\tconst FrameBuffer::Plane &plane = buffer->planes()[i];\n-\t\tconst FrameMetadata::Plane &meta = buffer->metadata().planes[i];\n+\t\tconst FrameMetadata::Plane &meta = buffer->metadata().planes()[i];\n \n \t\tuint8_t *data = planeData_[&plane];\n \t\tunsigned int length = std::min(meta.bytesused, plane.length);\ndiff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp\nindex e4f8419a9063..d44a98babd05 100644\n--- a/src/libcamera/framebuffer.cpp\n+++ b/src/libcamera/framebuffer.cpp\n@@ -91,8 +91,14 @@ LOG_DEFINE_CATEGORY(Buffer)\n */\n \n /**\n- * \\var FrameMetadata::planes\n- * \\brief Array of per-plane metadata\n+ * \\fn FrameMetadata::planes()\n+ * \\copydoc FrameMetadata::planes() const\n+ */\n+\n+/**\n+ * \\fn FrameMetadata::planes() const\n+ * \\brief Retrieve the array of per-plane metadata\n+ * \\return The array of per-plane metadata\n */\n \n /**\n@@ -210,7 +216,7 @@ FrameBuffer::FrameBuffer(const std::vector &planes, unsigned int cookie)\n \t: Extensible(std::make_unique()), planes_(planes),\n \t cookie_(cookie)\n {\n-\tmetadata_.planes.resize(planes_.size());\n+\tmetadata_.planes_.resize(planes_.size());\n \n \tunsigned int offset = 0;\n \tbool isContiguous = true;\ndiff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\nindex e729e608448c..296304fbd979 100644\n--- a/src/libcamera/v4l2_videodevice.cpp\n+++ b/src/libcamera/v4l2_videodevice.cpp\n@@ -1538,7 +1538,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n \t\t\tunsigned int length = 0;\n \n \t\t\tfor (auto [i, plane] : utils::enumerate(planes)) {\n-\t\t\t\tbytesused += metadata.planes[i].bytesused;\n+\t\t\t\tbytesused += metadata.planes()[i].bytesused;\n \t\t\t\tlength += plane.length;\n \n \t\t\t\tif (i != planes.size() - 1 && bytesused != length) {\n@@ -1562,7 +1562,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n \t\t\t * V4L2 buffer is guaranteed to be equal at this point.\n \t\t\t */\n \t\t\tfor (auto [i, plane] : utils::enumerate(planes)) {\n-\t\t\t\tv4l2Planes[i].bytesused = metadata.planes[i].bytesused;\n+\t\t\t\tv4l2Planes[i].bytesused = metadata.planes()[i].bytesused;\n \t\t\t\tv4l2Planes[i].length = plane.length;\n \t\t\t}\n \t\t} else {\n@@ -1570,7 +1570,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n \t\t\t * Single-planar API with a single plane in the buffer\n \t\t\t * is trivial to handle.\n \t\t\t */\n-\t\t\tbuf.bytesused = metadata.planes[0].bytesused;\n+\t\t\tbuf.bytesused = metadata.planes()[0].bytesused;\n \t\t\tbuf.length = planes[0].length;\n \t\t}\n \n@@ -1699,9 +1699,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n \t\t\t\treturn buffer;\n \t\t\t}\n \n-\t\t\tmetadata.planes[i].bytesused =\n+\t\t\tmetadata.planes()[i].bytesused =\n \t\t\t\tstd::min(plane.length, bytesused);\n-\t\t\tbytesused -= metadata.planes[i].bytesused;\n+\t\t\tbytesused -= metadata.planes()[i].bytesused;\n \t\t}\n \t} else if (multiPlanar) {\n \t\t/*\n@@ -1710,9 +1710,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n \t\t * V4L2 buffer is guaranteed to be equal at this point.\n \t\t */\n \t\tfor (unsigned int i = 0; i < numV4l2Planes; ++i)\n-\t\t\tmetadata.planes[i].bytesused = planes[i].bytesused;\n+\t\t\tmetadata.planes()[i].bytesused = planes[i].bytesused;\n \t} else {\n-\t\tmetadata.planes[0].bytesused = buf.bytesused;\n+\t\tmetadata.planes()[0].bytesused = buf.bytesused;\n \t}\n \n \treturn buffer;\ndiff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\nindex 1536b2b5bd66..ac853e360aea 100644\n--- a/src/qcam/main_window.cpp\n+++ b/src/qcam/main_window.cpp\n@@ -756,7 +756,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)\n \n \tqDebug().noquote()\n \t\t<< QString(\"seq: %1\").arg(metadata.sequence, 6, 10, QLatin1Char('0'))\n-\t\t<< \"bytesused:\" << metadata.planes[0].bytesused\n+\t\t<< \"bytesused:\" << metadata.planes()[0].bytesused\n \t\t<< \"timestamp:\" << metadata.timestamp\n \t\t<< \"fps:\" << Qt::fixed << qSetRealNumberPrecision(2) << fps;\n \ndiff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\nindex 40226601f9fd..d2ef036974f4 100644\n--- a/src/qcam/viewfinder_gl.cpp\n+++ b/src/qcam/viewfinder_gl.cpp\n@@ -125,7 +125,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer,\n \t/*\n \t * \\todo Get the stride from the buffer instead of computing it naively\n \t */\n-\tstride_ = buffer->metadata().planes[0].bytesused / size_.height();\n+\tstride_ = buffer->metadata().planes()[0].bytesused / size_.height();\n \tupdate();\n \tbuffer_ = buffer;\n }\ndiff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp\nindex efa1d412584b..a0bf99b0b522 100644\n--- a/src/qcam/viewfinder_qt.cpp\n+++ b/src/qcam/viewfinder_qt.cpp\n@@ -87,7 +87,7 @@ void ViewFinderQt::render(libcamera::FrameBuffer *buffer,\n \t}\n \n \tunsigned char *memory = mem.data();\n-\tsize_t size = buffer->metadata().planes[0].bytesused;\n+\tsize_t size = buffer->metadata().planes()[0].bytesused;\n \n \t{\n \t\tQMutexLocker locker(&mutex_);\ndiff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\nindex d926a7b77083..07d2250bb846 100644\n--- a/src/v4l2/v4l2_camera_proxy.cpp\n+++ b/src/v4l2/v4l2_camera_proxy.cpp\n@@ -211,7 +211,7 @@ void V4L2CameraProxy::updateBuffers()\n \n \t\tswitch (fmd.status) {\n \t\tcase FrameMetadata::FrameSuccess:\n-\t\t\tbuf.bytesused = fmd.planes[0].bytesused;\n+\t\t\tbuf.bytesused = fmd.planes()[0].bytesused;\n \t\t\tbuf.field = V4L2_FIELD_NONE;\n \t\t\tbuf.timestamp.tv_sec = fmd.timestamp / 1000000000;\n \t\t\tbuf.timestamp.tv_usec = fmd.timestamp % 1000000;\n","prefixes":["libcamera-devel","v2","17/27"]}