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>
This commit is contained in:
Laurent Pinchart 2021-09-02 04:29:03 +03:00
parent 9df775c757
commit 32635054bc
10 changed files with 34 additions and 20 deletions

View file

@ -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: "; std::cout << " seq: " << std::setw(6) << std::setfill('0') << metadata.sequence << " bytesused: ";
unsigned int nplane = 0; unsigned int nplane = 0;
for (const FrameMetadata::Plane &plane : metadata.planes) for (const FrameMetadata::Plane &plane : metadata.planes())
{ {
std::cout << plane.bytesused; std::cout << plane.bytesused;
if (++nplane < metadata.planes.size()) std::cout << "/"; if (++nplane < metadata.planes().size()) std::cout << "/";
} }
std::cout << std::endl; std::cout << std::endl;

View file

@ -13,6 +13,7 @@
#include <vector> #include <vector>
#include <libcamera/base/class.h> #include <libcamera/base/class.h>
#include <libcamera/base/span.h>
#include <libcamera/file_descriptor.h> #include <libcamera/file_descriptor.h>
@ -34,7 +35,14 @@ struct FrameMetadata {
Status status; Status status;
unsigned int sequence; unsigned int sequence;
uint64_t timestamp; 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 class FrameBuffer final : public Extensible

View file

@ -374,9 +374,9 @@ void CameraSession::processRequest(Request *request)
<< " bytesused: "; << " bytesused: ";
unsigned int nplane = 0; unsigned int nplane = 0;
for (const FrameMetadata::Plane &plane : metadata.planes) { for (const FrameMetadata::Plane &plane : metadata.planes()) {
info << plane.bytesused; info << plane.bytesused;
if (++nplane < metadata.planes.size()) if (++nplane < metadata.planes().size())
info << "/"; info << "/";
} }
} }

View file

@ -110,7 +110,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer)
for (unsigned int i = 0; i < buffer->planes().size(); ++i) { for (unsigned int i = 0; i < buffer->planes().size(); ++i) {
const FrameBuffer::Plane &plane = buffer->planes()[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]; uint8_t *data = planeData_[&plane];
unsigned int length = std::min(meta.bytesused, plane.length); unsigned int length = std::min(meta.bytesused, plane.length);

View file

@ -91,8 +91,14 @@ LOG_DEFINE_CATEGORY(Buffer)
*/ */
/** /**
* \var FrameMetadata::planes * \fn FrameMetadata::planes()
* \brief Array of per-plane metadata * \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), : Extensible(std::make_unique<Private>()), planes_(planes),
cookie_(cookie) cookie_(cookie)
{ {
metadata_.planes.resize(planes_.size()); metadata_.planes_.resize(planes_.size());
unsigned int offset = 0; unsigned int offset = 0;
bool isContiguous = true; bool isContiguous = true;

View file

@ -1543,7 +1543,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)
unsigned int length = 0; unsigned int length = 0;
for (auto [i, plane] : utils::enumerate(planes)) { for (auto [i, plane] : utils::enumerate(planes)) {
bytesused += metadata.planes[i].bytesused; bytesused += metadata.planes()[i].bytesused;
length += plane.length; length += plane.length;
if (i != planes.size() - 1 && bytesused != 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. * V4L2 buffer is guaranteed to be equal at this point.
*/ */
for (auto [i, plane] : utils::enumerate(planes)) { 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; v4l2Planes[i].length = plane.length;
} }
} else { } else {
@ -1575,7 +1575,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)
* Single-planar API with a single plane in the buffer * Single-planar API with a single plane in the buffer
* is trivial to handle. * is trivial to handle.
*/ */
buf.bytesused = metadata.planes[0].bytesused; buf.bytesused = metadata.planes()[0].bytesused;
buf.length = planes[0].length; buf.length = planes[0].length;
} }
@ -1704,9 +1704,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()
return buffer; return buffer;
} }
metadata.planes[i].bytesused = metadata.planes()[i].bytesused =
std::min(plane.length, bytesused); std::min(plane.length, bytesused);
bytesused -= metadata.planes[i].bytesused; bytesused -= metadata.planes()[i].bytesused;
} }
} else if (multiPlanar) { } else if (multiPlanar) {
/* /*
@ -1715,9 +1715,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()
* V4L2 buffer is guaranteed to be equal at this point. * V4L2 buffer is guaranteed to be equal at this point.
*/ */
for (unsigned int i = 0; i < numV4l2Planes; ++i) for (unsigned int i = 0; i < numV4l2Planes; ++i)
metadata.planes[i].bytesused = planes[i].bytesused; metadata.planes()[i].bytesused = planes[i].bytesused;
} else { } else {
metadata.planes[0].bytesused = buf.bytesused; metadata.planes()[0].bytesused = buf.bytesused;
} }
return buffer; return buffer;

View file

@ -756,7 +756,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
qDebug().noquote() qDebug().noquote()
<< QString("seq: %1").arg(metadata.sequence, 6, 10, QLatin1Char('0')) << QString("seq: %1").arg(metadata.sequence, 6, 10, QLatin1Char('0'))
<< "bytesused:" << metadata.planes[0].bytesused << "bytesused:" << metadata.planes()[0].bytesused
<< "timestamp:" << metadata.timestamp << "timestamp:" << metadata.timestamp
<< "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps; << "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps;

View file

@ -125,7 +125,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer,
/* /*
* \todo Get the stride from the buffer instead of computing it naively * \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(); update();
buffer_ = buffer; buffer_ = buffer;
} }

View file

@ -87,7 +87,7 @@ void ViewFinderQt::render(libcamera::FrameBuffer *buffer,
} }
unsigned char *memory = mem.data(); unsigned char *memory = mem.data();
size_t size = buffer->metadata().planes[0].bytesused; size_t size = buffer->metadata().planes()[0].bytesused;
{ {
QMutexLocker locker(&mutex_); QMutexLocker locker(&mutex_);

View file

@ -211,7 +211,7 @@ void V4L2CameraProxy::updateBuffers()
switch (fmd.status) { switch (fmd.status) {
case FrameMetadata::FrameSuccess: case FrameMetadata::FrameSuccess:
buf.bytesused = fmd.planes[0].bytesused; buf.bytesused = fmd.planes()[0].bytesused;
buf.field = V4L2_FIELD_NONE; buf.field = V4L2_FIELD_NONE;
buf.timestamp.tv_sec = fmd.timestamp / 1000000000; buf.timestamp.tv_sec = fmd.timestamp / 1000000000;
buf.timestamp.tv_usec = fmd.timestamp % 1000000; buf.timestamp.tv_usec = fmd.timestamp % 1000000;