libcamera: framebuffer: Prevent modifying the number of metadata planes

The number of metadata planes should always match the number of frame
buffer planes. Enforce this by making the vector private and providing
accessor functions.

As this changes the public API, update all in-tree users accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>

10 files changed, 34 insertions, 20 deletions

diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst
index 462efe24..84e522c0 100644
--- a/Documentation/guides/application-developer.rst
+++ b/Documentation/guides/application-developer.rst
@@ -427,10 +427,10 @@ the Frame sequence number and details of the planes.
    std::cout << " seq: " << std::setw(6) << std::setfill('0') << metadata.sequence << " bytesused: ";
 
    unsigned int nplane = 0;
-   for (const FrameMetadata::Plane &plane : metadata.planes)
+   for (const FrameMetadata::Plane &plane : metadata.planes())
    {
        std::cout << plane.bytesused;
-       if (++nplane < metadata.planes.size()) std::cout << "/";
+       if (++nplane < metadata.planes().size()) std::cout << "/";
    }
 
    std::cout << std::endl;
diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h
index fd68ed0a..7f2f176a 100644
--- a/include/libcamera/framebuffer.h
+++ b/include/libcamera/framebuffer.h
@@ -13,6 +13,7 @@
 #include <vector>
 
 #include <libcamera/base/class.h>
+#include <libcamera/base/span.h>
 
 #include <libcamera/file_descriptor.h>
 
@@ -34,7 +35,14 @@ struct FrameMetadata {
 	Status status;
 	unsigned int sequence;
 	uint64_t timestamp;
-	std::vector<Plane> planes;
+
+	Span<Plane> planes() { return planes_; }
+	Span<const Plane> planes() const { return planes_; }
+
+private:
+	friend class FrameBuffer;
+
+	std::vector<Plane> planes_;
 };
 
 class FrameBuffer final : public Extensible
diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
index 60d640f2..32a373a9 100644
--- a/src/cam/camera_session.cpp
+++ b/src/cam/camera_session.cpp
@@ -374,9 +374,9 @@ void CameraSession::processRequest(Request *request)
 		     << " bytesused: ";
 
 		unsigned int nplane = 0;
-		for (const FrameMetadata::Plane &plane : metadata.planes) {
+		for (const FrameMetadata::Plane &plane : metadata.planes()) {
 			info << plane.bytesused;
-			if (++nplane < metadata.planes.size())
+			if (++nplane < metadata.planes().size())
 				info << "/";
 		}
 	}
diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp
index 0b529e3e..0fc7d621 100644
--- a/src/cam/file_sink.cpp
+++ b/src/cam/file_sink.cpp
@@ -110,7 +110,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer)
 
 	for (unsigned int i = 0; i < buffer->planes().size(); ++i) {
 		const FrameBuffer::Plane &plane = buffer->planes()[i];
-		const FrameMetadata::Plane &meta = buffer->metadata().planes[i];
+		const FrameMetadata::Plane &meta = buffer->metadata().planes()[i];
 
 		uint8_t *data = planeData_[&plane];
 		unsigned int length = std::min(meta.bytesused, plane.length);
diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
index e4f8419a..d44a98ba 100644
--- a/src/libcamera/framebuffer.cpp
+++ b/src/libcamera/framebuffer.cpp
@@ -91,8 +91,14 @@ LOG_DEFINE_CATEGORY(Buffer)
  */
 
 /**
- * \var FrameMetadata::planes
- * \brief Array of per-plane metadata
+ * \fn FrameMetadata::planes()
+ * \copydoc FrameMetadata::planes() const
+ */
+
+/**
+ * \fn FrameMetadata::planes() const
+ * \brief Retrieve the array of per-plane metadata
+ * \return The array of per-plane metadata
  */
 
 /**
@@ -210,7 +216,7 @@ FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
 	: Extensible(std::make_unique<Private>()), planes_(planes),
 	  cookie_(cookie)
 {
-	metadata_.planes.resize(planes_.size());
+	metadata_.planes_.resize(planes_.size());
 
 	unsigned int offset = 0;
 	bool isContiguous = true;
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index 81209e48..837a59d9 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -1543,7 +1543,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)
 			unsigned int length = 0;
 
 			for (auto [i, plane] : utils::enumerate(planes)) {
-				bytesused += metadata.planes[i].bytesused;
+				bytesused += metadata.planes()[i].bytesused;
 				length += plane.length;
 
 				if (i != planes.size() - 1 && bytesused != length) {
@@ -1567,7 +1567,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)
 			 * V4L2 buffer is guaranteed to be equal at this point.
 			 */
 			for (auto [i, plane] : utils::enumerate(planes)) {
-				v4l2Planes[i].bytesused = metadata.planes[i].bytesused;
+				v4l2Planes[i].bytesused = metadata.planes()[i].bytesused;
 				v4l2Planes[i].length = plane.length;
 			}
 		} else {
@@ -1575,7 +1575,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)
 			 * Single-planar API with a single plane in the buffer
 			 * is trivial to handle.
 			 */
