[libcamera-devel] [PATCH v3 17/30] libcamera: framebuffer: Prevent modifying the number of metadata planes
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Sep 7 00:56:23 CEST 2021
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 at ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
---
include/libcamera/framebuffer.h | 10 +++++++++-
src/cam/camera_session.cpp | 4 ++--
src/cam/file_sink.cpp | 2 +-
src/libcamera/framebuffer.cpp | 12 +++++++++---
src/libcamera/v4l2_videodevice.cpp | 14 +++++++-------
src/qcam/main_window.cpp | 2 +-
src/qcam/viewfinder_gl.cpp | 2 +-
src/qcam/viewfinder_qt.cpp | 2 +-
src/v4l2/v4l2_camera_proxy.cpp | 2 +-
9 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h
index fd68ed0a139d..7f2f176af691 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 60d640f2b15c..32a373a99b72 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 0b529e3eb767..0fc7d621f50b 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 e4f8419a9063..d44a98babd05 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 81209e485c24..837a59d9bae2 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 1536b2b5bd66..ac853e360aea 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 40226601f9fd..d2ef036974f4 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 efa1d412584b..a0bf99b0b522 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 d926a7b77083..07d2250bb846 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;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list