-			buf.bytesused = metadata.planes[0].bytesused;
+			buf.bytesused = metadata.planes()[0].bytesused;
 			buf.length = planes[0].length;
 		}
 
@@ -1704,9 +1704,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()
 				return buffer;
 			}
 
-			metadata.planes[i].bytesused =
+			metadata.planes()[i].bytesused =
 				std::min(plane.length, bytesused);
-			bytesused -= metadata.planes[i].bytesused;
+			bytesused -= metadata.planes()[i].bytesused;
 		}
 	} else if (multiPlanar) {
 		/*
@@ -1715,9 +1715,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()
 		 * V4L2 buffer is guaranteed to be equal at this point.
 		 */
 		for (unsigned int i = 0; i < numV4l2Planes; ++i)
-			metadata.planes[i].bytesused = planes[i].bytesused;
+			metadata.planes()[i].bytesused = planes[i].bytesused;
 	} else {
-		metadata.planes[0].bytesused = buf.bytesused;
+		metadata.planes()[0].bytesused = buf.bytesused;
 	}
 
 	return buffer;
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index 1536b2b5..ac853e36 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -756,7 +756,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
 
 	qDebug().noquote()
 		<< QString("seq: %1").arg(metadata.sequence, 6, 10, QLatin1Char('0'))
-		<< "bytesused:" << metadata.planes[0].bytesused
+		<< "bytesused:" << metadata.planes()[0].bytesused
 		<< "timestamp:" << metadata.timestamp
 		<< "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps;
 
diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
index 40226601..d2ef0369 100644
--- a/src/qcam/viewfinder_gl.cpp
+++ b/src/qcam/viewfinder_gl.cpp
@@ -125,7 +125,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer,
 	/*
 	 * \todo Get the stride from the buffer instead of computing it naively
 	 */
-	stride_ = buffer->metadata().planes[0].bytesused / size_.height();
+	stride_ = buffer->metadata().planes()[0].bytesused / size_.height();
 	update();
 	buffer_ = buffer;
 }
diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp
index efa1d412..a0bf99b0 100644
--- a/src/qcam/viewfinder_qt.cpp
+++ b/src/qcam/viewfinder_qt.cpp
@@ -87,7 +87,7 @@ void ViewFinderQt::render(libcamera::FrameBuffer *buffer,
 	}
 
 	unsigned char *memory = mem.data();
-	size_t size = buffer->metadata().planes[0].bytesused;
+	size_t size = buffer->metadata().planes()[0].bytesused;
 
 	{
 		QMutexLocker locker(&mutex_);
diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
index d926a7b7..07d2250b 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -211,7 +211,7 @@ void V4L2CameraProxy::updateBuffers()
 
 		switch (fmd.status) {
 		case FrameMetadata::FrameSuccess:
-			buf.bytesused = fmd.planes[0].bytesused;
+			buf.bytesused = fmd.planes()[0].bytesused;
 			buf.field = V4L2_FIELD_NONE;
 			buf.timestamp.tv_sec = fmd.timestamp / 1000000000;
 			buf.timestamp.tv_usec = fmd.timestamp % 1000000;